Skip to content

Add args to CmdSeqIn #4962

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Lex-ari wants to merge 4 commits into
base: devel
Choose a base branch
from
Open

Add args to CmdSeqIn #4962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
41c3dad
Add args to CmdSeqIn
Lex-ari Apr 7, 2026
205518e
Merge branch 'devel' into Lex-ari-CmdSeqIn-args
Lex-ari Apr 7, 2026
44f9be0
Fix function header
Lex-ari Apr 7, 2026
5a7ad18
Add RUN_ARGS handlers + UTs
Lex-ari Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions Svc/CmdSequencer/CmdSequencerImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -162,15 +165,19 @@ 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();
}

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);
}

Expand Down Expand Up @@ -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{0, 0};
this->seqStartOut_out(0, this->m_sequence->getStringFileName(), emptyArgs);
}
this->cmdResponse_out(opcode, cmdSeq, Fw::CmdResponse::OK);
}
Expand Down
3 changes: 2 additions & 1 deletion Svc/CmdSequencer/CmdSequencerImpl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions Svc/CmdSequencer/test/ut/CmdSequencerTester.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion Svc/CmdSequencer/test/ut/ImmediateBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion Svc/CmdSequencer/test/ut/InvalidFiles.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
22 changes: 20 additions & 2 deletions Svc/FpySequencer/FpySequencer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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<I32>(sequencer_getState()));
Expand All @@ -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;
Copy link
Copy Markdown
Collaborator

@zimri-leisher zimri-leisher Apr 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a cleaner way of passing around the pending sequence arguments without adding new state.

I described it in an earlier comment:

Important: you're going to have to add the SeqArgs struct to the SequenceExecutionArgs struct, and pass it in on line 370 here. You should probably add a new SM action called "pushArgsToStack" or something, which happens when cmd_RUN is received,

this->sequencer_sendSignal_cmd_RUN(FpySequencer_SequenceExecutionArgs(fileName, block));

// only respond if the user doesn't want us to block further execution
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -358,14 +374,16 @@ 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) {
// can only run a seq while in idle
if (sequencer_getState() != State::IDLE) {
this->log_WARNING_HI_InvalidSeqRunCall(static_cast<I32>(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));
}

Expand Down
23 changes: 21 additions & 2 deletions Svc/FpySequencer/FpySequencer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
//!
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -470,7 +486,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
Expand Down Expand Up @@ -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
Expand Down
25 changes: 16 additions & 9 deletions Svc/FpySequencer/FpySequencerCommands.fppi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
) \
Copy link
Copy Markdown
Collaborator

@zimri-leisher zimri-leisher Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command needs an argument buf too. And that would mean that we'd need a RUN_VALIDATED_ARGS command. However, I think it would be okay to skip this for now, because this command is not used frequently.

Please make a github issue to track adding the args buf to this command when we fix the GDS.

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
Expand All @@ -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
opcode 10 priority 7 assert
2 changes: 1 addition & 1 deletion Svc/FpySequencer/FpySequencerStack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<FwAssertArgType>(this->size),
static_cast<FwAssertArgType>(srcSize));
memcpy(this->top(), src, srcSize);
Expand Down
32 changes: 30 additions & 2 deletions Svc/FpySequencer/FpySequencerStateMachine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Copy Markdown
Collaborator

@zimri-leisher zimri-leisher Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way this PR is currently implemented, sendSignal_result_failure does not do what you expect it does here. It actually would do nothing, and the FSW would try to run the sequence. Could be quite bad. This is because, when this action is called, you're in the RUN state. Homework for you: make sure you know which signals do what in the RUN state. Many signals won't do anything.

Once you make the modification I mentioned regarding the SequenceExecutionArgs, this function will just set the m_pendingSeqArgs blindly. See action_setSequenceFilePath.

Then, the size check will go in the FpySequencer::validate method. You can just return Fw::Success::FAILURE.

Please also add a unit test for this case.

// 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<Fpy::StackSizeType>(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
Expand Down Expand Up @@ -343,8 +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
this->seqStartOut_out(0, this->m_sequenceFilePath);
// report that the sequence started to internal callers with the actual args
this->seqStartOut_out(0, this->m_sequenceFilePath, this->m_pendingSeqArgs);
}
}
// ----------------------------------------------------------------------
Expand Down
4 changes: 3 additions & 1 deletion Svc/FpySequencer/FpySequencerStateMachine.fppi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
Loading
Loading