Skip to content

Add sync_extruder_steppers#4489

Closed
Tircown wants to merge 3 commits into
Klipper3d:masterfrom
Tircown:work-sync-extruder-steppers
Closed

Add sync_extruder_steppers#4489
Tircown wants to merge 3 commits into
Klipper3d:masterfrom
Tircown:work-sync-extruder-steppers

Conversation

@Tircown

@Tircown Tircown commented Jul 14, 2021

Copy link
Copy Markdown
Contributor

Add the possibility to synchronize multiple extruder steppers.
Typical usage is to mimic extruder with extruder1 on an IDEX printer printing in duplication or mirrored mode.
Related to PR #4464, replace #4445.

Signed-off-by: Fabrice GALLET tircown@gmail.com

@KevinOConnor

Copy link
Copy Markdown
Collaborator

Thanks. As high-level feedback, though, I think this change should wait until we've reworked the code so that the existing SYNC_STEPPER_TO_EXTRUDER command can be used with extruder stepper motors. That is, I'd prefer to not add a new command that does something very similar to an existing command.

Also, the help text on the new command mentions that the command syncs "heaters and stepper", but that doesn't seem to be the case?

-Kevin

@KevinOConnor KevinOConnor added the other work first Topic waiting for other changes to complete label Jul 26, 2021
@Tircown

Tircown commented Jul 26, 2021

Copy link
Copy Markdown
Contributor Author

Thank you for the comment.

The mention of heating is an oversight from the original version (PR 4445).

I do understand that it's not right to add and then remove a G-code command. Duplication and mirroring modes require the extruder and extruder1 to make the same movements. Unless to declare dummy pins in extruder1 and an extruder_stepper section with the real pins and use the SYNC_STEPPER_TO_EXTUDER command, I don't know how to perform this task from g-code commands without wasting pins. I assume that dummy pins must exist for the given mcu.

Have you defined a schedule for the extruder.py rework? Can I help on some points?

@KevinOConnor

Copy link
Copy Markdown
Collaborator

Just to be clear, I'm not suggesting that one define an [extruder_stepper] config section to synchronize multiple extruders - I'm saying I think the existing SYNC_STEPPER_TO_EXTUDER command should work for either extruder or extruder_stepper.

Have you defined a schedule for the extruder.py rework? Can I help on some points?

I haven't looked at it in sufficient detail. I'll try to take a closer look at it.

-Kevin

@Tircown

Tircown commented Jul 27, 2021

Copy link
Copy Markdown
Contributor Author

Thank you for the comment.
Got it.

@KevinOConnor KevinOConnor removed the other work first Topic waiting for other changes to complete label Jan 8, 2022
@KevinOConnor

Copy link
Copy Markdown
Collaborator

I do think it would be useful to merge this support into Klipper. If there is still interest in working on this then please update it to the latest code and I will prioritize a review of this work.

Thanks,
-Kevin

@KevinOConnor KevinOConnor added the pending feedback Topic is pending feedback from submitter label Jan 8, 2022
Add ability to synchronize extruder steppers.
Help line correction: no heaters are synchronized in this version.
@Tircown Tircown force-pushed the work-sync-extruder-steppers branch from 2477982 to 1bae660 Compare January 10, 2022 19:56
An oversight from a previous version. The heater is not used anymore.
@Tircown

Tircown commented Jan 10, 2022

Copy link
Copy Markdown
Contributor Author

Thank you. The PR is ready to be reviewed and hopefully merged.

@KevinOConnor KevinOConnor removed the pending feedback Topic is pending feedback from submitter label Jan 11, 2022
@KevinOConnor KevinOConnor self-assigned this Jan 11, 2022
@KevinOConnor

Copy link
Copy Markdown
Collaborator

Thanks. At a high-level, I think it makes sense to add this support. However, I think it would be preferable to refactor the existing SYNC_STEPPER_TO_EXTRUDER than to introduce a new SYNC_EXTRUDER_STEPPERS command.

I have attempted to do this in PR #5143 . Unfortunately, I don't have a good way to test that PR. Would you be able to review that PR and see if it meets the requirements here?

-Kevin

@Tircown

Tircown commented Jan 16, 2022

Copy link
Copy Markdown
Contributor Author

Thanks. I think I can close this PR since #5143 seems to work perfectly now for what we need in idex modes.

@Tircown Tircown closed this Jan 18, 2022
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants