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 laser_retro support #603

Merged
merged 7 commits into from
Feb 12, 2021

Conversation

doisyg
Copy link
Contributor

@doisyg doisyg commented Feb 2, 2021

As discussed here gazebosim/gz-rendering#181, this is the last PR needed for complete laser_retro support

Guillaume Doisy added 4 commits February 2, 2021 09:29
Signed-off-by: Guillaume Doisy <[email protected]>
Signed-off-by: Guillaume Doisy <[email protected]>
Signed-off-by: Guillaume Doisy <[email protected]>
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Feb 2, 2021
@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Feb 2, 2021
@chapulina chapulina added the close the gap Features from Gazebo-classic label Feb 2, 2021
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Thanks, this works for me! We just need an ign-rendering3 release before merging. Would it be possible to add an example world that exercises the feature? Just something simple with a box that has custom laser_retro values and a sensor that captures it, and some instructions up top for the user to check the data? Thanks!

src/SdfEntityCreator.cc Outdated Show resolved Hide resolved
@doisyg
Copy link
Contributor Author

doisyg commented Feb 9, 2021

. Would it be possible to add an example world that exercises the feature? Just something simple with a box that has custom laser_retro values and a sensor that captures it, and some instructions up top for the user to check the data?

Sure, maybe an example in ros_ign_gazebo_demos https:/ignitionrobotics/ros_ign/tree/ros2/ros_ign_gazebo_demos, what do you think ?

@chapulina
Copy link
Contributor

maybe an example in ros_ign_gazebo_demos

Yeah that works too! If adding one to this repository under examples/worlds isn't too much trouble, that's also a good place, since we've been putting many demos there.

doisyg and others added 2 commits February 10, 2021 10:04
Signed-off-by: Guillaume Doisy <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
@doisyg
Copy link
Contributor Author

doisyg commented Feb 10, 2021

If adding one to this repository under examples/worlds isn't too much trouble, that's also a good place, since we've been putting many demos there.

Done, hope it helps

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good to me! We've made an ign-rendering release.

Do you mind merging ign-gazebo3 into your branch so that the branch is up-to-date and CI is retriggered? It looks like I can't push to your branch.

Thanks!

@chapulina
Copy link
Contributor

Ouch, still missing an SDFormat release. I'm on it.

@chapulina
Copy link
Contributor

@osrf-jenkins run tests please

@doisyg
Copy link
Contributor Author

doisyg commented Mar 16, 2021

Hi @chapulina, would it be possible to do a release of ignition-gazebo3 in order to enjoy this PR from the standard binary installation? Thx!

@chapulina
Copy link
Contributor

would it be possible to do a release of ignition-gazebo3

Sure, I'll see if I can get #534 in tomorrow and then trigger a release 👍

@chapulina
Copy link
Contributor

@doisyg : #688

@doisyg
Copy link
Contributor Author

doisyg commented Mar 17, 2021

@doisyg : #688

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel close the gap Features from Gazebo-classic needs upstream release Blocked by a release of an upstream library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants