osrf / vrx

Virtual RobotX (VRX) resources.
Apache License 2.0
421 stars 189 forks source link

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

Closed M1chaelM closed 1 year ago

M1chaelM commented 1 year ago

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

M1chaelM commented 1 year ago

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 commented 1 year ago

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.