CHANGELOG.md

@@ -20,6 +20,10 @@ will consitute of a breaking change warranting a new major release:
 
 - Adapted HK data rates to new table for LEOP SAFE mode.
 - GPS controller HK is now generated periodically as well.
+- Better mode combination checks for assembly components. This includes:
+  - IMTQ assembly
+  - Syrlinks assembly
+  - Dual Lane Assembly
 - RWs are no longer commanded by the ACS Controller during safe mode. Instead the RW speed command
   is set to 0 as part or the `doShutDown` of the RW handler.
 

mission/acs/str/StarTrackerHandler.cpp

@@ -277,7 +277,7 @@ void StarTrackerHandler::performOperationHook() {
   }
 }
 
-Submode_t StarTrackerHandler::getInitialSubmode() { return SUBMODE_BOOTLOADER; }
+Submode_t StarTrackerHandler::getInitialSubmode() { return startracker::SUBMODE_BOOTLOADER; }
 
 ReturnValue_t StarTrackerHandler::buildNormalDeviceCommand(DeviceCommandId_t* id) {
   if (strHelperHandlingSpecialRequest) {
@@ -705,7 +705,7 @@ ReturnValue_t StarTrackerHandler::isModeCombinationValid(Mode_t mode, Submode_t
         return INVALID_SUBMODE;
       }
     case MODE_ON:
-      if (submode == SUBMODE_BOOTLOADER || submode == SUBMODE_FIRMWARE) {
+      if (submode == startracker::SUBMODE_BOOTLOADER || submode == startracker::SUBMODE_FIRMWARE) {
         return returnvalue::OK;
       } else {
         return INVALID_SUBMODE;
@@ -736,6 +736,7 @@ void StarTrackerHandler::doTransition(Mode_t modeFrom, Submode_t subModeFrom) {
 }
 
 void StarTrackerHandler::doOnTransition(Submode_t subModeFrom) {
+  using namespace startracker;
   uint8_t dhbSubmode = getSubmode();
   // We hide that the transition to submode firmware actually goes through the submode bootloader.
   // This is because the startracker always starts in bootloader mode but we want to allow direct
@@ -762,6 +763,7 @@ void StarTrackerHandler::doOnTransition(Submode_t subModeFrom) {
 }
 
 void StarTrackerHandler::doNormalTransition(Mode_t modeFrom, Submode_t subModeFrom) {
+  using namespace startracker;
   if (subModeFrom == SUBMODE_FIRMWARE) {
     setMode(MODE_NORMAL);
   } else if (subModeFrom == SUBMODE_BOOTLOADER) {
@@ -787,7 +789,7 @@ void StarTrackerHandler::bootFirmware(Mode_t toMode) {
       if (toMode == MODE_NORMAL) {
         setMode(toMode, 0);
       } else {
-        setMode(toMode, SUBMODE_FIRMWARE);
+        setMode(toMode, startracker::SUBMODE_FIRMWARE);
       }
       sif::info << "STR: Firmware boot success" << std::endl;
       internalState = InternalState::IDLE;

mission/acs/str/StarTrackerHandler.h

@@ -54,9 +54,6 @@ class StarTrackerHandler : public DeviceHandlerBase {
 
   Submode_t getInitialSubmode() override;
 
-  static const Submode_t SUBMODE_BOOTLOADER = 1;
-  static const Submode_t SUBMODE_FIRMWARE = 2;
-
  protected:
   void doStartUp() override;
   void doShutDown() override;

mission/acs/str/strHelpers.h

@@ -11,6 +11,9 @@
 
 namespace startracker {
 
+static const Submode_t SUBMODE_BOOTLOADER = 1;
+static const Submode_t SUBMODE_FIRMWARE = 2;
+
 /**
  * @brief    Returns the frame type field of a decoded frame.
  */

mission/system/acs/AcsBoardAssembly.cpp

@@ -256,10 +256,11 @@ ReturnValue_t AcsBoardAssembly::initialize() {
   return AssemblyBase::initialize();
 }
 
-ReturnValue_t AcsBoardAssembly::checkAndHandleHealthStates(Mode_t deviceMode,
+ReturnValue_t AcsBoardAssembly::checkAndHandleHealthStates(Mode_t commandedMode,
-                                                           Submode_t deviceSubmode) {
+                                                           Submode_t commandedSubmode) {
   using namespace returnvalue;
   ReturnValue_t status = returnvalue::OK;
+  bool healthNeedsToBeOverwritten = false;
   auto checkAcsBoardSensorGroup = [&](object_id_t o0, object_id_t o1, object_id_t o2,
                                       object_id_t o3) {
     HealthState h0 = healthHelper.healthTable->getHealth(o0);
@@ -272,6 +273,7 @@ ReturnValue_t AcsBoardAssembly::checkAndHandleHealthStates(Mode_t deviceMode,
       overwriteDeviceHealth(o1, h1);
       overwriteDeviceHealth(o2, h2);
       overwriteDeviceHealth(o3, h3);
+      healthNeedsToBeOverwritten = true;
     }
     if (h0 == EXTERNAL_CONTROL or h1 == EXTERNAL_CONTROL or h2 == EXTERNAL_CONTROL or
         h3 == EXTERNAL_CONTROL) {
@@ -285,6 +287,7 @@ ReturnValue_t AcsBoardAssembly::checkAndHandleHealthStates(Mode_t deviceMode,
   if (healthHelper.healthTable->getHealth(helper.healthDevGps0) == PERMANENT_FAULTY and
       healthHelper.healthTable->getHealth(helper.healthDevGps1) == FAULTY) {
     overwriteDeviceHealth(helper.healthDevGps1, FAULTY);
+    healthNeedsToBeOverwritten = true;
   } else if (healthHelper.healthTable->getHealth(helper.healthDevGps1) == PERMANENT_FAULTY and
              healthHelper.healthTable->getHealth(helper.healthDevGps0) == FAULTY) {
     overwriteDeviceHealth(helper.healthDevGps0, FAULTY);
@@ -294,14 +297,24 @@ ReturnValue_t AcsBoardAssembly::checkAndHandleHealthStates(Mode_t deviceMode,
                           healthHelper.healthTable->getHealth(helper.healthDevGps0));
     overwriteDeviceHealth(helper.healthDevGps1,
                           healthHelper.healthTable->getHealth(helper.healthDevGps1));
+    healthNeedsToBeOverwritten = true;
   }
 
-  if (deviceSubmode == duallane::DUAL_MODE) {
+  if (commandedSubmode == duallane::DUAL_MODE) {
     checkAcsBoardSensorGroup(helper.mgm0Lis3IdSideA, helper.mgm1Rm3100IdSideA,
                              helper.mgm2Lis3IdSideB, helper.mgm3Rm3100IdSideB);
     checkAcsBoardSensorGroup(helper.gyro0AdisIdSideA, helper.gyro1L3gIdSideA,
+
                              helper.gyro2AdisIdSideB, helper.gyro3L3gIdSideB);
   }
+  if (healthNeedsToBeOverwritten) {
+    // If we are overwriting the health states, we are already in a transition to dual mode,
+    // and we would like that transition to complete. The default behaviour is to go back to the
+    // old mode. We force our behaviour by overwriting the internal modes.
+    mode = commandedMode;
+    submode = commandedSubmode;
+    return NEED_TO_CHANGE_HEALTH;
+  }
   return status;
 }
 

mission/system/acs/DualLaneAssemblyBase.cpp

@@ -36,6 +36,8 @@ void DualLaneAssemblyBase::performChildOperation() {
 void DualLaneAssemblyBase::startTransition(Mode_t mode, Submode_t submode) {
   using namespace duallane;
   pwrStateMachine.reset();
+  dualToSingleSideTransition = false;
+  sideSwitchState = SideSwitchState::NONE;
 
   if (mode != MODE_OFF) {
     // Special exception: A transition from dual side to single mode must be handled like
@@ -112,8 +114,11 @@ ReturnValue_t DualLaneAssemblyBase::pwrStateMachineWrapper() {
 
 ReturnValue_t DualLaneAssemblyBase::isModeCombinationValid(Mode_t mode, Submode_t submode) {
   using namespace duallane;
-  if (submode != A_SIDE and submode != B_SIDE and submode != DUAL_MODE) {
+  if (mode != MODE_OFF and (submode != A_SIDE and submode != B_SIDE and submode != DUAL_MODE)) {
-    return returnvalue::FAILED;
+    return HasModesIF::INVALID_SUBMODE;
+  }
+  if(mode == MODE_OFF and submode != SUBMODE_NONE) {
+    return HasModesIF::INVALID_SUBMODE;
   }
   return returnvalue::OK;
 }

mission/system/acs/DualLaneAssemblyBase.h

@@ -70,7 +70,8 @@ class DualLaneAssemblyBase : public AssemblyBase, public ConfirmsFailuresIF {
    * @param submode
    */
   virtual ReturnValue_t pwrStateMachineWrapper();
-  virtual ReturnValue_t isModeCombinationValid(Mode_t mode, Submode_t submode) override;
+  ReturnValue_t isModeCombinationValid(Mode_t mode, Submode_t submode) override;
+
   /**
    * Custom recovery implementation to ensure that the power lines are commanded off for a
    * recovery.

mission/system/acs/ImtqAssembly.cpp

@@ -35,9 +35,12 @@ ReturnValue_t ImtqAssembly::checkChildrenStateOn(Mode_t wantedMode, Submode_t wa
 
 ReturnValue_t ImtqAssembly::isModeCombinationValid(Mode_t mode, Submode_t submode) {
   if (mode == MODE_ON or mode == DeviceHandlerIF::MODE_NORMAL or mode == MODE_OFF) {
+    if(submode != SUBMODE_NONE) {
+      return HasModesIF::INVALID_SUBMODE;
+    }
     return returnvalue::OK;
   }
-  return returnvalue::FAILED;
+  return HasModesIF::INVALID_MODE;
 }
 
 ReturnValue_t ImtqAssembly::checkAndHandleHealthState(Mode_t deviceMode, Submode_t deviceSubmode) {

mission/system/acs/StrAssembly.cpp

@@ -2,6 +2,8 @@
 
 #include <eive/objects.h>
 
+#include "mission/acs/str/strHelpers.h"
+
 StrAssembly::StrAssembly(object_id_t objectId) : AssemblyBase(objectId) {
   ModeListEntry entry;
   entry.setObject(objects::STAR_TRACKER);
@@ -31,5 +33,12 @@ ReturnValue_t StrAssembly::checkChildrenStateOn(Mode_t wantedMode, Submode_t wan
 }
 
 ReturnValue_t StrAssembly::isModeCombinationValid(Mode_t mode, Submode_t submode) {
+  if ((mode == DeviceHandlerIF::MODE_NORMAL or mode == MODE_OFF) and submode != SUBMODE_NONE) {
+    return HasModesIF::INVALID_SUBMODE;
+  }
+  if (mode == MODE_ON and
+      (submode != startracker::SUBMODE_BOOTLOADER and submode != startracker::SUBMODE_FIRMWARE)) {
+    return HasModesIF::INVALID_SUBMODE;
+  }
   return returnvalue::OK;
 }

mission/system/acs/SusAssembly.cpp

@@ -26,8 +26,8 @@ ReturnValue_t SusAssembly::commandChildren(Mode_t mode, Submode_t submode) {
   }
   if (recoveryState == RecoveryState::RECOVERY_IDLE) {
     result = checkAndHandleHealthStates(mode, submode);
-    if (result == NEED_TO_CHANGE_HEALTH) {
+    if (result != returnvalue::OK) {
-      return returnvalue::OK;
+      return result;
     }
   }
   if (recoveryState != RecoveryState::RECOVERY_STARTED) {
@@ -47,26 +47,9 @@ ReturnValue_t SusAssembly::handleNormalOrOnModeCmd(Mode_t mode, Submode_t submod
   bool needsSecondStep = false;
   handleSideSwitchStates(submode, needsSecondStep);
   auto cmdSeq = [&](object_id_t objectId, Mode_t devMode, uint8_t tableIdx) {
-    if (mode == devMode) {
+    if (isModeCommandable(objectId, devMode)) {
       modeTable[tableIdx].setMode(mode);
-    } else if (mode == DeviceHandlerIF::MODE_NORMAL) {
+      modeTable[tableIdx].setSubmode(SUBMODE_NONE);
-      if (isUseable(objectId, devMode)) {
-        if (devMode == MODE_ON) {
-          modeTable[tableIdx].setMode(mode);
-          modeTable[tableIdx].setSubmode(SUBMODE_NONE);
-        } else {
-          modeTable[tableIdx].setMode(MODE_ON);
-          modeTable[tableIdx].setSubmode(SUBMODE_NONE);
-          if (internalState != STATE_SECOND_STEP) {
-            needsSecondStep = true;
-          }
-        }
-      }
-    } else if (mode == MODE_ON) {
-      if (isUseable(objectId, devMode)) {
-        modeTable[tableIdx].setMode(MODE_ON);
-        modeTable[tableIdx].setSubmode(SUBMODE_NONE);
-      }
     }
   };
   switch (submode) {
@@ -134,38 +117,31 @@ ReturnValue_t SusAssembly::initialize() {
   return AssemblyBase::initialize();
 }
 
-bool SusAssembly::isUseable(object_id_t object, Mode_t mode) {
-  if (healthHelper.healthTable->isFaulty(object)) {
-    return false;
-  }
-
-  // Check if device is already in target mode
-  if (childrenMap[object].mode == mode) {
-    return true;
-  }
-
-  if (healthHelper.healthTable->isCommandable(object)) {
-    return true;
-  }
-  return false;
-}
-
 void SusAssembly::refreshHelperModes() {
   for (uint8_t idx = 0; idx < helper.susModes.size(); idx++) {
     helper.susModes[idx] = childrenMap[helper.susIds[idx]].mode;
   }
 }
 
-ReturnValue_t SusAssembly::checkAndHandleHealthStates(Mode_t deviceMode, Submode_t deviceSubmode) {
+ReturnValue_t SusAssembly::checkAndHandleHealthStates(Mode_t commandedMode,
+                                                      Submode_t commandedSubmode) {
   using namespace returnvalue;
   ReturnValue_t status = returnvalue::OK;
+  bool needsHealthOverwritten = false;
   auto checkSusGroup = [&](object_id_t devNom, object_id_t devRed) {
     HealthState healthNom = healthHelper.healthTable->getHealth(devNom);
     HealthState healthRed = healthHelper.healthTable->getHealth(devRed);
-    if ((healthNom == FAULTY or healthNom == PERMANENT_FAULTY) and
+    if (healthNom == PERMANENT_FAULTY and healthRed == FAULTY) {
-        (healthRed == FAULTY or healthRed == PERMANENT_FAULTY)) {
+      overwriteDeviceHealth(devRed, healthRed);
+      needsHealthOverwritten = true;
+    } else if (healthNom == FAULTY and healthRed == PERMANENT_FAULTY) {
+      overwriteDeviceHealth(devNom, healthNom);
+      needsHealthOverwritten = true;
+    } else if ((healthNom == FAULTY or healthNom == PERMANENT_FAULTY) and
+               (healthRed == FAULTY or healthRed == PERMANENT_FAULTY)) {
       overwriteDeviceHealth(devNom, healthNom);
       overwriteDeviceHealth(devRed, healthRed);
+      needsHealthOverwritten = true;
     }
   };
   auto checkHealthForOneDev = [&](object_id_t dev) {
@@ -174,7 +150,7 @@ ReturnValue_t SusAssembly::checkAndHandleHealthStates(Mode_t deviceMode, Submode
       modeHelper.setForced(true);
     }
   };
-  if (deviceSubmode == duallane::DUAL_MODE) {
+  if (commandedSubmode == duallane::DUAL_MODE) {
     uint8_t idx = 0;
     for (idx = 0; idx < 6; idx++) {
       checkSusGroup(helper.susIds[idx], helper.susIds[idx + 6]);
@@ -184,5 +160,12 @@ ReturnValue_t SusAssembly::checkAndHandleHealthStates(Mode_t deviceMode, Submode
       checkHealthForOneDev(helper.susIds[idx]);
     }
   }
+  if (needsHealthOverwritten) {
+    mode = commandedMode;
+    submode = commandedSubmode;
+    // We need second step instead of NEED_TO_CHANGE_HEALTH because we do not want recovery
+    // handling.
+    return NEED_TO_CHANGE_HEALTH;
+  }
   return status;
 }

mission/system/acs/SusAssembly.h

@@ -56,13 +56,6 @@ class SusAssembly : public DualLaneAssemblyBase {
   ReturnValue_t commandChildren(Mode_t mode, Submode_t submode) override;
   ReturnValue_t checkChildrenStateOn(Mode_t wantedMode, Submode_t wantedSubmode) override;
 
-  /**
-   * Check whether it makes sense to send mode commands to the device
-   * @param object
-   * @param mode
-   * @return
-   */
-  bool isUseable(object_id_t object, Mode_t mode);
   void powerStateMachine(Mode_t mode, Submode_t submode);
   ReturnValue_t handleNormalOrOnModeCmd(Mode_t mode, Submode_t submode);
   void refreshHelperModes();

mission/system/com/SyrlinksAssembly.cpp

@@ -1,6 +1,7 @@
 #include "SyrlinksAssembly.h"
 
 #include <eive/objects.h>
+#include <mission/com/defs.h>
 
 using namespace returnvalue;
 
@@ -34,10 +35,16 @@ ReturnValue_t SyrlinksAssembly::checkChildrenStateOn(Mode_t wantedMode, Submode_
 }
 
 ReturnValue_t SyrlinksAssembly::isModeCombinationValid(Mode_t mode, Submode_t submode) {
-  if (mode == MODE_ON or mode == DeviceHandlerIF::MODE_NORMAL or mode == MODE_OFF) {
+  if (mode == MODE_ON or mode == DeviceHandlerIF::MODE_NORMAL) {
+    if (submode >= com::Submode::NUM_SUBMODES or submode < com::Submode::RX_ONLY) {
+      return HasModesIF::INVALID_SUBMODE;
+    }
     return returnvalue::OK;
   }
-  return returnvalue::FAILED;
+  if(mode == MODE_OFF and submode != SUBMODE_NONE) {
+    return HasModesIF::INVALID_SUBMODE;
+  }
+  return returnvalue::OK;
 }
 
 ReturnValue_t SyrlinksAssembly::checkAndHandleHealthState(Mode_t deviceMode,

mission/system/objects/PowerStateMachineBase.cpp

@@ -1,5 +1,7 @@
 #include "PowerStateMachineBase.h"
 
+#include "fsfw/serviceinterface.h"
+
 PowerStateMachineBase::PowerStateMachineBase(PowerSwitchIF *pwrSwitcher, dur_millis_t checkTimeout)
     : pwrSwitcher(pwrSwitcher), checkTimeout(checkTimeout) {}
 

tmtc

@@ -1 +1 @@
-Subproject commit 50668ca7a74edd4219456e393cd10f7858591130
+Subproject commit dcf7d0af71f6ba9d569f9f56604e9245a0233427