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

Allow topic and service to construct messages from description files #428

Merged
merged 3 commits into from
Mar 11, 2024

Conversation

mjcarroll
Copy link
Contributor

🦟 Bug fix

The current implementation allows for additional messages to be read from these files, it was just not properly configured for the files to be accessible.

Summary

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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.

The current implementation allows for additional messages to be read from these files, it was just not properly configured for the files to be accessible.

Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll mjcarroll changed the title Allow topic and service to construct messages from description files. Allow topic and service to construct messages from description files Feb 27, 2024
@mjcarroll mjcarroll self-assigned this Feb 27, 2024
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Feb 27, 2024
@mjcarroll mjcarroll changed the base branch from gz-msgs10 to gz-msgs9 February 27, 2024 13:28
@mjcarroll
Copy link
Contributor Author

@osrf-jenkins retest this please

Copy link
Contributor

@shameekganguly shameekganguly left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Collaborator

@caguero caguero left a comment

Choose a reason for hiding this comment

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

How are you testing it? Is this example easily backportable?

https://gazebosim.org/api/msgs/10/messagegeneration.html

@mjcarroll
Copy link
Contributor Author

How are you testing it? Is this example easily backportable?

I can add a test. Since this is already targeting gz-msgs9 (garden), I don't think there is a reason to backport it any further?

@caguero
Copy link
Collaborator

caguero commented Feb 27, 2024

How are you testing it? Is this example easily backportable?

I can add a test. Since this is already targeting gz-msgs9 (garden), I don't think there is a reason to backport it any further?

I mean the example from gz-msgs10 to gz-msgs9. I think we could use this example to test it.

Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll
Copy link
Contributor Author

I think we could use this example to test it.

Added an INTEGRATION_descriptors test with a few more descriptors configurations to exercise this.

Copy link

codecov bot commented Mar 8, 2024

Codecov Report

Attention: Patch coverage is 95.83333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 96.16%. Comparing base (29cfe33) to head (fbe8627).

Files Patch % Lines
src/Factory.cc 95.83% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           gz-msgs9     #428      +/-   ##
============================================
+ Coverage     95.57%   96.16%   +0.59%     
============================================
  Files            10       10              
  Lines          1062     1069       +7     
============================================
+ Hits           1015     1028      +13     
+ Misses           47       41       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll mjcarroll requested a review from caguero March 8, 2024 20:33
@mjcarroll
Copy link
Contributor Author

@osrf-jenkins retest this please

@azeey azeey added 🌱 garden Ignition Garden and removed 🎵 harmonic Gazebo Harmonic labels Mar 11, 2024
@mjcarroll mjcarroll merged commit fd36d69 into gz-msgs9 Mar 11, 2024
14 checks passed
@mjcarroll mjcarroll deleted the mjcarroll/desc_files branch March 11, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants