Make name separator configurable (#675)

* Make name separator configurable

Remove the stripping of underscore while we're at it, this
can be done by overriding set_name if required

* Make Mover use the same name

* Fix test

* Fix review comments

src/ophyd_async/core/_device.py

@@ -71,13 +71,16 @@ class Device(HasName, Connectable):
     _connect_task: asyncio.Task | None = None
     # The mock if we have connected in mock mode
     _mock: LazyMock | None = None
+    # The separator to use when making child names
+    _child_name_separator: str = "-"
 
     def __init__(
         self, name: str = "", connector: DeviceConnector | None = None
     ) -> None:
         self._connector = connector or DeviceConnector()
         self._connector.create_children_from_annotations(self)
-        self.set_name(name)
+        if name:
+            self.set_name(name)
 
     @property
     def name(self) -> str:
@@ -97,21 +100,30 @@ def log(self) -> LoggerAdapter:
             getLogger("ophyd_async.devices"), {"ophyd_async_device_name": self.name}
         )
 
-    def set_name(self, name: str):
+    def set_name(self, name: str, *, child_name_separator: str | None = None) -> None:
         """Set ``self.name=name`` and each ``self.child.name=name+"-child"``.
 
         Parameters
         ----------
         name:
             New name to set
+        child_name_separator:
+            Use this as a separator instead of "-". Use "_" instead to make the same
+            names as the equivalent ophyd sync device.
         """
         self._name = name
+        if child_name_separator:
+            self._child_name_separator = child_name_separator
         # Ensure logger is recreated after a name change
         if "log" in self.__dict__:
             del self.log
-        for child_name, child in self.children():
-            child_name = f"{self.name}-{child_name.strip('_')}" if self.name else ""
-            child.set_name(child_name)
+        for attr_name, child in self.children():
+            child_name = (
+                f"{self.name}{self._child_name_separator}{attr_name}"
+                if self.name
+                else ""
+            )
+            child.set_name(child_name, child_name_separator=self._child_name_separator)
 
     def __setattr__(self, name: str, value: Any) -> None:
         # Bear in mind that this function is called *a lot*, so
@@ -147,6 +159,10 @@ async def connect(
         timeout:
             Time to wait before failing with a TimeoutError.
         """
+        assert hasattr(self, "_connector"), (
+            f"{self}: doesn't have attribute `_connector`,"
+            " did you call `super().__init__` in your `__init__` method?"
+        )
         if mock:
             # Always connect in mock mode serially
             if isinstance(mock, LazyMock):
@@ -247,6 +263,8 @@ class DeviceCollector:
     set_name:
         If True, call ``device.set_name(variable_name)`` on all collected
         Devices
+    child_name_separator:
+        Use this as a separator if we call ``set_name``.
     connect:
         If True, call ``device.connect(mock)`` in parallel on all
         collected Devices
@@ -271,11 +289,13 @@ class DeviceCollector:
     def __init__(
         self,
         set_name=True,
+        child_name_separator: str = "-",
         connect=True,
         mock=False,
         timeout: float = 10.0,
     ):
         self._set_name = set_name
+        self._child_name_separator = child_name_separator
         self._connect = connect
         self._mock = mock
         self._timeout = timeout
@@ -311,7 +331,7 @@ async def _on_exit(self) -> None:
         for name, obj in self._objects_on_exit.items():
             if name not in self._names_on_enter and isinstance(obj, Device):
                 if self._set_name and not obj.name:
-                    obj.set_name(name)
+                    obj.set_name(name, child_name_separator=self._child_name_separator)
                 if self._connect:
                     connect_coroutines[name] = obj.connect(
                         self._mock, timeout=self._timeout

src/ophyd_async/epics/demo/_mover.py

@@ -37,8 +37,8 @@ def __init__(self, prefix: str, name="") -> None:
 
         super().__init__(name=name)
 
-    def set_name(self, name: str):
-        super().set_name(name)
+    def set_name(self, name: str, *, child_name_separator: str | None = None) -> None:
+        super().set_name(name, child_name_separator=child_name_separator)
         # Readback should be named the same as its parent in read()
         self.readback.set_name(name)
 

src/ophyd_async/epics/motor.py

@@ -91,8 +91,8 @@ def __init__(self, prefix: str, name="") -> None:
 
         super().__init__(name=name)
 
-    def set_name(self, name: str):
-        super().set_name(name)
+    def set_name(self, name: str, *, child_name_separator: str | None = None) -> None:
+        super().set_name(name, child_name_separator=child_name_separator)
         # Readback should be named the same as its parent in read()
         self.user_readback.set_name(name)
 

tests/core/test_device.py

@@ -102,7 +102,7 @@ async def test_children_of_device_have_set_names_and_get_connected(
 ):
     assert parent.name == "parent"
     assert parent.child1.name == "parent-child1"
-    assert parent._child2.name == "parent-child2"
+    assert parent._child2.name == "parent-_child2"
     assert parent.dict_with_children.name == "parent-dict_with_children"
     assert parent.dict_with_children[123].name == "parent-dict_with_children-123"
 
@@ -112,6 +112,20 @@ async def test_children_of_device_have_set_names_and_get_connected(
     assert parent.dict_with_children[123].connected
 
 
+async def test_children_of_device_with_different_separator(
+    parent: DummyDeviceGroup,
+):
+    for separator in ("_", None):
+        # The second time round, check that it doesn't change name if
+        # we pass None, as this is what PviConnector does
+        parent.set_name("parent", child_name_separator=separator)
+        assert parent.name == "parent"
+        assert parent.child1.name == "parent_child1"
+        assert parent._child2.name == "parent__child2"
+        assert parent.dict_with_children.name == "parent_dict_with_children"
+        assert parent.dict_with_children[123].name == "parent_dict_with_children_123"
+
+
 async def test_device_with_device_collector():
     async with DeviceCollector(mock=True):
         parent = DummyDeviceGroup("parent")
@@ -120,7 +134,7 @@ async def test_device_with_device_collector():
     assert parent.parent is None
     assert parent.child1.name == "parent-child1"
     assert parent.child1.parent == parent
-    assert parent._child2.name == "parent-child2"
+    assert parent._child2.name == "parent-_child2"
     assert parent._child2.parent == parent
     assert parent.dict_with_children.name == "parent-dict_with_children"
     assert parent.dict_with_children.parent == parent

tests/epics/demo/test_demo.py

@@ -315,7 +315,7 @@ async def test_assembly_renaming() -> None:
     thing.set_name("foo")
     assert thing.x.name == "foo-x"
     assert thing.x.velocity.name == "foo-x-velocity"
-    assert thing.x.stop_.name == "foo-x-stop"
+    assert thing.x.stop_.name == "foo-x-stop_"
 
 
 async def test_dynamic_sensor_group_disconnected():