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

gazebo10 -> gazebo11 forward port #2816

Merged
merged 86 commits into from
Aug 12, 2020
Merged

gazebo10 -> gazebo11 forward port #2816

merged 86 commits into from
Aug 12, 2020

Conversation

j-rivero
Copy link
Contributor

@j-rivero j-rivero commented Aug 6, 2020

No description provided.

ahcorde and others added 8 commits July 13, 2020 10:42
Signed-off-by: ahcorde <ahcorde@gmail.com>
* fix sensor manager max update rate after spawning new sensors

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* changelog

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* revert changes in Visual.cc

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* revert changes

Signed-off-by: Ian Chen <ichen@osrfoundation.org>
* update changelog
* bump to 9.13.2

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Co-authored-by: Steve Peters <scpeters@openrobotics.org>
* Added test to check collisions equal to zero

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Included feedback

Signed-off-by: ahcorde <ahcorde@gmail.com>

* make linters happy

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Update Visual_TEST.cc
plugins/CMakeLists.txt Outdated Show resolved Hide resolved
@chapulina chapulina added the 11 Gazebo 11 label Aug 7, 2020
@j-rivero
Copy link
Contributor Author

The ABI report message:

DepthCamera.hh
namespace gazebo::rendering
[+] DepthCamera::CreateNormalsTexture ( std::__cxx11::string const& _textureName )  

| Change | Effect
| -- | --
1st parameter _textureName has been renamed to _normalsName. | No effect.

Does not seem like a problem to me.

…ers (#2809)

Signed-off-by: Alejandro Hernández <ahcorde@gmail.com>

Co-authored-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Co-authored-by: Louise Poubel <louise@openrobotics.org>
Co-authored-by: Steve Peters <scpeters@openrobotics.org>
@j-rivero
Copy link
Contributor Author

Forward port of #2809 to solve problems on compilation on Brew.

@iche033
Copy link
Contributor

iche033 commented Aug 11, 2020

looks good to me. Waiting for results from jenkins build

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.

Just noticed a couple of things

gazebo/rendering/Visual_TEST.cc Outdated Show resolved Hide resolved
test/integration/joint_spawn.cc Outdated Show resolved Hide resolved
@scpeters
Copy link
Member

The ABI report message:

DepthCamera.hh
namespace gazebo::rendering
[+] DepthCamera::CreateNormalsTexture ( std::__cxx11::string const& _textureName )  

| Change | Effect
| -- | --
1st parameter _textureName has been renamed to _normalsName. | No effect.

Does not seem like a problem to me.

the DepthCameraPlugin v-table has also changed, which is a more significant problem, though for a plugin

it also complained about this in the 9 -> 10 forward port: #2796 (comment)

@chapulina
Copy link
Contributor

the DepthCameraPlugin v-table has also changed, which is a more significant problem, though for a plugin

I suspect that was considered acceptable when merging this PR into Gazebo 9, although I didn't find any explicit discussion about it.

OnNewReflectanceFrame is pushing down OnNewNormalsFrame, but both functions have been recently added and haven't been released into Gazebo 11.0.0, see:

https://github.com/osrf/gazebo/blob/gazebo11_11.0.0/plugins/DepthCameraPlugin.hh

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@scpeters
Copy link
Member

the DepthCameraPlugin v-table has also changed, which is a more significant problem, though for a plugin

I suspect that was considered acceptable when merging this PR into Gazebo 9, although I didn't find any explicit discussion about it.

OnNewReflectanceFrame is pushing down OnNewNormalsFrame, but both functions have been recently added and haven't been released into Gazebo 11.0.0, see:

https://github.com/osrf/gazebo/blob/gazebo11_11.0.0/plugins/DepthCameraPlugin.hh

I think we'll just have to rebuild gazebo_plugins:

@chapulina
Copy link
Contributor

I think we'll just have to rebuild gazebo_plugins:

The ABI report says "Call of any virtual method at higher position in this class or its subclasses may result in crash or incorrect behavior of applications."

But no higher virtual methods have ever been released. So I don't think this should break anyone already compiling against Gazebo 11, right?

@iche033
Copy link
Contributor

iche033 commented Aug 11, 2020

I don't think we've released a new gazebo 11 minor version with the OnNewNormalsFrame function right? So it's position relative to OnNewReflectanceFrame should be fine as long as the two are at the bottom of the vtable.

@scpeters
Copy link
Member

I don't think we've released a new gazebo 11 minor version with the OnNewNormalsFrame function right? So it's position relative to OnNewReflectanceFrame should be fine as long as the two are at the bottom of the vtable.

ok, thanks, I think that makes it ok.

@j-rivero
Copy link
Contributor Author

All the CI looks good to me. Ready to go.

@j-rivero j-rivero merged commit f59bc6f into gazebo11 Aug 12, 2020
@j-rivero j-rivero deleted the gazebo10_to_gazebo11 branch August 12, 2020 11:15
@scpeters scpeters mentioned this pull request Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
11 Gazebo 11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants