Marlin
Marlin copied to clipboard
MarlinFirmware
[BUG] redundant temperature reports when heating with wait
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
Interesting...
code is duplicated.. removing it solves the issue, see diff in my next post
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.
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)
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)
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
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()
opened a branch on my repo to fix the issue: https://github.com/petaflot/Marlin/tree/redundant_temperature_reports
This suspect code was added in https://github.com/MarlinFirmware/Marlin/pull/11949
@petaflot can you verify the open PR resolves the issue on youre end?
@petaflot can you verify the open PR resolves the issue on youre end?
please give me a few days
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[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}