More rigorous actions at startup

AstroStucky commented 1 year ago

Linked Issues:


Jira Ticket 🎟️	OCEANWATER-1181
Jira Ticket 🎟️	OCEANWATER-1182
Jira Ticket 🎟️	OCEANWATER-1186

Summary of Changes

Any publishers in action servers now wait for at least one node to have subscribed to their topic before publishing a message. For CameraCapture and any pan/tilt actions not waiting for would break the action if it was called too soon.
Subscriber classes in subscribers.py now block for some timeout or until they have receive their expected message.
The Subscriber in PanTiltMixin was replaced with the more rigorous topic subscription handler mentioned in the above bullet.
Work arounds in automated tests were removed, because the problems they work around were fixed.

Test

Run the following tests. Everything should have passed when they conclude. rostest ow_sim_tests test_arm_move_joint.test rostest ow_sim_tests test_arm_move_joints.test rostest ow_sim_tests test_camera_capture.test rostest ow_sim_tests test_camera_set_exposure.test rostest ow_sim_tests test_pan_tilt_move_cartesian.test rostest ow_sim_tests test_pan_tilt_move_joints.test

AstroStucky commented 1 year ago

I had to push an update because I noticed a potential bug. Sorry for the last second changes. This is good to review now.

kmdalal commented 1 year ago

Is it easy to interactively reproduce any of the problems that this PR fixes? If so, could you briefly describe how?

AstroStucky commented 1 year ago

Yes, easily enough. Call any of the following actions before launching the sim.

rosrun ow_lander arm_move_joint.py
rosrun ow_lander camera_capture.py
rosrun ow_lander pan_tilt_move_cartesian.py
rosrun ow_lander pan_tilt_move_joints.py

You will see each one fail in a variety of ways. One of them even brings down the action servers with it. If you do the same thing in this branch, the actions will simply execute when the server becomes ready

Calling an action before the start of the sim probably sounds like an unlikely use case, but it's not a use case so much as a method to force what I consider an inter-process timing bug.

kmdalal commented 1 year ago

The first rostest had failures and an error. Log attached here: failure1.log

kmdalal commented 1 year ago

My repeat of failed test 1 (logged in previous comment) passed, so not sure if you want to consider that first failure an unrelated issue. All the other tests passed on first try!

AstroStucky commented 1 year ago

The first rostest had failures and an error. Log attached here: failure1.log

I am glad you encountered this! It indicates I need to increase the timeout. I have just done that and reran every test at least once, and reran test_arm_move_joint.test 4 times total. They all passed.

Feel free to retest as much as you feel necessary, but I think I solve this problem.

AstroStucky commented 1 year ago

Just reran every test once again following the merge of noetic-devel, and they all still pass.

nasa / ow_simulator