diff options
author | Christian Hammacher <bmasterc@gmail.com> | 2022-05-02 18:09:16 +0300 |
---|---|---|
committer | Christian Hammacher <bmasterc@gmail.com> | 2022-05-02 18:09:16 +0300 |
commit | d03ec3c3f1b59b100e9cabd60568c420f610b2f7 (patch) | |
tree | 13aec33201b02a43aadb9b3689524bdd1051bac3 /src | |
parent | 860851e00fff7bb0024638b1c6c9283b6b7fdf2d (diff) |
Made SBC protocol more resilient
Fixed some bugs affecting SPI error recovery for SBC mode
Diffstat (limited to 'src')
-rw-r--r-- | src/SBC/DataTransfer.cpp | 110 | ||||
-rw-r--r-- | src/SBC/DataTransfer.h | 9 | ||||
-rw-r--r-- | src/SBC/SbcInterface.cpp | 11 |
3 files changed, 88 insertions, 42 deletions
diff --git a/src/SBC/DataTransfer.cpp b/src/SBC/DataTransfer.cpp index ddf488a5..f6fc1779 100644 --- a/src/SBC/DataTransfer.cpp +++ b/src/SBC/DataTransfer.cpp @@ -721,25 +721,44 @@ void DataTransfer::ExchangeData() noexcept setup_spi(rxBuffer, txBuffer, bytesToExchange); } -void DataTransfer::ResetTransfer(bool ownRequest) noexcept +void DataTransfer::RestartTransfer(bool ownRequest) noexcept { if (reprap.Debug(moduleSbcInterface)) { debugPrintf(ownRequest ? "Resetting transfer\n" : "Resetting transfer due to Sbc request\n"); } + failedTransfers++; if (ownRequest) { - // Invalidate the data to send - txResponse = TransferResponse::BadResponse; - Cache::FlushBeforeDMASend(&txResponse, sizeof(txResponse)); - state = InternalTransferState::Resetting; - setup_spi(&rxResponse, &txResponse, sizeof(uint32_t)); + if (rxHeader.dataLength > 0 || txPointer > 0) + { + // Transfer bad data response and restart the transfer + txResponse = TransferResponse::BadResponse; + Cache::FlushBeforeDMASend(&txResponse, sizeof(txResponse)); + state = InternalTransferState::Resetting; + setup_spi(&rxResponse, &txResponse, sizeof(uint32_t)); + } + else + { + // No data was transferred so we are still in sync. Continue with the next transfer + lastTransferNumber = rxHeader.sequenceNumber - 1; + ExchangeHeader(); + } } else { - // Sbc wants to reset the state - ExchangeHeader(); + if (state != InternalTransferState::ExchangingHeader) + { + // SBC wants to restart the transfer + ExchangeHeader(); + } + else + { + // Last data response exchange failed, try to perform it again + ExchangeResponse(TransferResponse::Success); + state = InternalTransferState::ExchangingDataResponseRetry; + } } } @@ -780,13 +799,17 @@ TransferState DataTransfer::DoTransfer() noexcept { case InternalTransferState::ExchangingHeader: { - Cache::InvalidateAfterDMAReceive(&rxHeader, sizeof(rxHeader)); // (1) Exchanged transfer headers + Cache::InvalidateAfterDMAReceive(&rxHeader, sizeof(rxHeader)); const uint32_t headerResponse = *reinterpret_cast<const uint32_t*>(&rxHeader); if (headerResponse == TransferResponse::BadResponse) { - // Sbc wants to restart the transfer - ResetTransfer(false); + // SBC received a bad response code. We must have been happy if we got here, else RRF would have complained + if (reprap.Debug(moduleSbcInterface)) + { + debugPrintf("Retrying data response exchange\n"); + } + RestartTransfer(false); break; } @@ -840,11 +863,6 @@ TransferState DataTransfer::DoTransfer() noexcept return IsConnectionReset() ? TransferState::connectionReset : TransferState::finished; } } - else if (rxResponse == TransferResponse::BadResponse) - { - // Sbc wants to restart the transfer - ResetTransfer(false); - } else if (rxResponse == TransferResponse::BadHeaderChecksum || txResponse == TransferResponse::BadHeaderChecksum) { // Failed to exchange header, restart the full transfer @@ -853,23 +871,18 @@ TransferState DataTransfer::DoTransfer() noexcept } else { - // Received invalid response code - ResetTransfer(true); + // Restart the full transfer + RestartTransfer(rxResponse != TransferResponse::BadResponse); } break; case InternalTransferState::ExchangingData: { - Cache::InvalidateAfterDMAReceive(rxBuffer, rxHeader.dataLength); // (3) Exchanged data + Cache::InvalidateAfterDMAReceive(rxBuffer, rxHeader.dataLength); if (*reinterpret_cast<uint32_t*>(rxBuffer) == TransferResponse::BadResponse) { - if (reprap.Debug(moduleSbcInterface)) - { - debugPrintf("Resetting state due to Sbc request\n"); - } - failedTransfers++; - ExchangeHeader(); + RestartTransfer(false); break; } @@ -889,7 +902,7 @@ TransferState DataTransfer::DoTransfer() noexcept } case InternalTransferState::ExchangingDataResponse: - // (4) Exchanged response to data transfer + // (4a) Exchanged response to data transfer Cache::InvalidateAfterDMAReceive(&rxResponse, sizeof(rxResponse)); if (rxResponse == TransferResponse::Success && txResponse == TransferResponse::Success) { @@ -900,29 +913,54 @@ TransferState DataTransfer::DoTransfer() noexcept return IsConnectionReset() ? TransferState::connectionReset : TransferState::finished; } - if (rxResponse == TransferResponse::BadResponse) - { - // Sbc wants to restart the transfer - ResetTransfer(false); - } - else if (rxResponse == TransferResponse::BadDataChecksum || txResponse == TransferResponse::BadDataChecksum) + if (rxResponse == TransferResponse::BadDataChecksum || txResponse == TransferResponse::BadDataChecksum) { // Resend the data if a checksum error occurred checksumErrors++; ExchangeData(); } + else if (rxResponse == TransferResponse::BadResponse) + { + // Restart the full transfer + RestartTransfer(false); + } else { - // Received invalid response, reset the SPI transfer - ResetTransfer(true); + // Restart the data response transfer + RestartTransfer(true); + state = InternalTransferState::ResettingDataResponse; + } + break; + + case InternalTransferState::ExchangingDataResponseRetry: + // (4b) Exchanged response to data transfer when new transfer is being started (fallback on bad response) + Cache::InvalidateAfterDMAReceive(&rxResponse, sizeof(rxResponse)); + if (rxResponse == TransferResponse::Success && txResponse == TransferResponse::Success) + { + // Retry succeeded + ExchangeHeader(); + } + else + { + // Retry failed, reset the connection + if (reprap.Debug(moduleSbcInterface)) + { + debugPrintf("Data response retry failed (sent %08" PRIx32 ", got %08" PRIx32 ")\n", txResponse, rxResponse); + } + return TransferState::connectionReset; } break; case InternalTransferState::Resetting: - // Transmitted bad response, attempt to start a new transfer + // Transmitted bad response, attempt to restart the connection ExchangeHeader(); break; + case InternalTransferState::ResettingDataResponse: + // Transmitted bad response after data response exchange, attempt to restart the data response exchange + ExchangeResponse(TransferResponse::Success); + break; + default: // Should never get here. If we do, this probably means that StartNextTransfer has not been called ExchangeHeader(); @@ -952,7 +990,7 @@ void DataTransfer::StartNextTransfer() noexcept txHeader.crcData = CalcCRC32(txBuffer, txPointer); txHeader.crcHeader = CalcCRC32(reinterpret_cast<const char *>(&txHeader), sizeof(TransferHeader) - sizeof(uint32_t)); - // Begin SPI transfe + // Begin SPI transfer ExchangeHeader(); } diff --git a/src/SBC/DataTransfer.h b/src/SBC/DataTransfer.h index bdc87bc9..3520ed41 100644 --- a/src/SBC/DataTransfer.h +++ b/src/SBC/DataTransfer.h @@ -97,8 +97,10 @@ private: ExchangingHeaderResponse, ExchangingData, ExchangingDataResponse, + ExchangingDataResponseRetry, ProcessingData, - Resetting + Resetting, + ResettingDataResponse } state; // Transfer properties @@ -137,7 +139,7 @@ private: void ExchangeHeader() noexcept; void ExchangeResponse(uint32_t response) noexcept; void ExchangeData() noexcept; - void ResetTransfer(bool ownRequest) noexcept; + void RestartTransfer(bool ownRequest) noexcept; uint32_t CalcCRC32(const char *buffer, size_t length) const noexcept; template<typename T> const T *ReadDataHeader() noexcept; @@ -155,8 +157,7 @@ private: inline bool DataTransfer::IsConnectionReset() const noexcept { - uint16_t nextTransferNumber = lastTransferNumber + 1; - return lastTransferNumber != 0 && (nextTransferNumber != rxHeader.sequenceNumber); + return lastTransferNumber != 0 && (lastTransferNumber + 1 != rxHeader.sequenceNumber); } inline size_t DataTransfer::PacketsToRead() const noexcept diff --git a/src/SBC/SbcInterface.cpp b/src/SBC/SbcInterface.cpp index bf6696de..7c039e95 100644 --- a/src/SBC/SbcInterface.cpp +++ b/src/SBC/SbcInterface.cpp @@ -154,8 +154,12 @@ void SbcInterface::Spin() noexcept { numSbcTimeouts++; } + reprap.GetPlatform().MessageF(NetworkInfoMessage, "Lost connection to SBC due to %s timeout\n", hadSbcTimeout ? "remote" : "local"); + } + else + { + reprap.GetPlatform().Message(NetworkInfoMessage, "Lost connection to SBC due to connection reset\n"); } - reprap.GetPlatform().MessageF(NetworkInfoMessage, "Lost connection to SBC due to %s timeout\n", hadSbcTimeout ? "remote" : "local"); // Invalidate local resources InvalidateResources(); @@ -176,7 +180,10 @@ void SbcInterface::Spin() noexcept } // Handle exchanged data and kick off the next transfer - ExchangeData(); + if (!hadReset) + { + ExchangeData(); + } transfer.StartNextTransfer(); } else if (hadTimeout || hadReset) |