feat(api): register module instances on os events

api/Pipfile

@@ -20,6 +20,7 @@ wheel = "==0.30.0"
 coverage = "==4.4.2"
 mypy = "==0.740"
 colorama = "*"
+typing-extensions = "*"
 
 [packages]
 numpy = "==1.15.1"
@@ -29,3 +30,4 @@ aiohttp = "==3.4.4"
 jsonschema = "==3.0.2"
 jsonrpcserver = "==4.0.3"
 systemd-python = {version="==234", sys_platform="== 'linux'"}
+aionotify = "==0.2.0"

api/setup.py

@@ -115,6 +115,7 @@ def get_version():
     'numpy>=1.15.1',
     'urwid==1.3.1',
     'jsonschema>=2.5.1',
+    'aionotify==0.2.0',
 ]
 
 

api/src/opentrons/api/session.py

@@ -143,7 +143,6 @@ def __init__(self, name, protocol, hardware, loop, broker, motion_lock):
         self._motion_lock = motion_lock
 
     def prepare(self):
-        self._hardware.discover_modules()
         self.refresh()
 
     def get_instruments(self):
@@ -232,7 +231,7 @@ def on_command(message):
                     API.build_hardware_simulator,
                     instrs,
                     [mod.name()
-                     for mod in self._hardware.attached_modules.values()],
+                     for mod in self._hardware.attached_modules],
                     strict_attached_instruments=False)
                 sim.home()
                 self._simulating_ctx = ProtocolContext(

api/src/opentrons/drivers/mag_deck/driver.py

@@ -159,7 +159,7 @@ def __init__(self, config={}):
 
     def connect(self, port=None) -> str:
         '''
-        :param port: '/dev/modules/ttyn_magdeck'
+        :param port: '/dev/ot_module_magdeck[#]'
         NOTE: Using the symlink above to connect makes sure that the robot
         connects/reconnects to the module even after a device
         reset/reconnection

api/src/opentrons/execute.py

@@ -95,7 +95,6 @@ def _build_hwcontroller():
                                            bundled_data=bundled_data,
                                            api_version=checked_version)
     context._hw_manager.hardware.cache_instruments()
-    context._hw_manager.hardware.discover_modules()
     return context
 
 

api/src/opentrons/hardware_control/__init__.py

@@ -16,17 +16,13 @@
 import functools
 import inspect
 import logging
-from typing import Any, Dict, Union, List, Optional, Tuple
+from typing import Dict, Union, List, Optional, TYPE_CHECKING
 from opentrons import types as top_types
 from opentrons.util import linal
 from .simulator import Simulator
 from opentrons.config import robot_configs, pipette_config
 from .pipette import Pipette
-try:
-    from .controller import Controller
-except ModuleNotFoundError:
-    # implies windows
-    Controller = None  # type: ignore
+from .controller import Controller
 from . import modules
 from .types import Axis, HardwareAPILike, CriticalPoint
 
@@ -102,7 +98,7 @@ def __init__(self,
             top_types.Mount.LEFT: None,
             top_types.Mount.RIGHT: None
         }
-        self._attached_modules: Dict[str, Any] = {}
+        self._attached_modules: List[modules.AbstractModule] = []
         self._last_moved_mount: Optional[top_types.Mount] = None
         # The motion lock synchronizes calls to long-running physical tasks
         # involved in motion. This fixes issue where for instance a move()
@@ -128,13 +124,15 @@ async def build_hardware_controller(
         :param loop: An event loop to use. If not specified, use the result of
                      :py:meth:`asyncio.get_event_loop`.
         """
-        if None is Controller:
-            raise RuntimeError(
-                'The hardware controller may only be instantiated on a robot')
         checked_loop = loop or asyncio.get_event_loop()
         backend = Controller(config)
         await backend.connect(port)
-        return cls(backend, config=config, loop=checked_loop)
+        api_instance = cls(backend, config=config, loop=checked_loop)
+        checked_loop.create_task(backend.watch_modules(
+                loop=checked_loop,
+                register_modules=api_instance.register_modules,
+                ))
+        return api_instance
 
     @classmethod
     def build_hardware_simulator(
@@ -155,11 +153,15 @@ def build_hardware_simulator(
 
         if None is attached_modules:
             attached_modules = []
-        return cls(Simulator(attached_instruments,
-                             attached_modules,
-                             config, loop,
-                             strict_attached_instruments),
-                   config=config, loop=loop)
+        checked_loop = loop or asyncio.get_event_loop()
+        backend = Simulator(attached_instruments,
+                            attached_modules,
+                            config, checked_loop,
+                            strict_attached_instruments)
+        api_instance = cls(backend, config=config, loop=checked_loop)
+        checked_loop.create_task(backend.watch_modules(
+            register_modules=api_instance.register_modules))
+        return api_instance
 
     def __repr__(self):
         return '<{} using backend {}>'.format(type(self),
@@ -398,8 +400,8 @@ async def update_firmware(
                                                    checked_loop,
                                                    explicit_modeset)
 
-    def _call_on_attached_modules(self, method):
-        for module in self.attached_modules.values():
+    def _call_on_attached_modules(self, method: str):
+        for module in self.attached_modules:
             maybe_module_method = getattr(module, method, None)
             if callable(maybe_module_method):
                 maybe_module_method()
@@ -473,7 +475,6 @@ async def reset(self):
         information about their presence or state.
         """
         await self.cache_instruments()
-        await self.discover_modules()
 
     # Gantry/frame (i.e. not pipette) action API
     @_log_call
@@ -1352,46 +1353,29 @@ def set_pipette_speed(self, mount,
                 'blow_out_flow_rate',
                 self._plunger_flowrate(this_pipette, blow_out, 'dispense'))
 
-    @_log_call
-    async def discover_modules(self):
-        discovered = {port + model: (port, model)
-                      for port, model in self._backend.get_attached_modules()}
-        these = set(discovered.keys())
-        known = set(self._attached_modules.keys())
-        new = these - known
-        gone = known - these
-        for mod in gone:
-            self._attached_modules.pop(mod)
-            self._log.info(f"Module {mod} disconnected")
-        for mod in new:
-            self._attached_modules[mod]\
-                = await self._backend.build_module(discovered[mod][0],
-                                                   discovered[mod][1],
-                                                   self.pause_with_message)
-            self._log.info(f"Module {mod} discovered and attached")
-        return list(self._attached_modules.values())
-
-    @_log_call
-    async def update_module(
-            self, module: modules.AbstractModule,
-            firmware_file: str,
-            loop: asyncio.AbstractEventLoop = None) -> Tuple[bool, str]:
-        """ Update a module's firmware.
-
-        Returns (ok, message) where ok is True if the update succeeded and
-        message is a human readable message.
-        """
-        details = (module.port, module.name())
-        mod = self._attached_modules.pop(details[0] + details[1])
-        try:
-            new_mod = await self._backend.update_module(
-                mod, firmware_file, loop)
-        except modules.UpdateError as e:
-            return False, e.msg
-        else:
-            new_details = new_mod.port + new_mod.device_info['model']
-            self._attached_modules[new_details] = new_mod
-            return True, 'firmware update successful'
+    async def register_modules(
+            self,
+            new_modules: List[modules.ModuleAtPort] = None,
+            removed_modules: List[modules.ModuleAtPort] = None
+            ) -> None:
+        if new_modules is None:
+            new_modules = []
+        if removed_modules is None:
+            removed_modules = []
+        for port, name in removed_modules:
+            self._attached_modules = [mod for mod in self._attached_modules
+                                      if mod.port != port]
+            self._log.info(f"Module {name} disconnected"
+                           f" from port {port}")
+
+        for port, name in new_modules:
+            new_instance = await self._backend.build_module(
+                    port,
+                    name,
+                    self.pause_with_message)
+            self._attached_modules.append(new_instance)
+            self._log.info(f"Module {name} discovered and attached"
+                           f" at port {port}")
 
     async def _do_tp(self, pip, mount) -> top_types.Point:
         """ Execute the work of tip probe.

api/src/opentrons/hardware_control/adapters.py

@@ -91,25 +91,6 @@ def __del__(self):
             if thread_loop.is_running():
                 thread_loop.call_soon_threadsafe(lambda: thread_loop.stop())
 
-    def discover_modules(self):
-        loop = object.__getattribute__(self, '_loop')
-        api = object.__getattribute__(self, '_api')
-        discovered_mods = self.call_coroutine_sync(loop, api.discover_modules)
-        async_mods = {mod.port: mod for mod in discovered_mods}
-
-        these = set(async_mods.keys())
-        known = set(self._cached_sync_mods.keys())
-        new = these - known
-        gone = known - these
-
-        for mod_port in gone:
-            self._cached_sync_mods.pop(mod_port)
-        for mod_port in new:
-            self._cached_sync_mods[mod_port] \
-                = SynchronousAdapter(async_mods[mod_port])
-
-        return list(self._cached_sync_mods.values())
-
     @staticmethod
     def call_coroutine_sync(loop, to_call, *args, **kwargs):
         fut = asyncio.run_coroutine_threadsafe(to_call(*args, **kwargs), loop)