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

List subscribed topics when running topic list #379

Merged
merged 6 commits into from
Feb 8, 2023
Merged

Conversation

caguero
Copy link
Collaborator

@caguero caguero commented Jan 20, 2023

🎉 New feature

Requires gazebosim/gz-msgs#322

Closes #263

Summary

This patch includes the subscribed topics in the output of gz topic -l.

Test it

In one terminal run:

gz topic -e -t /foo

In another terminal run:

gz topic -l

Your output should be:

/foo

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Carlos Agüero <caguero@openrobotics.org>
Signed-off-by: Carlos Agüero <caguero@openrobotics.org>
@caguero caguero requested a review from nkoenig January 20, 2023 18:16
@caguero
Copy link
Collaborator Author

caguero commented Jan 20, 2023

@j-rivero , this PR requires msgs10. Is it possible to setup a msgs10 nightly release to make CI happy?

@caguero caguero added the needs upstream release Blocked by a release of an upstream library label Jan 20, 2023
Signed-off-by: Carlos Agüero <caguero@openrobotics.org>
@@ -218,6 +218,10 @@ namespace gz
/// \param[in] _pub Information of the remote subscriber.
public: void OnEndRegistration(const MessagePublisher &_pub);

/// \brief Callback executed when a SUBSCRIBERS request is received.
/// \param[in] _pub Request.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// \param[in] _pub Request.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in a1af664

Signed-off-by: Carlos Agüero <caguero@openrobotics.org>
Signed-off-by: Carlos Agüero <caguero@openrobotics.org>
@codecov
Copy link

codecov bot commented Feb 3, 2023

Codecov Report

Merging #379 (a1af664) into main (c89f55e) will increase coverage by 0.14%.
The diff coverage is 98.64%.

@@            Coverage Diff             @@
##             main     #379      +/-   ##
==========================================
+ Coverage   87.44%   87.58%   +0.14%     
==========================================
  Files          60       60              
  Lines        5177     5236      +59     
==========================================
+ Hits         4527     4586      +59     
  Misses        650      650              
Impacted Files Coverage Δ
src/Publisher.cc 99.36% <94.11%> (-0.64%) ⬇️
include/gz/transport/Discovery.hh 84.74% <100.00%> (+1.11%) ⬆️
include/gz/transport/HandlerStorage.hh 100.00% <100.00%> (ø)
include/gz/transport/TopicStorage.hh 100.00% <100.00%> (ø)
src/NodeShared.cc 78.34% <100.00%> (+0.58%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Jaeyoung-Lim
Copy link

@caguero Thanks for adding this feature, this was a great pain point when trying to develop on gazebo

Signed-off-by: Carlos Agüero <caguero@openrobotics.org>
@caguero
Copy link
Collaborator Author

caguero commented Feb 5, 2023

@j-rivero , this PR requires msgs10. Is it possible to setup a msgs10 nightly release to make CI happy?

@j-rivero , this PR requires msgs10. Is it possible to setup a msgs10 nightly release to make CI happy?

Nightlies available: https://packages.osrfoundation.org/gazebo/ubuntu-nightly/pool/main/g/gz-msgs10/

@caguero
Copy link
Collaborator Author

caguero commented Feb 7, 2023

CI is happy now, good to review @ahcorde!

@caguero caguero removed the needs upstream release Blocked by a release of an upstream library label Feb 7, 2023
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

LGTM, just a quick question: Is there any chance to backport this to Garden or lower versions ?

@caguero
Copy link
Collaborator Author

caguero commented Feb 8, 2023

LGTM, just a quick question: Is there any chance to backport this to Garden or lower versions ?

Not easily because we're breaking ABI here and in gz-msgs. That's why this is targeting Harmonic.

@caguero caguero merged commit 4bbf528 into main Feb 8, 2023
@caguero caguero deleted the topic_list_subs_main branch February 8, 2023 14:48
@iche033 iche033 added the 🎵 harmonic Gazebo Harmonic label Feb 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants