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()