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

Improved filament unload (like original MMU2s) #20147

Merged

Conversation

niccoreyes
Copy link
Contributor

@niccoreyes niccoreyes commented Nov 15, 2020

Description

The original prusa firmware has parameters to make sure there is no filament when switching tools, if so, keep unloading until no filament is detected. I noticed this wasn't implemented in the mmu2.cpp of marlin 2.0.7.1 since it assumes no filament was ever loaded if a different tool was used or load to nozzle was used. It relies on the filament ramming parameters which can be unreliable for flexible materials because it could sometimes keep it loaded in the gears.

Benefits

The PRUSA_MMU2_S_MODE in marlin added benefits for Loading filament and responding to the IR trigger similar to the orignal MMU2s. I added a few more lines similar to the original for the MMU menu function -> load to nozzle which would do a filament ramming first, then keep unretracting if filament sensor is still loaded despite the ramming.

For the T# to a different tool, filament ramming is dependent on the slicer, however there are times where flexible filament may still get stuck in the gears and marlin still doesn't have the function to make sure it is unloaded before letting the MMU unretract with the U0 command.

Configurations

Added "//#define MMU_IR_UNLOAD_MOVE" in the Configuration_adv.h
I'm not sure if I should use my whole config for an SKR-Bear-Prusa, but I made a separate config for the bare minimum enabled settings to compile

Related Issues

One concern is that I used a "while" statement to keep unloading until filament sensor is untriggered, I'm still trying to figure out how the whole mmu cycle works in marlin, but it works well so far on my build.

I'm still not sure if I should keep the line
command(MMU_CMD_U0); //unloading from mmu side but not printer
in the tool_change function
commented out MMU_CMD_U0 in my recent commit for tool_change()
but this is necessary for load_to_nozzle()

@sjasonsmith
Copy link
Contributor

Can you describe the scenario where the previous implementation does not work well? I would like to understand how it might ruin print jobs or even damage hardware.

@sjasonsmith
Copy link
Contributor

The comment above this configuration block says:

  • Using a sensor like the MMU2S
  • This mode requires a MK3S extruder with a sensor at the extruder idler, like the MMU2S.

It seems everyone will have this sensor. Have you thought about changing behavior, without making it a configuration option?
Is there any reason an MMU2S user would NOT want this option? Maybe problems with clear filament?

@GMagician
Copy link
Contributor

GMagician commented Nov 16, 2020

AFAIK MK3S runout sensor works in a different position respect to normal.
Normal runout detect filament just before gear, so once detected it may be unloaded by printer.
MMU runout detect filament detect filament out of gear and printer can't unload it.

this cnf works only when mmu has its own filament runout detection. Extruder one is just used to detect filament into gears (properly loaded).

I think not all people have this cnf (old mmu or pre mk3s extruder)

@niccoreyes
Copy link
Contributor Author

niccoreyes commented Nov 16, 2020

sorry for my late reply, GMagician is right about the MK3S extruder, it's not dependent on filament color hence this will only work on an MK3S-Type extruder (inclusive of the other S variants) that's why I made it under the PRUSA_MMU2_S_MODE

  • additional info, the S variant was designed to improve MMU2 reliability by sensing filament loading directly into the gears instead of a runout sensor just being above the gears

However, I want to add the missed out function of the idler sensor during the UNLOADING not only loading with this line in the Prusa FW as my reference.

Capture

