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

Update README (after #122) #126

Merged
merged 4 commits into from
May 11, 2021
Merged

Update README (after #122) #126

merged 4 commits into from
May 11, 2021

Conversation

nunoguedelha
Copy link
Contributor

No description provided.

@nunoguedelha nunoguedelha self-assigned this May 10, 2021
@nunoguedelha nunoguedelha force-pushed the nunoguedelha-patch-1 branch from b2b01d5 to 0105058 Compare May 10, 2021 19:55
@nunoguedelha nunoguedelha mentioned this pull request May 10, 2021
@nunoguedelha nunoguedelha changed the title Update README (#122) Update README (after #122) May 11, 2021
Update with w.r.t. changes from after #122.
@nunoguedelha nunoguedelha force-pushed the nunoguedelha-patch-1 branch from 66c0537 to 374e9c8 Compare May 11, 2021 04:52
…a package +floatingBaseBalancingTorqueControlWithSimulator

This avoids to:
- Add the model parent folder path to the Matlab path, as well as for every new model (example) installed.
  It would quickly become a mess with the installation of multiple examples, with problems of file names
  collision.
- Having 'folder1/folder2/model.mdl' and 'folder1' in the Matlab path wouldn't work well either because we
  wouldn't be able to call 'folder2/model.mdl' for opening the model from anywhere.
- From there, the easiest solution is to use namespaces provided by Matlab packages.
@nunoguedelha
Copy link
Contributor Author

nunoguedelha commented May 11, 2021

Follow-up of #122 :

@gabrielenava

  • Could you add matlab-whole-body-simulator also to the list of dependencies in the main WBC readme, and specify that it is an optional dependency for Gazebo-free simulations?

  • Also, feel free to remove the previous simulink-balancing simulator, also from the list of available controllers (and to add instead your simulator).

  • We can add the simulink-balancing-simulator to the list of legacy controllers in the README, in the past I moved old controllers in WBI-Toolbox-Controllers but in this case maybe it is just sufficient to point the last commit where the simulink-balancing simulator is still present.

@traversaro

  • Why do we need to set manually setenv('YARP_ROBOT_NAME','iCubGazeboV2_5') ? Can't we set it somewhere in the example initialization or similar?
  • Could we install torqueControlBalancingWithSimu.mdl, perhaps in a matlab/examples directory or similar? ...

ONGOING...
(the package +floatingBaseBalancingTorqueControlWithSimulator just needs to be copied to the install folder mex/examples and mex/examples needs to be added t MATLABPATH the same way as mex/simulink.

By the way it's a bit complex the installation framework there...

@traversaro
Copy link
Member

By the way it's a bit complex the installation framework there...

In which sense? Can't we add a install(DIRECTORY ...) and install(FILES ..) and add the files that we need to install where we need to install them?

@nunoguedelha
Copy link
Contributor Author

Can't we add a install(DIRECTORY ...) and install(FILES ..) a

Yes, I ended up chosing that. Initially I was trying to follow the local whole-body-controllers pattern, but I'll just go with the install(DIRECTORY ...) and install(FILES ..) etc ...

- minor hack: in the main CMakeLists.txt of the whole-body-controllers, the script returns to the caller before running 'add_subdirectory(controllers)'. As a result, the respective CMakeLists.txt doesn't run, so we had to add the copy commands directly to the main CMakeLists.txt.
@nunoguedelha
Copy link
Contributor Author

The installation is handled and tested. Had t choose a not so clean and elegant solution due to the logic around the automatic code generation handled in the main CMakeLists.txt. I'm just checking now the Conda scripts...

@traversaro
Copy link
Member

The installation is handled and tested. Had t choose a not so clean and elegant solution due to the logic around the automatic code generation handled in the main CMakeLists.txt. I'm just checking now the Conda scripts...

Can we mark the PR as ready in the meanwhile? So we can review and release it, and if more problem arise we can fix them later.

@nunoguedelha nunoguedelha marked this pull request as ready for review May 11, 2021 16:47
@traversaro
Copy link
Member

As this PR is targeted on master, I guess it supersedes #115 ?

@nunoguedelha
Copy link
Contributor Author

no, no, sorry, I had rebased on devel and forgot to change the target here. It's targeted on devel.

@nunoguedelha
Copy link
Contributor Author

there were some templates missing, along with multisheller scripts updates. Pushing now..

@nunoguedelha
Copy link
Contributor Author

there were some templates missing, along with multisheller scripts updates. Pushing now..

nonsense! those are robotology-superbuild changes.

@nunoguedelha nunoguedelha changed the base branch from master to devel May 11, 2021 17:52
@nunoguedelha
Copy link
Contributor Author

Merging...

@nunoguedelha nunoguedelha merged commit 32ba3a1 into devel May 11, 2021
@nunoguedelha nunoguedelha deleted the nunoguedelha-patch-1 branch May 11, 2021 17:57
nunoguedelha added a commit to robotology/robotology-superbuild that referenced this pull request May 11, 2021
nunoguedelha added a commit to robotology/robotology-superbuild that referenced this pull request May 12, 2021
nunoguedelha added a commit to robotology/robotology-superbuild that referenced this pull request May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants