feat(api): Pipette id included in GET /pipettes

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

@@ -2,7 +2,7 @@
 import logging
 from time import sleep
 from threading import Event
-from typing import Dict
+from typing import Dict, Optional
 
 from serial.serialutil import SerialException
 
@@ -349,29 +349,27 @@ def _recursive_update_position(retries):
 
         self._update_position(updated_position)
 
-    def read_pipette_id(self, mount):
+    def read_pipette_id(self, mount) -> Optional[str]:
         '''
-        Reads in an attached pipette's UUID
-        The UUID is unique to this pipette, and is a string of unknown length
+        Reads in an attached pipette's ID
+        The ID is unique to this pipette, and is a string of unknown length
 
-        mount:
-            String (str) with value 'left' or 'right'
+        :param mount: string with value 'left' or 'right'
+        :return id string, or None
         '''
+        res: Optional[str] = None
         if self.simulating:
             res = '1234567890'
         else:
             res = self._read_from_pipette(GCODES['READ_INSTRUMENT_ID'], mount)
-        if res:
-            ret = {'pipette_id': res}
-        else:
-            ret = {'message': 'Error: Pipette ID read failed'}
-        return ret
+        return res
 
-    def read_pipette_model(self, mount):
+    def read_pipette_model(self, mount) -> Optional[str]:
         '''
         Reads an attached pipette's MODEL
         The MODEL is a unique string for this model of pipette
 
+        :param mount: string with value 'left' or 'right'
         :return model string, or None
         '''
         if self.simulating:
@@ -393,8 +391,8 @@ def read_pipette_model(self, mount):
 
     def write_pipette_id(self, mount, data_string):
         '''
-        Writes to an attached pipette's UUID memory location
-        The UUID is unique to this pipette, and is a string of unknown length
+        Writes to an attached pipette's ID memory location
+        The ID is unique to this pipette, and is a string of unknown length
 
         NOTE: To enable write-access to the pipette, it's button must be held
 
@@ -959,7 +957,7 @@ def _setup(self):
         self.pop_axis_max_speed()
         self.pop_speed()
 
-    def _read_from_pipette(self, gcode, mount):
+    def _read_from_pipette(self, gcode, mount) -> Optional[str]:
         '''
         Read from an attached pipette's internal memory. The gcode used
         determines which portion of memory is read and returned.
@@ -992,6 +990,7 @@ def _read_from_pipette(self, gcode, mount):
                 return _byte_array_to_ascii_string(res[mount])
         except (ParseError, AssertionError, SmoothieError):
             pass
