Skip to content

Commit

Permalink
Try to fix device error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
alexdewar committed Oct 6, 2023
1 parent 2839974 commit 68b7619
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 29 deletions.
31 changes: 16 additions & 15 deletions finesse/gui/device_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ def __init__(
# pubsub subscriptions
pub.subscribe(self._on_device_opened, f"device.opened.{topic}")
pub.subscribe(self._on_device_closed, f"device.closed.{topic}")
pub.subscribe(self._show_error_message, f"device.error.{topic}")

def _on_device_selected(self, device_idx: int) -> None:
"""Swap out the parameter combo boxes for the current device.
Expand Down Expand Up @@ -220,6 +221,21 @@ def _on_device_closed(self) -> None:
self._set_combos_enabled(True)
self._open_close_btn.setText("Open")

def _show_error_message(
self, instance: DeviceInstanceRef, error: BaseException
) -> None:
"""Show an error message when something has gone wrong with the device.
Todo:
The name of the device isn't currently very human readable.
"""
QMessageBox(
QMessageBox.Icon.Critical,
"A device error has occurred",
"A fatal error has occurred with the "
f"{instance.topic} device: {error!s}",
).exec()

def _on_open_close_clicked(self) -> None:
"""Open/close the connection of the chosen device when the button is pushed."""
if self._open_close_btn.text() == "Open":
Expand All @@ -239,21 +255,6 @@ def __init__(self) -> None:

# pubsub topics
pub.subscribe(self._on_device_list, "device.list")
pub.subscribe(self._show_error_message, "device.error")

def _show_error_message(
self, instance: DeviceInstanceRef, error: BaseException
) -> None:
"""Show an error message when something has gone wrong with the device.
Todo:
The name of the device isn't currently very human readable.
"""
QMessageBox(
QMessageBox.Icon.Critical,
"A device error has occurred",
f"A fatal error has occurred with the {instance.topic} device: {error!s}",
).exec()

def _on_device_list(
self, device_types: dict[DeviceBaseTypeInfo, list[DeviceTypeInfo]]
Expand Down
11 changes: 7 additions & 4 deletions finesse/gui/measure_script/script.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
from schema import And, Or, Schema, SchemaError
from statemachine import State, StateMachine

from ...config import ANGLE_PRESETS, STEPPER_MOTOR_TOPIC
from ...em27_status import EM27Status
from ..error_message import show_error_message
from finesse.config import ANGLE_PRESETS, STEPPER_MOTOR_TOPIC
from finesse.device_info import DeviceInstanceRef
from finesse.em27_status import EM27Status
from finesse.gui.error_message import show_error_message


@dataclass(frozen=True)
Expand Down Expand Up @@ -407,7 +408,9 @@ def unpause(self) -> None:
case self.waiting_to_measure:
self.finish_waiting_for_measure()

def _on_stepper_motor_error(self, error: BaseException) -> None:
def _on_stepper_motor_error(
self, instance: DeviceInstanceRef, error: BaseException
) -> None:
"""Call abort()."""
self.abort()

Expand Down
8 changes: 6 additions & 2 deletions finesse/hardware/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from datetime import datetime

from finesse.config import NUM_TEMPERATURE_MONITOR_CHANNELS, TEMPERATURE_MONITOR_TOPIC
from finesse.device_info import DeviceBaseTypeInfo, DeviceTypeInfo
from finesse.device_info import DeviceBaseTypeInfo, DeviceInstanceRef, DeviceTypeInfo

from . import data_file_writer # noqa: F401
from .plugins import load_device_types
Expand Down Expand Up @@ -68,7 +68,11 @@ def _try_get_temperatures() -> list[Decimal] | None:
try:
return dev.get_temperatures()
except Exception as error:
pub.sendMessage(f"device.error.{TEMPERATURE_MONITOR_TOPIC}", error=error)
pub.sendMessage(
f"device.error.{TEMPERATURE_MONITOR_TOPIC}",
instance=DeviceInstanceRef(TEMPERATURE_MONITOR_TOPIC),
error=error,
)
return None


Expand Down
11 changes: 9 additions & 2 deletions finesse/hardware/data_file_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from pubsub import pub

from finesse import config
from finesse.device_info import DeviceInstanceRef

from .plugins.stepper_motor import get_stepper_motor_instance
from .plugins.temperature import get_temperature_controller_instance
Expand Down Expand Up @@ -64,7 +65,11 @@ def _get_stepper_motor_angle() -> tuple[float, bool]:
else:
return (float("nan"), True)
except Exception as error:
pub.sendMessage(f"device.error.{config.STEPPER_MOTOR_TOPIC}", error=error)
pub.sendMessage(
f"device.error.{config.STEPPER_MOTOR_TOPIC}",
instance=DeviceInstanceRef(config.STEPPER_MOTOR_TOPIC),
error=error,
)
return (float("nan"), False)


Expand All @@ -82,8 +87,10 @@ def _get_hot_bb_power() -> float:
try:
return hot_bb.power * 100 / config.TC4820_MAX_POWER
except Exception as error:
instance = DeviceInstanceRef(config.TEMPERATURE_CONTROLLER_TOPIC, hot_bb.name)
pub.sendMessage(
f"device.error.{config.TEMPERATURE_CONTROLLER_TOPIC}.{hot_bb.name}",
f"device.error.{instance.topic}",
instance=instance,
error=error,
)
return float("nan")
Expand Down
4 changes: 3 additions & 1 deletion finesse/hardware/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ def _open_device(
devices[instance] = cls.from_params(**params)
except Exception as error:
logging.error(f"Failed to open {base_type} device: {str(error)}")
pub.sendMessage("device.error", instance=instance, error=error)
pub.sendMessage(
f"device.error.{instance.topic}", instance=instance, error=error
)
else:
logging.info("Opened device")

Expand Down
12 changes: 10 additions & 2 deletions finesse/hardware/plugins/stepper_motor/stepper_motor_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
from pubsub import pub

from finesse.config import ANGLE_PRESETS, STEPPER_MOTOR_TOPIC
from finesse.device_info import DeviceInstanceRef
from finesse.hardware.device_base import DeviceBase
from finesse.hardware.plugins import register_base_device_type
from finesse.hardware.pubsub_decorators import pubsub_errors

error_wrap = pubsub_errors(f"device.error.{STEPPER_MOTOR_TOPIC}")
error_wrap = pubsub_errors(
f"device.error.{STEPPER_MOTOR_TOPIC}",
instance=DeviceInstanceRef(STEPPER_MOTOR_TOPIC),
)
"""Broadcast exceptions via pubsub."""


Expand Down Expand Up @@ -40,7 +44,11 @@ def __init__(self) -> None:
@staticmethod
def send_error_message(error: BaseException) -> None:
"""Send an error message when a device error has occurred."""
pub.sendMessage(f"device.error.{STEPPER_MOTOR_TOPIC}", error=error)
pub.sendMessage(
f"device.error.{STEPPER_MOTOR_TOPIC}",
instance=DeviceInstanceRef(STEPPER_MOTOR_TOPIC),
error=error,
)

@staticmethod
def preset_angle(name: str) -> float:
Expand Down
5 changes: 3 additions & 2 deletions finesse/hardware/pubsub_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import logging
import traceback
from collections.abc import Callable
from typing import Any

from decorator import decorator
from pubsub import pub
Expand All @@ -18,7 +19,7 @@ def _error_occurred(error_topic: str, error: BaseException) -> None:
pub.sendMessage(error_topic, error=error)


def pubsub_errors(error_topic: str) -> Callable:
def pubsub_errors(error_topic: str, **extra_kwargs: Any) -> Callable:
"""Catch exceptions and broadcast via pubsub.
Args:
Expand All @@ -29,7 +30,7 @@ def wrapped(func: Callable, *args, **kwargs):
try:
return func(*args, **kwargs)
except Exception as error:
_error_occurred(error_topic, error)
_error_occurred(error_topic, error, **extra_kwargs)

return decorator(wrapped)

Expand Down
5 changes: 4 additions & 1 deletion tests/gui/measure_script/test_script_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from statemachine import State

from finesse.config import STEPPER_MOTOR_TOPIC
from finesse.device_info import DeviceInstanceRef
from finesse.em27_status import EM27Status
from finesse.gui.measure_script.script import Script, ScriptRunner, _poll_em27_status

Expand Down Expand Up @@ -285,7 +286,9 @@ def test_abort(
def test_on_stepper_motor_error(runner: ScriptRunner) -> None:
"""Test that the _on_stepper_motor_error() method calls abort()."""
with patch.object(runner, "abort") as abort_mock:
runner._on_stepper_motor_error(RuntimeError("hello"))
runner._on_stepper_motor_error(
instance=DeviceInstanceRef(STEPPER_MOTOR_TOPIC), error=RuntimeError("hello")
)
abort_mock.assert_called_once_with()


Expand Down

0 comments on commit 68b7619

Please sign in to comment.