From 95117e778af0e197dfe19b7ccab5e6d07fb6a735 Mon Sep 17 00:00:00 2001
From: Talley Lambert <talley.lambert@gmail.com>
Date: Sun, 21 Jul 2024 16:28:28 -0400
Subject: [PATCH] fix: fix valueChanged signals on PropertyWidget (#352)

---
 src/pymmcore_widgets/_property_widget.py | 21 ++++++++++++---------
 tests/test_prop_widget.py                | 13 +++++++++++--
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/src/pymmcore_widgets/_property_widget.py b/src/pymmcore_widgets/_property_widget.py
index 72c0e5a29..59c94feb8 100644
--- a/src/pymmcore_widgets/_property_widget.py
+++ b/src/pymmcore_widgets/_property_widget.py
@@ -336,10 +336,12 @@ def __init__(
         self.layout().addWidget(cast(QWidget, self._value_widget))
         self.destroyed.connect(self._disconnect)
 
-    def _try_update_from_core(self) -> None:
+    def _try_update_from_core(self) -> Any:
         # set current value from core, ignoring errors
+        value = ""
         with contextlib.suppress(RuntimeError, ValueError):
-            self._value_widget.setValue(self._mmc.getProperty(*self._dp))
+            value = self._mmc.getProperty(*self._dp)
+            self._value_widget.setValue(value)
 
         # disable for any device init state besides 0 (Uninitialized)
         if hasattr(self._mmc, "getDeviceInitializationState") and (
@@ -347,6 +349,7 @@ def _try_update_from_core(self) -> None:
             and self._mmc.getDeviceInitializationState(self._device_label)
         ):
             self.setDisabled(True)
+        return value
 
     # connect events and queue for disconnection on widget destroyed
     def _on_core_change(self, dev_label: str, prop_name: str, new_val: Any) -> None:
@@ -355,13 +358,13 @@ def _on_core_change(self, dev_label: str, prop_name: str, new_val: Any) -> None:
                 self._value_widget.setValue(new_val)
 
     def _on_value_widget_change(self, value: Any) -> None:
-        if not self._updates_core:
-            return
-        try:
-            self._mmc.setProperty(self._device_label, self._prop_name, value)
-        except (RuntimeError, ValueError):
-            # if there's an error when updating mmcore, reset widget value to mmcore
-            self._try_update_from_core()
+        if self._updates_core:
+            try:
+                self._mmc.setProperty(self._device_label, self._prop_name, value)
+            except (RuntimeError, ValueError):
+                # if there's an error when updating mmcore, reset widget value to mmcore
+                value = self._try_update_from_core()
+        self.valueChanged.emit(value)
 
     def _disconnect(self) -> None:
         with contextlib.suppress(RuntimeError):
diff --git a/tests/test_prop_widget.py b/tests/test_prop_widget.py
index 127127f55..240e69b3a 100644
--- a/tests/test_prop_widget.py
+++ b/tests/test_prop_widget.py
@@ -32,7 +32,7 @@ def _assert_equal(a, b):
 
 
 @pytest.mark.parametrize("dev, prop", dev_props)
-def test_property_widget(dev, prop, qtbot):
+def test_property_widget(dev, prop, qtbot) -> None:
     wdg = PropertyWidget(dev, prop, mmcore=CORE)
     qtbot.addWidget(wdg)
     if CORE.isPropertyReadOnly(dev, prop) or prop in (
@@ -83,7 +83,16 @@ def test_property_widget(dev, prop, qtbot):
     _assert_equal(wdg.value(), start_val)
 
 
-def test_reset(global_mmcore, qtbot):
+def test_prop_widget_signals(global_mmcore: CMMCorePlus, qtbot):
+    wdg = PropertyWidget("Camera", "Binning", connect_core=False)
+    qtbot.addWidget(wdg)
+    assert wdg.value() == "1"
+    with qtbot.waitSignal(wdg.valueChanged, timeout=1000):
+        wdg._value_widget.setValue(2)
+    assert wdg.value() == "2"
+
+
+def test_reset(global_mmcore: CMMCorePlus, qtbot) -> None:
     wdg = PropertyWidget("Camera", "Binning", mmcore=global_mmcore)
     qtbot.addWidget(wdg)
     global_mmcore.loadSystemConfiguration()