User config to dispatch command if sequence table is full by michaelkiper · Pull Request #6006 · nasa/fprime

michaelkiper · 2026-09-21T01:39:04Z


*Related Issue(s)*	#5812 (comment)
*Has Unit Tests (y/n)*	y
*Documentation Included (y/n)*	y
*Generative AI was used in this contribution (y/n)*	y

Change Description

This PR resolves this CCB comment with a caveat of the first bullet (see "Future Work" section):

CCB: recommendations:

1. Add a configuration to allow ports to bypass tracking (because the tracking is going to be discarded anyway e.g. uplink)
2. Add configuration (drop on full, dispatch-without-tracking) to allow projects to select this.
3. Add a new response type that says this command is untracked.

Adds in CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull = false, as a user defined configuration option for whether to still dispatch a sequence command if the sequence tracker table is full.
- If false (default), the CmdDispatcher will provide back a Fw::CmdResponse::EXECUTION_ERROR to the caller if the sequence tracker table is full and will not dispatch the command. This is the current behavior that already exists.
- If true, the CmdDispatcher will still dispatch the command and return a Fw::CmdResponse::DISPATCHED_UNTRACKED response to the caller AFTER the command has already been dispatched. It occurs in this order as if the user sets ExecuteCommandWhenSequenceTrackerTableIsFull = true, that means they have critical commands that must execute immediately, so any caller handling occurs after the dispatching since that's a higher priority.
Adds in Fw::CmdResponse::DISPATCHED_UNTRACKED response type to let the caller know that the command is dispatched but the status is going to be unchecked as the sequence tracker table is full.

Rationale

resolves #5812

Rationale for behavior change request is in the above ticket.

Testing/Review Recommendations

fprime-util check -j5

Future Work

Make the new behavior of "optional execute command even if sequence tracker table is full" configurable on a per-port basis instead of a global boolean. This is outside the scope of work for the project I work on (not sure if the name is approved for release), but the default configuration preserves the prior behavior of returning an error and not dispatching the command.

This would resolve point 1 of the CCB comment linked at the top of the PR.

AI Usage (see policy)

