ComQueue queue ordering, drop mode, & command to reprioritize by Willmac16 · Pull Request #4674 · nasa/fprime

Willmac16 · 2026-01-30T18:39:20Z


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

Change Description

Adds the abilities to change the ordering and drop mode of each queue in the com queue at compile time.

Also adds the ability to reprioritize queues w/ a command

Rationale

Many channels benefit from alternative queue ordering & drop mode (e.g. new telemetry is likely more important to preserve than stale telemetry). Additionally, mission demands like anomaly resolution may require the ability to reonfigure the priority of different data streams into the com queue.

Testing/Review Recommendations

Ensure the return scheme for Queue DROP_OLDEST mode feels appropriate. Decide if compile time vs command-able split in features matches expected usage. Verify that testing looks sufficient.

Future Work

N/A

AI Usage (see policy)

Claude Code for the Web (Sonnet 4.5) was used to perform initial implementation. I went through and read, reorganized, & refactored the code

…mQueue This change adds the ability to configure each ComQueue input channel with: - Queue mode: FIFO (First-In-First-Out) or LIFO (Last-In-First-Out) - Overflow mode: DROP_NEWEST or DROP_OLDEST Changes: - Added QueueMode and OverflowMode enums to ComQueue.fpp - Extended QueueConfigurationEntry with mode and overflowMode fields - Modified Types::Queue to support LIFO dequeue and drop oldest overflow - Added CircularBuffer::trim() method for removing from the back - Updated ComQueue to pass mode configuration to underlying queues The mode and overflow behavior are configured at compile time through the QueueConfigurationTable passed to the configure() method. Priority can be adjusted at initialization by modifying the configuration table. Note: Runtime priority changes would require adding a command interface, which has been deferred due to FPP syntax compatibility considerations.

- Added unit tests for Queue FIFO/LIFO modes - Added unit tests for Queue DROP_NEWEST/DROP_OLDEST overflow modes - Added tests for all combinations (FIFO+DROP_OLDEST, LIFO+DROP_OLDEST) - Added CircularBuffer::trim() tests (TrimOkRule, TrimBadRule) - Added trim() method to CircularState for test infrastructure - Updated CMakeLists.txt to include Queue_ut test target - All 10 Queue tests and 10 CircularBuffer tests passing

Tests added: - FIFO mode verification for COM queues - LIFO mode verification for COM queues - DROP_NEWEST overflow mode (rejects when full) - DROP_OLDEST overflow mode (drops oldest to make room) - LIFO + DROP_OLDEST combined mode - FIFO mode verification for Buffer queues - LIFO mode verification for Buffer queues - DROP_OLDEST overflow mode for Buffer queues Implementation: - Added 8 new test methods to ComQueueTester class - Added corresponding TEST cases in ComQueueTestMain - Included QueueModeEnumAc.hpp and OverflowModeEnumAc.hpp in ComQueue.hpp - All 15 tests passing (7 existing + 8 new) Coverage: - Tests both COM and Buffer queue types - Tests all combinations of queue modes and overflow modes - Verifies correct ordering (FIFO vs LIFO) - Verifies overflow behavior (DROP_NEWEST vs DROP_OLDEST) - Validates events and buffer returns

Added SET_QUEUE_PRIORITY command to ComQueue component that allows runtime adjustment of queue priorities. The command: - Validates queue index and priority values - Updates the priority in the m_prioritizedList - Re-sorts the prioritized list to maintain ordering - Emits QueuePriorityChanged event on success - Emits InvalidQueueIndex event and returns VALIDATION_ERROR for invalid inputs Also added comprehensive unit tests covering: - Successful priority changes - Invalid queue index handling - Invalid priority value handling All 18 unit tests pass.

Refactored to use Types::QueueMode and Types::QueueOverflowMode from the Queue class instead of duplicating enum definitions in ComQueue.fpp. This change: - Removed QueueMode and OverflowMode enum definitions from ComQueue.fpp - Removed includes of QueueModeEnumAc.hpp and OverflowModeEnumAc.hpp - Updated QueueConfigurationEntry and QueueMetadata to use Types:: enums - Removed enum conversion code in configure() method - Updated all test code to use Types::QUEUE_FIFO, Types::QUEUE_LIFO, Types::QUEUE_DROP_NEWEST, and Types::QUEUE_DROP_OLDEST All 18 unit tests continue to pass.