+        return None
 
     def _write_to_pipette(self, gcode, mount, data_string):
         '''

api/src/opentrons/hardware_control/__init__.py

@@ -112,7 +112,7 @@ def build_hardware_controller(
     @classmethod
     def build_hardware_simulator(
             cls,
-            attached_instruments: Dict[top_types.Mount, str] = None,
+            attached_instruments: Dict[top_types.Mount, Dict[str, Optional[str]]] = None,  # noqa E501
             attached_modules: List[str] = None,
             config: robot_configs.robot_config = None,
             loop: asyncio.AbstractEventLoop = None) -> 'API':
@@ -167,15 +167,17 @@ async def cache_instruments(self):
         """
         self._log.info("Updating instrument model cache")
         for mount in top_types.Mount:
-            instrument_model = self._backend.get_attached_instrument(mount)
-            if instrument_model:
-                self._attached_instruments[mount] = Pipette(instrument_model)
+            instrument_data = self._backend.get_attached_instrument(mount)
+            if instrument_data['model']:
+                self._attached_instruments[mount] = Pipette(
+                    instrument_data['model'], instrument_data['id'])
         mod_log.info("Instruments found:{}".format(self._attached_instruments))
 
     @property
     def attached_instruments(self):
         configs = ['name', 'min_volume', 'max_volume',
-                   'aspirate_flow_rate', 'dispense_flow_rate']
+                   'aspirate_flow_rate', 'dispense_flow_rate',
+                   'pipette_id']
         instruments = {top_types.Mount.LEFT: {},
                        top_types.Mount.RIGHT: {}}
         for mount in top_types.Mount:

api/src/opentrons/hardware_control/controller.py

@@ -82,8 +82,10 @@ def home(self, axes: List[Axis] = None) -> Dict[str, float]:
             args = tuple()
         return self._smoothie_driver.home(*args)
 
-    def get_attached_instrument(self, mount) -> Optional[str]:
-        return self._smoothie_driver.read_pipette_model(mount.name.lower())
+    def get_attached_instrument(self, mount) -> Dict[str, Optional[str]]:
+        model = self._smoothie_driver.read_pipette_model(mount.name.lower())
+        _id = self._smoothie_driver.read_pipette_id(mount.name.lower())
+        return {'model': model, 'id': _id}
 
     def set_active_current(self, axis, amp):
         self._smoothie_driver.set_active_current({axis.name: amp})

api/src/opentrons/hardware_control/pipette.py

@@ -13,11 +13,12 @@ class Pipette:
     control API. Its only purpose is to gather state.
     """
 
-    def __init__(self, model: str) -> None:
+    def __init__(self, model: str, pipette_id: str) -> None:
         self._config = pipette_config.load(model)
         self._name = model
         self._current_volume = 0.0
         self._has_tip = False
+        self._pipette_id = pipette_id
 
     @property
     def config(self) -> pipette_config.pipette_config:
@@ -30,6 +31,10 @@ def update_config_item(self, elem_name: str, elem_val: Any):
     def name(self) -> str:
         return self._name
 
+    @property
+    def pipette_id(self) -> str:
+        return self._pipette_id
+
     @property
     def critical_point(self) -> Point:
         """ The vector from the pipette's origin to its critical point """
@@ -98,5 +103,6 @@ def as_dict(self) -> Dict[str, Union[str, float]]:
         config_dict = self.config._asdict()
         config_dict.update({'current_volume': self.current_volume,
                             'name': self.name,
+                            'pipette_id': self.pipette_id,
                             'has_tip': self.has_tip})
         return config_dict

api/src/opentrons/hardware_control/simulator.py

@@ -11,10 +11,11 @@ class Simulator:
     hardware actions. It is suitable for use on a dev machine or on
     a robot with no smoothie connected.
     """
-    def __init__(self,
-                 attached_instruments: Dict[types.Mount, str],
-                 attached_modules: List[str],
-                 config, loop) -> None:
+    def __init__(
+            self,
+            attached_instruments: Dict[types.Mount, Dict[str, Optional[str]]],
+            attached_modules: List[str],
+            config, loop) -> None:
         self._config = config
         self._loop = loop
         self._attached_instruments = attached_instruments
@@ -29,8 +30,9 @@ def home(self, axes: List[Axis] = None) -> Dict[str, float]:
         # driver_3_0-> HOMED_POSITION
         return {'X': 418, 'Y': 353, 'Z': 218, 'A': 218, 'B': 19, 'C': 19}
 
-    def get_attached_instrument(self, mount) -> Optional[str]:
-        return self._attached_instruments.get(mount, None)
+    def get_attached_instrument(self, mount) -> Dict[str, Optional[str]]:
+        return self._attached_instruments.get(
+            mount, {'mount': None, 'id': None})
 
     def set_active_current(self, axis, amp):
         pass

api/src/opentrons/legacy_api/robot/robot.py

@@ -109,7 +109,8 @@ def __init__(self, config=None):
 
         self.INSTRUMENT_DRIVERS_CACHE = {}
         self._instruments = {}
-        self.model_by_mount = {'left': None, 'right': None}
+        self.model_by_mount = {'left': {'model': None, 'id': None},
+                               'right': {'model': None, 'id': None}}
 
         # TODO (artyom, 09182017): once protocol development experience
         # in the light of Session concept is fully fleshed out, we need
@@ -227,10 +228,43 @@ def reset(self):
         return self
 
     def cache_instrument_models(self):
+        """
+        Queries Smoothie for the model and ID strings of attached pipettes, and
+        saves them so they can be reported without querying Smoothie again (as
+        this could interrupt a command if done during a run or other movement).
+
+        Shape of return dict should be:
+
+        ```
+        {
+          "left": {
+            "model": "<model_string>" or None,
+            "id": "<pipette_id_string>" or None
+          },
+          "right": {
+            "model": "<model_string>" or None,
+            "id": "<pipette_id_string>" or None
+          }
+        }
+        ```
+
+        :return: a dict with pipette data (shape described above)
+        """
         log.debug("Updating instrument model cache")
         for mount in self.model_by_mount.keys():
-            self.model_by_mount[mount] = self._driver.read_pipette_model(mount)
-            log.debug("{}: {}".format(mount, self.model_by_mount[mount]))
+            model_value = self._driver.read_pipette_model(mount)
+            if model_value:
+                id_response = self._driver.read_pipette_id(mount)
+            else:
+                id_response = None
+            self.model_by_mount[mount] = {
+                'model': model_value,
+                'id': id_response
+            }
+            log.debug("{}: {} [{}]".format(
+                mount,
+                self.model_by_mount[mount]['model'],
+                self.model_by_mount[mount]['id']))
 
     def turn_on_button_light(self):
         self._driver.turn_on_blue_button_light()
@@ -800,7 +834,8 @@ def get_attached_pipettes(self):
         left_data = {
                 'mount_axis': 'z',
                 'plunger_axis': 'b',
-                'model': self.model_by_mount['left'],
+                'model': self.model_by_mount['left']['model'],
+                'id': self.model_by_mount['left']['id']
             }
         left_model = left_data.get('model')
         if left_model:
@@ -810,7 +845,8 @@ def get_attached_pipettes(self):
         right_data = {
                 'mount_axis': 'a',
                 'plunger_axis': 'c',
-                'model': self.model_by_mount['right']
+                'model': self.model_by_mount['right']['model'],
+                'id': self.model_by_mount['right']['id']
             }
         right_model = right_data.get('model')
         if right_model:

api/src/opentrons/server/endpoints/control.py

@@ -28,13 +28,15 @@ async def get_attached_pipettes(request):
         'model': 'p300_single_v1',
         'tip_length': 51.7,
         'mount_axis': 'z',
-        'plunger_axis': 'b'
+        'plunger_axis': 'b',
+        'id': '<pipette id string>'
       },
       'right': {
         'model': 'p10_multi_v1',
         'tip_length': 40,
         'mount_axis': 'a',
-        'plunger_axis': 'c'
+        'plunger_axis': 'c',
+        'id': '<pipette id string>'
       }
     }
     ```

api/src/opentrons/tools/write_pipette_memory.py

@@ -41,15 +41,14 @@ def write_identifiers(robot, mount, new_id, new_model):
     '''
     robot._driver.write_pipette_id(mount, new_id)
     read_id = robot._driver.read_pipette_id(mount)
