From c3b2e048807dec0529c013bd69f666ba1c5b7bc2 Mon Sep 17 00:00:00 2001 From: Thomas Boyer-Chammard <49786685+thomas-bc@users.noreply.github.com> Date: Thu, 8 May 2025 10:54:54 -0700 Subject: [PATCH] Use data return pattern on Uplink and standardize port names (#3546) * First pass at Svc + TcpClient implementation * Revert FileUplink changes * Add copy (with allocation/deallocation) to FprimeRouter to simplify buffer management * Update FprimeRouter UTs * Update FprimeDeframer UTs * Update FrameAccumulator UTs * Update ComStub UTs * Update missing Drv and UTs * Update ComInterface to use ComDataWithContext on output * Update Ref/RPI topology * Fix spelling * Fix test typo * Update Udp component and UTs * Rename data ports and standardize "Return" naming pattern * Fix variable name * Adapt UTs * Update Communication Adapter Interface docs * Full SDD updates * Spelling & nits and details * Put formatting back to original * Update Deframer interface to include bufferReturn * Address review comments --- Drv/ByteStreamDriverModel/docs/sdd.md | 2 +- Drv/Interfaces/ByteStreamDriverInterface.fppi | 7 +- Drv/LinuxUartDriver/LinuxUartDriver.cpp | 7 +- Drv/LinuxUartDriver/LinuxUartDriver.fpp | 3 + Drv/LinuxUartDriver/LinuxUartDriver.hpp | 8 +- Drv/TcpClient/TcpClient.fpp | 3 + Drv/TcpClient/TcpClientComponentImpl.cpp | 6 +- Drv/TcpClient/TcpClientComponentImpl.hpp | 8 ++ Drv/TcpClient/test/ut/TcpClientTestMain.cpp | 5 ++ Drv/TcpClient/test/ut/TcpClientTester.cpp | 13 ++- Drv/TcpClient/test/ut/TcpClientTester.hpp | 2 + Drv/TcpServer/TcpServer.fpp | 3 + Drv/TcpServer/TcpServerComponentImpl.cpp | 6 +- Drv/TcpServer/TcpServerComponentImpl.hpp | 7 ++ Drv/TcpServer/test/ut/TcpServerTestMain.cpp | 5 ++ Drv/TcpServer/test/ut/TcpServerTester.cpp | 13 ++- Drv/TcpServer/test/ut/TcpServerTester.hpp | 2 + Drv/Udp/Udp.fpp | 2 + Drv/Udp/UdpComponentImpl.cpp | 6 +- Drv/Udp/UdpComponentImpl.hpp | 18 ++-- Drv/Udp/test/ut/UdpTestMain.cpp | 5 ++ Drv/Udp/test/ut/UdpTester.cpp | 13 ++- Drv/Udp/test/ut/UdpTester.hpp | 3 + RPI/Top/topology.fpp | 73 +++++++++------- Ref/Top/topology.fpp | 85 ++++++++++-------- Svc/ComQueue/ComQueue.cpp | 8 +- Svc/ComQueue/ComQueue.fpp | 5 +- Svc/ComQueue/ComQueue.hpp | 4 +- Svc/ComQueue/docs/sdd.md | 13 +-- Svc/ComQueue/test/ut/ComQueueTester.cpp | 22 ++--- Svc/ComStub/ComStub.cpp | 17 ++-- Svc/ComStub/ComStub.fpp | 15 ++-- Svc/ComStub/ComStub.hpp | 19 ++-- Svc/ComStub/docs/sdd.md | 27 +++--- Svc/ComStub/test/ut/ComStubTestMain.cpp | 5 ++ Svc/ComStub/test/ut/ComStubTester.cpp | 86 +++++++++++-------- Svc/ComStub/test/ut/ComStubTester.hpp | 15 ++-- Svc/FprimeDeframer/FprimeDeframer.cpp | 17 ++-- Svc/FprimeDeframer/FprimeDeframer.fpp | 3 - Svc/FprimeDeframer/FprimeDeframer.hpp | 11 ++- Svc/FprimeDeframer/docs/sdd.md | 30 ++----- .../test/ut/FprimeDeframerTestMain.cpp | 5 ++ .../test/ut/FprimeDeframerTester.cpp | 40 +++++---- .../test/ut/FprimeDeframerTester.hpp | 3 + Svc/FprimeFramer/docs/sdd.md | 21 +++-- Svc/FprimeRouter/FprimeRouter.cpp | 40 +++++---- Svc/FprimeRouter/FprimeRouter.fpp | 3 + Svc/FprimeRouter/FprimeRouter.hpp | 8 ++ Svc/FprimeRouter/docs/sdd.md | 38 ++++---- .../test/ut/FprimeRouterTestMain.cpp | 5 ++ .../test/ut/FprimeRouterTester.cpp | 39 +++++++-- .../test/ut/FprimeRouterTester.hpp | 12 +++ Svc/FrameAccumulator/FrameAccumulator.cpp | 17 ++-- Svc/FrameAccumulator/FrameAccumulator.fpp | 3 +- Svc/FrameAccumulator/FrameAccumulator.hpp | 13 ++- Svc/FrameAccumulator/docs/sdd.md | 10 ++- .../test/ut/FrameAccumulatorTestMain.cpp | 4 + .../test/ut/FrameAccumulatorTester.cpp | 70 +++++++++------ .../test/ut/FrameAccumulatorTester.hpp | 3 + Svc/Interfaces/ComInterface.fppi | 11 ++- Svc/Interfaces/DeframerInterface.fppi | 10 ++- Svc/Interfaces/FrameAccumulatorInterface.fppi | 10 ++- Svc/Interfaces/RouterInterface.fppi | 12 +++ Svc/Interfaces/docs/sdd.md | 2 +- .../communication-adapter-interface.md | 72 ++++++++-------- 65 files changed, 685 insertions(+), 368 deletions(-) diff --git a/Drv/ByteStreamDriverModel/docs/sdd.md b/Drv/ByteStreamDriverModel/docs/sdd.md index 304d1a807c..2eb17a9c18 100644 --- a/Drv/ByteStreamDriverModel/docs/sdd.md +++ b/Drv/ByteStreamDriverModel/docs/sdd.md @@ -8,7 +8,7 @@ The outgoing stream is represented by the input `send` port; other components ca ### Send The manager component (for example a radio manager) initiates the transfer of send data by calling the "send" port. -The caller will provide a `Fw::Buffer` containing the data to send. The driver component **must** perform a callback on its `dataReturnOut` port to return the status of that send as well as returning ownership of the `Fw::Buffer` to the caller. +The caller will provide a `Fw::Buffer` containing the data to send. The driver component **must** perform a callback on its `sendReturnOut` port to return the status of that send as well as returning ownership of the `Fw::Buffer` to the caller. These responses are an enumeration whose values are described in the following table: | Value | Description | Buffer Ownership | diff --git a/Drv/Interfaces/ByteStreamDriverInterface.fppi b/Drv/Interfaces/ByteStreamDriverInterface.fppi index e2a22f149b..a1e69d7458 100644 --- a/Drv/Interfaces/ByteStreamDriverInterface.fppi +++ b/Drv/Interfaces/ByteStreamDriverInterface.fppi @@ -7,5 +7,8 @@ @ Invoke this port to send data out the driver guarded input port $send: Fw.BufferSend - @ Port invoked to return ownership of sent data back to the sender - output port dataReturnOut: Drv.ByteStreamData + @ Port returning ownership of data received on $send port + output port sendReturnOut: Drv.ByteStreamData + + @ Port receiving back ownership of data sent out on $recv port + guarded input port recvReturnIn: Fw.BufferSend diff --git a/Drv/LinuxUartDriver/LinuxUartDriver.cpp b/Drv/LinuxUartDriver/LinuxUartDriver.cpp index 8e304e7765..baf809c9a3 100644 --- a/Drv/LinuxUartDriver/LinuxUartDriver.cpp +++ b/Drv/LinuxUartDriver/LinuxUartDriver.cpp @@ -311,7 +311,12 @@ void LinuxUartDriver ::send_handler(const FwIndexType portNum, Fw::Buffer& serBu } } // Return the buffer back to the caller - dataReturnOut_out(0, serBuffer, status); + sendReturnOut_out(0, serBuffer, status); +} + + +void LinuxUartDriver::recvReturnIn_handler(FwIndexType portNum, Fw::Buffer& fwBuffer) { + this->deallocate_out(0, fwBuffer); } void LinuxUartDriver ::serialReadTaskEntry(void* ptr) { diff --git a/Drv/LinuxUartDriver/LinuxUartDriver.fpp b/Drv/LinuxUartDriver/LinuxUartDriver.fpp index 38b8078172..203574bd75 100644 --- a/Drv/LinuxUartDriver/LinuxUartDriver.fpp +++ b/Drv/LinuxUartDriver/LinuxUartDriver.fpp @@ -11,6 +11,9 @@ module Drv { @ Allocation port used for allocating memory in the receive task output port allocate: Fw.BufferGet + @ Deallocation of allocated buffers + output port deallocate: Fw.BufferSend + # ---------------------------------------------------------------------- # Special ports # ---------------------------------------------------------------------- diff --git a/Drv/LinuxUartDriver/LinuxUartDriver.hpp b/Drv/LinuxUartDriver/LinuxUartDriver.hpp index 7e98ba0ff2..498490f219 100644 --- a/Drv/LinuxUartDriver/LinuxUartDriver.hpp +++ b/Drv/LinuxUartDriver/LinuxUartDriver.hpp @@ -94,8 +94,14 @@ class LinuxUartDriver final : public LinuxUartDriverComponentBase { //! Handler implementation for serialSend //! void send_handler(FwIndexType portNum, /*!< The port number*/ - Fw::Buffer& serBuffer); + Fw::Buffer& serBuffer) override; + //! Handler implementation for recvReturnIn + //! + //! Port receiving back ownership of data sent out on $recv port + void recvReturnIn_handler(FwIndexType portNum, //!< The port number + Fw::Buffer& fwBuffer //!< The buffer + ) override; PlatformIntType m_fd; //!< file descriptor returned for I/O device U32 m_allocationSize; //!< size of allocation request to memory manager diff --git a/Drv/TcpClient/TcpClient.fpp b/Drv/TcpClient/TcpClient.fpp index e9ad3152e0..38537ee4c0 100644 --- a/Drv/TcpClient/TcpClient.fpp +++ b/Drv/TcpClient/TcpClient.fpp @@ -6,5 +6,8 @@ module Drv { @ Allocation for received data output port allocate: Fw.BufferGet + @ Deallocation of allocated buffers + output port deallocate: Fw.BufferSend + } } diff --git a/Drv/TcpClient/TcpClientComponentImpl.cpp b/Drv/TcpClient/TcpClientComponentImpl.cpp index 1d74886bb0..e7f0e47231 100644 --- a/Drv/TcpClient/TcpClientComponentImpl.cpp +++ b/Drv/TcpClient/TcpClientComponentImpl.cpp @@ -91,7 +91,11 @@ void TcpClientComponentImpl::send_handler(const FwIndexType portNum, Fw::Buffer& break; } // Return the buffer and status to the caller - this->dataReturnOut_out(0, fwBuffer, returnStatus); + this->sendReturnOut_out(0, fwBuffer, returnStatus); +} + +void TcpClientComponentImpl::recvReturnIn_handler(FwIndexType portNum, Fw::Buffer& fwBuffer) { + this->deallocate_out(0, fwBuffer); } } // end namespace Drv diff --git a/Drv/TcpClient/TcpClientComponentImpl.hpp b/Drv/TcpClient/TcpClientComponentImpl.hpp index becf5968d2..72fcf604bd 100644 --- a/Drv/TcpClient/TcpClientComponentImpl.hpp +++ b/Drv/TcpClient/TcpClientComponentImpl.hpp @@ -126,6 +126,14 @@ class TcpClientComponentImpl final : public TcpClientComponentBase, public Socke */ void send_handler(const FwIndexType portNum, Fw::Buffer& fwBuffer) override; + //! Handler implementation for recvReturnIn + //! + //! Port receiving back ownership of data sent out on $recv port + void recvReturnIn_handler(FwIndexType portNum, //!< The port number + Fw::Buffer& fwBuffer //!< The buffer + ) override; + + Drv::TcpClientSocket m_socket; //!< Socket implementation // Member variable to store the buffer size diff --git a/Drv/TcpClient/test/ut/TcpClientTestMain.cpp b/Drv/TcpClient/test/ut/TcpClientTestMain.cpp index 300b10004b..f8dac3d331 100644 --- a/Drv/TcpClient/test/ut/TcpClientTestMain.cpp +++ b/Drv/TcpClient/test/ut/TcpClientTestMain.cpp @@ -15,6 +15,11 @@ TEST(Nominal, TcpClientBasicReceiveThread) { tester.test_receive_thread(); } +TEST(Nominal, TcpClientBufferDeallocation) { + Drv::TcpClientTester tester; + tester.test_buffer_deallocation(); +} + TEST(Reconnect, TcpClientMultiMessaging) { Drv::TcpClientTester tester; tester.test_multiple_messaging(); diff --git a/Drv/TcpClient/test/ut/TcpClientTester.cpp b/Drv/TcpClient/test/ut/TcpClientTester.cpp index dd9ae9b741..d5c507d277 100644 --- a/Drv/TcpClient/test/ut/TcpClientTester.cpp +++ b/Drv/TcpClient/test/ut/TcpClientTester.cpp @@ -80,8 +80,8 @@ void TcpClientTester ::test_with_loop(U32 iterations, bool recv_thread) { m_data_buffer.setSize(sizeof(m_data_storage)); size = Drv::Test::fill_random_buffer(m_data_buffer); invoke_to_send(0, m_data_buffer); - ASSERT_from_dataReturnOut_SIZE(i + 1); - Drv::ByteStreamStatus status = this->fromPortHistory_dataReturnOut->at(i).status; + ASSERT_from_sendReturnOut_SIZE(i + 1); + Drv::ByteStreamStatus status = this->fromPortHistory_sendReturnOut->at(i).status; EXPECT_EQ(status, ByteStreamStatus::OP_OK); Drv::Test::receive_all(server, server_fd, buffer, size); Drv::Test::validate_random_buffer(m_data_buffer, buffer); @@ -180,6 +180,15 @@ void TcpClientTester ::test_no_automatic_recv_connection() { server.terminate(server_fd); } +void TcpClientTester ::test_buffer_deallocation() { + U8 data[1]; + Fw::Buffer buffer(data, sizeof(data)); + this->invoke_to_recvReturnIn(0, buffer); + ASSERT_from_deallocate_SIZE(1); // incoming buffer should be deallocated + ASSERT_EQ(this->fromPortHistory_deallocate->at(0).fwBuffer.getData(), data); + ASSERT_EQ(this->fromPortHistory_deallocate->at(0).fwBuffer.getSize(), sizeof(data)); +} + // ---------------------------------------------------------------------- // Handler overrides for typed from ports // ---------------------------------------------------------------------- diff --git a/Drv/TcpClient/test/ut/TcpClientTester.hpp b/Drv/TcpClient/test/ut/TcpClientTester.hpp index 392d0c9b4d..09db5e8486 100644 --- a/Drv/TcpClient/test/ut/TcpClientTester.hpp +++ b/Drv/TcpClient/test/ut/TcpClientTester.hpp @@ -67,6 +67,8 @@ namespace Drv { void test_with_loop(U32 iterations, bool recv_thread=false); + void test_buffer_deallocation(); + private: // ---------------------------------------------------------------------- diff --git a/Drv/TcpServer/TcpServer.fpp b/Drv/TcpServer/TcpServer.fpp index 49fdfcc248..ce2df810a7 100644 --- a/Drv/TcpServer/TcpServer.fpp +++ b/Drv/TcpServer/TcpServer.fpp @@ -6,5 +6,8 @@ module Drv { @ Allocation for received data output port allocate: Fw.BufferGet + @ Deallocation of allocated buffers + output port deallocate: Fw.BufferSend + } } diff --git a/Drv/TcpServer/TcpServerComponentImpl.cpp b/Drv/TcpServer/TcpServerComponentImpl.cpp index 3e42151f93..15137a99e0 100644 --- a/Drv/TcpServer/TcpServerComponentImpl.cpp +++ b/Drv/TcpServer/TcpServerComponentImpl.cpp @@ -139,7 +139,11 @@ void TcpServerComponentImpl::send_handler(const FwIndexType portNum, Fw::Buffer& break; } // Return the buffer and status to the caller - this->dataReturnOut_out(0, fwBuffer, returnStatus); + this->sendReturnOut_out(0, fwBuffer, returnStatus); +} + +void TcpServerComponentImpl::recvReturnIn_handler(FwIndexType portNum, Fw::Buffer& fwBuffer) { + this->deallocate_out(0, fwBuffer); } } // end namespace Drv diff --git a/Drv/TcpServer/TcpServerComponentImpl.hpp b/Drv/TcpServer/TcpServerComponentImpl.hpp index b6d500b8b6..1268e7c73c 100644 --- a/Drv/TcpServer/TcpServerComponentImpl.hpp +++ b/Drv/TcpServer/TcpServerComponentImpl.hpp @@ -161,6 +161,13 @@ class TcpServerComponentImpl final : public TcpServerComponentBase, public Socke */ void send_handler(const FwIndexType portNum, Fw::Buffer& fwBuffer) override; + //! Handler implementation for recvReturnIn + //! + //! Port receiving back ownership of data sent out on $recv port + void recvReturnIn_handler(FwIndexType portNum, //!< The port number + Fw::Buffer& fwBuffer //!< The buffer + ) override; + Drv::TcpServerSocket m_socket; //!< Socket implementation FwSizeType m_allocation_size; //!< Member variable to store the buffer size diff --git a/Drv/TcpServer/test/ut/TcpServerTestMain.cpp b/Drv/TcpServer/test/ut/TcpServerTestMain.cpp index e60f75c3d0..c1b739007c 100644 --- a/Drv/TcpServer/test/ut/TcpServerTestMain.cpp +++ b/Drv/TcpServer/test/ut/TcpServerTestMain.cpp @@ -4,6 +4,11 @@ #include "TcpServerTester.hpp" +TEST(Nominal, TcpServerBufferDeallocation) { + Drv::TcpServerTester tester; + tester.test_buffer_deallocation(); +} + TEST(Nominal, TcpServerBasicMessaging) { Drv::TcpServerTester tester; tester.test_basic_messaging(); diff --git a/Drv/TcpServer/test/ut/TcpServerTester.cpp b/Drv/TcpServer/test/ut/TcpServerTester.cpp index 05052d4318..b064bd926b 100644 --- a/Drv/TcpServer/test/ut/TcpServerTester.cpp +++ b/Drv/TcpServer/test/ut/TcpServerTester.cpp @@ -81,8 +81,8 @@ void TcpServerTester ::test_with_loop(U32 iterations, bool recv_thread) { m_data_buffer.setSize(sizeof(m_data_storage)); size = Drv::Test::fill_random_buffer(m_data_buffer); invoke_to_send(0, m_data_buffer); - ASSERT_from_dataReturnOut_SIZE(i + 1); - Drv::ByteStreamStatus status = this->fromPortHistory_dataReturnOut->at(i).status; + ASSERT_from_sendReturnOut_SIZE(i + 1); + Drv::ByteStreamStatus status = this->fromPortHistory_sendReturnOut->at(i).status; EXPECT_EQ(status, ByteStreamStatus::OP_OK) << "On iteration: " << i << " and receive thread: " << recv_thread; Drv::Test::receive_all(client, client_fd, buffer, size); @@ -217,6 +217,15 @@ void TcpServerTester ::test_no_automatic_recv_connection() { this->component.terminate(); } +void TcpServerTester ::test_buffer_deallocation() { + U8 data[1]; + Fw::Buffer buffer(data, sizeof(data)); + this->invoke_to_recvReturnIn(0, buffer); + ASSERT_from_deallocate_SIZE(1); // incoming buffer should be deallocated + ASSERT_EQ(this->fromPortHistory_deallocate->at(0).fwBuffer.getData(), data); + ASSERT_EQ(this->fromPortHistory_deallocate->at(0).fwBuffer.getSize(), sizeof(data)); +} + // ---------------------------------------------------------------------- // Handlers for typed from ports // ---------------------------------------------------------------------- diff --git a/Drv/TcpServer/test/ut/TcpServerTester.hpp b/Drv/TcpServer/test/ut/TcpServerTester.hpp index 3d749da620..f8434ad0fe 100644 --- a/Drv/TcpServer/test/ut/TcpServerTester.hpp +++ b/Drv/TcpServer/test/ut/TcpServerTester.hpp @@ -75,6 +75,8 @@ namespace Drv { void test_no_automatic_send_connection(); void test_no_automatic_recv_connection(); + + void test_buffer_deallocation(); bool wait_on_change(bool open, U32 iterations); bool wait_on_started(bool open, U32 iterations); diff --git a/Drv/Udp/Udp.fpp b/Drv/Udp/Udp.fpp index 7d49930350..08f27cf829 100644 --- a/Drv/Udp/Udp.fpp +++ b/Drv/Udp/Udp.fpp @@ -5,5 +5,7 @@ module Drv { output port allocate: Fw.BufferGet + output port deallocate: Fw.BufferSend + } } diff --git a/Drv/Udp/UdpComponentImpl.cpp b/Drv/Udp/UdpComponentImpl.cpp index 109b29f642..d8c9b3c2a7 100644 --- a/Drv/Udp/UdpComponentImpl.cpp +++ b/Drv/Udp/UdpComponentImpl.cpp @@ -100,7 +100,11 @@ void UdpComponentImpl::send_handler(const FwIndexType portNum, Fw::Buffer& fwBuf break; } // Return the buffer and status to the caller - this->dataReturnOut_out(0, fwBuffer, returnStatus); + this->sendReturnOut_out(0, fwBuffer, returnStatus); +} + +void UdpComponentImpl::recvReturnIn_handler(FwIndexType portNum, Fw::Buffer& fwBuffer) { + this->deallocate_out(0, fwBuffer); } } // end namespace Drv diff --git a/Drv/Udp/UdpComponentImpl.hpp b/Drv/Udp/UdpComponentImpl.hpp index 38ced2d473..77e9d3f083 100644 --- a/Drv/Udp/UdpComponentImpl.hpp +++ b/Drv/Udp/UdpComponentImpl.hpp @@ -99,7 +99,7 @@ PROTECTED: * * \return IpSocket reference */ - IpSocket& getSocketHandler(); + IpSocket& getSocketHandler() override; /** * \brief returns a buffer to fill with data @@ -109,7 +109,7 @@ PROTECTED: * * \return Fw::Buffer to fill with data */ - Fw::Buffer getBuffer(); + Fw::Buffer getBuffer() override; /** * \brief sends a buffer to be filled with data @@ -119,12 +119,12 @@ PROTECTED: * * \return Fw::Buffer filled with data to send out */ - void sendBuffer(Fw::Buffer buffer, SocketIpStatus status); + void sendBuffer(Fw::Buffer buffer, SocketIpStatus status) override; /** * \brief called when the IPv4 system has been connected */ - void connected(); + void connected() override; PRIVATE: @@ -146,7 +146,15 @@ PROTECTED: * \param portNum: fprime port number of the incoming port call * \param fwBuffer: buffer containing data to be sent */ - void send_handler(const FwIndexType portNum, Fw::Buffer& fwBuffer); + void send_handler(const FwIndexType portNum, Fw::Buffer& fwBuffer) override; + + + //! Handler implementation for recvReturnIn + //! + //! Port receiving back ownership of data sent out on $recv port + void recvReturnIn_handler(FwIndexType portNum, //!< The port number + Fw::Buffer& fwBuffer //!< The buffer + ) override; Drv::UdpSocket m_socket; //!< Socket implementation diff --git a/Drv/Udp/test/ut/UdpTestMain.cpp b/Drv/Udp/test/ut/UdpTestMain.cpp index 606c7ea908..ca44c2858e 100644 --- a/Drv/Udp/test/ut/UdpTestMain.cpp +++ b/Drv/Udp/test/ut/UdpTestMain.cpp @@ -24,6 +24,11 @@ TEST(Reconnect, UdpReceiveThreadReconnect) { tester.test_advanced_reconnect(); } +TEST(Reconnect, UdpBufferDeallocation) { + Drv::UdpTester tester; + tester.test_buffer_deallocation(); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/Drv/Udp/test/ut/UdpTester.cpp b/Drv/Udp/test/ut/UdpTester.cpp index 460b27e59d..9be52e9cb4 100644 --- a/Drv/Udp/test/ut/UdpTester.cpp +++ b/Drv/Udp/test/ut/UdpTester.cpp @@ -92,8 +92,8 @@ void UdpTester::test_with_loop(U32 iterations, bool recv_thread) { m_data_buffer.setSize(sizeof(m_data_storage)); size = Drv::Test::fill_random_buffer(m_data_buffer); invoke_to_send(0, m_data_buffer); - ASSERT_from_dataReturnOut_SIZE(i + 1); - Drv::ByteStreamStatus status = this->fromPortHistory_dataReturnOut->at(i).status; + ASSERT_from_sendReturnOut_SIZE(i + 1); + Drv::ByteStreamStatus status = this->fromPortHistory_sendReturnOut->at(i).status; EXPECT_EQ(status, ByteStreamStatus::OP_OK); Drv::Test::receive_all(udp2, udp2_fd, buffer, size); Drv::Test::validate_random_buffer(m_data_buffer, buffer); @@ -158,6 +158,15 @@ void UdpTester ::test_advanced_reconnect() { test_with_loop(10, true); // Up to 10 * RECONNECT_MS } +void UdpTester ::test_buffer_deallocation() { + U8 data[1]; + Fw::Buffer buffer(data, sizeof(data)); + this->invoke_to_recvReturnIn(0, buffer); + ASSERT_from_deallocate_SIZE(1); // incoming buffer should be deallocated + ASSERT_EQ(this->fromPortHistory_deallocate->at(0).fwBuffer.getData(), data); + ASSERT_EQ(this->fromPortHistory_deallocate->at(0).fwBuffer.getSize(), sizeof(data)); +} + // ---------------------------------------------------------------------- // Handlers for typed from ports // ---------------------------------------------------------------------- diff --git a/Drv/Udp/test/ut/UdpTester.hpp b/Drv/Udp/test/ut/UdpTester.hpp index 85ea3ec124..1405decef7 100644 --- a/Drv/Udp/test/ut/UdpTester.hpp +++ b/Drv/Udp/test/ut/UdpTester.hpp @@ -69,6 +69,9 @@ namespace Drv { //! void test_advanced_reconnect(); + //! Test buffer deallocation + void test_buffer_deallocation(); + // Helpers void test_with_loop(U32 iterations, bool recv_thread=false); diff --git a/RPI/Top/topology.fpp b/RPI/Top/topology.fpp index 750e3b20e1..a8abd2cc98 100644 --- a/RPI/Top/topology.fpp +++ b/RPI/Top/topology.fpp @@ -63,26 +63,26 @@ module RPI { # ---------------------------------------------------------------------- connections Downlink { - eventLogger.PktSend -> comQueue.comPacketQueueIn[0] - chanTlm.PktSend -> comQueue.comPacketQueueIn[1] - fileDownlink.bufferSendOut -> comQueue.bufferQueueIn[0] - - comQueue.queueSend -> framer.dataIn + eventLogger.PktSend -> comQueue.comPacketQueueIn[0] + chanTlm.PktSend -> comQueue.comPacketQueueIn[1] + fileDownlink.bufferSendOut -> comQueue.bufferQueueIn[0] comQueue.bufferReturnOut[0] -> fileDownlink.bufferReturn - framer.dataReturnOut -> comQueue.bufferReturnIn - framer.bufferAllocate -> commsBufferManager.bufferGetCallee + comQueue.dataOut -> framer.dataIn + framer.dataReturnOut -> comQueue.dataReturnIn + + framer.bufferAllocate -> commsBufferManager.bufferGetCallee framer.bufferDeallocate -> commsBufferManager.bufferSendIn - framer.dataOut -> comStub.comDataIn - comStub.dataReturnOut -> framer.dataReturnIn - comDriver.dataReturnOut -> comStub.dataReturnIn + framer.dataOut -> comStub.dataIn + comStub.dataReturnOut -> framer.dataReturnIn - comDriver.ready -> comStub.drvConnected - comStub.drvDataOut -> comDriver.$send + comStub.drvSendOut -> comDriver.$send + comDriver.sendReturnOut -> comStub.drvSendReturnIn + comDriver.ready -> comStub.drvConnected comStub.comStatusOut -> framer.comStatusIn - framer.comStatusOut -> comQueue.comStatusIn + framer.comStatusOut -> comQueue.comStatusIn } connections FaultProtection { @@ -125,34 +125,41 @@ module RPI { rpiDemo.SpiReadWrite -> spiDrv.SpiReadWrite } - connections MemoryAllocations { - comDriver.allocate -> commsBufferManager.bufferGetCallee - fileUplink.bufferSendOut -> commsBufferManager.bufferSendIn - frameAccumulator.bufferAllocate -> commsBufferManager.bufferGetCallee - frameAccumulator.bufferDeallocate -> commsBufferManager.bufferSendIn - fprimeRouter.bufferDeallocate -> commsBufferManager.bufferSendIn - deframer.bufferDeallocate -> commsBufferManager.bufferSendIn - } - connections UART { rpiDemo.UartBuffers -> uartBufferManager.bufferSendIn rpiDemo.UartWrite -> uartDrv.$send uartDrv.$recv -> rpiDemo.UartRead uartDrv.allocate -> uartBufferManager.bufferGetCallee - uartDrv.dataReturnOut -> rpiDemo.UartWriteReturn + uartDrv.sendReturnOut -> rpiDemo.UartWriteReturn } connections Uplink { - comDriver.$recv -> comStub.drvDataIn - comStub.comDataOut -> frameAccumulator.dataIn - - frameAccumulator.frameOut -> deframer.framedIn - deframer.deframedOut -> fprimeRouter.dataIn - - fprimeRouter.commandOut -> cmdDisp.seqCmdBuff - fprimeRouter.fileOut -> fileUplink.bufferSendIn - - cmdDisp.seqCmdStatus -> fprimeRouter.cmdResponseIn + # ComDriver buffer allocations + comDriver.allocate -> commsBufferManager.bufferGetCallee + comDriver.deallocate -> commsBufferManager.bufferSendIn + # ComDriver <-> ComStub + comDriver.$recv -> comStub.drvReceiveIn + comStub.drvReceiveReturnOut -> comDriver.recvReturnIn + # ComStub <-> FrameAccumulator + comStub.dataOut -> frameAccumulator.dataIn + frameAccumulator.dataReturnOut -> comStub.dataReturnIn + # FrameAccumulator buffer allocations + frameAccumulator.bufferDeallocate -> commsBufferManager.bufferSendIn + frameAccumulator.bufferAllocate -> commsBufferManager.bufferGetCallee + # FrameAccumulator <-> Deframer + frameAccumulator.dataOut -> deframer.dataIn + deframer.dataReturnOut -> frameAccumulator.dataReturnIn + # Deframer <-> Router + deframer.dataOut -> fprimeRouter.dataIn + fprimeRouter.dataReturnOut -> deframer.dataReturnIn + # Router buffer allocations + fprimeRouter.bufferAllocate -> commsBufferManager.bufferGetCallee + fprimeRouter.bufferDeallocate -> commsBufferManager.bufferSendIn + # Router <-> CmdDispatcher/FileUplink + fprimeRouter.commandOut -> cmdDisp.seqCmdBuff + cmdDisp.seqCmdStatus -> fprimeRouter.cmdResponseIn + fprimeRouter.fileOut -> fileUplink.bufferSendIn + fileUplink.bufferSendOut -> fprimeRouter.fileBufferReturnIn } } diff --git a/Ref/Top/topology.fpp b/Ref/Top/topology.fpp index 36eff8549c..3461626c17 100644 --- a/Ref/Top/topology.fpp +++ b/Ref/Top/topology.fpp @@ -15,6 +15,10 @@ module Ref { TELEMETRY } + enum Ports_ComBufferQueue { + FILE_DOWNLINK + } + topology Ref { # ---------------------------------------------------------------------- @@ -92,28 +96,30 @@ module Ref { # ---------------------------------------------------------------------- connections Downlink { - dpCat.fileOut -> fileDownlink.SendFile + # Data Products + dpCat.fileOut -> fileDownlink.SendFile fileDownlink.FileComplete -> dpCat.fileDone - - eventLogger.PktSend -> comQueue.comPacketQueueIn[Ports_ComPacketQueue.EVENTS] - tlmSend.PktSend -> comQueue.comPacketQueueIn[Ports_ComPacketQueue.TELEMETRY] - fileDownlink.bufferSendOut -> comQueue.bufferQueueIn[0] - comQueue.bufferReturnOut[0] -> fileDownlink.bufferReturn - - comQueue.queueSend -> fprimeFramer.dataIn - fprimeFramer.dataReturnOut -> comQueue.bufferReturnIn - fprimeFramer.comStatusOut -> comQueue.comStatusIn - - fprimeFramer.bufferAllocate -> commsBufferManager.bufferGetCallee + # Inputs to ComQueue (events, telemetry, file) + eventLogger.PktSend -> comQueue.comPacketQueueIn[Ports_ComPacketQueue.EVENTS] + tlmSend.PktSend -> comQueue.comPacketQueueIn[Ports_ComPacketQueue.TELEMETRY] + fileDownlink.bufferSendOut -> comQueue.bufferQueueIn[Ports_ComBufferQueue.FILE_DOWNLINK] + comQueue.bufferReturnOut[Ports_ComBufferQueue.FILE_DOWNLINK] -> fileDownlink.bufferReturn + # ComQueue <-> Framer + comQueue.dataOut -> fprimeFramer.dataIn + fprimeFramer.dataReturnOut -> comQueue.dataReturnIn + # Buffer Management for Framer + fprimeFramer.bufferAllocate -> commsBufferManager.bufferGetCallee fprimeFramer.bufferDeallocate -> commsBufferManager.bufferSendIn - - fprimeFramer.dataOut -> comStub.comDataIn + # Framer <-> ComStub + fprimeFramer.dataOut -> comStub.dataIn comStub.dataReturnOut -> fprimeFramer.dataReturnIn - comStub.comStatusOut -> fprimeFramer.comStatusIn - - comStub.drvDataOut -> comDriver.$send - comDriver.dataReturnOut -> comStub.dataReturnIn - comDriver.ready -> comStub.drvConnected + # ComStub <-> ComDriver + comStub.drvSendOut -> comDriver.$send + comDriver.sendReturnOut -> comStub.drvSendReturnIn + comDriver.ready -> comStub.drvConnected + # ComStatus + comStub.comStatusOut -> fprimeFramer.comStatusIn + fprimeFramer.comStatusOut -> comQueue.comStatusIn } connections FaultProtection { @@ -163,25 +169,32 @@ module Ref { } connections Uplink { - - comDriver.allocate -> commsBufferManager.bufferGetCallee - comDriver.$recv -> comStub.drvDataIn - comStub.comDataOut -> frameAccumulator.dataIn - - frameAccumulator.frameOut -> deframer.framedIn - frameAccumulator.bufferAllocate -> commsBufferManager.bufferGetCallee + # ComDriver buffer allocations + comDriver.allocate -> commsBufferManager.bufferGetCallee + comDriver.deallocate -> commsBufferManager.bufferSendIn + # ComDriver <-> ComStub + comDriver.$recv -> comStub.drvReceiveIn + comStub.drvReceiveReturnOut -> comDriver.recvReturnIn + # ComStub <-> FrameAccumulator + comStub.dataOut -> frameAccumulator.dataIn + frameAccumulator.dataReturnOut -> comStub.dataReturnIn + # FrameAccumulator buffer allocations frameAccumulator.bufferDeallocate -> commsBufferManager.bufferSendIn - deframer.bufferDeallocate -> commsBufferManager.bufferSendIn - deframer.deframedOut -> fprimeRouter.dataIn - - fprimeRouter.commandOut -> cmdDisp.seqCmdBuff - fprimeRouter.fileOut -> fileUplink.bufferSendIn + frameAccumulator.bufferAllocate -> commsBufferManager.bufferGetCallee + # FrameAccumulator <-> Deframer + frameAccumulator.dataOut -> deframer.dataIn + deframer.dataReturnOut -> frameAccumulator.dataReturnIn + # Deframer <-> Router + deframer.dataOut -> fprimeRouter.dataIn + fprimeRouter.dataReturnOut -> deframer.dataReturnIn + # Router buffer allocations + fprimeRouter.bufferAllocate -> commsBufferManager.bufferGetCallee fprimeRouter.bufferDeallocate -> commsBufferManager.bufferSendIn - - cmdDisp.seqCmdStatus -> fprimeRouter.cmdResponseIn - - fileUplink.bufferSendOut -> commsBufferManager.bufferSendIn - + # Router <-> CmdDispatcher/FileUplink + fprimeRouter.commandOut -> cmdDisp.seqCmdBuff + cmdDisp.seqCmdStatus -> fprimeRouter.cmdResponseIn + fprimeRouter.fileOut -> fileUplink.bufferSendIn + fileUplink.bufferSendOut -> fprimeRouter.fileBufferReturnIn } connections DataProducts { diff --git a/Svc/ComQueue/ComQueue.cpp b/Svc/ComQueue/ComQueue.cpp index 7a779c9687..8526abf64b 100644 --- a/Svc/ComQueue/ComQueue.cpp +++ b/Svc/ComQueue/ComQueue.cpp @@ -193,7 +193,7 @@ void ComQueue::run_handler(const FwIndexType portNum, U32 context) { this->tlmWrite_buffQueueDepth(buffQueueDepth); } -void ComQueue ::bufferReturnIn_handler(FwIndexType portNum, +void ComQueue ::dataReturnIn_handler(FwIndexType portNum, Fw::Buffer& data, const ComCfg::FrameContext& context) { static_assert(std::numeric_limits::is_signed, "FwIndexType must be signed"); @@ -205,7 +205,7 @@ void ComQueue ::bufferReturnIn_handler(FwIndexType portNum, // Failing this assert means that context.apid was modified since ComQueue set it, which should not happen FW_ASSERT(bufferReturnPortNum < BUFFER_PORT_COUNT, static_cast(bufferReturnPortNum)); if (bufferReturnPortNum >= 0) { - // It is a coding error not to connect the associated bufferReturnOut port for each bufferReturnIn port + // It is a coding error not to connect the associated bufferReturnOut port for each dataReturnIn port FW_ASSERT(this->isConnected_bufferReturnOut_OutputPort(bufferReturnPortNum), static_cast(bufferReturnPortNum)); // If this is a buffer port, return the buffer to the BufferDownlink this->bufferReturnOut_out(bufferReturnPortNum, data); @@ -263,7 +263,7 @@ void ComQueue::sendComBuffer(Fw::ComBuffer& comBuffer, FwIndexType queueIndex) { // Context APID is set to the queue index for now. A future implementation may want this to be configurable ComCfg::FrameContext context; context.setcomQueueIndex(queueIndex); - this->queueSend_out(0, outBuffer, context); + this->dataOut_out(0, outBuffer, context); // Set state to WAITING for the status to come back this->m_state = WAITING; } @@ -275,7 +275,7 @@ void ComQueue::sendBuffer(Fw::Buffer& buffer, FwIndexType queueIndex) { // Context APID is set to the queue index for now. A future implementation may want this to be configurable ComCfg::FrameContext context; context.setcomQueueIndex(queueIndex); - this->queueSend_out(0, buffer, context); + this->dataOut_out(0, buffer, context); // Set state to WAITING for the status to come back this->m_state = WAITING; diff --git a/Svc/ComQueue/ComQueue.fpp b/Svc/ComQueue/ComQueue.fpp index ad85de3f70..37b18403b9 100644 --- a/Svc/ComQueue/ComQueue.fpp +++ b/Svc/ComQueue/ComQueue.fpp @@ -17,7 +17,7 @@ module Svc { # ---------------------------------------------------------------------- @ Port for emitting data ready to be sent - output port queueSend: Svc.ComDataWithContext + output port dataOut: Svc.ComDataWithContext @ Port for receiving the status signal async input port comStatusIn: Fw.SuccessCondition @@ -31,9 +31,8 @@ module Svc { @ Port array for returning ownership of Fw::Buffer to its original sender output port bufferReturnOut: [ComQueueBufferPorts] Fw.BufferSend - # It is appropriate for this port to be sync since it is just a passthrough @ Port for receiving Fw::Buffer whose ownership needs to be handed back - sync input port bufferReturnIn: Svc.ComDataWithContext + sync input port dataReturnIn: Svc.ComDataWithContext @ Port for scheduling telemetry output async input port run: Svc.Sched drop diff --git a/Svc/ComQueue/ComQueue.hpp b/Svc/ComQueue/ComQueue.hpp index 92d35ffa4e..86ae925178 100644 --- a/Svc/ComQueue/ComQueue.hpp +++ b/Svc/ComQueue/ComQueue.hpp @@ -148,10 +148,10 @@ class ComQueue final : public ComQueueComponentBase { U32 context /*! [!NOTE] > ComQueue also has the port instances for autocoded functionality for events, telemetry and time. diff --git a/Svc/ComQueue/test/ut/ComQueueTester.cpp b/Svc/ComQueue/test/ut/ComQueueTester.cpp index 540973879e..bbc67a2080 100644 --- a/Svc/ComQueue/test/ut/ComQueueTester.cpp +++ b/Svc/ComQueue/test/ut/ComQueueTester.cpp @@ -68,7 +68,7 @@ void ComQueueTester ::emitOneAndCheck(FwIndexType expectedIndex, FwSizeType expectedSize) { emitOne(); // Check that the data buffers are identical (size + data) - Fw::Buffer emittedBuffer = this->fromPortHistory_queueSend->at(expectedIndex).data; + Fw::Buffer emittedBuffer = this->fromPortHistory_dataOut->at(expectedIndex).data; ASSERT_EQ(expectedSize, emittedBuffer.getSize()); for (FwSizeType i = 0; i < expectedSize; i++) { ASSERT_EQ(emittedBuffer.getData()[i], expectedData[i]); @@ -158,17 +158,17 @@ void ComQueueTester ::testPrioritySend() { } // Check that nothing has yet been sent - ASSERT_from_queueSend_SIZE(0); + ASSERT_from_dataOut_SIZE(0); for (FwIndexType index = 0; index < ComQueue::TOTAL_PORT_COUNT; index++) { U8 orderKey; - U32 previousSize = fromPortHistory_queueSend->size(); + U32 previousSize = fromPortHistory_dataOut->size(); emitOne(); - ASSERT_EQ(fromPortHistory_queueSend->size(), (index + 1)); + ASSERT_EQ(fromPortHistory_dataOut->size(), (index + 1)); // Check that the size changed by exactly one - ASSERT_EQ(fromPortHistory_queueSend->size(), (previousSize + 1)); + ASSERT_EQ(fromPortHistory_dataOut->size(), (previousSize + 1)); - orderKey = fromPortHistory_queueSend->at(index).data.getData()[0]; + orderKey = fromPortHistory_dataOut->at(index).data.getData()[0]; ASSERT_EQ(orderKey, index); } clearFromPortHistory(); @@ -295,7 +295,7 @@ void ComQueueTester ::testReadyFirst() { invoke_to_comPacketQueueIn(portNum, comBuffer, 0); dispatchAll(); - Fw::Buffer emittedBuffer = this->fromPortHistory_queueSend->at(portNum).data; + Fw::Buffer emittedBuffer = this->fromPortHistory_dataOut->at(portNum).data; ASSERT_EQ(emittedBuffer.getSize(), comBuffer.getBuffLength()); for (FwSizeType i = 0; i < emittedBuffer.getSize(); i++) { ASSERT_EQ(emittedBuffer.getData()[i], comBuffer.getBuffAddr()[i]); @@ -307,7 +307,7 @@ void ComQueueTester ::testReadyFirst() { emitOne(); invoke_to_bufferQueueIn(portNum, buffer); dispatchAll(); - Fw::Buffer emittedBuffer = this->fromPortHistory_queueSend->at(portNum).data; + Fw::Buffer emittedBuffer = this->fromPortHistory_dataOut->at(portNum).data; ASSERT_EQ(emittedBuffer.getSize(), buffer.getSize()); for (FwSizeType i = 0; i < buffer.getSize(); i++) { ASSERT_EQ(buffer.getData()[i], emittedBuffer.getData()[i]); @@ -328,7 +328,7 @@ void ComQueueTester ::testContextData() { emitOne(); // Currently, the APID is set to the queue index, which is the same as the port number for COM ports FwIndexType expectedApid = portNum; - auto emittedContext = this->fromPortHistory_queueSend->at(portNum).context; + auto emittedContext = this->fromPortHistory_dataOut->at(portNum).context; ASSERT_EQ(expectedApid, emittedContext.getcomQueueIndex()); } clearFromPortHistory(); @@ -338,7 +338,7 @@ void ComQueueTester ::testContextData() { emitOne(); // APID is queue index, which is COM_PORT_COUNT + portNum for BUFFER ports FwIndexType expectedApid = portNum + ComQueue::COM_PORT_COUNT; - auto emittedContext = this->fromPortHistory_queueSend->at(portNum).context; + auto emittedContext = this->fromPortHistory_dataOut->at(portNum).context; ASSERT_EQ(expectedApid, emittedContext.getcomQueueIndex()); } clearFromPortHistory(); @@ -354,7 +354,7 @@ void ComQueueTester ::testBufferQueueReturn() { for(FwIndexType portNum = 0; portNum < ComQueue::TOTAL_PORT_COUNT; portNum++){ clearFromPortHistory(); context.setcomQueueIndex(portNum); - invoke_to_bufferReturnIn(0, buffer, context); + invoke_to_dataReturnIn(0, buffer, context); // APIDs that correspond to an buffer originating from a Fw.Com port // do no get deallocated – APIDs that correspond to a Fw.Buffer do if (portNum < ComQueue::COM_PORT_COUNT) { diff --git a/Svc/ComStub/ComStub.cpp b/Svc/ComStub/ComStub.cpp index d05b919008..464a355ab3 100644 --- a/Svc/ComStub/ComStub.cpp +++ b/Svc/ComStub/ComStub.cpp @@ -23,10 +23,10 @@ ComStub::~ComStub() {} // Handler implementations for user-defined typed input ports // ---------------------------------------------------------------------- -void ComStub::comDataIn_handler(const FwIndexType portNum, Fw::Buffer& sendBuffer, const ComCfg::FrameContext& context) { +void ComStub::dataIn_handler(const FwIndexType portNum, Fw::Buffer& sendBuffer, const ComCfg::FrameContext& context) { FW_ASSERT(!this->m_reinitialize || !this->isConnected_comStatusOut_OutputPort(0)); // A message should never get here if we need to reinitialize is needed this->m_storedContext = context; // Store the context of the current message - this->drvDataOut_out(0, sendBuffer); + this->drvSendOut_out(0, sendBuffer); } void ComStub::drvConnected_handler(const FwIndexType portNum) { @@ -37,15 +37,16 @@ void ComStub::drvConnected_handler(const FwIndexType portNum) { } } -void ComStub::drvDataIn_handler(const FwIndexType portNum, +void ComStub::drvReceiveIn_handler(const FwIndexType portNum, Fw::Buffer& recvBuffer, const Drv::ByteStreamStatus& recvStatus) { if (recvStatus.e == Drv::ByteStreamStatus::OP_OK) { - this->comDataOut_out(0, recvBuffer); + ComCfg::FrameContext emptyContext; // ComStub knows nothing about the received bytes, so use an empty context + this->dataOut_out(0, recvBuffer, emptyContext); } } -void ComStub ::dataReturnIn_handler(FwIndexType portNum, //!< The port number +void ComStub ::drvSendReturnIn_handler(FwIndexType portNum, //!< The port number Fw::Buffer& fwBuffer, //!< The buffer const Drv::ByteStreamStatus& sendStatus) { if (sendStatus != Drv::ByteStreamStatus::SEND_RETRY) { @@ -60,7 +61,7 @@ void ComStub ::dataReturnIn_handler(FwIndexType portNum, //!< The port number if (this->m_retry_count < this->RETRY_LIMIT) { // If we have not yet retried more than the retry limit, attempt to retry this->m_retry_count++; - this->drvDataOut_out(0, fwBuffer); + this->drvSendOut_out(0, fwBuffer); } else { // If retried too many times, return buffer and log failure this->dataReturnOut_out(0, fwBuffer, this->m_storedContext); @@ -70,4 +71,8 @@ void ComStub ::dataReturnIn_handler(FwIndexType portNum, //!< The port number } } +void ComStub ::dataReturnIn_handler(FwIndexType portNum, Fw::Buffer& fwBuffer, const ComCfg::FrameContext& context) { + this->drvReceiveReturnOut_out(0, fwBuffer); +} + } // end namespace Svc diff --git a/Svc/ComStub/ComStub.fpp b/Svc/ComStub/ComStub.fpp index 57d5eab852..a317f62b86 100644 --- a/Svc/ComStub/ComStub.fpp +++ b/Svc/ComStub/ComStub.fpp @@ -10,14 +10,17 @@ module Svc { @ Ready signal when driver is connected sync input port drvConnected: Drv.ByteStreamReady - @ Receive (read) data from driver. This gets forwarded to comDataOut - sync input port drvDataIn: Drv.ByteStreamData + @ Receive (read) data from driver. This gets forwarded to dataOut + sync input port drvReceiveIn: Drv.ByteStreamData - @ Send (write) data to the driver. This gets invoked on comDataIn invocation - output port drvDataOut: Fw.BufferSend + @ Send (write) data to the driver. This gets invoked on dataIn invocation + output port drvSendOut: Fw.BufferSend - @ Callback from drvDataOut (retrieving status and ownership of sent buffer) - sync input port dataReturnIn: Drv.ByteStreamData + @ Callback from drvSendOut (retrieving status and ownership of sent buffer) + sync input port drvSendReturnIn: Drv.ByteStreamData + + @ Returning ownership of buffer that came in on drvReceiveIn + output port drvReceiveReturnOut: Fw.BufferSend } } diff --git a/Svc/ComStub/ComStub.hpp b/Svc/ComStub/ComStub.hpp index 7db4ba0c5d..8afeff8612 100644 --- a/Svc/ComStub/ComStub.hpp +++ b/Svc/ComStub/ComStub.hpp @@ -35,11 +35,11 @@ class ComStub final : public ComStubComponentBase { // Handler implementations for user-defined typed input ports // ---------------------------------------------------------------------- - //! Handler implementation for comDataIn + //! Handler implementation for dataIn //! //! Comms data is coming in meaning there is a request for ComStub to send data on the wire //! For ComStub, this means we send the data to the underlying driver (e.g. TCP/UDP/UART) - void comDataIn_handler(const FwIndexType portNum, /*!< The port number*/ + void dataIn_handler(const FwIndexType portNum, /*!< The port number*/ Fw::Buffer& sendBuffer, const ComCfg::FrameContext& context) override; @@ -47,18 +47,25 @@ class ComStub final : public ComStubComponentBase { //! void drvConnected_handler(const FwIndexType portNum) override; - //! Handler implementation for drvDataIn + //! Handler implementation for drvReceiveIn //! //! Data is coming in from the driver (meaning it has been read from the wire). - //! ComStub forwards this to the comDataOut port - void drvDataIn_handler(const FwIndexType portNum, + //! ComStub forwards this to the dataOut port + void drvReceiveIn_handler(const FwIndexType portNum, /*!< The port number*/ Fw::Buffer& recvBuffer, const Drv::ByteStreamStatus& recvStatus) override; //! Handler implementation for dataReturnIn //! + //! Port receiving back ownership of buffer sent out on dataOut + void dataReturnIn_handler(FwIndexType portNum, //!< The port number + Fw::Buffer& fwBuffer, //!< The buffer + const ComCfg::FrameContext& context) override; + + //! Handler implementation for drvSendReturnIn + //! //! Buffer ownership and status returning from a Driver "send" operation - void dataReturnIn_handler(FwIndexType portNum, //!< The port number + void drvSendReturnIn_handler(FwIndexType portNum, //!< The port number Fw::Buffer& fwBuffer, //!< The buffer const Drv::ByteStreamStatus& recvStatus) override; diff --git a/Svc/ComStub/docs/sdd.md b/Svc/ComStub/docs/sdd.md index fe1c157ef5..ebd2e8ea97 100644 --- a/Svc/ComStub/docs/sdd.md +++ b/Svc/ComStub/docs/sdd.md @@ -55,32 +55,37 @@ be useful | Kind | Name | Port Type | Usage | |--------------|----------------|-----------------------|-----------------------------------------------------------------------------------| -| `sync input` | `comDataIn` | `Svc.ComDataWithContext` | Port receiving `Fw::Buffer`s for transmission out `drvDataOut` | +| `sync input` | `dataIn` | `Svc.ComDataWithContext` | Port receiving `Fw::Buffer`s for transmission out `drvSendOut` | | `output` | `comStatusOut` | `Svc.ComStatus` | Port indicating success or failure to attached `Svc::ComQueue` | -| `output` | `comDataOut` | `Drv.ByteStreamRecv` | Port providing received `Fw::Buffers` to a potential `Svc::Deframer` | +| `output` | `dataOut` | `Svc.ComDataWithContext` | Port providing received `Fw::Buffers` to the broader application (typically a Deframer) | +| `output` | `dataReturnOut` | `Svc.ComDataWithContext` | Port returning ownership of data that came in on `dataIn` | +| `sync input` | `dataReturnIn` | `Svc.ComDataWithContext` | Port receiving back ownership of buffer sent out on `dataOut` | **Byte Stream Driver Model Ports** | Kind | Name | Port Type | Usage | |--------------|----------------|-----------------------|-----------------------------------------------------------------------------------| | `sync input` | `drvConnected` | `Drv.ByteStreamReady` | Port called when the underlying driver has connected | -| `sync input` | `drvDataIn` | `Drv.ByteStreamRecv` | Port receiving `Fw::Buffers` from underlying communications bus driver | -| `output` | `drvDataOut` | `Drv.ByteStreamSend` | Port providing received `Fw::Buffers` to the underlying communications bus driver | +| `sync input` | `drvReceiveIn` | `Drv.ByteStreamRecv` | Port receiving `Fw::Buffers` from underlying communications bus driver | +| `output` | `drvSendOut` | `Drv.ByteStreamSend` | Port providing received `Fw::Buffers` to the underlying communications bus driver | +| `sync input` | `drvSendReturnIn` | `Drv.ByteStreamData` | Port receiving status and ownership of buffer sent out on `drvSendOut` | +| `output` | `drvReceiveReturnOut` | `Fw.BufferSend` | Port returning ownership of buffer that came in on `drvReceiveIn` | ### 4.2. State, Configuration, and Runtime Setup -`Svc::ComStub` has only stores a boolean `m_reinitialize` indicating when it should send `Fw::Success::SUCCESS` in +`Svc::ComStub` stores a boolean `m_reinitialize` indicating when it should send `Fw::Success::SUCCESS` in response to a driver reconnection event. This is to implement the Communication Adapter Protocol of a -[communication adapter interface](../../../docs/reference/communication-adapter-interface.md). +[communication adapter interface](../../../docs/reference/communication-adapter-interface.md). It also keeps +track of a `m_retry_count` to limit the number of retries on an attempt to send data. ### 4.3. Port Handlers -#### 4.3.1 comDataIn +#### 4.3.1 dataIn -The `comDataIn` port handler receives an `Fw::Buffer` from the F´ system for transmission to the ground. Typically, it +The `dataIn` port handler receives an `Fw::Buffer` from the F´ system for transmission to the ground. Typically, it is connected to the output of the `Svc::Framer` component. In this `Svc::ComStub` implementation, it passes this -`Fw::Buffer` directly to the `drvDataOut` port. It will retry when that port responds with a `RETRY` request. Otherwise, +`Fw::Buffer` directly to the `drvSendOut` port. It will retry when that port responds with a `RETRY` request. Otherwise, the `comStatusOut` port will be invoked to indicate success or failure. Retries attempts are limited before the port asserts. @@ -89,7 +94,7 @@ asserts. This port receives the connected signal from the driver and responds with exactly one `READY` invocation to the `comStatusOut` port. This starts downlink. This occurs each time the driver reconnects. -#### 4.3.1 drvDataIn +#### 4.3.1 drvReceiveIn -The `drvDataIn` handler receives data read from the driver and supplies it out the `comDataOut` port. It is usually +The `drvReceiveIn` handler receives data read from the driver and supplies it out the `dataOut` port. It is usually connected to the `Svc::Deframer` component diff --git a/Svc/ComStub/test/ut/ComStubTestMain.cpp b/Svc/ComStub/test/ut/ComStubTestMain.cpp index 80531e4a56..bdd7611e2d 100644 --- a/Svc/ComStub/test/ut/ComStubTestMain.cpp +++ b/Svc/ComStub/test/ut/ComStubTestMain.cpp @@ -20,6 +20,11 @@ TEST(Nominal, Fail) { tester.test_fail(); } +TEST(Nominal, BufferReturn) { + Svc::ComStubTester tester; + tester.test_buffer_return(); +} + TEST(OffNominal, Retry) { Svc::ComStubTester tester; tester.test_retry(); diff --git a/Svc/ComStub/test/ut/ComStubTester.cpp b/Svc/ComStub/test/ut/ComStubTester.cpp index 6dbdade24e..337a281e75 100644 --- a/Svc/ComStub/test/ut/ComStubTester.cpp +++ b/Svc/ComStub/test/ut/ComStubTester.cpp @@ -55,14 +55,15 @@ void ComStubTester ::test_basic() { this->fill(buffer); // Downlink - invoke_to_comDataIn(0, buffer, context); - ASSERT_from_drvDataOut_SIZE(1); - ASSERT_from_drvDataOut(0, buffer); + invoke_to_dataIn(0, buffer, context); + ASSERT_from_drvSendOut_SIZE(1); + ASSERT_from_drvSendOut(0, buffer); // Uplink - invoke_to_drvDataIn(0, buffer, Drv::ByteStreamStatus::OP_OK); - ASSERT_from_comDataOut_SIZE(1); - ASSERT_from_comDataOut(0, buffer); + ComCfg::FrameContext emptyContext; + invoke_to_drvReceiveIn(0, buffer, Drv::ByteStreamStatus::OP_OK); + ASSERT_from_dataOut_SIZE(1); + ASSERT_from_dataOut(0, buffer, emptyContext); } void ComStubTester ::test_fail() { @@ -75,13 +76,13 @@ void ComStubTester ::test_fail() { ComCfg::FrameContext context; // Downlink - invoke_to_comDataIn(0, buffer, context); - ASSERT_from_drvDataOut_SIZE(1); - ASSERT_from_drvDataOut(0, buffer); + invoke_to_dataIn(0, buffer, context); + ASSERT_from_drvSendOut_SIZE(1); + ASSERT_from_drvSendOut(0, buffer); // Uplink - invoke_to_drvDataIn(0, buffer, Drv::ByteStreamStatus::OTHER_ERROR); - ASSERT_from_comDataOut_SIZE(0); // receiving failure should not send anything + invoke_to_drvReceiveIn(0, buffer, Drv::ByteStreamStatus::OTHER_ERROR); + ASSERT_from_dataOut_SIZE(0); // receiving failure should not send anything } void ComStubTester ::test_retry() { @@ -99,16 +100,16 @@ void ComStubTester ::test_retry() { } // Retrying for as many times as the RETRY_LIMIT should be ok for (FwIndexType i = 0; i < this->component.RETRY_LIMIT; i++) { - invoke_to_dataReturnIn(0, buffers[i], Drv::ByteStreamStatus::SEND_RETRY); - // Test we have indeed retried (data sent on drvDataOut) - ASSERT_from_drvDataOut_SIZE(static_cast(i + 1)); - ASSERT_from_drvDataOut(static_cast(i), buffers[i]); + invoke_to_drvSendReturnIn(0, buffers[i], Drv::ByteStreamStatus::SEND_RETRY); + // Test we have indeed retried (data sent on drvSendOut) + ASSERT_from_drvSendOut_SIZE(static_cast(i + 1)); + ASSERT_from_drvSendOut(static_cast(i), buffers[i]); } - ASSERT_from_drvDataOut_SIZE(static_cast(this->component.RETRY_LIMIT)); + ASSERT_from_drvSendOut_SIZE(static_cast(this->component.RETRY_LIMIT)); ASSERT_EQ(this->component.m_retry_count, this->component.RETRY_LIMIT); // Retry one more time should block from retrying and reset retry count - invoke_to_dataReturnIn(0, buffers[MAX_ITERS - 1], Drv::ByteStreamStatus::SEND_RETRY); - ASSERT_from_drvDataOut_SIZE(static_cast(this->component.RETRY_LIMIT)); // no drvDataOut sent when SEND_RETRY + invoke_to_drvSendReturnIn(0, buffers[MAX_ITERS - 1], Drv::ByteStreamStatus::SEND_RETRY); + ASSERT_from_drvSendOut_SIZE(static_cast(this->component.RETRY_LIMIT)); // no drvSendOut sent when SEND_RETRY ASSERT_from_dataReturnOut_SIZE(1); // buffer ownership was returned ASSERT_EQ(this->component.m_retry_count, 0); } @@ -116,7 +117,7 @@ void ComStubTester ::test_retry() { void ComStubTester ::test_retry_reset() { this->test_initial(); FwIndexType MAX_ITERS = static_cast(this->component.RETRY_LIMIT + 1); - U32 expected_drvDataOut_count = 0; + U32 expected_drvSendOut_count = 0; // Make small individual buffers for testing U8 storage[MAX_ITERS][8]; @@ -130,40 +131,51 @@ void ComStubTester ::test_retry_reset() { // Retrying for as many times as the RETRY_LIMIT should be ok for (FwIndexType i = 0; i < this->component.RETRY_LIMIT; i++) { - invoke_to_dataReturnIn(0, buffers[i], Drv::ByteStreamStatus::SEND_RETRY); - ASSERT_from_drvDataOut(expected_drvDataOut_count, buffers[i]); - expected_drvDataOut_count++; // trick: increment now to use as index prior and size after - ASSERT_from_drvDataOut_SIZE(expected_drvDataOut_count); + invoke_to_drvSendReturnIn(0, buffers[i], Drv::ByteStreamStatus::SEND_RETRY); + ASSERT_from_drvSendOut(expected_drvSendOut_count, buffers[i]); + expected_drvSendOut_count++; // trick: increment now to use as index prior and size after + ASSERT_from_drvSendOut_SIZE(expected_drvSendOut_count); } - // Now, we receive a OP_OK, which should not retry (drvDataOut should not be called) and reset the retry count - ASSERT_from_drvDataOut_SIZE(expected_drvDataOut_count); // no drvDataOut sent when OP_OK - invoke_to_dataReturnIn(0, buffers[0], Drv::ByteStreamStatus::OP_OK); - ASSERT_from_drvDataOut_SIZE(expected_drvDataOut_count); // no drvDataOut sent when OP_OK + // Now, we receive a OP_OK, which should not retry (drvSendOut should not be called) and reset the retry count + ASSERT_from_drvSendOut_SIZE(expected_drvSendOut_count); // no drvSendOut sent when OP_OK + invoke_to_drvSendReturnIn(0, buffers[0], Drv::ByteStreamStatus::OP_OK); + ASSERT_from_drvSendOut_SIZE(expected_drvSendOut_count); // no drvSendOut sent when OP_OK // Now that retry count is reset, we can retry again without a problem for (FwIndexType i = 0; i < this->component.RETRY_LIMIT; i++) { - invoke_to_dataReturnIn(0, buffers[i], Drv::ByteStreamStatus::SEND_RETRY); - ASSERT_from_drvDataOut(expected_drvDataOut_count, buffers[i]); - expected_drvDataOut_count++; // trick: increment now to use as index prior and size after - ASSERT_from_drvDataOut_SIZE(expected_drvDataOut_count); + invoke_to_drvSendReturnIn(0, buffers[i], Drv::ByteStreamStatus::SEND_RETRY); + ASSERT_from_drvSendOut(expected_drvSendOut_count, buffers[i]); + expected_drvSendOut_count++; // trick: increment now to use as index prior and size after + ASSERT_from_drvSendOut_SIZE(expected_drvSendOut_count); } - ASSERT_from_drvDataOut_SIZE(expected_drvDataOut_count); // no drvDataOut sent when OP_OK + ASSERT_from_drvSendOut_SIZE(expected_drvSendOut_count); // no drvSendOut sent when OP_OK +} + +void ComStubTester ::test_buffer_return() { + U8 data[1]; + Fw::Buffer buffer(data, sizeof(data)); + ComCfg::FrameContext context; + this->invoke_to_dataReturnIn(0, buffer, context); + ASSERT_from_drvReceiveReturnOut_SIZE(1); // incoming buffer should be returned + ASSERT_EQ(this->fromPortHistory_drvReceiveReturnOut->at(0).fwBuffer.getData(), data); + ASSERT_EQ(this->fromPortHistory_drvReceiveReturnOut->at(0).fwBuffer.getSize(), sizeof(data)); } // ---------------------------------------------------------------------- // Handlers for typed from ports // ---------------------------------------------------------------------- -void ComStubTester ::from_comDataOut_handler(const FwIndexType portNum, - Fw::Buffer& recvBuffer) { - this->pushFromPortEntry_comDataOut(recvBuffer); +void ComStubTester ::from_dataOut_handler(const FwIndexType portNum, + Fw::Buffer& recvBuffer, + const ComCfg::FrameContext& context) { + this->pushFromPortEntry_dataOut(recvBuffer, context); } void ComStubTester ::from_comStatusOut_handler(const FwIndexType portNum, Fw::Success& condition) { this->pushFromPortEntry_comStatusOut(condition); } -void ComStubTester ::from_drvDataOut_handler(const FwIndexType portNum, Fw::Buffer& sendBuffer) { - this->pushFromPortEntry_drvDataOut(sendBuffer); +void ComStubTester ::from_drvSendOut_handler(const FwIndexType portNum, Fw::Buffer& sendBuffer) { + this->pushFromPortEntry_drvSendOut(sendBuffer); } diff --git a/Svc/ComStub/test/ut/ComStubTester.hpp b/Svc/ComStub/test/ut/ComStubTester.hpp index a4b00d0226..90b1af35f0 100644 --- a/Svc/ComStub/test/ut/ComStubTester.hpp +++ b/Svc/ComStub/test/ut/ComStubTester.hpp @@ -60,15 +60,20 @@ class ComStubTester : public ComStubGTestBase { //! void test_retry_reset(); + //! Tests buffer is returned + //! + void test_buffer_return(); private: // ---------------------------------------------------------------------- // Handlers for typed from ports // ---------------------------------------------------------------------- - //! Handler for from_comDataOut + //! Handler for from_dataOut //! - void from_comDataOut_handler(const FwIndexType portNum, //!< The port number - Fw::Buffer& recvBuffer); + void from_dataOut_handler(const FwIndexType portNum, //!< The port number + Fw::Buffer& recvBuffer, + const ComCfg::FrameContext& context //!< The context + ); //! Handler for from_comStatusOut //! @@ -76,9 +81,9 @@ class ComStubTester : public ComStubGTestBase { Fw::Success& condition //!< Status of communication state ); - //! Handler for from_drvDataOut + //! Handler for from_drvSendOut //! - void from_drvDataOut_handler(const FwIndexType portNum, //!< The port number + void from_drvSendOut_handler(const FwIndexType portNum, //!< The port number Fw::Buffer& sendBuffer); private: diff --git a/Svc/FprimeDeframer/FprimeDeframer.cpp b/Svc/FprimeDeframer/FprimeDeframer.cpp index 0622e9c0d3..9f816fdf23 100644 --- a/Svc/FprimeDeframer/FprimeDeframer.cpp +++ b/Svc/FprimeDeframer/FprimeDeframer.cpp @@ -25,11 +25,11 @@ FprimeDeframer ::~FprimeDeframer() {} // Handler implementations for user-defined typed input ports // ---------------------------------------------------------------------- -void FprimeDeframer ::framedIn_handler(FwIndexType portNum, Fw::Buffer& data, const ComCfg::FrameContext& context) { +void FprimeDeframer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, const ComCfg::FrameContext& context) { if (data.getSize() < FprimeProtocol::FrameHeader::SERIALIZED_SIZE + FprimeProtocol::FrameTrailer::SERIALIZED_SIZE) { // Incoming buffer is not long enough to contain a valid frame (header+trailer) this->log_WARNING_HI_InvalidBufferReceived(); - this->bufferDeallocate_out(0, data); // drop the frame + this->dataReturnOut_out(0, data, context); // drop the frame return; } @@ -46,7 +46,7 @@ void FprimeDeframer ::framedIn_handler(FwIndexType portNum, Fw::Buffer& data, co const FprimeProtocol::FrameHeader defaultValue; if (header.getstartWord() != defaultValue.getstartWord()) { this->log_WARNING_HI_InvalidStartWord(); - this->bufferDeallocate_out(0, data); + this->dataReturnOut_out(0, data, context); // drop the frame return; } // We expect the frame size to be size of header + body (of size specified in header) + trailer @@ -54,7 +54,7 @@ void FprimeDeframer ::framedIn_handler(FwIndexType portNum, Fw::Buffer& data, co FprimeProtocol::FrameTrailer::SERIALIZED_SIZE; if (data.getSize() < expectedFrameSize) { this->log_WARNING_HI_InvalidLengthReceived(); - this->bufferDeallocate_out(0, data); + this->dataReturnOut_out(0, data, context); // drop the frame return; } @@ -77,7 +77,7 @@ void FprimeDeframer ::framedIn_handler(FwIndexType portNum, Fw::Buffer& data, co // Check that the CRC in the trailer of the frame matches the computed CRC if (trailer.getcrcField() != computedCrc.asBigEndianU32()) { this->log_WARNING_HI_InvalidChecksum(); - this->bufferDeallocate_out(0, data); + this->dataReturnOut_out(0, data, context); // drop the frame return; } @@ -88,7 +88,12 @@ void FprimeDeframer ::framedIn_handler(FwIndexType portNum, Fw::Buffer& data, co data.setSize(data.getSize() - FprimeProtocol::FrameHeader::SERIALIZED_SIZE - FprimeProtocol::FrameTrailer::SERIALIZED_SIZE); // Emit the deframed data - this->deframedOut_out(0, data, context); + this->dataOut_out(0, data, context); } +void FprimeDeframer ::dataReturnIn_handler(FwIndexType portNum, Fw::Buffer& fwBuffer, const ComCfg::FrameContext& context) { + this->dataReturnOut_out(0, fwBuffer, context); +} + + } // namespace Svc diff --git a/Svc/FprimeDeframer/FprimeDeframer.fpp b/Svc/FprimeDeframer/FprimeDeframer.fpp index ad8d1b9f00..55232fbec8 100644 --- a/Svc/FprimeDeframer/FprimeDeframer.fpp +++ b/Svc/FprimeDeframer/FprimeDeframer.fpp @@ -10,9 +10,6 @@ module Svc { include "../Interfaces/DeframerInterface.fppi" - @ Port for deallocating dropped frames - output port bufferDeallocate: Fw.BufferSend - @ An invalid frame was received (too short to be a frame) event InvalidBufferReceived \ severity warning high \ diff --git a/Svc/FprimeDeframer/FprimeDeframer.hpp b/Svc/FprimeDeframer/FprimeDeframer.hpp index f6313777c7..6d74d7a356 100644 --- a/Svc/FprimeDeframer/FprimeDeframer.hpp +++ b/Svc/FprimeDeframer/FprimeDeframer.hpp @@ -36,9 +36,18 @@ class FprimeDeframer final : public FprimeDeframerComponentBase { //! //! Port to receive framed data. The handler will strip the header and trailer from the frame //! and pass the deframed data to the deframed output port. - void framedIn_handler(FwIndexType portNum, //!< The port number + void dataIn_handler(FwIndexType portNum, //!< The port number Fw::Buffer& data, const ComCfg::FrameContext& context) override; + + //! Handler implementation for dataReturnIn + //! + //! Port receiving back ownership of sent frame buffers + void dataReturnIn_handler(FwIndexType portNum, //!< The port number + Fw::Buffer& data, //!< The buffer + const ComCfg::FrameContext& context) override; + + }; } // namespace Svc diff --git a/Svc/FprimeDeframer/docs/sdd.md b/Svc/FprimeDeframer/docs/sdd.md index b8ed3ec52c..9afee4513f 100644 --- a/Svc/FprimeDeframer/docs/sdd.md +++ b/Svc/FprimeDeframer/docs/sdd.md @@ -2,15 +2,13 @@ The `Svc::FprimeDeframer` component receives F´ frames on its input port, takes off the header and trailer (sometimes referred to as "footer"), and passes the encapsulated payload to a downstream component (usually the [`Svc.FprimeRouter`](../../FprimeRouter/docs/sdd.md)). -Following the [F Prime Protocol frame specification](../../FprimeProtocol/docs/sdd.md), the `Svc::FprimeDeframer` validates the passed in `Fw.Buffer` to ensure it represents a valid frame (see [Frame validation](#frame-validation)), extract the payload from the frame, and outputs the payload on the `deframedOut` output port. +Following the [F Prime Protocol frame specification](../../FprimeProtocol/docs/sdd.md), the `Svc::FprimeDeframer` validates the passed in `Fw.Buffer` to ensure it represents a valid frame (see [Frame validation](#frame-validation)), extract the payload from the frame, and outputs the payload on the `dataOut` output port. ## Internals -The `Svc::FprimeDeframer` component is an implementation of the [DeframerInterface](../../Interfaces/DeframerInterface.fppi) for the F´ communications protocol. It receives an F´ frame (in a [Fw::Buffer](../../../Fw/Buffer/docs/sdd.md) object) on its `framedIn` input port, modifies the input buffer to remove the header and trailer, and sends it out through its `deframedOut` output port. +The `Svc::FprimeDeframer` component is an implementation of the [DeframerInterface](../../Interfaces/DeframerInterface.fppi) for the F´ communications protocol. It receives an F´ frame (in a [Fw::Buffer](../../../Fw/Buffer/docs/sdd.md) object) on its `dataIn` input port, modifies the input buffer to remove the header and trailer, and sends it out through its `dataOut` output port. -Ownership of the buffer is transferred to the component connected to the `deframedOut` output port. The input buffer is modified by subtracting the header and trailer size from the buffer's length, and offsetting the buffer's data pointer to point to the start of the packet data. - -The `Svc::FprimeDeframer` component does not perform any validation of the frame. It is expected that the frame is valid and well-formed. The validation should be performed by an upstream component, such as [`Svc::FrameAccumulator`](../../FrameAccumulator/docs/sdd.md). +Ownership of the buffer is transferred to the component connected to the `dataOut` output port. The input buffer is modified by subtracting the header and trailer size from the buffer's length, and offsetting the buffer's data pointer to point to the start of the packet data. The `Svc::FprimeDeframer` does not support deframing multiple packets in a single frame (i.e. concatenated packets) as this is not supported by the F´ communications protocol. @@ -28,35 +26,25 @@ If any of these conditions are not met, the frame is dropped meaning no payload The `Svc::FprimeDeframer` component is used in the uplink stack of many reference F´ application such as [the tutorials source code](https://github.com/fprime-community#tutorials). - ## Diagrams The below diagram shows a typical configuration in which the `Svc::FprimeDeframer` can be used. This is the configuration used in the [the tutorials source code](https://github.com/fprime-community#tutorials). It is receiving accumulated frames from a [Svc::FrameAccumulator](../../FrameAccumulator/docs/sdd.md) and passes packets to a [Svc::FprimeRouter](../../FprimeRouter/docs/sdd.md) for routing to other components. ![./img/deframer_uplink_stack.png](./img/deframer_uplink_stack.png) - -## Class Diagram - -```mermaid -classDiagram - class FprimeDeframer~PassiveComponent~ { - + void framedIn_handler(FwIndexType portNum, Fw::Buffer& data, const ComCfg::FrameContext& context) - } -``` - - ## Requirements Requirement | Description | Rationale | Verification Method ----------- | ----------- | ----------| ------------------- SVC-DEFRAMER-001 | `Svc::FprimeDeframer` shall extract the payload field from input buffers that represent a valid F Prime frame as specified by the [F Prime Protocol](../../FprimeProtocol/docs/sdd.md) | Deframe valid frames and extract payload | Unit test | -SVC-DEFRAMER-002 | `Svc::FprimeDeframer` shall deallocate input buffers that are not a valid F Prime frame as specified by the [F Prime Protocol](../../FprimeProtocol/docs/sdd.md) | Drop invalid frames | Unit test | +SVC-DEFRAMER-002 | `Svc::FprimeDeframer` shall return ownership of input buffers that are not a valid F Prime frame as specified by the [F Prime Protocol](../../FprimeProtocol/docs/sdd.md) | Drop invalid frames | Unit test | ## Port Descriptions | Kind | Name | Type | Description | |---|---|---|---| -| `guarded input` | framedIn | `Svc.ComDataWithContext` | Receives a frame with optional context data | -| `output` | deframedOut | `Svc.ComDataWithContext` | Receives a frame with optional context data | -| `output` | bufferDeallocate | `Fw.BufferSend` | Port for deallocating dropped frames | +| `guarded input` | `dataIn` | `Svc.ComDataWithContext` | Receives a frame for deframing | +| `output` | `dataOut` | `Svc.ComDataWithContext` | Emits deframed data (F´ packets) | +| `sync input` | `dataReturnIn` | `Svc.ComDataWithContext` | Receives ownership of the emitted data back | +| `output` | `dataReturnOut` | `Svc.ComDataWithContext` | Returns ownership of the input buffer back to the sender | + diff --git a/Svc/FprimeDeframer/test/ut/FprimeDeframerTestMain.cpp b/Svc/FprimeDeframer/test/ut/FprimeDeframerTestMain.cpp index 43d85aa261..ccf09fce2c 100644 --- a/Svc/FprimeDeframer/test/ut/FprimeDeframerTestMain.cpp +++ b/Svc/FprimeDeframer/test/ut/FprimeDeframerTestMain.cpp @@ -37,6 +37,11 @@ TEST(FprimeDeframer, testIncorrectCrc) { tester.testIncorrectCrc(); } +TEST(FprimeDeframer, testDataReturn) { + Svc::FprimeDeframerTester tester; + tester.testDataReturn(); +} + int main(int argc, char** argv) { STest::Random::seed(); ::testing::InitGoogleTest(&argc, argv); diff --git a/Svc/FprimeDeframer/test/ut/FprimeDeframerTester.cpp b/Svc/FprimeDeframer/test/ut/FprimeDeframerTester.cpp index c56b6a4b45..a745bbb78c 100644 --- a/Svc/FprimeDeframer/test/ut/FprimeDeframerTester.cpp +++ b/Svc/FprimeDeframer/test/ut/FprimeDeframerTester.cpp @@ -34,10 +34,10 @@ void FprimeDeframerTester ::testNominalFrame() { this->injectChecksum(data, sizeof(data)); this->mockReceiveData(data, sizeof(data)); - ASSERT_from_deframedOut_SIZE(1); // something emitted on deframedOut - ASSERT_from_bufferDeallocate_SIZE(0); // nothing emitted on bufferDeallocate - // Assert that the data that was emitted on deframedOut is equal to Data field above (randomByte) - ASSERT_EQ(this->fromPortHistory_deframedOut->at(0).data.getData()[0], randomByte); + ASSERT_from_dataOut_SIZE(1); // something emitted on dataOut + ASSERT_from_dataReturnOut_SIZE(0); // nothing emitted on dataReturnOut + // Assert that the data that was emitted on dataOut is equal to Data field above (randomByte) + ASSERT_EQ(this->fromPortHistory_dataOut->at(0).data.getData()[0], randomByte); ASSERT_EVENTS_SIZE(0); // no events emitted } @@ -48,8 +48,8 @@ void FprimeDeframerTester ::testIncorrectLengthToken() { this->injectChecksum(data, sizeof(data)); this->mockReceiveData(data, sizeof(data)); - ASSERT_from_deframedOut_SIZE(0); // nothing emitted on deframedOut - ASSERT_from_bufferDeallocate_SIZE(1); // invalid buffer was deallocated + ASSERT_from_dataOut_SIZE(0); // nothing emitted on dataOut + ASSERT_from_dataReturnOut_SIZE(1); // invalid buffer was deallocated // Check which event was emitted ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted ASSERT_EVENTS_InvalidLengthReceived_SIZE(1); // event was emitted for invalid length @@ -62,8 +62,8 @@ void FprimeDeframerTester ::testIncorrectStartWord() { this->injectChecksum(data, sizeof(data)); this->mockReceiveData(data, sizeof(data)); - ASSERT_from_deframedOut_SIZE(0); // nothing emitted on deframedOut - ASSERT_from_bufferDeallocate_SIZE(1); // invalid buffer was deallocated + ASSERT_from_dataOut_SIZE(0); // nothing emitted on dataOut + ASSERT_from_dataReturnOut_SIZE(1); // invalid buffer was deallocated // Check which event was emitted ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted ASSERT_EVENTS_InvalidStartWord_SIZE(1); // event was emitted for invalid start word @@ -73,8 +73,8 @@ void FprimeDeframerTester ::testIncorrectCrc() { // Frame: | F´ start word | Length = 1 | Data | INCORRECT Checksum | U8 data[13] = {0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00}; this->mockReceiveData(data, sizeof(data)); - ASSERT_from_deframedOut_SIZE(0); // nothing emitted on deframedOut - ASSERT_from_bufferDeallocate_SIZE(1); // invalid buffer was deallocated + ASSERT_from_dataOut_SIZE(0); // nothing emitted on dataOut + ASSERT_from_dataReturnOut_SIZE(1); // invalid buffer was deallocated // Check which event was emitted ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted ASSERT_EVENTS_InvalidChecksum_SIZE(1); // event was emitted for invalid checksum @@ -84,8 +84,8 @@ void FprimeDeframerTester::testTruncatedFrame() { // Send a truncated frame, too short to be valid U8 data[11] = {0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; this->mockReceiveData(data, sizeof(data)); - ASSERT_from_deframedOut_SIZE(0); // nothing emitted on deframedOut - ASSERT_from_bufferDeallocate_SIZE(1); // invalid buffer was deallocated + ASSERT_from_dataOut_SIZE(0); // nothing emitted on dataOut + ASSERT_from_dataReturnOut_SIZE(1); // invalid buffer was deallocated // Check which event was emitted ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted ASSERT_EVENTS_InvalidBufferReceived_SIZE(1); // event was emitted for invalid buffer @@ -94,13 +94,23 @@ void FprimeDeframerTester::testTruncatedFrame() { void FprimeDeframerTester::testZeroSizeFrame() { // Send an empty frame, too short to be valid this->mockReceiveData(nullptr, 0); - ASSERT_from_deframedOut_SIZE(0); // nothing emitted on deframedOut - ASSERT_from_bufferDeallocate_SIZE(1); // invalid buffer was deallocated + ASSERT_from_dataOut_SIZE(0); // nothing emitted on dataOut + ASSERT_from_dataReturnOut_SIZE(1); // invalid buffer was deallocated // Check which event was emitted ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted ASSERT_EVENTS_InvalidBufferReceived_SIZE(1); // event was emitted for invalid buffer } +void FprimeDeframerTester::testDataReturn() { + U8 data[1]; + Fw::Buffer buffer(data, sizeof(data)); + ComCfg::FrameContext nullContext; + this->invoke_to_dataReturnIn(0, buffer, nullContext); + ASSERT_from_dataReturnOut_SIZE(1); // incoming buffer should be deallocated + ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getData(), data); + ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getSize(), sizeof(data)); +} + // ---------------------------------------------------------------------- // Test Helpers // ---------------------------------------------------------------------- @@ -124,7 +134,7 @@ void FprimeDeframerTester::injectChecksum(U8* data, FwSizeType size) { void FprimeDeframerTester::mockReceiveData(U8* data, FwSizeType size) { ComCfg::FrameContext nullContext; Fw::Buffer buffer(data, static_cast(size)); - this->invoke_to_framedIn(0, buffer, nullContext); + this->invoke_to_dataIn(0, buffer, nullContext); } } // namespace Svc diff --git a/Svc/FprimeDeframer/test/ut/FprimeDeframerTester.hpp b/Svc/FprimeDeframer/test/ut/FprimeDeframerTester.hpp index 08bac6a7b9..6818bbdf2a 100644 --- a/Svc/FprimeDeframer/test/ut/FprimeDeframerTester.hpp +++ b/Svc/FprimeDeframer/test/ut/FprimeDeframerTester.hpp @@ -58,6 +58,9 @@ class FprimeDeframerTester : public FprimeDeframerGTestBase { //! Test receiving a frame with an incorrect Crc field void testIncorrectCrc(); + //! Test bufferReturn passthrough + void testDataReturn(); + private: // ---------------------------------------------------------------------- // Helper functions diff --git a/Svc/FprimeFramer/docs/sdd.md b/Svc/FprimeFramer/docs/sdd.md index a059be4523..a91dc94027 100644 --- a/Svc/FprimeFramer/docs/sdd.md +++ b/Svc/FprimeFramer/docs/sdd.md @@ -4,11 +4,11 @@ The `Svc::FprimeFramer` is an implementation of the [FramerInterface](../../Inte It receives data (an F´ packet) on input and produces an [F´ frame](../../FprimeProtocol/docs/sdd.md) on its output port as a result. Please refer to the [F Prime frame specification](../../FprimeProtocol/docs/sdd.md) for details on the frame format. -### Diagrams +It is designed to receive packets from a [`Svc::ComQueue`](../../ComQueue/docs/sdd.md) and passes frames to a [Communications Adapter](../../Interfaces/docs/sdd.md), such as a Radio manager component or [`Svc::ComStub`](../../ComStub/docs/sdd.md), for transmission on the wire. -Below is the common configuration in which the `Svc::FprimeFramer` can be used. It is receiving packets from a [`Svc::ComQueue`](../../ComQueue/docs/sdd.md) and passes frames to a [Communications Adapter](../../Interfaces/docs/sdd.md), such as a Radio manager component (or a [`Svc::ComStub`](../../ComStub/docs/sdd.md)), for transmission. +## Usage Examples -![./img/framer-topology.png](./img/framer-topology.png) +The `Svc::FprimeFramer` component is used in the uplink stack of many reference F´ application such as [the tutorials source code](https://github.com/fprime-community#tutorials). ## Internals @@ -25,12 +25,14 @@ On receiving a data packet, the `Svc::FprimeFramer` performs the following actio ## Port Descriptions -| Kind | Name | Port Type | Usage | -|---|---|---|---| -| `guarded input` | `dataIn` | `Svc.ComDataWithContext` | Port to receive data to frame, in a Fw::Buffer with optional context| -| `output` | `dataOut` | `Svc.ComDataWithContext` | Port to output framed data, with optional context, for follow-up framing| -| `sync input` | `comStatusIn` | `Fw.SuccessCondition` | Port receiving the general status from the downstream component| -| `output` | `comStatusOut` | `Fw.SuccessCondition` | Port receiving indicating the status of framer for receiving more data| +| Kind | Name | Port Type | Usage | +|-----------------|-----------------|--------------------------|--------------------------------------------------------------------------| +| `guarded input` | `dataIn` | `Svc.ComDataWithContext` | Port to receive data to frame, in a Fw::Buffer with optional context | +| `output` | `dataOut` | `Svc.ComDataWithContext` | Port to output framed data, with optional context, for follow-up framing | +| `sync input` | `dataReturnIn` | `Svc.ComDataWithContext` | Port to receive back ownership of buffer sent out of `dataOut` | +| `output` | `dataReturnOut` | `Svc.ComDataWithContext` | Port to return ownership of buffer received on `dataIn` | +| `sync input` | `comStatusIn` | `Fw.SuccessCondition` | Port receiving the general status from the downstream component | +| `output` | `comStatusOut` | `Fw.SuccessCondition` | Port receiving indicating the status of framer for receiving more data | ## Requirements @@ -39,4 +41,5 @@ On receiving a data packet, the `Svc::FprimeFramer` performs the following actio | SVC-FPRIME_FRAMER-001 | `Svc::FprimeFramer` shall accept data buffers (packets) stored in `Fw::Buffer` through its `dataIn` input port | Unit Test | | SVC-FPRIME_FRAMER-002 | `Svc::FprimeFramer` shall emit one F Prime frame on its `framedOut` output port for each packet received on `dataIn` input port | Unit Test | | SVC-FPRIME_FRAMER-003 | `Svc::FprimeFramer` shall emit F Prime frames that conforms to the [F´ frame specification](../../FprimeProtocol/docs/sdd.md) | Unit Test | +| SVC-FPRIME_FRAMER-004 | `Svc::FprimeFramer` shall pass through all `Fw.SuccessCondition` received on `comStatusIn` to `comStatusOut` | Unit Test | diff --git a/Svc/FprimeRouter/FprimeRouter.cpp b/Svc/FprimeRouter/FprimeRouter.cpp index 660c1b8e3c..45e1ef6087 100644 --- a/Svc/FprimeRouter/FprimeRouter.cpp +++ b/Svc/FprimeRouter/FprimeRouter.cpp @@ -32,9 +32,6 @@ void FprimeRouter ::dataIn_handler(FwIndexType portNum, Fw::Buffer& packetBuffer status = esb.deserialize(packetType); } - // Whether to deallocate the packet buffer - bool deallocate = true; - // Process the packet if (status == Fw::FW_SERIALIZE_OK) { U8* const packetData = packetBuffer.getData(); @@ -57,13 +54,16 @@ void FprimeRouter ::dataIn_handler(FwIndexType portNum, Fw::Buffer& packetBuffer } // Handle a file packet case Fw::ComPacket::FW_PACKET_FILE: { - // If the file uplink output port is connected, - // send the file packet. Otherwise take no action. + // If the file uplink output port is connected, send the file packet. Otherwise take no action. if (this->isConnected_fileOut_OutputPort(0)) { - // Send the packet buffer - this->fileOut_out(0, packetBuffer); - // Transfer ownership of the packetBuffer to the receiver - deallocate = false; + // Copy buffer into a new allocated buffer. This lets us return the original buffer with dataReturnOut, + // and FprimeRouter can handle the deallocation of the file buffer when it returns on fileBufferReturnIn + Fw::Buffer packetBufferCopy = this->bufferAllocate_out(0, packetBuffer.getSize()); + auto copySerializer = packetBufferCopy.getSerializer(); + status = copySerializer.serialize(packetBuffer.getData(), packetBuffer.getSize(), Fw::Serialization::OMIT_LENGTH); + FW_ASSERT(status == Fw::FW_SERIALIZE_OK, status); + // Send the copied buffer out. It will come back on fileBufferReturnIn once the receiver is done with it + this->fileOut_out(0, packetBufferCopy); } break; } @@ -71,9 +71,14 @@ void FprimeRouter ::dataIn_handler(FwIndexType portNum, Fw::Buffer& packetBuffer // Packet type is not known to the F Prime protocol. If the unknownDataOut port is // connected, forward packet and context for further processing if (this->isConnected_unknownDataOut_OutputPort(0)) { - this->unknownDataOut_out(0, packetBuffer, context); - // Transfer ownership of the packetBuffer to the receiver - deallocate = false; + // Copy buffer into a new allocated buffer. This lets us return the original buffer with dataReturnOut, + // and FprimeRouter can handle the deallocation of the unknown buffer when it returns on bufferReturnIn + Fw::Buffer packetBufferCopy = this->bufferAllocate_out(0, packetBuffer.getSize()); + auto copySerializer = packetBufferCopy.getSerializer(); + status = copySerializer.serialize(packetBuffer.getData(), packetBuffer.getSize(), Fw::Serialization::OMIT_LENGTH); + FW_ASSERT(status == Fw::FW_SERIALIZE_OK, status); + // Send the copied buffer out. It will come back on fileBufferReturnIn once the receiver is done with it + this->unknownDataOut_out(0, packetBufferCopy, context); } } } @@ -81,10 +86,8 @@ void FprimeRouter ::dataIn_handler(FwIndexType portNum, Fw::Buffer& packetBuffer this->log_WARNING_HI_DeserializationError(status); } - if (deallocate) { - // Deallocate the packet buffer - this->bufferDeallocate_out(0, packetBuffer); - } + // Return ownership of the incoming packetBuffer + this->dataReturnOut_out(0, packetBuffer, context); } void FprimeRouter ::cmdResponseIn_handler(FwIndexType portNum, @@ -93,4 +96,9 @@ void FprimeRouter ::cmdResponseIn_handler(FwIndexType portNum, const Fw::CmdResponse& response) { // Nothing to do } + +void FprimeRouter ::fileBufferReturnIn_handler(FwIndexType portNum, Fw::Buffer& fwBuffer) { + this->bufferDeallocate_out(0, fwBuffer); +} + } // namespace Svc diff --git a/Svc/FprimeRouter/FprimeRouter.fpp b/Svc/FprimeRouter/FprimeRouter.fpp index baff261a9d..89fcd0cced 100644 --- a/Svc/FprimeRouter/FprimeRouter.fpp +++ b/Svc/FprimeRouter/FprimeRouter.fpp @@ -10,6 +10,9 @@ module Svc { @ Port for forwarding non-recognized packet types output port unknownDataOut: Svc.ComDataWithContext + @ Port for allocating buffers + output port bufferAllocate: Fw.BufferGet + @ Port for deallocating buffers output port bufferDeallocate: Fw.BufferSend diff --git a/Svc/FprimeRouter/FprimeRouter.hpp b/Svc/FprimeRouter/FprimeRouter.hpp index cb3bed691a..6a21bb8369 100644 --- a/Svc/FprimeRouter/FprimeRouter.hpp +++ b/Svc/FprimeRouter/FprimeRouter.hpp @@ -44,6 +44,14 @@ class FprimeRouter final : public FprimeRouterComponentBase { U32 cmdSeq, //!< The command sequence number const Fw::CmdResponse& response //!< The command response ) override; + + //! Handler implementation for fileBufferReturnIn + //! + //! Port for receiving ownership back of buffers sent on fileOut + void fileBufferReturnIn_handler(FwIndexType portNum, //!< The port number + Fw::Buffer& fwBuffer //!< The buffer + ) override; + }; } // namespace Svc diff --git a/Svc/FprimeRouter/docs/sdd.md b/Svc/FprimeRouter/docs/sdd.md index d4bfe76089..374a4182fd 100644 --- a/Svc/FprimeRouter/docs/sdd.md +++ b/Svc/FprimeRouter/docs/sdd.md @@ -4,7 +4,13 @@ The `Svc::FprimeRouter` component routes F´ packets (such as command or file pa The `Svc::FprimeRouter` component receives F´ packets (as [Fw::Buffer](../../../Fw/Buffer/docs/sdd.md) objects) and routes them to other components through synchronous port calls. The input port of type `Svc.ComDataWithContext` passes this Fw.Buffer object along with optional context data which can help for routing. The current F Prime protocol does not use this context data, but is nevertheless present in the interface for compatibility with other protocols which may for example pass APIDs in the frame headers. -The `Svc::FprimeRouter` component supports `Fw::ComPacket::FW_PACKET_COMMAND` and `Fw::ComPacket::FW_PACKET_FILE` packet types. Unknown packet types are forwarded on the `unknownDataOut` port, which a project-specific component can connect to for custom routing. In the case of unknown data being forwarded, the ownership of the packet data `Fw::Buffer` object is passed to the receiver. +The `Svc::FprimeRouter` component supports `Fw::ComPacket::FW_PACKET_COMMAND` and `Fw::ComPacket::FW_PACKET_FILE` packet types. Unknown packet types are forwarded on the `unknownDataOut` port, which a project-specific component can connect to for custom routing. + +About memory management, all buffers sent by `Svc::FprimeRouter` on the `fileOut` and `unknownDataOut` ports are expected to be returned to the router through the `fileBufferReturnIn` port for deallocation. + +## Custom Routing + +The `Svc::FprimeRouter` component is designed to be extensible through the use of a project-specific router. The `unknownDataOut` port can be connected to a project-specific component that can receive all unknown packet types. This component can then implement custom handling of these unknown packets. After processing, the project-specific component shall return the received buffer to the `Svc::FprimeRouter` component through the `fileBufferReturnIn` port (named this way as it only receives file packets in the common use-case), which will deallocate the buffer. ## Usage Examples @@ -16,26 +22,18 @@ In the canonical uplink communications stack, `Svc::FprimeRouter` is connected t ![uplink_stack](../../FprimeDeframer/docs/img/deframer_uplink_stack.png) -## Class Diagram - - -```mermaid -classDiagram - class FprimeRouter~PassiveComponent~ { - + void dataIn_handler(portNum, packetBuffer, contextBuffer) - + void cmdResponseIn_handler(portNum, opcode, cmdSeq, response) - } -``` - ## Port Descriptions -| Name | Description | Type | -|---|---|---| -| `dataIn: Svc.ComDataWithContext` | Receiving Fw::Buffer with context buffer from Deframer | `guarded input` | -| `commandOut: Fw.Com` | Port for sending command packets as Fw::ComBuffers | `output` | -| `fileOut: Fw.BufferSend` | Port for sending file packets as Fw::Buffer (ownership passed to receiver) | `output` | -| `unknownDataOut: Svc.ComDataWithContext` | Port forwarding unknown data (useful for adding custom routing rules with a project-defined router) | `output` | -| `output`| bufferDeallocate | `Fw.BufferSend` | Port for deallocating buffers once routed | +| Kind | Name | Type | Description | +|---|---|---|---| +| `guarded input` | `dataIn` | `Svc.ComDataWithContext` | Receiving Fw::Buffer with context buffer from Deframer +| `guarded input` | `dataReturnOut` | `Svc.ComDataWithContext` | Returning ownership of buffer received on `dataIn` +| `output` | `commandOut` | `Fw.Com` | Port for sending command packets as Fw::ComBuffers | +| `output` | `fileOut` | `Fw.BufferSend` | Port for sending file packets as Fw::Buffer (ownership passed to receiver) | +| `sync input` | `fileBufferReturnIn` | `Fw.BufferSend` | Receiving back ownership of buffer sent on `fileOut` and `unknownDataOut` | +| `output` | `unknownDataOut` | `Svc.ComDataWithContext` | Port forwarding unknown data (useful for adding custom routing rules with a project-defined router) | +| `output`| `bufferAllocate` | `Fw.BufferGet` | Port for allocating buffers, allowing copy of received data | +| `output`| `bufferDeallocate` | `Fw.BufferSend` | Port for deallocating buffers | ## Requirements @@ -46,3 +44,5 @@ SVC-ROUTER-002 | `Svc::FprimeRouter` shall route packets of type `Fw::ComPacket: SVC-ROUTER-003 | `Svc::FprimeRouter` shall route packets of type `Fw::ComPacket::FW_PACKET_FILE` to the `fileOut` output port. | Routing file packets | Unit test | SVC-ROUTER-004 | `Svc::FprimeRouter` shall route data that is neither `Fw::ComPacket::FW_PACKET_COMMAND` nor `Fw::ComPacket::FW_PACKET_FILE` to the `unknownDataOut` output port. | Allows for projects to provide custom routing for additional (project-specific) uplink data types | Unit test | SVC-ROUTER-005 | `Svc::FprimeRouter` shall emit warning events if serialization errors occur during processing of incoming packets | Aid in diagnosing uplink issues | Unit test | +SVC-ROUTER-005 | `Svc::FprimeRouter` shall make a copy of buffers that represent a `FW_PACKET_FILE` | Aid in memory management of file buffers | Unit test | +SVC-ROUTER-005 | `Svc::FprimeRouter` shall return ownership of all buffers received on `dataIn` through `dataReturnOut` | Memory management | Unit test | diff --git a/Svc/FprimeRouter/test/ut/FprimeRouterTestMain.cpp b/Svc/FprimeRouter/test/ut/FprimeRouterTestMain.cpp index 98688b89af..08053af6a3 100644 --- a/Svc/FprimeRouter/test/ut/FprimeRouterTestMain.cpp +++ b/Svc/FprimeRouter/test/ut/FprimeRouterTestMain.cpp @@ -28,6 +28,11 @@ TEST(FprimeRouter, TestRouteUnknownPacketUnconnected) { Svc::FprimeRouterTester tester(true); tester.testRouteUnknownPacketUnconnected(); } +TEST(FprimeRouter, TestBufferReturn) { + COMMENT("Deallocate a returning buffer"); + Svc::FprimeRouterTester tester; + tester.testBufferReturn(); +} TEST(FprimeRouter, TestCommandResponse) { COMMENT("Handle a command response (no-op)"); Svc::FprimeRouterTester tester; diff --git a/Svc/FprimeRouter/test/ut/FprimeRouterTester.cpp b/Svc/FprimeRouter/test/ut/FprimeRouterTester.cpp index 34aac1ce3c..f2a38552c6 100644 --- a/Svc/FprimeRouter/test/ut/FprimeRouterTester.cpp +++ b/Svc/FprimeRouter/test/ut/FprimeRouterTester.cpp @@ -33,7 +33,8 @@ void FprimeRouterTester ::testRouteComInterface() { ASSERT_from_commandOut_SIZE(1); // one command packet emitted ASSERT_from_fileOut_SIZE(0); // no file packet emitted ASSERT_from_unknownDataOut_SIZE(0); // no unknown data emitted - ASSERT_from_bufferDeallocate_SIZE(1); // command packets are deallocated by the router + ASSERT_from_dataReturnOut_SIZE(1); // data ownership should always be returned + ASSERT_from_bufferAllocate_SIZE(0); // no buffer allocation for Com packets } void FprimeRouterTester ::testRouteFileInterface() { @@ -41,7 +42,8 @@ void FprimeRouterTester ::testRouteFileInterface() { ASSERT_from_commandOut_SIZE(0); // no command packet emitted ASSERT_from_fileOut_SIZE(1); // one file packet emitted ASSERT_from_unknownDataOut_SIZE(0); // no unknown data emitted - ASSERT_from_bufferDeallocate_SIZE(0); // no deallocation (file packets' ownership is transferred to the receiver) + ASSERT_from_dataReturnOut_SIZE(1); // data ownership should always be returned + ASSERT_from_bufferAllocate_SIZE(1); // file packet was copied into a new allocated buffer } void FprimeRouterTester ::testRouteUnknownPacket() { @@ -49,15 +51,26 @@ void FprimeRouterTester ::testRouteUnknownPacket() { ASSERT_from_commandOut_SIZE(0); // no command packet emitted ASSERT_from_fileOut_SIZE(0); // no file packet emitted ASSERT_from_unknownDataOut_SIZE(1); // one unknown data emitted - ASSERT_from_bufferDeallocate_SIZE(0); // no deallocation (unknown data ownership is transferred to the receiver) + ASSERT_from_dataReturnOut_SIZE(1); // data ownership should always be returned + ASSERT_from_bufferAllocate_SIZE(1); // unknown packet was copied into a new allocated buffer } void FprimeRouterTester ::testRouteUnknownPacketUnconnected() { this->mockReceivePacketType(Fw::ComPacket::FW_PACKET_UNKNOWN); ASSERT_from_commandOut_SIZE(0); // no command packet emitted ASSERT_from_fileOut_SIZE(0); // no file packet emitted - ASSERT_from_unknownDataOut_SIZE(0); // zero unknown data emitted - ASSERT_from_bufferDeallocate_SIZE(1); // test that buffer is deallocated when output port is not connected + ASSERT_from_unknownDataOut_SIZE(0); // zero unknown data emitted when port is unconnected + ASSERT_from_dataReturnOut_SIZE(1); // data ownership should always be returned + ASSERT_from_bufferAllocate_SIZE(0); // no buffer allocation when port is unconnected +} + +void FprimeRouterTester ::testBufferReturn() { + U8 data[1]; + Fw::Buffer buffer(data, sizeof(data)); + this->invoke_to_fileBufferReturnIn(0, buffer); + ASSERT_from_bufferDeallocate_SIZE(1); // incoming buffer should be deallocated + ASSERT_EQ(this->fromPortHistory_bufferDeallocate->at(0).fwBuffer.getData(), data); + ASSERT_EQ(this->fromPortHistory_bufferDeallocate->at(0).fwBuffer.getSize(), sizeof(data)); } void FprimeRouterTester ::testCommandResponse() { @@ -87,15 +100,27 @@ void FprimeRouterTester::connectPortsExceptUnknownData() { this->component.set_logOut_OutputPort(0, this->get_from_logOut(0)); this->component.set_logTextOut_OutputPort(0, this->get_from_logTextOut(0)); this->component.set_timeCaller_OutputPort(0, this->get_from_timeCaller(0)); - // Connect typed input ports this->connect_to_cmdResponseIn(0, this->component.get_cmdResponseIn_InputPort(0)); this->connect_to_dataIn(0, this->component.get_dataIn_InputPort(0)); - + this->connect_to_fileBufferReturnIn(0, this->component.get_fileBufferReturnIn_InputPort(0)); // Connect typed output ports + this->component.set_bufferAllocate_OutputPort(0, this->get_from_bufferAllocate(0)); this->component.set_bufferDeallocate_OutputPort(0, this->get_from_bufferDeallocate(0)); this->component.set_commandOut_OutputPort(0, this->get_from_commandOut(0)); + this->component.set_dataReturnOut_OutputPort(0, this->get_from_dataReturnOut(0)); this->component.set_fileOut_OutputPort(0, this->get_from_fileOut(0)); } +// ---------------------------------------------------------------------- +// Port handler overrides +// ---------------------------------------------------------------------- +Fw::Buffer FprimeRouterTester::from_bufferAllocate_handler(FwIndexType portNum, U32 size) { + this->pushFromPortEntry_bufferAllocate(size); + this->m_buffer.setData(this->m_buffer_slot); + this->m_buffer.setSize(size); + ::memset(this->m_buffer.getData(), 0, size); + return this->m_buffer; +} + } // namespace Svc diff --git a/Svc/FprimeRouter/test/ut/FprimeRouterTester.hpp b/Svc/FprimeRouter/test/ut/FprimeRouterTester.hpp index dd62d51fe2..bc32e6a4a7 100644 --- a/Svc/FprimeRouter/test/ut/FprimeRouterTester.hpp +++ b/Svc/FprimeRouter/test/ut/FprimeRouterTester.hpp @@ -56,6 +56,9 @@ class FprimeRouterTester : public FprimeRouterGTestBase { //! Route a packet of unknown type void testRouteUnknownPacketUnconnected(); + //! Deallocate a returning buffer + void testBufferReturn(); + //! Invoke the command response input port void testCommandResponse(); @@ -76,6 +79,12 @@ class FprimeRouterTester : public FprimeRouterGTestBase { //! Mock the reception of a packet of a specific type void mockReceivePacketType(Fw::ComPacket::ComPacketType packetType); + // ---------------------------------------------------------------------- + // Port handler overrides + // ---------------------------------------------------------------------- + //! Overriding bufferAllocate handler to be able to request a buffer in component tests + Fw::Buffer from_bufferAllocate_handler(FwIndexType portNum, U32 size) override; + private: // ---------------------------------------------------------------------- // Member variables @@ -83,6 +92,9 @@ class FprimeRouterTester : public FprimeRouterGTestBase { //! The component under test FprimeRouter component; + + Fw::Buffer m_buffer; // buffer to be returned by mocked bufferAllocate call + U8 m_buffer_slot[64]; }; } // namespace Svc diff --git a/Svc/FrameAccumulator/FrameAccumulator.cpp b/Svc/FrameAccumulator/FrameAccumulator.cpp index 7243b51ebb..79acf7d563 100644 --- a/Svc/FrameAccumulator/FrameAccumulator.cpp +++ b/Svc/FrameAccumulator/FrameAccumulator.cpp @@ -50,14 +50,14 @@ void FrameAccumulator ::cleanup() { // Handler implementations for user-defined typed input ports // ---------------------------------------------------------------------- -void FrameAccumulator ::dataIn_handler(FwIndexType portNum, Fw::Buffer& buffer) { +void FrameAccumulator ::dataIn_handler(FwIndexType portNum, Fw::Buffer& buffer, const ComCfg::FrameContext& context) { // Check whether there is data to process if (buffer.isValid()) { + // The buffer is not necessarily a full frame, so the attached context has no meaning and we ignore it this->processBuffer(buffer); } - // TODO: rework the uplink deallocation logic to use the bufferReturn chaining pattern - // Deallocate the buffer - this->bufferDeallocate_out(0, buffer); + // Return ownership of the incoming data + this->dataReturnOut_out(0, buffer, context); } void FrameAccumulator ::processBuffer(Fw::Buffer& buffer) { @@ -136,7 +136,7 @@ void FrameAccumulator ::processRing() { static_cast(m_inRing.get_allocated_size()), static_cast(remaining), static_cast(size_out)); ComCfg::FrameContext context; - this->frameOut_out(0, buffer, context); + this->dataOut_out(0, buffer, context); } else { // No buffer is available, we need to exit and try again later this->log_WARNING_HI_NoBufferAvailable(); @@ -163,4 +163,11 @@ void FrameAccumulator ::processRing() { } } } + +void FrameAccumulator ::dataReturnIn_handler(FwIndexType portNum, Fw::Buffer& fwBuffer, const ComCfg::FrameContext& context) { + // Frame buffer ownership is returned to the component. Component had allocated with a buffer manager, + // so we return it to the buffer manager for deallocation + this->bufferDeallocate_out(0, fwBuffer); +} + } // namespace Svc diff --git a/Svc/FrameAccumulator/FrameAccumulator.fpp b/Svc/FrameAccumulator/FrameAccumulator.fpp index 8a4b76cbc3..10fe2251e1 100644 --- a/Svc/FrameAccumulator/FrameAccumulator.fpp +++ b/Svc/FrameAccumulator/FrameAccumulator.fpp @@ -7,8 +7,7 @@ module Svc { # ---------------------------------------------------------------------- include "../Interfaces/FrameAccumulatorInterface.fppi" - - @ Port for deallocating buffers received on dataIn. + @ Port for deallocating buffers holding extracted frames output port bufferDeallocate: Fw.BufferSend @ Port for allocating buffer to hold extracted frame diff --git a/Svc/FrameAccumulator/FrameAccumulator.hpp b/Svc/FrameAccumulator/FrameAccumulator.hpp index 9635ec4a42..a6bd4128a4 100644 --- a/Svc/FrameAccumulator/FrameAccumulator.hpp +++ b/Svc/FrameAccumulator/FrameAccumulator.hpp @@ -48,9 +48,18 @@ class FrameAccumulator final : public FrameAccumulatorComponentBase { //! Handler implementation for dataIn //! - //! Receives raw data from a ByteStreamDriver, ComStub, or other buffer producing component + //! Receive stream of bytes from a ComInterface component void dataIn_handler(FwIndexType portNum, //!< The port number - Fw::Buffer& recvBuffer) override; + Fw::Buffer& recvBuffer, + const ComCfg::FrameContext& context) override; + + //! Handler implementation for bufferReturnIn + //! + //! Port receiving ownership back of buffers sent on dataOut + void dataReturnIn_handler(FwIndexType portNum, //!< The port number + Fw::Buffer& fwBuffer, //!< The buffer + const ComCfg::FrameContext& context //!< The context object + ) override; PRIVATE: //! \brief process raw buffer diff --git a/Svc/FrameAccumulator/docs/sdd.md b/Svc/FrameAccumulator/docs/sdd.md index 34e24c1ae6..0b7fb0e45f 100644 --- a/Svc/FrameAccumulator/docs/sdd.md +++ b/Svc/FrameAccumulator/docs/sdd.md @@ -2,7 +2,7 @@ The `Svc::FrameAccumulator` component accumulates a stream of data (sequence of [Fw::Buffer](../../../Fw/Buffer/docs/sdd.md) objects) to extract full frames from. -The `Svc::FrameAccumulator` accepts as input a sequence of byte buffers, which typically come from a ground data system via a [ByteStreamDriver](../../../Drv/ByteStreamDriverModel/docs/sdd.md). It extracts the frames from the sequence of buffers and emits them on the `frameOut` output port. +The `Svc::FrameAccumulator` accepts as input a sequence of byte buffers, which typically come from a ground data system via a [ByteStreamDriver](../../../Drv/ByteStreamDriverModel/docs/sdd.md). It extracts the frames from the sequence of buffers and emits them on the `dataOut` output port. ## Internals @@ -19,7 +19,7 @@ The uplink frames need not be aligned on the buffer boundaries, and each frame m The `Svc::FrameAccumulator` receives `Fw::Buffer` objects on its `dataIn` input port. These buffers are accumulated in a `Utils::CircularBuffer`. Every time a new buffer is accumulated into the circular buffer, the `Svc::FrameAccumulator` enters a loop to `detect()` a frame within the circular buffer, starting at the current head of the circular buffer. The `Svc::FrameDetector` returns one of three results: - `NO_FRAME_DETECTED`: indicates no valid frame is present at the head of the circular buffer (for example, start word does not match the current head of the circular buffer). The `Svc::FrameAccumulator` rotates the circular buffer one byte and loops over to `detect()` again, or break the loop if the circular buffer is exhausted. -- `FRAME_DETECTED`: indicates there is a frame at the current head of the circular buffer. The `Svc::FrameAccumulator` allocates a new `Fw::Buffer` object to hold the frame, copies the detected frame from the circular buffer into the new `Fw::Buffer` object, and emits the new `Fw::Buffer` object (containing the frame) on its `frameOut` output port. The `Svc::FrameAccumulator` then rotates the circular buffer to remove the data that was just extracted, and deallocates the original `Fw::Buffer` that was received on the `dataIn` input port. +- `FRAME_DETECTED`: indicates there is a frame at the current head of the circular buffer. The `Svc::FrameAccumulator` allocates a new `Fw::Buffer` object to hold the frame, copies the detected frame from the circular buffer into the new `Fw::Buffer` object, and emits the new `Fw::Buffer` object (containing the frame) on its `dataOut` output port. The `Svc::FrameAccumulator` then rotates the circular buffer to remove the data that was just extracted, and deallocates the original `Fw::Buffer` that was received on the `dataIn` input port. - `MORE_DATA_NEEDED`: indicates that more data is needed to determine whether there is a valid frame. The `Svc::FrameAccumulator` deallocates the original `Fw::Buffer` that was received on the `dataIn` input port and halts execution, effectively waiting for the next `Fw::Buffer` to be received on the `dataIn` input port. @@ -85,7 +85,9 @@ SVC-FRAME-ACCUMULATOR-004 | `Svc::FrameAccumulator` shall accept byte buffers co | Kind | Name | Type | Description | |---|---|---|---| -| `guarded input` | dataIn | `Drv.ByteStreamRecv` | Receives raw data from a ByteStreamDriver, ComStub, or other buffer producing component | -| `output` | frameOut | `Svc.ComDataWithContext` | Port for sending an extracted frame out | +| `guarded input` | dataIn | `Svc.ComDataWithContext` | Receives a stream of byte buffers from a [Communication Adapter](../../Interfaces/docs/sdd.md) | +| `output` | dataOut | `Svc.ComDataWithContext` | Port for sending an extracted frame out | | `output` | bufferAllocate | `Fw.BufferGet` | Port for allocating buffer to hold extracted frame | | `output`| bufferDeallocate | `Fw.BufferSend` | Port for deallocating buffers received on dataIn. | +| `output` | dataReturnOut | `Svc.ComDataWithContext` | Port for returning ownership of buffers received on dataIn | +| `sync input` | dataReturnIn | `Svc.ComDataWithContext` | Receiving back ownership of buffers sent on dataOut | diff --git a/Svc/FrameAccumulator/test/ut/FrameAccumulatorTestMain.cpp b/Svc/FrameAccumulator/test/ut/FrameAccumulatorTestMain.cpp index c84b984d42..5f29fd93a1 100644 --- a/Svc/FrameAccumulator/test/ut/FrameAccumulatorTestMain.cpp +++ b/Svc/FrameAccumulator/test/ut/FrameAccumulatorTestMain.cpp @@ -42,6 +42,10 @@ TEST(FrameAccumulator, testAccumulateBuffersEmitManyFrames) { tester.testAccumulateBuffersEmitManyFrames(); } +TEST(FrameAccumulator, testBufferReturnDeallocation) { + Svc::FrameAccumulatorTester tester; + tester.testBufferReturnDeallocation(); +} int main(int argc, char** argv) { STest::Random::seed(); diff --git a/Svc/FrameAccumulator/test/ut/FrameAccumulatorTester.cpp b/Svc/FrameAccumulator/test/ut/FrameAccumulatorTester.cpp index b0b96c2b14..c084ab08be 100644 --- a/Svc/FrameAccumulator/test/ut/FrameAccumulatorTester.cpp +++ b/Svc/FrameAccumulator/test/ut/FrameAccumulatorTester.cpp @@ -37,15 +37,16 @@ void FrameAccumulatorTester ::testFrameDetected() { U32 buffer_size = STest::Random::lowerUpper(1, 1024); U8 data[buffer_size]; Fw::Buffer buffer(data, buffer_size); + ComCfg::FrameContext context; // Set the mock detector to report success of size_out = buffer_size this->mockDetector.set_next_result(FrameDetector::Status::FRAME_DETECTED, buffer_size); // Receive the buffer on dataIn - this->invoke_to_dataIn(0, buffer); + this->invoke_to_dataIn(0, buffer, context); // Checks - ASSERT_from_bufferDeallocate_SIZE(1); // input buffer was deallocated - ASSERT_from_frameOut_SIZE(1); // frame was sent + ASSERT_from_dataReturnOut_SIZE(1); // input buffer ownership was returned + ASSERT_from_dataOut_SIZE(1); // frame was sent ASSERT_EQ(this->component.m_inRing.get_allocated_size(), 0); // no data left in ring buffer - ASSERT_EQ(this->fromPortHistory_frameOut->at(0).data.getSize(), buffer_size); // all data was sent out + ASSERT_EQ(this->fromPortHistory_dataOut->at(0).data.getSize(), buffer_size); // all data was sent out } void FrameAccumulatorTester ::testMoreDataNeeded() { @@ -53,13 +54,14 @@ void FrameAccumulatorTester ::testMoreDataNeeded() { U32 buffer_size = STest::Random::lowerUpper(1, 1024); U8 data[buffer_size]; Fw::Buffer buffer(data, buffer_size); + ComCfg::FrameContext context; // Set the mock detector to report more data needed this->mockDetector.set_next_result(FrameDetector::Status::MORE_DATA_NEEDED, buffer_size + 1); // Receive the buffer on dataIn - this->invoke_to_dataIn(0, buffer); + this->invoke_to_dataIn(0, buffer, context); // Checks - ASSERT_from_bufferDeallocate_SIZE(1); // input buffer was deallocated - ASSERT_from_frameOut_SIZE(0); // frame was not sent (waiting on more data) + ASSERT_from_dataReturnOut_SIZE(1); // input buffer ownership was returned + ASSERT_from_dataOut_SIZE(0); // frame was not sent (waiting on more data) ASSERT_EQ(this->component.m_inRing.get_allocated_size(), buffer_size); // data left in ring buffer } @@ -68,13 +70,14 @@ void FrameAccumulatorTester ::testNoFrameDetected() { U32 buffer_size = STest::Random::lowerUpper(1, 1024); U8 data[buffer_size]; Fw::Buffer buffer(data, buffer_size); + ComCfg::FrameContext context; // Set the mock detector this->mockDetector.set_next_result(FrameDetector::Status::NO_FRAME_DETECTED, 0); // Receive the buffer on dataIn - this->invoke_to_dataIn(0, buffer); + this->invoke_to_dataIn(0, buffer, context); // Checks - ASSERT_from_bufferDeallocate_SIZE(1); // input buffer was deallocated - ASSERT_from_frameOut_SIZE(0); // No frame was sent out + ASSERT_from_dataReturnOut_SIZE(1); // input buffer ownership was returned + ASSERT_from_dataOut_SIZE(0); // No frame was sent out ASSERT_EQ(this->component.m_inRing.get_allocated_size(), 0); // all data was consumed and discarded } @@ -83,11 +86,12 @@ void FrameAccumulatorTester ::testReceiveZeroSizeBuffer() { // Prepare a zero size buffer U8 data[1] = {0}; Fw::Buffer buffer(data, 0); + ComCfg::FrameContext context; // Receive the buffer on dataIn - this->invoke_to_dataIn(0, buffer); + this->invoke_to_dataIn(0, buffer, context); // Checks - ASSERT_from_bufferDeallocate_SIZE(1); // input buffer was deallocated - ASSERT_from_frameOut_SIZE(0); // No frame was sent out + ASSERT_from_dataReturnOut_SIZE(1); // input buffer ownership was returned + ASSERT_from_dataOut_SIZE(0); // No frame was sent out ASSERT_EQ(this->component.m_inRing.get_allocated_size(), 0); // No data in ring buffer ASSERT_EQ(this->component.m_inRing.m_head_idx, 0); } @@ -99,18 +103,19 @@ void FrameAccumulatorTester ::testAccumulateTwoBuffers() { U8 data2[buffer2_size]; Fw::Buffer buffer1(data1, buffer1_size); Fw::Buffer buffer2(data2, buffer2_size); + ComCfg::FrameContext context; this->mockDetector.set_next_result(FrameDetector::Status::MORE_DATA_NEEDED, buffer2_size); // Receive the buffer on dataIn - this->invoke_to_dataIn(0, buffer1); + this->invoke_to_dataIn(0, buffer1, context); // Next result is detection of a full frame, size = buffer1_size + buffer2_size this->mockDetector.set_next_result(FrameDetector::Status::FRAME_DETECTED, buffer1_size + buffer2_size ); // Receive the buffer on dataIn - this->invoke_to_dataIn(0, buffer2); + this->invoke_to_dataIn(0, buffer2, context); // Checks - ASSERT_from_bufferDeallocate_SIZE(2); // both input buffers deallocated - ASSERT_from_frameOut_SIZE(1); // Exactly one frame was sent out + ASSERT_from_dataReturnOut_SIZE(2); // both input buffers ownership were returned + ASSERT_from_dataOut_SIZE(1); // Exactly one frame was sent out ASSERT_EQ(this->component.m_inRing.get_allocated_size(), 0); // No data in ring buffer } @@ -119,10 +124,10 @@ void FrameAccumulatorTester ::testAccumulateBuffersEmitFrame() { U32 buffer_count = 0; this->mockAccumulateFullFrame(frame_size, buffer_count); // Checks - ASSERT_from_bufferDeallocate_SIZE(buffer_count); // all input buffers deallocated - ASSERT_from_frameOut_SIZE(1); // Exactly one frame was sent out + ASSERT_from_dataReturnOut_SIZE(buffer_count); // all input buffers ownership were returned + ASSERT_from_dataOut_SIZE(1); // Exactly one frame was sent out ASSERT_EQ(this->component.m_inRing.get_allocated_size(), 0); // No data left in ring buffer - ASSERT_EQ(this->fromPortHistory_frameOut->at(0).data.getSize(), frame_size); // accumulated buffer size + ASSERT_EQ(this->fromPortHistory_dataOut->at(0).data.getSize(), frame_size); // accumulated buffer size } void FrameAccumulatorTester ::testAccumulateBuffersEmitManyFrames() { @@ -137,17 +142,27 @@ void FrameAccumulatorTester ::testAccumulateBuffersEmitManyFrames() { this->mockAccumulateFullFrame(frame_size, buffer_count); total_buffer_received += buffer_count; - ASSERT_from_bufferDeallocate_SIZE(total_buffer_received); // all input buffers deallocated - ASSERT_from_frameOut_SIZE(i+1); // Exactly one frame was sent out + ASSERT_from_dataReturnOut_SIZE(total_buffer_received); // all input buffers returned + ASSERT_from_dataOut_SIZE(i+1); // Exactly one frame was sent out ASSERT_EQ(this->component.m_inRing.get_allocated_size(), 0); // No data left in ring buffer - ASSERT_EQ(this->fromPortHistory_frameOut->at(i).data.getSize(), frame_size); // accumulated buffer size + ASSERT_EQ(this->fromPortHistory_dataOut->at(i).data.getSize(), frame_size); // accumulated buffer size } // Final checks - ASSERT_from_bufferDeallocate_SIZE(total_buffer_received); // all input buffers deallocated - ASSERT_from_frameOut_SIZE(max_iters); // Exactly max_iters frames were sent out + ASSERT_from_dataReturnOut_SIZE(total_buffer_received); // all input buffers returned + ASSERT_from_dataOut_SIZE(max_iters); // Exactly max_iters frames were sent out ASSERT_EQ(this->component.m_inRing.get_allocated_size(), 0); // No data left in ring buffer } +void FrameAccumulatorTester ::testBufferReturnDeallocation() { + U8 data[1]; + Fw::Buffer buffer(data, sizeof(data)); + ComCfg::FrameContext context; + this->invoke_to_dataReturnIn(0, buffer, context); + ASSERT_from_bufferDeallocate_SIZE(1); // incoming buffer should be deallocated + ASSERT_EQ(this->fromPortHistory_bufferDeallocate->at(0).fwBuffer.getData(), data); + ASSERT_EQ(this->fromPortHistory_bufferDeallocate->at(0).fwBuffer.getSize(), sizeof(data)); +} + // ---------------------------------------------------------------------- // Helper functions // ---------------------------------------------------------------------- @@ -163,6 +178,7 @@ void FrameAccumulatorTester ::mockAccumulateFullFrame(U32& frame_size, U32& buff U32 buffer_size; Fw::Buffer buffer(data, 0); U32 accumulated_size = 0; + ComCfg::FrameContext context; // Send multiple buffers with MORE_DATA_NEEDED for (U32 i = 0; i < iters; i++) { @@ -171,7 +187,7 @@ void FrameAccumulatorTester ::mockAccumulateFullFrame(U32& frame_size, U32& buff buffer.setSize(buffer_size); // Detector reports MORE_DATA_NEEDED and size needed bigger than accumulated size so far this->mockDetector.set_next_result(FrameDetector::Status::MORE_DATA_NEEDED, accumulated_size + 1); - this->invoke_to_dataIn(0, buffer); + this->invoke_to_dataIn(0, buffer, context); } // Send last buffer with FRAME_DETECTED @@ -181,7 +197,7 @@ void FrameAccumulatorTester ::mockAccumulateFullFrame(U32& frame_size, U32& buff // Send last buffer with finally FRAME_DETECTED and total accumulated + last buffer this->mockDetector.set_next_result(FrameDetector::Status::FRAME_DETECTED, accumulated_size); // Receive the last buffer on dataIn - this->invoke_to_dataIn(0, buffer); + this->invoke_to_dataIn(0, buffer, context); frame_size = accumulated_size; buffer_count = iters + 1; } diff --git a/Svc/FrameAccumulator/test/ut/FrameAccumulatorTester.hpp b/Svc/FrameAccumulator/test/ut/FrameAccumulatorTester.hpp index 8101edfbe0..191990b7ec 100644 --- a/Svc/FrameAccumulator/test/ut/FrameAccumulatorTester.hpp +++ b/Svc/FrameAccumulator/test/ut/FrameAccumulatorTester.hpp @@ -63,6 +63,9 @@ class FrameAccumulatorTester : public FrameAccumulatorGTestBase { //! Test accumulation of multiple random-size buffer into frames successively void testAccumulateBuffersEmitManyFrames(); + //! Test returning ownership of a buffer + void testBufferReturnDeallocation(); + private: // ---------------------------------------------------------------------- // Helper functions diff --git a/Svc/Interfaces/ComInterface.fppi b/Svc/Interfaces/ComInterface.fppi index 7d34b8412b..629096011e 100644 --- a/Svc/Interfaces/ComInterface.fppi +++ b/Svc/Interfaces/ComInterface.fppi @@ -3,10 +3,10 @@ # ---------------------------------------------------------------------- @ Data to be sent on the wire (coming in to the component) -sync input port comDataIn: Svc.ComDataWithContext +sync input port dataIn: Svc.ComDataWithContext -@ Data received from the wire (coming out of the component) -output port comDataOut: Fw.BufferSend +@ Data received from the wire (going out of the component) +output port dataOut: Svc.ComDataWithContext @ Status of the last transmission output port comStatusOut: Fw.SuccessCondition @@ -15,5 +15,8 @@ output port comStatusOut: Fw.SuccessCondition # Memory management # ---------------------------------------------------------------------- -@ Returning ownership of data that came in on comDataIn +@ Port returning ownership of data that came in on dataIn output port dataReturnOut: Svc.ComDataWithContext + +@ Port receiving back ownership of buffer sent out on dataOut +sync input port dataReturnIn: Svc.ComDataWithContext diff --git a/Svc/Interfaces/DeframerInterface.fppi b/Svc/Interfaces/DeframerInterface.fppi index 139bf30c54..be554f0904 100644 --- a/Svc/Interfaces/DeframerInterface.fppi +++ b/Svc/Interfaces/DeframerInterface.fppi @@ -1,5 +1,11 @@ @ Port to receive framed data, with optional context -guarded input port framedIn: Svc.ComDataWithContext +guarded input port dataIn: Svc.ComDataWithContext @ Port to output deframed data, with optional context -output port deframedOut: Svc.ComDataWithContext +output port dataOut: Svc.ComDataWithContext + +@ Port for returning ownership of received buffers to deframe +output port dataReturnOut: Svc.ComDataWithContext + +@ Port receiving back ownership of sent buffers +sync input port dataReturnIn: Svc.ComDataWithContext diff --git a/Svc/Interfaces/FrameAccumulatorInterface.fppi b/Svc/Interfaces/FrameAccumulatorInterface.fppi index 72f1635e08..76dabda8c1 100644 --- a/Svc/Interfaces/FrameAccumulatorInterface.fppi +++ b/Svc/Interfaces/FrameAccumulatorInterface.fppi @@ -1,5 +1,11 @@ @ Receive raw bytes from a ComInterface (e.g. ComStub) -guarded input port dataIn: Fw.BufferSend +guarded input port dataIn: Svc.ComDataWithContext @ Port for sending an extracted frame out -output port frameOut: Svc.ComDataWithContext +output port dataOut: Svc.ComDataWithContext + +@ Port for returning ownership of buffers received on dataIn +output port dataReturnOut: Svc.ComDataWithContext + +@ Port receiving back ownership of buffers sent on frameOut +sync input port dataReturnIn: Svc.ComDataWithContext diff --git a/Svc/Interfaces/RouterInterface.fppi b/Svc/Interfaces/RouterInterface.fppi index dafb91b0bb..19b0aec680 100644 --- a/Svc/Interfaces/RouterInterface.fppi +++ b/Svc/Interfaces/RouterInterface.fppi @@ -1,9 +1,21 @@ +# --------------------------------------------- +# Router <-> Deframers +# --------------------------------------------- @ Receiving data (Fw::Buffer) to be routed with optional context to help with routing sync input port dataIn: Svc.ComDataWithContext +@ Port for returning ownership of data (includes Fw.Buffer) received on dataIn +output port dataReturnOut: Svc.ComDataWithContext + +# --------------------------------------------- +# Router <-> CmdDispatch/FileUplink +# --------------------------------------------- @ Port for sending file packets as Fw::Buffer (ownership passed to receiver) output port fileOut: Fw.BufferSend +@ Port for receiving ownership back of buffers sent on fileOut +sync input port fileBufferReturnIn: Fw.BufferSend + @ Port for sending command packets as Fw::ComBuffers output port commandOut: Fw.Com diff --git a/Svc/Interfaces/docs/sdd.md b/Svc/Interfaces/docs/sdd.md index 18549bd19a..d51b650317 100644 --- a/Svc/Interfaces/docs/sdd.md +++ b/Svc/Interfaces/docs/sdd.md @@ -4,7 +4,7 @@ The Svc interfaces are a set of `.fppi` files that define FPP interfaces for com ## Svc/ComInterface -The `Svc/ComInterface` is an interface for implementing the [Communications Adapter Interface](../../../docs/reference/communication-adapter-interface.md). +The `Svc/ComInterface` is an interface for implementing the [Communications Adapter Interface](../../../docs/reference/communication-adapter-interface.md). [`Svc::ComStub`](../../ComStub/docs/sdd.md) implements this interface and uses a ByteStream driver to send and receive data on a TCP/UDP/UART link, and is often used in development and early testing. ## Svc/DeframerInterface diff --git a/docs/reference/communication-adapter-interface.md b/docs/reference/communication-adapter-interface.md index e28d040ea5..98705556c2 100644 --- a/docs/reference/communication-adapter-interface.md +++ b/docs/reference/communication-adapter-interface.md @@ -13,50 +13,51 @@ protocol to ensure that data messages do not overload a communication interface. ## Ports -The communication adapter interface is composed of three ports. These ports are used to transmit outgoing data through +The `Svc.ComDataWithContext` port type is used to transmit comms data between components of the F´ system. This port type is composed of a `Fw::Buffer` object and a `ComCfg::FrameContext` object. The `Fw::Buffer` object is used to transmit the data, while the `ComCfg::FrameContext` object is used to provide context for the data being transmitted, such as header information once the header has been consumed. + +The communication adapter interface is composed of five ports. These ports are used to transmit outgoing data through some communication hardware and receive incoming data from that same hardware. These ports share types with the byte stream driver model for backwards compatibility. -| Kind | Suggested Name | Port Type | Usage | -|----------|----------------|-----------------------|----------------------------------------------------------------| -| `input` | `comDataIn` | `Svc.ComDataWithContext` | Port receiving `Fw::Buffer` objects for outgoing transmission (to be sent on the wire) | -| `output` | `comDataOut` | `Fw.BufferSend` | Port producing incoming `Fw::Buffer` objects (received on the wire) | -| `output` | `dataReturnOut` | `Svc.ComDataWithContext` | Port returning ownership of the `Fw::Buffer` sent on the `comDataIn` port | -| `output` | `comStatusOut` | `Fw.SuccessCondition` | Port indicating status of outgoing transmission. See protocol. | +| Kind | Suggested Name | Port Type | Description | +|----------|----------------|--------------------------|----------------------------------------------------------------| +| `input` | `dataIn` | `Svc.ComDataWithContext` | Port receiving data for outgoing transmission (to be sent on the wire) | +| `output` | `dataOut` | `Svc.ComDataWithContext` | Port producing data from incoming transmissions (received on the wire) | +| `output` | `dataReturnOut`| `Svc.ComDataWithContext` | Port for sending back ownership of the `Fw::Buffer` received on the `dataIn` port | +| `input` | `dataReturnIn` | `Svc.ComDataWithContext` | Port for receiving back ownership of the `Fw::Buffer` sent on the `dataOut` port | +| `output` | `comStatusOut` | `Fw.SuccessCondition` | Port indicating status of outgoing transmission. See protocol. | > [!NOTE] -> About buffer management: after receiving a buffer on the `comDataIn` port, the communication component must return ownership of said buffer through the `dataReturnOut` port. The common scenario is to connect `comDataIn` and `dataReturnOut` to the same component, so that the sender can handle deallocation. -> This is done with a callback so that `comDataIn` can be an asynchronous port if needed. +> About buffer management: after receiving a buffer on the `dataIn` port, the communication component must return ownership of said buffer through the `dataReturnOut` port. The common scenario is to connect `dataIn` and `dataReturnOut` to the same component, so that the sender can handle deallocation. This is done with a callback so that `dataIn` can be an asynchronous port if needed. +> Similarly, the buffer sent out on `dataOut` is expected to be returned through the `dataReturnIn` port. -### comDataIn Description +### dataIn Description -This port receives data from an F´ application in the form of an argument of `Fw::Buffer` type. This data is intended to -be sent out the communications interface managed by this component (often referred to as _the wire_). From the perspective of the application this is -the outgoing data port. +This port receives data from an F´ application in the form of an argument of `Fw::Buffer` type and a `ComCfg::FrameContext` context. This data is intended to be sent out the communications interface managed by this component (often referred to as _the wire_). From the perspective of the application this is the _outgoing data_ port. -### comDataOut Description +### dataOut Description -This port receives data from the communication interface managed by this component and provides it to the F´ application -in the form of an argument of `Fw::Buffer` type. From the perspective of the application this is the incoming data port. +This port produces data to the F´ application once it is received on the wire. From the perspective of the application this is the _incoming data_ port. ### dataReturnOut Description -This port is used to receive a callback returning ownership of the `Fw::Buffer` object that was sent on the `comDataIn` port. Ownership is typically returned to the sender. A callback is used so that `comDataIn` may be an asynchronous port if needed. +This port is used to send a callback returning ownership of the `Fw::Buffer` object that was received on the `dataIn` port. Ownership is typically returned to the sender. A callback is used so that `dataIn` may be an asynchronous port if needed. + +### dataReturnIn Description + +This port is used to receive a callback returning ownership of the `Fw::Buffer` object that was sent on the `dataOut` port. Ownership is typically returned from the receiver of dataOut. A callback is used so that `dataOut` may be an asynchronous call if needed. ### comStatusOut Description -This port carries a status of `Fw::Success::SUCCESS` or `Fw::Success::FAILURE` typically in response to a call to the -`comDataIn` port described above. +This port carries a status of `Fw::Success::SUCCESS` or `Fw::Success::FAILURE` typically in response to a call to the `dataIn` port described above. > [!NOTE] > it is critical to obey the protocol as described in the protocol section below. ## Communication Queue Protocol -`Svc::ComQueue` queues messages until the communication adapter is ready to receive these messages. For each -Fw::Success::SUCCESS message received, `Svc::ComQueue` will emit one message. `Svc::ComQueue` will not emit messages -at any other time. This implies several things: +`Svc::ComQueue` queues messages until the communication adapter is ready to receive these messages. For each `Fw::Success::SUCCESS` message received, `Svc::ComQueue` will emit one message. `Svc::ComQueue` will not emit messages at any other time. This implies several things: 1. An initial `Fw::Success::SUCCESS` message must be sent to `Svc::ComQueue` to initiate data flow 2. A `Fw::Success::SUCCESS` must be eventually received in response to each message for data to continue to flow @@ -65,22 +66,19 @@ These implications are discussed in more depth in the appropriate protocol secti ## Framer Status Protocol -Framing typically happens between `Svc::ComQueue` and a communications adapter. The `Svc::Framer` component implements -this protocol in order to interoperate with these other components. The action taken by this protocol is dependent on -the number of framed messages (frames) sent to the communication adapter in response to each message received from -`Svc::ComQueue`. +Framing typically happens between `Svc::ComQueue` and a communications adapter. The action taken by this protocol is dependent on the number of framed messages (frames) sent to the communication adapter in response to each message received from `Svc::ComQueue`. + +The `Svc::FprimeFramer` implements the Framer status protocol by emitting a frame for each message received from `Svc::ComQueue`. Therefore, the `Svc::FprimeFramer` receives a status from the communication adapter on each sent frame and forwards it back to `Svc::ComQueue`. This allows the data flow to continue, or pause if the communication adapter is unable to receive more data. + +Framer implementations may choose to implement a different behavior. For example, a framer may choose to accept multiple messages from `Svc::ComQueue` and concatenate them in a single frame. In this case, the framer implementation must manage the flow of statuses accordingly. This is summarized in the table below. | Produced Frames | Action | Rationale | |-----------------|----------------------------------------|----------------------------------------------------------| | 0 | Send one `Fw::Success::SUCCESS` status | Data must continue to flow to produce a frame | -| 1 | Pass through received statuses | Frame produced and communication adapter produces status | +| 1 | Pass through received status from ComQueue | Frame produced and communication adapter produces status | -When a frame is produced the communication adapter is responsible for determining the status of the message and its -statuses must be passed through. When no frame is produced, the communication adapter does not receive a frame and is -incapable of producing a status. `Svc::Framer` must act appropriately in this case. - -> [!NOTE] -> `Svc::Framer` must also pass through the initiation message from the communication adapter to start data flow. +> [!IMPORTANT] +> All framer implementations must pass through the initiation message from the communication adapter to start data flow. ## Communication Adapter Protocol @@ -89,16 +87,16 @@ transmissions. This is done with the `comStatus` port. A communication status is | Status | Description | |----------------------|-----------------------------------------------------------------------------------| -| Fw::Success::SUCCESS | *Communication adapter* transmission succeeded and is ready for more data. | +| Fw::Success::SUCCESS | *Communication adapter* transmission succeeded and is ready for more data. | | Fw::Success::FAILURE | Last transmission failed; *communication adapter* is unable to receive more data. | * Fw::Success::SUCCESS may also indicate a connection/reconnection success when data flow must be initiated. -A *Communication Adapter* shall emit either Fw::Success::SUCCESS or Fw::Success::FAILURE via the `comStatus` port once -for each call received on `comDataIn`. Additionally, a *Communication Adapter* shall emit Fw::Success::SUCCESS once at +A *Communication Adapter* shall emit either Fw::Success::SUCCESS or Fw::Success::FAILURE via the `comStatusOut` port once +for each call received on `dataIn`. Additionally, a *Communication Adapter* shall emit Fw::Success::SUCCESS once at startup to indicate communication is initially ready and once after each Fw::Success::FAILURE event to indicate that communication has been restored. By emitting Fw::Success::SUCCESS after any failure, the communication adapter ensures that each received message eventually results in a Fw::Success::SUCCESS. > [!NOTE] -> It is imperative that *Communication Adapters* implement the `comStatus` protocol correctly. +> It is imperative that *Communication Adapters* implement the `comStatusOut` protocol correctly.