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

generate_wamv.py should not produce a compliance error on an empty WAMV #658

Merged
merged 4 commits into from May 25, 2023

Conversation

M1chaelM
Copy link
Collaborator

This fixes issue #640

To test, follow this tutorial: https://github.com/osrf/vrx/wiki/empty_wamv_tutorial

Make sure there is no error message in the output; i.e. after running

ros2 launch vrx_gazebo generate_wamv.launch.py component_yaml:=`pwd`/empty_component_config.yaml thruster_yaml:=`pwd`/empty_thruster_config.yaml wamv_target:=`pwd`/wamv_target.urdf wamv_locked:=False

you should NOT see:

[generate_wamv.py-1] [ERROR] [1683911879.733853761] [configure_wamv]: 
[generate_wamv.py-1] This component/thruster configuration is NOT compliant with the (current) VRX constraints. A urdf file will be created, but please note that the above errors must be fixed for this to be a valid configuration for the VRX competition.

@M1chaelM
Copy link
Collaborator Author

😕 Testing is failing with the following error:

--- stderr: ros_gz_bridge
CMake Error at CMakeLists.txt:81 (find_package):
  By not providing "Findactuator_msgs.cmake" in CMAKE_MODULE_PATH this
  project has asked CMake to find a package configuration file provided by
  "actuator_msgs", but CMake did not find one.

  Could not find a package configuration file provided by "actuator_msgs"
  with any of the following names:

    actuator_msgsConfig.cmake
    actuator_msgs-config.cmake

  Add the installation prefix of "actuator_msgs" to CMAKE_PREFIX_PATH or set
  "actuator_msgs_DIR" to a directory containing one of the above files.  If
  "actuator_msgs" provides a separate development package or SDK, be sure it
  has been installed.

I don't think this was produced by the 4 character change introduced in the PR. @caguero Are you aware of anything changing on the backend?

@M1chaelM
Copy link
Collaborator Author

M1chaelM commented May 23, 2023

It seems that the checkout action has deprecated version 2.4.0, and we should update to v3. I did this, but it produces a new error:

Run actions/checkout@v3
/usr/bin/docker exec  2134273f20e92c767cdeb1c546e1a2024fc02fd0e0153b5854f4cc3e4aebc593 sh -c "cat /etc/*release | grep ^ID"
node:internal/fs/utils:345
    throw err;
    ^

This appears to be caused by the fact that our test image has a non-root user, and Github actions expects everyone to run as root (see actions/checkout#1014, actions/checkout#956, and https://docs.github.com/en/actions/creating-actions/dockerfile-support-for-github-actions#user).

Overriding the container user and setting it to root works around the issue, but then the test fails with the same error as before. By updating the ros_gz repository I was able to reproduce this locally, so it seems to be something introduced recently in ros_gz.

Given the timing, I suspect https://github.com/gazebosim/ros_gz/pull/394/files is the culprit.

@M1chaelM
Copy link
Collaborator Author

The recent change to ros_gz introduced a new dependency on ros-humble-actuator-messages (see the explanation here: gazebosim/ros_gz#397). I've fixed this by adding it to the CI image. Long term, I think it would be better to move to a binary install of Gazebo Garden (see #662 ) since that's generally more accessible to our users.

@M1chaelM M1chaelM requested review from caguero and j-herman May 23, 2023 22:27
@M1chaelM
Copy link
Collaborator Author

@j-herman Do you have time to take a look at this? It should be pretty quick and it fixes the CI, which affects our other PRs.

Copy link
Collaborator

@j-herman j-herman left a comment

Choose a reason for hiding this comment

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

Works as expected - no errors

@M1chaelM M1chaelM merged commit 75e307e into main May 25, 2023
1 check 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.

generate_wamv.launch.py gives compliance error on empty WAMV
2 participants