Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

L3 fixes #237

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

L3 fixes #237

wants to merge 1 commit into from

Conversation

osresearch
Copy link

This patch fixes #235 and #236 to correctly validate L3 commands with non-zero jerk2 and also flips the gJerk1 and gJerk2 globals if the sign on gSteps1 or gSteps2 is negative in legacy mode.

This patch fixes evil-mad#235 and evil-mad#236 to correctly validate L3 commands
with non-zero jerk2 and also flips the gJerk1 and gJerk2 globals
if the sign on gSteps1 or gSteps2 is negative in legacy mode.
@oskay oskay requested a review from EmbeddedMan November 25, 2024 15:59
Copy link
Contributor

@EmbeddedMan EmbeddedMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from gJerk1 to gJerk2 is clearly a type (sorry!) and I'm sure that's correct.

The other two changes feel right, but I don't have a deep enough understanding of the mathematics to know that they are correct. @oskay do you think negating the Jerks is correct?

@osresearch
Copy link
Author

I'm also not positive that the inversion of vel/accel/jerk if steps is negative is right in any case, but it is the documented behaviour for LM so it is odd that L3 doesn't do it for jerk as well.

Normally the sign of the Rate parameters are used to indicate initial motor direction. When Rate signs are used for direction, then Steps parameters should be positive. However, if the sign of Rate is positive, then the sign of the Steps parameters can be used to control initial motor direction instead. Internally, if an axis has a negative Steps and a positive Rate, the EBB code will flip the sign on Rate and Steps as well as Accel to make the math work out.

@EmbeddedMan
Copy link
Contributor

I agree - it would feel really weird that this same step should only be in one of those two commands. If it's in LM it feels like it should be in L3. But @oskay is the real mind behind all of the math here, so I trust his judgment on if this change should be made or not.

@EmbeddedMan
Copy link
Contributor

@oskay Would you like me to do a test build with these changes so you can test it out before merging? Or should I just merge it into master and then do a new official release? Realistically I'm unlikely to have time at this point to create specific tests to exercise these changes, so if you want to test it first you'll need to come up with some test cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sign flipping in L3 mode
2 participants