Skip to content

Commit 2d3e09b

Browse files
thomas-bcthomas-bc
authored
Add FW_PACKET_FILE descriptor to FileDownlink and FileUplink packets (#3488)
* FileDownlink FileUplink changes * Fix static analysis... * Update tests to account for packet descriptor * Update with new serializer/deserializer * Address Review comment * formatting * Add casts and normalize offset method
1 parent a2141d9 commit 2d3e09b

File tree

9 files changed

+428
-537
lines changed

9 files changed

+428
-537
lines changed

‎Svc/FileDownlink/FileDownlink.cpp‎

Lines changed: 292 additions & 367 deletions
Large diffs are not rendered by default.

‎Svc/FileDownlink/test/ut/FileDownlinkTester.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,10 @@ namespace Svc {
669669
Fw::FilePacket& filePacket
670670
)
671671
{
672-
const Fw::SerializeStatus status = filePacket.fromBuffer(buffer);
672+
// buffer contains the FW_PACKET_FILE descriptor - we remove it before deserializing into the filePacket
673+
Fw::Buffer packetDataBuffer(buffer.getData() + sizeof(FwPacketDescriptorType),
674+
buffer.getSize() - sizeof(FwPacketDescriptorType));
675+
const Fw::SerializeStatus status = filePacket.fromBuffer(packetDataBuffer);
673676
ASSERT_EQ(Fw::FW_SERIALIZE_OK, status);
674677
}
675678

‎Svc/FileUplink/Events.fppi‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,11 @@ event DecodeError(
8585
severity warning high \
8686
id 9 \
8787
format "Unable to decode file packet. Status: {}"
88+
89+
@ Invalid packet received
90+
event InvalidPacketReceived(
91+
packetType: U32 @< The packet type received
92+
) \
93+
severity warning high \
94+
id 10 \
95+
format "Invalid packet received. Wrong packet type: {}"

‎Svc/FileUplink/FileUplink.cpp‎

Lines changed: 103 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -10,222 +10,196 @@
1010
//
1111
// ======================================================================
1212

13-
#include <Svc/FileUplink/FileUplink.hpp>
14-
#include <Fw/Types/Assert.hpp>
13+
#include <Fw/Com/ComPacket.hpp>
1514
#include <Fw/FPrimeBasicTypes.hpp>
15+
#include <Fw/Types/Assert.hpp>
16+
#include <Svc/FileUplink/FileUplink.hpp>
1617

1718
namespace Svc {
1819

19-
// ----------------------------------------------------------------------
20-
// Construction, initialization, and destruction
21-
// ----------------------------------------------------------------------
20+
// ----------------------------------------------------------------------
21+
// Construction, initialization, and destruction
22+
// ----------------------------------------------------------------------
2223

23-
FileUplink ::
24-
FileUplink(const char *const name) :
25-
FileUplinkComponentBase(name),
24+
FileUplink ::FileUplink(const char* const name)
25+
: FileUplinkComponentBase(name),
2626
m_receiveMode(START),
2727
m_lastSequenceIndex(0),
2828
m_lastPacketWriteStatus(Os::File::MAX_STATUS),
2929
m_filesReceived(this),
3030
m_packetsReceived(this),
31-
m_warnings(this)
32-
{
31+
m_warnings(this) {}
3332

34-
}
33+
FileUplink ::~FileUplink() {}
3534

36-
FileUplink ::
37-
~FileUplink()
38-
{
35+
// ----------------------------------------------------------------------
36+
// Handler implementations for user-defined typed input ports
37+
// ----------------------------------------------------------------------
3938

40-
}
39+
void FileUplink ::bufferSendIn_handler(const FwIndexType portNum, Fw::Buffer& buffer) {
40+
// If packet is too small to contain a packet type, log + deallocate and return
41+
if (buffer.getSize() < sizeof(FwPacketDescriptorType)) {
42+
this->log_WARNING_HI_InvalidPacketReceived(Fw::ComPacket::FW_PACKET_UNKNOWN);
43+
this->bufferSendOut_out(0, buffer);
44+
return;
45+
}
4146

42-
// ----------------------------------------------------------------------
43-
// Handler implementations for user-defined typed input ports
44-
// ----------------------------------------------------------------------
47+
// Read the packet type from the packet buffer
48+
FwPacketDescriptorType packetType;
49+
Fw::SerializeStatus status = buffer.getDeserializer().deserialize(packetType);
50+
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, status);
4551

46-
void FileUplink ::
47-
bufferSendIn_handler(
48-
const FwIndexType portNum,
49-
Fw::Buffer& buffer
50-
)
51-
{
52+
// If packet type is not a file packet, log + deallocate and return
53+
if (packetType != Fw::ComPacket::FW_PACKET_FILE) {
54+
this->log_WARNING_HI_InvalidPacketReceived(packetType);
55+
this->bufferSendOut_out(0, buffer);
56+
return;
57+
}
58+
59+
// Deserialize the file packet contents into Fw::FilePacket (remove packet type token)
60+
Fw::Buffer packetBuffer(buffer.getData() + sizeof(packetType),
61+
buffer.getSize() - static_cast<Fw::Buffer::SizeType>(sizeof(packetType)));
5262
Fw::FilePacket filePacket;
53-
const Fw::SerializeStatus status = filePacket.fromBuffer(buffer);
63+
status = filePacket.fromBuffer(packetBuffer);
5464
if (status != Fw::FW_SERIALIZE_OK) {
5565
this->log_WARNING_HI_DecodeError(status);
5666
} else {
5767
Fw::FilePacket::Type header_type = filePacket.asHeader().getType();
5868
switch (header_type) {
59-
case Fw::FilePacket::T_START:
60-
this->handleStartPacket(filePacket.asStartPacket());
61-
break;
62-
case Fw::FilePacket::T_DATA:
63-
this->handleDataPacket(filePacket.asDataPacket());
64-
break;
65-
case Fw::FilePacket::T_END:
66-
this->handleEndPacket(filePacket.asEndPacket());
67-
break;
68-
case Fw::FilePacket::T_CANCEL:
69-
this->handleCancelPacket();
70-
break;
71-
default:
72-
FW_ASSERT(0);
73-
break;
69+
case Fw::FilePacket::T_START:
70+
this->handleStartPacket(filePacket.asStartPacket());
71+
break;
72+
case Fw::FilePacket::T_DATA:
73+
this->handleDataPacket(filePacket.asDataPacket());
74+
break;
75+
case Fw::FilePacket::T_END:
76+
this->handleEndPacket(filePacket.asEndPacket());
77+
break;
78+
case Fw::FilePacket::T_CANCEL:
79+
this->handleCancelPacket();
80+
break;
81+
default:
82+
FW_ASSERT(0);
83+
break;
7484
}
7585
}
7686
this->bufferSendOut_out(0, buffer);
77-
}
78-
79-
void FileUplink ::
80-
pingIn_handler(
81-
const FwIndexType portNum,
82-
U32 key
83-
)
84-
{
85-
// return key
86-
this->pingOut_out(0,key);
87-
}
88-
89-
// ----------------------------------------------------------------------
90-
// Private helper functions
91-
// ----------------------------------------------------------------------
92-
93-
void FileUplink ::
94-
handleStartPacket(const Fw::FilePacket::StartPacket& startPacket)
95-
{
87+
}
88+
89+
void FileUplink ::pingIn_handler(const FwIndexType portNum, U32 key) {
90+
// return key
91+
this->pingOut_out(0, key);
92+
}
93+
94+
// ----------------------------------------------------------------------
95+
// Private helper functions
96+
// ----------------------------------------------------------------------
97+
98+
void FileUplink ::handleStartPacket(const Fw::FilePacket::StartPacket& startPacket) {
9699
// Clear all event throttles in preparation for new start packet
97100
this->log_WARNING_HI_FileWriteError_ThrottleClear();
98101
this->log_WARNING_HI_InvalidReceiveMode_ThrottleClear();
99102
this->log_WARNING_HI_PacketOutOfBounds_ThrottleClear();
100103
this->log_WARNING_HI_PacketOutOfOrder_ThrottleClear();
101104
this->m_packetsReceived.packetReceived();
102105
if (this->m_receiveMode != START) {
103-
this->m_file.osFile.close();
104-
this->m_warnings.invalidReceiveMode(Fw::FilePacket::T_START);
106+
this->m_file.osFile.close();
107+
this->m_warnings.invalidReceiveMode(Fw::FilePacket::T_START);
105108
}
106109
const Os::File::Status status = this->m_file.open(startPacket);
107110
if (status == Os::File::OP_OK) {
108-
this->goToDataMode();
109-
}
110-
else {
111-
this->m_warnings.fileOpen(this->m_file.name);
112-
this->goToStartMode();
111+
this->goToDataMode();
112+
} else {
113+
this->m_warnings.fileOpen(this->m_file.name);
114+
this->goToStartMode();
113115
}
114-
}
116+
}
115117

116-
void FileUplink ::
117-
handleDataPacket(const Fw::FilePacket::DataPacket& dataPacket)
118-
{
118+
void FileUplink ::handleDataPacket(const Fw::FilePacket::DataPacket& dataPacket) {
119119
this->m_packetsReceived.packetReceived();
120120
if (this->m_receiveMode != DATA) {
121-
this->m_warnings.invalidReceiveMode(Fw::FilePacket::T_DATA);
122-
return;
121+
this->m_warnings.invalidReceiveMode(Fw::FilePacket::T_DATA);
122+
return;
123123
}
124124

125125
const U32 sequenceIndex = dataPacket.asHeader().getSequenceIndex();
126126

127127
// skip this packet if it is a duplicate and it has already been written
128-
if (this->m_lastPacketWriteStatus == Os::File::OP_OK &&
129-
this->checkDuplicatedPacket(sequenceIndex)) {
128+
if (this->m_lastPacketWriteStatus == Os::File::OP_OK && this->checkDuplicatedPacket(sequenceIndex)) {
130129
return;
131130
}
132131

133132
this->checkSequenceIndex(sequenceIndex);
134133
const U32 byteOffset = dataPacket.getByteOffset();
135134
const U32 dataSize = dataPacket.getDataSize();
136135
if (byteOffset + dataSize > this->m_file.size) {
137-
this->m_warnings.packetOutOfBounds(sequenceIndex, this->m_file.name);
138-
return;
136+
this->m_warnings.packetOutOfBounds(sequenceIndex, this->m_file.name);
137+
return;
139138
}
140-
const Os::File::Status status = this->m_file.write(
141-
dataPacket.getData(),
142-
byteOffset,
143-
dataSize
144-
);
139+
const Os::File::Status status = this->m_file.write(dataPacket.getData(), byteOffset, dataSize);
145140
if (status != Os::File::OP_OK) {
146-
this->m_warnings.fileWrite(this->m_file.name);
141+
this->m_warnings.fileWrite(this->m_file.name);
147142
}
148143

149144
this->m_lastPacketWriteStatus = status;
150-
}
145+
}
151146

152-
void FileUplink ::
153-
handleEndPacket(const Fw::FilePacket::EndPacket& endPacket)
154-
{
147+
void FileUplink ::handleEndPacket(const Fw::FilePacket::EndPacket& endPacket) {
155148
this->m_packetsReceived.packetReceived();
156149
if (this->m_receiveMode == DATA) {
157-
this->m_filesReceived.fileReceived();
158-
this->checkSequenceIndex(endPacket.asHeader().getSequenceIndex());
159-
this->compareChecksums(endPacket);
160-
this->log_ACTIVITY_HI_FileReceived(this->m_file.name);
161-
}
162-
else {
163-
this->m_warnings.invalidReceiveMode(Fw::FilePacket::T_END);
150+
this->m_filesReceived.fileReceived();
151+
this->checkSequenceIndex(endPacket.asHeader().getSequenceIndex());
152+
this->compareChecksums(endPacket);
153+
this->log_ACTIVITY_HI_FileReceived(this->m_file.name);
154+
} else {
155+
this->m_warnings.invalidReceiveMode(Fw::FilePacket::T_END);
164156
}
165157
this->goToStartMode();
166-
}
158+
}
167159

168-
void FileUplink ::
169-
handleCancelPacket()
170-
{
160+
void FileUplink ::handleCancelPacket() {
171161
this->m_packetsReceived.packetReceived();
172162
this->log_ACTIVITY_HI_UplinkCanceled();
173163
this->goToStartMode();
174-
}
164+
}
175165

176-
void FileUplink ::
177-
checkSequenceIndex(const U32 sequenceIndex)
178-
{
166+
void FileUplink ::checkSequenceIndex(const U32 sequenceIndex) {
179167
if (sequenceIndex != this->m_lastSequenceIndex + 1) {
180-
this->m_warnings.packetOutOfOrder(
181-
sequenceIndex,
182-
this->m_lastSequenceIndex
183-
);
168+
this->m_warnings.packetOutOfOrder(sequenceIndex, this->m_lastSequenceIndex);
184169
}
185170
this->m_lastSequenceIndex = sequenceIndex;
186-
}
171+
}
187172

188-
bool FileUplink ::
189-
checkDuplicatedPacket(const U32 sequenceIndex)
190-
{
173+
bool FileUplink ::checkDuplicatedPacket(const U32 sequenceIndex) {
191174
// check for duplicate packet
192175
if (sequenceIndex == this->m_lastSequenceIndex) {
193-
this->m_warnings.packetDuplicate(sequenceIndex);
194-
return true;
176+
this->m_warnings.packetDuplicate(sequenceIndex);
177+
return true;
195178
}
196179

197180
return false;
198-
}
181+
}
199182

200-
void FileUplink ::
201-
compareChecksums(const Fw::FilePacket::EndPacket& endPacket)
202-
{
183+
void FileUplink ::compareChecksums(const Fw::FilePacket::EndPacket& endPacket) {
203184
CFDP::Checksum computed, stored;
204185
this->m_file.getChecksum(computed);
205186
endPacket.getChecksum(stored);
206187
if (computed != stored) {
207-
this->m_warnings.badChecksum(
208-
computed.getValue(),
209-
stored.getValue()
210-
);
188+
this->m_warnings.badChecksum(computed.getValue(), stored.getValue());
211189
}
212-
}
190+
}
213191

214-
void FileUplink ::
215-
goToStartMode()
216-
{
192+
void FileUplink ::goToStartMode() {
217193
this->m_file.osFile.close();
218194
this->m_receiveMode = START;
219195
this->m_lastSequenceIndex = 0;
220196
this->m_lastPacketWriteStatus = Os::File::MAX_STATUS;
221-
}
197+
}
222198

223-
void FileUplink ::
224-
goToDataMode()
225-
{
199+
void FileUplink ::goToDataMode() {
226200
this->m_receiveMode = DATA;
227201
this->m_lastSequenceIndex = 0;
228202
this->m_lastPacketWriteStatus = Os::File::MAX_STATUS;
229-
}
230-
231203
}
204+
205+
} // namespace Svc

‎Svc/FileUplink/test/ut/FileUplinkTester.cpp‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <cstring>
1515

1616
#include "FileUplinkTester.hpp"
17+
#include "Fw/Com/ComPacket.hpp"
1718

1819
#define INSTANCE 0
1920
#define MAX_HISTORY_SIZE 10
@@ -611,11 +612,17 @@ namespace Svc {
611612

612613
this->clearHistory();
613614

614-
const size_t bufferSize = filePacket.bufferSize();
615+
const size_t bufferSize = filePacket.bufferSize() + sizeof(FwPacketDescriptorType);
615616
U8 bufferData[bufferSize];
616617
Fw::Buffer buffer(bufferData, bufferSize);
617618

618-
const Fw::SerializeStatus status = filePacket.toBuffer(buffer);
619+
// Serialize the packet descriptor FW_PACKET_FILE to the buffer
620+
Fw::SerializeStatus status = buffer.getSerializer().serialize(Fw::ComPacket::FW_PACKET_FILE);
621+
FW_ASSERT(status == Fw::FW_SERIALIZE_OK);
622+
// Serialize the filePacket content into the buffer after the packet descriptor token
623+
Fw::Buffer offsetBuffer(buffer.getData() + sizeof(FwPacketDescriptorType),
624+
bufferSize - sizeof(FwPacketDescriptorType));
625+
status = filePacket.toBuffer(offsetBuffer);
619626
ASSERT_EQ(Fw::FW_SERIALIZE_OK, status);
620627

621628
this->invoke_to_bufferSendIn(0, buffer);

‎Svc/FprimeRouter/FprimeRouter.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,6 @@ void FprimeRouter ::dataIn_handler(FwIndexType portNum, Fw::Buffer& packetBuffer
6060
// If the file uplink output port is connected,
6161
// send the file packet. Otherwise take no action.
6262
if (this->isConnected_fileOut_OutputPort(0)) {
63-
// Make sure we can cast down to U32 without overflow
64-
FW_ASSERT((packetSize - sizeof(packetType)) < std::numeric_limits<U32>::max(),
65-
static_cast<FwAssertArgType>(packetSize - sizeof(packetType)));
66-
// Shift the packet buffer to skip the packet type
67-
// The FileUplink component does not expect the packet
68-
// type to be there.
69-
packetBuffer.setData(packetData + sizeof(packetType));
70-
packetBuffer.setSize(static_cast<U32>(packetSize - sizeof(packetType)));
7163
// Send the packet buffer
7264
this->fileOut_out(0, packetBuffer);
7365
// Transfer ownership of the packetBuffer to the receiver

0 commit comments

Comments
 (0)