From aa746276aeb7278d6028c70b316f2ba7c6bbca41 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 13 Apr 2023 21:15:10 +0200 Subject: [PATCH 1/6] disable TCS test --- mission/controller/ThermalController.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mission/controller/ThermalController.cpp b/mission/controller/ThermalController.cpp index b12088ae..82efb85c 100644 --- a/mission/controller/ThermalController.cpp +++ b/mission/controller/ThermalController.cpp @@ -17,7 +17,7 @@ #include // Enabling this should trigger a special event which in turn should trigger a system reaction. -#define LOWER_SYRLINKS_UPPER_LIMITS 1 +#define LOWER_SYRLINKS_UPPER_LIMITS 0 #define LOWER_EBAND_UPPER_LIMITS 0 #define LOWER_PLOC_UPPER_LIMITS 0 From af354bd9fb3551d6c853e5116a264051c15ae89f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 13 Apr 2023 23:24:32 +0200 Subject: [PATCH 2/6] systemctl helper --- bsp_q7s/core/CoreController.cpp | 37 +++++++++++++++++-- bsp_q7s/core/CoreController.h | 20 ++-------- linux/acs/StrComHandler.cpp | 2 +- mission/sysDefs.h | 21 ++++++++++- unittest/controller/testThermalController.cpp | 4 +- 5 files changed, 61 insertions(+), 23 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 8486bd09..8961a578 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/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,38 @@ 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(data[0]); + std::string unitName = std::string(reinterpret_cast(data + 1), size - 1); + std::ostringstream oss("systemctl "); + switch (cmdType) { + case (core::SystemctlCmd::START): { + oss << "start "; + break; + } + case (core::SystemctlCmd::STOP): { + oss << "stop "; + break; + } + case (core::SystemctlCmd::RESTART): { + oss << "restart "; + break; + } + } + 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,7 +365,7 @@ 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): { + case (EXECUTE_SHELL_CMD_BLOCKING): { std::string cmd = std::string(cmd, size); if (cmdExecutor.getCurrentState() == CommandExecutor::States::PENDING or shellCmdIsExecuting) { diff --git a/bsp_q7s/core/CoreController.h b/bsp_q7s/core/CoreController.h index 0f2d4033..c1f5e40d 100644 --- a/bsp_q7s/core/CoreController.h +++ b/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"; diff --git a/linux/acs/StrComHandler.cpp b/linux/acs/StrComHandler.cpp index 3c88a71b..51c66f18 100644 --- a/linux/acs/StrComHandler.cpp +++ b/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) { diff --git a/mission/sysDefs.h b/mission/sysDefs.h index 7a729d6e..b73e8f02 100644 --- a/mission/sysDefs.h +++ b/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,8 @@ 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 SYSTEMCTL_CMD_EXECUTOR = 42; static constexpr uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::CORE; diff --git a/unittest/controller/testThermalController.cpp b/unittest/controller/testThermalController.cpp index 0e12d94d..c2166ca0 100644 --- a/unittest/controller/testThermalController.cpp +++ b/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); From c5b26eade48bcef9de622da632b8093a48d5270b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 13 Apr 2023 23:34:11 +0200 Subject: [PATCH 3/6] now it compiles --- bsp_q7s/core/CoreController.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 8961a578..05bf859b 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -1339,7 +1339,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); From f645b97ba3e65abde011da50facaaaf5d0bd7dac Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 14 Apr 2023 00:08:38 +0200 Subject: [PATCH 4/6] bump tmtc --- tmtc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tmtc b/tmtc index 33fd280e..8993ccdf 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit 33fd280e517a55175072ea336c7f8e85bce6e55a +Subproject commit 8993ccdf66c54765fc2a10c26b9c340b7e6b9ffd From e17b8d2ec4e3990cbcc4ba60aa72ec3bf0a4d8a2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 14 Apr 2023 00:21:28 +0200 Subject: [PATCH 5/6] add non-blocking shell cmd executor --- bsp_q7s/core/CoreController.cpp | 13 ++++++++++++- mission/sysDefs.h | 1 + tmtc | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 05bf859b..1931e19a 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -313,7 +313,7 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ } core::SystemctlCmd cmdType = static_cast(data[0]); std::string unitName = std::string(reinterpret_cast(data + 1), size - 1); - std::ostringstream oss("systemctl "); + std::ostringstream oss("systemctl ", std::ostringstream::ate); switch (cmdType) { case (core::SystemctlCmd::START): { oss << "start "; @@ -327,6 +327,9 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ oss << "restart "; break; } + default: { + return HasActionsIF::INVALID_PARAMETERS; + } } oss << unitName; int result = std::system(oss.str().c_str()); @@ -366,6 +369,14 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ return actionReboot(data, size); } case (EXECUTE_SHELL_CMD_BLOCKING): { + std::string cmd = std::string(cmd, size); + int result = std::system(cmd.c_str()); + if (result != 0) { + return returnvalue::FAILED; + } + return EXECUTION_FINISHED; + } + case (EXECUTE_SHELL_CMD_NON_BLOCKING): { std::string cmd = std::string(cmd, size); if (cmdExecutor.getCurrentState() == CommandExecutor::States::PENDING or shellCmdIsExecuting) { diff --git a/mission/sysDefs.h b/mission/sysDefs.h index b73e8f02..e8ada9f8 100644 --- a/mission/sysDefs.h +++ b/mission/sysDefs.h @@ -56,6 +56,7 @@ static constexpr ActionId_t MOUNT_OTHER_COPY = 33; static constexpr ActionId_t REBOOT_OBC = 34; 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; diff --git a/tmtc b/tmtc index 8993ccdf..005e15b2 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit 8993ccdf66c54765fc2a10c26b9c340b7e6b9ffd +Subproject commit 005e15b21bdb58109b90ffbe4be9fc76e85ca38f From bc6531f7a5df222f17d66c2677b80fdaf0b56c04 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 14 Apr 2023 00:29:21 +0200 Subject: [PATCH 6/6] bugfix for shell cmd executors --- CHANGELOG.md | 1 + bsp_q7s/core/CoreController.cpp | 9 +++++---- tmtc | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1166f514..731c87d8 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 1931e19a..e1033943 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -369,20 +369,21 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ return actionReboot(data, size); } case (EXECUTE_SHELL_CMD_BLOCKING): { - std::string cmd = std::string(cmd, size); - int result = std::system(cmd.c_str()); + std::string cmdToExecute = std::string(reinterpret_cast(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 cmd = std::string(cmd, size); + std::string cmdToExecute = std::string(reinterpret_cast(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; diff --git a/tmtc b/tmtc index 005e15b2..f8da9cff 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit 005e15b21bdb58109b90ffbe4be9fc76e85ca38f +Subproject commit f8da9cff7c5d6d6bdd483f90ccefb67b2d1e11a4