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.

Sign up for GitHub

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

Jump to bottom

Add repo matlab-whole-body-simulator #689

Merged
merged 9 commits into from
Apr 25, 2021
Merged

Add repo matlab-whole-body-simulator #689

merged 9 commits into from
Apr 25, 2021

Conversation

nunoguedelha
Copy link
Collaborator

@nunoguedelha nunoguedelha commented Apr 15, 2021 •
edited
Loading

Implements #576 .

  • Add the path to the simulator library RobotDynamicsWithContacts in the
    startup_robotology_superbuild.m.
  • Created the file Build*** for adding the package to the superbuild.
  • Updated the superbuild Logic for loading the package within the "Dynamics"
    profile.

Depends on ami-iit/matlab-whole-body-simulator#36.

@nunoguedelha nunoguedelha self-assigned this Apr 15, 2021
@nunoguedelha nunoguedelha linked an issue Apr 15, 2021 that may be closed by this pull request
Add https://github.com/dic-iit/matlab-whole-body-simulators #576
Closed
@traversaro
Copy link
Member

Hi @nunoguedelha, in the PR it seems that nothing is installed, and the matlab path is update to point to the source directory rather then the install folder. Even if for MATLAB project the installation phase just means copying some files, it is important to install properly the MATLAB/Simulink files that are necessary to use matlab-whole-body-simulator, even to support the use case:

conda install -c conda-forge -c robotology matlab-whole-body-simulator

@traversaro
Copy link
Member

We can discuss on teams if you prefer.

@nunoguedelha
Copy link
Collaborator Author

Yes, I was still understanding what was the proper thing to do. What was bothering me was the fact that in the case of this repo, the proper installation would consist in copying almost everything to the install folder. I guess it should be almost everything except the main test model. Let's discuss on Team.

@traversaro
Copy link
Member

What was bothering me was the fact that in the case of this repo, the proper installation would consist in copying almost everything to the install folder.

Yes, that is basically the case with most interpreted languages. The main difference is that for example Python has its own tools to install Python-only packages (pip install .) while MATLAB does not have this because it is seldom used in the context of a "scientific software distribution" such as the robotology-superbuild.

@nunoguedelha
Copy link
Collaborator Author

After the last changes, I've tested the steps:

  1. Changed releases/latest.releases.yaml, adding the section: 
    matlab-whole-body-simulator:
    type: git
    url: https://github.com/dic-iit/matlab-whole-body-simulator.git
    version: v2.0.0-alpha-1
  2. cmake ../
  3. Set ROBOTOLOGY_PROJECT_TAGS to LatestRelease
  4. configure and generate
  5. make update-all

The new repo is cloned, as shown below:

...
Scanning dependencies of target matlab-whole-body-simulator-update
[ 82%] Creating directories for 'matlab-whole-body-simulator'
[ 82%] Performing download step (git clone) for 'matlab-whole-body-simulator'
Cloning into 'matlab-whole-body-simulator'...
Note: switching to 'v2.0.0-alpha-1'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at f9cea00 Issue #35: Completed CMake configuration
[ 82%] Performing update step for 'matlab-whole-body-simulator'
[ 82%] Built target matlab-whole-body-simulator-update
...

@nunoguedelha
Copy link
Collaborator Author

Solving now some build issues with YARP_telemetry and <matio.h> include. Should be something messed up in my environment...

@nunoguedelha
Copy link
Collaborator Author

nunoguedelha commented Apr 19, 2021 •
edited
Loading

The full superbuild is building now after fixing the CMakeLists.txt in ami-iit/matlab-whole-body-simulator#36. All the repo matlab-whole-body-simulator relevant files were installed in the superbuild install folder,as well as the robot model (RRbot1) files.

Fixing now some minor issues for compiling the model in MATLAB (the multisheller files) ...

@nunoguedelha
Copy link
Collaborator Author

Everything is working now:

  • Could build the whole superbuild with profile ROBOTOLOGY_ENABLE_DYNAMICS, option ROBOTOLOGY_USES_MATLAB.
  • checked that the repo is properly cloned to robotology-superbuild/src/matlab-whole-body-simulator, and the installation (required library files are copied to the superbuild install folder build/install/mex/+mwbs, the new robot model and meshes files are copied to build/install/share/RRbot).
  • Run the model in MATLAB/Simulink using both methods:
    • clone matlab-whole-body-simulator outside the superbuild, install dependencies through Conda.
    • install matlab-whole-body-simulator through the superbuild.

@nunoguedelha
Copy link
Collaborator Author

I need to update the READMEs, otherwise it's ready for review.

@nunoguedelha nunoguedelha marked this pull request as ready for review April 20, 2021 02:45
traversaro
traversaro reviewed Apr 20, 2021
View reviewed changes
Copy link
Member

@traversaro traversaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes on the environment variables needs to be propagate also to the following files:

