Repository: robojackets/robocup-software
Issue: Restarting single robot behaviors isn't always the best thing to do

In coordinated pass, if the receiver is temporarily assigned to a different robot (which happens occasionally), the receiver gets restarted, which messes up coordinated pass transitions, causing the pass to hang indefinitely.

Restarting a single robot behavior when a new robot is re-assigned is often the correct thing to do, but not always.  This needs some thought.


Changed files:
soccer/gameplay/skills/pass_receive.py, soccer/gameplay/skills/pivot_kick.py, soccer/gameplay/single_robot_composite_behavior.py

diff --git a/soccer/gameplay/single_robot_composite_behavior.py b/soccer/gameplay/single_robot_composite_behavior.py
index 0fe8e6243c..bad8fd7660 100644
--- a/soccer/gameplay/single_robot_composite_behavior.py
+++ b/soccer/gameplay/single_robot_composite_behavior.py
@@ -10,6 +10,27 @@ import role_assignment
 # Note: The method-resolution-order in python dictates that as we have it listed below, SingleRobotBehavior methods take precedence over CompositeBehavior methods
 class SingleRobotCompositeBehavior(single_robot_behavior.SingleRobotBehavior,
                                    composite_behavior.CompositeBehavior):
+    ## Constructor.
+    # If the behavior should not be restarted when it is assigned to a different robot, then autorestart must be false
+    # @param continuous should the behavior runs until it is manually stopped
+    # @param autorestart function governing the behavior's restarting itself when its robot is switched
+    def __init__(self,
+                 continuous=False,
+                 autorestart= lambda: True):
+        single_robot_behavior.SingleRobotBehavior.__init__(
+            self,
+            continuous=continuous)
+        composite_behavior.CompositeBehavior.__init__(self,
+                                                      continuous=continuous)
+        self.autorestart = autorestart
+
+    @property
+    def autorestart(self):
+        return self._autorestart
+
+    @autorestart.setter
+    def autorestart(self, value):
+        self._autorestart = value
 
     ## we over-ride this to enforce the rule that there can't be more than one subbehavior
     def add_subbehavior(self, bhvr, name, required=True, priority=100):
@@ -27,7 +48,7 @@ class SingleRobotCompositeBehavior(single_robot_behavior.SingleRobotBehavior,
             reqs = composite_behavior.CompositeBehavior.role_requirements(self)
             if self.robot != None:
                 for req in role_assignment.iterate_role_requirements_tree_leaves(
-                        reqs):
+                    reqs):
                     req.previous_shell_id = self.robot.shell_id()
 
             return reqs
@@ -44,19 +65,18 @@ class SingleRobotCompositeBehavior(single_robot_behavior.SingleRobotBehavior,
             # extract robot from the one leaf in the tree
             # we don't know how deep the tree is, which is why we use the tree leaf iterator
             for assignment_tuple in role_assignment.iterate_role_requirements_tree_leaves(
-                    assignments):
+                assignments):
                 self.robot = assignment_tuple[1]
         else:
             single_robot_behavior.SingleRobotBehavior.assign_roles(self,
                                                                    assignments)
 
-        # Most single robot composite behaviors assume (rightly so) that the robot they're
-        # dealing with won't change. PivotKick for example runs the Capture skill, then
-        # the Aim skill to do its stuff. It'd be pretty weird if the robot executing Aim
-        # was different from the one that just ran Capture. Here we just restart the behavior
-        # If it gets assigned a new robot.
+        # Sometimes the robot executing a single_robot_composite_behavior changes in the
+        # middle of the behavior. For some plays, this means we shouild restart the whole
+        # behavior for the new robot (autorestart = True). For others, it is more important to continue the new
+        # robot where the old robot left off (autorestart = False). 
         if oldBot != None and self.robot != None and oldBot.shell_id(
-        ) != self.robot.shell_id():
+        ) != self.robot.shell_id() and self.autorestart():
             logging.info(
                 "SingleRobotCompositeBehavior: robot changed, restarting behavior")
             self.restart()
diff --git a/soccer/gameplay/skills/pass_receive.py b/soccer/gameplay/skills/pass_receive.py
index 55af6a95af..33fbff3b6f 100644
--- a/soccer/gameplay/skills/pass_receive.py
+++ b/soccer/gameplay/skills/pass_receive.py
@@ -46,7 +46,8 @@ class PassReceive(single_robot_composite_behavior.SingleRobotCompositeBehavior
         receiving = 3
 
     def __init__(self, captureFunction=(lambda: skills.capture.Capture())):
-        super().__init__(continuous=False)
+        super().__init__(continuous=False,
+                         autorestart= self.autorestart)
 
         self.ball_kicked = False
         self._target_pos = None
@@ -88,6 +89,11 @@ class PassReceive(single_robot_composite_behavior.SingleRobotCompositeBehavior
             lambda: self.subbehavior_with_name('capture').state == behavior.Behavior.State.failed or self.check_failure() or time.time() - self.kicked_time > PassReceive.DesperateTimeout,
             'ball missed :(')
 
+    ## This function will detirmine how this behavior reacts to its robot changing. If the ball is kicked and the robot switches, restarting causes all sorts of hitches, so we prevent this.
+    # Note: False means the behavior will not restart
+    def autorestart():
+        return not self.ball_kicked
+
     ## set this to True to let the receiver know that the pass has started and the ball's in motion
     # Default: False
     @property
diff --git a/soccer/gameplay/skills/pivot_kick.py b/soccer/gameplay/skills/pivot_kick.py
index 04d34e3016..d38902b46a 100644
--- a/soccer/gameplay/skills/pivot_kick.py
+++ b/soccer/gameplay/skills/pivot_kick.py
@@ -22,7 +22,9 @@ class PivotKick(single_robot_composite_behavior.SingleRobotCompositeBehavior,
         kicking = 4
 
     def __init__(self):
-        super().__init__()
+        single_robot_composite_behavior.SingleRobotCompositeBehavior.__init__(
+            self)
+        skills._kick._Kick.__init__(self)
 
         for state in PivotKick.State:
             self.add_state(state, behavior.Behavior.State.running)