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

Fix for sensor pointer null when navsat plugin in included in sdf #2176

Merged
merged 5 commits into from
Oct 30, 2023
Merged

Fix for sensor pointer null when navsat plugin in included in sdf #2176

merged 5 commits into from
Oct 30, 2023

Conversation

ashBabu
Copy link
Contributor

@ashBabu ashBabu commented Sep 27, 2023

🦟 Bug fix

Fixes #2163

Summary

When the play button is pressed in Gazebo, the terminal window shows an error as described in #2163.

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.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

Test it

  • create a model.sdf with the following navsat plugin
<sensor name="navsat" type="navsat">
   <navsat>
        ....
   </navsat>
   <plugin  filename="libignition-gazebo-navsat-system"  
            name="ignition::gazebo::systems::NavSat">
   </plugin>
</sensor>
  • ign gazebo model.sdf
  • Press the play button
  • There wont be any error on the terminal

@ashBabu ashBabu requested a review from mjcarroll as a code owner September 27, 2023 21:40
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Sep 27, 2023
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

Minor formatting issues.

Can you also update the title of the PR?

src/Conversions.cc Outdated Show resolved Hide resolved
src/Conversions.cc Show resolved Hide resolved
src/Conversions.cc Outdated Show resolved Hide resolved
@ashBabu ashBabu changed the title Ign gazebo6 Fix for sensor pointer null when navsat plugin in included in sdf Sep 28, 2023
@ashBabu
Copy link
Contributor Author

ashBabu commented Sep 28, 2023

Minor formatting issues.

Can you also update the title of the PR?

done

@ashBabu
Copy link
Contributor Author

ashBabu commented Oct 2, 2023

@mjcarroll : I am not sure why some of the checks were unsuccessful. Is there something that I could do to get this approved? Thanks

@mjcarroll
Copy link
Contributor

I am not sure why some of the checks were unsuccessful.

I'm still seeing the code not wrapped at 80 characters, that may be triggering a lint failure.

I believe the other failures are related to #2190, so you won't need to fix them.

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

@osrf-jenkins retest this please

src/Conversions.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

just a minor typo otherwise looks good to me

Co-authored-by: Ian Chen <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll mjcarroll merged commit 28728c9 into gazebosim:ign-gazebo6 Oct 30, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants