[BUG] redundant temperature reports when heating with wait #26805

petaflot · 2024-02-20T09:45:49Z

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

Yes, and the problem still exists.

Bug Description

M109 and M190 triggers the reporting of temperatures, in addition to the reports triggered by M155

Bug Timeline

probably old

Expected behavior

redundant temperature reports have been stripped:

T:45.79 /0.00 B:59.99 /60.00 @:0 B@:20
T:45.64 /0.00 B:60.01 /60.00 @:0 B@:18
T:45.57 /0.00 B:60.00 /60.00 @:0 B@:19
T:45.36 /0.00 B:60.03 /60.00 @:0 B@:15
T:45.27 /0.00 B:59.99 /60.00 @:0 B@:19
T:45.12 /0.00 B:59.97 /60.00 @:0 B@:22
T:45.07 /0.00 B:60.01 /60.00 @:0 B@:18
; setting temperature with M109
T:44.98 /70.00 B:60.01 /60.00 @:0 B@:18 W:?
T:44.95 /70.00 B:59.99 /60.00 @:127 B@:20 W:?
T:45.21 /70.00 B:60.00 /60.00 @:127 B@:19 W:?
T:46.05 /70.00 B:59.97 /60.00 @:127 B@:22 W:?
T:47.49 /70.00 B:60.01 /60.00 @:127 B@:18 W:?
T:49.75 /70.00 B:59.99 /60.00 @:127 B@:21 W:?
T:51.87 /70.00 B:60.00 /60.00 @:127 B@:19 W:?
T:54.29 /70.00 B:60.01 /60.00 @:127 B@:17 W:?
T:56.81 /70.00 B:59.99 /60.00 @:127 B@:21 W:?
T:59.85 /70.00 B:60.00 /60.00 @:65 B@:19 W:?
T:62.36 /70.00 B:60.01 /60.00 @:24 B@:19 W:?
T:64.60 /70.00 B:60.00 /60.00 @:0 B@:19 W:?
T:66.30 /70.00 B:60.00 /60.00 @:0 B@:20 W:?
T:67.70 /70.00 B:59.99 /60.00 @:0 B@:20 W:?
T:68.46 /70.00 B:60.00 /60.00 @:0 B@:19 W:?
T:68.92 /70.00 B:59.98 /60.00 @:0 B@:21 W:?
T:69.05 /70.00 B:60.00 /60.00 @:0 B@:19 W:4
T:69.07 /70.00 B:60.03 /60.00 @:4 B@:16 W:3
T:68.92 /70.00 B:59.98 /60.00 @:13 B@:22 W:2
T:68.76 /70.00 B:60.00 /60.00 @:19 B@:19 W:1
T:68.57 /70.00 B:59.92 /60.00 @:23 B@:28 W:0
; target temperature reached
T:68.39 /70.00 B:60.00 /60.00 @:25 B@:19
T:68.31 /70.00 B:59.97 /60.00 @:24 B@:22
T:68.49 /70.00 B:59.99 /60.00 @:21 B@:21
T:68.76 /70.00 B:59.98 /60.00 @:17 B@:21

Actual behavior

