From d824f8ba9e378b8615afcf82a77e35ad75159ef5 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 22 Mar 2023 10:38:41 +0100 Subject: [PATCH] some more improvements for DLL --- CHANGELOG.md | 14 ++++++ .../startracker/ArcsecDatalinkLayer.cpp | 50 ++++--------------- .../devices/startracker/ArcsecDatalinkLayer.h | 29 +++++++---- linux/devices/startracker/StrComHandler.cpp | 5 +- thirdparty/arcsec_star_tracker | 2 +- 5 files changed, 46 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b1c6de0..71bf5f6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,20 @@ will consitute of a breaking change warranting a new major release: - Bugfix for STR: Some action commands wrongfully declined. - STR: No normal command handling while a special request like an image upload is active. +- STR: Fix weird issues on datalink layer data reception which sometimes occur. + +## Changed + +- STR: Move datalink layer to `StrComHandler` completely. DLL is now completely hidden from + device handler. +- STR: Is now scheduled twice in ACS PST. +- `StrHelper` renamed to `StrComHandler`, is now a `DeviceHandlerIF` directly and does not wrap + a separate UART COM interface anymore. + +## Added + +- Add `reset` function for `ArcsecDatalinkLayer` and use it in `StrComHandler` before going to sleep + to ensure consistent state of reply reception. # [v1.39.0] 2023-03-21 diff --git a/linux/devices/startracker/ArcsecDatalinkLayer.cpp b/linux/devices/startracker/ArcsecDatalinkLayer.cpp index 7269341a..eb62cec1 100644 --- a/linux/devices/startracker/ArcsecDatalinkLayer.cpp +++ b/linux/devices/startracker/ArcsecDatalinkLayer.cpp @@ -1,16 +1,16 @@ #include "ArcsecDatalinkLayer.h" -ArcsecDatalinkLayer::ArcsecDatalinkLayer() : decodeRingBuf(4096, true) { slipInit(); } +ArcsecDatalinkLayer::ArcsecDatalinkLayer() : decodeRingBuf(BUFFER_LENGTHS, true) { slipInit(); } ArcsecDatalinkLayer::~ArcsecDatalinkLayer() {} ReturnValue_t ArcsecDatalinkLayer::checkRingBufForFrame(const uint8_t** decodedFrame, size_t& frameLen) { size_t currentLen = decodeRingBuf.getAvailableReadData(); - decodeRingBuf.readData(decodedRxFrame, currentLen); + decodeRingBuf.readData(rxAnalysisBuffer, currentLen); for (size_t idx = 0; idx < currentLen; idx++) { enum arc_dec_result decResult = - arc_transport_decode_body(decodedRxFrame[idx], &slipInfo, decodedRxFrame, &rxFrameSize); + arc_transport_decode_body(rxAnalysisBuffer[idx], &slipInfo, decodedRxFrame, &rxFrameSize); switch (decResult) { case ARC_DEC_INPROGRESS: { break; @@ -52,46 +52,14 @@ ReturnValue_t ArcsecDatalinkLayer::feedData(const uint8_t* rawData, size_t rawDa return decodeRingBuf.writeData(rawData, rawDataLen); } -void ArcsecDatalinkLayer::slipInit() { - slipInfo.buffer = rxBuffer; - slipInfo.maxlength = startracker::MAX_FRAME_SIZE; - slipInfo.length = 0; - slipInfo.unescape_next = 0; - slipInfo.prev_state = SLIP_COMPLETE; +void ArcsecDatalinkLayer::reset() { + slipInit(); + decodeRingBuf.clear(); } -// ReturnValue_t ArcsecDatalinkLayer::decodeFrame(const uint8_t* rawData, size_t rawDataSize, -// size_t* bytesLeft) { -// size_t bytePos = 0; -// for (bytePos = 0; bytePos < rawDataSize; bytePos++) { -// enum arc_dec_result decResult = -// arc_transport_decode_body(*(rawData + bytePos), &slipInfo, decodedRxFrame, &rxFrameSize); -// *bytesLeft = rawDataSize - bytePos - 1; -// switch (decResult) { -// case ARC_DEC_INPROGRESS: { -// if (bytePos == rawDataSize - 1) { -// return DEC_IN_PROGRESS; -// } -// continue; -// } -// case ARC_DEC_ERROR_FRAME_SHORT: -// return REPLY_TOO_SHORT; -// case ARC_DEC_ERROR_CHECKSUM: -// return CRC_FAILURE; -// case ARC_DEC_ASYNC: -// case ARC_DEC_SYNC: { -// // Reset length of SLIP struct for next frame -// slipInfo.length = 0; -// return returnvalue::OK; -// } -// default: -// sif::debug << "ArcsecDatalinkLayer::decodeFrame: Unknown result code" << std::endl; -// break; -// return returnvalue::FAILED; -// } -// } -// return returnvalue::FAILED; -// } +void ArcsecDatalinkLayer::slipInit() { + slip_decode_init(rxBufferArc, sizeof(rxBufferArc), &slipInfo); +} void ArcsecDatalinkLayer::encodeFrame(const uint8_t* data, size_t length, const uint8_t** txFrame, size_t& size) { diff --git a/linux/devices/startracker/ArcsecDatalinkLayer.h b/linux/devices/startracker/ArcsecDatalinkLayer.h index 1831360f..bfc82d90 100644 --- a/linux/devices/startracker/ArcsecDatalinkLayer.h +++ b/linux/devices/startracker/ArcsecDatalinkLayer.h @@ -25,6 +25,8 @@ class ArcsecDatalinkLayer { static const uint8_t STATUS_OK = 0; + static constexpr size_t BUFFER_LENGTHS = 4096; + ArcsecDatalinkLayer(); virtual ~ArcsecDatalinkLayer(); @@ -36,16 +38,17 @@ class ArcsecDatalinkLayer { */ ReturnValue_t feedData(const uint8_t* rawData, size_t rawDataLen); - ReturnValue_t checkRingBufForFrame(const uint8_t** decodedFrame, size_t& frameLen); - /** - * @brief Applies decoding to data referenced by rawData pointer - * TODO: To be deleted soon, replaced by proper buffering. - * @param rawData Pointer to raw data received from star tracker - * @param rawDataSize Size of raw data stream - * @param remainingBytes Number of bytes left + * Runs the arcsec datalink layer decoding algorithm on the data in the ring buffer, decoding + * frames in the process. + * @param decodedFrame + * @param frameLen + * @return + * - returnvalue::OK if a frame was found + * - DEC_IN_PROGRESS if frame decoding is in progress + * - Anything else is a decoding error */ - // ReturnValue_t decodeFrame(const uint8_t* rawData, size_t rawDataSize, size_t* bytesLeft); + ReturnValue_t checkRingBufForFrame(const uint8_t** decodedFrame, size_t& frameLen); /** * @brief SLIP encodes data pointed to by data pointer. @@ -55,15 +58,19 @@ class ArcsecDatalinkLayer { */ void encodeFrame(const uint8_t* data, size_t length, const uint8_t** txFrame, size_t& frameLen); + void reset(); + private: static const uint8_t ID_OFFSET = 1; static const uint8_t STATUS_OFFSET = 2; - // Used by arcsec slip decoding function process received data - uint8_t rxBuffer[startracker::MAX_FRAME_SIZE]; + // User to buffer and analyse data and allow feeding and checking for frames asychronously. SimpleRingBuffer decodeRingBuf; - uint8_t rxAnalysisBuffer[4096]; + uint8_t rxAnalysisBuffer[BUFFER_LENGTHS]; + // Used by arcsec slip decoding function to process received data. This should only be written + // to or read from by arcsec functions! + uint8_t rxBufferArc[startracker::MAX_FRAME_SIZE]; // Decoded frame will be copied to this buffer uint8_t decodedRxFrame[startracker::MAX_FRAME_SIZE]; // Size of decoded frame diff --git a/linux/devices/startracker/StrComHandler.cpp b/linux/devices/startracker/StrComHandler.cpp index dfa917c3..85be7871 100644 --- a/linux/devices/startracker/StrComHandler.cpp +++ b/linux/devices/startracker/StrComHandler.cpp @@ -42,6 +42,7 @@ ReturnValue_t StrComHandler::performOperation(uint8_t operationCode) { while (true) { lock->lockMutex(); state = InternalState::SLEEPING; + datalinkLayer.reset(); lock->unlockMutex(); semaphore.acquire(); switch (state) { @@ -154,7 +155,9 @@ ReturnValue_t StrComHandler::startImageDownload(std::string path) { return returnvalue::OK; } -void StrComHandler::stopProcess() { terminate = true; } +void StrComHandler::stopProcess() { + terminate = true; +} void StrComHandler::setDownloadImageName(std::string filename) { downloadImage.filename = filename; diff --git a/thirdparty/arcsec_star_tracker b/thirdparty/arcsec_star_tracker index c535e149..cbb3b24d 160000 --- a/thirdparty/arcsec_star_tracker +++ b/thirdparty/arcsec_star_tracker @@ -1 +1 @@ -Subproject commit c535e1494f2fdb54becd7c338fe959c3672298b3 +Subproject commit cbb3b24dc1993b727735fd63576088401ba351ec