-    _assert_the_same(new_id, read_id['pipette_id'])
+    _assert_the_same(new_id, read_id)
     robot._driver.write_pipette_model(mount, new_model)
     read_model = robot._driver.read_pipette_model(mount)
     _assert_the_same(new_model, read_model)
 
 
 def check_previous_data(robot, mount):
     old_id = robot._driver.read_pipette_id(mount)
-    old_id = old_id.get('pipette_id')
     old_model = robot._driver.read_pipette_model(mount)
     if old_id and old_model:
         print(

api/tests/opentrons/drivers/test_driver.py

@@ -472,7 +472,7 @@ def _new_send_message(self, command, timeout=None):
     driver.simulating = False
     read_id = driver.read_pipette_id('left')
     driver.simulating = True
-    assert read_id == {'pipette_id': test_id}
+    assert read_id == test_id
 
     driver.write_pipette_model('left', test_model)
     driver.simulating = False

api/tests/opentrons/hardware_control/test_instruments.py

@@ -2,40 +2,38 @@
 from opentrons import types
 from opentrons import hardware_control as hc
 from opentrons.hardware_control.types import Axis
-from opentrons.hardware_control.pipette import Pipette
+
+
+LEFT_PIPETTE_MODEL = 'p10_single_v1'
+LEFT_PIPETTE_ID = 'testy'
 
 
 @pytest.fixture
 def dummy_instruments():
-    dummy_instruments_attached = {types.Mount.LEFT: 'p10_single_v1',
-                                  types.Mount.RIGHT: None}
+    dummy_instruments_attached = {
+        types.Mount.LEFT: {
+            'model': LEFT_PIPETTE_MODEL,
+            'id': LEFT_PIPETTE_ID
+        },
+        types.Mount.RIGHT: {
+            'model': None,
+            'id': None
+        }
+    }
     return dummy_instruments_attached
 
 
-def attached_instruments(inst):
-    """
-    Format inst dict like the public 'attached_instruments' property
-    """
-    configs = ['name', 'min_volume', 'max_volume',
-               'aspirate_flow_rate', 'dispense_flow_rate']
-    instr_objects = {mount: Pipette(model) if model else None
-                     for mount, model in inst.items()}
-    instruments = {types.Mount.LEFT: {}, types.Mount.RIGHT: {}}
-    for mount in types.Mount:
-        if not instr_objects[mount]:
-            continue
-        for key in configs:
-            instruments[mount][key] = instr_objects[mount].as_dict()[key]
-    return instruments
-
-
 async def test_cache_instruments(dummy_instruments, loop):
+    expected_keys = [
+        'name', 'min_volume', 'max_volume', 'aspirate_flow_rate',
+        'dispense_flow_rate', 'pipette_id']
+
     hw_api = hc.API.build_hardware_simulator(
         attached_instruments=dummy_instruments,
         loop=loop)
     await hw_api.cache_instruments()
-    assert hw_api.attached_instruments == attached_instruments(
-        dummy_instruments)
+    assert sorted(hw_api.attached_instruments[types.Mount.LEFT].keys()) == \
+        sorted(expected_keys)
 
 
 @pytest.mark.skipif(not hc.Controller,
@@ -44,17 +42,29 @@ async def test_cache_instruments(dummy_instruments, loop):
 async def test_cache_instruments_hc(monkeypatch, dummy_instruments,
                                     hardware_controller_lockfile,
                                     running_on_pi, cntrlr_mock_connect, loop):
+    expected_keys = [
+        'name', 'min_volume', 'max_volume', 'aspirate_flow_rate',
+        'dispense_flow_rate', 'pipette_id']
 
     hw_api_cntrlr = hc.API.build_hardware_controller(loop=loop)
 
-    def mock_driver_method(mount):
-        attached_pipette = {'left': 'p10_single_v1', 'right': None}
+    def mock_driver_model(mount):
+        attached_pipette = {'left': LEFT_PIPETTE_MODEL, 'right': None}
         return attached_pipette[mount]
+
+    def mock_driver_id(mount):
+        attached_pipette = {'left': LEFT_PIPETTE_ID, 'right': None}
+        return attached_pipette[mount]
+
     monkeypatch.setattr(hw_api_cntrlr._backend._smoothie_driver,
-                        'read_pipette_model', mock_driver_method)
+                        'read_pipette_model', mock_driver_model)
+    monkeypatch.setattr(hw_api_cntrlr._backend._smoothie_driver,
+                        'read_pipette_id', mock_driver_id)
+
     await hw_api_cntrlr.cache_instruments()
-    assert hw_api_cntrlr.attached_instruments == attached_instruments(
-        dummy_instruments)
+    assert sorted(
+        hw_api_cntrlr.attached_instruments[types.Mount.LEFT].keys()) == \
+        sorted(expected_keys)
 
 
 async def test_aspirate(dummy_instruments, loop):