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

Exclude Objects #4716

Closed
wants to merge 11 commits into from
Closed

Conversation

troy-jacobson
Copy link

module: Exclude Objects

Adding Klipper functionality to support cancelling objects while printing.

This module keeps track of motion in and out of objects and adjusts movements as needed. It also
tracks object status and provides that to clients.

The Klipper module is relatively simple, and only provides one piece of the workflow. Support from Moonraker is underway to pre-process gcode files from various slicers with the extended gcode commands supplied by this module. UI's such as Fluidd and Mainsail will provide the user facing controls.

There has been a small group sharing code. In addition to the Moonraker work, I have Fluidd code that will be submitted shortly. Mainsail support is also underway.

Signed-off-by: Troy Jacobson [email protected]
Co-authored-by: Franklyn [email protected]
Co-authored-by: Eric Callahan [email protected]

@kageurufu
Copy link
Contributor

kageurufu commented Sep 23, 2021

A reference implementation of the preprocessor is available at https://github.com/kageurufu/cancelobject-preprocessor and works for Cura, IdeaMaker, and Slic3r-family. Other slicers are fairly trivial to implement at this point, and I plan on adding a conversion for M486 -> our syntax. The benefit over macros are getting inferred POLYGON and CENTER.

Integration into moonraker is also in progress, to simplify the overall workflow. Native slicer support is also in discussion with a few slicer developers.

EDIT: The preprocessor is now on pypi as well, for ease of distribution and integration into other projects

EDIT: A implementation of Moonraker with cancellation processing built in is available at https://github.com/kageurufu/moonraker/tree/work-cancel-object

@KevinOConnor
Copy link
Collaborator

Thanks. Seems like useful functionality. Alas, it'll likely take me some time before I'll be able to review it. What is the current status - is this mergeable and fully useble now, or are there some dependencies needed first?

-Kevin

@kageurufu
Copy link
Contributor

kageurufu commented Oct 11, 2021

The Klipper-side module is complete, and fully functional through raw G-Code. @meteyou has the Mainsail support ready to go as well, and we have a working Fluidd fork.

The preprocessor itself is feature-complete enough, I haven't released official binaries on Github yet, but there are builds on the github actions. Moonraker support is still in the works: its functional, but we are rewriting things to move the preprocessing out of the main metadata generation task to smooth out the process.

I've also tested this PR against the Python 3 branch, and everything works fine there!

EDIT: Just to note- the Moonraker support is not required for Mainsail/Fluidd to be fully functional.

@wlhlm
Copy link
Contributor

wlhlm commented Nov 21, 2021

exclude_object.py has execute permissions. There is no need to run it directly, so it's probably better to change it to 0644 like the rest of the files in klippy/extras.

module: Exclude Objects

Adding Klipper functionality to support cancelling objects while printing.

This module keeps track of motion in and out of objects and adjusts movements as needed.  It also
tracks object status and provides that to clients.

The Klipper module is relatively simple, and only provides one piece of the workflow.  Support from Moonraker is underway to pre-process gcode files from various slicers with the extended gcode commands supplied by this module.  UI's such as Fluidd and Mainsail will provide the user facing controls.

There has been a small group sharing code.  In addition to the Moonraker work, I have Fluidd code that will be submitted shortly.  Mainsail support is also underway.

Signed-off-by: Troy Jacobson <[email protected]>
Co-authored-by: Franklyn <[email protected]>
Co-authored-by: Eric Callahan <[email protected]>
@Arksine
Copy link
Collaborator

Arksine commented Nov 22, 2021

I'm going to try to do a detailed review of the module soon. One thing I would recommend is to rebase your branch against master so the PR doesn't have 173 commits, it makes the review a bit easier.

@troy-jacobson troy-jacobson force-pushed the exclude-object-pr branch 3 times, most recently from 7ed239d to 89acce7 Compare November 22, 2021 17:34
@troy-jacobson
Copy link
Author

