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

If sensor_config.yaml fails compliance once, it fails all day #16

Open
osrf-migration opened this issue Oct 30, 2019 · 4 comments
Open
Labels
bug Something isn't working major

Comments

@osrf-migration
Copy link

Original report (archived issue) by Michael McCarrin (Bitbucket: m1chaelm).


Our prepare_team_wamv.bash script checks that sensors are compliant by grepping through the $HOME/.ros/log/latest/wamv_config-wamv_generator-1.log for the string “ERROR”. This method produces some false positives. In particular, since the log can contain messages from multiple consecutive runs, any run that produces the string ERROR once will cause compliance checks to fail until the logs are rotated.

A workaround is to delete the log file or removing the lines that say ERROR and run again. Ideally we would record the output of our compliance check directly instead of searching the log.

@osrf-migration
Copy link
Author

Original comment by Brian Bingham (Bitbucket: brian_bingham).


Good catch.

It looks like there was attempt to clear the logs before running the ROS node. Can you try this PR to see if it works for you so we don’t inadvertently forget the workaround in the future?

https://osrf-migration.github.io/vrx-gh-pages/#!/osrf/vrx-docker/pull-requests/9/clear-logs-to-avoid-false-positives/diff

@osrf-migration osrf-migration added major bug Something isn't working labels May 4, 2020
@M1chaelM
Copy link
Collaborator

M1chaelM commented Nov 26, 2021

This problem is a consequence of some of our deeper design decisions. It is probably within reach to fix these decisions rather than to use a workaround.

What is happening

  • prepare_team_wamv.bash in the vrx-docker repository calls generate_wamv.launch in the vrx repository.
  • generate_wamv.launch calls generate_wamv.py (This layer just sets up ROS and the parameter server).
  • generate_wamv.py calls configure_wamv.py (This layer is meant to hide the implementation of generate_wamv. The .py on that script should be removed.)
  • configure_wamv.py calls functions create_thruster_xacro and create_component_xacro. These functions both call the function create_xacro_file from utils.py. They pass in the compliance tests from ComponentCompliance and ThrusterCompliance to create_xacro_file as arguments.
  • create_xacro_file writes the appropriate urdfs to the file system and also runs the compliance checks, then passes the results of the compliance checks back to the create_thruster_xacro / create_component_xacro functions.
  • These functions in turn pass the results back to the main function of configure_wamv.py.
  • If the results are negative (not compliant), configure_wamv.py writes an error message to ROS logs.
  • The prepare_team_wamv.bash script greps through the logs to see whether they contain the word "ERROR".
  • Depending on whether they do or not, the output of either false or true is written to compliant.txt

How component compliance is checked

The ComponentCompliance object does the following:

Initialization

  • Load bounding boxes info from vrx/vrx_gazebo/config/wamv_config/component_compliance/bounding_boxes.yaml
  • Load default_parameters by parsing all .xacro files stored in wamv_gazebo/urdf/components (using Python string methods) and reading the default values listed there.
    • Note this homemade parser has bugs and would break if the strings "name" or "params" appeared in an unexpected place or order, or contained escaped quotation marks.
  • Load numeric limits on components from vrx/vrx_gazebo/config/wamv_config/component_compliance/numeric.yaml

Check parameters

  • Take a component type and its parameters as an argument.
  • Check whether the name of the component is a key in the default_parameters dictionary.
  • Check whether each given parameter is listed in the numeric.yaml file under that component name.
  • Substitute the default parameters wherever parameters have not been specified. This appears to be done just to handle the case where one or more of the xyz parameters uses the default.
  • Check whether the xyz coordinates are in bounds given by the bounding_boxes.yaml

Check numbers

  • Take component type and quantity as arguments
  • Check whether quantity is greater than max listed in numeric.yaml

How thruster compliance is checked

The ThrusterCompliance object does the following:

Initialization

  • Almost identical to ComponentCompliance with some minor path changes. In particular, all the same variables are set up but with different values. Seems to suggest that this should not be a separate class.

Check parameters

  • Again, almost identical. The minor differences could be handled with an if statement.

Check numbers

  • Completely identical but with some different variable names.

How bounding boxes are checked

  • Bounding boxes are pulled from the bounding_boxes.yaml file.
  • They are initialized with a name, pose, size and capacity from that file.
  • Pose and size are np.arrays of floats and capacity is an integer.
  • In our example capacity is set to "-1" for components (meaning unlimited), and 1 for all thrusters.
  • Pose is 1x6 and size is 1x3.
  • The fit method is used to check whether a sensor or component fits in a box.

fit(self, pose)

  • Takes a 1x3 pose array.
  • Subtracts the xyz pose of the box, which produces coordinates relative to the center of the box.
  • For each x,y,z in the shifted coordinates, checks whether the absolute value of the difference is greater than half of the size of the box in that dimension. This tells us whether the coordinate is in the box.
  • Checks whether we have too many items in a box (just for thrusters)
  • No effort is made to prevent components from overlapping.

What should happen

  • Generating a WAMV and testing for compliance should be separated.
    • We may want to generate WAMVs in cases where we don't care about compliance.
    • We might want to check the compliance of a WAMV that has already been generated.
  • Compliance is specific to the competition, not the VRX environment in general, so compliance checking code should be moved to vrx-docker.
  • We do not need to check our own default parameters for compliance every time, so there is no need to fill in the default values before running the checks.
  • There is also no need to create a 3d position. The coordinates are checked independently anyway.

@M1chaelM
Copy link
Collaborator

M1chaelM commented Dec 1, 2021

First step: create a branch of vrx-docker and add compliance checking scripts. It should contain:

  • Both copies of bounding_boxes.yaml, renamed component_boxes.yaml and thruster_boxes.yaml
  • Both copies of numeric.yaml, similarly renamed.
  • A copy of compliance.py to be modified.
  • A function that does the following:
    • Open a yaml file with a sensor or thruster description and load as a dictionary.
    • Iterate through the keys in the dictionary and run the compliance num_test on the list of objects associated with each key.
    • Iterate through each list of objects and run the param_test on the key, object pair.
    • Return true if everything passes, otherwise false.
  • A function that writes the results to the file system.

@M1chaelM
Copy link
Collaborator

M1chaelM commented Dec 7, 2021

Fixing compliant.py

  • Combine ThrusterCompliance and ComponentCompliance classes into a single Compliance class.
  • Remove default parameter section.
  • Change box check to test coordinates one at a time.
  • Make a new function check_specs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working major
Projects
None yet
Development

No branches or pull requests

2 participants