fix(api): do not handle smoothie alarms from halt() (#3721)

When changing the smoothie driver in 628c6c4 to
be more robust and have better locking, the changes to the main comms sequence
around write_with_retries and handle_return started trying to handle and recover
from _all_ smoothie alarms, including the one generated by a call to
robot.halt() toggling the estop gpios. This interferes with (read: breaks
catastrophically) protocol cancel, which relies on this sequence:

run() rpc call         smoothie      stop rpc call
  |                       |                |
  smoothie cmd start -> executing          |
  |                       | (estop) <-   halt
  smoothie cmd raises <- ALARM             |
  |                       |                |
  exc raised, call done   |                |
  (cancel)                |                |
                         recover     <-   M999
                         executing   <-  home...

and if the smoothie cmd in run handles the alarm and tries to recover, then it
never cancels the protocol.

To fix this, raise a different exception that the normal error handling
machinery doesn't catch and therefore propagates all the way up.

api/src/opentrons/drivers/smoothie_drivers/driver_3_0.py

@@ -121,6 +121,19 @@ def __str__(self):
         return f'SmoothieError: {self.command} returned {self.ret_code}'
 
 
+class SmoothieAlarm(Exception):
+    def __init__(self, ret_code: str = None, command: str = None) -> None:
+        self.ret_code = ret_code
+        self.command = command
+        super().__init__()
+
+    def __repr__(self):
+        return f'<SmoothieAlarm: {self.ret_code} from {self.command}>'
+
+    def __str__(self):
+        return f'SmoothieAlarm: {self.command} returned {self.ret_code}'
+
+
 class ParseError(Exception):
     pass
 
@@ -349,6 +362,7 @@ def __exit__(self, *args, **kwargs):
                     pass
 
             self._serial_lock = DummyLock()
+        self._is_hard_halting = Event()
 
     @property
     def homed_position(self):
@@ -938,20 +952,38 @@ def _send_command_unsynchronized(self,
             command + SMOOTHIE_COMMAND_TERMINATOR,
             5.0, DEFAULT_COMMAND_RETRIES)
         cmd_ret = self._remove_unwanted_characters(command, cmd_ret)
-        self._handle_return(cmd_ret, GCODES['HOME'] in command)
+        self._handle_return(cmd_ret)
         wait_ret = serial_communication.write_and_return(
             GCODES['WAIT'] + SMOOTHIE_COMMAND_TERMINATOR,
             SMOOTHIE_ACK, self._connection, timeout=12000)
         wait_ret = self._remove_unwanted_characters(
             GCODES['WAIT'], wait_ret)
-        self._handle_return(wait_ret, GCODES['HOME'] in command)
+        self._handle_return(wait_ret)
         return cmd_ret.strip()
 
-    def _handle_return(self, ret_code: str, was_home: bool):
-        # Smoothieware returns error state if a switch was hit while moving
-        if (ERROR_KEYWORD in ret_code.lower()) or \
-                (ALARM_KEYWORD in ret_code.lower()):
-            raise SmoothieError(ret_code)
+    def _handle_return(self, ret_code: str):
+        """ Check the return string from smoothie for an error condition.
+
+        Usually raises a SmoothieError, which can be handled by the error
+        handling in write_with_retries. However, if the hard halt line has
+        been set, we need to catch that halt and _not_ handle it, since it
+        is used for things like cancelling protocols and needs to be
+        handled elsewhere. In that case, we raise SmoothieAlarm, which isn't
+        (and shouldn't be) handled by the normal error handling.
+        """
+        is_alarm = ALARM_KEYWORD in ret_code.lower()
+        is_error = ERROR_KEYWORD in ret_code.lower()
+        if self._is_hard_halting.is_set():
+            # This is the alarm from setting the hard halt
+            if is_alarm:
+                self._is_hard_halting.clear()
+                raise SmoothieAlarm(ret_code)
+            elif is_error:
+                # this would be a race condition
+                raise SmoothieError(ret_code)
+        else:
+            if is_alarm or is_error:
+                raise SmoothieError(ret_code)
 
     def _remove_unwanted_characters(self, command, response):
         # smoothieware can enter a weird state, where it repeats back
@@ -1553,6 +1585,7 @@ def _smoothie_hard_halt(self):
         if self.simulating:
             pass
         else:
+            self._is_hard_halting.set()
             gpio.set_low(gpio.OUTPUT_PINS['HALT'])
             sleep(0.25)
             gpio.set_high(gpio.OUTPUT_PINS['HALT'])

api/tests/opentrons/drivers/test_driver.py

@@ -4,6 +4,8 @@
 
 from tests.opentrons.conftest import fuzzy_assert
 from opentrons.config.robot_configs import DEFAULT_GANTRY_STEPS_PER_MM
+from opentrons.drivers import serial_communication
+from opentrons.drivers.smoothie_drivers import driver_3_0
 
 
 def position(x, y, z, a, b, c):
@@ -886,8 +888,6 @@ def test_pause_in_protocol(model):
 
 @pytest.mark.api1_only
 def test_send_command_with_retry(model, monkeypatch):
-    from opentrons.drivers import serial_communication
-
     robot = model.robot
     robot._driver.simulating = False
 
@@ -1022,3 +1022,26 @@ def send_command_mock(self, command, timeout=None):
     # from pprint import pprint
     # pprint(current_log)
     assert current_log == expected
+
+
+@pytest.mark.api1_only
+def test_alarm_unhandled(model, monkeypatch):
+    driver = model.robot._driver
+    driver.simulating = False
+    killmsg = 'ALARM: Kill button pressed - reset or M999 to continue\r\n'
+
+    def fake_write_and_return(cmdstr, ack, conn, timeout=None):
+
+        return cmdstr + killmsg
+
+    monkeypatch.setattr(serial_communication, 'write_and_return',
+                        fake_write_and_return)
+    assert serial_communication.write_and_return is fake_write_and_return
+    driver.move({'X': 0})
+
+    driver._is_hard_halting.set()
+
+    with pytest.raises(driver_3_0.SmoothieAlarm):
+        driver.move({'X': 25})
+
+    assert not driver._is_hard_halting.is_set()