Skip to content

Add gz-sim-yarp-plugins package #1628

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

Merged
merged 16 commits into from
Apr 4, 2024

Conversation

xela-95
Copy link
Member

@xela-95 xela-95 commented Mar 29, 2024

@xela-95
Copy link
Member Author

xela-95 commented Apr 3, 2024

@traversaro I was looking how to enable the build of gz-sim-yarp-plugins project to test everything works. This comes with the new option ROBOTOLOGY_USES_GZ that is disabled by default and mutually exclusive wrt ROBOTOLOGY_USES_GAZEBO.

Should I configure an additional step similar to

- name: Configure [Ubuntu]
if: contains(matrix.os, 'ubuntu')
shell: bash
run: |
cd ${ROBOTOLOGY_SUPERBUILD_SOURCE_DIR}
mkdir -p build
cd build
cmake -C ${ROBOTOLOGY_SUPERBUILD_SOURCE_DIR}/.ci/initial-cache.gh.cmake -G"${{ matrix.cmake_generator }}" -DYCM_EP_ADDITIONAL_CMAKE_ARGS:STRING="-DMatlab_ROOT_DIR:PATH=${GHA_Matlab_ROOT_DIR} -DMatlab_MEX_EXTENSION:STRING=${GHA_Matlab_MEX_EXTENSION}" -DROBOTOLOGY_USES_MATLAB:BOOL=ON -DYCM_BOOTSTRAP_VERBOSE=ON -DNON_INTERACTIVE_BUILD:BOOL=TRUE -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} ${{ matrix.project_tags_cmake_options }} ..

with the ROBOTOLOGY_USES_GZ option set to ON?

@traversaro
Copy link
Member

Ah, this thing of mutual exclusivity is complex and is going to be an headache. Until now, the logic in the CI was always: "just enable everything". Now I guess we need either to:

  • Have different builds for ROBOTOLOGY_USES_GZ and ROBOTOLOGY_USES_GAZEBO, doubling the build matrix
  • At the end of the build, add a step that enables ROBOTOLOGY_USES_GZ and disables ROBOTOLOGY_USES_GAZEBO (or the contrary)

Feel free to follow the path that make more sense to you, probably the second one is easier.

@xela-95
Copy link
Member Author

xela-95 commented Apr 3, 2024

@traversaro in

### Compile the robotology-superbuild
In a terminal in which you activate the `robsub` environment, you can compile.
On **Linux** or on **macOS** with an Intel-based processor, run:
~~~
cd robotology-superbuild
mkdir build
cd build
cmake ..
cmake --build . --config Release
~~~
On **Windows**, run:
~~~
cd robotology-superbuild
mkdir build
cd build
cmake -G"Visual Studio 16 2019" ..
cmake --build . --config Release
~~~
is not mentioned that when configuring the project the CMAKE_INSTALL_PREFIX variable should be set to CONDA_PREFIX. Is it on purpose?

@traversaro
Copy link
Member

The superbuild is a bit special, as it does not have any install step. In its build (i.e. the invocation of make or ninja) step, it calls the install of all the subprojects. All the subprojects are installed in the build/install location, and this can't be easily changed. To consume projects compiled by the superbuild, one should source setup.sh script or similar, not install in $CONDA_PREFIX.

@xela-95
Copy link
Member Author

xela-95 commented Apr 3, 2024

The superbuild is a bit special, as it does not have any install step. In its build (i.e. the invocation of make or ninja) step, it calls the install of all the subprojects. All the subprojects are installed in the build/install location, and this can't be easily changed. To consume projects compiled by the superbuild, one should source setup.sh script or similar, not install in $CONDA_PREFIX.

Ok it's clear now thanks!

@xela-95

This comment was marked as resolved.

@xela-95
Copy link
Member Author

xela-95 commented Apr 3, 2024

@traversaro should I add the environment variable GZ_SIM_RESOURCE_PATH as it has been done for GAZEBO_MODEL_PATH to find the iCub and ErgoCub models for Gazebo Classic?

@traversaro
Copy link
Member

@traversaro should I add the environment variable GZ_SIM_RESOURCE_PATH as it has been done for GAZEBO_MODEL_PATH to find the iCub and ErgoCub models for Gazebo Classic?

Yes. Eventually we will add gz-sim support there, so let's already do it so we can forget about this.

@traversaro
Copy link
Member

Note that probably given all the problems related to co-installation of gz-sim and gazebo-classic we can skip the CI part for now.

@traversaro
Copy link
Member

I left a bunch of comments, but the gist is the following: never use "Gazebo" without specification, it is going to be too confusing otherwise for users that called "Classic Gazebo" Gazebo for years.

@xela-95
Copy link
Member Author

xela-95 commented Apr 4, 2024

I left a bunch of comments, but the gist is the following: never use "Gazebo" without specification, it is going to be too confusing otherwise for users that called "Classic Gazebo" Gazebo for years.

Ok, what term we should use for new Gazebo? Modern Gazebo or gz-sim?

@xela-95 xela-95 force-pushed the add-gz-sim-yarp-plugins branch from 4e51d08 to 795dfdc Compare April 4, 2024 10:18
@xela-95
Copy link
Member Author

xela-95 commented Apr 4, 2024

I tested the usage of ergocub-software and gz-sim-yarp-plugins from the robotology-superbuild on Ubuntu 22.04 and WSL, both with conda-forgeand apt dependencies. Both worked as expected.

@xela-95 xela-95 marked this pull request as ready for review April 4, 2024 13:04
@traversaro
Copy link
Member

Thanks, I think we can proceed with merging. Existing conda users may need to pay attention as now gazebo needs to be installed manually and is not installed by the long list of dependencies in the README, but I guess it should be easy to manage as a change. @xela-95 can I squash?

@xela-95
Copy link
Member Author

xela-95 commented Apr 4, 2024

Thanks, I think we can proceed with merging. Existing conda users may need to pay attention as now gazebo needs to be installed manually and is not installed by the long list of dependencies in the README, but I guess it should be easy to manage as a change. @xela-95 can I squash?

Yep exactly, now gazebo has to be installed manually also from conda, but I think this will not overcomplicate the workflow too much.

Sure, squeeze! 🧽

@traversaro traversaro merged commit c1e20b0 into robotology:master Apr 4, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants