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

Revisit lrauv_ignition_plugins testing suite #189

Closed
2 of 4 tasks
Tracked by #150
hidmic opened this issue Apr 6, 2022 · 5 comments
Closed
2 of 4 tasks
Tracked by #150

Revisit lrauv_ignition_plugins testing suite #189

hidmic opened this issue Apr 6, 2022 · 5 comments
Assignees

Comments

@hidmic
Copy link
Collaborator

hidmic commented Apr 6, 2022

Motivation

Integration tests have been broken for a while now. Recent attempts to fix them have been met with modelling limitations, upstream changes, inadequate tuning, etc (see #187). To optimize our time, we need to clearly define what use cases we want to validate in CI, update our tests to match expectations, and ensure our code base passes.

Definition of Done

  • Review existing lrauv_ignition_plugins tests
  • Define/discuss tests to match use cases
  • Update tests to match definitions
  • Make tests (and CI) pass
@hidmic hidmic self-assigned this Apr 6, 2022
@hidmic
Copy link
Collaborator Author

hidmic commented Apr 6, 2022

FYI @arjo129. I take that YoYoCircle, PitchMass at a depth, and PitchAndDepthMassVBS are a must (at the bare minimum).

@hidmic
Copy link
Collaborator Author

hidmic commented Apr 7, 2022

After going over all tests in lrauv_ignition_plugins, I think the following expectations summarize their intent.


  • Environment
    • World control
      • Vehicles can be spawned
      • State messages are correct
    • Hydrodynamic modelling
      • Damping forces induce a terminal velocity
    • External forces
      • Surface winds drag the vehicle
  • Vehicle
    • Sensors
      • AHRS
        • Measurements' frame conventions are correct
        • Measurements are consistent with state
    • Actuators
      • Mass shifter
        • Vehicle can be made to pitch upwards
        • Vehicle can be made to pitch downwards
      • Oil bladder
        • Vehicle can be made neutrally buoyant
        • Vehicle can be made positively buoyant
        • Vehicle can be made negatively buoyant
      • Drop weight
        • Vehicle can be forced to float (positive buoyancy)
      • Horizontal fins + Propeller
        • Vehicle can develop a thrust towards starboard
        • Vehicle can develop a thrust towards port
      • Vertical fins + Propeller
        • Vehicle can develop an upwards thrust
        • Vehicle can develop an downwards thrust
    • Communications
      • Acoustic transceiver
        • Range bearing estimates are correct
        • Messages can be sent and received
    • Mission control
      • Control (w/ no missions)
        • Speed can be controlled
        • Heading can be controlled
        • Pitch can be controlled at a depth (as pitch control in the fluid density interface is unstable)
        • Pitch + depth can be controlled (as pure depth control is unstable due to hydrodynamics)
      • Missions (incl. startup/default missions)
        • YoYoCircle mission is conducted successfully

Note that for a test suite to validate those expectations, only mission control tests would need the LRAUV controller. Moreover, no missions would be required for test controllers. While this is not mandatory --many actuator tests currently use the controller in one way or another--, decoupling means the scope of tests (and their failure modes) is much narrower.

@arjo129 @caguero am I missing anything?

@arjo129
Copy link
Member

arjo129 commented Apr 8, 2022

Hey @hidmic this set of requirements is very well documented. I think you're spot on with the Mission Control tests I'll just throw my two cents of why every test was written the way it was just to check if we are not missing anything.

lrauv_description

The tests here verify that the vehicle model has hydrostatic stability. The vehicle has a slight offset between the center of buoyancy and center of mass to ensure a restoring moment. If both these tests pass we know the vehicle has hydrostatic stability.

  • TEST(Stability, NeutralBuoyancy) - Checks that the vehicle is actually neutrally buoyant. We need to be very careful here because small floating point error propagates.
  • TEST(Stability, RestoringMoment) - Makes sure that there is a restoring moment.We start the vehicle at a pitch and the vehicle should behave like a simple oscillator as the restoring moment will kick in and there is no hydrodynamics enabled to damp oscillations.

lrauv_ignition_plugins

Vehicle dynamics tests

This set of tests checks the behavior of our vehicle's physics in ignition.
We don't integrate with the controller here as these help us identify bugs
and short comings of our physics system.

  • test_buoyancy_action.cc - Unit test that checks the buoyancy engine can inflate and deflate and the vehicle heads in the correct direction (up/down). Known issue: Significant lateral translation due to parameterization of Fossen's equations.
  • test_controller.cc - Check tethyscomms plugin (in charge of bridging ignition to controller) actually loads.
  • test_drop_weight.cc - Tests that the drop weight actually works and causes the
    vehicle to float up.
  • test_elevator.cc- Check the elevator actually controls the direction of motion. Also checks the sign convention of the actuators. Elevators use the LiftDrag plugin.
  • test_hydrodynamics_equilibrium_velocity.cc - This checks the hydrodynamic plugin does two things. (I) It is rotation invariant (checks transforms are correct) and (II) the damping terms actually limit the vehicles top speed to the specified vehicle velocity of 1m/s in the forward direction. See https://github.com/osrf/lrauv/wiki. Does NOT use TethysComms plugin.
  • test_mass_shifter.cc - Checks the pitching without the controller. Useful for identifying stability issues in our hydrostatics.
  • test_rudder.cc - Similar to test_elevator.

Environmental

  • test_spawn.cc - Tests spawning mechanism.
  • test_state_msg.cc - Coordinate convention madness. @chapulina Would be a good person to ask about this.

Acoustics

  • test_comms.cc - Test acoustic communication is actually correct.
  • test_acoustic_comms_orientation.cc - Test range bearing plugin follows correct conventions. Range bearing builds on acoustic comm infra-structure.
  • test_range_bearing.cc - Can be safely deleted. Superceded by test_acoustic_comms_orientation

Integration tests

Note: These are often the flaky ones in part because of the async nature of the test. The goal is to catch errors while integrating (flipped axis, wrong assumptions).

  • test_mission_depth_vbs.cc - Vehicle should sink. There is some unavoidable lateral translation due to our parameterization of Fossen's equations.
  • test_mission_pitch_and_depth_mass_vbs.cc - Vehicle should sink. There is some unavoidable lateral translation but less than test_mission_depth_vbs.cc. The vehicle should maintain its pitch. Note: there is huge overshoot in the control of the depth in this mode. This is because of the slow fill rate of the Variable buoyancy system. We have checked this with @braanan who performed tank tests. There is both lateral translation and overshoot in real life as well.
  • test_mission_pitch_mass.cc - Vehicle should pitch forward. There will be some small oscillations once the vehicle is pitched. This is natural as the hydrodynamics won't damp all the oscillation. Keep in mind the simulation runs at 100x speed so small oscillations seem amplified. Has to be run at depth. The
    other thing this test checks is that our assumption about the axis of the mass shifter are the same as MBARI's. We have previously run into this issue.
  • test_mission_yoyo_circle.cc - Complex integration where the vehicle should "yoyo". This test should pass. It is flaky because of the async nature of these tests. You occassionally have to toggle the timeouts when a new image becomes available. This should start at the surface.

A couple of notes on Hydrodynamics and Hydrostatics

Most obvious instabilities are caused by poor hydrostatics. During the early phase of the project we ran into lots of trouble as we overlooked the restoring moment. Tests that catch this are ones involving pitch mass etc. It should not be a problem anymore since the autogenerated model and tests in lrauv_description gaurantee this stability.

With regards to Fossen's equations they are a linear approximation. The equations are far from ideal. Our parameterization breaks down when we reverse and move too much at a pitch. With regards to the pitch this is expected as the scaling is not uniform in all axis. For the most part this should not be a problem as in real life operations the vehicle moves like a torpedo forward.

@hidmic
Copy link
Collaborator Author

hidmic commented Apr 8, 2022

@arjo129 thanks for the write-up!

I definitely missed the lrauv_description tests.

Note: These are often the flaky ones in part because of the async nature of the test.

Certainly, though I've noticed that often times the simulation is run asynchronously when it could have been run synchronously. To cope with mission control being asynchronous we can only restrict what the vehicle does (e.g. having the vehicle go to surface after a mission ends forces us to somehow time it).

Note: there is huge overshoot in the control of the depth in this mode. This is because of the slow fill rate of the Variable buoyancy system. We have checked this with @braanan who performed tank tests. There is both lateral translation and overshoot in real life as well.

I cannot stress enough how important this is. We should document it. Also, we should probably extend timeouts to allow controller outputs to converge.

Our parameterization breaks down when we reverse and move too much at a pitch.

This may be worth a revisit, some day. Until then, we should document this in our hydrodynamics plugin documentation.


On a related note, I've noticed a circular dependency between both packages: lrauv_ignition_plugins needs lrauv_description for tests, lrauv_description needs lrauv_ignition_plugins for modelling (and thus why we have to source the install tree before testing).

How about adding an lrauv_system_tests package that depends on both and condenses all tests? The comments in this ticket would become the documentation for that package and its tests.

@caguero caguero mentioned this issue Apr 20, 2022
39 tasks
This was referenced Apr 20, 2022
@arjo129
Copy link
Member

arjo129 commented Apr 27, 2022

Thanks for doing this @hidmic. Solved by #195-#199

@arjo129 arjo129 closed this as completed Apr 27, 2022
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

No branches or pull requests

2 participants