Tests for AreaDetector drivers/plugins to insure attributes are regul…

…arly named (#490)

* changed some __init__ variable names and added new test for subclasses

* pv set in stone so change attribute name to match

* subsequent changes to attribute rename elsewhere

* changing other PR attribute names to keep regularly named test happy

src/ophyd_async/epics/adcore/__init__.py

@@ -1,4 +1,10 @@
-from ._core_io import ADBaseIO, DetectorState, NDFileHDFIO, NDPluginStatsIO
+from ._core_io import (
+    ADBaseIO,
+    DetectorState,
+    NDArrayBaseIO,
+    NDFileHDFIO,
+    NDPluginStatsIO,
+)
 from ._core_logic import (
     DEFAULT_GOOD_STATES,
     ADBaseShapeProvider,
@@ -21,6 +27,7 @@
 __all__ = [
     "ADBaseIO",
     "DetectorState",
+    "NDArrayBaseIO",
     "NDFileHDFIO",
     "NDPluginStatsIO",
     "DEFAULT_GOOD_STATES",

src/ophyd_async/epics/adcore/_core_io.py

@@ -22,18 +22,19 @@ def __init__(self, prefix: str, name: str = "") -> None:
         self.acquire = epics_signal_rw_rbv(bool, prefix + "Acquire")
         self.array_size_x = epics_signal_r(int, prefix + "ArraySizeX_RBV")
         self.array_size_y = epics_signal_r(int, prefix + "ArraySizeY_RBV")
-        self.data_type = epics_signal_r(ADBaseDataType, prefix + "NDDataType_RBV")
+        self.nd_data_type = epics_signal_r(ADBaseDataType, prefix + "NDDataType_RBV")
         self.array_counter = epics_signal_rw_rbv(int, prefix + "ArrayCounter")
         # There is no _RBV for this one
         self.wait_for_plugins = epics_signal_rw(bool, prefix + "WaitForPlugins")
-
         super().__init__(name=name)
 
 
 class NDPluginBaseIO(NDArrayBaseIO):
     def __init__(self, prefix: str, name: str = "") -> None:
         self.nd_array_port = epics_signal_rw_rbv(str, prefix + "NDArrayPort")
-        self.enable_callback = epics_signal_rw_rbv(Callback, prefix + "EnableCallbacks")
+        self.enable_callbacks = epics_signal_rw_rbv(
+            Callback, prefix + "EnableCallbacks"
+        )
         self.nd_array_address = epics_signal_rw_rbv(int, prefix + "NDArrayAddress")
         self.array_size0 = epics_signal_r(int, prefix + "ArraySize0_RBV")
         self.array_size1 = epics_signal_r(int, prefix + "ArraySize1_RBV")
@@ -49,20 +50,20 @@ class NDPluginStatsIO(NDPluginBaseIO):
     """
 
     def __init__(self, prefix: str, name: str = "") -> None:
-        self.total = epics_signal_rw(float, prefix + "TotalArray")
-        self.statistics = epics_signal_rw(bool, prefix + "ComputeStatistics")
-        self.statistics_background_width = epics_signal_rw(int, prefix + "BgdWidth")
-        self.centroid = epics_signal_rw(bool, prefix + "ComputeCentroid")
+        self.total_array = epics_signal_rw(float, prefix + "TotalArray")
+        self.compute_statistics = epics_signal_rw(bool, prefix + "ComputeStatistics")
+        self.bgd_width = epics_signal_rw(int, prefix + "BgdWidth")
+        self.compute_centroid = epics_signal_rw(bool, prefix + "ComputeCentroid")
         self.centroid_threshold = epics_signal_rw(float, prefix + "CentroidThreshold")
-        self.profiles = epics_signal_rw(bool, prefix + "ComputeProfiles")
+        self.compute_profiles = epics_signal_rw(bool, prefix + "ComputeProfiles")
         self.profile_size_x = epics_signal_rw(int, prefix + "ProfileSizeX")
-        self.profile_cursor_x = epics_signal_rw(int, prefix + "CursorX")
+        self.cursor_x = epics_signal_rw(int, prefix + "CursorX")
         self.profile_size_y = epics_signal_rw(int, prefix + "ProfileSizeY")
-        self.profile_cursor_y = epics_signal_rw(int, prefix + "CursorY")
-        self.histogram = epics_signal_rw(bool, prefix + "ComputeHistogram")
-        self.histogram_max = epics_signal_rw(float, prefix + "HistMax")
-        self.histogram_min = epics_signal_rw(float, prefix + "HistMin")
-        self.histogram_size = epics_signal_rw(int, prefix + "HistSize")
+        self.cursor_y = epics_signal_rw(int, prefix + "CursorY")
+        self.compute_histogram = epics_signal_rw(bool, prefix + "ComputeHistogram")
+        self.hist_max = epics_signal_rw(float, prefix + "HistMax")
+        self.hist_min = epics_signal_rw(float, prefix + "HistMin")
+        self.hist_size = epics_signal_rw(int, prefix + "HistSize")
         super().__init__(prefix, name)
 
 
@@ -132,5 +133,5 @@ def __init__(self, prefix: str, name="") -> None:
         self.xml_file_name = epics_signal_rw_rbv(str, prefix + "XMLFileName")
         self.array_size0 = epics_signal_r(int, prefix + "ArraySize0")
         self.array_size1 = epics_signal_r(int, prefix + "ArraySize1")
-        self.create_dir_depth = epics_signal_rw(int, prefix + "CreateDirectory")
+        self.create_directory = epics_signal_rw(int, prefix + "CreateDirectory")
         super().__init__(prefix, name)

src/ophyd_async/epics/adcore/_core_logic.py

@@ -26,7 +26,7 @@ async def __call__(self) -> tuple:
         shape = await asyncio.gather(
             self._driver.array_size_y.get_value(),
             self._driver.array_size_x.get_value(),
-            self._driver.data_type.get_value(),
+            self._driver.nd_data_type.get_value(),
         )
         return shape
 

src/ophyd_async/epics/adcore/_hdf_writer.py

@@ -60,7 +60,7 @@ async def open(self, multiplier: int = 1) -> Dict[str, DataKey]:
             self.hdf.file_path.set(file_path),
             self.hdf.file_name.set(info.filename),
             self.hdf.file_template.set("%s/%s.h5"),
-            self.hdf.create_dir_depth.set(info.create_dir_depth),
+            self.hdf.create_directory.set(info.create_dir_depth),
             self.hdf.file_write_mode.set(FileWriteMode.stream),
             # Never use custom xml layout file but use the one defined
             # in the source code file NDFileHDF5LayoutXML.cpp

src/ophyd_async/epics/adkinetix/_kinetix_io.py

@@ -25,5 +25,7 @@ def __init__(self, prefix: str, name: str = "") -> None:
         self.trigger_mode = epics_signal_rw_rbv(
             KinetixTriggerMode, prefix + "TriggerMode"
         )
-        self.mode = epics_signal_rw_rbv(KinetixReadoutMode, prefix + "ReadoutPortIdx")
+        self.readout_port_idx = epics_signal_rw_rbv(
+            KinetixReadoutMode, prefix + "ReadoutPortIdx"
+        )
         super().__init__(prefix, name)

src/ophyd_async/epics/adpilatus/_pilatus_controller.py

@@ -54,7 +54,7 @@ async def arm(
         # is actually ready. Should wait for that too or we risk dropping
         # a frame
         await wait_for_value(
-            self._drv.armed_for_triggers,
+            self._drv.armed,
             True,
             timeout=DEFAULT_TIMEOUT,
         )

src/ophyd_async/epics/adpilatus/_pilatus_io.py

@@ -19,5 +19,5 @@ def __init__(self, prefix: str, name: str = "") -> None:
         self.trigger_mode = epics_signal_rw_rbv(
             PilatusTriggerMode, prefix + "TriggerMode"
         )
-        self.armed_for_triggers = epics_signal_r(bool, prefix + "Armed")
+        self.armed = epics_signal_r(bool, prefix + "Armed")
         super().__init__(prefix, name)

src/ophyd_async/epics/advimba/_vimba_controller.py

@@ -39,7 +39,7 @@ async def arm(
     ) -> AsyncStatus:
         await asyncio.gather(
             self._drv.trigger_mode.set(TRIGGER_MODE[trigger]),
-            self._drv.expose_mode.set(EXPOSE_OUT_MODE[trigger]),
+            self._drv.exposure_mode.set(EXPOSE_OUT_MODE[trigger]),
             self._drv.num_images.set(num),
             self._drv.image_mode.set(adcore.ImageMode.multiple),
         )
@@ -49,9 +49,9 @@ async def arm(
         ]:
             await self._drv.acquire_time.set(exposure)
         if trigger != DetectorTrigger.internal:
-            self._drv.trig_source.set(VimbaTriggerSource.line1)
+            self._drv.trigger_source.set(VimbaTriggerSource.line1)
         else:
-            self._drv.trig_source.set(VimbaTriggerSource.freerun)
+            self._drv.trigger_source.set(VimbaTriggerSource.freerun)
         return await adcore.start_acquiring_driver_and_ensure_status(self._drv)
 
     async def disarm(self):

src/ophyd_async/epics/advimba/_vimba_io.py

@@ -50,15 +50,17 @@ class VimbaDriverIO(adcore.ADBaseIO):
 
     def __init__(self, prefix: str, name: str = "") -> None:
         # self.pixel_format = epics_signal_rw_rbv(PixelFormat, prefix + "PixelFormat")
-        self.convert_format = epics_signal_rw_rbv(
+        self.convert_pixel_format = epics_signal_rw_rbv(
             VimbaConvertFormat, prefix + "ConvertPixelFormat"
         )  # Pixel format of data outputted to AD
-        self.trig_source = epics_signal_rw_rbv(
+        self.trigger_source = epics_signal_rw_rbv(
             VimbaTriggerSource, prefix + "TriggerSource"
         )
         self.trigger_mode = epics_signal_rw_rbv(VimbaOnOff, prefix + "TriggerMode")
-        self.overlap = epics_signal_rw_rbv(VimbaOverlap, prefix + "TriggerOverlap")
-        self.expose_mode = epics_signal_rw_rbv(
+        self.trigger_overlap = epics_signal_rw_rbv(
+            VimbaOverlap, prefix + "TriggerOverlap"
+        )
+        self.exposure_mode = epics_signal_rw_rbv(
             VimbaExposeOutMode, prefix + "ExposureMode"
         )
         super().__init__(prefix, name)

tests/epics/adpilatus/test_pilatus.py

@@ -60,7 +60,7 @@ async def test_trigger_mode_set(
     expected_trigger_mode: adpilatus.PilatusTriggerMode,
 ):
     async def trigger_and_complete():
-        set_mock_value(test_adpilatus.drv.armed_for_triggers, True)
+        set_mock_value(test_adpilatus.drv.armed, True)
         status = await test_adpilatus.controller.arm(
             num=1,
             trigger=detector_trigger,
@@ -135,7 +135,7 @@ async def dummy_open(multiplier: int = 0):
         return {}
 
     test_adpilatus.writer.open = dummy_open
-    set_mock_value(test_adpilatus.drv.armed_for_triggers, True)
+    set_mock_value(test_adpilatus.drv.armed, True)
     await test_adpilatus.prepare(
         TriggerInfo(
             number=1, trigger=DetectorTrigger.internal, deadtime=1.0, livetime=1.0

tests/epics/adpilatus/test_pilatus_controller.py

@@ -27,7 +27,7 @@ async def test_pilatus_controller(
     pilatus: adpilatus.PilatusController,
     pilatus_driver: adpilatus.PilatusDriverIO,
 ):
-    set_mock_value(pilatus_driver.armed_for_triggers, True)
+    set_mock_value(pilatus_driver.armed, True)
     status = await pilatus.arm(num=1, trigger=DetectorTrigger.constant_gate)
     await status
 

tests/epics/advimba/test_vimba.py

@@ -30,39 +30,39 @@ async def test_get_deadtime(
 
 
 async def test_arming_trig_modes(test_advimba: advimba.VimbaDetector):
-    set_mock_value(test_advimba.drv.trig_source, "Freerun")
+    set_mock_value(test_advimba.drv.trigger_source, "Freerun")
     set_mock_value(test_advimba.drv.trigger_mode, "Off")
-    set_mock_value(test_advimba.drv.expose_mode, "Timed")
+    set_mock_value(test_advimba.drv.exposure_mode, "Timed")
 
     async def setup_trigger_mode(trig_mode: DetectorTrigger):
         await test_advimba.controller.arm(num=1, trigger=trig_mode)
         # Prevent timeouts
         set_mock_value(test_advimba.drv.acquire, True)
 
     # Default TriggerSource
-    assert (await test_advimba.drv.trig_source.get_value()) == "Freerun"
+    assert (await test_advimba.drv.trigger_source.get_value()) == "Freerun"
     assert (await test_advimba.drv.trigger_mode.get_value()) == "Off"
-    assert (await test_advimba.drv.expose_mode.get_value()) == "Timed"
+    assert (await test_advimba.drv.exposure_mode.get_value()) == "Timed"
 
     await setup_trigger_mode(DetectorTrigger.edge_trigger)
-    assert (await test_advimba.drv.trig_source.get_value()) == "Line1"
+    assert (await test_advimba.drv.trigger_source.get_value()) == "Line1"
     assert (await test_advimba.drv.trigger_mode.get_value()) == "On"
-    assert (await test_advimba.drv.expose_mode.get_value()) == "Timed"
+    assert (await test_advimba.drv.exposure_mode.get_value()) == "Timed"
 
     await setup_trigger_mode(DetectorTrigger.constant_gate)
-    assert (await test_advimba.drv.trig_source.get_value()) == "Line1"
+    assert (await test_advimba.drv.trigger_source.get_value()) == "Line1"
     assert (await test_advimba.drv.trigger_mode.get_value()) == "On"
-    assert (await test_advimba.drv.expose_mode.get_value()) == "TriggerWidth"
+    assert (await test_advimba.drv.exposure_mode.get_value()) == "TriggerWidth"
 
     await setup_trigger_mode(DetectorTrigger.internal)
-    assert (await test_advimba.drv.trig_source.get_value()) == "Freerun"
+    assert (await test_advimba.drv.trigger_source.get_value()) == "Freerun"
     assert (await test_advimba.drv.trigger_mode.get_value()) == "Off"
-    assert (await test_advimba.drv.expose_mode.get_value()) == "Timed"
+    assert (await test_advimba.drv.exposure_mode.get_value()) == "Timed"
 
     await setup_trigger_mode(DetectorTrigger.variable_gate)
-    assert (await test_advimba.drv.trig_source.get_value()) == "Line1"
+    assert (await test_advimba.drv.trigger_source.get_value()) == "Line1"
     assert (await test_advimba.drv.trigger_mode.get_value()) == "On"
-    assert (await test_advimba.drv.expose_mode.get_value()) == "TriggerWidth"
+    assert (await test_advimba.drv.exposure_mode.get_value()) == "TriggerWidth"
 
 
 async def test_hints_from_hdf_writer(test_advimba: advimba.VimbaDetector):

tests/epics/test_areadetector_subclass_naming.py

@@ -0,0 +1,32 @@
+import inflection
+import pytest
+
+from ophyd_async.core import Device, Signal
+
+# Need to import them all so the subclass walking gets all subclasses
+# If we forget then the full test suite will find the subclasses, but
+# running just this test will only get the ones at the top of this file
+from ophyd_async.epics import (
+    adaravis,  # noqa
+    adcore,
+    adkinetix,  # noqa
+    adpilatus,  # noqa
+    adsimdetector,  # noqa
+    advimba,  # noqa
+)
+
+
+def get_rec_subclasses(cls: type):
+    yield cls
+    for subcls in cls.__subclasses__():
+        yield from get_rec_subclasses(subcls)
+
+
+@pytest.mark.parametrize("cls", list(get_rec_subclasses(adcore.NDArrayBaseIO)))
+async def test_regularly_named_attributes(cls: Device):
+    io = cls("")
+    for name, signal in io.children():
+        assert isinstance(signal, Signal)
+        # Strip off the ca:// prefix and an _RBV suffix
+        pv = signal.source.split("://")[-1].split("_RBV")[0]
+        assert inflection.underscore(pv) == name