-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
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
Comments
To compensate extruder sign you may change |
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. 3.0 - (0.01 - 0.01 - 0.01 - 0.6)*n 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. |
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. |
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 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. |
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.
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. 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 |
What I can't understand is why you have, with G10/G11, a not ignored unretraction. Retract is done by: I'm not gone further to check "slicer retraction/unretraction" |
With no reason clear to me but maybe you can try with |
Ok, I added
And here is a fragment of the log file. Result is the same.
|
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 also added a "current position print" in fwretract: 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 |
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. |
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 |
looking at your log it seems that retract is failing, if so, planner.synchronize() should be written before 'destination=current' |
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.
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 |
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 |
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. |
This is log and gcode I used... |
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. |
I was able to replicate it. It has something to do with step or jerk. |
Ok. I think I could found it... MIN_STEPS_PER_SEGMENT. |
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. |
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. |
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. |
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. If you have any idea what logs can I get, or what should I test, I am ready to help. |
@justvlade last change to PR should fix it |
Closing since a PR has been created. Please move the discussion over there: #26067 |
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.
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.
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. |
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
https://www.thingiverse.com/thing:4408535/files
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
Configuration.h
andConfiguration_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
The text was updated successfully, but these errors were encountered: