From 54af75195c4861bb6189b79982b72bbe87428241 Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Mon, 6 Nov 2023 19:50:15 +0100 Subject: [PATCH 01/24] Implement Error Handling for Edu Component --- Sts1CobcSw/Edu/Edu.cpp | 230 ++++++++++++++------------- Sts1CobcSw/Edu/Edu.hpp | 16 +- Sts1CobcSw/Edu/Structs.hpp | 1 - Sts1CobcSw/EduListenerThread.cpp | 109 +++++++------ Sts1CobcSw/EduProgramQueueThread.cpp | 6 +- Sts1CobcSw/Utility/Outcome.hpp | 50 ++++++ 6 files changed, 235 insertions(+), 177 deletions(-) create mode 100644 Sts1CobcSw/Utility/Outcome.hpp diff --git a/Sts1CobcSw/Edu/Edu.cpp b/Sts1CobcSw/Edu/Edu.cpp index 548f10f6..7d0ba2b3 100644 --- a/Sts1CobcSw/Edu/Edu.cpp +++ b/Sts1CobcSw/Edu/Edu.cpp @@ -59,16 +59,16 @@ auto cepDataBuffer = std::array{}; // TODO: Rework -> Send(EduBasicCommand command) -> void; auto SendCommand(Byte commandId) -> void; -[[nodiscard]] auto SendData(std::span data) -> ErrorCode; +[[nodiscard]] auto SendData(std::span data) -> Result; // TODO: Make this read and return a Type instead of having to provide a destination. Use // Deserialize<>() internally. -[[nodiscard]] auto UartReceive(std::span destination) -> ErrorCode; -[[nodiscard]] auto UartReceive(void * destination) -> ErrorCode; +[[nodiscard]] auto UartReceive(std::span destination) -> Result; +[[nodiscard]] auto UartReceive(void * destination) -> Result; auto FlushUartBuffer() -> void; -[[nodiscard]] auto CheckCrc32(std::span data) -> ErrorCode; -[[nodiscard]] auto GetStatusCommunication() -> Status; -[[nodiscard]] auto ReturnResultCommunication() -> ResultInfo; -[[nodiscard]] auto ReturnResultRetry() -> ResultInfo; +[[nodiscard]] auto CheckCrc32(std::span data) -> Result; +[[nodiscard]] auto GetStatusCommunication() -> Result; +[[nodiscard]] auto ReturnResultCommunication() -> Result; +[[nodiscard]] auto ReturnResultRetry() -> Result; void MockWriteToFile(std::span data); auto Print(std::span data, int nRows = 30) -> void; // NOLINT @@ -104,7 +104,7 @@ auto TurnOff() -> void // TODO: Implement this -auto StoreArchive([[maybe_unused]] StoreArchiveData const & data) -> std::int32_t +auto StoreArchive([[maybe_unused]] StoreArchiveData const & data) -> Result { return 0; } @@ -129,7 +129,7 @@ auto StoreArchive([[maybe_unused]] StoreArchiveData const & data) -> std::int32_ //! @param timeout The available execution time for the student program //! //! @returns A relevant error code -auto ExecuteProgram(ExecuteProgramData const & data) -> ErrorCode +auto ExecuteProgram(ExecuteProgramData const & data) -> Result { RODOS::PRINTF("ExecuteProgram(programId = %d, startTime = %" PRIi32 ", timeout = %d)\n", data.programId, @@ -138,7 +138,8 @@ auto ExecuteProgram(ExecuteProgramData const & data) -> ErrorCode // Check if data command was successful auto serialData = Serialize(data); auto errorCode = SendData(serialData); - if(errorCode != ErrorCode::success) + // TODO: OUTCOME_TRY + if(errorCode.has_error()) { return errorCode; } @@ -156,10 +157,6 @@ auto ExecuteProgram(ExecuteProgramData const & data) -> ErrorCode } switch(answer) { - case cmdAck: - { - return ErrorCode::success; - } case cmdNack: { return ErrorCode::nack; @@ -181,9 +178,8 @@ auto ExecuteProgram(ExecuteProgramData const & data) -> ErrorCode //! <- [N/ACK] //! <- [N/ACK] //! @returns A relevant error code -auto StopProgram() -> ErrorCode +auto StopProgram() -> Result { - return ErrorCode::success; // std::array dataBuf = {stopProgram}; // auto errorCode = SendData(dataBuf); @@ -216,41 +212,49 @@ auto StopProgram() -> ErrorCode //! @returns A status containing (Status Type, [Program ID], [Queue ID], [Exit Code], Error //! Code). Values in square brackets are only valid if the relevant Status Type is //! returned. -auto GetStatus() -> Status +auto GetStatus() -> Result { RODOS::PRINTF("GetStatus()\n"); auto serialData = Serialize(getStatusId); auto sendDataError = SendData(serialData); - if(sendDataError != ErrorCode::success) + // TODO: OUTCOME_TRY, and change name + if(sendDataError.has_error()) { - RODOS::PRINTF(" Returned .statusType = %d, .errorCode = %d\n", - static_cast(StatusType::invalid), - static_cast(sendDataError)); - return Status{.statusType = StatusType::invalid, .errorCode = sendDataError}; + return sendDataError.error(); + // RODOS::PRINTF(" Returned .statusType = %d, .errorCode = %d\n", + // static_cast(StatusType::invalid), + // static_cast(sendDataError)); + // return Status{.statusType = StatusType::invalid, .errorCode = sendDataError}; } - Status status; + Result status = ErrorCode::noErrorCodeSet; std::size_t errorCount = 0; do { status = GetStatusCommunication(); - if(status.errorCode == ErrorCode::success) + if(status.has_value()) { SendCommand(cmdAck); break; } + FlushUartBuffer(); SendCommand(cmdNack); } while(errorCount++ < maxNNackRetries); - RODOS::PRINTF( - " .statusType = %d\n .errorCode = %d\n .programId = %d\n .startTime = %" PRIi32 - "\n exitCode = %d\n", - static_cast(status.statusType), - static_cast(status.errorCode), - status.programId, - status.startTime, - status.exitCode); + if(status.has_value()) + { + RODOS::PRINTF(" .statusType = %d\n .programId = %d\n .startTime = %" PRIi32 + "\n .exitCode = %d\n", + static_cast(status.value().statusType), + status.value().programId, + status.value().startTime, + status.value().exitCode); + } + else + { + RODOS::PRINTF(" .errorCode = %d\n", status.error()); + } return status; } @@ -258,71 +262,72 @@ auto GetStatus() -> Status //! @brief Communication function for GetStatus() to separate a single try from //! retry logic. //! @returns The received EDU status -auto GetStatusCommunication() -> Status +auto GetStatusCommunication() -> Result { // Get header data auto headerBuffer = Buffer{}; auto headerReceiveError = UartReceive(headerBuffer); auto headerData = Deserialize(headerBuffer); - if(headerReceiveError != ErrorCode::success) + // TODO: OUTCOME_TRY + if(headerReceiveError.has_error()) { - return Status{.statusType = StatusType::invalid, .errorCode = headerReceiveError}; + return headerReceiveError.error(); } if(headerData.command != cmdData) { - return Status{.statusType = StatusType::invalid, .errorCode = ErrorCode::invalidCommand}; + return ErrorCode::invalidCommand; + // return Status{.statusType = StatusType::invalid, .errorCode = ErrorCode::invalidCommand}; } if(headerData.length == 0U) { - return Status{.statusType = StatusType::invalid, .errorCode = ErrorCode::invalidLength}; + return ErrorCode::invalidLength; } // Get the status type code auto statusType = 0_b; auto statusErrorCode = UartReceive(&statusType); - if(statusErrorCode != ErrorCode::success) + // TODO: OUTCOME_TRY + if(statusErrorCode.has_error()) { - return Status{.statusType = StatusType::invalid, .errorCode = statusErrorCode}; + return statusErrorCode.error(); } if(statusType == noEventCode) { if(headerData.length != nNoEventBytes) { - return Status{.statusType = StatusType::invalid, .errorCode = ErrorCode::invalidLength}; + return ErrorCode::invalidLength; } std::array statusTypeArray = {statusType}; auto crc32Error = CheckCrc32(std::span(statusTypeArray)); - if(crc32Error != ErrorCode::success) + if(crc32Error.has_error()) { - return Status{.statusType = StatusType::invalid, .errorCode = crc32Error}; + return crc32Error.error(); } - return Status{.statusType = StatusType::noEvent, - .programId = 0, - .startTime = 0, - .exitCode = 0, - .errorCode = ErrorCode::success}; + return Status{ + .statusType = StatusType::noEvent, .programId = 0, .startTime = 0, .exitCode = 0}; } if(statusType == programFinishedCode) { if(headerData.length != nProgramFinishedBytes) { - return Status{.statusType = StatusType::invalid, .errorCode = ErrorCode::invalidLength}; + return ErrorCode::invalidLength; } auto dataBuffer = Buffer{}; auto programFinishedError = UartReceive(dataBuffer); - if(programFinishedError != ErrorCode::success) + // TODO: OUTCOME_TRY + if(programFinishedError.has_error()) { - return Status{.statusType = StatusType::invalid, .errorCode = programFinishedError}; + return programFinishedError.error(); } // Create another Buffer which includes the status type that was received beforehand because @@ -331,31 +336,32 @@ auto GetStatusCommunication() -> Status fullDataBuffer[0] = statusType; std::copy(dataBuffer.begin(), dataBuffer.end(), fullDataBuffer.begin() + 1); auto crc32Error = CheckCrc32(fullDataBuffer); - if(crc32Error != ErrorCode::success) + // TODO:OUTCOME_TRY + if(crc32Error.has_error()) { - return Status{.statusType = StatusType::invalid, .errorCode = crc32Error}; + return crc32Error.error(); } auto programFinishedData = Deserialize(dataBuffer); return Status{.statusType = StatusType::programFinished, .programId = programFinishedData.programId, .startTime = programFinishedData.startTime, - .exitCode = programFinishedData.exitCode, - .errorCode = ErrorCode::success}; + .exitCode = programFinishedData.exitCode}; } if(statusType == resultsReadyCode) { if(headerData.length != nResultsReadyBytes) { - return Status{.statusType = StatusType::invalid, .errorCode = ErrorCode::invalidLength}; + return ErrorCode::invalidLength; } auto dataBuffer = Buffer{}; auto resultsReadyError = UartReceive(dataBuffer); - if(resultsReadyError != ErrorCode::success) + // TODO:OUTCOME_TRY + if(resultsReadyError.has_error()) { - return Status{.statusType = StatusType::invalid, .errorCode = resultsReadyError}; + return resultsReadyError.error(); } // Create another Buffer which includes the status type that was received beforehand because @@ -364,22 +370,22 @@ auto GetStatusCommunication() -> Status fullDataBuffer[0] = statusType; std::copy(dataBuffer.begin(), dataBuffer.end(), fullDataBuffer.begin() + 1); auto crc32Error = CheckCrc32(fullDataBuffer); - if(crc32Error != ErrorCode::success) + // TODO: OUTCOME_TRY + if(crc32Error.has_error()) { - return Status{.statusType = StatusType::invalid, .errorCode = crc32Error}; + return crc32Error.error(); } auto resultsReadyData = Deserialize(dataBuffer); return Status{.statusType = StatusType::resultsReady, .programId = resultsReadyData.programId, - .startTime = resultsReadyData.startTime, - .errorCode = ErrorCode::success}; + .startTime = resultsReadyData.startTime}; } - return Status{.statusType = StatusType::invalid, .errorCode = ErrorCode::invalidStatusType}; + return ErrorCode::invalidStatusType; } -auto ReturnResult() -> ResultInfo +auto ReturnResult() -> Result { // DEBUG RODOS::PRINTF("ReturnResult()\n"); @@ -388,9 +394,9 @@ auto ReturnResult() -> ResultInfo // Send command auto serialCommand = Serialize(returnResultId); auto commandError = SendData(serialCommand); - if(commandError != ErrorCode::success) + if(commandError.has_error()) { - return ResultInfo{.errorCode = commandError, .resultSize = 0U}; + return commandError.error(); } // DEBUG @@ -399,7 +405,7 @@ auto ReturnResult() -> ResultInfo std::size_t totalResultSize = 0U; std::size_t packets = 0U; - ResultInfo resultInfo; + Result resultInfo = ErrorCode::noErrorCodeSet; // TODO: Turn into for loop while(packets < maxNPackets) { @@ -408,21 +414,21 @@ auto ReturnResult() -> ResultInfo // END DEBUG resultInfo = ReturnResultRetry(); // DEBUG - RODOS::PRINTF("ResultInfo{errorCode = %d, resultSize = %d}\n", - static_cast(resultInfo.errorCode), - static_cast(resultInfo.resultSize)); - // END DEBUG - if(resultInfo.errorCode != ErrorCode::success) + if(resultInfo.has_error()) { + auto errorCode = resultInfo.error(); + RODOS::PRINTF(" ResultResultRetry() resulted in an error : %d", + static_cast(errorCode)); break; } + // END DEBUG // RODOS::PRINTF("\nWriting to file...\n"); // TODO: Actually write to a file - totalResultSize += resultInfo.resultSize; + totalResultSize += resultInfo.value(); packets++; } - return ResultInfo{.errorCode = resultInfo.errorCode, .resultSize = totalResultSize}; + return totalResultSize; } @@ -430,23 +436,23 @@ auto ReturnResult() -> ResultInfo //! the actual ReturnResult function. The communication happens in ReturnResultCommunication. //! //! @returns An error code and the number of received bytes in ResultInfo -auto ReturnResultRetry() -> ResultInfo +auto ReturnResultRetry() -> Result { - ResultInfo resultInfo; + Result result = ErrorCode::noErrorCodeSet; std::size_t errorCount = 0U; do { - resultInfo = ReturnResultCommunication(); - if(resultInfo.errorCode == ErrorCode::success - or resultInfo.errorCode == ErrorCode::successEof) + result = ReturnResultCommunication(); + // TODO: That is ugly, maybe a successCommunicationEnum would be better + if(result.has_value() or (result.has_error() and result.error() == ErrorCode::successEof)) { - SendCommand(cmdAck); - return resultInfo; + SendCommand(cmdNack); + return result; } FlushUartBuffer(); SendCommand(cmdNack); } while(errorCount++ < maxNNackRetries); - return resultInfo; + return result.value(); } @@ -454,32 +460,32 @@ auto ReturnResultRetry() -> ResultInfo // directly and instead writes to a non-primary RAM bank as an intermediate step. // // Simple results -> 1 round should work with DMA to RAM -auto ReturnResultCommunication() -> ResultInfo +auto ReturnResultCommunication() -> Result { // Receive command // If no result is available, the command will be NACK, // otherwise DATA Byte command = 0_b; auto commandError = UartReceive(&command); - if(commandError != ErrorCode::success) + if(commandError.has_error()) { - return ResultInfo{.errorCode = commandError, .resultSize = 0U}; + return commandError.error(); } if(command == cmdNack) { // TODO: necessary to differentiate errors or just return success with resultSize 0? - return ResultInfo{.errorCode = ErrorCode::noResultAvailable, .resultSize = 0U}; + return ErrorCode::noResultAvailable; } if(command == cmdEof) { - return ResultInfo{.errorCode = ErrorCode::successEof, .resultSize = 0U}; + return ErrorCode::successEof; } if(command != cmdData) { // DEBUG RODOS::PRINTF("\nNot DATA command\n"); // END DEBUG - return ResultInfo{.errorCode = ErrorCode::invalidCommand, .resultSize = 0U}; + return ErrorCode::invalidCommand; } // DEBUG @@ -488,15 +494,16 @@ auto ReturnResultCommunication() -> ResultInfo auto dataLengthBuffer = Buffer{}; auto lengthError = UartReceive(dataLengthBuffer); - if(lengthError != ErrorCode::success) + // TODO: OUTCOME_TRY + if(lengthError.has_error()) { - return ResultInfo{.errorCode = lengthError, .resultSize = 0U}; + return lengthError.error(); } auto actualDataLength = Deserialize(dataLengthBuffer); if(actualDataLength == 0U or actualDataLength > maxDataLength) { - return ResultInfo{.errorCode = ErrorCode::invalidLength, .resultSize = 0U}; + return ErrorCode::invalidLength; } // DEBUG @@ -507,9 +514,9 @@ auto ReturnResultCommunication() -> ResultInfo auto dataError = UartReceive( std::span(cepDataBuffer.begin(), cepDataBuffer.begin() + actualDataLength)); - if(dataError != ErrorCode::success) + if(dataError.has_error()) { - return ResultInfo{.errorCode = dataError, .resultSize = 0U}; + return dataError.error(); } // DEBUG @@ -519,16 +526,16 @@ auto ReturnResultCommunication() -> ResultInfo auto crc32Error = CheckCrc32( std::span(cepDataBuffer.begin(), cepDataBuffer.begin() + actualDataLength)); - if(crc32Error != ErrorCode::success) + if(crc32Error.has_error()) { - return ResultInfo{.errorCode = crc32Error, .resultSize = 0U}; + return crc32Error.error(); } // DEBUG RODOS::PRINTF("\nSuccess\n"); // END DEBUG - return {ErrorCode::success, actualDataLength}; + return actualDataLength; } @@ -547,14 +554,15 @@ auto ReturnResultCommunication() -> ResultInfo //! @param currentTime A unix timestamp //! //! @returns A relevant error code -auto UpdateTime(UpdateTimeData const & data) -> ErrorCode +auto UpdateTime(UpdateTimeData const & data) -> Result { RODOS::PRINTF("UpdateTime()\n"); auto serialData = Serialize(data); auto errorCode = SendData(serialData); - if(errorCode != ErrorCode::success) + // TODO:OUTCOME_TRY + if(errorCode.has_error()) { - return errorCode; + return errorCode.error(); } // On success, wait for second N/ACK @@ -571,10 +579,6 @@ auto UpdateTime(UpdateTimeData const & data) -> ErrorCode } switch(answer) { - case cmdAck: - { - return ErrorCode::success; - } case cmdNack: { return ErrorCode::nack; @@ -601,7 +605,7 @@ void SendCommand(Byte commandId) //! @brief Send a data packet over UART to the EDU. //! //! @param data The data to be sent -auto SendData(std::span data) -> ErrorCode +auto SendData(std::span data) -> Result { std::size_t const nBytes = data.size(); if(nBytes >= maxDataLength) @@ -634,10 +638,10 @@ auto SendData(std::span data) -> ErrorCode // RODOS::PRINTF("[Edu] answer in sendData is now : %c\n", static_cast(answer)); switch(answer) { - case cmdAck: - { - return ErrorCode::success; - } + // case cmdAck: + //{ + // return ErrorCode::success; + // } case cmdNack: { nackCount++; @@ -659,7 +663,7 @@ auto SendData(std::span data) -> ErrorCode //! //! @returns A relevant EDU error code // TODO: Use hal::ReadFrom() -auto UartReceive(std::span destination) -> ErrorCode +auto UartReceive(std::span destination) -> Result { if(size(destination) > maxDataLength) { @@ -679,7 +683,6 @@ auto UartReceive(std::span destination) -> ErrorCode } totalReceivedBytes += nReceivedBytes; } - return ErrorCode::success; } @@ -689,7 +692,7 @@ auto UartReceive(std::span destination) -> ErrorCode //! //! @returns A relevant EDU error code // TODO: Use hal::ReadFrom() -auto UartReceive(void * destination) -> ErrorCode +auto UartReceive(void * destination) -> Result { uart.suspendUntilDataReady(RODOS::NOW() + eduTimeout); auto nReceivedBytes = uart.read(destination, 1); @@ -697,7 +700,6 @@ auto UartReceive(void * destination) -> ErrorCode { return ErrorCode::timeout; } - return ErrorCode::success; } @@ -722,7 +724,7 @@ auto FlushUartBuffer() -> void } -auto CheckCrc32(std::span data) -> ErrorCode +auto CheckCrc32(std::span data) -> Result { auto const computedCrc32 = utility::Crc32(data); @@ -743,7 +745,8 @@ auto CheckCrc32(std::span data) -> ErrorCode // RODOS::PRINTF("\n"); // END DEBUG - if(receiveError != ErrorCode::success) + // TODO: OUTCOME_TRY + if(receiveError.has_error()) { return receiveError; } @@ -751,7 +754,6 @@ auto CheckCrc32(std::span data) -> ErrorCode { return ErrorCode::wrongChecksum; } - return ErrorCode::success; } diff --git a/Sts1CobcSw/Edu/Edu.hpp b/Sts1CobcSw/Edu/Edu.hpp index fc1201aa..5e9fb0d9 100644 --- a/Sts1CobcSw/Edu/Edu.hpp +++ b/Sts1CobcSw/Edu/Edu.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include @@ -12,16 +13,19 @@ namespace sts1cobcsw::edu { +template +using Result = outcome_v2::experimental::status_result; + auto Initialize() -> void; auto TurnOn() -> void; auto TurnOff() -> void; // TODO: Why does this return a std::int32_t? -[[nodiscard]] auto StoreArchive(StoreArchiveData const & data) -> std::int32_t; -[[nodiscard]] auto ExecuteProgram(ExecuteProgramData const & data) -> ErrorCode; -[[nodiscard]] auto StopProgram() -> ErrorCode; +[[nodiscard]] auto StoreArchive(StoreArchiveData const & data) -> Result; +[[nodiscard]] auto ExecuteProgram(ExecuteProgramData const & data) -> Result; +[[nodiscard]] auto StopProgram() -> Result; // TODD: Find better name (or maybe even mechanism) for GetStatus -[[nodiscard]] auto GetStatus() -> Status; -[[nodiscard]] auto ReturnResult() -> ResultInfo; -[[nodiscard]] auto UpdateTime(UpdateTimeData const & data) -> ErrorCode; +[[nodiscard]] auto GetStatus() -> Result; +[[nodiscard]] auto ReturnResult() -> Result; +[[nodiscard]] auto UpdateTime(UpdateTimeData const & data) -> Result; } diff --git a/Sts1CobcSw/Edu/Structs.hpp b/Sts1CobcSw/Edu/Structs.hpp index fe4ff2a2..bdf3a15e 100644 --- a/Sts1CobcSw/Edu/Structs.hpp +++ b/Sts1CobcSw/Edu/Structs.hpp @@ -56,7 +56,6 @@ struct Status std::uint16_t programId = 0; std::int32_t startTime = 0; std::uint8_t exitCode = 0; - ErrorCode errorCode = ErrorCode::noErrorCodeSet; }; diff --git a/Sts1CobcSw/EduListenerThread.cpp b/Sts1CobcSw/EduListenerThread.cpp index 04e74bdf..38f41b6b 100644 --- a/Sts1CobcSw/EduListenerThread.cpp +++ b/Sts1CobcSw/EduListenerThread.cpp @@ -52,12 +52,11 @@ class EduListenerThread : public RODOS::StaticThread<> // RODOS::PRINTF("[EduListenerThread] Edu is alive and has an update\n"); // Communicate with EDU - auto status = edu::GetStatus(); + auto statusResult = edu::GetStatus(); // RODOS::PRINTF("EduStatus : %d, EduErrorcode %d\n", status.statusType, // status.errorCode); - if(status.errorCode != edu::ErrorCode::success - and status.errorCode != edu::ErrorCode::successEof) + if(statusResult.has_error()) { // RODOS::PRINTF("[EduListenerThread] GetStatus() error code : %d.\n", // status.errorCode); @@ -72,67 +71,71 @@ class EduListenerThread : public RODOS::StaticThread<> // success.\n"); } - switch(status.statusType) + if(statusResult.has_value()) { - case edu::StatusType::programFinished: + auto status = statusResult.value(); + switch(status.statusType) { - // Program has finished - // Find the correspongind queueEntry and update it, then resume edu queue - // thread - if(status.exitCode == 0) + case edu::StatusType::programFinished: { - edu::UpdateProgramStatusHistory( - status.programId, - status.startTime, - edu::ProgramStatus::programExecutionSucceeded); + // Program has finished + // Find the correspongind queueEntry and update it, then resume edu + // queue thread + if(status.exitCode == 0) + { + edu::UpdateProgramStatusHistory( + status.programId, + status.startTime, + edu::ProgramStatus::programExecutionSucceeded); + } + else + { + edu::UpdateProgramStatusHistory( + status.programId, + status.startTime, + edu::ProgramStatus::programExecutionFailed); + } + ResumeEduProgramQueueThread(); + break; } - else + case edu::StatusType::resultsReady: { + // Edu wants to send result file + // Send return result to Edu, Communicate, and interpret the results to + // update the S&H Entry from 3 or 4 to 5. + auto resultsInfo = edu::ReturnResult(); + // TODO: Naming + if(resultsInfo.has_error()) + { + /* + RODOS::PRINTF( + "[EduListenerThread] Error Code From ReturnResult() : %d.\n", + errorCode); + RODOS::PRINTF( + "[EduListenerThread] Communication error after call to " + "ReturnResult().\n"); + */ + ResumeEduCommunicationErrorThread(); + } + else + { + // RODOS::PRINTF( + // "[EduListenerThread] Call to ReturnResults() resulted in " + // "success.\n"); + } + // break; + edu::UpdateProgramStatusHistory( status.programId, status.startTime, - edu::ProgramStatus::programExecutionFailed); + edu::ProgramStatus::resultFileTransfered); + break; } - ResumeEduProgramQueueThread(); - break; - } - case edu::StatusType::resultsReady: - { - // Edu wants to send result file - // Send return result to Edu, Communicate, and interpret the results to - // update the S&H Entry from 3 or 4 to 5. - auto resultsInfo = edu::ReturnResult(); - auto errorCode = resultsInfo.errorCode; - if(errorCode != edu::ErrorCode::success - and errorCode != edu::ErrorCode::successEof) + case edu::StatusType::invalid: + case edu::StatusType::noEvent: { - /* - RODOS::PRINTF( - "[EduListenerThread] Error Code From ReturnResult() : %d.\n", - errorCode); - RODOS::PRINTF( - "[EduListenerThread] Communication error after call to " - "ReturnResult().\n"); - */ - ResumeEduCommunicationErrorThread(); + break; } - else - { - // RODOS::PRINTF( - // "[EduListenerThread] Call to ReturnResults() resulted in " - // "success.\n"); - } - // break; - - edu::UpdateProgramStatusHistory(status.programId, - status.startTime, - edu::ProgramStatus::resultFileTransfered); - break; - } - case edu::StatusType::invalid: - case edu::StatusType::noEvent: - { - break; } } } diff --git a/Sts1CobcSw/EduProgramQueueThread.cpp b/Sts1CobcSw/EduProgramQueueThread.cpp index 22657c5f..d4381862 100644 --- a/Sts1CobcSw/EduProgramQueueThread.cpp +++ b/Sts1CobcSw/EduProgramQueueThread.cpp @@ -95,9 +95,9 @@ class EduProgramQueueThread : public RODOS::StaticThread auto errorCode = edu::UpdateTime(edu::UpdateTimeData{.currentTime = utility::GetUnixUtc()}); - if(errorCode != edu::ErrorCode::success) + if(errorCode.has_error()) { - RODOS::PRINTF("UpdateTime error code : %d\n", static_cast(errorCode)); + RODOS::PRINTF("UpdateTime error code : %d\n", static_cast(errorCode.error())); RODOS::PRINTF( "[EduProgramQueueThread] Communication error after call to UpdateTime().\n"); ResumeEduCommunicationErrorThread(); @@ -129,7 +129,7 @@ class EduProgramQueueThread : public RODOS::StaticThread errorCode = edu::ExecuteProgram(executeProgramData); // errorCode = edu::ErrorCode::success; - if(errorCode != edu::ErrorCode::success) + if(errorCode.has_error()) { RODOS::PRINTF( "[EduProgramQueueThread] Communication error after call to " diff --git a/Sts1CobcSw/Utility/Outcome.hpp b/Sts1CobcSw/Utility/Outcome.hpp new file mode 100644 index 00000000..131c9981 --- /dev/null +++ b/Sts1CobcSw/Utility/Outcome.hpp @@ -0,0 +1,50 @@ +#pragma once + +#if defined(GENERIC_SYSTEM) + #define OUTCOME_DISABLE_EXECINFO + #define SYSTEM_ERROR2_NOT_POSIX + #define SYSTEM_ERROR2_FATAL(msg) RODOS::hwResetAndReboot() +#endif + + +#include + +#include + + +struct RebootPolicy : outcome_v2::experimental::policy::base +{ + template + // NOLINTNEXTLINE(readability-identifier-naming) + static constexpr void wide_value_check(Impl && self) + { + //! Call RODOS::hwResetAndReboot() whenever .value() is called on an object that does not + //! contain a value + if(!base::_has_value(std::forward(self))) + { + RODOS::hwResetAndReboot(); + } + } + + template + // NOLINTNEXTLINE(readability-identifier-naming) + static constexpr void wide_error_check(Impl && self) + { + //! Call RODOS::hwResetAndReboot() whenever .error() is called on an object that does not + //! contain an error + if(!base::_has_error(std::forward(self))) + { + RODOS::hwResetAndReboot(); + } + } + + template + // NOLINTNEXTLINE(readability-identifier-naming) + static constexpr void wide_exception_check(Impl && self) + { + if(!base::_has_exception(std::forward(self))) + { + RODOS::hwResetAndReboot(); + } + } +}; From a7bcb8c0c1994bc0a98085b4a021f7a759b42a89 Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Wed, 8 Nov 2023 15:26:11 +0100 Subject: [PATCH 02/24] Add unit test for outcome library --- Sts1CobcSw/Edu/Edu.cpp | 14 ++++ Sts1CobcSw/Edu/EduMock.cpp | 16 ++-- Tests/.clang-tidy | 1 - Tests/UnitTests/CMakeLists.txt | 3 + Tests/UnitTests/Outcome.test.cpp | 136 +++++++++++++++++++++++++++++++ 5 files changed, 159 insertions(+), 11 deletions(-) create mode 100644 Tests/UnitTests/Outcome.test.cpp diff --git a/Sts1CobcSw/Edu/Edu.cpp b/Sts1CobcSw/Edu/Edu.cpp index 7d0ba2b3..7699405b 100644 --- a/Sts1CobcSw/Edu/Edu.cpp +++ b/Sts1CobcSw/Edu/Edu.cpp @@ -157,6 +157,11 @@ auto ExecuteProgram(ExecuteProgramData const & data) -> Result } switch(answer) { + // TODO: Return outcome::success + // case cmdAck: + //{ + // return ; + //} case cmdNack: { return ErrorCode::nack; @@ -180,6 +185,7 @@ auto ExecuteProgram(ExecuteProgramData const & data) -> Result //! @returns A relevant error code auto StopProgram() -> Result { + return outcome_v2::success(); // std::array dataBuf = {stopProgram}; // auto errorCode = SendData(dataBuf); @@ -579,6 +585,10 @@ auto UpdateTime(UpdateTimeData const & data) -> Result } switch(answer) { + // case cmdAck: + //{ + // return ErrorCode::succes; + // } case cmdNack: { return ErrorCode::nack; @@ -638,6 +648,7 @@ auto SendData(std::span data) -> Result // RODOS::PRINTF("[Edu] answer in sendData is now : %c\n", static_cast(answer)); switch(answer) { + // TODO: Return outcome::success // case cmdAck: //{ // return ErrorCode::success; @@ -683,6 +694,8 @@ auto UartReceive(std::span destination) -> Result } totalReceivedBytes += nReceivedBytes; } + // TODO: Test this + return outcome_v2::success(); } @@ -754,6 +767,7 @@ auto CheckCrc32(std::span data) -> Result { return ErrorCode::wrongChecksum; } + // TODO: Return outcome::successs } diff --git a/Sts1CobcSw/Edu/EduMock.cpp b/Sts1CobcSw/Edu/EduMock.cpp index 74bee4fe..f7f48257 100644 --- a/Sts1CobcSw/Edu/EduMock.cpp +++ b/Sts1CobcSw/Edu/EduMock.cpp @@ -52,7 +52,7 @@ auto TurnOff() -> void // NOLINTNEXTLINE(readability-convert-member-functions-to-static) -auto StoreArchive(StoreArchiveData const & data) -> std::int32_t +auto StoreArchive(StoreArchiveData const & data) -> Result { PrintFormattedSystemUtc(); PRINTF("Call to StoreArchive(programId = %d)\n", data.programId); @@ -61,7 +61,7 @@ auto StoreArchive(StoreArchiveData const & data) -> std::int32_t // NOLINTNEXTLINE(readability-convert-member-functions-to-static) -auto ExecuteProgram(ExecuteProgramData const & data) -> ErrorCode +auto ExecuteProgram(ExecuteProgramData const & data) -> Result { PrintFormattedSystemUtc(); PRINTF("Call to ExecuteProgram(programId = %d, startTime = %" PRIi32 ", timeout = %d)\n", @@ -73,7 +73,7 @@ auto ExecuteProgram(ExecuteProgramData const & data) -> ErrorCode // NOLINTNEXTLINE(readability-convert-member-functions-to-static) -auto StopProgram() -> ErrorCode +auto StopProgram() -> Result { PrintFormattedSystemUtc(); PRINTF("Call to StopProgram()\n"); @@ -82,20 +82,16 @@ auto StopProgram() -> ErrorCode // NOLINTNEXTLINE(readability-convert-member-functions-to-static) -auto GetStatus() -> Status +auto GetStatus() -> Result { PrintFormattedSystemUtc(); PRINTF("Call to GetStatus()\n"); - return {.statusType = StatusType::invalid, - .programId = 0, - .startTime = 0, - .exitCode = 0, - .errorCode = ErrorCode::success}; + return Status{.statusType = StatusType::invalid, .programId = 0, .startTime = 0, .exitCode = 0}; } // NOLINTNEXTLINE(readability-convert-member-functions-to-static) -auto UpdateTime(UpdateTimeData const & data) -> ErrorCode +auto UpdateTime(UpdateTimeData const & data) -> Result { PrintFormattedSystemUtc(); PRINTF("Call to UpdateTime(currentTime = %" PRIi32 ")\n", data.currentTime); diff --git a/Tests/.clang-tidy b/Tests/.clang-tidy index 50ac4ada..1747a357 100644 --- a/Tests/.clang-tidy +++ b/Tests/.clang-tidy @@ -1,4 +1,3 @@ Checks: "-readability-identifier-length, \ -*magic-numbers*" InheritParentConfig: true - diff --git a/Tests/UnitTests/CMakeLists.txt b/Tests/UnitTests/CMakeLists.txt index 08dabf4c..072cd4a2 100644 --- a/Tests/UnitTests/CMakeLists.txt +++ b/Tests/UnitTests/CMakeLists.txt @@ -13,6 +13,9 @@ endif() add_program(Serial Serial.test.cpp) target_link_libraries(Sts1CobcSwTests_Serial PRIVATE Catch2::Catch2WithMain Sts1CobcSw_Serial) +add_program(Outcome Outcome.test.cpp) +target_link_libraries(Sts1CobcSwTests_Outcome PRIVATE Catch2::Catch2WithMain) + get_property( all_unit_test_targets DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} diff --git a/Tests/UnitTests/Outcome.test.cpp b/Tests/UnitTests/Outcome.test.cpp new file mode 100644 index 00000000..c62136ec --- /dev/null +++ b/Tests/UnitTests/Outcome.test.cpp @@ -0,0 +1,136 @@ +// Test the outcome libarary + +#include +#include + +#include + +// First define a policy +struct AbortPolicy : outcome_v2::experimental::policy::base +{ + template + // NOLINTNEXTLINE(readability-identifier-naming) + static constexpr void wide_value_check(Impl && self) + { + //! Call RODOS::hwResetAndReboot() whenever .value() is called on an object that does not + //! contain a value + if(!base::_has_value(std::forward(self))) + { + std::abort(); + } + } + + template + // NOLINTNEXTLINE(readability-identifier-naming) + static constexpr void wide_error_check(Impl && self) + { + //! Call RODOS::hwResetAndReboot() whenever .error() is called on an object that does not + //! contain an error + if(!base::_has_error(std::forward(self))) + { + std::abort(); + } + } + + template + // NOLINTNEXTLINE(readability-identifier-naming) + static constexpr void wide_exception_check(Impl && self) + { + if(!base::_has_exception(std::forward(self))) + { + std::abort(); + } + } +}; + +enum class ConversionErrc +{ + success = 0, // 0 should not represent an error + emptyString = 1, // (for rationale, see tutorial on error codes) + illegalChar = 2, + tooLong = 3, +}; + +template +using Result = outcome_v2::experimental::status_result; + + +auto Convert(const std::string & str) noexcept -> Result +{ + if(str.empty()) + { + return ConversionErrc::emptyString; + } + + if(!std::all_of(str.begin(), str.end(), ::isdigit)) + { + return ConversionErrc::illegalChar; + } + + if(str.length() > 9) + { + return ConversionErrc::tooLong; + } + + // NOLINTNEXTLINE (cert-err34-c) + return atoi(str.c_str()); +} + + +TEST_CASE("Inspecting result") +{ + Result result = outcome_v2::success(); + REQUIRE(result.has_value()); + REQUIRE(result); // Boolean cast + + // Test each defined convertion error + result = Convert("abc"); + REQUIRE(result.has_error()); + REQUIRE(result.error() == ConversionErrc::illegalChar); + + result = Convert("314159265359"); + REQUIRE(result.has_error()); + REQUIRE(result.error() == ConversionErrc::tooLong); + + result = Convert(""); + REQUIRE(result.has_error()); + REQUIRE(result.error() == ConversionErrc::emptyString); + + + // Test success + result = Convert("278"); + REQUIRE(result.has_value()); + REQUIRE(result); // Boolean cast + REQUIRE(result.value() == 278); +} + +// Dummy function to chain with Convert +// Return type is a pair just to display how OUTCOME_TRY works with different return<> types +auto Add(int op1, const std::string & str) -> Result> +{ + // From https://ned14.github.io/outcome/tutorial/essential/result/inspecting/ + // Our control statement means: + // if Convert() returned failure, this same error information should be returned from Add(), + // even though Add() and Convert() have different result<> types. + // If Covnert returned success, we create variable op2 of type int with the value returned from + // Convert. If control goes to subsequent line, it means Convert succeeded and variable of type + // BigInt is in scope. + OUTCOME_TRY(auto op2, Convert(str)); + return std::make_pair(op1 + op2, op2); +} + + +TEST_CASE("TRY macro") +{ + // Test failure + auto result1 = Add(1, "3.14"); + REQUIRE(not result1.has_value()); + REQUIRE(result1.has_error()); + REQUIRE(result1.error() == ConversionErrc::illegalChar); + + // Test success + auto result2 = Add(1, "278"); + REQUIRE(result2.has_value()); + REQUIRE(result2); + REQUIRE(result2.value().first == 279); +} From f098fb4132a64d5cef973bcc89451bf1441d1bc4 Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Wed, 8 Nov 2023 15:51:53 +0100 Subject: [PATCH 03/24] Use OUTCOME_TRY macro in Edu.cpp --- Sts1CobcSw/Edu/Edu.cpp | 99 +++++++++--------------------------------- 1 file changed, 21 insertions(+), 78 deletions(-) diff --git a/Sts1CobcSw/Edu/Edu.cpp b/Sts1CobcSw/Edu/Edu.cpp index 7699405b..f86df7bd 100644 --- a/Sts1CobcSw/Edu/Edu.cpp +++ b/Sts1CobcSw/Edu/Edu.cpp @@ -137,12 +137,7 @@ auto ExecuteProgram(ExecuteProgramData const & data) -> Result data.timeout); // Check if data command was successful auto serialData = Serialize(data); - auto errorCode = SendData(serialData); - // TODO: OUTCOME_TRY - if(errorCode.has_error()) - { - return errorCode; - } + OUTCOME_TRY(SendData(serialData)); // eduTimeout != timeout argument for data! // timeout specifies the time the student program has to execute @@ -157,11 +152,10 @@ auto ExecuteProgram(ExecuteProgramData const & data) -> Result } switch(answer) { - // TODO: Return outcome::success - // case cmdAck: - //{ - // return ; - //} + case cmdAck: + { + return outcome_v2::success(); + } case cmdNack: { return ErrorCode::nack; @@ -222,16 +216,7 @@ auto GetStatus() -> Result { RODOS::PRINTF("GetStatus()\n"); auto serialData = Serialize(getStatusId); - auto sendDataError = SendData(serialData); - // TODO: OUTCOME_TRY, and change name - if(sendDataError.has_error()) - { - return sendDataError.error(); - // RODOS::PRINTF(" Returned .statusType = %d, .errorCode = %d\n", - // static_cast(StatusType::invalid), - // static_cast(sendDataError)); - // return Status{.statusType = StatusType::invalid, .errorCode = sendDataError}; - } + OUTCOME_TRY(SendData(serialData)); Result status = ErrorCode::noErrorCodeSet; std::size_t errorCount = 0; @@ -272,15 +257,9 @@ auto GetStatusCommunication() -> Result { // Get header data auto headerBuffer = Buffer{}; - auto headerReceiveError = UartReceive(headerBuffer); + OUTCOME_TRY(UartReceive(headerBuffer)); auto headerData = Deserialize(headerBuffer); - // TODO: OUTCOME_TRY - if(headerReceiveError.has_error()) - { - return headerReceiveError.error(); - } - if(headerData.command != cmdData) { return ErrorCode::invalidCommand; @@ -294,13 +273,7 @@ auto GetStatusCommunication() -> Result // Get the status type code auto statusType = 0_b; - auto statusErrorCode = UartReceive(&statusType); - - // TODO: OUTCOME_TRY - if(statusErrorCode.has_error()) - { - return statusErrorCode.error(); - } + OUTCOME_TRY(UartReceive(&statusType)); if(statusType == noEventCode) { @@ -328,25 +301,14 @@ auto GetStatusCommunication() -> Result } auto dataBuffer = Buffer{}; - auto programFinishedError = UartReceive(dataBuffer); - - // TODO: OUTCOME_TRY - if(programFinishedError.has_error()) - { - return programFinishedError.error(); - } + OUTCOME_TRY(UartReceive(dataBuffer)); // Create another Buffer which includes the status type that was received beforehand because // it is needed to calculate the CRC32 checksum auto fullDataBuffer = std::array{}; fullDataBuffer[0] = statusType; std::copy(dataBuffer.begin(), dataBuffer.end(), fullDataBuffer.begin() + 1); - auto crc32Error = CheckCrc32(fullDataBuffer); - // TODO:OUTCOME_TRY - if(crc32Error.has_error()) - { - return crc32Error.error(); - } + OUTCOME_TRY(CheckCrc32(fullDataBuffer)); auto programFinishedData = Deserialize(dataBuffer); return Status{.statusType = StatusType::programFinished, @@ -364,23 +326,15 @@ auto GetStatusCommunication() -> Result auto dataBuffer = Buffer{}; auto resultsReadyError = UartReceive(dataBuffer); - // TODO:OUTCOME_TRY - if(resultsReadyError.has_error()) - { - return resultsReadyError.error(); - } + OUTCOME_TRY(UartReceive(dataBuffer)); // Create another Buffer which includes the status type that was received beforehand because // it is needed to calculate the CRC32 checksum auto fullDataBuffer = std::array{}; fullDataBuffer[0] = statusType; std::copy(dataBuffer.begin(), dataBuffer.end(), fullDataBuffer.begin() + 1); - auto crc32Error = CheckCrc32(fullDataBuffer); - // TODO: OUTCOME_TRY - if(crc32Error.has_error()) - { - return crc32Error.error(); - } + OUTCOME_TRY(CheckCrc32(fullDataBuffer)); + auto resultsReadyData = Deserialize(dataBuffer); return Status{.statusType = StatusType::resultsReady, .programId = resultsReadyData.programId, @@ -499,12 +453,7 @@ auto ReturnResultCommunication() -> Result // END DEBUG auto dataLengthBuffer = Buffer{}; - auto lengthError = UartReceive(dataLengthBuffer); - // TODO: OUTCOME_TRY - if(lengthError.has_error()) - { - return lengthError.error(); - } + OUTCOME_TRY(UartReceive(dataLengthBuffer)); auto actualDataLength = Deserialize(dataLengthBuffer); if(actualDataLength == 0U or actualDataLength > maxDataLength) @@ -564,12 +513,7 @@ auto UpdateTime(UpdateTimeData const & data) -> Result { RODOS::PRINTF("UpdateTime()\n"); auto serialData = Serialize(data); - auto errorCode = SendData(serialData); - // TODO:OUTCOME_TRY - if(errorCode.has_error()) - { - return errorCode.error(); - } + OUTCOME_TRY(SendData(serialData)); // On success, wait for second N/ACK // TODO: (Daniel) Change to UartReceive() @@ -649,10 +593,10 @@ auto SendData(std::span data) -> Result switch(answer) { // TODO: Return outcome::success - // case cmdAck: - //{ - // return ErrorCode::success; - // } + case cmdAck: + { + return outcome_v2::success(); + } case cmdNack: { nackCount++; @@ -694,7 +638,6 @@ auto UartReceive(std::span destination) -> Result } totalReceivedBytes += nReceivedBytes; } - // TODO: Test this return outcome_v2::success(); } @@ -750,7 +693,7 @@ auto CheckCrc32(std::span data) -> Result auto crc32Buffer = Buffer{}; - auto receiveError = UartReceive(crc32Buffer); + OUTCOME_TRY(UartReceive(crc32Buffer)); // DEBUG // RODOS::PRINTF("Received CRC: "); @@ -767,7 +710,7 @@ auto CheckCrc32(std::span data) -> Result { return ErrorCode::wrongChecksum; } - // TODO: Return outcome::successs + return outcome_v2::success(); } From 489b813daabfe338b146f0d88ba4dc06bb1232cc Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Sat, 11 Nov 2023 13:43:49 +0100 Subject: [PATCH 04/24] Fix typos and improve Outcome.test.cpp --- Sts1CobcSw/Edu/Edu.cpp | 2 +- Tests/UnitTests/Outcome.test.cpp | 55 ++++++++++++++++++++++++++------ 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/Sts1CobcSw/Edu/Edu.cpp b/Sts1CobcSw/Edu/Edu.cpp index f86df7bd..c0410eb6 100644 --- a/Sts1CobcSw/Edu/Edu.cpp +++ b/Sts1CobcSw/Edu/Edu.cpp @@ -531,7 +531,7 @@ auto UpdateTime(UpdateTimeData const & data) -> Result { // case cmdAck: //{ - // return ErrorCode::succes; + // return ErrorCode::success; // } case cmdNack: { diff --git a/Tests/UnitTests/Outcome.test.cpp b/Tests/UnitTests/Outcome.test.cpp index c62136ec..7c60dabc 100644 --- a/Tests/UnitTests/Outcome.test.cpp +++ b/Tests/UnitTests/Outcome.test.cpp @@ -1,4 +1,4 @@ -// Test the outcome libarary +// Test the outcome library #include #include @@ -83,7 +83,7 @@ TEST_CASE("Inspecting result") REQUIRE(result.has_value()); REQUIRE(result); // Boolean cast - // Test each defined convertion error + // Test each defined conversion error result = Convert("abc"); REQUIRE(result.has_error()); REQUIRE(result.error() == ConversionErrc::illegalChar); @@ -104,6 +104,20 @@ TEST_CASE("Inspecting result") REQUIRE(result.value() == 278); } + +// Dummy function to chain with Convert +auto WriteData(int * buffer, bool shouldSucceed) -> Result +{ + if(shouldSucceed) + { + *buffer = 1; + return outcome_v2::success(); + } + // Return some dummy error here, just to check that it is handled correctly + return ConversionErrc::emptyString; +} + + // Dummy function to chain with Convert // Return type is a pair just to display how OUTCOME_TRY works with different return<> types auto Add(int op1, const std::string & str) -> Result> @@ -112,25 +126,46 @@ auto Add(int op1, const std::string & str) -> Result> // Our control statement means: // if Convert() returned failure, this same error information should be returned from Add(), // even though Add() and Convert() have different result<> types. - // If Covnert returned success, we create variable op2 of type int with the value returned from + // If Convert returned success, we create variable op2 of type int with the value returned from // Convert. If control goes to subsequent line, it means Convert succeeded and variable of type // BigInt is in scope. OUTCOME_TRY(auto op2, Convert(str)); + + return std::make_pair(op1 + op2, op2); } +auto Write(bool shouldSucceed) -> Result +{ + // Same thing as in Add(), but this time the function that we are calling returns void in case + // of success, thus we do not need to provide two parameters to OUTCOME_TRY + int buffer = 0; + OUTCOME_TRY(WriteData(&buffer, shouldSucceed)); + return buffer; +} TEST_CASE("TRY macro") { // Test failure - auto result1 = Add(1, "3.14"); - REQUIRE(not result1.has_value()); - REQUIRE(result1.has_error()); + auto result1 = Write(/*shouldSucceed=*/false); + assert(result1.has_error()); REQUIRE(result1.error() == ConversionErrc::illegalChar); // Test success - auto result2 = Add(1, "278"); - REQUIRE(result2.has_value()); - REQUIRE(result2); - REQUIRE(result2.value().first == 279); + auto result2 = Write(/*shouldSucceed=*/true); + assert(result2.has_value()); + assert(result2); + REQUIRE(result2.value() == 1); + + // Test failure + auto result3 = Add(1, "3.14"); + REQUIRE(not result3.has_value()); + REQUIRE(result3.has_error()); + REQUIRE(result3.error() == ConversionErrc::illegalChar); + + // Test success + auto result4 = Add(1, "278"); + REQUIRE(result4.has_value()); + REQUIRE(result4); + REQUIRE(result4.value().first == 279); } From 5359e767a193c87e219b15d6c0788dc54bd3a674 Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Sat, 11 Nov 2023 14:21:38 +0100 Subject: [PATCH 05/24] Fix Experimental outcome macro definitions --- Sts1CobcSw/Utility/Outcome.hpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Sts1CobcSw/Utility/Outcome.hpp b/Sts1CobcSw/Utility/Outcome.hpp index 131c9981..a8c541cd 100644 --- a/Sts1CobcSw/Utility/Outcome.hpp +++ b/Sts1CobcSw/Utility/Outcome.hpp @@ -1,8 +1,6 @@ #pragma once -#if defined(GENERIC_SYSTEM) - #define OUTCOME_DISABLE_EXECINFO - #define SYSTEM_ERROR2_NOT_POSIX +#if defined(SYSTEM_ERROR2_NOT_POSIX) #define SYSTEM_ERROR2_FATAL(msg) RODOS::hwResetAndReboot() #endif From 052b5b5e6b3045a7c50c541b8f478235be9cdd14 Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Sat, 11 Nov 2023 14:46:09 +0100 Subject: [PATCH 06/24] Fix missing include of algorithm header in test --- Tests/UnitTests/Outcome.test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/UnitTests/Outcome.test.cpp b/Tests/UnitTests/Outcome.test.cpp index 7c60dabc..08b4afc6 100644 --- a/Tests/UnitTests/Outcome.test.cpp +++ b/Tests/UnitTests/Outcome.test.cpp @@ -3,6 +3,7 @@ #include #include +#include #include // First define a policy From 5a8eb21a64b26eadac18dd299576c559e6e76f3d Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Sat, 11 Nov 2023 14:52:54 +0100 Subject: [PATCH 07/24] Fix error in Outcome test --- Tests/UnitTests/Outcome.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/UnitTests/Outcome.test.cpp b/Tests/UnitTests/Outcome.test.cpp index 08b4afc6..00b4006a 100644 --- a/Tests/UnitTests/Outcome.test.cpp +++ b/Tests/UnitTests/Outcome.test.cpp @@ -150,7 +150,7 @@ TEST_CASE("TRY macro") // Test failure auto result1 = Write(/*shouldSucceed=*/false); assert(result1.has_error()); - REQUIRE(result1.error() == ConversionErrc::illegalChar); + REQUIRE(result1.error() == ConversionErrc::emptyString); // Test success auto result2 = Write(/*shouldSucceed=*/true); From d627f7f10c82251ccb45056bae9506de939ac9a0 Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Sat, 11 Nov 2023 17:15:50 +0100 Subject: [PATCH 08/24] Rework Edu.cpp and edu error codes Remove edu::ErrorCode::success/successEof/noErrorCodeSet and modify related files accordingly, sometimes by getting rid of do-while loops. --- Sts1CobcSw/Edu/Edu.cpp | 117 ++++++++++++------ Sts1CobcSw/Edu/Edu.hpp | 2 +- Sts1CobcSw/Edu/EduMock.cpp | 14 +-- Sts1CobcSw/Edu/Enums.hpp | 5 +- Sts1CobcSw/Edu/Structs.hpp | 2 +- Sts1CobcSw/EduListenerThread.cpp | 1 + .../EduCommandTests/EduCommands.test.cpp | 51 ++++++-- 7 files changed, 127 insertions(+), 65 deletions(-) diff --git a/Sts1CobcSw/Edu/Edu.cpp b/Sts1CobcSw/Edu/Edu.cpp index c0410eb6..b0f7290f 100644 --- a/Sts1CobcSw/Edu/Edu.cpp +++ b/Sts1CobcSw/Edu/Edu.cpp @@ -67,8 +67,8 @@ auto SendCommand(Byte commandId) -> void; auto FlushUartBuffer() -> void; [[nodiscard]] auto CheckCrc32(std::span data) -> Result; [[nodiscard]] auto GetStatusCommunication() -> Result; -[[nodiscard]] auto ReturnResultCommunication() -> Result; -[[nodiscard]] auto ReturnResultRetry() -> Result; +[[nodiscard]] auto ReturnResultCommunication() -> Result; +[[nodiscard]] auto ReturnResultRetry() -> Result; void MockWriteToFile(std::span data); auto Print(std::span data, int nRows = 30) -> void; // NOLINT @@ -218,41 +218,40 @@ auto GetStatus() -> Result auto serialData = Serialize(getStatusId); OUTCOME_TRY(SendData(serialData)); - Result status = ErrorCode::noErrorCodeSet; std::size_t errorCount = 0; - do + + while(true) { - status = GetStatusCommunication(); - if(status.has_value()) + auto status = GetStatusCommunication(); + if(status) { SendCommand(cmdAck); - break; + RODOS::PRINTF( + " .statusType = %d\n .programId = %d\n .startTime = %d\n exitCode = %d\n", + status.value().statusType, + status.value().programId, + status.value().startTime, + status.value().exitCode); + return status; } - + // Error in GetStatusCommunication() FlushUartBuffer(); SendCommand(cmdNack); - } while(errorCount++ < maxNNackRetries); + errorCount++; - if(status.has_value()) - { - RODOS::PRINTF(" .statusType = %d\n .programId = %d\n .startTime = %" PRIi32 - "\n .exitCode = %d\n", - static_cast(status.value().statusType), - status.value().programId, - status.value().startTime, - status.value().exitCode); - } - else - { - RODOS::PRINTF(" .errorCode = %d\n", status.error()); + if(errorCount >= maxNNackRetries) + { + RODOS::PRINTF(" .errorCode = %d\n", status.error()); + return status.error(); + } } - return status; } //! @brief Communication function for GetStatus() to separate a single try from //! retry logic. //! @returns The received EDU status +// NOLINTNEXTLINE(readability-function-cognitive-complexity) auto GetStatusCommunication() -> Result { // Get header data @@ -345,7 +344,7 @@ auto GetStatusCommunication() -> Result } -auto ReturnResult() -> Result +auto ReturnResult() -> Result { // DEBUG RODOS::PRINTF("ReturnResult()\n"); @@ -366,29 +365,38 @@ auto ReturnResult() -> Result std::size_t totalResultSize = 0U; std::size_t packets = 0U; Result resultInfo = ErrorCode::noErrorCodeSet; - // TODO: Turn into for loop + // TODO: Turn into for loop while(packets < maxNPackets) { // DEBUG // RODOS::PRINTF("\nPacket %d\n", static_cast(packets)); // END DEBUG - resultInfo = ReturnResultRetry(); + auto resultInfo = ReturnResultRetry(); + // TYPE Result // DEBUG + + // Break if returned an error or reached EOF if(resultInfo.has_error()) { auto errorCode = resultInfo.error(); RODOS::PRINTF(" ResultResultRetry() resulted in an error : %d", static_cast(errorCode)); - break; + return resultInfo.error(); } + if(resultInfo.value().reachedEof) + { + RODOS::PRINTF(" ResultResultRetry() reached EOF\n"); + return {/*reachedEof*/ true, totalResultSize}; + } + // END DEBUG // RODOS::PRINTF("\nWriting to file...\n"); // TODO: Actually write to a file - totalResultSize += resultInfo.value(); + totalResultSize += resultInfo.value().resultSize; packets++; } - return totalResultSize; + return {false, totalResultSize}; } @@ -396,23 +404,49 @@ auto ReturnResult() -> Result //! the actual ReturnResult function. The communication happens in ReturnResultCommunication. //! //! @returns An error code and the number of received bytes in ResultInfo -auto ReturnResultRetry() -> Result +auto ReturnResultRetry() -> Result { - Result result = ErrorCode::noErrorCodeSet; std::size_t errorCount = 0U; - do + + // TODO: infinite loop could be avoided by setting + // errorCount <= maxNNackRetries as the termination condition + while(true) { - result = ReturnResultCommunication(); - // TODO: That is ugly, maybe a successCommunicationEnum would be better - if(result.has_value() or (result.has_error() and result.error() == ErrorCode::successEof)) + auto resultInfo = ReturnResultCommunication(); + if(resultInfo.has_value()) { - SendCommand(cmdNack); - return result; + SendCommand(cmdAck); + // returns {reachedEof, resultSize} + return resultInfo.value(); } + + // Error in ReturnResultCommunication() FlushUartBuffer(); SendCommand(cmdNack); - } while(errorCount++ < maxNNackRetries); - return result.value(); + errorCount++; + if(errorCount == maxNNackRetries) + { + return resultInfo.error(); + } + } + + + // Result result = ErrorCode::noErrorCodeSet; + // std::size_t errorCount = 0U; + // // TODO: CHange this + // do + // { + // result = ReturnResultCommunication(); + // // Could have reached EOF or not + // if(result.has_value()) + // { + // SendCommand(cmdAck); + // return result; + // } + // FlushUartBuffer(); + // SendCommand(cmdNack); + // } while(errorCount++ < maxNNackRetries); + // return result.value(); } @@ -420,7 +454,7 @@ auto ReturnResultRetry() -> Result // directly and instead writes to a non-primary RAM bank as an intermediate step. // // Simple results -> 1 round should work with DMA to RAM -auto ReturnResultCommunication() -> Result +auto ReturnResultCommunication() -> Result { // Receive command // If no result is available, the command will be NACK, @@ -438,7 +472,7 @@ auto ReturnResultCommunication() -> Result } if(command == cmdEof) { - return ErrorCode::successEof; + return {/*reachedEof*/ true, 0_usize}; } if(command != cmdData) { @@ -469,6 +503,7 @@ auto ReturnResultCommunication() -> Result auto dataError = UartReceive( std::span(cepDataBuffer.begin(), cepDataBuffer.begin() + actualDataLength)); + // TODO: OUTCOME_TRY if(dataError.has_error()) { return dataError.error(); @@ -490,7 +525,7 @@ auto ReturnResultCommunication() -> Result RODOS::PRINTF("\nSuccess\n"); // END DEBUG - return actualDataLength; + return {false, actualDataLength}; } diff --git a/Sts1CobcSw/Edu/Edu.hpp b/Sts1CobcSw/Edu/Edu.hpp index 5e9fb0d9..02351931 100644 --- a/Sts1CobcSw/Edu/Edu.hpp +++ b/Sts1CobcSw/Edu/Edu.hpp @@ -26,6 +26,6 @@ auto TurnOff() -> void; [[nodiscard]] auto StopProgram() -> Result; // TODD: Find better name (or maybe even mechanism) for GetStatus [[nodiscard]] auto GetStatus() -> Result; -[[nodiscard]] auto ReturnResult() -> Result; +[[nodiscard]] auto ReturnResult() -> Result; [[nodiscard]] auto UpdateTime(UpdateTimeData const & data) -> Result; } diff --git a/Sts1CobcSw/Edu/EduMock.cpp b/Sts1CobcSw/Edu/EduMock.cpp index f7f48257..c3df4acc 100644 --- a/Sts1CobcSw/Edu/EduMock.cpp +++ b/Sts1CobcSw/Edu/EduMock.cpp @@ -68,7 +68,7 @@ auto ExecuteProgram(ExecuteProgramData const & data) -> Result data.programId, data.startTime, data.timeout); - return ErrorCode::success; + return outcome_v2::success(); } @@ -77,7 +77,7 @@ auto StopProgram() -> Result { PrintFormattedSystemUtc(); PRINTF("Call to StopProgram()\n"); - return ErrorCode::success; + return outcome_v2::success(); } @@ -95,7 +95,7 @@ auto UpdateTime(UpdateTimeData const & data) -> Result { PrintFormattedSystemUtc(); PRINTF("Call to UpdateTime(currentTime = %" PRIi32 ")\n", data.currentTime); - return ErrorCode::success; + return outcome_v2::success(); } @@ -108,20 +108,20 @@ auto SendCommand(Byte commandId) -> void // NOLINTNEXTLINE(readability-convert-member-functions-to-static) -auto SendData(std::span data) -> ErrorCode +auto SendData(std::span data) -> Result { PrintFormattedSystemUtc(); PRINTF("Call to SendData(size(data) = %d)\n", size(data)); - return ErrorCode::success; + return outcome_v2::success(); } // NOLINTNEXTLINE(readability-convert-member-functions-to-static) -auto UartReceive([[maybe_unused]] std::span destination) -> ErrorCode +auto UartReceive([[maybe_unused]] std::span destination) -> Result { PrintFormattedSystemUtc(); PRINTF("Call to UartReceive(size(destination) = %d)\n", size(destination)); - return ErrorCode::success; + return outcome_v2::success(); } diff --git a/Sts1CobcSw/Edu/Enums.hpp b/Sts1CobcSw/Edu/Enums.hpp index a937fdc9..2d919a3e 100644 --- a/Sts1CobcSw/Edu/Enums.hpp +++ b/Sts1CobcSw/Edu/Enums.hpp @@ -6,10 +6,7 @@ namespace sts1cobcsw::edu { enum class ErrorCode { - noErrorCodeSet, - success, - successEof, - invalidResult, + invalidResult = 1, bufferTooSmall, uartNotInitialized, timeout, diff --git a/Sts1CobcSw/Edu/Structs.hpp b/Sts1CobcSw/Edu/Structs.hpp index bdf3a15e..13d1fe77 100644 --- a/Sts1CobcSw/Edu/Structs.hpp +++ b/Sts1CobcSw/Edu/Structs.hpp @@ -76,7 +76,7 @@ struct ProgramFinishedStatus struct ResultInfo { - ErrorCode errorCode = ErrorCode::noErrorCodeSet; + bool reachedEof = false; std::size_t resultSize = 0; }; } diff --git a/Sts1CobcSw/EduListenerThread.cpp b/Sts1CobcSw/EduListenerThread.cpp index 38f41b6b..5e82d292 100644 --- a/Sts1CobcSw/EduListenerThread.cpp +++ b/Sts1CobcSw/EduListenerThread.cpp @@ -35,6 +35,7 @@ class EduListenerThread : public RODOS::StaticThread<> } + // NOLINTNEXTLINE(readability-function-cognitive-complexity) void run() override { TIME_LOOP(0, timeLoopPeriod) diff --git a/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp b/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp index 4cbe5116..3e877aeb 100644 --- a/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp +++ b/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp @@ -64,7 +64,14 @@ class EduCommandsTest : public RODOS::StaticThread<> auto currentTime = utility::GetUnixUtc(); PRINTF("Sending UpdateTime(currentTime = %d)\n", static_cast(currentTime)); auto errorCode = edu::UpdateTime({.currentTime = currentTime}); - PRINTF("Returned error code: %d\n", static_cast(errorCode)); + if(errorCode.has_error()) + { + PRINTF("Returned error code: %d\n", static_cast(errorCode.error())); + } + else + { + PRINTF("UpdateTime executed successfully\n"); + } break; } case 'e': @@ -94,28 +101,50 @@ class EduCommandsTest : public RODOS::StaticThread<> timeout); auto errorCode = edu::ExecuteProgram( {.programId = programId, .startTime = startTime, .timeout = timeout}); - PRINTF("Returned error code: %d\n", static_cast(errorCode)); + // TODO: Fix naming + if(errorCode.has_error()) + { + PRINTF("Returned error code: %d\n", static_cast(errorCode.error())); + } + else + { + PRINTF("Execute Program Returned no error"); + } break; } case 'g': { PRINTF("Sending GetStatus()\n"); - auto status = edu::GetStatus(); + auto statusResult = edu::GetStatus(); PRINTF("Returned status:\n"); - PRINTF(" type = %d\n", static_cast(status.statusType)); - PRINTF(" program ID = %d\n", static_cast(status.programId)); - PRINTF(" startTime = %d\n", static_cast(status.startTime)); - PRINTF(" exit code = %d\n", static_cast(status.exitCode)); - PRINTF(" error code = %d\n", static_cast(status.errorCode)); + if(statusResult.has_error()) + { + PRINTF(" error code = %d\n", static_cast(statusResult.error())); + } + else + { + auto status = statusResult.value(); + PRINTF(" type = %d\n", static_cast(status.statusType)); + PRINTF(" program ID = %d\n", static_cast(status.programId)); + PRINTF(" startTime = %d\n", static_cast(status.startTime)); + PRINTF(" exit code = %d\n", static_cast(status.exitCode)); + } break; } case 'r': { PRINTF("Sending ReturnResult()\n"); - auto resultInfo = edu::ReturnResult(); + auto resultStatus = edu::ReturnResult(); PRINTF("Returned result info:\n"); - PRINTF(" error code = %d\n", static_cast(resultInfo.errorCode)); - PRINTF(" result size = %d\n", static_cast(resultInfo.resultSize)); + if(resultStatus.has_error()) + { + PRINTF(" error code = %d\n", static_cast(resultStatus.error())); + } + else + { + PRINTF(" result size = %d\n", + static_cast(resultStatus.value().resultSize)); + } break; } default: From 0d41ced626899b15d829a1d53b897091f3d79c55 Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Sat, 11 Nov 2023 17:47:27 +0100 Subject: [PATCH 09/24] Fix Outcome bug when returning a ResultInfo structure --- Sts1CobcSw/Edu/Edu.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Sts1CobcSw/Edu/Edu.cpp b/Sts1CobcSw/Edu/Edu.cpp index b0f7290f..ef3bd843 100644 --- a/Sts1CobcSw/Edu/Edu.cpp +++ b/Sts1CobcSw/Edu/Edu.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -386,7 +387,7 @@ auto ReturnResult() -> Result if(resultInfo.value().reachedEof) { RODOS::PRINTF(" ResultResultRetry() reached EOF\n"); - return {/*reachedEof*/ true, totalResultSize}; + return ResultInfo{.reachedEof = true, .resultSize = totalResultSize}; } // END DEBUG @@ -396,7 +397,7 @@ auto ReturnResult() -> Result totalResultSize += resultInfo.value().resultSize; packets++; } - return {false, totalResultSize}; + return ResultInfo{.reachedEof = false, .resultSize = totalResultSize}; } @@ -472,7 +473,7 @@ auto ReturnResultCommunication() -> Result } if(command == cmdEof) { - return {/*reachedEof*/ true, 0_usize}; + return ResultInfo{.reachedEof = true, .resultSize = 0_usize}; } if(command != cmdData) { @@ -525,7 +526,7 @@ auto ReturnResultCommunication() -> Result RODOS::PRINTF("\nSuccess\n"); // END DEBUG - return {false, actualDataLength}; + return ResultInfo{.reachedEof = false, .resultSize = actualDataLength}; } @@ -691,6 +692,7 @@ auto UartReceive(void * destination) -> Result { return ErrorCode::timeout; } + return outcome_v2::success(); } From 499d95a123e5ab1bef2d86cc3d600f5179d10df4 Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Sun, 3 Dec 2023 19:11:49 +0100 Subject: [PATCH 10/24] Fix errors from rebase --- Sts1CobcSw/Edu/Edu.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Sts1CobcSw/Edu/Edu.cpp b/Sts1CobcSw/Edu/Edu.cpp index ef3bd843..2fd38362 100644 --- a/Sts1CobcSw/Edu/Edu.cpp +++ b/Sts1CobcSw/Edu/Edu.cpp @@ -365,7 +365,6 @@ auto ReturnResult() -> Result std::size_t totalResultSize = 0U; std::size_t packets = 0U; - Result resultInfo = ErrorCode::noErrorCodeSet; // TODO: Turn into for loop while(packets < maxNPackets) { @@ -473,7 +472,7 @@ auto ReturnResultCommunication() -> Result } if(command == cmdEof) { - return ResultInfo{.reachedEof = true, .resultSize = 0_usize}; + return ResultInfo{.reachedEof = true, .resultSize = 0U}; } if(command != cmdData) { @@ -738,11 +737,6 @@ auto CheckCrc32(std::span data) -> Result // RODOS::PRINTF("\n"); // END DEBUG - // TODO: OUTCOME_TRY - if(receiveError.has_error()) - { - return receiveError; - } if(computedCrc32 != Deserialize(crc32Buffer)) { return ErrorCode::wrongChecksum; From f639fbec5a0188cb826e3a8bfa4c741d6bb1059f Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sat, 9 Dec 2023 10:11:05 +0000 Subject: [PATCH 11/24] Refactor Outcome.test.cpp - Add missing #includes - Add missing blank lines - Improve comments and remove wrong ones - Use east const - ... --- Tests/UnitTests/Outcome.test.cpp | 41 +++++++++++++++++--------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/Tests/UnitTests/Outcome.test.cpp b/Tests/UnitTests/Outcome.test.cpp index 00b4006a..28547480 100644 --- a/Tests/UnitTests/Outcome.test.cpp +++ b/Tests/UnitTests/Outcome.test.cpp @@ -4,7 +4,11 @@ #include #include +#include #include +#include +#include + // First define a policy struct AbortPolicy : outcome_v2::experimental::policy::base @@ -13,8 +17,6 @@ struct AbortPolicy : outcome_v2::experimental::policy::base // NOLINTNEXTLINE(readability-identifier-naming) static constexpr void wide_value_check(Impl && self) { - //! Call RODOS::hwResetAndReboot() whenever .value() is called on an object that does not - //! contain a value if(!base::_has_value(std::forward(self))) { std::abort(); @@ -25,8 +27,6 @@ struct AbortPolicy : outcome_v2::experimental::policy::base // NOLINTNEXTLINE(readability-identifier-naming) static constexpr void wide_error_check(Impl && self) { - //! Call RODOS::hwResetAndReboot() whenever .error() is called on an object that does not - //! contain an error if(!base::_has_error(std::forward(self))) { std::abort(); @@ -44,6 +44,7 @@ struct AbortPolicy : outcome_v2::experimental::policy::base } }; + enum class ConversionErrc { success = 0, // 0 should not represent an error @@ -52,32 +53,35 @@ enum class ConversionErrc tooLong = 3, }; + template using Result = outcome_v2::experimental::status_result; -auto Convert(const std::string & str) noexcept -> Result +auto Convert(std::string const & str) noexcept -> Result { if(str.empty()) { return ConversionErrc::emptyString; } - if(!std::all_of(str.begin(), str.end(), ::isdigit)) + // NOLINTNEXTLINE(readability-identifier-length) + if(!std::all_of(str.begin(), str.end(), [](unsigned char c) { return std::isdigit(c); })) { return ConversionErrc::illegalChar; } - if(str.length() > 9) + if(str.length() > std::numeric_limits::digits10) { return ConversionErrc::tooLong; } - // NOLINTNEXTLINE (cert-err34-c) + // NOLINTNEXTLINE(cert-err34-c) return atoi(str.c_str()); } +// NOLINTNEXTLINE(cert-err58-cpp) TEST_CASE("Inspecting result") { Result result = outcome_v2::success(); @@ -119,23 +123,21 @@ auto WriteData(int * buffer, bool shouldSucceed) -> Result } -// Dummy function to chain with Convert -// Return type is a pair just to display how OUTCOME_TRY works with different return<> types -auto Add(int op1, const std::string & str) -> Result> +// Dummy function to chain with Convert. Return type is a pair just to display how OUTCOME_TRY works +// with different return types +auto Add(int op1, std::string const & str) -> Result> { // From https://ned14.github.io/outcome/tutorial/essential/result/inspecting/ - // Our control statement means: - // if Convert() returned failure, this same error information should be returned from Add(), - // even though Add() and Convert() have different result<> types. - // If Convert returned success, we create variable op2 of type int with the value returned from - // Convert. If control goes to subsequent line, it means Convert succeeded and variable of type - // BigInt is in scope. + // Our control statement means: if Convert() returned failure, this same error information + // should be returned from Add(), even though Add() and Convert() have different result<> types. + // If Convert() returned success, we create variable op2 of type int with the value returned + // from Convert(). If control goes to subsequent line, it means Convert() succeeded and variable + // of type int is in scope. OUTCOME_TRY(auto op2, Convert(str)); - - return std::make_pair(op1 + op2, op2); } + auto Write(bool shouldSucceed) -> Result { // Same thing as in Add(), but this time the function that we are calling returns void in case @@ -145,6 +147,7 @@ auto Write(bool shouldSucceed) -> Result return buffer; } + TEST_CASE("TRY macro") { // Test failure From bfa32a5d50f3a5cd13b454302f94fdde9b68b15b Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sat, 9 Dec 2023 10:22:08 +0000 Subject: [PATCH 12/24] Remove outdated NOLINTNEXTLINE() comments in EduMock.cpp --- Sts1CobcSw/Edu/EduMock.cpp | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/Sts1CobcSw/Edu/EduMock.cpp b/Sts1CobcSw/Edu/EduMock.cpp index c3df4acc..39e18043 100644 --- a/Sts1CobcSw/Edu/EduMock.cpp +++ b/Sts1CobcSw/Edu/EduMock.cpp @@ -27,7 +27,6 @@ auto ResumeEduProgramQueueThread() -> void // TODO: This file is not used at all right now. Think about the mocking later. namespace edu { -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) auto Initialize() -> void { PrintFormattedSystemUtc(); @@ -35,7 +34,6 @@ auto Initialize() -> void } -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) auto TurnOn() -> void { PrintFormattedSystemUtc(); @@ -43,7 +41,6 @@ auto TurnOn() -> void } -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) auto TurnOff() -> void { PrintFormattedSystemUtc(); @@ -51,7 +48,6 @@ auto TurnOff() -> void } -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) auto StoreArchive(StoreArchiveData const & data) -> Result { PrintFormattedSystemUtc(); @@ -60,7 +56,6 @@ auto StoreArchive(StoreArchiveData const & data) -> Result } -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) auto ExecuteProgram(ExecuteProgramData const & data) -> Result { PrintFormattedSystemUtc(); @@ -72,7 +67,6 @@ auto ExecuteProgram(ExecuteProgramData const & data) -> Result } -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) auto StopProgram() -> Result { PrintFormattedSystemUtc(); @@ -81,7 +75,6 @@ auto StopProgram() -> Result } -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) auto GetStatus() -> Result { PrintFormattedSystemUtc(); @@ -90,7 +83,6 @@ auto GetStatus() -> Result } -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) auto UpdateTime(UpdateTimeData const & data) -> Result { PrintFormattedSystemUtc(); @@ -99,7 +91,6 @@ auto UpdateTime(UpdateTimeData const & data) -> Result } -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) auto SendCommand(Byte commandId) -> void { PrintFormattedSystemUtc(); @@ -107,7 +98,6 @@ auto SendCommand(Byte commandId) -> void } -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) auto SendData(std::span data) -> Result { PrintFormattedSystemUtc(); @@ -116,7 +106,6 @@ auto SendData(std::span data) -> Result } -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) auto UartReceive([[maybe_unused]] std::span destination) -> Result { PrintFormattedSystemUtc(); @@ -125,7 +114,6 @@ auto UartReceive([[maybe_unused]] std::span destination) -> Result } -// NOLINTNEXTLINE(readability-convert-member-functions-to-static) void FlushUartBuffer() { PrintFormattedSystemUtc(); From abb62e26a4023bdedaba65b4292e7a0cd4a449d6 Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sat, 9 Dec 2023 10:54:10 +0000 Subject: [PATCH 13/24] Remove unused functions in EduMock.cpp --- Sts1CobcSw/Edu/EduMock.cpp | 39 +------------------------------------- 1 file changed, 1 insertion(+), 38 deletions(-) diff --git a/Sts1CobcSw/Edu/EduMock.cpp b/Sts1CobcSw/Edu/EduMock.cpp index 39e18043..f6d6d608 100644 --- a/Sts1CobcSw/Edu/EduMock.cpp +++ b/Sts1CobcSw/Edu/EduMock.cpp @@ -10,14 +10,7 @@ using RODOS::PRINTF; using utility::PrintFormattedSystemUtc; -// TODO: Move this to the proper file -auto ResumeEduErrorCommunicationThread() -> void -{ - PRINTF("\nCall to ResumeEduErrorCommunicationThread()\n"); - PrintFormattedSystemUtc(); -} - - +// TODO: Move this to EduProgramQueueThreadMock.cpp or something auto ResumeEduProgramQueueThread() -> void { PRINTF("Call to ResumeEduProgramQueueThread()\n"); @@ -89,35 +82,5 @@ auto UpdateTime(UpdateTimeData const & data) -> Result PRINTF("Call to UpdateTime(currentTime = %" PRIi32 ")\n", data.currentTime); return outcome_v2::success(); } - - -auto SendCommand(Byte commandId) -> void -{ - PrintFormattedSystemUtc(); - PRINTF("Call to SendCommand(commandId = 0x%02x)\n", static_cast(commandId)); -} - - -auto SendData(std::span data) -> Result -{ - PrintFormattedSystemUtc(); - PRINTF("Call to SendData(size(data) = %d)\n", size(data)); - return outcome_v2::success(); -} - - -auto UartReceive([[maybe_unused]] std::span destination) -> Result -{ - PrintFormattedSystemUtc(); - PRINTF("Call to UartReceive(size(destination) = %d)\n", size(destination)); - return outcome_v2::success(); -} - - -void FlushUartBuffer() -{ - PrintFormattedSystemUtc(); - PRINTF("Call to FlushUartBuffer()\n"); -} } } From 587cfdf87bc2d212b91585283641644e2f4a99eb Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sat, 9 Dec 2023 11:26:42 +0000 Subject: [PATCH 14/24] Rename reachedEof to eofIsReached --- Sts1CobcSw/Edu/Edu.cpp | 12 ++++++------ Sts1CobcSw/Edu/Structs.hpp | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Sts1CobcSw/Edu/Edu.cpp b/Sts1CobcSw/Edu/Edu.cpp index 2fd38362..bc72aae6 100644 --- a/Sts1CobcSw/Edu/Edu.cpp +++ b/Sts1CobcSw/Edu/Edu.cpp @@ -383,10 +383,10 @@ auto ReturnResult() -> Result static_cast(errorCode)); return resultInfo.error(); } - if(resultInfo.value().reachedEof) + if(resultInfo.value().eofIsReached) { RODOS::PRINTF(" ResultResultRetry() reached EOF\n"); - return ResultInfo{.reachedEof = true, .resultSize = totalResultSize}; + return ResultInfo{.eofIsReached = true, .resultSize = totalResultSize}; } // END DEBUG @@ -396,7 +396,7 @@ auto ReturnResult() -> Result totalResultSize += resultInfo.value().resultSize; packets++; } - return ResultInfo{.reachedEof = false, .resultSize = totalResultSize}; + return ResultInfo{.eofIsReached = false, .resultSize = totalResultSize}; } @@ -416,7 +416,7 @@ auto ReturnResultRetry() -> Result if(resultInfo.has_value()) { SendCommand(cmdAck); - // returns {reachedEof, resultSize} + // returns {eofIsReached, resultSize} return resultInfo.value(); } @@ -472,7 +472,7 @@ auto ReturnResultCommunication() -> Result } if(command == cmdEof) { - return ResultInfo{.reachedEof = true, .resultSize = 0U}; + return ResultInfo{.eofIsReached = true, .resultSize = 0U}; } if(command != cmdData) { @@ -525,7 +525,7 @@ auto ReturnResultCommunication() -> Result RODOS::PRINTF("\nSuccess\n"); // END DEBUG - return ResultInfo{.reachedEof = false, .resultSize = actualDataLength}; + return ResultInfo{.eofIsReached = false, .resultSize = actualDataLength}; } diff --git a/Sts1CobcSw/Edu/Structs.hpp b/Sts1CobcSw/Edu/Structs.hpp index 13d1fe77..dfdc7d06 100644 --- a/Sts1CobcSw/Edu/Structs.hpp +++ b/Sts1CobcSw/Edu/Structs.hpp @@ -76,7 +76,7 @@ struct ProgramFinishedStatus struct ResultInfo { - bool reachedEof = false; + bool eofIsReached = false; std::size_t resultSize = 0; }; } From 0659cd78e683ef666941df062a4c7c17cb3d23ca Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sat, 9 Dec 2023 15:08:00 +0000 Subject: [PATCH 15/24] Refactor Edu.cpp - Use OUTCOME_TRY() whenever possible - Fix two bugs introduced by using the Outcome library - Make some stylistic changes --- Sts1CobcSw/Edu/Edu.cpp | 73 ++++++++++++------------------------------ 1 file changed, 21 insertions(+), 52 deletions(-) diff --git a/Sts1CobcSw/Edu/Edu.cpp b/Sts1CobcSw/Edu/Edu.cpp index bc72aae6..ff2cebe4 100644 --- a/Sts1CobcSw/Edu/Edu.cpp +++ b/Sts1CobcSw/Edu/Edu.cpp @@ -283,14 +283,9 @@ auto GetStatusCommunication() -> Result } std::array statusTypeArray = {statusType}; - auto crc32Error = CheckCrc32(std::span(statusTypeArray)); - if(crc32Error.has_error()) - { - return crc32Error.error(); - } + OUTCOME_TRY(CheckCrc32(std::span(statusTypeArray))); - return Status{ - .statusType = StatusType::noEvent, .programId = 0, .startTime = 0, .exitCode = 0}; + return Status{.statusType = StatusType::noEvent}; } if(statusType == programFinishedCode) @@ -325,7 +320,6 @@ auto GetStatusCommunication() -> Result } auto dataBuffer = Buffer{}; - auto resultsReadyError = UartReceive(dataBuffer); OUTCOME_TRY(UartReceive(dataBuffer)); // Create another Buffer which includes the status type that was received beforehand because @@ -353,20 +347,16 @@ auto ReturnResult() -> Result // Send command auto serialCommand = Serialize(returnResultId); - auto commandError = SendData(serialCommand); - if(commandError.has_error()) - { - return commandError.error(); - } + OUTCOME_TRY(SendData(serialCommand)); // DEBUG // RODOS::PRINTF("\nStart receiving result\n"); // END DEBUG std::size_t totalResultSize = 0U; - std::size_t packets = 0U; + std::size_t nPackets = 0U; // TODO: Turn into for loop - while(packets < maxNPackets) + while(nPackets < maxNPackets) { // DEBUG // RODOS::PRINTF("\nPacket %d\n", static_cast(packets)); @@ -375,17 +365,18 @@ auto ReturnResult() -> Result // TYPE Result // DEBUG - // Break if returned an error or reached EOF + // Break if an error is returned if(resultInfo.has_error()) { auto errorCode = resultInfo.error(); - RODOS::PRINTF(" ResultResultRetry() resulted in an error : %d", + RODOS::PRINTF(" ReturnResultRetry() resulted in an error : %d", static_cast(errorCode)); return resultInfo.error(); } + // or if EOF is reached if(resultInfo.value().eofIsReached) { - RODOS::PRINTF(" ResultResultRetry() reached EOF\n"); + RODOS::PRINTF(" ReturnResultRetry() reached EOF\n"); return ResultInfo{.eofIsReached = true, .resultSize = totalResultSize}; } @@ -394,7 +385,7 @@ auto ReturnResult() -> Result // TODO: Actually write to a file totalResultSize += resultInfo.value().resultSize; - packets++; + nPackets++; } return ResultInfo{.eofIsReached = false, .resultSize = totalResultSize}; } @@ -460,11 +451,7 @@ auto ReturnResultCommunication() -> Result // If no result is available, the command will be NACK, // otherwise DATA Byte command = 0_b; - auto commandError = UartReceive(&command); - if(commandError.has_error()) - { - return commandError.error(); - } + OUTCOME_TRY(UartReceive(&command)); if(command == cmdNack) { // TODO: necessary to differentiate errors or just return success with resultSize 0? @@ -488,8 +475,8 @@ auto ReturnResultCommunication() -> Result auto dataLengthBuffer = Buffer{}; OUTCOME_TRY(UartReceive(dataLengthBuffer)); - auto actualDataLength = Deserialize(dataLengthBuffer); + if(actualDataLength == 0U or actualDataLength > maxDataLength) { return ErrorCode::invalidLength; @@ -500,26 +487,15 @@ auto ReturnResultCommunication() -> Result // END DEBUG // Get the actual data - auto dataError = UartReceive( - std::span(cepDataBuffer.begin(), cepDataBuffer.begin() + actualDataLength)); - - // TODO: OUTCOME_TRY - if(dataError.has_error()) - { - return dataError.error(); - } + OUTCOME_TRY(UartReceive( + std::span(cepDataBuffer.begin(), cepDataBuffer.begin() + actualDataLength))); // DEBUG // RODOS::PRINTF("\nCheck CRC\n"); // END DEBUG - auto crc32Error = CheckCrc32( - std::span(cepDataBuffer.begin(), cepDataBuffer.begin() + actualDataLength)); - - if(crc32Error.has_error()) - { - return crc32Error.error(); - } + OUTCOME_TRY(CheckCrc32( + std::span(cepDataBuffer.begin(), cepDataBuffer.begin() + actualDataLength))); // DEBUG RODOS::PRINTF("\nSuccess\n"); @@ -551,23 +527,17 @@ auto UpdateTime(UpdateTimeData const & data) -> Result OUTCOME_TRY(SendData(serialData)); // On success, wait for second N/ACK - // TODO: (Daniel) Change to UartReceive() // TODO: Refactor this common pattern into a function // TODO: Implement read functions that return a type and internally use Deserialize() auto answer = 0x00_b; - uart.suspendUntilDataReady(RODOS::NOW() + eduTimeout); + OUTCOME_TRY(UartReceive(&answer)); - auto nReadBytes = uart.read(&answer, 1); - if(nReadBytes == 0) - { - return ErrorCode::timeout; - } switch(answer) { - // case cmdAck: - //{ - // return ErrorCode::success; - // } + case cmdAck: + { + return outcome_v2::success(); + } case cmdNack: { return ErrorCode::nack; @@ -627,7 +597,6 @@ auto SendData(std::span data) -> Result // RODOS::PRINTF("[Edu] answer in sendData is now : %c\n", static_cast(answer)); switch(answer) { - // TODO: Return outcome::success case cmdAck: { return outcome_v2::success(); From 76b222d5aeed4f21b4170f93388f1daba6141dba Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sat, 9 Dec 2023 16:04:16 +0000 Subject: [PATCH 16/24] Rename some return values in EduCommands.test.cpp --- .../EduCommandTests/EduCommands.test.cpp | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp b/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp index 3e877aeb..7315c313 100644 --- a/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp +++ b/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp @@ -63,10 +63,10 @@ class EduCommandsTest : public RODOS::StaticThread<> { auto currentTime = utility::GetUnixUtc(); PRINTF("Sending UpdateTime(currentTime = %d)\n", static_cast(currentTime)); - auto errorCode = edu::UpdateTime({.currentTime = currentTime}); - if(errorCode.has_error()) + auto result = edu::UpdateTime({.currentTime = currentTime}); + if(result.has_error()) { - PRINTF("Returned error code: %d\n", static_cast(errorCode.error())); + PRINTF("Returned error code: %d\n", static_cast(result.error())); } else { @@ -99,12 +99,12 @@ class EduCommandsTest : public RODOS::StaticThread<> programId, startTime, timeout); - auto errorCode = edu::ExecuteProgram( + auto result = edu::ExecuteProgram( {.programId = programId, .startTime = startTime, .timeout = timeout}); // TODO: Fix naming - if(errorCode.has_error()) + if(result.has_error()) { - PRINTF("Returned error code: %d\n", static_cast(errorCode.error())); + PRINTF("Returned error code: %d\n", static_cast(result.error())); } else { @@ -115,15 +115,15 @@ class EduCommandsTest : public RODOS::StaticThread<> case 'g': { PRINTF("Sending GetStatus()\n"); - auto statusResult = edu::GetStatus(); + auto result = edu::GetStatus(); PRINTF("Returned status:\n"); - if(statusResult.has_error()) + if(result.has_error()) { - PRINTF(" error code = %d\n", static_cast(statusResult.error())); + PRINTF(" error code = %d\n", static_cast(result.error())); } else { - auto status = statusResult.value(); + auto status = result.value(); PRINTF(" type = %d\n", static_cast(status.statusType)); PRINTF(" program ID = %d\n", static_cast(status.programId)); PRINTF(" startTime = %d\n", static_cast(status.startTime)); @@ -134,16 +134,15 @@ class EduCommandsTest : public RODOS::StaticThread<> case 'r': { PRINTF("Sending ReturnResult()\n"); - auto resultStatus = edu::ReturnResult(); + auto result = edu::ReturnResult(); PRINTF("Returned result info:\n"); - if(resultStatus.has_error()) + if(result.has_error()) { - PRINTF(" error code = %d\n", static_cast(resultStatus.error())); + PRINTF(" error code = %d\n", static_cast(result.error())); } else { - PRINTF(" result size = %d\n", - static_cast(resultStatus.value().resultSize)); + PRINTF(" result size = %d\n", static_cast(result.value().resultSize)); } break; } From cf2337bd81aa2e892fa8edeee821a0e1744d355a Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sun, 10 Dec 2023 16:25:56 +0000 Subject: [PATCH 17/24] Refactor EduListenerThread.cpp a bit --- Sts1CobcSw/EduListenerThread.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Sts1CobcSw/EduListenerThread.cpp b/Sts1CobcSw/EduListenerThread.cpp index 5e82d292..27d47233 100644 --- a/Sts1CobcSw/EduListenerThread.cpp +++ b/Sts1CobcSw/EduListenerThread.cpp @@ -35,7 +35,6 @@ class EduListenerThread : public RODOS::StaticThread<> } - // NOLINTNEXTLINE(readability-function-cognitive-complexity) void run() override { TIME_LOOP(0, timeLoopPeriod) @@ -47,7 +46,7 @@ class EduListenerThread : public RODOS::StaticThread<> eduIsAliveBufferForListener.get(eduIsAlive); // RODOS::PRINTF("[EduListenerThread] Read eduHasUpdate pin\n"); - // TODO: Check if edu is alive + // TODO: Check if EDU is alive if(eduIsAlive and eduHasUpdate) { // RODOS::PRINTF("[EduListenerThread] Edu is alive and has an update\n"); @@ -104,9 +103,8 @@ class EduListenerThread : public RODOS::StaticThread<> // Edu wants to send result file // Send return result to Edu, Communicate, and interpret the results to // update the S&H Entry from 3 or 4 to 5. - auto resultsInfo = edu::ReturnResult(); - // TODO: Naming - if(resultsInfo.has_error()) + auto resultInfoResult = edu::ReturnResult(); + if(resultInfoResult.has_error()) { /* RODOS::PRINTF( From cbac2e5ae3b023d2accfb540ed2cac719309f75d Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sun, 10 Dec 2023 16:45:13 +0000 Subject: [PATCH 18/24] Rename some return values in EduProgramQueueThread.cpp --- Sts1CobcSw/EduProgramQueueThread.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Sts1CobcSw/EduProgramQueueThread.cpp b/Sts1CobcSw/EduProgramQueueThread.cpp index d4381862..659fad37 100644 --- a/Sts1CobcSw/EduProgramQueueThread.cpp +++ b/Sts1CobcSw/EduProgramQueueThread.cpp @@ -93,11 +93,12 @@ class EduProgramQueueThread : public RODOS::StaticThread RODOS::PRINTF("Resuming here after first wait.\n"); utility::PrintFormattedSystemUtc(); - auto errorCode = + auto updateTimeResult = edu::UpdateTime(edu::UpdateTimeData{.currentTime = utility::GetUnixUtc()}); - if(errorCode.has_error()) + if(updateTimeResult.has_error()) { - RODOS::PRINTF("UpdateTime error code : %d\n", static_cast(errorCode.error())); + RODOS::PRINTF("UpdateTime error code : %d\n", + static_cast(updateTimeResult.error())); RODOS::PRINTF( "[EduProgramQueueThread] Communication error after call to UpdateTime().\n"); ResumeEduCommunicationErrorThread(); @@ -126,10 +127,10 @@ class EduProgramQueueThread : public RODOS::StaticThread auto executeProgramData = edu::ExecuteProgramData{ .programId = programId, .startTime = startTime, .timeout = timeout}; // Start Process - errorCode = edu::ExecuteProgram(executeProgramData); + auto executeProgramResult = edu::ExecuteProgram(executeProgramData); // errorCode = edu::ErrorCode::success; - if(errorCode.has_error()) + if(executeProgramResult.has_error()) { RODOS::PRINTF( "[EduProgramQueueThread] Communication error after call to " From 0a1610c315c9b17e60e6075fb41aeef3a46bd1a1 Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Sat, 16 Dec 2023 18:39:44 +0100 Subject: [PATCH 19/24] Add logging for Outcome RebootPolicy --- Sts1CobcSw/Utility/Outcome.hpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Sts1CobcSw/Utility/Outcome.hpp b/Sts1CobcSw/Utility/Outcome.hpp index a8c541cd..ba469623 100644 --- a/Sts1CobcSw/Utility/Outcome.hpp +++ b/Sts1CobcSw/Utility/Outcome.hpp @@ -20,6 +20,8 @@ struct RebootPolicy : outcome_v2::experimental::policy::base //! contain a value if(!base::_has_value(std::forward(self))) { + RODOS::PRINTF( + "Error: The value is not present. Performing hardware reset and reboot.\n"); RODOS::hwResetAndReboot(); } } @@ -32,6 +34,8 @@ struct RebootPolicy : outcome_v2::experimental::policy::base //! contain an error if(!base::_has_error(std::forward(self))) { + RODOS::PRINTF( + "Error: The error is not present. Performing hardware reset and reboot.\n"); RODOS::hwResetAndReboot(); } } @@ -42,6 +46,8 @@ struct RebootPolicy : outcome_v2::experimental::policy::base { if(!base::_has_exception(std::forward(self))) { + RODOS::PRINTF( + "Error: The exception is not present. Performing hardware reset and reboot.\n"); RODOS::hwResetAndReboot(); } } From 61cc8c3c2ac290eee5a17a3698ccb0f7aee45348 Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Sat, 16 Dec 2023 18:49:44 +0100 Subject: [PATCH 20/24] Use REQUIRE() instead of assert() in outcome test --- Tests/UnitTests/Outcome.test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/UnitTests/Outcome.test.cpp b/Tests/UnitTests/Outcome.test.cpp index 28547480..70ecc09a 100644 --- a/Tests/UnitTests/Outcome.test.cpp +++ b/Tests/UnitTests/Outcome.test.cpp @@ -152,13 +152,13 @@ TEST_CASE("TRY macro") { // Test failure auto result1 = Write(/*shouldSucceed=*/false); - assert(result1.has_error()); + REQUIRE(result1.has_error()); REQUIRE(result1.error() == ConversionErrc::emptyString); // Test success auto result2 = Write(/*shouldSucceed=*/true); - assert(result2.has_value()); - assert(result2); + REQUIRE(result2.has_value()); + REQUIRE(result2); REQUIRE(result2.value() == 1); // Test failure From 6914beea28295616e34f8f03478b9912a727e1df Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Sat, 16 Dec 2023 19:12:32 +0100 Subject: [PATCH 21/24] Move Outcome.hpp into its own component --- Sts1CobcSw/Edu/Edu.hpp | 2 +- Sts1CobcSw/{Utility => ErrorHandling}/Outcome.hpp | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename Sts1CobcSw/{Utility => ErrorHandling}/Outcome.hpp (100%) diff --git a/Sts1CobcSw/Edu/Edu.hpp b/Sts1CobcSw/Edu/Edu.hpp index 02351931..9da93324 100644 --- a/Sts1CobcSw/Edu/Edu.hpp +++ b/Sts1CobcSw/Edu/Edu.hpp @@ -3,8 +3,8 @@ #include #include +#include #include -#include #include diff --git a/Sts1CobcSw/Utility/Outcome.hpp b/Sts1CobcSw/ErrorHandling/Outcome.hpp similarity index 100% rename from Sts1CobcSw/Utility/Outcome.hpp rename to Sts1CobcSw/ErrorHandling/Outcome.hpp From 2861215744ee30f1c0e3c37af5e688a3b51f9b68 Mon Sep 17 00:00:00 2001 From: Jerome Hue Date: Sat, 16 Dec 2023 20:54:27 +0100 Subject: [PATCH 22/24] Update naming scheme for Result variables --- Sts1CobcSw/Edu/Edu.cpp | 37 ++++++++++++++++---------------- Sts1CobcSw/EduListenerThread.cpp | 13 +++++------ 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/Sts1CobcSw/Edu/Edu.cpp b/Sts1CobcSw/Edu/Edu.cpp index ff2cebe4..83149d51 100644 --- a/Sts1CobcSw/Edu/Edu.cpp +++ b/Sts1CobcSw/Edu/Edu.cpp @@ -223,16 +223,17 @@ auto GetStatus() -> Result while(true) { - auto status = GetStatusCommunication(); - if(status) + auto getStatusCommunicationResult = GetStatusCommunication(); + if(getStatusCommunicationResult.has_value()) { + auto status = getStatusCommunicationResult.value(); SendCommand(cmdAck); RODOS::PRINTF( " .statusType = %d\n .programId = %d\n .startTime = %d\n exitCode = %d\n", - status.value().statusType, - status.value().programId, - status.value().startTime, - status.value().exitCode); + status.statusType, + status.programId, + status.startTime, + status.exitCode); return status; } // Error in GetStatusCommunication() @@ -242,8 +243,8 @@ auto GetStatus() -> Result if(errorCount >= maxNNackRetries) { - RODOS::PRINTF(" .errorCode = %d\n", status.error()); - return status.error(); + RODOS::PRINTF(" .errorCode = %d\n", getStatusCommunicationResult.error()); + return getStatusCommunicationResult.error(); } } } @@ -361,20 +362,20 @@ auto ReturnResult() -> Result // DEBUG // RODOS::PRINTF("\nPacket %d\n", static_cast(packets)); // END DEBUG - auto resultInfo = ReturnResultRetry(); + auto returnResultRetryResult = ReturnResultRetry(); // TYPE Result // DEBUG // Break if an error is returned - if(resultInfo.has_error()) + if(returnResultRetryResult.has_error()) { - auto errorCode = resultInfo.error(); + auto errorCode = returnResultRetryResult.error(); RODOS::PRINTF(" ReturnResultRetry() resulted in an error : %d", static_cast(errorCode)); - return resultInfo.error(); + return returnResultRetryResult.error(); } // or if EOF is reached - if(resultInfo.value().eofIsReached) + if(returnResultRetryResult.value().eofIsReached) { RODOS::PRINTF(" ReturnResultRetry() reached EOF\n"); return ResultInfo{.eofIsReached = true, .resultSize = totalResultSize}; @@ -384,7 +385,7 @@ auto ReturnResult() -> Result // RODOS::PRINTF("\nWriting to file...\n"); // TODO: Actually write to a file - totalResultSize += resultInfo.value().resultSize; + totalResultSize += returnResultRetryResult.value().resultSize; nPackets++; } return ResultInfo{.eofIsReached = false, .resultSize = totalResultSize}; @@ -403,12 +404,12 @@ auto ReturnResultRetry() -> Result // errorCount <= maxNNackRetries as the termination condition while(true) { - auto resultInfo = ReturnResultCommunication(); - if(resultInfo.has_value()) + auto returnResultCommunicationResult = ReturnResultCommunication(); + if(returnResultCommunicationResult.has_value()) { SendCommand(cmdAck); // returns {eofIsReached, resultSize} - return resultInfo.value(); + return returnResultCommunicationResult.value(); } // Error in ReturnResultCommunication() @@ -417,7 +418,7 @@ auto ReturnResultRetry() -> Result errorCount++; if(errorCount == maxNNackRetries) { - return resultInfo.error(); + return returnResultCommunicationResult.error(); } } diff --git a/Sts1CobcSw/EduListenerThread.cpp b/Sts1CobcSw/EduListenerThread.cpp index 27d47233..2759509a 100644 --- a/Sts1CobcSw/EduListenerThread.cpp +++ b/Sts1CobcSw/EduListenerThread.cpp @@ -52,11 +52,11 @@ class EduListenerThread : public RODOS::StaticThread<> // RODOS::PRINTF("[EduListenerThread] Edu is alive and has an update\n"); // Communicate with EDU - auto statusResult = edu::GetStatus(); + auto getStatusResult = edu::GetStatus(); // RODOS::PRINTF("EduStatus : %d, EduErrorcode %d\n", status.statusType, // status.errorCode); - if(statusResult.has_error()) + if(getStatusResult.has_error()) { // RODOS::PRINTF("[EduListenerThread] GetStatus() error code : %d.\n", // status.errorCode); @@ -65,15 +65,12 @@ class EduListenerThread : public RODOS::StaticThread<> // GetStatus().\n"); ResumeEduCommunicationErrorThread(); } - else + + if(getStatusResult.has_value()) { // RODOS::PRINTF("[EduListenerThread] Call to GetStatus() resulted in // success.\n"); - } - - if(statusResult.has_value()) - { - auto status = statusResult.value(); + auto status = getStatusResult.value(); switch(status.statusType) { case edu::StatusType::programFinished: From 3fd77d64bc1904fce1daa7dbe83e9e8d442eb12f Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sun, 17 Dec 2023 14:31:12 +0000 Subject: [PATCH 23/24] Add library target for Outcome and rename folder --- CMakeLists.txt | 1 + Sts1CobcSw/Edu/CMakeLists.txt | 2 +- Sts1CobcSw/Edu/Edu.hpp | 2 +- Sts1CobcSw/Outcome/CMakeLists.txt | 2 ++ Sts1CobcSw/{ErrorHandling => Outcome}/Outcome.hpp | 0 5 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 Sts1CobcSw/Outcome/CMakeLists.txt rename Sts1CobcSw/{ErrorHandling => Outcome}/Outcome.hpp (100%) diff --git a/CMakeLists.txt b/CMakeLists.txt index e01e2dd8..a69012ea 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -27,6 +27,7 @@ endif() add_library(Sts1CobcSw_Dummy STATIC) add_library(Sts1CobcSw_Edu STATIC) +add_library(Sts1CobcSw_Outcome INTERFACE) add_library(Sts1CobcSw_Serial INTERFACE) add_library(Sts1CobcSw_Utility STATIC) add_program(HelloDummy) diff --git a/Sts1CobcSw/Edu/CMakeLists.txt b/Sts1CobcSw/Edu/CMakeLists.txt index d542bdbf..2ff1aff4 100644 --- a/Sts1CobcSw/Edu/CMakeLists.txt +++ b/Sts1CobcSw/Edu/CMakeLists.txt @@ -1,5 +1,5 @@ target_sources(Sts1CobcSw_Edu PRIVATE ProgramQueue.cpp ProgramStatusHistory.cpp) -target_link_libraries(Sts1CobcSw_Edu PUBLIC rodos::rodos Sts1CobcSw_Serial) +target_link_libraries(Sts1CobcSw_Edu PUBLIC rodos::rodos Sts1CobcSw_Serial Sts1CobcSw_Outcome) target_link_libraries(Sts1CobcSw_Edu PRIVATE Sts1CobcSw_Utility) if(CMAKE_SYSTEM_NAME STREQUAL Generic) diff --git a/Sts1CobcSw/Edu/Edu.hpp b/Sts1CobcSw/Edu/Edu.hpp index 9da93324..60776385 100644 --- a/Sts1CobcSw/Edu/Edu.hpp +++ b/Sts1CobcSw/Edu/Edu.hpp @@ -3,7 +3,7 @@ #include #include -#include +#include #include #include diff --git a/Sts1CobcSw/Outcome/CMakeLists.txt b/Sts1CobcSw/Outcome/CMakeLists.txt new file mode 100644 index 00000000..c4b7ab93 --- /dev/null +++ b/Sts1CobcSw/Outcome/CMakeLists.txt @@ -0,0 +1,2 @@ +# TODO: When Outcome is installed via CMake link it here +target_link_libraries(Sts1CobcSw_Outcome INTERFACE rodos::rodos) diff --git a/Sts1CobcSw/ErrorHandling/Outcome.hpp b/Sts1CobcSw/Outcome/Outcome.hpp similarity index 100% rename from Sts1CobcSw/ErrorHandling/Outcome.hpp rename to Sts1CobcSw/Outcome/Outcome.hpp From 19d4d65b1add3ba5d1bb233612cd01586afe0206 Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Sun, 17 Dec 2023 14:41:05 +0000 Subject: [PATCH 24/24] Update naming scheme for Result variables, part 2 --- Sts1CobcSw/EduListenerThread.cpp | 4 +-- .../EduCommandTests/EduCommands.test.cpp | 32 +++++++++++-------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/Sts1CobcSw/EduListenerThread.cpp b/Sts1CobcSw/EduListenerThread.cpp index 2759509a..6957af34 100644 --- a/Sts1CobcSw/EduListenerThread.cpp +++ b/Sts1CobcSw/EduListenerThread.cpp @@ -100,8 +100,8 @@ class EduListenerThread : public RODOS::StaticThread<> // Edu wants to send result file // Send return result to Edu, Communicate, and interpret the results to // update the S&H Entry from 3 or 4 to 5. - auto resultInfoResult = edu::ReturnResult(); - if(resultInfoResult.has_error()) + auto returnResultResult = edu::ReturnResult(); + if(returnResultResult.has_error()) { /* RODOS::PRINTF( diff --git a/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp b/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp index 7315c313..ad222ef4 100644 --- a/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp +++ b/Tests/HardwareTests/EduCommandTests/EduCommands.test.cpp @@ -63,10 +63,11 @@ class EduCommandsTest : public RODOS::StaticThread<> { auto currentTime = utility::GetUnixUtc(); PRINTF("Sending UpdateTime(currentTime = %d)\n", static_cast(currentTime)); - auto result = edu::UpdateTime({.currentTime = currentTime}); - if(result.has_error()) + auto updateTimeResult = edu::UpdateTime({.currentTime = currentTime}); + if(updateTimeResult.has_error()) { - PRINTF("Returned error code: %d\n", static_cast(result.error())); + PRINTF("Returned error code: %d\n", + static_cast(updateTimeResult.error())); } else { @@ -99,12 +100,13 @@ class EduCommandsTest : public RODOS::StaticThread<> programId, startTime, timeout); - auto result = edu::ExecuteProgram( + auto executeProgramResult = edu::ExecuteProgram( {.programId = programId, .startTime = startTime, .timeout = timeout}); // TODO: Fix naming - if(result.has_error()) + if(executeProgramResult.has_error()) { - PRINTF("Returned error code: %d\n", static_cast(result.error())); + PRINTF("Returned error code: %d\n", + static_cast(executeProgramResult.error())); } else { @@ -115,15 +117,15 @@ class EduCommandsTest : public RODOS::StaticThread<> case 'g': { PRINTF("Sending GetStatus()\n"); - auto result = edu::GetStatus(); + auto getStatusResult = edu::GetStatus(); PRINTF("Returned status:\n"); - if(result.has_error()) + if(getStatusResult.has_error()) { - PRINTF(" error code = %d\n", static_cast(result.error())); + PRINTF(" error code = %d\n", static_cast(getStatusResult.error())); } else { - auto status = result.value(); + auto status = getStatusResult.value(); PRINTF(" type = %d\n", static_cast(status.statusType)); PRINTF(" program ID = %d\n", static_cast(status.programId)); PRINTF(" startTime = %d\n", static_cast(status.startTime)); @@ -134,15 +136,17 @@ class EduCommandsTest : public RODOS::StaticThread<> case 'r': { PRINTF("Sending ReturnResult()\n"); - auto result = edu::ReturnResult(); + auto returnResultResult = edu::ReturnResult(); PRINTF("Returned result info:\n"); - if(result.has_error()) + if(returnResultResult.has_error()) { - PRINTF(" error code = %d\n", static_cast(result.error())); + PRINTF(" error code = %d\n", + static_cast(returnResultResult.error())); } else { - PRINTF(" result size = %d\n", static_cast(result.value().resultSize)); + PRINTF(" result size = %d\n", + static_cast(returnResultResult.value().resultSize)); } break; }