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

fix(extruderPanel): Modified ExtruderControlPanelControl to use _CLIENT_EXTRUDE and _CLIENT_RETRACT for correct extrusion mode after operation #1964

Closed

Conversation

kraftaksvk
Copy link

Description

This PR changes the way how Extruder Control Panel sends motion requests to the extruder.
Originally, when pressing EXTRUDE, the panel would send out this command:

M83
G1 E${this.feedamount} F${this.feedrate * 60}

RETRACT command:

M83
G1 E-${this.feedamount} F${this.feedrate * 60}

This puts the extruder movements into Relative mode and stays like that until M82 is emmited, resulting in unwanted behavior if using Absolute mode otherwise.

My Pull Request replaces that EXTRUDE command with:

_CLIENT_EXTRUDE LENGTH=${this.feedamount} SPEED=${this.feedrate}

...and RETRACT command with:

_CLIENT_RETRACT LENGTH=${this.feedamount} SPEED=${this.feedrate}

By utilizing _CLIENT_EXTRUDE and _CLIENT_RETRACT defined in mainsail.cfg, we address the issue of movement modes, as these macros take care of retaining the same movement mode, and as a bonus it also handles firmware retract and cold extrusion protection.

Related Tickets & Documents

none

Mobile & Desktop Screenshots/Recordings

Before:

before

After:

after

(PROBE_EXTRUSION checks whether Absolute mode is present)

[optional] Are there any post-deployment tasks we need to perform?

none

Signed-off-by: Marek Rozemberg [email protected]

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Aug 7, 2024
@meteyou
Copy link
Member

meteyou commented Aug 7, 2024

hey @kraftaksvk!

Thank you very much for the PR, but I have some questions about it.

  • just to use _CLIENT_EXTRUDE and _CLIENT_RETRACT is not possible, because not every instance has these macros (there are also users which don't use the mainsail.cfg). So, it can only use these macros when they exist
  • all users with firmware retract cannot use these macros because the macros ignore the length of the panel and will only extrude/retract the current retract length
  • can_extrude will be handled by Mainsail itself and the buttons will be disabled, when the extruder temp is lower than min_extrude_temp

Can you also describe the exact issue you want to fix with that? Maybe we can find a better solution to fix your issue.

@kraftaksvk
Copy link
Author

Hi @meteyou. Well, those are some good points 😅😅. I figured that mainsail.cfg would be the same in every instance as it is read-only. My issue is as described, the buttons on the panel force M83 every time, so if the user doesn't notice, and forgets to send specifically M82 (G90 doesn't seem to affect) manually or in a start macro, bad things could happen (personal experience). I would like to see similar behaviour as with _CLIENT_EXTRUDE, which tolerates the previous movement mode and applies it after move. I hope we can find another way then!

@meteyou
Copy link
Member

meteyou commented Aug 8, 2024

To be honest, an M82 in the start marco is an essential requirement for an absolute extrude print. Even if a relative retract is made in the end gcode (which is normal), this is required at print_start.
Furthermore, this is not a fix if a macro has been copied from somewhere else and uses relative extrusion. To be sure that a print with absolute extrusion works, you have to set it in your start gcode.

@kraftaksvk
Copy link
Author

I agree. I'm going to close this PR. Have a nice day!

@kraftaksvk kraftaksvk closed this Aug 8, 2024
@meteyou
Copy link
Member

meteyou commented Aug 8, 2024

Feel free to contact us if you find another problem with relative extrusion outside of print start.

@mryel00 also wants to test something without _client_extrude, maybe this would also be a possible fix for this issue without "external macros".

@mryel00
Copy link
Member

mryel00 commented Aug 8, 2024

@kraftaksvk My PR #1965 is up. The code about the names is just rudimentary and will be fixed by @meteyou. I'm not a web dev 😅
I would appreciate if you can test it. This approach should be more generic and hopefully work on every setup, without messing stuff up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants