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

FR: scaled_xy_accel support for limited_cartesian #24

Open
richfelker opened this issue Aug 15, 2022 · 5 comments
Open

FR: scaled_xy_accel support for limited_cartesian #24

richfelker opened this issue Aug 15, 2022 · 5 comments

Comments

@richfelker
Copy link

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
Copy link
Collaborator

Hi @richfelker

I'm not aware of how this works?

Best regards,
Lasse

@richfelker
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link

Piezoid commented Jul 2, 2023

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.

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

No branches or pull requests

3 participants