… data dropped

name in enqueue

Utils/Types/test/ut/CircularBuffer/CircularRules.hpp

+ */
+struct TrimOkRule : public STest::Rule<MockTypes::CircularState> {
+    // Constructor
+    TrimOkRule(const char* const name);


Utils/Types/test/ut/CircularBuffer/CircularRules.hpp

+ */
+struct TrimBadRule : public STest::Rule<MockTypes::CircularState> {
+    // Constructor
+    TrimBadRule(const char* const name);


Svc/ComQueue/ComQueue.cpp

    this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::OK);
 }

+void ComQueue::SET_QUEUE_PRIORITY_cmdHandler(FwOpcodeType opCode,


Utils/Types/Queue.cpp

 namespace Types {

-Queue::Queue() : m_internal(), m_message_size(0) {}
+Queue::Queue() : m_internal(), m_message_size(0), m_mode(QUEUE_FIFO), m_overflow_mode(QUEUE_DROP_NEWEST) {}


Utils/Types/Queue.cpp

+            return Fw::FW_DESERIALIZE_BUFFER_EMPTY;
+        }
+        FwSizeType offset = current_size - m_message_size;
+        result = m_internal.peek(message, m_message_size, offset);


Utils/Types/Queue.cpp

+
+    if (m_mode == QUEUE_FIFO) {
+        // FIFO: Dequeue from the front (oldest message)
+        result = m_internal.peek(message, m_message_size, 0);


Utils/Types/Queue.cpp

              static_cast<FwAssertArgType>(m_message_size));  // Message size is as expected
-    return m_internal.serialize(message, m_message_size);
+
+    Fw::SerializeStatus status = m_internal.serialize(message, m_message_size);


Utils/Types/Queue.cpp

    m_internal.setup(storage, total_needed_size);
    m_message_size = message_size;
+    m_mode = mode;
+    m_overflow_mode = overflow_mode;


Utils/Types/Queue.cpp

              static_cast<FwAssertArgType>(depth), static_cast<FwAssertArgType>(message_size));
    m_internal.setup(storage, total_needed_size);
    m_message_size = message_size;
+    m_mode = mode;


Svc/ComQueue/ComQueue.cpp

+    // Validate queue index
+    if (queueIndex < 0 || queueIndex >= TOTAL_PORT_COUNT) {
+        this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::VALIDATION_ERROR);
+        return;


Svc/ComQueue/ComQueue.cpp

+
+    // Validate queue index
+    if (queueIndex < 0 || queueIndex >= TOTAL_PORT_COUNT) {
+        this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::VALIDATION_ERROR);


Svc/ComQueue/ComQueue.cpp

+
+    // Validate queue index
+    if (queueIndex < 0 || queueIndex >= TOTAL_PORT_COUNT) {
+        this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::VALIDATION_ERROR);


Svc/ComQueue/ComQueue.cpp

    // Acquire the queue that we need to drain
-    FwIndexType queueIndex =
-        (queueType == QueueType::COM_QUEUE) ? index : static_cast<FwIndexType>(index + COM_PORT_COUNT);
+    FwIndexType queueIndex = this->getQueueNum(queueType, index);


Svc/ComQueue/ComQueue.cpp

    // Acquire the queue that we need to drain
-    FwIndexType queueIndex =
-        (queueType == QueueType::COM_QUEUE) ? index : static_cast<FwIndexType>(index + COM_PORT_COUNT);
+    FwIndexType queueIndex = this->getQueueNum(queueType, index);


Svc/ComQueue/ComQueue.cpp

