Index: sources/update/MsgLink.h =================================================================== diff -u -r2ee835c4c687263fd8852a431db1223cc0e8055c -ra7aa183c6847cc21ad3943c61d797c2824edcaf5 --- sources/update/MsgLink.h (.../MsgLink.h) (revision 2ee835c4c687263fd8852a431db1223cc0e8055c) +++ sources/update/MsgLink.h (.../MsgLink.h) (revision a7aa183c6847cc21ad3943c61d797c2824edcaf5) @@ -94,6 +94,46 @@ * while attempting the software update or cyber attack. */ class MsgLink { +protected: + + /*! + * \brief A Tiny statemachine like enum class. + * + * Protected instead of private to aid testing. + * + * None - In this state a retry method is used. + * Retries are not time based per-say as this is an + * embedded processor, a certain count of retries + * must expire. NACKs in this mode are just a reason + * to retry, just as no response is a reason to retry. + * + * Waiting - In this state, an ACK with a value > 1 + * was received while in "None" state. This means + * the previous message was received and an + * attempt to do that work is in progres but it + * will take a while. It is expected to take + * upto (ackValue-1)*200ms. + * if it times out: + * likely something bad enough occured that + * a user wants to be involved. + * if an ack is received: go to the next step now. + * if a nack is received: treat as a timeout. + * + * Success / Failure - We were in "waiting" and got + * an Ack / Nack respectively. + * + * The "timout" value can't be set multiple times + * for one incoming message and this prevents anything + * in FW being capable of making the UI update looking + * like it got locked up. + */ + enum TimeoutEnum { + None = 0, ///< Not using timeout. + Waiting = 1, ///< Waiting till next ack/nack or timeout. + Success = 2, ///< While waiting got Ack. + Failure = 3 ///< While waiting got Nack. + }; + public: /*! @@ -105,6 +145,7 @@ _retries(0), _expectedId(0), _msgIdSlot(0), + _timeoutMode(TimeoutEnum::None), _wasData(false), _dataBuf({0,0,0,0, {0}}) { @@ -156,9 +197,6 @@ // We are expecting data. _expecting = true; - // Default no extra sleep. - _extraSleep = 0; - _wasData = (msgId == SwUpdateCanMsgIds::DataBufferId_HD_FW) || (msgId == SwUpdateCanMsgIds::DataBufferId_HD_FPGA) || @@ -184,29 +222,56 @@ if (_wasData) { memcpy(&_dataBuf, msg, sizeof(SwUpdateDataBuffer)); } - - //const uint32 bytes_per_packet = 8; - for (uint32 ii = 0; ii < maxEffort; ii++) { - + + // _timeoutMode: + // == 0 - Normal use. + // 1 - Wait till time. + // 2 - Was waiting for timeout but got an ACK. + // 3 - Was waiting for timeout but got a NACK. + _timeoutMode = TimeoutEnum::None; + for (uint32 effortSpent; effortSpent < maxEffort; effortSpent++) { + // No longer expecting! if (!_expecting) { break; } - _ew.re_arm(); + _ew.re_arm(); - // Call indirectly to make this more testable. - this->sendPacket(msgId, msg, length); + // If in timeout mode we're waiting for an ack/nack or time to expire. + // We got an ack so we know the message was received, but we're not to + // continue till we another ack/nack or treat timeout as fail. + if (TimeoutEnum::None == _timeoutMode) { + // Call indirectly to make this more testable. + this->sendPacket(msgId, msg, length); + } // Wait to either retry or success. if (_ew.wait()) { _expecting = false; - if (_extraSleep != 0) { - _ew.sleep(_extraSleep); + + if (TimeoutEnum::Waiting == _timeoutMode) { + continue; } - return true; + // We got a NACK while timing out, abort this so the user can + // note that a long thing failed and can feel they are in control of + // retrying something that was significant. + return (_timeoutMode != 3) ? true : false; + } - _retries++; + + // No retries in _timeoutMode. + if (TimeoutEnum::None == _timeoutMode) { + _retries++; + } else { + // Just keep waiting? + const auto timeNow = std::chrono::steady_clock::now(); + if (timeNow > _timeout) { + // We timed out. Fail. + return false; + } + continue; + } // If we're still expecting then we do a retry. // Before retry send a command that should clear us. @@ -218,34 +283,9 @@ resync.rand = 0; SwUpdate_createSecurity((uint8 *)&resync, sizeof(SwUpdateCommand)); - // Either we could wait for the retry ACK or not. - // For now do not, its unclear what would be ideal and either ought to work. - #if 0 - uint16 old_expectedId = _expectedId; - uint16 old_msgIdSlot = _msgIdSlot; - bool oldWasData = _wasData; - _expectedId = SwUpdateCanMsgIds::ResponseId; - _msgIdSlot = resync.id; - _wasData = false; - - // Sleep, arm, send. std::this_thread::sleep_for(g_ms_pause); - _ew.re_arm(); this->sendPacket(SwUpdateCanMsgIds::CommandId, (uint8*)&resync, 8); - _ew.wait(); - - // Reset state and then sleep a bit. - _expectedId = old_expectedId; - _msgIdSlot = old_msgIdSlot; - _wasData = oldWasData; std::this_thread::sleep_for(g_ms_pause); - - #else - std::this_thread::sleep_for(g_ms_pause); - this->sendPacket(SwUpdateCanMsgIds::CommandId, (uint8*)&resync, 8); - std::this_thread::sleep_for(g_ms_pause); - #endif - } } @@ -292,14 +332,34 @@ if (msgId == SwUpdateCanMsgIds::ResponseId) { SwUpdateResponse *resp = (SwUpdateResponse *)pData; if (resp->id == _msgIdSlot) { - if (resp->ackNack >= 1) { - rv = true; - if (resp->ackNack != 1) { - _extraSleep = (uint16_t)(resp->ackNack * gAckSleepScale); + + // _timeoutMode ackNack | rv _timeout _timeoutMode + // None 0 | F + // None 1 | T + // None >1 | T calc. 1 + // !None 0 | T 3 + // !None 1 | T 2 + // !None >1 | T + if (TimeoutEnum::None == _timeoutMode) { + if (resp->ackNack >= 1) { + rv = true; + if (resp->ackNack > 1) { + // Only set the timeout once. + // This way a bug in the FW code that sends us replies + // can't put it this an infinite stall (which would have to be aborted by a user). + _timeout = std::chrono::steady_clock::now() + + std::chrono::duration(resp->ackNack * gAckSleepScale); + _timeoutMode = TimeoutEnum::Waiting; + } } + } else { + // _timeoutMode > 0 + rv = true; + _timeoutMode = + resp->ackNack == 0 ? TimeoutEnum::Failure : + resp->ackNack == 1 ? TimeoutEnum::Success : TimeoutEnum::Waiting; } } - rv = ((resp->id == _msgIdSlot) && (resp->ackNack == 1)); } else { rv = true; } @@ -313,15 +373,18 @@ } return rv; } - + + protected: EventWait _ew; ///< Wait for time or event. bool _expecting; ///< Expecting data. uint32 _retries; ///< Total number of retries. uint16 _expectedId; ///< Expected message Id. uint8 _msgIdSlot; ///< Message slot. - uint16 _extraSleep; ///< Extra sleep request from FW. + TimeoutEnum _timeoutMode; ///< Timeout enum. + std::chrono::time_point _timeout; ///< Timeout expire time. + bool _wasData; ///< Was data. SwUpdateDataBuffer _dataBuf; ///< Data buffer.