Skip to content

Commit

Permalink
refactor(api): refactor smoothie and module response parsing (#7571)
Browse files Browse the repository at this point in the history
* add to driver utils.

* mag deck driver uses new methods

* smoothie driver uses command builder

* thermocycler uses parse temperature functions from utils.

* remove redundant parse functions.

* use static type checking to remove type checks.
device information response only includes the three keys.

* remove confusing code in update_temperature.

* Update api/tests/opentrons/drivers/test_utils.py

Co-authored-by: Sanniti Pimpley <[email protected]>

* add test cases to temp and plate temp parsers that show we ignore extra values.

Co-authored-by: Sanniti Pimpley <[email protected]>
  • Loading branch information
amitlissack and sanni-t authored Apr 30, 2021
1 parent e8f2ae9 commit e9e7051
Show file tree
Hide file tree
Showing 10 changed files with 328 additions and 272 deletions.
104 changes: 14 additions & 90 deletions api/src/opentrons/drivers/mag_deck/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
from threading import Event, Lock
from time import sleep
from typing import Dict, Optional, Mapping, Tuple

from opentrons.drivers.utils import ParseError
from serial.serialutil import SerialException # type: ignore

from opentrons.drivers import serial_communication
from opentrons.drivers import serial_communication, utils
from opentrons.drivers.serial_communication import SerialNoResponse

"""
Expand Down Expand Up @@ -58,100 +60,22 @@ class MagDeckError(Exception):
pass


class ParseError(Exception):
pass


def _parse_string_value_from_substring(substring) -> str:
"""
Returns the ascii value in the expected string "N:aa11bb22", where "N" is
the key, and "aa11bb22" is string value to be returned
"""
try:
value = substring.split(':')[1]
return str(value)
except (ValueError, IndexError, TypeError, AttributeError):
log.exception('Unexpected arg to _parse_string_value_from_substring:')
raise ParseError(
'Unexpected arg to _parse_string_value_from_substring: {}'.format(
substring))


def _parse_number_from_substring(substring) -> Optional[float]:
"""
Returns the number in the expected string "N:12.3", where "N" is the
key, and "12.3" is a floating point value
For the magnetic module's height response like "height:12.34" "
"height:none" should return a None value
"""
try:
value = substring.split(':')[1]
if value.strip().lower() == 'none':
return None
return round(float(value), GCODE_ROUNDING_PRECISION)
except (ValueError, IndexError, TypeError, AttributeError):
log.exception('Unexpected argument to _parse_number_from_substring:')
raise ParseError(
'Unexpected argument to _parse_number_from_substring: {}'.format(
substring))


def _parse_key_from_substring(substring) -> str:
"""
Returns the key in the expected string "N:12.3", where "N" is the
key, and "12.3" is a floating point value
"""
try:
return substring.split(':')[0]
except (ValueError, IndexError, TypeError, AttributeError):
log.exception('Unexpected argument to _parse_key_from_substring:')
raise ParseError(
'Unexpected argument to _parse_key_from_substring: {}'.format(
substring))


def _parse_device_information(device_info_string) -> Dict[str, str]:
"""
Parse the magnetic module's device information response.
Example response from magnetic module:
"serial:aa11 model:bb22 version:cc33"
"""
error_msg = 'Unexpected argument to _parse_device_information: {}'.format(
device_info_string)
if not device_info_string or \
not isinstance(device_info_string, str):
raise ParseError(error_msg)
parsed_values = device_info_string.strip().split(' ')
if len(parsed_values) < 3:
log.error(error_msg)
raise ParseError(error_msg)
res = {
_parse_key_from_substring(s): _parse_string_value_from_substring(s)
for s in parsed_values[:3]
}
for key in ['model', 'version', 'serial']:
if key not in res:
raise ParseError(error_msg)
return res


def _parse_distance_response(distance_string) -> float:
"""
Parse responses of 'GET_PLATE_HEIGHT' & 'GET_CURRENT_POSITION'
Example response of-
GET_PLATE_HEIGHT: "height:12.34"
GET_CURRENT_POSITION: "Z:12.34"
"""
err_msg = 'Unexpected argument to _parse_distance_response: {}'.format(
distance_string)
if not distance_string or \
not isinstance(distance_string, str):
raise ParseError(err_msg)
if 'Z' not in distance_string and 'height' not in distance_string:
raise ParseError(err_msg)
return _parse_number_from_substring( # type: ignore
distance_string.strip()) # (preconditions checked above)
data = utils.parse_key_values(distance_string)
val = data.get('Z', data.get('height'))
if val is None:
raise utils.ParseError(
error_message='Unexpected argument to _parse_distance_response',
parse_source=distance_string
)

return utils.parse_number(val, GCODE_ROUNDING_PRECISION)


class SimulatingDriver:
Expand Down Expand Up @@ -431,10 +355,10 @@ def _update_mag_position(self) -> str:
self._mag_position = distance
return ''

def _recursive_get_info(self, retries) -> dict:
def _recursive_get_info(self, retries) -> Dict[str, str]:
try:
device_info = self._send_command(GCODES['DEVICE_INFO'])
return _parse_device_information(device_info)
return utils.parse_device_information(device_info)
except ParseError as e:
retries -= 1
if retries <= 0:
Expand Down
66 changes: 21 additions & 45 deletions api/src/opentrons/drivers/smoothie_drivers/driver_3_0.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
from opentrons.drivers import serial_communication
from opentrons.drivers.types import MoveSplits
from opentrons.drivers.utils import (
AxisMoveTimestamp, parse_key_from_substring, parse_number_from_substring)
AxisMoveTimestamp, parse_key_values, parse_number, parse_optional_number
)
from opentrons.drivers.rpi_drivers.gpio_simulator import SimulatingGPIOCharDev
from opentrons.drivers.rpi_drivers.dev_types import GPIODriverLike
from opentrons.system import smoothie_update
Expand Down Expand Up @@ -173,36 +174,16 @@ class ParseError(Exception):
pass


def _parse_number_from_substring(smoothie_substring):
"""
Returns the number in the expected string "N:12.3", where "N" is the
axis, and "12.3" is a floating point value for the axis' position
"""
return parse_number_from_substring(smoothie_substring,
GCODE_ROUNDING_PRECISION)


def _parse_axis_from_substring(smoothie_substring):
"""
Returns the axis in the expected string "N:12.3", where "N" is the
axis, and "12.3" is a floating point value for the axis' position
"""
return parse_key_from_substring(
smoothie_substring
).title() # upper 1st letter


def _parse_position_response(raw_axis_values):
parsed_values = raw_axis_values.strip().split(' ')
if len(parsed_values) < 8:
msg = 'Unexpected response in _parse_position_response: {}'.format(
raw_axis_values)
def _parse_position_response(raw_axis_values) -> Dict[str, float]:
parsed_values = parse_key_values(raw_axis_values)
if len(parsed_values) < 6:
msg = f'Unexpected response in _parse_position_response: {raw_axis_values}'
log.error(msg)
raise ParseError(msg)

data = {
_parse_axis_from_substring(s): _parse_number_from_substring(s)
for s in parsed_values[2:] # remove first two items ('ok', 'MCS:')
k.title(): parse_number(v, GCODE_ROUNDING_PRECISION)
for k, v in parsed_values.items()
}
return data

Expand Down Expand Up @@ -259,11 +240,11 @@ def _parse_switch_values(raw_switch_values: str) -> Dict[str, bool]:
if 'Probe: ' in raw_switch_values:
raw_switch_values = raw_switch_values.replace('Probe: ', 'Probe:')

parsed_values = raw_switch_values.strip().split(' ')
parsed_values = parse_key_values(raw_switch_values)
res = {
_parse_axis_from_substring(s): bool(_parse_number_from_substring(s))
for s in parsed_values
if any([n in s for n in ['max', 'Probe']])
k.title(): bool(parse_optional_number(v, rounding_val=GCODE_ROUNDING_PRECISION))
for (k, v) in parsed_values.items()
if any(n in k for n in ['max', 'Probe'])
}
# remove the extra "_max" character from each axis key in the dict
res = {
Expand All @@ -279,24 +260,19 @@ def _parse_switch_values(raw_switch_values: str) -> Dict[str, bool]:

def _parse_homing_status_values(raw_homing_status_values):
"""
Parse the Smoothieware response to a G28.6 command (homing-status)
A "1" means it has been homed, and "0" means it has not been homed
Parse the Smoothieware response to a G28.6 command (homing-status)
A "1" means it has been homed, and "0" means it has not been homed
Example response after homing just X axis:
"X:1 Y:0 Z:0 A:0 B:0 C:0"
Example response after homing just X axis:
"X:1 Y:0 Z:0 A:0 B:0 C:0"
returns: dict
Key is axis, value is True if the axis needs to be homed
returns: dict
Key is axis, value is True if the axis needs to be homed
"""
if not raw_homing_status_values or \
not isinstance(raw_homing_status_values, str):
raise ParseError(
'Unexpected argument to _parse_homing_status_values: {}'.format(
raw_homing_status_values))
parsed_values = raw_homing_status_values.strip().split(' ')
parsed_values = parse_key_values(raw_homing_status_values)
res = {
_parse_axis_from_substring(s): bool(_parse_number_from_substring(s))
for s in parsed_values
k.title(): bool(parse_number(v, GCODE_ROUNDING_PRECISION))
for k, v in parsed_values.items()
}
if len(list(AXES) & res.keys()) != 6:
raise ParseError(
Expand Down
56 changes: 20 additions & 36 deletions api/src/opentrons/drivers/temp_deck/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@

from opentrons.drivers import serial_communication, utils
from opentrons.drivers.serial_communication import SerialNoResponse
from opentrons.drivers.types import Temperature

'''
"""
- Driver is responsible for providing an interface for the temp-deck
- Driver is the only system component that knows about the temp-deck's GCODES
or how the temp-deck communications
- Driver is NOT responsible interpreting the temperatures or states in any way
or knowing anything about what the device is being used for
'''
"""

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -118,7 +119,7 @@ def __init__(self, config={}):
self._connection = None
self._config = config

self._temperature = {'current': 25, 'target': None}
self._temperature = Temperature(current=25, target=None)
self._update_thread = None
self._port = None
self._lock = None
Expand Down Expand Up @@ -178,7 +179,7 @@ async def set_temperature(self, celsius) -> str:
'{0} S{1}'.format(GCODES['SET_TEMP'], celsius))
except (TempDeckError, SerialException, SerialNoResponse) as e:
return str(e)
self._temperature.update({'target': celsius})
self._temperature.target = celsius
while self.status != 'holding at target':
await asyncio.sleep(0.1)
return ''
Expand All @@ -189,26 +190,13 @@ def start_set_temperature(self, celsius) -> str:
utils.TEMPDECK_GCODE_ROUNDING_PRECISION)
self._send_command(
'{0} S{1}'.format(GCODES['SET_TEMP'], celsius))
self._temperature.update({'target': celsius})
self._temperature.target = celsius
return ''

# NOTE: only present to support apiV1 non-blocking by default behavior
def legacy_set_temperature(self, celsius) -> str:
self.run_flag.wait()
celsius = round(float(celsius),
utils.TEMPDECK_GCODE_ROUNDING_PRECISION)
try:
self._send_command(
'{0} S{1}'.format(GCODES['SET_TEMP'], celsius))
except (TempDeckError, SerialException, SerialNoResponse) as e:
return str(e)
self._temperature.update({'target': celsius})
return ''

def update_temperature(self, default=None) -> str:
def update_temperature(self) -> str:
if self._update_thread and self._update_thread.is_alive():
updated_temperature = default or self._temperature.copy()
self._temperature.update(updated_temperature)
# No need to do anything. The updater is already running.
pass
else:
def _update():
try:
Expand All @@ -223,17 +211,17 @@ def _update():
return ''

@property
def target(self) -> Optional[int]:
return self._temperature.get('target')
def target(self) -> Optional[float]:
return self._temperature.target

@property
def temperature(self) -> int:
return self._temperature['current'] # type: ignore
def temperature(self) -> float:
return self._temperature.current # type: ignore

def _get_status(self) -> str:
# Separate function for testability
current = self._temperature['current']
target = self._temperature.get('target')
current = self._temperature.current
target = self._temperature.target
delta = 0.7
if target:
diff = target - current # type: ignore
Expand All @@ -251,7 +239,7 @@ def status(self) -> str:
return self._get_status()

def get_device_info(self) -> Mapping[str, str]:
'''
"""
Queries Temp-Deck for its build version, model, and serial number
returns: dict
Expand All @@ -266,7 +254,7 @@ def get_device_info(self) -> Mapping[str, str]:
Example input from Temp-Deck's serial response:
"serial:aa11bb22 model:aa11bb22 version:aa11bb22"
'''
"""
return self._get_info(DEFAULT_COMMAND_RETRIES)

def pause(self):
Expand Down Expand Up @@ -301,18 +289,15 @@ def _connect_to_port(self, port=None):
raise SerialException(error_msg)

def _wait_for_ack(self):
'''
"""
This methods writes a sequence of newline characters, which will
guarantee temp-deck responds with 'ok\r\nok\r\n' within 1 seconds
'''
"""
self._send_command('\r\n', timeout=DEFAULT_TEMP_DECK_TIMEOUT)

# Potential place for command optimization (buffering, flushing, etc)
def _send_command(
self, command, timeout=DEFAULT_TEMP_DECK_TIMEOUT, tag=None):
"""
"""
assert self._lock, 'not connected'
with self._lock:
command_line = command + ' ' + TEMP_DECK_COMMAND_TERMINATOR
Expand Down Expand Up @@ -353,9 +338,8 @@ def _recursive_update_temperature(self, retries):
res = self._send_command(
GCODES['GET_TEMP'],
tag=f'tempdeck {id(self)} rut')
res = utils.parse_temperature_response(
self._temperature = utils.parse_temperature_response(
res, utils.TEMPDECK_GCODE_ROUNDING_PRECISION)
self._temperature.update(res)
return None
except utils.ParseError as e:
retries -= 1
Expand Down
Loading

0 comments on commit e9e7051

Please sign in to comment.