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

Send ROS_DISTRO to clients via metadata field #134

Merged
merged 1 commit into from
Jan 13, 2023

Conversation

achim-k
Copy link
Collaborator

@achim-k achim-k commented Jan 12, 2023

Public-Facing Changes

  • Send current $ROS_DISTRO to clients via serverInfo's metadata field

Description

Blocked by foxglove/ws-protocol#334

@@ -194,6 +194,7 @@ class Server final : public ServerInterface {
explicit Server(std::string name, LogCallback logger,
const std::vector<std::string>& capabilities,
const std::vector<std::string>& supportedEncodings = {},
const std::unordered_map<std::string, std::string>& metadata = {},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For simplicity, I decided to make this a map of string-string pairs. When e.g. using std::variant, we would have to provide json conversions for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I made an indifferent comment on the spec PR, but after seeing this I think we should change the spec to <string, string>

@achim-k achim-k merged commit 08e35d0 into main Jan 13, 2023
@achim-k achim-k deleted the achim/metadata_ros_distro branch January 13, 2023 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants