Repository navigation
Tc exit options - #51
3DPrintDemon wants to merge 6 commits into
Conversation
Added configuration options for tool change behavior, including parking location and nozzle cleaning settings.
Added variables for exit pickup and restore coordinates in the tool change macros. Enhanced G-code macros to handle tool change logic more robustly, including checks for valid restore positions and scrub functionality.
charliemayall
left a comment
There was a problem hiding this comment.
A few comments.
Firstly, some of this is feeling a little out of scope. There are so many brush solutions out there, all of which will likely have their own macros, so I don't see the need for bringing that into this repo when it will be so variable between user specific setups.
Actual issues with the changes:
- restore_pos defaults to True despite claiming these features are off by default.
- restore_x / restore_y have no setter, so every mid-print toolchange would travel to X50 Y50.
- Gating of positions based on clearance_y and dock_dir is questionably necessary, and performed incorrectly.
| variable_park_on_ooze_pad: False | ||
| variable_pad_x: 10.00 | ||
| variable_pad_y: 26.00 | ||
| variable_restore_pos: True |
There was a problem hiding this comment.
Doesn't agree with claim that these features are off by default
There was a problem hiding this comment.
claim is not all features are off by default, but reverts to default exit behaviour when they are turned off.
| variable_rotation_dist_overridden: 0 | ||
| variable_exit_pickup: False | ||
| variable_valid_restore: True | ||
| variable_restore_x: 50 |
There was a problem hiding this comment.
no setter exists for either of restore_x or restore_y
There was a problem hiding this comment.
missed from original file, genuine mistake that has now been added. Thanks
| {% if tp.dock_dir == -1 and (tp.tcs_start_y|float < tp.clearance_y|float or tp.tcs_fin_y|float < tp.clearance_y|float) %} | ||
| RESPOND TYPE=error MSG="⚠️ TOOL CHANGE tc_scrub tcs_start_y or tcs_fin_y is set less than clearance_y please correct this to activate this feature!" | ||
|
|
||
| {% elif tp.dock_dir == 1 and (tp.tcs_start_y|float < tp.clearance_y|float or tp.tcs_fin_y|float > tp.clearance_y|float) %} |
There was a problem hiding this comment.
If you do decide to keep this check, correct the operator.
| {% elif tp.dock_dir == 1 and (tp.tcs_start_y|float < tp.clearance_y|float or tp.tcs_fin_y|float > tp.clearance_y|float) %} | |
| {% elif tp.dock_dir == 1 and (tp.tcs_start_y|float > tp.clearance_y|float or tp.tcs_fin_y|float > tp.clearance_y|float) %} |
There was a problem hiding this comment.
Ah you're quite correct again, missed that one too. Thats what writing at midnight after a full day's work & family time does for you! Thank you for the sanity check there!
| {% set v = printer["gcode_macro _TOOLCHANGE_VARS"] %} | ||
| {% set final_f = (printer.configfile.settings.printer.max_velocity * 60 * v.speed_mult) %} | ||
|
|
||
| {% if tp.dock_dir == -1 and (tp.tcs_start_y|float < tp.clearance_y|float or tp.tcs_fin_y|float < tp.clearance_y|float) %} |
There was a problem hiding this comment.
Consider if these checks are needed. clearance_y is not reserved exclusively for tool change activities, I have my purge and brush setup within it.
There was a problem hiding this comment.
This is true, it could well be a usable space with some users, I'm kinda thinking of maybe less advanced users that might not realise the potential pitfalls here & maybe trying to keep them out the realms of the complexities of moving into possible dock collisions. I appreciate the comments & thank you for the input. I will think it over for sure!
| M109 S{params.TARGET|default(0)|float} | ||
| {% endif %} | ||
|
|
||
| {% if printer.print_stats.state in ["printing", "paused"] and tp.tc_scrub == True and tp.restore_pos == False %} |
There was a problem hiding this comment.
Why does scrub have to require restore_pos==False as well?
Added _RESTORE_GRABBER macro to manage tool change positions.
The brush. Possibly a little far reaching I agree but not un-useful for it to be an integrated option for a simple system that users don't have change, modify, add or edit files to use. Print a simple brush/pad, fit it, turn on a variable & use it for each tool change, gives a quick simple clean job done. Do you think users would not want this? If some didn't & wanted something more complex couldn't those users just turn it off & do the modifications for whatever they did want?
|
Refactor exit pickup logic and remove unused _TC_SCRUB macro.
Removed unused variables related to nozzle cleaning and pass count.
|
...Well after sleeping on it I decided to remove the built in nozzle clean stuff as I'm fairly sure of the outcome of this PR if left in. The restore_pos stuff is genuinely useful & people have said so, they're already using it & preferring it over the current default, & have even appropriated it into their own forks. I left the park on ooze pad in as that could be useful too maybe, but again, happy to remove that if unwanted. I'll leave the tc_scrub in my main fork along with a load of other stuff, if anyone is interested. Happy to add the tc_scrub back in here if requested, but I'll be assuming it won't be. Right gotta run out the door to work now, so if there's any mistakes in the edits/deletions let me know & I'll fix them when I get home later. |
charliemayall
left a comment
There was a problem hiding this comment.
Changes all look good except the one typo I've commented on.
Co-authored-by: Charlie <81966433+charliemayall@users.noreply.github.com>
|
Read through this and the whole thread with @charliemayall. His three points were the right ones and you have dealt with two of them; This now conflicts with main. The The invalid-restore path leaves Z unrestored, and that is the one I would call a hazard. When {% if v.valid_restore %}
G0 X{v.restore_x|float} Y{v.restore_y|float} F...
M400
G0 Z{params.Z|float} F...
{% endif %}So the guard that exists to keep you out of a dock also silently skips the Z restore, and the head stays at toolchange height. The message says "Skipping restore", which reads as "falling back to normal", but normal would be the plain
The dock-collision check does not check what it says it checks. {% if (v.restore_x == tp[p_x] and v.restore_y|float == p_y) or (i == act and v.restore_y|float < p_y) %}The first clause is exact float equality between a captured print position and a configured dock X. That is worth knowing alongside Charlie's point about whether the check should exist. He runs his purge and brush inside
Whitespace. Mixed tabs and spaces through the new blocks, including inside On Worth saying plainly: #48 was the report that motivated this, and its reporter has since confirmed the cause was sparse prime tower enabled by mistake in their slicer, and that the stock macros work. That does not make this feature worthless, since heating into the tower to catch ooze is a real and separate benefit and I want it available. It does mean the argument for making it everyone's default is weaker than it looked in August. Ship it defaulting to False, let people turn it on, and we can revisit the default once it has some hours on other people's machines. Sorry this took so long to get a proper read. The feature is worth having. |
|
@ogland Yep can bring the #46 change in, no worries, just not had time this week. Ah yes that restore path, I can see that's less than ideal, I think maybe this is a remnant from before removing the park/scrub restriction that was originally included & it was a little different perhaps, forgive me its been a few weeks away from this now. I'll definitely update that there though for sure, good call. The first clause in dock collision check is purely a simple check to purposefully see if the restore values being presented are the exit positions of another dock - it's there to stop the system putting the toolhead back in front of that dock while holding another tool at the start of a print. For example.... If you start your printer, home all axes, load all tools with filament & leave it holding T7 & then start a print the printer will automatically change to T0 to home, QGL, other/etc while starting the print. By doing this the restore values you get will be for the exit location where the T7 pickup left it, in front of T7 dock. The printer will dutifully grab T0 & then run all the way back to park it needlessly in front of the T7 dock again while it continues with the start process. This check is to stop that needless movement & confusing parking behaviour occurring. The second clause as you say indeed doesn't need to be in the loop there, it could be moved outside to fire once only & then perhaps go to the check loop if it passes. The rest of the park location lock-out/check stuff has been removed in an edit after speaking with Charlie for the exact reason of users having brush or ooze systems inside the docking areas, totally agree with that. Thanks for picking up on spacing, will normalise spacing, absolutely not an issue. I do try to keep things tidy. By all means we can default the restore system to False for everyone to turn on if they wish to. No biggie on that one. In regards to your comment about #48 motivating this feature. The restore behaviour used here (for the most part, more basic form, but same travel behaviour) was actually sent to you a couple of months ago by file on Discord before this Gihtub repo had published any macro files publicly, way back in preproduction while I was designing the SV08 & SV08 Max integrations & I had just gotten my machine working. It was then made available within my fork's main branch a fair time before creating the branch for this PR, I commented in #48 as I thought this feature might benefit that user at the time. Good to know his issue has been resolved by other means. True I then I created this PR after he confirmed it helped him, so I thought it might benefit others also. However I originally created the basic restore behaviour to give a bit more breathing room for changes above print surfaces & any prime tower, not to mention ease puckering first layer changes - especially as at the time all change moves had incorporated Z movements. My personal preference was always to raise Z, travel XY, do a thing, travel back XY then lower Z to restore. The stock system raises, travels, does a thing, lowers, then travels back to restore. For me that was the important factor & impetuous behind this feature all those weeks ago. I think we spoke about it briefly, but I do remember you were very busy at the time & had lots going on there. I am absolutely more than happy to take onboard your excellent feedback once again & edit things to conform to the needs here. Totally not a problem at all as I'd love to see this one being merged, while I know it's not a strictly necessary thing I do think folks might find it of some use, plus it's always nice to have a little choice too. I'll make the described edits & get back to you with an update! Thanks for taking the time go through this one! |
|
This needs a rebase and some changes before it can go in, because main has moved under it. Since #96 a pickup is grab, load cell seat check (with one retry), then finish, and the heater only comes on in _PICKUP_FINISH after the tool is recorded (#93). The restore move happens during _LINEAR_EXIT, which is before the seat check, so a failed or half seated pickup would be detected with the head already lowered over the print or the tower, and the retry's X/Y re-home would start from there. If you want to pick it up again: make it a separate exit step that runs after _PICKUP_FINISH (the _TC_EXIT_MOVE idea from earlier), default off, keep the z_hop exit height until the head is back over its target, and the earlier review points still stand (invalid-restore path skipping Z, the pad path never restoring Z, float equality in the dock check, _TC_G0 overloading). |
Add TC exit options for users.
restore_pos operation....
This variable enables an active restore system that changes the default TC return behaviour. So instead of picking up a tool & immediately restoring the TC Z height at the clearance_y threshold & heating this variable enables the toolhead to pass the clearance-y location & returns it to the location where the TC was called. Preferably the purge/wipe tower (use slicer setting) once at the restored XY coordinates the toolhead then lowers back to the restored Z height & waits to fully heat.
This makes sure the toolhead passes cleanly above the top of any print on the bed & heats the nozzle into the purge tower catching any oozing filament as it dos so.
The system runs checks that will bypass functions with warnings if set variables encroach on docked tools clearance Y values & if restore_pos locations are in front of other tool's dock & match their exit positions.
The ooze_pad park & tc_scrub clean nozzle systems can be combined, however the restore pos & nozzle clean can not. - tc_scrub REMOVED
The ooze_pad variable overrides the restore_pos setting.