Merge pull request 'systemctl helper' (#596) from feature_systemctl_helper into develop

Reviewed-on: #596
Reviewed-by: Marius Eggert <eggertm@irs.uni-stuttgart.de>

CHANGELOG.md

@@ -25,6 +25,7 @@ will consitute of a breaking change warranting a new major release:
   bytes were not written properly.
 - Bugfix for STR where an invalid reply was received for special requests
   like firmware updates.
+- Bugfix for shell command executors in command controller which lead to a crash.
 
 ## Added
 

bsp_q7s/core/CoreController.cpp

@@ -9,7 +9,6 @@
 #include "fsfw/serviceinterface/ServiceInterface.h"
 #include "fsfw/timemanager/Stopwatch.h"
 #include "fsfw/version.h"
-#include "mission/sysDefs.h"
 #include "watchdog/definitions.h"
 #if OBSW_ADD_TMTC_UDP_SERVER == 1
 #include "fsfw/osal/common/UdpTmTcBridge.h"
@@ -118,7 +117,7 @@ void CoreController::performControlOperation() {
     bool replyReceived = false;
     // TODO: We could read the data in the ring buffer and send it as an action data reply.
     if (cmdExecutor.check(replyReceived) == CommandExecutor::EXECUTION_FINISHED) {
-      actionHelper.finish(true, successRecipient, core::EXECUTE_SHELL_CMD);
+      actionHelper.finish(true, successRecipient, core::EXECUTE_SHELL_CMD_BLOCKING);
       shellCmdIsExecuting = false;
       cmdReplyBuf.clear();
       while (not cmdRepliesSizes.empty()) {
@@ -304,6 +303,41 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_
       // Completion will be reported by SD card state machine
       return returnvalue::OK;
     }
+    case (SYSTEMCTL_CMD_EXECUTOR): {
+      // Expect one byte systemctl command type and a unit name with at least one byte as minimum.
+      if (size < 2) {
+        return HasActionsIF::INVALID_PARAMETERS;
+      }
+      if (data[0] >= core::SystemctlCmd::NUM_CMDS) {
+        return HasActionsIF::INVALID_PARAMETERS;
+      }
+      core::SystemctlCmd cmdType = static_cast<core::SystemctlCmd>(data[0]);
+      std::string unitName = std::string(reinterpret_cast<const char *>(data + 1), size - 1);
+      std::ostringstream oss("systemctl ", std::ostringstream::ate);
+      switch (cmdType) {
+        case (core::SystemctlCmd::START): {
+          oss << "start ";
+          break;
+        }
+        case (core::SystemctlCmd::STOP): {
+          oss << "stop ";
+          break;
+        }
+        case (core::SystemctlCmd::RESTART): {
+          oss << "restart ";
+          break;
+        }
+        default: {
+          return HasActionsIF::INVALID_PARAMETERS;
+        }
+      }
+      oss << unitName;
+      int result = std::system(oss.str().c_str());
+      if (result != 0) {
+        return returnvalue::FAILED;
+      }
+      return EXECUTION_FINISHED;
+    }
     case (SWITCH_IMG_LOCK): {
       if (size != 3) {
         return HasActionsIF::INVALID_PARAMETERS;
@@ -334,13 +368,22 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_
       // Warning: This function will never return, because it reboots the system
       return actionReboot(data, size);
     }
-    case (EXECUTE_SHELL_CMD): {
-      std::string cmd = std::string(cmd, size);
+    case (EXECUTE_SHELL_CMD_BLOCKING): {
+      std::string cmdToExecute = std::string(reinterpret_cast<const char*>(data), size);
+      int result = std::system(cmdToExecute.c_str());
+      if (result != 0) {
+        // TODO: Data reply with returnalue maybe?
+        return returnvalue::FAILED;
+      }
+      return EXECUTION_FINISHED;
+    }
+    case (EXECUTE_SHELL_CMD_NON_BLOCKING): {
+      std::string cmdToExecute = std::string(reinterpret_cast<const char*>(data), size);
       if (cmdExecutor.getCurrentState() == CommandExecutor::States::PENDING or
           shellCmdIsExecuting) {
         return HasActionsIF::IS_BUSY;
       }
-      cmdExecutor.load(cmd, false, false);
+      cmdExecutor.load(cmdToExecute, false, false);
       ReturnValue_t result = cmdExecutor.execute();
       if (result != returnvalue::OK) {
         return result;
@@ -1308,7 +1351,7 @@ void CoreController::performMountedSdCardOperations() {
   auto mountedSdCardOp = [&](sd::SdCard sdCard, std::string mntPoint) {
     if (not performOneShotSdCardOpsSwitch) {
       std::ostringstream path;
-      path << mntPoint << "/" << CONF_FOLDER;
+      path << mntPoint << "/" << core::CONF_FOLDER;
       std::error_code e;
       if (not std::filesystem::exists(path.str()), e) {
         bool created = std::filesystem::create_directory(path.str(), e);

bsp_q7s/core/CoreController.h

@@ -18,17 +18,11 @@
 #include "bsp_q7s/fs/SdCardManager.h"
 #include "events/subsystemIdRanges.h"
 #include "fsfw/controller/ExtendedControllerBase.h"
+#include "mission/sysDefs.h"
 
 class Timer;
 class SdCardManager;
 
-namespace xsc {
-
-enum Chip : int { CHIP_0, CHIP_1, NO_CHIP, SELF_CHIP, ALL_CHIP };
-enum Copy : int { COPY_0, COPY_1, NO_COPY, SELF_COPY, ALL_COPY };
-
-}  // namespace xsc
-
 struct RebootFile {
   static constexpr uint8_t DEFAULT_MAX_BOOT_CNT = 10;
 
@@ -61,18 +55,12 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe
   static constexpr char CHIP_STATE_FILE[] = "/tmp/chip_prot_status.txt";
   static constexpr char CURR_COPY_FILE[] = "/tmp/curr_copy.txt";
 
-  static constexpr char CONF_FOLDER[] = "conf";
-
-  static constexpr char VERSION_FILE_NAME[] = "version.txt";
-  static constexpr char REBOOT_FILE_NAME[] = "reboot.txt";
-  static constexpr char TIME_FILE_NAME[] = "time_backup.txt";
-
   const std::string VERSION_FILE =
-      "/" + std::string(CONF_FOLDER) + "/" + std::string(VERSION_FILE_NAME);
+      "/" + std::string(core::CONF_FOLDER) + "/" + std::string(core::VERSION_FILE_NAME);
   const std::string REBOOT_FILE =
-      "/" + std::string(CONF_FOLDER) + "/" + std::string(REBOOT_FILE_NAME);
+      "/" + std::string(core::CONF_FOLDER) + "/" + std::string(core::REBOOT_FILE_NAME);
   const std::string BACKUP_TIME_FILE =
-      "/" + std::string(CONF_FOLDER) + "/" + std::string(TIME_FILE_NAME);
+      "/" + std::string(core::CONF_FOLDER) + "/" + std::string(core::TIME_FILE_NAME);
 
   static constexpr char CHIP_0_COPY_0_MOUNT_DIR[] = "/tmp/mntupdate-xdi-qspi0-nom-rootfs";
   static constexpr char CHIP_0_COPY_1_MOUNT_DIR[] = "/tmp/mntupdate-xdi-qspi0-gold-rootfs";

linux/acs/StrComHandler.cpp

@@ -451,7 +451,7 @@ ReturnValue_t StrComHandler::performFlashWrite() {
       return returnvalue::OK;
     }
     result = writeNextSegment(idx);
-    if(result != returnvalue::OK) {
+    if (result != returnvalue::OK) {
       return result;
     }
     if (idx % 50 == 0) {

mission/sysDefs.h

@@ -13,7 +13,25 @@ enum Mode : Mode_t { BOOT = 5, SAFE = acs::AcsMode::SAFE, PTG_IDLE = acs::AcsMod
 
 }
 
+namespace xsc {
+
+enum Chip : int { CHIP_0, CHIP_1, NO_CHIP, SELF_CHIP, ALL_CHIP };
+enum Copy : int { COPY_0, COPY_1, NO_COPY, SELF_COPY, ALL_COPY };
+
+}  // namespace xsc
+
 namespace core {
+
+// TODO: Support for status? Or maybe some command to quickly get information whether a unit
+// is running.
+enum SystemctlCmd : uint8_t { START = 0, STOP = 1, RESTART = 2, NUM_CMDS = 3 };
+
+static constexpr char CONF_FOLDER[] = "conf";
+
+static constexpr char VERSION_FILE_NAME[] = "version.txt";
+static constexpr char REBOOT_FILE_NAME[] = "reboot.txt";
+static constexpr char TIME_FILE_NAME[] = "time_backup.txt";
+
 static constexpr ActionId_t LIST_DIRECTORY_INTO_FILE = 0;
 static constexpr ActionId_t ANNOUNCE_VERSION = 1;
 static constexpr ActionId_t ANNOUNCE_CURRENT_IMAGE = 2;
@@ -37,7 +55,9 @@ static constexpr ActionId_t MOUNT_OTHER_COPY = 33;
 //! Reboot using the reboot command
 static constexpr ActionId_t REBOOT_OBC = 34;
 
-static constexpr ActionId_t EXECUTE_SHELL_CMD = 40;
+static constexpr ActionId_t EXECUTE_SHELL_CMD_BLOCKING = 40;
+static constexpr ActionId_t EXECUTE_SHELL_CMD_NON_BLOCKING = 41;
+static constexpr ActionId_t SYSTEMCTL_CMD_EXECUTOR = 42;
 
 static constexpr uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::CORE;
 

tmtc

@@ -1 +1 @@
-Subproject commit 33fd280e517a55175072ea336c7f8e85bce6e55a
+Subproject commit f8da9cff7c5d6d6bdd483f90ccefb67b2d1e11a4

unittest/controller/testThermalController.cpp

@@ -47,8 +47,8 @@ TEST_CASE("Thermal Controller", "[ThermalController]") {
 
   CommandMessage modeMessage;
 
-  ModeMessage::setModeMessage(&modeMessage, ModeMessage::CMD_MODE_COMMAND,
-                              HasModesIF::MODE_ON, HasModesIF::SUBMODE_NONE);
+  ModeMessage::setModeMessage(&modeMessage, ModeMessage::CMD_MODE_COMMAND, HasModesIF::MODE_ON,
+                              HasModesIF::SUBMODE_NONE);
 
   MessageQueueIF* commandQueue =
       QueueFactory::instance()->createMessageQueue(5, MessageQueueMessage::MAX_MESSAGE_SIZE);