S-curve jerk control fixes as requested in #3744 #3746
Conversation
grandixximo
changed the title
Add infrastructure to output precise jerk values from the trajectory planner to joint commands during S-curve (planner_type=1) coordinated motion. Previously, joint jerk values were derived from cubic interpolation, which introduces numerical delays and inaccuracies. This change passes the exact path jerk and direction vector from the TP through emcmotStatus, allowing control.c to compute per-joint jerk as: joint_jerk = path_jerk * direction. Changes: - motion.h: Add current_acc, current_jerk, current_dir fields to emcmot_status_t - tc.c/tc.h: Add tcGetCurrentTangentUnitVector() for direction at current progress - tp.c: Output S-curve motion state in tpUpdateMovementStatus() - control.c: Override joint jerk values for XYZ joints in S-curve mode This improves jerk accuracy for Cartesian machines using S-curve planning, which is important for servo tuning and motion smoothness analysis. ABC-UVW to come in future
|
last commit fixes the jerk-cmd output for XYZ |
|
The current master and your master branch from which you request to be pulled from show following warnings triggered by cppcheck (executed within src as ../scripts/cppcheck.sh) refering to ini_maxjerk not being initialized. No idea if this is related to your work, but anyway you may have an immediate idea how to best address this. |
|
warnings should be fixed, yes were due to my changes, forgot to add in NML |
…performance that will hit the limit sensors on homing
|
This is to improve default behavior when max_jerk is not set and planner type is set to 1 |
|
I don;t think it fails cause of me |
|
I think definitely something with this 24.04 arm dependencies, not something I did @andypugh is it something you can take a look at? |
|
it seems there are tests in the testsuite that expect but in fact happens.
|
|
I see, actually 0 is not very good as default, because the users like PCW don't set a value then planner_type 1 cannot function proper, so I gave 10k which is what I thought was a sweet spot, or we could set 1e99 ? but YangYang said that may also cause issues, we never tested with such high values Can I change the tests then? |
Yes the tests should be changed in lockstep with the code. You can run the tests locally before committing, that usually is way faster than waiting for the CI. |
|
Tests are fixed, not sure why doesn't build on arm |
That looks like a temporary hickup in debian sid, should go away by itself. Having sid in the CI always is a bit of a gamble. |
|
All green, that's what I like to see 🤭 |
I think I commented elsewhere. People are used to tune max acceleration on their machines, and IMO recommended default max jerk limit should not be too low so max accel can be reached between a few to maybe 100 servo periods. After all machine is supposed to run more or less fine with that accel limit and trapezoidal speed. Testing with my experimantal 5th order bezier speed profile let me set max acceleration much higher (like ×2) on a stepper machine that would stall out with the trapezoidal speed profile at that acceleration. I didn't try yet on real hardware because machine is in use currently, but I think even high jerk limits like 5 servo periods will make a noticeable difference, especially when doing something like jog around or milling an adaptive toolpath with small feeds and rapids intertwined. Going to 1e99 should result in more or less trapezoidal speed profile without any negative side effects, if it doesn't, that needs to be fixed IMO. |
|
1e99 will break the calculations nothing will move. double T1 = amax / jerk; // Nearly zero when jerk = 1e+99 distance / 1e+99 becomes extremely small almost zero 1e9 is more reasonable, and will get something like trapez, I will set it as default, and cap it as max. |
If 0 is no good max jerk, and the NML initialisation possibly being used from what cppcheck states, should it then also be initialised to a different value than 0 in 34578c3? |
|
@smoe Thanks you for bringing this to my attention, I think we should keep the NML initialization at 0.0 intentionally, it's really just a safety fallback since all NML messages get populated from system state before they're used (see emccanon.cc). Having 0.0 as the struct default gives us a nice property: if something goes wrong and a message doesn't get populated, it safely falls back to trapezoidal mode rather than silently using S-curve. It's a clear "uninitialized" sentinel value. The real user-facing default of 1e9 is set where it should be, at the system level in initraj.cc. I think keeping them different makes sense here, but let me know if you feel strongly about it, I can definitely initialize at 1e9 if you'd prefer to see that. All cppchecks pass, if I'm thinking about this wrong let me know. |
|
could this be merged in the weekend? would love to have it in master, YangYang wants to rebase on new master after the merge, to make sure everything lines up before continue development, can that happen or will this be on hold for longer? Don't want to be pushy, just trying to figure out how to proceed on our end. |
3 participants
Following discussion with Sigma1912 in #3744
Added ini warning and changed pin names