(That's the reason why we try to minimize the env variables that is necessary to modify/change for each project).

@nunoguedelha
Copy link
Collaborator Author

The changes on the environment variables needs to be propagate also to the following files:

* All the other files in https://github.com/robotology/robotology-superbuild/tree/master/cmake/template

* The docs in https://github.com/robotology/robotology-superbuild/blob/master/doc/environment-variables-configuration.md

(That's the reason why we try to minimize the env variables that is necessary to modify/change for each project).

Sorry @traversaro , I had forgotten to commit and push my latest fixes from yesterday (13229c6). I had tested with that. But a few others are missing anyway. Fixing...

@nunoguedelha
Copy link
Collaborator Author

The changes on the environment variables needs to be propagate also to the following files:

* All the other files in https://github.com/robotology/robotology-superbuild/tree/master/cmake/template

* The docs in https://github.com/robotology/robotology-superbuild/blob/master/doc/environment-variables-configuration.md

(That's the reason why we try to minimize the env variables that is necessary to modify/change for each project).

Sorry @traversaro , I had forgotten to commit and push my latest fixes from yesterday (13229c6). I had tested with that. But a few others are missing anyway. Fixing...

Done in b24f8ee.

@nunoguedelha
Copy link
Collaborator Author

Rework completed.

@nunoguedelha
Copy link
Collaborator Author

@traversaro , I've completed the changes as per what we discussed offline.

@traversaro
Copy link
Member

Can you merge devel in master of matlab-whole-body-simulator? Otherwise the CI will fail as the master branch that is used does not have the CMake build files.

@nunoguedelha
Copy link
Collaborator Author

nunoguedelha commented Apr 24, 2021 •
edited
Loading

Can you merge devel in master of matlab-whole-body-simulator? Otherwise the CI will fail as the master branch that is used does not have the CMake build files.

Done. Requested your review for the PR.

I guess that in ProjectsTagsStable.cmake, for all the projects missing there, the default branch is master. but should I update ProjectsTagsUnstable.cmake?

Ah wait, it's the same right, default branch shall be master...?

@nunoguedelha
Issue #576: Add repo matlab-whole-body-simulator
41addc1 
- Add the path to the simulator library `RobotDynamicsWithContacts` in the
  `startup_robotology_superbuild.m`.
- Created the file `Build***` for adding the package to the superbuild.
- Updated the superbuild Logic for loading the package within the "Dynamics"
  profile.
@nunoguedelha
Issue #576: Fixed the CMake configuration
cde7b2c 
- Fixed the path to the simulator library `RobotDynamicsWithContacts` in the `startup_robotology_superbuild.m`.
- Updated the superbuild "setup" script.
- Added the multisheller "xxx_activate.msh" and "xxx_deactivate.msh" scripts for the conda package manager.
@nunoguedelha
Fix variables YARP_DATA_DIRS, GAZEBO_MODEL_PATH setting through activ…
49e82e2 
…ate/deactivate multisheller scripts
@nunoguedelha
Issue #576: Updated the env variables in the templates, and other min…
7105c8d 
…or fixes

- Updated the new repo tag in latest.releases.yaml file.
- Updated the logic to build matlab-whole-body-simulator:
  depends on ROBOTOLOGY_USES_MATLAB.
@nunoguedelha
Issue #576: Rework after review.
a0b850d 
- Further updated environment variables in the cmake/templates.
- Removed transitive dependencies from the project cmake configuration.
- Remove tabs from the conda activate/deactivate msh files.
- Updated documentation.
@nunoguedelha
Issue #576: Removed dependency on whole-body-controllers.
d9a18a1 
This dependency can create a problem if whole-body-controllers starts depending
on matlab-whole-body-simulator (see robotology/whole-body-controllers#95 (comment)).
As the dependency is not required at install time, we can drop it.
@nunoguedelha
Removed lib/+mwbs/+RobotDynamicsWithContacts from the envs (we only…
084fec8 
… need `mex`)

- This was due to moving the .slx files to `mex` in the install path:
  ami-iit/matlab-whole-body-simulator@e1f5aba
  ami-iit/matlab-whole-body-simulator@c331a7a
@nunoguedelha
Remove leftover mex/+mwbs/+RobotDynamicsWithContacts from the envs …
b8c1f55 
…set in the act/deact conda scripts
@nunoguedelha nunoguedelha force-pushed the feature/add-repo-matlab-simulator-to-superbuild branch from 8fcb8e9 to b8c1f55 Compare April 24, 2021 19:14
@traversaro traversaro closed this Apr 25, 2021
@traversaro traversaro reopened this Apr 25, 2021
@traversaro traversaro merged commit 8c45dd3 into master Apr 25, 2021
@traversaro traversaro deleted the feature/add-repo-matlab-simulator-to-superbuild branch April 25, 2021 14:20
@traversaro
Copy link
Member

Ah wait, it's the same right, default branch shall be master...?

Exactly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@traversaro traversaro traversaro approved these changes

@gabrielenava gabrielenava Awaiting requested review from gabrielenava

Assignees

@nunoguedelha nunoguedelha

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add https://github.com/dic-iit/matlab-whole-body-simulators
2 participants
@nunoguedelha @traversaro