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 functionalities for optical tactile plugin #431

Merged
merged 27 commits into from
May 3, 2021

Conversation

mcres
Copy link
Contributor

@mcres mcres commented Oct 28, 2020

This PR is a change of target from this PR in my fork.

This PR adds the following:

  • Service for enabling/disabling the plugin
  • Topic for the normal forces computed
  • Test
  • Visualize the contacts from the contact sensor

Signed-off-by: Martiño Crespo <[email protected]>
Signed-off-by: Martiño Crespo <[email protected]>
Signed-off-by: Martiño Crespo <[email protected]>
Signed-off-by: Martiño Crespo <[email protected]>
Signed-off-by: Martiño Crespo <[email protected]>
@mcres
Copy link
Contributor Author

mcres commented Oct 28, 2020

CC @mabelzhang

@mabelzhang mabelzhang self-assigned this Oct 28, 2020
@mabelzhang
Copy link
Contributor

mabelzhang commented Oct 28, 2020

Will review after #229 is merged (it's very close), then will update feature branch to reduce the diffs here.

@chapulina chapulina added the 🔮 dome Ignition Dome label Oct 28, 2020
@chapulina
Copy link
Contributor

#229 is in, moving this to "In Review"!

@chapulina
Copy link
Contributor

then will update feature branch to reduce the diffs here.

I just noticed that this PR isn't targeting ign-gazebo4. Can it be retargeted and rebased, @mabelzhang ?

@chapulina chapulina changed the base branch from optical_tactile_plugin to ign-gazebo4 January 13, 2021 21:37
Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina added the tests Broken or missing tests / testing infra label Jan 13, 2021
Copy link
Contributor

@mabelzhang mabelzhang left a comment

Choose a reason for hiding this comment

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

I reviewed most of the files and pushed some changes, mostly for style.
Only file I still need to review is the tests.

@mabelzhang
Copy link
Contributor

I'm saving the Mac CI error message from the new test, in case the link dies before I come back to fix it.
Might need to add some directives for Mac.

149: [ RUN      ] OpticalTactilePluginTest.ForcesOnPlane
149: [Err] [Ogre2RenderEngine.cc:436] Unable to find Ogre Plugin[/usr/local/opt/ogre2.1/lib/OGRE-2.1/RenderSystem_GL3Plus]. Rendering will not be possible.Make sure you have installed OGRE properly.
149: [Err] [Ogre2RenderEngine.cc:739]  Unable to create the rendering window

@chapulina
Copy link
Contributor

I pushed some tweaks in 50f4250. This is working well for me! We just need to check the macOS tests. They're currently failing for a different reason, which will be fixed by #792 .

@chapulina
Copy link
Contributor

The macOS tests are failing to load Ogre just like in gazebosim/gz-sensors#66 and gazebosim/gz-gui#145, so I disabled the test for macOS on 4ffad4d

Copy link
Contributor

@mabelzhang mabelzhang left a comment

Choose a reason for hiding this comment

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

Approving because Mac failing tests were my only concern

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.

Good to go! 🚀

@chapulina chapulina merged commit 064b3ec into gazebosim:ign-gazebo4 May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome tests Broken or missing tests / testing infra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants