Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Split Deframer into FrameAccumulator Deframer and Router #3250

Open
wants to merge 50 commits into
base: devel
Choose a base branch
from
Open

Split Deframer into FrameAccumulator Deframer and Router #3250

wants to merge 50 commits into from

Conversation

thomas-bc
Copy link
Collaborator

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

Change Description

(GitHub doesn't let me re-open #2900 for some reason..., so re-opening here)

Implements #2763

Split up Deframer into 3 components:

  • FrameAccumulator
  • FprimeDeframer
  • Router

Rationale

Better reusability and chaining of deframers, preliminary work for CCSDS integrations

TODO

!!! UPDATE fprime-util new --deployment

To discuss

thomas-bc and others added 30 commits June 7, 2024 14:11
@thomas-bc
Split Router out of Deframer - initial implementation
7b65b2e
@thomas-bc
Rename variable to packetBuffer and resolve conversion warnings
4983004
@thomas-bc
Add test cases for each incoming packet type
01e62e4
@thomas-bc
Adjust comments and stuff
450a78a
@thomas-bc
Update Ref topology with Router
3774af5
@LeStarch @thomas-bc
WIP - FrameAccumulator
982db63
@thomas-bc
Add lightweight deframer
e5f1ba2
@LeStarch @thomas-bc
Working frame detector, accumulator, and fprime packet
2192847
@LeStarch @thomas-bc
CCSDS implementation and better checksums
728a2a3
@LeStarch @thomas-bc
Fixing overly small start word to use SCID as well
cce1aeb
@LeStarch @thomas-bc
Fixing frame accumulator for new structure
7c9486a
@LeStarch @thomas-bc
Minor cleanup to frame detectors
88eba2b
@thomas-bc
Add CCSDS Deframers and F´ minimal deframer
3d6852e
@thomas-bc
Clean up comments
7706122
@thomas-bc
Update Ref topology with Accumulator/Deframer/Router
c7db8e4
@thomas-bc
Update buffer manager
251caa9
@thomas-bc
Merge latest devel to get latest changes
1acb643
@thomas-bc
Upgrade Router.cpp to latest Fw::Logger
44ad2ae
@thomas-bc
Update RPI deployment to use FrameAccumulator/Router
4b17736
@thomas-bc
Remove CCSDS Deframers
e6458d3
@thomas-bc
Merge branch 'devel' into preliminary-ccsds-work
0da8df1
@thomas-bc
Merge branch 'devel' into preliminary-ccsds-work
6538397
@thomas-bc
Merge branch 'devel' into preliminary-ccsds-work
006f27e
@thomas-bc
Merge branch 'devel' into preliminary-ccsds-work
bcde1ee
@thomas-bc
Add first pass at UTs
44f4292
@thomas-bc
add buffer accumulation testing
13f8cd1
@thomas-bc
start SDD and rename author
e2b5426
@thomas-bc
Fix static analysis warnings and add comments to UTs
5c22baf
@thomas-bc
Spelling
82fcd9b
@thomas-bc
Add FprimeFrameDetector UTs
9007b8e
@thomas-bc thomas-bc added the Update Instructions Needed Need to add instructions in the release notes for updates. label Feb 20, 2025
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 20, 2025
View reviewed changes
Svc/FprimeDeframer/FprimeDeframer.cpp
// Component construction and destruction
// ----------------------------------------------------------------------

FprimeDeframer ::FprimeDeframer(const char* const compName) : FprimeDeframerComponentBase(compName) {}

Check notice

Code scanning / CodeQL

Use of basic integral type Note

compName uses the basic integral type char rather than a typedef with size and signedness.
Svc/FprimeDeframer/FprimeDeframer.cpp
// Handler implementations for user-defined typed input ports
// ----------------------------------------------------------------------

void FprimeDeframer ::framedIn_handler(FwIndexType portNum, Fw::Buffer& data, Fw::Buffer& context) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

portNum uses the basic integral type int rather than a typedef with size and signedness.
Svc/FrameAccumulator/FrameAccumulator.cpp
// ----------------------------------------------------------------------
// Component construction and destruction
// ----------------------------------------------------------------------

Check notice

Code scanning / CodeQL

Use of basic integral type Note

compName uses the basic integral type char rather than a typedef with size and signedness.
Svc/FrameAccumulator/FrameAccumulator.cpp
this->m_memory = nullptr;
}
}

