Make sure robot.cancel_actions will return

[robin-mueller]

Currently, the Robot.cancel_actions() method will wait for the Robot.canceling_execution flag becoming False indefinitely when Robot.execute_action() fails to set self.executing_action = False before Robot.cancel_actions() (e.g. running in a separate thread) checks that in the if statement.

pyrobosim/pyrobosim/pyrobosim/core/robot.py

Lines 955 to 958 in c36d198

	if self.executing_action or self.executing_plan:
	self.canceling_execution = True
	while self.canceling_execution:
	time.sleep(0.1)

This is because Robot.canceling_execution is only updated by Robot.execute_plan() and not Robot.execute_action().

In this PR I suggest an alternative function design for canceling actions as well as plans using Robot.cancel_actions() that ensures it will complete when the respective execution callback returns.

[sea-bass]

I think there are cases in which self.executing_action is false but self.executing_nav is true.

Basically, there are actions that can directly tell a robot to follow a path without going through the high-level action interface.

It essentially involves running Robot.follow_path() directly:

pyrobosim/pyrobosim/pyrobosim/core/robot.py

Line 307 in 93803a2

def follow_path(

Which is done here:

pyrobosim/pyrobosim_ros/pyrobosim_ros/ros_interface.py

Line 554 in 93803a2

def robot_path_follow_callback(self, goal_handle, robot=None):

[robin-mueller]

You're right, I wasn't thinking of that. So is it better to add self.canceling_execution = False to the end of Robot.execute_actions()?

Since Robot.cancel_actions() is executed within a separate thread when using the ROS interface, it's difficult to make sure that self.canceling_execution is set to True before the execution callback resets the flag to False. If the cancellation thread does this after the execution callback performs self.canceling_execution = False, we enter an infinite loop.

[sea-bass]

Changes look good now.

Agree with you that cancellation is not thread safe, so there could be weird timing.

Maybe one easy, but not too hacky, option is to have a blocking version of cancellation (as is done now) for use outside ROS, and one that is not blocking which just sets a boolean value and returns immediately. So no infinitely running thread.

[robin-mueller]

Probably an async cancellation method would be nice to have yes, but I guess cancel_actions would be thread safe with this PR since the cancellation flags are set properly now.

At least the modified test and my application work correctly.

The crucial change is that cancel_actions now only waits for the respective flag if it hasn't been set before by another thread (e.g. the execution thread). I assume that with python's interpreter lock, checking a boolean flag can be considered thread safe.