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 8486bd09..e1033943 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,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(data[0]); + std::string unitName = std::string(reinterpret_cast(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(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(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); 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..e8ada9f8 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,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; diff --git a/tmtc b/tmtc index 33fd280e..f8da9cff 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit 33fd280e517a55175072ea336c7f8e85bce6e55a +Subproject commit f8da9cff7c5d6d6bdd483f90ccefb67b2d1e11a4 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);