-
Notifications
You must be signed in to change notification settings - Fork 13
updated to ZenohUTranspoart, updated signature, fixed dependencies and added clang-format.sh #28
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
updated to ZenohUTranspoart, updated signature, fixed dependencies and added clang-format.sh #28
Conversation
PLeVasseur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @MaximilianToe -- thanks for updating this! Left a few comments and questions.
| void notifyListener(const v1::UMessage& message); | ||
| void listenThread(); // listen for incoming messages (thread) | ||
| void cleanupListener(CallableConn listener) override {} | ||
| void cleanupListener(const CallableConn& listener) override {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this file needed anymore, now that we're using the Zenoh transport? 🤔 I'm in favor of mercilessly cutting out code to make a more maintainable codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MaximilianToe and I chatted.
Plan is:
- merge this
- he'll open another PR tackling
pubsub - can then remove all traces of the socket transport from this repo
Lemme know if I got anything mixed Max!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good!
| * SPDX-FileCopyrightText: 2024 General Motors GTO LLC | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| #include <spdlog/spdlog.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a client and server for rpc. Am I missing where the source is for the publisher and subscriber in pubsub?
PLeVasseur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick turn-around and flexibility @MaximilianToe!
| void notifyListener(const v1::UMessage& message); | ||
| void listenThread(); // listen for incoming messages (thread) | ||
| void cleanupListener(CallableConn listener) override {} | ||
| void cleanupListener(const CallableConn& listener) override {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MaximilianToe and I chatted.
Plan is:
- merge this
- he'll open another PR tackling
pubsub - can then remove all traces of the socket transport from this repo
Lemme know if I got anything mixed Max!
This PR targets the following issues:
Closes #23 #24 #25 #27