T:45.79 /0.00 B:59.99 /60.00 @:0 B@:20
T:45.64 /0.00 B:60.01 /60.00 @:0 B@:18
T:45.57 /0.00 B:60.00 /60.00 @:0 B@:19
T:45.36 /0.00 B:60.03 /60.00 @:0 B@:15
T:45.27 /0.00 B:59.99 /60.00 @:0 B@:19
T:45.12 /0.00 B:59.97 /60.00 @:0 B@:22
T:45.07 /0.00 B:60.01 /60.00 @:0 B@:18
; setting temperature with M109
T:44.98 /70.00 B:60.01 /60.00 @:0 B@:18 W:?
T:44.95 /70.00 B:59.99 /60.00 @:127 B@:20 W:?
T:44.95 /70.00 B:59.99 /60.00 @:127 B@:20
T:45.21 /70.00 B:60.00 /60.00 @:127 B@:19 W:?
T:46.05 /70.00 B:59.97 /60.00 @:127 B@:22 W:?
T:46.05 /70.00 B:59.97 /60.00 @:127 B@:22
T:47.49 /70.00 B:60.01 /60.00 @:127 B@:18 W:?
T:49.75 /70.00 B:59.99 /60.00 @:127 B@:21 W:?
T:49.75 /70.00 B:59.99 /60.00 @:127 B@:21
T:51.87 /70.00 B:60.00 /60.00 @:127 B@:19 W:?
T:54.29 /70.00 B:60.01 /60.00 @:127 B@:17 W:?
T:54.29 /70.00 B:60.01 /60.00 @:127 B@:17
T:56.81 /70.00 B:59.99 /60.00 @:127 B@:21 W:?
T:59.85 /70.00 B:60.00 /60.00 @:65 B@:19 W:?
T:59.85 /70.00 B:60.00 /60.00 @:65 B@:19
T:62.36 /70.00 B:60.01 /60.00 @:24 B@:19 W:?
T:64.60 /70.00 B:60.00 /60.00 @:0 B@:19 W:?
T:64.60 /70.00 B:60.00 /60.00 @:0 B@:19
T:66.30 /70.00 B:60.00 /60.00 @:0 B@:20 W:?
T:67.70 /70.00 B:59.99 /60.00 @:0 B@:20 W:?
T:67.70 /70.00 B:59.99 /60.00 @:0 B@:20
T:68.46 /70.00 B:60.00 /60.00 @:0 B@:19 W:?
T:68.92 /70.00 B:59.98 /60.00 @:0 B@:21 W:?
T:68.92 /70.00 B:59.98 /60.00 @:0 B@:21
T:69.05 /70.00 B:60.00 /60.00 @:0 B@:19 W:4
T:69.07 /70.00 B:60.03 /60.00 @:4 B@:16 W:3
T:69.07 /70.00 B:60.03 /60.00 @:4 B@:16
T:68.92 /70.00 B:59.98 /60.00 @:13 B@:22 W:2
T:68.76 /70.00 B:60.00 /60.00 @:19 B@:19 W:1
T:68.76 /70.00 B:60.00 /60.00 @:19 B@:19
T:68.57 /70.00 B:59.92 /60.00 @:23 B@:28 W:0
; target temperature reached
T:68.39 /70.00 B:60.00 /60.00 @:25 B@:19
T:68.31 /70.00 B:59.97 /60.00 @:24 B@:22
T:68.49 /70.00 B:59.99 /60.00 @:21 B@:21
T:68.76 /70.00 B:59.98 /60.00 @:17 B@:21

Steps to Reproduce

M155 S2 ; print some temperature reports
; optional: do something that takes some time, ie.
G28
; setting temperature with M109
M109 S70 ; set nozzle temperature, wait
; target temperature reached

Version of Marlin Firmware

bugfix-2.1.x

Additional information & file uploads

configuration not needed

The text was updated successfully, but these errors were encountered:

ellensp · 2024-02-20T10:40:14Z

Interesting...

code is duplicated.. removing it solves the issue, see diff in my next post

petaflot · 2024-02-20T10:43:45Z

I like it so much when removing code fixes things XD

ellensp · 2024-02-20T10:53:06Z

this diff removes the duplication

