Fix protocol error recovery

Communication timeout and Protocol Errors are now distinguished.
In case of a Protocol Error, the printer waits for heartBeatTimeout to allow filling up the input UART buffer (we expect the MMU still produces some bytes).
Once the timeout elapsed, the input UART buffer is cleared and a new Start Sequence is initiated.

Firmware/mmu2_protocol_logic.cpp

@@ -7,8 +7,7 @@
 namespace MMU2 {
 
 StepStatus ProtocolLogicPartBase::ProcessFINDAReqSent(StepStatus finishedRV, State nextState){
-    auto expmsg = logic->ExpectingMessage(linkLayerTimeout);
+    if (auto expmsg = logic->ExpectingMessage(linkLayerTimeout); expmsg != MessageReady)
-    if (expmsg != MessageReady)
         return expmsg;
     logic->findaPressed = logic->rsp.paramValue;
     state = nextState;
@@ -44,16 +43,9 @@ void ProtocolLogicPartBase::SendButton(uint8_t btn){
     state = State::ButtonSent;
 }
 
-StepStatus ProtocolLogic::ProcessUARTByte(uint8_t c) {
+void ProtocolLogicPartBase::SendVersion(uint8_t stage) {
-    switch (protocol.DecodeResponse(c)) {
+    logic->SendMsg(RequestMsg(RequestMsgCodes::Version, stage));
-    case DecodeStatus::MessageCompleted:
+    state = (State)((uint_fast8_t)State::S0Sent + stage);
-        return MessageReady;
-    case DecodeStatus::NeedMoreData:
-        return Processing;
-    case DecodeStatus::Error:
-    default:
-        return ProtocolError;
-    }
 }
 
 // searches for "ok\n" in the incoming serial data (that's the usual response of the old MMU FW)
@@ -133,54 +125,92 @@ void ProtocolLogic::SendMsg(RequestMsg rq) {
 }
 
 void StartSeq::Restart() {
-    state = State::S0Sent;
+    SendVersion(0);
-    logic->SendMsg(RequestMsg(RequestMsgCodes::Version, 0));
 }
 
 StepStatus StartSeq::Step() {
-    auto expmsg = logic->ExpectingMessage(linkLayerTimeout);
+    if (auto expmsg = logic->ExpectingMessage(linkLayerTimeout); expmsg != MessageReady)
-    if (expmsg != MessageReady)
         return expmsg;
 
     // solve initial handshake
     switch (state) {
     case State::S0Sent: // received response to S0 - major
-        logic->mmuFwVersionMajor = logic->rsp.paramValue;
+        if( logic->rsp.request.code != RequestMsgCodes::Version || logic->rsp.request.value != 0 ){
-        if (logic->mmuFwVersionMajor != 2) {
+            // got a response to something else - protocol corruption probably, repeat the query
-            return VersionMismatch;
+                SendVersion(0);
+        } else {
+            logic->mmuFwVersionMajor = logic->rsp.paramValue;
+            if (logic->mmuFwVersionMajor != 2) {
+                return VersionMismatch;
+            }
+            logic->dataTO.Reset(); // got meaningful response from the MMU, stop data layer timeout tracking
+                SendVersion(1);
         }
-        logic->dataTO.Reset(); // got meaningful response from the MMU, stop data layer timeout tracking
-        logic->SendMsg(RequestMsg(RequestMsgCodes::Version, 1));
-        state = State::S1Sent;
         break;
     case State::S1Sent: // received response to S1 - minor
-        logic->mmuFwVersionMinor = logic->rsp.paramValue;
+        if( logic->rsp.request.code != RequestMsgCodes::Version || logic->rsp.request.value != 1 ){
-        if (logic->mmuFwVersionMinor != 0) {
+            // got a response to something else - protocol corruption probably, repeat the query OR restart the comm by issuing S0?
-            return VersionMismatch;
+                SendVersion(1);
+        } else {
+            logic->mmuFwVersionMinor = logic->rsp.paramValue;
+            if (logic->mmuFwVersionMinor != 0) {
+                return VersionMismatch;
+            }
+                SendVersion(2);
         }
-        logic->SendMsg(RequestMsg(RequestMsgCodes::Version, 2));
-        state = State::S2Sent;
         break;
     case State::S2Sent: // received response to S2 - revision
-        logic->mmuFwVersionBuild = logic->rsp.paramValue;
+        if( logic->rsp.request.code != RequestMsgCodes::Version || logic->rsp.request.value != 2 ){
-        if (logic->mmuFwVersionBuild < 18) {
+            // got a response to something else - protocol corruption probably, repeat the query OR restart the comm by issuing S0?
-            return VersionMismatch;
+                SendVersion(2);
+        } else {
+            logic->mmuFwVersionBuild = logic->rsp.paramValue;
+            if (logic->mmuFwVersionBuild < 18) {
+                return VersionMismatch;
+            }
+            // Start General Interrogation after line up.
+            // For now we just send the state of the filament sensor, but we may request
+            // data point states from the MMU as well. TBD in the future, especially with another protocol
+            SendAndUpdateFilamentSensor();
         }
-        // Start General Interrogation after line up.
-        // For now we just send the state of the filament sensor, but we may request
-        // data point states from the MMU as well. TBD in the future, especially with another protocol
-        SendAndUpdateFilamentSensor();
         break;
     case State::FilamentSensorStateSent:
         state = State::Ready;
         return Finished;
         break;
+    case State::RecoveringProtocolError:
+        // timer elapsed, clear the input buffer
+            while (logic->uart->read() >= 0)
+                ;
+            SendVersion(0);
+        break;
     default:
         return VersionMismatch;
     }
     return Processing;
 }
 
+void DelayedRestart::Restart() {
+    state = State::RecoveringProtocolError;
+}
+
+StepStatus DelayedRestart::Step() {
+    switch (state) {
+    case State::RecoveringProtocolError:
+        if (logic->Elapsed(heartBeatPeriod)) { // this basically means, that we are waiting until there is some traffic on
+            while (logic->uart->read() != -1)
+                ; // clear the input buffer
+            // switch to StartSeq
+            logic->Start();
+        }
+        return Processing;
+        break;
+    default:
+        break;
+    }
+    return Finished;
+}
+
 void Command::Restart() {
     state = State::CommandSent;
     logic->SendMsg(logic->command.rq);
@@ -189,8 +219,7 @@ void Command::Restart() {
 StepStatus Command::Step() {
     switch (state) {
     case State::CommandSent: {
-        auto expmsg = logic->ExpectingMessage(linkLayerTimeout);
+        if (auto expmsg = logic->ExpectingMessage(linkLayerTimeout); expmsg != MessageReady)
-        if (expmsg != MessageReady)
             return expmsg;
 
         switch (logic->rsp.paramCode) { // the response should be either accepted or rejected
@@ -217,12 +246,10 @@ StepStatus Command::Step() {
             CheckAndReportAsyncEvents();
         }
         break;
-    case State::QuerySent: {
+    case State::QuerySent:
-        auto expmsg = logic->ExpectingMessage(linkLayerTimeout);
+        if (auto expmsg = logic->ExpectingMessage(linkLayerTimeout); expmsg != MessageReady)
-        if (expmsg != MessageReady)
             return expmsg;
-        }
+        [[fallthrough]];
-        // [[fallthrough]];
     case State::ContinueFromIdle:
         switch (logic->rsp.paramCode) {
         case ResponseMsgParamCodes::Processing:
@@ -251,21 +278,18 @@ StepStatus Command::Step() {
             return ProtocolError;
         }
         break;
-    case State::FilamentSensorStateSent:{
+    case State::FilamentSensorStateSent:
-        auto expmsg = logic->ExpectingMessage(linkLayerTimeout);
+        if (auto expmsg = logic->ExpectingMessage(linkLayerTimeout); expmsg != MessageReady)
-        if (expmsg != MessageReady)
             return expmsg;
         SendFINDAQuery();
-        } break;
+        break;
     case State::FINDAReqSent:
         return ProcessFINDAReqSent(Processing, State::Wait);
     case State::ButtonSent:{
         // button is never confirmed ... may be it should be
-        auto expmsg = logic->ExpectingMessage(linkLayerTimeout);
+        if (auto expmsg = logic->ExpectingMessage(linkLayerTimeout); expmsg != MessageReady)
-        if (expmsg != MessageReady)
             return expmsg;
-        if (logic->rsp.paramCode == ResponseMsgParamCodes::Accepted)
+        if (logic->rsp.paramCode == ResponseMsgParamCodes::Accepted) {
-        {
             // Button was accepted, decrement the retry.
             mmu2.DecrementRetryAttempts();
         }
@@ -289,9 +313,8 @@ StepStatus Idle::Step() {
             return Processing;
         }
         break;
-    case State::QuerySent: { // check UART
+    case State::QuerySent: // check UART
-        auto expmsg = logic->ExpectingMessage(linkLayerTimeout);
+        if (auto expmsg = logic->ExpectingMessage(linkLayerTimeout); expmsg != MessageReady)
-        if (expmsg != MessageReady)
             return expmsg;
         // If we are accidentally in Idle and we receive something like "T0 P1" - that means the communication dropped out while a command was in progress.
         // That causes no issues here, we just need to switch to Command processing and continue there from now on.
@@ -331,7 +354,7 @@ StepStatus Idle::Step() {
         }
         SendFINDAQuery();
         return Processing;
-    } break;
+        break;
     case State::FINDAReqSent:
         return ProcessFINDAReqSent(Finished, State::Ready);
     default:
@@ -351,21 +374,31 @@ StepStatus Idle::Step() {
 ProtocolLogic::ProtocolLogic(MMU2Serial *uart)
     : stopped(this)
     , startSeq(this)
+    , delayedRestart(this)
     , idle(this)
     , command(this)
     , currentState(&stopped)
     , plannedRq(RequestMsgCodes::unknown, 0)
     , lastUARTActivityMs(0)
+    , dataTO()
     , rsp(RequestMsg(RequestMsgCodes::unknown, 0), ResponseMsgParamCodes::unknown, 0)
     , state(State::Stopped)
     , lrb(0)
     , uart(uart)
+    , errorCode(ErrorCode::OK)
+    , progressCode(ProgressCode::OK)
+    , buttonCode(NoButton)
     , lastFSensor((uint8_t)WhereIsFilament())
+    , findaPressed(false)
+    , mmuFwVersionMajor(0)
+    , mmuFwVersionMinor(0)
+    , mmuFwVersionBuild(0)
 {}
 
 void ProtocolLogic::Start() {
     state = State::InitSequence;
     currentState = &startSeq;
+    protocol.ResetResponseDecoder(); // important - finished delayed restart relies on this
     startSeq.Restart();
 }
 
@@ -446,13 +479,6 @@ void ProtocolLogic::SwitchToIdle() {
     idle.Restart();
 }
 
-void ProtocolLogic::HandleCommunicationTimeout() {
-    uart->flush(); // clear the output buffer
-    currentState = &startSeq;
-    state = State::InitSequence;
-    startSeq.Restart();
-}
-
 bool ProtocolLogic::Elapsed(uint32_t timeout) const {
     return _millis() >= (lastUARTActivityMs + timeout);
 }
@@ -554,9 +580,7 @@ void ProtocolLogic::LogResponse(){
     SERIAL_ECHOLN();
 }
 
-StepStatus ProtocolLogic::HandleCommError(const char *msg, StepStatus ss){
+StepStatus ProtocolLogic::SuppressShortDropOuts(const char *msg, StepStatus ss) {
-    protocol.ResetResponseDecoder();
-    HandleCommunicationTimeout();
     if( dataTO.Record(ss) ){
         LogError(msg);
         return dataTO.InitialCause();
@@ -565,6 +589,21 @@ StepStatus ProtocolLogic::HandleCommError(const char *msg, StepStatus ss){
     }
 }
 
+StepStatus ProtocolLogic::HandleCommunicationTimeout() {
+    uart->flush(); // clear the output buffer
+    protocol.ResetResponseDecoder();
+    Start();
+    return SuppressShortDropOuts("Communication timeout", CommunicationTimeout);
+}
+
+StepStatus ProtocolLogic::HandleProtocolError() {
+    uart->flush(); // clear the output buffer
+    state = State::InitSequence;
+    currentState = &delayedRestart;
+    delayedRestart.Restart();
+    return SuppressShortDropOuts("Protocol Error", ProtocolError);
+}
+
 StepStatus ProtocolLogic::Step() {
     if( ! currentState->ExpectsResponse() ){ // if not waiting for a response, activate a planned request immediately
         ActivatePlannedRequest();
@@ -587,8 +626,7 @@ StepStatus ProtocolLogic::Step() {
                 currentStatus = Processing;
             }
         }
-        }
+    } break;
-        break;
     case CommandRejected:
         // we have to repeat it - that's the only thing we can do
         // no change in state
@@ -606,10 +644,10 @@ StepStatus ProtocolLogic::Step() {
         Stop(); // cannot continue
         break;
     case ProtocolError:
-        currentStatus = HandleCommError("Protocol error", ProtocolError);
+        currentStatus = HandleProtocolError();
         break;
     case CommunicationTimeout:
-        currentStatus = HandleCommError("Communication timeout", CommunicationTimeout);
+        currentStatus = HandleCommunicationTimeout();
         break;
     default:
         break;

Firmware/mmu2_protocol_logic.h

@@ -77,7 +77,7 @@ protected:
         Ready,
         Wait,
 
-        S0Sent,
+        S0Sent, // beware - due to optimization reasons these SxSent must be kept one after another
         S1Sent,
         S2Sent,
         QuerySent,
@@ -86,7 +86,8 @@ protected:
         FINDAReqSent,
         ButtonSent,
 
-        ContinueFromIdle
+        ContinueFromIdle,
+        RecoveringProtocolError
     };
 
     State state; ///< internal state of the sub-automaton
@@ -108,6 +109,8 @@ protected:
     void SendAndUpdateFilamentSensor();
 
     void SendButton(uint8_t btn);
+
+    void SendVersion(uint8_t stage);
 };
 
 /// Starting sequence of the communication with the MMU.
@@ -122,6 +125,14 @@ public:
     StepStatus Step() override;
 };
 
+class DelayedRestart : public ProtocolLogicPartBase {
+public:
+    inline DelayedRestart(ProtocolLogic *logic)
+        : ProtocolLogicPartBase(logic) {}
+    void Restart() override;
+    StepStatus Step() override;
+};
+
 /// A command and its lifecycle.
 /// CommandSent:
 /// - the command was placed into the UART TX buffer, awaiting response from the MMU
@@ -250,13 +261,12 @@ public:
 #ifndef UNITTEST
 private:
 #endif
-    
-    StepStatus ProcessUARTByte(uint8_t c);
     StepStatus ExpectingMessage(uint32_t timeout);
     void SendMsg(RequestMsg rq);
     void SwitchToIdle();
-    void HandleCommunicationTimeout();
+    StepStatus SuppressShortDropOuts(const char *msg, StepStatus ss);
-    StepStatus HandleCommError(const char *msg, StepStatus ss);
+    StepStatus HandleCommunicationTimeout();
+    StepStatus HandleProtocolError();
     bool Elapsed(uint32_t timeout) const;
     void RecordUARTActivity();
     void RecordReceivedByte(uint8_t c);
@@ -276,6 +286,7 @@ private:
     // individual sub-state machines - may be they can be combined into a union since only one is active at once
     Stopped stopped;
     StartSeq startSeq;
+    DelayedRestart delayedRestart;
     Idle idle;
     Command command;
     ProtocolLogicPartBase *currentState; ///< command currently being processed
@@ -327,6 +338,7 @@ private:
     friend class Command;
     friend class Idle;
     friend class StartSeq;
+    friend class DelayedRestart;
 
     friend class MMU2;
 };