+
+FwIndexType ComQueue::getQueueNum(Svc::QueueType queueType, FwIndexType portNum) {
+    // Acquire the queue that we need to drain
+    return static_cast<FwIndexType>(portNum + ((queueType == QueueType::COM_QUEUE) ? 0 : COM_PORT_COUNT));


LeStarch · 2026-02-02T19:27:49Z

Svc/ComQueue/ComQueueCommands.fppi

+async command SET_QUEUE_PRIORITY(
+                                queueType: QueueType @< The Queue data type
+                                indexType: FwIndexType @< The index of the queue (within the supplied type) to modify
+                                newPriority: U32 @< New priority value for the queue


This uses a U32.... Is this the right type?

Priority is set in the C++ as a FwIndexType. We should keep this for consistency, no?

I switched all the priority instances to FwIndexType. Good Catch! Now I'm wondering if there's anywhere that ensures the priority is >= 0 for the configuration on startup

It looks like negative priorities won't get populated, but I don't see anything that will explicitly warn you

Svc/ComQueue/ComQueueEvents.fppi

Svc/ComQueue/ComQueueTelemetry.fppi

Svc/ComQueue/ComQueue.cpp

Svc/ComQueue/ComQueueCommands.fppi

+async command SET_QUEUE_PRIORITY(
+                                queueType: QueueType @< The Queue data type
+                                indexType: FwIndexType @< The index of the queue (within the supplied type) to modify
+                                newPririty: FwIndexType @< New priority value for the queue


FwIndexType &#

priority

SET_QUEUE_PRIORITY

Svc/ComQueue/ComQueue.hpp

 // ----------------------------------------------------------------------

 class ComQueue final : public ComQueueComponentBase {
+    // Added to enable easy testing of queue reprioritization


LeStarch

@Willmac16 looks good to me, I would recommend a static assert that TOTAL_PORTS >= 1 for future proofing, but it looks good to me!

LeStarch · 2026-02-05T21:25:49Z

Svc/ComQueue/ComQueue.cpp

+            break;  // Since we shouldn't find more than one queue at this port index
+        }
+    }
+


Should assert (static_assert) TOTAL_PORT_COUNT >= 1

Svc/ComQueue/ComQueue.cpp

    this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::OK);
 }

+void ComQueue::SET_QUEUE_PRIORITY_cmdHandler(FwOpcodeType opCode,


Utils/Types/Queue.cpp

 namespace Types {

-Queue::Queue() : m_internal(), m_message_size(0) {}
+Queue::Queue() : m_internal(), m_message_size(0), m_mode(QUEUE_FIFO), m_overflow_mode(QUEUE_DROP_NEWEST) {}


Utils/Types/Queue.cpp

+            return Fw::FW_DESERIALIZE_BUFFER_EMPTY;
+        }
+        FwSizeType offset = current_size - m_message_size;
+        result = m_internal.peek(message, m_message_size, offset);


Utils/Types/Queue.cpp

+
+    if (m_mode == QUEUE_FIFO) {
+        // FIFO: Dequeue from the front (oldest message)
+        result = m_internal.peek(message, m_message_size, 0);


Utils/Types/Queue.cpp

              static_cast<FwAssertArgType>(m_message_size));  // Message size is as expected
-    return m_internal.serialize(message, m_message_size);
+
+    Fw::SerializeStatus status = m_internal.serialize(message, m_message_size);


Utils/Types/Queue.cpp

    m_internal.setup(storage, total_needed_size);
    m_message_size = message_size;
+    m_mode = mode;
+    m_overflow_mode = overflow_mode;


Utils/Types/Queue.cpp

              static_cast<FwAssertArgType>(depth), static_cast<FwAssertArgType>(message_size));
    m_internal.setup(storage, total_needed_size);
    m_message_size = message_size;
+    m_mode = mode;


Svc/ComQueue/ComQueue.cpp

+    // Validate queue index
+    if (queueIndex < 0 || queueIndex >= TOTAL_PORT_COUNT) {
+        this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::VALIDATION_ERROR);
+        return;


Svc/ComQueue/ComQueue.cpp

+
+    // Validate queue index
+    if (queueIndex < 0 || queueIndex >= TOTAL_PORT_COUNT) {
+        this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::VALIDATION_ERROR);