diff --git a/Marlin/src/module/temperature.cpp b/Marlin/src/module/temperature.cpp
index 9b071ecc36..0bb73dad20 100644
--- a/Marlin/src/module/temperature.cpp
+++ b/Marlin/src/module/temperature.cpp
@@ -4488,7 +4488,7 @@ void Temperature::isr() {
 
       bool wants_to_cool = false;
       celsius_float_t target_temp = -1.0, old_temp = 9999.0;
-      millis_t now, next_temp_ms = 0, cool_check_ms = 0;
+      millis_t now, cool_check_ms = 0;
       wait_for_heatup = true;
       do {
         // Target temperature might be changed during the loop
@@ -4501,19 +4501,6 @@ void Temperature::isr() {
         }
 
         now = millis();
-        if (ELAPSED(now, next_temp_ms)) { // Print temp & remaining time every 1s while waiting
-          next_temp_ms = now + 1000UL;
-          print_heater_states(target_extruder);
-          #if TEMP_RESIDENCY_TIME > 0
-            SString<20> s(F(" W:"));
-            if (residency_start_ms)
-              s += long((SEC_TO_MS(TEMP_RESIDENCY_TIME) - (now - residency_start_ms)) / 1000UL);
-            else
-              s += '?';
-            s.echo();
-          #endif
-          SERIAL_EOL();
-        }
 
         idle();
         gcode.reset_stepper_timeout(); // Keep steppers powered
@@ -4626,7 +4613,7 @@ void Temperature::isr() {
 
       bool wants_to_cool = false;
       celsius_float_t target_temp = -1, old_temp = 9999;
-      millis_t now, next_temp_ms = 0, cool_check_ms = 0;
+      millis_t now, cool_check_ms = 0;
       wait_for_heatup = true;
       do {
         // Target temperature might be changed during the loop
@@ -4639,19 +4626,6 @@ void Temperature::isr() {
         }
 
         now = millis();
-        if (ELAPSED(now, next_temp_ms)) { //Print Temp Reading every 1 second while heating up.
-          next_temp_ms = now + 1000UL;
-          print_heater_states(active_extruder);
-          #if TEMP_BED_RESIDENCY_TIME > 0
-            SString<20> s(F(" W:"));
-            if (residency_start_ms)
-              s += long((SEC_TO_MS(TEMP_BED_RESIDENCY_TIME) - (now - residency_start_ms)) / 1000UL);
-            else
-              s += '?';
-            s.echo();
-          #endif
-          SERIAL_EOL();
-        }
 
         idle();
         gcode.reset_stepper_timeout(); // Keep steppers powered

but I notice this looses TEMP_RESIDENCY_TIME and TEMP_BED_RESIDENCY_TIME from the report (ts not in M155)
ie

          #if TEMP_RESIDENCY_TIME > 0
            SString<20> s(F(" W:"));
            if (residency_start_ms)
              s += long((SEC_TO_MS(TEMP_RESIDENCY_TIME) - (now - residency_start_ms)) / 1000UL);
            else
              s += '?';
            s.echo();
          #endif

you can you test this part for now. (ive tested it in the simulator)

I also worry how many clients are going to break with this fixed.

petaflot · 2024-02-20T11:29:47Z

I'm testing it now,

it definitely removed too much (:W x is missing) for both M109 and (extruder) M190 (bed). don't have a chamber.

I'm a little puzzled because I still found this:

4818           print_heater_states(active_extruder);
4819           #if TEMP_CHAMBER_RESIDENCY_TIME > 0
4820             SString<20> s(F(" W:"));
4821             if (residency_start_ms)
4822               s += long((SEC_TO_MS(TEMP_CHAMBER_RESIDENCY_TIME) - (now - residency_start_ms)) / 1000UL);
4823             else
4824               s += '?';
4825             s.echo();
4826           #endif

4919           #if TEMP_COOLER_RESIDENCY_TIME > 0
4920             SString<20> s(F(" W:"));
4921             if (residency_start_ms)
4922               s += long((SEC_TO_MS(TEMP_COOLER_RESIDENCY_TIME) - (now - residency_start_ms)) / 1000UL);
4923             else
4924               s += '?';
4925             s.echo();
4926           #endif

(there are other)

also, this completely removes temperature reports for M109 and M190 if temperatures are not auto-reported (M155 S0)

petaflot · 2024-02-20T11:55:43Z

I would suggest adding an optional residency_start_ms argument to void Temperature::print_heater_states( and test if auto report temperatures is enabled before calling it again ; then the "W: x" suffix can be added.something along the line of https://github.com/petaflot/Marlin/blob/redundant_temperature_reports/Marlin/src/module/temperature.cpp (don't know how to show a diff at this point)

petaflot · 2024-02-20T12:07:52Z

I updated my proposition (it only cleans up the code a little, does NOT remove the redundancy) but it won't compile:

Marlin/src/module/temperature.cpp: In static member function 'static void Temperature::print_heater_states(int8_t)':
Marlin/src/module/temperature.cpp:4427:10: error: 'residency_start_ms' was not declared in this scope
 4427 |     if ( residency_start_ms >= 0 ) {
      |          ^~~~~~~~~~~~~~~~~~
Marlin/src/module/temperature.cpp:4431:55: error: 'now' was not declared in this scope; did you mean 'pow'?
 4431 |          s += long((SEC_TO_MS(TEMP_RESIDENCY_TIME) - (now - residency_start_ms)) / 1000UL);

I don't know C well enough to fix it at this point

petaflot · 2024-02-20T12:15:51Z

there are these next_report_ms and next_temp_ms with related checks, AFAIK everything could go in print_heater_states()

I also don't see a reason why SERIAL_EOL(); is not in print_heater_states()

petaflot · 2024-02-20T12:35:06Z

opened a branch on my repo to fix the issue: https://github.com/petaflot/Marlin/tree/redundant_temperature_reports

ellensp · 2024-02-22T09:08:07Z

This suspect code was added in #11949

InsanityAutomation · 2024-04-08T13:46:51Z

@petaflot can you verify the open PR resolves the issue on youre end?

petaflot · 2024-04-14T08:58:45Z

@petaflot can you verify the open PR resolves the issue on youre end?

please give me a few days

petaflot · 2024-04-23T10:12:48Z

it works. I still think there's room for code cleanup though...

petaflot · 2024-04-24T07:30:43Z

closing as not planned because of the cleanup required

petaflot · 2024-04-24T12:52:34Z

following discussion on #26952

petaflot · 2024-04-24T12:52:41Z

following discussion on #26952

github-actions · 2024-06-24T01:27:39Z

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.

petaflot added the Bug: Potential ? label Feb 20, 2024

ellensp added Bug: Confirmed ! and removed Bug: Potential ? labels Feb 20, 2024

thisiskeithb added the C: Serial Comms label Apr 8, 2024

InsanityAutomation mentioned this issue Apr 8, 2024

Fix Duplicated Temp Report #26952

Merged

thisiskeithb linked a pull request Apr 8, 2024 that will close this issue

Fix Duplicated Temp Report #26952

Merged

petaflot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 24, 2024

petaflot reopened this Apr 24, 2024

thinkyhead closed this as completed in #26952 Apr 24, 2024

github-actions bot locked and limited conversation to collaborators Jun 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG] redundant temperature reports when heating with wait #26805

[BUG] redundant temperature reports when heating with wait #26805

petaflot commented Feb 20, 2024 •

edited

Loading

ellensp commented Feb 20, 2024 •

edited

Loading

petaflot commented Feb 20, 2024

ellensp commented Feb 20, 2024 •

edited

Loading

petaflot commented Feb 20, 2024 •

edited

Loading

petaflot commented Feb 20, 2024 •

edited

Loading

petaflot commented Feb 20, 2024

petaflot commented Feb 20, 2024

petaflot commented Feb 20, 2024

ellensp commented Feb 22, 2024 •

edited

Loading

InsanityAutomation commented Apr 8, 2024

petaflot commented Apr 14, 2024

petaflot commented Apr 23, 2024

petaflot commented Apr 24, 2024

petaflot commented Apr 24, 2024

petaflot commented Apr 24, 2024

github-actions bot commented Jun 24, 2024

[BUG] redundant temperature reports when heating with wait #26805

[BUG] redundant temperature reports when heating with wait #26805

Comments

petaflot commented Feb 20, 2024 • edited Loading

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

Bug Description

Bug Timeline

Expected behavior

Actual behavior

Steps to Reproduce

Version of Marlin Firmware

Additional information & file uploads

ellensp commented Feb 20, 2024 • edited Loading

petaflot commented Feb 20, 2024

ellensp commented Feb 20, 2024 • edited Loading

petaflot commented Feb 20, 2024 • edited Loading

petaflot commented Feb 20, 2024 • edited Loading

petaflot commented Feb 20, 2024

petaflot commented Feb 20, 2024

petaflot commented Feb 20, 2024

ellensp commented Feb 22, 2024 • edited Loading

InsanityAutomation commented Apr 8, 2024

petaflot commented Apr 14, 2024

petaflot commented Apr 23, 2024

petaflot commented Apr 24, 2024

petaflot commented Apr 24, 2024

petaflot commented Apr 24, 2024

github-actions bot commented Jun 24, 2024

petaflot commented Feb 20, 2024 •

edited

Loading

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

ellensp commented Feb 20, 2024 •

edited

Loading

ellensp commented Feb 20, 2024 •

edited

Loading

petaflot commented Feb 20, 2024 •

edited

Loading

petaflot commented Feb 20, 2024 •

edited

Loading

ellensp commented Feb 22, 2024 •

edited

Loading