From b06a9c7635c45d68a6ff769ee49d02e1e1ce53e6 Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Mon, 12 Oct 2026 00:32:08 +0000 Subject: [PATCH 01/13] Svc/GenericHub: drop message with event and counter on allocation failure instead of asserting Closes #6166 --- Svc/GenericHub/GenericHub.cpp | 31 +++- Svc/GenericHub/GenericHub.fpp | 32 ++++ Svc/GenericHub/GenericHub.hpp | 15 +- Svc/GenericHub/docs/sdd.md | 49 +++++- Svc/GenericHub/test/ut/GenericHubTestMain.cpp | 10 ++ Svc/GenericHub/test/ut/GenericHubTester.cpp | 141 +++++++++++++++++- Svc/GenericHub/test/ut/GenericHubTester.hpp | 25 ++++ .../design-patterns/hub-pattern.md | 7 +- 8 files changed, 295 insertions(+), 15 deletions(-) diff --git a/Svc/GenericHub/GenericHub.cpp b/Svc/GenericHub/GenericHub.cpp index 5ad46cd8b28..af35df1fd80 100644 --- a/Svc/GenericHub/GenericHub.cpp +++ b/Svc/GenericHub/GenericHub.cpp @@ -12,6 +12,7 @@ #include #include +#include "Fw/Cmd/CmdPacket.hpp" #include "Fw/Logger/Logger.hpp" #include "Fw/Types/Assert.hpp" @@ -24,15 +25,25 @@ namespace Svc { // Construction, initialization, and destruction // ---------------------------------------------------------------------- -GenericHub::GenericHub(const char* const compName) : GenericHubComponentBase(compName) {} +GenericHub::GenericHub(const char* const compName) : GenericHubComponentBase(compName), m_numAllocationFailures(0) {} GenericHub::~GenericHub() {} -void GenericHub::send_data(const HubType type, const FwIndexType port, const U8* data, const FwSizeType size) { +bool GenericHub::send_data(const HubType type, const FwIndexType port, const U8* data, const FwSizeType size) { FW_ASSERT(data != nullptr); Fw::SerializeStatus status; + const FwSizeType needed = size + sizeof(U32) + sizeof(U32) + sizeof(FwBuffSizeType); // Buffer to send and a buffer used to write to it - Fw::Buffer outgoing = allocate_out(0, static_cast(size + sizeof(U32) + sizeof(U32) + sizeof(FwBuffSizeType))); + Fw::Buffer outgoing = allocate_out(0, needed); + // Allocation may fail (e.g. pool exhausted): drop the message rather than assert + if ((outgoing.getData() == nullptr) || (outgoing.getSize() < needed)) { + if (outgoing.getData() != nullptr) { + deallocate_out(0, outgoing); + } + this->m_numAllocationFailures++; + this->log_WARNING_HI_BufferAllocationFailed(static_cast(type), port, needed); + return false; + } auto serialize = outgoing.getSerializer(); // Write data to our buffer status = serialize.serializeFrom(static_cast(type)); @@ -43,6 +54,7 @@ void GenericHub::send_data(const HubType type, const FwIndexType port, const U8* FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast(status)); outgoing.setSize(serialize.getSize()); toBufferDriver_out(0, outgoing); + return true; } // ---------------------------------------------------------------------- @@ -74,7 +86,14 @@ void GenericHub ::cmdDispIn_handler(FwIndexType portNum, Fw::ComBuffer& data, U3 FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast(status)); FwSizeType size = serializer.getSize(); - this->send_data(HUB_TYPE_CMD_DISP, portNum, buffer, size); + if (!this->send_data(HUB_TYPE_CMD_DISP, portNum, buffer, size)) { + // The command was dropped: fail it back to the caller so it is not left waiting for a response + Fw::CmdPacket cmdPkt; + (void)cmdPkt.deserializeFrom(data); // opcode is 0 if the packet is malformed, as in CmdDispatcher + if (this->isConnected_cmdRespOut_OutputPort(portNum)) { + this->cmdRespOut_out(portNum, cmdPkt.getOpCode(), context, Fw::CmdResponse::EXECUTION_ERROR); + } + } } void GenericHub ::cmdRespIn_handler(FwIndexType portNum, @@ -252,6 +271,10 @@ void GenericHub::fromBufferDriver_handler(const FwIndexType portNum, Fw::Buffer& } } +void GenericHub::schedIn_handler(FwIndexType portNum, U32 context) { + this->tlmWrite_NumAllocationFailures(this->m_numAllocationFailures); +} + void GenericHub::toBufferDriverReturn_handler(FwIndexType portNum, Fw::Buffer& fwBuffer) { // Deallocate the existing buffer deallocate_out(portNum, fwBuffer); diff --git a/Svc/GenericHub/GenericHub.fpp b/Svc/GenericHub/GenericHub.fpp index 6a3856497c6..6db8ebb1571 100644 --- a/Svc/GenericHub/GenericHub.fpp +++ b/Svc/GenericHub/GenericHub.fpp @@ -209,6 +209,38 @@ module Svc { @ remote command response sync input port cmdRespIn: [CmdDispatcherSequencePorts] Fw.CmdResponse + # ---------------------------------------------------------------------- + # Scheduling, events, and telemetry + # ---------------------------------------------------------------------- + + @ Schedule input port, used to emit telemetry + sync input port schedIn: Svc.Sched + + @ Event port + event port Log + + @ Text event port + text event port LogText + + @ Time get port + time get port Time + + @ Telemetry port + telemetry port Tlm + + @ Buffer allocation failed; the outgoing message was dropped + event BufferAllocationFailed( + msgType: U32 @< The hub message type (HubType) of the dropped message + $port: FwIndexType @< The input port index of the dropped message + $size: FwSizeType @< The requested allocation size + ) \ + severity warning high \ + format "Failed to allocate {} byte buffer for message type {} on port {}" \ + throttle 5 + + @ The number of failed buffer allocations + telemetry NumAllocationFailures: U32 update on change + } } diff --git a/Svc/GenericHub/GenericHub.hpp b/Svc/GenericHub/GenericHub.hpp index 14d3ea0e42d..b194b3b419a 100644 --- a/Svc/GenericHub/GenericHub.hpp +++ b/Svc/GenericHub/GenericHub.hpp @@ -13,6 +13,7 @@ #ifndef Svc_GenericHub_HPP #define Svc_GenericHub_HPP +#include #include "Svc/GenericHub/GenericHubComponentAc.hpp" namespace Svc { @@ -86,6 +87,13 @@ class GenericHub final : public GenericHubComponentBase { void fromBufferDriver_handler(const FwIndexType portNum, /*!< The port number*/ Fw::Buffer& fwBuffer) override; + //! Handler implementation for schedIn + //! + //! Schedule input port, used to emit telemetry + void schedIn_handler(FwIndexType portNum, //!< The port number + U32 context //!< The call order + ) override; + //! Handler implementation for toBufferDriverReturn //! //! Port for receiving buffers sent on toBufferDriver and then returned @@ -121,7 +129,12 @@ class GenericHub final : public GenericHubComponentBase { ) override; // Helpers and members - void send_data(const HubType type, const FwIndexType port, const U8* data, const FwSizeType size); + + //! Frame and send data on toBufferDriver. Returns false if the message was dropped because no + //! sufficiently sized buffer could be allocated. + bool send_data(const HubType type, const FwIndexType port, const U8* data, const FwSizeType size); + + std::atomic m_numAllocationFailures; //!< Count of dropped messages; incremented from any caller thread }; } // end namespace Svc diff --git a/Svc/GenericHub/docs/sdd.md b/Svc/GenericHub/docs/sdd.md index b45591530ff..c79590903f3 100644 --- a/Svc/GenericHub/docs/sdd.md +++ b/Svc/GenericHub/docs/sdd.md @@ -105,6 +105,18 @@ genericHub.deallocate -> bufferManager.bufferSendIn `toBufferDriver` and `toBufferDriverReturn` come from `Drv.PassiveBufferDriverClientSend`. +### 2a. Hub health: scheduling, events, and telemetry + +| Port | Type | Purpose | +|---|---|---| +| `schedIn` | `Svc.Sched` | Rate-group input; emits the hub's own telemetry. | +| `Log`, `LogText`, `Time`, `Tlm` | special ports | The hub's own events, text events, time, and telemetry. | + +These ports report the hub's health; they are distinct from the pass-through +`eventIn`/`eventOut` and `tlmIn`/`tlmOut` ports. `schedIn` should be driven +from a rate group whenever the hub's event and telemetry ports are wired +through the hub itself (see [Allocation failure](#allocation-failure)). + ### 3. Buffer driver to hub: receive interface | Port | Type | Purpose | @@ -233,6 +245,27 @@ For a locally produced message, the hub: 4. receives ownership back through `toBufferDriverReturn`; and 5. returns the buffer to the manager through `deallocate`. +### Allocation failure + +If `allocate` returns an invalid buffer (null data, or smaller than the framed +message), the hub drops the message: it returns a non-null undersized buffer +through `deallocate`, increments the `NumAllocationFailures` counter, and emits +the `BufferAllocationFailed` event (throttled to 5) with the message type, port +index, and requested size. Buffers received on `bufferIn` are still returned +through `bufferInReturn`. The hub remains operational and resumes sending once +allocation succeeds. A dropped `cmdDispIn` command is failed back to its caller +with `Fw::CmdResponse::EXECUTION_ERROR` on the matching `cmdRespOut` port; a +dropped `cmdRespIn` response cannot be reported to the far side and is lost. + +The counter is read from any caller thread (`std::atomic`) and written to +telemetry only from `schedIn`. Emitting telemetry on the failure path would +re-enter the hub whenever the hub is the deployment's telemetry sink, so the +`schedIn` port is required for the channel to be observable. + +Size the pool for the worst case: the capacity of every queue downstream of +`toBufferDriver`, plus buffers held by the driver, plus one per thread that +can call the hub's input ports concurrently. + For `bufferIn`, the hub copies the incoming data into the newly allocated transport buffer and immediately returns the original buffer through the matching `bufferInReturn` port. The hub never sends the original pointer @@ -291,12 +324,9 @@ that handling around or within the component. Received messages with an invalid type, size, or port index, or for an unconnected output port, are dropped without an event or telemetry. -GenericHub does not check the buffer returned by `allocate`. If the allocator -returns an invalid buffer (for example, `Svc::BufferManager` with no free -buffer of sufficient size), serialization fails a `FW_ASSERT`. Size the pool for -the worst case: the capacity of every queue downstream of `toBufferDriver`, -plus buffers held by the driver, plus one per thread that can call the hub's -input ports concurrently. +Outgoing messages are dropped when the allocator returns an invalid buffer +(for example, `Svc::BufferManager` with no free buffer of sufficient size); see +[Allocation failure](#allocation-failure). The hub is not itself an event source or telemetry database. Its `eventOut` and `tlmOut` ports still need to be wired to the deployment's event manager and @@ -311,6 +341,12 @@ telemetry database, respectively. | GENHUB-003 | The GenericHub shall deserialize incoming transport buffers to output port and buffer calls. | Unit test | | GENHUB-004 | The GenericHub shall work with another GenericHub to send port and buffer calls. | Unit test | | GENHUB-005 | The GenericHub shall forward command dispatches and command responses between deployments. | Unit test | +| GENHUB-006 | When `allocate` returns an invalid buffer (null data, or size less than requested), the GenericHub shall drop the outgoing message instead of asserting and shall remain operational. | Unit test | +| GENHUB-007 | On each such drop, the GenericHub shall emit a `WARNING_HI` event identifying the message type, port index, and requested size. | Unit test | +| GENHUB-008 | The GenericHub shall count allocation failures and report the count as a telemetry channel from `schedIn`. | Unit test, integration test | +| GENHUB-009 | An undersized but valid buffer returned by `allocate` shall be returned through `deallocate`. | Unit test | +| GENHUB-010 | Buffers received on `bufferIn` shall be returned through `bufferInReturn` whether or not the outgoing message is dropped. | Unit test | +| GENHUB-011 | A `cmdDispIn` command dropped on allocation failure shall be failed back to its caller with `EXECUTION_ERROR` on the matching `cmdRespOut` port. | Unit test | ## Example formations @@ -333,3 +369,4 @@ The following diagrams show progressively more detail: | 2023-06-09 | Added telemetry and event helpers | | 2026-07-09 | Documented command routing, current configuration constants, buffer-driver interfaces, framing, and buffer lifecycle | | 2026-09-29 | Documented threading, command index pairing, cross-deployment type agreement, silent receive drops, and allocation failure | +| 2026-10-11 | Allocation failure drops the message with an event and counter instead of asserting; added `schedIn` and the hub's own event/telemetry ports | diff --git a/Svc/GenericHub/test/ut/GenericHubTestMain.cpp b/Svc/GenericHub/test/ut/GenericHubTestMain.cpp index 96d88f7113f..cef7b8e9a24 100644 --- a/Svc/GenericHub/test/ut/GenericHubTestMain.cpp +++ b/Svc/GenericHub/test/ut/GenericHubTestMain.cpp @@ -57,6 +57,16 @@ TEST(Invalid, TestDeserializationGuards) { tester.test_invalid_deserialization_paths(); } +TEST(Invalid, TestAllocationFailure) { + Svc::GenericHubTester tester; + tester.test_allocation_failure(); +} + +TEST(Invalid, TestCommandDispatchAllocationFailure) { + Svc::GenericHubTester tester; + tester.test_command_dispatch_allocation_failure(); +} + int main(int argc, char** argv) { STest::Random::seed(); ::testing::InitGoogleTest(&argc, argv); diff --git a/Svc/GenericHub/test/ut/GenericHubTester.cpp b/Svc/GenericHub/test/ut/GenericHubTester.cpp index cab5d2688d9..292557343e1 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.cpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.cpp @@ -12,6 +12,7 @@ #include "GenericHubTester.hpp" #include +#include "Fw/Com/ComPacket.hpp" #define INSTANCE 0 #define MAX_HISTORY_SIZE 10000 @@ -33,7 +34,9 @@ GenericHubTester ::GenericHubTester() m_buffer_out(0), m_current_port(0), m_cmdDispOutPort(0), - m_cmdRespOutPort(0) { + m_cmdRespOutPort(0), + m_allocation_mode(ALLOCATE_OK), + m_allocation_failures(0) { this->initComponents(); this->connectPorts(); } @@ -314,6 +317,115 @@ void GenericHubTester ::test_invalid_deserialization_paths() { clearFromPortHistory(); } +void GenericHubTester ::assert_dropped(GenericHub::HubType type, FwIndexType port, FwSizeType payloadSize) { + const FwSizeType needed = payloadSize + sizeof(U32) + sizeof(U32) + sizeof(FwBuffSizeType); + ASSERT_from_toBufferDriver_SIZE(0); + // The event is throttled after EVENTID_BUFFERALLOCATIONFAILED_THROTTLE drops; the counter is not + if (m_allocation_failures < GenericHubComponentBase::EVENTID_BUFFERALLOCATIONFAILED_THROTTLE) { + ASSERT_EVENTS_BufferAllocationFailed_SIZE(1); + ASSERT_EVENTS_BufferAllocationFailed(0, static_cast(type), port, needed); + } else { + ASSERT_EVENTS_BufferAllocationFailed_SIZE(0); + } + m_allocation_failures++; + this->clearHistory(); +} + +void GenericHubTester ::test_allocation_failure() { + this->clearHistory(); + m_allocation_mode = ALLOCATE_EMPTY; + + // Serial port call is dropped + Fw::ComBuffer comm; + random_fill(comm, FW_COM_BUFFER_MAX_SIZE); + invoke_to_serialIn(1, comm); + assert_dropped(GenericHub::HUB_TYPE_PORT, 1, comm.getSize()); + + // Buffer is dropped but still returned to its owner + m_buffer.set(m_data_store, STest::Pick::lowerUpper(1, FW_COM_BUFFER_MAX_SIZE)); + invoke_to_bufferIn(2, m_buffer); + ASSERT_from_bufferInReturn_SIZE(1); + ASSERT_from_bufferInReturn(0, m_buffer); + assert_dropped(GenericHub::HUB_TYPE_BUFFER, 2, m_buffer.getSize()); + + // Event is dropped + Fw::LogBuffer logArgs; + Fw::Time time(100, 200); + fill(logArgs, STest::Pick::lowerUpper(0, FW_LOG_BUFFER_MAX_SIZE)); + invoke_to_eventIn(0, 123, time, Fw::LogSeverity::WARNING_HI, logArgs); + assert_dropped(GenericHub::HUB_TYPE_EVENT, 0, + sizeof(FwEventIdType) + Fw::Time::SERIALIZED_SIZE + Fw::LogSeverity::SERIALIZED_SIZE + + sizeof(FwSizeStoreType) + logArgs.getSize()); + + // Telemetry is dropped + Fw::TlmBuffer tlmVal; + fill(tlmVal, STest::Pick::lowerUpper(0, FW_TLM_BUFFER_MAX_SIZE)); + invoke_to_tlmIn(0, 123, time, tlmVal); + assert_dropped(GenericHub::HUB_TYPE_CHANNEL, 0, + sizeof(FwChanIdType) + Fw::Time::SERIALIZED_SIZE + sizeof(FwSizeStoreType) + tlmVal.getSize()); + + // Command response is dropped + invoke_to_cmdRespIn(1, 0x3A56BF8C, 825, Fw::CmdResponse::OK); + assert_dropped(GenericHub::HUB_TYPE_CMD_RESP, 1, + sizeof(FwOpcodeType) + sizeof(U32) + Fw::CmdResponse::SERIALIZED_SIZE); + ASSERT_from_deallocate_SIZE(0); + + // An undersized buffer is dropped and returned to the allocator; the event is now throttled + m_allocation_mode = ALLOCATE_UNDERSIZED; + invoke_to_serialIn(0, comm); + ASSERT_from_deallocate_SIZE(1); + ASSERT_EQ(m_allocate.getData(), nullptr) << "Undersized buffer was not deallocated"; + assert_dropped(GenericHub::HUB_TYPE_PORT, 0, comm.getSize()); + + // Counter is reported on schedIn, and only when it changes + invoke_to_schedIn(0, 0); + ASSERT_TLM_NumAllocationFailures_SIZE(1); + ASSERT_TLM_NumAllocationFailures(0, m_allocation_failures); + invoke_to_schedIn(0, 0); + ASSERT_TLM_NumAllocationFailures_SIZE(1); + this->clearHistory(); + + // Counter keeps counting while the event is throttled + m_allocation_mode = ALLOCATE_EMPTY; + for (U32 i = 0; i < 10; i++) { + invoke_to_serialIn(0, comm); + assert_dropped(GenericHub::HUB_TYPE_PORT, 0, comm.getSize()); + } + invoke_to_schedIn(0, 0); + ASSERT_TLM_NumAllocationFailures(0, m_allocation_failures); + this->clearHistory(); + + // Hub keeps working once allocation succeeds again + m_allocation_mode = ALLOCATE_OK; + send_random_comm(0); + ASSERT_EVENTS_BufferAllocationFailed_SIZE(0); +} + +void GenericHubTester ::test_command_dispatch_allocation_failure() { + const FwIndexType port = 1; + const FwOpcodeType opCode = 0x1234; + const U32 context = 77; + Fw::ComBuffer buffer; + this->clearHistory(); + // Build a command packet: descriptor, opcode, then random argument bytes + ASSERT_EQ(Fw::FW_SERIALIZE_OK, + buffer.serializeFrom(static_cast(Fw::ComPacketType::FW_PACKET_COMMAND))); + ASSERT_EQ(Fw::FW_SERIALIZE_OK, buffer.serializeFrom(opCode)); + for (U32 i = 0; i < STest::Pick::lowerUpper(0, FW_CMD_ARG_BUFFER_MAX_SIZE); i++) { + ASSERT_EQ(Fw::FW_SERIALIZE_OK, buffer.serializeFrom(static_cast(STest::Pick::any()))); + } + + m_allocation_mode = ALLOCATE_EMPTY; + invoke_to_cmdDispIn(port, buffer, context); + + // Dropped command is failed back to the caller on the same port index + ASSERT_from_cmdDispOut_SIZE(0); + ASSERT_from_cmdRespOut_SIZE(1); + ASSERT_from_cmdRespOut(0, opCode, context, Fw::CmdResponse::EXECUTION_ERROR); + ASSERT_EQ(m_cmdRespOutPort, port); + assert_dropped(GenericHub::HUB_TYPE_CMD_DISP, port, buffer.getSize() + sizeof(U32)); +} + // ---------------------------------------------------------------------- // Handlers for typed from ports // ---------------------------------------------------------------------- @@ -390,10 +502,24 @@ void GenericHubTester ::from_serialOut_handler(FwIndexType portNum, /*!< Fw::Buffer GenericHubTester ::from_allocate_handler(const FwIndexType portNum, const FwSizeType size) { EXPECT_EQ(m_allocate.getData(), nullptr) << "Allocation buffer is still in use"; EXPECT_LE(size, sizeof(m_data_for_allocation)) << "Allocation buffer size mismatch"; - m_allocate.set(m_data_for_allocation, size); + switch (m_allocation_mode) { + case ALLOCATE_EMPTY: + return Fw::Buffer(); + case ALLOCATE_UNDERSIZED: + m_allocate.set(m_data_for_allocation, size - 1); + break; + default: + m_allocate.set(m_data_for_allocation, size); + break; + } return m_allocate; } +void GenericHubTester ::from_deallocate_handler(const FwIndexType portNum, Fw::Buffer& fwBuffer) { + this->pushFromPortEntry_deallocate(fwBuffer); + m_allocate.set(nullptr, 0); +} + void GenericHubTester ::from_bufferInReturn_handler(const FwIndexType portNum, Fw::Buffer& fwBuffer) { // Check buffer deallocations here ASSERT_EQ(fwBuffer.getData(), m_buffer.getData()) << "Ensure that the buffer was deallocated"; @@ -484,6 +610,17 @@ void GenericHubTester ::connectPorts() { // dataDeallocate this->componentIn.set_deallocate_OutputPort(0, this->get_from_deallocate(0)); + // schedIn, events, telemetry and time (sending side only) + this->connect_to_schedIn(0, this->componentIn.get_schedIn_InputPort(0)); + this->componentIn.set_Log_OutputPort(0, this->get_from_Log(0)); + this->componentIn.set_LogText_OutputPort(0, this->get_from_LogText(0)); + this->componentIn.set_Time_OutputPort(0, this->get_from_Time(0)); + this->componentIn.set_Tlm_OutputPort(0, this->get_from_Tlm(0)); + // Sending side fails dropped commands back to their caller + for (FwIndexType i = 0; i < GenericHubComponentBase::NUM_CMDRESPOUT_OUTPUT_PORTS; i++) { + this->componentIn.set_cmdRespOut_OutputPort(i, this->get_from_cmdRespOut(i)); + } + // bufferInReturn for (FwIndexType i = 0; i < GenericHubCfg::NumBufferInputPorts; i++) { this->componentIn.set_bufferInReturn_OutputPort(i, this->get_from_bufferInReturn(i)); diff --git a/Svc/GenericHub/test/ut/GenericHubTester.hpp b/Svc/GenericHub/test/ut/GenericHubTester.hpp index d669c4265f1..995ff970cda 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.hpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.hpp @@ -80,6 +80,14 @@ class GenericHubTester : public GenericHubGTestBase { //! void test_invalid_deserialization_paths(); + //! Test that outgoing messages are dropped, reported, and counted when allocation fails + //! + void test_allocation_failure(); + + //! Test that a command dropped on allocation failure is failed back to its caller + //! + void test_command_dispatch_allocation_failure(); + private: // ---------------------------------------------------------------------- // Handlers for typed from ports @@ -142,6 +150,11 @@ class GenericHubTester : public GenericHubGTestBase { void from_fromBufferDriverReturn_handler(const FwIndexType portNum, /*!< The port number*/ Fw::Buffer& fwBuffer); + //! Handler for from_deallocate + //! + void from_deallocate_handler(const FwIndexType portNum, /*!< The port number*/ + Fw::Buffer& fwBuffer); + private: // ---------------------------------------------------------------------- // Handlers for serial from ports @@ -154,6 +167,16 @@ class GenericHubTester : public GenericHubGTestBase { ); private: + //! How from_allocate_handler responds to the hub + enum AllocationMode { + ALLOCATE_OK, //!< Return a buffer of the requested size + ALLOCATE_EMPTY, //!< Return an empty (null) buffer + ALLOCATE_UNDERSIZED //!< Return a valid buffer one byte short of the request + }; + + //! Assert that the last send was dropped with a single BufferAllocationFailed event for the given payload + void assert_dropped(GenericHub::HubType type, FwIndexType port, FwSizeType payloadSize); + void send_random_comm(U32 port); void send_random_buffer(U32 port); @@ -207,6 +230,8 @@ class GenericHubTester : public GenericHubGTestBase { U32 m_current_port; FwIndexType m_cmdDispOutPort; FwIndexType m_cmdRespOutPort; + AllocationMode m_allocation_mode; + U32 m_allocation_failures; //!< Drops injected so far, mirrors the hub's counter U8 m_data_store[DATA_SIZE]; U8 m_data_for_allocation[DATA_SIZE]; }; diff --git a/docs/user-manual/design-patterns/hub-pattern.md b/docs/user-manual/design-patterns/hub-pattern.md index 6e7c381066e..d974558e953 100644 --- a/docs/user-manual/design-patterns/hub-pattern.md +++ b/docs/user-manual/design-patterns/hub-pattern.md @@ -69,8 +69,11 @@ port. buffer-driver interface expected by GenericHub. - Wire command ports with the same indices on both hubs; the index is how a command response finds its originator. -- Size the hub's buffer pool for the worst case. GenericHub asserts when - allocation fails. +- Size the hub's buffer pool for the worst case. When allocation fails, + GenericHub drops the message, emits `BufferAllocationFailed`, and counts it + in `NumAllocationFailures`; drive `schedIn` from a rate group so the counter + is reported. A dropped `cmdDispIn` command is failed back to its caller with + `EXECUTION_ERROR`. - Monitor the link outside the hub. GenericHub reports no transport errors and silently drops invalid received messages. From 5490eff0c17a5c82531ae163464e7308488cbaa9 Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Mon, 12 Oct 2026 00:46:36 +0000 Subject: [PATCH 02/13] Svc/GenericHub: report NumAllocationFailures on every schedIn so late-connecting ground sees it --- Svc/GenericHub/GenericHub.fpp | 2 +- Svc/GenericHub/test/ut/GenericHubTester.cpp | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Svc/GenericHub/GenericHub.fpp b/Svc/GenericHub/GenericHub.fpp index 6db8ebb1571..9f3ac2d7879 100644 --- a/Svc/GenericHub/GenericHub.fpp +++ b/Svc/GenericHub/GenericHub.fpp @@ -239,7 +239,7 @@ module Svc { throttle 5 @ The number of failed buffer allocations - telemetry NumAllocationFailures: U32 update on change + telemetry NumAllocationFailures: U32 } diff --git a/Svc/GenericHub/test/ut/GenericHubTester.cpp b/Svc/GenericHub/test/ut/GenericHubTester.cpp index 292557343e1..5731f774469 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.cpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.cpp @@ -377,12 +377,10 @@ void GenericHubTester ::test_allocation_failure() { ASSERT_EQ(m_allocate.getData(), nullptr) << "Undersized buffer was not deallocated"; assert_dropped(GenericHub::HUB_TYPE_PORT, 0, comm.getSize()); - // Counter is reported on schedIn, and only when it changes + // Counter is reported on schedIn invoke_to_schedIn(0, 0); ASSERT_TLM_NumAllocationFailures_SIZE(1); ASSERT_TLM_NumAllocationFailures(0, m_allocation_failures); - invoke_to_schedIn(0, 0); - ASSERT_TLM_NumAllocationFailures_SIZE(1); this->clearHistory(); // Counter keeps counting while the event is throttled From 6333672112fc3d5c24a5e292d26fa23f081c344a Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Mon, 12 Oct 2026 01:03:21 +0000 Subject: [PATCH 03/13] Svc/GenericHub: address local review (event format order, docs, UT assertions, (void) on ignored send_data results) --- Svc/GenericHub/GenericHub.cpp | 10 ++++----- Svc/GenericHub/GenericHub.fpp | 4 ++-- Svc/GenericHub/docs/sdd.md | 22 ++++++++++++++----- Svc/GenericHub/test/ut/GenericHubTester.cpp | 15 +++++++++---- .../design-patterns/hub-pattern.md | 8 ++++--- 5 files changed, 39 insertions(+), 20 deletions(-) diff --git a/Svc/GenericHub/GenericHub.cpp b/Svc/GenericHub/GenericHub.cpp index af35df1fd80..19850d39fc4 100644 --- a/Svc/GenericHub/GenericHub.cpp +++ b/Svc/GenericHub/GenericHub.cpp @@ -62,7 +62,7 @@ bool GenericHub::send_data(const HubType type, const FwIndexType port, const U8* // ---------------------------------------------------------------------- void GenericHub::bufferIn_handler(const FwIndexType portNum, Fw::Buffer& fwBuffer) { - this->send_data(HUB_TYPE_BUFFER, portNum, fwBuffer.getData(), fwBuffer.getSize()); + (void)this->send_data(HUB_TYPE_BUFFER, portNum, fwBuffer.getData(), fwBuffer.getSize()); bufferInReturn_out(portNum, fwBuffer); } @@ -113,7 +113,7 @@ void GenericHub ::cmdRespIn_handler(FwIndexType portNum, FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK); FwSizeType size = serializer.getSize(); - this->send_data(HUB_TYPE_CMD_RESP, portNum, buffer, size); + (void)this->send_data(HUB_TYPE_CMD_RESP, portNum, buffer, size); } void GenericHub::fromBufferDriver_handler(const FwIndexType portNum, Fw::Buffer& fwBuffer) { @@ -299,7 +299,7 @@ void GenericHub::eventIn_handler(const FwIndexType portNum, status = serializer.serializeFrom(args); FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK); FwSizeType size = serializer.getSize(); - this->send_data(HubType::HUB_TYPE_EVENT, portNum, buffer, size); + (void)this->send_data(HubType::HUB_TYPE_EVENT, portNum, buffer, size); } void GenericHub::tlmIn_handler(const FwIndexType portNum, FwChanIdType id, Fw::Time& timeTag, Fw::TlmBuffer& val) { @@ -314,7 +314,7 @@ void GenericHub::tlmIn_handler(const FwIndexType portNum, FwChanIdType id, Fw::T status = serializer.serializeFrom(val); FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK); FwSizeType size = serializer.getSize(); - this->send_data(HubType::HUB_TYPE_CHANNEL, portNum, buffer, size); + (void)this->send_data(HubType::HUB_TYPE_CHANNEL, portNum, buffer, size); } // ---------------------------------------------------------------------- @@ -324,7 +324,7 @@ void GenericHub::tlmIn_handler(const FwIndexType portNum, FwChanIdType id, Fw::T void GenericHub::serialIn_handler(FwIndexType portNum, /*!< The port number*/ Fw::LinearBufferBase& Buffer /*!< The serialization buffer*/ ) { - send_data(HUB_TYPE_PORT, portNum, Buffer.getBuffAddr(), Buffer.getSize()); + (void)send_data(HUB_TYPE_PORT, portNum, Buffer.getBuffAddr(), Buffer.getSize()); } } // end namespace Svc diff --git a/Svc/GenericHub/GenericHub.fpp b/Svc/GenericHub/GenericHub.fpp index 9f3ac2d7879..d1f969b3604 100644 --- a/Svc/GenericHub/GenericHub.fpp +++ b/Svc/GenericHub/GenericHub.fpp @@ -51,7 +51,7 @@ module Svc { # 1. Invoke allocate to allocate a buffer B. # 2. Serialize the hub message type (event, telemetry, serial, buffer), # the port number, and the data into B. - # 3. Emit B on toBufferDriver. + # 3. Emit B on toBufferDriver (or drop the message if the allocation failed). # # Sample connections: # @@ -235,7 +235,7 @@ module Svc { $size: FwSizeType @< The requested allocation size ) \ severity warning high \ - format "Failed to allocate {} byte buffer for message type {} on port {}" \ + format "Buffer allocation failed for message type {} on port {}: requested {} bytes" \ throttle 5 @ The number of failed buffer allocations diff --git a/Svc/GenericHub/docs/sdd.md b/Svc/GenericHub/docs/sdd.md index c79590903f3..6fd28f50129 100644 --- a/Svc/GenericHub/docs/sdd.md +++ b/Svc/GenericHub/docs/sdd.md @@ -46,7 +46,8 @@ the buffer lifecycle described later. A pointer must never be passed through the hub: a pointer is meaningful only in the address space that allocated the pointed-to object. -GenericHub is passive and keeps no state between calls. Each handler runs on +GenericHub is passive. Its only state between calls is the atomic +`NumAllocationFailures` counter and the `BufferAllocationFailed` throttle. Each handler runs on its caller's thread, and several producers may be in the send path at once, so the component connected to `toBufferDriver` must accept concurrent calls (for example, the asynchronous inputs of `Svc::ComQueue`). @@ -255,12 +256,20 @@ index, and requested size. Buffers received on `bufferIn` are still returned through `bufferInReturn`. The hub remains operational and resumes sending once allocation succeeds. A dropped `cmdDispIn` command is failed back to its caller with `Fw::CmdResponse::EXECUTION_ERROR` on the matching `cmdRespOut` port; a -dropped `cmdRespIn` response cannot be reported to the far side and is lost. +dropped `cmdRespIn` response cannot be reported to the far side and is lost; +the originating `Svc::CmdDispatcher` has no response timeout, so each loss +permanently occupies one of its sequence-tracker slots. Size the pool so that +`cmdRespIn` never drops. The counter is read from any caller thread (`std::atomic`) and written to telemetry only from `schedIn`. Emitting telemetry on the failure path would re-enter the hub whenever the hub is the deployment's telemetry sink, so the -`schedIn` port is required for the channel to be observable. +`schedIn` port is required for the channel to be observable. The event is +emitted on the failure path; when the hub is also the deployment's event sink +(`event connections instance hub` routes `hub.Log` back into `hub.eventIn`), +each failure re-enters the hub once per event until the throttle (5) is +reached, after which failures are only counted. Existing deployments must add +the `schedIn` connection to see the counter. Size the pool for the worst case: the capacity of every queue downstream of `toBufferDriver`, plus buffers held by the driver, plus one per thread that @@ -328,7 +337,8 @@ Outgoing messages are dropped when the allocator returns an invalid buffer (for example, `Svc::BufferManager` with no free buffer of sufficient size); see [Allocation failure](#allocation-failure). -The hub is not itself an event source or telemetry database. Its `eventOut` +Apart from its own `BufferAllocationFailed` event and `NumAllocationFailures` +channel, the hub is not an event source or telemetry database. Its `eventOut` and `tlmOut` ports still need to be wired to the deployment's event manager and telemetry database, respectively. @@ -342,8 +352,8 @@ telemetry database, respectively. | GENHUB-004 | The GenericHub shall work with another GenericHub to send port and buffer calls. | Unit test | | GENHUB-005 | The GenericHub shall forward command dispatches and command responses between deployments. | Unit test | | GENHUB-006 | When `allocate` returns an invalid buffer (null data, or size less than requested), the GenericHub shall drop the outgoing message instead of asserting and shall remain operational. | Unit test | -| GENHUB-007 | On each such drop, the GenericHub shall emit a `WARNING_HI` event identifying the message type, port index, and requested size. | Unit test | -| GENHUB-008 | The GenericHub shall count allocation failures and report the count as a telemetry channel from `schedIn`. | Unit test, integration test | +| GENHUB-007 | On such a drop, the GenericHub shall emit a `WARNING_HI` event (throttled to 5) identifying the message type, port index, and requested size. | Unit test | +| GENHUB-008 | The GenericHub shall count allocation failures and report the count as a telemetry channel from `schedIn`. | Unit test, integration test (fprime-generic-hub-reference) | | GENHUB-009 | An undersized but valid buffer returned by `allocate` shall be returned through `deallocate`. | Unit test | | GENHUB-010 | Buffers received on `bufferIn` shall be returned through `bufferInReturn` whether or not the outgoing message is dropped. | Unit test | | GENHUB-011 | A `cmdDispIn` command dropped on allocation failure shall be failed back to its caller with `EXECUTION_ERROR` on the matching `cmdRespOut` port. | Unit test | diff --git a/Svc/GenericHub/test/ut/GenericHubTester.cpp b/Svc/GenericHub/test/ut/GenericHubTester.cpp index 5731f774469..defde3575ef 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.cpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.cpp @@ -320,6 +320,15 @@ void GenericHubTester ::test_invalid_deserialization_paths() { void GenericHubTester ::assert_dropped(GenericHub::HubType type, FwIndexType port, FwSizeType payloadSize) { const FwSizeType needed = payloadSize + sizeof(U32) + sizeof(U32) + sizeof(FwBuffSizeType); ASSERT_from_toBufferDriver_SIZE(0); + // Only a non-null (undersized) buffer is handed back to the allocator; telemetry is only written from schedIn + if (m_allocation_mode == ALLOCATE_UNDERSIZED) { + ASSERT_from_deallocate_SIZE(1); + ASSERT_EQ(this->fromPortHistory_deallocate->at(0).fwBuffer.getData(), m_data_for_allocation); + ASSERT_EQ(this->fromPortHistory_deallocate->at(0).fwBuffer.getSize(), needed - 1); + } else { + ASSERT_from_deallocate_SIZE(0); + } + ASSERT_TLM_NumAllocationFailures_SIZE(0); // The event is throttled after EVENTID_BUFFERALLOCATIONFAILED_THROTTLE drops; the counter is not if (m_allocation_failures < GenericHubComponentBase::EVENTID_BUFFERALLOCATIONFAILED_THROTTLE) { ASSERT_EVENTS_BufferAllocationFailed_SIZE(1); @@ -368,13 +377,10 @@ void GenericHubTester ::test_allocation_failure() { invoke_to_cmdRespIn(1, 0x3A56BF8C, 825, Fw::CmdResponse::OK); assert_dropped(GenericHub::HUB_TYPE_CMD_RESP, 1, sizeof(FwOpcodeType) + sizeof(U32) + Fw::CmdResponse::SERIALIZED_SIZE); - ASSERT_from_deallocate_SIZE(0); // An undersized buffer is dropped and returned to the allocator; the event is now throttled m_allocation_mode = ALLOCATE_UNDERSIZED; invoke_to_serialIn(0, comm); - ASSERT_from_deallocate_SIZE(1); - ASSERT_EQ(m_allocate.getData(), nullptr) << "Undersized buffer was not deallocated"; assert_dropped(GenericHub::HUB_TYPE_PORT, 0, comm.getSize()); // Counter is reported on schedIn @@ -409,7 +415,8 @@ void GenericHubTester ::test_command_dispatch_allocation_failure() { ASSERT_EQ(Fw::FW_SERIALIZE_OK, buffer.serializeFrom(static_cast(Fw::ComPacketType::FW_PACKET_COMMAND))); ASSERT_EQ(Fw::FW_SERIALIZE_OK, buffer.serializeFrom(opCode)); - for (U32 i = 0; i < STest::Pick::lowerUpper(0, FW_CMD_ARG_BUFFER_MAX_SIZE); i++) { + const FwSizeType argBytes = STest::Pick::lowerUpper(0, FW_CMD_ARG_BUFFER_MAX_SIZE); + for (FwSizeType i = 0; i < argBytes; i++) { ASSERT_EQ(Fw::FW_SERIALIZE_OK, buffer.serializeFrom(static_cast(STest::Pick::any()))); } diff --git a/docs/user-manual/design-patterns/hub-pattern.md b/docs/user-manual/design-patterns/hub-pattern.md index d974558e953..34fc15be084 100644 --- a/docs/user-manual/design-patterns/hub-pattern.md +++ b/docs/user-manual/design-patterns/hub-pattern.md @@ -62,8 +62,9 @@ port. is effectively a fat pointer) is valid only in the address space that owns the pointed-to object; send serialized values instead. - Wire received event and telemetry outputs to the deployment's event manager - and telemetry database. A hub transports events and telemetry, but is not - itself an event source or telemetry database. + and telemetry database. A hub transports events and telemetry; apart from + its own `BufferAllocationFailed` event and `NumAllocationFailures` channel, + it is not an event source or telemetry database. - Use a buffer driver at each end of the transport. The `Drv::ByteStreamBufferAdapter` can pair a byte-stream driver with the buffer-driver interface expected by GenericHub. @@ -72,7 +73,8 @@ port. - Size the hub's buffer pool for the worst case. When allocation fails, GenericHub drops the message, emits `BufferAllocationFailed`, and counts it in `NumAllocationFailures`; drive `schedIn` from a rate group so the counter - is reported. A dropped `cmdDispIn` command is failed back to its caller with + is reported (deployments written before `schedIn` existed must add this + connection). A dropped `cmdDispIn` command is failed back to its caller with `EXECUTION_ERROR`. - Monitor the link outside the hub. GenericHub reports no transport errors and silently drops invalid received messages. From 0cd559bc4363246a2be0fb90b9f02c025202d4ca Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Mon, 12 Oct 2026 01:12:59 +0000 Subject: [PATCH 04/13] Svc/GenericHub: address local review iter 2 (Fw::Success from send_data, cmdRespIn drop wording, schedIn and msgType docs, malformed-command UT) --- Svc/GenericHub/GenericHub.cpp | 13 ++++--- Svc/GenericHub/GenericHub.fpp | 2 +- Svc/GenericHub/GenericHub.hpp | 7 ++-- Svc/GenericHub/docs/sdd.md | 38 ++++++++++++--------- Svc/GenericHub/test/ut/GenericHubTester.cpp | 11 ++++++ 5 files changed, 46 insertions(+), 25 deletions(-) diff --git a/Svc/GenericHub/GenericHub.cpp b/Svc/GenericHub/GenericHub.cpp index 19850d39fc4..480db0d3003 100644 --- a/Svc/GenericHub/GenericHub.cpp +++ b/Svc/GenericHub/GenericHub.cpp @@ -29,7 +29,7 @@ GenericHub::GenericHub(const char* const compName) : GenericHubComponentBase(com GenericHub::~GenericHub() {} -bool GenericHub::send_data(const HubType type, const FwIndexType port, const U8* data, const FwSizeType size) { +Fw::Success GenericHub::send_data(const HubType type, const FwIndexType port, const U8* data, const FwSizeType size) { FW_ASSERT(data != nullptr); Fw::SerializeStatus status; const FwSizeType needed = size + sizeof(U32) + sizeof(U32) + sizeof(FwBuffSizeType); @@ -42,7 +42,7 @@ bool GenericHub::send_data(const HubType type, const FwIndexType port, const U8* } this->m_numAllocationFailures++; this->log_WARNING_HI_BufferAllocationFailed(static_cast(type), port, needed); - return false; + return Fw::Success::FAILURE; } auto serialize = outgoing.getSerializer(); // Write data to our buffer @@ -54,7 +54,7 @@ bool GenericHub::send_data(const HubType type, const FwIndexType port, const U8* FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast(status)); outgoing.setSize(serialize.getSize()); toBufferDriver_out(0, outgoing); - return true; + return Fw::Success::SUCCESS; } // ---------------------------------------------------------------------- @@ -62,6 +62,7 @@ bool GenericHub::send_data(const HubType type, const FwIndexType port, const U8* // ---------------------------------------------------------------------- void GenericHub::bufferIn_handler(const FwIndexType portNum, Fw::Buffer& fwBuffer) { + // A drop is counted and evented in send_data; this port has no status to return (void)this->send_data(HUB_TYPE_BUFFER, portNum, fwBuffer.getData(), fwBuffer.getSize()); bufferInReturn_out(portNum, fwBuffer); } @@ -86,7 +87,7 @@ void GenericHub ::cmdDispIn_handler(FwIndexType portNum, Fw::ComBuffer& data, U3 FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast(status)); FwSizeType size = serializer.getSize(); - if (!this->send_data(HUB_TYPE_CMD_DISP, portNum, buffer, size)) { + if (this->send_data(HUB_TYPE_CMD_DISP, portNum, buffer, size) != Fw::Success::SUCCESS) { // The command was dropped: fail it back to the caller so it is not left waiting for a response Fw::CmdPacket cmdPkt; (void)cmdPkt.deserializeFrom(data); // opcode is 0 if the packet is malformed, as in CmdDispatcher @@ -113,6 +114,7 @@ void GenericHub ::cmdRespIn_handler(FwIndexType portNum, FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK); FwSizeType size = serializer.getSize(); + // A dropped response cannot be reported to the far side (see SDD, "Allocation failure") (void)this->send_data(HUB_TYPE_CMD_RESP, portNum, buffer, size); } @@ -299,6 +301,7 @@ void GenericHub::eventIn_handler(const FwIndexType portNum, status = serializer.serializeFrom(args); FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK); FwSizeType size = serializer.getSize(); + // A drop is counted and evented in send_data; this port has no status to return (void)this->send_data(HubType::HUB_TYPE_EVENT, portNum, buffer, size); } @@ -314,6 +317,7 @@ void GenericHub::tlmIn_handler(const FwIndexType portNum, FwChanIdType id, Fw::T status = serializer.serializeFrom(val); FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK); FwSizeType size = serializer.getSize(); + // A drop is counted and evented in send_data; this port has no status to return (void)this->send_data(HubType::HUB_TYPE_CHANNEL, portNum, buffer, size); } @@ -324,6 +328,7 @@ void GenericHub::tlmIn_handler(const FwIndexType portNum, FwChanIdType id, Fw::T void GenericHub::serialIn_handler(FwIndexType portNum, /*!< The port number*/ Fw::LinearBufferBase& Buffer /*!< The serialization buffer*/ ) { + // A drop is counted and evented in send_data; this port has no status to return (void)send_data(HUB_TYPE_PORT, portNum, Buffer.getBuffAddr(), Buffer.getSize()); } diff --git a/Svc/GenericHub/GenericHub.fpp b/Svc/GenericHub/GenericHub.fpp index d1f969b3604..c69341476ca 100644 --- a/Svc/GenericHub/GenericHub.fpp +++ b/Svc/GenericHub/GenericHub.fpp @@ -106,7 +106,7 @@ module Svc { @ for sending remote commands sync input port cmdDispIn: [CmdDispatcherSequencePorts] Fw.Com - @ Command response back from remote hub + @ Command response back from remote hub, or EXECUTION_ERROR for a command dropped on allocation failure output port cmdRespOut: [CmdDispatcherSequencePorts] Fw.CmdResponse # ---------------------------------------------------------------------- diff --git a/Svc/GenericHub/GenericHub.hpp b/Svc/GenericHub/GenericHub.hpp index b194b3b419a..196a1f5c5cb 100644 --- a/Svc/GenericHub/GenericHub.hpp +++ b/Svc/GenericHub/GenericHub.hpp @@ -14,6 +14,7 @@ #define Svc_GenericHub_HPP #include +#include "Fw/Types/SuccessEnumAc.hpp" #include "Svc/GenericHub/GenericHubComponentAc.hpp" namespace Svc { @@ -130,9 +131,9 @@ class GenericHub final : public GenericHubComponentBase { // Helpers and members - //! Frame and send data on toBufferDriver. Returns false if the message was dropped because no - //! sufficiently sized buffer could be allocated. - bool send_data(const HubType type, const FwIndexType port, const U8* data, const FwSizeType size); + //! Frame and send data on toBufferDriver. Returns Fw::Success::FAILURE if the message was dropped because + //! no sufficiently sized buffer could be allocated. + Fw::Success send_data(const HubType type, const FwIndexType port, const U8* data, const FwSizeType size); std::atomic m_numAllocationFailures; //!< Count of dropped messages; incremented from any caller thread }; diff --git a/Svc/GenericHub/docs/sdd.md b/Svc/GenericHub/docs/sdd.md index 6fd28f50129..04531075311 100644 --- a/Svc/GenericHub/docs/sdd.md +++ b/Svc/GenericHub/docs/sdd.md @@ -67,7 +67,7 @@ remote hub. | `bufferIn` | `[GenericHubCfg.NumBufferInputPorts] Fw.BufferSend` | Sends buffer data to the remote deployment. The hub copies the data and returns the incoming buffer. | | `bufferInReturn` | `[GenericHubCfg.NumBufferInputPorts] Fw.BufferSend` | Returns buffers received on `bufferIn` to their senders. It must match `bufferIn`. | | `cmdDispIn` | `[CmdDispatcherSequencePorts] Fw.Com` | Accepts a command forwarded by a command splitter. | -| `cmdRespOut` | `[CmdDispatcherSequencePorts] Fw.CmdResponse` | Emits a command response received from the remote hub. | +| `cmdRespOut` | `[CmdDispatcherSequencePorts] Fw.CmdResponse` | Emits a command response received from the remote hub, or `EXECUTION_ERROR` for a `cmdDispIn` command the hub dropped on allocation failure. | Typical connections include: @@ -114,9 +114,9 @@ genericHub.deallocate -> bufferManager.bufferSendIn | `Log`, `LogText`, `Time`, `Tlm` | special ports | The hub's own events, text events, time, and telemetry. | These ports report the hub's health; they are distinct from the pass-through -`eventIn`/`eventOut` and `tlmIn`/`tlmOut` ports. `schedIn` should be driven -from a rate group whenever the hub's event and telemetry ports are wired -through the hub itself (see [Allocation failure](#allocation-failure)). +`eventIn`/`eventOut` and `tlmIn`/`tlmOut` ports. `NumAllocationFailures` is +written only from `schedIn`, so drive `schedIn` from a rate group in every +deployment that wants the counter (see [Allocation failure](#allocation-failure)). ### 3. Buffer driver to hub: receive interface @@ -174,14 +174,17 @@ the buffer on `toBufferDriver`. On receipt, the hub validates the type, deserializes the port and payload size, checks that the payload size matches the received buffer, and dispatches according to the message type. -| Message type | Payload and behavior | -|---|---| -| Serial (`HUB_TYPE_PORT`) | The payload is the serialized by-value argument list for `serialIn`. The receiver wraps the payload in an `Fw::ExternalSerializeBuffer` and invokes the matching `serialOut` port. | -| Buffer (`HUB_TYPE_BUFFER`) | The sender copies the incoming buffer data into a newly allocated transport buffer. The receiver adjusts the received buffer metadata to the payload and emits it on the matching `bufferOut` port. | -| Event (`HUB_TYPE_EVENT`) | The payload contains the event ID, time tag, severity, and `Fw::LogBuffer` arguments. The receiver deserializes these values and invokes `eventOut`. | -| Telemetry (`HUB_TYPE_CHANNEL`) | The payload contains the channel ID, time tag, and `Fw::TlmBuffer` value. The receiver deserializes these values and invokes `tlmOut`. | -| Command dispatch (`HUB_TYPE_CMD_DISP`) | The payload contains the serialized `Fw::ComBuffer` command data followed by its `U32` context. The receiver invokes the matching `cmdDispOut` port. | -| Command response (`HUB_TYPE_CMD_RESP`) | The payload contains the opcode, command sequence, and `Fw::CmdResponse`. The receiver invokes the matching `cmdRespOut` port. | +| Message type | `msgType` | Payload and behavior | +|---|---|---| +| Serial (`HUB_TYPE_PORT`) | 0 | The payload is the serialized by-value argument list for `serialIn`. The receiver wraps the payload in an `Fw::ExternalSerializeBuffer` and invokes the matching `serialOut` port. | +| Buffer (`HUB_TYPE_BUFFER`) | 1 | The sender copies the incoming buffer data into a newly allocated transport buffer. The receiver adjusts the received buffer metadata to the payload and emits it on the matching `bufferOut` port. | +| Event (`HUB_TYPE_EVENT`) | 2 | The payload contains the event ID, time tag, severity, and `Fw::LogBuffer` arguments. The receiver deserializes these values and invokes `eventOut`. | +| Telemetry (`HUB_TYPE_CHANNEL`) | 3 | The payload contains the channel ID, time tag, and `Fw::TlmBuffer` value. The receiver deserializes these values and invokes `tlmOut`. | +| Command dispatch (`HUB_TYPE_CMD_DISP`) | 4 | The payload contains the serialized `Fw::ComBuffer` command data followed by its `U32` context. The receiver invokes the matching `cmdDispOut` port. | +| Command response (`HUB_TYPE_CMD_RESP`) | 5 | The payload contains the opcode, command sequence, and `Fw::CmdResponse`. The receiver invokes the matching `cmdRespOut` port. | + +The `msgType` column is the value reported by the `BufferAllocationFailed` +event. Malformed or invalid messages are not dispatched, and the received transport buffer is returned to the driver. A message whose destination port is invalid @@ -256,10 +259,11 @@ index, and requested size. Buffers received on `bufferIn` are still returned through `bufferInReturn`. The hub remains operational and resumes sending once allocation succeeds. A dropped `cmdDispIn` command is failed back to its caller with `Fw::CmdResponse::EXECUTION_ERROR` on the matching `cmdRespOut` port; a -dropped `cmdRespIn` response cannot be reported to the far side and is lost; -the originating `Svc::CmdDispatcher` has no response timeout, so each loss -permanently occupies one of its sequence-tracker slots. Size the pool so that -`cmdRespIn` never drops. +dropped `cmdRespIn` response cannot be reported to the far side and is lost, +so the originating sequencer or splitter never receives a response for that +command. Size the pool so that `cmdRespIn` never drops. The event throttle is +not synchronized, so its count is approximate when several threads drop +concurrently. The counter is read from any caller thread (`std::atomic`) and written to telemetry only from `schedIn`. Emitting telemetry on the failure path would @@ -353,7 +357,7 @@ telemetry database, respectively. | GENHUB-005 | The GenericHub shall forward command dispatches and command responses between deployments. | Unit test | | GENHUB-006 | When `allocate` returns an invalid buffer (null data, or size less than requested), the GenericHub shall drop the outgoing message instead of asserting and shall remain operational. | Unit test | | GENHUB-007 | On such a drop, the GenericHub shall emit a `WARNING_HI` event (throttled to 5) identifying the message type, port index, and requested size. | Unit test | -| GENHUB-008 | The GenericHub shall count allocation failures and report the count as a telemetry channel from `schedIn`. | Unit test, integration test (fprime-generic-hub-reference) | +| GENHUB-008 | The GenericHub shall count allocation failures and report the count as a telemetry channel from `schedIn`. | Unit test, integration test (fprime-generic-hub-reference, companion change) | | GENHUB-009 | An undersized but valid buffer returned by `allocate` shall be returned through `deallocate`. | Unit test | | GENHUB-010 | Buffers received on `bufferIn` shall be returned through `bufferInReturn` whether or not the outgoing message is dropped. | Unit test | | GENHUB-011 | A `cmdDispIn` command dropped on allocation failure shall be failed back to its caller with `EXECUTION_ERROR` on the matching `cmdRespOut` port. | Unit test | diff --git a/Svc/GenericHub/test/ut/GenericHubTester.cpp b/Svc/GenericHub/test/ut/GenericHubTester.cpp index defde3575ef..56cea843c0f 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.cpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.cpp @@ -429,6 +429,17 @@ void GenericHubTester ::test_command_dispatch_allocation_failure() { ASSERT_from_cmdRespOut(0, opCode, context, Fw::CmdResponse::EXECUTION_ERROR); ASSERT_EQ(m_cmdRespOutPort, port); assert_dropped(GenericHub::HUB_TYPE_CMD_DISP, port, buffer.getSize() + sizeof(U32)); + + // A malformed (non-command) packet that is dropped is still failed back, with the reserved opcode 0 + buffer.resetSer(); + ASSERT_EQ(Fw::FW_SERIALIZE_OK, + buffer.serializeFrom(static_cast(Fw::ComPacketType::FW_PACKET_TELEM))); + invoke_to_cmdDispIn(port, buffer, context); + ASSERT_from_cmdDispOut_SIZE(0); + ASSERT_from_cmdRespOut_SIZE(1); + ASSERT_from_cmdRespOut(0, 0, context, Fw::CmdResponse::EXECUTION_ERROR); + ASSERT_EQ(m_cmdRespOutPort, port); + assert_dropped(GenericHub::HUB_TYPE_CMD_DISP, port, buffer.getSize() + sizeof(U32)); } // ---------------------------------------------------------------------- From d2991cf8dc537e206d46f4ebb5129b893571ebda Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Mon, 12 Oct 2026 01:20:34 +0000 Subject: [PATCH 05/13] Svc/GenericHub: read the dropped command's opcode with a fresh cursor (CmdSplitter consumes the ComBuffer before forwarding) --- Svc/GenericHub/GenericHub.cpp | 5 ++++- Svc/GenericHub/test/ut/GenericHubTester.cpp | 7 ++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Svc/GenericHub/GenericHub.cpp b/Svc/GenericHub/GenericHub.cpp index 480db0d3003..5716810bb34 100644 --- a/Svc/GenericHub/GenericHub.cpp +++ b/Svc/GenericHub/GenericHub.cpp @@ -89,8 +89,11 @@ void GenericHub ::cmdDispIn_handler(FwIndexType portNum, Fw::ComBuffer& data, U3 if (this->send_data(HUB_TYPE_CMD_DISP, portNum, buffer, size) != Fw::Success::SUCCESS) { // The command was dropped: fail it back to the caller so it is not left waiting for a response + // Read the opcode with a fresh cursor: an upstream CmdSplitter may already have consumed `data` + Fw::ExternalSerializeBuffer reader(data.getBuffAddr(), data.getSize()); + (void)reader.setBuffLen(data.getSize()); Fw::CmdPacket cmdPkt; - (void)cmdPkt.deserializeFrom(data); // opcode is 0 if the packet is malformed, as in CmdDispatcher + (void)cmdPkt.deserializeFrom(reader); // opcode is 0 if the packet is malformed, as in CmdDispatcher if (this->isConnected_cmdRespOut_OutputPort(portNum)) { this->cmdRespOut_out(portNum, cmdPkt.getOpCode(), context, Fw::CmdResponse::EXECUTION_ERROR); } diff --git a/Svc/GenericHub/test/ut/GenericHubTester.cpp b/Svc/GenericHub/test/ut/GenericHubTester.cpp index 56cea843c0f..8dd32028edb 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.cpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.cpp @@ -12,6 +12,7 @@ #include "GenericHubTester.hpp" #include +#include "Fw/Cmd/CmdPacket.hpp" #include "Fw/Com/ComPacket.hpp" #define INSTANCE 0 @@ -420,10 +421,14 @@ void GenericHubTester ::test_command_dispatch_allocation_failure() { ASSERT_EQ(Fw::FW_SERIALIZE_OK, buffer.serializeFrom(static_cast(STest::Pick::any()))); } + // As CmdSplitter does before forwarding, consume the buffer so its deserialization cursor is at the end + Fw::CmdPacket splitterPkt; + ASSERT_EQ(Fw::FW_SERIALIZE_OK, splitterPkt.deserializeFrom(buffer)); + m_allocation_mode = ALLOCATE_EMPTY; invoke_to_cmdDispIn(port, buffer, context); - // Dropped command is failed back to the caller on the same port index + // Dropped command is failed back to the caller on the same port index, with the right opcode ASSERT_from_cmdDispOut_SIZE(0); ASSERT_from_cmdRespOut_SIZE(1); ASSERT_from_cmdRespOut(0, opCode, context, Fw::CmdResponse::EXECUTION_ERROR); From fbecee634391f6370456c49ca49d301e1a04677f Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Wed, 21 Oct 2026 23:18:33 +0000 Subject: [PATCH 06/13] Svc/GenericHub: report allocation failures once per episode (atomic flag + Fw::Logger) instead of throttle 5 --- Svc/GenericHub/GenericHub.cpp | 12 +++++++-- Svc/GenericHub/GenericHub.fpp | 3 +-- Svc/GenericHub/GenericHub.hpp | 1 + Svc/GenericHub/docs/sdd.md | 24 ++++++++++------- Svc/GenericHub/test/ut/GenericHubTester.cpp | 26 +++++++++++++++---- Svc/GenericHub/test/ut/GenericHubTester.hpp | 14 +++++++++- .../design-patterns/hub-pattern.md | 5 ++-- 7 files changed, 63 insertions(+), 22 deletions(-) diff --git a/Svc/GenericHub/GenericHub.cpp b/Svc/GenericHub/GenericHub.cpp index 5716810bb34..b19fd6ceb02 100644 --- a/Svc/GenericHub/GenericHub.cpp +++ b/Svc/GenericHub/GenericHub.cpp @@ -25,7 +25,8 @@ namespace Svc { // Construction, initialization, and destruction // ---------------------------------------------------------------------- -GenericHub::GenericHub(const char* const compName) : GenericHubComponentBase(compName), m_numAllocationFailures(0) {} +GenericHub::GenericHub(const char* const compName) + : GenericHubComponentBase(compName), m_numAllocationFailures(0), m_reportingFailure(false) {} GenericHub::~GenericHub() {} @@ -41,7 +42,13 @@ Fw::Success GenericHub::send_data(const HubType type, const FwIndexType port, co deallocate_out(0, outgoing); } this->m_numAllocationFailures++; - this->log_WARNING_HI_BufferAllocationFailed(static_cast(type), port, needed); + // Report once per failure episode (re-armed by the next successful send); later drops are only counted + if (!this->m_reportingFailure.exchange(true)) { + Fw::Logger::log("GenericHub: buffer allocation failed for message type %" PRI_U32 + " on port %" PRI_FwIndexType ": requested %" PRI_FwSizeType " bytes\n", + static_cast(type), port, needed); + this->log_WARNING_HI_BufferAllocationFailed(static_cast(type), port, needed); + } return Fw::Success::FAILURE; } auto serialize = outgoing.getSerializer(); @@ -54,6 +61,7 @@ Fw::Success GenericHub::send_data(const HubType type, const FwIndexType port, co FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast(status)); outgoing.setSize(serialize.getSize()); toBufferDriver_out(0, outgoing); + this->m_reportingFailure.store(false); return Fw::Success::SUCCESS; } diff --git a/Svc/GenericHub/GenericHub.fpp b/Svc/GenericHub/GenericHub.fpp index c69341476ca..55c49699939 100644 --- a/Svc/GenericHub/GenericHub.fpp +++ b/Svc/GenericHub/GenericHub.fpp @@ -235,8 +235,7 @@ module Svc { $size: FwSizeType @< The requested allocation size ) \ severity warning high \ - format "Buffer allocation failed for message type {} on port {}: requested {} bytes" \ - throttle 5 + format "Buffer allocation failed for message type {} on port {}: requested {} bytes" @ The number of failed buffer allocations telemetry NumAllocationFailures: U32 diff --git a/Svc/GenericHub/GenericHub.hpp b/Svc/GenericHub/GenericHub.hpp index 196a1f5c5cb..312e9a684f9 100644 --- a/Svc/GenericHub/GenericHub.hpp +++ b/Svc/GenericHub/GenericHub.hpp @@ -136,6 +136,7 @@ class GenericHub final : public GenericHubComponentBase { Fw::Success send_data(const HubType type, const FwIndexType port, const U8* data, const FwSizeType size); std::atomic m_numAllocationFailures; //!< Count of dropped messages; incremented from any caller thread + std::atomic m_reportingFailure; //!< Set by the first drop of a failure episode, cleared by a successful send }; } // end namespace Svc diff --git a/Svc/GenericHub/docs/sdd.md b/Svc/GenericHub/docs/sdd.md index 04531075311..6359045c63c 100644 --- a/Svc/GenericHub/docs/sdd.md +++ b/Svc/GenericHub/docs/sdd.md @@ -47,7 +47,8 @@ the hub: a pointer is meaningful only in the address space that allocated the pointed-to object. GenericHub is passive. Its only state between calls is the atomic -`NumAllocationFailures` counter and the `BufferAllocationFailed` throttle. Each handler runs on +`NumAllocationFailures` counter and the atomic flag that arms the `BufferAllocationFailed` +report. Each handler runs on its caller's thread, and several producers may be in the send path at once, so the component connected to `toBufferDriver` must accept concurrent calls (for example, the asynchronous inputs of `Svc::ComQueue`). @@ -254,16 +255,17 @@ For a locally produced message, the hub: If `allocate` returns an invalid buffer (null data, or smaller than the framed message), the hub drops the message: it returns a non-null undersized buffer through `deallocate`, increments the `NumAllocationFailures` counter, and emits -the `BufferAllocationFailed` event (throttled to 5) with the message type, port -index, and requested size. Buffers received on `bufferIn` are still returned +on the first drop of a failure episode, the `BufferAllocationFailed` event +(and an `Fw::Logger` line, for the side of the link whose events cannot get +out) with the message type, port index, and requested size. Later drops of the +same episode are only counted; the next successful send re-arms the report. +Buffers received on `bufferIn` are still returned through `bufferInReturn`. The hub remains operational and resumes sending once allocation succeeds. A dropped `cmdDispIn` command is failed back to its caller with `Fw::CmdResponse::EXECUTION_ERROR` on the matching `cmdRespOut` port; a dropped `cmdRespIn` response cannot be reported to the far side and is lost, so the originating sequencer or splitter never receives a response for that -command. Size the pool so that `cmdRespIn` never drops. The event throttle is -not synchronized, so its count is approximate when several threads drop -concurrently. +command. Size the pool so that `cmdRespIn` never drops. The counter is read from any caller thread (`std::atomic`) and written to telemetry only from `schedIn`. Emitting telemetry on the failure path would @@ -271,9 +273,11 @@ re-enter the hub whenever the hub is the deployment's telemetry sink, so the `schedIn` port is required for the channel to be observable. The event is emitted on the failure path; when the hub is also the deployment's event sink (`event connections instance hub` routes `hub.Log` back into `hub.eventIn`), -each failure re-enters the hub once per event until the throttle (5) is -reached, after which failures are only counted. Existing deployments must add -the `schedIn` connection to see the counter. +the one event of an episode re-enters the hub, is dropped, and adds one to the +counter. Existing deployments must add the `schedIn` connection to see the +counter. Retrying or queueing dropped messages was considered and deferred: +the hub is passive so a queue would only move the sizing problem; size the +pool instead. Size the pool for the worst case: the capacity of every queue downstream of `toBufferDriver`, plus buffers held by the driver, plus one per thread that @@ -356,7 +360,7 @@ telemetry database, respectively. | GENHUB-004 | The GenericHub shall work with another GenericHub to send port and buffer calls. | Unit test | | GENHUB-005 | The GenericHub shall forward command dispatches and command responses between deployments. | Unit test | | GENHUB-006 | When `allocate` returns an invalid buffer (null data, or size less than requested), the GenericHub shall drop the outgoing message instead of asserting and shall remain operational. | Unit test | -| GENHUB-007 | On such a drop, the GenericHub shall emit a `WARNING_HI` event (throttled to 5) identifying the message type, port index, and requested size. | Unit test | +| GENHUB-007 | On such a drop, the GenericHub shall emit a `WARNING_HI` event (once per failure episode) identifying the message type, port index, and requested size. | Unit test | | GENHUB-008 | The GenericHub shall count allocation failures and report the count as a telemetry channel from `schedIn`. | Unit test, integration test (fprime-generic-hub-reference, companion change) | | GENHUB-009 | An undersized but valid buffer returned by `allocate` shall be returned through `deallocate`. | Unit test | | GENHUB-010 | Buffers received on `bufferIn` shall be returned through `bufferInReturn` whether or not the outgoing message is dropped. | Unit test | diff --git a/Svc/GenericHub/test/ut/GenericHubTester.cpp b/Svc/GenericHub/test/ut/GenericHubTester.cpp index 8dd32028edb..75ec5bff195 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.cpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.cpp @@ -38,6 +38,7 @@ GenericHubTester ::GenericHubTester() m_cmdRespOutPort(0), m_allocation_mode(ALLOCATE_OK), m_allocation_failures(0) { + Fw::Logger::registerLogger(&m_logger); this->initComponents(); this->connectPorts(); } @@ -330,13 +331,17 @@ void GenericHubTester ::assert_dropped(GenericHub::HubType type, FwIndexType por ASSERT_from_deallocate_SIZE(0); } ASSERT_TLM_NumAllocationFailures_SIZE(0); - // The event is throttled after EVENTID_BUFFERALLOCATIONFAILED_THROTTLE drops; the counter is not - if (m_allocation_failures < GenericHubComponentBase::EVENTID_BUFFERALLOCATIONFAILED_THROTTLE) { + // Only the first drop of a failure episode is evented; every drop is counted + if (!m_in_failure_episode) { ASSERT_EVENTS_BufferAllocationFailed_SIZE(1); ASSERT_EVENTS_BufferAllocationFailed(0, static_cast(type), port, needed); + ASSERT_EQ(m_logger.m_count, 1); } else { ASSERT_EVENTS_BufferAllocationFailed_SIZE(0); + ASSERT_EQ(m_logger.m_count, 0); } + m_logger.m_count = 0; + m_in_failure_episode = true; m_allocation_failures++; this->clearHistory(); } @@ -379,7 +384,7 @@ void GenericHubTester ::test_allocation_failure() { assert_dropped(GenericHub::HUB_TYPE_CMD_RESP, 1, sizeof(FwOpcodeType) + sizeof(U32) + Fw::CmdResponse::SERIALIZED_SIZE); - // An undersized buffer is dropped and returned to the allocator; the event is now throttled + // An undersized buffer is dropped and returned to the allocator; still the same episode, no new event m_allocation_mode = ALLOCATE_UNDERSIZED; invoke_to_serialIn(0, comm); assert_dropped(GenericHub::HUB_TYPE_PORT, 0, comm.getSize()); @@ -390,7 +395,7 @@ void GenericHubTester ::test_allocation_failure() { ASSERT_TLM_NumAllocationFailures(0, m_allocation_failures); this->clearHistory(); - // Counter keeps counting while the event is throttled + // Counter keeps counting for the rest of the episode m_allocation_mode = ALLOCATE_EMPTY; for (U32 i = 0; i < 10; i++) { invoke_to_serialIn(0, comm); @@ -400,10 +405,21 @@ void GenericHubTester ::test_allocation_failure() { ASSERT_TLM_NumAllocationFailures(0, m_allocation_failures); this->clearHistory(); - // Hub keeps working once allocation succeeds again + // Hub keeps working once allocation succeeds again, which re-arms the report m_allocation_mode = ALLOCATE_OK; send_random_comm(0); ASSERT_EVENTS_BufferAllocationFailed_SIZE(0); + m_in_failure_episode = false; + this->clearHistory(); + + // A new failure episode is evented again, once + m_allocation_mode = ALLOCATE_EMPTY; + invoke_to_serialIn(0, comm); + assert_dropped(GenericHub::HUB_TYPE_PORT, 0, comm.getSize()); + invoke_to_serialIn(0, comm); + assert_dropped(GenericHub::HUB_TYPE_PORT, 0, comm.getSize()); + invoke_to_schedIn(0, 0); + ASSERT_TLM_NumAllocationFailures(0, m_allocation_failures); } void GenericHubTester ::test_command_dispatch_allocation_failure() { diff --git a/Svc/GenericHub/test/ut/GenericHubTester.hpp b/Svc/GenericHub/test/ut/GenericHubTester.hpp index 995ff970cda..2fd23650134 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.hpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.hpp @@ -13,6 +13,7 @@ #ifndef TESTER_HPP #define TESTER_HPP #include +#include #include "GenericHubGTestBase.hpp" #include "Svc/GenericHub/GenericHub.hpp" @@ -21,6 +22,15 @@ namespace Svc { +//! Fw::Logger sink counting the hub's console diagnostics +class CountingLogger : public Fw::Logger { + public: + U32 m_count = 0; + + protected: + void writeMessage(const Fw::ConstStringBase& message) override { m_count++; } +}; + class GenericHubTester : public GenericHubGTestBase { // ---------------------------------------------------------------------- // Construction and destruction @@ -230,8 +240,10 @@ class GenericHubTester : public GenericHubGTestBase { U32 m_current_port; FwIndexType m_cmdDispOutPort; FwIndexType m_cmdRespOutPort; + CountingLogger m_logger; AllocationMode m_allocation_mode; - U32 m_allocation_failures; //!< Drops injected so far, mirrors the hub's counter + bool m_in_failure_episode = false; //!< True once a drop occurred and no successful send followed + U32 m_allocation_failures; //!< Drops injected so far, mirrors the hub's counter U8 m_data_store[DATA_SIZE]; U8 m_data_for_allocation[DATA_SIZE]; }; diff --git a/docs/user-manual/design-patterns/hub-pattern.md b/docs/user-manual/design-patterns/hub-pattern.md index 34fc15be084..bdaffa3b85a 100644 --- a/docs/user-manual/design-patterns/hub-pattern.md +++ b/docs/user-manual/design-patterns/hub-pattern.md @@ -71,8 +71,9 @@ port. - Wire command ports with the same indices on both hubs; the index is how a command response finds its originator. - Size the hub's buffer pool for the worst case. When allocation fails, - GenericHub drops the message, emits `BufferAllocationFailed`, and counts it - in `NumAllocationFailures`; drive `schedIn` from a rate group so the counter + GenericHub drops the message, emits `BufferAllocationFailed` (once per + failure episode, re-armed by the next successful send) and counts every + drop in `NumAllocationFailures`; drive `schedIn` from a rate group so the counter is reported (deployments written before `schedIn` existed must add this connection). A dropped `cmdDispIn` command is failed back to its caller with `EXECUTION_ERROR`. From 8bcb1d2382c80794a240c9e96096bf003cc2237a Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Wed, 21 Oct 2026 23:24:16 +0000 Subject: [PATCH 07/13] Svc/GenericHub UT: unregister the counting logger on tester teardown --- Svc/GenericHub/test/ut/GenericHubTester.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Svc/GenericHub/test/ut/GenericHubTester.cpp b/Svc/GenericHub/test/ut/GenericHubTester.cpp index 75ec5bff195..344b3b609cb 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.cpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.cpp @@ -43,7 +43,9 @@ GenericHubTester ::GenericHubTester() this->connectPorts(); } -GenericHubTester ::~GenericHubTester() {} +GenericHubTester ::~GenericHubTester() { + Fw::Logger::registerLogger(nullptr); +} // ---------------------------------------------------------------------- // Tests From 723858aac33b38b0355417cf96aaa7eaf8d27043 Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Thu, 5 Nov 2026 02:51:47 +0000 Subject: [PATCH 08/13] Svc/GenericHub: type the hub message type as an FPP enum; UTs for self-sink and deferred buffer return --- Svc/GenericHub/GenericHub.cpp | 37 ++--- Svc/GenericHub/GenericHub.fpp | 12 +- Svc/GenericHub/GenericHub.hpp | 20 +-- Svc/GenericHub/test/ut/GenericHubTestMain.cpp | 10 ++ Svc/GenericHub/test/ut/GenericHubTester.cpp | 128 +++++++++++++++--- Svc/GenericHub/test/ut/GenericHubTester.hpp | 27 +++- 6 files changed, 174 insertions(+), 60 deletions(-) diff --git a/Svc/GenericHub/GenericHub.cpp b/Svc/GenericHub/GenericHub.cpp index b19fd6ceb02..0694996ce39 100644 --- a/Svc/GenericHub/GenericHub.cpp +++ b/Svc/GenericHub/GenericHub.cpp @@ -30,7 +30,10 @@ GenericHub::GenericHub(const char* const compName) GenericHub::~GenericHub() {} -Fw::Success GenericHub::send_data(const HubType type, const FwIndexType port, const U8* data, const FwSizeType size) { +Fw::Success GenericHub::send_data(const GenericHub_HubType::T type, + const FwIndexType port, + const U8* data, + const FwSizeType size) { FW_ASSERT(data != nullptr); Fw::SerializeStatus status; const FwSizeType needed = size + sizeof(U32) + sizeof(U32) + sizeof(FwBuffSizeType); @@ -47,7 +50,7 @@ Fw::Success GenericHub::send_data(const HubType type, const FwIndexType port, co Fw::Logger::log("GenericHub: buffer allocation failed for message type %" PRI_U32 " on port %" PRI_FwIndexType ": requested %" PRI_FwSizeType " bytes\n", static_cast(type), port, needed); - this->log_WARNING_HI_BufferAllocationFailed(static_cast(type), port, needed); + this->log_WARNING_HI_BufferAllocationFailed(type, port, needed); } return Fw::Success::FAILURE; } @@ -71,7 +74,7 @@ Fw::Success GenericHub::send_data(const HubType type, const FwIndexType port, co void GenericHub::bufferIn_handler(const FwIndexType portNum, Fw::Buffer& fwBuffer) { // A drop is counted and evented in send_data; this port has no status to return - (void)this->send_data(HUB_TYPE_BUFFER, portNum, fwBuffer.getData(), fwBuffer.getSize()); + (void)this->send_data(GenericHub_HubType::HUB_TYPE_BUFFER, portNum, fwBuffer.getData(), fwBuffer.getSize()); bufferInReturn_out(portNum, fwBuffer); } @@ -95,7 +98,7 @@ void GenericHub ::cmdDispIn_handler(FwIndexType portNum, Fw::ComBuffer& data, U3 FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast(status)); FwSizeType size = serializer.getSize(); - if (this->send_data(HUB_TYPE_CMD_DISP, portNum, buffer, size) != Fw::Success::SUCCESS) { + if (this->send_data(GenericHub_HubType::HUB_TYPE_CMD_DISP, portNum, buffer, size) != Fw::Success::SUCCESS) { // The command was dropped: fail it back to the caller so it is not left waiting for a response // Read the opcode with a fresh cursor: an upstream CmdSplitter may already have consumed `data` Fw::ExternalSerializeBuffer reader(data.getBuffAddr(), data.getSize()); @@ -126,11 +129,11 @@ void GenericHub ::cmdRespIn_handler(FwIndexType portNum, FwSizeType size = serializer.getSize(); // A dropped response cannot be reported to the far side (see SDD, "Allocation failure") - (void)this->send_data(HUB_TYPE_CMD_RESP, portNum, buffer, size); + (void)this->send_data(GenericHub_HubType::HUB_TYPE_CMD_RESP, portNum, buffer, size); } void GenericHub::fromBufferDriver_handler(const FwIndexType portNum, Fw::Buffer& fwBuffer) { - HubType type = HUB_TYPE_MAX; + GenericHub_HubType::T type = GenericHub_HubType::HUB_TYPE_PORT; // Set once type_in is validated below U32 type_in = 0; U32 port = 0; FwBuffSizeType size = 0; @@ -145,12 +148,12 @@ void GenericHub::fromBufferDriver_handler(const FwIndexType portNum, Fw::Buffer& status = incoming.deserializeTo(type_in); } // If the deserialization was good, but the type is invalid, set invalid data - if ((status == Fw::FW_SERIALIZE_OK) && (type_in >= HUB_TYPE_MAX)) { + if ((status == Fw::FW_SERIALIZE_OK) && (type_in >= static_cast(GenericHub_HubType::NUM_CONSTANTS))) { status = Fw::FW_DESERIALIZE_INVALID_DATA; } // If the deserialization was good and the type was valid, set the type and move onto port deserialization else if (status == Fw::FW_SERIALIZE_OK) { - type = static_cast(type_in); + type = static_cast(type_in); status = incoming.deserializeTo(port); } // If the deserialization was good, move onto size deserialization @@ -163,7 +166,7 @@ void GenericHub::fromBufferDriver_handler(const FwIndexType portNum, Fw::Buffer& // invokeSerial deserializes arguments before calling a normal invoke, this will return ownership immediately U8* rawData = fwBuffer.getData() + sizeof(U32) + sizeof(U32) + sizeof(FwBuffSizeType); FwSizeType rawSize = fwBuffer.getSize() - sizeof(U32) - sizeof(U32) - sizeof(FwBuffSizeType); - if (type == HUB_TYPE_PORT) { + if (type == GenericHub_HubType::HUB_TYPE_PORT) { // Com buffer representations should be copied before the call returns, so we need not "allocate" new data Fw::ExternalSerializeBuffer wrapper(rawData, rawSize); status = wrapper.setBuffLen(rawSize); @@ -175,7 +178,7 @@ void GenericHub::fromBufferDriver_handler(const FwIndexType portNum, Fw::Buffer& } // Deallocate the existing buffer fromBufferDriverReturn_out(0, fwBuffer); - } else if (type == HUB_TYPE_BUFFER) { + } else if (type == GenericHub_HubType::HUB_TYPE_BUFFER) { // Fw::Buffers can reuse the existing data buffer as the storage type! No deallocation done. fwBuffer.set(rawData, rawSize, fwBuffer.getContext()); // Confirm that the port is valid and connected before calling out @@ -186,7 +189,7 @@ void GenericHub::fromBufferDriver_handler(const FwIndexType portNum, Fw::Buffer& // Return the buffer if the port is invalid or not connected to avoid leaks fromBufferDriverReturn_out(0, fwBuffer); } - } else if (type == HUB_TYPE_EVENT) { + } else if (type == GenericHub_HubType::HUB_TYPE_EVENT) { FwEventIdType id; Fw::Time timeTag; Fw::LogSeverity severity; @@ -211,7 +214,7 @@ void GenericHub::fromBufferDriver_handler(const FwIndexType portNum, Fw::Buffer& } // Deallocate the existing buffer fromBufferDriverReturn_out(0, fwBuffer); - } else if (type == HUB_TYPE_CHANNEL) { + } else if (type == GenericHub_HubType::HUB_TYPE_CHANNEL) { FwChanIdType id; Fw::Time timeTag; Fw::TlmBuffer val; @@ -232,7 +235,7 @@ void GenericHub::fromBufferDriver_handler(const FwIndexType portNum, Fw::Buffer& // Return the received buffer fromBufferDriverReturn_out(0, fwBuffer); - } else if (type == HUB_TYPE_CMD_DISP) { + } else if (type == GenericHub_HubType::HUB_TYPE_CMD_DISP) { U32 context; // Check that the size is sufficient for the context // The wrapped command must fit the ComBuffer capacity; SERIALIZED_SIZE is the capacity @@ -257,7 +260,7 @@ void GenericHub::fromBufferDriver_handler(const FwIndexType portNum, Fw::Buffer& } // Deallocate the existing buffer fromBufferDriverReturn_out(0, fwBuffer); - } else if (type == HUB_TYPE_CMD_RESP) { + } else if (type == GenericHub_HubType::HUB_TYPE_CMD_RESP) { FwOpcodeType opCode; U32 cmdSeq; Fw::CmdResponse response; @@ -313,7 +316,7 @@ void GenericHub::eventIn_handler(const FwIndexType portNum, FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK); FwSizeType size = serializer.getSize(); // A drop is counted and evented in send_data; this port has no status to return - (void)this->send_data(HubType::HUB_TYPE_EVENT, portNum, buffer, size); + (void)this->send_data(GenericHub_HubType::HUB_TYPE_EVENT, portNum, buffer, size); } void GenericHub::tlmIn_handler(const FwIndexType portNum, FwChanIdType id, Fw::Time& timeTag, Fw::TlmBuffer& val) { @@ -329,7 +332,7 @@ void GenericHub::tlmIn_handler(const FwIndexType portNum, FwChanIdType id, Fw::T FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK); FwSizeType size = serializer.getSize(); // A drop is counted and evented in send_data; this port has no status to return - (void)this->send_data(HubType::HUB_TYPE_CHANNEL, portNum, buffer, size); + (void)this->send_data(GenericHub_HubType::HUB_TYPE_CHANNEL, portNum, buffer, size); } // ---------------------------------------------------------------------- @@ -340,7 +343,7 @@ void GenericHub::serialIn_handler(FwIndexType portNum, /*!< The port num Fw::LinearBufferBase& Buffer /*!< The serialization buffer*/ ) { // A drop is counted and evented in send_data; this port has no status to return - (void)send_data(HUB_TYPE_PORT, portNum, Buffer.getBuffAddr(), Buffer.getSize()); + (void)send_data(GenericHub_HubType::HUB_TYPE_PORT, portNum, Buffer.getBuffAddr(), Buffer.getSize()); } } // end namespace Svc diff --git a/Svc/GenericHub/GenericHub.fpp b/Svc/GenericHub/GenericHub.fpp index 55c49699939..c87ae21c619 100644 --- a/Svc/GenericHub/GenericHub.fpp +++ b/Svc/GenericHub/GenericHub.fpp @@ -42,6 +42,16 @@ module Svc { @| ---------------------------------------------------------------------- passive component GenericHub { + @ Type of serialized data on the wire, allowing the receiving hub to expand it + enum HubType : U32 { + HUB_TYPE_PORT @< Port type transmission + HUB_TYPE_BUFFER @< Buffer type transmission + HUB_TYPE_EVENT @< Event transmission + HUB_TYPE_CHANNEL @< Telemetry channel type + HUB_TYPE_CMD_DISP @< Command dispatch type + HUB_TYPE_CMD_RESP @< Command response type + } + # ---------------------------------------------------------------------- # Ports for sending data from FSW to the hub # ---------------------------------------------------------------------- @@ -230,7 +240,7 @@ module Svc { @ Buffer allocation failed; the outgoing message was dropped event BufferAllocationFailed( - msgType: U32 @< The hub message type (HubType) of the dropped message + msgType: HubType @< The hub message type of the dropped message $port: FwIndexType @< The input port index of the dropped message $size: FwSizeType @< The requested allocation size ) \ diff --git a/Svc/GenericHub/GenericHub.hpp b/Svc/GenericHub/GenericHub.hpp index 312e9a684f9..53b0cf47fdf 100644 --- a/Svc/GenericHub/GenericHub.hpp +++ b/Svc/GenericHub/GenericHub.hpp @@ -21,21 +21,6 @@ namespace Svc { class GenericHub final : public GenericHubComponentBase { public: - /** - * HubType: - * - * Type of serialized data on the wire. Allows for expanding them on the opposing end. - */ - enum HubType { - HUB_TYPE_PORT, //!< Port type transmission - HUB_TYPE_BUFFER, //!< Buffer type transmission - HUB_TYPE_EVENT, //!< Event transmission - HUB_TYPE_CHANNEL, //!< Telemetry channel type - HUB_TYPE_CMD_DISP, //!< Command dispatch type - HUB_TYPE_CMD_RESP, //!< Command response type - HUB_TYPE_MAX - }; - // ---------------------------------------------------------------------- // Construction, initialization, and destruction // ---------------------------------------------------------------------- @@ -133,7 +118,10 @@ class GenericHub final : public GenericHubComponentBase { //! Frame and send data on toBufferDriver. Returns Fw::Success::FAILURE if the message was dropped because //! no sufficiently sized buffer could be allocated. - Fw::Success send_data(const HubType type, const FwIndexType port, const U8* data, const FwSizeType size); + Fw::Success send_data(const GenericHub_HubType::T type, + const FwIndexType port, + const U8* data, + const FwSizeType size); std::atomic m_numAllocationFailures; //!< Count of dropped messages; incremented from any caller thread std::atomic m_reportingFailure; //!< Set by the first drop of a failure episode, cleared by a successful send diff --git a/Svc/GenericHub/test/ut/GenericHubTestMain.cpp b/Svc/GenericHub/test/ut/GenericHubTestMain.cpp index cef7b8e9a24..25711a68a72 100644 --- a/Svc/GenericHub/test/ut/GenericHubTestMain.cpp +++ b/Svc/GenericHub/test/ut/GenericHubTestMain.cpp @@ -67,6 +67,16 @@ TEST(Invalid, TestCommandDispatchAllocationFailure) { tester.test_command_dispatch_allocation_failure(); } +TEST(Invalid, TestSelfSinkAllocationFailure) { + Svc::GenericHubTester tester(true); + tester.test_self_sink(); +} + +TEST(Nominal, TestDeferredBufferReturn) { + Svc::GenericHubTester tester; + tester.test_deferred_return(); +} + int main(int argc, char** argv) { STest::Random::seed(); ::testing::InitGoogleTest(&argc, argv); diff --git a/Svc/GenericHub/test/ut/GenericHubTester.cpp b/Svc/GenericHub/test/ut/GenericHubTester.cpp index 344b3b609cb..77bf34e868e 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.cpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.cpp @@ -24,7 +24,7 @@ namespace Svc { // Construction and destruction // ---------------------------------------------------------------------- -GenericHubTester ::GenericHubTester() +GenericHubTester ::GenericHubTester(bool selfSink) : GenericHubGTestBase("Tester", MAX_HISTORY_SIZE), componentIn("GenericHubIn"), componentOut("GenericHubOut"), @@ -37,6 +37,7 @@ GenericHubTester ::GenericHubTester() m_cmdDispOutPort(0), m_cmdRespOutPort(0), m_allocation_mode(ALLOCATE_OK), + m_self_sink(selfSink), m_allocation_failures(0) { Fw::Logger::registerLogger(&m_logger); this->initComponents(); @@ -279,33 +280,33 @@ void GenericHubTester ::test_invalid_deserialization_paths() { // Invalid type should be dropped clearFromPortHistory(); - this->send_from_driver_packet(static_cast(GenericHub::HUB_TYPE_MAX), 0, 0, nullptr, 0); + this->send_from_driver_packet(static_cast(GenericHub_HubType::NUM_CONSTANTS), 0, 0, nullptr, 0); ASSERT_from_fromBufferDriverReturn_SIZE(1); this->assert_no_outputs_sent(); // Declared payload size mismatch should be dropped clearFromPortHistory(); - this->send_from_driver_packet(static_cast(GenericHub::HUB_TYPE_PORT), 0, 1, nullptr, 0); + this->send_from_driver_packet(static_cast(GenericHub_HubType::HUB_TYPE_PORT), 0, 1, nullptr, 0); ASSERT_from_fromBufferDriverReturn_SIZE(1); this->assert_no_outputs_sent(); // Invalid serialOut destination port should not invoke output clearFromPortHistory(); - this->send_from_driver_packet(static_cast(GenericHub::HUB_TYPE_PORT), + this->send_from_driver_packet(static_cast(GenericHub_HubType::HUB_TYPE_PORT), this->componentOut.getNum_serialOut_OutputPorts(), 0, nullptr, 0); ASSERT_from_fromBufferDriverReturn_SIZE(1); this->assert_no_outputs_sent(); // Invalid bufferOut destination port should return the buffer clearFromPortHistory(); - this->send_from_driver_packet(static_cast(GenericHub::HUB_TYPE_BUFFER), + this->send_from_driver_packet(static_cast(GenericHub_HubType::HUB_TYPE_BUFFER), this->componentOut.getNum_bufferOut_OutputPorts(), 0, nullptr, 0); ASSERT_from_fromBufferDriverReturn_SIZE(1); this->assert_no_outputs_sent(); // Command dispatch path: raw payload too small to include context clearFromPortHistory(); - this->send_from_driver_packet(static_cast(GenericHub::HUB_TYPE_CMD_DISP), 0, 0, nullptr, 0); + this->send_from_driver_packet(static_cast(GenericHub_HubType::HUB_TYPE_CMD_DISP), 0, 0, nullptr, 0); ASSERT_from_fromBufferDriverReturn_SIZE(1); this->assert_no_outputs_sent(); @@ -313,15 +314,15 @@ void GenericHubTester ::test_invalid_deserialization_paths() { clearFromPortHistory(); const FwBuffSizeType tooLargeCmdSize = static_cast(Fw::ComBuffer::SERIALIZED_SIZE + sizeof(U32) + 1); - this->send_from_driver_packet(static_cast(GenericHub::HUB_TYPE_CMD_DISP), 0, tooLargeCmdSize, m_data_store, - tooLargeCmdSize); + this->send_from_driver_packet(static_cast(GenericHub_HubType::HUB_TYPE_CMD_DISP), 0, tooLargeCmdSize, + m_data_store, tooLargeCmdSize); ASSERT_from_fromBufferDriverReturn_SIZE(1); this->assert_no_outputs_sent(); clearFromPortHistory(); } -void GenericHubTester ::assert_dropped(GenericHub::HubType type, FwIndexType port, FwSizeType payloadSize) { +void GenericHubTester ::assert_dropped(GenericHub_HubType::T type, FwIndexType port, FwSizeType payloadSize) { const FwSizeType needed = payloadSize + sizeof(U32) + sizeof(U32) + sizeof(FwBuffSizeType); ASSERT_from_toBufferDriver_SIZE(0); // Only a non-null (undersized) buffer is handed back to the allocator; telemetry is only written from schedIn @@ -356,21 +357,21 @@ void GenericHubTester ::test_allocation_failure() { Fw::ComBuffer comm; random_fill(comm, FW_COM_BUFFER_MAX_SIZE); invoke_to_serialIn(1, comm); - assert_dropped(GenericHub::HUB_TYPE_PORT, 1, comm.getSize()); + assert_dropped(GenericHub_HubType::HUB_TYPE_PORT, 1, comm.getSize()); // Buffer is dropped but still returned to its owner m_buffer.set(m_data_store, STest::Pick::lowerUpper(1, FW_COM_BUFFER_MAX_SIZE)); invoke_to_bufferIn(2, m_buffer); ASSERT_from_bufferInReturn_SIZE(1); ASSERT_from_bufferInReturn(0, m_buffer); - assert_dropped(GenericHub::HUB_TYPE_BUFFER, 2, m_buffer.getSize()); + assert_dropped(GenericHub_HubType::HUB_TYPE_BUFFER, 2, m_buffer.getSize()); // Event is dropped Fw::LogBuffer logArgs; Fw::Time time(100, 200); fill(logArgs, STest::Pick::lowerUpper(0, FW_LOG_BUFFER_MAX_SIZE)); invoke_to_eventIn(0, 123, time, Fw::LogSeverity::WARNING_HI, logArgs); - assert_dropped(GenericHub::HUB_TYPE_EVENT, 0, + assert_dropped(GenericHub_HubType::HUB_TYPE_EVENT, 0, sizeof(FwEventIdType) + Fw::Time::SERIALIZED_SIZE + Fw::LogSeverity::SERIALIZED_SIZE + sizeof(FwSizeStoreType) + logArgs.getSize()); @@ -378,18 +379,18 @@ void GenericHubTester ::test_allocation_failure() { Fw::TlmBuffer tlmVal; fill(tlmVal, STest::Pick::lowerUpper(0, FW_TLM_BUFFER_MAX_SIZE)); invoke_to_tlmIn(0, 123, time, tlmVal); - assert_dropped(GenericHub::HUB_TYPE_CHANNEL, 0, + assert_dropped(GenericHub_HubType::HUB_TYPE_CHANNEL, 0, sizeof(FwChanIdType) + Fw::Time::SERIALIZED_SIZE + sizeof(FwSizeStoreType) + tlmVal.getSize()); // Command response is dropped invoke_to_cmdRespIn(1, 0x3A56BF8C, 825, Fw::CmdResponse::OK); - assert_dropped(GenericHub::HUB_TYPE_CMD_RESP, 1, + assert_dropped(GenericHub_HubType::HUB_TYPE_CMD_RESP, 1, sizeof(FwOpcodeType) + sizeof(U32) + Fw::CmdResponse::SERIALIZED_SIZE); // An undersized buffer is dropped and returned to the allocator; still the same episode, no new event m_allocation_mode = ALLOCATE_UNDERSIZED; invoke_to_serialIn(0, comm); - assert_dropped(GenericHub::HUB_TYPE_PORT, 0, comm.getSize()); + assert_dropped(GenericHub_HubType::HUB_TYPE_PORT, 0, comm.getSize()); // Counter is reported on schedIn invoke_to_schedIn(0, 0); @@ -401,7 +402,7 @@ void GenericHubTester ::test_allocation_failure() { m_allocation_mode = ALLOCATE_EMPTY; for (U32 i = 0; i < 10; i++) { invoke_to_serialIn(0, comm); - assert_dropped(GenericHub::HUB_TYPE_PORT, 0, comm.getSize()); + assert_dropped(GenericHub_HubType::HUB_TYPE_PORT, 0, comm.getSize()); } invoke_to_schedIn(0, 0); ASSERT_TLM_NumAllocationFailures(0, m_allocation_failures); @@ -417,9 +418,9 @@ void GenericHubTester ::test_allocation_failure() { // A new failure episode is evented again, once m_allocation_mode = ALLOCATE_EMPTY; invoke_to_serialIn(0, comm); - assert_dropped(GenericHub::HUB_TYPE_PORT, 0, comm.getSize()); + assert_dropped(GenericHub_HubType::HUB_TYPE_PORT, 0, comm.getSize()); invoke_to_serialIn(0, comm); - assert_dropped(GenericHub::HUB_TYPE_PORT, 0, comm.getSize()); + assert_dropped(GenericHub_HubType::HUB_TYPE_PORT, 0, comm.getSize()); invoke_to_schedIn(0, 0); ASSERT_TLM_NumAllocationFailures(0, m_allocation_failures); } @@ -451,7 +452,7 @@ void GenericHubTester ::test_command_dispatch_allocation_failure() { ASSERT_from_cmdRespOut_SIZE(1); ASSERT_from_cmdRespOut(0, opCode, context, Fw::CmdResponse::EXECUTION_ERROR); ASSERT_EQ(m_cmdRespOutPort, port); - assert_dropped(GenericHub::HUB_TYPE_CMD_DISP, port, buffer.getSize() + sizeof(U32)); + assert_dropped(GenericHub_HubType::HUB_TYPE_CMD_DISP, port, buffer.getSize() + sizeof(U32)); // A malformed (non-command) packet that is dropped is still failed back, with the reserved opcode 0 buffer.resetSer(); @@ -462,13 +463,87 @@ void GenericHubTester ::test_command_dispatch_allocation_failure() { ASSERT_from_cmdRespOut_SIZE(1); ASSERT_from_cmdRespOut(0, 0, context, Fw::CmdResponse::EXECUTION_ERROR); ASSERT_EQ(m_cmdRespOutPort, port); - assert_dropped(GenericHub::HUB_TYPE_CMD_DISP, port, buffer.getSize() + sizeof(U32)); + assert_dropped(GenericHub_HubType::HUB_TYPE_CMD_DISP, port, buffer.getSize() + sizeof(U32)); } // ---------------------------------------------------------------------- // Handlers for typed from ports // ---------------------------------------------------------------------- +void GenericHubTester ::test_self_sink() { + this->clearHistory(); + Fw::ComBuffer comm; + random_fill(comm, FW_COM_BUFFER_MAX_SIZE); + + // Pool empty: the one event of the episode re-enters eventIn, is dropped and counted; no cascade + m_allocation_mode = ALLOCATE_EMPTY; + invoke_to_serialIn(0, comm); + ASSERT_from_toBufferDriver_SIZE(0); + ASSERT_EQ(m_logger.m_count, 1); + invoke_to_schedIn(0, 0); + ASSERT_TLM_NumAllocationFailures(0, 2); + m_logger.m_count = 0; + this->clearHistory(); + + // A successful send ends the episode + m_allocation_mode = ALLOCATE_OK; + send_random_comm(0); + this->clearHistory(); + + // Pool recovers on the re-entry: the event frame goes out and re-arms the report + m_allocation_mode = ALLOCATE_EMPTY_ONCE; + invoke_to_serialIn(0, comm); + ASSERT_from_toBufferDriver_SIZE(1); + ASSERT_from_eventOut_SIZE(1); + ASSERT_EQ(fromPortHistory_eventOut->at(0).severity, Fw::LogSeverity::WARNING_HI); + ASSERT_EQ(m_logger.m_count, 1); + invoke_to_schedIn(0, 0); + ASSERT_TLM_NumAllocationFailures(0, 3); + m_logger.m_count = 0; + this->clearHistory(); + + // The next drop starts a new episode: reported (and dropped on re-entry) once more + m_allocation_mode = ALLOCATE_EMPTY; + invoke_to_serialIn(0, comm); + ASSERT_from_toBufferDriver_SIZE(0); + ASSERT_EQ(m_logger.m_count, 1); + invoke_to_schedIn(0, 0); + ASSERT_TLM_NumAllocationFailures(0, 5); +} + +void GenericHubTester ::returnTask(void* ptr) { + GenericHubTester* tester = static_cast(ptr); + tester->invoke_to_toBufferDriverReturn(0, tester->m_deferred); +} + +void GenericHubTester ::test_deferred_return() { + this->clearHistory(); + m_defer_return = true; + Fw::ComBuffer comm; + random_fill(comm, FW_COM_BUFFER_MAX_SIZE); + invoke_to_serialIn(0, comm); + ASSERT_from_toBufferDriver_SIZE(1); + ASSERT_from_deallocate_SIZE(0); + ASSERT_NE(m_deferred.getData(), nullptr); + + // Return the frame from another thread once the sending call has completed + Os::Task task; + Os::Task::Arguments arguments(Fw::String("HubReturn"), GenericHubTester::returnTask, this); + ASSERT_EQ(Os::Task::Status::OP_OK, task.start(arguments)); + ASSERT_EQ(Os::Task::Status::OP_OK, task.join()); + ASSERT_from_deallocate_SIZE(1); + ASSERT_EQ(this->fromPortHistory_deallocate->at(0).fwBuffer.getData(), m_deferred.getData()); + ASSERT_EQ(this->fromPortHistory_deallocate->at(0).fwBuffer.getSize(), m_deferred.getSize()); + ASSERT_EVENTS_BufferAllocationFailed_SIZE(0); + invoke_to_schedIn(0, 0); + ASSERT_TLM_NumAllocationFailures(0, 0); + + // The hub keeps sending normally afterwards + m_defer_return = false; + this->clearHistory(); + send_random_comm(0); +} + void GenericHubTester ::from_eventOut_handler(const FwIndexType portNum, FwEventIdType id, Fw::Time& timeTag, @@ -504,6 +579,10 @@ void GenericHubTester ::from_toBufferDriver_handler(const FwIndexType portNum, F << "Incorrect data pointer deallocated"; // Reuse m_allocate to pass into the otherside of the hub this->pushFromPortEntry_toBufferDriver(fwBuffer); + if (m_defer_return) { + m_deferred = fwBuffer; // An asynchronous consumer (e.g. Svc::ComQueue) returns it later + return; + } invoke_to_fromBufferDriver(0, fwBuffer); } @@ -544,6 +623,9 @@ Fw::Buffer GenericHubTester ::from_allocate_handler(const FwIndexType portNum, c switch (m_allocation_mode) { case ALLOCATE_EMPTY: return Fw::Buffer(); + case ALLOCATE_EMPTY_ONCE: + m_allocation_mode = ALLOCATE_OK; + return Fw::Buffer(); case ALLOCATE_UNDERSIZED: m_allocate.set(m_data_for_allocation, size - 1); break; @@ -651,7 +733,11 @@ void GenericHubTester ::connectPorts() { // schedIn, events, telemetry and time (sending side only) this->connect_to_schedIn(0, this->componentIn.get_schedIn_InputPort(0)); - this->componentIn.set_Log_OutputPort(0, this->get_from_Log(0)); + if (m_self_sink) { + this->componentIn.set_Log_OutputPort(0, this->componentIn.get_eventIn_InputPort(0)); + } else { + this->componentIn.set_Log_OutputPort(0, this->get_from_Log(0)); + } this->componentIn.set_LogText_OutputPort(0, this->get_from_LogText(0)); this->componentIn.set_Time_OutputPort(0, this->get_from_Time(0)); this->componentIn.set_Tlm_OutputPort(0, this->get_from_Tlm(0)); diff --git a/Svc/GenericHub/test/ut/GenericHubTester.hpp b/Svc/GenericHub/test/ut/GenericHubTester.hpp index 2fd23650134..c0fa4b285f8 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.hpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.hpp @@ -14,6 +14,8 @@ #define TESTER_HPP #include #include +#include +#include #include "GenericHubGTestBase.hpp" #include "Svc/GenericHub/GenericHub.hpp" @@ -38,8 +40,9 @@ class GenericHubTester : public GenericHubGTestBase { public: //! Construct object GenericHubTester + //! \param selfSink route the sending hub's own Log port back into its eventIn (hub as the event sink) //! - GenericHubTester(); + explicit GenericHubTester(bool selfSink = false); //! Destroy object GenericHubTester //! @@ -98,6 +101,14 @@ class GenericHubTester : public GenericHubGTestBase { //! void test_command_dispatch_allocation_failure(); + //! Test that a hub acting as its own event sink drops the re-entered event without cascading + //! + void test_self_sink(); + + //! Test that an outgoing buffer may be returned later, from another thread + //! + void test_deferred_return(); + private: // ---------------------------------------------------------------------- // Handlers for typed from ports @@ -179,13 +190,16 @@ class GenericHubTester : public GenericHubGTestBase { private: //! How from_allocate_handler responds to the hub enum AllocationMode { - ALLOCATE_OK, //!< Return a buffer of the requested size - ALLOCATE_EMPTY, //!< Return an empty (null) buffer - ALLOCATE_UNDERSIZED //!< Return a valid buffer one byte short of the request + ALLOCATE_OK, //!< Return a buffer of the requested size + ALLOCATE_EMPTY, //!< Return an empty (null) buffer + ALLOCATE_EMPTY_ONCE, //!< Return an empty buffer, then switch to ALLOCATE_OK + ALLOCATE_UNDERSIZED //!< Return a valid buffer one byte short of the request }; + //! Task routine returning the deferred buffer on toBufferDriverReturn + static void returnTask(void* ptr); //! Assert that the last send was dropped with a single BufferAllocationFailed event for the given payload - void assert_dropped(GenericHub::HubType type, FwIndexType port, FwSizeType payloadSize); + void assert_dropped(GenericHub_HubType::T type, FwIndexType port, FwSizeType payloadSize); void send_random_comm(U32 port); @@ -242,6 +256,9 @@ class GenericHubTester : public GenericHubGTestBase { FwIndexType m_cmdRespOutPort; CountingLogger m_logger; AllocationMode m_allocation_mode; + bool m_self_sink; //!< Log is wired back into eventIn instead of the tester + bool m_defer_return = false; //!< from_toBufferDriver keeps the buffer instead of forwarding it + Fw::Buffer m_deferred; //!< Buffer kept by from_toBufferDriver while m_defer_return is set bool m_in_failure_episode = false; //!< True once a drop occurred and no successful send followed U32 m_allocation_failures; //!< Drops injected so far, mirrors the hub's counter U8 m_data_store[DATA_SIZE]; From c7efba297bda6fbd7458136f92da64d97388f008 Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Thu, 5 Nov 2026 02:53:09 +0000 Subject: [PATCH 09/13] Svc/GenericHub docs: transport formations (ComQueue + ComStub recommended), transport requirements and sizing rule; hub-pattern and ComQueue SDD cross-references --- Svc/ComQueue/docs/sdd.md | 4 + Svc/GenericHub/docs/sdd.md | 133 ++++++++++++++++-- .../design-patterns/hub-pattern.md | 45 +++++- 3 files changed, 165 insertions(+), 17 deletions(-) diff --git a/Svc/ComQueue/docs/sdd.md b/Svc/ComQueue/docs/sdd.md index 860f14cd93c..c9b31b27bbe 100644 --- a/Svc/ComQueue/docs/sdd.md +++ b/Svc/ComQueue/docs/sdd.md @@ -121,6 +121,10 @@ It does the following: In the case where the component is already in `READY` state, this will process the queue immediately after the buffer is added to the queue. +`Svc::GenericHub` is a `bufferQueueIn` producer in the recommended hub wiring (see the +[GenericHub SDD](../../GenericHub/docs/sdd.md#transport-formations)); its frames start with a `U32` message type rather +than a packet descriptor, so the APID read into the frame context is always 0 and is not consumed downstream. + #### 4.5.2 comPacketQueueIn The `comPacketQueueIn` port handler receives an `Fw::ComBuffer` data type and a port number. It does the following: diff --git a/Svc/GenericHub/docs/sdd.md b/Svc/GenericHub/docs/sdd.md index 6359045c63c..914c96a3c9f 100644 --- a/Svc/GenericHub/docs/sdd.md +++ b/Svc/GenericHub/docs/sdd.md @@ -164,7 +164,7 @@ commandDispatcher.seqCmdStatus[0] -> genericHub.cmdRespIn[0] Every message sent to the buffer driver has this outer framing: -1. a `U32` message-type discriminator; +1. a `U32` message-type discriminator (the `Svc.GenericHub.HubType` enum); 2. a `U32` port index; 3. an `FwBuffSizeType` payload size; and 4. the payload bytes. @@ -184,8 +184,8 @@ the received buffer, and dispatches according to the message type. | Command dispatch (`HUB_TYPE_CMD_DISP`) | 4 | The payload contains the serialized `Fw::ComBuffer` command data followed by its `U32` context. The receiver invokes the matching `cmdDispOut` port. | | Command response (`HUB_TYPE_CMD_RESP`) | 5 | The payload contains the opcode, command sequence, and `Fw::CmdResponse`. The receiver invokes the matching `cmdRespOut` port. | -The `msgType` column is the value reported by the `BufferAllocationFailed` -event. +The `msgType` column is the numeric value of the `Svc.GenericHub.HubType` +enumerator reported by the `BufferAllocationFailed` event. Malformed or invalid messages are not dispatched, and the received transport buffer is returned to the driver. A message whose destination port is invalid @@ -275,13 +275,11 @@ emitted on the failure path; when the hub is also the deployment's event sink (`event connections instance hub` routes `hub.Log` back into `hub.eventIn`), the one event of an episode re-enters the hub, is dropped, and adds one to the counter. Existing deployments must add the `schedIn` connection to see the -counter. Retrying or queueing dropped messages was considered and deferred: -the hub is passive so a queue would only move the sizing problem; size the -pool instead. - -Size the pool for the worst case: the capacity of every queue downstream of -`toBufferDriver`, plus buffers held by the driver, plus one per thread that -can call the hub's input ports concurrently. +counter. The hub itself never retries or queues a dropped message: it is +passive, and a queue inside it would only move the sizing problem. Buffering +across a link outage belongs between the hub and the driver; see +[Transport formations](#transport-formations) for the recommended wiring and +the pool-sizing rule. For `bufferIn`, the hub copies the incoming data into the newly allocated transport buffer and immediately returns the original buffer through the @@ -350,6 +348,18 @@ channel, the hub is not an event source or telemetry database. Its `eventOut` and `tlmOut` ports still need to be wired to the deployment's event manager and telemetry database, respectively. +The hub does not reassemble streams: `fromBufferDriver` accepts a buffer only +when it holds exactly one hub message. The transport must therefore preserve +message boundaries, or F´ framing must be added around the link (see +[Transport formations](#transport-formations)). + +In the queued formation, `Svc::ComQueue` reads the first +`FwPacketDescriptorType` of every buffer as an APID for the frame context. A +hub message begins with the `U32` message type (values 0–5), so that APID +always reads 0; nothing downstream of the hub consumes it. No framer sits +between `Svc::ComQueue` and `Svc::ComStub`: the hub's own header is the +framing on a boundary-preserving transport. + ## Requirements | Name | Description | Validation | @@ -365,6 +375,108 @@ telemetry database, respectively. | GENHUB-009 | An undersized but valid buffer returned by `allocate` shall be returned through `deallocate`. | Unit test | | GENHUB-010 | Buffers received on `bufferIn` shall be returned through `bufferInReturn` whether or not the outgoing message is dropped. | Unit test | | GENHUB-011 | A `cmdDispIn` command dropped on allocation failure shall be failed back to its caller with `EXECUTION_ERROR` on the matching `cmdRespOut` port. | Unit test | +| GENHUB-012 | The GenericHub shall remain functionally correct when `toBufferDriver` is connected to an asynchronous consumer that returns buffers on another thread at a later time (for example `Svc.ComQueue.bufferQueueIn`). | Unit test, integration test (fprime-generic-hub-reference) | +| GENHUB-013 | The GenericHub documentation shall define the recommended transport formation (`Svc.ComQueue` → `Svc.ComStub` → driver), the message-boundary requirement on the transport, and the pool- and queue-sizing rule. | Inspection | + +## Transport formations + +The hub is passive and holds nothing between calls: a message it cannot hand +to the driver is dropped (see [Allocation failure](#allocation-failure)). What +happens between the hub and the transport is decided by the topology. Two +formations are in use. + +### Adapter formation + +```text +producers --sync--> GenericHub --toBufferDriver--> Drv.ByteStreamBufferAdapter --$send--> driver + ^ toBufferDriverReturn <---- bufferInReturn (same call) +driver --$recv--> Drv.ByteStreamBufferAdapter --bufferOut--> GenericHub.fromBufferDriver --> consumers +``` + +The socket write happens on the producer's thread, inside the producer's port +call. A driver error is reported by the adapter (`DriverNotReady`, +`DataSendError`, both `WARNING_LO`) and the message is lost; nothing is held +while the link is down. + +### Queued formation (recommended) + +```text +producers --sync--> GenericHub --toBufferDriver (Fw.BufferSend)--> Svc.ComQueue.bufferQueueIn[0] [producer thread] + ^ toBufferDriverReturn <------------------- Svc.ComQueue.bufferReturnOut[0] + Svc.ComQueue.dataOut --Svc.ComDataWithContext--> Svc.ComStub.dataIn --drvSendOut--> driver.$send [ComQueue thread] + Svc.ComQueue.dataReturnIn <-- Svc.ComStub.dataReturnOut ; Svc.ComQueue.comStatusIn <-- Svc.ComStub.comStatusOut + Svc.ComStub.drvConnected <-- driver.ready [driver task] +receive side as in the adapter formation (driver.$recv -> adapter -> GenericHub.fromBufferDriver) +``` + +| Connection | Purpose | +|---|---| +| `hub.toBufferDriver -> comQueue.bufferQueueIn[0]` | Asynchronous, hooked input: the producer only enqueues the buffer handle and returns. | +| `comQueue.bufferReturnOut[0] -> hub.toBufferDriverReturn` | The frame comes back after it was sent (or dropped); the hub returns it to the pool from the `ComQueue` thread. | +| `comQueue.dataOut -> comStub.dataIn`, `comStub.dataReturnOut -> comQueue.dataReturnIn` | The `ComQueue` thread performs the driver write through the communication adapter interface. | +| `comStub.comStatusOut -> comQueue.comStatusIn` | `SUCCESS` releases the next frame; `FAILURE` pauses the queue until the driver reconnects. | +| `driver.ready -> comStub.drvConnected` | Starts the flow and resumes it after a failure. | +| `driver.$send`/`$recv`, `comStub.drvSendOut`, allocation ports | As in the adapter formation; `comStub.drvReceiveIn` stays unconnected. | + +This formation moves the socket write off the producers' threads, holds up to +the configured queue depth of frames while the driver cannot send, resumes +when the driver re-announces readiness, and adds no component to the +framework. `Svc::ComQueue` needs a `run` connection from a rate group for its +telemetry, and its command, event and telemetry ports wired as usual. The +hub's `send_data` behaviour is identical in both formations; only the +consumer of `toBufferDriver` changes (GENHUB-012). + +Residual loss in this formation: the frame in flight when the driver reports +`OTHER_ERROR` (or exhausts `SEND_RETRY`) is returned to the pool without an +event or counter from `Svc::ComStub`; a command lost there is not failed back. +`Svc::ComRetry` is not part of this formation: it is entered from the +`ComQueue` thread and from the driver's reconnect task and is not thread-safe. + +### Transport requirements + +- **Message boundaries.** The hub accepts one message per received buffer + (see [Idiosyncrasies](#idiosyncrasies)). `Drv.Udp` preserves datagram + boundaries and needs nothing else. Stream transports (`Drv.TcpClient`, + `Drv.TcpServer`, `Drv.LinuxUartDriver`) do not: wrap the link in the + standard F´ framing, `Svc.FprimeFramer` on the send side (between + `Svc.ComQueue` and `Svc.ComStub`) and `Svc.FrameAccumulator` + + `Svc.FprimeDeframer` on the receive side. Known gap: `Svc.FprimeDeframer` + emits `Svc.ComDataWithContext` while `fromBufferDriver` is `Fw.BufferSend`, + so a project wiring framing needs a small receive-side adapter that passes + the deframed buffer to the hub and returns it afterwards; the framework does + not provide one. +- **Readiness after a send error.** `Svc::ComQueue` resumes only when + `Svc::ComStub` receives `drvConnected` again. The TCP drivers re-announce + readiness after they reconnect. `Drv.Udp` announces readiness once, at + open, and keeps its socket after a failed send, so over UDP a send error + stalls the queued formation until the driver reconnects; every later frame + is held and, once the pool is empty, dropped at the hub. UDP is therefore + not recommended for the queued formation. + +### Sizing + +Let `N` be the number of hub buffers in the pool, `D` the `Svc::ComQueue` +depth configured for `bufferQueueIn[0]`, and `Q` the `Svc::ComQueue` instance +queue size. + +- `D >= N`: the managed queue can hold every buffer the pool can produce, so + the only drop point is the hub (counted and evented once per episode) and + `Svc::ComQueue` never overflows. +- `Q >= N + 8`: the instance queue carries the `N` buffer handles plus the + status, rate-group and command messages that can be in flight. +- Buffer size `S >= FW_COM_BUFFER_MAX_SIZE + 2 * sizeof(U32) + + sizeof(FwBuffSizeType)` for event, telemetry, command and serial traffic; + `bufferIn` payloads need `S` at least their largest size plus that header. +- Give the hub its own pool (or `Svc.BufferManager` bin): when the bin is + shared with the driver's receive allocation, receive pressure starves sends. +- In the adapter formation `D` and `Q` do not apply; size `N` for the number + of producer threads that can be in the hub at once plus the buffers the + driver holds. + +Operator signatures: `NumAllocationFailures` rising with `BufferManager` +`NoBuffs` rising means the pool is too small or the link is slower than the +producers; `buffQueueDepth[0]` reported at `D` on the same ticks means the +link, not the pool, is the bottleneck; `QueueOverflow` means `D < N`. ## Example formations @@ -388,3 +500,4 @@ The following diagrams show progressively more detail: | 2026-07-09 | Documented command routing, current configuration constants, buffer-driver interfaces, framing, and buffer lifecycle | | 2026-09-29 | Documented threading, command index pairing, cross-deployment type agreement, silent receive drops, and allocation failure | | 2026-10-11 | Allocation failure drops the message with an event and counter instead of asserting; added `schedIn` and the hub's own event/telemetry ports | +| 2026-11-05 | Message type is the `HubType` FPP enum; documented the transport formations (queued formation recommended), transport requirements, and sizing rule | diff --git a/docs/user-manual/design-patterns/hub-pattern.md b/docs/user-manual/design-patterns/hub-pattern.md index bdaffa3b85a..40c2220106d 100644 --- a/docs/user-manual/design-patterns/hub-pattern.md +++ b/docs/user-manual/design-patterns/hub-pattern.md @@ -65,9 +65,16 @@ port. and telemetry database. A hub transports events and telemetry; apart from its own `BufferAllocationFailed` event and `NumAllocationFailures` channel, it is not an event source or telemetry database. -- Use a buffer driver at each end of the transport. The - `Drv::ByteStreamBufferAdapter` can pair a byte-stream driver with the - buffer-driver interface expected by GenericHub. +- Use a buffer driver at each end of the transport, and wire the send side + through `Svc.ComQueue` and `Svc.ComStub` (see + [Recommended wiring](#recommended-wiring)). `Drv::ByteStreamBufferAdapter` + pairs a byte-stream driver's receive side with the hub. +- Use a transport that preserves message boundaries. The hub expects one + message per received buffer and does not reassemble streams: `Drv.Udp` + does this natively; TCP and UART links need the standard F´ framing + (`Svc.FprimeFramer` on the send side, `Svc.FrameAccumulator` + + `Svc.FprimeDeframer` on the receive side) and a small receive-side adapter, + as described in the [GenericHub SDD](../../../Svc/GenericHub/docs/sdd.md#transport-requirements). - Wire command ports with the same indices on both hubs; the index is how a command response finds its originator. - Size the hub's buffer pool for the worst case. When allocation fails, @@ -78,16 +85,40 @@ port. connection). A dropped `cmdDispIn` command is failed back to its caller with `EXECUTION_ERROR`. - Monitor the link outside the hub. GenericHub reports no transport errors and - silently drops invalid received messages. + silently drops invalid received messages; `Svc.ComQueue` reports queue depth + and overflow, `Drv::ByteStreamBufferAdapter` reports driver errors. + +## Recommended wiring + +The hub is passive and never queues: when it cannot get a buffer it drops the +message, emits `BufferAllocationFailed` and counts the drop. Buffering across +a link outage belongs between the hub and the driver. The recommended send +path reuses the framework's communication-adapter components: + +```text +producers -> GenericHub.toBufferDriver -> Svc.ComQueue -> Svc.ComStub -> driver + GenericHub.toBufferDriverReturn <- Svc.ComQueue.bufferReturnOut + Svc.ComStub.drvConnected <- driver.ready +``` + +`Svc.ComQueue` takes the buffer on its own thread, so producers never block on +the socket; it holds frames while the driver cannot send and resumes when the +driver re-announces readiness. Size the queue depth to at least the hub's +buffer count (`D >= N`, instance queue `Q >= N + 8`) so that the hub remains +the single drop point. The receive side stays `driver -> adapter -> GenericHub`. +Over `Drv.Udp` a failed send stalls this path until the driver reconnects, so +prefer TCP or UART with F´ framing. Details, the full connection table, and the +sizing rule are in the +[GenericHub SDD](../../../Svc/GenericHub/docs/sdd.md#transport-formations). ## Putting it together A common layout is: ```text -Deployment A Deployment B -------------- ------------- -FSW -> GenericHub -> transport -> GenericHub -> FSW +Deployment A Deployment B +------------- ------------- +FSW -> GenericHub -> ComQueue -> ComStub -> transport -> GenericHub -> FSW ``` For a runnable worked example, see From 0676f7f878387a9db185d7574d4538b781712d0d Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Thu, 5 Nov 2026 03:09:38 +0000 Subject: [PATCH 10/13] Svc/GenericHub SDD: per-traffic minimum buffer size in the sizing rule (local review iter 4) --- Svc/GenericHub/docs/sdd.md | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Svc/GenericHub/docs/sdd.md b/Svc/GenericHub/docs/sdd.md index 914c96a3c9f..af523552d7d 100644 --- a/Svc/GenericHub/docs/sdd.md +++ b/Svc/GenericHub/docs/sdd.md @@ -464,9 +464,17 @@ queue size. `Svc::ComQueue` never overflows. - `Q >= N + 8`: the instance queue carries the `N` buffer handles plus the status, rate-group and command messages that can be in flight. -- Buffer size `S >= FW_COM_BUFFER_MAX_SIZE + 2 * sizeof(U32) + - sizeof(FwBuffSizeType)` for event, telemetry, command and serial traffic; - `bufferIn` payloads need `S` at least their largest size plus that header. +- Buffer size `S >= max(payload) + 2 * sizeof(U32) + sizeof(FwBuffSizeType)` + (the hub header), where the largest payload per traffic kind is + `FW_COM_BUFFER_MAX_SIZE + sizeof(U32)` for commands (the `Fw::ComBuffer` + plus its context), `sizeof(FwEventIdType) + Fw::Time::SERIALIZED_SIZE + + Fw::LogSeverity::SERIALIZED_SIZE + sizeof(FwSizeStoreType) + + FW_LOG_BUFFER_MAX_SIZE` for events, `sizeof(FwChanIdType) + + Fw::Time::SERIALIZED_SIZE + sizeof(FwSizeStoreType) + FW_TLM_BUFFER_MAX_SIZE` + for telemetry, and the largest serialized argument list or `bufferIn` + payload for serial and buffer traffic. Sizing to `FW_COM_BUFFER_MAX_SIZE` + plus the header alone drops every maximum-size event, telemetry and command + frame. - Give the hub its own pool (or `Svc.BufferManager` bin): when the bin is shared with the driver's receive allocation, receive pressure starves sends. - In the adapter formation `D` and `Q` do not apply; size `N` for the number From 60fc985a8953d3ef99ae14b0229bf2046a3cf4ff Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Thu, 5 Nov 2026 03:10:45 +0000 Subject: [PATCH 11/13] Svc/GenericHub docs: API-break note in change log, send-error and Q-size operator notes, APID wording (local review iter 4) --- Svc/ComQueue/docs/sdd.md | 3 ++- Svc/GenericHub/docs/sdd.md | 17 +++++++++++------ docs/user-manual/design-patterns/hub-pattern.md | 4 +++- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/Svc/ComQueue/docs/sdd.md b/Svc/ComQueue/docs/sdd.md index c9b31b27bbe..00a860efa04 100644 --- a/Svc/ComQueue/docs/sdd.md +++ b/Svc/ComQueue/docs/sdd.md @@ -123,7 +123,8 @@ is added to the queue. `Svc::GenericHub` is a `bufferQueueIn` producer in the recommended hub wiring (see the [GenericHub SDD](../../GenericHub/docs/sdd.md#transport-formations)); its frames start with a `U32` message type rather -than a packet descriptor, so the APID read into the frame context is always 0 and is not consumed downstream. +than a packet descriptor, so the APID read into the frame context is 0 with the default `U16` `FwPacketDescriptorType` and is not +consumed downstream. #### 4.5.2 comPacketQueueIn The `comPacketQueueIn` port handler receives an `Fw::ComBuffer` data type and a port number. diff --git a/Svc/GenericHub/docs/sdd.md b/Svc/GenericHub/docs/sdd.md index af523552d7d..b24baecead8 100644 --- a/Svc/GenericHub/docs/sdd.md +++ b/Svc/GenericHub/docs/sdd.md @@ -26,8 +26,10 @@ For example, logical connections between components in deployments A and B: The driver paired with each hub must be a **buffer driver**: a combination of components that sends and receives `Fw::Buffer` objects across the transport. -The reference implementation uses a `Drv::ByteStreamDriver` together with a -`Drv::ByteStreamBufferAdapter`. Other transport-specific drivers may be used +The reference implementation uses `Drv::Udp` with `Svc::ComQueue` and +`Svc::ComStub` on the send side and a `Drv::ByteStreamBufferAdapter` on the +receive side (see [Transport formations](#transport-formations)). Other +transport-specific drivers may be used provided that they implement the buffer-driver interfaces described below. ## Design @@ -355,8 +357,10 @@ message boundaries, or F´ framing must be added around the link (see In the queued formation, `Svc::ComQueue` reads the first `FwPacketDescriptorType` of every buffer as an APID for the frame context. A -hub message begins with the `U32` message type (values 0–5), so that APID -always reads 0; nothing downstream of the hub consumes it. No framer sits +hub message begins with the `U32` message type (values 0–5), so with the +default `U16` `FwPacketDescriptorType` that APID always reads 0 (with a `U32` +descriptor it reads the message type); nothing downstream of the hub consumes +it. No framer sits between `Svc::ComQueue` and `Svc::ComStub`: the hub's own header is the framing on a boundary-preserving transport. @@ -484,7 +488,8 @@ queue size. Operator signatures: `NumAllocationFailures` rising with `BufferManager` `NoBuffs` rising means the pool is too small or the link is slower than the producers; `buffQueueDepth[0]` reported at `D` on the same ticks means the -link, not the pool, is the bottleneck; `QueueOverflow` means `D < N`. +link, not the pool, is the bottleneck; `QueueOverflow` means `D < N`; +`buffIngressDropped[0]` rising with no event means `Q` is too small. ## Example formations @@ -508,4 +513,4 @@ The following diagrams show progressively more detail: | 2026-07-09 | Documented command routing, current configuration constants, buffer-driver interfaces, framing, and buffer lifecycle | | 2026-09-29 | Documented threading, command index pairing, cross-deployment type agreement, silent receive drops, and allocation failure | | 2026-10-11 | Allocation failure drops the message with an event and counter instead of asserting; added `schedIn` and the hub's own event/telemetry ports | -| 2026-11-05 | Message type is the `HubType` FPP enum; documented the transport formations (queued formation recommended), transport requirements, and sizing rule | +| 2026-11-05 | Message type is the `HubType` FPP enum (`Svc::GenericHub_HubType`, replacing the C++ `Svc::GenericHub::HubType` enum; `HUB_TYPE_MAX` becomes `NUM_CONSTANTS`); documented the transport formations (queued formation recommended), transport requirements, and sizing rule | diff --git a/docs/user-manual/design-patterns/hub-pattern.md b/docs/user-manual/design-patterns/hub-pattern.md index 40c2220106d..f0a8a436609 100644 --- a/docs/user-manual/design-patterns/hub-pattern.md +++ b/docs/user-manual/design-patterns/hub-pattern.md @@ -86,7 +86,9 @@ port. `EXECUTION_ERROR`. - Monitor the link outside the hub. GenericHub reports no transport errors and silently drops invalid received messages; `Svc.ComQueue` reports queue depth - and overflow, `Drv::ByteStreamBufferAdapter` reports driver errors. + and overflow, `Drv::ByteStreamBufferAdapter` reports receive errors. In the + recommended wiring a failed send is not evented (see the SDD's residual-loss + note). ## Recommended wiring From b9b3bff1faf09b11be0abf63d48630e45eb57903 Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Thu, 5 Nov 2026 03:11:29 +0000 Subject: [PATCH 12/13] Svc/GenericHub docs: point the send-interface samples at the recommended formation; state the UDP trade-off in hub-pattern (local review iter 4) --- Svc/GenericHub/GenericHub.fpp | 3 +++ Svc/GenericHub/docs/sdd.md | 4 ++++ docs/user-manual/design-patterns/hub-pattern.md | 9 ++++++--- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Svc/GenericHub/GenericHub.fpp b/Svc/GenericHub/GenericHub.fpp index c87ae21c619..6ad8f1e0e6a 100644 --- a/Svc/GenericHub/GenericHub.fpp +++ b/Svc/GenericHub/GenericHub.fpp @@ -130,6 +130,9 @@ module Svc { # genericHub.toBufferDriver -> bufferDriver.bufferIn # bufferDriver.bufferInReturn -> genericHub.toBufferDriverReturn # genericHub.deallocate -> bufferManager.bufferSendIn + # + # The recommended formation routes toBufferDriver through Svc.ComQueue and + # Svc.ComStub instead of a ByteStreamBufferAdapter; see docs/sdd.md. # ---------------------------------------------------------------------- @ This interface provides ports allocate and deallocate diff --git a/Svc/GenericHub/docs/sdd.md b/Svc/GenericHub/docs/sdd.md index b24baecead8..5e9e2c7be52 100644 --- a/Svc/GenericHub/docs/sdd.md +++ b/Svc/GenericHub/docs/sdd.md @@ -86,6 +86,10 @@ genericHub.cmdRespOut[0] -> commandSplitter.seqCmdStatus[0] ### 2. Hub to buffer driver: send interface +The connections below show the adapter formation; the recommended +formation routes `toBufferDriver` into `Svc.ComQueue.bufferQueueIn[0]` +instead (see [Transport formations](#transport-formations)). + The hub sends serialized messages through the buffer driver using imported interfaces: diff --git a/docs/user-manual/design-patterns/hub-pattern.md b/docs/user-manual/design-patterns/hub-pattern.md index f0a8a436609..7a432ff9dc1 100644 --- a/docs/user-manual/design-patterns/hub-pattern.md +++ b/docs/user-manual/design-patterns/hub-pattern.md @@ -108,9 +108,12 @@ the socket; it holds frames while the driver cannot send and resumes when the driver re-announces readiness. Size the queue depth to at least the hub's buffer count (`D >= N`, instance queue `Q >= N + 8`) so that the hub remains the single drop point. The receive side stays `driver -> adapter -> GenericHub`. -Over `Drv.Udp` a failed send stalls this path until the driver reconnects, so -prefer TCP or UART with F´ framing. Details, the full connection table, and the -sizing rule are in the +Over `Drv.Udp` a failed send stalls this path until the driver reconnects (the +reference deployment accepts that: UDP needs no framing, and the stall is +visible as `NumAllocationFailures` rising while `buffQueueDepth` sits at the +queue depth); TCP and UART recover after a reconnect but need F´ framing plus a +receive-side adapter that the framework does not provide. Details, the full +connection table, and the sizing rule are in the [GenericHub SDD](../../../Svc/GenericHub/docs/sdd.md#transport-formations). ## Putting it together From 82f06a583a555ee8a7d55ae905c475401024efd5 Mon Sep 17 00:00:00 2001 From: "thomas.boyer.chammard" Date: Thu, 5 Nov 2026 03:14:10 +0000 Subject: [PATCH 13/13] Svc/GenericHub UT: factor the self-sink drop steps into a helper (local review iter 4) --- Svc/GenericHub/test/ut/GenericHubTester.cpp | 28 ++++++++++----------- Svc/GenericHub/test/ut/GenericHubTester.hpp | 2 ++ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/Svc/GenericHub/test/ut/GenericHubTester.cpp b/Svc/GenericHub/test/ut/GenericHubTester.cpp index 77bf34e868e..15be1bb20bc 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.cpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.cpp @@ -470,28 +470,33 @@ void GenericHubTester ::test_command_dispatch_allocation_failure() { // Handlers for typed from ports // ---------------------------------------------------------------------- -void GenericHubTester ::test_self_sink() { - this->clearHistory(); +void GenericHubTester ::self_sink_drop(AllocationMode mode, U32 framesOut, U32 expectedFailures) { Fw::ComBuffer comm; random_fill(comm, FW_COM_BUFFER_MAX_SIZE); - - // Pool empty: the one event of the episode re-enters eventIn, is dropped and counted; no cascade - m_allocation_mode = ALLOCATE_EMPTY; + m_allocation_mode = mode; invoke_to_serialIn(0, comm); - ASSERT_from_toBufferDriver_SIZE(0); + ASSERT_from_toBufferDriver_SIZE(framesOut); ASSERT_EQ(m_logger.m_count, 1); invoke_to_schedIn(0, 0); - ASSERT_TLM_NumAllocationFailures(0, 2); + ASSERT_TLM_NumAllocationFailures(0, expectedFailures); m_logger.m_count = 0; this->clearHistory(); +} + +void GenericHubTester ::test_self_sink() { + this->clearHistory(); + // Pool empty: the one event of the episode re-enters eventIn, is dropped and counted; no cascade + self_sink_drop(ALLOCATE_EMPTY, 0, 2); // A successful send ends the episode m_allocation_mode = ALLOCATE_OK; send_random_comm(0); this->clearHistory(); - // Pool recovers on the re-entry: the event frame goes out and re-arms the report + // Pool recovers on the re-entry: the event frame (the hub's only event) goes out and re-arms the report m_allocation_mode = ALLOCATE_EMPTY_ONCE; + Fw::ComBuffer comm; + random_fill(comm, FW_COM_BUFFER_MAX_SIZE); invoke_to_serialIn(0, comm); ASSERT_from_toBufferDriver_SIZE(1); ASSERT_from_eventOut_SIZE(1); @@ -503,12 +508,7 @@ void GenericHubTester ::test_self_sink() { this->clearHistory(); // The next drop starts a new episode: reported (and dropped on re-entry) once more - m_allocation_mode = ALLOCATE_EMPTY; - invoke_to_serialIn(0, comm); - ASSERT_from_toBufferDriver_SIZE(0); - ASSERT_EQ(m_logger.m_count, 1); - invoke_to_schedIn(0, 0); - ASSERT_TLM_NumAllocationFailures(0, 5); + self_sink_drop(ALLOCATE_EMPTY, 0, 5); } void GenericHubTester ::returnTask(void* ptr) { diff --git a/Svc/GenericHub/test/ut/GenericHubTester.hpp b/Svc/GenericHub/test/ut/GenericHubTester.hpp index c0fa4b285f8..85e812c7999 100644 --- a/Svc/GenericHub/test/ut/GenericHubTester.hpp +++ b/Svc/GenericHub/test/ut/GenericHubTester.hpp @@ -195,6 +195,8 @@ class GenericHubTester : public GenericHubGTestBase { ALLOCATE_EMPTY_ONCE, //!< Return an empty buffer, then switch to ALLOCATE_OK ALLOCATE_UNDERSIZED //!< Return a valid buffer one byte short of the request }; + //! One self-sink drop: serialIn with the given allocation mode, then check frames out, logger and counter + void self_sink_drop(AllocationMode mode, U32 framesOut, U32 expectedFailures); //! Task routine returning the deferred buffer on toBufferDriverReturn static void returnTask(void* ptr);