My implementation is different though. I somehow threw the printer into a "while" statement instead of using "machine states" (if I used the term correctly) in making sure its unloaded that a printer state of unloading until no filament has been detected. Concerns why this might fail on the hardware side is:

  1. what if the user hasn't calibrated the idler ir sensor (aka extruder ir of the gears) and the printer might get stuck in an unloading state. The watchdog timer might kick in and kill the printer midprint when the user could just pause and make necessary adjustments
  2. ^ therefore an advanced pause should be called instead of being stuck in the while loop if a certain unloading timeout is reached (proposed solution if ever but I haven't implemented in code)

Normally Prusa slicer automatically inserts gcode for filament unloading approximately outside the gears (approx -49.50mm of filament retraction as hardcoded as part of the wipe tower block) that's why it has a high probability of being unloaded by the time T0-T1/other numbers is called at this point in time FOR solid filament like PLA/ABS/PETG at the current state of Marlin's PRUSA_MMU2_S_MODE. However the e steps/extrusion multiplier required to unload flexible material may be different because the filament width changes depending on the tension pressed on it, elongation or compression along the path may change the steps necessary to fully unload the filament before the mmu Pulls it away. If the filament is indeed consistent, a remix of the MK3S extruder with a longer filament path length may also benefit with the printer side doing Filament checks during unload and guarantees that it is unloaded from the gears.

to summarize:
At the current state of MMU2S (S version to be specific) in marlin, it doesn't utilize the idler during the unloading to make sure that no filament is left. As a result, the MMU might pull the filament and grind or slide on it even if the material is still loaded in the gears which can easily be cross checked by the Printer's idler IR sensor before telling the MMU unit to pull it away

@niccoreyes
Copy link
Contributor Author

niccoreyes commented Nov 16, 2020

*more details from prusa firmware
details

@sjasonsmith sjasonsmith self-requested a review November 19, 2020 06:54
@sjasonsmith sjasonsmith self-assigned this Nov 19, 2020
@sjasonsmith sjasonsmith removed their request for review November 19, 2020 06:55
@sjasonsmith
Copy link
Contributor

sjasonsmith commented Nov 19, 2020

@niccoreyes I pushed a change to resolve merge conflicts with the other recent MMU changes. That unfortunately did not go as planned so I pushed it back to the original state. I'll push the merge/cleanup in a minute.

edit: actually that wasn't right either. Sorry about this, it will be straightened out soon.

@sjasonsmith
Copy link
Contributor

OK, so it actually had done what I wanted, but GitHub was slow to reflect it in the view and it mistakenly showed up as hundreds of files modified in the PR. A few moments later it was fine.

I've merged this with the other recent MMU changes and done some very minor cleanup (alignment, trailing spaces, comments).

Please make sure this is still working after all the other MMU changes in the past couple days. I will also post some additional comments on the code itself.

Copy link
Contributor

@sjasonsmith sjasonsmith left a comment

Choose a reason for hiding this comment

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

As I read the code more closes to understand how the MMU works, I had some more questions. Some of these answers may seem obvious to you, so thank you for taking the time to answer them for me.

#if ENABLED(MMU_IR_UNLOAD_MOVE)
if (FILAMENT_PRESENT()) {
DEBUG_ECHOLNPGM("Unloading\n");
//filament_ramming(); //unloading from printer side
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, filament_ramming is used when pulling filament from the nozzle, to produce a good shape to work with the MMU.

Why would you only do this when loading to nozzle, and not during tool change? It seems that tool change should have to unload filament from the nozzle also.

If it is ok to use filament_ramming in both cases, perhaps you could move this into MMU2::unload(). This also had the advantage that it would apply to the Unload command also.

Copy link
Contributor Author

@niccoreyes niccoreyes Nov 19, 2020

Choose a reason for hiding this comment

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

to explain this, there are two sources of ramming sequences for unloading

One is defined by the Slicer (at the moment only Prusa Slicer/Super Slicer) which is converted into Gcode before executing the T# command from an actual print
image

and the Second is the internal ramming sequence from the printer when using the LCD since there is no source of Gcode to instruct it
image

load filament to nozzle () is a function called only when operating the LCD screen but not related or called during any of the printer-to-mmu protocols. It can only rely on filament_ramming built in to form the tip but isn't ideal in an actual long print and less reliable with different types of materials that's why prusa slicer has filament ramming as part of the settings which varies per filament setting.

It's the tool_change() function that loads the filament to the gears during printing as called by the T# command. By principle, the tool_change() is a less instruction-full function for the printer since the preceding and succeeding instructions to the printer is generated by the slicer but it coordinates with the MMU unit as to tell which filament # the user wants at the moment. This is where the slicer-generated-filament ramming gcode is inserted before the T# command, and tool change loading command gcode after.

  • my addition to keep unloading will help guarantee that the filament is unloaded in the case that the slicer generated ramming and unload gcode is insufficient before telling the mmu unit to pull the filament and transition to another

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for this description. It may be worth adding come comments to the code describing some of these reasons. They may not be obvious to new people in the code why some unloads perform ramming and others do not.

DEBUG_ECHOLNPGM("Unloading\n");
//filament_ramming(); //unloading from printer side
ENABLE_AXIS_E0();
while (FILAMENT_PRESENT())
Copy link
Contributor

Choose a reason for hiding this comment

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

This and the other loop could continue forever if the filament is jammed. It should have a limit so that if the unload fails correct action can occur.

What is the correct action? I'm not sure...perhaps trigger filament runout?

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the manage_response probably exists for the purpose of pausing if something goes wrong. Your loops probably need to define a maximum retract distance. If that is exceeded you would call manage_response with the expected arguments to allow the user to clear the issue.

Copy link
Member

Choose a reason for hiding this comment

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

A maximum distance can be enforced for these loops using a simple countdown, based on whatever maximum length is determined for each case. I'll leave it to @niccoreyes to determine the appropriate limits.

@niccoreyes
Copy link
Contributor Author

niccoreyes commented Nov 19, 2020

added proposed cleanups, but I haven't made a solution for a way to fix the two while(FILAMENT_PRESENT()) sections at the moment

also I removed the MMU_CMD_U0 command at line 977 since I realized that the succeeding MMU_CMD_T0+index will coordinate with mmu the current extruder anyway on the mmu unit.

will do some tests tomorrow

@sjasonsmith
Copy link
Contributor

@niccoreyes what do you think would be the right response if filament can't unload? I can come up with a few options:

  1. Return an error in some way - probably useless because G-code would keep running

  2. Kill the printer - Prevents damage, but ruins the print. EASY to implement.

  3. Perform a filament runout pause...I'm not sure how hard that will be to implement, but seems like the best user experience.

I presume if you are using an MMU you always have "filament runout" support enabled? I have never actually used one myself.

@sjasonsmith
Copy link
Contributor

Regarding my last comment, you can probably use manage_response here. I've commented elsewhere about it.

@sjasonsmith
Copy link
Contributor

It seems like the infinite loop is the major outstanding issue. Please try using manage_response to resolve that. It will be great if you can fake a jam for testing purposes to make sure it works as intended.

@niccoreyes
Copy link
Contributor Author

niccoreyes commented Nov 23, 2020

Sorry for the late reply, lots of exams lately. I haven't exhausted testing the MMU function for the week yet. MMU prints do take a while
But I think calling the runout menu would be preferable

I haven't explored the runout menu functions and how it is called, but it can be activated with "runout sensor: off" from the configuration right?
I noticed that the MK3s type sensor triggers the runout sensor when printing TPU sometimes, because as the gears presses on it the filament can be so compliant to the pressure and the lever slightly moves enough not to be sensed by the IR sensor sometimes midprint. My prints with the mmu + PLA / PETG / ABS has been great with the "runout sensor" function turned on, but not for TPU. I tend to turn off "runout sensor: " from the lcd menu when doing MMU TPU prints and had more success, nevertheless the load to gears() and tool_change() function works ok since it reads the pin of the runout/filament sensor directly.

I'm assuming the mmu runout menu can be called independently from the whole "runout sensor" feature.

I might get back to coding a timeout function in the weekend, sorry for the delay 😥 I wanna do more tests just to be sure everything is good and user friendly.

@GMagician
Copy link
Contributor

I may be wrong but runout sensor should not work with mmu as usual. It's used only to be sure tha filament has been "taken" by gears or released by "gears". During print is pinda sensor that should detect filament runout, and that sensor is on mmu2

@sjasonsmith
Copy link
Contributor

I suggested using the manage_response function instead of trying to manually do a normal runout, because manage_response is how problems during existing load/unload commands are handled. This in some way requests feedback from the user to resolve the issue.

All I can do right now is point you in what I think is the right direction, since I don't actually use this hardware. If @GMagician has experience with the MMU code their input could really help.

@GMagician
Copy link
Contributor

If @GMagician has experience with the MMU code their input could really help.

sorry, no experience, it's just "AFAIK"

@thinkyhead thinkyhead changed the title Added better unload feature similar from the Original MMU2s Improved filament unload (like original MMU2s) Nov 26, 2020
@thinkyhead
Copy link
Member

Presumably there is no need for ENABLE_AXIS_E0 before a move, because the stepper ISR should automatically enable the E axis as needed. If that isn't happening and this won't work without them, then we might want to investigate.

@niccoreyes
Copy link
Contributor Author

niccoreyes commented Nov 27, 2020

Presumably there is no need for ENABLE_AXIS_E0 before a move, because the stepper ISR should automatically enable the E axis as needed. If that isn't happening and this won't work without them, then we might want to investigate.

I see, I guess can remove these later as I test, I paralleled the those sections from the non-mmu2s extruder. I don't really have any reason to keep it there if that were the case. I'll be working on testing these for the weekend.

I also added a fix for the DEBUG_ECHOLNPAIR near the unscaled mmu2 moves where it fails to compile
I think the compiler was searching for an initiated "fr_mm_m" variable that existed before yesterday's commits

@sjasonsmith
Copy link
Contributor

@niccoreyes are you intending to come back to finish up this PR?

Some of us would probably be willing to help work through remaining issues, but have no way to actually test a change.

@sjasonsmith sjasonsmith removed their assignment Jan 18, 2021
thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request Mar 5, 2021
@thinkyhead
Copy link
Member

I've merged the latest code into this and cleaned up extra calls to ENABLE_AXIS_E0. Now it's just waiting on the refinements discussed above to handle bad conditions safely and to prevent infinite looping. We're looking for MMU2 testers to fill in the details.

@JohnnyTheOne
Copy link
Contributor

JohnnyTheOne commented Mar 6, 2021

when compiling I get this error

Marlin\src\feature\mmu\mmu2.cpp: In static member function 'static bool MMU2::load_filament_to_nozzle(uint8_t)':
Marlin\src\feature\mmu\mmu2.cpp:924:5: error: 'filament_ramming' was not declared in this scope
  924 |     filament_ramming();                             // Unloading instructions from printer side when operating LCD

@GMagician
Copy link
Contributor

@JohnnyTheOne you may replace such line with:
execute_extruder_sequence((const E_Step *)ramming_sequence, sizeof(ramming_sequence) / sizeof(E_Step));
I think original function has been removed because it was just used once. I think now it should be readded with an inline attribute

thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request Feb 21, 2023
@thinkyhead
Copy link
Member

Does this represent an improvement in its current state, or does it need more fixing? I can merge the current state into bugfix-2.1.x for wider testing if it's reasonably functional. If it's a general improvement in its current state I can also include it in 2.1.3; if not it can be reserved for inclusion in 2.1.4.

@MarlinFirmware MarlinFirmware deleted a comment from GMagician Mar 3, 2023
@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 2 times, most recently from e90c213 to 4b9bb85 Compare March 7, 2023 05:17
@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 5 times, most recently from 27df113 to 8d31429 Compare March 25, 2023 04:25
@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 3 times, most recently from de391db to 0f34163 Compare April 12, 2023 05:14
@thinkyhead thinkyhead merged commit 001d1fd into MarlinFirmware:bugfix-2.1.x May 4, 2023
EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request May 8, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
thinkyhead pushed a commit that referenced this pull request May 13, 2023
oponyx pushed a commit to oponyx/Marlin that referenced this pull request May 14, 2023
oponyx pushed a commit to oponyx/Marlin that referenced this pull request May 14, 2023
EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request May 17, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request May 17, 2023
tspiva pushed a commit to tspiva/Marlin that referenced this pull request May 25, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
tspiva pushed a commit to tspiva/Marlin that referenced this pull request May 25, 2023
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 15, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 15, 2023
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 18, 2023
rerickson1 pushed a commit to rerickson1/Marlin that referenced this pull request Dec 16, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
rerickson1 pushed a commit to rerickson1/Marlin that referenced this pull request Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: Filament Sensor Needs: Testing Testing is needed for this change Needs: Work More work is needed PR: Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants