From d42b6798e0f5de706de75686d48fb44def65d0da Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 11 Apr 2023 17:29:32 +0200 Subject: [PATCH 1/6] better reply result handling --- CHANGELOG.md | 1 + linux/acs/AcsBoardPolling.cpp | 20 ++++++++++++++++---- linux/acs/SusPolling.cpp | 4 ++-- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc5bfc7b..57d12f2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ will consitute of a breaking change warranting a new major release: SD card switch from 0 to 1 and vice-versa works without errors from persistent TM stores now. - Add a way for the SUS polling to detect broken or off devices by checking the retrieved temperature for the all-ones value (0x0fff). +- Better reply result handling for the ACS board devices. ## Changed diff --git a/linux/acs/AcsBoardPolling.cpp b/linux/acs/AcsBoardPolling.cpp index b289ef70..175bd693 100644 --- a/linux/acs/AcsBoardPolling.cpp +++ b/linux/acs/AcsBoardPolling.cpp @@ -127,6 +127,7 @@ ReturnValue_t AcsBoardPolling::sendMessage(CookieIF* cookie, const uint8_t* send adis.ownReply.cfgWasSet = false; adis.ownReply.dataWasSet = false; } + adis.replyResult = returnvalue::FAILED; adis.mode = req->mode; } return returnvalue::OK; @@ -145,6 +146,7 @@ ReturnValue_t AcsBoardPolling::sendMessage(CookieIF* cookie, const uint8_t* send } else { gyro.ownReply.cfgWasSet = false; } + gyro.replyResult = returnvalue::FAILED; gyro.mode = req->mode; } return returnvalue::OK; @@ -163,6 +165,7 @@ ReturnValue_t AcsBoardPolling::sendMessage(CookieIF* cookie, const uint8_t* send mgm.ownReply.dataWasSet = false; mgm.ownReply.temperatureWasSet = false; } + mgm.replyResult = returnvalue::FAILED; mgm.mode = req->mode; } return returnvalue::OK; @@ -180,6 +183,7 @@ ReturnValue_t AcsBoardPolling::sendMessage(CookieIF* cookie, const uint8_t* send } else { mgm.ownReply.dataWasRead = false; } + mgm.replyResult = returnvalue::FAILED; mgm.mode = req->mode; } return returnvalue::OK; @@ -309,18 +313,18 @@ void AcsBoardPolling::gyroL3gHandler(GyroL3g& l3g) { std::memcpy(cmdBuf.data() + 1, l3g.sensorCfg, 5); result = spiComIF.sendMessage(l3g.cookie, cmdBuf.data(), 6); if (result != returnvalue::OK) { - l3g.replyResult = returnvalue::OK; + l3g.replyResult = result; } // Ignore useless reply and red config cmdBuf[0] = l3gd20h::CTRL_REG_1 | l3gd20h::AUTO_INCREMENT_MASK | l3gd20h::READ_MASK; std::memset(cmdBuf.data() + 1, 0, 5); result = spiComIF.sendMessage(l3g.cookie, cmdBuf.data(), 6); if (result != returnvalue::OK) { - l3g.replyResult = returnvalue::OK; + l3g.replyResult = result; } result = spiComIF.readReceivedMessage(l3g.cookie, &rawReply, &dummy); if (result != returnvalue::OK) { - l3g.replyResult = returnvalue::OK; + l3g.replyResult = result; } MutexGuard mg(ipcLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX); // Cross check configuration as verification that communication is working @@ -331,6 +335,7 @@ void AcsBoardPolling::gyroL3gHandler(GyroL3g& l3g) { return; } } + l3g.replyResult = returnvalue::OK; l3g.performStartup = false; l3g.ownReply.cfgWasSet = true; l3g.ownReply.sensitivity = l3gd20h::ctrlReg4ToSensitivity(l3g.sensorCfg[3]); @@ -357,6 +362,7 @@ void AcsBoardPolling::gyroL3gHandler(GyroL3g& l3g) { return; } } + l3g.replyResult = returnvalue::OK; l3g.ownReply.statusReg = rawReply[l3gd20h::STATUS_IDX]; l3g.ownReply.angVelocities[0] = (rawReply[l3gd20h::OUT_X_H] << 8) | rawReply[l3gd20h::OUT_X_L]; l3g.ownReply.angVelocities[1] = (rawReply[l3gd20h::OUT_Y_H] << 8) | rawReply[l3gd20h::OUT_Y_L]; @@ -495,6 +501,7 @@ void AcsBoardPolling::gyroAdisHandler(GyroAdis& gyro) { gyro.ownReply.cfg.prodId = prodId; gyro.ownReply.data.sensitivity = adis1650x::rangMdlToSensitivity(gyro.ownReply.cfg.rangMdl); gyro.performStartup = false; + gyro.replyResult = returnvalue::OK; } // Read regular registers std::memcpy(cmdBuf.data(), adis1650x::BURST_READ_ENABLE.data(), @@ -533,6 +540,7 @@ void AcsBoardPolling::gyroAdisHandler(GyroAdis& gyro) { } MutexGuard mg(ipcLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX); + gyro.replyResult = returnvalue::OK; gyro.ownReply.dataWasSet = true; gyro.ownReply.cfg.diagStat = rawReply[2] << 8 | rawReply[3]; gyro.ownReply.data.angVelocities[0] = (rawReply[4] << 8) | rawReply[5]; @@ -590,6 +598,7 @@ void AcsBoardPolling::mgmLis3Handler(MgmLis3& mgm) { } // Done here. We can always read back config and data during periodic handling mgm.performStartup = false; + mgm.replyResult = returnvalue::OK; } cmdBuf[0] = mgmLis3::readCommand(mgmLis3::CTRL_REG1, true); std::memset(cmdBuf.data() + 1, 0, mgmLis3::NR_OF_DATA_AND_CFG_REGISTERS); @@ -607,7 +616,7 @@ void AcsBoardPolling::mgmLis3Handler(MgmLis3& mgm) { // Verify communication by re-checking config if (rawReply[1] != mgm.cfg[0] or rawReply[2] != mgm.cfg[1] or rawReply[3] != mgm.cfg[2] or rawReply[4] != mgm.cfg[3] or rawReply[5] != mgm.cfg[4]) { - mgm.replyResult = result; + mgm.replyResult = returnvalue::FAILED; return; } { @@ -634,6 +643,7 @@ void AcsBoardPolling::mgmLis3Handler(MgmLis3& mgm) { return; } MutexGuard mg(ipcLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX); + mgm.replyResult = returnvalue::OK; mgm.ownReply.temperatureWasSet = true; mgm.ownReply.temperatureRaw = (rawReply[2] << 8) | rawReply[1]; } @@ -704,6 +714,7 @@ void AcsBoardPolling::mgmRm3100Handler(MgmRm3100& mgm) { return; } mgm.performStartup = false; + mgm.replyResult = returnvalue::OK; } // Regular read operation cmdBuf[0] = mgmRm3100::MEASUREMENT_REG_START | mgmRm3100::READ_MASK; @@ -725,6 +736,7 @@ void AcsBoardPolling::mgmRm3100Handler(MgmRm3100& mgm) { mgm.ownReply.scaleFactors[idx] = 1.0 / mgmRm3100::DEFAULT_GAIN; } mgm.ownReply.dataWasRead = true; + mgm.replyResult = returnvalue::OK; // Bitshift trickery to account for 24 bit signed value. mgm.ownReply.mgmValuesRaw[0] = ((rawReply[1] << 24) | (rawReply[2] << 16) | (rawReply[3] << 8)) >> 8; diff --git a/linux/acs/SusPolling.cpp b/linux/acs/SusPolling.cpp index 0e09b02a..1da5ef5e 100644 --- a/linux/acs/SusPolling.cpp +++ b/linux/acs/SusPolling.cpp @@ -96,7 +96,7 @@ ReturnValue_t SusPolling::readReceivedMessage(CookieIF* cookie, uint8_t** buffer if (susIdx < 0) { return FAILED; } - if(susDevs[susIdx].replyResult != returnvalue::OK) { + if (susDevs[susIdx].replyResult != returnvalue::OK) { return susDevs[susIdx].replyResult; } MutexGuard mg(ipcLock); @@ -170,7 +170,7 @@ ReturnValue_t SusPolling::handleSusPolling() { susDevs[idx].ownReply.tempRaw = ((rawReply[0] & 0x0f) << 8) | rawReply[1]; // Reply is all ones. Sensor is probably off or faulty when // it should not be. - if(susDevs[idx].ownReply.tempRaw == 0x0fff) { + if (susDevs[idx].ownReply.tempRaw == 0x0fff) { susDevs[idx].replyResult = returnvalue::FAILED; } else { susDevs[idx].replyResult = returnvalue::OK; From 776a53b2437abc545a5685826bf874b58fd66ddb Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 11 Apr 2023 18:03:07 +0200 Subject: [PATCH 2/6] dont send anything in break CD --- mission/acs/GyrAdis1650XHandler.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mission/acs/GyrAdis1650XHandler.cpp b/mission/acs/GyrAdis1650XHandler.cpp index 57033e80..147b9e57 100644 --- a/mission/acs/GyrAdis1650XHandler.cpp +++ b/mission/acs/GyrAdis1650XHandler.cpp @@ -56,6 +56,9 @@ ReturnValue_t GyrAdis1650XHandler::buildNormalDeviceCommand(DeviceCommandId_t *i ReturnValue_t GyrAdis1650XHandler::buildTransitionDeviceCommand(DeviceCommandId_t *id) { switch (internalState) { case (InternalState::STARTUP): { + if(breakCountdown.isBusy()) { + return NOTHING_TO_SEND; + } *id = adis1650x::REQUEST; return preparePeriodicRequest(acs::SimpleSensorMode::NORMAL); } From 7c36660000cd974909ff674cdedb085730b0a676 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 11 Apr 2023 18:15:12 +0200 Subject: [PATCH 3/6] that was nasty --- linux/acs/AcsBoardPolling.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/linux/acs/AcsBoardPolling.cpp b/linux/acs/AcsBoardPolling.cpp index 175bd693..938d0e15 100644 --- a/linux/acs/AcsBoardPolling.cpp +++ b/linux/acs/AcsBoardPolling.cpp @@ -31,7 +31,7 @@ ReturnValue_t AcsBoardPolling::performOperation(uint8_t operationCode) { ipcLock->unlockMutex(); semaphore->acquire(); // Give all tasks or the PST some time to submit all consecutive requests. - TaskFactory::delayTask(2); + TaskFactory::delayTask(3); { // Measured to take 0-1 ms in debug build. // Stopwatch watch; @@ -113,7 +113,8 @@ ReturnValue_t AcsBoardPolling::sendMessage(CookieIF* cookie, const uint8_t* send if (req->mode != adis.mode) { if (req->mode == acs::SimpleSensorMode::NORMAL) { adis.type = req->type; - adis.countdown.setTimeout(adis1650x::START_UP_TIME); + // The initial countdown is handled by the device handler now. + // adis.countdown.setTimeout(adis1650x::START_UP_TIME); if (adis.type == adis1650x::Type::ADIS16507) { adis.ownReply.data.accelScaling = adis1650x::ACCELEROMETER_RANGE_16507; } else if (adis.type == adis1650x::Type::ADIS16505) { @@ -449,20 +450,15 @@ ReturnValue_t AcsBoardPolling::readAdisCfg(SpiCookie& cookie, size_t transferLen void AcsBoardPolling::gyroAdisHandler(GyroAdis& gyro) { ReturnValue_t result; acs::SimpleSensorMode mode = acs::SimpleSensorMode::OFF; - bool cdHasTimedOut = false; bool mustPerformStartup = false; { MutexGuard mg(ipcLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX); mode = gyro.mode; - cdHasTimedOut = gyro.countdown.hasTimedOut(); mustPerformStartup = gyro.performStartup; } if (mode == acs::SimpleSensorMode::OFF) { return; } - if (not cdHasTimedOut) { - return; - } if (mustPerformStartup) { uint8_t regList[6]; // Read configuration From 5925de94e73ec215c930e4b08780f81339750e42 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 11 Apr 2023 18:16:58 +0200 Subject: [PATCH 4/6] changelog --- CHANGELOG.md | 1 + mission/acs/GyrAdis1650XHandler.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57d12f2c..857cd052 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ will consitute of a breaking change warranting a new major release: - Add a way for the SUS polling to detect broken or off devices by checking the retrieved temperature for the all-ones value (0x0fff). - Better reply result handling for the ACS board devices. +- ADIS1650X initial timeout handling now performed in device handler now. ## Changed diff --git a/mission/acs/GyrAdis1650XHandler.cpp b/mission/acs/GyrAdis1650XHandler.cpp index 147b9e57..b9e0b049 100644 --- a/mission/acs/GyrAdis1650XHandler.cpp +++ b/mission/acs/GyrAdis1650XHandler.cpp @@ -56,7 +56,7 @@ ReturnValue_t GyrAdis1650XHandler::buildNormalDeviceCommand(DeviceCommandId_t *i ReturnValue_t GyrAdis1650XHandler::buildTransitionDeviceCommand(DeviceCommandId_t *id) { switch (internalState) { case (InternalState::STARTUP): { - if(breakCountdown.isBusy()) { + if (breakCountdown.isBusy()) { return NOTHING_TO_SEND; } *id = adis1650x::REQUEST; From ff28b628a420d9b24653419e171a0a1958f5a405 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 11 Apr 2023 18:17:30 +0200 Subject: [PATCH 5/6] leave that value --- linux/acs/AcsBoardPolling.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/acs/AcsBoardPolling.cpp b/linux/acs/AcsBoardPolling.cpp index 938d0e15..bc3dff74 100644 --- a/linux/acs/AcsBoardPolling.cpp +++ b/linux/acs/AcsBoardPolling.cpp @@ -31,7 +31,7 @@ ReturnValue_t AcsBoardPolling::performOperation(uint8_t operationCode) { ipcLock->unlockMutex(); semaphore->acquire(); // Give all tasks or the PST some time to submit all consecutive requests. - TaskFactory::delayTask(3); + TaskFactory::delayTask(2); { // Measured to take 0-1 ms in debug build. // Stopwatch watch; From b3f5e7460935d4c1890f7d22226dd2377120bb08 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 11 Apr 2023 18:18:33 +0200 Subject: [PATCH 6/6] typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 857cd052..9956115d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ will consitute of a breaking change warranting a new major release: - Add a way for the SUS polling to detect broken or off devices by checking the retrieved temperature for the all-ones value (0x0fff). - Better reply result handling for the ACS board devices. -- ADIS1650X initial timeout handling now performed in device handler now. +- ADIS1650X initial timeout handling now performed in device handler. ## Changed