@wlhlm Fixed the permission.
@Arksine Fixed the merge/rebase issue.

if self.next_transform:
logging.debug('Disabling ExcludeObject as a move transform')
self.gcode_move.set_move_transform(self.next_transform, force=True)
self.next_transform = None

This comment was marked as resolved.


def get_position(self):
self.last_position[:] = self.next_transform.get_position()
self.last_delta = [0., 0., 0., 0.]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The self.last_delta attribute doesn't seem to be used anywhere.

def _normal_move(self, newpos, speed):
self.last_position_extruded[:] = newpos
self.last_position[:] = newpos
self.next_transform.move(newpos, speed)
Copy link
Collaborator

@Arksine Arksine Dec 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how this works without tracking and offsetting the amount of extrusion that has been excluded. I can see that you offset extrusion in _move_from_excluded_region(), but I would think that the next "normal move" would significantly jump forward in extrusion.

@Arksine
Copy link
Collaborator

Arksine commented Dec 10, 2021

I left a few comments, I'm mostly unsure about the extrusion offset. Once I have a good understanding of how your approach is working, there are some other things to consider:

  1. What happens when an object is excluded while the tool is printing in that location? This is likely going to affect retraction, and IIRC different slicers handle it in different ways.
  2. If my understanding of the extrusion offset is correct, we likely need to offset individual extruders in for setups with multiple extruders.

Based on feedback from Arksine, reworked the move, get_position and
related methods to track using an offset.

Signed-off-by: Troy Jacobson <[email protected]>
Signed-off-by: Troy Jacobson <[email protected]>
@KevinOConnor
Copy link
Collaborator

Thanks. What's the current state of this PR?

-Kevin

@kageurufu
Copy link
Contributor

@KevinOConnor

Sorry for the delay. It's pretty well ready to go as far as I've been aware, Moonraker has full support for processing uploaded G-Code for cancellation, the python module there is fully functional standalone for processing within your slicer.

Cancellation works correctly in testing, and the fork has been tested by a variety of users to great success so far. I believe @Arksine was intending to review this as well when they have the time

@wlhlm
Copy link
Contributor

wlhlm commented Feb 8, 2022

There is currently a problematic interaction between this module and tuning_tower caused by the handling of the move_transformers. @troy-jacobson is aware of this.


import logging
import json
from datetime import datetime
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there is an unused import here.

self.max_position_excluded)
logging.debug("EXO: New position: %s", " ".join(str(x) for x in newpos))
logging.debug("EXO: Offset: %s", " ".join(str(x) for x in offset))
self.log_next_moves = 10
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a heads up, you are going to need to remove the debug logging per the contributing guidelines.

# differences between the last object printed and excluded one.
self.extruder_adj = self.max_position_excluded \
- self.last_position_excluded[3] \
- (self.max_position_extruded - self.last_position_extruded[3])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I know what is happening here, let me verify that I have it correct.

  • If the extruder is retracted before we entered the object, but not retracted before we exit, we are adding an additional retract on the next normal move that isn't a z hop.
  • If the extruder is not retracted before we enter the object, and retracted before the exit, we are going to detract (ie: prime) on the next normal move that isn't a z hop.

I'm guessing we are making certain assumptions how slicers behave. My concern is what will happen if the user cancels the object while its in progress? Could it result in an undesirable prime?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is the intended logic around retraction, and the intent is to ensure that the filament is in the expected position in the extruder prior to moving to the next object.

You're correct that this is assuming a particular behavior on the part of the slicers, but this behavior has been consistent between slicers so far. This is relatively simple logic and I'm comfortable with that it's doing. If there's a slicer or something that needs different behavior, that could probably be addressed in preprocessing.

@Arksine
Copy link
Collaborator

Arksine commented Feb 14, 2022

Sorry it took me a while to get to this. I went ahead and looked at the updates. I think we are close, and I know its testing well. I had some minor comments and the question about a "what if" when attempting to compensate for retraction. Once that is hammered out we can rebase this and it will be approved from my perspective.

