From 41c3dad4fdefec926630f70b917b9c13b2c44f87 Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Mon, 6 Apr 2026 19:10:49 -0500 Subject: [PATCH 01/18] Add args to CmdSeqIn --- Svc/CmdSequencer/CmdSequencerImpl.cpp | 17 +++++++++++++---- Svc/CmdSequencer/CmdSequencerImpl.hpp | 3 ++- Svc/CmdSequencer/test/ut/CmdSequencerTester.cpp | 12 ++++++++---- Svc/CmdSequencer/test/ut/ImmediateBase.cpp | 3 ++- Svc/CmdSequencer/test/ut/InvalidFiles.cpp | 3 ++- Svc/FpySequencer/FpySequencer.cpp | 3 ++- Svc/FpySequencer/FpySequencer.hpp | 2 +- Svc/FpySequencer/FpySequencerStateMachine.cpp | 5 ++++- .../test/ut/FpySequencerTestMain.cpp | 11 ++++++----- Svc/Seq/Seq.fpp | 5 +++++ Svc/SeqDispatcher/SeqDispatcher.cpp | 13 ++++++++++--- Svc/SeqDispatcher/SeqDispatcher.hpp | 6 ++++-- .../test/ut/SeqDispatcherTester.cpp | 5 +++-- .../test/ut/SeqDispatcherTester.hpp | 3 ++- default/config/AcConstants.fpp | 2 ++ 15 files changed, 66 insertions(+), 27 deletions(-) diff --git a/Svc/CmdSequencer/CmdSequencerImpl.cpp b/Svc/CmdSequencer/CmdSequencerImpl.cpp index f105e65b984..567ac3a4d19 100644 --- a/Svc/CmdSequencer/CmdSequencerImpl.cpp +++ b/Svc/CmdSequencer/CmdSequencerImpl.cpp @@ -100,7 +100,10 @@ void CmdSequencerComponentImpl::CS_RUN_cmdHandler(FwOpcodeType opCode, if (AUTO == this->m_stepMode) { this->m_runMode = RUNNING; if (this->isConnected_seqStartOut_OutputPort(0)) { - this->seqStartOut_out(0, this->m_sequence->getStringFileName()); + // Create empty SeqArgs as placeholder + // Use parameterized constructor to ensure m_size is initialized to 0 + Svc::SeqArgs emptyArgs(0, 0); + this->seqStartOut_out(0, this->m_sequence->getStringFileName(), emptyArgs); } this->performCmd_Step(); } @@ -162,7 +165,10 @@ void CmdSequencerComponentImpl::doSequenceRun(const Fw::StringBase& filename) { if (AUTO == this->m_stepMode) { this->m_runMode = RUNNING; if (this->isConnected_seqStartOut_OutputPort(0)) { - this->seqStartOut_out(0, this->m_sequence->getStringFileName()); + // Create empty SeqArgs as placeholder + // Use parameterized constructor to ensure m_size is initialized to 0 + Svc::SeqArgs emptyArgs(0, 0); + this->seqStartOut_out(0, this->m_sequence->getStringFileName(), emptyArgs); } this->performCmd_Step(); } @@ -170,7 +176,8 @@ void CmdSequencerComponentImpl::doSequenceRun(const Fw::StringBase& filename) { this->log_ACTIVITY_HI_CS_PortSequenceStarted(this->m_sequence->getLogFileName()); } -void CmdSequencerComponentImpl::seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& filename) { +void CmdSequencerComponentImpl::seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& filename, const Svc::SeqArgs& args) { + (void)args; // Suppress unused parameter warning this->doSequenceRun(filename); } @@ -322,7 +329,9 @@ void CmdSequencerComponentImpl ::CS_START_cmdHandler(FwOpcodeType opcode, U32 cm this->performCmd_Step(); this->log_ACTIVITY_HI_CS_CmdStarted(this->m_sequence->getLogFileName()); if (this->isConnected_seqStartOut_OutputPort(0)) { - this->seqStartOut_out(0, this->m_sequence->getStringFileName()); + // Create empty SeqArgs as placeholder + Svc::SeqArgs emptyArgs; + this->seqStartOut_out(0, this->m_sequence->getStringFileName(), emptyArgs); } this->cmdResponse_out(opcode, cmdSeq, Fw::CmdResponse::OK); } diff --git a/Svc/CmdSequencer/CmdSequencerImpl.hpp b/Svc/CmdSequencer/CmdSequencerImpl.hpp index c10c5d01fe7..035a250f4f1 100644 --- a/Svc/CmdSequencer/CmdSequencerImpl.hpp +++ b/Svc/CmdSequencer/CmdSequencerImpl.hpp @@ -520,7 +520,8 @@ class CmdSequencerComponentImpl final : public CmdSequencerComponentBase { //! Handler for input port seqRunIn void seqRunIn_handler(FwIndexType portNum, //!< The port number - const Fw::StringBase& filename //!< The sequence file + const Fw::StringBase& filename, //!< The sequence file + const Svc::SeqArgs& args //!< Sequence arguments (not currently used) ) override; //! Handler implementation for seqDispatchIn diff --git a/Svc/CmdSequencer/test/ut/CmdSequencerTester.cpp b/Svc/CmdSequencer/test/ut/CmdSequencerTester.cpp index 59990fed6b9..3f38e9b89be 100644 --- a/Svc/CmdSequencer/test/ut/CmdSequencerTester.cpp +++ b/Svc/CmdSequencer/test/ut/CmdSequencerTester.cpp @@ -271,7 +271,8 @@ void CmdSequencerTester ::parameterizedDataReadErrors(SequenceFiles::File& file) void CmdSequencerTester ::parameterizedNeverLoaded() { // Try to run a sequence Fw::String fArg(""); - this->invoke_to_seqRunIn(0, fArg); + Svc::SeqArgs emptyArgs(0, 0); + this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert seqDone response ASSERT_from_seqDone_SIZE(1); @@ -474,7 +475,8 @@ void CmdSequencerTester ::runSequence(const U32 cmdSeq, const char* const fileNa void CmdSequencerTester ::runSequenceByPortCall(const char* const fileName) { // Invoke the seqRun port Fw::String fArg(fileName); - this->invoke_to_seqRunIn(0, fArg); + Svc::SeqArgs emptyArgs(0, 0); + this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert no command response ASSERT_CMD_RESPONSE_SIZE(0); @@ -500,7 +502,8 @@ void CmdSequencerTester ::runSequenceByFileDispatcherPortCall(const char* const void CmdSequencerTester ::runLoadedSequence() { // Invoke the port Fw::String fArg(""); - this->invoke_to_seqRunIn(0, fArg); + Svc::SeqArgs emptyArgs(0, 0); + this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert no command response ASSERT_CMD_RESPONSE_SIZE(0); @@ -530,7 +533,8 @@ void CmdSequencerTester ::startNewSequence(const char* const fileName) { ASSERT_EVENTS_CS_InvalidMode_SIZE(1); // Invoke sequence port Fw::String fArg(fileName); - this->invoke_to_seqRunIn(0, fArg); + Svc::SeqArgs emptyArgs(0, 0); + this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert response on seqDone ASSERT_from_seqDone_SIZE(1); diff --git a/Svc/CmdSequencer/test/ut/ImmediateBase.cpp b/Svc/CmdSequencer/test/ut/ImmediateBase.cpp index 7a483082971..f2f69d375df 100644 --- a/Svc/CmdSequencer/test/ut/ImmediateBase.cpp +++ b/Svc/CmdSequencer/test/ut/ImmediateBase.cpp @@ -166,7 +166,8 @@ void CmdSequencerTester ::parameterizedLoadRunRun(SequenceFiles::File& file, con this->parameterizedAutoByPort(file, numCommands, bound); // Try to run a loaded sequence Fw::String fArg(""); - this->invoke_to_seqRunIn(0, fArg); + Svc::SeqArgs emptyArgs(0, 0); + this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert seqDone response ASSERT_from_seqDone_SIZE(1); diff --git a/Svc/CmdSequencer/test/ut/InvalidFiles.cpp b/Svc/CmdSequencer/test/ut/InvalidFiles.cpp index e90041dabad..29add70bdb4 100644 --- a/Svc/CmdSequencer/test/ut/InvalidFiles.cpp +++ b/Svc/CmdSequencer/test/ut/InvalidFiles.cpp @@ -218,7 +218,8 @@ void CmdSequencerTester ::MissingCRC() { ASSERT_TLM_CS_Errors(0, 2); // Run the sequence by port call Fw::String fArg(file.getName()); - this->invoke_to_seqRunIn(0, fArg); + Svc::SeqArgs emptyArgs(0, 0); + this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert seqDone response ASSERT_from_seqDone_SIZE(1); diff --git a/Svc/FpySequencer/FpySequencer.cpp b/Svc/FpySequencer/FpySequencer.cpp index 4fd12c50de1..6345819c082 100644 --- a/Svc/FpySequencer/FpySequencer.cpp +++ b/Svc/FpySequencer/FpySequencer.cpp @@ -391,7 +391,8 @@ void FpySequencer::cmdResponseIn_handler(FwIndexType portNum, //!< T } //! Handler for input port seqRunIn -void FpySequencer::seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& filename) { +void FpySequencer::seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& filename, const Svc::SeqArgs& args) { + (void)args; // Suppress unused parameter warning // can only run a seq while in idle if (sequencer_getState() != State::IDLE) { this->log_WARNING_HI_InvalidSeqRunCall(static_cast(sequencer_getState())); diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index 703229d5b66..192ed7ec25a 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -481,7 +481,7 @@ class FpySequencer : public FpySequencerComponentBase { ) override; //! Handler for input port seqRunIn - void seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& filename) override; + void seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& filename, const Svc::SeqArgs& args) override; //! Handler for input port pingIn void pingIn_handler(FwIndexType portNum, //!< The port number diff --git a/Svc/FpySequencer/FpySequencerStateMachine.cpp b/Svc/FpySequencer/FpySequencerStateMachine.cpp index c256d59fb67..1bbd80837bb 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.cpp +++ b/Svc/FpySequencer/FpySequencerStateMachine.cpp @@ -346,7 +346,10 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_report_seqStart ) { if (this->isConnected_seqStartOut_OutputPort(0)) { // report that the sequence started to internal callers - this->seqStartOut_out(0, this->m_sequenceFilePath); + // Create empty SeqArgs as placeholder + // Use parameterized constructor to ensure m_size is initialized to 0 + Svc::SeqArgs emptyArgs(0, 0); + this->seqStartOut_out(0, this->m_sequenceFilePath, emptyArgs); } } // ---------------------------------------------------------------------- diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index d4abc9d8724..1cccc86312d 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -2134,7 +2134,7 @@ TEST_F(FpySequencerTester, cmd_RUN) { ASSERT_EQ(tester_get_m_statementsDispatched(), 0); dispatchUntilState(State::RUNNING_AWAITING_STATEMENT_RESPONSE); ASSERT_from_seqStartOut_SIZE(1); - ASSERT_from_seqStartOut(0, Fw::String("test.bin")); + ASSERT_from_seqStartOut(0, Fw::String("test.bin"), Svc::SeqArgs(0, 0)); ASSERT_EQ(tester_get_m_sequencesStarted(), 1); dispatchUntilState(State::IDLE); ASSERT_EQ(tester_get_m_statementsDispatched(), 1); @@ -2151,7 +2151,7 @@ TEST_F(FpySequencerTester, cmd_RUN) { ASSERT_from_seqDoneOut_SIZE(0); dispatchUntilState(State::VALIDATING); ASSERT_from_seqStartOut_SIZE(1); - ASSERT_from_seqStartOut(0, Fw::String("test.bin")); + ASSERT_from_seqStartOut(0, Fw::String("test.bin"), Svc::SeqArgs(0, 0)); dispatchUntilState(State::RUNNING_AWAITING_STATEMENT_RESPONSE); dispatchUntilState(State::IDLE); ASSERT_CMD_RESPONSE_SIZE(1); @@ -3277,14 +3277,15 @@ TEST_F(FpySequencerTester, seqRunIn) { add_NO_OP(); writeToFile("test.bin"); - invoke_to_seqRunIn(0, Fw::String("test.bin")); + Svc::SeqArgs emptyArgs; + invoke_to_seqRunIn(0, Fw::String("test.bin"), emptyArgs); this->tester_doDispatch(); dispatchUntilState(State::VALIDATING); dispatchUntilState(State::RUNNING_AWAITING_STATEMENT_RESPONSE); dispatchUntilState(State::IDLE); ASSERT_from_seqStartOut_SIZE(1); - ASSERT_from_seqStartOut(0, Fw::String("test.bin")); + ASSERT_from_seqStartOut(0, Fw::String("test.bin"), Svc::SeqArgs(0, 0)); ASSERT_from_seqDoneOut_SIZE(1); ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::OK); @@ -3292,7 +3293,7 @@ TEST_F(FpySequencerTester, seqRunIn) { // try running while already running this->tester_setState(State::RUNNING_DISPATCH_STATEMENT); - invoke_to_seqRunIn(0, Fw::String("test.bin")); + invoke_to_seqRunIn(0, Fw::String("test.bin"), emptyArgs); // dispatch cmd this->tester_doDispatch(); ASSERT_EVENTS_InvalidSeqRunCall_SIZE(1); diff --git a/Svc/Seq/Seq.fpp b/Svc/Seq/Seq.fpp index 1caf1ce037a..04625ab0d4c 100644 --- a/Svc/Seq/Seq.fpp +++ b/Svc/Seq/Seq.fpp @@ -1,8 +1,13 @@ module Svc { + struct SeqArgs { + $size: FwSizeType + args: [SequenceArgumentsMaxSize] U8 + } default { $size = 0 } @ Port to request a sequence be run port CmdSeqIn( filename: string size 240 @< The sequence file + args: SeqArgs @< Sequence arguments (placeholder - not currently processed) ) @ Port to cancel a sequence diff --git a/Svc/SeqDispatcher/SeqDispatcher.cpp b/Svc/SeqDispatcher/SeqDispatcher.cpp index 0275e1800b9..d548d71a4e0 100644 --- a/Svc/SeqDispatcher/SeqDispatcher.cpp +++ b/Svc/SeqDispatcher/SeqDispatcher.cpp @@ -47,12 +47,18 @@ void SeqDispatcher::runSequence(FwIndexType sequencerIdx, const Fw::ConstStringB this->m_dispatchedCount++; this->tlmWrite_dispatchedCount(this->m_dispatchedCount); - this->seqRunOut_out(sequencerIdx, this->m_entryTable[sequencerIdx].sequenceRunning); + + // Create empty SeqArgs (no arguments for command-initiated sequences) + // Use parameterized constructor to ensure m_size is initialized to 0 + Svc::SeqArgs emptyArgs(0, 0); + this->seqRunOut_out(sequencerIdx, this->m_entryTable[sequencerIdx].sequenceRunning, emptyArgs); } void SeqDispatcher::seqStartIn_handler(FwIndexType portNum, //!< The port number - const Fw::StringBase& fileName //!< The sequence file name + const Fw::StringBase& fileName, //!< The sequence file name + const Svc::SeqArgs& args //!< Sequence arguments (not currently used) ) { + (void)args; // Suppress unused parameter warning FW_ASSERT(portNum >= 0 && portNum < SeqDispatcherSequencerPorts, static_cast(portNum)); if (this->m_entryTable[portNum].state == SeqDispatcher_CmdSequencerState::RUNNING_SEQUENCE_BLOCK || this->m_entryTable[portNum].state == SeqDispatcher_CmdSequencerState::RUNNING_SEQUENCE_NO_BLOCK) { @@ -121,7 +127,8 @@ void SeqDispatcher::seqDoneIn_handler(FwIndexType portNum, //!< The } //! Handler for input port seqRunIn -void SeqDispatcher::seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& fileName) { +void SeqDispatcher::seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& fileName, const Svc::SeqArgs& args) { + (void)args; // Suppress unused parameter warning FwIndexType idx = this->getNextAvailableSequencerIdx(); // no available sequencers if (idx == -1) { diff --git a/Svc/SeqDispatcher/SeqDispatcher.hpp b/Svc/SeqDispatcher/SeqDispatcher.hpp index 6ae6b670308..3f5fdc58cb3 100644 --- a/Svc/SeqDispatcher/SeqDispatcher.hpp +++ b/Svc/SeqDispatcher/SeqDispatcher.hpp @@ -40,12 +40,14 @@ class SeqDispatcher final : public SeqDispatcherComponentBase { //! Handler for input port seqStartIn void seqStartIn_handler(FwIndexType portNum, //!< The port number - const Fw::StringBase& fileName //!< The sequence file + const Fw::StringBase& fileName, //!< The sequence file + const Svc::SeqArgs& args //!< Optional sequence arguments ); //! Handler for input port seqRunIn void seqRunIn_handler(FwIndexType portNum, //!< The port number - const Fw::StringBase& fileName //!< The sequence file + const Fw::StringBase& fileName, //!< The sequence file + const Svc::SeqArgs& args //!< Optional sequence arguments ); private: diff --git a/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp b/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp index 5b3d88c7a52..639c468862d 100644 --- a/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp +++ b/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp @@ -85,9 +85,10 @@ void SeqDispatcherTester::testLogStatus() { } void SeqDispatcherTester::seqRunOut_handler(FwIndexType portNum, //!< The port number - const Fw::StringBase& filename //!< The sequence file + const Fw::StringBase& filename, //!< The sequence file + const Svc::SeqArgs& args //!< Sequence arguments ) { - this->pushFromPortEntry_seqRunOut(filename); + this->pushFromPortEntry_seqRunOut(filename, args); } } // namespace Svc diff --git a/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.hpp b/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.hpp index f3ed71369f3..23adb89cb7f 100644 --- a/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.hpp +++ b/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.hpp @@ -47,7 +47,8 @@ class SeqDispatcherTester : public SeqDispatcherGTestBase { // ---------------------------------------------------------------------- void seqRunOut_handler(FwIndexType portNum, //!< The port number - const Fw::StringBase& filename //!< The sequence file + const Fw::StringBase& filename, //!< The sequence file + const Svc::SeqArgs& args //!< Sequence arguments ); private: diff --git a/default/config/AcConstants.fpp b/default/config/AcConstants.fpp index 09e911ea83b..a2f9c3bcfdc 100644 --- a/default/config/AcConstants.fpp +++ b/default/config/AcConstants.fpp @@ -59,3 +59,5 @@ constant FwAssertTextSize = 256 @ the constants FW_ASSERT_TEXT_SIZE and FW_LOG_STRING_MAX_SIZE, set @ in FpConfig.h. constant AssertFatalAdapterEventFileSize = FileNameStringSize + +constant SequenceArgumentsMaxSize = 512 \ No newline at end of file From 44f9be0df1d84097dce95f0583b4f08ef755d1f2 Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Tue, 7 Apr 2026 10:17:48 -0500 Subject: [PATCH 02/18] Fix function header --- Svc/CmdSequencer/CmdSequencerImpl.cpp | 2 +- Svc/SeqDispatcher/SeqDispatcher.cpp | 15 ++++++--------- Svc/SeqDispatcher/SeqDispatcher.hpp | 2 +- default/config/AcConstants.fpp | 1 + 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/Svc/CmdSequencer/CmdSequencerImpl.cpp b/Svc/CmdSequencer/CmdSequencerImpl.cpp index 567ac3a4d19..ed99e9f6a4d 100644 --- a/Svc/CmdSequencer/CmdSequencerImpl.cpp +++ b/Svc/CmdSequencer/CmdSequencerImpl.cpp @@ -330,7 +330,7 @@ void CmdSequencerComponentImpl ::CS_START_cmdHandler(FwOpcodeType opcode, U32 cm this->log_ACTIVITY_HI_CS_CmdStarted(this->m_sequence->getLogFileName()); if (this->isConnected_seqStartOut_OutputPort(0)) { // Create empty SeqArgs as placeholder - Svc::SeqArgs emptyArgs; + Svc::SeqArgs emptyArgs(0, 0); this->seqStartOut_out(0, this->m_sequence->getStringFileName(), emptyArgs); } this->cmdResponse_out(opcode, cmdSeq, Fw::CmdResponse::OK); diff --git a/Svc/SeqDispatcher/SeqDispatcher.cpp b/Svc/SeqDispatcher/SeqDispatcher.cpp index d548d71a4e0..f82cf376947 100644 --- a/Svc/SeqDispatcher/SeqDispatcher.cpp +++ b/Svc/SeqDispatcher/SeqDispatcher.cpp @@ -26,7 +26,7 @@ FwIndexType SeqDispatcher::getNextAvailableSequencerIdx() { return -1; } -void SeqDispatcher::runSequence(FwIndexType sequencerIdx, const Fw::ConstStringBase& fileName, Fw::Wait block) { +void SeqDispatcher::runSequence(FwIndexType sequencerIdx, const Fw::ConstStringBase& fileName, Fw::Wait block, const Svc::SeqArgs& args) { // this function is only designed for internal usage // we can guarantee it cannot be called with input that would fail FW_ASSERT(sequencerIdx >= 0 && sequencerIdx < SeqDispatcherSequencerPorts, @@ -47,11 +47,7 @@ void SeqDispatcher::runSequence(FwIndexType sequencerIdx, const Fw::ConstStringB this->m_dispatchedCount++; this->tlmWrite_dispatchedCount(this->m_dispatchedCount); - - // Create empty SeqArgs (no arguments for command-initiated sequences) - // Use parameterized constructor to ensure m_size is initialized to 0 - Svc::SeqArgs emptyArgs(0, 0); - this->seqRunOut_out(sequencerIdx, this->m_entryTable[sequencerIdx].sequenceRunning, emptyArgs); + this->seqRunOut_out(sequencerIdx, this->m_entryTable[sequencerIdx].sequenceRunning, args); } void SeqDispatcher::seqStartIn_handler(FwIndexType portNum, //!< The port number @@ -128,7 +124,6 @@ void SeqDispatcher::seqDoneIn_handler(FwIndexType portNum, //!< The //! Handler for input port seqRunIn void SeqDispatcher::seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& fileName, const Svc::SeqArgs& args) { - (void)args; // Suppress unused parameter warning FwIndexType idx = this->getNextAvailableSequencerIdx(); // no available sequencers if (idx == -1) { @@ -136,7 +131,7 @@ void SeqDispatcher::seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& return; } - this->runSequence(idx, fileName, Fw::Wait::NO_WAIT); + this->runSequence(idx, fileName, Fw::Wait::NO_WAIT, args); } // ---------------------------------------------------------------------- // Command handler implementations @@ -154,7 +149,9 @@ void SeqDispatcher ::RUN_cmdHandler(const FwOpcodeType opCode, return; } - this->runSequence(idx, fileName, block); + // Empty Args Placeholder + Svc::SeqArgs emptyArgs(0, 0); + this->runSequence(idx, fileName, block, emptyArgs); if (block == Fw::Wait::NO_WAIT) { // return instantly diff --git a/Svc/SeqDispatcher/SeqDispatcher.hpp b/Svc/SeqDispatcher/SeqDispatcher.hpp index 3f5fdc58cb3..26d931cc569 100644 --- a/Svc/SeqDispatcher/SeqDispatcher.hpp +++ b/Svc/SeqDispatcher/SeqDispatcher.hpp @@ -70,7 +70,7 @@ class SeqDispatcher final : public SeqDispatcherComponentBase { FwIndexType getNextAvailableSequencerIdx(); - void runSequence(FwIndexType sequencerIdx, const Fw::ConstStringBase& fileName, Fw::Wait block); + void runSequence(FwIndexType sequencerIdx, const Fw::ConstStringBase& fileName, Fw::Wait block, const Svc::SeqArgs& args); // ---------------------------------------------------------------------- // Command handler implementations diff --git a/default/config/AcConstants.fpp b/default/config/AcConstants.fpp index a2f9c3bcfdc..d1575320ecf 100644 --- a/default/config/AcConstants.fpp +++ b/default/config/AcConstants.fpp @@ -60,4 +60,5 @@ constant FwAssertTextSize = 256 @ in FpConfig.h. constant AssertFatalAdapterEventFileSize = FileNameStringSize +@ The maximum size in bytes for passing sequence arguments through CmdSeqIn ports constant SequenceArgumentsMaxSize = 512 \ No newline at end of file From 5a7ad1895cee9ec53c659c14419142cbd8a4523a Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Thu, 9 Apr 2026 14:50:03 -0500 Subject: [PATCH 03/18] Add RUN_ARGS handlers + UTs --- Svc/CmdSequencer/CmdSequencerImpl.cpp | 6 +- .../test/ut/CmdSequencerTester.cpp | 8 +- Svc/CmdSequencer/test/ut/ImmediateBase.cpp | 2 +- Svc/CmdSequencer/test/ut/InvalidFiles.cpp | 2 +- Svc/FpySequencer/FpySequencer.cpp | 21 ++- Svc/FpySequencer/FpySequencer.hpp | 21 ++- Svc/FpySequencer/FpySequencerCommands.fppi | 25 ++-- Svc/FpySequencer/FpySequencerStack.cpp | 2 +- Svc/FpySequencer/FpySequencerStateMachine.cpp | 35 ++++- .../FpySequencerStateMachine.fppi | 4 +- .../test/ut/FpySequencerTestMain.cpp | 80 ++++++++++ .../test/ut/FpySequencerTester.hpp | 3 + Svc/Seq/Seq.fpp | 4 +- Svc/SeqDispatcher/SeqDispatcher.cpp | 16 +- Svc/SeqDispatcher/SeqDispatcher.hpp | 8 + Svc/SeqDispatcher/SeqDispatcherCommands.fppi | 10 +- .../test/ut/SeqDispatcherTestMain.cpp | 20 +++ .../test/ut/SeqDispatcherTester.cpp | 137 ++++++++++++++++++ .../test/ut/SeqDispatcherTester.hpp | 4 + default/config/AcConstants.fpp | 5 +- 20 files changed, 378 insertions(+), 35 deletions(-) diff --git a/Svc/CmdSequencer/CmdSequencerImpl.cpp b/Svc/CmdSequencer/CmdSequencerImpl.cpp index ed99e9f6a4d..6442ed4abe5 100644 --- a/Svc/CmdSequencer/CmdSequencerImpl.cpp +++ b/Svc/CmdSequencer/CmdSequencerImpl.cpp @@ -102,7 +102,7 @@ void CmdSequencerComponentImpl::CS_RUN_cmdHandler(FwOpcodeType opCode, if (this->isConnected_seqStartOut_OutputPort(0)) { // Create empty SeqArgs as placeholder // Use parameterized constructor to ensure m_size is initialized to 0 - Svc::SeqArgs emptyArgs(0, 0); + Svc::SeqArgs emptyArgs{0, 0}; this->seqStartOut_out(0, this->m_sequence->getStringFileName(), emptyArgs); } this->performCmd_Step(); @@ -167,7 +167,7 @@ void CmdSequencerComponentImpl::doSequenceRun(const Fw::StringBase& filename) { if (this->isConnected_seqStartOut_OutputPort(0)) { // Create empty SeqArgs as placeholder // Use parameterized constructor to ensure m_size is initialized to 0 - Svc::SeqArgs emptyArgs(0, 0); + Svc::SeqArgs emptyArgs{0, 0}; this->seqStartOut_out(0, this->m_sequence->getStringFileName(), emptyArgs); } this->performCmd_Step(); @@ -330,7 +330,7 @@ void CmdSequencerComponentImpl ::CS_START_cmdHandler(FwOpcodeType opcode, U32 cm this->log_ACTIVITY_HI_CS_CmdStarted(this->m_sequence->getLogFileName()); if (this->isConnected_seqStartOut_OutputPort(0)) { // Create empty SeqArgs as placeholder - Svc::SeqArgs emptyArgs(0, 0); + Svc::SeqArgs emptyArgs{0, 0}; this->seqStartOut_out(0, this->m_sequence->getStringFileName(), emptyArgs); } this->cmdResponse_out(opcode, cmdSeq, Fw::CmdResponse::OK); diff --git a/Svc/CmdSequencer/test/ut/CmdSequencerTester.cpp b/Svc/CmdSequencer/test/ut/CmdSequencerTester.cpp index 3f38e9b89be..c43b9d1f04d 100644 --- a/Svc/CmdSequencer/test/ut/CmdSequencerTester.cpp +++ b/Svc/CmdSequencer/test/ut/CmdSequencerTester.cpp @@ -271,7 +271,7 @@ void CmdSequencerTester ::parameterizedDataReadErrors(SequenceFiles::File& file) void CmdSequencerTester ::parameterizedNeverLoaded() { // Try to run a sequence Fw::String fArg(""); - Svc::SeqArgs emptyArgs(0, 0); + Svc::SeqArgs emptyArgs{0, 0}; this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert seqDone response @@ -475,7 +475,7 @@ void CmdSequencerTester ::runSequence(const U32 cmdSeq, const char* const fileNa void CmdSequencerTester ::runSequenceByPortCall(const char* const fileName) { // Invoke the seqRun port Fw::String fArg(fileName); - Svc::SeqArgs emptyArgs(0, 0); + Svc::SeqArgs emptyArgs{0, 0}; this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert no command response @@ -502,7 +502,7 @@ void CmdSequencerTester ::runSequenceByFileDispatcherPortCall(const char* const void CmdSequencerTester ::runLoadedSequence() { // Invoke the port Fw::String fArg(""); - Svc::SeqArgs emptyArgs(0, 0); + Svc::SeqArgs emptyArgs{0, 0}; this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert no command response @@ -533,7 +533,7 @@ void CmdSequencerTester ::startNewSequence(const char* const fileName) { ASSERT_EVENTS_CS_InvalidMode_SIZE(1); // Invoke sequence port Fw::String fArg(fileName); - Svc::SeqArgs emptyArgs(0, 0); + Svc::SeqArgs emptyArgs{0, 0}; this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert response on seqDone diff --git a/Svc/CmdSequencer/test/ut/ImmediateBase.cpp b/Svc/CmdSequencer/test/ut/ImmediateBase.cpp index f2f69d375df..20a1d0cfefc 100644 --- a/Svc/CmdSequencer/test/ut/ImmediateBase.cpp +++ b/Svc/CmdSequencer/test/ut/ImmediateBase.cpp @@ -166,7 +166,7 @@ void CmdSequencerTester ::parameterizedLoadRunRun(SequenceFiles::File& file, con this->parameterizedAutoByPort(file, numCommands, bound); // Try to run a loaded sequence Fw::String fArg(""); - Svc::SeqArgs emptyArgs(0, 0); + Svc::SeqArgs emptyArgs{0, 0}; this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert seqDone response diff --git a/Svc/CmdSequencer/test/ut/InvalidFiles.cpp b/Svc/CmdSequencer/test/ut/InvalidFiles.cpp index 29add70bdb4..71e480e8571 100644 --- a/Svc/CmdSequencer/test/ut/InvalidFiles.cpp +++ b/Svc/CmdSequencer/test/ut/InvalidFiles.cpp @@ -218,7 +218,7 @@ void CmdSequencerTester ::MissingCRC() { ASSERT_TLM_CS_Errors(0, 2); // Run the sequence by port call Fw::String fArg(file.getName()); - Svc::SeqArgs emptyArgs(0, 0); + Svc::SeqArgs emptyArgs{0, 0}; this->invoke_to_seqRunIn(0, fArg, emptyArgs); this->clearAndDispatch(); // Assert seqDone response diff --git a/Svc/FpySequencer/FpySequencer.cpp b/Svc/FpySequencer/FpySequencer.cpp index 9aac344e391..3e0ab837977 100644 --- a/Svc/FpySequencer/FpySequencer.cpp +++ b/Svc/FpySequencer/FpySequencer.cpp @@ -23,6 +23,7 @@ FpySequencer ::FpySequencer(const char* const compName) m_sequenceBlockState(), m_savedOpCode(0), m_savedCmdSeq(0), + m_pendingSeqArgs(0, 0), m_goalState(), m_sequencesStarted(0), m_statementsDispatched(0), @@ -40,6 +41,16 @@ void FpySequencer::RUN_cmdHandler(FwOpcodeType opCode, //!< The op const Fw::CmdStringArg& fileName, //!< The name of the sequence file FpySequencer_BlockState block //!< Return command status when complete or not ) { + // Empty args and delegate to RUN_ARGS handler + Svc::SeqArgs emptyArgs{0, 0}; + this->RUN_ARGS_cmdHandler(opCode, cmdSeq, fileName, block, emptyArgs); +} + +void FpySequencer ::RUN_ARGS_cmdHandler(FwOpcodeType opCode, + U32 cmdSeq, + const Fw::CmdStringArg& fileName, + Svc::FpySequencer_BlockState block, + Svc::SeqArgs buffer) { // can only run a seq while in idle if (sequencer_getState() != State::IDLE) { this->log_WARNING_HI_InvalidCommand(static_cast(sequencer_getState())); @@ -53,6 +64,8 @@ void FpySequencer::RUN_cmdHandler(FwOpcodeType opCode, //!< The op this->m_savedCmdSeq = cmdSeq; } + // Store args for pushArgsToStack action + this->m_pendingSeqArgs = buffer; this->sequencer_sendSignal_cmd_RUN(FpySequencer_SequenceExecutionArgs(fileName, block)); // only respond if the user doesn't want us to block further execution @@ -80,6 +93,9 @@ void FpySequencer::VALIDATE_cmdHandler(FwOpcodeType opCode, //!< Th this->m_savedOpCode = opCode; this->m_savedCmdSeq = cmdSeq; + // VALIDATE command doesn't receive args via command interface, use empty SeqArgs + // Store empty args for pushArgsToStack action + this->m_pendingSeqArgs = Svc::SeqArgs{0, 0}; this->sequencer_sendSignal_cmd_VALIDATE( FpySequencer_SequenceExecutionArgs(fileName, FpySequencer_BlockState::BLOCK)); } @@ -359,14 +375,15 @@ void FpySequencer::cmdResponseIn_handler(FwIndexType portNum, //!< T //! Handler for input port seqRunIn void FpySequencer::seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& filename, const Svc::SeqArgs& args) { - (void)args; // Suppress unused parameter warning // can only run a seq while in idle if (sequencer_getState() != State::IDLE) { this->log_WARNING_HI_InvalidSeqRunCall(static_cast(sequencer_getState())); return; } - // seqRunIn is never blocking + // seqRunIn is never blocking - store args for pushArgsToStack action + // Args must be serialized in F' big-endian format by the caller before being sent + this->m_pendingSeqArgs = args; this->sequencer_sendSignal_cmd_RUN(FpySequencer_SequenceExecutionArgs(filename, FpySequencer_BlockState::NO_BLOCK)); } diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index 5d2933c80eb..c7bffd24a0f 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -104,7 +104,7 @@ class FpySequencer : public FpySequencerComponentBase { // pushes a byte array to the top of the stack from the source array // leaves the source array unmodified // does not convert endianness - void push(U8* src, Fpy::StackSizeType size); + void push(const U8* src, Fpy::StackSizeType size); // pushes zero bytes to the stack void pushZeroes(Fpy::StackSizeType byteCount); @@ -145,6 +145,14 @@ class FpySequencer : public FpySequencerComponentBase { const Fw::CmdStringArg& fileName, //!< The name of the sequence file FpySequencer_BlockState block //!< Return command status when complete or not ) override; + + //! Handler implementation for command RUN_ARGS + void RUN_ARGS_cmdHandler(FwOpcodeType opCode, //!< The opcode + U32 cmdSeq, //!< The command sequence number + const Fw::CmdStringArg& fileName, //!< The name of the sequence file + Svc::FpySequencer_BlockState block, //!< Return command status when complete or not + Svc::SeqArgs buffer //!< Arguments to pass to the sequencer + ) override; //! Handler for command VALIDATE //! @@ -366,6 +374,14 @@ class FpySequencer : public FpySequencerComponentBase { Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal ) override; + //! Implementation for action pushArgsToStack of state machine Svc_FpySequencer_SequencerStateMachine + //! + //! pushes sequence arguments to the stack + void Svc_FpySequencer_SequencerStateMachine_action_pushArgsToStack( + SmId smId, //!< The state machine id + Svc_FpySequencer_SequencerStateMachine::Signal signal //!< The signal + ) override; + //! Implementation for action clearBreakpoint of state machine Svc_FpySequencer_SequencerStateMachine //! //! clears the breakpoint, allowing execution of the sequence to continue @@ -600,6 +616,9 @@ class FpySequencer : public FpySequencerComponentBase { FwOpcodeType m_savedOpCode; U32 m_savedCmdSeq; + // sequence arguments to push to stack when entering RUNNING state + Svc::SeqArgs m_pendingSeqArgs; + // 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/FpySequencerCommands.fppi b/Svc/FpySequencer/FpySequencerCommands.fppi index b1f45bb5b07..8f047d5063e 100644 --- a/Svc/FpySequencer/FpySequencerCommands.fppi +++ b/Svc/FpySequencer/FpySequencerCommands.fppi @@ -6,25 +6,32 @@ async command RUN( ) \ opcode 0 priority 7 assert +async command RUN_ARGS( + fileName: string size FileNameStringSize @< The name of the sequence file + $block: BlockState @< Return command status when complete or not + buffer: Svc.SeqArgs @< Arguments to pass to the sequencer + ) \ + opcode 1 priority 7 assert + @ Loads and validates a sequence # prio: lower than sm sig and CANCEL async command VALIDATE( fileName: string size FileNameStringSize @< The name of the sequence file ) \ - opcode 1 priority 7 assert + opcode 2 priority 7 assert @ Must be called after VALIDATE. Runs the sequence that was validated. # prio: lower than sm sig and CANCEL async command RUN_VALIDATED( $block: BlockState @< Return command status when complete or not ) \ - opcode 2 priority 7 assert + opcode 3 priority 7 assert @ Cancels a running or validated sequence. After running CANCEL, the sequencer @ should return to IDLE # less prio than sm sig, but higher than everything else async command CANCEL() \ - opcode 3 priority 8 assert + opcode 4 priority 8 assert @ Sets the breakpoint which will pause the execution of the sequencer when @ reached, until unpaused by the CONTINUE command. Will pause just before @@ -34,31 +41,31 @@ async command SET_BREAKPOINT( stmtIdx: U32 @< The directive index to pause execution before. breakOnce: bool @< Whether or not to break only once at this breakpoint ) \ - opcode 4 priority 7 assert + opcode 5 priority 7 assert @ Pauses the execution of the sequencer, just before it is about to dispatch the next directive, @ until unpaused by the CONTINUE command, or stepped by the STEP command. This command is only valid @ substates of the RUNNING state that are not RUNNING.PAUSED. async command BREAK() \ - opcode 5 priority 7 assert + opcode 6 priority 7 assert @ Continues the automatic execution of the sequence after it has been paused. If a breakpoint is still @ set, it may pause again on that breakpoint. This command is only valid in the RUNNING.PAUSED state. async command CONTINUE() \ - opcode 6 priority 7 assert + opcode 7 priority 7 assert @ Clears the breakpoint, but does not continue executing the sequence. This command @ is valid in all states. This happens automatically when a sequence ends execution. async command CLEAR_BREAKPOINT() \ - opcode 7 priority 7 assert + opcode 8 priority 7 assert @ Dispatches and awaits the result of the next directive, or ends the sequence if no more directives remain. Returns @ to the RUNNING.PAUSED state if the directive executes successfully. This command is only valid in the RUNNING.PAUSED state. async command STEP() \ - opcode 8 priority 7 assert + opcode 9 priority 7 assert @ Writes the contents of the stack to a file. This command is only valid in the RUNNING.PAUSED state. async command DUMP_STACK_TO_FILE( fileName: string size FileNameStringSize @< The name of the output file ) \ - opcode 9 priority 7 assert \ No newline at end of file + opcode 10 priority 7 assert \ No newline at end of file diff --git a/Svc/FpySequencer/FpySequencerStack.cpp b/Svc/FpySequencer/FpySequencerStack.cpp index cde70382c13..0ddf3e707a3 100644 --- a/Svc/FpySequencer/FpySequencerStack.cpp +++ b/Svc/FpySequencer/FpySequencerStack.cpp @@ -122,7 +122,7 @@ void FpySequencer::Stack::pop(U8* dest, Fpy::StackSizeType destSize) { // pushes a byte array to the top of the stack from the source array // leaves the source array unmodified // does not convert endianness -void FpySequencer::Stack::push(U8* src, Fpy::StackSizeType srcSize) { +void FpySequencer::Stack::push(const U8* src, Fpy::StackSizeType srcSize) { FW_ASSERT(this->size + srcSize <= Fpy::MAX_STACK_SIZE, static_cast(this->size), static_cast(srcSize)); memcpy(this->top(), src, srcSize); diff --git a/Svc/FpySequencer/FpySequencerStateMachine.cpp b/Svc/FpySequencer/FpySequencerStateMachine.cpp index e50faf32459..8e23dba6e52 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.cpp +++ b/Svc/FpySequencer/FpySequencerStateMachine.cpp @@ -255,6 +255,34 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_incrementSequen this->m_sequencesStarted++; } +//! Implementation for action pushArgsToStack of state machine Svc_FpySequencer_SequencerStateMachine +//! +//! pushes sequence arguments to the stack +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_pendingSeqArgs; + + // Early return if no arguments provided + if (args.get_size() == 0) { + return; + } + + Fpy::StackSizeType availableSpace = Fpy::MAX_STACK_SIZE - this->m_runtime.stack.size; + + if (args.get_size() > availableSpace) { + // Args too large - fail the sequence gracefully. + this->sequencer_sendSignal_result_failure(); + return; + } + + // Push args buffer to stack. Args are already serialized in big-endian format + // by F' serialization system, so no endianness conversion is needed. + 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 @@ -343,11 +371,8 @@ 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 - // Create empty SeqArgs as placeholder - // Use parameterized constructor to ensure m_size is initialized to 0 - Svc::SeqArgs emptyArgs(0, 0); - this->seqStartOut_out(0, this->m_sequenceFilePath, emptyArgs); + // report that the sequence started to internal callers with the actual args + this->seqStartOut_out(0, this->m_sequenceFilePath, this->m_pendingSeqArgs); } } // ---------------------------------------------------------------------- diff --git a/Svc/FpySequencer/FpySequencerStateMachine.fppi b/Svc/FpySequencer/FpySequencerStateMachine.fppi index 317723e91cf..876056456ec 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.fppi +++ b/Svc/FpySequencer/FpySequencerStateMachine.fppi @@ -169,6 +169,8 @@ state machine SequencerStateMachine { action checkStatementTimeout @ increments the m_sequencesStarted counter action incrementSequenceCounter + @ pushes sequence arguments to the stack + action pushArgsToStack @ reports that a breakpoint was hit action report_seqBroken @ sets the breakpoint to the provided args @@ -226,7 +228,7 @@ state machine SequencerStateMachine { state RUNNING { @ start with a fresh baked runtime, and tick up the @ sequence counter - entry do { resetRuntime, incrementSequenceCounter } + entry do { resetRuntime, incrementSequenceCounter, pushArgsToStack } initial enter BREAK_CHECK diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index 13d53a2ae01..ac9af063716 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -2117,6 +2117,46 @@ TEST_F(FpySequencerTester, cmd_RUN) { removeFile("test.bin"); } +TEST_F(FpySequencerTester, cmd_RUN_ARGS) { + allocMem(); + add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack + add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack + add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args + writeToFile("test.bin"); + + // Pass two U32 args: 10 and 20 + Svc::SeqArgs args{0, 0}; + Fw::ExternalSerializeBuffer argBuf(args.get_buffer(), SequenceArgumentsMaxSize); + U32 arg1 = 10, arg2 = 20; + ASSERT_EQ(argBuf.serializeFrom(arg1), Fw::FW_SERIALIZE_OK); + ASSERT_EQ(argBuf.serializeFrom(arg2), Fw::FW_SERIALIZE_OK); + args.set_size(argBuf.getSize()); + + sendCmd_RUN_ARGS(0, 0, Fw::String("test.bin"), FpySequencer_BlockState::BLOCK, args); + dispatchUntilState(State::VALIDATING); + ASSERT_EQ(tester_get_m_sequencesStarted(), 0); + ASSERT_EQ(tester_get_m_statementsDispatched(), 0); + dispatchUntilState(State::RUNNING_AWAITING_STATEMENT_RESPONSE); + ASSERT_from_seqStartOut_SIZE(1); + ASSERT_EQ(tester_get_m_sequencesStarted(), 1); + + // Verify both args are on stack (8 bytes) + auto* runtime = tester_get_m_runtime_ptr(); + ASSERT_EQ(runtime->stack.size, static_cast(8)); + + dispatchUntilState(State::IDLE); + ASSERT_EQ(tester_get_m_statementsDispatched(), 3); // LOAD_REL, LOAD_REL, DISCARD + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_ARGS(), 0, Fw::CmdResponse::OK); + ASSERT_from_seqDoneOut_SIZE(1); + ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::OK); + + // Stack should be empty after discards + ASSERT_EQ(runtime->stack.size, static_cast(0)); + + removeFile("test.bin"); +} + TEST_F(FpySequencerTester, cmd_VALIDATE) { sendCmd_VALIDATE(0, 0, Fw::String("invalid seq")); // should try validating, then go to idle cuz it failed @@ -3192,6 +3232,46 @@ TEST_F(FpySequencerTester, seqRunIn) { removeFile("test.bin"); } +TEST_F(FpySequencerTester, seqRunInArgs) { + allocMem(); + add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack + add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack + add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args + writeToFile("test.bin"); + + // Pass two U32 args: 10 and 20 + Svc::SeqArgs args{0, 0}; + Fw::ExternalSerializeBuffer argBuf(args.get_buffer(), SequenceArgumentsMaxSize); + U32 arg1 = 10, arg2 = 20; + ASSERT_EQ(argBuf.serializeFrom(arg1), Fw::FW_SERIALIZE_OK); + ASSERT_EQ(argBuf.serializeFrom(arg2), Fw::FW_SERIALIZE_OK); + args.set_size(argBuf.getSize()); + + invoke_to_seqRunIn(0, Fw::String("test.bin"), args); + dispatchUntilState(State::VALIDATING); + ASSERT_EQ(tester_get_m_sequencesStarted(), 0); + ASSERT_EQ(tester_get_m_statementsDispatched(), 0); + dispatchUntilState(State::RUNNING_AWAITING_STATEMENT_RESPONSE); + ASSERT_from_seqStartOut_SIZE(1); + ASSERT_EQ(tester_get_m_sequencesStarted(), 1); + + // Verify both args are on stack (8 bytes) + auto* runtime = tester_get_m_runtime_ptr(); + ASSERT_EQ(runtime->stack.size, static_cast(8)); + + dispatchUntilState(State::IDLE); + ASSERT_EQ(tester_get_m_statementsDispatched(), 3); // LOAD_REL, LOAD_REL, DISCARD + ASSERT_from_seqStartOut_SIZE(1); + ASSERT_from_seqStartOut(0, Fw::String("test.bin"), args); + ASSERT_from_seqDoneOut_SIZE(1); + ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::OK); + + // Stack should be empty after discards + ASSERT_EQ(runtime->stack.size, static_cast(0)); + + removeFile("test.bin"); +} + // ---------------------------------------------------------------------- // Stack Unit Tests // ---------------------------------------------------------------------- diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp index 58ef6294703..be59d972cbb 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp @@ -277,6 +277,9 @@ class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test //! Get the OPCODE_RUN value static FwOpcodeType get_OPCODE_RUN() { return FpySequencerComponentBase::OPCODE_RUN; } + //! Get the OPCODE_RUN_ARGS value + static FwOpcodeType get_OPCODE_RUN_ARGS() { return FpySequencerComponentBase::OPCODE_RUN_ARGS; } + //! Get the OPCODE_VALIDATE value static FwOpcodeType get_OPCODE_VALIDATE() { return FpySequencerComponentBase::OPCODE_VALIDATE; } diff --git a/Svc/Seq/Seq.fpp b/Svc/Seq/Seq.fpp index 04625ab0d4c..94f52ee8bae 100644 --- a/Svc/Seq/Seq.fpp +++ b/Svc/Seq/Seq.fpp @@ -1,8 +1,8 @@ module Svc { struct SeqArgs { $size: FwSizeType - args: [SequenceArgumentsMaxSize] U8 - } default { $size = 0 } + buffer: [SequenceArgumentsMaxSize] U8 + } default { $size = 0, buffer = 0 } @ Port to request a sequence be run port CmdSeqIn( diff --git a/Svc/SeqDispatcher/SeqDispatcher.cpp b/Svc/SeqDispatcher/SeqDispatcher.cpp index f82cf376947..e5d60091057 100644 --- a/Svc/SeqDispatcher/SeqDispatcher.cpp +++ b/Svc/SeqDispatcher/SeqDispatcher.cpp @@ -137,10 +137,22 @@ void SeqDispatcher::seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& // Command handler implementations // ---------------------------------------------------------------------- +// RUN command delegates to RUN_ARGS with empty arguments for backward compatibility void SeqDispatcher ::RUN_cmdHandler(const FwOpcodeType opCode, const U32 cmdSeq, const Fw::CmdStringArg& fileName, Fw::Wait block) { + // Create empty args and delegate to RUN_ARGS handler + Svc::SeqArgs emptyArgs{0, 0}; + this->RUN_ARGS_cmdHandler(opCode, cmdSeq, fileName, block, emptyArgs); +} + +// RUN_ARGS command dispatches a sequence with optional arguments to the first available sequencer +void SeqDispatcher ::RUN_ARGS_cmdHandler(const FwOpcodeType opCode, + const U32 cmdSeq, + const Fw::CmdStringArg& fileName, + Fw::Wait block, + Svc::SeqArgs buffer) { FwIndexType idx = this->getNextAvailableSequencerIdx(); // no available sequencers if (idx == -1) { @@ -149,9 +161,7 @@ void SeqDispatcher ::RUN_cmdHandler(const FwOpcodeType opCode, return; } - // Empty Args Placeholder - Svc::SeqArgs emptyArgs(0, 0); - this->runSequence(idx, fileName, block, emptyArgs); + this->runSequence(idx, fileName, block, buffer); if (block == Fw::Wait::NO_WAIT) { // return instantly diff --git a/Svc/SeqDispatcher/SeqDispatcher.hpp b/Svc/SeqDispatcher/SeqDispatcher.hpp index 26d931cc569..bbf4bdb3ed1 100644 --- a/Svc/SeqDispatcher/SeqDispatcher.hpp +++ b/Svc/SeqDispatcher/SeqDispatcher.hpp @@ -83,6 +83,14 @@ class SeqDispatcher final : public SeqDispatcherComponentBase { const Fw::CmdStringArg& fileName, /*!< The name of the sequence file*/ Fw::Wait block); + //! Implementation for RUN_ARGS command handler + //! + void RUN_ARGS_cmdHandler(const FwOpcodeType opCode, /*!< The opcode*/ + const U32 cmdSeq, /*!< The command sequence number*/ + const Fw::CmdStringArg& fileName, /*!< The name of the sequence file*/ + Fw::Wait block, /*!< Return command status when complete or not*/ + Svc::SeqArgs buffer); /*!< Arguments to pass to a sequencer*/ + void LOG_STATUS_cmdHandler(const FwOpcodeType opCode, /*!< The opcode*/ const U32 cmdSeq); /*!< The command sequence number*/ }; diff --git a/Svc/SeqDispatcher/SeqDispatcherCommands.fppi b/Svc/SeqDispatcher/SeqDispatcherCommands.fppi index 378c2f6ec7a..5da052bb6d6 100644 --- a/Svc/SeqDispatcher/SeqDispatcherCommands.fppi +++ b/Svc/SeqDispatcher/SeqDispatcherCommands.fppi @@ -5,5 +5,13 @@ async command RUN( ) \ opcode 0 +@ Dispatches a sequence with arguments to the first available sequencer +async command RUN_ARGS( + fileName: string size 240 @< The name of the sequence file + $block: Fw.Wait @< Return command status when complete or not + buffer: Svc.SeqArgs @< Arguments to pass to a sequencer + ) \ + opcode 1 + @ Logs via Events the state of each connected command sequencer -async command LOG_STATUS() opcode 1 \ No newline at end of file +async command LOG_STATUS() opcode 2 \ No newline at end of file diff --git a/Svc/SeqDispatcher/test/ut/SeqDispatcherTestMain.cpp b/Svc/SeqDispatcher/test/ut/SeqDispatcherTestMain.cpp index ee84f3b4203..66a96299f1f 100644 --- a/Svc/SeqDispatcher/test/ut/SeqDispatcherTestMain.cpp +++ b/Svc/SeqDispatcher/test/ut/SeqDispatcherTestMain.cpp @@ -14,6 +14,26 @@ TEST(Nominal, testLogStatus) { tester.testLogStatus(); } +TEST(RunArgs, testRunArgsWithValidArguments) { + Svc::SeqDispatcherTester tester; + tester.testRunArgsWithValidArguments(); +} + +TEST(RunArgs, testRunArgsWithMaxSizedArguments) { + Svc::SeqDispatcherTester tester; + tester.testRunArgsWithMaxSizedArguments(); +} + +TEST(RunArgs, testRunArgsNoSequencersAvailable) { + Svc::SeqDispatcherTester tester; + tester.testRunArgsNoSequencersAvailable(); +} + +TEST(RunArgs, testRunArgsBlockingVsNonBlocking) { + Svc::SeqDispatcherTester tester; + tester.testRunArgsBlockingVsNonBlocking(); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp b/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp index 639c468862d..7cab086ffab 100644 --- a/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp +++ b/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp @@ -91,4 +91,141 @@ void SeqDispatcherTester::seqRunOut_handler(FwIndexType portNum, //!< this->pushFromPortEntry_seqRunOut(filename, args); } +// Test RUN_ARGS with valid arguments - verify arguments are propagated correctly +void SeqDispatcherTester::testRunArgsWithValidArguments() { + // Create test arguments with some data + // Note: Keep size small to fit within FW_CMD_ARG_BUFFER_MAX_SIZE constraints + // Total command payload must fit: filename (~44 bytes) + Fw::Wait (4 bytes) + SeqArgs + // With FW_CMD_ARG_BUFFER_MAX_SIZE ~= 500 bytes, SeqArgs should be < 400 bytes total + Svc::SeqArgs testArgs{0, 0}; + + // Send RUN_ARGS command with non-blocking mode + sendCmd_RUN_ARGS(0, 0, Fw::String("test"), Fw::Wait::NO_WAIT, testArgs); + this->component.doDispatch(); + + // Should get immediate response for non-blocking + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, SeqDispatcher::OPCODE_RUN_ARGS, 0, Fw::CmdResponse::OK); + + // Verify that seqRunOut was called with correct arguments + ASSERT_from_seqRunOut_SIZE(1); + ASSERT_from_seqRunOut(0, Fw::String("test"), testArgs); + + // Verify telemetry + ASSERT_TLM_dispatchedCount(0, 1); + ASSERT_TLM_sequencersAvailable(0, SeqDispatcherSequencerPorts - 1); +} + +// Test RUN_ARGS with maximum-sized arguments - test boundary conditions +void SeqDispatcherTester::testRunArgsWithMaxSizedArguments() { + // Create test arguments with a payload to test buffer handling + // Note: The actual max size is limited by FW_CMD_ARG_BUFFER_MAX_SIZE (~500 bytes) + // After accounting for filename (~44 bytes) and Fw::Wait (4 bytes), we have ~450 bytes + // With SeqArgs overhead (FwSizeType = 8 bytes), buffer can be ~440 bytes + // We use 400 bytes to stay safely within limits + Svc::SeqArgs largeArgs(400, 0); + U8* buffer = largeArgs.get_buffer(); + for (FwSizeType i = 0; i < 400; i++) { + buffer[i] = static_cast(i % 256); + } + + // Send RUN_ARGS command with large arguments (use short filename to save space) + sendCmd_RUN_ARGS(0, 0, Fw::String("test"), Fw::Wait::NO_WAIT, largeArgs); + this->component.doDispatch(); + + // Should get immediate response for non-blocking + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, SeqDispatcher::OPCODE_RUN_ARGS, 0, Fw::CmdResponse::OK); + + // Verify that seqRunOut was called with correct large arguments + ASSERT_from_seqRunOut_SIZE(1); + ASSERT_from_seqRunOut(0, Fw::String("test"), largeArgs); + + // Verify telemetry + ASSERT_TLM_dispatchedCount(0, 1); +} + +// Test RUN_ARGS when no sequencers available - verify error handling +void SeqDispatcherTester::testRunArgsNoSequencersAvailable() { + // Fill all sequencers + Svc::SeqArgs emptyArgs{0, 0}; + for (int i = 0; i < SeqDispatcherSequencerPorts; i++) { + sendCmd_RUN_ARGS(0, 0, Fw::String("test"), Fw::Wait::WAIT, emptyArgs); + this->component.doDispatch(); + // no response because blocking + ASSERT_CMD_RESPONSE_SIZE(0); + } + ASSERT_TLM_sequencersAvailable(SeqDispatcherSequencerPorts - 1, 0); + this->clearHistory(); + + // Now try to send another sequence when all are busy + Svc::SeqArgs testArgs; + testArgs.set_size(0); + + sendCmd_RUN_ARGS(0, 0, Fw::String("test"), Fw::Wait::WAIT, testArgs); + this->component.doDispatch(); + + // Should get error response + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, SeqDispatcher::OPCODE_RUN_ARGS, 0, Fw::CmdResponse::EXECUTION_ERROR); + + // Should get warning event + ASSERT_EVENTS_SIZE(1); + ASSERT_EVENTS_NoAvailableSequencers_SIZE(1); + + // Verify no seqRunOut was called + ASSERT_from_seqRunOut_SIZE(0); +} + +// Test RUN_ARGS with blocking vs non-blocking behavior +void SeqDispatcherTester::testRunArgsBlockingVsNonBlocking() { + Svc::SeqArgs testArgs{0, 0}; + + // Test non-blocking mode - should get immediate response + sendCmd_RUN_ARGS(0, 0, Fw::String("nonblocking"), Fw::Wait::NO_WAIT, testArgs); + this->component.doDispatch(); + + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, SeqDispatcher::OPCODE_RUN_ARGS, 0, Fw::CmdResponse::OK); + ASSERT_from_seqRunOut_SIZE(1); + this->clearHistory(); + + // Free up the sequencer + this->invoke_to_seqDoneIn(0, 0, 0, Fw::CmdResponse::OK); + this->component.doDispatch(); + this->clearHistory(); + + // Test blocking mode - should NOT get immediate response + sendCmd_RUN_ARGS(0, 0, Fw::String("blocking"), Fw::Wait::WAIT, testArgs); + this->component.doDispatch(); + + ASSERT_CMD_RESPONSE_SIZE(0); // No response yet + ASSERT_from_seqRunOut_SIZE(1); + ASSERT_from_seqRunOut(0, Fw::String("blocking"), testArgs); + + // Now complete the sequence + this->invoke_to_seqDoneIn(0, 0, 0, Fw::CmdResponse::OK); + this->component.doDispatch(); + + // Should now get response + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, SeqDispatcher::OPCODE_RUN_ARGS, 0, Fw::CmdResponse::OK); + + // Test blocking mode with error response + this->clearHistory(); + sendCmd_RUN_ARGS(0, 0, Fw::String("blocking_error"), Fw::Wait::WAIT, testArgs); + this->component.doDispatch(); + + ASSERT_CMD_RESPONSE_SIZE(0); // No response yet + + // Complete with error + this->invoke_to_seqDoneIn(0, 0, 0, Fw::CmdResponse::EXECUTION_ERROR); + this->component.doDispatch(); + + // Should get error response + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, SeqDispatcher::OPCODE_RUN_ARGS, 0, Fw::CmdResponse::EXECUTION_ERROR); + ASSERT_TLM_errorCount(0, 1); +} + } // namespace Svc diff --git a/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.hpp b/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.hpp index 23adb89cb7f..7787180db52 100644 --- a/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.hpp +++ b/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.hpp @@ -40,6 +40,10 @@ class SeqDispatcherTester : public SeqDispatcherGTestBase { void testDispatch(); void testLogStatus(); + void testRunArgsWithValidArguments(); + void testRunArgsWithMaxSizedArguments(); + void testRunArgsNoSequencersAvailable(); + void testRunArgsBlockingVsNonBlocking(); private: // ---------------------------------------------------------------------- diff --git a/default/config/AcConstants.fpp b/default/config/AcConstants.fpp index d1575320ecf..8a375bf6e19 100644 --- a/default/config/AcConstants.fpp +++ b/default/config/AcConstants.fpp @@ -61,4 +61,7 @@ constant FwAssertTextSize = 256 constant AssertFatalAdapterEventFileSize = FileNameStringSize @ The maximum size in bytes for passing sequence arguments through CmdSeqIn ports -constant SequenceArgumentsMaxSize = 512 \ No newline at end of file +@ Note: This must fit within FW_CMD_ARG_BUFFER_MAX_SIZE along with other command arguments +@ Total serialized size: fileName (~44 bytes) + Fw::Wait (4 bytes) + SeqArgs (8 + buffer_size) +@ With FW_CMD_ARG_BUFFER_MAX_SIZE ~= 500, we limit this to 400 bytes +constant SequenceArgumentsMaxSize = 400 \ No newline at end of file From 1e828ec9785f33dca21a59b1204c937f239edb14 Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Wed, 15 Apr 2026 17:21:52 -0500 Subject: [PATCH 04/18] Fix argument sizing / states --- Svc/CmdSequencer/Commands.fppi | 4 +- Svc/FpySequencer/FpySequencer.cpp | 39 +++--- Svc/FpySequencer/FpySequencer.hpp | 40 ++++-- Svc/FpySequencer/FpySequencerCommands.fppi | 24 ++-- Svc/FpySequencer/FpySequencerStateMachine.cpp | 32 ++++- .../FpySequencerStateMachine.fppi | 13 +- .../FpySequencerValidationState.cpp | 6 + .../test/ut/FpySequencerTestMain.cpp | 115 ++++++++++++++++++ .../test/ut/FpySequencerTester.hpp | 3 + Svc/Seq/Seq.fpp | 4 +- Svc/SeqDispatcher/SeqDispatcherCommands.fppi | 4 +- .../test/ut/SeqDispatcherTester.cpp | 11 +- default/config/AcConstants.fpp | 9 +- 13 files changed, 246 insertions(+), 58 deletions(-) diff --git a/Svc/CmdSequencer/Commands.fppi b/Svc/CmdSequencer/Commands.fppi index c7a4f837688..77018885160 100644 --- a/Svc/CmdSequencer/Commands.fppi +++ b/Svc/CmdSequencer/Commands.fppi @@ -2,14 +2,14 @@ @ Run a command sequence file async command CS_RUN( - fileName: string size 240 @< The name of the sequence file + fileName: string size FileNameStringSize @< The name of the sequence file $block: BlockState @< Return command status when complete or not ) \ opcode 0 @ Validate a command sequence file async command CS_VALIDATE( - fileName: string size 240 @< The name of the sequence file + fileName: string size FileNameStringSize @< The name of the sequence file ) \ opcode 1 diff --git a/Svc/FpySequencer/FpySequencer.cpp b/Svc/FpySequencer/FpySequencer.cpp index 3e0ab837977..be70a2e3e1e 100644 --- a/Svc/FpySequencer/FpySequencer.cpp +++ b/Svc/FpySequencer/FpySequencer.cpp @@ -23,7 +23,6 @@ FpySequencer ::FpySequencer(const char* const compName) m_sequenceBlockState(), m_savedOpCode(0), m_savedCmdSeq(0), - m_pendingSeqArgs(0, 0), m_goalState(), m_sequencesStarted(0), m_statementsDispatched(0), @@ -42,15 +41,15 @@ void FpySequencer::RUN_cmdHandler(FwOpcodeType opCode, //!< The op FpySequencer_BlockState block //!< Return command status when complete or not ) { // Empty args and delegate to RUN_ARGS handler - Svc::SeqArgs emptyArgs{0, 0}; - this->RUN_ARGS_cmdHandler(opCode, cmdSeq, fileName, block, emptyArgs); + this->RUN_ARGS_cmdHandler(opCode, cmdSeq, fileName, block, Svc::SeqArgs{0, 0}); } -void FpySequencer ::RUN_ARGS_cmdHandler(FwOpcodeType opCode, - U32 cmdSeq, - const Fw::CmdStringArg& fileName, - Svc::FpySequencer_BlockState block, - Svc::SeqArgs buffer) { +void FpySequencer ::RUN_ARGS_cmdHandler(FwOpcodeType opCode, //!< The opcode + U32 cmdSeq, //!< The command sequence number + const Fw::CmdStringArg& fileName, //!< The name of the sequence file + Svc::FpySequencer_BlockState block, //!< Return command status when complete or not + Svc::SeqArgs args //!< Arguments to pass to the sequencer +) { // can only run a seq while in idle if (sequencer_getState() != State::IDLE) { this->log_WARNING_HI_InvalidCommand(static_cast(sequencer_getState())); @@ -65,8 +64,7 @@ void FpySequencer ::RUN_ARGS_cmdHandler(FwOpcodeType opCode, } // Store args for pushArgsToStack action - this->m_pendingSeqArgs = buffer; - this->sequencer_sendSignal_cmd_RUN(FpySequencer_SequenceExecutionArgs(fileName, block)); + this->sequencer_sendSignal_cmd_RUN(FpySequencer_SequenceExecutionArgs(fileName, block, args)); // only respond if the user doesn't want us to block further execution if (block == FpySequencer_BlockState::NO_BLOCK) { @@ -95,18 +93,26 @@ void FpySequencer::VALIDATE_cmdHandler(FwOpcodeType opCode, //!< Th // VALIDATE command doesn't receive args via command interface, use empty SeqArgs // Store empty args for pushArgsToStack action - this->m_pendingSeqArgs = Svc::SeqArgs{0, 0}; this->sequencer_sendSignal_cmd_VALIDATE( - FpySequencer_SequenceExecutionArgs(fileName, FpySequencer_BlockState::BLOCK)); + FpySequencer_SequenceExecutionArgs(fileName, FpySequencer_BlockState::BLOCK, Svc::SeqArgs{0, 0})); +} + +void FpySequencer::RUN_VALIDATED_cmdHandler( + FwOpcodeType opCode, //!< The opcode + U32 cmdSeq, //!< The command sequence number + FpySequencer_BlockState block //!< Return command status when complete or not +) { + this->RUN_VALIDATED_ARGS_cmdHandler(opCode, cmdSeq, block, Svc::SeqArgs{0, 0}); } //! Handler for command RUN_VALIDATED //! //! Runs a previously validated sequence -void FpySequencer::RUN_VALIDATED_cmdHandler( +void FpySequencer::RUN_VALIDATED_ARGS_cmdHandler( FwOpcodeType opCode, //!< The opcode U32 cmdSeq, //!< The command sequence number - FpySequencer_BlockState block //!< Return command status when complete or not + FpySequencer_BlockState block, //!< Return command status when complete or not + Svc::SeqArgs args //!< Arguments to pass to the sequencer ) { // can only RUN_VALIDATED if we have validated and are awaiting this exact cmd if (sequencer_getState() != State::AWAITING_CMD_RUN_VALIDATED) { @@ -121,7 +127,7 @@ void FpySequencer::RUN_VALIDATED_cmdHandler( this->m_savedCmdSeq = cmdSeq; } - this->sequencer_sendSignal_cmd_RUN_VALIDATED(FpySequencer_SequenceExecutionArgs(this->m_sequenceFilePath, block)); + this->sequencer_sendSignal_cmd_RUN_VALIDATED(FpySequencer_SequenceExecutionArgs(this->m_sequenceFilePath, block, args)); // only respond if the user doesn't want us to block further execution if (block == FpySequencer_BlockState::NO_BLOCK) { @@ -383,8 +389,7 @@ void FpySequencer::seqRunIn_handler(FwIndexType portNum, const Fw::StringBase& f // seqRunIn is never blocking - store args for pushArgsToStack action // Args must be serialized in F' big-endian format by the caller before being sent - this->m_pendingSeqArgs = args; - this->sequencer_sendSignal_cmd_RUN(FpySequencer_SequenceExecutionArgs(filename, FpySequencer_BlockState::NO_BLOCK)); + this->sequencer_sendSignal_cmd_RUN(FpySequencer_SequenceExecutionArgs(filename, FpySequencer_BlockState::NO_BLOCK, args)); } //! Handler for input port tlmWrite diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index c7bffd24a0f..ea3e785cb26 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -151,7 +151,7 @@ class FpySequencer : public FpySequencerComponentBase { U32 cmdSeq, //!< The command sequence number const Fw::CmdStringArg& fileName, //!< The name of the sequence file Svc::FpySequencer_BlockState block, //!< Return command status when complete or not - Svc::SeqArgs buffer //!< Arguments to pass to the sequencer + Svc::SeqArgs args //!< Arguments to pass to the sequencer ) override; //! Handler for command VALIDATE @@ -162,14 +162,23 @@ class FpySequencer : public FpySequencerComponentBase { const Fw::CmdStringArg& fileName //!< The name of the sequence file ) override; - //! Handler for command RUN_VALIDATED + //! Handler implementation for command RUN_VALIDATED //! - //! Runs a previously validated sequence - void RUN_VALIDATED_cmdHandler(FwOpcodeType opCode, //!< The opcode - U32 cmdSeq, //!< The command sequence number - FpySequencer_BlockState block //!< Return command status when complete or not + //! Must be called after VALIDATE. Runs the sequence that was validated. + void RUN_VALIDATED_cmdHandler(FwOpcodeType opCode, //!< The opcode + U32 cmdSeq, //!< The command sequence number + Svc::FpySequencer_BlockState block //!< Return command status when complete or not ) override; + //! Handler implementation for command RUN_VALIDATED_ARGS + //! + //! Must be called after VALIDATE. Runs the sequence that was validated with arguments + void RUN_VALIDATED_ARGS_cmdHandler(FwOpcodeType opCode, //!< The opcode + U32 cmdSeq, //!< The command sequence number + Svc::FpySequencer_BlockState block, //!< Return command status when complete or not + Svc::SeqArgs args //!< Arguments to pass to the sequencer + ) override; + //! Handler for command CANCEL //! //! Cancels a running or validated sequence @@ -260,6 +269,15 @@ class FpySequencer : public FpySequencerComponentBase { 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( + SmId smId, //!< The state machine id + Svc_FpySequencer_SequencerStateMachine::Signal signal, //!< The signal + const Svc::FpySequencer_SequenceExecutionArgs& value //!< The value + ) override; //! Implementation for action validate of state machine Svc_FpySequencer_SequencerStateMachine //! @@ -341,6 +359,14 @@ class FpySequencer : public FpySequencerComponentBase { 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 //! @@ -617,7 +643,7 @@ class FpySequencer : public FpySequencerComponentBase { U32 m_savedCmdSeq; // sequence arguments to push to stack when entering RUNNING state - Svc::SeqArgs m_pendingSeqArgs; + 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 diff --git a/Svc/FpySequencer/FpySequencerCommands.fppi b/Svc/FpySequencer/FpySequencerCommands.fppi index 8f047d5063e..1850941a440 100644 --- a/Svc/FpySequencer/FpySequencerCommands.fppi +++ b/Svc/FpySequencer/FpySequencerCommands.fppi @@ -23,15 +23,23 @@ async command VALIDATE( @ Must be called after VALIDATE. Runs the sequence that was validated. # prio: lower than sm sig and CANCEL async command RUN_VALIDATED( - $block: BlockState @< Return command status when complete or not + $block: BlockState @< Return command status when complete or not ) \ opcode 3 priority 7 assert +@ Must be called after VALIDATE. Runs the sequence that was validated. +# prio: lower than sm sig and CANCEL +async command RUN_VALIDATED_ARGS( + $block: BlockState @< Return command status when complete or not + buffer: Svc.SeqArgs @< Arguments to pass to the sequencer +) \ + opcode 4 priority 7 assert + @ Cancels a running or validated sequence. After running CANCEL, the sequencer @ should return to IDLE # less prio than sm sig, but higher than everything else async command CANCEL() \ - opcode 4 priority 8 assert + opcode 5 priority 8 assert @ Sets the breakpoint which will pause the execution of the sequencer when @ reached, until unpaused by the CONTINUE command. Will pause just before @@ -41,31 +49,31 @@ async command SET_BREAKPOINT( stmtIdx: U32 @< The directive index to pause execution before. breakOnce: bool @< Whether or not to break only once at this breakpoint ) \ - opcode 5 priority 7 assert + opcode 6 priority 7 assert @ Pauses the execution of the sequencer, just before it is about to dispatch the next directive, @ until unpaused by the CONTINUE command, or stepped by the STEP command. This command is only valid @ substates of the RUNNING state that are not RUNNING.PAUSED. async command BREAK() \ - opcode 6 priority 7 assert + opcode 7 priority 7 assert @ Continues the automatic execution of the sequence after it has been paused. If a breakpoint is still @ set, it may pause again on that breakpoint. This command is only valid in the RUNNING.PAUSED state. async command CONTINUE() \ - opcode 7 priority 7 assert + opcode 8 priority 7 assert @ Clears the breakpoint, but does not continue executing the sequence. This command @ is valid in all states. This happens automatically when a sequence ends execution. async command CLEAR_BREAKPOINT() \ - opcode 8 priority 7 assert + opcode 9 priority 7 assert @ Dispatches and awaits the result of the next directive, or ends the sequence if no more directives remain. Returns @ to the RUNNING.PAUSED state if the directive executes successfully. This command is only valid in the RUNNING.PAUSED state. async command STEP() \ - opcode 9 priority 7 assert + opcode 10 priority 7 assert @ Writes the contents of the stack to a file. This command is only valid in the RUNNING.PAUSED state. async command DUMP_STACK_TO_FILE( fileName: string size FileNameStringSize @< The name of the output file ) \ - opcode 10 priority 7 assert \ No newline at end of file + opcode 11 priority 7 assert \ No newline at end of file diff --git a/Svc/FpySequencer/FpySequencerStateMachine.cpp b/Svc/FpySequencer/FpySequencerStateMachine.cpp index 8e23dba6e52..5f37afe56bb 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.cpp +++ b/Svc/FpySequencer/FpySequencerStateMachine.cpp @@ -40,6 +40,18 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_setSequenceBloc 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(); +} + //! Implementation for action report_seqSucceeded of state machine //! Svc_FpySequencer_SequencerStateMachine //! @@ -262,7 +274,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_pendingSeqArgs; + const Svc::SeqArgs& args = this->m_sequenceArgs; // Early return if no arguments provided if (args.get_size() == 0) { @@ -273,7 +285,8 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_pushArgsToStack if (args.get_size() > availableSpace) { // Args too large - fail the sequence gracefully. - this->sequencer_sendSignal_result_failure(); + // TODO: Move this to validate() step and validate the size there. + this->sequencer_sendSignal_stmtResponse_unexpected(); return; } @@ -293,6 +306,16 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_clearSequenceFi 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 @@ -371,8 +394,9 @@ 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 with the actual args - this->seqStartOut_out(0, this->m_sequenceFilePath, this->m_pendingSeqArgs); + // 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); } } // ---------------------------------------------------------------------- diff --git a/Svc/FpySequencer/FpySequencerStateMachine.fppi b/Svc/FpySequencer/FpySequencerStateMachine.fppi index 876056456ec..6941c7d3dff 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.fppi +++ b/Svc/FpySequencer/FpySequencerStateMachine.fppi @@ -1,6 +1,7 @@ struct SequenceExecutionArgs { filePath: string size FileNameStringSize $block: BlockState + buffer: Svc.SeqArgs } struct BreakpointArgs { @@ -53,6 +54,8 @@ state machine SequencerStateMachine { 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 @ performs all steps necessary for sequence validation, and raises a signal result_success or result_failure action validate @@ -84,13 +87,15 @@ state machine SequencerStateMachine { action clearSequenceFile @ 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 } + entry do { clearBreakpoint, setGoalState_IDLE, clearSequenceFile, clearSequenceArguments} # --> # wait for a cmd @@ -98,9 +103,9 @@ state machine SequencerStateMachine { # commands # #################### # validate does not take as input a block state, only a path - on cmd_VALIDATE do { setGoalState_VALID, setSequenceFilePath } enter VALIDATING + on cmd_VALIDATE do { setGoalState_VALID, setSequenceFilePath} enter VALIDATING # run takes both path and block state - on cmd_RUN do { setGoalState_RUNNING, setSequenceFilePath, setSequenceBlockState } enter VALIDATING + on cmd_RUN do { setGoalState_RUNNING, setSequenceFilePath, setSequenceBlockState, setSequenceArguments} enter VALIDATING on cmd_SET_BREAKPOINT do { setBreakpoint } on cmd_CLEAR_BREAKPOINT do { clearBreakpoint } @@ -148,7 +153,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 { setSequenceBlockState, setSequenceArguments } enter RUNNING on cmd_SET_BREAKPOINT do { setBreakpoint } on cmd_CLEAR_BREAKPOINT do { clearBreakpoint } diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index fdf2edc03e4..4b40f463c4e 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -99,6 +99,12 @@ Fw::Success FpySequencer::validate() { return Fw::Success::FAILURE; } + Fpy::StackSizeType availableSpace = Fpy::MAX_STACK_SIZE - this->m_runtime.stack.size; + + if (this->m_sequenceArgs.get_size() > availableSpace) { + return Fw::Success::FAILURE; + } + return Fw::Success::SUCCESS; } diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index ac9af063716..883609e759e 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -2157,6 +2157,28 @@ TEST_F(FpySequencerTester, cmd_RUN_ARGS) { removeFile("test.bin"); } +TEST_F(FpySequencerTester, cmd_RUN_ARGS_oversized) { + allocMem(); + add_NO_OP(); + writeToFile("test.bin"); + + // Create args that exceed MAX_STACK_SIZE + Svc::SeqArgs largeArgs{0, 0}; + // Set size to MAX_STACK_SIZE + 1 to trigger overflow + largeArgs.set_size(Fpy::MAX_STACK_SIZE + 1); + + sendCmd_RUN_ARGS(0, 0, Fw::String("test.bin"), FpySequencer_BlockState::BLOCK, largeArgs); + dispatchUntilState(State::VALIDATING); + // should fail when trying to push args to stack + dispatchUntilState(State::IDLE); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); + ASSERT_from_seqDoneOut_SIZE(1); + ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::EXECUTION_ERROR); + + removeFile("test.bin"); +} + TEST_F(FpySequencerTester, cmd_VALIDATE) { sendCmd_VALIDATE(0, 0, Fw::String("invalid seq")); // should try validating, then go to idle cuz it failed @@ -2223,6 +2245,99 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED) { ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED(), 0, Fw::CmdResponse::OK); } +TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS) { + // should fail because in idle + this->tester_setState(State::IDLE); + Svc::SeqArgs emptyArgs{0, 0}; + sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::NO_BLOCK, emptyArgs); + dispatchCurrentMessages(cmp); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); + this->clearHistory(); + + allocMem(); + add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack + add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack + add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args + writeToFile("test.bin"); + sendCmd_VALIDATE(0, 0, Fw::String("test.bin")); + dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); + this->clearHistory(); + + // Pass two U32 args: 10 and 20 + Svc::SeqArgs args{0, 0}; + Fw::ExternalSerializeBuffer argBuf(args.get_buffer(), SequenceArgumentsMaxSize); + U32 arg1 = 10, arg2 = 20; + ASSERT_EQ(argBuf.serializeFrom(arg1), Fw::FW_SERIALIZE_OK); + ASSERT_EQ(argBuf.serializeFrom(arg2), Fw::FW_SERIALIZE_OK); + args.set_size(argBuf.getSize()); + + // should succeed immediately + sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::NO_BLOCK, args); + this->tester_doDispatch(); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::OK); + dispatchUntilState(State::RUNNING_AWAITING_STATEMENT_RESPONSE); + ASSERT_EQ(tester_get_m_sequencesStarted(), 1); + + // Verify both args are on stack (8 bytes) + auto* runtime = tester_get_m_runtime_ptr(); + ASSERT_EQ(runtime->stack.size, static_cast(8)); + + dispatchUntilState(State::IDLE); + ASSERT_EQ(tester_get_m_statementsDispatched(), 3); // LOAD_REL, LOAD_REL, DISCARD + ASSERT_CMD_RESPONSE_SIZE(1); + clearHistory(); + + sendCmd_VALIDATE(0, 0, Fw::String("test.bin")); + dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); + this->clearHistory(); + // should succeed immediately + sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::BLOCK, args); + this->tester_doDispatch(); + ASSERT_CMD_RESPONSE_SIZE(0); + dispatchUntilState(State::RUNNING_AWAITING_STATEMENT_RESPONSE); + ASSERT_EQ(tester_get_m_sequencesStarted(), 2); + + // Args should be on stack again (8 bytes) + ASSERT_EQ(runtime->stack.size, static_cast(8)); + + // should go back to IDLE + dispatchUntilState(State::IDLE); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::OK); + + // Stack should be empty after discards + ASSERT_EQ(runtime->stack.size, static_cast(0)); + + removeFile("test.bin"); +} + +TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS_oversized) { + allocMem(); + add_NO_OP(); + writeToFile("test.bin"); + sendCmd_VALIDATE(0, 0, Fw::String("test.bin")); + dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); + this->clearHistory(); + + // Create args that exceed MAX_STACK_SIZE + Svc::SeqArgs largeArgs{0, 0}; + largeArgs.set_size(Fpy::MAX_STACK_SIZE + 1); + + sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::BLOCK, largeArgs); + this->tester_doDispatch(); + ASSERT_CMD_RESPONSE_SIZE(0); + // should fail when trying to push args to stack + dispatchUntilState(State::IDLE); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); + ASSERT_from_seqDoneOut_SIZE(1); + ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::EXECUTION_ERROR); + + removeFile("test.bin"); +} + TEST_F(FpySequencerTester, cmd_CANCEL) { this->tester_setState(State::IDLE); sendCmd_CANCEL(0, 0); diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp index be59d972cbb..6d806cc7586 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp @@ -286,6 +286,9 @@ class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test //! Get the OPCODE_RUN_VALIDATED value static FwOpcodeType get_OPCODE_RUN_VALIDATED() { return FpySequencerComponentBase::OPCODE_RUN_VALIDATED; } + //! Get the OPCODE_RUN_VALIDATED_ARGS value + static FwOpcodeType get_OPCODE_RUN_VALIDATED_ARGS() { return FpySequencerComponentBase::OPCODE_RUN_VALIDATED_ARGS; } + //! Get the OPCODE_CANCEL value static FwOpcodeType get_OPCODE_CANCEL() { return FpySequencerComponentBase::OPCODE_CANCEL; } diff --git a/Svc/Seq/Seq.fpp b/Svc/Seq/Seq.fpp index 94f52ee8bae..21ca3b90db0 100644 --- a/Svc/Seq/Seq.fpp +++ b/Svc/Seq/Seq.fpp @@ -6,8 +6,8 @@ module Svc { @ Port to request a sequence be run port CmdSeqIn( - filename: string size 240 @< The sequence file - args: SeqArgs @< Sequence arguments (placeholder - not currently processed) + filename: string size FileNameStringSize @< The sequence file + args: SeqArgs @< Sequence arguments ) @ Port to cancel a sequence diff --git a/Svc/SeqDispatcher/SeqDispatcherCommands.fppi b/Svc/SeqDispatcher/SeqDispatcherCommands.fppi index 5da052bb6d6..35e2608b93e 100644 --- a/Svc/SeqDispatcher/SeqDispatcherCommands.fppi +++ b/Svc/SeqDispatcher/SeqDispatcherCommands.fppi @@ -1,13 +1,13 @@ @ Dispatches a sequence to the first available sequencer async command RUN( - fileName: string size 240 @< The name of the sequence file + fileName: string size FileNameStringSize @< The name of the sequence file $block: Fw.Wait @< Return command status when complete or not ) \ opcode 0 @ Dispatches a sequence with arguments to the first available sequencer async command RUN_ARGS( - fileName: string size 240 @< The name of the sequence file + fileName: string size FileNameStringSize @< The name of the sequence file $block: Fw.Wait @< Return command status when complete or not buffer: Svc.SeqArgs @< Arguments to pass to a sequencer ) \ diff --git a/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp b/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp index 7cab086ffab..763699f1327 100644 --- a/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp +++ b/Svc/SeqDispatcher/test/ut/SeqDispatcherTester.cpp @@ -5,6 +5,7 @@ // ====================================================================== #include "SeqDispatcherTester.hpp" +#include "config/FppConstantsAc.hpp" namespace Svc { @@ -118,14 +119,10 @@ void SeqDispatcherTester::testRunArgsWithValidArguments() { // Test RUN_ARGS with maximum-sized arguments - test boundary conditions void SeqDispatcherTester::testRunArgsWithMaxSizedArguments() { - // Create test arguments with a payload to test buffer handling - // Note: The actual max size is limited by FW_CMD_ARG_BUFFER_MAX_SIZE (~500 bytes) - // After accounting for filename (~44 bytes) and Fw::Wait (4 bytes), we have ~450 bytes - // With SeqArgs overhead (FwSizeType = 8 bytes), buffer can be ~440 bytes - // We use 400 bytes to stay safely within limits - Svc::SeqArgs largeArgs(400, 0); + constexpr FwSizeType TEST_ARG_SIZE = SequenceArgumentsMaxSize; + Svc::SeqArgs largeArgs(TEST_ARG_SIZE, 0); U8* buffer = largeArgs.get_buffer(); - for (FwSizeType i = 0; i < 400; i++) { + for (FwSizeType i = 0; i < TEST_ARG_SIZE; i++) { buffer[i] = static_cast(i % 256); } diff --git a/default/config/AcConstants.fpp b/default/config/AcConstants.fpp index 8a375bf6e19..7da403f9739 100644 --- a/default/config/AcConstants.fpp +++ b/default/config/AcConstants.fpp @@ -49,7 +49,7 @@ constant DpManagerNumPorts = 5 constant DpWriterNumProcPorts = 5 @ The size of a file name string -constant FileNameStringSize = 200 +constant FileNameStringSize = 240 @ The size of an assert text string constant FwAssertTextSize = 256 @@ -61,7 +61,6 @@ constant FwAssertTextSize = 256 constant AssertFatalAdapterEventFileSize = FileNameStringSize @ The maximum size in bytes for passing sequence arguments through CmdSeqIn ports -@ Note: This must fit within FW_CMD_ARG_BUFFER_MAX_SIZE along with other command arguments -@ Total serialized size: fileName (~44 bytes) + Fw::Wait (4 bytes) + SeqArgs (8 + buffer_size) -@ With FW_CMD_ARG_BUFFER_MAX_SIZE ~= 500, we limit this to 400 bytes -constant SequenceArgumentsMaxSize = 400 \ No newline at end of file +@ Note: This must fit within FW_CMD_ARG_BUFFER_MAX_SIZE along with cmd arguments using Svc::SeqArgs +@ Total serialized size: fileName (~200 bytes) + Fw::Wait (4 bytes) + SeqArgs (8 + buffer_size) +constant SequenceArgumentsMaxSize = FW_CMD_ARG_BUFFER_MAX_SIZE - FileNameStringSize - 8 - 8 \ No newline at end of file From 2cf37a1602757023bbfafd98d8165850eb930aab Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Wed, 15 Apr 2026 18:05:28 -0500 Subject: [PATCH 05/18] Removed cmd_RUN_VALIDATED_ARGS --- Svc/FpySequencer/FpySequencer.cpp | 32 ++++++------ Svc/FpySequencer/FpySequencer.hpp | 18 +++---- Svc/FpySequencer/FpySequencerCommands.fppi | 10 ++-- Svc/FpySequencer/FpySequencerStateMachine.cpp | 9 ---- .../FpySequencerStateMachine.fppi | 4 +- .../test/ut/FpySequencerTestMain.cpp | 49 ++++++++----------- .../test/ut/FpySequencerTester.hpp | 6 +-- 7 files changed, 55 insertions(+), 73 deletions(-) diff --git a/Svc/FpySequencer/FpySequencer.cpp b/Svc/FpySequencer/FpySequencer.cpp index be70a2e3e1e..c921b466b8c 100644 --- a/Svc/FpySequencer/FpySequencer.cpp +++ b/Svc/FpySequencer/FpySequencer.cpp @@ -79,6 +79,16 @@ void FpySequencer::VALIDATE_cmdHandler(FwOpcodeType opCode, //!< Th U32 cmdSeq, //!< The command sequence number const Fw::CmdStringArg& fileName //!< The name of the sequence file ) { + this->VALIDATE_ARGS_cmdHandler(opCode, cmdSeq, fileName, Svc::SeqArgs{0, 0}); +} + +//! Handler implementation for command VALIDATE_ARGS +//! +//! Loads and validates a sequence with arguments +void FpySequencer ::VALIDATE_ARGS_cmdHandler(FwOpcodeType opCode, + U32 cmdSeq, + const Fw::CmdStringArg& fileName, + Svc::SeqArgs buffer) { // can only validate a seq while in idle if (sequencer_getState() != State::IDLE) { this->log_WARNING_HI_InvalidCommand(static_cast(sequencer_getState())); @@ -91,28 +101,16 @@ void FpySequencer::VALIDATE_cmdHandler(FwOpcodeType opCode, //!< Th this->m_savedOpCode = opCode; this->m_savedCmdSeq = cmdSeq; - // VALIDATE command doesn't receive args via command interface, use empty SeqArgs - // Store empty args for pushArgsToStack action + // VALIDATE_ARGS receives args via command interface + // Store args for pushArgsToStack action when RUN_VALIDATED is called this->sequencer_sendSignal_cmd_VALIDATE( - FpySequencer_SequenceExecutionArgs(fileName, FpySequencer_BlockState::BLOCK, Svc::SeqArgs{0, 0})); + FpySequencer_SequenceExecutionArgs(fileName, FpySequencer_BlockState::BLOCK, buffer)); } void FpySequencer::RUN_VALIDATED_cmdHandler( FwOpcodeType opCode, //!< The opcode U32 cmdSeq, //!< The command sequence number - FpySequencer_BlockState block //!< Return command status when complete or not -) { - this->RUN_VALIDATED_ARGS_cmdHandler(opCode, cmdSeq, block, Svc::SeqArgs{0, 0}); -} - -//! Handler for command RUN_VALIDATED -//! -//! Runs a previously validated sequence -void FpySequencer::RUN_VALIDATED_ARGS_cmdHandler( - FwOpcodeType opCode, //!< The opcode - U32 cmdSeq, //!< The command sequence number - FpySequencer_BlockState block, //!< Return command status when complete or not - Svc::SeqArgs args //!< Arguments to pass to the sequencer + FpySequencer_BlockState block //!< Return command status when complete or not ) { // can only RUN_VALIDATED if we have validated and are awaiting this exact cmd if (sequencer_getState() != State::AWAITING_CMD_RUN_VALIDATED) { @@ -127,7 +125,7 @@ void FpySequencer::RUN_VALIDATED_ARGS_cmdHandler( this->m_savedCmdSeq = cmdSeq; } - this->sequencer_sendSignal_cmd_RUN_VALIDATED(FpySequencer_SequenceExecutionArgs(this->m_sequenceFilePath, block, args)); + this->sequencer_sendSignal_cmd_RUN_VALIDATED(FpySequencer_SequenceExecutionArgs(this->m_sequenceFilePath, block, this->m_sequenceArgs)); // only respond if the user doesn't want us to block further execution if (block == FpySequencer_BlockState::NO_BLOCK) { diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index ea3e785cb26..f61527c4629 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -161,6 +161,15 @@ class FpySequencer : public FpySequencerComponentBase { U32 cmdSeq, //!< The command sequence number const Fw::CmdStringArg& fileName //!< The name of the sequence file ) override; + + //! Handler implementation for command VALIDATE_ARGS + //! + //! Loads and validates a sequence with arguments + void VALIDATE_ARGS_cmdHandler(FwOpcodeType opCode, //!< The opcode + U32 cmdSeq, //!< The command sequence number + const Fw::CmdStringArg& fileName, //!< The name of the sequence file + Svc::SeqArgs buffer //!< Arguments to pass to the sequencer + ) override; //! Handler implementation for command RUN_VALIDATED //! @@ -170,15 +179,6 @@ class FpySequencer : public FpySequencerComponentBase { Svc::FpySequencer_BlockState block //!< Return command status when complete or not ) override; - //! Handler implementation for command RUN_VALIDATED_ARGS - //! - //! Must be called after VALIDATE. Runs the sequence that was validated with arguments - void RUN_VALIDATED_ARGS_cmdHandler(FwOpcodeType opCode, //!< The opcode - U32 cmdSeq, //!< The command sequence number - Svc::FpySequencer_BlockState block, //!< Return command status when complete or not - Svc::SeqArgs args //!< Arguments to pass to the sequencer - ) override; - //! Handler for command CANCEL //! //! Cancels a running or validated sequence diff --git a/Svc/FpySequencer/FpySequencerCommands.fppi b/Svc/FpySequencer/FpySequencerCommands.fppi index 1850941a440..723acf8a846 100644 --- a/Svc/FpySequencer/FpySequencerCommands.fppi +++ b/Svc/FpySequencer/FpySequencerCommands.fppi @@ -20,18 +20,18 @@ async command VALIDATE( ) \ opcode 2 priority 7 assert -@ Must be called after VALIDATE. Runs the sequence that was validated. +@ Loads and validates a sequence with arguments # prio: lower than sm sig and CANCEL -async command RUN_VALIDATED( - $block: BlockState @< Return command status when complete or not +async command VALIDATE_ARGS( + fileName: string size FileNameStringSize @< The name of the sequence file + buffer: Svc.SeqArgs @< Arguments to pass to the sequencer ) \ opcode 3 priority 7 assert @ Must be called after VALIDATE. Runs the sequence that was validated. # prio: lower than sm sig and CANCEL -async command RUN_VALIDATED_ARGS( +async command RUN_VALIDATED( $block: BlockState @< Return command status when complete or not - buffer: Svc.SeqArgs @< Arguments to pass to the sequencer ) \ opcode 4 priority 7 assert diff --git a/Svc/FpySequencer/FpySequencerStateMachine.cpp b/Svc/FpySequencer/FpySequencerStateMachine.cpp index 5f37afe56bb..2a25e68587c 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.cpp +++ b/Svc/FpySequencer/FpySequencerStateMachine.cpp @@ -281,15 +281,6 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_pushArgsToStack return; } - Fpy::StackSizeType availableSpace = Fpy::MAX_STACK_SIZE - this->m_runtime.stack.size; - - if (args.get_size() > availableSpace) { - // Args too large - fail the sequence gracefully. - // TODO: Move this to validate() step and validate the size there. - this->sequencer_sendSignal_stmtResponse_unexpected(); - return; - } - // Push args buffer to stack. Args are already serialized in big-endian format // by F' serialization system, so no endianness conversion is needed. this->m_runtime.stack.push(args.get_buffer(), diff --git a/Svc/FpySequencer/FpySequencerStateMachine.fppi b/Svc/FpySequencer/FpySequencerStateMachine.fppi index 6941c7d3dff..9970ed460c0 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.fppi +++ b/Svc/FpySequencer/FpySequencerStateMachine.fppi @@ -103,7 +103,7 @@ state machine SequencerStateMachine { # commands # #################### # validate does not take as input a block state, only a path - on cmd_VALIDATE do { setGoalState_VALID, setSequenceFilePath} enter VALIDATING + 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 @@ -153,7 +153,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, setSequenceArguments } enter RUNNING + on cmd_RUN_VALIDATED do { setSequenceBlockState } enter RUNNING on cmd_SET_BREAKPOINT do { setBreakpoint } on cmd_CLEAR_BREAKPOINT do { clearBreakpoint } diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index 883609e759e..8374caea361 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -2169,7 +2169,7 @@ TEST_F(FpySequencerTester, cmd_RUN_ARGS_oversized) { sendCmd_RUN_ARGS(0, 0, Fw::String("test.bin"), FpySequencer_BlockState::BLOCK, largeArgs); dispatchUntilState(State::VALIDATING); - // should fail when trying to push args to stack + // should fail during validation when checking args size dispatchUntilState(State::IDLE); ASSERT_CMD_RESPONSE_SIZE(1); ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); @@ -2245,24 +2245,12 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED) { ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED(), 0, Fw::CmdResponse::OK); } -TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS) { - // should fail because in idle - this->tester_setState(State::IDLE); - Svc::SeqArgs emptyArgs{0, 0}; - sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::NO_BLOCK, emptyArgs); - dispatchCurrentMessages(cmp); - ASSERT_CMD_RESPONSE_SIZE(1); - ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); - this->clearHistory(); - +TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS) { allocMem(); add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args writeToFile("test.bin"); - sendCmd_VALIDATE(0, 0, Fw::String("test.bin")); - dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); - this->clearHistory(); // Pass two U32 args: 10 and 20 Svc::SeqArgs args{0, 0}; @@ -2272,11 +2260,20 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS) { ASSERT_EQ(argBuf.serializeFrom(arg2), Fw::FW_SERIALIZE_OK); args.set_size(argBuf.getSize()); + sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), args); + dispatchUntilState(State::VALIDATING); + ASSERT_EQ(tester_get_m_sequencesStarted(), 0); + ASSERT_EQ(tester_get_m_statementsDispatched(), 0); + dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::OK); + this->clearHistory(); + // should succeed immediately - sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::NO_BLOCK, args); + sendCmd_RUN_VALIDATED(0, 0, FpySequencer_BlockState::NO_BLOCK); this->tester_doDispatch(); ASSERT_CMD_RESPONSE_SIZE(1); - ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::OK); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED(), 0, Fw::CmdResponse::OK); dispatchUntilState(State::RUNNING_AWAITING_STATEMENT_RESPONSE); ASSERT_EQ(tester_get_m_sequencesStarted(), 1); @@ -2289,11 +2286,11 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS) { ASSERT_CMD_RESPONSE_SIZE(1); clearHistory(); - sendCmd_VALIDATE(0, 0, Fw::String("test.bin")); + sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), args); dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); this->clearHistory(); // should succeed immediately - sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::BLOCK, args); + sendCmd_RUN_VALIDATED(0, 0, FpySequencer_BlockState::BLOCK); this->tester_doDispatch(); ASSERT_CMD_RESPONSE_SIZE(0); dispatchUntilState(State::RUNNING_AWAITING_STATEMENT_RESPONSE); @@ -2305,7 +2302,7 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS) { // should go back to IDLE dispatchUntilState(State::IDLE); ASSERT_CMD_RESPONSE_SIZE(1); - ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::OK); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED(), 0, Fw::CmdResponse::OK); // Stack should be empty after discards ASSERT_EQ(runtime->stack.size, static_cast(0)); @@ -2313,25 +2310,21 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS) { removeFile("test.bin"); } -TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS_oversized) { +TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_oversized) { allocMem(); add_NO_OP(); writeToFile("test.bin"); - sendCmd_VALIDATE(0, 0, Fw::String("test.bin")); - dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); - this->clearHistory(); // Create args that exceed MAX_STACK_SIZE Svc::SeqArgs largeArgs{0, 0}; largeArgs.set_size(Fpy::MAX_STACK_SIZE + 1); - sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::BLOCK, largeArgs); - this->tester_doDispatch(); - ASSERT_CMD_RESPONSE_SIZE(0); - // should fail when trying to push args to stack + sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), largeArgs); + dispatchUntilState(State::VALIDATING); + // should fail during validation when checking args size dispatchUntilState(State::IDLE); ASSERT_CMD_RESPONSE_SIZE(1); - ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); ASSERT_from_seqDoneOut_SIZE(1); ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::EXECUTION_ERROR); diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp index 6d806cc7586..ab3c76f7a0a 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp @@ -283,12 +283,12 @@ class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test //! Get the OPCODE_VALIDATE value static FwOpcodeType get_OPCODE_VALIDATE() { return FpySequencerComponentBase::OPCODE_VALIDATE; } + //! Get the OPCODE_VALIDATE_ARGS value + static FwOpcodeType get_OPCODE_VALIDATE_ARGS() { return FpySequencerComponentBase::OPCODE_VALIDATE_ARGS; } + //! Get the OPCODE_RUN_VALIDATED value static FwOpcodeType get_OPCODE_RUN_VALIDATED() { return FpySequencerComponentBase::OPCODE_RUN_VALIDATED; } - //! Get the OPCODE_RUN_VALIDATED_ARGS value - static FwOpcodeType get_OPCODE_RUN_VALIDATED_ARGS() { return FpySequencerComponentBase::OPCODE_RUN_VALIDATED_ARGS; } - //! Get the OPCODE_CANCEL value static FwOpcodeType get_OPCODE_CANCEL() { return FpySequencerComponentBase::OPCODE_CANCEL; } From 750cff71a17983dec4a9e0d15db1a82dad9e30ce Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Wed, 15 Apr 2026 19:21:54 -0500 Subject: [PATCH 06/18] Fix math --- default/config/AcConstants.fpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/default/config/AcConstants.fpp b/default/config/AcConstants.fpp index 7da403f9739..a21825f600e 100644 --- a/default/config/AcConstants.fpp +++ b/default/config/AcConstants.fpp @@ -62,5 +62,5 @@ constant AssertFatalAdapterEventFileSize = FileNameStringSize @ The maximum size in bytes for passing sequence arguments through CmdSeqIn ports @ Note: This must fit within FW_CMD_ARG_BUFFER_MAX_SIZE along with cmd arguments using Svc::SeqArgs -@ Total serialized size: fileName (~200 bytes) + Fw::Wait (4 bytes) + SeqArgs (8 + buffer_size) -constant SequenceArgumentsMaxSize = FW_CMD_ARG_BUFFER_MAX_SIZE - FileNameStringSize - 8 - 8 \ No newline at end of file +@ Total serialized size: string length prefix + fileName + BlockState + SeqArgs.size field + buffer +constant SequenceArgumentsMaxSize = FW_CMD_ARG_BUFFER_MAX_SIZE - sizeof(FwSizeStoreType) - FileNameStringSize - sizeof(U8) - sizeof(FwSizeType) \ No newline at end of file From d81aefeead2128ca6eabc59a6bc17baa4c1f745b Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Wed, 15 Apr 2026 19:23:01 -0500 Subject: [PATCH 07/18] Fix comment --- default/config/AcConstants.fpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/default/config/AcConstants.fpp b/default/config/AcConstants.fpp index a21825f600e..b1a6127d276 100644 --- a/default/config/AcConstants.fpp +++ b/default/config/AcConstants.fpp @@ -62,5 +62,5 @@ constant AssertFatalAdapterEventFileSize = FileNameStringSize @ The maximum size in bytes for passing sequence arguments through CmdSeqIn ports @ Note: This must fit within FW_CMD_ARG_BUFFER_MAX_SIZE along with cmd arguments using Svc::SeqArgs -@ Total serialized size: string length prefix + fileName + BlockState + SeqArgs.size field + buffer +@ Total serialized size: string length prefix + fileName + BlockState + SeqArgs(size + buffer) constant SequenceArgumentsMaxSize = FW_CMD_ARG_BUFFER_MAX_SIZE - sizeof(FwSizeStoreType) - FileNameStringSize - sizeof(U8) - sizeof(FwSizeType) \ No newline at end of file From 6930ce7fc9cbac2df2c046c075185ee13b1a6f4e Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Thu, 16 Apr 2026 18:58:39 -0500 Subject: [PATCH 08/18] Add arg names to header and valid --- Svc/FpySequencer/FpySequencer.hpp | 6 + Svc/FpySequencer/FpySequencerEvents.fppi | 7 + Svc/FpySequencer/FpySequencerStateMachine.cpp | 1 + Svc/FpySequencer/FpySequencerTypes.fpp | 29 +++- .../FpySequencerValidationState.cpp | 126 ++++++++++++++++++ 5 files changed, 168 insertions(+), 1 deletion(-) diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index f61527c4629..b3496aad124 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -645,6 +645,9 @@ class FpySequencer : public FpySequencerComponentBase { // sequence arguments to push to stack when entering RUNNING state Svc::SeqArgs m_sequenceArgs{}; + // the expected argument size from arg_specs (schema version 6+) + Fpy::StackSizeType m_expectedArgSize{0}; + // 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 @@ -746,6 +749,9 @@ class FpySequencer : public FpySequencerComponentBase { // reads and validates the header from the m_sequenceBuffer // return SUCCESS if sequence is valid, FAILURE otherwise Fw::Success readHeader(); + // reads and parses arg_specs from the sequence file (schema version 6+) + // return SUCCESS if successful, FAILURE otherwise + Fw::Success readArgSpecs(Os::File& file); // reads and validates the body from the m_sequenceBuffer // return SUCCESS if sequence is valid, FAILURE otherwise Fw::Success readBody(); diff --git a/Svc/FpySequencer/FpySequencerEvents.fppi b/Svc/FpySequencer/FpySequencerEvents.fppi index 683021ab628..a393f25ae5b 100644 --- a/Svc/FpySequencer/FpySequencerEvents.fppi +++ b/Svc/FpySequencer/FpySequencerEvents.fppi @@ -215,6 +215,13 @@ event TooManySequenceDirectives( severity warning high \ format "A sequence specified it had {} directives but the max was {}" +event ArgSizeMismatch( + expected: Fpy.StackSizeType + actual: FwSizeType +) \ + severity warning high \ + format "Expected {} bytes of arguments based on arg_specs, but received {}" + event SequencePaused( stmtIdx: U32 ) \ diff --git a/Svc/FpySequencer/FpySequencerStateMachine.cpp b/Svc/FpySequencer/FpySequencerStateMachine.cpp index 2a25e68587c..ecafac99d45 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.cpp +++ b/Svc/FpySequencer/FpySequencerStateMachine.cpp @@ -305,6 +305,7 @@ void FpySequencer ::Svc_FpySequencer_SequencerStateMachine_action_clearSequenceA Svc_FpySequencer_SequencerStateMachine::Signal signal ) { this->m_sequenceArgs = {0, 0}; + this->m_expectedArgSize = 0; } //! Implementation for action clearBreakpoint of state machine Svc_FpySequencer_SequencerStateMachine diff --git a/Svc/FpySequencer/FpySequencerTypes.fpp b/Svc/FpySequencer/FpySequencerTypes.fpp index 69d0fb42db5..a6b42fa2e4e 100644 --- a/Svc/FpySequencer/FpySequencerTypes.fpp +++ b/Svc/FpySequencer/FpySequencerTypes.fpp @@ -1,7 +1,7 @@ module Svc { module Fpy { @ the current schema version (must be representable in U8) - constant SCHEMA_VERSION = 5; + constant SCHEMA_VERSION = 6; @ the type which everything referencing a size or offset on the stack is represented in # we use a U32 because U16 is too small (would only allow up to 65 kB max stack size) @@ -130,6 +130,27 @@ module Svc { CMD_FAIL = 17 } + @ Maximum length for argument or type names in arg_specs + @ Fpy serializes these as U8 length prefix, so max is 255 + constant MAX_ARG_SPEC_NAME_LEN = 255 + + @ Argument specification describing an input argument's name, type, and stack size + @ NOTE: This struct does NOT use FPP's auto-generated serialization! + @ Serialization is handled manually in C++ to match Fpy's format: + @ [U8 argNameLen][argNameLen bytes][U8 typeNameLen][typeNameLen bytes][U32 size] + struct ArgSpec { + @ Length of the argument name (0-255) + argNameLen: U8 + @ Argument name as UTF-8 bytes (not null-terminated) + argName: [MAX_ARG_SPEC_NAME_LEN] U8 + @ Length of the type name (0-255) + typeNameLen: U8 + @ Type name as UTF-8 bytes (not null-terminated) + typeName: [MAX_ARG_SPEC_NAME_LEN] U8 + @ Size of this argument on the stack in bytes + size: StackSizeType + } default { argNameLen = 0, argName = [0], typeNameLen = 0, typeName = [0], size = 0 } + struct Header { @ the major version of the FSW majorVersion: U8 @@ -149,6 +170,12 @@ module Svc { @ the size of the body in bytes bodySize: U32 + + @ Argument specifications (schema version 6+) + @ Only the first argumentCount entries are valid + @ NOTE: These are NOT included in SERIALIZED_SIZE as they are variable-length + @ and must be deserialized manually in C++ + argSpecs: [MAX_SEQUENCE_ARG_COUNT] ArgSpec } default { majorVersion = 0, minorVersion = 0, patchVersion = 0, schemaVersion = 0, argumentCount = 0, statementCount = 0, bodySize = 0 } struct Footer { diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index 4b40f463c4e..3716a0446c1 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -61,6 +61,17 @@ Fw::Success FpySequencer::validate() { return Fw::Success::FAILURE; } + // Read and parse arg_specs (schema version 6+) + if (this->m_sequenceObj.get_header().get_schemaVersion() >= 6) { + readStatus = this->readArgSpecs(sequenceFile); + if (readStatus != Fw::Success::SUCCESS) { + return Fw::Success::FAILURE; + } + } else { + // For older schema versions, no arg_specs validation needed + this->m_expectedArgSize = 0; + } + readStatus = readBytes(sequenceFile, this->m_sequenceObj.get_header().get_bodySize(), FpySequencer_FileReadStage::BODY); @@ -105,6 +116,14 @@ Fw::Success FpySequencer::validate() { return Fw::Success::FAILURE; } + // Validate argument size matches expected size from arg_specs (schema version 6+) + if (this->m_sequenceObj.get_header().get_schemaVersion() >= 6 && this->m_expectedArgSize > 0) { + if (this->m_sequenceArgs.get_size() != this->m_expectedArgSize) { + this->log_WARNING_HI_ArgSizeMismatch(this->m_expectedArgSize, this->m_sequenceArgs.get_size()); + return Fw::Success::FAILURE; + } + } + return Fw::Success::SUCCESS; } @@ -141,6 +160,113 @@ Fw::Success FpySequencer::readHeader() { return Fw::Success::SUCCESS; } +// reads and parses arg_specs from the sequence file (schema version 6+) +// stores them in the Header struct and calculates the total expected argument size +// return SUCCESS if successful, FAILURE otherwise +Fw::Success FpySequencer::readArgSpecs(Os::File& file) { + FW_ASSERT(file.isOpen()); + + const U8 argumentCount = this->m_sequenceObj.get_header().get_argumentCount(); + + // If no arguments, no arg_specs to read + if (argumentCount == 0) { + this->m_expectedArgSize = 0; + return Fw::Success::SUCCESS; + } + + Fpy::StackSizeType totalExpectedSize = 0; + + for (U8 i = 0; i < argumentCount; i++) { + // Get reference to the ArgSpec we're populating + Fpy::ArgSpec& argSpec = this->m_sequenceObj.get_header().get_argSpecs()[i]; + + // Read arg_name length + Fw::Success readStatus = this->readBytes(file, 1, FpySequencer_FileReadStage::BODY); + if (readStatus != Fw::Success::SUCCESS) { + return Fw::Success::FAILURE; + } + U8 argNameLen = 0; + Fw::SerializeStatus deserStatus = this->m_sequenceBuffer.deserializeTo(argNameLen); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); + return Fw::Success::FAILURE; + } + argSpec.set_argNameLen(argNameLen); + + // Read arg_name string and store it + if (argNameLen > 0) { + readStatus = this->readBytes(file, argNameLen, FpySequencer_FileReadStage::BODY); + if (readStatus != Fw::Success::SUCCESS) { + return Fw::Success::FAILURE; + } + // Store the arg_name bytes in the ArgSpec + for (U8 j = 0; j < argNameLen; j++) { + U8 byte; + deserStatus = this->m_sequenceBuffer.deserializeTo(byte); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + return Fw::Success::FAILURE; + } + argSpec.get_argName()[j] = byte; + } + } + + // Read type_name length + readStatus = this->readBytes(file, 1, FpySequencer_FileReadStage::BODY); + if (readStatus != Fw::Success::SUCCESS) { + return Fw::Success::FAILURE; + } + U8 typeNameLen = 0; + deserStatus = this->m_sequenceBuffer.deserializeTo(typeNameLen); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); + return Fw::Success::FAILURE; + } + argSpec.set_typeNameLen(typeNameLen); + + // Read type_name string and store it + if (typeNameLen > 0) { + readStatus = this->readBytes(file, typeNameLen, FpySequencer_FileReadStage::BODY); + if (readStatus != Fw::Success::SUCCESS) { + return Fw::Success::FAILURE; + } + // Store the type_name bytes in the ArgSpec + for (U8 j = 0; j < typeNameLen; j++) { + U8 byte; + deserStatus = this->m_sequenceBuffer.deserializeTo(byte); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + return Fw::Success::FAILURE; + } + argSpec.get_typeName()[j] = byte; + } + } + + // Read size field (StackSizeType = U32, 4 bytes, big-endian) + readStatus = this->readBytes(file, sizeof(Fpy::StackSizeType), FpySequencer_FileReadStage::BODY); + if (readStatus != Fw::Success::SUCCESS) { + return Fw::Success::FAILURE; + } + Fpy::StackSizeType argSize = 0; + deserStatus = this->m_sequenceBuffer.deserializeTo(argSize); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); + return Fw::Success::FAILURE; + } + argSpec.set_size(argSize); + + // Accumulate the expected argument size + totalExpectedSize += argSize; + } + + this->m_expectedArgSize = totalExpectedSize; + return Fw::Success::SUCCESS; +} + // reads and validates the body from the m_sequenceBuffer // return SUCCESS if sequence is valid, FAILURE otherwise Fw::Success FpySequencer::readBody() { From ce1a60a8b77fe8e253cf3911812f9dbe0b5c4c6a Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Thu, 16 Apr 2026 19:29:52 -0500 Subject: [PATCH 09/18] Validation fixes --- Svc/FpySequencer/FpySequencerEvents.fppi | 32 +++++++++++++++++++ .../FpySequencerValidationState.cpp | 25 ++++++++++++++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/Svc/FpySequencer/FpySequencerEvents.fppi b/Svc/FpySequencer/FpySequencerEvents.fppi index a393f25ae5b..1d640806be8 100644 --- a/Svc/FpySequencer/FpySequencerEvents.fppi +++ b/Svc/FpySequencer/FpySequencerEvents.fppi @@ -222,6 +222,38 @@ event ArgSizeMismatch( severity warning high \ format "Expected {} bytes of arguments based on arg_specs, but received {}" +event ArgSpecTotalSizeExceedsStackLimit( + currentTotal: Fpy.StackSizeType + argSize: Fpy.StackSizeType + maxStackSize: Fpy.StackSizeType +) \ + severity warning high \ + format "Arg spec total size {} + {} would exceed max stack size {}" + +event ArgSpecSizeExceedsStackLimit( + argIndex: U8 + argSize: Fpy.StackSizeType + maxStackSize: Fpy.StackSizeType +) \ + severity warning high \ + format "Arg spec[{}] size {} exceeds max stack size {}" + +event ArgSpecNameTooLong( + argIndex: U8 + nameLen: U8 + maxLen: U8 +) \ + severity warning high \ + format "Arg spec[{}] name length {} exceeds maximum {}" + +event ArgSpecTypeTooLong( + argIndex: U8 + typeLen: U8 + maxLen: U8 +) \ + severity warning high \ + format "Arg spec[{}] type length {} exceeds maximum {}" + event SequencePaused( stmtIdx: U32 ) \ diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index 3716a0446c1..13fc8a43eb7 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -117,7 +117,7 @@ Fw::Success FpySequencer::validate() { } // Validate argument size matches expected size from arg_specs (schema version 6+) - if (this->m_sequenceObj.get_header().get_schemaVersion() >= 6 && this->m_expectedArgSize > 0) { + if (this->m_sequenceObj.get_header().get_schemaVersion() >= 6) { if (this->m_sequenceArgs.get_size() != this->m_expectedArgSize) { this->log_WARNING_HI_ArgSizeMismatch(this->m_expectedArgSize, this->m_sequenceArgs.get_size()); return Fw::Success::FAILURE; @@ -193,6 +193,10 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } + if (argNameLen > Fpy::MAX_ARG_SPEC_NAME_LEN) { + this->log_WARNING_HI_ArgSpecNameTooLong(i, argNameLen, Fpy::MAX_ARG_SPEC_NAME_LEN); + return Fw::Success::FAILURE; + } argSpec.set_argNameLen(argNameLen); // Read arg_name string and store it @@ -206,6 +210,9 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { U8 byte; deserStatus = this->m_sequenceBuffer.deserializeTo(byte); if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } argSpec.get_argName()[j] = byte; @@ -225,6 +232,10 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } + if (typeNameLen > Fpy::MAX_ARG_SPEC_NAME_LEN) { + this->log_WARNING_HI_ArgSpecTypeTooLong(i, typeNameLen, Fpy::MAX_ARG_SPEC_NAME_LEN); + return Fw::Success::FAILURE; + } argSpec.set_typeNameLen(typeNameLen); // Read type_name string and store it @@ -238,6 +249,9 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { U8 byte; deserStatus = this->m_sequenceBuffer.deserializeTo(byte); if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } argSpec.get_typeName()[j] = byte; @@ -257,8 +271,17 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } + if (argSize > Fpy::MAX_STACK_SIZE) { + this->log_WARNING_HI_ArgSpecSizeExceedsStackLimit(i, argSize, Fpy::MAX_STACK_SIZE); + return Fw::Success::FAILURE; + } argSpec.set_size(argSize); + // Check for overflow before accumulation + if (totalExpectedSize > Fpy::MAX_STACK_SIZE - argSize) { + this->log_WARNING_HI_ArgSpecTotalSizeExceedsStackLimit(totalExpectedSize, argSize, Fpy::MAX_STACK_SIZE); + return Fw::Success::FAILURE; + } // Accumulate the expected argument size totalExpectedSize += argSize; } From b24a6f79ea060f74374fb96cd418f270bc2267db Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Fri, 17 Apr 2026 18:17:23 -0500 Subject: [PATCH 10/18] Fixes validation --- Svc/FpySequencer/FpySequencer.hpp | 2 +- Svc/FpySequencer/FpySequencerEvents.fppi | 33 ++----- Svc/FpySequencer/FpySequencerTypes.fpp | 13 +-- .../FpySequencerValidationState.cpp | 89 ++++++------------- .../test/ut/FpySequencerTestMain.cpp | 32 +++---- .../test/ut/FpySequencerTester.cpp | 56 ++++++++++-- .../test/ut/FpySequencerTester.hpp | 1 + 7 files changed, 95 insertions(+), 131 deletions(-) diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index b3496aad124..26934178826 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -645,7 +645,7 @@ class FpySequencer : public FpySequencerComponentBase { // sequence arguments to push to stack when entering RUNNING state Svc::SeqArgs m_sequenceArgs{}; - // the expected argument size from arg_specs (schema version 6+) + // the expected argument size from arg_specs Fpy::StackSizeType m_expectedArgSize{0}; // the goal state is the state that we're trying to reach in the sequencer diff --git a/Svc/FpySequencer/FpySequencerEvents.fppi b/Svc/FpySequencer/FpySequencerEvents.fppi index 1d640806be8..3a571d915f4 100644 --- a/Svc/FpySequencer/FpySequencerEvents.fppi +++ b/Svc/FpySequencer/FpySequencerEvents.fppi @@ -218,41 +218,18 @@ event TooManySequenceDirectives( event ArgSizeMismatch( expected: Fpy.StackSizeType actual: FwSizeType + filePath: string ) \ severity warning high \ - format "Expected {} bytes of arguments based on arg_specs, but received {}" - -event ArgSpecTotalSizeExceedsStackLimit( - currentTotal: Fpy.StackSizeType - argSize: Fpy.StackSizeType - maxStackSize: Fpy.StackSizeType -) \ - severity warning high \ - format "Arg spec total size {} + {} would exceed max stack size {}" + format "Expected {} bytes of arguments, but received {} in sequence file {}" -event ArgSpecSizeExceedsStackLimit( - argIndex: U8 +event ArgTotalSizeExceedsStackLimit( argSize: Fpy.StackSizeType maxStackSize: Fpy.StackSizeType + filePath: string ) \ severity warning high \ - format "Arg spec[{}] size {} exceeds max stack size {}" - -event ArgSpecNameTooLong( - argIndex: U8 - nameLen: U8 - maxLen: U8 -) \ - severity warning high \ - format "Arg spec[{}] name length {} exceeds maximum {}" - -event ArgSpecTypeTooLong( - argIndex: U8 - typeLen: U8 - maxLen: U8 -) \ - severity warning high \ - format "Arg spec[{}] type length {} exceeds maximum {}" + format "Adding argument of size {} would exceed max stack size {} in sequence file {}" event SequencePaused( stmtIdx: U32 diff --git a/Svc/FpySequencer/FpySequencerTypes.fpp b/Svc/FpySequencer/FpySequencerTypes.fpp index a6b42fa2e4e..530b0a370cf 100644 --- a/Svc/FpySequencer/FpySequencerTypes.fpp +++ b/Svc/FpySequencer/FpySequencerTypes.fpp @@ -137,7 +137,6 @@ module Svc { @ Argument specification describing an input argument's name, type, and stack size @ NOTE: This struct does NOT use FPP's auto-generated serialization! @ Serialization is handled manually in C++ to match Fpy's format: - @ [U8 argNameLen][argNameLen bytes][U8 typeNameLen][typeNameLen bytes][U32 size] struct ArgSpec { @ Length of the argument name (0-255) argNameLen: U8 @@ -148,8 +147,8 @@ module Svc { @ Type name as UTF-8 bytes (not null-terminated) typeName: [MAX_ARG_SPEC_NAME_LEN] U8 @ Size of this argument on the stack in bytes - size: StackSizeType - } default { argNameLen = 0, argName = [0], typeNameLen = 0, typeName = [0], size = 0 } + argSize: StackSizeType + } default { argNameLen = 0, argName = 0, typeNameLen = 0, typeName = 0, argSize = 0 } struct Header { @ the major version of the FSW @@ -170,12 +169,6 @@ module Svc { @ the size of the body in bytes bodySize: U32 - - @ Argument specifications (schema version 6+) - @ Only the first argumentCount entries are valid - @ NOTE: These are NOT included in SERIALIZED_SIZE as they are variable-length - @ and must be deserialized manually in C++ - argSpecs: [MAX_SEQUENCE_ARG_COUNT] ArgSpec } default { majorVersion = 0, minorVersion = 0, patchVersion = 0, schemaVersion = 0, argumentCount = 0, statementCount = 0, bodySize = 0 } struct Footer { @@ -192,7 +185,7 @@ module Svc { header: Header @ an array of size m_header.argumentCount mapping argument position to local @ variable index - args: [MAX_SEQUENCE_ARG_COUNT] U8 + args: [MAX_SEQUENCE_ARG_COUNT] ArgSpec statements: [MAX_SEQUENCE_STATEMENT_COUNT] Statement footer: Footer } diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index 13fc8a43eb7..5d45e0133d5 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -61,15 +61,10 @@ Fw::Success FpySequencer::validate() { return Fw::Success::FAILURE; } - // Read and parse arg_specs (schema version 6+) - if (this->m_sequenceObj.get_header().get_schemaVersion() >= 6) { - readStatus = this->readArgSpecs(sequenceFile); - if (readStatus != Fw::Success::SUCCESS) { - return Fw::Success::FAILURE; - } - } else { - // For older schema versions, no arg_specs validation needed - this->m_expectedArgSize = 0; + // Read and parse arg_specs + readStatus = this->readArgSpecs(sequenceFile); + if (readStatus != Fw::Success::SUCCESS) { + return Fw::Success::FAILURE; } readStatus = @@ -116,12 +111,10 @@ Fw::Success FpySequencer::validate() { return Fw::Success::FAILURE; } - // Validate argument size matches expected size from arg_specs (schema version 6+) - if (this->m_sequenceObj.get_header().get_schemaVersion() >= 6) { - if (this->m_sequenceArgs.get_size() != this->m_expectedArgSize) { - this->log_WARNING_HI_ArgSizeMismatch(this->m_expectedArgSize, this->m_sequenceArgs.get_size()); - return Fw::Success::FAILURE; - } + if (this->m_sequenceArgs.get_size() != this->m_expectedArgSize) { + this->log_WARNING_HI_ArgSizeMismatch(this->m_expectedArgSize, this->m_sequenceArgs.get_size(), + this->m_sequenceFilePath); + return Fw::Success::FAILURE; } return Fw::Success::SUCCESS; @@ -160,46 +153,36 @@ Fw::Success FpySequencer::readHeader() { return Fw::Success::SUCCESS; } -// reads and parses arg_specs from the sequence file (schema version 6+) -// stores them in the Header struct and calculates the total expected argument size +// reads and validates arg_specs from the m_sequenceBuffer +// stores them in the Sequence.args array and calculates the total expected argument size // return SUCCESS if successful, FAILURE otherwise Fw::Success FpySequencer::readArgSpecs(Os::File& file) { FW_ASSERT(file.isOpen()); const U8 argumentCount = this->m_sequenceObj.get_header().get_argumentCount(); - - // If no arguments, no arg_specs to read - if (argumentCount == 0) { - this->m_expectedArgSize = 0; - return Fw::Success::SUCCESS; - } - Fpy::StackSizeType totalExpectedSize = 0; + Fw::SerializeStatus deserStatus; + Fw::Success readStatus; + // Read and deserialize each arg_spec incrementally since they're variable-length for (U8 i = 0; i < argumentCount; i++) { - // Get reference to the ArgSpec we're populating - Fpy::ArgSpec& argSpec = this->m_sequenceObj.get_header().get_argSpecs()[i]; - - // Read arg_name length - Fw::Success readStatus = this->readBytes(file, 1, FpySequencer_FileReadStage::BODY); + Fpy::ArgSpec& argSpec = this->m_sequenceObj.get_args()[i]; + // Read and deserialize arg_name length + readStatus = this->readBytes(file, sizeof(U8), FpySequencer_FileReadStage::BODY); if (readStatus != Fw::Success::SUCCESS) { return Fw::Success::FAILURE; } U8 argNameLen = 0; - Fw::SerializeStatus deserStatus = this->m_sequenceBuffer.deserializeTo(argNameLen); + deserStatus = this->m_sequenceBuffer.deserializeTo(argNameLen); if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { this->log_WARNING_HI_FileReadDeserializeError( FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } - if (argNameLen > Fpy::MAX_ARG_SPEC_NAME_LEN) { - this->log_WARNING_HI_ArgSpecNameTooLong(i, argNameLen, Fpy::MAX_ARG_SPEC_NAME_LEN); - return Fw::Success::FAILURE; - } argSpec.set_argNameLen(argNameLen); - // Read arg_name string and store it + // Read and deserialize arg_name string if (argNameLen > 0) { readStatus = this->readBytes(file, argNameLen, FpySequencer_FileReadStage::BODY); if (readStatus != Fw::Success::SUCCESS) { @@ -219,8 +202,8 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { } } - // Read type_name length - readStatus = this->readBytes(file, 1, FpySequencer_FileReadStage::BODY); + // Read and deserialize type_name length + readStatus = this->readBytes(file, sizeof(U8), FpySequencer_FileReadStage::BODY); if (readStatus != Fw::Success::SUCCESS) { return Fw::Success::FAILURE; } @@ -232,13 +215,9 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } - if (typeNameLen > Fpy::MAX_ARG_SPEC_NAME_LEN) { - this->log_WARNING_HI_ArgSpecTypeTooLong(i, typeNameLen, Fpy::MAX_ARG_SPEC_NAME_LEN); - return Fw::Success::FAILURE; - } argSpec.set_typeNameLen(typeNameLen); - // Read type_name string and store it + // Read and deserialize type_name string if (typeNameLen > 0) { readStatus = this->readBytes(file, typeNameLen, FpySequencer_FileReadStage::BODY); if (readStatus != Fw::Success::SUCCESS) { @@ -258,7 +237,7 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { } } - // Read size field (StackSizeType = U32, 4 bytes, big-endian) + // Read and deserialize size field readStatus = this->readBytes(file, sizeof(Fpy::StackSizeType), FpySequencer_FileReadStage::BODY); if (readStatus != Fw::Success::SUCCESS) { return Fw::Success::FAILURE; @@ -271,18 +250,14 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } - if (argSize > Fpy::MAX_STACK_SIZE) { - this->log_WARNING_HI_ArgSpecSizeExceedsStackLimit(i, argSize, Fpy::MAX_STACK_SIZE); - return Fw::Success::FAILURE; - } - argSpec.set_size(argSize); // Check for overflow before accumulation if (totalExpectedSize > Fpy::MAX_STACK_SIZE - argSize) { - this->log_WARNING_HI_ArgSpecTotalSizeExceedsStackLimit(totalExpectedSize, argSize, Fpy::MAX_STACK_SIZE); + this->log_WARNING_HI_ArgTotalSizeExceedsStackLimit(argSize, Fpy::MAX_STACK_SIZE, + this->m_sequenceFilePath); return Fw::Success::FAILURE; } - // Accumulate the expected argument size + argSpec.set_argSize(argSize); totalExpectedSize += argSize; } @@ -294,20 +269,6 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { // return SUCCESS if sequence is valid, FAILURE otherwise Fw::Success FpySequencer::readBody() { Fw::SerializeStatus deserStatus; - // deser body: - // deser arg mappings - for (U8 argMappingIdx = 0; argMappingIdx < this->m_sequenceObj.get_header().get_argumentCount(); argMappingIdx++) { - // serializable register index of arg $argMappingIdx - // TODO should probably check that this serReg is inside range - deserStatus = this->m_sequenceBuffer.deserializeTo(this->m_sequenceObj.get_args()[argMappingIdx]); - if (deserStatus != Fw::FW_SERIALIZE_OK) { - this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), - this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); - return Fw::Success::FAILURE; - } - } - // deser statements for (U16 statementIdx = 0; statementIdx < this->m_sequenceObj.get_header().get_statementCount(); statementIdx++) { // deser statement diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index 8374caea361..1b3d2365b26 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -2119,6 +2119,8 @@ TEST_F(FpySequencerTester, cmd_RUN) { TEST_F(FpySequencerTester, cmd_RUN_ARGS) { allocMem(); + add_ARG_SPEC("arg1", "U32", 4); + add_ARG_SPEC("arg2", "U32", 4); add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args @@ -2247,6 +2249,8 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED) { TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS) { allocMem(); + add_ARG_SPEC("arg1", "U32", 4); + add_ARG_SPEC("arg2", "U32", 4); add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args @@ -2594,43 +2598,25 @@ TEST_F(FpySequencerTester, readHeader) { } TEST_F(FpySequencerTester, readBody) { - U8 data[Fpy::MAX_SEQUENCE_ARG_COUNT + Fpy::MAX_SEQUENCE_STATEMENT_COUNT * Fpy::Statement::SERIALIZED_SIZE]; + U8 data[Fpy::MAX_SEQUENCE_STATEMENT_COUNT * Fpy::Statement::SERIALIZED_SIZE]; tester_get_m_sequenceBuffer_ptr()->setExtBuffer(data, sizeof(data)); - // write some args mappings - for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_ARG_COUNT; ii++) { - // map arg idx ii to serReg pos 123 - ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(static_cast(123)), - Fw::SerializeStatus::FW_SERIALIZE_OK); - } - // write some statements + // write some statements (no arg mappings in body anymore, arg_specs are separate) Fpy::Statement stmt(Fpy::DirectiveId::NO_OP, Fw::StatementArgBuffer()); for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_STATEMENT_COUNT; ii++) { ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(stmt), Fw::SerializeStatus::FW_SERIALIZE_OK); } - tester_get_m_sequenceObj_ptr()->get_header().set_argumentCount(Fpy::MAX_SEQUENCE_ARG_COUNT); tester_get_m_sequenceObj_ptr()->get_header().set_statementCount(Fpy::MAX_SEQUENCE_STATEMENT_COUNT); ASSERT_EQ(tester_readBody(), Fw::Success::SUCCESS); - for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_ARG_COUNT; ii++) { - ASSERT_EQ(tester_get_m_sequenceObj_ptr()->get_args()[ii], 123); - } - for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_STATEMENT_COUNT; ii++) { ASSERT_EQ(tester_get_m_sequenceObj_ptr()->get_statements()[ii], stmt); } tester_get_m_sequenceBuffer_ptr()->resetSer(); - tester_get_m_sequenceObj_ptr()->get_header().set_statementCount(0); - // now see what happens if we don't write enough args - for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_ARG_COUNT - 1; ii++) { - // map arg idx ii to serReg pos 123 - ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(static_cast(123)), - Fw::SerializeStatus::FW_SERIALIZE_OK); - } - // don't write any stmts otherwise their bytes will be interpreted as arg mappings and it will trigger - // the wrong branch + tester_get_m_sequenceObj_ptr()->get_header().set_statementCount(1); + // don't write any statements - should fail ASSERT_EQ(tester_readBody(), Fw::Success::FAILURE); // now see what happens if we don't write enough stmts @@ -3342,6 +3328,8 @@ TEST_F(FpySequencerTester, seqRunIn) { TEST_F(FpySequencerTester, seqRunInArgs) { allocMem(); + add_ARG_SPEC("arg1", "U32", 4); + add_ARG_SPEC("arg2", "U32", 4); add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.cpp b/Svc/FpySequencer/test/ut/FpySequencerTester.cpp index 46ebeeb2d23..72cbc514fdc 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.cpp @@ -63,21 +63,36 @@ void FpySequencerTester::writeToFile(const char* name, FwSizeType maxBytes) { Fw::ExternalSerializeBuffer buf; buf.setExtBuffer(data, sizeof(data)); - // first let's calculate the size of the body. do this by just writing the body, - // then calculating how big that was, then clearing and writing the header, then writing the body again - for (U32 ii = 0; ii < seq.get_header().get_argumentCount(); ii++) { - ASSERT_EQ(buf.serializeFrom(seq.get_args()[ii]), Fw::SerializeStatus::FW_SERIALIZE_OK); - } + // Calculate body size (statements only, arg_specs go in header section) for (U32 ii = 0; ii < seq.get_header().get_statementCount(); ii++) { ASSERT_EQ(buf.serializeFrom(seq.get_statements()[ii]), Fw::SerializeStatus::FW_SERIALIZE_OK); } seq.get_header().set_bodySize(static_cast(buf.getSize())); buf.resetSer(); + // Write header using FPP auto-generated serialization ASSERT_EQ(buf.serializeFrom(seq.get_header()), Fw::SerializeStatus::FW_SERIALIZE_OK); + + // Write arg_specs in Fpy variable-length format for (U32 ii = 0; ii < seq.get_header().get_argumentCount(); ii++) { - ASSERT_EQ(buf.serializeFrom(seq.get_args()[ii]), Fw::SerializeStatus::FW_SERIALIZE_OK); + const Fpy::ArgSpec& argSpec = seq.get_args()[ii]; + // Write arg_name length and bytes (without F Prime length prefix) + ASSERT_EQ(buf.serializeFrom(argSpec.get_argNameLen()), Fw::SerializeStatus::FW_SERIALIZE_OK); + if (argSpec.get_argNameLen() > 0) { + ASSERT_EQ(buf.serializeFrom(argSpec.get_argName(), argSpec.get_argNameLen(), Fw::Serialization::OMIT_LENGTH), + Fw::SerializeStatus::FW_SERIALIZE_OK); + } + // Write type_name length and bytes (without F Prime length prefix) + ASSERT_EQ(buf.serializeFrom(argSpec.get_typeNameLen()), Fw::SerializeStatus::FW_SERIALIZE_OK); + if (argSpec.get_typeNameLen() > 0) { + ASSERT_EQ(buf.serializeFrom(argSpec.get_typeName(), argSpec.get_typeNameLen(), Fw::Serialization::OMIT_LENGTH), + Fw::SerializeStatus::FW_SERIALIZE_OK); + } + // Write size + ASSERT_EQ(buf.serializeFrom(argSpec.get_argSize()), Fw::SerializeStatus::FW_SERIALIZE_OK); } + + // Write statements for (U32 ii = 0; ii < seq.get_header().get_statementCount(); ii++) { ASSERT_EQ(buf.serializeFrom(seq.get_statements()[ii]), Fw::SerializeStatus::FW_SERIALIZE_OK); } @@ -104,6 +119,35 @@ void FpySequencerTester::removeFile(const char* name) { Os::FileSystem::removeFile(name); } +void FpySequencerTester::add_ARG_SPEC(const char* argName, const char* typeName, Fpy::StackSizeType argSize) { + U8 argCount = seq.get_header().get_argumentCount(); + FW_ASSERT(argCount < Fpy::MAX_SEQUENCE_ARG_COUNT); + + Fpy::ArgSpec& argSpec = seq.get_args()[argCount]; + + // Set arg_name + U8 argNameLen = static_cast(strlen(argName)); + FW_ASSERT(argNameLen <= Fpy::MAX_ARG_SPEC_NAME_LEN); + argSpec.set_argNameLen(argNameLen); + for (U8 i = 0; i < argNameLen; i++) { + argSpec.get_argName()[i] = static_cast(argName[i]); + } + + // Set type_name + U8 typeNameLen = static_cast(strlen(typeName)); + FW_ASSERT(typeNameLen <= Fpy::MAX_ARG_SPEC_NAME_LEN); + argSpec.set_typeNameLen(typeNameLen); + for (U8 i = 0; i < typeNameLen; i++) { + argSpec.get_typeName()[i] = static_cast(typeName[i]); + } + + // Set size + argSpec.set_argSize(argSize); + + // Increment argument count + seq.get_header().set_argumentCount(argCount + 1); +} + void FpySequencerTester::resetRuntime() { // explicitly call dtor cmp.m_runtime.~Runtime(); diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp index ab3c76f7a0a..141d2cace68 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp @@ -79,6 +79,7 @@ class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test void writeToFile(const char* name, FwSizeType maxBytes = Fpy::Sequence::SERIALIZED_SIZE); void removeFile(const char* name); void addDirective(Fpy::DirectiveId id, Fw::StatementArgBuffer& buf); + void add_ARG_SPEC(const char* argName, const char* typeName, Fpy::StackSizeType argSize); void add_WAIT_REL(); void add_WAIT_REL(FpySequencer_WaitRelDirective dir); From 8bc7240cfae734f821e6bfde61ca653953e5caf7 Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Fri, 17 Apr 2026 18:27:26 -0500 Subject: [PATCH 11/18] Remove redundant if --- .../FpySequencerValidationState.cpp | 56 +++++++++---------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index 5d45e0133d5..6e0e32266e4 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -183,23 +183,21 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { argSpec.set_argNameLen(argNameLen); // Read and deserialize arg_name string - if (argNameLen > 0) { - readStatus = this->readBytes(file, argNameLen, FpySequencer_FileReadStage::BODY); - if (readStatus != Fw::Success::SUCCESS) { + readStatus = this->readBytes(file, argNameLen, FpySequencer_FileReadStage::BODY); + if (readStatus != Fw::Success::SUCCESS) { + return Fw::Success::FAILURE; + } + // Store the arg_name bytes in the ArgSpec + for (U8 j = 0; j < argNameLen; j++) { + U8 byte; + deserStatus = this->m_sequenceBuffer.deserializeTo(byte); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } - // Store the arg_name bytes in the ArgSpec - for (U8 j = 0; j < argNameLen; j++) { - U8 byte; - deserStatus = this->m_sequenceBuffer.deserializeTo(byte); - if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { - this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), - this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); - return Fw::Success::FAILURE; - } - argSpec.get_argName()[j] = byte; - } + argSpec.get_argName()[j] = byte; } // Read and deserialize type_name length @@ -218,23 +216,21 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { argSpec.set_typeNameLen(typeNameLen); // Read and deserialize type_name string - if (typeNameLen > 0) { - readStatus = this->readBytes(file, typeNameLen, FpySequencer_FileReadStage::BODY); - if (readStatus != Fw::Success::SUCCESS) { + readStatus = this->readBytes(file, typeNameLen, FpySequencer_FileReadStage::BODY); + if (readStatus != Fw::Success::SUCCESS) { + return Fw::Success::FAILURE; + } + // Store the type_name bytes in the ArgSpec + for (U8 j = 0; j < typeNameLen; j++) { + U8 byte; + deserStatus = this->m_sequenceBuffer.deserializeTo(byte); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); return Fw::Success::FAILURE; } - // Store the type_name bytes in the ArgSpec - for (U8 j = 0; j < typeNameLen; j++) { - U8 byte; - deserStatus = this->m_sequenceBuffer.deserializeTo(byte); - if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { - this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), - this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); - return Fw::Success::FAILURE; - } - argSpec.get_typeName()[j] = byte; - } + argSpec.get_typeName()[j] = byte; } // Read and deserialize size field From d89d0c8ad54efadeba36ad484c4ac7762f00d92b Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Fri, 17 Apr 2026 18:43:47 -0500 Subject: [PATCH 12/18] Direct deser --- .../FpySequencerValidationState.cpp | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index 6e0e32266e4..db189334f04 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -187,17 +187,13 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { if (readStatus != Fw::Success::SUCCESS) { return Fw::Success::FAILURE; } - // Store the arg_name bytes in the ArgSpec - for (U8 j = 0; j < argNameLen; j++) { - U8 byte; - deserStatus = this->m_sequenceBuffer.deserializeTo(byte); - if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { - this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), - this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); - return Fw::Success::FAILURE; - } - argSpec.get_argName()[j] = byte; + FwSizeType argNameSize = static_cast(argNameLen); + deserStatus = this->m_sequenceBuffer.deserializeTo(argSpec.get_argName(), argNameSize, Fw::Serialization::OMIT_LENGTH); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); + return Fw::Success::FAILURE; } // Read and deserialize type_name length @@ -220,17 +216,13 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { if (readStatus != Fw::Success::SUCCESS) { return Fw::Success::FAILURE; } - // Store the type_name bytes in the ArgSpec - for (U8 j = 0; j < typeNameLen; j++) { - U8 byte; - deserStatus = this->m_sequenceBuffer.deserializeTo(byte); - if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { - this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), - this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); - return Fw::Success::FAILURE; - } - argSpec.get_typeName()[j] = byte; + FwSizeType typeNameSize = static_cast(typeNameLen); + deserStatus = this->m_sequenceBuffer.deserializeTo(argSpec.get_typeName(), typeNameSize, Fw::Serialization::OMIT_LENGTH); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); + return Fw::Success::FAILURE; } // Read and deserialize size field From 2fb89bdb58fe91a35f76869f923c71eed082a4f3 Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Fri, 17 Apr 2026 19:00:15 -0500 Subject: [PATCH 13/18] Fix ut --- Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index 1b3d2365b26..bfb98f27f30 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -2119,8 +2119,8 @@ TEST_F(FpySequencerTester, cmd_RUN) { TEST_F(FpySequencerTester, cmd_RUN_ARGS) { allocMem(); - add_ARG_SPEC("arg1", "U32", 4); - add_ARG_SPEC("arg2", "U32", 4); + add_ARG_SPEC("arg1", "U32", sizeof(U32)); + add_ARG_SPEC("arg2", "U32", sizeof(U32)); add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args @@ -2249,8 +2249,8 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED) { TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS) { allocMem(); - add_ARG_SPEC("arg1", "U32", 4); - add_ARG_SPEC("arg2", "U32", 4); + add_ARG_SPEC("arg1", "U32", sizeof(U32)); + add_ARG_SPEC("arg2", "U32", sizeof(U32)); add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args @@ -3328,8 +3328,8 @@ TEST_F(FpySequencerTester, seqRunIn) { TEST_F(FpySequencerTester, seqRunInArgs) { allocMem(); - add_ARG_SPEC("arg1", "U32", 4); - add_ARG_SPEC("arg2", "U32", 4); + add_ARG_SPEC("arg1", "U32", sizeof(U32)); + add_ARG_SPEC("arg2", "U32", sizeof(U32)); add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args From 97e491bd9502e143d2b2ee4603552a80d7402159 Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Fri, 17 Apr 2026 19:21:20 -0500 Subject: [PATCH 14/18] UT fixes --- Svc/FpySequencer/FpySequencer.hpp | 3 + Svc/FpySequencer/FpySequencerEvents.fppi | 14 ++ .../FpySequencerValidationState.cpp | 102 ++++++------- .../test/ut/FpySequencerTestMain.cpp | 134 +++++++++++++++++- 4 files changed, 199 insertions(+), 54 deletions(-) diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index 26934178826..d3c575c0f81 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -752,6 +752,9 @@ class FpySequencer : public FpySequencerComponentBase { // reads and parses arg_specs from the sequence file (schema version 6+) // return SUCCESS if successful, FAILURE otherwise Fw::Success readArgSpecs(Os::File& file); + // helper function to read and deserialize a variable-length string field (length byte + string bytes) + // returns the length via outLength parameter and writes string data to buffer + Fw::Success deserializeStringField(Os::File& file, U8* buffer, U8& outLength); // reads and validates the body from the m_sequenceBuffer // return SUCCESS if sequence is valid, FAILURE otherwise Fw::Success readBody(); diff --git a/Svc/FpySequencer/FpySequencerEvents.fppi b/Svc/FpySequencer/FpySequencerEvents.fppi index 3a571d915f4..f9d6131f165 100644 --- a/Svc/FpySequencer/FpySequencerEvents.fppi +++ b/Svc/FpySequencer/FpySequencerEvents.fppi @@ -231,6 +231,20 @@ event ArgTotalSizeExceedsStackLimit( severity warning high \ format "Adding argument of size {} would exceed max stack size {} in sequence file {}" +event InvalidArgSpec( + filePath: string +) \ + severity warning high \ + format "Invalid argument specification in sequence file {}: arg_name or type_name cannot be empty" + +event ArgSpecStringLengthExceedsMax( + length: U8 + max: U8 + filePath: string +) \ + severity warning high \ + format "Argument specification string length {} exceeds maximum {} in sequence file {}" + event SequencePaused( stmtIdx: U32 ) \ diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index db189334f04..d97514b427f 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -153,6 +153,53 @@ Fw::Success FpySequencer::readHeader() { return Fw::Success::SUCCESS; } +// Helper function to read and deserialize a variable-length string field +// Reads length byte, then the string data into the provided buffer +Fw::Success FpySequencer::deserializeStringField(Os::File& file, U8* buffer, U8& outLength) { + // Read length byte + Fw::Success readStatus = this->readBytes(file, sizeof(U8), FpySequencer_FileReadStage::BODY); + if (readStatus != Fw::Success::SUCCESS) { + return Fw::Success::FAILURE; + } + + Fw::SerializeStatus deserStatus = this->m_sequenceBuffer.deserializeTo(outLength); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); + return Fw::Success::FAILURE; + } + + // Validate length is non-zero (reject empty strings) + if (outLength == 0) { + this->log_WARNING_HI_InvalidArgSpec(this->m_sequenceFilePath); + return Fw::Success::FAILURE; + } + + // Validate length doesn't exceed buffer capacity + if (outLength > Fpy::MAX_ARG_SPEC_NAME_LEN) { + this->log_WARNING_HI_ArgSpecStringLengthExceedsMax(outLength, Fpy::MAX_ARG_SPEC_NAME_LEN, this->m_sequenceFilePath); + return Fw::Success::FAILURE; + } + + // Read string bytes + readStatus = this->readBytes(file, outLength, FpySequencer_FileReadStage::BODY); + if (readStatus != Fw::Success::SUCCESS) { + return Fw::Success::FAILURE; + } + + FwSizeType actualLen = static_cast(outLength); + deserStatus = this->m_sequenceBuffer.deserializeTo(buffer, actualLen, Fw::Serialization::OMIT_LENGTH); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); + return Fw::Success::FAILURE; + } + + return Fw::Success::SUCCESS; +} + // reads and validates arg_specs from the m_sequenceBuffer // stores them in the Sequence.args array and calculates the total expected argument size // return SUCCESS if successful, FAILURE otherwise @@ -167,63 +214,22 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { // Read and deserialize each arg_spec incrementally since they're variable-length for (U8 i = 0; i < argumentCount; i++) { Fpy::ArgSpec& argSpec = this->m_sequenceObj.get_args()[i]; - // Read and deserialize arg_name length - readStatus = this->readBytes(file, sizeof(U8), FpySequencer_FileReadStage::BODY); - if (readStatus != Fw::Success::SUCCESS) { - return Fw::Success::FAILURE; - } - U8 argNameLen = 0; - deserStatus = this->m_sequenceBuffer.deserializeTo(argNameLen); - if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { - this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), - this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); - return Fw::Success::FAILURE; - } - argSpec.set_argNameLen(argNameLen); - // Read and deserialize arg_name string - readStatus = this->readBytes(file, argNameLen, FpySequencer_FileReadStage::BODY); + // Read arg_name (length + string) + U8 argNameLen = 0; + readStatus = this->deserializeStringField(file, argSpec.get_argName(), argNameLen); if (readStatus != Fw::Success::SUCCESS) { return Fw::Success::FAILURE; } - FwSizeType argNameSize = static_cast(argNameLen); - deserStatus = this->m_sequenceBuffer.deserializeTo(argSpec.get_argName(), argNameSize, Fw::Serialization::OMIT_LENGTH); - if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { - this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), - this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); - return Fw::Success::FAILURE; - } + argSpec.set_argNameLen(argNameLen); - // Read and deserialize type_name length - readStatus = this->readBytes(file, sizeof(U8), FpySequencer_FileReadStage::BODY); - if (readStatus != Fw::Success::SUCCESS) { - return Fw::Success::FAILURE; - } + // Read type_name (length + string) U8 typeNameLen = 0; - deserStatus = this->m_sequenceBuffer.deserializeTo(typeNameLen); - if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { - this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), - this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); - return Fw::Success::FAILURE; - } - argSpec.set_typeNameLen(typeNameLen); - - // Read and deserialize type_name string - readStatus = this->readBytes(file, typeNameLen, FpySequencer_FileReadStage::BODY); + readStatus = this->deserializeStringField(file, argSpec.get_typeName(), typeNameLen); if (readStatus != Fw::Success::SUCCESS) { return Fw::Success::FAILURE; } - FwSizeType typeNameSize = static_cast(typeNameLen); - deserStatus = this->m_sequenceBuffer.deserializeTo(argSpec.get_typeName(), typeNameSize, Fw::Serialization::OMIT_LENGTH); - if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { - this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), - this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); - return Fw::Success::FAILURE; - } + argSpec.set_typeNameLen(typeNameLen); // Read and deserialize size field readStatus = this->readBytes(file, sizeof(Fpy::StackSizeType), FpySequencer_FileReadStage::BODY); diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index bfb98f27f30..11165ef4542 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -2121,8 +2121,8 @@ TEST_F(FpySequencerTester, cmd_RUN_ARGS) { allocMem(); add_ARG_SPEC("arg1", "U32", sizeof(U32)); add_ARG_SPEC("arg2", "U32", sizeof(U32)); - add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack - add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack + add_LOAD_REL(0, sizeof(U32)); // Load first arg (U32 at offset 0) - duplicates it on stack + add_LOAD_REL(4, sizeof(U32)); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args writeToFile("test.bin"); @@ -2251,8 +2251,8 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS) { allocMem(); add_ARG_SPEC("arg1", "U32", sizeof(U32)); add_ARG_SPEC("arg2", "U32", sizeof(U32)); - add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack - add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack + add_LOAD_REL(0, sizeof(U32)); // Load first arg (U32 at offset 0) - duplicates it on stack + add_LOAD_REL(4, sizeof(U32)); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args writeToFile("test.bin"); @@ -2335,6 +2335,128 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_oversized) { removeFile("test.bin"); } +TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_zero_length_arg_name) { + allocMem(); + clearSeq(); + + // Create arg_spec with zero-length arg name (invalid) + add_ARG_SPEC("", "U32", sizeof(U32)); // Empty string for arg name + add_NO_OP(); + writeToFile("test.bin"); + + // Create valid args buffer + Svc::SeqArgs args{0, 0}; + Fw::ExternalSerializeBuffer argBuf(args.get_buffer(), SequenceArgumentsMaxSize); + U32 arg1Val = 42; + ASSERT_EQ(argBuf.serializeFrom(arg1Val), Fw::FW_SERIALIZE_OK); + args.set_size(argBuf.getSize()); + + sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), args); + dispatchUntilState(State::VALIDATING); + // Should fail during arg_spec deserialization due to zero-length arg name + dispatchUntilState(State::IDLE); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); + ASSERT_from_seqDoneOut_SIZE(1); + ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::EXECUTION_ERROR); + + removeFile("test.bin"); +} + +TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_zero_length_type_name) { + allocMem(); + clearSeq(); + + // Create arg_spec with zero-length type name (invalid) + add_ARG_SPEC("arg1", "", sizeof(U32)); // Empty string for type name + add_NO_OP(); + writeToFile("test.bin"); + + // Create valid args buffer + Svc::SeqArgs args{0, 0}; + Fw::ExternalSerializeBuffer argBuf(args.get_buffer(), SequenceArgumentsMaxSize); + U32 arg1Val = 42; + ASSERT_EQ(argBuf.serializeFrom(arg1Val), Fw::FW_SERIALIZE_OK); + args.set_size(argBuf.getSize()); + + sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), args); + dispatchUntilState(State::VALIDATING); + // Should fail during arg_spec deserialization due to zero-length type name + dispatchUntilState(State::IDLE); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); + ASSERT_from_seqDoneOut_SIZE(1); + ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::EXECUTION_ERROR); + + removeFile("test.bin"); +} + +TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_max_length_strings) { + allocMem(); + clearSeq(); + + // Create arg_spec with maximum length strings (255 bytes each) + char maxLengthName[256]; + memset(maxLengthName, 'A', 255); + maxLengthName[255] = '\0'; + + char maxLengthType[256]; + memset(maxLengthType, 'B', 255); + maxLengthType[255] = '\0'; + + add_ARG_SPEC(maxLengthName, maxLengthType, sizeof(U32)); + add_LOAD_REL(0, sizeof(U32)); + add_DISCARD(sizeof(U32) * 2); // Discard loaded copy + original + writeToFile("test.bin"); + + // Create valid args buffer + Svc::SeqArgs args{0, 0}; + Fw::ExternalSerializeBuffer argBuf(args.get_buffer(), SequenceArgumentsMaxSize); + U32 arg1Val = 123; + ASSERT_EQ(argBuf.serializeFrom(arg1Val), Fw::FW_SERIALIZE_OK); + args.set_size(argBuf.getSize()); + + sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), args); + dispatchUntilState(State::VALIDATING); + dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::OK); + + removeFile("test.bin"); +} + +TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_size_mismatch) { + allocMem(); + clearSeq(); + + // Create arg_spec claiming U32 (4 bytes) but provide wrong size + add_ARG_SPEC("badArg", "U32", sizeof(U64)); // Type says U32, size says U64 (8 bytes) + add_NO_OP(); + writeToFile("test.bin"); + + // Create args buffer with actual U32 (4 bytes) + Svc::SeqArgs args{0, 0}; + Fw::ExternalSerializeBuffer argBuf(args.get_buffer(), SequenceArgumentsMaxSize); + U32 arg1Val = 99; + ASSERT_EQ(argBuf.serializeFrom(arg1Val), Fw::FW_SERIALIZE_OK); + args.set_size(argBuf.getSize()); + + sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), args); + dispatchUntilState(State::VALIDATING); + // Should fail due to size mismatch: expected 8 bytes (from arg_spec), got 4 bytes (from args) + dispatchUntilState(State::IDLE); + ASSERT_CMD_RESPONSE_SIZE(1); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); + ASSERT_from_seqDoneOut_SIZE(1); + ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::EXECUTION_ERROR); + + // Verify ArgSizeMismatch event was logged + ASSERT_EVENTS_SIZE(1); + ASSERT_EVENTS_ArgSizeMismatch_SIZE(1); + + removeFile("test.bin"); +} + TEST_F(FpySequencerTester, cmd_CANCEL) { this->tester_setState(State::IDLE); sendCmd_CANCEL(0, 0); @@ -3330,8 +3452,8 @@ TEST_F(FpySequencerTester, seqRunInArgs) { allocMem(); add_ARG_SPEC("arg1", "U32", sizeof(U32)); add_ARG_SPEC("arg2", "U32", sizeof(U32)); - add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack - add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack + add_LOAD_REL(0, sizeof(U32)); // Load first arg (U32 at offset 0) - duplicates it on stack + add_LOAD_REL(4, sizeof(U32)); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args writeToFile("test.bin"); From 31282aa5dfcda14bbe60c300d319683c8e0ec5e9 Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Wed, 22 Apr 2026 15:29:16 -0700 Subject: [PATCH 15/18] Validation Fixes --- Svc/FpySequencer/FpySequencer.hpp | 6 - Svc/FpySequencer/FpySequencerStateMachine.cpp | 1 - Svc/FpySequencer/FpySequencerTypes.fpp | 12 +- .../FpySequencerValidationState.cpp | 129 ++++++------------ .../test/ut/FpySequencerTestMain.cpp | 20 +-- .../test/ut/FpySequencerTester.cpp | 41 ++---- .../test/ut/FpySequencerTester.hpp | 2 +- 7 files changed, 66 insertions(+), 145 deletions(-) diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index d3c575c0f81..b14cae4ee3b 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -645,9 +645,6 @@ class FpySequencer : public FpySequencerComponentBase { // sequence arguments to push to stack when entering RUNNING state Svc::SeqArgs m_sequenceArgs{}; - // the expected argument size from arg_specs - Fpy::StackSizeType m_expectedArgSize{0}; - // 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 @@ -749,9 +746,6 @@ class FpySequencer : public FpySequencerComponentBase { // reads and validates the header from the m_sequenceBuffer // return SUCCESS if sequence is valid, FAILURE otherwise Fw::Success readHeader(); - // reads and parses arg_specs from the sequence file (schema version 6+) - // return SUCCESS if successful, FAILURE otherwise - Fw::Success readArgSpecs(Os::File& file); // helper function to read and deserialize a variable-length string field (length byte + string bytes) // returns the length via outLength parameter and writes string data to buffer Fw::Success deserializeStringField(Os::File& file, U8* buffer, U8& outLength); diff --git a/Svc/FpySequencer/FpySequencerStateMachine.cpp b/Svc/FpySequencer/FpySequencerStateMachine.cpp index ecafac99d45..2a25e68587c 100644 --- a/Svc/FpySequencer/FpySequencerStateMachine.cpp +++ b/Svc/FpySequencer/FpySequencerStateMachine.cpp @@ -305,7 +305,6 @@ void FpySequencer ::Svc_FpySequencer_SequencerStateMachine_action_clearSequenceA Svc_FpySequencer_SequencerStateMachine::Signal signal ) { this->m_sequenceArgs = {0, 0}; - this->m_expectedArgSize = 0; } //! Implementation for action clearBreakpoint of state machine Svc_FpySequencer_SequencerStateMachine diff --git a/Svc/FpySequencer/FpySequencerTypes.fpp b/Svc/FpySequencer/FpySequencerTypes.fpp index 530b0a370cf..ade2b06c1f8 100644 --- a/Svc/FpySequencer/FpySequencerTypes.fpp +++ b/Svc/FpySequencer/FpySequencerTypes.fpp @@ -138,17 +138,11 @@ module Svc { @ NOTE: This struct does NOT use FPP's auto-generated serialization! @ Serialization is handled manually in C++ to match Fpy's format: struct ArgSpec { - @ Length of the argument name (0-255) - argNameLen: U8 - @ Argument name as UTF-8 bytes (not null-terminated) - argName: [MAX_ARG_SPEC_NAME_LEN] U8 - @ Length of the type name (0-255) - typeNameLen: U8 - @ Type name as UTF-8 bytes (not null-terminated) - typeName: [MAX_ARG_SPEC_NAME_LEN] U8 + argName: string size MAX_ARG_SPEC_NAME_LEN + typeName: string size MAX_ARG_SPEC_NAME_LEN @ Size of this argument on the stack in bytes argSize: StackSizeType - } default { argNameLen = 0, argName = 0, typeNameLen = 0, typeName = 0, argSize = 0 } + } default {argName = "", typeName = "", argSize = 0 } struct Header { @ the major version of the FSW diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index d97514b427f..4109585f9fd 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -61,12 +61,6 @@ Fw::Success FpySequencer::validate() { return Fw::Success::FAILURE; } - // Read and parse arg_specs - readStatus = this->readArgSpecs(sequenceFile); - if (readStatus != Fw::Success::SUCCESS) { - return Fw::Success::FAILURE; - } - readStatus = readBytes(sequenceFile, this->m_sequenceObj.get_header().get_bodySize(), FpySequencer_FileReadStage::BODY); @@ -111,12 +105,6 @@ Fw::Success FpySequencer::validate() { return Fw::Success::FAILURE; } - if (this->m_sequenceArgs.get_size() != this->m_expectedArgSize) { - this->log_WARNING_HI_ArgSizeMismatch(this->m_expectedArgSize, this->m_sequenceArgs.get_size(), - this->m_sequenceFilePath); - return Fw::Success::FAILURE; - } - return Fw::Success::SUCCESS; } @@ -153,89 +141,56 @@ Fw::Success FpySequencer::readHeader() { return Fw::Success::SUCCESS; } -// Helper function to read and deserialize a variable-length string field -// Reads length byte, then the string data into the provided buffer -Fw::Success FpySequencer::deserializeStringField(Os::File& file, U8* buffer, U8& outLength) { - // Read length byte - Fw::Success readStatus = this->readBytes(file, sizeof(U8), FpySequencer_FileReadStage::BODY); - if (readStatus != Fw::Success::SUCCESS) { - return Fw::Success::FAILURE; - } - - Fw::SerializeStatus deserStatus = this->m_sequenceBuffer.deserializeTo(outLength); - if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { - this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), - this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); - return Fw::Success::FAILURE; - } - - // Validate length is non-zero (reject empty strings) - if (outLength == 0) { - this->log_WARNING_HI_InvalidArgSpec(this->m_sequenceFilePath); - return Fw::Success::FAILURE; - } - - // Validate length doesn't exceed buffer capacity - if (outLength > Fpy::MAX_ARG_SPEC_NAME_LEN) { - this->log_WARNING_HI_ArgSpecStringLengthExceedsMax(outLength, Fpy::MAX_ARG_SPEC_NAME_LEN, this->m_sequenceFilePath); - return Fw::Success::FAILURE; - } - - // Read string bytes - readStatus = this->readBytes(file, outLength, FpySequencer_FileReadStage::BODY); - if (readStatus != Fw::Success::SUCCESS) { - return Fw::Success::FAILURE; - } - - FwSizeType actualLen = static_cast(outLength); - deserStatus = this->m_sequenceBuffer.deserializeTo(buffer, actualLen, Fw::Serialization::OMIT_LENGTH); - if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { - this->log_WARNING_HI_FileReadDeserializeError( - FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast(deserStatus), - this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize()); - return Fw::Success::FAILURE; - } - - return Fw::Success::SUCCESS; -} - -// reads and validates arg_specs from the m_sequenceBuffer -// stores them in the Sequence.args array and calculates the total expected argument size -// return SUCCESS if successful, FAILURE otherwise -Fw::Success FpySequencer::readArgSpecs(Os::File& file) { - FW_ASSERT(file.isOpen()); - +// reads and validates the body from the m_sequenceBuffer +// return SUCCESS if sequence is valid, FAILURE otherwise +Fw::Success FpySequencer::readBody() { + Fw::SerializeStatus deserStatus; + const U8 argumentCount = this->m_sequenceObj.get_header().get_argumentCount(); Fpy::StackSizeType totalExpectedSize = 0; - Fw::SerializeStatus deserStatus; - Fw::Success readStatus; - - // Read and deserialize each arg_spec incrementally since they're variable-length + + // deser arguments + // Read and deserialize each arg_spec incrementally since they're variable-length for (U8 i = 0; i < argumentCount; i++) { Fpy::ArgSpec& argSpec = this->m_sequenceObj.get_args()[i]; - + Fw::String myString{}; // Read arg_name (length + string) - U8 argNameLen = 0; - readStatus = this->deserializeStringField(file, argSpec.get_argName(), argNameLen); - if (readStatus != Fw::Success::SUCCESS) { + deserStatus = this->m_sequenceBuffer.deserializeTo(myString); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, + this->m_sequenceFilePath, + static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), + this->m_sequenceBuffer.getSize() + ); return Fw::Success::FAILURE; } - argSpec.set_argNameLen(argNameLen); + if (myString.length() == 0) { + this->log_WARNING_HI_InvalidArgSpec(this->m_sequenceFilePath); + return Fw::Success::FAILURE; + } + argSpec.set_argName(myString); // Read type_name (length + string) - U8 typeNameLen = 0; - readStatus = this->deserializeStringField(file, argSpec.get_typeName(), typeNameLen); - if (readStatus != Fw::Success::SUCCESS) { + deserStatus = this->m_sequenceBuffer.deserializeTo(myString); + if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { + this->log_WARNING_HI_FileReadDeserializeError( + FpySequencer_FileReadStage::BODY, + this->m_sequenceFilePath, + static_cast(deserStatus), + this->m_sequenceBuffer.getDeserializeSizeLeft(), + this->m_sequenceBuffer.getSize() + ); return Fw::Success::FAILURE; } - argSpec.set_typeNameLen(typeNameLen); - - // Read and deserialize size field - readStatus = this->readBytes(file, sizeof(Fpy::StackSizeType), FpySequencer_FileReadStage::BODY); - if (readStatus != Fw::Success::SUCCESS) { + if (myString.length() == 0) { + this->log_WARNING_HI_InvalidArgSpec(this->m_sequenceFilePath); return Fw::Success::FAILURE; } + argSpec.set_typeName(myString); + + // Read and deserialize size field Fpy::StackSizeType argSize = 0; deserStatus = this->m_sequenceBuffer.deserializeTo(argSize); if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) { @@ -254,15 +209,7 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) { argSpec.set_argSize(argSize); totalExpectedSize += argSize; } - - this->m_expectedArgSize = totalExpectedSize; - return Fw::Success::SUCCESS; -} - -// reads and validates the body from the m_sequenceBuffer -// return SUCCESS if sequence is valid, FAILURE otherwise -Fw::Success FpySequencer::readBody() { - Fw::SerializeStatus deserStatus; + // deser statements for (U16 statementIdx = 0; statementIdx < this->m_sequenceObj.get_header().get_statementCount(); statementIdx++) { // deser statement diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index 11165ef4542..57980c3dcf9 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -2119,8 +2119,8 @@ TEST_F(FpySequencerTester, cmd_RUN) { TEST_F(FpySequencerTester, cmd_RUN_ARGS) { allocMem(); - add_ARG_SPEC("arg1", "U32", sizeof(U32)); - add_ARG_SPEC("arg2", "U32", sizeof(U32)); + addArgumentSpec("arg1", "U32", sizeof(U32)); + addArgumentSpec("arg2", "U32", sizeof(U32)); add_LOAD_REL(0, sizeof(U32)); // Load first arg (U32 at offset 0) - duplicates it on stack add_LOAD_REL(4, sizeof(U32)); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args @@ -2249,8 +2249,8 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED) { TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS) { allocMem(); - add_ARG_SPEC("arg1", "U32", sizeof(U32)); - add_ARG_SPEC("arg2", "U32", sizeof(U32)); + addArgumentSpec("arg1", "U32", sizeof(U32)); + addArgumentSpec("arg2", "U32", sizeof(U32)); add_LOAD_REL(0, sizeof(U32)); // Load first arg (U32 at offset 0) - duplicates it on stack add_LOAD_REL(4, sizeof(U32)); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args @@ -2340,7 +2340,7 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_zero_length_arg_name) { clearSeq(); // Create arg_spec with zero-length arg name (invalid) - add_ARG_SPEC("", "U32", sizeof(U32)); // Empty string for arg name + addArgumentSpec("", "U32", sizeof(U32)); // Empty string for arg name add_NO_OP(); writeToFile("test.bin"); @@ -2368,7 +2368,7 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_zero_length_type_name) { clearSeq(); // Create arg_spec with zero-length type name (invalid) - add_ARG_SPEC("arg1", "", sizeof(U32)); // Empty string for type name + addArgumentSpec("arg1", "", sizeof(U32)); // Empty string for type name add_NO_OP(); writeToFile("test.bin"); @@ -2404,7 +2404,7 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_max_length_strings) { memset(maxLengthType, 'B', 255); maxLengthType[255] = '\0'; - add_ARG_SPEC(maxLengthName, maxLengthType, sizeof(U32)); + addArgumentSpec(maxLengthName, maxLengthType, sizeof(U32)); add_LOAD_REL(0, sizeof(U32)); add_DISCARD(sizeof(U32) * 2); // Discard loaded copy + original writeToFile("test.bin"); @@ -2430,7 +2430,7 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_size_mismatch) { clearSeq(); // Create arg_spec claiming U32 (4 bytes) but provide wrong size - add_ARG_SPEC("badArg", "U32", sizeof(U64)); // Type says U32, size says U64 (8 bytes) + addArgumentSpec("badArg", "U32", sizeof(U64)); // Type says U32, size says U64 (8 bytes) add_NO_OP(); writeToFile("test.bin"); @@ -3450,8 +3450,8 @@ TEST_F(FpySequencerTester, seqRunIn) { TEST_F(FpySequencerTester, seqRunInArgs) { allocMem(); - add_ARG_SPEC("arg1", "U32", sizeof(U32)); - add_ARG_SPEC("arg2", "U32", sizeof(U32)); + addArgumentSpec("arg1", "U32", sizeof(U32)); + addArgumentSpec("arg2", "U32", sizeof(U32)); add_LOAD_REL(0, sizeof(U32)); // Load first arg (U32 at offset 0) - duplicates it on stack add_LOAD_REL(4, sizeof(U32)); // Load second arg (U32 at offset 4) - duplicates it on stack add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.cpp b/Svc/FpySequencer/test/ut/FpySequencerTester.cpp index 72cbc514fdc..dd037f5e5f3 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.cpp @@ -63,10 +63,14 @@ void FpySequencerTester::writeToFile(const char* name, FwSizeType maxBytes) { Fw::ExternalSerializeBuffer buf; buf.setExtBuffer(data, sizeof(data)); - // Calculate body size (statements only, arg_specs go in header section) + // Calculate body size (statements and arg_specs) for (U32 ii = 0; ii < seq.get_header().get_statementCount(); ii++) { ASSERT_EQ(buf.serializeFrom(seq.get_statements()[ii]), Fw::SerializeStatus::FW_SERIALIZE_OK); + } + for (U32 ii = 0; ii < seq.get_header().get_argumentCount(); ii++) { + ASSERT_EQ(buf.serializeFrom(seq.get_args()[ii]), Fw::SerializeStatus::FW_SERIALIZE_OK); } + seq.get_header().set_bodySize(static_cast(buf.getSize())); buf.resetSer(); @@ -76,18 +80,9 @@ void FpySequencerTester::writeToFile(const char* name, FwSizeType maxBytes) { // Write arg_specs in Fpy variable-length format for (U32 ii = 0; ii < seq.get_header().get_argumentCount(); ii++) { const Fpy::ArgSpec& argSpec = seq.get_args()[ii]; - // Write arg_name length and bytes (without F Prime length prefix) - ASSERT_EQ(buf.serializeFrom(argSpec.get_argNameLen()), Fw::SerializeStatus::FW_SERIALIZE_OK); - if (argSpec.get_argNameLen() > 0) { - ASSERT_EQ(buf.serializeFrom(argSpec.get_argName(), argSpec.get_argNameLen(), Fw::Serialization::OMIT_LENGTH), - Fw::SerializeStatus::FW_SERIALIZE_OK); - } - // Write type_name length and bytes (without F Prime length prefix) - ASSERT_EQ(buf.serializeFrom(argSpec.get_typeNameLen()), Fw::SerializeStatus::FW_SERIALIZE_OK); - if (argSpec.get_typeNameLen() > 0) { - ASSERT_EQ(buf.serializeFrom(argSpec.get_typeName(), argSpec.get_typeNameLen(), Fw::Serialization::OMIT_LENGTH), - Fw::SerializeStatus::FW_SERIALIZE_OK); - } + ASSERT_EQ(buf.serializeFrom(argSpec.get_argName()), Fw::SerializeStatus::FW_SERIALIZE_OK); + + ASSERT_EQ(buf.serializeFrom(argSpec.get_typeName()), Fw::SerializeStatus::FW_SERIALIZE_OK); // Write size ASSERT_EQ(buf.serializeFrom(argSpec.get_argSize()), Fw::SerializeStatus::FW_SERIALIZE_OK); } @@ -119,33 +114,25 @@ void FpySequencerTester::removeFile(const char* name) { Os::FileSystem::removeFile(name); } -void FpySequencerTester::add_ARG_SPEC(const char* argName, const char* typeName, Fpy::StackSizeType argSize) { +void FpySequencerTester::addArgumentSpec(Fw::String argName, Fw::String typeName, Fpy::StackSizeType argSize) { U8 argCount = seq.get_header().get_argumentCount(); FW_ASSERT(argCount < Fpy::MAX_SEQUENCE_ARG_COUNT); Fpy::ArgSpec& argSpec = seq.get_args()[argCount]; // Set arg_name - U8 argNameLen = static_cast(strlen(argName)); - FW_ASSERT(argNameLen <= Fpy::MAX_ARG_SPEC_NAME_LEN); - argSpec.set_argNameLen(argNameLen); - for (U8 i = 0; i < argNameLen; i++) { - argSpec.get_argName()[i] = static_cast(argName[i]); - } + FW_ASSERT(argName.length() <= Fpy::MAX_ARG_SPEC_NAME_LEN); + argSpec.set_argName(argName); // Set type_name - U8 typeNameLen = static_cast(strlen(typeName)); - FW_ASSERT(typeNameLen <= Fpy::MAX_ARG_SPEC_NAME_LEN); - argSpec.set_typeNameLen(typeNameLen); - for (U8 i = 0; i < typeNameLen; i++) { - argSpec.get_typeName()[i] = static_cast(typeName[i]); - } + FW_ASSERT(typeName.length() <= Fpy::MAX_ARG_SPEC_NAME_LEN); + argSpec.set_typeName(typeName); // Set size argSpec.set_argSize(argSize); // Increment argument count - seq.get_header().set_argumentCount(argCount + 1); + seq.get_header().set_argumentCount(++argCount); } void FpySequencerTester::resetRuntime() { diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp index 141d2cace68..da84a3691e5 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp @@ -79,7 +79,7 @@ class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test void writeToFile(const char* name, FwSizeType maxBytes = Fpy::Sequence::SERIALIZED_SIZE); void removeFile(const char* name); void addDirective(Fpy::DirectiveId id, Fw::StatementArgBuffer& buf); - void add_ARG_SPEC(const char* argName, const char* typeName, Fpy::StackSizeType argSize); + void addArgumentSpec(Fw::String argName, Fw::String typeName, Fpy::StackSizeType argSize); void add_WAIT_REL(); void add_WAIT_REL(FpySequencer_WaitRelDirective dir); From c4c9f1e399bb43c77eede222f85c73b96b0ef6f9 Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Thu, 23 Apr 2026 11:46:37 -0500 Subject: [PATCH 16/18] Validate Arg Buff --- Svc/FpySequencer/FpySequencer.hpp | 4 ++++ Svc/FpySequencer/FpySequencerValidationState.cpp | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index b14cae4ee3b..153e2cce312 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -634,6 +634,10 @@ class FpySequencer : public FpySequencerComponentBase { // live running computation of CRC (updated as we read) U32 m_computedCRC; + // Size of arguments read in current sequence. Used for validation between + // User provided arguments and what is requested of the sequence. + Fpy::StackSizeType m_totalReadArgumentSize{0}; + // whether or not the sequence we're about to run should return immediately or // block on completion FpySequencer_BlockState m_sequenceBlockState; diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index 4109585f9fd..5f89d78f516 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -73,7 +73,7 @@ Fw::Success FpySequencer::validate() { if (readStatus != Fw::Success::SUCCESS) { return Fw::Success::FAILURE; } - + // read footer bytes but don't include in CRC readStatus = this->readBytes(sequenceFile, Fpy::Footer::SERIALIZED_SIZE, FpySequencer_FileReadStage::FOOTER, false); @@ -147,7 +147,7 @@ Fw::Success FpySequencer::readBody() { Fw::SerializeStatus deserStatus; const U8 argumentCount = this->m_sequenceObj.get_header().get_argumentCount(); - Fpy::StackSizeType totalExpectedSize = 0; + this->m_totalReadArgumentSize = 0; // deser arguments // Read and deserialize each arg_spec incrementally since they're variable-length @@ -201,13 +201,21 @@ Fw::Success FpySequencer::readBody() { } // Check for overflow before accumulation - if (totalExpectedSize > Fpy::MAX_STACK_SIZE - argSize) { + if (m_totalReadArgumentSize > Fpy::MAX_STACK_SIZE - argSize) { this->log_WARNING_HI_ArgTotalSizeExceedsStackLimit(argSize, Fpy::MAX_STACK_SIZE, this->m_sequenceFilePath); return Fw::Success::FAILURE; } argSpec.set_argSize(argSize); - totalExpectedSize += argSize; + m_totalReadArgumentSize += argSize; + } + + // Validate total argument size + if (this->m_totalReadArgumentSize != this->m_sequenceArgs.get_size()){ + this->log_WARNING_HI_ArgSizeMismatch(this->m_sequenceObj.get_header().get_argumentCount(), + this->m_sequenceArgs.get_size(), + this->m_sequenceFilePath); + return Fw::Success::FAILURE; } // deser statements From 9443dd0585213f49af721adb2b016b1c435b2c4b Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Thu, 30 Apr 2026 14:05:12 -0500 Subject: [PATCH 17/18] Renamed types / reordering --- Svc/FpySequencer/FpySequencer.hpp | 5 +--- .../FpySequencerValidationState.cpp | 16 +++------- .../test/ut/FpySequencerTestMain.cpp | 29 +++++++++---------- .../test/ut/FpySequencerTester.cpp | 8 ++--- 4 files changed, 22 insertions(+), 36 deletions(-) diff --git a/Svc/FpySequencer/FpySequencer.hpp b/Svc/FpySequencer/FpySequencer.hpp index 153e2cce312..8d9b16c91ea 100644 --- a/Svc/FpySequencer/FpySequencer.hpp +++ b/Svc/FpySequencer/FpySequencer.hpp @@ -636,7 +636,7 @@ class FpySequencer : public FpySequencerComponentBase { // Size of arguments read in current sequence. Used for validation between // User provided arguments and what is requested of the sequence. - Fpy::StackSizeType m_totalReadArgumentSize{0}; + Fpy::StackSizeType m_totalExpectedArgSize{0}; // whether or not the sequence we're about to run should return immediately or // block on completion @@ -750,9 +750,6 @@ class FpySequencer : public FpySequencerComponentBase { // reads and validates the header from the m_sequenceBuffer // return SUCCESS if sequence is valid, FAILURE otherwise Fw::Success readHeader(); - // helper function to read and deserialize a variable-length string field (length byte + string bytes) - // returns the length via outLength parameter and writes string data to buffer - Fw::Success deserializeStringField(Os::File& file, U8* buffer, U8& outLength); // reads and validates the body from the m_sequenceBuffer // return SUCCESS if sequence is valid, FAILURE otherwise Fw::Success readBody(); diff --git a/Svc/FpySequencer/FpySequencerValidationState.cpp b/Svc/FpySequencer/FpySequencerValidationState.cpp index 5f89d78f516..3074aba5444 100644 --- a/Svc/FpySequencer/FpySequencerValidationState.cpp +++ b/Svc/FpySequencer/FpySequencerValidationState.cpp @@ -147,7 +147,7 @@ Fw::Success FpySequencer::readBody() { Fw::SerializeStatus deserStatus; const U8 argumentCount = this->m_sequenceObj.get_header().get_argumentCount(); - this->m_totalReadArgumentSize = 0; + this->m_totalExpectedArgSize = 0; // deser arguments // Read and deserialize each arg_spec incrementally since they're variable-length @@ -166,10 +166,6 @@ Fw::Success FpySequencer::readBody() { ); return Fw::Success::FAILURE; } - if (myString.length() == 0) { - this->log_WARNING_HI_InvalidArgSpec(this->m_sequenceFilePath); - return Fw::Success::FAILURE; - } argSpec.set_argName(myString); // Read type_name (length + string) @@ -184,10 +180,6 @@ Fw::Success FpySequencer::readBody() { ); return Fw::Success::FAILURE; } - if (myString.length() == 0) { - this->log_WARNING_HI_InvalidArgSpec(this->m_sequenceFilePath); - return Fw::Success::FAILURE; - } argSpec.set_typeName(myString); // Read and deserialize size field @@ -201,17 +193,17 @@ Fw::Success FpySequencer::readBody() { } // Check for overflow before accumulation - if (m_totalReadArgumentSize > Fpy::MAX_STACK_SIZE - argSize) { + if (m_totalExpectedArgSize > Fpy::MAX_STACK_SIZE - argSize) { this->log_WARNING_HI_ArgTotalSizeExceedsStackLimit(argSize, Fpy::MAX_STACK_SIZE, this->m_sequenceFilePath); return Fw::Success::FAILURE; } argSpec.set_argSize(argSize); - m_totalReadArgumentSize += argSize; + m_totalExpectedArgSize += argSize; } // Validate total argument size - if (this->m_totalReadArgumentSize != this->m_sequenceArgs.get_size()){ + if (this->m_totalExpectedArgSize != this->m_sequenceArgs.get_size()){ this->log_WARNING_HI_ArgSizeMismatch(this->m_sequenceObj.get_header().get_argumentCount(), this->m_sequenceArgs.get_size(), this->m_sequenceFilePath); diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index 57980c3dcf9..37dfbb464c8 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -2339,7 +2339,7 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_zero_length_arg_name) { allocMem(); clearSeq(); - // Create arg_spec with zero-length arg name (invalid) + // Create arg_spec with zero-length arg name (valid) addArgumentSpec("", "U32", sizeof(U32)); // Empty string for arg name add_NO_OP(); writeToFile("test.bin"); @@ -2353,12 +2353,11 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_zero_length_arg_name) { sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), args); dispatchUntilState(State::VALIDATING); - // Should fail during arg_spec deserialization due to zero-length arg name - dispatchUntilState(State::IDLE); + ASSERT_EQ(tester_get_m_sequencesStarted(), 0); + ASSERT_EQ(tester_get_m_statementsDispatched(), 0); + dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); ASSERT_CMD_RESPONSE_SIZE(1); - ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); - ASSERT_from_seqDoneOut_SIZE(1); - ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::EXECUTION_ERROR); + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::OK); removeFile("test.bin"); } @@ -2367,7 +2366,7 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_zero_length_type_name) { allocMem(); clearSeq(); - // Create arg_spec with zero-length type name (invalid) + // Create arg_spec with zero-length type name (valid) addArgumentSpec("arg1", "", sizeof(U32)); // Empty string for type name add_NO_OP(); writeToFile("test.bin"); @@ -2381,13 +2380,12 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_zero_length_type_name) { sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), args); dispatchUntilState(State::VALIDATING); - // Should fail during arg_spec deserialization due to zero-length type name - dispatchUntilState(State::IDLE); + ASSERT_EQ(tester_get_m_sequencesStarted(), 0); + ASSERT_EQ(tester_get_m_statementsDispatched(), 0); + dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED); ASSERT_CMD_RESPONSE_SIZE(1); - ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR); - ASSERT_from_seqDoneOut_SIZE(1); - ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::EXECUTION_ERROR); - + ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::OK); + removeFile("test.bin"); } @@ -2429,15 +2427,14 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_size_mismatch) { allocMem(); clearSeq(); - // Create arg_spec claiming U32 (4 bytes) but provide wrong size - addArgumentSpec("badArg", "U32", sizeof(U64)); // Type says U32, size says U64 (8 bytes) + addArgumentSpec("arg", "U64", sizeof(U64)); // U64 Arg add_NO_OP(); writeToFile("test.bin"); // Create args buffer with actual U32 (4 bytes) Svc::SeqArgs args{0, 0}; Fw::ExternalSerializeBuffer argBuf(args.get_buffer(), SequenceArgumentsMaxSize); - U32 arg1Val = 99; + U32 arg1Val = 99; // Passing in a U32 ASSERT_EQ(argBuf.serializeFrom(arg1Val), Fw::FW_SERIALIZE_OK); args.set_size(argBuf.getSize()); diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.cpp b/Svc/FpySequencer/test/ut/FpySequencerTester.cpp index dd037f5e5f3..855480aaf32 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.cpp @@ -63,13 +63,13 @@ void FpySequencerTester::writeToFile(const char* name, FwSizeType maxBytes) { Fw::ExternalSerializeBuffer buf; buf.setExtBuffer(data, sizeof(data)); - // Calculate body size (statements and arg_specs) - for (U32 ii = 0; ii < seq.get_header().get_statementCount(); ii++) { - ASSERT_EQ(buf.serializeFrom(seq.get_statements()[ii]), Fw::SerializeStatus::FW_SERIALIZE_OK); - } + // Calculate body size (arg_specs and statements) for (U32 ii = 0; ii < seq.get_header().get_argumentCount(); ii++) { ASSERT_EQ(buf.serializeFrom(seq.get_args()[ii]), Fw::SerializeStatus::FW_SERIALIZE_OK); } + for (U32 ii = 0; ii < seq.get_header().get_statementCount(); ii++) { + ASSERT_EQ(buf.serializeFrom(seq.get_statements()[ii]), Fw::SerializeStatus::FW_SERIALIZE_OK); + } seq.get_header().set_bodySize(static_cast(buf.getSize())); buf.resetSer(); From 33f895c727bf89ba17c02adf04e326305a38c554 Mon Sep 17 00:00:00 2001 From: Alex Mariano Date: Thu, 30 Apr 2026 16:26:02 -0500 Subject: [PATCH 18/18] argsUT --- Svc/FpySequencer/FpySequencerEvents.fppi | 13 ----- .../test/ut/FpySequencerTestMain.cpp | 58 ++++++++++++++++--- .../test/ut/FpySequencerTester.cpp | 4 ++ .../test/ut/FpySequencerTester.hpp | 1 + 4 files changed, 54 insertions(+), 22 deletions(-) diff --git a/Svc/FpySequencer/FpySequencerEvents.fppi b/Svc/FpySequencer/FpySequencerEvents.fppi index f9d6131f165..f5b102be775 100644 --- a/Svc/FpySequencer/FpySequencerEvents.fppi +++ b/Svc/FpySequencer/FpySequencerEvents.fppi @@ -231,19 +231,6 @@ event ArgTotalSizeExceedsStackLimit( severity warning high \ format "Adding argument of size {} would exceed max stack size {} in sequence file {}" -event InvalidArgSpec( - filePath: string -) \ - severity warning high \ - format "Invalid argument specification in sequence file {}: arg_name or type_name cannot be empty" - -event ArgSpecStringLengthExceedsMax( - length: U8 - max: U8 - filePath: string -) \ - severity warning high \ - format "Argument specification string length {} exceeds maximum {} in sequence file {}" event SequencePaused( stmtIdx: U32 diff --git a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp index 37dfbb464c8..38ae17fc3f1 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp @@ -2717,10 +2717,56 @@ TEST_F(FpySequencerTester, readHeader) { } TEST_F(FpySequencerTester, readBody) { - U8 data[Fpy::MAX_SEQUENCE_STATEMENT_COUNT * Fpy::Statement::SERIALIZED_SIZE]; + FwSizeType argSpecSize = Fpy::MAX_SEQUENCE_ARG_COUNT * Fpy::ArgSpec::SERIALIZED_SIZE; + FwSizeType stmtSize = Fpy::MAX_SEQUENCE_STATEMENT_COUNT * Fpy::Statement::SERIALIZED_SIZE; - tester_get_m_sequenceBuffer_ptr()->setExtBuffer(data, sizeof(data)); - // write some statements (no arg mappings in body anymore, arg_specs are separate) + U8 data[argSpecSize + stmtSize]; + + tester_get_m_sequenceBuffer_ptr()->setExtBuffer(data, sizeof(data)); + + // write some argSpecs + 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); + for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_ARG_COUNT; ii++) { + Fw::String argName; + argName.format("arg%u", ii); + Fw::String typeName("U32"); + ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(argName), Fw::SerializeStatus::FW_SERIALIZE_OK); + ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(typeName), Fw::SerializeStatus::FW_SERIALIZE_OK); + ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(static_cast(sizeof(U32))), Fw::SerializeStatus::FW_SERIALIZE_OK); + } + tester_get_m_sequenceObj_ptr()->get_header().set_argumentCount(Fpy::MAX_SEQUENCE_ARG_COUNT); + tester_get_m_sequenceObj_ptr()->get_header().set_statementCount(0); + + ASSERT_EQ(tester_readBody(), Fw::Success::SUCCESS); + + for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_ARG_COUNT; ii++) { + Fw::String expectedArgName; + expectedArgName.format("arg%u", ii); + ASSERT_EQ(tester_get_m_sequenceObj_ptr()->get_args()[ii].get_argName(), expectedArgName); + ASSERT_EQ(tester_get_m_sequenceObj_ptr()->get_args()[ii].get_typeName(), Fw::String("U32")); + ASSERT_EQ(tester_get_m_sequenceObj_ptr()->get_args()[ii].get_argSize(), sizeof(U32)); + } + + // check not writing enough arguments + // -1 intended mistake + tester_get_m_sequenceBuffer_ptr()->resetSer(); + + for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_ARG_COUNT - 1; ii++) { + ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(""), Fw::SerializeStatus::FW_SERIALIZE_OK); + ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(""), Fw::SerializeStatus::FW_SERIALIZE_OK); + ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(static_cast(sizeof(U32))), Fw::SerializeStatus::FW_SERIALIZE_OK); + } + ASSERT_EQ(tester_readBody(), Fw::Success::FAILURE); + + tester_get_m_sequenceBuffer_ptr()->resetSer(); + + // write some statements + Svc::SeqArgs noArgs{0, 0}; + tester_set_m_sequenceArgs(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++) { ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(stmt), Fw::SerializeStatus::FW_SERIALIZE_OK); @@ -2740,13 +2786,7 @@ TEST_F(FpySequencerTester, readBody) { // now see what happens if we don't write enough stmts tester_get_m_sequenceBuffer_ptr()->resetSer(); - tester_get_m_sequenceObj_ptr()->get_header().set_argumentCount(Fpy::MAX_SEQUENCE_ARG_COUNT); tester_get_m_sequenceObj_ptr()->get_header().set_statementCount(Fpy::MAX_SEQUENCE_STATEMENT_COUNT); - for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_ARG_COUNT; ii++) { - // map arg idx ii to serReg pos 123 - ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(static_cast(123)), - Fw::SerializeStatus::FW_SERIALIZE_OK); - } // the -1 here is the intended mistake for (U32 ii = 0; ii < Fpy::MAX_SEQUENCE_STATEMENT_COUNT - 1; ii++) { ASSERT_EQ(tester_get_m_sequenceBuffer_ptr()->serializeFrom(stmt), Fw::SerializeStatus::FW_SERIALIZE_OK); diff --git a/Svc/FpySequencer/test/ut/FpySequencerTester.cpp b/Svc/FpySequencer/test/ut/FpySequencerTester.cpp index 855480aaf32..2b38f67f04e 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.cpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.cpp @@ -666,6 +666,10 @@ void FpySequencerTester::tester_set_m_computedCRC(U32 crc) { this->cmp.m_computedCRC = crc; } +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 da84a3691e5..fb2ff3ecdeb 100644 --- a/Svc/FpySequencer/test/ut/FpySequencerTester.hpp +++ b/Svc/FpySequencer/test/ut/FpySequencerTester.hpp @@ -257,6 +257,7 @@ class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test Fw::Success tester_readBody(); Fw::Success tester_readHeader(); void tester_set_m_computedCRC(U32 crc); + void tester_set_m_sequenceArgs(Svc::SeqArgs args); Svc::FpySequencer::BreakpointInfo* tester_get_m_breakpoint_ptr(); Svc::Signal tester_checkStatementTimeout(); Svc::Signal tester_checkShouldWake();