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

ControlBoardDriver: support use of IJointCoupling devices in gazebo_yarp_controlboard and drop support for hardcoded HandMK5 coupling #671

Merged
merged 12 commits into from
Dec 21, 2023

Conversation

Nicogene
Copy link
Member

@Nicogene Nicogene commented Dec 13, 2023

This PR wants to refactor the handMk5 coupling handler in order to use the couplingXCubHandMK5 device:

And the IJointCoupling interface

Moreover it simplify the handling of the coupling, the coupling hadler used to be a vector of handlers supporting more coupling per part, but since none is using this feature I changed the code in order to use only one coupling per part.

It fixes #470

@Nicogene Nicogene self-assigned this Dec 18, 2023
@Nicogene Nicogene marked this pull request as ready for review December 18, 2023 11:09
@Nicogene
Copy link
Member Author

Nicogene commented Dec 18, 2023

I tested these changes both with the robots that uses the "old" pipeline (e.g. iCubGazebo2_5_visuomanip) and ergoCubGazeboV1* and it works fine.

The only problem is that these changes requires YARP 3.9.0, then all the conda-based CI are broken, how can we proceed in this sense @traversaro ?

@traversaro
Copy link
Member

The only problem is that these changes requires YARP 3.9.0, then all the conda-based CI are broken, how can we proceed in this sense @traversaro ?

I checked, the use of yarp 3.9 in stable branches is blocked by robotology/robotology-superbuild#1548, basically the fact that icub-main master does not compile with yarp 3.9, and only icub-main devel compiles with yarp 3.9, so we can't use yarp 3.9 in stable branches of the superbuild at the moment. Can we quickly align you, me and @pattacini to understand how to handle this?

@pattacini
Copy link
Member

icub-main@master is now aligned with icub-main@devel.

@pattacini
Copy link
Member

icub-main@master is now aligned with icub-main@devel.

Actually, this is not doable because of the icub-firmware-* deps.
icub-main@master reset to its previous commit.

@traversaro will apply a dedicated patch.

@traversaro traversaro closed this Dec 20, 2023
@traversaro traversaro reopened this Dec 20, 2023
@traversaro
Copy link
Member

YARP 3.9 is now available with LatestReleases or Stable in the superbuild, and has been released on conda-forge:

@Nicogene
Copy link
Member Author

The only problem is that these changes requires YARP 3.9.0, then all the conda-based CI are broken, how can we proceed in this sense @traversaro ?

The CI is now fine!

Copy link
Member

@traversaro traversaro left a comment

Choose a reason for hiding this comment

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

Minor comments, otherwise it seems ok. Can you write something on this in the CHANGELOG? Thanks!

@traversaro
Copy link
Member

cc @mebbaid

@traversaro
Copy link
Member

I think we can add "Fix #470" in the PR body. :)

@Nicogene Nicogene requested a review from traversaro December 20, 2023 12:57
CHANGELOG.md Outdated Show resolved Hide resolved
@traversaro
Copy link
Member

Unless @xEnVrE has some comments, for me this is ready to merge @Nicogene . Given that there is a strictly coupling between this and the ergocub-software PR, how do you want to proceed @Nicogene ?

Co-authored-by: Silvio Traversaro <[email protected]>
@traversaro
Copy link
Member

@Nicogene can I merge with squash? Furthermore, are you ready to merge icub-tech-iit/ergocub-software#178? I would merge (and eventually release, if we are ready to do so) both at the same time, to avoid breaking the superbuild Stable and Unstable branches.

@Nicogene
Copy link
Member Author

Nicogene commented Dec 21, 2023

@Nicogene can I merge with squash?

Yes sure!

Furthermore, are you ready to merge icub-tech-iit/ergocub-software#178?

Yes I was awaiting to merge both PR togheter too

how do you want to proceed @Nicogene ?

I will put in the ergocub-software README the required version once we release GYP

@traversaro traversaro changed the title ControlBoardDriver: start using IJointCoupling for handMk5 ControlBoardDriver: support use of IJointCoupling devices in gazebo_yarp_controlboard and drop support for hardcoded HandMK5 coupling Dec 21, 2023
@traversaro traversaro merged commit c892802 into robotology:master Dec 21, 2023
5 checks passed
@Nicogene Nicogene deleted the refactorHandMk5CouplingHandler branch December 21, 2023 10:55
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.

Move gazebo_yarp_controlboard coupling logic to dynamically loaded plugins
3 participants