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

Handling demo red ball in gazebo #32

Merged
merged 3 commits into from
Apr 21, 2021
Merged

Conversation

vvasco
Copy link

@vvasco vvasco commented Apr 20, 2021

This PR updates demoRedBall and pf3dTracker conf files to handle the red ball demo within gazebo.
More specifically:

pf3dTracker

The following files have been added:

  • red_ball_gazebo.bmp and redball-gazebo.csv model to deal with gazebo red ball's color and shape respectively;
  • pf3dTracker-gazebo.ini that loads the previous files.

demoRedBall

The following changes have been made:

  • the simulation flag allows to run the demo in simulation;

  • the respond method contains three new commands:

    • start: to start the demo;
    • stop: to stop the demo;
    • update_pose x y z: to update the ball position;

    Note: the behavior of the demo on the real robot has not changed and it automatically starts when running the module.

  • added a gazebo folder with the ball model and gazebo world;

Also, a demoRedBall_gazebo.xml.template including the required modules and the related conf files is available.
The demoRedBall documentation has also been updated.

Here is an example of the demo running in gazebo:

redball-gazebo.mp4

Finally, these changes have also been successfully tested on iCubErzelli02 to be sure that the demo on the real robot is not affected.

- added conf files for pf3dTracker
- added gazebo folder with ball model and gazebo world
- updated demoRedBall with simulation flag and start / stop / update_pose commands
- updated demoRedBall documentation
@vvasco vvasco requested review from pattacini and vtikha April 20, 2021 14:59
@vvasco vvasco self-assigned this Apr 20, 2021
Copy link
Member

@pattacini pattacini left a comment

Choose a reason for hiding this comment

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

Fantastic addition @vvasco 🚀

I've only a comment to make: can we consider updating the test introduced in robotology/icub-tests#24 as well?

Actually, that test was meant for checking at least part of the pipeline.
We should merge the pathways.

Is this addition meant for testing or is it a general feature?

@vvasco
Copy link
Author

vvasco commented Apr 20, 2021

I've had a quick look at the test and I think we can easily merge the simulation pathway.
The addition is meant to be a general feature, but it might be also useful for testing purposes.

@pattacini
Copy link
Member

pattacini commented Apr 20, 2021

I think we can easily merge the simulation pathway.

Thanks 👍🏻

Copy link
Member

@pattacini pattacini left a comment

Choose a reason for hiding this comment

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

CI is failing under Windows because GAZEBO is not found.
Perhaps, it'd be worth adding the CMake logic to deal with this condition.

@vvasco
Copy link
Author

vvasco commented Apr 20, 2021

The CMake logic is there but by default the GAZEBO_AVAILABLE option is set to ON. I can set this to be OFF instead by default.

@pattacini
Copy link
Member

The CMake logic is there but by default the GAZEBO_AVAILABLE option is set to ON. I can set this to be OFF instead by default.

Uhm, GAZEBO_AVAILABLE should be the result the check.

@pattacini
Copy link
Member

In a nutshell find_package(GAZEBO) shouldn't have the REQUIRED option enabled; rather, you check whether it is available by reading GAZEBO_FOUND as the result of the find_package macro.

Copy link
Member

@vtikha vtikha left a comment

Choose a reason for hiding this comment

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

LGTM 🎉🎉🎉

@pattacini
Copy link
Member

Waiting for the CI to complete and then I'll merge the PR!

Copy link
Member

@pattacini pattacini left a comment

Choose a reason for hiding this comment

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

Apparently, we do still have problems:

2021-04-20T16:22:55.1630643Z D:\a\icub-basic-demos\icub-basic-demos\demoRedBall\src\main.cpp(2048,18): error C2065: 'simulation': undeclared identifier [D:\a\icub-basic-demos\icub-basic-demos\build\demoRedBall\demoRedBall.vcxproj]

@vvasco
Copy link
Author

vvasco commented Apr 21, 2021

There was a leftover, I fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants