From 9f90b5eb3e3ace3d7b9caed4a6d37ea017ba0049 Mon Sep 17 00:00:00 2001 From: Zimri Leisher Date: Thu, 18 Jun 2026 17:44:28 -0500 Subject: [PATCH 1/4] Fix no response from validate, refactor to prevent this class of issue, work on improving handling of path truncation --- Svc/FpySequencer/FpySequencer.cpp | 21 ++-- Svc/FpySequencer/FpySequencer.hpp | 71 ++++-------- Svc/FpySequencer/FpySequencerDirectives.cpp | 16 +-- Svc/FpySequencer/FpySequencerEvents.fppi | 46 ++++---- Svc/FpySequencer/FpySequencerParams.fppi | 6 - Svc/FpySequencer/FpySequencerRunState.cpp | 6 +- Svc/FpySequencer/FpySequencerStateMachine.cpp | 107 +++++------------- .../FpySequencerStateMachine.fppi | 36 +++--- Svc/FpySequencer/FpySequencerTelemetry.fppi | 3 - .../FpySequencerValidationState.cpp | 57 +++++++--- Svc/FpySequencer/docs/sdd.md | 6 +- .../test/ut/FpySequencerTestMain.cpp | 41 +++---- .../test/ut/FpySequencerTestSequences.cpp | 25 ++++ .../test/ut/FpySequencerTester.cpp | 16 +-- .../test/ut/FpySequencerTester.hpp | 9 +- default/config/FpySequencerCfg.fpp | 9 +- 16 files changed, 215 insertions(+), 260 deletions(-) diff --git a/Svc/FpySequencer/FpySequencer.cpp b/Svc/FpySequencer/FpySequencer.cpp index a41600e16a1..1df92b76747 100644 --- a/Svc/FpySequencer/FpySequencer.cpp +++ b/Svc/FpySequencer/FpySequencer.cpp @@ -17,14 +17,13 @@ FpySequencer ::FpySequencer(const char* const compName) : FpySequencerComponentBase(compName), m_sequenceBuffer(), m_allocatorId(0), - m_sequenceFilePath(""), + m_sequenceExecArgs(), + m_fullSequenceFilePath(), m_sequenceObj(), m_computedCRC(), m_totalExpectedArgSize(0), - m_sequenceBlockState(), m_savedOpCode(0), m_savedCmdSeq(0), - m_sequenceArgs(0, 0), m_goalState(), m_sequencesStarted(0), m_statementsDispatched(0), @@ -126,8 +125,13 @@ void FpySequencer::RUN_VALIDATED_cmdHandler(FwOpcodeType opCode, //!< The opcod this->m_savedCmdSeq = cmdSeq; } - this->sequencer_sendSignal_cmd_RUN_VALIDATED( - FpySequencer_SequenceExecutionArgs(this->m_sequenceFilePath, block, this->m_sequenceArgs)); + // copy it + FpySequencer_SequenceExecutionArgs execArgs = this->m_sequenceExecArgs; + // update just the block state + // the rest should stay the same + execArgs.set_blockState(block); + + this->sequencer_sendSignal_cmd_RUN_VALIDATED(execArgs); // only respond if the user doesn't want us to block further execution if (block == BlockState::NO_BLOCK) { @@ -414,7 +418,7 @@ void FpySequencer::tlmWrite_handler(FwIndexType portNum, //!< The port number this->tlmWrite_LastDirectiveError(this->m_tlm.lastDirectiveError); this->tlmWrite_DirectiveErrorIndex(this->m_tlm.directiveErrorIndex); this->tlmWrite_DirectiveErrorId(this->m_tlm.directiveErrorId); - this->tlmWrite_SeqPath(this->m_sequenceFilePath); + this->tlmWrite_SeqPath(this->m_fullSequenceFilePath); this->tlmWrite_BreakpointIndex(this->m_breakpoint.breakpointIndex); this->tlmWrite_BreakOnlyOnceOnBreakpoint(this->m_breakpoint.breakOnlyOnceOnBreakpoint); @@ -488,7 +492,6 @@ void FpySequencer::updateDebugTelemetryStruct() { void FpySequencer::parametersLoaded() { parameterUpdated(PARAMID_STATEMENT_TIMEOUT_SECS); - parameterUpdated(PARAMID_SEQ_BASE_DIR); } void FpySequencer::parameterUpdated(FwPrmIdType id) { @@ -498,10 +501,6 @@ void FpySequencer::parameterUpdated(FwPrmIdType id) { this->tlmWrite_PRM_STATEMENT_TIMEOUT_SECS(this->paramGet_STATEMENT_TIMEOUT_SECS(valid)); break; } - case PARAMID_SEQ_BASE_DIR: { - this->tlmWrite_PRM_SEQ_BASE_DIR(this->paramGet_SEQ_BASE_DIR(valid)); - break; - } default: { FW_ASSERT(0, static_cast(id)); // coding error, forgot to include in switch statement } diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index 8e39b89413e..42a79993e47 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -258,28 +258,10 @@ class FpySequencer : public FpySequencerComponentBase { Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal ) override; - //! Implementation for action setSequenceFilePath of state machine Svc_FpySequencer_SequencerStateMachine + //! Implementation for action setSequenceExecArgs of state machine Svc_FpySequencer_SequencerStateMachine //! - //! sets the current sequence file path member var - void Svc_FpySequencer_SequencerStateMachine_action_setSequenceFilePath( - SmId smId, //!< The state machine id - Svc_FpySequencer_SequencerStateMachine::Signal signal, //!< The signal - const Svc::FpySequencer_SequenceExecutionArgs& value //!< The value - ) override; - - //! Implementation for action setSequenceBlockState of state machine Svc_FpySequencer_SequencerStateMachine - //! - //! sets the block state of the sequence to be run - void Svc_FpySequencer_SequencerStateMachine_action_setSequenceBlockState( - SmId smId, //!< The state machine id - Svc_FpySequencer_SequencerStateMachine::Signal signal, //!< The signal - const Svc::FpySequencer_SequenceExecutionArgs& value //!< The value - ) override; - - //! Implementation for action setSequenceArguments of state machine Svc_FpySequencer_SequencerStateMachine - //! - //! sets the arguments to pass to the sequence - void Svc_FpySequencer_SequencerStateMachine_action_setSequenceArguments( + //! sets the sequence execution arguments (seq path, block state, seq args) + void Svc_FpySequencer_SequencerStateMachine_action_setSequenceExecArgs( SmId smId, //!< The state machine id Svc_FpySequencer_SequencerStateMachine::Signal signal, //!< The signal const Svc::FpySequencer_SequenceExecutionArgs& value //!< The value @@ -358,22 +340,6 @@ class FpySequencer : public FpySequencerComponentBase { Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal ) override; - //! Implementation for action clearSequenceFile of state machine Svc_FpySequencer_SequencerStateMachine - //! - //! clears all variables related to the loading/validating of the sequence file - void Svc_FpySequencer_SequencerStateMachine_action_clearSequenceFile( - SmId smId, //!< The state machine id - Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal - ) override; - - //! Implementation for action clearSequenceArguments of state machine Svc_FpySequencer_SequencerStateMachine - //! - //! clears arguments - void Svc_FpySequencer_SequencerStateMachine_action_clearSequenceArguments( - SmId smId, //!< The state machine id - Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal - ) override; - //! Implementation for action checkShouldWake of state machine Svc_FpySequencer_SequencerStateMachine //! //! checks if sequencer should wake from sleep @@ -422,6 +388,15 @@ class FpySequencer : public FpySequencerComponentBase { Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal ) override; + //! Implementation for action clearSequenceExecArgs of state machine Svc_FpySequencer_SequencerStateMachine + //! + //! clears the sequence execution arguments (seq path, block state, seq args) + //! and the m_fullSequenceFilePath member var + void Svc_FpySequencer_SequencerStateMachine_action_clearSequenceExecArgs( + SmId smId, //!< The state machine id + Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal + ) override; + //! Implementation for action report_seqBroken of state machine Svc_FpySequencer_SequencerStateMachine //! //! reports that a breakpoint was hit @@ -645,29 +620,31 @@ class FpySequencer : public FpySequencerComponentBase { // id of allocator that gave us m_sequenceBuffer FwEnumStoreType m_allocatorId; - // assigned by the user via cmd - // length is FileNameStringSize - Fw::FileNameString m_sequenceFilePath; + // the execution arguments of the sequence: + // * the file name + // * the block state + // * the sequence runtime args + // See the struct member docstrings for more info + Svc::FpySequencer_SequenceExecutionArgs m_sequenceExecArgs; + + // the full (including the SEQ_BASE_DIR prefix) path + // to the sequence file. Note: not updated until validate() is run + Fw::FileNameString m_fullSequenceFilePath; + // the sequence, loaded in memory Fpy::Sequence m_sequenceObj; // live running computation of CRC (updated as we read) Utils::Hash m_computedCRC; // Size of arguments read in current sequence. Used for validation between - // User provided arguments and what is requested of the sequence. + // user provided arguments and what is requested of the sequence. Fpy::StackSizeType m_totalExpectedArgSize; - // whether or not the sequence we're about to run should return immediately or - // block on completion - Svc::BlockState m_sequenceBlockState; // if we are to block on completion, save the opCode and cmdSeq we should // return FwOpcodeType m_savedOpCode; U32 m_savedCmdSeq; - // sequence arguments to push to stack when entering RUNNING state - Svc::SeqArgs m_sequenceArgs{}; - // the goal state is the state that we're trying to reach in the sequencer // if it's RUNNING, then we should promptly go to RUNNING once we validate the // sequence. if it's VALID, we should wait after VALIDATING diff --git a/Svc/FpySequencer/FpySequencerDirectives.cpp b/Svc/FpySequencer/FpySequencerDirectives.cpp index 169025e36b2..3373f955b67 100644 --- a/Svc/FpySequencer/FpySequencerDirectives.cpp +++ b/Svc/FpySequencer/FpySequencerDirectives.cpp @@ -1119,7 +1119,7 @@ Signal FpySequencer::exit_directiveHandler(const FpySequencer_ExitDirective& dir } // otherwise, kill the sequence here // raise the user defined error code as an event - this->log_WARNING_HI_SequenceExitedWithError(this->m_sequenceFilePath, errorCode); + this->log_WARNING_HI_SequenceExitedWithError(this->m_fullSequenceFilePath, errorCode); error = DirectiveError::EXIT_WITH_ERROR; return Signal::stmtResponse_failure; } @@ -1602,25 +1602,25 @@ Signal FpySequencer::popEvent_directiveHandler(const FpySequencer_PopEventDirect // Emit the appropriate event based on severity switch (severity) { case Fw::LogSeverity::FATAL: - this->log_FATAL_LogFatal(this->m_sequenceFilePath, messageStr); + this->log_FATAL_LogFatal(this->m_fullSequenceFilePath, messageStr); break; case Fw::LogSeverity::WARNING_HI: - this->log_WARNING_HI_LogWarningHi(this->m_sequenceFilePath, messageStr); + this->log_WARNING_HI_LogWarningHi(this->m_fullSequenceFilePath, messageStr); break; case Fw::LogSeverity::WARNING_LO: - this->log_WARNING_LO_LogWarningLo(this->m_sequenceFilePath, messageStr); + this->log_WARNING_LO_LogWarningLo(this->m_fullSequenceFilePath, messageStr); break; case Fw::LogSeverity::COMMAND: - this->log_COMMAND_LogCommand(this->m_sequenceFilePath, messageStr); + this->log_COMMAND_LogCommand(this->m_fullSequenceFilePath, messageStr); break; case Fw::LogSeverity::ACTIVITY_HI: - this->log_ACTIVITY_HI_LogActivityHi(this->m_sequenceFilePath, messageStr); + this->log_ACTIVITY_HI_LogActivityHi(this->m_fullSequenceFilePath, messageStr); break; case Fw::LogSeverity::ACTIVITY_LO: - this->log_ACTIVITY_LO_LogActivityLo(this->m_sequenceFilePath, messageStr); + this->log_ACTIVITY_LO_LogActivityLo(this->m_fullSequenceFilePath, messageStr); break; case Fw::LogSeverity::DIAGNOSTIC: - this->log_DIAGNOSTIC_LogDiagnostic(this->m_sequenceFilePath, messageStr); + this->log_DIAGNOSTIC_LogDiagnostic(this->m_fullSequenceFilePath, messageStr); break; default: error = DirectiveError::INVALID_ARG; diff --git a/Svc/FpySequencer/FpySequencerEvents.fppi b/Svc/FpySequencer/FpySequencerEvents.fppi index 9188de053a7..33fbf5ca371 100644 --- a/Svc/FpySequencer/FpySequencerEvents.fppi +++ b/Svc/FpySequencer/FpySequencerEvents.fppi @@ -15,7 +15,7 @@ event FileOpenError( errorCode: I32 ) \ severity warning high \ - format "File open error encountered while opening {}: {}" + format "File open error encountered while opening '{}': {}" event FileWriteError( writeSize: FwSizeType @@ -23,7 +23,7 @@ event FileWriteError( errorCode: I32 ) \ severity warning high \ - format "File write error encountered while writing {} bytes to {}: {}" + format "File write error encountered while writing {} bytes to '{}': {}" event FileReadError( readStage: FileReadStage @@ -31,14 +31,14 @@ event FileReadError( errorCode: I32 ) \ severity warning high \ - format "File read error encountered while reading {} of file {}: {}" + format "File read error encountered while reading {} of file '{}': {}" event EndOfFileError( readStage: FileReadStage filePath: string ) \ severity warning high \ - format "End of file encountered unexpectedly while reading {} of file {}" + format "End of file encountered unexpectedly while reading {} of file '{}'" event FileReadDeserializeError( readStage: FileReadStage @@ -48,7 +48,7 @@ event FileReadDeserializeError( buffLength: U64 ) \ severity warning high \ - format "Deserialize error encountered while reading {} of file {}: {} ({} bytes left out of {})" + format "Deserialize error encountered while reading {} of file '{}': {} ({} bytes left out of {})" event WrongSchemaVersion( expected: U8 @@ -75,21 +75,21 @@ event InsufficientBufferSpace( filePath: string ) \ severity warning high \ - format "Buffer capacity of {} was not big enough for sequence {}" + format "Buffer capacity of {} was not big enough for sequence '{}'" event FileApiError( filePath: string errorCode: I32 ) \ severity warning high \ - format "File system API error encountered while operating on file {}: {}" + format "File system API error encountered while operating on file '{}': {}" event SequenceFilePathTooLong( baseDir: string fileName: string ) \ severity warning high \ - format "Sequence file path was truncated: base directory {} and file name {} together exceed the maximum path length" + format "SEQ_BASE_DIR '{}' and file name '{}' together exceed the maximum path length" # runtime @@ -100,26 +100,26 @@ event CommandFailed( response: Fw.CmdResponse ) \ severity warning high \ - format "Failed to execute command opcode {} index {} in sequence file {}: response was {}" + format "Failed to execute command opcode {} index {} in sequence file '{}': response was {}" event SequenceDone( filePath: string ) \ severity activity high \ - format "Completed sequence file {}" + format "Completed sequence file '{}'" event SequenceCancelled( filePath: string ) \ severity activity high \ - format "Cancelled sequence file {}" + format "Cancelled sequence file '{}'" event SequenceExitedWithError( filePath: string errorCode: U8 ) \ severity warning high \ - format "Sequence {} exited with error code {}" + format "Sequence '{}' exited with error code {}" event UnknownSequencerDirective( $opcode: U8 @@ -127,7 +127,7 @@ event UnknownSequencerDirective( filePath: string ) \ severity warning high \ - format "Unknown sequencer directive id {} at index {} in file {}" + format "Unknown sequencer directive id {} at index {} in file '{}'" event CmdResponseWhileNotRunningSequence( $state: I32 @@ -209,7 +209,7 @@ event CommandTimedOut( filePath: string ) \ severity warning high \ - format "A command opcode {} at index {} timed out in sequence {}, causing the sequence to fail" + format "A command opcode {} at index {} timed out in sequence '{}', causing the sequence to fail" event DirectiveTimedOut( opCode: U8 @@ -217,7 +217,7 @@ event DirectiveTimedOut( filePath: string ) \ severity warning high \ - format "A directive opcode {} at index {} timed out in sequence {}, causing the sequence to fail" + format "A directive opcode {} at index {} timed out in sequence '{}', causing the sequence to fail" event TooManySequenceArgs( count: U8 @@ -239,7 +239,7 @@ event ArgSizeMismatch( filePath: string ) \ severity warning high \ - format "Expected {} bytes of arguments, but received {} in sequence file {}" + format "Expected {} bytes of arguments, but received {} in sequence file '{}'" event ArgTotalSizeExceedsStackLimit( argSize: Fpy.StackSizeType @@ -269,46 +269,46 @@ event LogFatal( message: string ) \ severity fatal \ - format "Sequence {}: {}" + format "Sequence '{}': {}" event LogWarningHi( filePath: string message: string ) \ severity warning high \ - format "Sequence {}: {}" + format "Sequence '{}': {}" event LogWarningLo( filePath: string message: string ) \ severity warning low \ - format "Sequence {}: {}" + format "Sequence '{}': {}" event LogCommand( filePath: string message: string ) \ severity command \ - format "Sequence {}: {}" + format "Sequence '{}': {}" event LogActivityHi( filePath: string message: string ) \ severity activity high \ - format "Sequence {}: {}" + format "Sequence '{}': {}" event LogActivityLo( filePath: string message: string ) \ severity activity low \ - format "Sequence {}: {}" + format "Sequence '{}': {}" event LogDiagnostic( filePath: string message: string ) \ severity diagnostic \ - format "Sequence {}: {}" + format "Sequence '{}': {}" diff --git a/Svc/FpySequencer/FpySequencerParams.fppi b/Svc/FpySequencer/FpySequencerParams.fppi index af6fd92ca86..091aa4f300c 100644 --- a/Svc/FpySequencer/FpySequencerParams.fppi +++ b/Svc/FpySequencer/FpySequencerParams.fppi @@ -3,9 +3,3 @@ @ accuracy of this timeout is determined by the rate group driving this @ component. it will be rounded up param STATEMENT_TIMEOUT_SECS: F32 default 0 - -@ the base directory relative to which sequence file paths are resolved. -@ a '/' separator is always inserted between this base dir and the input -@ sequence file path before resolution occurs following the rules of -@ Os::File::open. -param SEQ_BASE_DIR: string size FileNameStringSize default Fpy.DEFAULT_SEQ_BASE_DIR \ No newline at end of file diff --git a/Svc/FpySequencer/FpySequencerRunState.cpp b/Svc/FpySequencer/FpySequencerRunState.cpp index 9eb275366da..2166e592697 100644 --- a/Svc/FpySequencer/FpySequencerRunState.cpp +++ b/Svc/FpySequencer/FpySequencerRunState.cpp @@ -472,7 +472,7 @@ Fw::Success FpySequencer::deserializeDirective(const Fpy::Statement& stmt, Direc default: { // unsure what this opcode is. check compiler version matches sequencer this->log_WARNING_HI_UnknownSequencerDirective(stmt.get_opCode(), this->currentStatementIdx(), - this->m_sequenceFilePath); + this->m_fullSequenceFilePath); return Fw::Success::FAILURE; } } @@ -729,10 +729,10 @@ Signal FpySequencer::checkStatementTimeout() { this->m_runtime.currentStatementOpcode == Fpy::DirectiveId::STACK_CMD) { // if we were executing a command, warn that the cmd timed out with its opcode this->log_WARNING_HI_CommandTimedOut(this->m_runtime.currentCmdOpcode, this->currentStatementIdx(), - this->m_sequenceFilePath); + this->m_fullSequenceFilePath); } else { this->log_WARNING_HI_DirectiveTimedOut(this->m_runtime.currentStatementOpcode, this->currentStatementIdx(), - this->m_sequenceFilePath); + this->m_fullSequenceFilePath); } return Signal::result_checkStatementTimeout_statementTimeout; diff --git a/Svc/FpySequencer/FpySequencerStateMachine.cpp b/Svc/FpySequencer/FpySequencerStateMachine.cpp index 24df09bec6b..96f30b9913d 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.cpp +++ b/Svc/FpySequencer/FpySequencerStateMachine.cpp @@ -16,66 +16,15 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_signalEntered( this->sequencer_sendSignal_entered(); } -//! Implementation for action setSequenceFilePath of state machine -//! Svc_FpySequencer_SequencerStateMachine +//! Implementation for action setSequenceExecArgs of state machine Svc_FpySequencer_SequencerStateMachine //! -//! sets the current sequence file path member var, resolving it against -//! the SEQ_BASE_DIR parameter so that subsequent telemetry, events, and -//! file IO see the fully qualified path -void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_setSequenceFilePath( +//! sets the sequence execution arguments (seq path, block state, seq args) +void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_setSequenceExecArgs( SmId smId, //!< The state machine id Svc_FpySequencer_SequencerStateMachine::Signal signal, //!< The signal const Svc::FpySequencer_SequenceExecutionArgs& value //!< The value ) { - Fw::ParamValid valid; - Fw::ParamString baseDir = this->paramGet_SEQ_BASE_DIR(valid); - if (baseDir.length() == 0) { - // the assignment here ensures the string is null terminated - // because it uses the string_copy method - // also, the filePath string in SequenceExecutionArgs is guaranteed - // to be truncated to FileNameStringSize chars, so it will not - // be truncated by this assignment - this->m_sequenceFilePath = value.get_filePath(); - return; - } - - Fw::FormatStatus status; - // the result will get truncated to FileNameStringSize - status = this->m_sequenceFilePath.format("%s/%s", baseDir.toChar(), value.get_filePath().toChar()); - if (status == Fw::FormatStatus::SUCCESS) { - return; - } - - // the only runtime-reachable non-success status is OVERFLOWED, which means the - // base dir and file name together are longer than the sequence file path buffer. - // the other statuses can only result from a bad format string literal, which is a - // coding error. let the user know the path was truncated; validate() will then fail - // to open the (truncated) path and report a FileOpenError. - FW_ASSERT(status == Fw::FormatStatus::OVERFLOWED, static_cast(status)); - this->log_WARNING_HI_SequenceFilePathTooLong(baseDir, value.get_filePath()); -} - -//! Implementation for action setSequenceBlockState of state machine -//! Svc_FpySequencer_SequencerStateMachine -//! -//! sets the block state of the sequence to be run -void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_setSequenceBlockState( - SmId smId, //!< The state machine id - Svc_FpySequencer_SequencerStateMachine::Signal signal, //!< The signal - const Svc::FpySequencer_SequenceExecutionArgs& value //!< The value -) { - this->m_sequenceBlockState = value.get_block(); -} - -//! Implementation for action setSequenceArguments of state machine -//! Svc_FpySequencer_SequencerStateMachine -//! -//! sets the arguments of the sequence to be run -void FpySequencer ::Svc_FpySequencer_SequencerStateMachine_action_setSequenceArguments( - SmId smId, - Svc_FpySequencer_SequencerStateMachine::Signal signal, - const Svc::FpySequencer_SequenceExecutionArgs& value) { - this->m_sequenceArgs = value.get_buffer(); + this->m_sequenceExecArgs = value; } //! Implementation for action report_seqSucceeded of state machine @@ -87,7 +36,7 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_report_seqSucce Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal ) { this->m_tlm.sequencesSucceeded++; - this->log_ACTIVITY_HI_SequenceDone(this->m_sequenceFilePath); + this->log_ACTIVITY_HI_SequenceDone(this->m_fullSequenceFilePath); if (this->isConnected_seqDoneOut_OutputPort(0)) { // report that the sequence succeeded to internal callers this->seqDoneOut_out(0, 0, 0, Fw::CmdResponse::OK); @@ -103,7 +52,11 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_report_seqCance Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal ) { this->m_tlm.sequencesCancelled++; - this->log_ACTIVITY_HI_SequenceCancelled(this->m_sequenceFilePath); + // m_fullSequenceFilePath is only initialized after validate() is called + // CANCEL can only go through after validate() is called, so this is + // okay. cuz the alternative would be we're in IDLE, and CANCEL + // gets rejected in IDLE + this->log_ACTIVITY_HI_SequenceCancelled(this->m_fullSequenceFilePath); if (this->isConnected_seqDoneOut_OutputPort(0)) { // report that the sequence failed to internal callers this->seqDoneOut_out(0, 0, 0, Fw::CmdResponse::EXECUTION_ERROR); @@ -179,7 +132,7 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_sendCmdResponse SmId smId, //!< The state machine id Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal ) { - if (this->m_sequenceBlockState == BlockState::BLOCK) { + if (this->m_sequenceExecArgs.get_blockState() == BlockState::BLOCK) { // respond if we were waiting on a response this->cmdResponse_out(this->m_savedOpCode, this->m_savedCmdSeq, Fw::CmdResponse::OK); } @@ -193,7 +146,7 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_sendCmdResponse SmId smId, //!< The state machine id Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal ) { - if (this->m_sequenceBlockState == BlockState::BLOCK) { + if (this->m_sequenceExecArgs.get_blockState() == BlockState::BLOCK) { // respond if we were waiting on a response this->cmdResponse_out(this->m_savedOpCode, this->m_savedCmdSeq, Fw::CmdResponse::EXECUTION_ERROR); } @@ -300,7 +253,7 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_pushArgsToStack SmId smId, //!< The state machine id Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal ) { - const Svc::SeqArgs& args = this->m_sequenceArgs; + const Svc::SeqArgs& args = this->m_sequenceExecArgs.get_runArgsBuf(); // Early return if no arguments provided if (args.get_size() == 0) { @@ -312,25 +265,6 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_pushArgsToStack this->m_runtime.stack.push(args.get_buffer(), static_cast(args.get_size())); } -//! Implementation for action clearSequenceFile of state machine Svc_FpySequencer_SequencerStateMachine -//! -//! clears all variables related to the loading/validating of the sequence file -void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_clearSequenceFile( - SmId smId, //!< The state machine id - Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal -) { - this->m_sequenceFilePath = ""; -} - -//! Implementation for action clearSequenceArguments of state machine Svc_FpySequencer_SequencerStateMachine -//! -//! clears all arguments of the sequence file -void FpySequencer ::Svc_FpySequencer_SequencerStateMachine_action_clearSequenceArguments( - SmId smId, - Svc_FpySequencer_SequencerStateMachine::Signal signal) { - this->m_sequenceArgs = {0, 0}; -} - //! Implementation for action clearBreakpoint of state machine Svc_FpySequencer_SequencerStateMachine //! //! clears the breakpoint, allowing execution of the sequence to continue @@ -344,6 +278,18 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_clearBreakpoint this->m_breakpoint.breakBeforeNextLine = false; } +//! Implementation for action clearSequenceExecArgs of state machine Svc_FpySequencer_SequencerStateMachine +//! +//! clears the sequence execution arguments (seq path, block state, seq args) +//! and the m_fullSequenceFilePath member var +void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_clearSequenceExecArgs( + SmId smId, //!< The state machine id + Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal +) { + new (&this->m_sequenceExecArgs) FpySequencer_SequenceExecutionArgs(); + new (&this->m_fullSequenceFilePath) Fw::FileNameString(); +} + //! Implementation for action report_seqBroken of state machine Svc_FpySequencer_SequencerStateMachine //! //! reports that a breakpoint was hit @@ -411,7 +357,8 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_report_seqStart if (this->isConnected_seqStartOut_OutputPort(0)) { // report that the sequence started to internal callers // NOTE: Sequence Arguments would be cleared if a VALIDATION command is sent, not a full RUN command. - this->seqStartOut_out(0, this->m_sequenceFilePath, this->m_sequenceArgs); + // TODO ^ what does this comment mean? + this->seqStartOut_out(0, this->m_fullSequenceFilePath, this->m_sequenceExecArgs.get_runArgsBuf()); } } // ---------------------------------------------------------------------- diff --git a/Svc/FpySequencer/FpySequencerStateMachine.fppi b/Svc/FpySequencer/FpySequencerStateMachine.fppi index a6661160c3f..821be6945a0 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.fppi +++ b/Svc/FpySequencer/FpySequencerStateMachine.fppi @@ -1,7 +1,11 @@ struct SequenceExecutionArgs { + @ the path of the sequence file to run filePath: string size FileNameStringSize - $block: Svc.BlockState - buffer: Svc.SeqArgs + @ whether or not the sequence we're about to run should return immediately or + @ block on completion + $blockState: Svc.BlockState + @ sequence arguments to push to stack when entering RUNNING state + runArgsBuf: Svc.SeqArgs } struct BreakpointArgs { @@ -50,12 +54,8 @@ state machine SequencerStateMachine { #################### @ simply raises the "entered" signal action signalEntered - @ sets the current sequence file path member var - action setSequenceFilePath: SequenceExecutionArgs - @ sets the block state of the sequence to be run - action setSequenceBlockState: SequenceExecutionArgs - @ sets the arguments to pass to the sequence - action setSequenceArguments: SequenceExecutionArgs + @ sets the sequence execution arguments (seq path, block state, seq args) + action setSequenceExecArgs: SequenceExecutionArgs @ performs all steps necessary for sequence validation, and raises a signal result_success or result_failure action validate @@ -83,29 +83,26 @@ state machine SequencerStateMachine { @ responds to the calling command with EXECUTION_ERROR action sendCmdResponse_EXECUTION_ERROR - @ clears all variables related to the loading/validating of the sequence file - action clearSequenceFile + @ clears the sequence execution arguments (seq path, block state, seq args) + @ and the m_fullSequenceFilePath member var + action clearSequenceExecArgs @ clears the breakpoint setting action clearBreakpoint - @ clears arguments - action clearSequenceArguments initial enter IDLE @ sequencer is ready to load, validate and run a sequence state IDLE { - # start with an unset goal state, and no debugging settings - entry do { clearBreakpoint, setGoalState_IDLE, clearSequenceFile, clearSequenceArguments} + # start with an unset goal state, no sequence execution args, and no debugging settings + entry do { clearBreakpoint, setGoalState_IDLE, clearSequenceExecArgs } # --> # wait for a cmd #################### # commands # #################### - # validate does not take as input a block state, only a path - on cmd_VALIDATE do { setGoalState_VALID, setSequenceFilePath, setSequenceArguments} enter VALIDATING - # run takes both path and block state - on cmd_RUN do { setGoalState_RUNNING, setSequenceFilePath, setSequenceBlockState, setSequenceArguments} enter VALIDATING + on cmd_VALIDATE do { setGoalState_VALID, setSequenceExecArgs } enter VALIDATING + on cmd_RUN do { setGoalState_RUNNING, setSequenceExecArgs } enter VALIDATING on cmd_SET_BREAKPOINT do { setBreakpoint } on cmd_CLEAR_BREAKPOINT do { clearBreakpoint } @@ -152,8 +149,7 @@ state machine SequencerStateMachine { #################### @ cancelled the sequence after we validated it on cmd_CANCEL do { report_seqCancelled } enter IDLE - @ the sequence path has already been decided on, so only set the sequenceShouldBlock var - on cmd_RUN_VALIDATED do { setSequenceBlockState } enter RUNNING + on cmd_RUN_VALIDATED do { setSequenceExecArgs } enter RUNNING on cmd_SET_BREAKPOINT do { setBreakpoint } on cmd_CLEAR_BREAKPOINT do { clearBreakpoint } diff --git a/Svc/FpySequencer/FpySequencerTelemetry.fppi b/Svc/FpySequencer/FpySequencerTelemetry.fppi index d333f6945a3..f1d0110f4c8 100644 --- a/Svc/FpySequencer/FpySequencerTelemetry.fppi +++ b/Svc/FpySequencer/FpySequencerTelemetry.fppi @@ -60,6 +60,3 @@ telemetry BreakBeforeNextLine: bool update on change @ value of prm STATEMENT_TIMEOUT_SECS telemetry PRM_STATEMENT_TIMEOUT_SECS: F32 update on change - -@ value of prm SEQ_BASE_DIR -telemetry PRM_SEQ_BASE_DIR: string size FileNameStringSize update on change diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index 866d021de35..3e2100d5f76 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -26,17 +26,39 @@ void FpySequencer::deallocateBuffer(Fw::MemAllocator& allocator) { // loads the sequence in memory, and does header/crc/integrity checks. // return SUCCESS if sequence is valid, FAILURE otherwise Fw::Success FpySequencer::validate() { - FW_ASSERT(this->m_sequenceFilePath.length() > 0); - // crc needs to be initialized with a particular value // for the calculation to work this->m_computedCRC.init(); + // concat the seq base dir prefix to the seq file path + Fw::ParamString baseDir = this->paramGet_SEQ_BASE_DIR(valid); + if (baseDir.length() > 0) { + // the result will get truncated to FileNameStringSize + Fw::FormatStatus status = this->m_fullSequenceFilePath.format("%s/%s", baseDir.toChar(), + this->m_sequenceExecArgs.get_filePath().toChar()); + + if (status != Fw::FormatStatus::SUCCESS) { + // the only runtime-reachable non-success status is OVERFLOWED, which means the + // base dir and file name together are longer than the sequence file path buffer. + // the other statuses can only result from a bad format string literal, which is a + // coding error. + FW_ASSERT(status == Fw::FormatStatus::OVERFLOWED, static_cast(status)); + this->log_WARNING_HI_SequenceFilePathTooLong(baseDir, this->m_sequenceExecArgs.get_filePath()); + return Fw::Success::FAILURE; + } + } else { + // the assignment here ensures the string is null terminated + // because it uses the string_copy method + // also, the filePath string in SequenceExecutionArgs is guaranteed + // to be truncated to FileNameStringSize chars, so it will not + // be truncated by this assignment + this->m_fullSequenceFilePath = this->m_sequenceExecArgs.get_filePath(); + } Os::File sequenceFile; - Os::File::Status openStatus = sequenceFile.open(this->m_sequenceFilePath.toChar(), Os::File::OPEN_READ); + Os::File::Status openStatus = sequenceFile.open(this->m_fullSequenceFilePath.toChar(), Os::File::OPEN_READ); if (openStatus != Os::File::Status::OP_OK) { - this->log_WARNING_HI_FileOpenError(this->m_sequenceFilePath, static_cast(openStatus)); + this->log_WARNING_HI_FileOpenError(this->m_fullSequenceFilePath, static_cast(openStatus)); return Fw::Success::FAILURE; } @@ -87,14 +109,14 @@ Fw::Success FpySequencer::validate() { FwSizeType sequenceFileSize; Os::File::Status sizeStatus = sequenceFile.size(sequenceFileSize); if (sizeStatus != Os::File::Status::OP_OK) { - this->log_WARNING_HI_FileApiError(this->m_sequenceFilePath, static_cast(sizeStatus)); + this->log_WARNING_HI_FileApiError(this->m_fullSequenceFilePath, static_cast(sizeStatus)); return Fw::Success::FAILURE; } FwSizeType sequenceFilePosition; Os::File::Status positionStatus = sequenceFile.position(sequenceFilePosition); if (positionStatus != Os::File::Status::OP_OK) { - this->log_WARNING_HI_FileApiError(this->m_sequenceFilePath, static_cast(positionStatus)); + this->log_WARNING_HI_FileApiError(this->m_fullSequenceFilePath, static_cast(positionStatus)); return Fw::Success::FAILURE; } @@ -105,7 +127,7 @@ Fw::Success FpySequencer::validate() { Fpy::StackSizeType availableSpace = Fpy::MAX_STACK_SIZE - this->m_runtime.stack.size; - if (this->m_sequenceArgs.get_size() > availableSpace) { + if (this->m_sequenceExecArgs.get_runArgsBuf().get_size() > availableSpace) { return Fw::Success::FAILURE; } @@ -119,7 +141,7 @@ Fw::Success FpySequencer::readHeader() { Fw::SerializeStatus deserStatus = this->m_sequenceBuffer.deserializeTo(this->m_sequenceObj.get_header()); if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::HEADER, this->m_sequenceFilePath, static_cast(deserStatus), + FpySequencer_FileReadStage::HEADER, this->m_fullSequenceFilePath, static_cast(deserStatus), this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } @@ -160,7 +182,7 @@ Fw::Success FpySequencer::readBody() { deserStatus = this->m_sequenceBuffer.deserializeTo(argSpec); if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + FpySequencer_FileReadStage::BODY, this->m_fullSequenceFilePath, static_cast(deserStatus), this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } @@ -175,9 +197,10 @@ Fw::Success FpySequencer::readBody() { } // Validate total argument size - if (this->m_totalExpectedArgSize != this->m_sequenceArgs.get_size()) { - this->log_WARNING_HI_ArgSizeMismatch(this->m_totalExpectedArgSize, this->m_sequenceArgs.get_size(), - this->m_sequenceFilePath); + if (this->m_totalExpectedArgSize != this->m_sequenceExecArgs.get_runArgsBuf().get_size()) { + this->log_WARNING_HI_ArgSizeMismatch(this->m_totalExpectedArgSize, + this->m_sequenceExecArgs.get_runArgsBuf().get_size(), + this->m_fullSequenceFilePath); return Fw::Success::FAILURE; } @@ -187,7 +210,7 @@ Fw::Success FpySequencer::readBody() { deserStatus = this->m_sequenceBuffer.deserializeTo(this->m_sequenceObj.get_statements()[statementIdx]); if (deserStatus != Fw::FW_SERIALIZE_OK) { this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + FpySequencer_FileReadStage::BODY, this->m_fullSequenceFilePath, static_cast(deserStatus), this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } @@ -201,7 +224,7 @@ Fw::Success FpySequencer::readFooter() { Fw::SerializeStatus deserStatus = this->m_sequenceBuffer.deserializeTo(this->m_sequenceObj.get_footer()); if (deserStatus != Fw::FW_SERIALIZE_OK) { this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::FOOTER, this->m_sequenceFilePath, static_cast(deserStatus), + FpySequencer_FileReadStage::FOOTER, this->m_fullSequenceFilePath, static_cast(deserStatus), this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } @@ -233,19 +256,19 @@ Fw::Success FpySequencer::readBytes(Os::File& file, // if this fails, then you need to give the sequencer more buffer memory. pass in a bigger number // to fpySeq.allocateBuffer(). This is usually done in topology setup CPP if (expectedReadLen > capacity) { - this->log_WARNING_HI_InsufficientBufferSpace(static_cast(capacity), this->m_sequenceFilePath); + this->log_WARNING_HI_InsufficientBufferSpace(static_cast(capacity), this->m_fullSequenceFilePath); return Fw::Success::FAILURE; } Os::File::Status fileStatus = file.read(this->m_sequenceBuffer.getBuffAddr(), actualReadLen); if (fileStatus != Os::File::OP_OK) { - this->log_WARNING_HI_FileReadError(readStage, this->m_sequenceFilePath, static_cast(fileStatus)); + this->log_WARNING_HI_FileReadError(readStage, this->m_fullSequenceFilePath, static_cast(fileStatus)); return Fw::Success::FAILURE; } if (actualReadLen < expectedReadLen) { - this->log_WARNING_HI_EndOfFileError(readStage, this->m_sequenceFilePath); + this->log_WARNING_HI_EndOfFileError(readStage, this->m_fullSequenceFilePath); return Fw::Success::FAILURE; } diff --git a/Svc/FpySequencer/docs/sdd.md b/Svc/FpySequencer/docs/sdd.md index 229b2d97b0c..e99bbe83b46 100644 --- a/Svc/FpySequencer/docs/sdd.md +++ b/Svc/FpySequencer/docs/sdd.md @@ -49,7 +49,7 @@ The following diagram represents the states of the `FpySequencer`. stateDiagram-v2 direction LR state "IDLE -entry / clearBreakpoint, clearSequenceFile +entry / clearBreakpoint, clearSequenceExecArgs ------------------------------------------------------ cmd_SET_BREAKPOINT / setBreakpoint cmd_CLEAR_BREAKPOINT / clearBreakpoint @@ -108,8 +108,8 @@ entry / clearBreakpoint, clearSequenceFile } - IDLE --> VALIDATING: cmd_VALIDATE/setSequenceFilePath - IDLE --> VALIDATING: cmd_RUN/setSequenceFilePath + IDLE --> VALIDATING: cmd_VALIDATE/setSequenceExecArgs + IDLE --> VALIDATING: cmd_RUN/setSequenceExecArgs VALID --> RUNNING: if cmd_RUN VALID --> AWAITING_CMD_RUN_VALIDATED: if cmd_VALIDATE diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index 919e6175e45..af256f5301c 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -1277,7 +1277,7 @@ TEST_F(FpySequencerTester, exit) { ASSERT_EQ(result, Signal::stmtResponse_failure); ASSERT_EQ(err, DirectiveError::EXIT_WITH_ERROR); ASSERT_EVENTS_SequenceExitedWithError_SIZE(1); - ASSERT_EVENTS_SequenceExitedWithError(0, tester_get_m_sequenceFilePath().toChar(), 123); + ASSERT_EVENTS_SequenceExitedWithError(0, tester_get_m_sequenceExecArgs_ptr()->get_filePath().toChar(), 123); } TEST_F(FpySequencerTester, discard) { @@ -2793,7 +2793,7 @@ TEST_F(FpySequencerTester, readBody) { tester_get_m_sequenceBuffer_ptr()->resetSer(); Svc::SeqArgs maxArgs{0, 0}; maxArgs.set_size(Fpy::MAX_SEQUENCE_ARG_COUNT * sizeof(U32)); - tester_set_m_sequenceArgs(maxArgs); + tester_get_m_sequenceExecArgs_ptr()->set_runArgsBuf(maxArgs); for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_ARG_COUNT; ii++) { Fw::String argName; argName.format("arg%u", ii); @@ -2832,7 +2832,7 @@ TEST_F(FpySequencerTester, readBody) { // write some statements Svc::SeqArgs noArgs{0, 0}; - tester_set_m_sequenceArgs(noArgs); + tester_get_m_sequenceExecArgs_ptr()->set_runArgsBuf(noArgs); tester_get_m_sequenceObj_ptr()->get_header().set_argumentCount(0); Fpy::Statement stmt(Fpy::DirectiveId::NO_OP, Fw::StatementArgBuffer()); for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_STATEMENT_COUNT; ii++) { @@ -2926,7 +2926,7 @@ TEST_F(FpySequencerTester, validate) { writeToFile("test.bin"); U8 data[Fpy::Sequence::SERIALIZED_SIZE] = {0}; tester_get_m_sequenceBuffer_ptr()->setExtBuffer(data, sizeof(data)); - tester_set_m_sequenceFilePath("test.bin"); + tester_get_m_sequenceExecArgs_ptr()->set_filePath(Fw::String("test.bin")); ASSERT_EQ(tester_validate(), Fw::Success::SUCCESS); // cause validation failure to open @@ -2984,7 +2984,7 @@ TEST_F(FpySequencerTester, validate) { TEST_F(FpySequencerTester, seqBaseDir_resolvesPath) { // a relative base dir should be prepended to the file name and the resolved - // file should actually open and validate — m_sequenceFilePath (used by + // file should actually open and validate — m_fullSequenceFilePath (used by // tlm/events/file IO) should hold the fully resolved path allocMem(); add_NO_OP(); @@ -2999,7 +2999,7 @@ TEST_F(FpySequencerTester, seqBaseDir_resolvesPath) { dispatchUntilState(State::VALIDATING); dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); - ASSERT_EQ(tester_get_m_sequenceFilePath(), Fw::String("seq_dir/test.bin")); + ASSERT_EQ(tester_get_m_fullSequenceFilePath(), Fw::String("seq_dir/test.bin")); ASSERT_CMD_RESPONSE_SIZE(1); ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE(), 0, Fw::CmdResponse::OK); @@ -3026,7 +3026,7 @@ TEST_F(FpySequencerTester, seqBaseDir_resolvesAbsolutePath) { dispatchUntilState(State::VALIDATING); dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); - ASSERT_EQ(tester_get_m_sequenceFilePath(), Fw::String(absFilePath)); + ASSERT_EQ(tester_get_m_sequenceExecArgs_ptr()->get_filePath(), Fw::String(absFilePath)); ASSERT_CMD_RESPONSE_SIZE(1); ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE(), 0, Fw::CmdResponse::OK); @@ -3035,7 +3035,7 @@ TEST_F(FpySequencerTester, seqBaseDir_resolvesAbsolutePath) { } TEST_F(FpySequencerTester, seqBaseDir_emptyKeepsRawPath) { - // empty base dir — m_sequenceFilePath should be the raw user-provided path + // empty base dir — m_sequenceExecArgs.get_filePath() should be the raw user-provided path allocMem(); add_NO_OP(); writeToFile("test.bin"); @@ -3048,7 +3048,7 @@ TEST_F(FpySequencerTester, seqBaseDir_emptyKeepsRawPath) { dispatchUntilState(State::VALIDATING); dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); - ASSERT_EQ(tester_get_m_sequenceFilePath(), Fw::String("test.bin")); + ASSERT_EQ(tester_get_m_sequenceExecArgs_ptr()->get_filePath(), Fw::String("test.bin")); removeFile("test.bin"); } @@ -3091,12 +3091,13 @@ TEST_F(FpySequencerTester, seqBaseDir_pathTooLongTruncates) { std::string longFileName(fileNameLen, 'b'); FpySequencer_SequenceExecutionArgs args; args.set_filePath(Fw::String(longFileName.c_str())); - tester_setSequenceFilePath(args); + tester_setSequenceExecArgs(args); ASSERT_EVENTS_SequenceFilePathTooLong_SIZE(1); ASSERT_EQ(this->eventHistory_SequenceFilePathTooLong->at(0).baseDir, Fw::LogStringArg(longBaseDir.c_str())); // the stored path was truncated to the maximum length that fits the buffer - ASSERT_EQ(tester_get_m_sequenceFilePath().length(), static_cast(FileNameStringSize)); + ASSERT_EQ(tester_get_m_sequenceExecArgs_ptr()->get_filePath().length(), + static_cast(FileNameStringSize)); } TEST_F(FpySequencerTester, cmd_DUMP_STACK_TO_FILE_openErrorLogsDumpFileName) { @@ -3115,7 +3116,7 @@ TEST_F(FpySequencerTester, cmd_DUMP_STACK_TO_FILE_openErrorLogsDumpFileName) { // make the loaded sequence path distinct from the dump path so that, if the // event were to (incorrectly) log m_sequenceFilePath, this test would fail - tester_set_m_sequenceFilePath("loaded_sequence.bin"); + tester_get_m_sequenceExecArgs_ptr()->set_filePath(Fw::String("loaded_sequence.bin")); this->clearHistory(); // dump into a directory that doesn't exist so the open fails @@ -4790,7 +4791,7 @@ TEST_F(FpySequencerTester, popEvent) { ASSERT_EQ(result, Signal::stmtResponse_success); ASSERT_EQ(err, DirectiveError::NO_ERROR); ASSERT_EVENTS_LogFatal_SIZE(1); - ASSERT_EVENTS_LogFatal(0, tester_get_m_sequenceFilePath().toChar(), testMsg); + ASSERT_EVENTS_LogFatal(0, tester_get_m_sequenceExecArgs_ptr()->get_filePath().toChar(), testMsg); clearEvents(); } @@ -4806,7 +4807,7 @@ TEST_F(FpySequencerTester, popEvent) { ASSERT_EQ(result, Signal::stmtResponse_success); ASSERT_EQ(err, DirectiveError::NO_ERROR); ASSERT_EVENTS_LogWarningHi_SIZE(1); - ASSERT_EVENTS_LogWarningHi(0, tester_get_m_sequenceFilePath().toChar(), testMsg); + ASSERT_EVENTS_LogWarningHi(0, tester_get_m_sequenceExecArgs_ptr()->get_filePath().toChar(), testMsg); clearEvents(); } @@ -4822,7 +4823,7 @@ TEST_F(FpySequencerTester, popEvent) { ASSERT_EQ(result, Signal::stmtResponse_success); ASSERT_EQ(err, DirectiveError::NO_ERROR); ASSERT_EVENTS_LogWarningLo_SIZE(1); - ASSERT_EVENTS_LogWarningLo(0, tester_get_m_sequenceFilePath().toChar(), testMsg); + ASSERT_EVENTS_LogWarningLo(0, tester_get_m_sequenceExecArgs_ptr()->get_filePath().toChar(), testMsg); clearEvents(); } @@ -4838,7 +4839,7 @@ TEST_F(FpySequencerTester, popEvent) { ASSERT_EQ(result, Signal::stmtResponse_success); ASSERT_EQ(err, DirectiveError::NO_ERROR); ASSERT_EVENTS_LogCommand_SIZE(1); - ASSERT_EVENTS_LogCommand(0, tester_get_m_sequenceFilePath().toChar(), testMsg); + ASSERT_EVENTS_LogCommand(0, tester_get_m_sequenceExecArgs_ptr()->get_filePath().toChar(), testMsg); clearEvents(); } @@ -4854,7 +4855,7 @@ TEST_F(FpySequencerTester, popEvent) { ASSERT_EQ(result, Signal::stmtResponse_success); ASSERT_EQ(err, DirectiveError::NO_ERROR); ASSERT_EVENTS_LogActivityHi_SIZE(1); - ASSERT_EVENTS_LogActivityHi(0, tester_get_m_sequenceFilePath().toChar(), testMsg); + ASSERT_EVENTS_LogActivityHi(0, tester_get_m_sequenceExecArgs_ptr()->get_filePath().toChar(), testMsg); clearEvents(); } @@ -4870,7 +4871,7 @@ TEST_F(FpySequencerTester, popEvent) { ASSERT_EQ(result, Signal::stmtResponse_success); ASSERT_EQ(err, DirectiveError::NO_ERROR); ASSERT_EVENTS_LogActivityLo_SIZE(1); - ASSERT_EVENTS_LogActivityLo(0, tester_get_m_sequenceFilePath().toChar(), testMsg); + ASSERT_EVENTS_LogActivityLo(0, tester_get_m_sequenceExecArgs_ptr()->get_filePath().toChar(), testMsg); clearEvents(); } @@ -4886,7 +4887,7 @@ TEST_F(FpySequencerTester, popEvent) { ASSERT_EQ(result, Signal::stmtResponse_success); ASSERT_EQ(err, DirectiveError::NO_ERROR); ASSERT_EVENTS_LogDiagnostic_SIZE(1); - ASSERT_EVENTS_LogDiagnostic(0, tester_get_m_sequenceFilePath().toChar(), testMsg); + ASSERT_EVENTS_LogDiagnostic(0, tester_get_m_sequenceExecArgs_ptr()->get_filePath().toChar(), testMsg); clearEvents(); } @@ -4935,7 +4936,7 @@ TEST_F(FpySequencerTester, popEvent) { ASSERT_EQ(result, Signal::stmtResponse_success); ASSERT_EQ(err, DirectiveError::NO_ERROR); ASSERT_EVENTS_LogActivityHi_SIZE(1); - ASSERT_EVENTS_LogActivityHi(0, tester_get_m_sequenceFilePath().toChar(), ""); + ASSERT_EVENTS_LogActivityHi(0, tester_get_m_sequenceExecArgs_ptr()->get_filePath().toChar(), ""); clearEvents(); } } diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestSequences.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestSequences.cpp index 11658d65a96..2f2581009d0 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestSequences.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestSequences.cpp @@ -130,4 +130,29 @@ TEST_F(FpySequencerTester, NotTrueSeq) { ASSERT_EQ(tester_get_m_tlm_ptr()->lastDirectiveError, DirectiveError::NO_ERROR); } +TEST_F(FpySequencerTester, ValidateAfterNoBlockRun) { + allocMem(); + add_NO_OP(); + writeToFile("test.bin"); + + sendCmd_RUN(0, 0, Fw::String("test.bin"), BlockState::NO_BLOCK); + dispatchUntilState(State::VALIDATING); + dispatchUntilState(State::IDLE); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_from_seqDoneOut_SIZE(1); + ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::OK); + + // okay, ran a sequence with no block. running validate after it should + // return a response + + this->clearHistory(); + + sendCmd_VALIDATE(0, 0, Fw::String("test.bin")); + dispatchUntilState(State::VALIDATING); + dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE(), 0, Fw::CmdResponse::OK); + this->clearHistory(); +} + } // namespace Svc diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.cpp b/Svc/FpySequencer/test/ut/FpySequencerTester.cpp index 96b41403c19..9badc1f41d8 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.cpp @@ -658,17 +658,17 @@ Fw::Success FpySequencerTester::tester_readHeader() { } // Get & Set cmp variables -Fw::String FpySequencerTester::tester_get_m_sequenceFilePath() { - return this->cmp.m_sequenceFilePath; +FpySequencer_SequenceExecutionArgs* FpySequencerTester::tester_get_m_sequenceExecArgs_ptr() { + return &this->cmp.m_sequenceExecArgs; } -void FpySequencerTester::tester_set_m_sequenceFilePath(Fw::String str) { - this->cmp.m_sequenceFilePath = str; +Fw::String FpySequencerTester::tester_get_m_fullSequenceFilePath() { + return this->cmp.m_fullSequenceFilePath; } -void FpySequencerTester::tester_setSequenceFilePath(const Svc::FpySequencer_SequenceExecutionArgs& args) { +void FpySequencerTester::tester_setSequenceExecArgs(const Svc::FpySequencer_SequenceExecutionArgs& args) { // the action ignores smId/signal; pass any valid values - this->cmp.Svc_FpySequencer_SequencerStateMachine_action_setSequenceFilePath( + this->cmp.Svc_FpySequencer_SequencerStateMachine_action_setSequenceExecArgs( FpySequencer::SmId::sequencer, Svc::FpySequencer_SequencerStateMachineStateMachineBase::Signal::cmd_VALIDATE, args); } @@ -707,10 +707,6 @@ U32 FpySequencerTester::tester_finalize_m_computedCRC() { return finalCrc; } -void FpySequencerTester::tester_set_m_sequenceArgs(Svc::SeqArgs args) { - this->cmp.m_sequenceArgs = args; -} - // Get cmp member pointers FpySequencer::Runtime* FpySequencerTester::tester_get_m_runtime_ptr() { return &(this->cmp.m_runtime); diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp index eeb0c29cfc8..87c26340e0a 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp @@ -251,11 +251,11 @@ class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test Fpy::Sequence* tester_get_m_sequenceObj_ptr(); Svc::Signal tester_dispatchStatement(); Fw::Success tester_validate(); - Fw::String tester_get_m_sequenceFilePath(); - void tester_set_m_sequenceFilePath(Fw::String str); - // directly invokes the setSequenceFilePath state machine action, bypassing the + FpySequencer_SequenceExecutionArgs* tester_get_m_sequenceExecArgs_ptr(); + Fw::String tester_get_m_fullSequenceFilePath(); + // directly invokes the setSequenceExecArgs state machine action, bypassing the // command path (which truncates string args to FW_CMD_STRING_MAX_SIZE) - void tester_setSequenceFilePath(const Svc::FpySequencer_SequenceExecutionArgs& args); + void tester_setSequenceExecArgs(const Svc::FpySequencer_SequenceExecutionArgs& args); Fw::Success tester_readBytes(Os::File& file, FwSizeType readLen, FpySequencer_FileReadStage readStage, @@ -267,7 +267,6 @@ class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test void tester_init_m_computedCRC(); void tester_update_m_computedCRC(const U8* buffer, FwSizeType bufferSize); U32 tester_finalize_m_computedCRC(); - void tester_set_m_sequenceArgs(Svc::SeqArgs args); Svc::FpySequencer::BreakpointInfo* tester_get_m_breakpoint_ptr(); Svc::Signal tester_checkStatementTimeout(); Svc::Signal tester_checkShouldWake(); diff --git a/default/config/FpySequencerCfg.fpp b/default/config/FpySequencerCfg.fpp index abdedafe3a1..38028f479a4 100644 --- a/default/config/FpySequencerCfg.fpp +++ b/default/config/FpySequencerCfg.fpp @@ -8,9 +8,10 @@ module Svc { dictionary constant MAX_STACK_SIZE = 65535 @ the maximum number of bytes in a directive dictionary constant MAX_DIRECTIVE_SIZE = 2048 - @ the default value of the SEQ_BASE_DIR parameter. suffixed to - @ the input sequence file path before resolution occurs following - @ the rules of Os::File::open. trailing slash optional - dictionary constant DEFAULT_SEQ_BASE_DIR = "" + @ the base directory relative to which sequence file paths are resolved. + @ prefixed to the input sequence file path before resolution occurs following + @ the rules of Os::File::open. trailing slash optional + @ a '/' separator is always inserted between this base dir and the input seq path + dictionary constant SEQ_BASE_DIR = "" } } \ No newline at end of file From 26c05440c7b70f8beb99a812d33fba6208cc98e9 Mon Sep 17 00:00:00 2001 From: Zimri Leisher Date: Fri, 19 Jun 2026 23:20:43 -0500 Subject: [PATCH 2/4] Get tests passing --- Svc/FpySequencer/FpySequencer.hpp | 6 +++ Svc/FpySequencer/FpySequencerStateMachine.cpp | 9 ++-- .../FpySequencerValidationState.cpp | 14 +++-- .../test/ut/FpySequencerTestMain.cpp | 52 +++++++------------ .../test/ut/FpySequencerTester.hpp | 13 ++++- 5 files changed, 51 insertions(+), 43 deletions(-) diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index 42a79993e47..4a4c5563434 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -739,6 +739,12 @@ class FpySequencer : public FpySequencerComponentBase { // Validation state // ---------------------------------------------------------------------- + // returns the base directory against which sequence file paths are resolved + // in validate(). defaults to the SEQ_BASE_DIR config constant. declared + // virtual so that unit tests (and specialized deployments) can swap out the + // base dir, since SEQ_BASE_DIR is otherwise fixed at compile time + virtual const char* getSequenceBaseDir() const; + // loads the sequence in memory, and does header/crc/integrity checks. // return SUCCESS if sequence is valid, FAILURE otherwise Fw::Success validate(); diff --git a/Svc/FpySequencer/FpySequencerStateMachine.cpp b/Svc/FpySequencer/FpySequencerStateMachine.cpp index 96f30b9913d..31e8c73d354 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.cpp +++ b/Svc/FpySequencer/FpySequencerStateMachine.cpp @@ -355,10 +355,11 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_report_seqStart Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal ) { if (this->isConnected_seqStartOut_OutputPort(0)) { - // report that the sequence started to internal callers - // NOTE: Sequence Arguments would be cleared if a VALIDATION command is sent, not a full RUN command. - // TODO ^ what does this comment mean? - this->seqStartOut_out(0, this->m_fullSequenceFilePath, this->m_sequenceExecArgs.get_runArgsBuf()); + // report that the sequence started to internal callers. + // we report the raw operator-supplied path rather than m_fullSequenceFilePath + // because this action runs in the VALIDATING entry, before validate() resolves + // the base dir prefix into m_fullSequenceFilePath + this->seqStartOut_out(0, this->m_sequenceExecArgs.get_filePath(), this->m_sequenceExecArgs.get_runArgsBuf()); } } // ---------------------------------------------------------------------- diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index 3e2100d5f76..e6637862056 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -23,6 +23,12 @@ void FpySequencer::deallocateBuffer(Fw::MemAllocator& allocator) { this->m_sequenceBuffer.clear(); } +// returns the base directory against which sequence file paths are resolved. +// defaults to the SEQ_BASE_DIR config constant; overridable in tests/deployments +const char* FpySequencer::getSequenceBaseDir() const { + return Fpy::SEQ_BASE_DIR; +} + // loads the sequence in memory, and does header/crc/integrity checks. // return SUCCESS if sequence is valid, FAILURE otherwise Fw::Success FpySequencer::validate() { @@ -31,10 +37,10 @@ Fw::Success FpySequencer::validate() { this->m_computedCRC.init(); // concat the seq base dir prefix to the seq file path - Fw::ParamString baseDir = this->paramGet_SEQ_BASE_DIR(valid); - if (baseDir.length() > 0) { + const char* baseDir = this->getSequenceBaseDir(); + if (baseDir != nullptr && baseDir[0] != '\0') { // the result will get truncated to FileNameStringSize - Fw::FormatStatus status = this->m_fullSequenceFilePath.format("%s/%s", baseDir.toChar(), + Fw::FormatStatus status = this->m_fullSequenceFilePath.format("%s/%s", baseDir, this->m_sequenceExecArgs.get_filePath().toChar()); if (status != Fw::FormatStatus::SUCCESS) { @@ -43,7 +49,7 @@ Fw::Success FpySequencer::validate() { // the other statuses can only result from a bad format string literal, which is a // coding error. FW_ASSERT(status == Fw::FormatStatus::OVERFLOWED, static_cast(status)); - this->log_WARNING_HI_SequenceFilePathTooLong(baseDir, this->m_sequenceExecArgs.get_filePath()); + this->log_WARNING_HI_SequenceFilePathTooLong(Fw::String(baseDir), this->m_sequenceExecArgs.get_filePath()); return Fw::Success::FAILURE; } } else { diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index af256f5301c..7862f03065c 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -2991,8 +2991,7 @@ TEST_F(FpySequencerTester, seqBaseDir_resolvesPath) { ASSERT_EQ(Os::FileSystem::createDirectory("seq_dir"), Os::FileSystem::Status::OP_OK); writeToFile("seq_dir/test.bin"); - paramSet_SEQ_BASE_DIR(Fw::ParamString("seq_dir"), Fw::ParamValid::VALID); - paramSend_SEQ_BASE_DIR(0, 0); + cmp.m_baseDirOverride = Fw::String("seq_dir"); this->clearHistory(); sendCmd_VALIDATE(0, 0, Fw::String("test.bin")); @@ -3009,8 +3008,7 @@ TEST_F(FpySequencerTester, seqBaseDir_resolvesPath) { TEST_F(FpySequencerTester, seqBaseDir_resolvesAbsolutePath) { // an absolute base dir should be prepended just the same, and the resolved - // absolute path should open and validate. (the base dir must fit in the - // SEQ_BASE_DIR param, FW_PARAM_STRING_MAX_SIZE, so use a short /tmp path) + // absolute path should open and validate. allocMem(); add_NO_OP(); const char* absBaseDir = "/tmp/fpy_seq_abs"; @@ -3018,15 +3016,14 @@ TEST_F(FpySequencerTester, seqBaseDir_resolvesAbsolutePath) { ASSERT_EQ(Os::FileSystem::createDirectory(absBaseDir), Os::FileSystem::Status::OP_OK); writeToFile(absFilePath); - paramSet_SEQ_BASE_DIR(Fw::ParamString(absBaseDir), Fw::ParamValid::VALID); - paramSend_SEQ_BASE_DIR(0, 0); + cmp.m_baseDirOverride = Fw::String(absBaseDir); this->clearHistory(); sendCmd_VALIDATE(0, 0, Fw::String("test.bin")); dispatchUntilState(State::VALIDATING); dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); - ASSERT_EQ(tester_get_m_sequenceExecArgs_ptr()->get_filePath(), Fw::String(absFilePath)); + ASSERT_EQ(tester_get_m_fullSequenceFilePath(), Fw::String(absFilePath)); ASSERT_CMD_RESPONSE_SIZE(1); ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE(), 0, Fw::CmdResponse::OK); @@ -3040,15 +3037,15 @@ TEST_F(FpySequencerTester, seqBaseDir_emptyKeepsRawPath) { add_NO_OP(); writeToFile("test.bin"); - paramSet_SEQ_BASE_DIR(Fw::ParamString(""), Fw::ParamValid::VALID); - paramSend_SEQ_BASE_DIR(0, 0); + cmp.m_baseDirOverride = Fw::String(""); this->clearHistory(); sendCmd_VALIDATE(0, 0, Fw::String("test.bin")); dispatchUntilState(State::VALIDATING); dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); - ASSERT_EQ(tester_get_m_sequenceExecArgs_ptr()->get_filePath(), Fw::String("test.bin")); + // with an empty base dir, the resolved path is just the raw user path + ASSERT_EQ(tester_get_m_fullSequenceFilePath(), Fw::String("test.bin")); removeFile("test.bin"); } @@ -3057,8 +3054,7 @@ TEST_F(FpySequencerTester, seqBaseDir_fileOpenLogsResolvedPath) { // a base dir that doesn't exist makes file open fail. the FileOpenError // event should report the fully resolved path, not the user-supplied one allocMem(); - paramSet_SEQ_BASE_DIR(Fw::ParamString("nonexistent_dir"), Fw::ParamValid::VALID); - paramSend_SEQ_BASE_DIR(0, 0); + cmp.m_baseDirOverride = Fw::String("nonexistent_dir"); this->clearHistory(); sendCmd_VALIDATE(0, 0, Fw::String("test.bin")); @@ -3069,35 +3065,33 @@ TEST_F(FpySequencerTester, seqBaseDir_fileOpenLogsResolvedPath) { ASSERT_EQ(this->eventHistory_FileOpenError->at(0).filePath, Fw::LogStringArg("nonexistent_dir/test.bin")); } -TEST_F(FpySequencerTester, seqBaseDir_pathTooLongTruncates) { +TEST_F(FpySequencerTester, seqBaseDir_pathTooLongFailsValidation) { // if SEQ_BASE_DIR plus the user-supplied file name together exceed the - // sequence file path buffer (FileNameStringSize), the resolved path gets - // truncated. the sequencer should log SequenceFilePathTooLong so the operator - // knows why, rather than silently acting on a wrong (truncated) path. + // sequence file path buffer (FileNameStringSize), resolving the full path + // would overflow. validate() must fail and log SequenceFilePathTooLong so the + // operator knows why, rather than silently acting on a wrong (truncated) path. // size the base dir and file name so that "/" is exactly one // character too long for the sequence file path buffer (max strlen - // FileNameStringSize), forcing truncation. the separator accounts for the +1: + // FileNameStringSize), forcing the overflow. the separator accounts for the +1: // baseDirLen + 1 (separator) + fileNameLen == FileNameStringSize + 1 const FwSizeType baseDirLen = 8; const FwSizeType fileNameLen = FileNameStringSize - baseDirLen; std::string longBaseDir(baseDirLen, 'a'); - paramSet_SEQ_BASE_DIR(Fw::ParamString(longBaseDir.c_str()), Fw::ParamValid::VALID); - paramSend_SEQ_BASE_DIR(0, 0); - this->clearHistory(); + cmp.m_baseDirOverride = Fw::String(longBaseDir.c_str()); - // call the action directly because the command path would truncate the file + // set the exec args directly because the command path would truncate the file // name to FW_CMD_STRING_MAX_SIZE before it could ever overflow. std::string longFileName(fileNameLen, 'b'); FpySequencer_SequenceExecutionArgs args; args.set_filePath(Fw::String(longFileName.c_str())); tester_setSequenceExecArgs(args); + this->clearHistory(); + // validate() should reject the sequence because the resolved path overflows + ASSERT_EQ(tester_validate(), Fw::Success::FAILURE); ASSERT_EVENTS_SequenceFilePathTooLong_SIZE(1); ASSERT_EQ(this->eventHistory_SequenceFilePathTooLong->at(0).baseDir, Fw::LogStringArg(longBaseDir.c_str())); - // the stored path was truncated to the maximum length that fits the buffer - ASSERT_EQ(tester_get_m_sequenceExecArgs_ptr()->get_filePath().length(), - static_cast(FileNameStringSize)); } TEST_F(FpySequencerTester, cmd_DUMP_STACK_TO_FILE_openErrorLogsDumpFileName) { @@ -3130,16 +3124,6 @@ TEST_F(FpySequencerTester, cmd_DUMP_STACK_TO_FILE_openErrorLogsDumpFileName) { ASSERT_EQ(this->eventHistory_FileOpenError->at(0).filePath, Fw::LogStringArg("nonexistent_dir/dump.bin")); } -TEST_F(FpySequencerTester, prmSeqBaseDirTlm) { - // setting the param should emit the telemetry channel via parameterUpdated - Fw::ParamString val("/seq"); - paramSet_SEQ_BASE_DIR(val, Fw::ParamValid::VALID); - paramSend_SEQ_BASE_DIR(0, 0); - - ASSERT_TLM_PRM_SEQ_BASE_DIR_SIZE(1); - ASSERT_TLM_PRM_SEQ_BASE_DIR(0, val.toChar()); -} - TEST_F(FpySequencerTester, allocateBuffer) { Fw::MallocAllocator alloc; cmp.allocateBuffer(0, alloc, 100); diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp index 87c26340e0a..16b4d66cddf 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp @@ -12,6 +12,17 @@ namespace Svc { +// Test subclass that lets each test choose the base directory against which +// sequence file paths are resolved. SEQ_BASE_DIR is a compile-time config +// constant in production, but tests need to drive different values, so we +// override the getSequenceBaseDir() helper to return a settable member. +class TestFpySequencer : public FpySequencer { + public: + TestFpySequencer(const char* compName) : FpySequencer(compName), m_baseDirOverride(Fpy::SEQ_BASE_DIR) {} + const char* getSequenceBaseDir() const override { return this->m_baseDirOverride.toChar(); } + Fw::String m_baseDirOverride; +}; + class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test { // ---------------------------------------------------------------------- // Construction and destruction @@ -53,7 +64,7 @@ class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test //! The component under test //! - FpySequencer cmp; + TestFpySequencer cmp; FpySequencer& component; // for compatibility // dispatches events from the queue until the component reaches the given state From 438e12bbe74ecce85e47928948c8f91236a413f3 Mon Sep 17 00:00:00 2001 From: zimri-leisher Date: Wed, 24 Jun 2026 20:43:11 -0400 Subject: [PATCH 3/4] Update state machine sdd --- Svc/FpySequencer/docs/sdd.md | 81 +++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 34 deletions(-) diff --git a/Svc/FpySequencer/docs/sdd.md b/Svc/FpySequencer/docs/sdd.md index e99bbe83b46..891764bec0f 100644 --- a/Svc/FpySequencer/docs/sdd.md +++ b/Svc/FpySequencer/docs/sdd.md @@ -49,14 +49,15 @@ The following diagram represents the states of the `FpySequencer`. stateDiagram-v2 direction LR state "IDLE -entry / clearBreakpoint, clearSequenceExecArgs +entry / clearBreakpoint, setGoalState_IDLE, clearSequenceExecArgs ------------------------------------------------------ cmd_SET_BREAKPOINT / setBreakpoint - cmd_CLEAR_BREAKPOINT / clearBreakpoint + cmd_CLEAR_BREAKPOINT / clearBreakpoint " as IDLE state "VALIDATING - enter / report_seqStarted, validate + entry / report_seqStarted, signalEntered + entered / validate ------------------------------------------------------ cmd_SET_BREAKPOINT / setBreakpoint cmd_CLEAR_BREAKPOINT / clearBreakpoint @@ -65,64 +66,76 @@ entry / clearBreakpoint, clearSequenceExecArgs state VALID <> state "AWAITING_CMD_RUN_VALIDATED - enter / resp_OK + entry / sendCmdResponse_OK ------------------------------------------------------ cmd_SET_BREAKPOINT / setBreakpoint cmd_CLEAR_BREAKPOINT / clearBreakpoint " as AWAITING_CMD_RUN_VALIDATED - state "RUNNING entry / resetRuntime + state "RUNNING + entry / resetRuntime, incrementSequenceCounter, pushArgsToStack + ------------------------------------------------------ + cmd_BREAK / setBreakBeforeNextLine + cmd_SET_BREAKPOINT / setBreakpoint + cmd_CLEAR_BREAKPOINT / clearBreakpoint " as RUNNING { state BREAK_CHECK <> state "DISPATCH_STATEMENT - enter / dispatch + entry / dispatchStatement " as DISPATCH_STATEMENT + state "AWAITING_STATEMENT_RESPONSE + checkTimersIn / checkStatementTimeout + " as AWAITING_STATEMENT_RESPONSE + + state "SLEEPING + checkTimersIn / checkShouldWake, checkStatementTimeout + " as SLEEPING + state "PAUSED - entry / clearBreakBeforeNextLine, if breakOnce: clearBreakpoint - ------------------------------------------------------------- + entry / signalEntered, clearBreakBeforeNextLine + entered / if breakOnce: clearBreakpoint " as PAUSED [*] --> BREAK_CHECK - BREAK_CHECK --> PAUSED: if break - BREAK_CHECK --> DISPATCH_STATEMENT: if not break + BREAK_CHECK --> PAUSED: if shouldBreak / report_seqBroken + BREAK_CHECK --> DISPATCH_STATEMENT: else PAUSED --> DISPATCH_STATEMENT: cmd_CONTINUE - PAUSED --> DISPATCH_STATEMENT: cmdSTEP/setBreakBeforeNextLine - - DISPATCH_STATEMENT --> [*]: noMoreStatements/resp_OK - DISPATCH_STATEMENT --> [*]: failure/resp_EXECUTION_ERROR - DISPATCH_STATEMENT --> AWAITING_STATEMENT_RESPONSE: success - + PAUSED --> DISPATCH_STATEMENT: cmd_STEP / setBreakBeforeNextLine - AWAITING_STATEMENT_RESPONSE --> [*]: failure/resp_EXECUTION_ERROR - AWAITING_STATEMENT_RESPONSE --> [*]: timeout - AWAITING_STATEMENT_RESPONSE --> SLEEPING: beginSleep - AWAITING_STATEMENT_RESPONSE --> BREAK_CHECK: success + DISPATCH_STATEMENT --> [*]: result_dispatchStatement_noMoreStatements / report_seqSucceeded, sendCmdResponse_OK + DISPATCH_STATEMENT --> [*]: result_dispatchStatement_failure / report_seqFailed, sendCmdResponse_EXECUTION_ERROR + DISPATCH_STATEMENT --> AWAITING_STATEMENT_RESPONSE: result_dispatchStatement_success - SLEEPING --> [*]: timeout/resp_EXECUTION_ERROR - SLEEPING --> [*]: error/resp_EXECUTION_ERROR + AWAITING_STATEMENT_RESPONSE --> BREAK_CHECK: stmtResponse_success + AWAITING_STATEMENT_RESPONSE --> SLEEPING: stmtResponse_beginSleep + AWAITING_STATEMENT_RESPONSE --> [*]: stmtResponse_failure / report_seqFailed, sendCmdResponse_EXECUTION_ERROR + AWAITING_STATEMENT_RESPONSE --> [*]: result_checkStatementTimeout_statementTimeout / report_seqFailed, sendCmdResponse_EXECUTION_ERROR + AWAITING_STATEMENT_RESPONSE --> [*]: result_timeOpFailed / report_seqFailed, sendCmdResponse_EXECUTION_ERROR - SLEEPING --> BREAK_CHECK: shouldWake + SLEEPING --> BREAK_CHECK: result_checkShouldWake_wakeup + SLEEPING --> [*]: result_checkStatementTimeout_statementTimeout / report_seqFailed, sendCmdResponse_EXECUTION_ERROR + SLEEPING --> [*]: result_timeOpFailed / report_seqFailed, sendCmdResponse_EXECUTION_ERROR } - IDLE --> VALIDATING: cmd_VALIDATE/setSequenceExecArgs - IDLE --> VALIDATING: cmd_RUN/setSequenceExecArgs + IDLE --> VALIDATING: cmd_VALIDATE / setGoalState_VALID, setSequenceExecArgs + IDLE --> VALIDATING: cmd_RUN / setGoalState_RUNNING, setSequenceExecArgs - VALID --> RUNNING: if cmd_RUN - VALID --> AWAITING_CMD_RUN_VALIDATED: if cmd_VALIDATE + VALID --> RUNNING: if goalStateIs_RUNNING + VALID --> AWAITING_CMD_RUN_VALIDATED: else - VALIDATING --> IDLE: failure/seqFailed,resp_EXECUTION_ERROR - VALIDATING --> VALID: success - VALIDATING --> IDLE: cmd_CANCEL/seqCancelled,resp_EXECUTION_ERROR + VALIDATING --> VALID: result_success + VALIDATING --> IDLE: result_failure / report_seqFailed, sendCmdResponse_EXECUTION_ERROR + VALIDATING --> IDLE: cmd_CANCEL / report_seqCancelled, sendCmdResponse_EXECUTION_ERROR - AWAITING_CMD_RUN_VALIDATED --> IDLE: cmd_CANCEL/seqCancelled - AWAITING_CMD_RUN_VALIDATED --> RUNNING: cmd_RUN_VALIDATED + AWAITING_CMD_RUN_VALIDATED --> RUNNING: cmd_RUN_VALIDATED / setSequenceExecArgs + AWAITING_CMD_RUN_VALIDATED --> IDLE: cmd_CANCEL / report_seqCancelled - RUNNING --> IDLE: failure - RUNNING --> IDLE: noMoreStatements + RUNNING --> IDLE: cmd_CANCEL / report_seqCancelled, sendCmdResponse_EXECUTION_ERROR + RUNNING --> IDLE: stmtResponse_unexpected / report_seqFailed, sendCmdResponse_EXECUTION_ERROR ``` From 580aa4688376f18fbab98635fb1476684d8109f5 Mon Sep 17 00:00:00 2001 From: zimri-leisher Date: Wed, 24 Jun 2026 20:57:44 -0400 Subject: [PATCH 4/4] fmt --- Svc/FpySequencer/FpySequencerValidationState.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index e6637862056..c0f90a1d246 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -40,8 +40,8 @@ Fw::Success FpySequencer::validate() { const char* baseDir = this->getSequenceBaseDir(); if (baseDir != nullptr && baseDir[0] != '\0') { // the result will get truncated to FileNameStringSize - Fw::FormatStatus status = this->m_fullSequenceFilePath.format("%s/%s", baseDir, - this->m_sequenceExecArgs.get_filePath().toChar()); + Fw::FormatStatus status = + this->m_fullSequenceFilePath.format("%s/%s", baseDir, this->m_sequenceExecArgs.get_filePath().toChar()); if (status != Fw::FormatStatus::SUCCESS) { // the only runtime-reachable non-success status is OVERFLOWED, which means the