Adding ConfigSignal version of get_current_settings and retrieve_settings. (#803)

* Variation of store_settings which only handles configuration attributes in StandardReadable devices

* Moved walk_config_signals to _readable

* defined config_names before loop

* walk_config_signals now checks for bluesky.protocols.Configurable so I moved it back to _signal.py. The test should work now.

* Ran ruff

* Clearing pre-commit,typechecking with tox

* Update src/ophyd_async/core/_signal.py

Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>

* Reverted suggested change

* Added config signal only plans to handle settings including get_current_config_settings and retrieve_config_settings.

* Added only_config=False kwarg to store_config, get_current_settings, and retrieve_settings to optionally only handle configuration signals instead of all SignalRWs.

---------

Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>

Author: burkeds

src/ophyd_async/plan_stubs/__init__.py

@@ -13,7 +13,6 @@
     apply_settings_if_different,
     get_current_settings,
     retrieve_settings,
-    store_config_settings,
     store_settings,
 )
 
@@ -30,5 +29,4 @@
     "get_current_settings",
     "retrieve_settings",
     "store_settings",
-    "store_config_settings",
 ]

src/ophyd_async/plan_stubs/_settings.py

@@ -33,51 +33,61 @@ def _get_values_of_signals(
 
 
 @plan
-def get_current_settings(device: Device) -> MsgGenerator[Settings]:
-    """Get current settings on `Device`."""
-    signals = walk_rw_signals(device)
+def get_current_settings(
+    device: Device, only_config: bool = False
+) -> MsgGenerator[Settings]:
+    """Get current settings on `Device`.
+
+    If `only_config` is True, get current configuration settings on `Configurable`.
+    """
+    if only_config:
+        signals = yield from wait_for_awaitable(walk_config_signals(device))
+    else:
+        signals = walk_rw_signals(device)
     named_values = yield from _get_values_of_signals(signals)
     signal_values = {signals[name]: value for name, value in named_values.items()}
     return Settings(device, signal_values)
 
 
 @plan
 def store_settings(
-    provider: SettingsProvider, name: str, device: Device
+    provider: SettingsProvider, name: str, device: Device, only_config: bool = False
 ) -> MsgGenerator[None]:
     """Walk a Device for SignalRWs and store their values.
 
-    :param provider: The provider to store the settings with.
-    :param name: The name to store the settings under.
-    :param device: The Device to walk for SignalRWs.
-    """
-    signals = walk_rw_signals(device)
-    named_values = yield from _get_values_of_signals(signals)
-    yield from wait_for_awaitable(provider.store(name, named_values))
-
-
-@plan
-def store_config_settings(
-    provider: SettingsProvider, name: str, device: Device
-) -> MsgGenerator[None]:
-    """Walk a Device for SignalRWs and store their values.
+    If `only_config` is True, store only configuration settings on `Configurable`.
 
     :param provider: The provider to store the settings with.
     :param name: The name to store the settings under.
     :param device: The Device to walk for SignalRWs.
+    :param only_config: If True, store only configuration settings.
     """
-    signals = yield from wait_for_awaitable(walk_config_signals(device))
+    if only_config:
+        signals = yield from wait_for_awaitable(walk_config_signals(device))
+    else:
+        signals = walk_rw_signals(device)
     named_values = yield from _get_values_of_signals(signals)
     yield from wait_for_awaitable(provider.store(name, named_values))
 
 
 @plan
 def retrieve_settings(
-    provider: SettingsProvider, name: str, device: Device
+    provider: SettingsProvider, name: str, device: Device, only_config: bool = False
 ) -> MsgGenerator[Settings]:
-    """Retrieve named Settings for a Device from a provider."""
+    """Retrieve named Settings for a Device from a provider.
+
+    If `only_config` is True, retrieve only configuration settings on `Configurable`.
+
+    :param provider: The provider to retrieve the settings from.
+    :param name: The name of the settings to retrieve.
+    :param device: The Device to retrieve the settings for.
+    :param only_config: If True, retrieve only configuration settings.
+    """
     named_values = yield from wait_for_awaitable(provider.retrieve(name))
-    signals = walk_rw_signals(device)
+    if only_config:
+        signals = yield from wait_for_awaitable(walk_config_signals(device))
+    else:
+        signals = walk_rw_signals(device)
     unknown_names = set(named_values) - set(signals)
     if unknown_names:
         raise NameError(f"Unknown signal names {sorted(unknown_names)}")

src/ophyd_async/testing/_one_of_everything.py

@@ -105,6 +105,9 @@ def __init__(self, name=""):
             self.ndarray = soft_signal_rw(np.ndarray, np.array(([1, 2, 3], [4, 5, 6])))
         super().__init__(name)
 
+    async def get_signal_values(self):
+        return await _get_signal_values(self)
+
 
 async def _get_signal_values(child: Device) -> dict[SignalRW, Any]:
     if isinstance(child, SignalRW):

tests/plan_stubs/test_settings.py

@@ -2,6 +2,7 @@
 from unittest.mock import call
 
 import bluesky.plan_stubs as bps
+import numpy as np
 import pytest
 import yaml
 
@@ -11,7 +12,6 @@
     apply_settings_if_different,
     get_current_settings,
     retrieve_settings,
-    store_config_settings,
     store_settings,
 )
 from ophyd_async.testing import (
@@ -31,6 +31,13 @@ async def parent_device() -> ParentOfEverythingDevice:
     return device
 
 
+@pytest.fixture
+async def every_parent_device() -> OneOfEverythingDevice:
+    device = OneOfEverythingDevice("parent")
+    await device.connect(mock=True)
+    return device
+
+
 async def test_get_current_settings(RE, parent_device: ParentOfEverythingDevice):
     expected_values = await parent_device.get_signal_values()
 
@@ -41,6 +48,25 @@ def my_plan():
     RE(my_plan())
 
 
+async def test_get_current_config_settings(
+    RE, every_parent_device: OneOfEverythingDevice
+):
+    expected_values = await every_parent_device.get_signal_values()
+
+    def my_plan():
+        current_settings = yield from get_current_settings(
+            every_parent_device, only_config=True
+        )
+        current_settings = dict(current_settings)
+        for key, value in current_settings.items():
+            if isinstance(value, np.ndarray):
+                assert np.array_equal(value, expected_values[key])
+            else:
+                assert value == expected_values[key]
+
+    RE(my_plan())
+
+
 async def test_store_settings(RE, parent_device: ParentOfEverythingDevice, tmp_path):
     provider = YamlSettingsProvider(tmp_path)
 
@@ -54,19 +80,18 @@ def my_plan():
 
 
 async def test_store_config_settings(
-    RE, parent_device: OneOfEverythingDevice, tmp_path
+    RE, every_parent_device: OneOfEverythingDevice, tmp_path
 ):
     provider = YamlSettingsProvider(tmp_path)
 
     def my_plan():
-        yield from store_config_settings(provider, "test_file", parent_device)
+        yield from store_settings(
+            provider, "test_file", every_parent_device, only_config=True
+        )
         with open(tmp_path / "test_file.yaml") as actual_file:
             actual_data = yaml.safe_load(actual_file)
-        with open(TEST_DATA / "test_yaml_save.yaml") as expected_file:
+        with open(TEST_DATA / "test_yaml_config_save.yaml") as expected_file:
             expected_data = yaml.safe_load(expected_file)
-        # Remove the keys that shouldn't be expected
-        expected_data.pop("_sig_rw", None)
-        expected_data.pop("sig_rw", None)
         assert actual_data == expected_data
 
     RE(my_plan())
@@ -108,6 +133,43 @@ def my_plan():
     RE(my_plan())
 
 
+async def test_retrieve_and_apply_config_settings(
+    RE, every_parent_device: OneOfEverythingDevice
+):
+    provider = YamlSettingsProvider(TEST_DATA)
+    expected_values = await every_parent_device.get_signal_values()
+    serialized_values = {}
+    # Override the table to be the serialized version so it compares equal
+    for sig, value in expected_values.items():
+        if isinstance(value, ExampleTable):
+            serialized_values[sig] = {
+                k: pytest.approx(v) for k, v in value.model_dump().items()
+            }
+        else:
+            serialized_values[sig] = pytest.approx(value)
+
+    def my_plan():
+        m = get_mock(every_parent_device)
+        settings = yield from retrieve_settings(
+            provider, "test_yaml_config_save", every_parent_device, only_config=True
+        )
+        assert dict(settings) == serialized_values
+        assert not m.mock_calls
+        yield from apply_settings(settings)
+        assert len(m.mock_calls) == 19
+        m.reset_mock()
+        assert not m.mock_calls
+        yield from apply_settings_if_different(settings, apply_settings)
+        assert not m.mock_calls
+        yield from bps.abs_set(every_parent_device.a_str, "foo", wait=True)
+        assert m.mock_calls == [call.a_str.put("foo", wait=True)]
+        m.reset_mock()
+        yield from apply_settings_if_different(settings, apply_settings)
+        assert m.mock_calls == [call.a_str.put("test_string", wait=True)]
+
+    RE(my_plan())
+
+
 async def test_ignored_settings(RE, parent_device: ParentOfEverythingDevice):
     def my_plan():
         m = get_mock(parent_device)

tests/test_data/test_yaml_config_save.yaml

@@ -0,0 +1,39 @@
+a_bool: true
+a_float: 1.234
+a_int: 1
+a_str: test_string
+enum: Bbb
+enuma:
+- Aaa
+- Ccc
+float32a: [-3.4028234663852886e+38, 3.4028234663852886e+38, 1.1754943508222875e-38,
+  1.401298464324817e-45, 0.0, 1.2339999675750732, 234000.0, 3.4499998946557753e-06]
+float64a: [-1.7976931348623157e+308, 1.7976931348623157e+308, 2.2250738585072014e-308,
+  5.0e-324, 0.0, 1.234, 234000.0, 3.45e-06]
+int16a: [-32768, 32767, 0, 1, 2, 3, 4]
+int32a: [-2147483648, 2147483647, 0, 1, 2, 3, 4]
+int64a: [-9223372036854775808, 9223372036854775807, 0, 1, 2, 3, 4]
+int8a: [-128, 127, 0, 1, 2, 3, 4]
+ndarray: [[1, 2, 3], [4, 5, 6]]
+stra:
+- one
+- two
+- three
+table:
+  a_bool: [false, false, true, true]
+  a_enum:
+  - Aaa
+  - Bbb
+  - Aaa
+  - Ccc
+  a_float: [1.8, 8.2, -6.0, 32.9887]
+  a_int: [1, 8, -9, 32]
+  a_str:
+  - Hello
+  - World
+  - Foo
+  - Bar
+uint16a: [0, 65535, 0, 1, 2, 3, 4]
+uint32a: [0, 4294967295, 0, 1, 2, 3, 4]
+uint64a: [0, 18446744073709551615, 0, 1, 2, 3, 4]
+uint8a: [0, 255, 0, 1, 2, 3, 4]