Skip to content

Commit

Permalink
Fix copying of data product containers (#2711)
Browse files Browse the repository at this point in the history 
* Revise copy semantics in Fw::SerializeBufferBase

Update unit tests to match

* Revise copy semantics of DpContainer

* Revise DpTest

* Update spelling

* Revise SerializeBufferBase, ExternalSerializeBuffer

* Fix constructor initialization

* Restore private copy constructor
  • Loading branch information
@bocchino
bocchino authored May 1, 2024
1 parent cdac751 commit 0e1addb
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 55 deletions.
12 changes: 7 additions & 5 deletions .github/actions/spelling/expect.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,13 +268,14 @@ doxyindexer
doxyrules
doxysearch
Doxywizard
DPCFG
dpi
dpc
DPCAT
DPCATALOG
DPCFG
dpi
DPMANAGER
DPWRITER
dps
DPWRITER
DRAINBUFFERS
drv
dsdl
Expand Down Expand Up @@ -314,6 +315,7 @@ errmsg
ERRORCHECK
errornum
ert
esb
ethanchee
etime
eturn
Expand Down Expand Up @@ -433,8 +435,8 @@ Guire
handcoded
hardtoaccess
hashvalue
HEADERSIZE
hdr
HEADERSIZE
headlessly
heapifying
hexid
Expand Down Expand Up @@ -1000,10 +1002,10 @@ subgrouping
subhist
subhistory
subpage
suseconds
subseconds
subtargets
suppr
suseconds
SVCLOGFILE
SVCLOGFILEL
svipc
Expand Down
72 changes: 41 additions & 31 deletions FppTest/dp/DpTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ DpTest::DpTest(const char* const compName,
const DataArrayRecordData& dataArrayRecordData,
const Fw::StringBase& a_stringRecordData)
: DpTestComponentBase(compName),
m_container(),
u32RecordData(u32RecordData),
dataRecordData(dataRecordData),
u8ArrayRecordData(u8ArrayRecordData),
Expand Down Expand Up @@ -60,45 +61,40 @@ void DpTest::schedIn_handler(const NATIVE_INT_TYPE portNum, U32 context) {
this->dpRequest_Container6(CONTAINER_6_DATA_SIZE);
// Get a buffer for Container 1
{
DpContainer container;
Fw::Success status = this->dpGet_Container1(CONTAINER_1_DATA_SIZE, container);
Fw::Success status = this->dpGet_Container1(CONTAINER_1_DATA_SIZE, this->m_container);
FW_ASSERT(status == Fw::Success::SUCCESS, status);
// Check the container
this->checkContainer(container, ContainerId::Container1, CONTAINER_1_PACKET_SIZE,
this->checkContainer(this->m_container, ContainerId::Container1, CONTAINER_1_PACKET_SIZE,
DpTest::ContainerPriority::Container1);
}
// Get a buffer for Container 2
{
DpContainer container;
Fw::Success status = this->dpGet_Container2(CONTAINER_2_DATA_SIZE, container);
Fw::Success status = this->dpGet_Container2(CONTAINER_2_DATA_SIZE, this->m_container);
FW_ASSERT(status == Fw::Success::SUCCESS);
// Check the container
this->checkContainer(container, ContainerId::Container2, CONTAINER_2_PACKET_SIZE,
this->checkContainer(this->m_container, ContainerId::Container2, CONTAINER_2_PACKET_SIZE,
DpTest::ContainerPriority::Container2);
}
// Get a buffer for Container 3
{
DpContainer container;
Fw::Success status = this->dpGet_Container3(CONTAINER_3_DATA_SIZE, container);
Fw::Success status = this->dpGet_Container3(CONTAINER_3_DATA_SIZE, this->m_container);
// This one should fail
FW_ASSERT(status == Fw::Success::FAILURE);
}
// Get a buffer for Container 4
{
DpContainer container;
Fw::Success status = this->dpGet_Container4(CONTAINER_4_DATA_SIZE, container);
Fw::Success status = this->dpGet_Container4(CONTAINER_4_DATA_SIZE, this->m_container);
FW_ASSERT(status == Fw::Success::SUCCESS);
// Check the container
this->checkContainer(container, ContainerId::Container4, CONTAINER_4_PACKET_SIZE,
this->checkContainer(this->m_container, ContainerId::Container4, CONTAINER_4_PACKET_SIZE,
DpTest::ContainerPriority::Container4);
}
// Get a buffer for Container 5
{
DpContainer container;
Fw::Success status = this->dpGet_Container5(CONTAINER_5_DATA_SIZE, container);
Fw::Success status = this->dpGet_Container5(CONTAINER_5_DATA_SIZE, this->m_container);
FW_ASSERT(status == Fw::Success::SUCCESS);
// Check the container
this->checkContainer(container, ContainerId::Container5, CONTAINER_5_PACKET_SIZE,
this->checkContainer(this->m_container, ContainerId::Container5, CONTAINER_5_PACKET_SIZE,
DpTest::ContainerPriority::Container5);
}
}
Expand All @@ -108,112 +104,126 @@ void DpTest::schedIn_handler(const NATIVE_INT_TYPE portNum, U32 context) {
// ----------------------------------------------------------------------

void DpTest ::dpRecv_Container1_handler(DpContainer& container, Fw::Success::T status) {
// Test container assignment
this->m_container = container;
if (status == Fw::Success::SUCCESS) {
auto serializeStatus = Fw::FW_SERIALIZE_OK;
for (FwSizeType i = 0; i < CONTAINER_1_DATA_SIZE; ++i) {
serializeStatus = container.serializeRecord_U32Record(this->u32RecordData);
serializeStatus = this->m_container.serializeRecord_U32Record(this->u32RecordData);
if (serializeStatus == Fw::FW_SERIALIZE_NO_ROOM_LEFT) {
break;
}
FW_ASSERT(serializeStatus == Fw::FW_SERIALIZE_OK, status);
}
// Use the time stamp from the time get port
this->dpSend(container);
this->dpSend(this->m_container);
}
}

void DpTest ::dpRecv_Container2_handler(DpContainer& container, Fw::Success::T status) {
// Test container assignment
this->m_container = container;
if (status == Fw::Success::SUCCESS) {
const DpTest_Data dataRecord(this->dataRecordData);
auto serializeStatus = Fw::FW_SERIALIZE_OK;
for (FwSizeType i = 0; i < CONTAINER_2_DATA_SIZE; ++i) {
serializeStatus = container.serializeRecord_DataRecord(dataRecord);
serializeStatus = this->m_container.serializeRecord_DataRecord(dataRecord);
if (serializeStatus == Fw::FW_SERIALIZE_NO_ROOM_LEFT) {
break;
}
FW_ASSERT(serializeStatus == Fw::FW_SERIALIZE_OK, status);
}
// Provide an explicit time stamp
this->dpSend(container, this->sendTime);
this->dpSend(this->m_container, this->sendTime);
}
}

void DpTest ::dpRecv_Container3_handler(DpContainer& container, Fw::Success::T status) {
// Test container assignment
this->m_container = container;
if (status == Fw::Success::SUCCESS) {
auto serializeStatus = Fw::FW_SERIALIZE_OK;
for (FwSizeType i = 0; i < CONTAINER_3_DATA_SIZE; ++i) {
serializeStatus =
container.serializeRecord_U8ArrayRecord(this->u8ArrayRecordData.data(), this->u8ArrayRecordData.size());
serializeStatus = this->m_container.serializeRecord_U8ArrayRecord(this->u8ArrayRecordData.data(),
this->u8ArrayRecordData.size());
if (serializeStatus == Fw::FW_SERIALIZE_NO_ROOM_LEFT) {
break;
}
FW_ASSERT(serializeStatus == Fw::FW_SERIALIZE_OK, status);
}
// Use the time stamp from the time get port
this->dpSend(container);
this->dpSend(this->m_container);
}
}

void DpTest ::dpRecv_Container4_handler(DpContainer& container, Fw::Success::T status) {
// Test container assignment
this->m_container = container;
if (status == Fw::Success::SUCCESS) {
auto serializeStatus = Fw::FW_SERIALIZE_OK;
for (FwSizeType i = 0; i < CONTAINER_4_DATA_SIZE; ++i) {
serializeStatus = container.serializeRecord_U32ArrayRecord(this->u32ArrayRecordData.data(),
this->u32ArrayRecordData.size());
serializeStatus = this->m_container.serializeRecord_U32ArrayRecord(this->u32ArrayRecordData.data(),
this->u32ArrayRecordData.size());
if (serializeStatus == Fw::FW_SERIALIZE_NO_ROOM_LEFT) {
break;
}
FW_ASSERT(serializeStatus == Fw::FW_SERIALIZE_OK, status);
}
// Use the time stamp from the time get port
this->dpSend(container);
this->dpSend(this->m_container);
}
}

void DpTest ::dpRecv_Container5_handler(DpContainer& container, Fw::Success::T status) {
// Test container assignment
this->m_container = container;
if (status == Fw::Success::SUCCESS) {
auto serializeStatus = Fw::FW_SERIALIZE_OK;
for (FwSizeType i = 0; i < CONTAINER_5_DATA_SIZE; ++i) {
serializeStatus = container.serializeRecord_DataArrayRecord(this->dataArrayRecordData.data(),
this->dataArrayRecordData.size());
serializeStatus = this->m_container.serializeRecord_DataArrayRecord(this->dataArrayRecordData.data(),
this->dataArrayRecordData.size());
if (serializeStatus == Fw::FW_SERIALIZE_NO_ROOM_LEFT) {
break;
}
FW_ASSERT(serializeStatus == Fw::FW_SERIALIZE_OK, status);
}
// Use the time stamp from the time get port
this->dpSend(container);
this->dpSend(this->m_container);
}
}

void DpTest ::dpRecv_Container6_handler(DpContainer& container, Fw::Success::T status) {
// Test container assignment
this->m_container = container;
if (status == Fw::Success::SUCCESS) {
auto serializeStatus = Fw::FW_SERIALIZE_OK;
for (FwSizeType i = 0; i < CONTAINER_6_DATA_SIZE; ++i) {
serializeStatus = container.serializeRecord_StringRecord(this->stringRecordData);
serializeStatus = this->m_container.serializeRecord_StringRecord(this->stringRecordData);
if (serializeStatus == Fw::FW_SERIALIZE_NO_ROOM_LEFT) {
break;
}
FW_ASSERT(serializeStatus == Fw::FW_SERIALIZE_OK, status);
}
// Use the time stamp from the time get port
this->dpSend(container);
this->dpSend(this->m_container);
}
}

void DpTest ::dpRecv_Container7_handler(DpContainer& container, Fw::Success::T status) {
// Test container assignment
this->m_container = container;
if (status == Fw::Success::SUCCESS) {
auto serializeStatus = Fw::FW_SERIALIZE_OK;
for (FwSizeType i = 0; i < CONTAINER_7_DATA_SIZE; ++i) {
serializeStatus = container.serializeRecord_StringArrayRecord(
serializeStatus = this->m_container.serializeRecord_StringArrayRecord(
this->stringArrayRecordData, FW_NUM_ARRAY_ELEMENTS(this->stringArrayRecordData));
if (serializeStatus == Fw::FW_SERIALIZE_NO_ROOM_LEFT) {
break;
}
FW_ASSERT(serializeStatus == Fw::FW_SERIALIZE_OK, status);
}
// Use the time stamp from the time get port
this->dpSend(container);
this->dpSend(this->m_container);
}
}

Expand Down
3 changes: 3 additions & 0 deletions FppTest/dp/DpTest.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ class DpTest : public DpTestComponentBase {
// Private member variables
// ----------------------------------------------------------------------

//! Stored container
DpContainer m_container;

//! U32Record data
const U32 u32RecordData;

Expand Down
4 changes: 1 addition & 3 deletions Fw/Buffer/test/ut/TestBuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,19 @@ void test_representations() {
}
Fw::SerializeStatus stat = sbb.serialize(100);
ASSERT_NE(stat, Fw::FW_SERIALIZE_OK);

// And that another call to repr resets it
sbb = buffer.getSerializeRepr();
sbb.resetSer();
ASSERT_EQ(sbb.serialize(0), Fw::FW_SERIALIZE_OK);

// Now deserialize all the things
U32 out;
sbb = buffer.getSerializeRepr();
sbb.setBuffLen(buffer.getSize());
for (U32 i = 0; i < sizeof(data)/4; i++) {
ASSERT_EQ(sbb.deserialize(out), Fw::FW_SERIALIZE_OK);
ASSERT_EQ(i, out);
}
ASSERT_NE(sbb.deserialize(out), Fw::FW_SERIALIZE_OK);
sbb = buffer.getSerializeRepr();
sbb.setBuffLen(buffer.getSize());
ASSERT_EQ(sbb.deserialize(out), Fw::FW_SERIALIZE_OK);
ASSERT_EQ(0, out);
Expand Down
17 changes: 15 additions & 2 deletions Fw/Dp/DpContainer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class DpContainer {

public:
// ----------------------------------------------------------------------
// Constructor
// Constructors and destructors
// ----------------------------------------------------------------------

//! Constructor for initialized container
Expand All @@ -70,6 +70,17 @@ class DpContainer {
//! Constructor for container with default initialization
DpContainer();

//! Destructor
virtual ~DpContainer() {}

protected:
// ----------------------------------------------------------------------
// Protected operators
// ----------------------------------------------------------------------

//! Copy assignment operator
DpContainer& operator=(const DpContainer& src) = default;

public:
// ----------------------------------------------------------------------
// Public member functions
Expand Down Expand Up @@ -260,7 +271,9 @@ class DpContainer {
Buffer m_buffer;

//! The data buffer
Fw::ExternalSerializeBuffer m_dataBuffer;
//! We use member copy semantics because m_dataBuffer points into m_buffer,
//! which is owned by this object
Fw::ExternalSerializeBufferWithMemberCopy m_dataBuffer;
};

} // end namespace Fw
Expand Down
Loading

0 comments on commit 0e1addb

Please sign in to comment.