Human dynamics remapper

robotology / human-dynamics-estimation

Software repository for estimating human dynamics

BSD 3-Clause "New" or "Revised" License

80 stars 28 forks source link

Human dynamics remapper #295

Closed Zweisteine96 closed 2 years ago

Zweisteine96 commented 2 years ago

Add a humanDynamicsRemapper (actually the client network wrapper, but for current naming consistency still call it "remapper") and related configuration files.

RiccardoGrieco commented 2 years ago

I think the PR should be merged into devel, not master.

lrapetti commented 2 years ago

I think the PR should be merged into devel, not master.

I would try to re-align with the previous choice of maintaining only a development branch as master, while stable versions of the code could be found among the old releases.

In this case, the existing devel branch was obtained by merging multiple development branches without doing change review. So it should be considered as a specific new feature development branch rather (we might also change its name). Indeed, @RiccardoGrieco if you agree I would keep targeting master with this PR

traversaro commented 2 years ago

@Zweisteine96 it is a good practice to have at least a small description in the PR that describes what the PR is about (no need for long documentation, even a single sentence is enough).

Zweisteine96 commented 2 years ago

@Zweisteine96 it is a good practice to have at least a small description in the PR that describes what the PR is about (no need for long documentation, even a single sentence is enough).

Hi @traversaro , thanks for the reminding. Just added some short descriptions. Last PR was in a hurry, forgot to make some clarifications. Sorry for the inconvenience

traversaro commented 2 years ago

Sorry for the inconvenience

No problem!

RiccardoGrieco commented 2 years ago

I think the PR should be merged into devel, not master.

I would try to re-align with the previous choice of maintaining only a development branch as master, while stable versions of the code could be found among the old releases.

In this case, the existing devel branch was obtained by merging multiple development branches without doing change review. So it should be considered as a specific new feature development branch rather (we might also change its name). Indeed, @RiccardoGrieco if you agree I would keep targeting master with this PR

Ok for me!

cc @prashanthr05

Zweisteine96 commented 2 years ago

Hi @lrapetti @RiccardoGrieco , I just commited and pushed the modifications including:

add a blank line at the end of HumanDynamicsRemapper.h and CMakeLists.txt files
change the robot name to Human-Dynmiacs-Remapper in HumanDynamicsRemapperExample.xml file

Zweisteine96 commented 2 years ago

cc @RiccardoGrieco @lrapetti , just updated the PR with addition of mutex protection :)

lrapetti commented 2 years ago

The CI seems to be failing for an error in the HumanStateRemapper (see https://github.com/robotology/human-dynamics-estimation/runs/6210855229?check_suite_focus=true), @Zweisteine96 is it compiling on your machine?

lrapetti commented 2 years ago

Thanks @Zweisteine96, we can merge it!