Svc/ComQueue/ComQueue.cpp

+
+    // Validate queue index
+    if (queueIndex < 0 || queueIndex >= TOTAL_PORT_COUNT) {
+        this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::VALIDATION_ERROR);


Svc/ComQueue/ComQueue.cpp

    // Acquire the queue that we need to drain
-    FwIndexType queueIndex =
-        (queueType == QueueType::COM_QUEUE) ? index : static_cast<FwIndexType>(index + COM_PORT_COUNT);
+    FwIndexType queueIndex = this->getQueueNum(queueType, index);


Svc/ComQueue/ComQueue.cpp

    // Acquire the queue that we need to drain
-    FwIndexType queueIndex =
-        (queueType == QueueType::COM_QUEUE) ? index : static_cast<FwIndexType>(index + COM_PORT_COUNT);
+    FwIndexType queueIndex = this->getQueueNum(queueType, index);


Utils/Types/test/ut/CircularBuffer/CircularRules.hpp

+ */
+struct TrimOkRule : public STest::Rule<MockTypes::CircularState> {
+    // Constructor
+    TrimOkRule(const char* const name);


Utils/Types/test/ut/CircularBuffer/CircularRules.hpp

+ */
+struct TrimBadRule : public STest::Rule<MockTypes::CircularState> {
+    // Constructor
+    TrimBadRule(const char* const name);


claude and others added 17 commits January 30, 2026 10:29

Add build-ut/ to .gitignore

cc8de5e

Rearrange cladue additions

4d8ca15

Port Utils/Types Cmake to new register_fprime_* format

55f8caf

Add DISCARDED_EXISTING to Fw::SerializeStatus

8f9974b

Queue now returns FW_SERIALIZE_DISCARDED_EXISTING if in DROP_OLDEST &…

01f99e0

… data dropped

Redo ComQueue Reprioritize Indexing

5c0530e

Remove "the the"

df2368f

Revert build-ut gitignore

52222c9

Put the command handlers back where they were

56d8f4b

Check & event upon the new queue return code

fc2e329

Update docs & rename getUnifiedQueueIndex to getQueueNum to match var

fc3cdd3

name in enqueue

More docs updates

3093d14

github-advanced-security bot found potential problems Feb 2, 2026

View reviewed changes

Willmac16 marked this pull request as ready for review February 2, 2026 14:26

Fix tweak to getQueueNum ternary

9430e48

github-advanced-security bot found potential problems Feb 2, 2026

View reviewed changes

LeStarch reviewed Feb 2, 2026

View reviewed changes

Willmac16 added 2 commits February 3, 2026 07:46

Correct Types & Alignment

07628d6

Type corrections in the cpp

e3405b0

github-advanced-security bot found potential problems Feb 3, 2026

View reviewed changes

Willmac16 added 3 commits February 3, 2026 07:52

Validate commanded priority is non-negative & switch UT priority to

ca8093a

FwIndexType &#

Don't comflate priority index & queue index + avoid static casting

02cf048

priority

Test for negative command args & ensure queues are sorted post

b8aaeb5

SET_QUEUE_PRIORITY

github-advanced-security bot found potential problems Feb 3, 2026

View reviewed changes

Spelling

6fccfc9

LeStarch previously approved these changes Feb 5, 2026

View reviewed changes

github-advanced-security bot found potential problems Feb 5, 2026

View reviewed changes

Add TOTAL_PORT_COUNT static assert

b8aaef9

Willmac16 dismissed LeStarch’s stale review via b8aaef9 February 5, 2026 22:27

formatting

00f04d6

Conversation

Willmac16 commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Description

Rationale

Testing/Review Recommendations

Future Work

AI Usage (see policy)

Uh oh!

Check warning

Check warning

Check notice

Check notice

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

LeStarch Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

LeStarch Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Willmac16 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Willmac16 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check failure

Check failure

LeStarch left a comment

Choose a reason for hiding this comment

Uh oh!

LeStarch Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Check notice

Check notice

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Willmac16 commented Jan 30, 2026 •

edited

Loading