From f06e86e6df8b317d02e7b1bb8254aeb5cbfffd25 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 1 Nov 2019 17:17:18 -0400 Subject: [PATCH 01/25] watch modules in controller, debug events --- .../opentrons/hardware_control/__init__.py | 1 + .../opentrons/hardware_control/controller.py | 31 +++++++++++++++++++ .../opentrons/hardware_control/simulator.py | 3 ++ 3 files changed, 35 insertions(+) diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index 0df729b7390..9ca7b49a1b6 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -134,6 +134,7 @@ async def build_hardware_controller( checked_loop = loop or asyncio.get_event_loop() backend = Controller(config) await backend.connect(port) + checked_loop.create_task(backend.watch_modules(checked_loop)) return cls(backend, config=config, loop=checked_loop) @classmethod diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index cc6a73fbd45..066190ae28d 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -2,6 +2,7 @@ from contextlib import contextmanager, ExitStack import logging from typing import Any, Dict, List, Optional, Tuple +import aionotify from opentrons.drivers.smoothie_drivers import driver_3_0 from opentrons.drivers.rpi_drivers import gpio @@ -38,6 +39,10 @@ def __init__(self, config): self._smoothie_driver = driver_3_0.SmoothieDriver_3_0_0( config=self.config, handle_locks=False) self._cached_fw_version: Optional[str] = None + self._module_watcher = aionotify.Watcher() + self._module_watcher.watch(alias='modules', + path='/dev/modules', + flags=(aionotify.Flags.CREATE | aionotify.Flags.DELETE)) def update_position(self) -> Dict[str, float]: self._smoothie_driver.update_position() @@ -113,6 +118,29 @@ def set_pipette_speed(self, val: float): def get_attached_modules(self) -> List[Tuple[str, str]]: return modules.discover() + async def watch_modules(self, loop: asyncio.AbstractEventLoop): + await self._module_watcher.setup(loop) + while not self._module_watcher.closed(): + event = await self._module_watcher.get_event() + MODULE_LOG.warning(f'EVENT CAUGHT: {event}') + + # 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()) + async def build_module(self, port: str, model: str, @@ -201,3 +229,6 @@ async def delay(self, duration_s: int): self.pause() await asyncio.sleep(duration_s) self.resume() + + def __exit__(self, exc_type, exc_value, traceback): + self._module_watcher.close() diff --git a/api/src/opentrons/hardware_control/simulator.py b/api/src/opentrons/hardware_control/simulator.py index 2847eb3ad04..87a0fb52f0b 100644 --- a/api/src/opentrons/hardware_control/simulator.py +++ b/api/src/opentrons/hardware_control/simulator.py @@ -197,6 +197,9 @@ def set_active_current(self, axis, amp): def get_attached_modules(self) -> List[Tuple[str, str]]: return self._attached_modules + async def watch_modules(self, loop: asyncio.AbstractEventLoop): + pass + @contextmanager def save_current(self): yield From 578ccd985a7201f533ed8c9da67d036f006101a3 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Wed, 6 Nov 2019 15:19:56 -0500 Subject: [PATCH 02/25] add and remove callbacks wired --- api/src/opentrons/api/session.py | 2 +- .../opentrons/hardware_control/__init__.py | 76 +++++++++++-------- .../opentrons/hardware_control/controller.py | 52 ++++++++----- .../hardware_control/modules/__init__.py | 26 ++++++- .../opentrons/hardware_control/simulator.py | 4 +- api/src/opentrons/protocol_api/contexts.py | 2 +- api/src/opentrons/server/endpoints/control.py | 6 +- api/src/opentrons/server/endpoints/update.py | 3 +- 8 files changed, 104 insertions(+), 67 deletions(-) diff --git a/api/src/opentrons/api/session.py b/api/src/opentrons/api/session.py index d1fa0822974..8c1e3f45fee 100755 --- a/api/src/opentrons/api/session.py +++ b/api/src/opentrons/api/session.py @@ -232,7 +232,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( diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index 9ca7b49a1b6..aa5b5a61c5a 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -22,11 +22,7 @@ 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 @@ -128,14 +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) - checked_loop.create_task(backend.watch_modules(checked_loop)) - return cls(backend, config=config, loop=checked_loop) + instance = cls(backend, config=config, loop=checked_loop) + await checked_loop.create_task(backend.watch_modules( + loop=checked_loop, + update_attached_modules=instance._update_attached_modules, + )) + return instance @classmethod def build_hardware_simulator( @@ -400,7 +397,7 @@ async def update_firmware( explicit_modeset) def _call_on_attached_modules(self, method): - for module in self.attached_modules.values(): + for module in self.attached_modules: maybe_module_method = getattr(module, method, None) if callable(maybe_module_method): maybe_module_method() @@ -1353,24 +1350,41 @@ 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()) + async def _update_attached_modules(self, + new_modules = [], + removed_modules = []): + for absolute_port, name in removed_modules: + self._attached_modules = [mod for mod in self._attached_modules + if mod.port != absolute_port] + self._log.info(f"Module {name} disconnected " \ + f" from port {absolute_port}") + + for absolute_port, name in new_modules: + new_instance = await self._backend.build_module(absolute_port, + name, + self.pause_with_message) + self._attached_modules.append(new_instance) + self._log.info(f"Module {name} discovered and attached " \ + f" at port {absolute_port}") + + # @_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) @_log_call async def update_module( @@ -1382,16 +1396,12 @@ async def update_module( 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 _do_tp(self, pip, mount) -> top_types.Point: diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index 066190ae28d..c5fa24d2101 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -115,31 +115,41 @@ def save_current(self): def set_pipette_speed(self, val: float): self._smoothie_driver.set_speed(val) - def get_attached_modules(self) -> List[Tuple[str, str]]: + def get_attached_modules(self) -> List[modules.ModuleAtPort]: return modules.discover() - async def watch_modules(self, loop: asyncio.AbstractEventLoop): + async def watch_modules(self, loop: asyncio.AbstractEventLoop, update_attached_modules): await self._module_watcher.setup(loop) - while not self._module_watcher.closed(): + update_attached_modules(new_modules=modules.discover()) + while not self._module_watcher.closed: event = await self._module_watcher.get_event() - MODULE_LOG.warning(f'EVENT CAUGHT: {event}') - - # 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()) + MODULE_LOG.info(f'\n\nEVENT CAUGHT: {event}\n\n') + flags = aionotify.Flags.parse(event.flags) + MODULE_LOG.info(f'\n\nFLAGS: {flags}\n\n') + maybe_module_at_port = modules.get_module_at_port(event.name) + if maybe_module_at_port is not None and aionotify.Flags.DELETE in flags: + update_attached_modules(removed_modules=[maybe_module_at_port]) + MODULE_LOG.info(f'Module Removed: {maybe_module_at_port}') + if maybe_module_at_port is not None and aionotify.Flags.CREATE in flags: + update_attached_modules(new_modules=[maybe_module_at_port]) + MODULE_LOG.info(f'Module Added: {maybe_module_at_port}') + + # 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()) async def build_module(self, port: str, diff --git a/api/src/opentrons/hardware_control/modules/__init__.py b/api/src/opentrons/hardware_control/modules/__init__.py index f03b56b62af..874794a43c6 100644 --- a/api/src/opentrons/hardware_control/modules/__init__.py +++ b/api/src/opentrons/hardware_control/modules/__init__.py @@ -3,6 +3,7 @@ import os import re from typing import List, Optional, Tuple +from collections import namedtuple from opentrons.config import IS_ROBOT from .mod_abc import AbstractModule @@ -12,6 +13,8 @@ log = logging.getLogger(__name__) +ModuleAtPort = namedtuple('ModuleAtPort', ('absolute_port', 'name')) + class UnsupportedModuleError(Exception): pass @@ -38,8 +41,22 @@ async def build( port, interrupt_callback=interrupt_callback, simulating=simulating) -def discover() -> List[Tuple[str, str]]: - """ Scan for connected modules and instantiate handler classes +def get_module_at_port(port: str) -> Optional[ModuleAtPort]: + """ Given a port, returns either a ModuleAtPort + if it is a recognized module, or None if not recognized. + """ + module_port_regex = re.compile('|'.join(MODULE_TYPES.keys()), re.I) + match = module_port_regex.search(port) + log.debug(f'\n\n\nGET MODULE AT PORT: {match}, port: {port}\n\n\n') + if match: + name = match.group().lower() + return ModuleAtPort(absolute_port=f'/dev/modules/{port}', name=name) + return None + + +def discover() -> List[ModuleAtPort]: + """ Scan for connected modules and return list of + tuples of serial ports and device names """ if IS_ROBOT and os.path.isdir('/dev/modules'): devices = os.listdir('/dev/modules') @@ -57,8 +74,9 @@ def discover() -> List[Tuple[str, str]]: log.warning("Unexpected module connected: {} on {}" .format(name, port)) continue - absolute_port = '/dev/modules/{}'.format(port) - discovered_modules.append((absolute_port, name)) + new_mod = ModuleAtPort(absolute_port=f'/dev/modules/{port}', + name=name) + discovered_modules.append(new_mod) log.debug('Discovered modules: {}'.format(discovered_modules)) return discovered_modules diff --git a/api/src/opentrons/hardware_control/simulator.py b/api/src/opentrons/hardware_control/simulator.py index 87a0fb52f0b..c29794c59f2 100644 --- a/api/src/opentrons/hardware_control/simulator.py +++ b/api/src/opentrons/hardware_control/simulator.py @@ -85,7 +85,7 @@ def __init__( self._config = config self._loop = loop self._attached_instruments = attached_instruments - self._attached_modules = [('mod' + str(idx), mod) + self._attached_modules = [ModuleAtPort(absolute_port=f'mod{str(idx)}', name=mod) for idx, mod in enumerate(attached_modules)] self._position = copy.copy(_HOME_POSITION) @@ -194,7 +194,7 @@ def get_attached_instruments( def set_active_current(self, axis, amp): pass - def get_attached_modules(self) -> List[Tuple[str, str]]: + def get_attached_modules(self) -> List[modules.ModuleAtPort]: return self._attached_modules async def watch_modules(self, loop: asyncio.AbstractEventLoop): diff --git a/api/src/opentrons/protocol_api/contexts.py b/api/src/opentrons/protocol_api/contexts.py index 2bf4ec1815b..cbbfa7b887f 100644 --- a/api/src/opentrons/protocol_api/contexts.py +++ b/api/src/opentrons/protocol_api/contexts.py @@ -412,7 +412,7 @@ def load_module( 'magdeck': MagneticModuleContext, 'tempdeck': TemperatureModuleContext, 'thermocycler': ThermocyclerContext}[resolved_name] - for mod in self._hw_manager.hardware.discover_modules(): + for mod in self._hw_manager.hardware.attached_modules: if mod.name() == resolved_name: hc_mod_instance = mod break diff --git a/api/src/opentrons/server/endpoints/control.py b/api/src/opentrons/server/endpoints/control.py index abc2b88f186..403f82f4332 100644 --- a/api/src/opentrons/server/endpoints/control.py +++ b/api/src/opentrons/server/endpoints/control.py @@ -129,7 +129,7 @@ async def get_attached_modules(request): """ hw = hw_from_req(request) if ff.use_protocol_api_v2(): - hw_mods = await hw.discover_modules() + hw_mods = hw.attached_modules module_data = [ { 'name': mod.name(), @@ -171,7 +171,7 @@ async def get_module_data(request): res = None if ff.use_protocol_api_v2(): - hw_mods = await hw.discover_modules() + hw_mods = hw.attached_modules else: hw_mods = hw.attached_modules.values() @@ -197,7 +197,7 @@ async def execute_module_command(request): args = data.get('args') if ff.use_protocol_api_v2(): - hw_mods = await hw.discover_modules() + hw_mods = hw.attached_modules else: hw_mods = hw.attached_modules.values() diff --git a/api/src/opentrons/server/endpoints/update.py b/api/src/opentrons/server/endpoints/update.py index 194c5cd26d6..8e384ec2f5e 100644 --- a/api/src/opentrons/server/endpoints/update.py +++ b/api/src/opentrons/server/endpoints/update.py @@ -54,8 +54,7 @@ async def update_module_firmware(request): async def _upload_to_module(hw, serialnum, fw_filename, loop): - await hw.discover_modules() - hw_mods = hw.attached_modules.values() + hw_mods = hw.attached_modules for module in hw_mods: if module.device_info.get('serial') == serialnum: log.info("Module with serial {} found".format(serialnum)) From 2794ac00ddcb24918b76bda9ffe46f98972c2f51 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Wed, 6 Nov 2019 15:52:55 -0500 Subject: [PATCH 03/25] debugging watch failure --- api/src/opentrons/hardware_control/__init__.py | 2 +- api/src/opentrons/hardware_control/controller.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index aa5b5a61c5a..4432a286317 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -98,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[Any] = [] 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() diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index c5fa24d2101..43b1e5899c9 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -120,7 +120,8 @@ def get_attached_modules(self) -> List[modules.ModuleAtPort]: async def watch_modules(self, loop: asyncio.AbstractEventLoop, update_attached_modules): await self._module_watcher.setup(loop) - update_attached_modules(new_modules=modules.discover()) + + await update_attached_modules(new_modules=modules.discover()) while not self._module_watcher.closed: event = await self._module_watcher.get_event() MODULE_LOG.info(f'\n\nEVENT CAUGHT: {event}\n\n') From 775de803a45b98a7b157be738ff78e38d43a4704 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Thu, 7 Nov 2019 15:16:00 -0500 Subject: [PATCH 04/25] intermediate var --- api/src/opentrons/hardware_control/controller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index 43b1e5899c9..bd7e6fe3142 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -11,7 +11,6 @@ from . import modules - MODULE_LOG = logging.getLogger(__name__) @@ -121,7 +120,8 @@ def get_attached_modules(self) -> List[modules.ModuleAtPort]: async def watch_modules(self, loop: asyncio.AbstractEventLoop, update_attached_modules): await self._module_watcher.setup(loop) - await update_attached_modules(new_modules=modules.discover()) + initial_modules = modules.discover() + await update_attached_modules(new_modules=initial_modules) while not self._module_watcher.closed: event = await self._module_watcher.get_event() MODULE_LOG.info(f'\n\nEVENT CAUGHT: {event}\n\n') From 4462543b14f6e381735533305a126719492ba2a9 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Thu, 7 Nov 2019 16:41:24 -0500 Subject: [PATCH 05/25] mkdir if not there --- api/src/opentrons/hardware_control/controller.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index bd7e6fe3142..c631cb007b6 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -39,6 +39,8 @@ def __init__(self, config): config=self.config, handle_locks=False) self._cached_fw_version: Optional[str] = None self._module_watcher = aionotify.Watcher() + if not os.path.isdir('/dev/modules'): + os.mkdir('/dev/modules') self._module_watcher.watch(alias='modules', path='/dev/modules', flags=(aionotify.Flags.CREATE | aionotify.Flags.DELETE)) From fd1089bd96027de94075139b0c8597874693753c Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 8 Nov 2019 12:10:43 -0500 Subject: [PATCH 06/25] all ports absolute and nuke /dev/modules subdir --- api/src/opentrons/drivers/mag_deck/driver.py | 2 +- .../opentrons/hardware_control/__init__.py | 12 +++---- .../opentrons/hardware_control/controller.py | 20 +++++------ .../hardware_control/modules/__init__.py | 13 ++++---- .../hardware_control/modules/update.py | 14 ++++---- .../opentrons/hardware_control/simulator.py | 8 +++-- .../opentrons/legacy_api/modules/__init__.py | 6 ++-- api/src/opentrons/legacy_api/robot/robot.py | 4 +-- api/tests/opentrons/labware/test_modules.py | 33 ++++++++++--------- .../server/test_control_endpoints.py | 2 +- .../opentrons/server/test_update_endpoints.py | 4 +-- 11 files changed, 60 insertions(+), 58 deletions(-) diff --git a/api/src/opentrons/drivers/mag_deck/driver.py b/api/src/opentrons/drivers/mag_deck/driver.py index 6c130490559..b0e1cc76656 100644 --- a/api/src/opentrons/drivers/mag_deck/driver.py +++ b/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 diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index 4432a286317..7fc6719e1c8 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -1353,19 +1353,19 @@ def set_pipette_speed(self, mount, async def _update_attached_modules(self, new_modules = [], removed_modules = []): - for absolute_port, name in removed_modules: + for port, name in removed_modules: self._attached_modules = [mod for mod in self._attached_modules - if mod.port != absolute_port] + if mod.port != port] self._log.info(f"Module {name} disconnected " \ - f" from port {absolute_port}") + f" from port {port}") - for absolute_port, name in new_modules: - new_instance = await self._backend.build_module(absolute_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 {absolute_port}") + f" at port {port}") # @_log_call # async def discover_modules(self): diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index c631cb007b6..70584a97ff1 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -39,10 +39,8 @@ def __init__(self, config): config=self.config, handle_locks=False) self._cached_fw_version: Optional[str] = None self._module_watcher = aionotify.Watcher() - if not os.path.isdir('/dev/modules'): - os.mkdir('/dev/modules') self._module_watcher.watch(alias='modules', - path='/dev/modules', + path='/dev', flags=(aionotify.Flags.CREATE | aionotify.Flags.DELETE)) def update_position(self) -> Dict[str, float]: @@ -129,13 +127,15 @@ async def watch_modules(self, loop: asyncio.AbstractEventLoop, update_attached_m MODULE_LOG.info(f'\n\nEVENT CAUGHT: {event}\n\n') flags = aionotify.Flags.parse(event.flags) MODULE_LOG.info(f'\n\nFLAGS: {flags}\n\n') - maybe_module_at_port = modules.get_module_at_port(event.name) - if maybe_module_at_port is not None and aionotify.Flags.DELETE in flags: - update_attached_modules(removed_modules=[maybe_module_at_port]) - MODULE_LOG.info(f'Module Removed: {maybe_module_at_port}') - if maybe_module_at_port is not None and aionotify.Flags.CREATE in flags: - update_attached_modules(new_modules=[maybe_module_at_port]) - MODULE_LOG.info(f'Module Added: {maybe_module_at_port}') + if 'ot_module' in event.name + maybe_module_at_port = modules.get_module_at_port(event.name) + if maybe_module_at_port is not None and aionotify.Flags.DELETE in flags: + update_attached_modules( + removed_modules=[maybe_module_at_port]) + MODULE_LOG.info(f'Module Removed: {maybe_module_at_port}') + if maybe_module_at_port is not None and aionotify.Flags.CREATE in flags: + update_attached_modules(new_modules=[maybe_module_at_port]) + MODULE_LOG.info(f'Module Added: {maybe_module_at_port}') # discovered = {port + model: (port, model) # for port, model in self._backend.get_attached_modules()} diff --git a/api/src/opentrons/hardware_control/modules/__init__.py b/api/src/opentrons/hardware_control/modules/__init__.py index 874794a43c6..d50e10d5a0f 100644 --- a/api/src/opentrons/hardware_control/modules/__init__.py +++ b/api/src/opentrons/hardware_control/modules/__init__.py @@ -1,6 +1,7 @@ import asyncio import logging import os +from glob import glob import re from typing import List, Optional, Tuple from collections import namedtuple @@ -13,7 +14,7 @@ log = logging.getLogger(__name__) -ModuleAtPort = namedtuple('ModuleAtPort', ('absolute_port', 'name')) +ModuleAtPort = namedtuple('ModuleAtPort', ('port', 'name')) class UnsupportedModuleError(Exception): @@ -50,7 +51,7 @@ def get_module_at_port(port: str) -> Optional[ModuleAtPort]: log.debug(f'\n\n\nGET MODULE AT PORT: {match}, port: {port}\n\n\n') if match: name = match.group().lower() - return ModuleAtPort(absolute_port=f'/dev/modules/{port}', name=name) + return ModuleAtPort(port=port, name=name) return None @@ -58,8 +59,8 @@ def discover() -> List[ModuleAtPort]: """ Scan for connected modules and return list of tuples of serial ports and device names """ - if IS_ROBOT and os.path.isdir('/dev/modules'): - devices = os.listdir('/dev/modules') + if IS_ROBOT: + devices = glob('/dev/ot_module*') else: devices = [] @@ -74,9 +75,7 @@ def discover() -> List[ModuleAtPort]: log.warning("Unexpected module connected: {} on {}" .format(name, port)) continue - new_mod = ModuleAtPort(absolute_port=f'/dev/modules/{port}', - name=name) - discovered_modules.append(new_mod) + discovered_modules.append(ModuleAtPort(port=port, name=name)) log.debug('Discovered modules: {}'.format(discovered_modules)) return discovered_modules diff --git a/api/src/opentrons/hardware_control/modules/update.py b/api/src/opentrons/hardware_control/modules/update.py index 01db7e528aa..befc1cc0f75 100644 --- a/api/src/opentrons/hardware_control/modules/update.py +++ b/api/src/opentrons/hardware_control/modules/update.py @@ -2,6 +2,7 @@ import logging import os import sys +from glob import glob from typing import Any, Dict, Optional, Tuple from .mod_abc import UploadFunction @@ -155,7 +156,7 @@ async def _port_on_mode_switch(ports_before_switch): ports_after_switch)) if len(new_ports) > 1: raise OSError('Multiple new ports found on mode switch') - new_port = '/dev/modules/{}'.format(new_ports[0]) + new_port = new_ports[0] return new_port @@ -173,7 +174,7 @@ async def _port_poll(is_old_bootloader, ports_before_switch=None): discovered_ports = list(filter( lambda x: x.endswith('bootloader'), ports)) if len(discovered_ports) == 1: - new_port = '/dev/modules/{}'.format(discovered_ports[0]) + new_port = discovered_ports[0] await asyncio.sleep(0.05) return new_port @@ -186,9 +187,8 @@ async def _discover_ports(): for attempt in range(2): # Measure for race condition where port is being switched in # between calls to isdir() and listdir() - try: - return os.listdir('/dev/modules') - except (FileNotFoundError, OSError): - pass + module_ports = glob('/dev/ot_module*') + if module_ports: + return module_ports await asyncio.sleep(2) - raise Exception("No /dev/modules found. Try again") + raise Exception("No ot_modules found in /dev. Try again") diff --git a/api/src/opentrons/hardware_control/simulator.py b/api/src/opentrons/hardware_control/simulator.py index c29794c59f2..54848eb6ffc 100644 --- a/api/src/opentrons/hardware_control/simulator.py +++ b/api/src/opentrons/hardware_control/simulator.py @@ -85,9 +85,11 @@ def __init__( self._config = config self._loop = loop self._attached_instruments = attached_instruments - self._attached_modules = [ModuleAtPort(absolute_port=f'mod{str(idx)}', name=mod) - for idx, mod - in enumerate(attached_modules)] + self._attached_modules = [ + ModuleAtPort( + port=f'/dev/ot_module_simulated_mod{str(idx)}', name=mod) + for idx, mod + in enumerate(attached_modules)] self._position = copy.copy(_HOME_POSITION) # Engaged axes start all true in smoothie for some reason so we # imitate that here diff --git a/api/src/opentrons/legacy_api/modules/__init__.py b/api/src/opentrons/legacy_api/modules/__init__.py index bb98ced490f..03e72726af2 100644 --- a/api/src/opentrons/legacy_api/modules/__init__.py +++ b/api/src/opentrons/legacy_api/modules/__init__.py @@ -1,4 +1,4 @@ -import os +from glob import glob import logging import re from typing import List, Tuple, Any @@ -76,8 +76,8 @@ def load(name, slot): # Note: this function should be called outside the robot class, because # of the circular dependency that it would create if imported into robot.py def discover() -> List[Tuple[str, Any]]: - if config.IS_ROBOT and os.path.isdir('/dev/modules'): - devices = os.listdir('/dev/modules') + if config.IS_ROBOT: + devices = glob('/dev/ot_module*') else: devices = [] diff --git a/api/src/opentrons/legacy_api/robot/robot.py b/api/src/opentrons/legacy_api/robot/robot.py index 3b9205c4f26..ca5c4041c7a 100755 --- a/api/src/opentrons/legacy_api/robot/robot.py +++ b/api/src/opentrons/legacy_api/robot/robot.py @@ -1173,9 +1173,9 @@ def discover_modules(self): self._attached_modules.pop(mod) for mod in new: module_class = modules.SUPPORTED_MODULES[discovered[mod][1]] - absolute_port = '/dev/modules/{}'.format(discovered[mod][0]) + port = discovered[mod][1] self._attached_modules[mod]\ - = module_class(port=absolute_port, broker=self.broker) + = module_class(port=port, broker=self.broker) try: self._attached_modules[mod].connect() diff --git a/api/tests/opentrons/labware/test_modules.py b/api/tests/opentrons/labware/test_modules.py index 42d6fa65428..16655893f59 100644 --- a/api/tests/opentrons/labware/test_modules.py +++ b/api/tests/opentrons/labware/test_modules.py @@ -72,9 +72,9 @@ def mock_get_info(self): monkeypatch.setattr(MagDeckDriver, 'connect', mock_connect) monkeypatch.setattr(MagDeckDriver, 'get_device_info', mock_get_info) monkeypatch.setattr(serial_communication, 'write_and_return', mock_write) - magdeck = modules.MagDeck(port='/dev/modules/tty1_magdeck') + magdeck = modules.MagDeck(port='/dev/ot_module_magdeck1') magdeck.connect() - robot._attached_modules = {'tty1_magdeckmagdeck': magdeck} + robot._attached_modules = {'/dev/ot_module_magdeck0magdeck': magdeck} modules.load('magdeck', '4') assert connected @@ -97,9 +97,9 @@ def mock_get_info(self): monkeypatch.setattr(TempDeckDriver, 'connect', mock_connect) monkeypatch.setattr(TempDeckDriver, 'get_device_info', mock_get_info) monkeypatch.setattr(serial_communication, 'write_and_return', mock_write) - tempdeck = modules.TempDeck(port='/dev/modules/tty1_tempdeck') + tempdeck = modules.TempDeck(port='/dev/ot_module_tempdeck1') tempdeck.connect() - robot._attached_modules = {'tty1_tempdecktempdeck': tempdeck} + robot._attached_modules = {'/dev/ot_module_tempdeck0tempdeck': tempdeck} modules.load('tempdeck', '5') assert connected tempdeck.disconnect() # Necessary to kill the thread started by connect() @@ -119,7 +119,7 @@ def test_load_correct_engage_height(robot, modules, labware): @pytest.fixture async def old_bootloader_module(): module = await hw_modules.build( - port='/dev/modules/tty0_tempdeck', + port='/dev/ot_module_tempdeck0', which='tempdeck', simulating=True, interrupt_callback=lambda x: None) @@ -131,7 +131,7 @@ async def old_bootloader_module(): @pytest.fixture async def new_bootloader_module(): module = await hw_modules.build( - port='/dev/modules/tty0_tempdeck', + port='/dev/ot_module_tempdeck0', which='tempdeck', simulating=True, interrupt_callback=lambda x: None) @@ -145,13 +145,13 @@ async def test_enter_bootloader( new_bootloader_module, virtual_smoothie_env, monkeypatch): async def mock_discover_ports_before_dfu_mode(): - return 'tty0_tempdeck' + return '/dev/ot_module_tempdeck0', def mock_enter_programming_mode(self): return 'ok\n\rok\n\r' async def mock_port_poll(_has_old_bootloader, ports_before_dfu_mode): - return '/dev/modules/tty0_bootloader' + return '/dev/ot_module_avrdude_bootloader' monkeypatch.setattr( TempDeckDriver, 'enter_programming_mode', mock_enter_programming_mode) @@ -165,7 +165,7 @@ async def mock_port_poll(_has_old_bootloader, ports_before_dfu_mode): new_bootloader_module._driver, new_bootloader_module.name()) - assert bootloader_port == '/dev/modules/tty0_bootloader' + assert bootloader_port == '/dev/ot_module_avrdude_bootloader' @pytest.mark.api2_only @@ -186,18 +186,18 @@ async def test_port_poll(virtual_smoothie_env, monkeypatch): # Case 1: Bootloader port is successfully opened on the module async def mock_discover_ports1(): - return ['tty0_magdeck', 'tty1_bootloader'] + return ['/dev/ot_module_magdeck0', '/dev/ot_module_avrdude_bootloader1'] monkeypatch.setattr(hw_modules.update, '_discover_ports', mock_discover_ports1) port_found = await asyncio.wait_for( hw_modules.update._port_poll(has_old_bootloader, None), hw_modules.update.PORT_SEARCH_TIMEOUT) - assert port_found == '/dev/modules/tty1_bootloader' + assert port_found == '/dev/ot_module_avrdude_bootloader1' # Case 2: Switching to bootloader mode failed async def mock_discover_ports2(): - return ['tty0_magdeck', 'tty1_tempdeck'] + return ['/dev/ot_module_magdeck0', '/dev/ot_module_tempdeck1'] monkeypatch.setattr(hw_modules.update, '_discover_ports', mock_discover_ports2) @@ -212,14 +212,15 @@ async def mock_discover_ports2(): async def test_old_bootloader_port_poll( virtual_smoothie_env, monkeypatch): - ports_before_switch = ['tty0_magdeck', 'tty1_tempdeck'] + ports_before_switch = [ + '/dev/ot_module_magdeck0', '/dev/ot_module_tempdeck1'] has_old_bootloader = True timeout = 0.1 monkeypatch.setattr(hw_modules.update, 'PORT_SEARCH_TIMEOUT', timeout) # Case 1: Bootloader is opened on same port async def mock_discover_ports(): - return ['tty0_magdeck', 'tty1_tempdeck'] + return ['/dev/ot_module_magdeck0', '/dev/ot_module_tempdeck1'] monkeypatch.setattr(hw_modules.update, '_discover_ports', mock_discover_ports) @@ -232,11 +233,11 @@ async def mock_discover_ports(): # Case 2: Bootloader is opened on a different port async def mock_discover_ports(): - return ['tty2_magdeck', 'tty1_tempdeck'] + return ['/dev/ot_module_magdeck2', '/dev/ot_module_tempdeck1'] monkeypatch.setattr(hw_modules.update, '_discover_ports', mock_discover_ports) port_found = await asyncio.wait_for( hw_modules.update._port_poll(has_old_bootloader, ports_before_switch), hw_modules.update.PORT_SEARCH_TIMEOUT) - assert port_found == '/dev/modules/tty2_magdeck' + assert port_found == '/dev/ot_module_magdeck2' diff --git a/api/tests/opentrons/server/test_control_endpoints.py b/api/tests/opentrons/server/test_control_endpoints.py index 6c03cbb6981..bac28f55f02 100644 --- a/api/tests/opentrons/server/test_control_endpoints.py +++ b/api/tests/opentrons/server/test_control_endpoints.py @@ -123,7 +123,7 @@ async def test_get_modules_v2( hw = async_server['com.opentrons.hardware'] monkeypatch.setattr(hw._backend, '_attached_modules', - [('/dev/modules/tty1_magdeck', 'magdeck')]) + [('/dev/ot_module_magdeck1', 'magdeck')]) await check_modules_response(async_client) diff --git a/api/tests/opentrons/server/test_update_endpoints.py b/api/tests/opentrons/server/test_update_endpoints.py index fe22342b73b..f1d1da0686b 100644 --- a/api/tests/opentrons/server/test_update_endpoints.py +++ b/api/tests/opentrons/server/test_update_endpoints.py @@ -122,7 +122,7 @@ def mock_get_attached_modules(module): return [('mod1', 'thermocycler')] async def mock_enter_bootloader(driver, module): - return '/dev/modules/tty0_bootloader' + return '/dev/ot_module_avrdude_bootloader0' monkeypatch.setattr(hw_modules.update, 'enter_bootloader', mock_enter_bootloader) @@ -168,7 +168,7 @@ async def test_fail_update_module_firmware( fd.write(bytes(0x1234)) async def mock_enter_bootloader(driver, module): - return '/dev/modules/tty0_bootloader' + return '/dev/ot_module_avrdude_bootloader0' def mock_get_attached_modules(module): return [('mod1', 'thermocycler')] From 4271f2da2d7258532461901fa8df10296f4d7f92 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 8 Nov 2019 13:43:18 -0500 Subject: [PATCH 07/25] fix up --- api/src/opentrons/hardware_control/controller.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index 70584a97ff1..000baa83a2a 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -127,9 +127,9 @@ async def watch_modules(self, loop: asyncio.AbstractEventLoop, update_attached_m MODULE_LOG.info(f'\n\nEVENT CAUGHT: {event}\n\n') flags = aionotify.Flags.parse(event.flags) MODULE_LOG.info(f'\n\nFLAGS: {flags}\n\n') - if 'ot_module' in event.name - maybe_module_at_port = modules.get_module_at_port(event.name) - if maybe_module_at_port is not None and aionotify.Flags.DELETE in flags: + if 'ot_module' in event.name: + maybe_module_at_port = modules.get_module_at_port(event.name) + if maybe_module_at_port is not None and aionotify.Flags.DELETE in flags: update_attached_modules( removed_modules=[maybe_module_at_port]) MODULE_LOG.info(f'Module Removed: {maybe_module_at_port}') From 437852c0a8eb0a8e9c7cea42b52911f0e0d089f9 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 8 Nov 2019 15:29:56 -0500 Subject: [PATCH 08/25] pull out debug logs --- api/src/opentrons/hardware_control/controller.py | 6 ++---- api/src/opentrons/hardware_control/modules/__init__.py | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index 000baa83a2a..a4a6a71fef2 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -124,17 +124,15 @@ async def watch_modules(self, loop: asyncio.AbstractEventLoop, update_attached_m await update_attached_modules(new_modules=initial_modules) while not self._module_watcher.closed: event = await self._module_watcher.get_event() - MODULE_LOG.info(f'\n\nEVENT CAUGHT: {event}\n\n') flags = aionotify.Flags.parse(event.flags) - MODULE_LOG.info(f'\n\nFLAGS: {flags}\n\n') if 'ot_module' in event.name: maybe_module_at_port = modules.get_module_at_port(event.name) if maybe_module_at_port is not None and aionotify.Flags.DELETE in flags: - update_attached_modules( + await update_attached_modules( removed_modules=[maybe_module_at_port]) MODULE_LOG.info(f'Module Removed: {maybe_module_at_port}') if maybe_module_at_port is not None and aionotify.Flags.CREATE in flags: - update_attached_modules(new_modules=[maybe_module_at_port]) + await update_attached_modules(new_modules=[maybe_module_at_port]) MODULE_LOG.info(f'Module Added: {maybe_module_at_port}') # discovered = {port + model: (port, model) diff --git a/api/src/opentrons/hardware_control/modules/__init__.py b/api/src/opentrons/hardware_control/modules/__init__.py index d50e10d5a0f..7b7bbe5a705 100644 --- a/api/src/opentrons/hardware_control/modules/__init__.py +++ b/api/src/opentrons/hardware_control/modules/__init__.py @@ -48,10 +48,9 @@ def get_module_at_port(port: str) -> Optional[ModuleAtPort]: """ module_port_regex = re.compile('|'.join(MODULE_TYPES.keys()), re.I) match = module_port_regex.search(port) - log.debug(f'\n\n\nGET MODULE AT PORT: {match}, port: {port}\n\n\n') if match: name = match.group().lower() - return ModuleAtPort(port=port, name=name) + return ModuleAtPort(port=f'/dev/{port}', name=name) return None From 1240bc50743282ee71a032b0f5e8bc48b3d0260c Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Mon, 11 Nov 2019 11:19:33 -0500 Subject: [PATCH 09/25] remove discover_modules calls and gate aionotify import --- api/src/opentrons/api/session.py | 1 - .../opentrons/hardware_control/__init__.py | 23 ++++-------------- .../opentrons/hardware_control/controller.py | 24 +++++-------------- api/src/opentrons/server/endpoints/update.py | 1 - api/tests/opentrons/conftest.py | 8 +++++++ .../hardware_control/test_modules.py | 20 ++++++++-------- 6 files changed, 28 insertions(+), 49 deletions(-) diff --git a/api/src/opentrons/api/session.py b/api/src/opentrons/api/session.py index 8c1e3f45fee..a0b752475e5 100755 --- a/api/src/opentrons/api/session.py +++ b/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): diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index 7fc6719e1c8..9400bfb8c04 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -128,7 +128,7 @@ async def build_hardware_controller( backend = Controller(config) await backend.connect(port) instance = cls(backend, config=config, loop=checked_loop) - await checked_loop.create_task(backend.watch_modules( + checked_loop.create_task(backend.watch_modules( loop=checked_loop, update_attached_modules=instance._update_attached_modules, )) @@ -1367,24 +1367,9 @@ async def _update_attached_modules(self, self._log.info(f"Module {name} discovered and attached " \ f" at port {port}") - # @_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) + # TODO: remove this function once APIv1 is sunset + async def discover_modules(self): + pass @_log_call async def update_module( diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index a4a6a71fef2..279dede8fe0 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -2,7 +2,10 @@ from contextlib import contextmanager, ExitStack import logging from typing import Any, Dict, List, Optional, Tuple -import aionotify +try: + import aionotify +except OSError: + pass from opentrons.drivers.smoothie_drivers import driver_3_0 from opentrons.drivers.rpi_drivers import gpio @@ -122,6 +125,7 @@ async def watch_modules(self, loop: asyncio.AbstractEventLoop, update_attached_m initial_modules = modules.discover() await update_attached_modules(new_modules=initial_modules) + MODULE_LOG.info(f'\n\nINIT MODULES: {initial_modules}\n\n') while not self._module_watcher.closed: event = await self._module_watcher.get_event() flags = aionotify.Flags.parse(event.flags) @@ -135,27 +139,11 @@ async def watch_modules(self, loop: asyncio.AbstractEventLoop, update_attached_m await update_attached_modules(new_modules=[maybe_module_at_port]) MODULE_LOG.info(f'Module Added: {maybe_module_at_port}') - # 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()) - async def build_module(self, port: str, model: str, interrupt_callback) -> modules.AbstractModule: + MODULE_LOG.info(f'\n\nBUILD Module {port}{model}') return await modules.build( port=port, which=model, diff --git a/api/src/opentrons/server/endpoints/update.py b/api/src/opentrons/server/endpoints/update.py index 8e384ec2f5e..522bc20a818 100644 --- a/api/src/opentrons/server/endpoints/update.py +++ b/api/src/opentrons/server/endpoints/update.py @@ -62,7 +62,6 @@ async def _upload_to_module(hw, serialnum, fw_filename, loop): new_instance = await asyncio.wait_for( modules.update_firmware(module, fw_filename, loop), UPDATE_TIMEOUT) - print(f'\n\n\ninstance {new_instance}\n') return f'Successully updated module {serialnum}', 200 except modules.UpdateError as e: return f'Bootloader error: {e}', 400 diff --git a/api/tests/opentrons/conftest.py b/api/tests/opentrons/conftest.py index 2f93f9d3799..57edc5ce79e 100755 --- a/api/tests/opentrons/conftest.py +++ b/api/tests/opentrons/conftest.py @@ -1,6 +1,12 @@ # Uncomment to enable logging during tests # import logging # from logging.config import dictConfig +try: + import aionotify +except OSError: + aionotify = None + pass +import sys import asyncio import contextlib import os @@ -203,6 +209,8 @@ def _should_skip_api2(request): and request.param != using_api2 +@pytest.mark.skipif(not aionotify, + reason="requires inotify (linux only)") @pytest.fixture( params=[ pytest.param(using_api1, marks=pytest.mark.apiv1), diff --git a/api/tests/opentrons/hardware_control/test_modules.py b/api/tests/opentrons/hardware_control/test_modules.py index dbf6fa81c00..4f8f4b942f4 100644 --- a/api/tests/opentrons/hardware_control/test_modules.py +++ b/api/tests/opentrons/hardware_control/test_modules.py @@ -5,7 +5,7 @@ async def test_get_modules_simulating(): mods = ['tempdeck', 'magdeck', 'thermocycler'] api = hardware_control.API.build_hardware_simulator(attached_modules=mods) - from_api = await api.discover_modules() + from_api = api.attached_modules assert sorted([mod.name() for mod in from_api]) == sorted(mods) @@ -15,22 +15,22 @@ async def test_module_caching(): attached_modules=mod_names) # Check that we can add and remove modules and the caching keeps up - found_mods = await api.discover_modules() + found_mods = api.attached_modules assert found_mods[0].name() == 'tempdeck' - new_mods = await api.discover_modules() + new_mods = api.attached_modules assert new_mods[0] is found_mods[0] api._backend._attached_modules.append(('mod2', 'magdeck')) - with_magdeck = await api.discover_modules() + with_magdeck = api.attached_modules assert len(with_magdeck) == 2 assert with_magdeck[0] is found_mods[0] api._backend._attached_modules = api._backend._attached_modules[1:] - only_magdeck = await api.discover_modules() + only_magdeck = api.attached_modules assert only_magdeck[0] is with_magdeck[1] # Check that two modules of the same kind on different ports are # distinct api._backend._attached_modules.append(('mod3', 'magdeck')) - two_magdecks = await api.discover_modules() + two_magdecks = api.attached_modules assert len(two_magdecks) == 2 assert two_magdecks[0] is with_magdeck[1] assert two_magdecks[1] is not two_magdecks[0] @@ -40,7 +40,7 @@ async def test_module_update_logic(monkeypatch): mod_names = ['tempdeck'] api = hardware_control.API.build_hardware_simulator( attached_modules=mod_names) - mods = await api.discover_modules() + mods = api.attached_modules old = mods[0] async def new_update_module(mod, ff, loop=None): @@ -50,7 +50,7 @@ async def new_update_module(mod, ff, loop=None): monkeypatch.setattr(api._backend, 'update_module', new_update_module) ok, msg = await api.update_module(mods[0], 'some_file') - mods = await api.discover_modules() + mods = api.attached_modules assert len(mods) == 1 assert mods[0] is not old @@ -87,8 +87,8 @@ async def mock_upload(port, firmware_file_path, upload_function, loop): monkeypatch.setattr(hardware_control.modules.update, 'upload_firmware', mock_upload) - modules = await api.discover_modules() + modules = api.attached_modules ok, msg = await api.update_module(modules[0], 'some-fake-file', loop) assert ok - new_modules = await api.discover_modules() + new_modules = api.attached_modules assert new_modules[0] is not modules[0] From fffed71e3220ce7689f420afed1e1211c2319d76 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Tue, 12 Nov 2019 15:25:52 -0500 Subject: [PATCH 10/25] simulator parity --- .../opentrons/hardware_control/__init__.py | 45 +++++---------- .../opentrons/hardware_control/controller.py | 23 ++------ .../opentrons/hardware_control/simulator.py | 25 +++----- api/tests/opentrons/conftest.py | 5 +- .../hardware_control/test_instruments.py | 10 ++-- .../hardware_control/test_modules.py | 57 ++++++++++--------- .../opentrons/server/test_update_endpoints.py | 2 +- 7 files changed, 67 insertions(+), 100 deletions(-) diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index 9400bfb8c04..cb97121f440 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -98,7 +98,7 @@ def __init__(self, top_types.Mount.LEFT: None, top_types.Mount.RIGHT: None } - self._attached_modules: List[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() @@ -127,12 +127,12 @@ async def build_hardware_controller( checked_loop = loop or asyncio.get_event_loop() backend = Controller(config) await backend.connect(port) - instance = 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, - update_attached_modules=instance._update_attached_modules, + register_modules=api_instance.register_modules, )) - return instance + return api_instance @classmethod def build_hardware_simulator( @@ -153,11 +153,16 @@ 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), @@ -1350,9 +1355,7 @@ def set_pipette_speed(self, mount, 'blow_out_flow_rate', self._plunger_flowrate(this_pipette, blow_out, 'dispense')) - async def _update_attached_modules(self, - new_modules = [], - removed_modules = []): + async def register_modules(self, new_modules = [], removed_modules = []): for port, name in removed_modules: self._attached_modules = [mod for mod in self._attached_modules if mod.port != port] @@ -1371,24 +1374,6 @@ async def _update_attached_modules(self, async def discover_modules(self): pass - @_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. - """ - try: - new_mod = await self._backend.update_module( - mod, firmware_file, loop) - except modules.UpdateError as e: - return False, e.msg - else: - return True, 'firmware update successful' - async def _do_tp(self, pip, mount) -> top_types.Point: """ Execute the work of tip probe. diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index 279dede8fe0..3328017c252 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -5,7 +5,7 @@ try: import aionotify except OSError: - pass + aionotify = None from opentrons.drivers.smoothie_drivers import driver_3_0 from opentrons.drivers.rpi_drivers import gpio @@ -117,14 +117,11 @@ def save_current(self): def set_pipette_speed(self, val: float): self._smoothie_driver.set_speed(val) - def get_attached_modules(self) -> List[modules.ModuleAtPort]: - return modules.discover() - - async def watch_modules(self, loop: asyncio.AbstractEventLoop, update_attached_modules): + async def watch_modules(self, loop: asyncio.AbstractEventLoop, register_modules): await self._module_watcher.setup(loop) initial_modules = modules.discover() - await update_attached_modules(new_modules=initial_modules) + await register_modules(new_modules=initial_modules) MODULE_LOG.info(f'\n\nINIT MODULES: {initial_modules}\n\n') while not self._module_watcher.closed: event = await self._module_watcher.get_event() @@ -132,33 +129,23 @@ async def watch_modules(self, loop: asyncio.AbstractEventLoop, update_attached_m if 'ot_module' in event.name: maybe_module_at_port = modules.get_module_at_port(event.name) if maybe_module_at_port is not None and aionotify.Flags.DELETE in flags: - await update_attached_modules( + await register_modules( removed_modules=[maybe_module_at_port]) MODULE_LOG.info(f'Module Removed: {maybe_module_at_port}') if maybe_module_at_port is not None and aionotify.Flags.CREATE in flags: - await update_attached_modules(new_modules=[maybe_module_at_port]) + await register_modules(new_modules=[maybe_module_at_port]) MODULE_LOG.info(f'Module Added: {maybe_module_at_port}') async def build_module(self, port: str, model: str, interrupt_callback) -> modules.AbstractModule: - MODULE_LOG.info(f'\n\nBUILD Module {port}{model}') return await modules.build( port=port, which=model, simulating=False, interrupt_callback=interrupt_callback) - async def update_module( - self, - module: modules.AbstractModule, - firmware_file: str, - loop: Optional[asyncio.AbstractEventLoop])\ - -> modules.AbstractModule: - return await modules.update_firmware( - module, firmware_file, loop) - async def connect(self, port: str = None): self._smoothie_driver.connect(port) await self.update_fw_version() diff --git a/api/src/opentrons/hardware_control/simulator.py b/api/src/opentrons/hardware_control/simulator.py index 54848eb6ffc..d2b0742a0d6 100644 --- a/api/src/opentrons/hardware_control/simulator.py +++ b/api/src/opentrons/hardware_control/simulator.py @@ -85,11 +85,7 @@ def __init__( self._config = config self._loop = loop self._attached_instruments = attached_instruments - self._attached_modules = [ - ModuleAtPort( - port=f'/dev/ot_module_simulated_mod{str(idx)}', name=mod) - for idx, mod - in enumerate(attached_modules)] + self._stubbed_attached_modules = attached_modules self._position = copy.copy(_HOME_POSITION) # Engaged axes start all true in smoothie for some reason so we # imitate that here @@ -196,11 +192,13 @@ def get_attached_instruments( def set_active_current(self, axis, amp): pass - def get_attached_modules(self) -> List[modules.ModuleAtPort]: - return self._attached_modules - - async def watch_modules(self, loop: asyncio.AbstractEventLoop): - pass + async def watch_modules(self, register_modules): + new_modules = [ + modules.ModuleAtPort( + port=f'/dev/ot_module_sim_{mod}{str(idx)}', name=mod) + for idx, mod + in enumerate(self._stubbed_attached_modules)] + await register_modules(new_modules=new_modules) @contextmanager def save_current(self): @@ -216,13 +214,6 @@ async def build_module(self, simulating=True, interrupt_callback=interrupt_callback) - async def update_module( - self, module: modules.AbstractModule, - firmware_file: str, - loop: Optional[asyncio.AbstractEventLoop])\ - -> modules.AbstractModule: - return module - @property def axis_bounds(self) -> Dict[str, Tuple[float, float]]: """ The (minimum, maximum) bounds for each axis. """ diff --git a/api/tests/opentrons/conftest.py b/api/tests/opentrons/conftest.py index 57edc5ce79e..2177db495f3 100755 --- a/api/tests/opentrons/conftest.py +++ b/api/tests/opentrons/conftest.py @@ -5,7 +5,6 @@ import aionotify except OSError: aionotify = None - pass import sys import asyncio import contextlib @@ -209,7 +208,7 @@ def _should_skip_api2(request): and request.param != using_api2 -@pytest.mark.skipif(not aionotify, +@pytest.mark.skipif(aionotify is None, reason="requires inotify (linux only)") @pytest.fixture( params=[ @@ -440,6 +439,8 @@ def hardware(request, loop, virtual_smoothie_env): yield hw +@pytest.mark.skipif(aionotify is None, + reason="requires inotify (linux only)") @pytest.fixture( params=[ pytest.param(using_api1, marks=pytest.mark.apiv1), diff --git a/api/tests/opentrons/hardware_control/test_instruments.py b/api/tests/opentrons/hardware_control/test_instruments.py index fdac8352a2e..8959c9bf653 100644 --- a/api/tests/opentrons/hardware_control/test_instruments.py +++ b/api/tests/opentrons/hardware_control/test_instruments.py @@ -1,6 +1,9 @@ import asyncio from unittest import mock - +try: + import aionotify +except OSError: + aionotify = None import pytest from opentrons import types from opentrons import hardware_control as hc @@ -95,9 +98,8 @@ async def test_backwards_compatibility(dummy_backwards_compatibility, loop): assert attached[mount]['max_volume'] == volumes[mount]['max'] -@pytest.mark.skipif(not hc.Controller, - reason='hardware controller not available ' - '(probably windows)') +@pytest.mark.skipif(aionotify is None, + reason='inotify not available') async def test_cache_instruments_hc(monkeypatch, dummy_instruments, hardware_controller_lockfile, running_on_pi, cntrlr_mock_connect, loop): diff --git a/api/tests/opentrons/hardware_control/test_modules.py b/api/tests/opentrons/hardware_control/test_modules.py index 4f8f4b942f4..eb9d158c174 100644 --- a/api/tests/opentrons/hardware_control/test_modules.py +++ b/api/tests/opentrons/hardware_control/test_modules.py @@ -1,10 +1,17 @@ import pytest +import asyncio +try: + import aionotify +except OSError: + aionotify = None import opentrons.hardware_control as hardware_control +from opentrons.hardware_control.modules import ModuleAtPort async def test_get_modules_simulating(): mods = ['tempdeck', 'magdeck', 'thermocycler'] api = hardware_control.API.build_hardware_simulator(attached_modules=mods) + await asyncio.sleep(0.05) from_api = api.attached_modules assert sorted([mod.name() for mod in from_api]) == sorted(mods) @@ -13,59 +20,53 @@ async def test_module_caching(): mod_names = ['tempdeck'] api = hardware_control.API.build_hardware_simulator( attached_modules=mod_names) + await asyncio.sleep(0.05) # Check that we can add and remove modules and the caching keeps up found_mods = api.attached_modules assert found_mods[0].name() == 'tempdeck' - new_mods = api.attached_modules - assert new_mods[0] is found_mods[0] - api._backend._attached_modules.append(('mod2', 'magdeck')) + await api.register_modules( + new_modules=[ModuleAtPort(port='/dev/ot_module_sim_tempdeck1', + name='magdeck') + ]) with_magdeck = api.attached_modules assert len(with_magdeck) == 2 assert with_magdeck[0] is found_mods[0] - api._backend._attached_modules = api._backend._attached_modules[1:] + print(f'\n\n\n{api.attached_modules[0].port}') + await api.register_modules( + removed_modules=[ModuleAtPort(port='/dev/ot_module_sim_tempdeck0', + name='tempdeck') + ]) only_magdeck = api.attached_modules assert only_magdeck[0] is with_magdeck[1] # Check that two modules of the same kind on different ports are # distinct - api._backend._attached_modules.append(('mod3', 'magdeck')) + await api.register_modules( + new_modules=[ModuleAtPort(port='/dev/ot_module_sim_magdeck2', + name='magdeck') + ]) two_magdecks = api.attached_modules assert len(two_magdecks) == 2 assert two_magdecks[0] is with_magdeck[1] assert two_magdecks[1] is not two_magdecks[0] -async def test_module_update_logic(monkeypatch): - mod_names = ['tempdeck'] - api = hardware_control.API.build_hardware_simulator( - attached_modules=mod_names) - mods = api.attached_modules - old = mods[0] - - async def new_update_module(mod, ff, loop=None): - return await hardware_control.modules.build( - 'weird-port', mod.name(), True, lambda x: None) - - monkeypatch.setattr(api._backend, 'update_module', new_update_module) - ok, msg = await api.update_module(mods[0], 'some_file') - - mods = api.attached_modules - assert len(mods) == 1 - - assert mods[0] is not old - - +@pytest.mark.skipif(aionotify is None, + reason="requires inotify (linux only)") @pytest.mark.skipif(not hardware_control.Controller, reason='hardware controller not available') async def test_module_update_integration(monkeypatch, loop, cntrlr_mock_connect, running_on_pi): api = await hardware_control.API.build_hardware_controller(loop=loop) - def mock_get_modules(): - return [('port1', 'tempdeck')] + def mock_attached_modules(): + return [ModuleAtPort(port='/dev/ot_module_sim_tempdeck0', + name='tempdeck') + ] - monkeypatch.setattr(api._backend, 'get_attached_modules', mock_get_modules) + monkeypatch.setattr(api, 'attached_modules', + mock_attached_modules) async def mock_build_module(port, model, callback): return await hardware_control.modules.build(port, diff --git a/api/tests/opentrons/server/test_update_endpoints.py b/api/tests/opentrons/server/test_update_endpoints.py index f1d1da0686b..9bec40213ac 100644 --- a/api/tests/opentrons/server/test_update_endpoints.py +++ b/api/tests/opentrons/server/test_update_endpoints.py @@ -119,7 +119,7 @@ async def test_update_module_firmware( fd.write(bytes(0x1234)) def mock_get_attached_modules(module): - return [('mod1', 'thermocycler')] + return [hw_modules.ModuleAtPort(port='mod1', name='thermocycler')] async def mock_enter_bootloader(driver, module): return '/dev/ot_module_avrdude_bootloader0' From 14a9458129f366a1419d58da2ce53b47399eaf03 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Tue, 12 Nov 2019 17:49:36 -0500 Subject: [PATCH 11/25] tests patched up --- .../hardware_control/modules/update.py | 2 +- api/src/opentrons/server/endpoints/control.py | 1 + api/tests/opentrons/cli/test_cli.py | 15 ++++++++---- api/tests/opentrons/conftest.py | 4 ++++ .../hardware_control/test_modules.py | 1 - .../server/test_control_endpoints.py | 12 ++++++---- .../opentrons/server/test_update_endpoints.py | 23 +++++++++++-------- 7 files changed, 37 insertions(+), 21 deletions(-) diff --git a/api/src/opentrons/hardware_control/modules/update.py b/api/src/opentrons/hardware_control/modules/update.py index befc1cc0f75..b1f03c41c74 100644 --- a/api/src/opentrons/hardware_control/modules/update.py +++ b/api/src/opentrons/hardware_control/modules/update.py @@ -172,7 +172,7 @@ async def _port_poll(is_old_bootloader, ports_before_switch=None): ports = await _discover_ports() if ports: discovered_ports = list(filter( - lambda x: x.endswith('bootloader'), ports)) + lambda x: 'bootloader' in x, ports)) if len(discovered_ports) == 1: new_port = discovered_ports[0] await asyncio.sleep(0.05) diff --git a/api/src/opentrons/server/endpoints/control.py b/api/src/opentrons/server/endpoints/control.py index 403f82f4332..1cca5a8fa34 100644 --- a/api/src/opentrons/server/endpoints/control.py +++ b/api/src/opentrons/server/endpoints/control.py @@ -128,6 +128,7 @@ async def get_attached_modules(request): } """ hw = hw_from_req(request) + print(f'\n\nhw: {hw}\n\n') if ff.use_protocol_api_v2(): hw_mods = hw.attached_modules module_data = [ diff --git a/api/tests/opentrons/cli/test_cli.py b/api/tests/opentrons/cli/test_cli.py index 3695c8313f4..6e3038e93a8 100755 --- a/api/tests/opentrons/cli/test_cli.py +++ b/api/tests/opentrons/cli/test_cli.py @@ -2,7 +2,10 @@ import pytest import sys import numpy as np - +try: + import aionotify +except OSError: + aionotify = None from opentrons.config import (CONFIG, robot_configs, advanced_settings as advs) @@ -65,6 +68,8 @@ def test_clear_config(mock_config, async_server): assert hardware.config == robot_configs.build_config({}, {}) +@pytest.mark.skipif(aionotify is None, + reason="requires inotify (linux only)") def test_save_and_clear_config(mock_config, sync_hardware, loop): # Clear should happen automatically after the following import, resetting # the deck cal to the default value @@ -202,10 +207,10 @@ def test_gantry_matrix_output( get_calibration_points(), hardware, loop=loop) expected = [ - [1.00, 0.00, 0.00, 0.00], - [0.00, 0.99852725, 0.00, 0.5132547], - [0.00, 0.00, 1.00, 0.00], - [0.00, 0.00, 0.00, 1.00]] + [1.00, 0.00, 0.00, 0.00], + [0.00, 0.99852725, 0.00, 0.5132547], + [0.00, 0.00, 1.00, 0.00], + [0.00, 0.00, 0.00, 1.00]] actual_points = { 1: (12.13, 9.5), diff --git a/api/tests/opentrons/conftest.py b/api/tests/opentrons/conftest.py index 2177db495f3..e904425af79 100755 --- a/api/tests/opentrons/conftest.py +++ b/api/tests/opentrons/conftest.py @@ -163,6 +163,8 @@ def using_api2(loop): hw_manager.set_config(old_config) +@pytest.mark.skipif(aionotify is None, + reason="requires inotify (linux only)") @contextlib.contextmanager def using_sync_api2(loop): if not os.environ.get('OT_API_FF_useProtocolApi2'): @@ -426,6 +428,8 @@ def virtual_smoothie_env(monkeypatch): monkeypatch.setenv('ENABLE_VIRTUAL_SMOOTHIE', 'false') +@pytest.mark.skipif(aionotify is None, + reason="requires inotify (linux only)") @pytest.fixture( params=[ pytest.param(using_api1, marks=pytest.mark.apiv1), diff --git a/api/tests/opentrons/hardware_control/test_modules.py b/api/tests/opentrons/hardware_control/test_modules.py index eb9d158c174..f6fe20dd149 100644 --- a/api/tests/opentrons/hardware_control/test_modules.py +++ b/api/tests/opentrons/hardware_control/test_modules.py @@ -32,7 +32,6 @@ async def test_module_caching(): with_magdeck = api.attached_modules assert len(with_magdeck) == 2 assert with_magdeck[0] is found_mods[0] - print(f'\n\n\n{api.attached_modules[0].port}') await api.register_modules( removed_modules=[ModuleAtPort(port='/dev/ot_module_sim_tempdeck0', name='tempdeck') diff --git a/api/tests/opentrons/server/test_control_endpoints.py b/api/tests/opentrons/server/test_control_endpoints.py index bac28f55f02..6b75333d6e3 100644 --- a/api/tests/opentrons/server/test_control_endpoints.py +++ b/api/tests/opentrons/server/test_control_endpoints.py @@ -121,9 +121,10 @@ def dummy_read_id(mount): async def test_get_modules_v2( async_server, loop, async_client, monkeypatch): hw = async_server['com.opentrons.hardware'] - monkeypatch.setattr(hw._backend, - '_attached_modules', - [('/dev/ot_module_magdeck1', 'magdeck')]) + magdeck = await hw._backend.build_module('/dev/ot_module_magdeck1', + 'magdeck', + lambda x: None) + monkeypatch.setattr(hw, 'attached_modules', [magdeck]) await check_modules_response(async_client) @@ -183,7 +184,10 @@ async def test_execute_module_command_v2( def dummy_get_attached_modules(): return [] - monkeypatch.setattr(hw, 'discover_modules', dummy_discover_modules) + magdeck = await hw._backend.build_module('/dev/ot_module_magdeck1', + 'magdeck', + lambda x: None) + monkeypatch.setattr(hw, 'attached_modules', [magdeck]) resp = await async_client.post('/modules/dummySerialMD', json={'command_type': 'deactivate'}) diff --git a/api/tests/opentrons/server/test_update_endpoints.py b/api/tests/opentrons/server/test_update_endpoints.py index 9bec40213ac..9f9d18abc64 100644 --- a/api/tests/opentrons/server/test_update_endpoints.py +++ b/api/tests/opentrons/server/test_update_endpoints.py @@ -110,6 +110,7 @@ async def test_update_module_firmware( async_client, async_server): + hw = async_server['com.opentrons.hardware'] client = async_client serial_num = 'dummySerialTC' fw_filename = 'dummyFirmware.hex' @@ -118,16 +119,16 @@ async def test_update_module_firmware( with open(os.path.join(tmpdir, fw_filename), 'wb') as fd: fd.write(bytes(0x1234)) - def mock_get_attached_modules(module): - return [hw_modules.ModuleAtPort(port='mod1', name='thermocycler')] + tc_module_at_port = hw_modules.ModuleAtPort( + port='/dev/ot_module_thermocycler1', + name='thermocycler') async def mock_enter_bootloader(driver, module): return '/dev/ot_module_avrdude_bootloader0' monkeypatch.setattr(hw_modules.update, 'enter_bootloader', mock_enter_bootloader) - monkeypatch.setattr(simulator.Simulator, 'get_attached_modules', - mock_get_attached_modules) + await hw.register_modules(new_modules=[tc_module_at_port]) # ========= Happy path ========== res_msg = {'message': f'Successully updated module {serial_num}', @@ -159,6 +160,7 @@ async def test_fail_update_module_firmware( async_client, async_server): + hw = async_server['com.opentrons.hardware'] client = async_client serial_num = 'dummySerialTC' fw_filename = 'dummyFirmware.hex' @@ -168,13 +170,14 @@ async def test_fail_update_module_firmware( fd.write(bytes(0x1234)) async def mock_enter_bootloader(driver, module): - return '/dev/ot_module_avrdude_bootloader0' + return '/dev/ot_module_samba_bootloader0' + + tc_module_at_port = hw_modules.ModuleAtPort( + port='/dev/ot_module_thermocycler1', + name='thermocycler') - def mock_get_attached_modules(module): - return [('mod1', 'thermocycler')] + await hw.register_modules(new_modules=[tc_module_at_port]) - monkeypatch.setattr(simulator.Simulator, 'get_attached_modules', - mock_get_attached_modules) monkeypatch.setattr(hw_modules.update, 'enter_bootloader', mock_enter_bootloader) @@ -185,7 +188,7 @@ def mock_get_attached_modules(module): async def mock_failed_upload_to_module1( port, firmware_file_path, upload_function, loop): - return ('mod1', (False, bootloader_error)) + return ('/dev/ot_module_thermocycler1', (False, bootloader_error)) expected_res1 = res_msg1 From 95d3559575e8cf97f9abc8c9e54804d52e7c0b1c Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Tue, 12 Nov 2019 18:16:35 -0500 Subject: [PATCH 12/25] feat(api): register module instances on os events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the hardware controller only became aware of underlying connected hardware modules when it was told to look via an http endpoint (only effectively used by the run app). In order to remove this unnecessary reliance, the hardware controller now watches the filesystem for changes using aionotify (a thin python wrapper of linux util). Changes to the physically connected modules should now be instantly reflected in the hardware controllers attached modules regardless of whether the hardware controller is observed by a third party or not. No more Schrödinger's modules re #3580 --- api/src/opentrons/server/endpoints/control.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/src/opentrons/server/endpoints/control.py b/api/src/opentrons/server/endpoints/control.py index 1cca5a8fa34..403f82f4332 100644 --- a/api/src/opentrons/server/endpoints/control.py +++ b/api/src/opentrons/server/endpoints/control.py @@ -128,7 +128,6 @@ async def get_attached_modules(request): } """ hw = hw_from_req(request) - print(f'\n\nhw: {hw}\n\n') if ff.use_protocol_api_v2(): hw_mods = hw.attached_modules module_data = [ From c4118c6ee2227f3922af3be262a3d9dda3bf8c7f Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Wed, 13 Nov 2019 11:34:11 -0500 Subject: [PATCH 13/25] address a few of seths comments --- api/src/opentrons/execute.py | 1 - .../opentrons/hardware_control/__init__.py | 13 ++++++------ .../opentrons/hardware_control/adapters.py | 19 ------------------ .../opentrons/hardware_control/controller.py | 20 ++++++++++++------- .../hardware_control/modules/__init__.py | 12 +++++------ api/src/opentrons/hardware_control/types.py | 6 ++++++ .../server/test_control_endpoints.py | 10 ---------- 7 files changed, 31 insertions(+), 50 deletions(-) diff --git a/api/src/opentrons/execute.py b/api/src/opentrons/execute.py index 62cb0b4b1ad..c5ec2a63d23 100644 --- a/api/src/opentrons/execute.py +++ b/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 diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index cb97121f440..eeb2025d7b8 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -400,7 +400,7 @@ async def update_firmware( return await self._backend.update_firmware(firmware_file, checked_loop, explicit_modeset) - +Callable[[AbstractModule], None] def _call_on_attached_modules(self, method): for module in self.attached_modules: maybe_module_method = getattr(module, method, None) @@ -476,7 +476,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 @@ -1355,7 +1354,11 @@ def set_pipette_speed(self, mount, 'blow_out_flow_rate', self._plunger_flowrate(this_pipette, blow_out, 'dispense')) - async def register_modules(self, new_modules = [], removed_modules = []): + async def register_modules(self, new_modules = None, removed_modules = 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] @@ -1370,10 +1373,6 @@ async def register_modules(self, new_modules = [], removed_modules = []): self._log.info(f"Module {name} discovered and attached " \ f" at port {port}") - # TODO: remove this function once APIv1 is sunset - async def discover_modules(self): - pass - async def _do_tp(self, pip, mount) -> top_types.Point: """ Execute the work of tip probe. diff --git a/api/src/opentrons/hardware_control/adapters.py b/api/src/opentrons/hardware_control/adapters.py index be108063c18..a35ee20db61 100644 --- a/api/src/opentrons/hardware_control/adapters.py +++ b/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) diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index 3328017c252..842ae35fc3a 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -12,7 +12,7 @@ import opentrons.config from opentrons.types import Mount -from . import modules +from . import modules, types MODULE_LOG = logging.getLogger(__name__) @@ -41,10 +41,16 @@ def __init__(self, config): self._smoothie_driver = driver_3_0.SmoothieDriver_3_0_0( config=self.config, handle_locks=False) self._cached_fw_version: Optional[str] = None - self._module_watcher = aionotify.Watcher() - self._module_watcher.watch(alias='modules', - path='/dev', - flags=(aionotify.Flags.CREATE | aionotify.Flags.DELETE)) + try: + self._module_watcher = aionotify.Watcher() + self._module_watcher.watch( + alias='modules', + path='/dev', + flags=(aionotify.Flags.CREATE | aionotify.Flags.DELETE)) + except NameError: + MODULE_LOG.info( + 'Failed to initiate aionotify,' + 'likely because not running on linux') def update_position(self) -> Dict[str, float]: self._smoothie_driver.update_position() @@ -117,12 +123,12 @@ def save_current(self): def set_pipette_speed(self, val: float): self._smoothie_driver.set_speed(val) - async def watch_modules(self, loop: asyncio.AbstractEventLoop, register_modules): + async def watch_modules(self, loop: asyncio.AbstractEventLoop, + register_modules: types.RegisterModules): await self._module_watcher.setup(loop) initial_modules = modules.discover() await register_modules(new_modules=initial_modules) - MODULE_LOG.info(f'\n\nINIT MODULES: {initial_modules}\n\n') while not self._module_watcher.closed: event = await self._module_watcher.get_event() flags = aionotify.Flags.parse(event.flags) diff --git a/api/src/opentrons/hardware_control/modules/__init__.py b/api/src/opentrons/hardware_control/modules/__init__.py index 7b7bbe5a705..279d5b6a8e4 100644 --- a/api/src/opentrons/hardware_control/modules/__init__.py +++ b/api/src/opentrons/hardware_control/modules/__init__.py @@ -6,7 +6,7 @@ from typing import List, Optional, Tuple from collections import namedtuple -from opentrons.config import IS_ROBOT +from opentrons.config import IS_ROBOT, IS_LINUX from .mod_abc import AbstractModule # Must import tempdeck and magdeck (and other modules going forward) so they # actually create the subclasses @@ -32,6 +32,8 @@ class AbsentModuleError(Exception): MODULE_TYPES = {cls.name(): cls for cls in AbstractModule.__subclasses__()} # type: ignore +MODULE_PORT_REGEX = re.compile('|'.join(MODULE_TYPES.keys()), re.I) + async def build( port: str, @@ -46,8 +48,7 @@ def get_module_at_port(port: str) -> Optional[ModuleAtPort]: """ Given a port, returns either a ModuleAtPort if it is a recognized module, or None if not recognized. """ - module_port_regex = re.compile('|'.join(MODULE_TYPES.keys()), re.I) - match = module_port_regex.search(port) + match = MODULE_PORT_REGEX.search(port) if match: name = match.group().lower() return ModuleAtPort(port=f'/dev/{port}', name=name) @@ -58,16 +59,15 @@ def discover() -> List[ModuleAtPort]: """ Scan for connected modules and return list of tuples of serial ports and device names """ - if IS_ROBOT: + if IS_ROBOT and IS_LINUX: devices = glob('/dev/ot_module*') else: devices = [] discovered_modules = [] - module_port_regex = re.compile('|'.join(MODULE_TYPES.keys()), re.I) for port in devices: - match = module_port_regex.search(port) + match = MODULE_PORT_REGEX.search(port) if match: name = match.group().lower() if name not in MODULE_TYPES: diff --git a/api/src/opentrons/hardware_control/types.py b/api/src/opentrons/hardware_control/types.py index 3f2f12a3466..2dda6bcc582 100644 --- a/api/src/opentrons/hardware_control/types.py +++ b/api/src/opentrons/hardware_control/types.py @@ -2,6 +2,7 @@ from typing import Tuple import opentrons.types +from .modules import ModuleAtPort class Axis(enum.Enum): @@ -91,3 +92,8 @@ class CriticalPoint(enum.Enum): The end of the front-most nozzle of a multipipette with a tip attached. Only relevant when a multichannel pipette is present. """ + + +RegisterModules = Callable[[List[ModuleAtPort], + List[ModuleAtPort]], + Coroutine[None, None, None]] diff --git a/api/tests/opentrons/server/test_control_endpoints.py b/api/tests/opentrons/server/test_control_endpoints.py index 6b75333d6e3..e54d092d758 100644 --- a/api/tests/opentrons/server/test_control_endpoints.py +++ b/api/tests/opentrons/server/test_control_endpoints.py @@ -151,15 +151,6 @@ async def check_modules_response(async_client): assert 'engaged' in body['modules'][0]['data'] -@pytest.fixture -async def dummy_discover_modules(): - mag_module = await modules.build('', 'magdeck', True, lambda x: None) - - async def stub(): - return [mag_module] - return stub - - @pytest.fixture def dummy_attached_leg_modules(): mag_module = legacy_modules.MagDeck() @@ -173,7 +164,6 @@ def dummy_attached_leg_modules(): @pytest.mark.api2_only async def test_execute_module_command_v2( - dummy_discover_modules, virtual_smoothie_env, loop, async_server, From 82366e998f1307c411e65f9afaed69edb958a815 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Wed, 13 Nov 2019 17:29:59 -0500 Subject: [PATCH 14/25] address more of seth's comments --- api/src/opentrons/hardware_control/controller.py | 11 ++++++----- api/src/opentrons/hardware_control/modules/update.py | 4 ++++ api/src/opentrons/hardware_control/simulator.py | 3 ++- api/src/opentrons/hardware_control/types.py | 8 ++++---- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index 842ae35fc3a..7c25cf7ee78 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -12,7 +12,8 @@ import opentrons.config from opentrons.types import Mount -from . import modules, types +from . import modules +from .types import RegisterModules MODULE_LOG = logging.getLogger(__name__) @@ -47,9 +48,9 @@ def __init__(self, config): alias='modules', path='/dev', flags=(aionotify.Flags.CREATE | aionotify.Flags.DELETE)) - except NameError: + except AttributeError: MODULE_LOG.info( - 'Failed to initiate aionotify,' + 'Failed to initiate aionotify, cannot watch modules,' 'likely because not running on linux') def update_position(self) -> Dict[str, float]: @@ -124,12 +125,12 @@ def set_pipette_speed(self, val: float): self._smoothie_driver.set_speed(val) async def watch_modules(self, loop: asyncio.AbstractEventLoop, - register_modules: types.RegisterModules): + register_modules: RegisterModules): await self._module_watcher.setup(loop) initial_modules = modules.discover() await register_modules(new_modules=initial_modules) - while not self._module_watcher.closed: + while (aionotify is not None) and (not self._module_watcher.closed): event = await self._module_watcher.get_event() flags = aionotify.Flags.parse(event.flags) if 'ot_module' in event.name: diff --git a/api/src/opentrons/hardware_control/modules/update.py b/api/src/opentrons/hardware_control/modules/update.py index b1f03c41c74..a1ebd0236ab 100644 --- a/api/src/opentrons/hardware_control/modules/update.py +++ b/api/src/opentrons/hardware_control/modules/update.py @@ -175,6 +175,10 @@ async def _port_poll(is_old_bootloader, ports_before_switch=None): lambda x: 'bootloader' in x, ports)) if len(discovered_ports) == 1: new_port = discovered_ports[0] + elif len(discovered_ports) > 1: + raise OSError('Multiple new bootloader ports' + 'found on mode switch') + await asyncio.sleep(0.05) return new_port diff --git a/api/src/opentrons/hardware_control/simulator.py b/api/src/opentrons/hardware_control/simulator.py index d2b0742a0d6..9954c098af4 100644 --- a/api/src/opentrons/hardware_control/simulator.py +++ b/api/src/opentrons/hardware_control/simulator.py @@ -10,6 +10,7 @@ configs) from opentrons.drivers.smoothie_drivers import SimulatingDriver from . import modules +from .types import RegisterModules MODULE_LOG = logging.getLogger(__name__) @@ -192,7 +193,7 @@ def get_attached_instruments( def set_active_current(self, axis, amp): pass - async def watch_modules(self, register_modules): + async def watch_modules(self, register_modules: RegisterModules): new_modules = [ modules.ModuleAtPort( port=f'/dev/ot_module_sim_{mod}{str(idx)}', name=mod) diff --git a/api/src/opentrons/hardware_control/types.py b/api/src/opentrons/hardware_control/types.py index 2dda6bcc582..b09d9c8d02c 100644 --- a/api/src/opentrons/hardware_control/types.py +++ b/api/src/opentrons/hardware_control/types.py @@ -1,5 +1,5 @@ import enum -from typing import Tuple +from typing import Tuple, Awaitable, Callable, Optional, List import opentrons.types from .modules import ModuleAtPort @@ -94,6 +94,6 @@ class CriticalPoint(enum.Enum): """ -RegisterModules = Callable[[List[ModuleAtPort], - List[ModuleAtPort]], - Coroutine[None, None, None]] +RegisterModules = Callable[[Optional[List[ModuleAtPort]], + Optional[List[ModuleAtPort]]], + Awaitable[None]] From f8c939fbc246173355d68f83281558f7ac6f7325 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 15 Nov 2019 13:34:06 -0500 Subject: [PATCH 15/25] add to pipfile and install requires --- api/Pipfile | 1 + api/setup.py | 1 + api/src/opentrons/hardware_control/__init__.py | 2 +- api/src/opentrons/hardware_control/controller.py | 6 ++++-- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/api/Pipfile b/api/Pipfile index e1ae08094e0..db73b2bbec6 100755 --- a/api/Pipfile +++ b/api/Pipfile @@ -29,3 +29,4 @@ aiohttp = "==3.4.4" jsonschema = "==3.0.2" jsonrpcserver = "==4.0.3" systemd-python = {version="==234", sys_platform="== 'linux'"} +aionotify = "==0.2.0" diff --git a/api/setup.py b/api/setup.py index 00310ba685a..40badf2d0b9 100755 --- a/api/setup.py +++ b/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', ] diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index eeb2025d7b8..b64c1e70a4c 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -401,7 +401,7 @@ async def update_firmware( checked_loop, explicit_modeset) Callable[[AbstractModule], None] - def _call_on_attached_modules(self, method): + 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): diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index 7c25cf7ee78..be0d4355aa1 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -126,11 +126,13 @@ def set_pipette_speed(self, val: float): async def watch_modules(self, loop: asyncio.AbstractEventLoop, register_modules: RegisterModules): - await self._module_watcher.setup(loop) + can_watch = aionotify is not None + if can_watch: + await self._module_watcher.setup(loop) initial_modules = modules.discover() await register_modules(new_modules=initial_modules) - while (aionotify is not None) and (not self._module_watcher.closed): + while can_watch and (not self._module_watcher.closed): event = await self._module_watcher.get_event() flags = aionotify.Flags.parse(event.flags) if 'ot_module' in event.name: From fa09544c0291882bf23c5aa614fd6441825d272a Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 15 Nov 2019 13:36:23 -0500 Subject: [PATCH 16/25] fixup cruft --- api/src/opentrons/hardware_control/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index b64c1e70a4c..5943a8a91fb 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -400,7 +400,6 @@ async def update_firmware( return await self._backend.update_firmware(firmware_file, checked_loop, explicit_modeset) -Callable[[AbstractModule], None] def _call_on_attached_modules(self, method: str): for module in self.attached_modules: maybe_module_method = getattr(module, method, None) From 0d535c9bdb92be228ec5e9c38ea3985ebe631802 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 15 Nov 2019 15:04:07 -0500 Subject: [PATCH 17/25] typing and lint --- Pipfile.lock | 30 ++++++++++ api/Pipfile | 1 + api/Pipfile.lock | 60 +++++++++++-------- .../opentrons/hardware_control/__init__.py | 19 ++++-- .../opentrons/hardware_control/controller.py | 18 +++--- .../hardware_control/modules/__init__.py | 3 +- .../opentrons/hardware_control/simulator.py | 6 +- api/src/opentrons/hardware_control/types.py | 12 ++-- api/src/opentrons/server/endpoints/update.py | 2 +- api/tests/opentrons/cli/test_cli.py | 2 +- api/tests/opentrons/conftest.py | 3 +- .../hardware_control/test_instruments.py | 2 +- .../hardware_control/test_modules.py | 2 +- api/tests/opentrons/labware/test_modules.py | 3 +- .../server/test_control_endpoints.py | 1 - .../opentrons/server/test_update_endpoints.py | 2 +- 16 files changed, 109 insertions(+), 57 deletions(-) create mode 100644 Pipfile.lock diff --git a/Pipfile.lock b/Pipfile.lock new file mode 100644 index 00000000000..fa6d953fcfd --- /dev/null +++ b/Pipfile.lock @@ -0,0 +1,30 @@ +{ + "_meta": { + "hash": { + "sha256": "ff390d9ac859b3161d51457dcbab884088758e28449e9e9ea9c325b19aa289ee" + }, + "pipfile-spec": 6, + "requires": { + "python_version": "3.6" + }, + "sources": [ + { + "name": "pypi", + "url": "https://pypi.org/simple", + "verify_ssl": true + } + ] + }, + "default": {}, + "develop": { + "typing-extensions": { + "hashes": [ + "sha256:091ecc894d5e908ac75209f10d5b4f118fbdb2eb1ede6a63544054bb1edb41f2", + "sha256:910f4656f54de5993ad9304959ce9bb903f90aadc7c67a0bef07e678014e892d", + "sha256:cf8b63fedea4d89bab840ecbb93e75578af28f76f66c35889bd7065f5af88575" + ], + "index": "pypi", + "version": "==3.7.4.1" + } + } +} diff --git a/api/Pipfile b/api/Pipfile index db73b2bbec6..248442917f3 100755 --- a/api/Pipfile +++ b/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" diff --git a/api/Pipfile.lock b/api/Pipfile.lock index c08c6f9b23a..3907c73cb4e 100644 --- a/api/Pipfile.lock +++ b/api/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "bec59e8f6b42b5996229d846f7037df7a3a0f3eb819a4ab486663f86c5e91ddb" + "sha256": "8c2780aac77bf75e82efada7fe47c13be759d08638f64ff44b88c275a3b740d9" }, "pipfile-spec": 6, "requires": { @@ -44,6 +44,14 @@ "index": "pypi", "version": "==3.4.4" }, + "aionotify": { + "hashes": [ + "sha256:385e1becfaac2d9f4326673033d53912ef9565b6febdedbec593ee966df392c6", + "sha256:64b702ad0eb115034533f9f62730a9253b79f5ff0fbf3d100c392124cdf12507" + ], + "index": "pypi", + "version": "==0.2.0" + }, "apply-defaults": { "hashes": [ "sha256:1ce26326a61d8773d38a9726a345c6525a91a6120d7333af79ad792dacb6246c" @@ -177,9 +185,9 @@ }, "pyrsistent": { "hashes": [ - "sha256:34b47fa169d6006b32e99d4b3c4031f155e6e68ebcc107d6454852e8e0ee6533" + "sha256:eb6545dbeb1aa69ab1fb4809bfbf5a8705e44d92ef8fc7c2361682a47c46c778" ], - "version": "==0.15.4" + "version": "==0.15.5" }, "pyserial": { "hashes": [ @@ -191,10 +199,10 @@ }, "six": { "hashes": [ - "sha256:3350809f0555b11f552448330d0b52d5f24c91a322ea4a15ef22629740f3761c", - "sha256:d16a0141ec1a18405cd4ce8b4613101da75da0e9a7aec5bdd4fa804d0e0eba73" + "sha256:1f1b7d42e254082a9db6279deae68afb421ceba6158efa6131de7b3003ee93fd", + "sha256:30f610279e8b2578cab6db20741130331735c781b56053c59c4076da27f06b66" ], - "version": "==1.12.0" + "version": "==1.13.0" }, "systemd-python": { "hashes": [ @@ -604,18 +612,18 @@ }, "pyparsing": { "hashes": [ - "sha256:6f98a7b9397e206d78cc01df10131398f1c8b8510a2f4d97d9abd82e1aacdd80", - "sha256:d9338df12903bbf5d65a0e4e87c2161968b10d2e489652bb47001d82a9b028b4" + "sha256:20f995ecd72f2a1f4bf6b072b63b22e2eb457836601e76d6e5dfcd75436acc1f", + "sha256:4ca62001be367f01bd3e92ecbb79070272a9d4964dce6a48a82ff0b8bc7e683a" ], - "version": "==2.4.2" + "version": "==2.4.5" }, "pytest": { "hashes": [ - "sha256:7e4800063ccfc306a53c461442526c5571e1462f61583506ce97e4da6a1d88c8", - "sha256:ca563435f4941d0cb34767301c27bc65c510cb82e90b9ecf9cb52dc2c63caaa0" + "sha256:15837d2880cb94821087bc07476892ea740696b20e90288fd6c19e44b435abdb", + "sha256:b6cf7ad9064049ee486586b3a0ddd70dc5136c40e1147e7d286efd77ba66c5eb" ], "index": "pypi", - "version": "==5.2.1" + "version": "==5.2.3" }, "pytest-aiohttp": { "hashes": [ @@ -663,10 +671,10 @@ }, "six": { "hashes": [ - "sha256:3350809f0555b11f552448330d0b52d5f24c91a322ea4a15ef22629740f3761c", - "sha256:d16a0141ec1a18405cd4ce8b4613101da75da0e9a7aec5bdd4fa804d0e0eba73" + "sha256:1f1b7d42e254082a9db6279deae68afb421ceba6158efa6131de7b3003ee93fd", + "sha256:30f610279e8b2578cab6db20741130331735c781b56053c59c4076da27f06b66" ], - "version": "==1.12.0" + "version": "==1.13.0" }, "snowballstemmer": { "hashes": [ @@ -727,10 +735,10 @@ }, "tqdm": { "hashes": [ - "sha256:abc25d0ce2397d070ef07d8c7e706aede7920da163c64997585d42d3537ece3d", - "sha256:dd3fcca8488bb1d416aa7469d2f277902f26260c45aa86b667b074cd44b3b115" + "sha256:9de4722323451eb7818deb0161d9d5523465353a6707a9f500d97ee42919b902", + "sha256:c1d677f3a85fa291b34bdf8f770f877119b9754b32673699653556f85e2c2f13" ], - "version": "==4.36.1" + "version": "==4.38.0" }, "twine": { "hashes": [ @@ -767,19 +775,19 @@ }, "typing-extensions": { "hashes": [ - "sha256:2ed632b30bb54fc3941c382decfd0ee4148f5c591651c9272473fea2c6397d95", - "sha256:b1edbbf0652660e32ae780ac9433f4231e7339c7f9a8057d0f042fcbcea49b87", - "sha256:d8179012ec2c620d3791ca6fe2bf7979d979acdbef1fca0bc56b37411db682ed" + "sha256:091ecc894d5e908ac75209f10d5b4f118fbdb2eb1ede6a63544054bb1edb41f2", + "sha256:910f4656f54de5993ad9304959ce9bb903f90aadc7c67a0bef07e678014e892d", + "sha256:cf8b63fedea4d89bab840ecbb93e75578af28f76f66c35889bd7065f5af88575" ], - "markers": "python_version < '3.7'", - "version": "==3.7.4" + "index": "pypi", + "version": "==3.7.4.1" }, "urllib3": { "hashes": [ - "sha256:3de946ffbed6e6746608990594d08faac602528ac7015ac28d33cee6a45b7398", - "sha256:9a107b99a5393caf59c7aa3c1249c16e6879447533d0887f4336dde834c7be86" + "sha256:a8a318824cc77d1fd4b2bec2ded92646630d7fe8619497b142c84a9e6f5a7293", + "sha256:f3c5fd51747d450d4dcf6f923c81f78f811aab8205fda64b0aba34a4e48b0745" ], - "version": "==1.25.6" + "version": "==1.25.7" }, "wcwidth": { "hashes": [ diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index 5943a8a91fb..7b8c4fe3438 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -16,7 +16,7 @@ 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 @@ -25,6 +25,8 @@ from .controller import Controller from . import modules from .types import Axis, HardwareAPILike, CriticalPoint +if TYPE_CHECKING: + from .types import RegisterModules # noqa (F501) mod_log = logging.getLogger(__name__) @@ -163,7 +165,6 @@ def build_hardware_simulator( register_modules=api_instance.register_modules)) return api_instance - def __repr__(self): return '<{} using backend {}>'.format(type(self), type(self._backend)) @@ -400,6 +401,7 @@ async def update_firmware( return await self._backend.update_firmware(firmware_file, checked_loop, explicit_modeset) + def _call_on_attached_modules(self, method: str): for module in self.attached_modules: maybe_module_method = getattr(module, method, None) @@ -1353,7 +1355,11 @@ def set_pipette_speed(self, mount, 'blow_out_flow_rate', self._plunger_flowrate(this_pipette, blow_out, 'dispense')) - async def register_modules(self, new_modules = None, removed_modules = None): + 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: @@ -1361,15 +1367,16 @@ async def register_modules(self, new_modules = None, removed_modules = None): 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 " \ + 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, + 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 " \ + self._log.info(f"Module {name} discovered and attached " f" at port {port}") async def _do_tp(self, pip, mount) -> top_types.Point: diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index be0d4355aa1..29590f54a1a 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -1,11 +1,11 @@ import asyncio from contextlib import contextmanager, ExitStack import logging -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING try: - import aionotify + import aionotify # type: ignore except OSError: - aionotify = None + aionotify = None # type: ignore from opentrons.drivers.smoothie_drivers import driver_3_0 from opentrons.drivers.rpi_drivers import gpio @@ -13,7 +13,9 @@ from opentrons.types import Mount from . import modules -from .types import RegisterModules + +if TYPE_CHECKING: + from .types import RegisterModules # noqa (F501) MODULE_LOG = logging.getLogger(__name__) @@ -125,7 +127,7 @@ def set_pipette_speed(self, val: float): self._smoothie_driver.set_speed(val) async def watch_modules(self, loop: asyncio.AbstractEventLoop, - register_modules: RegisterModules): + register_modules: 'RegisterModules'): can_watch = aionotify is not None if can_watch: await self._module_watcher.setup(loop) @@ -137,11 +139,13 @@ async def watch_modules(self, loop: asyncio.AbstractEventLoop, flags = aionotify.Flags.parse(event.flags) if 'ot_module' in event.name: maybe_module_at_port = modules.get_module_at_port(event.name) - if maybe_module_at_port is not None and aionotify.Flags.DELETE in flags: + if maybe_module_at_port is not None and \ + aionotify.Flags.DELETE in flags: await register_modules( removed_modules=[maybe_module_at_port]) MODULE_LOG.info(f'Module Removed: {maybe_module_at_port}') - if maybe_module_at_port is not None and aionotify.Flags.CREATE in flags: + if maybe_module_at_port is not None and \ + aionotify.Flags.CREATE in flags: await register_modules(new_modules=[maybe_module_at_port]) MODULE_LOG.info(f'Module Added: {maybe_module_at_port}') diff --git a/api/src/opentrons/hardware_control/modules/__init__.py b/api/src/opentrons/hardware_control/modules/__init__.py index 279d5b6a8e4..43773f00d4e 100644 --- a/api/src/opentrons/hardware_control/modules/__init__.py +++ b/api/src/opentrons/hardware_control/modules/__init__.py @@ -1,9 +1,8 @@ import asyncio import logging -import os from glob import glob import re -from typing import List, Optional, Tuple +from typing import List, Optional from collections import namedtuple from opentrons.config import IS_ROBOT, IS_LINUX diff --git a/api/src/opentrons/hardware_control/simulator.py b/api/src/opentrons/hardware_control/simulator.py index 9954c098af4..4a263363293 100644 --- a/api/src/opentrons/hardware_control/simulator.py +++ b/api/src/opentrons/hardware_control/simulator.py @@ -1,8 +1,7 @@ -import asyncio import copy import logging from threading import Event -from typing import Dict, Optional, List, Tuple +from typing import Dict, Optional, List, Tuple, TYPE_CHECKING from contextlib import contextmanager from opentrons import types from opentrons.config.pipette_config import (config_models, @@ -10,7 +9,8 @@ configs) from opentrons.drivers.smoothie_drivers import SimulatingDriver from . import modules -from .types import RegisterModules +if TYPE_CHECKING: + from .types import RegisterModules # noqa (F501) MODULE_LOG = logging.getLogger(__name__) diff --git a/api/src/opentrons/hardware_control/types.py b/api/src/opentrons/hardware_control/types.py index b09d9c8d02c..eaf8428bc75 100644 --- a/api/src/opentrons/hardware_control/types.py +++ b/api/src/opentrons/hardware_control/types.py @@ -1,5 +1,6 @@ import enum -from typing import Tuple, Awaitable, Callable, Optional, List +from typing import Tuple, List +from typing_extensions import Protocol import opentrons.types from .modules import ModuleAtPort @@ -94,6 +95,9 @@ class CriticalPoint(enum.Enum): """ -RegisterModules = Callable[[Optional[List[ModuleAtPort]], - Optional[List[ModuleAtPort]]], - Awaitable[None]] +class RegisterModules(Protocol): + async def __call__( + self, + new_modules: List[ModuleAtPort] = None, + removed_modules: List[ModuleAtPort] = None + ) -> None: ... diff --git a/api/src/opentrons/server/endpoints/update.py b/api/src/opentrons/server/endpoints/update.py index 522bc20a818..3ad888a5842 100644 --- a/api/src/opentrons/server/endpoints/update.py +++ b/api/src/opentrons/server/endpoints/update.py @@ -59,7 +59,7 @@ async def _upload_to_module(hw, serialnum, fw_filename, loop): if module.device_info.get('serial') == serialnum: log.info("Module with serial {} found".format(serialnum)) try: - new_instance = await asyncio.wait_for( + await asyncio.wait_for( modules.update_firmware(module, fw_filename, loop), UPDATE_TIMEOUT) return f'Successully updated module {serialnum}', 200 diff --git a/api/tests/opentrons/cli/test_cli.py b/api/tests/opentrons/cli/test_cli.py index 6e3038e93a8..6021940941e 100755 --- a/api/tests/opentrons/cli/test_cli.py +++ b/api/tests/opentrons/cli/test_cli.py @@ -5,7 +5,7 @@ try: import aionotify except OSError: - aionotify = None + aionotify = None # type: ignore from opentrons.config import (CONFIG, robot_configs, advanced_settings as advs) diff --git a/api/tests/opentrons/conftest.py b/api/tests/opentrons/conftest.py index e904425af79..a9d6d91a688 100755 --- a/api/tests/opentrons/conftest.py +++ b/api/tests/opentrons/conftest.py @@ -4,8 +4,7 @@ try: import aionotify except OSError: - aionotify = None -import sys + aionotify = None # type: ignore import asyncio import contextlib import os diff --git a/api/tests/opentrons/hardware_control/test_instruments.py b/api/tests/opentrons/hardware_control/test_instruments.py index 8959c9bf653..958ec14f91a 100644 --- a/api/tests/opentrons/hardware_control/test_instruments.py +++ b/api/tests/opentrons/hardware_control/test_instruments.py @@ -3,7 +3,7 @@ try: import aionotify except OSError: - aionotify = None + aionotify = None # type: ignore import pytest from opentrons import types from opentrons import hardware_control as hc diff --git a/api/tests/opentrons/hardware_control/test_modules.py b/api/tests/opentrons/hardware_control/test_modules.py index f6fe20dd149..9eab297e90e 100644 --- a/api/tests/opentrons/hardware_control/test_modules.py +++ b/api/tests/opentrons/hardware_control/test_modules.py @@ -3,7 +3,7 @@ try: import aionotify except OSError: - aionotify = None + aionotify = None # type: ignore import opentrons.hardware_control as hardware_control from opentrons.hardware_control.modules import ModuleAtPort diff --git a/api/tests/opentrons/labware/test_modules.py b/api/tests/opentrons/labware/test_modules.py index 16655893f59..3c5d64d08ac 100644 --- a/api/tests/opentrons/labware/test_modules.py +++ b/api/tests/opentrons/labware/test_modules.py @@ -186,7 +186,8 @@ async def test_port_poll(virtual_smoothie_env, monkeypatch): # Case 1: Bootloader port is successfully opened on the module async def mock_discover_ports1(): - return ['/dev/ot_module_magdeck0', '/dev/ot_module_avrdude_bootloader1'] + return ['/dev/ot_module_magdeck0', + '/dev/ot_module_avrdude_bootloader1'] monkeypatch.setattr(hw_modules.update, '_discover_ports', mock_discover_ports1) diff --git a/api/tests/opentrons/server/test_control_endpoints.py b/api/tests/opentrons/server/test_control_endpoints.py index e54d092d758..f9428f83295 100644 --- a/api/tests/opentrons/server/test_control_endpoints.py +++ b/api/tests/opentrons/server/test_control_endpoints.py @@ -7,7 +7,6 @@ from opentrons.legacy_api import modules as legacy_modules from opentrons.drivers.smoothie_drivers.driver_3_0 import SmoothieDriver_3_0_0 -from opentrons.hardware_control import modules from opentrons.config import pipette_config diff --git a/api/tests/opentrons/server/test_update_endpoints.py b/api/tests/opentrons/server/test_update_endpoints.py index 9f9d18abc64..4cb9b182153 100644 --- a/api/tests/opentrons/server/test_update_endpoints.py +++ b/api/tests/opentrons/server/test_update_endpoints.py @@ -8,7 +8,7 @@ from opentrons.server import init from opentrons.server.endpoints import update from opentrons.server.endpoints import serverlib_fallback -from opentrons.hardware_control import modules as hw_modules, simulator +from opentrons.hardware_control import modules as hw_modules async def test_restart( From ac44b49651d7fe53cbbe83f66dd15ebc3342f408 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 15 Nov 2019 15:50:05 -0500 Subject: [PATCH 18/25] type as string in simulator --- api/src/opentrons/hardware_control/simulator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/opentrons/hardware_control/simulator.py b/api/src/opentrons/hardware_control/simulator.py index 4a263363293..62e1d196718 100644 --- a/api/src/opentrons/hardware_control/simulator.py +++ b/api/src/opentrons/hardware_control/simulator.py @@ -193,7 +193,7 @@ def get_attached_instruments( def set_active_current(self, axis, amp): pass - async def watch_modules(self, register_modules: RegisterModules): + async def watch_modules(self, register_modules: 'RegisterModules'): new_modules = [ modules.ModuleAtPort( port=f'/dev/ot_module_sim_{mod}{str(idx)}', name=mod) From e97221154103d2579fec4e64c2ca098a728359f5 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 15 Nov 2019 15:54:58 -0500 Subject: [PATCH 19/25] no typing extensions if not type checking --- api/src/opentrons/hardware_control/__init__.py | 4 ++-- api/src/opentrons/hardware_control/types.py | 18 +++++++++++------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index 7b8c4fe3438..cee0ccb9a9f 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -1367,7 +1367,7 @@ async def register_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 " + self._log.info(f"Module {name} disconnected" f" from port {port}") for port, name in new_modules: @@ -1376,7 +1376,7 @@ async def register_modules( name, self.pause_with_message) self._attached_modules.append(new_instance) - self._log.info(f"Module {name} discovered and attached " + self._log.info(f"Module {name} discovered and attached" f" at port {port}") async def _do_tp(self, pip, mount) -> top_types.Point: diff --git a/api/src/opentrons/hardware_control/types.py b/api/src/opentrons/hardware_control/types.py index eaf8428bc75..7380e741767 100644 --- a/api/src/opentrons/hardware_control/types.py +++ b/api/src/opentrons/hardware_control/types.py @@ -1,6 +1,9 @@ import enum from typing import Tuple, List -from typing_extensions import Protocol +try: + from typing_extensions import Protocol +except ModuleNotFoundError: + Protocol = None import opentrons.types from .modules import ModuleAtPort @@ -95,9 +98,10 @@ class CriticalPoint(enum.Enum): """ -class RegisterModules(Protocol): - async def __call__( - self, - new_modules: List[ModuleAtPort] = None, - removed_modules: List[ModuleAtPort] = None - ) -> None: ... +if Protocol is not None: + class RegisterModules(Protocol): + async def __call__( + self, + new_modules: List[ModuleAtPort] = None, + removed_modules: List[ModuleAtPort] = None + ) -> None: ... From 6083721caf4fd11a2e17c94980586472e1206042 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 15 Nov 2019 15:57:41 -0500 Subject: [PATCH 20/25] block module update via http for now --- api/src/opentrons/server/http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/opentrons/server/http.py b/api/src/opentrons/server/http.py index 8c6c8b5b294..22fa2a94395 100644 --- a/api/src/opentrons/server/http.py +++ b/api/src/opentrons/server/http.py @@ -38,7 +38,7 @@ def __init__(self, app, log_file_path): '/modules/{serial}', control.execute_module_command) if config.feature_flags.use_protocol_api_v2(): self.app.router.add_post( - '/modules/{serial}/update', update.update_module_firmware) + '/modules/{serial}/update', update.cannot_update_firmware) else: self.app.router.add_post( '/modules/{serial}/update', update.cannot_update_firmware) From 75f933d5f9786f7746a60f0220cf677a674a568e Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 15 Nov 2019 17:52:24 -0500 Subject: [PATCH 21/25] skip update tests --- api/tests/opentrons/hardware_control/test_modules.py | 1 + api/tests/opentrons/server/test_update_endpoints.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/api/tests/opentrons/hardware_control/test_modules.py b/api/tests/opentrons/hardware_control/test_modules.py index 9eab297e90e..e3077a46312 100644 --- a/api/tests/opentrons/hardware_control/test_modules.py +++ b/api/tests/opentrons/hardware_control/test_modules.py @@ -51,6 +51,7 @@ async def test_module_caching(): assert two_magdecks[1] is not two_magdecks[0] +@pytest.mark.skip('update module endpoint is unused for now') @pytest.mark.skipif(aionotify is None, reason="requires inotify (linux only)") @pytest.mark.skipif(not hardware_control.Controller, diff --git a/api/tests/opentrons/server/test_update_endpoints.py b/api/tests/opentrons/server/test_update_endpoints.py index 4cb9b182153..363544929f3 100644 --- a/api/tests/opentrons/server/test_update_endpoints.py +++ b/api/tests/opentrons/server/test_update_endpoints.py @@ -101,6 +101,7 @@ async def test_ignore_updates( assert json.loads(r3body) == {'version': '3.1.3'} +@pytest.mark.skip('update module endpoint is unused for now') @pytest.mark.api2_only async def test_update_module_firmware( virtual_smoothie_env, @@ -151,6 +152,7 @@ async def mock_successful_upload_to_module( assert res == expected_res +@pytest.mark.skip('update module endpoint is unused for now') @pytest.mark.api2_only async def test_fail_update_module_firmware( virtual_smoothie_env, From d6025ba342b9bf2863ebb5b126f08811680c8712 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Mon, 18 Nov 2019 14:38:42 -0500 Subject: [PATCH 22/25] do not type check fallback module --- api/src/opentrons/hardware_control/types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/opentrons/hardware_control/types.py b/api/src/opentrons/hardware_control/types.py index 7380e741767..d31ab1890e7 100644 --- a/api/src/opentrons/hardware_control/types.py +++ b/api/src/opentrons/hardware_control/types.py @@ -3,7 +3,7 @@ try: from typing_extensions import Protocol except ModuleNotFoundError: - Protocol = None + Protocol = None # type: ignore import opentrons.types from .modules import ModuleAtPort From 82dac3fd02b8c7f01e137cb4c76973558a8dfb5a Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Mon, 18 Nov 2019 15:11:45 -0500 Subject: [PATCH 23/25] remove pipfile.lock --- Pipfile.lock | 30 ------------------------------ 1 file changed, 30 deletions(-) delete mode 100644 Pipfile.lock diff --git a/Pipfile.lock b/Pipfile.lock deleted file mode 100644 index fa6d953fcfd..00000000000 --- a/Pipfile.lock +++ /dev/null @@ -1,30 +0,0 @@ -{ - "_meta": { - "hash": { - "sha256": "ff390d9ac859b3161d51457dcbab884088758e28449e9e9ea9c325b19aa289ee" - }, - "pipfile-spec": 6, - "requires": { - "python_version": "3.6" - }, - "sources": [ - { - "name": "pypi", - "url": "https://pypi.org/simple", - "verify_ssl": true - } - ] - }, - "default": {}, - "develop": { - "typing-extensions": { - "hashes": [ - "sha256:091ecc894d5e908ac75209f10d5b4f118fbdb2eb1ede6a63544054bb1edb41f2", - "sha256:910f4656f54de5993ad9304959ce9bb903f90aadc7c67a0bef07e678014e892d", - "sha256:cf8b63fedea4d89bab840ecbb93e75578af28f76f66c35889bd7065f5af88575" - ], - "index": "pypi", - "version": "==3.7.4.1" - } - } -} From 171890836beaf0e2cb7d2018e22ff312669ea66f Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Mon, 18 Nov 2019 17:09:36 -0500 Subject: [PATCH 24/25] hardware controller tweaks from PR comments --- .../opentrons/hardware_control/__init__.py | 2 -- .../opentrons/hardware_control/controller.py | 32 +++++++++++-------- .../opentrons/hardware_control/dev_types.py | 12 +++++++ .../opentrons/hardware_control/simulator.py | 2 +- api/src/opentrons/hardware_control/types.py | 13 -------- 5 files changed, 32 insertions(+), 29 deletions(-) create mode 100644 api/src/opentrons/hardware_control/dev_types.py diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index cee0ccb9a9f..be052dc767a 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -25,8 +25,6 @@ from .controller import Controller from . import modules from .types import Axis, HardwareAPILike, CriticalPoint -if TYPE_CHECKING: - from .types import RegisterModules # noqa (F501) mod_log = logging.getLogger(__name__) diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index 29590f54a1a..0212eac8282 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -3,7 +3,7 @@ import logging from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING try: - import aionotify # type: ignore + import aionotify except OSError: aionotify = None # type: ignore @@ -15,7 +15,7 @@ from . import modules if TYPE_CHECKING: - from .types import RegisterModules # noqa (F501) + from .dev_types import RegisterModules # noqa (F501) MODULE_LOG = logging.getLogger(__name__) @@ -51,7 +51,7 @@ def __init__(self, config): path='/dev', flags=(aionotify.Flags.CREATE | aionotify.Flags.DELETE)) except AttributeError: - MODULE_LOG.info( + MODULE_LOG.warning( 'Failed to initiate aionotify, cannot watch modules,' 'likely because not running on linux') @@ -137,17 +137,23 @@ async def watch_modules(self, loop: asyncio.AbstractEventLoop, while can_watch and (not self._module_watcher.closed): event = await self._module_watcher.get_event() flags = aionotify.Flags.parse(event.flags) - if 'ot_module' in event.name: + if event is not None and 'ot_module' in event.name: maybe_module_at_port = modules.get_module_at_port(event.name) - if maybe_module_at_port is not None and \ - aionotify.Flags.DELETE in flags: + new_modules = None + removed_modules = None + if maybe_module_at_port is not None: + if aionotify.Flags.DELETE in flags: + removed_modules = [maybe_module_at_port] + MODULE_LOG.info( + f'Module Removed: {maybe_module_at_port}') + elif aionotify.Flags.CREATE in flags: + new_modules = [maybe_module_at_port] + MODULE_LOG.info( + f'Module Added: {maybe_module_at_port}') await register_modules( - removed_modules=[maybe_module_at_port]) - MODULE_LOG.info(f'Module Removed: {maybe_module_at_port}') - if maybe_module_at_port is not None and \ - aionotify.Flags.CREATE in flags: - await register_modules(new_modules=[maybe_module_at_port]) - MODULE_LOG.info(f'Module Added: {maybe_module_at_port}') + removed_modules=removed_modules, + new_modules=new_modules, + ) async def build_module(self, port: str, @@ -229,5 +235,5 @@ async def delay(self, duration_s: int): await asyncio.sleep(duration_s) self.resume() - def __exit__(self, exc_type, exc_value, traceback): + def __del__(self, exc_type, exc_value, traceback): self._module_watcher.close() diff --git a/api/src/opentrons/hardware_control/dev_types.py b/api/src/opentrons/hardware_control/dev_types.py new file mode 100644 index 00000000000..c48a95ed8d7 --- /dev/null +++ b/api/src/opentrons/hardware_control/dev_types.py @@ -0,0 +1,12 @@ +try: + from typing_extensions import Protocol +except ModuleNotFoundError: + Protocol = None # type: ignore + +if Protocol is not None: + class RegisterModules(Protocol): + async def __call__( + self, + new_modules: List[ModuleAtPort] = None, + removed_modules: List[ModuleAtPort] = None + ) -> None: ... diff --git a/api/src/opentrons/hardware_control/simulator.py b/api/src/opentrons/hardware_control/simulator.py index 62e1d196718..d1b7ee466d1 100644 --- a/api/src/opentrons/hardware_control/simulator.py +++ b/api/src/opentrons/hardware_control/simulator.py @@ -10,7 +10,7 @@ from opentrons.drivers.smoothie_drivers import SimulatingDriver from . import modules if TYPE_CHECKING: - from .types import RegisterModules # noqa (F501) + from .dev_types import RegisterModules # noqa (F501) MODULE_LOG = logging.getLogger(__name__) diff --git a/api/src/opentrons/hardware_control/types.py b/api/src/opentrons/hardware_control/types.py index d31ab1890e7..a9fcc4c04d4 100644 --- a/api/src/opentrons/hardware_control/types.py +++ b/api/src/opentrons/hardware_control/types.py @@ -1,9 +1,5 @@ import enum from typing import Tuple, List -try: - from typing_extensions import Protocol -except ModuleNotFoundError: - Protocol = None # type: ignore import opentrons.types from .modules import ModuleAtPort @@ -96,12 +92,3 @@ class CriticalPoint(enum.Enum): The end of the front-most nozzle of a multipipette with a tip attached. Only relevant when a multichannel pipette is present. """ - - -if Protocol is not None: - class RegisterModules(Protocol): - async def __call__( - self, - new_modules: List[ModuleAtPort] = None, - removed_modules: List[ModuleAtPort] = None - ) -> None: ... From 80b582f8ea9a236f846c35c9d2667bacf93a37ec Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Tue, 19 Nov 2019 10:47:28 -0500 Subject: [PATCH 25/25] dev types doc string and lint --- api/src/opentrons/hardware_control/__init__.py | 2 +- api/src/opentrons/hardware_control/controller.py | 4 ++-- api/src/opentrons/hardware_control/dev_types.py | 11 +++++++++++ api/src/opentrons/hardware_control/types.py | 3 +-- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index be052dc767a..1cb76543206 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -16,7 +16,7 @@ import functools import inspect import logging -from typing import Dict, Union, List, Optional, TYPE_CHECKING +from typing import Dict, Union, List, Optional from opentrons import types as top_types from opentrons.util import linal from .simulator import Simulator diff --git a/api/src/opentrons/hardware_control/controller.py b/api/src/opentrons/hardware_control/controller.py index 0212eac8282..3ec3326d859 100644 --- a/api/src/opentrons/hardware_control/controller.py +++ b/api/src/opentrons/hardware_control/controller.py @@ -3,7 +3,7 @@ import logging from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING try: - import aionotify + import aionotify # type: ignore except OSError: aionotify = None # type: ignore @@ -235,5 +235,5 @@ async def delay(self, duration_s: int): await asyncio.sleep(duration_s) self.resume() - def __del__(self, exc_type, exc_value, traceback): + def __del__(self): self._module_watcher.close() diff --git a/api/src/opentrons/hardware_control/dev_types.py b/api/src/opentrons/hardware_control/dev_types.py index c48a95ed8d7..9f46bb89f42 100644 --- a/api/src/opentrons/hardware_control/dev_types.py +++ b/api/src/opentrons/hardware_control/dev_types.py @@ -1,8 +1,19 @@ +from typing import List +from .modules import ModuleAtPort try: from typing_extensions import Protocol except ModuleNotFoundError: Protocol = None # type: ignore +# this file defines types that require dev dependencies +# and are only relevant for static typechecking. +# +# - code should be written so that this file can fail to import +# - or the things defined in here can be None at execution time +# - only types that match the above criteria should be put here +# - please include this file as close to a leaf as possible + + if Protocol is not None: class RegisterModules(Protocol): async def __call__( diff --git a/api/src/opentrons/hardware_control/types.py b/api/src/opentrons/hardware_control/types.py index a9fcc4c04d4..3f2f12a3466 100644 --- a/api/src/opentrons/hardware_control/types.py +++ b/api/src/opentrons/hardware_control/types.py @@ -1,8 +1,7 @@ import enum -from typing import Tuple, List +from typing import Tuple import opentrons.types -from .modules import ModuleAtPort class Axis(enum.Enum):