Regression of #4631: support spiral lift with timelapse gcode and multi-color prints #4848

ziehmon · 2024-09-20T12:02:27Z

Context

This is a regression of the previous pull request #4631, which had to be reverted due to bugs when used with multi-color/AMS Lite prints.

Cause

At the print sequence, where the described bug was happening, there is a difference between printing multi-filament with AMS Lite and printing single-filament:

Without multi-filament (so, without AMS Lite), the position before the Z hop for the layer change is executed, is X=0.00. This is set in "Timelapse G-Code" and will be executed as last step after the picture is taken. Other movements in the "Timelapse G-Code" exist too, but they also use a safe position, even though not x=0.0000 (e.g. for nozzle clumping detection).

But with multi-color (so, with AMS Lite), the position before the Z hop for the layer change is executed, is X=-48.2. This is set in "Change Filament G-Code" and is the last movement of the "wipe and shake" mechanism. That "wipe and shake" happens either after changing filament, and/or after extrusion calibration:

Solution

The cause of the bug raises two questions:

Why was there no check if the printer is moving out ouf the physical boundaries?

Answer: there is no boundary check implemented when the spiral Z hop gets executed. This is where the check is supposed to be done:

 //BBS: todo: check the arc move all in bed area, if not, then use lazy lift 	double radius = delta(2) / (2 * PI * atan(GCodeWriter::slope_threshold));
 Vec2d ij_offset = radius * delta_no_z.normalized();
 ij_offset = { -ij_offset(1), ij_offset(0) };
 slop_move = this->_spiral_travel_to_z(target(2), ij_offset, "spiral lift 	Z");

How can we solve the problem?
- Answer: we need to ensure that after the execution of gcode to change the filament, the position is safe for executing a spiral Z hop. This is already done for the timelapse gcode

Implementation

For a multi-filament/AMS Lite print, there are two scenarios where we need to set a safe position afterwards:

After wipe the filament change wipe is done, and when no extrusion calibration will be executed
When an extrusion calibration happens after filament change and another wipe was executed

The change is the same for both scenarios - the safe position just needs to be set in different sections of "Change Filament G-Code":

Before, without a safe position (X=-48.2):

G1 X-38.2 F18000
G1 X-48.2 F3000
G1 X-38.2 F18000 ;wipe and shake
G1 X-48.2 F3000
G1 X-38.2 F12000 ;wipe and shake
G1 X-48.2 F3000
M400

Afterwards, with a safe position (X=0.00):

G1 X-38.2 F18000
G1 X-48.2 F3000
G1 X-38.2 F18000 ;wipe and shake
G1 X-48.2 F3000
G1 X-38.2 F12000 ;wipe and shake
G1 X-48.2 F3000
G1 X0 F18000 ; move to safe pos
M400

Result after implementation of bugfix:

Summary

Additionally to the previous, included PR, this MR changes the change_filament_gcode (in GUI: "Change filament G-Code") for A1 and A1 Mini printers to set the postion safe (X=0.00) after filament was changed in two different scenarios.

This bugfix does not apply to anything else than A1 or A1 Mini. The original Z hop fix did not change behavior of other printers like P1S or X1C (non I3-Structure).

Test-cases

Lesson learned - the testing was not sufficient last time. I came up with the following test cases to make sure, we don't have another bug. Personally, I can only cover A1 test-cases without AMS Lite. Maybe Bambu Lab can test it at scale on their print farm.

Printer	AMS Lite	Timelapse	Result
A1	Not used	Used	TODO
A1	Used	Used	TODO
A1	Not used	Not used	TODO
A1	Used	Not used	TODO
A1 Mini	Not used	Used	TODO
A1 Mini	Used	Used	TODO
A1 Mini	Not used	Not used	TODO
A1 Mini	Used	Not used	TODO

Appendix

The existing implementation did only read the new Z position from the injected timelapse_gcode and flagged the position as unsafe because of this. This change reads X, Y and Z pos from the timelapgse_gcode and will keep the position state correct to enable safety checks required for using spiral Z hop. Because of this, spiral Z hop can be used everyhwere now. The same pattern is also applied for layer_change/toolhead gcode injection. The set_current_position_clear method is unused but will be kept in implementation for future scenarios.

XunZhangBambu · 2024-09-20T12:18:43Z