@troy-jacobson
Copy link
Author

Hey everyone. Sorry for the lapse in communication. I haven't had as much time to dedicate to this since the holidays. Here's the current status:

  • Overall, the cancel object functionality has been working great. In addition to the testing done through the Mainsail beta, it's been on my system for ages and I've yet to have an issue.
  • I believe the current MR has a fix for the retraction issues Arksine mentioned. I'll check in with him soon now that I have some time to work on this.
  • What I consider the main blocker right now is the issue with registering the transform. When used with other modules, say the tuning tower, the transforms are not unregistered in the reverse order they are registered resulting in the chain of transforms getting corrupted. I've looked at some ideas to fix this which will work most of the time, but may not be correct in all situations (especially if transforms other than the tuning tower can cause the same issue).

@Arksine
Copy link
Collaborator

Arksine commented Feb 15, 2022

I believe the current MR has a fix for the retraction issues Arksine mentioned. I'll check in with him soon now that I have some time to work on this.

Ok, I'll wait on pending feedback for the retraction issue. IIRC, you mentioned that this was to accommodate SuperSlicer. I'm curious, does this affect PrusaSlicer as well? If not, should we just consider it a bug in SuperSlicer and request that it get fixed there rather than attempt to compensate in Klipper?

What I consider the main blocker right now is the issue with registering the transform. When used with other modules, say the tuning tower, the transforms are not unregistered in the reverse order they are registered resulting in the chain of transforms getting corrupted. I've looked at some ideas to fix this which will work most of the time, but may not be correct in all situations (especially if transforms other than the tuning tower can cause the same issue).

The issue we are bumping into is that Tuning Tower is registered before Exclude Object is registered, then unregistered before Cancel Object is unregistered. This is effectively "breaking the chain", because when Tuning Tower is removed Exclude Object's next_transform is still referencing it. I think its best to get Kevin's feedback on this, I'm not sure how to fix it without changing how transforms are registered. We could document that exclude_object is not compatible with tuning_tower. To my knowledge there are no other "dynamic" transforms, bed_mesh, bed_skew, and bed_tilt are never unregistered...but that doesn't mean that more won't be added in the future.

@kageurufu
Copy link
Contributor

We could also just make exclude_object not dynamic, and short-circuit it's own logic when not needed

@Arksine
Copy link
Collaborator

Arksine commented Feb 15, 2022

TBH, its probably not valid to enable exclude_object if a tuning tower is in progress. We could just add something like the following to tuning_tower.py:

def is_testing(self):
    return self.normal_transform is not None

Before setting the transform in exclude_object we could look up the tuning tower and call is_testing() on it. If True, we abort registration.

@KevinOConnor
Copy link
Collaborator

TBH, its probably not valid to enable exclude_object if a tuning tower is in progress. We could just add something like the following to tuning_tower.py:

That sounds fine to me. We could add a more flexible transform registration/unregistration process, but that's likely overkill right now.

-Kevin

This commit mainly addresses conflicts between Exclude Object and
the Tuning Tower module register and unregister process.  Some
workflows that include both modules resulted in Klipper crashes.

The overall issue is that the current process for registering move
trasnforms in Klipper requires that transforms be unregistered in
the reverse order they were registered.  As it currently stands,
these two modules cannot guarantee that behavior.

This change gives priority to Tuning Tower by not enabling the
exclude object features if a tuning tower test is in progress.  It
also will not unregister the exclude object move transform if that
transform is not at the front of the list.

There are some logging cleanup changes as well.

Signed-off-by: Troy Jacobson <[email protected]>
@troy-jacobson
Copy link
Author

