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

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

Closed
yeshasvitirupachuri opened this issue May 30, 2019 · 27 comments
Closed

Comments

@yeshasvitirupachuri
Copy link
Member

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
Copy link
Member

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 robotology/robotology-superbuild#142 for draft docs on this.

@lrapetti
Copy link
Member

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
Copy link
Member

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, robotology/robotology-superbuild#205 and robotology/robotology-superbuild#204 need to be worked on.

@kouroshD
Copy link
Contributor

kouroshD commented Jun 3, 2019

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
Copy link
Contributor

kouroshD commented Jun 3, 2019

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

  • 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
Copy link
Member Author

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
Copy link
Contributor

kouroshD commented Jun 3, 2019

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
Copy link
Member Author

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 robotology/community#350

@DanielePucci
Copy link
Contributor

Please @kouroshD update this issue.

@kouroshD
Copy link
Contributor

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
Copy link
Contributor

DanielePucci commented Jul 29, 2019

Tasks to do:

  • tests on windows
  • tests on linux

@kouroshD

@kouroshD
Copy link
Contributor

kouroshD commented Aug 1, 2019

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: robotology/wearables#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_win64_269 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
Copy link
Contributor

kouroshD commented Aug 1, 2019

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
Copy link
Contributor

kouroshD commented Aug 1, 2019

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

cc @lrapetti @Yeshasvitvs @traversaro @DanielePucci

@kouroshD
Copy link
Contributor

kouroshD commented Aug 1, 2019

PR link: robotology/robotology-superbuild#231

@yeshasvitirupachuri
Copy link
Member Author

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
Copy link
Contributor

kouroshD commented Aug 2, 2019

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 robotology/wearables#25 to master?

@yeshasvitirupachuri
Copy link
Member Author

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
Copy link
Contributor

kouroshD commented Aug 2, 2019

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
Copy link
Member

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
Copy link
Member

lrapetti commented Aug 2, 2019

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
Copy link
Contributor

kouroshD commented Aug 3, 2019

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

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

@lrapetti
Copy link
Member

lrapetti commented Aug 5, 2019

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

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

@kouroshD
Copy link
Contributor

kouroshD commented Aug 6, 2019

@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
Copy link
Member

lrapetti commented Aug 6, 2019

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.

@claudia-lat claudia-lat modified the milestones: Iteration 27, Iteration 28 Aug 8, 2019
@kouroshD
Copy link
Contributor

kouroshD commented Aug 9, 2019

I have updated the wiki page!

@kouroshD
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants