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.launch.py gives compliance error on empty WAMV #640

Closed
M1chaelM opened this issue May 12, 2023 · 2 comments · Fixed by #658
Closed

generate_wamv.launch.py gives compliance error on empty WAMV #640

M1chaelM opened this issue May 12, 2023 · 2 comments · Fixed by #658
Assignees
Milestone

Comments

@M1chaelM
Copy link
Collaborator

Describe the bug
Running generate_wamv.launch.py with empty configuration files produces an empty WAM-V platform as expected, but also gives the following error:

[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.

Expected behavior
An empty WAM-V should be compliant. Are rules define maximum numbers of thrusters and components, but not minimums.

To Reproduce
List the steps to reproduce the problem:

  1. Run
touch empty_component_config.yaml
touch empty_thruster_config.yaml
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
  1. See error:
[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.

System Configuration:
Tell us about your system.

  • OS: Ubuntu 22.04
  • ROS Version: ROS 2, Humble
  • Gazebo Version: Gazebo Sim 7.0.0
  • Using VRX
@M1chaelM M1chaelM added this to the 2.3.0 milestone May 12, 2023
@M1chaelM M1chaelM self-assigned this May 23, 2023
@M1chaelM
Copy link
Collaborator Author

How this compliance check currently works:

  1. generate_wamv.launch.py calls the script generate_wamv.py and passes it the required variables.
  2. generate_wamv.py is a wrapper for configure_wamv.py.
  3. configure_wamv.py uses the create_component_xacro and create_thruster_xacro files to generate the urdf and check for compliance at the same time.
  4. These functions each call create_xacro_file, which is imported from utils.py
  5. create_xacro_file takes two compliance check functions as arguments: one to check number of components/thrusters and one to check their parameters.
  6. The 4 compliance check functions that are passed to create_xacro_file are each imported from compliance.py, where they are defined as methods of a ComponentCompliance and ThrusterCompliance class.
  7. If any of the 4 checks fail, the console will display the error listed above.
  8. The individual check that fails also uses rclpy to log error messages to wherever the ROS log directory is set by ROS_LOG_DIR variable. However, no error messages are currently being logged except the general error.

@M1chaelM
Copy link
Collaborator Author

It looks like the problem was in the create_xacro_file function, which was handling empty files as a special case and returning None (by default, since no value was specified), which was interpreted as False. I've created #658 to fix this.

As a side note, this code continues to be a source of headaches. Ideally we should not have to create a ROS node or build a WAMV to test for compliance. A script should just read the YAML files directly. I haven't tested but I also suspect logging is probably not working right because the logging calls in the compliance tests aren't attached to the node.

We have an old open issue #365 that addresses some of these problems, so I will close this when the PR is merged. We ma want to take another look at #365 now that VRX 2.0 is out.

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 a pull request may close this issue.

1 participant