Annex-Engineering/klipper_estimator

FR: scaled_xy_accel support for limited_cartesian

Closed this issue · 5 comments

richfelker commented

The axis limiter is for an out-of-tree Klipper feature, limited_cartesian kinematics with per-axis limits. However the good version of this feature has the capability to treat the per-axis acceleration limits as relative to the current M204 acceleration rather than global limits. It would be nice if klipper_estimator supported this convention too.

dalegaard commented

Hi @richfelker

I'm not aware of how this works?

Best regards,
Lasse

richfelker commented

See https://github.com/Piezoid/klipper/blob/work-peraxis/klippy/kinematics/limited_cartesian.py

The idea is that if one is using acceleration limits to control toolpath deviation/ringing/etc., the per-axis limits reflect differences in mass or stiffness of the axes. For example, if Y deviates ~4x as badly as X for a given acceleration, it's desirable to have Y acceleration 25% of X acceleration, but setting a low absolute limit on Y acceleration would cost a lot in print time for no benefit when you really only care about the deviation on outer walls. Having the Y accel limit scale as a % of M204 acceleration rather than being an absolute limit handles this in a continuous way and is also likely to represent the scaling of acceleration before steps are missed at very high M204 accel, too.

dalegaard commented

Hi @richfelker

Okay I think I understand. This shouldn't be too difficult to do, I'll take a look. I'm thinking of adding a scaled version of the current axis limiter to cover this use case.

I'll post a PR you can test out when I have it done.

Best regards,
Lasse

dalegaard commented

Hey @richfelker

I've taken a stab at this, please check the https://github.com/Annex-Engineering/klipper_estimator/tree/limited_kinematics branch.

I currently have no working printer, so testing this is a bit annoying. Hopefully you and Piezo can test it out for me :-)

Best regards,
Lasse

Piezoid commented

Note that the SCV fix (76ba4bee) is not implemented, as there is no way to detect if users have it from the config.

Possible solutions:

  • Add a config option to toggle the fix. It must be disabled by default to assure that it is enabled if and only if the config say so. Also supporting both modes will increase the code complexity.
  • Assume that every user of limited_ kinematics have the SCV fix, update install instructions to reflect that. But then, I won't be able to support beacon/z_calibration style install if the changes to klippy code are no longer optional.
  • Upstream it, since it shouldn't change the behavior outside of the use of limited_ kinematics. It won't be easy to find good arguments for a change intended to support external, unsuported code.