search

prusa3d / Prusa-Firmware-Buddy

Firmware for the Original Prusa MINI, Original Prusa MK4 and the Original Prusa XL 3D printers by Prusa Research.
Other
1.16k stars 226 forks source link

[BUG] M104.1 immediately sets the temp of future M104 #4116

Open Ro3Deee opened 2 months ago

Ro3Deee commented 2 months ago

Printer model

XL

Firmware version

6.0.4

Upgrades and modifications

No response

Printing from...

USB

Describe the bug

In my example, I have two tools ( T0 and T1). T1 is purged first, T0 second. A very strange effect (probably by fw planner) I found out (watching the display of my Prusa XL) that issuing a M104.1 on current tool has the effect of immediately setting the tool to the temperature that will be set in the future (but not the one given as its S parameter). In the picture bellow (also video below), M104.1 at line 489 (sets a temp of 225) has the effect of immediately setting T1 temperature to 199, as set by M104 at line 500: image

Video

https://youtu.be/8H7SEEuuDPg?feature=shared .

How to reproduce

;
; purge second tool
;
G1 F24000
P0 S1 L2 D0; park the tool
M109 T1 S230
T1 S1 L0 D0; pick the tool
G92 E0 ; reset extruder position

G0 X150 Y-7 Z10 F24000 ; move close to the sheet's edge
M104.1 T1 P36 S225
G0 E10 X140 Z0.2 F50 ; purge while moving towards the sheet
G0 X110 E9 F80 ; continue purging and wipe the nozzle
M73 P28 R23
G0 X107 Z0.05 F80 ; wipe, move close to the bed
G0 X104 Z0.2 F80 ; wipe, move quickly away from the bed
M73 P26 R23
G1 E-1.2 F2400 ; retract
 ; update slicer internal retract variable
G92 E0 ; reset extruder position

M104 S199 T1

from the attached g-code, sets the temp to 199 before purging the tool. See the video also

Expected behavior

the temp should be set to 199 after purging

Files

G-Code: 5cube_0.2mm_PLA_32m17s_noHeatQforT1.zip

Ro3Deee commented 2 months ago

this seems to be a known issue of prusa: Future gcodes are run as soon they hit the buffer https://www.youtube.com/clip/UgkxMqEZ6LTNr0QP2wWBl5PwlV-5Lx0BPWGi

Ro3Deee commented 2 months ago

related to this: https://github.com/MarlinFirmware/Marlin/issues/23239

Allowing temperature commands to execute early can cause a lot of issues, especially with cooldowns at the end of the print, causing underextrusion and even jamming, because the hot end starts to cool off before the print is even complete. https://github.com/MarlinFirmware/Marlin/issues/23239#issuecomment-985334895

the workaround is to use M400 command before M104 https://reprap.org/wiki/G-code#M400:_Wait_for_current_moves_to_finish

bkerler commented 2 months ago

Please don't reopen the same issue. Please close this issue in favor of #4115.

Ro3Deee commented 2 months ago

@bkerler , it is not the same issue as #4115 (M104.1 should not default to M104 for current tool). This issue is that currently M104.1 immediately sets the temp of future M104 . Of course, that fixing #4117 (really implement M104.1) resolves this bug also, but, if #4117 can't be resolved now, the current issue should be addressed.

I think this bug is caused by the fact that now M104.1 defaults to M104, which is executed immediately when it enters the buffer

Ro3Deee commented 2 months ago

edited the prev. comment. There are 3 different issues related to M104.1:

They are related, but not the same bug.

bkerler commented 2 months ago

@petrubecheru Thanks for pointing that out, however for me it seems to be the same issue, as P and Q aren't implemented and thus no timing can be handled. It would be better if you point out all three relations in one issue instead.

Ro3Deee commented 2 months ago

In my opinion even M104 (without subcde .1) should NOT be executed immediately when enters the buffer, before other instructions. specially for the current tool !

Ro3Deee commented 2 months ago

It would be better if you point out all three relations in one issue instead.

@bkerler , #4117 is the "master" bug. I will monitor each one and close them when resolved.

I am in no position to do bug management and prioritisation for Prusa. I reported the 3 different problems (some are an easy fix like M104.1 for current tool, some are more difficult like the one that executes M104 on buffer entry before other instructions). I search and provided the commits that introduced M104.1 in firmware and separately in PrusaSlicer and OrcaSlicer:

bkerler commented 2 months ago

yeah, I see @petrubecheru. I wrote a PR now and I'll keep in touch with the devs if the PR matches the expected behaviour or maybe they already have developed something internally already.

While I do see that #4117 is the "master" bug, it's still better to stick to the oldest issue #4115, close #4116 and #4117 and combine all three issues in #4115, as they are all related to the gcode M104.1 problem to prevent further confusion.

@danopernis Can you please keep track of these issues in order to line up with the PrusaSlicer team ?

Ro3Deee commented 2 months ago

commented on PR https://github.com/prusa3d/Prusa-Firmware-Buddy/pull/4143 :

PrusaSlicer puts both P and Q params in G-Code: P - - time in seconds till the temperature S is required (in standard mode) Q - - time in seconds till the temperature S is required (in stealth mode)

So the PR is incomplete. My proposal pseudo code:

      millis_t dwell_ms = 0;
        if (parser.seen('Q')) {//default Q because is longer
            dwell_ms = parser.ulongval('Q');
        } 
       if ( !in_stealth_mode && parser.seen('P')) {//if _printer NOT in stealth_ and has P param
            dwell_ms = parser.ulongval('P');
        }
bkerler commented 2 months ago

Actually the PR assumed that the temperature is set after some amount of time. But in fact, in order to reach the temperature until a specific period of time isn't anything that can be implemented using the gcode but instead only via the PID control which would require much more work (and I even think, it would never work at all in a way it should be, as reaching the temperature in a specific amount of time cannot be estimated easily). I even would say that it does make much more sense to wait until a temperature is reached and then continue instead.

github-actions[bot] commented 1 week ago

Thank you for your contribution to our project. This issue has not received any updates for 60 days and may be considered "stale." If this issue is still important to you, please add an update within the next 7 days to keep it open. Administrators can manually reopen the issue if necessary.

Ro3Deee commented 3 days ago

.