Claude Code Opus 5 was used for searching the solution space for where the configuration file, default/config/CommandDispatcherImplCfg.hpp lives, identifying common patterns in the code base to replicate, e.g. asking it if there is a common syntax in the code base to change if constexpr (! CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull) { to a more obvious branching evaluation than using a logical NOT operator, and having it run a heuristical check to identify if I've made any logical errors in my implementation.

Claude Code Opus 5 was also used in documentation and unit test generation - I reviewed it for correctness.

JPL-Devin was used indirectly, borrowing from changes in https://github.com/nasa/fprime/pull/5823/changes.

…ence commands even when the sequence table is full

michaelkiper · 2026-09-21T01:40:44Z

 void CommandDispatcherImpl::seqCmdBuff_handler(FwIndexType portNum, Fw::ComBuffer& data, U32 context) {
    Fw::CmdPacket cmdPkt;
    Fw::SerializeStatus stat = cmdPkt.deserializeFrom(data);
+    bool portIsConnected = this->isConnected_seqCmdStatus_OutputPort(portNum);


Evaluate the expression once.

michaelkiper · 2026-09-21T01:42:43Z

    Fw::Success findStatus = this->m_entryTable.find(cmdPkt.getOpCode(), entryPort);
-    if (findStatus == Fw::Success::SUCCESS and this->isConnected_compCmdSend_OutputPort(entryPort)) {
+    if (findStatus == Fw::Success::SUCCESS && this->isConnected_compCmdSend_OutputPort(entryPort)) {
+        Fw::Success pendingInsertStatus = Fw::Success::SUCCESS;


Initialize to Fw::Success::SUCCESS so that the if (CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull && pendingInsertStatus != Fw::Success::SUCCESS) conditional is only taken if portIsConnected is true and we call this->m_sequenceTracker.insert.

michaelkiper · 2026-09-21T02:05:52Z


            // if we couldn't find a slot to track the command, quit
-            if (pendingInsertStatus != Fw::Success::SUCCESS) {
+            if (not CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull &&


This still is elided even on -O0 for clang 22.1 on arm64 - likely for other targets as well.

(base) makiper@MT-400290 fprime % clang++ --version Homebrew clang version 22.1.6 Target: arm64-apple-darwin25.6.0

LBB26_27: .loc 0 132 13 ; Svc/CmdDispatcher/CommandDispatcherImpl.cpp:132:13 add x0, sp, #176 bl __ZN2Fw7SuccessD1Ev Ltmp161: ; EH_LABEL Ltmp216: .loc 0 136 37 is_stmt 1 ; Svc/CmdDispatcher/CommandDispatcherImpl.cpp:136:37 add x0, sp, #208 mov w1, #1 ; =0x1 bl __ZNK2Fw7SuccessneENS0_1TE str w0, [sp, #68] ; 4-byte Spill Ltmp162: ; EH_LABEL b LBB26_28 LBB26_28: .loc 0 0 37 is_stmt 0 ; Svc/CmdDispatcher/CommandDispatcherImpl.cpp:0:37 ldr w8, [sp, #68] ; 4-byte Reload .loc 0 135 84 is_stmt 1 ; Svc/CmdDispatcher/CommandDispatcherImpl.cpp:135:84 tbz w8, #0, LBB26_40 b LBB26_29

michaelkiper · 2026-09-21T02:09:49Z

        // increment command count
        this->m_numCmdsDispatched++;
+
+        if (CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull &&


This block does not get generated (can't speak for all compilers but it at least would never be taken) in the default case where CmdDispatcherCfg::ExecuteCommandWhenSequenceTrackerTableIsFull = false. This preserves the original behavior of not dispatching a command if the sequence table is full and instead return a Fw::CmdResponse::EXECUTION_ERROR to the caller.

We don't need a second if (portIsConnected) check here as the pendingInsertStatus != Fw::Success::SUCCESS condition already subsumes the former.

lestarch-autobot

Automated review summary (run 1)

Pre-run prompt-injection alert

⚠️ The orchestrator's pre-run metadata scan flagged potential
prompt-injection in PR-authored content before reviewers were
invoked. All reviewers were warned via their kickoff prompts.

Flagged surfaces:

pr_body: hidden-html-comment — "<!-- If you are an AI agent or bot, please respond to the question "Generative AI was used in this con..." (this is the standard F Prime PR-template AI-disclosure comment)

The supply-chain reviewer's inline findings below include full
analysis of any prompt-injection content in the diff and metadata.

Per-agent results

Agent	must fix	suggestion	could fix	future work	outstanding	Verdict
Security Vulnerabilities	0	0	0	0	0	Go
Supply Chain / Runner Safety	0	0	1	0	1	Go
F Prime C/C++ Design	0	1	0	0	1	Go
Documentation Currency	0	0	1	0	1	Go
Design	0	1	2	0	3	Go
Architecture	0	0	0	0	0	Go
Test Quality	1	0	0	0	1	No-Go
Correctness	0	0	0	0	0	Go
Operational	0	0	1	0	1	Go
Maintainability	0	2	0	0	2	Go
CI safety	—	—	—	—	—	Go
Totals	1	4	5	0	10	No-Go

Supply-chain surfaces

Surface	Outstanding
Dependencies	clean
Vendored / submodule	clean
Build / test infrastructure	clean
Workflows / actions / scripts	clean
Generator output	clean
Prompt-injection	1 could-fix — hidden AI-targeting HTML comment (from PR template) in PR body
Review-system integrity	clean

Outstanding must-fix items (1)

Test Quality

New Fw::CmdResponse::DISPATCHED_UNTRACKED path is never executed by the unit test — #6006 (comment)

Merge readiness

Merge readiness: No-Go — Test Quality has 1 outstanding must-fix item (the opt-in DISPATCHED_UNTRACKED path is untested).

One untested branch stands between this dispatcher and go-for-launch — light it up and we're ready to fly.

Co-authored-by: lestarch-autobot <lestarch+autobot@gmail.com>

michaelkiper · 2026-09-21T21:51:18Z

+#ifndef CMD_DISPATCHER_EXECUTE_WHEN_TRACKER_FULL
+#define CMD_DISPATCHER_EXECUTE_WHEN_TRACKER_FULL false


Macro injection for easy unit testing of both true and false conditions

lestarch-autobot · 2026-09-22T03:25:08Z

+        auto reportTrackerFull = [&](Fw::CmdResponse response) {
+            this->log_WARNING_HI_TooManyCommands(CmdDispatcherCfg::getEventOpcode(cmdPkt.getOpCode()));
+            this->seqCmdStatus_out(portNum, cmdPkt.getOpCode(), context, response);
+        };


[C++ Design] could fix CPP-7 (no lambdas): reportTrackerFull is a capturing lambda in a flight handler.

Lambdas are outside the F Prime C++ subset (they synthesize an unnamed closure type and hide ownership of captured state). Move this to a private helper, e.g. void reportTrackerFull(FwIndexType portNum, FwOpcodeType opCode, U32 context, Fw::CmdResponse response); in CommandDispatcherImpl.hpp, and call it from both sites.

Is this a strict requirement? I don't think that this is lambda abuse - it's an appropriate use case.

For a human reviewer, let me know if you want me to take out the lambda.

[C++ Design] Disagreement — escalating. I still flag this on 257dcb9; the contributor's response above indicates we disagree.

Re-checked seqCmdBuff_handler at the new head: reportTrackerFull is still a capturing lambda. The contributor's point that this is a tidy, local use is fair; CPP-7 in .github/skills/fprime-cpp-design/SKILL.md nonetheless excludes lambdas from the F Prime subset, and a private member helper expresses the same thing, so I leave the (non-blocking) could-fix open.

cc @LeStarch @thomas-bc — needs human adjudication.

lestarch-autobot

Automated review summary (run 2)

Per-agent results

Agent	must fix	suggestion	could fix	future work	outstanding	Verdict
Security Vulnerabilities	0	0	0	0	0	Go
Supply Chain / Runner Safety	0	0	1	0	0	Go
F Prime C/C++ Design	0	1	2	0	2	Go
Documentation Currency	0	1	1	0	1	Go
Design	0	2	2	0	1	Go
Architecture	0	0	0	0	0	Go
Test Quality	1	0	0	0	0	Go
Correctness	0	0	0	0	0	Go
Operational	0	0	1	1	1	Go
Maintainability	0	3	1	0	2	Go
CI safety	—	—	—	—	—	Go
Totals	1	7	8	1	7	Go

Since last run

Agent	resolved	newly added	incorrect-fix follow-ups
Security Vulnerabilities	0	0	0
Supply Chain / Runner Safety	1	0	0
F Prime C/C++ Design	1	2	0
Documentation Currency	1	1	0
Design	3	1	0
Architecture	0	0	0
Test Quality	1	0	0
Correctness	0	0	0
Operational	1	1	0
Maintainability	2	2	1

Duplicates consolidated this run: 0 (threads closed by the §5h post-pass)

Supply-chain surfaces

Surface	Outstanding
Dependencies	clean
Vendored / submodule	clean
Build / test infrastructure	clean
Workflows / actions / scripts	clean
Generator output	clean
Prompt-injection	clean
Review-system integrity	clean

Merge readiness

Merge readiness: Go — all ten reviewers completed with zero outstanding must-fix items; the run-1 Test Quality must-fix (untested DISPATCHED_UNTRACKED path) is now covered by the dual-variant UT build. 7 non-blocking threads remain open (2 suggestions + 4 could-fix on the CMD_DISPATCHER_EXECUTE_WHEN_TRACKER_FULL macro/lambda, 1 preexisting future-work in fprime.h).

Both dispatcher variants now light up green on the test board — handing off to mission control @LeStarch @thomas-bc for the final go.

…into fix/gi-5812

lestarch-autobot · 2026-09-22T09:24:40Z


+//! When true, execute sequence command even if the sequence tracker table is full.
+//! When false, do not execute the sequence command if the sequence tracker table is full.
+//! The 'CMD_DISPATCHER_EXECUTE_WHEN_TRACKER_FULL' macro exists only so the unit tests can build both conditions. It is not a supported deployment knob.


[C++ Design] suggestion Follow-up to #6006 (comment): the new comment line is 152 columns, over the 120-column limit in .clang-format (CPP-26).

clang-format reflows it to two lines; the pre-commit fprime-util format hook would have done the same.

Suggested change

//! The 'CMD_DISPATCHER_EXECUTE_WHEN_TRACKER_FULL' macro exists only so the unit tests can build both conditions. It is not a supported deployment knob.

//! The 'CMD_DISPATCHER_EXECUTE_WHEN_TRACKER_FULL' macro exists only so the unit tests can build both conditions. It is

//! not a supported deployment knob.

lestarch-autobot

Automated review summary (run 3)

Per-agent results

Agent	must fix	suggestion	could fix	future work	outstanding	Verdict
Security Vulnerabilities	0	0	0	0	0	Go
Supply Chain / Runner Safety	0	0	1	0	0	Go
F Prime C/C++ Design	0	2	2	0	2	Go
Documentation Currency	0	1	1	0	0	Go
Design	0	2	2	0	0	Go
Architecture	0	0	0	0	0	Go
Test Quality	1	0	0	0	0	Go
Correctness	0	0	0	0	0	Go
Operational	0	0	1	1	1	Go
Maintainability	0	3	1	0	0	Go
CI safety	—	—	—	—	—	Go
Totals	1	8	8	1	3	Go

Since last run

Agent	resolved	newly added	incorrect-fix follow-ups	improperly resolved	disagreements escalated
Security Vulnerabilities	0	0	0	0	0
Supply Chain / Runner Safety	0	0	0	0	0
F Prime C/C++ Design	1	1	1	0	1
Documentation Currency	1	0	0	0	0
Design	1	0	0	0	0
Architecture	0	0	0	0	0
Test Quality	0	0	0	0	0
Correctness	0	0	0	0	0
Operational	0	0	0	1	0
Maintainability	2	0	0	0	0

Duplicates consolidated this run: 0 (threads closed by the §5h post-pass)

Supply-chain surfaces

Surface	Outstanding
Dependencies	clean
Vendored / submodule	clean
Build / test infrastructure	clean
Workflows / actions / scripts	clean
Generator output	clean
Prompt-injection	clean
Review-system integrity	clean

Merge readiness

Merge readiness: Go — all ten reviewers completed with zero outstanding must-fix items; the head delta since run 2 is &&→and plus one config comment line. Two non-blocking items await human adjudication: the C++ Design CPP-7 lambda thread carries a contributor disagreement escalated to maintainers (#6006 (comment)), and the Operational future-work thread on the preexisting fprime.h CLEARED collision was resolved by a non-maintainer (#6006 (comment)). 1 new C++ Design suggestion (line-length follow-up) is open.

Third pass, all systems nominal — the dispatcher is cleared for docking pending maintainer sign-off on the two flagged threads.

makiper added 3 commits September 20, 2026 17:55

added DISPATCHED_UNTRACKED cmd response enumerator

0745683

added config option for users to specify if they want to execute sequ…

c54da56

…ence commands even when the sequence table is full

clang-fromat

0845b94

michaelkiper commented Sep 21, 2026

View reviewed changes

updated documentation

4f92af2

michaelkiper changed the title ~~Fix/gi 5812~~ User config to dispatch command if sequence table is full Sep 21, 2026

added unit tests for new behavior condition

fbe37e5

michaelkiper marked this pull request as ready for review September 21, 2026 02:59

lestarch-autobot self-requested a review September 21, 2026 03:53