Skip to content

Add FW_PACKET_FILE descriptor to FileDownlink and FileUplink logic#3488

Merged
thomas-bc merged 8 commits intonasa:develfrom
thomas-bc:file-up-down-token
Apr 16, 2025
Merged

Add FW_PACKET_FILE descriptor to FileDownlink and FileUplink logic#3488
thomas-bc merged 8 commits intonasa:develfrom
thomas-bc:file-up-down-token

Conversation

@thomas-bc
Copy link
Collaborator

@thomas-bc thomas-bc commented Apr 12, 2025

Related Issue(s)
Has Unit Tests (y/n)
Documentation Included (y/n)

Change Description

Fix #3467

Packet descriptor is now handled at reception/emission time within FileUplink/FileDownlink.

Rationale

No custom code in routing / framing needed anymore, etc. File packets are stamped when created.

@thomas-bc thomas-bc mentioned this pull request Apr 12, 2025
Merged
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 12, 2025
View reviewed changes
Svc/FileUplink/FileUplink.cpp
m_packetsReceived(this),
m_warnings(this)
{
m_warnings(this) {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Svc/FileUplink/FileUplink.cpp
m_warnings(this) {}

}
FileUplink ::~FileUplink() {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 12, 2025
View reviewed changes
Svc/FileUplink/FileUplink.cpp
FileUplink ::
FileUplink(const char *const name) :
FileUplinkComponentBase(name),
FileUplink ::FileUplink(const char* const name)

Check notice

Code scanning / CodeQL

Use of basic integral type Note

name uses the basic integral type char rather than a typedef with size and signedness.
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 14, 2025
View reviewed changes
Svc/FileDownlink/FileDownlink.cpp
sendFilePacket(const Fw::FilePacket& filePacket)
{
const U32 bufferSize = filePacket.bufferSize();
const U32 bufferSize = filePacket.bufferSize() + sizeof(FwPacketDescriptorType);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter filePacket has not been checked.
Svc/FileUplink/FileUplink.cpp

void FileUplink ::pingIn_handler(const FwIndexType portNum, U32 key) {
// return key
this->pingOut_out(0, key);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter key has not been checked.
Svc/FileUplink/FileUplink.cpp
else {
this->m_warnings.invalidReceiveMode(Fw::FilePacket::T_END);
this->m_filesReceived.fileReceived();
this->checkSequenceIndex(endPacket.asHeader().getSequenceIndex());

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter endPacket has not been checked.
Svc/FramingProtocol/FprimeProtocol.cpp
FpFrameHeader::TokenType total = real_data_size + FpFrameHeader::SIZE + HASH_DIGEST_LENGTH;
Fw::Buffer buffer = m_interface->allocate(total);

FpFrameHeader::TokenType totalSize = size + FpFrameHeader::SIZE + HASH_DIGEST_LENGTH;

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter size has not been checked.
@thomas-bc thomas-bc requested a review from LeStarch April 15, 2025 22:12
LeStarch
LeStarch requested changes Apr 16, 2025
View reviewed changes
Copy link
Collaborator

@LeStarch LeStarch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the one minor fix!

github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 16, 2025
View reviewed changes
Svc/FileDownlink/FileDownlink.cpp
const char *const name
) :
FileDownlinkComponentBase(name),
FileDownlink ::FileDownlink(const char* const name)

Check notice

Code scanning / CodeQL

Use of basic integral type Note

name uses the basic integral type char rather than a typedef with size and signedness.
Svc/FileDownlink/FileDownlink.cpp
{
}

void FileDownlink ::sendFile(const char* sourceFilename, const char* destFilename, U32 startOffset, U32 length) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

sourceFilename uses the basic integral type char rather than a typedef with size and signedness.
Svc/FileDownlink/FileDownlink.cpp
{
}

void FileDownlink ::sendFile(const char* sourceFilename, const char* destFilename, U32 startOffset, U32 length) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

destFilename uses the basic integral type char rather than a typedef with size and signedness.
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 16, 2025
View reviewed changes
Svc/FileDownlink/FileDownlink.cpp
static_cast<FwSizeType>(sizeof(struct FileEntry))
);
Os::Queue::Status stat =
m_fileQueue.create(Os::QueueString("fileDownlinkQueue"), static_cast<FwSizeType>(fileQueueDepth),

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter fileQueueDepth has not been checked.
Svc/FileDownlink/FileDownlink.cpp
}

void FileDownlink ::pingIn_handler(const FwIndexType portNum, U32 key) {
this->pingOut_out(0, key);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter key has not been checked.
Svc/FileDownlink/FileDownlink.cpp
default:
// Trigger assertion if given unknown status
FW_ASSERT(false);
this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::OK);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter opCode has not been checked.
Svc/FileDownlink/FileDownlink.cpp
default:
// Trigger assertion if given unknown status
FW_ASSERT(false);
this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::OK);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter cmdSeq has not been checked.
Svc/FileDownlink/FileDownlink.cpp
// ----------------------------------------------------------------------

Fw::CmdResponse FileDownlink ::statusToCmdResp(SendFileStatus status) {
switch (status.e) {

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter status has not been checked.
Svc/FileDownlink/FileDownlink.cpp
this->cmdResponse_out(this->m_curEntry.opCode, this->m_curEntry.cmdSeq, statusToCmdResp(resp));
void FileDownlink ::sendResponse(SendFileStatus resp) {
if (this->m_curEntry.source == FileDownlink::COMMAND) {
this->cmdResponse_out(this->m_curEntry.opCode, this->m_curEntry.cmdSeq, statusToCmdResp(resp));

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter resp has not been checked.
Svc/FileDownlink/FileDownlink.cpp
this->FileComplete_out(i, Svc::SendFileResponse(resp, this->m_curEntry.context));
for (FwIndexType i = 0; i < this->getNum_FileComplete_OutputPorts(); i++) {
if (this->isConnected_FileComplete_OutputPort(i)) {
this->FileComplete_out(i, Svc::SendFileResponse(resp, this->m_curEntry.context));

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter resp has not been checked.
Svc/FileDownlink/FileDownlink.cpp
sourceFilename,
destFilename
);
Os::File::Status status = this->m_file.open(sourceFilename, destFilename);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter sourceFilename has not been checked.
Svc/FileDownlink/FileDownlink.cpp
sourceFilename,
destFilename
);
Os::File::Status status = this->m_file.open(sourceFilename, destFilename);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter destFilename has not been checked.
Svc/FileDownlink/FileDownlink.cpp
// Check type is correct
FW_ASSERT(type < COUNT_PACKET_TYPE && type >= 0, static_cast<FwAssertArgType>(type));
// Wrap the buffer around our indexed memory.
buffer.setData(this->m_memoryStore[type]);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buffer has not been checked.
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 16, 2025
View reviewed changes
Svc/FileDownlink/FileDownlink.cpp
U32 fileQueueDepth
)
{
m_cntxId(0) {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Svc/FileDownlink/FileDownlink.cpp
return;
}

FileDownlink ::~FileDownlink() {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
@thomas-bc thomas-bc merged commit 2d3e09b into nasa:devel Apr 16, 2025
44 checks passed
@thomas-bc thomas-bc deleted the file-up-down-token branch April 16, 2025 17:23
@thomas-bc thomas-bc mentioned this pull request Apr 16, 2025
Merged
@thomas-bc thomas-bc added the Breaking Changes / Needs Release Notes Need to add instructions in the release notes for updates. label Apr 16, 2025
@thomas-bc
Copy link
Collaborator Author

thomas-bc commented Apr 16, 2025

Adding breaking change in that if a user were to do unexpected things with buffers coming out of FileDownlink/FileUplink, things would break. The common use cases do not break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LeStarch LeStarch LeStarch approved these changes

Assignees

No one assigned

Labels

Breaking Changes / Needs Release Notes Need to add instructions in the release notes for updates.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add packet type serialization in/out of FileDownlink/FileUplink

2 participants

@thomas-bc @LeStarch