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

[BUG] Incorrect filament runout motion sensor mm_countdown.runout counting which can produce false filament runout event or delay true runout event #26061

Closed
1 task done
justvlade opened this issue Jul 7, 2023 · 28 comments · Fixed by #26082

Comments

@justvlade
Copy link
Contributor

justvlade commented Jul 7, 2023

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

When using filament runout movement sensor, on printing of small parts, there is a change of false positive filament runout event. While motion sensor is not triggered as filament actually did no move even 1mm in total. As retract movement was not calculated correct, which made Marlin "think" that filament was moved enough already with no sensor reaction.

Bug Timeline

I installed runout sensor on 2.1.2.1 version, did not check earlier.

Expected behavior

No matter how many retraction/deretraction events are triggered only on actual forward movement should be triggered filament runout event.

Actual behavior

Runout event is triggered, while filament actually is not moved enough mm while printing.

Steps to Reproduce

  1. Download file Straight_Track_100mm.stl
    https://www.thingiverse.com/thing:4408535/files
  2. Compile Marlin Firmware with filament runout sensor support, filament change support and runout distance 3mm
  3. Use SuperSlicer with retraction on wipe, with retract 0.6mm, set "Retract amount before wipe 20%", "Minimal travel after retraction" to 0.1mm, set 0.08 or 0.04 layer height
  4. Slice and print model(I suggest skipping to 3.12mm or 4.64 Z height in gcode directly, also you can insert filament in feeder just it have a grip, so no actual filament used in printing, as only move is important)
  5. When it start to print bolts on rail, there will be a lot of retraction it will trigger filament change event.

Version of Marlin Firmware

2.1.2.1

Printer model

Ender 3

Electronics

Ender 2 Pro stock board (4.2.3)

Add-ons

Optical motion sensor (remix of 5710572 thing from thingeverse), BMG clone feeder, short-bowden direct, sensor is connected freely between spool and reverse bouden tube.

Bed Leveling

No Bed Leveling

Your Slicer

Other (explain below)

Host Software

SD Card (headless)

Don't forget to include

  • A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

My optical motion sensor react accuracy is near 1mm, I set FILAMENT_RUNOUT_DISTANCE_MM=3mm in settting.
Here is the issue. On some models with high number of small details and high retraction value, Marlin calculate some of the retractions and ignore some of them, and this can lead to runout, while 3mm actually was not moved. And as my sensor mostly 1mm accurate, you can assume that not only 3mm is not moved, but even 1mm was not really moved.
I checked runout.h file, to understand what really happens. Tried FILAMENT_RUNOUT_SENSOR_DEBUG const, but output not really informative. Only that mm_countdown.runout starting to decrease very fast while a lot of retractions.
So I checked source. The RunoutResponseDelayed.block_completed function ignores extruder movements when there is no x,y or z movement. But modern slicers like SuperSlicer can use wipe option, which means, that some of the retraction/deretraction is occured while moving and some without moving.
So I edited source, so I can see which E moves are ignored and which are not.

I will include my config (it is actually a default Ender 2 Pro config files, with minimal changes (like enabling runout sensor). I include config for bugfix release which I tested last.

I include runout.h file, which I changed to get my logs.
I include custom logs, which only include my custom info about which E moves were ignored and which were counted.
I also include gcode files I used to trigger the issue. I insert a tip of filament into BMG gears, and it takes 100mm to come to the nozzle from there, so it is enough to check gcode and not use any filament. So my gcode only use 30 degrees temperature.
retract_test_ps_464_fw.gcode - is prusa slicer 2.6 version, from 4.64 height. It uses FW retraction(I thought that it will help, but no it actually isn't). Corresponding log for it is report_bugfix_fwretract.txt. We see that only retract is counting and deretract is ignored, so on retraction events mm_countdown.runout start to grow, which is wrong, and while it will not trigger false positive, it will trigger false negative. You can just took out filament, and system will ignore that runout motion sensor is not triggered, as mm_countdown.runout is larger and larger. If there are a lot of retractions system will not stop at all, while printing them.

The rest two are SuperSlicer 2.4 gcode files. Those two with slicer retract and wiping. Also I tried firmware retract with no luck.
retract_test_ss_312_wipe_r06.gcode and report_bugfix_runout.txt this is only the end of log file, not from the begining, as Pronterface cut the rest. We see that runout distance become smaller and smaller, because some of retract/deretract movement is ignored, and finally a runout event triggered when mm_countdown.runout become negative.

retract_test_ss_464_wipe_r06.gcode and report_bugfix_overflow.txt in this situation is on the contrary, mm_countdown.runout starts growing, which is not correct too. Those are part of the same model, just different height.

Actually I tried disable the "if" part in RunoutResponseDelayed.block_completed function:
if (b->steps.x || b->steps.y || b->steps.z || did_pause_print) {
and it really solved the issue. After that all E movement starts to count and no false runout events triggered. Also it is seen in logs, that mm_countdown.runout show correct numbers. So actual runout state when filament is stuck or ended was triggered correctly.

I guess also this workaround should probably include RunoutResponseDelayed.filament_present function modification there should be added the "if" with such condition:
if (mm_countdown.runout[extruder] < runout_distance_mm) {

So mm_countdown.runout will not reset if it is already larger (due to adding retraction movement).

I guess this is needed to prevent resetting on random sensor reaction, for bouden systems with large retraction and short runout distance. Actually runout motion sensor should not trigger on backward movement, but it may because of shaking or for other reason.

I am not sure though if it is appropriate to use this code modification. For some reason there is check for xyz movement.

config+logs+gcode.zip

@GMagician
Copy link
Contributor

To compensate extruder sign you may change
const float mm = (b->direction_bits.e ? steps : -steps) * planner.mm_per_step[E_AXIS_N(e)];
to
const float mm = -steps * planner.mm_per_step[E_AXIS_N(e)];
in runout.h line 406. But if I'm not wrong looking at the code, when "motion is detected", runout value should be reset. Why, in you case, motion is not detected?

@justvlade
Copy link
Contributor Author

justvlade commented Jul 8, 2023

To compensate extruder sign you may change const float mm = (b->direction_bits.e ? steps : -steps) * planner.mm_per_step[E_AXIS_N(e)]; to const float mm = -steps * planner.mm_per_step[E_AXIS_N(e)]; in runout.h line 406. But if I'm not wrong looking at the code, when "motion is detected", runout value should be reset. Why, in you case, motion is not detected?

There is no trouble with extruder motion direction value it is correct. The trouble is that in some cases only one direction is taken into account. In one case only retract motion is added (well actually substracted). In other case only deretraction is used.

Motion is not detected because there is no motion(actually very few motion, but sensor cannot detect that). Let me try to explain this.

Extuder make small amount moves, like 1 step or so. This leads to series of small like 0.01mm moves. As you understand there should be 300 moves of 0.01mm to move 3mm of filament through feeder.

But those small moves are on different small islands of the model. So when moving between them there should be retract and deretract motion. For example retract length is 0.6mm. And if only retract or deretract motion is taken into account we have wrong calculated movement of runout length.

So we have 3mm left. Then we substract 0.01,0.01,0.01. So we have 2.97 left. Then we need to move to another part of the model. So we made retract 0.06mm. This leads in 2.97+0.6 = 4.57. So logically when we finish our movement, we should make deretraction. So 4.57-0.6 = 2.97mm. And this is correct value. BUT in some cases Marlin only use retract and in some only deretract.
So we either add 0.6 mm and not substract it. Or the opposite. Which may lead to this:

3.0 - (0.01 - 0.01 - 0.01 - 0.6)*n
Where n is number of small items. If n>5 obviously we have 0.6*6=3.6 substracted from initial 3.0mm distance. Which was substracted, because Marlin 'forgot' to compensate all those movements by retraction distance. And why sensor is not triggered? Lets calculate how actually filament moved (0.01+0.01+0.01)*6=0.18mm not even 1mm. All in all we have -0.78mm. Which actually means a runout event? Right? Wrong! Filament only moved 0.18mm. And optical sensor with 1mm accuracy cannot detect that.

Also there may be opposite scenario, when only retraction movement is added, which results with n=6 in my example will lead to filament left distance to be 6.78. Which means that even if there is no filament in feeder and we only make small moves followed by retraction it will never detect that filament is out. As filament left distance is only larger and larger.

If it is not clear what I mean, please tell me, I will try explain some other way.

@GMagician
Copy link
Contributor

runout mm are updated by stepper isr so Marlin shouldn't miss any step done by extruder (stepper.cpp line 2296). It sound weird to me that sometime it misses retraction or unretraction.
What do you get with FILAMENT_RUNOUT_SENSOR_DEBUG enabled?

@justvlade
Copy link
Contributor Author

justvlade commented Jul 8, 2023

runout mm are updated by stepper isr so Marlin shouldn't miss any step done by extruder (stepper.cpp line 2296). It sound weird to me that sometime it misses retraction or unretraction. What do you get with FILAMENT_RUNOUT_SENSOR_DEBUG enabled?

When I enable FILAMENT_RUNOUT_SENSOR_DEBUG I see that runout mm is drastically decreased or drastically increased, depends on gcode which I ran. There is no usefull information there, only runout mm value, but not the cause of it became negative or it became too large. So I used modified code to see which retraction or deretraction is skipped.(I included this in first post, but can to try describe it again if it is not clear what did what).

Marlin is not actually skipping, it ignores extruder movement if the following condition is not true
if (b->steps.x || b->steps.y || b->steps.z || did_pause_print) {

In other words it ignores E move if there is no X, Y or Z move. Which means, I guess, that it should ignore retraction and deretraction, both. But for some reason, in different moments one of E moves (retraction or deretraction) comes with (X,Y or Z) move combined. Of course if we use SuperSlicer nozzle wipe feature, we will have some retraction moves with X or/and Y moves, which already impacts on this event. BUT also what is interesting, that even if I switch to firmware retraction, when there obviously can not be any movement while retracting, as retract and deretract are managed with separate commands, the issue is still persists. One of the movements (retract or deretract) still comes with X,Y or Z movement and other without.

Actually I did not check which of those (b->steps.x || b->steps.y || b->steps.z || did_pause_print) is true when retract or deretract counts.

As I already written earlier, if I disable this condition (if (b->steps.x || b->steps.y || b->steps.z || did_pause_print) {) all of retract and deretract events start to count, and runout mm is always calculates correctly. But I am not sure, why this condition was added, and is it correct to remove it.

@justvlade
Copy link
Contributor Author

I created more logs with adding x, y, z steps information, to understand what happens when retraction/deretraction counts. There are two situations with Prusa Slicer firmware 0.4mm retract and with Super Slicer 0.6mm wipe retract. Both gcodes are taken from first post (retract_test_ps_464_fw.gcode and retract_test_ss_312_wipe_r06.gcode).

Each log have two version, only custom log info, the amount of E move, runout mm left, and x,y,z movement, or if E move ignore then obviously all steps moves are zero so they are not echoed.

The following code in runout.h was used for custom debug info.

static void block_completed(const block_t * const b) {
        const uint8_t e = b->extruder;
        const int32_t steps = b->steps.e;
        const float mm = (b->direction_bits.e ? steps : -steps) * planner.mm_per_step[E_AXIS_N(e)];
        if (b->steps.x || b->steps.y || b->steps.z || did_pause_print) { // Allow pause purge move to re-trigger runout state
        // if (b->steps.e) { // Allow pause purge move to re-trigger runout state
          // Only trigger on extrusion with XYZ movement to allow filament change and retract/recover.
          if (e < NUM_RUNOUT_SENSORS) mm_countdown.runout[e] -= mm;
          #if ENABLED(FILAMENT_SWITCH_AND_MOTION)
            if (e < NUM_MOTION_SENSORS) mm_countdown.motion[e] -= mm;
          #endif
          if (mm) {
            SERIAL_ECHO(F("E move mm: "), mm);
            SERIAL_ECHO(F("; left mm: "), mm_countdown.runout[e]);
            SERIAL_ECHO(F("; steps x: "), b->steps.x);
            SERIAL_ECHO(F("; steps y: "), b->steps.y);
            SERIAL_ECHO(F("; steps z: "), b->steps.z);

            SERIAL_EOL();
          }
        } else {
          if (mm) {
            SERIAL_ECHO(F("E ignore move mm: "), mm);
            SERIAL_ECHO(F("; left mm: "), mm_countdown.runout[e]);
            SERIAL_EOL();
          }
        }
      }

The other version of log includes GCODE commands output. Those files have less commands all in all, as pronterface limits buffer, so with more output of GCODE we have less movement info from block_completed.

As far as I see, with firmware retraction retract movement is taken into account when it have steps.x = 5.
While with slicer retract deretraction movement is taken into account with different values of steps.x and steps.y.

Here is my DEFAULT_AXIS_STEPS_PER_UNIT { 80, 80, 400, 411.3 } those are default for Ender 2/3 printers, except feeder steps which is a BMG clone. Full config also attached in zip file of the first post.

Actually I do not think that Marlin code interpreter is to blame, but this line 402 if (b->steps.x || b->steps.y || b->steps.z || did_pause_print) { condition should be revised, as it is a deprecated thinking that retract always made while printer head is in still position.

custom logs with xyz movement 2.zip

@GMagician
Copy link
Contributor

GMagician commented Jul 9, 2023

What I can't understand is why you have, with G10/G11, a not ignored unretraction.
G10 should be:
E ignore move mm: 0.40; left mm: 4.50
but I'm expecting a
E ignore move mm: -0.40; left mm: 4.50
on G11
both G10 and G11 don't involve X or Y movements

Retract is done by:
current_retract[active_extruder] = base_retract;
in FWRetract::retract
and unretraction is done by
current_retract[active_extruder] = 0;
in same method.
This variable is used to fake E destination, both G10 and G11 do a move to E0 (current E position for fairness) but planner receives E-0.4 by former and E0 by latter. This is why I'm not expecting a xstep=5 when E moves to unretract

I'm not gone further to check "slicer retraction/unretraction"

@GMagician
Copy link
Contributor

With no reason clear to me but maybe you can try with planner.synchronize(); in fwretract.cpp line172

@justvlade
Copy link
Contributor Author

With no reason clear to me but maybe you can try with planner.synchronize(); in fwretract.cpp line172

Ok, I added planner.synchronize(); as you suggested to 172 line. Here is code fragment. I even added debug message, to be sure.

  // Recover E, set_current_to_destination
    prepare_internal_move_to_destination(
      (swapping ? settings.swap_retract_recover_feedrate_mm_s : settings.retract_recover_feedrate_mm_s)
      * TERN1(RETRACT_SYNC_MIXING, (MIXING_STEPPERS))
    );
  }
  planner.synchronize(); SERIAL_ECHOLNPGM("planner.synchronize()");
  TERN_(RETRACT_SYNC_MIXING, mixer.T(old_mixing_tool));   // Restore original mixing tool

  retracted.set(active_extruder, retracting);             // Active extruder now retracted / recovered

  // If swap retract/recover update the retracted_swap flag too
  #if HAS_MULTI_EXTRUDER

And here is a fragment of the log file. Result is the same.

echo:M204 P1000
echo:G1 F1102
echo:G1 X46.258 Y34.724 E.01544
echo:G1 X46.206 Y34.724 E.01618
echo:G1 X46.18 Y34.679 E.01692
echo:G1 X46.206 Y34.634 E.01766
echo:G1 X46.258 Y34.634 E.0184
echo:M204 P1500
echo:G10
E move mm: 0.00; left mm: 6.78; steps x: 48; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 6.78; steps x: 0; steps y: 48; steps z: 0
E move mm: 0.00; left mm: 6.78; steps x: 48; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 6.77; steps x: 0; steps y: 45; steps z: 0
E move mm: 0.00; left mm: 6.77; steps x: 7; steps y: 4; steps z: 0
E move mm: -0.40; left mm: 7.17; steps x: 5; steps y: 0; steps z: 0
planner.synchronize()
echo:G92 E0
echo:G1 X46.258 Y34.279 F12000
echo:G1 X51.932 Y34.279
echo:M105
echo:G1 X51.932 Y34.379
echo:G11
E ignore move mm: 0.40; left mm: 7.17
planner.synchronize()
echo:G92 E0
echo:M204 P1000
echo:G1 F1102
echo:G1 X52.532 Y34.379 E.00372
echo:G1 X52.532 Y34.979 E.00744
echo:G1 X51.932 Y34.979 E.01116
echo:G1 X51.932 Y34.409 E.0147
echo:M204 P1500
echo:G1 X52.125 Y34.431 F12000
echo:G1 X52.284 Y34.679
echo:M204 P1000
echo:G1 F1102
echo:G1 X52.258 Y34.724 E.01544
echo:G1 X52.206 Y34.724 E.01618
echo:G1 X52.18 Y34.679 E.01692
echo:G1 X52.206 Y34.634 E.01766
echo:G1 X52.258 Y34.634 E.0184
echo:M204 P1500
echo:G10
E move mm: 0.00; left mm: 7.16; steps x: 48; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 7.16; steps x: 0; steps y: 48; steps z: 0
E move mm: 0.00; left mm: 7.16; steps x: 48; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 7.16; steps x: 0; steps y: 45; steps z: 0
E move mm: 0.00; left mm: 7.15; steps x: 7; steps y: 4; steps z: 0
E move mm: -0.40; left mm: 7.55; steps x: 5; steps y: 0; steps z: 0
planner.synchronize()
echo:G92 E0
echo:G1 X52.258 Y34.279 F12000
echo:G1 X51.946 Y50.113
echo:G1 X51.832 Y50.912
echo:G1 X51.932 Y50.912
echo:G1 X51.932 Y50.813
echo:G11
E ignore move mm: 0.40; left mm: 7.55
planner.synchronize()
echo:G92 E0
echo:M204 P1000
echo:G1 F1102
echo:G1 X51.932 Y50.213 E.00372
echo:G1 X52.532 Y50.213 E.00744
echo:G1 X52.532 Y50.813 E.01116
echo:G1 X51.962 Y50.813 E.0147
echo:M204 P1500
echo:G1 X51.984 Y50.619 F12000
echo:G1 X52.284 Y50.513
echo:M204 P1000
echo:G1 F1102
echo:G1 X52.258 Y50.558 E.01544
echo:G1 X52.206 Y50.558 E.01618
echo:G1 X52.18 Y50.513 E.01692
echo:G1 X52.206 Y50.467 E.01767
echo:G1 X52.258 Y50.467 E.01841
echo:M204 P1500
echo:G10
E move mm: 0.00; left mm: 7.55; steps x: 0; steps y: 48; steps z: 0
E move mm: 0.00; left mm: 7.54; steps x: 48; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 7.54; steps x: 0; steps y: 48; steps z: 0
E move mm: 0.00; left mm: 7.54; steps x: 46; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 7.53; steps x: 7; steps y: 4; steps z: 0
E move mm: -0.40; left mm: 7.93; steps x: 5; steps y: 0; steps z: 0
planner.synchronize()
echo:G92 E0
echo:G1 X52.258 Y50.113 F12000
echo:G1 X46.632 Y50.774
echo:G1 X46.632 Y50.912
echo:G1 X45.932 Y50.912
echo:G1 X45.932 Y50.813
echo:G11
E ignore move mm: 0.40; left mm: 7.93
planner.synchronize()
echo:G92 E0
echo:M204 P1000
echo:G1 F1102
echo:G1 X45.932 Y50.213 E.00372
echo:G1 X46.532 Y50.213 E.00744
echo:G1 X46.532 Y50.813 E.01116
echo:G1 X45.962 Y50.813 E.0147
echo:M204 P1500
echo:G1 X45.984 Y50.619 F12000
echo:G1 X46.284 Y50.513
echo:M204 P1000
echo:G1 F1102
echo:G1 X46.258 Y50.558 E.01544
echo:G1 X46.206 Y50.558 E.01618
echo:G1 X46.18 Y50.513 E.01692
echo:G1 X46.206 Y50.467 E.01767
echo:G1 X46.258 Y50.467 E.01841
echo:M204 P1500
echo:G10
E move mm: 0.00; left mm: 7.93; steps x: 0; steps y: 48; steps z: 0
E move mm: 0.00; left mm: 7.93; steps x: 48; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 7.92; steps x: 0; steps y: 48; steps z: 0
E move mm: 0.00; left mm: 7.92; steps x: 46; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 7.92; steps x: 7; steps y: 4; steps z: 0
E move mm: -0.40; left mm: 8.31; steps x: 5; steps y: 0; steps z: 0
planner.synchronize()
echo:G92 E0
echo:G1 X46.258 Y50.113 F12000
echo:G1 X51.932 Y66.746
echo:G1 X51.932 Y66.646

@GMagician
Copy link
Contributor

I tested your 'retract_test_ps_464_fw' (zip on first post) and you debug prints with my printer and all -0,40 / 0,40 movements are "ignored". I can't test you config but I should get what you get...
I'm however using bugfix

@GMagician
Copy link
Contributor

I also added a "current position print" in fwretract:
just below:
destination = current_position;
I added

  SERIAL_ECHOLNPGM("current_position.x ", current_position.x);
  SERIAL_ECHOLNPGM("current_position.y ", current_position.y);
  SERIAL_ECHOLNPGM("current_position.z ", current_position.z);
  SERIAL_ECHOLNPGM("current_position.e ", current_position.e);

too see where current position is when G10/G11 is executed

@justvlade
Copy link
Contributor Author

I tested your 'retract_test_ps_464_fw' (zip on first post) and you debug prints with my printer and all -0,40 / 0,40 movements are "ignored". I can't test you config but I should get what you get... I'm however using bugfix

I am using a recent bugfix too. Can you provide your config, so I can check parameters you are using? May be I will try to make my config as close as it is possible to check if it helps? I created specially as default config as possible, only minimal changing to enable runout sensors and fwretract.

@GMagician
Copy link
Contributor

I already did some check (I don't have motion runout but it should not change what block_completed sees) but mostly is like yours
Configuration_adv.zip

@GMagician
Copy link
Contributor

looking at your log it seems that retract is failing, if so, planner.synchronize() should be written before 'destination=current'

@justvlade
Copy link
Contributor Author

I have tried to make my config as close as possible, and checked. As I do not have switch endstop, I set FILAMENT_RUNOUT_DISTANCE_MM to 30mm. So from the start printer thinks that filament started to runout. And here is the beginning of my custom log.

Connecting...
echo:Unknown command: ""
Printer is now online.
echo:Now fresh file: retrac~2.gco
File opened: retrac~2.gco Size: 111033
File selected
//action:resume
E move mm: 10.00; left mm: 20.00; steps x: 0; steps y: 10400; steps z: 0
E move mm: 10.00; left mm: 10.00; steps x: 0; steps y: 10400; steps z: 0
E ignore move mm: -0.40; left mm: 10.00
E ignore move mm: 0.40; left mm: 10.00
E move mm: 0.00; left mm: 10.00; steps x: 48; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 9.99; steps x: 0; steps y: 48; steps z: 0
E move mm: 0.00; left mm: 9.99; steps x: 48; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 9.99; steps x: 0; steps y: 46; steps z: 0
E ignore move mm: -0.40; left mm: 9.99
E ignore move mm: 0.40; left mm: 9.99
E move mm: 0.00; left mm: 9.98; steps x: 0; steps y: 48; steps z: 0
E move mm: 0.00; left mm: 9.98; steps x: 48; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 9.97; steps x: 0; steps y: 48; steps z: 0
E move mm: 0.00; left mm: 9.97; steps x: 45; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 9.97; steps x: 7; steps y: 4; steps z: 0
E move mm: -0.40; left mm: 10.37; steps x: 5; steps y: 0; steps z: 0
E ignore move mm: 0.40; left mm: 10.37
E move mm: 0.00; left mm: 10.36; steps x: 0; steps y: 48; steps z: 0
E move mm: 0.00; left mm: 10.36; steps x: 48; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 10.35; steps x: 0; steps y: 48; steps z: 0
E move mm: 0.00; left mm: 10.35; steps x: 46; steps y: 0; steps z: 0
E move mm: 0.00; left mm: 10.35; steps x: 7; steps y: 4; steps z: 0
E move mm: -0.40; left mm: 10.75; steps x: 5; steps y: 0; steps z: 0
E ignore move mm: 0.40; left mm: 10.75

As you see at first it ignores both E retract moves, but only at the beginning, after that it is as usual.

I will try to write planner.synchronize() before destination=current now.

@GMagician
Copy link
Contributor

I restored my printer cnf but when I printed your gcode I printed it all and all -0.4/0.4 E movements where ignored...that's really weird

@GMagician
Copy link
Contributor

can you shrink gcode to the minimum. Only reason I can think is I missed somehow a movement "not" ignored

@justvlade
Copy link
Contributor Author

can you shrink gcode to the minimum. Only reason I can think is I missed somehow a movement "not" ignored

Well, actually it happens to me just from the beginning.

@GMagician
Copy link
Contributor

GMagician commented Jul 9, 2023

log.zip

This is log and gcode I used...
I'm using today bugfix

@justvlade
Copy link
Contributor Author

I will try to debug fwretract code. And will check on stock E3 board, which I also have. Though it have F303 GD chip which is not officially supported.

@GMagician
Copy link
Contributor

I was able to replicate it. It has something to do with step or jerk.

@GMagician
Copy link
Contributor

Ok. I think I could found it... MIN_STEPS_PER_SEGMENT.

@GMagician
Copy link
Contributor

This is a complex thing...MIN_STEPS_PER_SEGMENT may prevent a move but it seems that pending steps are added to next movement. Problem arise when next movement is a retract movement and previously missed steps are added to this. This "add" turns a simple "move only E" to move E and other axes then runout logic act on it...Maybe G10/G11 may be fixed by a planner.sync but "slicer" generated retractions may not be solved this way.
A solution should be resplit movements when added to planner if new movement is a "only E" but not sure if is correct to add more time consuming operation for a corner case situation. Maybe thinkyhead may say his option on this

@justvlade
Copy link
Contributor Author

justvlade commented Jul 9, 2023

Wow! Thanks for great work!

I would like to mention again my idea, may be leave planner work be, but revise runout logic. Is it really a bad idea to take in account every retract and deretract? The only trouble I see is that when retract is large, like on a bowden system, like 6mm. If we add 6mm to 3mm runout limit, we will have 9mm in total and if before deretract somehow happens runout sensor event, this may lead to reset runout limit to 3mm and after deretract it will become negative. But we can add condition, to reset runout limit only if it is smaller then 3mm, so to keep retract distance intact. But this is really a noob idea, as I am not fully understand how Marlin code works.

@GMagician
Copy link
Contributor

Dunno. planner.sync will not work because it will only purge queue but in this case the problem is not the queue pending blocks but that 'planner.position' is not where it should be. For the short search I did it seems there is no way to execute missing steps before adding the E retract steps.

@GMagician GMagician mentioned this issue Jul 10, 2023
@justvlade
Copy link
Contributor Author

I checked this patch (used this repository https://github.com/GMagician/Marlin/tree/fix-runout as I understand it have this exact patch). And can not detect any difference. I am attaching logs from my bugfix firmware which I used before in this conversation, with my minimal config and this config with fix-runout branch. To replicate the same situation I did some 'invention'. This also gave me an idea how you can test this without movement sensor. I moved sensor wheel with hand while first 'wipe out' moves of the gcode. When second move is in the 'middle' of it, I released the wheel and never touched since. I had to do this, because wheel turning would cause filament runout variable to reset in different times.
So I guess you can just click filament runout switch a little, while it is running first moves, then leave it be, and watch. But I guess it gave no difference anyway.

If you have any idea what logs can I get, or what should I test, I am ready to help.

patch-logs.zip

@GMagician
Copy link
Contributor

@justvlade last change to PR should fix it

@thisiskeithb
Copy link
Member

Closing since a PR has been created. Please move the discussion over there: #26067

@thisiskeithb thisiskeithb linked a pull request Jul 15, 2023 that will close this issue
justvlade added a commit to justvlade/Marlin that referenced this issue Jul 16, 2023
This should be considered as a draft idea, as there are some ideological contradictions in changing fundamental idea of runout. And needs approving that it does not harm other runout algorithms. This needs proper discussion with its pros and cons. I guess there is a reason(was not able to learn it though), why initial code excludes retraction/unretraction movement (or at least tries to do so).

Initial idea of fixing issue MarlinFirmware#26061

Unlike patch MarlinFirmware#26082 this idea have alternative to conservative view. Instead of trying to figure out which E move is retract and which not, we take into account any E move, as those are connected to physical filament movement. All in all forward and backward movement will compensate itself. 

Modern slicers often introduce new approach to retraction movement. For example Super Slicer allows to enable retraction wipe, which makes retraction during horizontal movement. Also there is an old idea of filament unretract compensation, when unretract is slightly larger then retract movement. Not all printer needed this fix, but if we ignore retract/unretract we will loose those compensation move as well.

This idea was inspired LCD_SHOW_E_TOTAL function.
I do not actually checked the code of filament summary feed calculation, but from user experience LCD_SHOW_E_TOTAL shows actual filament movement with retraction and unretraction, as it may become smaller while printing. And as I understand it is considered to be precise.
justvlade added a commit to justvlade/Marlin that referenced this issue Jul 19, 2023
This patch propose idea to take into account retract and unretract motion while computing runout left distance.

Initial idea of fixing issue MarlinFirmware#26061

Unlike patch MarlinFirmware#26082 this code works not only for firmware retract, or classic retract, but also for software wipe retracts, when retract done with horizontal motion.

Modern slicers often introduce new approach to retraction movement. For example Super Slicer allows to enable retraction wipe, which makes retraction during horizontal movement. Also there is an old idea of filament unretract compensation, when unretract is slightly larger then retract movement. Not all printer needed this fix, but if we ignore retract/unretract we will loose those compensation move as well.
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants