Skip to content

Conversation

@MaximilianToe
Copy link
Contributor

@MaximilianToe MaximilianToe commented Apr 30, 2025

This PR targets the following issues:

  • Replaces SocketUTransport with ZenohUTransport
  • update the signature of cleanupListener to make it compatible with the recent changes of up-cpp
  • update the dependencies after the changes in up-cpp
  • added clang-format.sh like in up-cpp

Closes #23 #24 #25 #27

Copy link
Contributor

@PLeVasseur PLeVasseur left a 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 {}
Copy link
Contributor

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.

Copy link
Contributor

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:

  1. merge this
  2. he'll open another PR tackling pubsub
  3. can then remove all traces of the socket transport from this repo

Lemme know if I got anything mixed Max!

Copy link
Contributor Author

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>
Copy link
Contributor

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?

Copy link
Contributor

@PLeVasseur PLeVasseur left a 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 {}
Copy link
Contributor

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:

  1. merge this
  2. he'll open another PR tackling pubsub
  3. can then remove all traces of the socket transport from this repo

Lemme know if I got anything mixed Max!

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.

rpc example using SocketUTransport fails with error

2 participants