From 12a3051c50e1771ade7ac3320c84641c1bf46a4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Wed, 18 Oct 2023 22:54:51 +0000 Subject: [PATCH 1/7] mmu: Add CheckErrorScreenUserInput Due to differences in 8-bit FW I had to implement two new get functions * GetCommandInProgress * GetLastErrorCode --- Firmware/mmu2.cpp | 7 +------ Firmware/mmu2.h | 6 ++++++ Firmware/mmu2_reporting.cpp | 8 ++++++++ Firmware/mmu2_reporting.h | 3 +++ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/Firmware/mmu2.cpp b/Firmware/mmu2.cpp index 47e29d16e..6d80c982c 100644 --- a/Firmware/mmu2.cpp +++ b/Firmware/mmu2.cpp @@ -191,12 +191,7 @@ void MMU2::mmu_loop() { void __attribute__((noinline)) MMU2::mmu_loop_inner(bool reportErrors) { logicStepLastStatus = LogicStep(reportErrors); // it looks like the mmu_loop doesn't need to be a blocking call - - if (isErrorScreenRunning()) { - // Call this every iteration to keep the knob rotation responsive - // This includes when mmu_loop is called within manage_response - ReportErrorHook((CommandInProgress)logic.CommandInProgress(), lastErrorCode, uint8_t(lastErrorSource)); - } + CheckErrorScreenUserInput(); } void MMU2::CheckFINDARunout() { diff --git a/Firmware/mmu2.h b/Firmware/mmu2.h index 750154ce0..f5eea95b9 100644 --- a/Firmware/mmu2.h +++ b/Firmware/mmu2.h @@ -160,9 +160,15 @@ public: /// @returns Current error code inline ErrorCode MMUCurrentErrorCode() const { return logic.Error(); } + /// @returns Command in progress + inline uint8_t GetCommandInProgress() const { return logic.CommandInProgress(); } + /// @returns Last error source inline ErrorSource MMULastErrorSource() const { return lastErrorSource; } + /// @returns Last error code + inline ErrorCode GetLastErrorCode() const { return lastErrorCode; } + /// @returns the version of the connected MMU FW. /// In the future we'll return the trully detected FW version Version GetMMUFWVersion() const { diff --git a/Firmware/mmu2_reporting.cpp b/Firmware/mmu2_reporting.cpp index e2ced92f1..5bbf4e60a 100644 --- a/Firmware/mmu2_reporting.cpp +++ b/Firmware/mmu2_reporting.cpp @@ -221,6 +221,14 @@ static bool is_mmu_error_monitor_active; // Set to false to allow the error screen to render again. static bool putErrorScreenToSleep; +void CheckErrorScreenUserInput() { + if (isErrorScreenRunning()) { + // Call this every iteration to keep the knob rotation responsive + // This includes when mmu_loop is called within manage_response + ReportErrorHook((CommandInProgress)mmu2.GetCommandInProgress(), mmu2.GetLastErrorCode(), mmu2.MMULastErrorSource()); + } +} + bool isErrorScreenRunning() { return is_mmu_error_monitor_active; } diff --git a/Firmware/mmu2_reporting.h b/Firmware/mmu2_reporting.h index 2a01afa0c..9429ecbac 100644 --- a/Firmware/mmu2_reporting.h +++ b/Firmware/mmu2_reporting.h @@ -29,6 +29,9 @@ void BeginReport(CommandInProgress cip, ProgressCode ec); /// Called at the end of every MMU operation void EndReport(CommandInProgress cip, ProgressCode ec); +/// Checks for error screen user input, if the error screen is open +void CheckErrorScreenUserInput(); + /// Return true if the printer's LCD is drawing the error screen bool isErrorScreenRunning(); From cd3372dc92732bc5c56e30b4c555454b0416c524 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Wed, 18 Oct 2023 22:57:34 +0000 Subject: [PATCH 2/7] mmu: formatting --- Firmware/mmu2.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/Firmware/mmu2.cpp b/Firmware/mmu2.cpp index 6d80c982c..442f2ef32 100644 --- a/Firmware/mmu2.cpp +++ b/Firmware/mmu2.cpp @@ -835,45 +835,58 @@ bool MMU2::manage_response(const bool move_axes, const bool turn_off_nozzle) { } StepStatus MMU2::LogicStep(bool reportErrors) { - CheckUserInput(); // Process any buttons before proceeding with another MMU Query - StepStatus ss = logic.Step(); + // Process any buttons before proceeding with another MMU Query + CheckUserInput(); + + const StepStatus ss = logic.Step(); switch (ss) { + case Finished: // At this point it is safe to trigger a runout and not interrupt the MMU protocol CheckFINDARunout(); break; + case Processing: OnMMUProgressMsg(logic.Progress()); break; + case ButtonPushed: lastButton = logic.Button(); LogEchoEvent_P(PSTR("MMU Button pushed")); CheckUserInput(); // Process the button immediately break; + case Interrupted: // can be silently handed over to a higher layer, no processing necessary at this spot break; + default: if (reportErrors) { switch (ss) { + case CommandError: ReportError(logic.Error(), ErrorSourceMMU); break; + case CommunicationTimeout: state = xState::Connecting; ReportError(ErrorCode::MMU_NOT_RESPONDING, ErrorSourcePrinter); break; + case ProtocolError: state = xState::Connecting; ReportError(ErrorCode::PROTOCOL_ERROR, ErrorSourcePrinter); break; + case VersionMismatch: StopKeepPowered(); ReportError(ErrorCode::VERSION_MISMATCH, ErrorSourcePrinter); break; + case PrinterError: ReportError(logic.PrinterError(), ErrorSourcePrinter); break; + default: break; } @@ -883,6 +896,7 @@ StepStatus MMU2::LogicStep(bool reportErrors) { if (logic.Running()) { state = xState::Active; } + return ss; } From ef33db9a712a59f2262e0f9f68935325bae2e9ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Wed, 18 Oct 2023 23:19:54 +0000 Subject: [PATCH 3/7] mmu: add DisableMMUInSettings power_on should not be modifying EEPROM_MMU_ENABLED. The code is never executed unless it's already been set. Only disable EEPROM_MMU_ENABLED through Buttons::DisableMMU Change in memory: Flash: -18 bytes SRAM: 0 bytes --- Firmware/mmu2.cpp | 11 +++++------ Firmware/mmu2_power.cpp | 4 ---- Firmware/mmu2_reporting.cpp | 6 ++++++ Firmware/mmu2_reporting.h | 3 +++ 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Firmware/mmu2.cpp b/Firmware/mmu2.cpp index 442f2ef32..6bd6f1605 100644 --- a/Firmware/mmu2.cpp +++ b/Firmware/mmu2.cpp @@ -69,7 +69,7 @@ void MMU2::Start() { void MMU2::Stop() { StopKeepPowered(); - PowerOff(); // This also disables the MMU in the EEPROM. + PowerOff(); } void MMU2::StopKeepPowered() { @@ -125,11 +125,9 @@ void MMU2::TriggerResetPin() { void MMU2::PowerCycle() { // cut the power to the MMU and after a while restore it // Sadly, MK3/S/+ cannot do this - // NOTE: the below will toggle the EEPROM var. Should we - // assert this function is never called in the MK3 FW? Do we even care? - PowerOff(); + Stop(); safe_delay_keep_alive(1000); - PowerOn(); + Start(); } void MMU2::PowerOff() { @@ -735,7 +733,8 @@ void MMU2::CheckUserInput() { // ... but mmu2_power.cpp knows this and triggers a soft-reset instead. break; case Buttons::DisableMMU: - Stop(); // Poweroff handles updating the EEPROM shutoff. + Stop(); + DisableMMUInSettings(); break; case Buttons::StopPrint: // @@TODO not sure if we shall handle this high level operation at this spot diff --git a/Firmware/mmu2_power.cpp b/Firmware/mmu2_power.cpp index 14f119156..4db6be1bc 100644 --- a/Firmware/mmu2_power.cpp +++ b/Firmware/mmu2_power.cpp @@ -4,7 +4,6 @@ #include "fastio.h" #include #include "mmu2.h" -#include "eeprom.h" namespace MMU2 { @@ -16,13 +15,10 @@ void power_on() { SET_OUTPUT(MMU_RST_PIN); // setup reset pin #endif //MMU_HWRESET - eeprom_update_byte((uint8_t *)EEPROM_MMU_ENABLED, true); - reset(); } void power_off() { - eeprom_update_byte((uint8_t *)EEPROM_MMU_ENABLED, false); } void reset() { diff --git a/Firmware/mmu2_reporting.cpp b/Firmware/mmu2_reporting.cpp index 5bbf4e60a..e6ae89b20 100644 --- a/Firmware/mmu2_reporting.cpp +++ b/Firmware/mmu2_reporting.cpp @@ -1,4 +1,5 @@ #include +#include "eeprom.h" #include "mmu2.h" #include "mmu2_log.h" #include "mmu2_reporting.h" @@ -344,6 +345,11 @@ void TryLoadUnloadReporter::DumpToSerial(){ MMU2_ECHO_MSGLN(buf); } +/// Disables MMU in EEPROM +void DisableMMUInSettings() { + eeprom_update_byte((uint8_t *)EEPROM_MMU_ENABLED, false); +} + void IncrementLoadFails(){ eeprom_increment_byte((uint8_t *)EEPROM_MMU_LOAD_FAIL); eeprom_increment_word((uint16_t *)EEPROM_MMU_LOAD_FAIL_TOT); diff --git a/Firmware/mmu2_reporting.h b/Firmware/mmu2_reporting.h index 9429ecbac..927c2b20b 100644 --- a/Firmware/mmu2_reporting.h +++ b/Firmware/mmu2_reporting.h @@ -84,6 +84,9 @@ bool MMUAvailable(); /// Global Enable/Disable use MMU (to be stored in EEPROM) bool UseMMU(); +/// Disables MMU in EEPROM +void DisableMMUInSettings(); + /// Increments EEPROM cell - number of failed loads into the nozzle /// Note: technically, this is not an MMU error but an error of the printer. void IncrementLoadFails(); From 7b9e70770965013443fbe521e1bbe93050e7239b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Wed, 18 Oct 2023 23:25:17 +0000 Subject: [PATCH 4/7] mmu: add ResetCommunicationTimeoutAttempts Adding __attribute__((noinline)) saves 14 bytes of flash Change in memory: Flash: +34 bytes SRAM: 0 bytes --- Firmware/mmu2.cpp | 2 +- Firmware/mmu2_protocol_logic.cpp | 9 +++++++-- Firmware/mmu2_protocol_logic.h | 2 ++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Firmware/mmu2.cpp b/Firmware/mmu2.cpp index 6bd6f1605..ced863fe0 100644 --- a/Firmware/mmu2.cpp +++ b/Firmware/mmu2.cpp @@ -63,8 +63,8 @@ void MMU2::Start() { // start the communication logic.Start(); - logic.ResetRetryAttempts(); + logic.ResetCommunicationTimeoutAttempts(); } void MMU2::Stop() { diff --git a/Firmware/mmu2_protocol_logic.cpp b/Firmware/mmu2_protocol_logic.cpp index 77a0da950..65b5a4115 100644 --- a/Firmware/mmu2_protocol_logic.cpp +++ b/Firmware/mmu2_protocol_logic.cpp @@ -256,7 +256,7 @@ StepStatus ProtocolLogic::ProcessVersionResponse(uint8_t stage) { SendVersion(stage); } } else { - dataTO.Reset(); // got a meaningful response from the MMU, stop data layer timeout tracking + ResetCommunicationTimeoutAttempts(); // got a meaningful response from the MMU, stop data layer timeout tracking SendVersion(stage + 1); } } @@ -774,7 +774,7 @@ void ProtocolLogic::LogResponse() { StepStatus ProtocolLogic::SuppressShortDropOuts(const char *msg_P, StepStatus ss) { if (dataTO.Record(ss)) { LogError(msg_P); - dataTO.Reset(); // prepare for another run of consecutive retries before firing an error + ResetCommunicationTimeoutAttempts(); // prepare for another run of consecutive retries before firing an error return dataTO.InitialCause(); } else { return Processing; // suppress short drop outs of communication @@ -865,6 +865,11 @@ void ProtocolLogic::ResetRetryAttempts() { retryAttempts = MAX_RETRIES; } +void __attribute__((noinline)) ProtocolLogic::ResetCommunicationTimeoutAttempts() { + SERIAL_ECHOLNPGM("RSTCommTimeout"); + dataTO.Reset(); +} + bool DropOutFilter::Record(StepStatus ss) { if (occurrences == maxOccurrences) { cause = ss; diff --git a/Firmware/mmu2_protocol_logic.h b/Firmware/mmu2_protocol_logic.h index bf884106b..6e46360ed 100644 --- a/Firmware/mmu2_protocol_logic.h +++ b/Firmware/mmu2_protocol_logic.h @@ -186,6 +186,8 @@ public: /// Reset the retryAttempts back to the default value void ResetRetryAttempts(); + void ResetCommunicationTimeoutAttempts(); + constexpr bool InAutoRetry() const { return inAutoRetry; } void SetInAutoRetry(bool iar) { inAutoRetry = iar; From 62b90fde28bfdb0cd74db4219cbe7c0983d32ddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Wed, 18 Oct 2023 23:34:41 +0000 Subject: [PATCH 5/7] mmu: remove ResetOnExit No change in memory --- Firmware/mmu2_error_converter.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/Firmware/mmu2_error_converter.cpp b/Firmware/mmu2_error_converter.cpp index 1c760b94b..52fa17fc5 100644 --- a/Firmware/mmu2_error_converter.cpp +++ b/Firmware/mmu2_error_converter.cpp @@ -186,20 +186,15 @@ const char * PrusaErrorButtonMore(){ return MSG_BTN_MORE; } -struct ResetOnExit { - ResetOnExit() = default; - ~ResetOnExit(){ - buttonSelectedOperation = ButtonOperations::NoOperation; - } -}; - Buttons ButtonPressed(ErrorCode ec) { if (buttonSelectedOperation == ButtonOperations::NoOperation) { return Buttons::NoButton; // no button } - - ResetOnExit ros; // clear buttonSelectedOperation on exit from this call - return ButtonAvailable(ec); + + const auto result = ButtonAvailable(ec); + buttonSelectedOperation = ButtonOperations::NoOperation; // Reset operation + + return result; } Buttons ButtonAvailable(ErrorCode ec) { From d56f2bc57e570c11d65645ed87866e64effbc619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Wed, 18 Oct 2023 23:42:59 +0000 Subject: [PATCH 6/7] mmu: remove obsolete EEPROM comments --- Firmware/mmu2.cpp | 2 +- Firmware/mmu2_power.cpp | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Firmware/mmu2.cpp b/Firmware/mmu2.cpp index ced863fe0..3797d9e54 100644 --- a/Firmware/mmu2.cpp +++ b/Firmware/mmu2.cpp @@ -55,7 +55,7 @@ MMU2::MMU2() void MMU2::Start() { mmu2Serial.begin(MMU_BAUD); - PowerOn(); // I repurposed this to serve as our EEPROM disable toggle. + PowerOn(); mmu2Serial.flush(); // make sure the UART buffer is clear before starting communication extruder = MMU2_NO_TOOL; diff --git a/Firmware/mmu2_power.cpp b/Firmware/mmu2_power.cpp index 4db6be1bc..163084458 100644 --- a/Firmware/mmu2_power.cpp +++ b/Firmware/mmu2_power.cpp @@ -7,8 +7,7 @@ namespace MMU2 { -// sadly, on MK3 we cannot do actual power cycle on HW... -// so we just block the MMU via EEPROM var instead. +// On MK3 we cannot do actual power cycle on HW. Instead trigger a hardware reset. void power_on() { #ifdef MMU_HWRESET WRITE(MMU_RST_PIN, 1); From 82e0124cb440937962b77f3efdbc9cf2d1b6a981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Thu, 19 Oct 2023 18:27:32 +0000 Subject: [PATCH 7/7] mmu: remove isErrorScreenRunning() No change in memory --- Firmware/mmu2_reporting.cpp | 6 +----- Firmware/mmu2_reporting.h | 3 --- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/Firmware/mmu2_reporting.cpp b/Firmware/mmu2_reporting.cpp index e6ae89b20..a44f03947 100644 --- a/Firmware/mmu2_reporting.cpp +++ b/Firmware/mmu2_reporting.cpp @@ -223,17 +223,13 @@ static bool is_mmu_error_monitor_active; static bool putErrorScreenToSleep; void CheckErrorScreenUserInput() { - if (isErrorScreenRunning()) { + if (is_mmu_error_monitor_active) { // Call this every iteration to keep the knob rotation responsive // This includes when mmu_loop is called within manage_response ReportErrorHook((CommandInProgress)mmu2.GetCommandInProgress(), mmu2.GetLastErrorCode(), mmu2.MMULastErrorSource()); } } -bool isErrorScreenRunning() { - return is_mmu_error_monitor_active; -} - bool TuneMenuEntered() { return putErrorScreenToSleep; } diff --git a/Firmware/mmu2_reporting.h b/Firmware/mmu2_reporting.h index 927c2b20b..90ac93b24 100644 --- a/Firmware/mmu2_reporting.h +++ b/Firmware/mmu2_reporting.h @@ -32,9 +32,6 @@ void EndReport(CommandInProgress cip, ProgressCode ec); /// Checks for error screen user input, if the error screen is open void CheckErrorScreenUserInput(); -/// Return true if the printer's LCD is drawing the error screen -bool isErrorScreenRunning(); - /// Return true if the error screen is sleeping in the background /// Error screen sleeps when the firmware is rendering complementary /// UI to resolve the error screen, for example tuning Idler Stallguard Threshold