Skip to content

Commit 2cf37a1

Browse files
committed
Removed cmd_RUN_VALIDATED_ARGS
1 parent 1e828ec commit 2cf37a1

7 files changed

Lines changed: 55 additions & 73 deletions

Svc/FpySequencer/FpySequencer.cpp

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,16 @@ void FpySequencer::VALIDATE_cmdHandler(FwOpcodeType opCode, //!< Th
7979
U32 cmdSeq, //!< The command sequence number
8080
const Fw::CmdStringArg& fileName //!< The name of the sequence file
8181
) {
82+
this->VALIDATE_ARGS_cmdHandler(opCode, cmdSeq, fileName, Svc::SeqArgs{0, 0});
83+
}
84+
85+
//! Handler implementation for command VALIDATE_ARGS
86+
//!
87+
//! Loads and validates a sequence with arguments
88+
void FpySequencer ::VALIDATE_ARGS_cmdHandler(FwOpcodeType opCode,
89+
U32 cmdSeq,
90+
const Fw::CmdStringArg& fileName,
91+
Svc::SeqArgs buffer) {
8292
// can only validate a seq while in idle
8393
if (sequencer_getState() != State::IDLE) {
8494
this->log_WARNING_HI_InvalidCommand(static_cast<I32>(sequencer_getState()));
@@ -91,28 +101,16 @@ void FpySequencer::VALIDATE_cmdHandler(FwOpcodeType opCode, //!< Th
91101
this->m_savedOpCode = opCode;
92102
this->m_savedCmdSeq = cmdSeq;
93103

94-
// VALIDATE command doesn't receive args via command interface, use empty SeqArgs
95-
// Store empty args for pushArgsToStack action
104+
// VALIDATE_ARGS receives args via command interface
105+
// Store args for pushArgsToStack action when RUN_VALIDATED is called
96106
this->sequencer_sendSignal_cmd_VALIDATE(
97-
FpySequencer_SequenceExecutionArgs(fileName, FpySequencer_BlockState::BLOCK, Svc::SeqArgs{0, 0}));
107+
FpySequencer_SequenceExecutionArgs(fileName, FpySequencer_BlockState::BLOCK, buffer));
98108
}
99109

100110
void FpySequencer::RUN_VALIDATED_cmdHandler(
101111
FwOpcodeType opCode, //!< The opcode
102112
U32 cmdSeq, //!< The command sequence number
103-
FpySequencer_BlockState block //!< Return command status when complete or not
104-
) {
105-
this->RUN_VALIDATED_ARGS_cmdHandler(opCode, cmdSeq, block, Svc::SeqArgs{0, 0});
106-
}
107-
108-
//! Handler for command RUN_VALIDATED
109-
//!
110-
//! Runs a previously validated sequence
111-
void FpySequencer::RUN_VALIDATED_ARGS_cmdHandler(
112-
FwOpcodeType opCode, //!< The opcode
113-
U32 cmdSeq, //!< The command sequence number
114-
FpySequencer_BlockState block, //!< Return command status when complete or not
115-
Svc::SeqArgs args //!< Arguments to pass to the sequencer
113+
FpySequencer_BlockState block //!< Return command status when complete or not
116114
) {
117115
// can only RUN_VALIDATED if we have validated and are awaiting this exact cmd
118116
if (sequencer_getState() != State::AWAITING_CMD_RUN_VALIDATED) {
@@ -127,7 +125,7 @@ void FpySequencer::RUN_VALIDATED_ARGS_cmdHandler(
127125
this->m_savedCmdSeq = cmdSeq;
128126
}
129127

130-
this->sequencer_sendSignal_cmd_RUN_VALIDATED(FpySequencer_SequenceExecutionArgs(this->m_sequenceFilePath, block, args));
128+
this->sequencer_sendSignal_cmd_RUN_VALIDATED(FpySequencer_SequenceExecutionArgs(this->m_sequenceFilePath, block, this->m_sequenceArgs));
131129

132130
// only respond if the user doesn't want us to block further execution
133131
if (block == FpySequencer_BlockState::NO_BLOCK) {

Svc/FpySequencer/FpySequencer.hpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,15 @@ class FpySequencer : public FpySequencerComponentBase {
161161
U32 cmdSeq, //!< The command sequence number
162162
const Fw::CmdStringArg& fileName //!< The name of the sequence file
163163
) override;
164+
165+
//! Handler implementation for command VALIDATE_ARGS
166+
//!
167+
//! Loads and validates a sequence with arguments
168+
void VALIDATE_ARGS_cmdHandler(FwOpcodeType opCode, //!< The opcode
169+
U32 cmdSeq, //!< The command sequence number
170+
const Fw::CmdStringArg& fileName, //!< The name of the sequence file
171+
Svc::SeqArgs buffer //!< Arguments to pass to the sequencer
172+
) override;
164173

165174
//! Handler implementation for command RUN_VALIDATED
166175
//!
@@ -170,15 +179,6 @@ class FpySequencer : public FpySequencerComponentBase {
170179
Svc::FpySequencer_BlockState block //!< Return command status when complete or not
171180
) override;
172181

173-
//! Handler implementation for command RUN_VALIDATED_ARGS
174-
//!
175-
//! Must be called after VALIDATE. Runs the sequence that was validated with arguments
176-
void RUN_VALIDATED_ARGS_cmdHandler(FwOpcodeType opCode, //!< The opcode
177-
U32 cmdSeq, //!< The command sequence number
178-
Svc::FpySequencer_BlockState block, //!< Return command status when complete or not
179-
Svc::SeqArgs args //!< Arguments to pass to the sequencer
180-
) override;
181-
182182
//! Handler for command CANCEL
183183
//!
184184
//! Cancels a running or validated sequence

Svc/FpySequencer/FpySequencerCommands.fppi

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,18 @@ async command VALIDATE(
2020
) \
2121
opcode 2 priority 7 assert
2222

23-
@ Must be called after VALIDATE. Runs the sequence that was validated.
23+
@ Loads and validates a sequence with arguments
2424
# prio: lower than sm sig and CANCEL
25-
async command RUN_VALIDATED(
26-
$block: BlockState @< Return command status when complete or not
25+
async command VALIDATE_ARGS(
26+
fileName: string size FileNameStringSize @< The name of the sequence file
27+
buffer: Svc.SeqArgs @< Arguments to pass to the sequencer
2728
) \
2829
opcode 3 priority 7 assert
2930

3031
@ Must be called after VALIDATE. Runs the sequence that was validated.
3132
# prio: lower than sm sig and CANCEL
32-
async command RUN_VALIDATED_ARGS(
33+
async command RUN_VALIDATED(
3334
$block: BlockState @< Return command status when complete or not
34-
buffer: Svc.SeqArgs @< Arguments to pass to the sequencer
3535
) \
3636
opcode 4 priority 7 assert
3737

Svc/FpySequencer/FpySequencerStateMachine.cpp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -281,15 +281,6 @@ void FpySequencer::Svc_FpySequencer_SequencerStateMachine_action_pushArgsToStack
281281
return;
282282
}
283283

284-
Fpy::StackSizeType availableSpace = Fpy::MAX_STACK_SIZE - this->m_runtime.stack.size;
285-
286-
if (args.get_size() > availableSpace) {
287-
// Args too large - fail the sequence gracefully.
288-
// TODO: Move this to validate() step and validate the size there.
289-
this->sequencer_sendSignal_stmtResponse_unexpected();
290-
return;
291-
}
292-
293284
// Push args buffer to stack. Args are already serialized in big-endian format
294285
// by F' serialization system, so no endianness conversion is needed.
295286
this->m_runtime.stack.push(args.get_buffer(),

Svc/FpySequencer/FpySequencerStateMachine.fppi

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ state machine SequencerStateMachine {
103103
# commands #
104104
####################
105105
# validate does not take as input a block state, only a path
106-
on cmd_VALIDATE do { setGoalState_VALID, setSequenceFilePath} enter VALIDATING
106+
on cmd_VALIDATE do { setGoalState_VALID, setSequenceFilePath, setSequenceArguments} enter VALIDATING
107107
# run takes both path and block state
108108
on cmd_RUN do { setGoalState_RUNNING, setSequenceFilePath, setSequenceBlockState, setSequenceArguments} enter VALIDATING
109109

@@ -153,7 +153,7 @@ state machine SequencerStateMachine {
153153
@ cancelled the sequence after we validated it
154154
on cmd_CANCEL do { report_seqCancelled } enter IDLE
155155
@ the sequence path has already been decided on, so only set the sequenceShouldBlock var
156-
on cmd_RUN_VALIDATED do { setSequenceBlockState, setSequenceArguments } enter RUNNING
156+
on cmd_RUN_VALIDATED do { setSequenceBlockState } enter RUNNING
157157

158158
on cmd_SET_BREAKPOINT do { setBreakpoint }
159159
on cmd_CLEAR_BREAKPOINT do { clearBreakpoint }

Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2169,7 +2169,7 @@ TEST_F(FpySequencerTester, cmd_RUN_ARGS_oversized) {
21692169

21702170
sendCmd_RUN_ARGS(0, 0, Fw::String("test.bin"), FpySequencer_BlockState::BLOCK, largeArgs);
21712171
dispatchUntilState(State::VALIDATING);
2172-
// should fail when trying to push args to stack
2172+
// should fail during validation when checking args size
21732173
dispatchUntilState(State::IDLE);
21742174
ASSERT_CMD_RESPONSE_SIZE(1);
21752175
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) {
22452245
ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED(), 0, Fw::CmdResponse::OK);
22462246
}
22472247

2248-
TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS) {
2249-
// should fail because in idle
2250-
this->tester_setState(State::IDLE);
2251-
Svc::SeqArgs emptyArgs{0, 0};
2252-
sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::NO_BLOCK, emptyArgs);
2253-
dispatchCurrentMessages(cmp);
2254-
ASSERT_CMD_RESPONSE_SIZE(1);
2255-
ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR);
2256-
this->clearHistory();
2257-
2248+
TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS) {
22582249
allocMem();
22592250
add_LOAD_REL(0, 4); // Load first arg (U32 at offset 0) - duplicates it on stack
22602251
add_LOAD_REL(4, 4); // Load second arg (U32 at offset 4) - duplicates it on stack
22612252
add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args
22622253
writeToFile("test.bin");
2263-
sendCmd_VALIDATE(0, 0, Fw::String("test.bin"));
2264-
dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED);
2265-
this->clearHistory();
22662254

22672255
// Pass two U32 args: 10 and 20
22682256
Svc::SeqArgs args{0, 0};
@@ -2272,11 +2260,20 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS) {
22722260
ASSERT_EQ(argBuf.serializeFrom(arg2), Fw::FW_SERIALIZE_OK);
22732261
args.set_size(argBuf.getSize());
22742262

2263+
sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), args);
2264+
dispatchUntilState(State::VALIDATING);
2265+
ASSERT_EQ(tester_get_m_sequencesStarted(), 0);
2266+
ASSERT_EQ(tester_get_m_statementsDispatched(), 0);
2267+
dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED);
2268+
ASSERT_CMD_RESPONSE_SIZE(1);
2269+
ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::OK);
2270+
this->clearHistory();
2271+
22752272
// should succeed immediately
2276-
sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::NO_BLOCK, args);
2273+
sendCmd_RUN_VALIDATED(0, 0, FpySequencer_BlockState::NO_BLOCK);
22772274
this->tester_doDispatch();
22782275
ASSERT_CMD_RESPONSE_SIZE(1);
2279-
ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::OK);
2276+
ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED(), 0, Fw::CmdResponse::OK);
22802277
dispatchUntilState(State::RUNNING_AWAITING_STATEMENT_RESPONSE);
22812278
ASSERT_EQ(tester_get_m_sequencesStarted(), 1);
22822279

@@ -2289,11 +2286,11 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS) {
22892286
ASSERT_CMD_RESPONSE_SIZE(1);
22902287
clearHistory();
22912288

2292-
sendCmd_VALIDATE(0, 0, Fw::String("test.bin"));
2289+
sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), args);
22932290
dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED);
22942291
this->clearHistory();
22952292
// should succeed immediately
2296-
sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::BLOCK, args);
2293+
sendCmd_RUN_VALIDATED(0, 0, FpySequencer_BlockState::BLOCK);
22972294
this->tester_doDispatch();
22982295
ASSERT_CMD_RESPONSE_SIZE(0);
22992296
dispatchUntilState(State::RUNNING_AWAITING_STATEMENT_RESPONSE);
@@ -2305,33 +2302,29 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS) {
23052302
// should go back to IDLE
23062303
dispatchUntilState(State::IDLE);
23072304
ASSERT_CMD_RESPONSE_SIZE(1);
2308-
ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::OK);
2305+
ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED(), 0, Fw::CmdResponse::OK);
23092306

23102307
// Stack should be empty after discards
23112308
ASSERT_EQ(runtime->stack.size, static_cast<Fpy::StackSizeType>(0));
23122309

23132310
removeFile("test.bin");
23142311
}
23152312

2316-
TEST_F(FpySequencerTester, cmd_RUN_VALIDATED_ARGS_oversized) {
2313+
TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_oversized) {
23172314
allocMem();
23182315
add_NO_OP();
23192316
writeToFile("test.bin");
2320-
sendCmd_VALIDATE(0, 0, Fw::String("test.bin"));
2321-
dispatchUntilState(State::AWAITING_CMD_RUN_VALIDATED);
2322-
this->clearHistory();
23232317

23242318
// Create args that exceed MAX_STACK_SIZE
23252319
Svc::SeqArgs largeArgs{0, 0};
23262320
largeArgs.set_size(Fpy::MAX_STACK_SIZE + 1);
23272321

2328-
sendCmd_RUN_VALIDATED_ARGS(0, 0, FpySequencer_BlockState::BLOCK, largeArgs);
2329-
this->tester_doDispatch();
2330-
ASSERT_CMD_RESPONSE_SIZE(0);
2331-
// should fail when trying to push args to stack
2322+
sendCmd_VALIDATE_ARGS(0, 0, Fw::String("test.bin"), largeArgs);
2323+
dispatchUntilState(State::VALIDATING);
2324+
// should fail during validation when checking args size
23322325
dispatchUntilState(State::IDLE);
23332326
ASSERT_CMD_RESPONSE_SIZE(1);
2334-
ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_RUN_VALIDATED_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR);
2327+
ASSERT_CMD_RESPONSE(0, Svc::FpySequencerTester::get_OPCODE_VALIDATE_ARGS(), 0, Fw::CmdResponse::EXECUTION_ERROR);
23352328
ASSERT_from_seqDoneOut_SIZE(1);
23362329
ASSERT_from_seqDoneOut(0, 0, 0, Fw::CmdResponse::EXECUTION_ERROR);
23372330

Svc/FpySequencer/test/ut/FpySequencerTester.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,12 +283,12 @@ class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test
283283
//! Get the OPCODE_VALIDATE value
284284
static FwOpcodeType get_OPCODE_VALIDATE() { return FpySequencerComponentBase::OPCODE_VALIDATE; }
285285

286+
//! Get the OPCODE_VALIDATE_ARGS value
287+
static FwOpcodeType get_OPCODE_VALIDATE_ARGS() { return FpySequencerComponentBase::OPCODE_VALIDATE_ARGS; }
288+
286289
//! Get the OPCODE_RUN_VALIDATED value
287290
static FwOpcodeType get_OPCODE_RUN_VALIDATED() { return FpySequencerComponentBase::OPCODE_RUN_VALIDATED; }
288291

289-
//! Get the OPCODE_RUN_VALIDATED_ARGS value
290-
static FwOpcodeType get_OPCODE_RUN_VALIDATED_ARGS() { return FpySequencerComponentBase::OPCODE_RUN_VALIDATED_ARGS; }
291-
292292
//! Get the OPCODE_CANCEL value
293293
static FwOpcodeType get_OPCODE_CANCEL() { return FpySequencerComponentBase::OPCODE_CANCEL; }
294294

0 commit comments

Comments
 (0)