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.

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

Add TCP Receiver #339

Merged
merged 4 commits into from
Jul 19, 2024
Merged

Add TCP Receiver #339

merged 4 commits into from
Jul 19, 2024

Conversation

jsouter
Copy link
Contributor

@jsouter jsouter commented Mar 7, 2024

Adds in the TCP rx thread required for merlin-detector.

Copy link
Contributor

@timcnicholls timcnicholls left a comment

Choose a reason for hiding this comment

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

Looks good! A few minor comments. It would be great if there was a dummy decoder plugin and integration test added for TCP receivers too 😉

cpp/frameReceiver/src/FrameReceiverTCPRxThread.cpp Outdated Show resolved Hide resolved
cpp/frameReceiver/src/FrameReceiverTCPRxThread.cpp Outdated Show resolved Hide resolved
@jsouter
Copy link
Contributor Author

jsouter commented Mar 25, 2024

Cheers Tim, I'll get to work on those changes

@jsouter
Copy link
Contributor Author

jsouter commented May 9, 2024

Looking at this again, have made the minor changes to error messages mentioned above and added in a remove_socket call in the TCPRxThread's cleanup_specific_service. Had some issues with getting the build to work when registering a new Dummy decoder class but now I have a DummyTCPFrameDecoder somewhat resembling the Merlin frame decoder. Will set about working on the actual unit tests soon.

clang-format on FrameReceiverTCPRxThread.cpp

fix date typo
@jsouter
Copy link
Contributor Author

jsouter commented Jul 15, 2024

Okay, I removed the *Lib.cpp files from the glob in the CMakeLists.txt as discussed and removed the namespacing of the registrations. I will note that because of recent changes to master using Lambda functions, we may need to add in a set(CMAKE_CXX_STANDARD 11) call somewhere in the CMake, which is absent from this PR.

@GDYendell GDYendell merged commit 0bb8b08 into odin-detector:master Jul 19, 2024
8 checks passed
@GDYendell
Copy link
Collaborator

Thanks @jsouter!

@jsouter jsouter deleted the tcp-receiver branch August 20, 2024 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants