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

Add Router component in uplink #2766

Closed
wants to merge 5 commits into from
Closed

Add Router component in uplink #2766

wants to merge 5 commits into from

Conversation

thomas-bc
Copy link
Collaborator

@thomas-bc thomas-bc commented Jun 8, 2024 •
edited
Loading

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

Change Description

Implements #2765

Rationale

See #2765

Testing/Review Recommendations

Deframer UTs are expected to fail at this point, since some functionality was removed.
We may want to work on a feature branch until all Router, Accumulator and Deframer-refactor are implemented and tests are adapted

Still opening this for preliminary review and CI checks

Future Work

  • Build Accumulator and refactor the Deframer to only include deframing protocol
  • Document Accumulator/Deframer/Router pattern
  • Update Tutorials and reference repos

github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 8, 2024
View reviewed changes
Svc/Deframer/Deframer.cpp
}

// Send the buffer out
bufferOut_out(0, packetBuffer);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter packetBuffer 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 Jun 8, 2024
View reviewed changes
Svc/Router/Router.cpp
// Handler implementations for user-defined typed input ports
// ----------------------------------------------------------------------

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

Check notice

Code scanning / CodeQL

Function too long Note

bufferIn_handler has too many lines (64, while 60 are 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
// 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
// Handler implementations for user-defined typed input ports
// ----------------------------------------------------------------------

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

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 Jun 8, 2024
View reviewed changes
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.
Svc/Router/Router.cpp
// Handler implementations for user-defined typed input ports
// ----------------------------------------------------------------------

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

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
// 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.
@Joshua-Anderson
Copy link
Contributor

Joshua-Anderson commented Jun 10, 2024 •
edited
Loading

Router might be a bit of an overloaded name, since lots of things can be routed within fprime deployments.

What about calling this FprimePktRouter - indicating this is a component for routing fprime packet formatted packets? (admittingly a bit of a mouthful, very open to different suggestions)

@thomas-bc
Copy link
Collaborator Author

thomas-bc commented Jun 14, 2024 •
edited
Loading

Agreed! Router is a little too generic
Closing until we coordinate the splitting of Deframer

@thomas-bc thomas-bc closed this Jun 14, 2024
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
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@thomas-bc @Joshua-Anderson