Add support for pressure advance on extruder steppers

[theophile]

This PR is intended to address the issue discussed here: #3038
It should cause an extruder's pressure advance settings to be applied to all extruder steppers synced with that extruder. It works as expected in my setup that has a single extruder stepper that is synced to only one of six extruders at a given time. I haven't been able to test with multiple extruder steppers synced to a single extruder, but it should work the same even in that case.

This also enables the use of the "SET_STEPPER_ENABLE" extended gcode command on extruder steppers. For my application, this is useful for disabling the extruder stepper to avoid needless wear on the BMG gears during parts of filament load/unload operations when the extruder stepper is "empty."

[KevinOConnor]

Thanks and sorry for the delay in responding.

My main high-level feedback is that I'm not sure if we should set pressure advance for an "extruder" (ie, hotend) or for each stepper motor. I'd be inclined to say that it should apply to the stepper motor and not the hotend though. There was some related discussion on this at #3920.

Separately, the stepper_enable change does not look correct to me. Unless I'm missing something, the stepper.PrinterStepper() call should have already added the given stepper motor to stepper_enable.

-Kevin

[theophile]

Thanks @KevinOConnor. I'll double check the stepper_enable code. Regarding the main issue, as I appreciate the discussion at #3920, I think a mixing hotend needs multiple stepper motors that can move independently from one another, so as to be able to control the rate at which multiple filaments are melted and then pushed out of the nozzle. I don't think the extruder_stepper module currently allows for that.

If the extruder_stepper module were to be reworked so as to allow for an extruder_stepper to be controlled independently and not simply "mirror" the moves of another extruder, then I think that should include the ability to set PA independently on the extruder_stepper as well. But the current behavior of the extruder_stepper module is that the extruder_stepper essentially just mirrors the moves of the synced extruder, with two exceptions (that I've observed anyway):

1. The step_distance (and I assume the new rotation_distance) of each stepper motor is applied independently; and
2. The extruder_stepper ignores PA moves regardless of how the synced extruder is configured.

The first distinction is good and helpful because it allows multiple stepper motors to be dialed in independently to ensure that they each push out precisely the same amount of filament on the same extrusion move.

The second distinction is, in my view, not expected or intuitive. I can't currently think of a use case where a user would want to have an extruder_stepper synced with an extruder, but not perform any of the PA moves that the main extruder performs. It seems most intuitive to me that when an extruder_stepper is synced with an extruder, it should mirror all the moves of the synced extruder, including PA moves, and not just the main extrusion moves. That's what this PR is trying to do.

[theophile]

@KevinOConnor, well it took me longer than I expected to get back to this, but you were right that the stepper_enable additions were unnecessary as the desired functionality is already present. I removed those lines and have updated the PR accordingly. I also merged upstream to make sure the other changes still merge with the master branch.

[KevinOConnor]

Thanks. My high-level feedback is that I think we need to implement some architectural changes to Klipper's treatment of extruder stepper motors before we can add enhancements in this area. Currently the klipper extruder config section defines the parameters to a hotend, a heater, a temperature sensor, and an extruder stepper motor. I think we need a more clear abstraction so that it is possible to better control "extruder stepper motors". If we can better control extruder stepper motors, it will make it easier to set the pressure advance parameters for each extruder stepper - which I think may be necessary in some cases.

FYI, this also impacts #4566 and #4489 .

-Kevin

[KevinOConnor]

Thanks. I'm not sure what the state of this PR is. If there is still interest in this topic then the PR will need to be updated and a reviewer will need to volunteer to review it ( https://www.klipper3d.org/CONTRIBUTING.html ). Unfortunately, due to time constraints on my side it is unlikely I will be reviewing this PR.

A good place to discuss this PR, get testers, and engage a reviewer is on the Klipper Discourse server ( https://www.klipper3d.org/Contact.html ).

-Kevin

[KevinOConnor]

FYI, PR #5143 adds support for calling SET_PRESSURE_ADVANCE on extruder_stepper objects.

-Kevin

[github-actions]

Unfortunately a reviewer has not assigned themselves to this GitHub Pull Request and it is therefore being closed. It is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.