From 0f8739cfc966678de2c96be62ef440b510ae8346 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Mon, 6 May 2024 16:37:52 -0400 Subject: [PATCH 1/5] chore(api): allow specifying module models to sim The simulator setup files let you specify modules, but you can only do it by specifying their types (i.e. magdeck, tempdeck) and not their modules (i.e. magneticModuleV1, temperatureModuleV2). This hasn't really mattered up til now because nothing cares, but RSQ-6 will have both desktop and ODD looking pretty hard at which generation of module is present on a flex, so we need a way to specify. This adds a model key to the module section of the simulator setup (and sets the test-flex.json and test.json up with it) so that you can specify module models. Closes RSQ-6 --- api/src/opentrons/hardware_control/api.py | 4 +++- .../hardware_control/backends/ot3simulator.py | 9 +++++---- .../hardware_control/backends/simulator.py | 14 ++++++++------ .../hardware_control/module_control.py | 11 ++++++++--- .../hardware_control/modules/types.py | 2 ++ api/src/opentrons/hardware_control/ot3api.py | 2 +- .../hardware_control/simulator_setup.py | 11 +++++++---- .../hardware_control/test_module_control.py | 7 ++++++- .../opentrons/hardware_control/test_modules.py | 18 +++++++++--------- .../hardware_control/test_simulator_setup.py | 13 +++++++++++++ .../hardware_control/test_thread_manager.py | 7 +++++-- .../opentrons/protocol_api_old/test_context.py | 5 ++++- robot-server/simulators/test-flex.json | 17 ++++------------- robot-server/simulators/test.json | 7 ++++++- 14 files changed, 81 insertions(+), 46 deletions(-) diff --git a/api/src/opentrons/hardware_control/api.py b/api/src/opentrons/hardware_control/api.py index 718d0d8796a..9edb28bddae 100644 --- a/api/src/opentrons/hardware_control/api.py +++ b/api/src/opentrons/hardware_control/api.py @@ -78,6 +78,8 @@ mod_log = logging.getLogger(__name__) +AttachedModuleSpec = Dict[str, List[Union[str, Tuple[str, str]]]] + class API( ExecutionManagerProvider, @@ -255,7 +257,7 @@ async def build_hardware_simulator( attached_instruments: Optional[ Dict[top_types.Mount, Dict[str, Optional[str]]] ] = None, - attached_modules: Optional[Dict[str, List[str]]] = None, + attached_modules: Optional[Dict[str, List[Tuple[str, Optional[str]]]]] = None, config: Optional[Union[RobotConfig, OT3Config]] = None, loop: Optional[asyncio.AbstractEventLoop] = None, strict_attached_instruments: bool = True, diff --git a/api/src/opentrons/hardware_control/backends/ot3simulator.py b/api/src/opentrons/hardware_control/backends/ot3simulator.py index e0c8fe1bc89..ff183d057ec 100644 --- a/api/src/opentrons/hardware_control/backends/ot3simulator.py +++ b/api/src/opentrons/hardware_control/backends/ot3simulator.py @@ -104,7 +104,7 @@ class OT3Simulator(FlexBackend): async def build( cls, attached_instruments: Dict[OT3Mount, Dict[str, Optional[str]]], - attached_modules: Dict[str, List[str]], + attached_modules: Dict[str, List[Tuple[str, Optional[str]]]], config: OT3Config, loop: asyncio.AbstractEventLoop, strict_attached_instruments: bool = True, @@ -130,7 +130,7 @@ async def build( def __init__( self, attached_instruments: Dict[OT3Mount, Dict[str, Optional[str]]], - attached_modules: Dict[str, List[str]], + attached_modules: Dict[str, List[Tuple[str, Optional[str]]]], config: OT3Config, loop: asyncio.AbstractEventLoop, strict_attached_instruments: bool = True, @@ -605,13 +605,14 @@ async def increase_z_l_hold_current(self) -> AsyncIterator[None]: @ensure_yield async def watch(self, loop: asyncio.AbstractEventLoop) -> None: new_mods_at_ports = [] - for mod, serials in self._stubbed_attached_modules.items(): - for serial in serials: + for mod, detail_tuple in self._stubbed_attached_modules.items(): + for (serial, maybe_model) in detail_tuple: new_mods_at_ports.append( modules.SimulatingModuleAtPort( port=f"/dev/ot_module_sim_{mod}{str(serial)}", name=mod, serial_number=serial, + model=maybe_model, ) ) await self.module_controls.register_modules(new_mods_at_ports=new_mods_at_ports) diff --git a/api/src/opentrons/hardware_control/backends/simulator.py b/api/src/opentrons/hardware_control/backends/simulator.py index 4066afa4bb5..a2ff17ad388 100644 --- a/api/src/opentrons/hardware_control/backends/simulator.py +++ b/api/src/opentrons/hardware_control/backends/simulator.py @@ -49,7 +49,7 @@ class Simulator: async def build( cls, attached_instruments: Dict[types.Mount, Dict[str, Optional[str]]], - attached_modules: Dict[str, List[str]], + attached_modules: Dict[str, List[Tuple[str, Optional[str]]]], config: RobotConfig, loop: asyncio.AbstractEventLoop, strict_attached_instruments: bool = True, @@ -71,8 +71,9 @@ async def build( This dict should map mounts to either empty dicts or to dicts containing 'model' and 'id' keys. - :param attached_modules: A list of module model names (e.g. - `'tempdeck'` or `'magdeck'`) representing + :param attached_modules: A map of module type names (e.g. + `'tempdeck'` or `'magdeck'`) to lists of (serial, model) + tuples representing modules the simulator should assume are attached. Like `attached_instruments`, used to make the simulator match the setup of the @@ -105,7 +106,7 @@ async def build( def __init__( self, attached_instruments: Dict[types.Mount, Dict[str, Optional[str]]], - attached_modules: Dict[str, List[str]], + attached_modules: Dict[str, List[Tuple[str, Optional[str]]]], config: RobotConfig, loop: asyncio.AbstractEventLoop, gpio_chardev: GPIODriverLike, @@ -333,13 +334,14 @@ def set_active_current(self, axis_currents: Dict[Axis, float]) -> None: @ensure_yield async def watch(self) -> None: new_mods_at_ports = [] - for mod, serials in self._stubbed_attached_modules.items(): - for serial in serials: + for mod, detail_pairs in self._stubbed_attached_modules.items(): + for (serial, maybe_model) in detail_pairs: new_mods_at_ports.append( modules.SimulatingModuleAtPort( port=f"/dev/ot_module_sim_{mod}{str(serial)}", name=mod, serial_number=serial, + model=maybe_model, ) ) await self.module_controls.register_modules(new_mods_at_ports=new_mods_at_ports) diff --git a/api/src/opentrons/hardware_control/module_control.py b/api/src/opentrons/hardware_control/module_control.py index 1d32731d026..780c7bc61e8 100644 --- a/api/src/opentrons/hardware_control/module_control.py +++ b/api/src/opentrons/hardware_control/module_control.py @@ -161,9 +161,14 @@ async def register_modules( port=mod.port, usb_port=mod.usb_port, type=modules.MODULE_TYPE_BY_NAME[mod.name], - sim_serial_number=mod.serial_number - if isinstance(mod, SimulatingModuleAtPort) - else None, + sim_serial_number=( + mod.serial_number + if isinstance(mod, SimulatingModuleAtPort) + else None + ), + sim_model=( + mod.model if isinstance(mod, SimulatingModuleAtPort) else None + ), ) self._available_modules.append(new_instance) log.info( diff --git a/api/src/opentrons/hardware_control/modules/types.py b/api/src/opentrons/hardware_control/modules/types.py index eb8054a87ee..ecaa1e99cc4 100644 --- a/api/src/opentrons/hardware_control/modules/types.py +++ b/api/src/opentrons/hardware_control/modules/types.py @@ -9,6 +9,7 @@ Tuple, Awaitable, Union, + Optional, cast, TYPE_CHECKING, ) @@ -129,6 +130,7 @@ class ModuleAtPort: @dataclass(kw_only=True) class SimulatingModuleAtPort(ModuleAtPort): serial_number: str + model: Optional[str] class BundledFirmware(NamedTuple): diff --git a/api/src/opentrons/hardware_control/ot3api.py b/api/src/opentrons/hardware_control/ot3api.py index 21c3f70dab7..8345a1bfba8 100644 --- a/api/src/opentrons/hardware_control/ot3api.py +++ b/api/src/opentrons/hardware_control/ot3api.py @@ -413,7 +413,7 @@ async def build_hardware_simulator( Dict[OT3Mount, Dict[str, Optional[str]]], Dict[top_types.Mount, Dict[str, Optional[str]]], ] = None, - attached_modules: Optional[Dict[str, List[str]]] = None, + attached_modules: Optional[Dict[str, List[Tuple[str, Optional[str]]]]] = None, config: Union[RobotConfig, OT3Config, None] = None, loop: Optional[asyncio.AbstractEventLoop] = None, strict_attached_instruments: bool = True, diff --git a/api/src/opentrons/hardware_control/simulator_setup.py b/api/src/opentrons/hardware_control/simulator_setup.py index 25fa17d36a1..3fa2df4656c 100644 --- a/api/src/opentrons/hardware_control/simulator_setup.py +++ b/api/src/opentrons/hardware_control/simulator_setup.py @@ -24,6 +24,7 @@ class ModuleCall: @dataclass(frozen=True) class ModuleItem: serial_number: str + model: str calls: List[ModuleCall] = field(default_factory=list) @@ -59,7 +60,7 @@ async def _simulator_for_setup( return await API.build_hardware_simulator( attached_instruments=setup.attached_instruments, attached_modules={ - k: [m.serial_number for m in v] + k: [(m.serial_number, m.model) for m in v] for k, v in setup.attached_modules.items() }, config=setup.config, @@ -73,7 +74,7 @@ async def _simulator_for_setup( return await OT3API.build_hardware_simulator( attached_instruments=setup.attached_instruments, attached_modules={ - k: [m.serial_number for m in v] + k: [(m.serial_number, m.model) for m in v] for k, v in setup.attached_modules.items() }, config=setup.config, @@ -114,7 +115,7 @@ def _thread_manager_for_setup( API.build_hardware_simulator, attached_instruments=setup.attached_instruments, attached_modules={ - k: [m.serial_number for m in v] + k: [(m.serial_number, m.model) for m in v] for k, v in setup.attached_modules.items() }, config=setup.config, @@ -128,7 +129,7 @@ def _thread_manager_for_setup( OT3API.build_hardware_simulator, attached_instruments=setup.attached_instruments, attached_modules={ - k: [m.serial_number for m in v] + k: [(m.serial_number, m.model) for m in v] for k, v in setup.attached_modules.items() }, config=setup.config, @@ -215,6 +216,7 @@ def _prepare_for_simulator_setup(key: str, value: Dict[str, Any]) -> Any: attached_modules.setdefault(key, []).append( ModuleItem( serial_number=obj["serial_number"], + model=obj["model"], calls=[ModuleCall(**data) for data in obj["calls"]], ) ) @@ -236,6 +238,7 @@ def _prepare_for_ot3_simulator_setup(key: str, value: Dict[str, Any]) -> Any: attached_modules.setdefault(key, []).append( ModuleItem( serial_number=obj["serial_number"], + model=obj["model"], calls=[ModuleCall(**data) for data in obj["calls"]], ) ) diff --git a/api/tests/opentrons/hardware_control/test_module_control.py b/api/tests/opentrons/hardware_control/test_module_control.py index 36fd6cb1793..e37aa3c45d7 100644 --- a/api/tests/opentrons/hardware_control/test_module_control.py +++ b/api/tests/opentrons/hardware_control/test_module_control.py @@ -66,7 +66,10 @@ def subject( ( [ SimulatingModuleAtPort( - port="/dev/foo", name="bar", serial_number="test-123" + port="/dev/foo", + name="bar", + serial_number="test-123", + model="mymodel", ) ] ), @@ -101,6 +104,7 @@ async def test_register_modules( usb_port=USBPort(name="baz", port_number=0), type=ModuleType.TEMPERATURE, sim_serial_number=None, + sim_model=None, ) ).then_return(module) @@ -151,6 +155,7 @@ async def test_register_modules_sort( port=matchers.Anything(), type=matchers.Anything(), sim_serial_number=None, + sim_model=None, ) ).then_return(mod) diff --git a/api/tests/opentrons/hardware_control/test_modules.py b/api/tests/opentrons/hardware_control/test_modules.py index eb3d0e48c6c..f246d3fa077 100644 --- a/api/tests/opentrons/hardware_control/test_modules.py +++ b/api/tests/opentrons/hardware_control/test_modules.py @@ -31,10 +31,10 @@ async def test_get_modules_simulating(): import opentrons.hardware_control as hardware_control mods = { - "tempdeck": ["111"], - "magdeck": ["222"], - "thermocycler": ["333"], - "heatershaker": ["444"], + "tempdeck": [("111", "temperatureModuleV1")], + "magdeck": [("222", "magneticModuleV2")], + "thermocycler": [("333", "thermocyclerModuleV1")], + "heatershaker": [("444", "heaterShakerModuleV1")], } api = await hardware_control.API.build_hardware_simulator(attached_modules=mods) await asyncio.sleep(0.05) @@ -47,7 +47,7 @@ async def test_get_modules_simulating(): async def test_module_caching(): import opentrons.hardware_control as hardware_control - mod_names = {"tempdeck": ["111"]} + mod_names = {"tempdeck": [("111", "temperatureModuleV1")]} api = await hardware_control.API.build_hardware_simulator( attached_modules=mod_names ) @@ -349,10 +349,10 @@ async def test_get_bundled_fw(monkeypatch, tmpdir): from opentrons.hardware_control import API mods = { - "tempdeck": ["111"], - "magdeck": ["222"], - "thermocycler": ["333"], - "heatershaker": ["444"], + "tempdeck": [("111", "temperatureModuleV1")], + "magdeck": [("222", "magneticModuleV2")], + "thermocycler": [("333", "thermocyclerModuleV1")], + "heatershaker": [("444", "heaterShakerModuleV1")], } api = await API.build_hardware_simulator(attached_modules=mods) diff --git a/api/tests/opentrons/hardware_control/test_simulator_setup.py b/api/tests/opentrons/hardware_control/test_simulator_setup.py index 2507a9969b3..63dca593bff 100644 --- a/api/tests/opentrons/hardware_control/test_simulator_setup.py +++ b/api/tests/opentrons/hardware_control/test_simulator_setup.py @@ -58,10 +58,12 @@ async def test_with_magdeck(setup_klass: Type[simulator_setup.SimulatorSetup]) - attached_modules={ "magdeck": [ simulator_setup.ModuleItem( + model="magneticModuleV1", serial_number="123", calls=[simulator_setup.ModuleCall("engage", kwargs={"height": 3})], ), simulator_setup.ModuleItem( + model="magneticModuleV2", serial_number="1234", calls=[simulator_setup.ModuleCall("engage", kwargs={"height": 5})], ), @@ -71,11 +73,13 @@ async def test_with_magdeck(setup_klass: Type[simulator_setup.SimulatorSetup]) - simulator = await simulator_setup.create_simulator(setup) assert isinstance(simulator.attached_modules[0], MagDeck) + assert simulator.attached_modules[0].model() == "magneticModuleV1" assert simulator.attached_modules[0].live_data == { "data": {"engaged": True, "height": 3}, "status": "engaged", } assert simulator.attached_modules[0].device_info["serial"] == "123" + assert simulator.attached_modules[1].model() == "magneticModuleV2" assert simulator.attached_modules[1].live_data == { "data": {"engaged": True, "height": 5}, "status": "engaged", @@ -91,6 +95,7 @@ async def test_with_thermocycler( attached_modules={ "thermocycler": [ simulator_setup.ModuleItem( + model="thermocyclerModuleV2", serial_number="123", calls=[ simulator_setup.ModuleCall( @@ -110,6 +115,7 @@ async def test_with_thermocycler( simulator = await simulator_setup.create_simulator(setup) assert isinstance(simulator.attached_modules[0], Thermocycler) + assert simulator.attached_modules[0].model() == "thermocyclerModuleV2" assert simulator.attached_modules[0].live_data == { "data": { "currentCycleIndex": None, @@ -136,6 +142,7 @@ async def test_with_tempdeck(setup_klass: Type[simulator_setup.SimulatorSetup]) attached_modules={ "tempdeck": [ simulator_setup.ModuleItem( + model="temperatureModuleV2", serial_number="123", calls=[ simulator_setup.ModuleCall( @@ -152,6 +159,7 @@ async def test_with_tempdeck(setup_klass: Type[simulator_setup.SimulatorSetup]) simulator = await simulator_setup.create_simulator(setup) assert isinstance(simulator.attached_modules[0], TempDeck) + assert simulator.attached_modules[0].model() == "temperatureModuleV2" assert simulator.attached_modules[0].live_data == { "data": {"currentTemp": 23, "targetTemp": 23}, "status": "holding at target", @@ -168,12 +176,14 @@ def test_persistence_ot2(tmpdir: str) -> None: attached_modules={ "magdeck": [ simulator_setup.ModuleItem( + model="magneticModuleV1", serial_number="111", calls=[simulator_setup.ModuleCall("engage", kwargs={"height": 3})], ) ], "tempdeck": [ simulator_setup.ModuleItem( + model="temperatureModuleV2", serial_number="111", calls=[ simulator_setup.ModuleCall( @@ -205,6 +215,7 @@ def test_persistence_ot3(tmpdir: str) -> None: attached_modules={ "magdeck": [ simulator_setup.ModuleItem( + model="magneticModuleV2", serial_number="mag-1", calls=[ simulator_setup.ModuleCall( @@ -216,6 +227,7 @@ def test_persistence_ot3(tmpdir: str) -> None: ], "tempdeck": [ simulator_setup.ModuleItem( + model="temperatureModuleV2", serial_number="temp-1", calls=[ simulator_setup.ModuleCall( @@ -229,6 +241,7 @@ def test_persistence_ot3(tmpdir: str) -> None: ], ), simulator_setup.ModuleItem( + model="temperatureModuleV1", serial_number="temp-2", calls=[ simulator_setup.ModuleCall( diff --git a/api/tests/opentrons/hardware_control/test_thread_manager.py b/api/tests/opentrons/hardware_control/test_thread_manager.py index 193740b4d75..9b48dac97b1 100644 --- a/api/tests/opentrons/hardware_control/test_thread_manager.py +++ b/api/tests/opentrons/hardware_control/test_thread_manager.py @@ -28,7 +28,7 @@ def test_build_fail_raises_exception(): def test_module_cache_add_entry(): """Test that _cached_modules updates correctly.""" - mod_names = {"tempdeck": ["111"]} + mod_names = {"tempdeck": [("111", "temperatureModuleV2")]} thread_manager = ThreadManager( API.build_hardware_simulator, attached_modules=mod_names ) @@ -49,7 +49,10 @@ def test_module_cache_add_entry(): async def test_module_cache_remove_entry(): """Test that module entry gets removed from cache when module detaches.""" - mod_names = {"tempdeck": ["111"], "magdeck": ["222"]} + mod_names = { + "tempdeck": [("111", "temperatureModuleV2")], + "magdeck": [("222", "magneticModuleV1")], + } thread_manager = ThreadManager( API.build_hardware_simulator, attached_modules=mod_names ) diff --git a/api/tests/opentrons/protocol_api_old/test_context.py b/api/tests/opentrons/protocol_api_old/test_context.py index c356c477f7f..9b93c1c4a9d 100644 --- a/api/tests/opentrons/protocol_api_old/test_context.py +++ b/api/tests/opentrons/protocol_api_old/test_context.py @@ -959,7 +959,10 @@ def test_order_of_module_load(): import opentrons.hardware_control as hardware_control import opentrons.protocol_api as protocol_api - mods = {"tempdeck": ["111", "333"], "thermocycler": ["222"]} + mods = { + "tempdeck": [("111", "temperatureModuleV1"), ("333", "temperatureModuleV2")], + "thermocycler": [("222", "thermocyclerModuleV2")], + } thread_manager = hardware_control.ThreadManager( hardware_control.API.build_hardware_simulator, attached_modules=mods ) diff --git a/robot-server/simulators/test-flex.json b/robot-server/simulators/test-flex.json index adc9543fc5a..ed694a8cb92 100644 --- a/robot-server/simulators/test-flex.json +++ b/robot-server/simulators/test-flex.json @@ -14,6 +14,7 @@ "attached_modules": { "thermocycler": [ { + "model": "thermocyclerModuleV2", "serial_number": "therm-123", "calls": [ { @@ -37,12 +38,14 @@ ], "heatershaker": [ { + "model": "heaterShakerModuleV1", "serial_number": "hs-123", "calls": [] } ], "tempdeck": [ { + "model": "temperatureModuleV2", "serial_number": "temp-123", "calls": [ { @@ -60,6 +63,7 @@ ] }, { + "model": "temperatureModuleV2", "serial_number": "temp-1234", "calls": [ { @@ -76,19 +80,6 @@ } ] } - ], - "magdeck": [ - { - "serial_number": "mag-123", - "calls": [ - { - "function_name": "engage", - "kwargs": { - "height": 4 - } - } - ] - } ] } } diff --git a/robot-server/simulators/test.json b/robot-server/simulators/test.json index c7ca49e9040..9789bb9e8b0 100644 --- a/robot-server/simulators/test.json +++ b/robot-server/simulators/test.json @@ -12,6 +12,7 @@ "attached_modules": { "thermocycler": [ { + "model": "thermocyclerModuleV1", "serial_number": "therm-123", "calls": [ { @@ -35,12 +36,14 @@ ], "heatershaker": [ { + "model": "heaterShakerModuleV1", "serial_number": "hs-123", "calls": [] } ], "tempdeck": [ { + "model": "temperatureModuleV1", "serial_number": "temp-123", "calls": [ { @@ -60,6 +63,7 @@ ], "magdeck": [ { + "model": "magneticModuleV2", "serial_number": "mag-123", "calls": [ { @@ -71,6 +75,7 @@ ] }, { + "model": "magneticModuleV1", "serial_number": "mag-1234", "calls": [ { @@ -83,4 +88,4 @@ } ] } -} \ No newline at end of file +} From c33379ae412a739007afbd9a7c04f16cfa1fc7ea Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Tue, 7 May 2024 09:50:01 -0400 Subject: [PATCH 2/5] fix up tests --- .../integration/test_modules.tavern.yaml | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/robot-server/tests/integration/test_modules.tavern.yaml b/robot-server/tests/integration/test_modules.tavern.yaml index 48220193df7..815b736acf7 100644 --- a/robot-server/tests/integration/test_modules.tavern.yaml +++ b/robot-server/tests/integration/test_modules.tavern.yaml @@ -19,7 +19,7 @@ stages: moduleModel: thermocyclerModuleV1 port: !anystr usbPort: !anydict - serial: !anystr + serial: therm-123 model: !anystr revision: !anystr fwVersion: !anystr @@ -42,7 +42,7 @@ stages: moduleModel: heaterShakerModuleV1 port: !anystr usbPort: !anydict - serial: !anystr + serial: hs-123 model: !anystr revision: !anystr fwVersion: !anystr @@ -62,7 +62,7 @@ stages: moduleModel: temperatureModuleV1 port: !anystr usbPort: !anydict - serial: !anystr + serial: temp-123 model: !anystr revision: !anystr fwVersion: !anystr @@ -73,10 +73,10 @@ stages: targetTemp: !anyfloat - name: magdeck displayName: magdeck - moduleModel: magneticModuleV1 + moduleModel: magneticModuleV2 port: !anystr usbPort: !anydict - serial: !anystr + serial: mag-123 model: !anystr revision: !anystr fwVersion: !anystr @@ -90,7 +90,7 @@ stages: moduleModel: magneticModuleV1 port: !anystr usbPort: !anydict - serial: !anystr + serial: mag-1234 model: !anystr revision: !anystr fwVersion: !anystr @@ -109,7 +109,7 @@ stages: meta: !anydict data: - id: !anystr - serialNumber: !anystr + serialNumber: therm-123 firmwareVersion: !anystr hardwareRevision: !anystr hasAvailableUpdate: !anybool @@ -130,7 +130,7 @@ stages: targetTemperature: !anyfloat holdTime: !anyfloat - id: !anystr - serialNumber: !anystr + serialNumber: hs-123 firmwareVersion: !anystr hardwareRevision: !anystr hasAvailableUpdate: !anybool @@ -149,7 +149,7 @@ stages: currentSpeed: !anyint currentTemperature: !anyfloat - id: !anystr - serialNumber: !anystr + serialNumber: temp-123 firmwareVersion: !anystr hardwareRevision: !anystr hasAvailableUpdate: !anybool @@ -165,12 +165,12 @@ stages: currentTemperature: !anyfloat targetTemperature: !anyfloat - id: !anystr - serialNumber: !anystr + serialNumber: mag-123 firmwareVersion: !anystr hardwareRevision: !anystr hasAvailableUpdate: !anybool moduleType: magneticModuleType - moduleModel: magneticModuleV1 + moduleModel: magneticModuleV2 usbPort: port: !anyint hub: !anybool @@ -181,7 +181,7 @@ stages: height: !anyfloat engaged: !anybool - id: !anystr - serialNumber: !anystr + serialNumber: mag-1234 firmwareVersion: !anystr hardwareRevision: !anystr hasAvailableUpdate: !anybool From f1330caea5dc9577682ca4bfd744f71b7ab7dd6e Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Tue, 7 May 2024 16:17:58 -0400 Subject: [PATCH 3/5] fix(api): Improve attached_modules spec This was getting a bit unwieldy; prior commits had made it a dict of lists of strings, and now this needed it to be a dict of lists of tuples of two strings, and all of this was terrible. Instead now it's a dict of lists of a specific dataclass, much nicer. While I was doing that I noticed that I didn't get any type errors, and that's because ThreadManager's constructor was erasing the types of parameters that you gave it, so I made it use a ParamSpec. Unfortunately, you can't alter wrapped function named arguments using type machinery right now, so I also had to change the way we specify nonblocking thread manager construction to use a wrapper function that sets an attr, but now that's type safe too. --- api/mypy.ini | 2 +- api/src/opentrons/__init__.py | 6 +- .../opentrons/hardware_control/__init__.py | 3 +- api/src/opentrons/hardware_control/api.py | 2 +- .../hardware_control/backends/ot3simulator.py | 16 +-- .../hardware_control/backends/simulator.py | 16 +-- .../hardware_control/modules/__init__.py | 2 + .../hardware_control/modules/types.py | 7 +- api/src/opentrons/hardware_control/ot3api.py | 2 +- .../hardware_control/scripts/repl.py | 2 - .../hardware_control/simulator_setup.py | 21 ++- .../hardware_control/thread_manager.py | 54 +++++++- .../hardware_control/test_modules.py | 129 ++++++++++-------- .../hardware_control/test_thread_manager.py | 72 ++++++++-- .../protocol_api_old/test_context.py | 10 +- robot-server/tests/subsystems/test_router.py | 6 +- 16 files changed, 240 insertions(+), 110 deletions(-) diff --git a/api/mypy.ini b/api/mypy.ini index 6cbbea90d34..5aff10a0c61 100644 --- a/api/mypy.ini +++ b/api/mypy.ini @@ -4,7 +4,7 @@ show_error_codes = True warn_unused_configs = True strict = True # TODO(mc, 2021-09-12): work through and remove these exclusions -exclude = tests/opentrons/(hardware_control/test_(?!(ot3|module_control)).*py|hardware_control/integration/|hardware_control/emulation/|hardware_control/modules/|protocols/advanced_control/|protocols/api_support/|protocols/duration/|protocols/execution/|protocols/fixtures/|protocols/geometry/) +exclude = tests/opentrons/(hardware_control/test_(?!(ot3|module_control|modules|thread_manager)).*py|hardware_control/integration/|hardware_control/emulation/|hardware_control/modules/|protocols/advanced_control/|protocols/api_support/|protocols/duration/|protocols/execution/|protocols/fixtures/|protocols/geometry/) [pydantic-mypy] init_forbid_extra = True diff --git a/api/src/opentrons/__init__.py b/api/src/opentrons/__init__.py index ac4e0c54262..086c663da0e 100755 --- a/api/src/opentrons/__init__.py +++ b/api/src/opentrons/__init__.py @@ -115,16 +115,14 @@ async def _create_thread_manager() -> ThreadManagedHardware: from opentrons.hardware_control.ot3api import OT3API thread_manager = ThreadManager( - OT3API.build_hardware_controller, + ThreadManager.nonblocking_builder(OT3API.build_hardware_controller), use_usb_bus=ff.rear_panel_integration(), - threadmanager_nonblocking=True, status_bar_enabled=ff.status_bar_enabled(), feature_flags=hw_types.HardwareFeatureFlags.build_from_ff(), ) else: thread_manager = ThreadManager( - HardwareAPI.build_hardware_controller, - threadmanager_nonblocking=True, + ThreadManager.nonblocking_builder(HardwareAPI.build_hardware_controller), port=_get_motor_control_serial_port(), firmware=_find_smoothie_file(), feature_flags=hw_types.HardwareFeatureFlags.build_from_ff(), diff --git a/api/src/opentrons/hardware_control/__init__.py b/api/src/opentrons/hardware_control/__init__.py index b49f1462249..d575a2eada5 100644 --- a/api/src/opentrons/hardware_control/__init__.py +++ b/api/src/opentrons/hardware_control/__init__.py @@ -38,7 +38,8 @@ ] HardwareControlAPI = Union[OT2HardwareControlAPI, OT3HardwareControlAPI] -ThreadManagedHardware = ThreadManager[HardwareControlAPI] +# this type ignore is because of https://github.com/python/mypy/issues/13437 +ThreadManagedHardware = ThreadManager[HardwareControlAPI] # type: ignore[misc] SyncHardwareAPI = SynchronousAdapter[HardwareControlAPI] __all__ = [ diff --git a/api/src/opentrons/hardware_control/api.py b/api/src/opentrons/hardware_control/api.py index 9edb28bddae..db21946920a 100644 --- a/api/src/opentrons/hardware_control/api.py +++ b/api/src/opentrons/hardware_control/api.py @@ -257,7 +257,7 @@ async def build_hardware_simulator( attached_instruments: Optional[ Dict[top_types.Mount, Dict[str, Optional[str]]] ] = None, - attached_modules: Optional[Dict[str, List[Tuple[str, Optional[str]]]]] = None, + attached_modules: Optional[Dict[str, List[modules.SimulatingModule]]] = None, config: Optional[Union[RobotConfig, OT3Config]] = None, loop: Optional[asyncio.AbstractEventLoop] = None, strict_attached_instruments: bool = True, diff --git a/api/src/opentrons/hardware_control/backends/ot3simulator.py b/api/src/opentrons/hardware_control/backends/ot3simulator.py index ff183d057ec..1e6d9756cf0 100644 --- a/api/src/opentrons/hardware_control/backends/ot3simulator.py +++ b/api/src/opentrons/hardware_control/backends/ot3simulator.py @@ -104,7 +104,7 @@ class OT3Simulator(FlexBackend): async def build( cls, attached_instruments: Dict[OT3Mount, Dict[str, Optional[str]]], - attached_modules: Dict[str, List[Tuple[str, Optional[str]]]], + attached_modules: Dict[str, List[modules.SimulatingModule]], config: OT3Config, loop: asyncio.AbstractEventLoop, strict_attached_instruments: bool = True, @@ -130,7 +130,7 @@ async def build( def __init__( self, attached_instruments: Dict[OT3Mount, Dict[str, Optional[str]]], - attached_modules: Dict[str, List[Tuple[str, Optional[str]]]], + attached_modules: Dict[str, List[modules.SimulatingModule]], config: OT3Config, loop: asyncio.AbstractEventLoop, strict_attached_instruments: bool = True, @@ -605,14 +605,14 @@ async def increase_z_l_hold_current(self) -> AsyncIterator[None]: @ensure_yield async def watch(self, loop: asyncio.AbstractEventLoop) -> None: new_mods_at_ports = [] - for mod, detail_tuple in self._stubbed_attached_modules.items(): - for (serial, maybe_model) in detail_tuple: + for mod_name, list_of_modules in self._stubbed_attached_modules.items(): + for module_details in list_of_modules: new_mods_at_ports.append( modules.SimulatingModuleAtPort( - port=f"/dev/ot_module_sim_{mod}{str(serial)}", - name=mod, - serial_number=serial, - model=maybe_model, + port=f"/dev/ot_module_sim_{mod_name}{str(module_details.serial_number)}", + name=mod_name, + serial_number=module_details.serial_number, + model=module_details.model, ) ) await self.module_controls.register_modules(new_mods_at_ports=new_mods_at_ports) diff --git a/api/src/opentrons/hardware_control/backends/simulator.py b/api/src/opentrons/hardware_control/backends/simulator.py index a2ff17ad388..59b3d9b9dfd 100644 --- a/api/src/opentrons/hardware_control/backends/simulator.py +++ b/api/src/opentrons/hardware_control/backends/simulator.py @@ -49,7 +49,7 @@ class Simulator: async def build( cls, attached_instruments: Dict[types.Mount, Dict[str, Optional[str]]], - attached_modules: Dict[str, List[Tuple[str, Optional[str]]]], + attached_modules: Dict[str, List[modules.SimulatingModule]], config: RobotConfig, loop: asyncio.AbstractEventLoop, strict_attached_instruments: bool = True, @@ -106,7 +106,7 @@ async def build( def __init__( self, attached_instruments: Dict[types.Mount, Dict[str, Optional[str]]], - attached_modules: Dict[str, List[Tuple[str, Optional[str]]]], + attached_modules: Dict[str, List[modules.SimulatingModule]], config: RobotConfig, loop: asyncio.AbstractEventLoop, gpio_chardev: GPIODriverLike, @@ -334,14 +334,14 @@ def set_active_current(self, axis_currents: Dict[Axis, float]) -> None: @ensure_yield async def watch(self) -> None: new_mods_at_ports = [] - for mod, detail_pairs in self._stubbed_attached_modules.items(): - for (serial, maybe_model) in detail_pairs: + for mod_name, list_of_modules in self._stubbed_attached_modules.items(): + for module_details in list_of_modules: new_mods_at_ports.append( modules.SimulatingModuleAtPort( - port=f"/dev/ot_module_sim_{mod}{str(serial)}", - name=mod, - serial_number=serial, - model=maybe_model, + port=f"/dev/ot_module_sim_{mod_name}{str(module_details.serial_number)}", + name=mod_name, + serial_number=module_details.serial_number, + model=module_details.model, ) ) await self.module_controls.register_modules(new_mods_at_ports=new_mods_at_ports) diff --git a/api/src/opentrons/hardware_control/modules/__init__.py b/api/src/opentrons/hardware_control/modules/__init__.py index dd8c531bdb1..8675bc59de9 100644 --- a/api/src/opentrons/hardware_control/modules/__init__.py +++ b/api/src/opentrons/hardware_control/modules/__init__.py @@ -12,6 +12,7 @@ UpdateError, ModuleAtPort, SimulatingModuleAtPort, + SimulatingModule, ModuleType, ModuleModel, TemperatureStatus, @@ -35,6 +36,7 @@ "UpdateError", "ModuleAtPort", "SimulatingModuleAtPort", + "SimulatingModule", "HeaterShaker", "ModuleType", "ModuleModel", diff --git a/api/src/opentrons/hardware_control/modules/types.py b/api/src/opentrons/hardware_control/modules/types.py index ecaa1e99cc4..f0980fca2b1 100644 --- a/api/src/opentrons/hardware_control/modules/types.py +++ b/api/src/opentrons/hardware_control/modules/types.py @@ -128,11 +128,16 @@ class ModuleAtPort: @dataclass(kw_only=True) -class SimulatingModuleAtPort(ModuleAtPort): +class SimulatingModule: serial_number: str model: Optional[str] +@dataclass(kw_only=True) +class SimulatingModuleAtPort(ModuleAtPort, SimulatingModule): + pass + + class BundledFirmware(NamedTuple): """Represents a versioned firmware file, generally bundled into the fs""" diff --git a/api/src/opentrons/hardware_control/ot3api.py b/api/src/opentrons/hardware_control/ot3api.py index 8345a1bfba8..5fedd5050ce 100644 --- a/api/src/opentrons/hardware_control/ot3api.py +++ b/api/src/opentrons/hardware_control/ot3api.py @@ -413,7 +413,7 @@ async def build_hardware_simulator( Dict[OT3Mount, Dict[str, Optional[str]]], Dict[top_types.Mount, Dict[str, Optional[str]]], ] = None, - attached_modules: Optional[Dict[str, List[Tuple[str, Optional[str]]]]] = None, + attached_modules: Optional[Dict[str, List[modules.SimulatingModule]]] = None, config: Union[RobotConfig, OT3Config, None] = None, loop: Optional[asyncio.AbstractEventLoop] = None, strict_attached_instruments: bool = True, diff --git a/api/src/opentrons/hardware_control/scripts/repl.py b/api/src/opentrons/hardware_control/scripts/repl.py index 1efbe0c2233..4e20a8dd9da 100644 --- a/api/src/opentrons/hardware_control/scripts/repl.py +++ b/api/src/opentrons/hardware_control/scripts/repl.py @@ -114,8 +114,6 @@ def synchronizer(*args: Any, **kwargs: Any) -> Any: def build_thread_manager() -> ThreadManager[Union[API, OT3API]]: return ThreadManager( API.build_hardware_controller, - use_usb_bus=ff.rear_panel_integration(), - update_firmware=update_firmware, feature_flags=HardwareFeatureFlags.build_from_ff(), ) diff --git a/api/src/opentrons/hardware_control/simulator_setup.py b/api/src/opentrons/hardware_control/simulator_setup.py index 3fa2df4656c..7767eff42e2 100644 --- a/api/src/opentrons/hardware_control/simulator_setup.py +++ b/api/src/opentrons/hardware_control/simulator_setup.py @@ -11,6 +11,7 @@ from opentrons.types import Mount from opentrons.hardware_control import API, HardwareControlAPI, ThreadManager from opentrons.hardware_control.types import OT3Mount, HardwareFeatureFlags +from opentrons.hardware_control.modules import SimulatingModule # Name and kwargs for a module function @@ -60,7 +61,10 @@ async def _simulator_for_setup( return await API.build_hardware_simulator( attached_instruments=setup.attached_instruments, attached_modules={ - k: [(m.serial_number, m.model) for m in v] + k: [ + SimulatingModule(serial_number=m.serial_number, model=m.model) + for m in v + ] for k, v in setup.attached_modules.items() }, config=setup.config, @@ -74,7 +78,10 @@ async def _simulator_for_setup( return await OT3API.build_hardware_simulator( attached_instruments=setup.attached_instruments, attached_modules={ - k: [(m.serial_number, m.model) for m in v] + k: [ + SimulatingModule(serial_number=m.serial_number, model=m.model) + for m in v + ] for k, v in setup.attached_modules.items() }, config=setup.config, @@ -115,7 +122,10 @@ def _thread_manager_for_setup( API.build_hardware_simulator, attached_instruments=setup.attached_instruments, attached_modules={ - k: [(m.serial_number, m.model) for m in v] + k: [ + SimulatingModule(serial_number=m.serial_number, model=m.model) + for m in v + ] for k, v in setup.attached_modules.items() }, config=setup.config, @@ -129,7 +139,10 @@ def _thread_manager_for_setup( OT3API.build_hardware_simulator, attached_instruments=setup.attached_instruments, attached_modules={ - k: [(m.serial_number, m.model) for m in v] + k: [ + SimulatingModule(serial_number=m.serial_number, model=m.model) + for m in v + ] for k, v in setup.attached_modules.items() }, config=setup.config, diff --git a/api/src/opentrons/hardware_control/thread_manager.py b/api/src/opentrons/hardware_control/thread_manager.py index c72ec3857b9..ed3669494b3 100644 --- a/api/src/opentrons/hardware_control/thread_manager.py +++ b/api/src/opentrons/hardware_control/thread_manager.py @@ -1,9 +1,9 @@ """Manager for the :py:class:`.hardware_control.API` thread.""" +import functools import threading import logging import asyncio import inspect -import functools import weakref from typing import ( Any, @@ -195,7 +195,10 @@ class ThreadManager(Generic[WrappedObj]): If you want to wait for the managed object's creation separately (with managed_thread_ready_blocking or managed_thread_ready_async) - then pass threadmanager_nonblocking=True as a kwarg + use the nonblocking_builder static method to add an attribute to the builder + function, i.e. + + thread_manager = ThreadManager(ThreadManager.nonblocking_builder(builder), ...) Example ------- @@ -206,15 +209,52 @@ class ThreadManager(Generic[WrappedObj]): >>> api_single_thread.sync.home() # call as blocking sync """ + Builder = ParamSpec("Builder") + Built = TypeVar("Built") + + @staticmethod + def nonblocking_builder( + builder: Callable[Builder, Awaitable[Built]] + ) -> Callable[Builder, Awaitable[Built]]: + """Wrap an instance of a builder function to make initializes that use it nonblocking. + + For instance, you can build a ThreadManager like this: + + thread_manager = ThreadManager(ThreadManager.nonblocking_builder(API.build_hardware_controller), ...) + + to make the initialize call return immediately so you can later wait on it via + managed_thread_ready_blocking or managed_thread_ready_async + """ + + @functools.wraps(builder) + async def wrapper( + *args: ThreadManager.Builder.args, **kwargs: ThreadManager.Builder.kwargs + ) -> ThreadManager.Built: + return await builder(*args, **kwargs) + + setattr(wrapper, "nonblocking", True) + return wrapper + def __init__( self, - builder: Callable[..., Awaitable[WrappedObj]], - *args: Any, - **kwargs: Any, + builder: Callable[Builder, Awaitable[WrappedObj]], + *args: Builder.args, + **kwargs: Builder.kwargs, ) -> None: """Build the ThreadManager. - :param builder: The API function to use + builder: The api function to use to build the instance. + + The args and kwargs will be forwarded to the builder function. + + Note: by default, this function will block until the managed thread is ready and the hardware controller + has been built. To make this function return immediately you can wrap its builder argument in + ThreadManager.nonblocking_builder(), like this: + + thread_manager = ThreadManager(ThreadManager.nonblocking_builder(API.build_hardware_controller), ...) + + Afterwards, you'll need to call ThreadManager.managed_thread_ready_blocking or its async variant before + you can actually use thei nstance. """ self._loop: Optional[asyncio.AbstractEventLoop] = None @@ -234,7 +274,7 @@ def __init__( asyncio.get_child_watcher() except NotImplementedError: pass - blocking = not kwargs.pop("threadmanager_nonblocking", False) + blocking = not getattr(builder, "nonblocking", False) target = object.__getattribute__(self, "_build_and_start_loop") thread = threading.Thread( target=target, diff --git a/api/tests/opentrons/hardware_control/test_modules.py b/api/tests/opentrons/hardware_control/test_modules.py index f246d3fa077..a23d6096873 100644 --- a/api/tests/opentrons/hardware_control/test_modules.py +++ b/api/tests/opentrons/hardware_control/test_modules.py @@ -1,9 +1,10 @@ import asyncio -import pytest - +from typing import AsyncIterator, Union, Type, TypeVar, Optional from pathlib import Path from unittest import mock + from packaging.version import Version +import pytest from opentrons.hardware_control import ExecutionManager from opentrons.hardware_control.modules import ModuleAtPort @@ -22,19 +23,27 @@ Thermocycler, HeaterShaker, AbstractModule, + SimulatingModule, + build as build_module, ) from opentrons.hardware_control.modules.mod_abc import parse_fw_version from opentrons.drivers.rpi_drivers.types import USBPort -async def test_get_modules_simulating(): +async def test_get_modules_simulating() -> None: import opentrons.hardware_control as hardware_control mods = { - "tempdeck": [("111", "temperatureModuleV1")], - "magdeck": [("222", "magneticModuleV2")], - "thermocycler": [("333", "thermocyclerModuleV1")], - "heatershaker": [("444", "heaterShakerModuleV1")], + "tempdeck": [ + SimulatingModule(serial_number="111", model="temperatureModuleV1") + ], + "magdeck": [SimulatingModule(serial_number="222", model="magneticModuleV2")], + "thermocycler": [ + SimulatingModule(serial_number="333", model="thermocyclerModuleV1") + ], + "heatershaker": [ + SimulatingModule(serial_number="444", model="heaterShakerModuleV1") + ], } api = await hardware_control.API.build_hardware_simulator(attached_modules=mods) await asyncio.sleep(0.05) @@ -44,10 +53,12 @@ async def test_get_modules_simulating(): await m.cleanup() -async def test_module_caching(): +async def test_module_caching() -> None: import opentrons.hardware_control as hardware_control - mod_names = {"tempdeck": [("111", "temperatureModuleV1")]} + mod_names = { + "tempdeck": [SimulatingModule(serial_number="111", model="temperatureModuleV1")] + } api = await hardware_control.API.build_hardware_simulator( attached_modules=mod_names ) @@ -97,7 +108,9 @@ async def test_module_caching(): ) async def test_create_simulating_module( module_model: ModuleModel, - expected_sim_type: AbstractModule, + expected_sim_type: Union[ + Type[MagDeck], Type[TempDeck], Type[Thermocycler], Type[HeaterShaker] + ], ) -> None: """It should create simulating module instance for specified module.""" import opentrons.hardware_control as hardware_control @@ -112,17 +125,16 @@ async def test_create_simulating_module( @pytest.fixture -async def mod_tempdeck(): - from opentrons.hardware_control import modules +async def mod_tempdeck() -> AsyncIterator[AbstractModule]: usb_port = USBPort( name="", - hub=None, + hub=False, port_number=0, device_path="/dev/ot_module_sim_tempdeck0", ) - tempdeck = await modules.build( + tempdeck = await build_module( port="/dev/ot_module_sim_tempdeck0", usb_port=usb_port, type=ModuleType.TEMPERATURE, @@ -136,17 +148,15 @@ async def mod_tempdeck(): @pytest.fixture -async def mod_magdeck(): - from opentrons.hardware_control import modules - +async def mod_magdeck() -> AsyncIterator[AbstractModule]: usb_port = USBPort( name="", - hub=None, + hub=False, port_number=0, device_path="/dev/ot_module_sim_magdeck0", ) - magdeck = await modules.build( + magdeck = await build_module( port="/dev/ot_module_sim_magdeck0", usb_port=usb_port, type=ModuleType.MAGNETIC, @@ -159,17 +169,15 @@ async def mod_magdeck(): @pytest.fixture -async def mod_thermocycler(): - from opentrons.hardware_control import modules - +async def mod_thermocycler() -> AsyncIterator[AbstractModule]: usb_port = USBPort( name="", - hub=None, + hub=False, port_number=0, device_path="/dev/ot_module_sim_thermocycler0", ) - thermocycler = await modules.build( + thermocycler = await build_module( port="/dev/ot_module_sim_thermocycler0", usb_port=usb_port, type=ModuleType.THERMOCYCLER, @@ -182,17 +190,15 @@ async def mod_thermocycler(): @pytest.fixture -async def mod_thermocycler_gen2(): - from opentrons.hardware_control import modules - +async def mod_thermocycler_gen2() -> AsyncIterator[AbstractModule]: usb_port = USBPort( name="", - hub=None, + hub=False, port_number=0, device_path="/dev/ot_module_sim_thermocycler0", ) - thermocycler = await modules.build( + thermocycler = await build_module( port="/dev/ot_module_sim_thermocycler0", usb_port=usb_port, type=ModuleType.THERMOCYCLER, @@ -206,17 +212,15 @@ async def mod_thermocycler_gen2(): @pytest.fixture -async def mod_heatershaker(): - from opentrons.hardware_control import modules - +async def mod_heatershaker() -> AsyncIterator[AbstractModule]: usb_port = USBPort( name="", - hub=None, + hub=False, port_number=0, device_path="/dev/ot_module_sim_heatershaker0", ) - heatershaker = await modules.build( + heatershaker = await build_module( port="/dev/ot_module_sim_heatershaker0", usb_port=usb_port, type=ModuleType.HEATER_SHAKER, @@ -229,17 +233,19 @@ async def mod_heatershaker(): async def test_module_update_integration( - monkeypatch, - mod_tempdeck, - mod_magdeck, - mod_thermocycler, - mod_heatershaker, - mod_thermocycler_gen2, -): + monkeypatch: pytest.MonkeyPatch, + mod_tempdeck: AbstractModule, + mod_magdeck: AbstractModule, + mod_thermocycler: AbstractModule, + mod_heatershaker: AbstractModule, + mod_thermocycler_gen2: AbstractModule, +) -> None: from opentrons.hardware_control import modules - def async_return(result): - f = asyncio.Future() + T = TypeVar("T") + + def async_return(result: T) -> "asyncio.Future[T]": + f: "asyncio.Future[T]" = asyncio.Future() f.set_result(result) return f @@ -253,7 +259,7 @@ def async_return(result): ) monkeypatch.setattr(modules.update, "upload_via_avrdude", upload_via_avrdude_mock) - async def mock_find_avrdude_bootloader_port(): + async def mock_find_avrdude_bootloader_port() -> str: return "ot_module_avrdude_bootloader1" monkeypatch.setattr( @@ -279,7 +285,7 @@ async def mock_find_avrdude_bootloader_port(): ) monkeypatch.setattr(modules.update, "upload_via_bossa", upload_via_bossa_mock) - async def mock_find_bossa_bootloader_port(): + async def mock_find_bossa_bootloader_port() -> str: return "ot_module_bossa_bootloader1" monkeypatch.setattr( @@ -297,7 +303,7 @@ async def mock_find_bossa_bootloader_port(): ) monkeypatch.setattr(modules.update, "upload_via_dfu", upload_via_dfu_mock) - async def mock_find_dfu_device_hs(pid: str, expected_device_count: int): + async def mock_find_dfu_device_hs(pid: str, expected_device_count: int) -> str: if expected_device_count == 2: return "df11" return "none" @@ -310,7 +316,7 @@ async def mock_find_dfu_device_hs(pid: str, expected_device_count: int): ) upload_via_dfu_mock.reset_mock() - async def mock_find_dfu_device_tc2(pid: str, expected_device_count: int): + async def mock_find_dfu_device_tc2(pid: str, expected_device_count: int) -> str: if expected_device_count == 3: return "df11" return "none" @@ -325,7 +331,7 @@ async def mock_find_dfu_device_tc2(pid: str, expected_device_count: int): mod_thermocycler_gen2 -async def test_get_bundled_fw(monkeypatch, tmpdir): +async def test_get_bundled_fw(monkeypatch: pytest.MonkeyPatch, tmpdir: Path) -> None: from opentrons.hardware_control import modules dummy_td_file = Path(tmpdir) / "temperature-module@v1.2.3.hex" @@ -349,10 +355,16 @@ async def test_get_bundled_fw(monkeypatch, tmpdir): from opentrons.hardware_control import API mods = { - "tempdeck": [("111", "temperatureModuleV1")], - "magdeck": [("222", "magneticModuleV2")], - "thermocycler": [("333", "thermocyclerModuleV1")], - "heatershaker": [("444", "heaterShakerModuleV1")], + "tempdeck": [ + SimulatingModule(serial_number="111", model="temperatureModuleV1") + ], + "magdeck": [SimulatingModule(serial_number="222", model="magneticModuleV2")], + "thermocycler": [ + SimulatingModule(serial_number="333", model="thermocyclerModuleV1") + ], + "heatershaker": [ + SimulatingModule(serial_number="444", model="heaterShakerModuleV1") + ], } api = await API.build_hardware_simulator(attached_modules=mods) @@ -375,8 +387,11 @@ async def test_get_bundled_fw(monkeypatch, tmpdir): async def test_get_thermocycler_bundled_fw( - mod_thermocycler, mod_thermocycler_gen2, monkeypatch, tmpdir -): + mod_thermocycler: AbstractModule, + mod_thermocycler_gen2: AbstractModule, + monkeypatch: pytest.MonkeyPatch, + tmpdir: Path, +) -> None: from opentrons.hardware_control import modules dummy_tc_file = Path(tmpdir) / "thermocycler@v0.1.2.bin" @@ -405,7 +420,7 @@ async def test_get_thermocycler_bundled_fw( (None, "magneticModuleV1"), ], ) -def test_magnetic_module_revision_parsing(revision, model): +def test_magnetic_module_revision_parsing(revision: Optional[str], model: str) -> None: assert MagDeck._model_from_revision(revision) == model @@ -422,7 +437,9 @@ def test_magnetic_module_revision_parsing(revision, model): (None, "temperatureModuleV1"), ], ) -def test_temperature_module_revision_parsing(revision, model): +def test_temperature_module_revision_parsing( + revision: Optional[str], model: str +) -> None: assert TempDeck._model_from_revision(revision) == model diff --git a/api/tests/opentrons/hardware_control/test_thread_manager.py b/api/tests/opentrons/hardware_control/test_thread_manager.py index 9b48dac97b1..db962c5b51b 100644 --- a/api/tests/opentrons/hardware_control/test_thread_manager.py +++ b/api/tests/opentrons/hardware_control/test_thread_manager.py @@ -1,10 +1,11 @@ import asyncio import weakref -from typing import Any +from typing import NoReturn, Optional +import threading import pytest -from opentrons.hardware_control.modules import ModuleAtPort +from opentrons.hardware_control.modules import ModuleAtPort, SimulatingModule from opentrons.hardware_control.thread_manager import ( ThreadManagerException, ThreadManager, @@ -12,11 +13,11 @@ from opentrons.hardware_control.api import API -async def _raise_exception() -> Any: +async def _raise_exception() -> NoReturn: raise Exception("oh no") -def test_build_fail_raises_exception(): +def test_build_fail_raises_exception() -> None: """ Test that a builder that raises an exception raises a ThreadManagerException @@ -25,10 +26,12 @@ def test_build_fail_raises_exception(): ThreadManager(_raise_exception) -def test_module_cache_add_entry(): +def test_module_cache_add_entry() -> None: """Test that _cached_modules updates correctly.""" - mod_names = {"tempdeck": [("111", "temperatureModuleV2")]} + mod_names = { + "tempdeck": [SimulatingModule(serial_number="111", model="temperatureModuleV2")] + } thread_manager = ThreadManager( API.build_hardware_simulator, attached_modules=mod_names ) @@ -47,11 +50,13 @@ def test_module_cache_add_entry(): assert mods == mods2 -async def test_module_cache_remove_entry(): +async def test_module_cache_remove_entry() -> None: """Test that module entry gets removed from cache when module detaches.""" mod_names = { - "tempdeck": [("111", "temperatureModuleV2")], - "magdeck": [("222", "magneticModuleV1")], + "tempdeck": [ + SimulatingModule(serial_number="111", model="temperatureModuleV2") + ], + "magdeck": [SimulatingModule(serial_number="222", model="magneticModuleV1")], } thread_manager = ThreadManager( API.build_hardware_simulator, attached_modules=mod_names @@ -60,7 +65,8 @@ async def test_module_cache_remove_entry(): mods_before = thread_manager.attached_modules assert len(mods_before) == 2 - loop: asyncio.AbstractEventLoop = thread_manager._loop + loop = thread_manager._loop + assert loop # The coroutine must be called using the threadmanager's loop. future = asyncio.run_coroutine_threadsafe( @@ -76,7 +82,51 @@ async def test_module_cache_remove_entry(): assert len(mods_after) == 1 -async def test_wraps_instance(): +async def test_wraps_instance() -> None: """It should expose the underlying type.""" thread_manager = ThreadManager(API.build_hardware_simulator) assert thread_manager.wraps_instance(API) + + +class Blocker: + """Test object for nonblocking construction.""" + + @classmethod + async def build_blocking( + cls, + wait_event: asyncio.Event, + running_event: threading.Event, + loop: Optional[asyncio.AbstractEventLoop] = None, + ) -> "Blocker": + """Build an instance.""" + inst = cls(loop) + running_event.set() + await wait_event.wait() + return inst + + def __init__(self, loop: Optional[asyncio.AbstractEventLoop] = None) -> None: + """Initialize an instance.""" + self._loop = loop + + async def clean_up(self) -> None: + """Allows cleanup.""" + pass + + +def test_nonblocking_via_attr() -> None: + """Its init should return immediately if the builder has a nonblocking attribute.""" + + wait_event = asyncio.Event() + running_event = threading.Event() + thread_manager = ThreadManager( # type: ignore[type-var] + ThreadManager.nonblocking_builder(Blocker.build_blocking), + running_event=running_event, + wait_event=wait_event, + ) + + running_event.wait() + assert not thread_manager._is_running.is_set() + assert thread_manager._loop + thread_manager._loop.call_soon_threadsafe(wait_event.set) + thread_manager.managed_thread_ready_blocking() + thread_manager.clean_up() diff --git a/api/tests/opentrons/protocol_api_old/test_context.py b/api/tests/opentrons/protocol_api_old/test_context.py index 9b93c1c4a9d..f2406500e6a 100644 --- a/api/tests/opentrons/protocol_api_old/test_context.py +++ b/api/tests/opentrons/protocol_api_old/test_context.py @@ -21,6 +21,7 @@ from opentrons.hardware_control import API, ThreadManagedHardware from opentrons.hardware_control.instruments.ot2.pipette import Pipette from opentrons.hardware_control.types import Axis +from opentrons.hardware_control.modules import SimulatingModule from opentrons.protocols.advanced_control import transfers as tf from opentrons.protocols.api_support import instrument as instrument_support from opentrons.protocols.api_support.types import APIVersion @@ -960,8 +961,13 @@ def test_order_of_module_load(): import opentrons.protocol_api as protocol_api mods = { - "tempdeck": [("111", "temperatureModuleV1"), ("333", "temperatureModuleV2")], - "thermocycler": [("222", "thermocyclerModuleV2")], + "tempdeck": [ + SimulatingModule(serial_number="111", model="temperatureModuleV1"), + SimulatingModule(serial_number="333", model="temperatureModuleV2"), + ], + "thermocycler": [ + SimulatingModule(serial_number="222", model="thermocyclerModuleV2") + ], } thread_manager = hardware_control.ThreadManager( hardware_control.API.build_hardware_simulator, attached_modules=mods diff --git a/robot-server/tests/subsystems/test_router.py b/robot-server/tests/subsystems/test_router.py index 387b5001a40..c77e291736c 100644 --- a/robot-server/tests/subsystems/test_router.py +++ b/robot-server/tests/subsystems/test_router.py @@ -1,6 +1,6 @@ """Tests for /subsystems routes.""" from datetime import datetime -from typing import Set, Dict, TYPE_CHECKING +from typing import Set, Dict, TYPE_CHECKING, cast from fastapi import Response, Request from starlette.datastructures import URL, MutableHeaders @@ -72,10 +72,10 @@ def thread_manager(decoy: Decoy, ot3_hardware_api: "OT3API") -> ThreadManagedHar from opentrons.hardware_control.ot3api import OT3API except ImportError: pytest.skip("Cannot run on OT-2 (for now)") - manager = decoy.mock(cls=ThreadManagedHardware) + manager = decoy.mock(cls=ThreadManagedHardware) # type: ignore[misc] decoy.when(manager.wrapped()).then_return(ot3_hardware_api) decoy.when(manager.wraps_instance(OT3API)).then_return(True) - return manager + return cast(ThreadManagedHardware, manager) def _build_attached_subsystem( From 963ff9ea350d13adefa6de10b76e69ee7ce47b67 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Tue, 7 May 2024 16:23:28 -0400 Subject: [PATCH 4/5] too compatible --- .../http_api/commands/test_load_module_failure.tavern.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/robot-server/tests/integration/http_api/commands/test_load_module_failure.tavern.yaml b/robot-server/tests/integration/http_api/commands/test_load_module_failure.tavern.yaml index 6691a65b7ee..312603f4899 100644 --- a/robot-server/tests/integration/http_api/commands/test_load_module_failure.tavern.yaml +++ b/robot-server/tests/integration/http_api/commands/test_load_module_failure.tavern.yaml @@ -6,7 +6,7 @@ marks: - parametrize: key: model vals: - - magneticModuleV2 + - thermocyclerModuleV2 stages: - name: Create Empty Run From 0c75395a1ba4fe26f380dd3e2dab5977be38da13 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Tue, 7 May 2024 17:03:03 -0400 Subject: [PATCH 5/5] docstring --- api/src/opentrons/hardware_control/backends/simulator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/opentrons/hardware_control/backends/simulator.py b/api/src/opentrons/hardware_control/backends/simulator.py index 59b3d9b9dfd..9441d478738 100644 --- a/api/src/opentrons/hardware_control/backends/simulator.py +++ b/api/src/opentrons/hardware_control/backends/simulator.py @@ -72,8 +72,8 @@ async def build( empty dicts or to dicts containing 'model' and 'id' keys. :param attached_modules: A map of module type names (e.g. - `'tempdeck'` or `'magdeck'`) to lists of (serial, model) - tuples representing + `'tempdeck'` or `'magdeck'`) to lists of SimulatingModel + dataclasses representing modules the simulator should assume are attached. Like `attached_instruments`, used to make the simulator match the setup of the