Just pushed changes to address the Tuning Tower issue. The change is basically a) don't load the exclude_object transform if a tuning tower test is active and b) don't unload the exclude_object transform if it's not the at the head of the list. I think this both fixes the Klipper crashes and maintains the expected behavior for users.
I thought about some alternatives, but I felt they may either make the tuning tower more difficult to use, figuring out how to unregister the exclude_object transform, or navigate non-intuitive side effects of how the two modules interact.
Here are the two scenarios:
A: I've been actively printing have have been using the exclude_object module. It's still loaded when starting a tuning tower. The tuning tower transform is loaded ahead of exclude object. Given that the tuning tower doesn't actually transform the movements, I can't see any reason why this state needs to be avoided. When the SD card reset is called at the start of the print, the exclude_object transform is not unloaded like it normally would be. Again, I'm not seeing any functional issues with this. After tuning is finished, a regular print is started. Again, the exclude_object transform is reused because at this time, the tuning tower transform is still enabled. Once the print starts extruding filament, however, the tuning tower recognizes that the test is done and unloads itself. The exclude_object transform is now at the front of the transform list. Subsequent prints will behave as normal with regards to registering and un-registering the exclude object move transform. During this time, the user will be able to cancel objects, including the tuning tower.
B) Neither the tuning tower nor exclude object transforms are loaded. A tuning test is started, loading that transform. If the STL for the test print has objects defined, the attempt to load the exclude object transform will be rejected and the exclude object functionality will not be enabled. After the tuning print, a regular print is started. At this point, the tuning test is still active so the exclude object transform is still not enabled. Once that print starts extruding, the tuning tower will detect that the test is finished and unload its transform. The next regular print will be able to enable the exclude object functionality as expected.
Does this sound OK to you? The second scenario has the quirk around the first print after the tower, but honestly given what's usually happening when doing tuning towers, I bet it would hardly be noticed. Also, I think the first scenario is going to be the most common one anyway.

@wlhlm
Copy link
Contributor

wlhlm commented Feb 28, 2022

I'm running into performance issues excluding an object on a relatively simple plate:

For every new kind of filament I'm printing I run a small plate of samples consisting of a filament swatch and a Voron test cube.

Here is how the plate looks:
Screenshot-2022-02-28-182006

I wanted to print another Voron cube, so I just reran the plate excluding the filament swatch. However I'm seeing significant stalling on layer change:

VID_20220227_202326Zsmall.mp4

Here is the SS project:
samples.3mf.zip

I can provide the GCode if it's of any help.

Running Klipper on python 3.10 on RPi 4B 2GB.

@Arksine
Copy link
Collaborator

Arksine commented Feb 28, 2022

That is typical behavior of exclude object. The gcode on the excluded portion of the layer must still be processed up through the exclude_object module, this takes some time and introduces a pause.

@CODeRUS
Copy link
Contributor

CODeRUS commented Feb 28, 2022

@Arksine out of curiosity: have you considered fast-forwarding sdcard position from start to end object macro?

@Arksine
Copy link
Collaborator

Arksine commented Feb 28, 2022

I'm not the lead dev on this, but I don't think that is feasible. You couldn't fast forward as you still need to check each line of G-Code to determine when the object ends. It is also necessary to process the excluded gcode moves so we can track how much extrusion is being excluded and offset the tool appropriately.

@kageurufu
Copy link
Contributor

@CODeRUS We have to do some processing of skipped G-Code to handle some edge cases, mostly around toolchanging and absolute extrusion

See https://github.com/troy-jacobson/klipper/blob/4c5e052d46f875a52da18b59f77bba99a1789bbd/klippy/extras/exclude_object.py#L142-L149

Fast-forwarding could be done in controlled situations, although it would probably require native slicer integration to get "right". If the preprocessor included the jump and extrusion offsets in the START/END_CURRENT_OBJECT it could possibly be done?

I'll experiment sometime, but the priority right now is getting this working in general.

@CODeRUS
Copy link
Contributor

CODeRUS commented Feb 28, 2022