Check notice

Code scanning / CodeQL

Use of basic integral type Note

allocationId uses the basic integral type unsigned int rather than a typedef with size and signedness.
Svc/FrameAccumulator/FrameAccumulator.cpp
}

void FrameAccumulator ::configure(const FrameDetector& detector, NATIVE_UINT_TYPE allocationId,
Fw::MemAllocator& allocator,

Check notice

Code scanning / CodeQL

Use of basic integral type Note

store_size uses the basic integral type unsigned long rather than a typedef with size and signedness.
Svc/FrameAccumulator/FrameDetector/FprimeFrameDetector.cpp
namespace Svc {
namespace FrameDetectors {

FrameDetector::Status FprimeFrameDetector::detect(const Types::CircularBuffer& data, FwSizeType& size_out) const {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

size_out uses the basic integral type unsigned long rather than a typedef with size and signedness.
Svc/Router/Router.cpp
// Component construction and destruction
// ----------------------------------------------------------------------

Router ::Router(const char* const compName) : RouterComponentBase(compName) {}

Check notice

Code scanning / CodeQL

Use of basic integral type Note

compName uses the basic integral type char rather than a typedef with size and signedness.
Svc/Router/Router.cpp
// Handler implementations for user-defined typed input ports
// ----------------------------------------------------------------------

void Router ::dataIn_handler(NATIVE_INT_TYPE portNum, Fw::Buffer& packetBuffer, Fw::Buffer& contextBuffer) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

portNum uses the basic integral type int rather than a typedef with size and signedness.
Svc/Router/Router.cpp
}

// Whether to deallocate the packet buffer
bool deallocate = true;

Check notice

Code scanning / CodeQL

Use of basic integral type Note

deallocate uses the basic integral type bool rather than a typedef with size and signedness.
Svc/Router/Router.cpp
}
}

void Router ::cmdResponseIn_handler(NATIVE_INT_TYPE portNum,

Check notice

Code scanning / CodeQL

Use of basic integral type Note

portNum uses the basic integral type int rather than a typedef with size and signedness.
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 20, 2025
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 20, 2025
View reviewed changes
Svc/FprimeDeframer/FprimeDeframer.cpp
// Component construction and destruction
// ----------------------------------------------------------------------

FprimeDeframer ::FprimeDeframer(const char* const compName) : FprimeDeframerComponentBase(compName) {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Svc/FprimeDeframer/FprimeDeframer.cpp

FprimeDeframer ::FprimeDeframer(const char* const compName) : FprimeDeframerComponentBase(compName) {}

FprimeDeframer ::~FprimeDeframer() {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Svc/FrameAccumulator/FrameAccumulator.cpp
// Component construction and destruction
// ----------------------------------------------------------------------

FrameAccumulator ::FrameAccumulator(const char* const compName) : FrameAccumulatorComponentBase(compName),

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Svc/FrameAccumulator/FrameAccumulator.cpp
this->m_memory = nullptr;
}
}

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Svc/FrameAccumulator/FrameAccumulator.cpp
}
// Deallocate the buffer
this->bufferDeallocate_out(0, buffer);
}

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Svc/FrameAccumulator/FrameDetector/FprimeFrameDetector.cpp
namespace Svc {
namespace FrameDetectors {

FrameDetector::Status FprimeFrameDetector::detect(const Types::CircularBuffer& data, FwSizeType& size_out) const {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Svc/Router/Router.cpp
// Component construction and destruction
// ----------------------------------------------------------------------

Router ::Router(const char* const compName) : RouterComponentBase(compName) {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Svc/Router/Router.cpp

Router ::Router(const char* const compName) : RouterComponentBase(compName) {}

Router ::~Router() {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Svc/Router/Router.cpp
// Handler implementations for user-defined typed input ports
// ----------------------------------------------------------------------

void Router ::dataIn_handler(NATIVE_INT_TYPE portNum, Fw::Buffer& packetBuffer, Fw::Buffer& contextBuffer) {

Check notice

Code scanning / CodeQL

Function too long Note

dataIn_handler has too many lines (64, while 60 are allowed).
Svc/Router/Router.cpp
// Handler implementations for user-defined typed input ports
// ----------------------------------------------------------------------

void Router ::dataIn_handler(NATIVE_INT_TYPE portNum, Fw::Buffer& packetBuffer, Fw::Buffer& contextBuffer) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 20, 2025
View reviewed changes
Svc/FprimeDeframer/FprimeDeframer.cpp
data.setData(data.getData() + FprimeProtocol::FrameHeader::SERIALIZED_SIZE);
data.setSize(data.getSize() - FprimeProtocol::FrameHeader::SERIALIZED_SIZE - FprimeProtocol::FrameTrailer::SERIALIZED_SIZE);

this->deframedOut_out(0, data, context);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter context has not been checked.
Svc/FrameAccumulator/FrameAccumulator.cpp
bool recoverable = false;
FW_ASSERT(std::numeric_limits<NATIVE_INT_TYPE>::max() >= store_size, static_cast<FwAssertArgType>(store_size));
NATIVE_UINT_TYPE store_size_int = static_cast<NATIVE_UINT_TYPE>(store_size);
void* data_void = allocator.allocate(allocationId, store_size_int, recoverable);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter allocator has not been checked.
Svc/FrameAccumulator/FrameAccumulator.cpp
bool recoverable = false;
FW_ASSERT(std::numeric_limits<NATIVE_INT_TYPE>::max() >= store_size, static_cast<FwAssertArgType>(store_size));
NATIVE_UINT_TYPE store_size_int = static_cast<NATIVE_UINT_TYPE>(store_size);
void* data_void = allocator.allocate(allocationId, store_size_int, recoverable);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter allocationId has not been checked.
Svc/FrameAccumulator/FrameAccumulator.cpp
U8* data = new(data_void) U8[store_size_int];
FW_ASSERT(data != nullptr);
FW_ASSERT(store_size_int >= store_size);
m_inRing.setup(data, store_size_int);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter detector has not been checked.
Svc/FrameAccumulator/FrameAccumulator.cpp
// Check whether there is data to process
if (status.e == Drv::RecvStatus::RECV_OK) {
// There is: process the data
this->processBuffer(buffer);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buffer has not been checked.
Svc/FrameAccumulator/FrameDetector/FprimeFrameDetector.cpp
}
// If the deserialized length token can not fit in current allocated size -> MORE_DATA_NEEDED
if (data.get_allocated_size() < FprimeProtocol::FrameHeader::SERIALIZED_SIZE + header.getlength() + FprimeProtocol::FrameTrailer::SERIALIZED_SIZE) {
size_out = header.getlength() + FprimeProtocol::FrameHeader::SERIALIZED_SIZE + FprimeProtocol::FrameTrailer::SERIALIZED_SIZE;

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter size_out has not been checked.
Svc/FrameAccumulator/FrameDetector/FprimeFrameDetector.cpp
size_out = header.getlength() + FprimeProtocol::FrameHeader::SERIALIZED_SIZE + FprimeProtocol::FrameTrailer::SERIALIZED_SIZE;
return Status::FRAME_DETECTED;

}

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter size_out has not been checked.
Svc/Router/Router.cpp
FwPacketDescriptorType packetType = Fw::ComPacket::FW_PACKET_UNKNOWN;
Fw::SerializeStatus status = Fw::FW_SERIALIZE_OK;
{
Fw::SerializeBufferBase& serial = packetBuffer.getSerializeRepr();

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter packetBuffer has not been checked.
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 20, 2025
View reviewed changes
Svc/FrameAccumulator/FrameDetector/FprimeFrameDetector.cpp
Utils::HashBuffer hashBuffer;

// Compute CRC over the transmitted data (header + body)
FwSizeType hash_field_size = header.getlength() + FprimeProtocol::FrameHeader::SERIALIZED_SIZE;

Check notice

Code scanning / CodeQL

Use of basic integral type Note

hash_field_size uses the basic integral type unsigned long rather than a typedef with size and signedness.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Update Instructions Needed 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.
2 participants
@thomas-bc @LeStarch