refactor(config): new message for reboot behavior

use a new SetConfig message for the reboot behavior, better than reusing
the RebootOrb, which could
lead to unwanted behavior in case jetson binary
tried to set a config with a firmware not handling the new fields

Signed-off-by: Cyril Fougeray <cyril.fougeray@toolsforhumanity.com>

Author: fouge

main_board/src/cli/cli.c

@@ -807,17 +807,22 @@ execute_boot_config(const struct shell *sh, size_t argc, char **argv)
 {
     if (argc < 2) {
         /* No argument: print current config */
-        reboot_behavior_t behavior = config_get_reboot_behavior();
-        shell_print(sh, "boot config: %s",
-                    behavior == BOOT_ALWAYS ? "always_on" : "button");
+        orb_mcu_main_SetConfig_RebootBehavior behavior =
+            config_get_reboot_behavior();
+        shell_print(
+            sh, "boot config: %s",
+            behavior ==
+                    orb_mcu_main_SetConfig_RebootBehavior_BOOT_AUTO_ALWAYS_ON
+                ? "always_on"
+                : "button");
         return 0;
     }
 
-    reboot_behavior_t behavior;
+    orb_mcu_main_SetConfig_RebootBehavior behavior;
     if (strcmp(argv[1], "always_on") == 0) {
-        behavior = BOOT_ALWAYS;
+        behavior = orb_mcu_main_SetConfig_RebootBehavior_BOOT_AUTO_ALWAYS_ON;
     } else if (strcmp(argv[1], "button") == 0) {
-        behavior = BOOT_BUTTON;
+        behavior = orb_mcu_main_SetConfig_RebootBehavior_BOOT_BUTTON_PRESS;
     } else {
         shell_error(sh, "Usage: orb boot_config [button|always_on]");
         return -EINVAL;

main_board/src/power/boot/boot.c

@@ -812,7 +812,9 @@ app_init_state(void)
     /* read persistent boot behavior from flash config */
     ret = config_init();
     ASSERT_SOFT(ret);
-    config_auto_boot = (config_get_reboot_behavior() == BOOT_ALWAYS);
+    config_auto_boot =
+        (config_get_reboot_behavior() ==
+         orb_mcu_main_SetConfig_RebootBehavior_BOOT_AUTO_ALWAYS_ON);
 
     LOG_INF_IMM("insta-boot: %d, post ota: %d, auto-boot: %d, "
                 "config-auto: %d",

main_board/src/runner/runner.c

@@ -389,67 +389,63 @@ handle_reboot_orb(job_t *job)
     MAKE_ASSERTS(orb_mcu_main_JetsonToMcu_reboot_orb_tag);
 
     uint32_t delay = msg->payload.reboot_orb.force_reboot_timeout_s;
-    orb_mcu_main_RebootOrb_BootBehavior behavior =
-        msg->payload.reboot_orb.reboot_behavior;
 
     if (delay != 0 && (delay > 60 || delay < 10)) {
         job_ack(orb_mcu_Ack_ErrorCode_RANGE, job);
         LOG_ERR("Reboot with delay > 60s or < 10s: %u", delay);
     } else {
-        int ret = RET_SUCCESS;
-        // Persist the requested reboot behavior if explicitly set
-        // BOOT_REBOOT_NEXT (0 / default) means no change to the boot config
-        if (behavior ==
-            orb_mcu_main_RebootOrb_BootBehavior_BOOT_BUTTON_PRESS_ALWAYS) {
-            ret = config_set_reboot_behavior(BOOT_BUTTON);
-            if (ret != RET_SUCCESS) {
-                LOG_ERR("Failed to persist reboot behavior: %d", ret);
-            }
-        } else if (behavior ==
-                   orb_mcu_main_RebootOrb_BootBehavior_BOOT_AUTO_ALWAYS_ON) {
-            ret = config_set_reboot_behavior(BOOT_ALWAYS);
-            if (ret != RET_SUCCESS) {
-                LOG_ERR("Failed to persist reboot behavior: %d", ret);
+        int ret =
+            backup_regs_write_byte(REBOOT_FLAG_OFFSET_BYTE, REBOOT_INSTABOOT);
+        if (ret == 0) {
+            if (delay != 0) {
+                // force reboot after `delay` seconds,
+                // but a shutdown request from the Jetson is preferred
+                // (SHUTDOWN_REQ gpio)
+                ret = reboot(delay);
+            } else {
+                LOG_INF("waiting for reboot request from Jetson");
             }
-        } else if (behavior ==
-                   orb_mcu_main_RebootOrb_BootBehavior_BOOT_REBOOT_NEXT) {
-            // on BOOT_REBOOT_NEXT, we keep the next reboot state in bbram
-            // (won't persist in case of power loss)
-            ret = backup_regs_write_byte(REBOOT_FLAG_OFFSET_BYTE,
-                                         REBOOT_INSTABOOT);
-            if (ret == 0) {
-                if (delay != 0) {
-                    // force reboot after `delay` seconds,
-                    // but a shutdown request from the Jetson is preferred
-                    // (SHUTDOWN_REQ gpio)
-                    ret = reboot(delay);
-                } else {
-                    LOG_INF("waiting for reboot request from Jetson");
-                }
 
-                if (ret == 0) {
+            if (ret == 0) {
 #ifdef CONFIG_MEMFAULT
-                    MEMFAULT_REBOOT_MARK_RESET_IMMINENT(
-                        kMfltRebootReason_JetsonRequestedRebootOrb);
+                MEMFAULT_REBOOT_MARK_RESET_IMMINENT(
+                    kMfltRebootReason_JetsonRequestedRebootOrb);
 #endif
-                    job_ack(orb_mcu_Ack_ErrorCode_SUCCESS, job);
-                    return;
-                }
+                job_ack(orb_mcu_Ack_ErrorCode_SUCCESS, job);
+                return;
             }
-            // failure setting flag or initiating reboot
-            // reset flag
-            backup_regs_write_byte(REBOOT_FLAG_OFFSET_BYTE, 0);
-        } else {
-            ret = RET_ERROR_INVALID_PARAM;
         }
+    }
 
-        if (ret == 0) {
+    // failure setting flag or initiating reboot
+    // reset flag
+    backup_regs_write_byte(REBOOT_FLAG_OFFSET_BYTE, 0);
+    job_ack(orb_mcu_Ack_ErrorCode_FAIL, job);
+}
+
+static void
+handle_set_config(job_t *job)
+{
+    orb_mcu_main_JetsonToMcu *msg = &job->message.jetson_cmd;
+    MAKE_ASSERTS(orb_mcu_main_JetsonToMcu_set_config_tag);
+
+    switch (msg->payload.set_config.which_config) {
+    case orb_mcu_main_SetConfig_reboot_behavior_tag: {
+        int ret = config_set_reboot_behavior(
+            msg->payload.set_config.config.reboot_behavior);
+        if (ret == RET_SUCCESS) {
             job_ack(orb_mcu_Ack_ErrorCode_SUCCESS, job);
-        } else if (ret == RET_ERROR_INVALID_PARAM) {
-            job_ack(orb_mcu_Ack_ErrorCode_RANGE, job);
         } else {
+            LOG_ERR("Failed to persist reboot behavior: %d", ret);
             job_ack(orb_mcu_Ack_ErrorCode_FAIL, job);
         }
+        break;
+    }
+    default:
+        LOG_WRN("Unknown config field: %u",
+                msg->payload.set_config.which_config);
+        job_ack(orb_mcu_Ack_ErrorCode_RANGE, job);
+        break;
     }
 }
 
@@ -1824,6 +1820,7 @@ static const hm_callback handle_message_callbacks[] = {
     [orb_mcu_main_JetsonToMcu_set_time_tag] = handle_set_time,
     [orb_mcu_main_JetsonToMcu_reboot_orb_tag] = handle_reboot_orb,
     [orb_mcu_main_JetsonToMcu_boot_complete_tag] = handle_boot_complete,
+    [orb_mcu_main_JetsonToMcu_set_config_tag] = handle_set_config,
     [orb_mcu_main_JetsonToMcu_start_triggering_rgb_face_camera_tag] =
         handle_start_triggering_rgb_face_camera_message,
     [orb_mcu_main_JetsonToMcu_stop_triggering_rgb_face_camera_tag] =
@@ -1846,7 +1843,7 @@ static const hm_callback handle_message_callbacks[] = {
 #endif
 };
 
-BUILD_ASSERT((ARRAY_SIZE(handle_message_callbacks) <= 56),
+BUILD_ASSERT((ARRAY_SIZE(handle_message_callbacks) <= 57),
              "It seems like the `handle_message_callbacks` array is too large");
 
 _Noreturn static void

main_board/src/system/config/config.c

@@ -1,5 +1,6 @@
 #include "config.h"
 #include "orb_logs.h"
+#include "zephyr/toolchain.h"
 #include <errors.h>
 #include <storage.h>
 #include <string.h>
@@ -18,10 +19,14 @@ LOG_MODULE_REGISTER(config, CONFIG_CONFIG_LOG_LEVEL);
  * maintain backward compatibility. New fields in a larger record will
  * be zero-initialized when reading an older (smaller) config.
  */
-typedef struct {
-    reboot_behavior_t boot;
+typedef struct __PACKED {
+    orb_mcu_main_SetConfig_RebootBehavior boot;
+    uint8_t _pad[7]; /* padding to ensure memory alignment */
 } persistent_config_t;
 
+BUILD_ASSERT(sizeof(persistent_config_t) % FLASH_WRITE_BLOCK_SIZE == 0,
+             "persistent_config_t not aligned to flash write block size: ");
+
 static struct storage_area_s config_storage_area;
 static persistent_config_t current_config;
 static bool initialized;
@@ -125,15 +130,15 @@ config_init(void)
     return ret;
 }
 
-reboot_behavior_t
+orb_mcu_main_SetConfig_RebootBehavior
 config_get_reboot_behavior(void)
 {
     __ASSERT(initialized, "config module not initialized");
     return current_config.boot;
 }
 
 int
-config_set_reboot_behavior(reboot_behavior_t behavior)
+config_set_reboot_behavior(orb_mcu_main_SetConfig_RebootBehavior behavior)
 {
     k_mutex_lock(&config_mutex, K_FOREVER);
 

main_board/src/system/config/config.h

@@ -1,23 +1,6 @@
 #pragma once
 
-#include <stdint.h>
-
-/**
- * @brief Reboot behavior configuration
- *
- * Determines how the Orb boots after a reboot.
- */
-typedef enum {
-    /// Wait for button press before booting (default/nominal behavior)
-    BOOT_BUTTON = 0,
-    /// Always boot immediately without waiting for button press
-    BOOT_ALWAYS = 1,
-
-    /// Force enum to 32-bit width
-    _REBOOT_BEHAVIOR_FORCE_32BIT = 0x7FFFFFFF,
-} reboot_behavior_t;
-_Static_assert(sizeof(reboot_behavior_t) == 4,
-               "reboot_behavior_t must be 4 bytes");
+#include <main.pb.h>
 
 /**
  * Initialize the persistent config module.
@@ -38,7 +21,7 @@ config_init(void);
  *
  * @return current reboot behavior (BOOT_BUTTON if no config was persisted)
  */
-reboot_behavior_t
+orb_mcu_main_SetConfig_RebootBehavior
 config_get_reboot_behavior(void);
 
 /**
@@ -53,4 +36,4 @@ config_get_reboot_behavior(void);
  * @retval RET_ERROR_INTERNAL flash write failure
  */
 int
-config_set_reboot_behavior(reboot_behavior_t behavior);
+config_set_reboot_behavior(orb_mcu_main_SetConfig_RebootBehavior behavior);