@Arksine sounds like something can be done on moonraker preprocessor side, end_object macro could contain all necessary values itself, with no need to process chunks in realtime again.

@kageurufu no problem, just my silly question? mostly out of context :)

@CODeRUS
Copy link
Contributor

CODeRUS commented Feb 28, 2022

btw, @Arksine is it possible to have macro for calling object processing for file manually? i have couple of scenarios. 1. is when long file is saved by chunks, object processing may be called to early. 2. is when i dont want to have exculude object being active for all my prints for time savings and etc., so i can call processing manually per file. having separate slicing profiles can be pita in this case.

@Arksine
Copy link
Collaborator

Arksine commented Feb 28, 2022

Moonraker imports @kageurufu's library to handle the processing. That said, even if the preprocessor made these calculations and provided the correct location to jump to, there are still outstanding issues:

  1. It isn't possible pre-calculate the portion of extrusion skipped if the object is canceled while its printing inside of that object, so you would need logic to process out the object at that point.
  2. There may be some gcode embedded in the object that should still be executed, ie M73
  3. This would require modification to Klipper's virtual_sdcard module, increasing complexity and maintenance in that portion of the code. You would have to sell Kevin on whether or not such a modification is worth the change there.

Its not possible to manually call the preprocessor from Moonraker, the purpose of adding that support was to automate the process. It should be possible to work with the library directly if you want to determine when and what files to process.

@CODeRUS
Copy link
Contributor

CODeRUS commented Feb 28, 2022

@Arksine thanks, just found debug output with correct command

~/moonraker-env/bin/python ~/moonraker/moonraker/components/file_manager/metadata.py -p ~/gcode_files -f "$1" --check-objects

can work with that. asked that question because i already had issues with not properly processed files, when processing was started while file is still uploading manually.

@kageurufu
Copy link
Contributor

kageurufu commented Feb 28, 2022

@CODeRUS no problem, I'm always glad to entertain requests. Helps keep you from boxing yourself into one mindset while building something

You can call the preprocessor manually, it's installed to ~/moonraker-env/bin/preprocess_cancellation by default, and running ~/moonraker-env/bin/preprocess_cancellation sdcard/some.gcode will, by default, rewrite the file in place. You can also call it from PrusaSlicer/SuperSlicer by downloading the executable from my releases page, and adding it to your Print Settings > Output options > Post-processing script section.

@glowtape
Copy link

glowtape commented Feb 28, 2022

I wonder if it would make sense to be able to define things like optional E- and Z-offsets to apply and undo when object exclusion starts and ends respectively, e.g. raise the nozzle a millimeter and retract the filament some to prevent oozing during stalls and the nozzle melting neighboring plastic depending on geometry.

@troy-jacobson
Copy link
Author

@wlhlm I would like to see the gcode file, if you still have it. That's a lot more of a pause than I'm used to seeing.
I've excluded large objects with gyroid infill and didn't seem to have issues. SS does end up with an extra move or two when switching layers in a situation like this, but it shouldn't be that long unless you're running with a lot of retraction.

@troy-jacobson
Copy link
Author

troy-jacobson commented Mar 1, 2022

@glowtape I don't think it should be necessary to have extra moves like that. There could be a situation, though, where we should tweak the preprocessing in order to clean up the between object moves for an individual slicer. I feel that this portion in klipper should be as simple as possible. If we need to provide hints or slicer specific work-arounds, they should be directed by the preprocessor.

Although I did take a peak at the config in the file you attached. The settings are for a slightly longer and slower retraction that I'm using, so that's probably what's happening.

It may be something we'd like to look at for a super slicer specific tweak.

@wlhlm
Copy link
Contributor

wlhlm commented Mar 1, 2022

@troy-jacobson

I would like to see the gcode file, if you still have it.

Sure thing.

wlhlm-samples-prusament-asa.gcode.gz

@troy-jacobson
Copy link
Author

