diff --git a/api/src/opentrons/drivers/temp_deck/driver.py b/api/src/opentrons/drivers/temp_deck/driver.py index cf37d52882a8..0df44e54ce9b 100644 --- a/api/src/opentrons/drivers/temp_deck/driver.py +++ b/api/src/opentrons/drivers/temp_deck/driver.py @@ -210,17 +210,25 @@ def update_temperature(self, default=None) -> str: updated_temperature = default or self._temperature.copy() self._temperature.update(updated_temperature) else: - # comment - try: - self._update_thread = Thread( - target=self._recursive_update_temperature, - args=[DEFAULT_COMMAND_RETRIES], - name='Tempdeck recursive update temperature') - self._update_thread.start() - except (TempDeckError, SerialException, SerialNoResponse) as e: - return str(e) + def _update(): + try: + self._recursive_update_temperature(retries=DEFAULT_COMMAND_RETRIES) + except (OSError, TempDeckError, SerialException, SerialNoResponse): + log.exception("UPDATE FAILED") + + self._update_thread = Thread( + target=_update(), + name='Tempdeck recursive update temperature') + self._update_thread.start() return '' + def _update_temperature(self): + self._update_thread = Thread( + target=self._recursive_update_temperature, + args=[DEFAULT_COMMAND_RETRIES], + name='Tempdeck recursive update temperature') + self._update_thread.start() + @property def target(self) -> Optional[int]: return self._temperature.get('target') diff --git a/api/src/opentrons/drivers/thermocycler/driver.py b/api/src/opentrons/drivers/thermocycler/driver.py index 2e99a5d6f3f0..d8decbbdec94 100644 --- a/api/src/opentrons/drivers/thermocycler/driver.py +++ b/api/src/opentrons/drivers/thermocycler/driver.py @@ -331,10 +331,12 @@ def send(self, command, callback): self._send_write_fd.write(b'c') def close(self): + log.debug("Halting TCPoller") self._halt_write_fd.write(b'q') def __del__(self): """ Clean up thread fifos""" + log.debug("Cleaning up thread fifos in TCPoller.") try: os.unlink(self._send_path) except NameError: @@ -376,9 +378,10 @@ async def connect(self, port: str) -> 'Thermocycler': return self def disconnect(self) -> 'Thermocycler': - if self.is_connected(): + if self.is_connected() or self._poller: self._poller.close() # type: ignore self._poller.join() # type: ignore + log.debug("TC poller stopped.") self._poller = None return self @@ -627,6 +630,7 @@ async def enter_programming_mode(self): def __del__(self): try: - self._poller.close() # type: ignore + if self._poller: + self._poller.close() # type: ignore except Exception: - log.exception('Exception while cleaning up Thermocycler:') + log.exception('Exception while cleaning up Thermocycler') diff --git a/api/src/opentrons/hardware_control/api.py b/api/src/opentrons/hardware_control/api.py index c1c49bf20243..51d31cf9c949 100644 --- a/api/src/opentrons/hardware_control/api.py +++ b/api/src/opentrons/hardware_control/api.py @@ -1733,7 +1733,7 @@ def _unregister_modules(self, for removed_mod in removed_modules: self._log.info(f"Module {removed_mod.name()} detached" f" from port {removed_mod.port}") - del removed_mod + removed_mod.cleanup() async def register_modules( self, diff --git a/api/src/opentrons/hardware_control/modules/magdeck.py b/api/src/opentrons/hardware_control/modules/magdeck.py index b5e3b840e4f5..f6d60ad37d5f 100644 --- a/api/src/opentrons/hardware_control/modules/magdeck.py +++ b/api/src/opentrons/hardware_control/modules/magdeck.py @@ -209,7 +209,7 @@ def _disconnect(self): if self._driver: self._driver.disconnect(port=self._port) - def __del__(self): + def cleanup(self) -> None: self._disconnect() async def prep_for_update(self) -> str: diff --git a/api/src/opentrons/hardware_control/modules/mod_abc.py b/api/src/opentrons/hardware_control/modules/mod_abc.py index a2edb1f90542..a6496e05a192 100644 --- a/api/src/opentrons/hardware_control/modules/mod_abc.py +++ b/api/src/opentrons/hardware_control/modules/mod_abc.py @@ -162,3 +162,11 @@ def name(cls) -> str: def bootloader(cls) -> UploadFunction: """ Method used to upload file to this module's bootloader. """ pass + + def cleanup(self) -> None: + """ Clean up the module instance. + + Clean up, i.e. stop pollers, disconnect serial, etc in preparation for + object destruction. + """ + pass diff --git a/api/src/opentrons/hardware_control/modules/tempdeck.py b/api/src/opentrons/hardware_control/modules/tempdeck.py index 10b32b88a5b8..fb36fd975369 100644 --- a/api/src/opentrons/hardware_control/modules/tempdeck.py +++ b/api/src/opentrons/hardware_control/modules/tempdeck.py @@ -242,8 +242,9 @@ async def _connect(self): self._poller = Poller(self._driver) self._poller.start() - def __del__(self): - if hasattr(self, '_poller') and self._poller: + def cleanup(self) -> None: + if self._poller: + log.debug("Stopping tempdeck poller.") self._poller.stop() async def prep_for_update(self) -> str: diff --git a/api/src/opentrons/hardware_control/modules/thermocycler.py b/api/src/opentrons/hardware_control/modules/thermocycler.py index 172bac172717..37b30057e8a7 100644 --- a/api/src/opentrons/hardware_control/modules/thermocycler.py +++ b/api/src/opentrons/hardware_control/modules/thermocycler.py @@ -347,3 +347,6 @@ async def prep_for_update(self): new_port = await update.find_bootloader_port() return new_port or self.port + + def cleanup(self) -> None: + self._driver.disconnect() diff --git a/api/src/opentrons/hardware_control/thread_manager.py b/api/src/opentrons/hardware_control/thread_manager.py index a3ad835e83cf..2b498c53b4b3 100644 --- a/api/src/opentrons/hardware_control/thread_manager.py +++ b/api/src/opentrons/hardware_control/thread_manager.py @@ -4,7 +4,7 @@ import logging import asyncio import functools -from typing import Generic, TypeVar, Any, Optional +from typing import Generic, TypeVar, Any, Optional, Dict from .adapters import SynchronousAdapter from .modules.mod_abc import AbstractModule @@ -108,7 +108,7 @@ def __init__(self, builder, *args, **kwargs): self._sync_managed_obj: Optional[SynchronousAdapter] = None is_running = threading.Event() self._is_running = is_running - + self._cached_modules: Dict[AbstractModule, CallBridger[AbstractModule]] = {} # TODO: remove this if we switch to python 3.8 # https://docs.python.org/3/library/asyncio-subprocess.html#subprocess-and-threads # noqa # On windows, the event loop and system interface is different and @@ -175,10 +175,9 @@ def clean_up(self): loop.call_soon_threadsafe(loop.stop) except Exception: pass - object.__getattribute__(self, 'wrap_module').cache_clear() + self._cached_modules = {} object.__getattribute__(self, '_thread').join() - @functools.lru_cache(8) def wrap_module( self, module: AbstractModule) -> CallBridger[AbstractModule]: return CallBridger(module, object.__getattribute__(self, '_loop')) @@ -193,7 +192,16 @@ def __getattribute__(self, attr_name): wrap = object.__getattribute__(self, 'wrap_module') managed = object.__getattribute__(self, 'managed_obj') attr = getattr(managed, attr_name) - return [wrap(mod) for mod in attr] + cached_mods = object.__getattribute__(self, '_cached_modules') + + # Update self._cached_modules to delete all removed modules' entries and add + # newly attached modules. Removing references to stale instances + # is necessary to allow the garbage collector to delete those objects from + # memory and cleanly stop all the threads associated with them. + cached_mods = { + module: cached_mods.get(module, wrap(module)) for module in attr} + object.__setattr__(self, '_cached_modules', cached_mods) + return cached_mods.values() elif attr_name == 'clean_up': # the wrapped object probably has this attr as well as us, and we # want to call both, with the wrapped one first