@ziehmon Thank you very much for continuing to work on fixing this PR. I think we could discuss a more general approach to fix the issue

We need to check the spiral lift to ensure that when executing the spiral lift, it does not exceed the range of the bed. If it goes beyond the range, it can be replaced with a normal lift.
We need to find a way to get the nozzle's position after executing custom G-code. Currently, only checking G0, G1, G2, and G3 commands is not sufficient, as other commands could also move the nozzle.

tsmith35 · 2024-09-20T13:54:59Z

@ziehmon, I submitted a PR for OrcaSlicer to revert the modifications until the corrected modification has been tested.

ziehmon · 2024-09-20T13:59:38Z

@ziehmon, I submitted a PR for OrcaSlicer to revert the modifications until the corrected modification has been tested.

Thank you! I think SoftFever already reverted it

ziehmon · 2024-09-20T14:58:24Z

@ziehmon Thank you very much for continuing to work on fixing this PR. I think we could discuss a more general approach to fix the issue

1. We need to check the spiral lift to ensure that when executing the spiral lift, it does not exceed the range of the bed. If it goes beyond the range, it can be replaced with a normal lift.

2. We need to find a way to get the nozzle's position after executing custom G-code. Currently, only checking G0, G1, G2, and G3 commands is not sufficient, as other commands could also move the nozzle.

@XunZhangBambu my pleasure, thank you for your support and review!

I absolutely agree. There are better approaches. The question is, how to implement them? From my perspective, we should do it in three steps.

1. Step: keep the current approach for now, but review that all gcode (timelapse, toolchange, machine_start/end) ends in the safe position of X=0.00 and Z>={last_layer_z}. For Z, this is already given and for X, this PR adds it.
2. Step: extent GCodeProcessor::get_last_pos_from_gcode in such a way, we reliably can retrieve all movement commands.
3. Step: in GCodeWriter::travel_to_xyz add a validation if the printer would move out of bounds. There is a comment already that mentions it still needs to be done here.

Step 3. requires Step 2. to work properly. I think I could implement all three of them.

However, I might need some time. Do you think, Step 1. is safe enough to begin with, and we ensure 2. and 3. in a separate Pull Request? Or are you concerned we might not catch other cases where the position is unknown?

XunZhangBambu · 2024-09-29T08:46:51Z

@ziehmon I think we can start from the third step. What needs to be done in the third step is to add detection of the hotbed boundary in the travel_to_xyz function. This should be the safest and simplest modification.

XunZhangBambu · 2024-09-29T08:48:08Z

@ziehmon Finding a safe position in all circumstances for the timelapse G-code will be quite complex and requires careful consideration.

ziehmon · 2024-09-29T14:16:57Z

@ziehmon I think we can start from the third step. What needs to be done in the third step is to add detection of the hotbed boundary in the travel_to_xyz function. This should be the safest and simplest modification.

I agree, but one challenge: the travel_to_xyz function would check if the hotbed boundary is exceeded before gcode for filament change and/or timelapse is injected. When the function travel_to_xyz runs, the position would be safe for travel since the injected gcode (timelapse and/or filament change) is unknown at this point.

Implementing the detection after the get_last_z_from_gcode would work, because this happens while processing the injected gcode, but then other moves (like travel_to_xyz) could still cause problems.

Do you have any idea how to solve this?

tsmith35 · 2024-09-29T15:51:09Z

Do you have any idea how to solve this?

Does Prusa Slicer have the spiral glitch? How do they handle this?

ziehmon · 2024-09-29T15:58:45Z

Do you have any idea how to solve this?

Does Prusa Slicer have the spiral glitch? How do they handle this?

I don't think so. This is purely an implementation issue caused by user configurable gcode injections. I can think of different approaches to fix this, but of course we're looking for the easiest and most effective one. I am looking forward to the answer from @XunZhangBambu since I might be overlooking something.

ziehmon · 2024-10-16T14:20:00Z

@ziehmon I think we can start from the third step. What needs to be done in the third step is to add detection of the hotbed boundary in the travel_to_xyz function. This should be the safest and simplest modification.

I agree, but one challenge: the travel_to_xyz function would check if the hotbed boundary is exceeded before gcode for filament change and/or timelapse is injected. When the function travel_to_xyz runs, the position would be safe for travel since the injected gcode (timelapse and/or filament change) is unknown at this point.

Implementing the detection after the get_last_z_from_gcode would work, because this happens while processing the injected gcode, but then other moves (like travel_to_xyz) could still cause problems.

Do you have any idea how to solve this?

@XunZhangBambu may I ask you to review my comment? This is a fundamental problem from my understanding.

XunZhangBambu · 2024-10-17T02:26:13Z

@ziehmon Of course ! The injected G-code is provided internally by us, we will ensure that the movements do not exceed the physical limits of the machine. So we only need to check the movements generated by code. By the way, we are also internally exploring ways to ensure that even if the injected G-code is skipped, the correct nozzle position can still be obtained after executing the injected G-code and this will take some time.

ziehmon · 2024-10-17T20:52:03Z

@XunZhangBambu thank you for your reply. I think we misunderstood each other, so let me summarize:

I raise this PR to change the injected filament change gcode to end in an safe, consistent and expected position (x=0, y=0)
You suggest we instead validate/check if the position is safe for a spiral Z hop, and disable it if not
I ask how to implement the validation, since the position is changed by the injected gcode after the decision is made in the gcode generator
You suggest we don't validate using the injected gcode position, but only validate using the gcode position from the "code", which contains invalid data since gocde is injected afterwards.

Please excuse me, but that does not make any sense to me.

In a nutshell, we need to ensure the injected filament change gcode ends in a safe, expected and consistent position (x=0, y=0) - just like other injected gcodes
- If we can't ensure this, the alternative is to validate/check the position before setting the Z hop type to spiral
- Implementing this validation/check is currently not possible, due to the injection happening after the Z hop type is selected.

The cause of the out of bounds move on #4631 is the unexpected and inconsistent filament change gcode position and validating it is very challenging since the layer processing is done when injections happen.

ziehmon · 2024-10-17T20:54:56Z

@tsmith35 may I ask for your opinion on this? I think you had a close look at the code too.

ziehmon · 2024-10-17T21:09:28Z

I am also willing to jump on a discord call or whatever you would prefer, myself and a lot of other people would like to see this fixed and are willing to contribute but we need to agree on an approach.

/cc @lanewei120

lanewei120 · 2024-10-18T01:45:57Z

I am also willing to jump on a discord call or whatever you would prefer, myself and a lot of other people would like to see this fixed and are willing to contribute but we need to agree on an approach.

/cc @lanewei120

@ziehmon could you send your discord account to our email ([email protected])
@XunZhangBambu will contact you by discord

XunZhangBambu · 2024-10-28T01:56:33Z

@XunZhangBambu thank you for your reply. I think we misunderstood each other, so let me summarize:

I raise this PR to change the injected filament change gcode to end in an safe, consistent and expected position (x=0, y=0)

You suggest we instead validate/check if the position is safe for a spiral Z hop, and disable it if not

I ask how to implement the validation, since the position is changed by the injected gcode after the decision is made in the gcode generator

You suggest we don't validate using the injected gcode position, but only validate using the gcode position from the "code", which contains invalid data since gocde is injected afterwards.

Please excuse me, but that does not make any sense to me.

In a nutshell, we need to ensure the injected filament change gcode ends in a safe, expected and consistent position (x=0, y=0) - just like other injected gcodes

If we can't ensure this, the alternative is to validate/check the position before setting the Z hop type to spiral

Implementing this validation/check is currently not possible, due to the injection happening after the Z hop type is selected.

The cause of the out of bounds move on #4631 is the unexpected and inconsistent filament change gcode position and validating it is very challenging since the layer processing is done when injections happen.

@ziehmon From my perspective, checking the inserted G-code still doesn't seem to solve this issue. Because in the slicing software, you are not certain whether the inserted G-code will definitely be executed, similar to timelapse G-code. I believe that by inserting a secure movement command (that cannot be skipped) at each unskippable part of such G-code, we can ensure that we obtain the correct position of the nozzle after executing the inserted G-code.

ziehmon · 2024-11-12T20:55:28Z

@XunZhangBambu thank you for your reply. I think we misunderstood each other, so let me summarize:

I raise this PR to change the injected filament change gcode to end in an safe, consistent and expected position (x=0, y=0)

You suggest we instead validate/check if the position is safe for a spiral Z hop, and disable it if not

I ask how to implement the validation, since the position is changed by the injected gcode after the decision is made in the gcode generator

You suggest we don't validate using the injected gcode position, but only validate using the gcode position from the "code", which contains invalid data since gocde is injected afterwards.

Please excuse me, but that does not make any sense to me.

In a nutshell, we need to ensure the injected filament change gcode ends in a safe, expected and consistent position (x=0, y=0) - just like other injected gcodes

If we can't ensure this, the alternative is to validate/check the position before setting the Z hop type to spiral

Implementing this validation/check is currently not possible, due to the injection happening after the Z hop type is selected.

The cause of the out of bounds move on #4631 is the unexpected and inconsistent filament change gcode position and validating it is very challenging since the layer processing is done when injections happen.

@ziehmon From my perspective, checking the inserted G-code still doesn't seem to solve this issue. Because in the slicing software, you are not certain whether the inserted G-code will definitely be executed, similar to timelapse G-code. I believe that by inserting a secure movement command (that cannot be skipped) at each unskippable part of such G-code, we can ensure that we obtain the correct position of the nozzle after executing the inserted G-code.

Hi @XunZhangBambu! Please excuse my late reply, I was on vacation. 🌤️

I really like your proposed solution. It is simple to implement, safe and maintainable. I will implement it in next days. We don't need a discord call then. Testing the change will probably be a lot of work but we have a great solution then.

Thanks for thinking about this again! :)

tsmith35 · 2024-11-12T20:58:31Z

I really like your proposed solution. It is simple to implement, safe and maintainable. I will implement it in next days. We don't need a discord call then. Testing the change will probably be a lot of work but we have a great solution then.

@ziehmon I have both an A1 (with AMS lite) and A1 mini (also with AMS lite), so that may help! Did you ever get any testing machines?

ziehmon · 2024-11-12T21:01:37Z

@tsmith35 you're faster than light.

I will definitely come back to your offer for testing with AMS Lite.

Unfortunately, I didn't receive an AMS Lite from Bambu, I assume it wasn't approved which is okay. It was a nice thought at least. Maybe they can test on their print farm though.

tsmith35 · 2024-11-13T01:58:27Z

@ziehmon I added a PR #5219 just a few minutes ago. It adds a "move to safe position" to each wipe instance during filament changes for each of the A1 and A1 mini 0.4 machine profiles. I don't see a downside to returning to X0 after each such wipe instance during filament changes, since not returning to X0 can have unexpected consequences. Will you take a look? Thanks.

tsmith35 · 2024-11-14T03:55:57Z

Simon is doing a world of good, so he is quite deserving of some test-bed equipment. I applaud you for assisting. It's a win-win. Tom

…

On Wed, Nov 13, 2024 at 7:41 PM SaltWei ***@***.***> wrote: I am also willing to jump on a discord call or whatever you would prefer, myself and a lot of other people would like to see this fixed and are willing to contribute but we need to agree on an approach. Hello, @ziehmon <https://github.com/ziehmon> Do you remember when I mentioned that I would help you with AMS Lite or a discount coupon? I sent you an email separately to discuss this matter, but it seems that your GitHub email is unable to receive emails. — Reply to this email directly, view it on GitHub <#4848 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB2UJVPKTS3R6HSKFYJ4AHL2AP5WDAVCNFSM6AAAAABOR4OQQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINZVGE3TQNBZGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

SaltWei · 2024-11-15T01:30:27Z

Hello @ziehmon . Is this your email ? [email protected].
We (Bambu Lab Marketing Team colleague and me) send a email to you but have no response yet. Could you please check your email and have a look?

ziehmon · 2024-11-18T10:20:19Z

Hello @ziehmon . Is this your email ? [email protected]. We (Bambu Lab Marketing Team colleague and me) send a email to you but have no response yet. Could you please check your email and have a look?

Hi @SaltWei! I received the email and just answered it, forgot to put you in CC. Thank you very much for reaching out! 🫶

SaltWei · 2024-11-18T11:27:50Z

Hello @ziehmon . Is this your email ? [email protected]. We (Bambu Lab Marketing Team colleague and me) send a email to you but have no response yet. Could you please check your email and have a look?

Hi @SaltWei! I received the email and just answered it, forgot to put you in CC. Thank you very much for reaching out! 🫶

My pleasure!

This was referenced Sep 20, 2024

X axis offset on multiple model when changing color with AMS lite #4841

Closed

Tsmith35 per BS PR4631 SoftFever/OrcaSlicer#6709

Merged

tsmith35 mentioned this pull request Sep 20, 2024

Revert "Tsmith35 per BS PR4631 (#6709)" SoftFever/OrcaSlicer#6845

Merged

lanewei120 requested a review from XunZhangBambu September 29, 2024 08:13

ziehmon mentioned this pull request Oct 17, 2024

Update filament_change_gcode to include upstream changes ziehmon/BambuStudio#1

Closed

ziehmon mentioned this pull request Nov 18, 2024

Added a move to safe position to A1 and A1 mini change filament gcode #5219

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Regression of #4631: support spiral lift with timelapse gcode and multi-color prints #4848

Regression of #4631: support spiral lift with timelapse gcode and multi-color prints #4848

ziehmon commented Sep 20, 2024 •

edited

Loading

XunZhangBambu commented Sep 20, 2024

tsmith35 commented Sep 20, 2024

ziehmon commented Sep 20, 2024

ziehmon commented Sep 20, 2024 •

edited

Loading

XunZhangBambu commented Sep 29, 2024

XunZhangBambu commented Sep 29, 2024

ziehmon commented Sep 29, 2024 •

edited

Loading

tsmith35 commented Sep 29, 2024

ziehmon commented Sep 29, 2024

ziehmon commented Oct 16, 2024

XunZhangBambu commented Oct 17, 2024

ziehmon commented Oct 17, 2024 •

edited

Loading

ziehmon commented Oct 17, 2024

ziehmon commented Oct 17, 2024

lanewei120 commented Oct 18, 2024 •

edited

Loading

XunZhangBambu commented Oct 28, 2024

ziehmon commented Nov 12, 2024

tsmith35 commented Nov 12, 2024

ziehmon commented Nov 12, 2024 •

edited

Loading

tsmith35 commented Nov 13, 2024 •

edited

Loading

tsmith35 commented Nov 14, 2024 via email

SaltWei commented Nov 15, 2024

ziehmon commented Nov 18, 2024

SaltWei commented Nov 18, 2024

Regression of #4631: support spiral lift with timelapse gcode and multi-color prints #4848

Are you sure you want to change the base?

Regression of #4631: support spiral lift with timelapse gcode and multi-color prints #4848

Conversation

ziehmon commented Sep 20, 2024 • edited Loading

Context

Cause

Solution

Implementation

Summary

Test-cases

Appendix

XunZhangBambu commented Sep 20, 2024

tsmith35 commented Sep 20, 2024

ziehmon commented Sep 20, 2024

ziehmon commented Sep 20, 2024 • edited Loading

XunZhangBambu commented Sep 29, 2024

XunZhangBambu commented Sep 29, 2024

ziehmon commented Sep 29, 2024 • edited Loading

tsmith35 commented Sep 29, 2024

ziehmon commented Sep 29, 2024

ziehmon commented Oct 16, 2024

XunZhangBambu commented Oct 17, 2024

ziehmon commented Oct 17, 2024 • edited Loading

ziehmon commented Oct 17, 2024

ziehmon commented Oct 17, 2024

lanewei120 commented Oct 18, 2024 • edited Loading

XunZhangBambu commented Oct 28, 2024

ziehmon commented Nov 12, 2024

tsmith35 commented Nov 12, 2024

ziehmon commented Nov 12, 2024 • edited Loading

tsmith35 commented Nov 13, 2024 • edited Loading

tsmith35 commented Nov 14, 2024 via email

SaltWei commented Nov 15, 2024

ziehmon commented Nov 18, 2024

SaltWei commented Nov 18, 2024

ziehmon commented Sep 20, 2024 •

edited

Loading

ziehmon commented Sep 20, 2024 •

edited

Loading

ziehmon commented Sep 29, 2024 •

edited

Loading

ziehmon commented Oct 17, 2024 •

edited

Loading

lanewei120 commented Oct 18, 2024 •

edited

Loading

ziehmon commented Nov 12, 2024 •

edited

Loading

tsmith35 commented Nov 13, 2024 •

edited

Loading