@JRHeaton If you're interested in trying this out, the best way is through Mainsail. There is documentation from the beta cycle that includes directions for klipper and mainsail. The currently released Mainsail has the UI side of the functionality. It's best to discuss any install questions there.

@troy-jacobson
Copy link
Author

troy-jacobson commented Mar 1, 2022

I've been processing the comments here for a while, and feel like I have a good path forward for all of the outstanding items except around the sd card reset. Here are some of my current thoughts and things that influenced the implementation.

I'm hesitant to delegate this to the preprocessor. If someone is in the middle of configuration, tuning, slicer changes, etc. they could easily run into a situation where the UI could be showing object information that is from an older print. That's why I looked for an automated solution.

I had thought about having the UI handle this, but then starting a print from an LCD could cause issues.

From a user's perspective, it's nice not to reset the state (the object lists, etc.) at the end of a print. It lets them review what happened. Especially if the print was cancelled.

That state can be handled independently from the transform. And if I can be pointed to a reliable way to detect the end of a print (including it being cancelled), I'd like to unregister the transform at that time but keep the other state for the user.

I'm not sure if this has any bearing on the reset implementation, but I'd like to have flexibility to tie into the Moonraker metadata in the future. One example could be reprinting only the cancelled objects.

@KevinOConnor
Copy link
Collaborator

I've been processing the comments here for a while, and feel like I have a good path forward for all of the outstanding items except around the sd card reset.

Okay. I don't object to an automated reset on an sdcard
reset.

As previously mentioned though, the event should use "virtual_sdcard" prefix and it would be preferable for the change to be in a separate commit.

Thanks.
-Kevin

@Arksine
Copy link
Collaborator

Arksine commented Mar 1, 2022

Perhaps it would still be a good idea to add a G-Code command that resets the transform. That way exclude_object isn't exclusively locked to Moonraker. For example, OctoPrint's cancel object plugin could implement it as an alternative, Repetier could implement it if they so chose, etc.

@gurusonwheels
Copy link

gurusonwheels commented Mar 1, 2022

im not a developer . but i have installed this yesterday on my voron 2.4 with mainsail os and mainsail web UI . it works very well and i dont see any pauses when i exclude an object . im just a end user and wanted to give you my 2 cents for whatever its worth .
i use superslicer on windows 10 . my voron has a 5 inch lcd with klipperscreen if you need something tested . as i say im not a developer so my linux skills is very week .
one thing i notice is the popup interface in mainsail web for excluding objects . it highlights the object on the left but not the actual name on the right . please make it highlight the name as well . makes it very hard to track down which one to click the exclude button when there small items being printed as it switches to fast . other than that kudos for a job well done so far .
exclude

@kageurufu
Copy link
Contributor

Alright, I have a updated branch for this PR ready at https://github.com/kageurufu/klipper/tree/exclude-object-pr, I think Troy got a bit busy, would it be acceptable to open a new PR, referencing this one?

@troy-jacobson
Copy link
Author

@kageurufu Did a fantastic job rewriting the commit history, formatting the documentation, and renaming the commands.
Between his work there and some final tweaks I made, I thing we have all of the requested changes covered.

I've been testing these changes for the past couple of days, including his preprocessor changes that use the new commands.

I opened a new pull request with these updates: #5326

@troy-jacobson troy-jacobson mentioned this pull request Mar 6, 2022
@gurusonwheels
Copy link

did this get abandoned ?

@maz0r
Copy link

maz0r commented May 7, 2022

did this get abandoned ?

No its moved to #5413

@KevinOConnor
Copy link
Collaborator

PR #5413 was merged.

-Kevin

@KevinOConnor KevinOConnor added the resolved Issue is thought to now be fixed label Jun 3, 2022
@github-actions
Copy link

This ticket is being closed because the underlying issue is now thought to be resolved.

Best regards,
~ Your friendly GitIssueBot

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

@github-actions github-actions bot closed this Jun 11, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
resolved Issue is thought to now be fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants