Integrating HDE-related repositories in the robotology-superbuild

yeshasvitirupachuri commented 5 years ago

To ensure that anyone in the lab that needs to use HDE-related software can use it easily. We need to understand if it is better to use the Dynamics Profile, or it is better to add a new "profile". If the compilation time increase substantially, it is better to add a new profile.

For previous discussions see https://github.com/dic-iit/component_andy/issues/159

traversaro commented 5 years ago

Probably a new profile may be more clean, especially if in the future more repos are added to it, and also because ideally we also get a mantainer for it. : ) See https://github.com/robotology/robotology-superbuild/issues/142 for draft docs on this.

lrapetti commented 5 years ago

Just as a note: the options are necessary on the PC running the producer devices (Windows machine running some wearables devices), while the PC on the consumer side running HDE can be on the standard profile.

traversaro commented 5 years ago

the options are necessary on the PC running the producer devices (Windows machine running some wearables devices), while the PC on the consumer side running HDE can be on the standard profile.

Hopefully if everything is configured correctly, this could be summarized as "On windows, compile the superbuild with ROBOTOLOGY_ENABLE_ICUB_HEAD , ROBOTOLOGY_ENABLE_HUMAN_DYNAMICS and ROBOTOLOGY_USES_ESDCAN, while on the consumed side just compile the superbuild with ROBOTOLOGY_ENABLE_HUMAN_DYNAMICS . However, https://github.com/robotology/robotology-superbuild/issues/205 and https://github.com/robotology/robotology-superbuild/issues/204 need to be worked on.

kouroshD commented 5 years ago

As suggested in this issue and #159, I have added HDE to superbuild.

The changes are done in the following branch in my repo: https://github.com/kouroshD/robotology-superbuild/tree/feature/addHDE

As soon as @Yeshasvitvs tests it, to check if it works fine, I will open a PR to robotology/superbuild.

The test and PR eventually addresses robotology/robotology-superbuild#205 and robotology/robotology-superbuild#204.

kouroshD commented 5 years ago

I test the changes in superbuild using macOS machine, and now we can build the HDE and wearables projects.

[x] macOS
[ ] Windows
[ ] Ubuntu

Also, currently in order to install successfully, we should manualy change the werables branch to feature/ICub-device-impl. Currently, there is an open PR#25 to metge it to master.

yeshasvitirupachuri commented 5 years ago

Also, currently in order to install successfully, we should manualy change the werables branch to feature/ICub-device-impl. Currently, there is an open PR#25 to metge it to master.

@kouroshD you can set wearables to master branch. feature/ICub-device-impl is not blocking wearables.

kouroshD commented 5 years ago

Also, currently in order to install successfully, we should manualy change the werables branch to feature/ICub-device-impl. Currently, there is an open PR#25 to metge it to master.

@kouroshD you can set wearables to master branch. feature/ICub-device-impl is not blocking wearables.

It has been set to master now. When building superbuild by default options, it throws error now. So, that's why we need to change the repo branch to feature/ICub-device-impl branch. Just as a record, I mention it here.

yeshasvitirupachuri commented 5 years ago

We can procede to test the superbuild in the Virtualizer machine (Windows) or the Monster. Just tag current configuration for precaution measures and then test. But we need to check that PR#25 @lrapetti It might be worth considering waiting a bit due to https://github.com/robotology/QA/issues/350

DanielePucci commented 5 years ago

Please @kouroshD update this issue.

kouroshD commented 5 years ago

I have rebased the code on top of master feature/addHDE_rebased and I have added human-gazebo (with no dependency to YARP, iDynTree, etc!) as well. I tried again on macOS, and I could build it. I tried to install on Xsens machine, in a new User, but I did not have administrative privileges, therefore I could not install. I try to install it in my user of Oculus Alienware Machine.

DanielePucci commented 5 years ago

Tasks to do:

[x] tests on windows
[x] tests on linux

@kouroshD

kouroshD commented 5 years ago

I tested the superbuild with the HDE in Windows machine and I could build it. However, I had following problems for the core profile of the superbuild, which there are open issues:

Since wearables now is in robotolgy and we are going to add it to superbuild, it is important to have ReadME documentation. Reference: https://github.com/robotology/wearables/issues/14

The dependencies should be updated: For Xsens: We need to have following Environment variables as well (which are currently missing):

c:\Program Files\Xsens\MVN SDK 2018.0.3\SDK Files\x64\include
c:\Program Files\Xsens\MVN SDK 2018.0.3\SDK Files\x64\lib

About the USB-CAN2: We need the update https://github.com/robotology/human-dynamics-estimation/wiki/Set-up-Machine-for-running-HDE#usb-can-2 according to https://github.com/dic-iit/component_andy/wiki/Set-up-Windows-machine-for-HDE-demo#usb-can-2 .

Also, we need to add following environment variable which is missing:

C:\Program Files\ESD\CAN\SDK\lib

Also, As I understand _"Connect the CAN-USB 2 interface device with the cable to the laptop. Open the Windows Device Manager application and select the unknown device. Select to update the driver and select can_usb2_win64269 directory. • USB-CAN should now be recognized and it will appear on Windows Device Manager application. Verify if the CAN is correctly identified in the Window Device Manager" this is a runtime dependency not at build/compile time.

@Yeshasvitvs , @lrapetti Please update the wiki accordingly as mentioned here, if possible.

We need to have a quick check on ubuntu machine, @Yeshasvitvs @diegoferigo @traversaro Can you check it please, since I do not have ubuntu machine.

In any case, I open the PR, since it worked in MacOS, it should be very similar for ubuntu as well. cc @DanielePucci

kouroshD commented 5 years ago

Just to write down the logics of what is doing in superbuild: -human-dynamics-estimation is now only built in MacOS or Ubuntu, but not Windows, as soon as we merge devel to master, we can remove the check for the windows machine . cc @lrapetti

Since HDE depends on ROBOTOLOGY_ENABLE_ICUB_HEAD option, but in this option we can not build xsensmt-yarp-driver, I have added a check for windows machine.

kouroshD commented 5 years ago

I have tested in ubuntu 16.04 . It works as well. A few points:

Minimum requirement for g++ and gcc versions are 7.4.0 (Thanks @Yeshasvitvs !)
This problem also there exist in ubuntu: robotology/robots-configuration#106
Wearable should manually checkout to feature/ICub-device-impl.

cc @lrapetti @Yeshasvitvs @traversaro @DanielePucci

kouroshD commented 5 years ago

PR link: https://github.com/robotology/robotology-superbuild/pull/231

yeshasvitirupachuri commented 5 years ago

Wearable should manually checkout to feature/ICub-device-impl

@kouroshD we can set the upstream tag to track feature/ICub-device-impl branch by changing from master

kouroshD commented 5 years ago

Wearable should manually checkout to feature/ICub-device-impl

@kouroshD we can set the upstream tag to track feature/ICub-device-impl branch by changing from master

It is not very clean to have another tag instead of master. I do not know if @traversaro will accept the PR on superbuild in that case. Is there any reason, we do not merge the open PR https://github.com/robotology/wearables/pull/25 to master?

yeshasvitirupachuri commented 5 years ago

It is not very clean to have another tag instead of master.

I believe it’s an acceptable practice (I remember we did something similar with HDE or wearables before) and much better than making an user change it manually.

@diegoferigo

kouroshD commented 5 years ago

Is there any reason, we do not merge the open PR robotology/wearables#25 to master? @Yeshasvitvs ?

I believe it’s an acceptable practice (I remember we did something similar with HDE or wearables before) and much better than making an user change it manually.

For me is fine, I leave this decision to @traversaro , since he is the maintainer of the superbuild.

traversaro commented 5 years ago

I believe it’s an acceptable practice (I remember we did something similar with HDE or wearables before) and much better than making an user change it manually.

We typically do that in https://github.com/robotology/robotology-superbuild/blob/master/cmake/ProjectsTagsMaster.cmake for having a centralized point in which to deal with this info. We did that in the past several times, see:

However, those were specific cases in which a base dependency (in those cases RTF or WB-Toolbox) was updated with a new major version that contained breaking changes (respectively RTF v2 and WB-Toolbox v5), and so we remained for a while on the old stable version waiting for the downstream projects to update to be compatible with the new major versions. Having the superbuild to point to a random-named branch is something that I strongly prefer to avoid unless there are strong reasons to do that. Can you imagine if all the subprojects of the superbuild had this "magic branches"? Unless there is a really strong motivation for this, I would really prefer to avoid pointing to non-stable branches.

lrapetti commented 5 years ago

The error that is currently thrown when compiling HumanDynamicEstiamtion in master with Wearable master is the following:

Scanning dependencies of target HumanWrenchProvider
[ 34%] Building CXX object devices/HumanWrenchProvider/CMakeFiles/HumanWrenchProvider.dir/HumanWrenchProvider.cpp.o
/Users/lrapetti/robotology/human-dynamics-estimation/devices/HumanWrenchProvider/HumanWrenchProvider.cpp:99:57: error: 
      no type named 'IVirtualJointKinSensor' in namespace 'wearable::sensor';
      did you mean 'IVirtualLinkKinSensor'?
  ...wearable::sensor::IVirtualJointKinSensor> robotJointWearableSensors;
     ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
                       IVirtualLinkKinSensor
/Users/lrapetti/robotology-superbuild/build/install/include/Wearable/IWear/Sensors/IVirtualLinkKinSensor.h:20:25: note: 
      'IVirtualLinkKinSensor' declared here
class wearable::sensor::IVirtualLinkKinSensor : public wearable::sensor::ISensor
                        ^
/Users/lrapetti/robotology/human-dynamics-estimation/devices/HumanWrenchProvider/HumanWrenchProvider.cpp:529:30: error: 
      no member named 'getJointPosition' in
      'wearable::sensor::IVirtualLinkKinSensor'
                jointSensor->getJointPosition(jointPosition);
                ~~~~~~~~~~~  ^
/Users/lrapetti/robotology/human-dynamics-estimation/devices/HumanWrenchProvider/HumanWrenchProvider.cpp:534:30: error: 
      no member named 'getJointVelocity' in
      'wearable::sensor::IVirtualLinkKinSensor'
                jointSensor->getJointVelocity(jointVelocity);
                ~~~~~~~~~~~  ^
/Users/lrapetti/robotology/human-dynamics-estimation/devices/HumanWrenchProvider/HumanWrenchProvider.cpp:736:31: error: 
      no member named 'getVirtualJointKinSensors' in 'wearable::IWear'
            if (pImpl->iWear->getVirtualJointKinSensors().size() < pImpl...
                ~~~~~~~~~~~~  ^
/Users/lrapetti/robotology/human-dynamics-estimation/devices/HumanWrenchProvider/HumanWrenchProvider.cpp:742:62: error: 
      no member named 'getVirtualJointKinSensors' in 'wearable::IWear'
            pImpl->robotJointWearableSensors = pImpl->iWear->getVirtualJ...
                                               ~~~~~~~~~~~~  ^
5 errors generated.
make[2]: *** [devices/HumanWrenchProvider/CMakeFiles/HumanWrenchProvider.dir/HumanWrenchProvider.cpp.o] Error 1
make[1]: *** [devices/HumanWrenchProvider/CMakeFiles/HumanWrenchProvider.dir/all] Error 2

kouroshD commented 5 years ago

@Yeshasvitvs , @lrapetti Please update the wiki accordingly as mentioned here, if possible.

@lrapetti @Yeshasvitvs can you update the wiki please!?

lrapetti commented 5 years ago

@lrapetti @Yeshasvitvs can you update the wiki please!?

@kouroshD Don't you have the rights to edit the wiki of HumanDynamicsEstimation repo?

kouroshD commented 5 years ago

@kouroshD Don't you have the rights to edit the wiki of HumanDynamicsEstimation repo?

Yes, I have. I will make the changes then. Since you and @Yeshasvitvs are the main maintainers of the repo, I asked you, so that you are aware of changes.

lrapetti commented 5 years ago

Yes, I have. I will make the changes then. Since you and @Yeshasvitvs are the main maintainers of the repo, I asked you, so that you are aware of changes.

Great, thanks. I'm fine with that.

kouroshD commented 5 years ago

I have updated the wiki page!

kouroshD commented 5 years ago

Finally, we have merged the PR. Therefore, we can close this issue. Thanks @traversaro

robotology / human-dynamics-estimation

Integrating HDE-related repositories in the robotology-superbuild #120