Merge pull request 'str transition fixes' (#498) from bugfix_str_mode_transitions into develop

Reviewed-on: #498
2023-03-20 17:56:10 +01:00 · 2023-03-20 17:56:10 +01:00 · 2f990d8bbc
commit 2f990d8bbc
parent e349504b41 8482d56fd0
4 changed files with 166 additions and 126 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -23,6 +23,8 @@ will consitute of a breaking change warranting a new major release:

 ## Changed

+- Bugfixes for STR mode transitions: Booting to mode ON with submode FIRMWARE now works properly.
+  Furthermore, the submode in the NORMAL mode now should be 0 instead of some ON mode submode.
 - Updated GYR bias values to newest measurements. This also corrects the ADIS values to always
  consit of just one digit.

--- a/2
+++ b/2
@ -1 +1 @@
-Subproject commit 43fd0b2f59c3aeb2d3f4db10cfad56ee3709d68d
+Subproject commit 227524a21da755d125bcb1a5ff67bcbc452f8cf9
--- a/linux/devices/startracker/StarTrackerHandler.cpp
+++ b/linux/devices/startracker/StarTrackerHandler.cpp
@ -268,18 +268,18 @@ void StarTrackerHandler::doStartUp() {
    default:
      return;
  }
-  setMode(_MODE_TO_ON, SUBMODE_BOOTLOADER);
+  startupState = StartupState::DONE;
+  internalState = InternalState::IDLE;
+  setMode(_MODE_TO_ON);
 }

 void StarTrackerHandler::doShutDown() {
  // If the star tracker is shutdown also stop all running processes in the image loader task
  strHelper->stopProcess();
-  setMode(_MODE_POWER_DOWN);
-}
-
-void StarTrackerHandler::doOffActivity() {
-  startupState = StartupState::IDLE;
  internalState = InternalState::IDLE;
+  startupState = StartupState::IDLE;
+  bootState = FwBootState::NONE;
+  setMode(_MODE_POWER_DOWN);
 }

 ReturnValue_t StarTrackerHandler::buildNormalDeviceCommand(DeviceCommandId_t* id) {
@ -302,81 +302,103 @@ ReturnValue_t StarTrackerHandler::buildNormalDeviceCommand(DeviceCommandId_t* id

 ReturnValue_t StarTrackerHandler::buildTransitionDeviceCommand(DeviceCommandId_t* id) {
  switch (internalState) {
-    case InternalState::BOOT:
-      *id = startracker::BOOT;
-      bootCountdown.setTimeout(BOOT_TIMEOUT);
-      internalState = InternalState::BOOT_DELAY;
-      return buildCommandFromCommand(*id, nullptr, 0);
-    case InternalState::REQ_VERSION:
-      internalState = InternalState::VERIFY_BOOT;
-      // Again read program to check if firmware boot was successful
-      *id = startracker::REQ_VERSION;
-      return buildCommandFromCommand(*id, nullptr, 0);
-    case InternalState::LOGLEVEL:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::LOGLEVEL;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::LIMITS:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::LIMITS;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::TRACKING:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::TRACKING;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::MOUNTING:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::MOUNTING;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::IMAGE_PROCESSOR:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::IMAGE_PROCESSOR;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::CAMERA:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::CAMERA;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::CENTROIDING:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::CENTROIDING;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::LISA:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::LISA;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::MATCHING:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::MATCHING;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::VALIDATION:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::VALIDATION;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::ALGO:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::ALGO;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::LOG_SUBSCRIPTION:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::LOGSUBSCRIPTION;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
-    case InternalState::DEBUG_CAMERA:
-      internalState = InternalState::WAIT_FOR_EXECUTION;
-      *id = startracker::DEBUG_CAMERA;
-      return buildCommandFromCommand(*id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()),
-                                     paramJsonFile.size());
+    case InternalState::BOOT_FIRMWARE: {
+      if (bootState == FwBootState::WAIT_FOR_EXECUTION or bootState == FwBootState::VERIFY_BOOT) {
+        return NOTHING_TO_SEND;
+      }
+      if (bootState == FwBootState::NONE) {
+        *id = startracker::BOOT;
+        bootCountdown.setTimeout(BOOT_TIMEOUT);
+        bootState = FwBootState::BOOT_DELAY;
+        return buildCommandFromCommand(*id, nullptr, 0);
+      }
+      if (bootState == FwBootState::BOOT_DELAY) {
+        if (bootCountdown.isBusy()) {
+          return NOTHING_TO_SEND;
+        }
+        bootState = FwBootState::REQ_VERSION;
+      }
+      switch (bootState) {
+        case (FwBootState::REQ_VERSION): {
+          bootState = FwBootState::VERIFY_BOOT;
+          // Again read program to check if firmware boot was successful
+          *id = startracker::REQ_VERSION;
+          return buildCommandFromCommand(*id, nullptr, 0);
+        }
+        case (FwBootState::LOGLEVEL): {
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::LOGLEVEL;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        }
+        case (FwBootState::LIMITS): {
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::LIMITS;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        }
+        case (FwBootState::TRACKING): {
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::TRACKING;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        }
+        case FwBootState::MOUNTING:
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::MOUNTING;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        case FwBootState::IMAGE_PROCESSOR:
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::IMAGE_PROCESSOR;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        case FwBootState::CAMERA:
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::CAMERA;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        case FwBootState::CENTROIDING:
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::CENTROIDING;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        case FwBootState::LISA:
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::LISA;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        case FwBootState::MATCHING:
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::MATCHING;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        case FwBootState::VALIDATION:
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::VALIDATION;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        case FwBootState::ALGO:
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::ALGO;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        case FwBootState::LOG_SUBSCRIPTION:
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::LOGSUBSCRIPTION;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        case FwBootState::DEBUG_CAMERA:
+          bootState = FwBootState::WAIT_FOR_EXECUTION;
+          *id = startracker::DEBUG_CAMERA;
+          return buildCommandFromCommand(
+              *id, reinterpret_cast<const uint8_t*>(paramJsonFile.c_str()), paramJsonFile.size());
+        default: {
+          sif::error << "STR: Unexpected boot state" << (int)bootState << std::endl;
+          return NOTHING_TO_SEND;
+        }
+      }
+    }
    case InternalState::BOOT_BOOTLOADER:
      internalState = InternalState::BOOTLOADER_CHECK;
      *id = startracker::SWITCH_TO_BOOTLOADER_PROGRAM;
@ -707,6 +729,15 @@ void StarTrackerHandler::doTransition(Mode_t modeFrom, Submode_t subModeFrom) {

 void StarTrackerHandler::doOnTransition(Submode_t subModeFrom) {
  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
+  // transitions to firmware mode.
+  if (startupState == StartupState::DONE) {
+    subModeFrom = SUBMODE_BOOTLOADER;
+  }
+  if (dhbSubmode == SUBMODE_NONE) {
+    bootFirmware(MODE_ON);
+  }
  if (dhbSubmode == SUBMODE_BOOTLOADER && subModeFrom == SUBMODE_FIRMWARE) {
    bootBootloader();
  } else if (dhbSubmode == SUBMODE_FIRMWARE && subModeFrom == SUBMODE_FIRMWARE) {
@ -736,19 +767,23 @@ void StarTrackerHandler::doNormalTransition(Mode_t modeFrom, Submode_t subModeFr
 void StarTrackerHandler::bootFirmware(Mode_t toMode) {
  switch (internalState) {
    case InternalState::IDLE:
-      internalState = InternalState::BOOT;
+      sif::info << "STR: Booting to firmware mode" << std::endl;
+      internalState = InternalState::BOOT_FIRMWARE;
      break;
-    case InternalState::BOOT_DELAY:
-      if (bootCountdown.hasTimedOut()) {
-        internalState = InternalState::REQ_VERSION;
-      }
+    case InternalState::BOOT_FIRMWARE:
      break;
    case InternalState::FAILED_FIRMWARE_BOOT:
      internalState = InternalState::IDLE;
      break;
    case InternalState::DONE:
-      setMode(toMode);
+      if (toMode == MODE_NORMAL) {
+        setMode(toMode, 0);
+      } else {
+        setMode(toMode, SUBMODE_FIRMWARE);
+      }
+      sif::info << "STR: Firmware boot success" << std::endl;
      internalState = InternalState::IDLE;
+      startupState = StartupState::IDLE;
      break;
    default:
      return;
@ -776,10 +811,11 @@ void StarTrackerHandler::setUpJsonCfgs(JsonConfigs& cfgs, const char* paramJsonF
 void StarTrackerHandler::bootBootloader() {
  if (internalState == InternalState::IDLE) {
    internalState = InternalState::BOOT_BOOTLOADER;
-  } else if (internalState == InternalState::BOOTING_BOOTLOADER_FAILED) {
+  } else if (internalState == InternalState::FAILED_BOOTLOADER_BOOT) {
    internalState = InternalState::IDLE;
  } else if (internalState == InternalState::DONE) {
    internalState = InternalState::IDLE;
+    startupState = StartupState::IDLE;
    setMode(MODE_ON);
  }
 }
@ -1934,7 +1970,7 @@ ReturnValue_t StarTrackerHandler::checkProgram() {
      if (startupState == StartupState::WAIT_CHECK_PROGRAM) {
        startupState = StartupState::DONE;
      }
-      if (internalState == InternalState::VERIFY_BOOT) {
+      if (bootState == FwBootState::VERIFY_BOOT) {
        sif::warning << "StarTrackerHandler::checkProgram: Failed to boot firmware" << std::endl;
        // Device handler will run into timeout and fall back to transition source mode
        triggerEvent(BOOTING_FIRMWARE_FAILED_EVENT);
@ -1947,11 +1983,11 @@ ReturnValue_t StarTrackerHandler::checkProgram() {
      if (startupState == StartupState::WAIT_CHECK_PROGRAM) {
        startupState = StartupState::BOOT_BOOTLOADER;
      }
-      if (internalState == InternalState::VERIFY_BOOT) {
-        internalState = InternalState::LOGLEVEL;
+      if (bootState == FwBootState::VERIFY_BOOT) {
+        bootState = FwBootState::LOGLEVEL;
      } else if (internalState == InternalState::BOOTLOADER_CHECK) {
        triggerEvent(BOOTING_BOOTLOADER_FAILED_EVENT);
-        internalState = InternalState::BOOTING_BOOTLOADER_FAILED;
+        internalState = InternalState::FAILED_BOOTLOADER_BOOT;
      }
      break;
    default:
@ -2025,54 +2061,55 @@ ReturnValue_t StarTrackerHandler::handleActionReplySet(LocalPoolDataSetBase& dat
 void StarTrackerHandler::handleStartup(const uint8_t* parameterId) {
  switch (*parameterId) {
    case (startracker::ID::LOG_LEVEL): {
-      internalState = InternalState::LIMITS;
+      bootState = FwBootState::LIMITS;
      break;
    }
    case (startracker::ID::LIMITS): {
-      internalState = InternalState::TRACKING;
+      bootState = FwBootState::TRACKING;
      break;
    }
    case (startracker::ID::TRACKING): {
-      internalState = InternalState::MOUNTING;
+      bootState = FwBootState::MOUNTING;
      break;
    }
    case (startracker::ID::MOUNTING): {
-      internalState = InternalState::IMAGE_PROCESSOR;
+      bootState = FwBootState::IMAGE_PROCESSOR;
      break;
    }
    case (startracker::ID::IMAGE_PROCESSOR): {
-      internalState = InternalState::CAMERA;
+      bootState = FwBootState::CAMERA;
      break;
    }
    case (startracker::ID::CAMERA): {
-      internalState = InternalState::CENTROIDING;
+      bootState = FwBootState::CENTROIDING;
      break;
    }
    case (startracker::ID::CENTROIDING): {
-      internalState = InternalState::LISA;
+      bootState = FwBootState::LISA;
      break;
    }
    case (startracker::ID::LISA): {
-      internalState = InternalState::MATCHING;
+      bootState = FwBootState::MATCHING;
      break;
    }
    case (startracker::ID::MATCHING): {
-      internalState = InternalState::VALIDATION;
+      bootState = FwBootState::VALIDATION;
      break;
    }
    case (startracker::ID::VALIDATION): {
-      internalState = InternalState::ALGO;
+      bootState = FwBootState::ALGO;
      break;
    }
    case (startracker::ID::ALGO): {
-      internalState = InternalState::LOG_SUBSCRIPTION;
+      bootState = FwBootState::LOG_SUBSCRIPTION;
      break;
    }
    case (startracker::ID::LOG_SUBSCRIPTION): {
-      internalState = InternalState::DEBUG_CAMERA;
+      bootState = FwBootState::DEBUG_CAMERA;
      break;
    }
    case (startracker::ID::DEBUG_CAMERA): {
+      bootState = FwBootState::NONE;
      internalState = InternalState::DONE;
      break;
    }
--- a/linux/devices/startracker/StarTrackerHandler.h
+++ b/linux/devices/startracker/StarTrackerHandler.h
@ -60,7 +60,6 @@ class StarTrackerHandler : public DeviceHandlerBase {
 protected:
  void doStartUp() override;
  void doShutDown() override;
-  void doOffActivity() override;
  ReturnValue_t buildNormalDeviceCommand(DeviceCommandId_t* id) override;
  ReturnValue_t buildTransitionDeviceCommand(DeviceCommandId_t* id) override;
  void fillCommandAndReplyMap() override;
@ -247,14 +246,31 @@ class StarTrackerHandler : public DeviceHandlerBase {

  NormalState normalState = NormalState::TEMPERATURE_REQUEST;

+  enum class StartupState {
+    IDLE,
+    CHECK_PROGRAM,
+    WAIT_CHECK_PROGRAM,
+    BOOT_BOOTLOADER,
+    WAIT_JCFG,
+    DONE
+  };
+  StartupState startupState = StartupState::IDLE;
+
  enum class InternalState {
    IDLE,
-    BOOT,
+    BOOT_FIRMWARE,
+    DONE,
+    FAILED_FIRMWARE_BOOT,
+    BOOT_BOOTLOADER,
+    BOOTLOADER_CHECK,
+    FAILED_BOOTLOADER_BOOT
+  };
+
+  enum class FwBootState {
+    NONE,
+    BOOT_DELAY,
    REQ_VERSION,
    VERIFY_BOOT,
-    STARTUP_CHECK,
-    BOOT_DELAY,
-    FIRMWARE_CHECK,
    LOGLEVEL,
    LIMITS,
    TRACKING,
@ -270,26 +286,11 @@ class StarTrackerHandler : public DeviceHandlerBase {
    LOG_SUBSCRIPTION,
    DEBUG_CAMERA,
    WAIT_FOR_EXECUTION,
-    DONE,
-    FAILED_FIRMWARE_BOOT,
-    BOOT_BOOTLOADER,
-    BOOTLOADER_CHECK,
-    BOOTING_BOOTLOADER_FAILED
  };
+  FwBootState bootState = FwBootState::NONE;

  InternalState internalState = InternalState::IDLE;

-  enum class StartupState {
-    IDLE,
-    CHECK_PROGRAM,
-    WAIT_CHECK_PROGRAM,
-    BOOT_BOOTLOADER,
-    WAIT_JCFG,
-    DONE
-  };
-
-  StartupState startupState = StartupState::IDLE;
-
  bool strHelperExecuting = false;

  const power::Switch_t powerSwitch = power::NO_SWITCH;