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

Add entity validation to OdometryPublisher #2326

Merged

Conversation

Bogdanov-am
Copy link

@Bogdanov-am Bogdanov-am commented Mar 1, 2024

🦟 Bug fix

Until SystemManager has the ability to unload system plugins, plugins require an explicit check of the validity of the entities used in the Update methods. Such a check was missing in OdometryPublisher, which led to non-critical but annoying errors in the console.

P.S. It's my first PR to open source project, sorry if i missing something

@Bogdanov-am Bogdanov-am requested a review from mjcarroll as a code owner March 1, 2024 19:48
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Mar 1, 2024
Copy link
Contributor

@azeey azeey 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 contribution! And welcome to open source development. I just have one comment.

Copy link

codecov bot commented Mar 1, 2024

Codecov Report

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

Project coverage is 65.95%. Comparing base (633ce72) to head (89a9a54).

❗ Current head 89a9a54 differs from pull request most recent head 50a9b4e. Consider uploading reports for the commit 50a9b4e to get more accurate results

Files Patch % Lines
...rc/systems/odometry_publisher/OdometryPublisher.cc 50.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           gz-sim8    #2326      +/-   ##
===========================================
- Coverage    65.97%   65.95%   -0.02%     
===========================================
  Files          327      327              
  Lines        31328    31332       +4     
===========================================
- Hits         20668    20665       -3     
- Misses       10660    10667       +7     

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

Signed-off-by: Anton Bogdanov <[email protected]>
@Bogdanov-am Bogdanov-am requested a review from azeey March 1, 2024 21:17
Signed-off-by: Anton Bogdanov <[email protected]>
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Thanks! If you're interested in contributing more, which we'd love, please take a look at these good first issues

@azeey azeey enabled auto-merge (squash) March 1, 2024 21:44
@azeey azeey merged commit 1349900 into gazebosim:gz-sim8 Mar 1, 2024
6 of 8 checks passed
@g-arjones
Copy link
Contributor

@azeey Any idea when this would be released?

@azeey
Copy link
Contributor

azeey commented Mar 11, 2024

We'll shoot for making a release this week.

@g-arjones
Copy link
Contributor

Awesome. Thanks for the heads up 👍

GauravKumar9920 pushed a commit to GauravKumar9920/gz-sim that referenced this pull request Mar 30, 2024
Until SystemManager has the ability to unload system plugins, plugins require an explicit check of the validity of the entities used in the Update methods. Such a check was missing in OdometryPublisher, which led to non-critical but annoying errors in the console.
---------

Signed-off-by: Anton Bogdanov <[email protected]>
Signed-off-by: Gaurav Kumar <[email protected]>
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.

3 participants