fix(bec_widgets): stop gating the hard-stop button on a live device check
_hard_stop_available() re-resolved hard_stop_device_name against the device manager and disabled the button if that lookup failed, checked once at construction with no way to recover for the widget's lifetime. In practice this disabled the button even for ftransy confirmed present and enabled from the CLI. Since _on_abort() already resolves the device and calls its controller inside its own try/except, the live pre-check was redundant and its false negatives cost more than they protected. The button is now enabled whenever a hard_stop_device_name was configured at all; a name that doesn't actually resolve fails safely (logged, no-op) at click time instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -46,10 +46,13 @@ class ConsoleButtonsWidget(BECWidget, QWidget):
|
||||
effectively immediate.
|
||||
|
||||
``hard_stop_device_name`` names the device whose ``.controller`` the
|
||||
hard stop is called on (e.g. ``"ftransy"`` for flomni). Without a
|
||||
device configured, or if that device isn't present/enabled in the
|
||||
current session, the button is disabled rather than silently doing
|
||||
nothing.
|
||||
hard stop is called on (e.g. ``"ftransy"`` for flomni). Without a device
|
||||
*name* configured, the button is disabled rather than silently doing
|
||||
nothing. A configured name that doesn't actually resolve to a device
|
||||
(e.g. missing from the current session's config) is not checked ahead
|
||||
of time -- it's re-resolved at click time in ``_on_abort()``, which
|
||||
already logs and no-ops on failure -- so the button being enabled is
|
||||
not a guarantee the device exists, only that a target was configured.
|
||||
"""
|
||||
|
||||
USER_ACCESS = ["message", "message.setter", "response", "clear_response"]
|
||||
@@ -69,23 +72,21 @@ class ConsoleButtonsWidget(BECWidget, QWidget):
|
||||
self._init_ui()
|
||||
|
||||
def _hard_stop_available(self) -> bool:
|
||||
"""True if hard_stop_device_name names a device that's actually
|
||||
present and enabled in this session -- checked once at construction
|
||||
time (the config doesn't change mid-session).
|
||||
"""True if a hard_stop_device_name was configured for this widget.
|
||||
|
||||
Device access is wrapped in try/except: DeviceManagerBase.__getattr__
|
||||
raises DeviceConfigError (not AttributeError) for an unknown device
|
||||
name, so a plain getattr(self.dev, name, None) would NOT fall back
|
||||
to the default and would propagate instead (same guard as
|
||||
SampleStorageWidget._check_flomni_available()).
|
||||
Deliberately does not check whether the name actually resolves to a
|
||||
live, enabled device: that live check (a getattr(self.dev, name)
|
||||
device-manager lookup) proved unreliable in practice -- a
|
||||
confirmed-present, confirmed-enabled device could still evaluate as
|
||||
unavailable here, permanently disabling the button for the widget's
|
||||
lifetime with no way to recover short of recreating the dock. Since
|
||||
whatever device is named is re-resolved with its own try/except in
|
||||
_on_abort() anyway, a stale or wrong name still fails safely
|
||||
(logged, no-op) instead of silently doing nothing -- so skipping the
|
||||
redundant, unreliable pre-check here only removes a false-negative
|
||||
failure mode, not real safety.
|
||||
"""
|
||||
if not self._hard_stop_device_name:
|
||||
return False
|
||||
try:
|
||||
device = getattr(self.dev, self._hard_stop_device_name, None)
|
||||
except Exception:
|
||||
return False
|
||||
return device is not None and getattr(device, "enabled", True)
|
||||
return bool(self._hard_stop_device_name)
|
||||
|
||||
def _init_ui(self):
|
||||
layout = QVBoxLayout(self)
|
||||
|
||||
Reference in New Issue
Block a user