robotology / walking-teleoperation

Software related to walking and teleoperation.
BSD 3-Clause "New" or "Revised" License
30 stars 14 forks source link

updated name of the YARP port from HDE #135

Closed davidegorbani closed 11 months ago

davidegorbani commented 11 months ago

After having merged https://github.com/robotology/human-dynamics-estimation/pull/367 it will be necessary to merge also this PR in order to use the yarpmanager applications since the name of the YARP ports exposed by the devices in HDE are changed.

lrapetti commented 11 months ago

PR in human-dynamics-estimation has been merged (see https://github.com/ami-iit/ifeel_utils/blob/main/software/iFeelHDE/CMakeLists.txt#L3) and we have tested the ergocub-teleoperation application using this branch of walking-teleoperation. The PR indeed is ready to be reviewed @davidegorbani @S-Dafarra @GiulioRomualdi

lrapetti commented 11 months ago

@S-Dafarra @GiulioRomualdi I don't know if there is, or if it would make sense to have, a minimum required version for the dependency, such that avoid installing non-compatible configuration files. In that case we might add

find_package(HumanDynamicsEstimation 3.0.0 REQUIRED).

after realeasing human-dynamics-estimation v3.0.0 (https://github.com/robotology/human-dynamics-estimation/issues/373)

lrapetti commented 11 months ago

cc @mebbaid

S-Dafarra commented 11 months ago

Thanks @davidegorbani @lrapetti. Can you please rebase on top of master. I have just merged https://github.com/robotology/walking-teleoperation/pull/133 where I deleted some old files (whose modifications is thus no more required :wink: )

Also, can you avoid changing the Xprize files? I prefer to keep them "stale" for the time being.

S-Dafarra commented 11 months ago

@S-Dafarra @GiulioRomualdi I don't know if there is, or if it would make sense to have, a minimum required version for the dependency, such that avoid installing non-compatible configuration files. In that case we might add

find_package(HumanDynamicsEstimation 3.0.0 REQUIRED).

after realeasing human-dynamics-estimation v3.0.0 (robotology/human-dynamics-estimation#373)

Good point, the dependency is specified in https://github.com/robotology/walking-teleoperation/blob/da230ab1177698cb2dc9032916b585a33deacc30/cmake/WalkingTeleoperationFindDependencies.cmake#L147-L148

lrapetti commented 11 months ago

@S-Dafarra @GiulioRomualdi I don't know if there is, or if it would make sense to have, a minimum required version for the dependency, such that avoid installing non-compatible configuration files. In that case we might add

find_package(HumanDynamicsEstimation 3.0.0 REQUIRED).

after realeasing human-dynamics-estimation v3.0.0 (robotology/human-dynamics-estimation#373)

Good point, the dependency is specified in

https://github.com/robotology/walking-teleoperation/blob/da230ab1177698cb2dc9032916b585a33deacc30/cmake/WalkingTeleoperationFindDependencies.cmake#L147-L148

I missed that, so we can:

lrapetti commented 11 months ago

I missed that, so we can:

  • add the dependency on v3.0.0. (even if its not out yet)
  • wait to merge it until release is done
  • not add the dependency, or add it later

As discussed at today meeting we will avoid adding the dependency.

davidegorbani commented 11 months ago

Hello @S-Dafarra, I rebased the branch and reverted the modification to the Xprise files. Should I do something else?

davidegorbani commented 11 months ago

Thank you @S-Dafarra !