From cbf5ca6d93028f46ba494bd2722984d33e250423 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 22 Dec 2022 17:17:23 +0100 Subject: [PATCH 1/8] minor fixes for MPSoC code --- .../devicedefinitions/PlocMPSoCDefinitions.h | 2 +- linux/devices/ploc/PlocMPSoCHandler.cpp | 26 +++---------------- linux/devices/ploc/PlocMPSoCHandler.h | 5 +--- 3 files changed, 5 insertions(+), 28 deletions(-) diff --git a/linux/devices/devicedefinitions/PlocMPSoCDefinitions.h b/linux/devices/devicedefinitions/PlocMPSoCDefinitions.h index 8a88a716..9cb609bf 100644 --- a/linux/devices/devicedefinitions/PlocMPSoCDefinitions.h +++ b/linux/devices/devicedefinitions/PlocMPSoCDefinitions.h @@ -152,7 +152,7 @@ class TcBase : public ploc::SpTcBase, public MPSoCReturnValuesIF { * sent and received packets. */ TcBase(ploc::SpTcParams params, uint16_t apid, uint16_t sequenceCount) - : ploc::SpTcBase(params, apid, sequenceCount) { + : ploc::SpTcBase(params, apid, 0, sequenceCount) { spParams.setFullPayloadLen(INIT_LENGTH); } diff --git a/linux/devices/ploc/PlocMPSoCHandler.cpp b/linux/devices/ploc/PlocMPSoCHandler.cpp index 28513314..2ac58335 100644 --- a/linux/devices/ploc/PlocMPSoCHandler.cpp +++ b/linux/devices/ploc/PlocMPSoCHandler.cpp @@ -184,12 +184,14 @@ void PlocMPSoCHandler::doShutDown() { powerState = PowerState::SHUTDOWN; break; case PowerState::OFF: + sequenceCount = 0; setMode(_MODE_POWER_DOWN); break; default: break; } #else + sequenceCount = 0; uartIsolatorSwitch.pullLow(); setMode(_MODE_POWER_DOWN); powerState = PowerState::OFF; @@ -340,7 +342,6 @@ ReturnValue_t PlocMPSoCHandler::scanForReply(const uint8_t* start, size_t remain } } - sequenceCount++; uint16_t recvSeqCnt = (*(start + 2) << 8 | *(start + 3)) & PACKET_SEQUENCE_COUNT_MASK; if (recvSeqCnt != sequenceCount) { triggerEvent(MPSOC_HANDLER_SEQUENCE_COUNT_MISMATCH, sequenceCount, recvSeqCnt); @@ -403,11 +404,9 @@ void PlocMPSoCHandler::handleEvent(EventMessage* eventMessage) { ReturnValue_t PlocMPSoCHandler::prepareTcMemWrite(const uint8_t* commandData, size_t commandDataLen) { ReturnValue_t result = returnvalue::OK; - sequenceCount++; mpsoc::TcMemWrite tcMemWrite(spParams, sequenceCount); result = tcMemWrite.buildPacket(commandData, commandDataLen); if (result != returnvalue::OK) { - sequenceCount--; return result; } finishTcPrep(tcMemWrite.getFullPacketLen()); @@ -417,11 +416,9 @@ ReturnValue_t PlocMPSoCHandler::prepareTcMemWrite(const uint8_t* commandData, ReturnValue_t PlocMPSoCHandler::prepareTcMemRead(const uint8_t* commandData, size_t commandDataLen) { ReturnValue_t result = returnvalue::OK; - sequenceCount++; mpsoc::TcMemRead tcMemRead(spParams, sequenceCount); result = tcMemRead.buildPacket(commandData, commandDataLen); if (result != returnvalue::OK) { - sequenceCount--; return result; } finishTcPrep(tcMemRead.getFullPacketLen()); @@ -435,12 +432,10 @@ ReturnValue_t PlocMPSoCHandler::prepareTcFlashDelete(const uint8_t* commandData, return MPSoCReturnValuesIF::NAME_TOO_LONG; } ReturnValue_t result = returnvalue::OK; - sequenceCount++; mpsoc::TcFlashDelete tcFlashDelete(spParams, sequenceCount); result = tcFlashDelete.buildPacket( std::string(reinterpret_cast(commandData), commandDataLen)); if (result != returnvalue::OK) { - sequenceCount--; return result; } finishTcPrep(tcFlashDelete.getFullPacketLen()); @@ -450,11 +445,9 @@ ReturnValue_t PlocMPSoCHandler::prepareTcFlashDelete(const uint8_t* commandData, ReturnValue_t PlocMPSoCHandler::prepareTcReplayStart(const uint8_t* commandData, size_t commandDataLen) { ReturnValue_t result = returnvalue::OK; - sequenceCount++; mpsoc::TcReplayStart tcReplayStart(spParams, sequenceCount); result = tcReplayStart.buildPacket(commandData, commandDataLen); if (result != returnvalue::OK) { - sequenceCount--; return result; } finishTcPrep(tcReplayStart.getFullPacketLen()); @@ -463,11 +456,9 @@ ReturnValue_t PlocMPSoCHandler::prepareTcReplayStart(const uint8_t* commandData, ReturnValue_t PlocMPSoCHandler::prepareTcReplayStop() { ReturnValue_t result = returnvalue::OK; - sequenceCount++; mpsoc::TcReplayStop tcReplayStop(spParams, sequenceCount); result = tcReplayStop.buildPacket(); if (result != returnvalue::OK) { - sequenceCount--; return result; } finishTcPrep(tcReplayStop.getFullPacketLen()); @@ -477,11 +468,9 @@ ReturnValue_t PlocMPSoCHandler::prepareTcReplayStop() { ReturnValue_t PlocMPSoCHandler::prepareTcDownlinkPwrOn(const uint8_t* commandData, size_t commandDataLen) { ReturnValue_t result = returnvalue::OK; - sequenceCount++; mpsoc::TcDownlinkPwrOn tcDownlinkPwrOn(spParams, sequenceCount); result = tcDownlinkPwrOn.buildPacket(commandData, commandDataLen); if (result != returnvalue::OK) { - sequenceCount--; return result; } finishTcPrep(tcDownlinkPwrOn.getFullPacketLen()); @@ -490,11 +479,9 @@ ReturnValue_t PlocMPSoCHandler::prepareTcDownlinkPwrOn(const uint8_t* commandDat ReturnValue_t PlocMPSoCHandler::prepareTcDownlinkPwrOff() { ReturnValue_t result = returnvalue::OK; - sequenceCount++; mpsoc::TcDownlinkPwrOff tcDownlinkPwrOff(spParams, sequenceCount); result = tcDownlinkPwrOff.buildPacket(); if (result != returnvalue::OK) { - sequenceCount--; return result; } finishTcPrep(tcDownlinkPwrOff.getFullPacketLen()); @@ -504,11 +491,9 @@ ReturnValue_t PlocMPSoCHandler::prepareTcDownlinkPwrOff() { ReturnValue_t PlocMPSoCHandler::prepareTcReplayWriteSequence(const uint8_t* commandData, size_t commandDataLen) { ReturnValue_t result = returnvalue::OK; - sequenceCount++; mpsoc::TcReplayWriteSeq tcReplayWriteSeq(spParams, sequenceCount); result = tcReplayWriteSeq.buildPacket(commandData, commandDataLen); if (result != returnvalue::OK) { - sequenceCount--; return result; } finishTcPrep(tcReplayWriteSeq.getFullPacketLen()); @@ -517,11 +502,9 @@ ReturnValue_t PlocMPSoCHandler::prepareTcReplayWriteSequence(const uint8_t* comm ReturnValue_t PlocMPSoCHandler::prepareTcModeReplay() { ReturnValue_t result = returnvalue::OK; - sequenceCount++; mpsoc::TcModeReplay tcModeReplay(spParams, sequenceCount); result = tcModeReplay.buildPacket(); if (result != returnvalue::OK) { - sequenceCount--; return result; } finishTcPrep(tcModeReplay.getFullPacketLen()); @@ -530,11 +513,9 @@ ReturnValue_t PlocMPSoCHandler::prepareTcModeReplay() { ReturnValue_t PlocMPSoCHandler::prepareTcModeIdle() { ReturnValue_t result = returnvalue::OK; - sequenceCount++; mpsoc::TcModeIdle tcModeIdle(spParams, sequenceCount); result = tcModeIdle.buildPacket(); if (result != returnvalue::OK) { - sequenceCount--; return result; } finishTcPrep(tcModeIdle.getFullPacketLen()); @@ -544,11 +525,9 @@ ReturnValue_t PlocMPSoCHandler::prepareTcModeIdle() { ReturnValue_t PlocMPSoCHandler::prepareTcCamCmdSend(const uint8_t* commandData, size_t commandDataLen) { ReturnValue_t result = returnvalue::OK; - sequenceCount++; mpsoc::TcCamcmdSend tcCamCmdSend(spParams, sequenceCount); result = tcCamCmdSend.buildPacket(commandData, commandDataLen); if (result != returnvalue::OK) { - sequenceCount--; return result; } finishTcPrep(tcCamCmdSend.getFullPacketLen()); @@ -560,6 +539,7 @@ void PlocMPSoCHandler::finishTcPrep(size_t packetLen) { nextReplyId = mpsoc::ACK_REPORT; rawPacket = commandBuffer; rawPacketLen = packetLen; + sequenceCount++; } ReturnValue_t PlocMPSoCHandler::verifyPacket(const uint8_t* start, size_t foundLen) { diff --git a/linux/devices/ploc/PlocMPSoCHandler.h b/linux/devices/ploc/PlocMPSoCHandler.h index a7d7a427..99a12515 100644 --- a/linux/devices/ploc/PlocMPSoCHandler.h +++ b/linux/devices/ploc/PlocMPSoCHandler.h @@ -107,10 +107,7 @@ class PlocMPSoCHandler : public DeviceHandlerBase, public CommandsActionsIF { MessageQueueIF* eventQueue = nullptr; MessageQueueIF* commandActionHelperQueue = nullptr; - // Initiate the sequence count with the maximum value. It is incremented before - // a packet is sent, so the first value will be 0 accordingly using - // the wrap around of the counter. - SourceSequenceCounter sequenceCount = SourceSequenceCounter(ccsds::LIMIT_SEQUENCE_COUNT - 1); + SourceSequenceCounter sequenceCount = SourceSequenceCounter(0); uint8_t commandBuffer[mpsoc::MAX_COMMAND_SIZE]; SpacePacketCreator creator; From 30aa802069019d325ad15361ed37dcaa7004edd2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 23 Dec 2022 10:13:26 +0100 Subject: [PATCH 2/8] minor udpates for PLOC SUPV code --- .../devicedefinitions/PlocSupervisorDefinitions.h | 10 +++------- linux/devices/ploc/PlocSupervisorHandler.cpp | 7 +++---- tmtc | 2 +- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/linux/devices/devicedefinitions/PlocSupervisorDefinitions.h b/linux/devices/devicedefinitions/PlocSupervisorDefinitions.h index cb0639fa..ffc310f0 100644 --- a/linux/devices/devicedefinitions/PlocSupervisorDefinitions.h +++ b/linux/devices/devicedefinitions/PlocSupervisorDefinitions.h @@ -730,17 +730,13 @@ class FactoryReset : public TcBase { : TcBase(params, Apid::DATA_LOGGER, static_cast(tc::DataLoggerServiceId::FACTORY_RESET), 0) {} - ReturnValue_t buildPacket(std::optional op) { - if (op) { - setLenFromPayloadLen(1); - } + ReturnValue_t buildPacket(uint8_t op) { + setLenFromPayloadLen(1); auto res = checkSizeAndSerializeHeader(); if (res != returnvalue::OK) { return res; } - if (op) { - payloadStart[0] = op.value(); - } + payloadStart[0] = op; return calcAndSetCrc(); } diff --git a/linux/devices/ploc/PlocSupervisorHandler.cpp b/linux/devices/ploc/PlocSupervisorHandler.cpp index 08c63b1a..74ea8780 100644 --- a/linux/devices/ploc/PlocSupervisorHandler.cpp +++ b/linux/devices/ploc/PlocSupervisorHandler.cpp @@ -1467,11 +1467,10 @@ ReturnValue_t PlocSupervisorHandler::prepareReadGpioCmd(const uint8_t* commandDa ReturnValue_t PlocSupervisorHandler::prepareFactoryResetCmd(const uint8_t* commandData, size_t len) { FactoryReset resetCmd(spParams); - std::optional op; - if (len > 0) { - op = commandData[0]; + if (len < 1) { + return HasActionsIF::INVALID_PARAMETERS; } - ReturnValue_t result = resetCmd.buildPacket(op); + ReturnValue_t result = resetCmd.buildPacket(commandData[0]); if (result != returnvalue::OK) { return result; } diff --git a/tmtc b/tmtc index 00991b92..20c2f615 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit 00991b92f15425aa80d2448ad304de46a08b5470 +Subproject commit 20c2f615555e5e7ddc03a2a22e225f75dff1c320 From 0163791e26884b05a79dfea7b4ffc6518b9469d1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 23 Dec 2022 10:17:54 +0100 Subject: [PATCH 3/8] bump tmtc --- tmtc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tmtc b/tmtc index 20c2f615..825a1972 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit 20c2f615555e5e7ddc03a2a22e225f75dff1c320 +Subproject commit 825a1972c4ca63f8044eff582a97b4221d513fec From 5e1d0543d7a521c89680eec1c2f05633c55e0317 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 23 Dec 2022 10:32:21 +0100 Subject: [PATCH 4/8] afmt --- linux/devices/ploc/PlocSupervisorHandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/devices/ploc/PlocSupervisorHandler.cpp b/linux/devices/ploc/PlocSupervisorHandler.cpp index 74ea8780..54af97ec 100644 --- a/linux/devices/ploc/PlocSupervisorHandler.cpp +++ b/linux/devices/ploc/PlocSupervisorHandler.cpp @@ -1468,7 +1468,7 @@ ReturnValue_t PlocSupervisorHandler::prepareFactoryResetCmd(const uint8_t* comma size_t len) { FactoryReset resetCmd(spParams); if (len < 1) { - return HasActionsIF::INVALID_PARAMETERS; + return HasActionsIF::INVALID_PARAMETERS; } ReturnValue_t result = resetCmd.buildPacket(commandData[0]); if (result != returnvalue::OK) { From 04b9b035025cd8f89dc27579d9ddfe261ecc23a4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 23 Dec 2022 11:00:28 +0100 Subject: [PATCH 5/8] bump tmtc --- tmtc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tmtc b/tmtc index 825a1972..5c675560 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit 825a1972c4ca63f8044eff582a97b4221d513fec +Subproject commit 5c675560eadadfbb5e674d9be87c206df09d1771 From cfae090de4c40c775d0fef574093780a5e1f65f9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 23 Dec 2022 11:46:01 +0100 Subject: [PATCH 6/8] improvements for ploc supv SW --- linux/devices/ploc/PlocSupervisorHandler.cpp | 38 +++++++++++--------- linux/devices/ploc/PlocSupervisorHandler.h | 8 ++--- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/linux/devices/ploc/PlocSupervisorHandler.cpp b/linux/devices/ploc/PlocSupervisorHandler.cpp index 54af97ec..17657b50 100644 --- a/linux/devices/ploc/PlocSupervisorHandler.cpp +++ b/linux/devices/ploc/PlocSupervisorHandler.cpp @@ -136,29 +136,34 @@ ReturnValue_t PlocSupervisorHandler::executeAction(ActionId_t actionId, } void PlocSupervisorHandler::doStartUp() { - if (setTimeDuringStartup) { - if (startupState == StartupState::OFF) { - bootTimeout.resetTimer(); - startupState = StartupState::BOOTING; - } - if (startupState == StartupState::BOOTING) { - if (bootTimeout.hasTimedOut()) { - uartIsolatorSwitch.pullHigh(); - uartManager.start(); + if (startupState == StartupState::OFF) { + bootTimeout.resetTimer(); + startupState = StartupState::BOOTING; + } + if (startupState == StartupState::BOOTING) { + if (bootTimeout.hasTimedOut()) { + uartIsolatorSwitch.pullHigh(); + uartManager.start(); + if (SET_TIME_DURING_BOOT) { startupState = StartupState::SET_TIME; + } else { + startupState = StartupState::ON; } } - if (startupState == StartupState::ON) { - setMode(_MODE_TO_ON); - } - } else { - uartIsolatorSwitch.pullHigh(); + } + if (startupState == StartupState::SET_TIME_EXECUTING) { + startupState = StartupState::ON; + } + if (startupState == StartupState::ON) { setMode(_MODE_TO_ON); } } void PlocSupervisorHandler::doShutDown() { setMode(_MODE_POWER_DOWN); + shutdownCmdSent = false; + packetInBuffer = false; + nextReplyId = supv::NONE; uartManager.stop(); uartIsolatorSwitch.pullLow(); startupState = StartupState::OFF; @@ -1896,9 +1901,8 @@ ReturnValue_t PlocSupervisorHandler::handleExecutionSuccessReport(ExecutionRepor break; } case supv::SET_TIME_REF: { - if (startupState == StartupState::SET_TIME_EXECUTING) { - startupState = StartupState::ON; - } + // We could only allow proper bootup when the time was set successfully, but + // this makes debugging difficult. break; } default: diff --git a/linux/devices/ploc/PlocSupervisorHandler.h b/linux/devices/ploc/PlocSupervisorHandler.h index 4ffe4166..85934cdc 100644 --- a/linux/devices/ploc/PlocSupervisorHandler.h +++ b/linux/devices/ploc/PlocSupervisorHandler.h @@ -97,11 +97,11 @@ class PlocSupervisorHandler : public DeviceHandlerBase { static const uint32_t COPY_ADC_TO_MRAM_TIMEOUT = 70000; // 60 s static const uint32_t MRAM_DUMP_TIMEOUT = 60000; - // 2 s - static const uint32_t BOOT_TIMEOUT = 2000; + // 5 s + static const uint32_t BOOT_TIMEOUT = 5000; enum class StartupState : uint8_t { OFF, BOOTING, SET_TIME, SET_TIME_EXECUTING, ON }; - bool setTimeDuringStartup = true; + static constexpr bool SET_TIME_DURING_BOOT = true; StartupState startupState = StartupState::OFF; @@ -138,8 +138,6 @@ class PlocSupervisorHandler : public DeviceHandlerBase { uint32_t receivedMramDumpPackets = 0; /** Set to true as soon as a complete space packet is present in the spacePacketBuffer */ bool packetInBuffer = false; - /** Points to the next free position in the space packet buffer */ - uint16_t bufferTop = 0; /** This buffer is used to concatenate space packets received in two different read steps */ uint8_t spacePacketBuffer[supv::MAX_PACKET_SIZE]; From 8fedaa43e909c9061728db0eafb82090cc225ba3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 23 Dec 2022 12:14:01 +0100 Subject: [PATCH 7/8] important bugfix for PLOC SUPV --- linux/devices/ploc/PlocSupervisorHandler.h | 4 ++-- linux/devices/ploc/PlocSupvUartMan.cpp | 12 ++++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/linux/devices/ploc/PlocSupervisorHandler.h b/linux/devices/ploc/PlocSupervisorHandler.h index 85934cdc..52e2619b 100644 --- a/linux/devices/ploc/PlocSupervisorHandler.h +++ b/linux/devices/ploc/PlocSupervisorHandler.h @@ -97,8 +97,8 @@ class PlocSupervisorHandler : public DeviceHandlerBase { static const uint32_t COPY_ADC_TO_MRAM_TIMEOUT = 70000; // 60 s static const uint32_t MRAM_DUMP_TIMEOUT = 60000; - // 5 s - static const uint32_t BOOT_TIMEOUT = 5000; + // 4 s + static const uint32_t BOOT_TIMEOUT = 4000; enum class StartupState : uint8_t { OFF, BOOTING, SET_TIME, SET_TIME_EXECUTING, ON }; static constexpr bool SET_TIME_DURING_BOOT = true; diff --git a/linux/devices/ploc/PlocSupvUartMan.cpp b/linux/devices/ploc/PlocSupvUartMan.cpp index a246d97c..1fd2b841 100644 --- a/linux/devices/ploc/PlocSupvUartMan.cpp +++ b/linux/devices/ploc/PlocSupvUartMan.cpp @@ -100,6 +100,7 @@ ReturnValue_t PlocSupvUartManager::performOperation(uint8_t operationCode) { state = InternalState::SLEEPING; lock->unlockMutex(); semaphore->acquire(); + putTaskToSleep = false; while (true) { if (putTaskToSleep) { performUartShutdown(); @@ -295,11 +296,14 @@ void PlocSupvUartManager::stop() { } void PlocSupvUartManager::start() { - MutexGuard mg(lock); - if (state != InternalState::SLEEPING and state != InternalState::GO_TO_SLEEP) { - return; + { + MutexGuard mg(lock); + if (state != InternalState::SLEEPING and state != InternalState::GO_TO_SLEEP) { + return; + } + state = InternalState::DEFAULT; } - state = InternalState::DEFAULT; + semaphore->release(); } From 2531de507aedde1995b8e3283ac4ba1f1cf2dfed Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 23 Dec 2022 12:18:44 +0100 Subject: [PATCH 8/8] bump changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7f27fc7..21bdd758 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ list yields a list of all related PRs for each release. # [unreleased] +## Fixed + +- PLOC SUPV: Minor adaptions and important bugfix for UART manager + ## Added - First version of ACS controller