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 stream crossover component #2070

Merged

LeStarch merged 7 commits into nasa:devel from ethancheez:StreamCrossover

Jun 10, 2023

Contributor

ethancheez commented Jun 5, 2023 •

edited

Loading


*Originating Project/Creator*	@ethancheez
*Affected Component*	StreamCrossover
*Affected Architectures(s)*	All
*Related Issue(s)*	N/A
*Has Unit Tests (y/n)*	No
*Builds Without Errors (y/n)*	Yes
*Unit Tests Pass (y/n)*	N/A
*Documentation Included (y/n)*	Yes

Change Description

Add stream crossover component as Drv::StreamCrossover

Rationale

Allow connection from ByteStreamRecv to ByteStreamSend

Testing/Review Recommendations

Implement unit tests.

ethancheez added 2 commits

May 31, 2023 11:57


          stream crossover component

8fa6837


          Add StreamCrossover Documentation

1618d52

ethancheez requested a review from LeStarch

June 5, 2023 16:34

github-advanced-security bot found potential problems

View reviewed changes

Drv/StreamCrossover/StreamCrossover.cpp Fixed Show resolved Hide resolved

Drv/StreamCrossover/StreamCrossover.cpp Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems

View reviewed changes

Drv/StreamCrossover/StreamCrossover.cpp Fixed Show fixed Hide fixed

Drv/StreamCrossover/StreamCrossover.cpp

+                StreamCrossover ::
+                  StreamCrossover(
+                      const char *const compName

Check notice

Code scanning / CodeQL

Use of basic integral type

compName uses the basic integral type char rather than a typedef with size and signedness.

Collaborator

timcanham commented Jun 5, 2023

LeStarch requested changes

View reviewed changes

Collaborator

LeStarch left a comment

We should do the following:

Fix this spelling error (add ethanechee to expect.txt)
Handle the send return value: emit an event.
Could we add a UT to test this?

Drv/StreamCrossover/StreamCrossover.cpp Fixed Show resolved Hide resolved


          Add unit tests, update expect.txt

fbd2474

github-advanced-security bot found potential problems

View reviewed changes

Drv/StreamCrossover/StreamCrossover.cpp

Comment on lines +35 to +36

		void StreamCrossover ::
		streamIn_handler(

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.

github-advanced-security bot found potential problems

View reviewed changes

Drv/StreamCrossover/StreamCrossover.cpp Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems

View reviewed changes

Drv/StreamCrossover/StreamCrossover.cpp Fixed Show fixed Hide fixed

ethancheez added 2 commits

June 6, 2023 13:23


          Add coverage for send error

a115cdf


          update test name from ToDo to TestBuffer

7dcfa42

LeStarch reviewed

View reviewed changes

Drv/StreamCrossover/StreamCrossover.cpp Show resolved Hide resolved

Drv/StreamCrossover/docs/sdd.md Outdated Show resolved Hide resolved

Drv/StreamCrossover/test/ut/Tester.cpp Outdated Show resolved Hide resolved


          Check recvStatus, and deallocate on error

1e7da3b

github-advanced-security bot found potential problems

View reviewed changes

Drv/StreamCrossover/StreamCrossover.cpp Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems

View reviewed changes

Drv/StreamCrossover/StreamCrossover.cpp

+                void StreamCrossover ::
+                  streamIn_handler(
+                      const NATIVE_INT_TYPE portNum,

Check notice

Code scanning / CodeQL

Use of basic integral type

portNum uses the basic integral type int rather than a typedef with size and signedness.

LeStarch approved these changes

View reviewed changes


          Check recvBuffer size, rename errorDeallocate

df046e0

github-advanced-security bot found potential problems

View reviewed changes

Drv/StreamCrossover/StreamCrossover.cpp

+                  if(recvStatus == Drv::RecvStatus::RECV_ERROR || recvBuffer.getSize() == 0)
+                  {
+                    this->log_WARNING_HI_StreamOutError(Drv::SendStatus::SEND_ERROR);
+                    this->errorDeallocate_out(0, recvBuffer);

Check warning

Code scanning / CodeQL

Unchecked function argument

This use of parameter recvBuffer has not been checked.

thomas-bc approved these changes

View reviewed changes

LeStarch approved these changes

View reviewed changes

LeStarch merged commit 415cb7f into nasa:devel

Boehm-Michael pushed a commit to Boehm-Michael/fprime that referenced this pull request


          Add stream crossover component (nasa#2070)

4a411e1

* stream crossover component

* Add StreamCrossover Documentation

* Add unit tests, update expect.txt

* Add coverage for send error

* update test name from ToDo to TestBuffer

* Check recvStatus, and deallocate on error

* Check recvBuffer size, rename errorDeallocate

thomas-bc pushed a commit that referenced this pull request


          Add stream crossover component (#2070)

7cca7cb

* stream crossover component

* Add StreamCrossover Documentation

* Add unit tests, update expect.txt

* Add coverage for send error

* update test name from ToDo to TestBuffer

* Check recvStatus, and deallocate on error

* Check recvBuffer size, rename errorDeallocate

thomas-bc added a commit that referenced this pull request


          Revert "Add stream crossover component (#2070)"

a0cacb6

This reverts commit 7cca7cb.

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

Labels

None yet