-
-
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
Fix Duplicated Temp Report #26952
Fix Duplicated Temp Report #26952
Conversation
I think you have missed the issue here (Or I have) It is my understanding that: M109/M190 should just sit there (with optional Busy processing lines) and eventually once temperature is reached return OK without reporting any temperatures. Currently with this PR they still report temperatures while waiting. If you enable M155 then use M109/M190 you still get two lines of temperature reports M104/M140 currently seem to correctly just returns, no temperature reporting |
Operator error. M155 without a parameter did not default to a time or off, its just quietly ignored. invalidating most of my testing. I still think M109/M190 should be quiet unless told to report. |
Confirmed behavior -
|
yes. but it's very simple and does not clean up the code (lot of redundancy, dozens of lines can be removed) |
I don't disagree but I don't like to mix clean up with bug fix... You tend to end up with new bugs for every bug you fix that way. |
I agree. It is hard to identify what changed when mixed with cleanup. I never reviewed this one to get it in because I was having trouble understanding the problem well enough to know if the change seemed correct, given the multiple comments seeming to be confused about proper and actual behavior. |
following that last comment I closed #26805 as completed ; please merge |
* bugfix-2.1.x: (111 commits) [cron] Bump distribution date (2024-04-25) 🩹 IA-Creality minor cleanup 🩹 Simple IA-Creality babystep patch 🚸 Fix duplicate temperature report (MarlinFirmware#26952) [cron] Bump distribution date (2024-04-24) ✏️ MPCTEMP_START => MPC_STARTED (MarlinFirmware#27002) 🔧 BIQU MicroProbe V2 pull-up warning (MarlinFirmware#27008) 🎨 Format pins which fail validation (MarlinFirmware#27007) ✅ CI - Validate Pins Formatting (MarlinFirmware#26996) [cron] Bump distribution date (2024-04-23) 🎨 Clean up after recent PRs [cron] Bump distribution date (2024-04-22) 🐛 Fix Flags<N> data storage width (MarlinFirmware#26995) ✅ Add additional unit tests for types.h (MarlinFirmware#26994) ✅ Unit test improvements (MarlinFirmware#26993) 🔧 Add RAMPS TMC SPI pins when !TMC_USE_SW_SPI (MarlinFirmware#26960) 🐛 Fix PID upon entering PID_FUNCTIONAL_RANGE (MarlinFirmware#26926) [cron] Bump distribution date (2024-04-21) 🎨Match unit test folder structure to code (MarlinFirmware#26990) ✅ Skip compile tests when editing unit tests (MarlinFirmware#26991) ...
Resolves issue #26805 which saw temp reports duplicated when in the waiting state. As autotemp reporting is now isolated the idle state wait heaters for M109 etc, use the global wait for heatup flag to suppress the report.
I cant see a logical reason why this bool was initialized to true. Each use case sets it as expected before entering its wait loop.