robotology / human-dynamics-estimation

Software repository for estimating human dynamics
BSD 3-Clause "New" or "Revised" License
80 stars 28 forks source link

Implement a HumanDynamicsRemapper #294

Closed Zweisteine96 closed 2 years ago

Zweisteine96 commented 2 years ago

Hi @lrapetti in this issue we want to implement a HumanDynamicsRemapper.

lrapetti commented 2 years ago

Just to add a bit of extra information, the role of the Remapper YARP device in general is to expose the data coming from a thrifted yarp data port, trought a cpp interface. The HumanDynamicsRemapper will:

As an example, from which you can start, it's the HumanStateRemapper device which reads HumanState.thrift messages, exposing the IHumanState interface. As you can observe, looking at the HumanStateRemapper class header you can observe that:

Then in the device implementation (HumanStateRemapper.cpp), the relevant part are:

cc @RiccardoGrieco

Zweisteine96 commented 2 years ago

Hi @lrapetti, I updated the following files to this branch in HDE repo:

About the humanDynamicsRemapper, there remains one point that I'm not very sure, that is, how could I determine the humanDynamicsDataPort shown here: https://github.com/robotology/human-dynamics-estimation/blob/humanDynamicsRemapper/remappers/HumanDynamicsRemapper/HumanDynamicsRemapper.cpp#L59.

lrapetti commented 2 years ago

About the humanDynamicsRemapper, there remains one point that I'm not very sure, that is, how could I determine the humanDynamicsDataPort shown here: https://github.com/robotology/human-dynamics-estimation/blob/humanDynamicsRemapper/remappers/HumanDynamicsRemapper/HumanDynamicsRemapper.cpp#L59.

This parameter will be passed trough configuration file as it is done for HumanStateRemapper in https://github.com/robotology/human-dynamics-estimation/blob/80b5e67dde7a8f9b8e675248ff3ad9109181ae7f/conf/xml/RobotPositionController_iCub.xml#L6-L8

Zweisteine96 commented 2 years ago

I checked the configuration file here: https://github.com/robotology/human-dynamics-estimation/tree/80b5e67dde7a8f9b8e675248ff3ad9109181ae7f/conf/xml, but didn't find one that is related to the human_dynamics_remapper, did I miss something? Or should I add one such configuration file manually?

lrapetti commented 2 years ago

didn't find one that is related to the human_dynamics_remapper, did I miss something? Or should I add one such configuration file manually?

Since it was not implemented, there are no configuration files for human_dynamics_remapper so you should create one.

What you can test is to try to run a yarprobotinterface with the human_dynamics_wrapper such as the file Human.xml (it has the dynamics wrapper here https://github.com/robotology/human-dynamics-estimation/blob/80b5e67dde7a8f9b8e675248ff3ad9109181ae7f/conf/xml/Human.xml#L223-L232).

And then create a new yarprobotinterface with the human_dynamics_wrapper only and run it and see if it runs fine.

traversaro commented 2 years ago

Just to add a bit of extra information, the role of the Remapper YARP device in general is to expose the data coming from a thrifted yarp data port, trought a cpp interface.

Just FYI, what you are describing here seems to be more a "Network Wrapper Client (NWC)", see https://www.yarp.it/git-master/nws_and_nwc_architecture.html . Tipically, a remapper device is not related at all to YARP ports/ROS topics or in any case any network.

A remapper device, see for example the one listed in https://www.yarp.it/latest/group__dev__impl__remappers.html permit to take devices that a group of "quantities" such as joint-related quantities or sensor related quantities, and permit to select a subset of this quantities. For graphical display, see Slide 10 from https://github.com/vvv-school/vvv18/blob/b9b57327555bf20224dd344a520731e62224005c/material/dynamics/yarp-motor-control-for-dynamics.pdf : wholebodyabstraction

lrapetti commented 2 years ago

Just FYI, what you are describing here seems to be more a "Network Wrapper Client (NWC)", see https://www.yarp.it/git-master/nws_and_nwc_architecture.html . Tipically, a remapper device is not related at all to YARP ports/ROS topics or in any case any network.

Ok, thanks a lot @traversaro for the clarification, I gues then we used wrong naming convention in this project. I guess the original source of confusion was the IWearRemapper device implemented in wearables which has both the role of a Client Network Wrapper and Remapper.

Indeed, if we want to align with what explained we should change all the Wrapper devices to Server Network Wrapper and the Remapper into Client Network Wrapper. However, I would consider this activity independent from @Zweisteine96 activity, we would eventually open a dedicated issue for this task.

Zweisteine96 commented 2 years ago

The humanDynamicsRemapper has already been implemented and an PR is opened for this modification. The naming problem will be fixed in the future. Now close this issue.

lrapetti commented 2 years ago

The humanDynamicsRemapper has already been implemented and an PR is opened for this modification. The naming problem will be fixed in the future. Now close this issue.

I am reopening the issue since usually we close issues after merging the associated PR.

lrapetti commented 2 years ago

Now the PR is merged, we can close it