Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated handling of duplicate devices. #11

Merged
merged 2 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions src/ophydregistry/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,10 @@ def __new__wrapper(self, cls, *args, **kwargs):
return obj

def register(
self, component: ophydobj.OphydObject, labels: Optional[Sequence] = None
self,
component: ophydobj.OphydObject,
labels: Optional[Sequence] = None,
warn_duplicates=True,
) -> ophydobj.OphydObject:
"""Register a device, component, etc so that it can be retrieved later.

Expand All @@ -534,6 +537,9 @@ def register(
labels
Device labels to use for registration. If `None` (default),
the devices *_ophyd_labels_* parameter will be used.
warn_duplicates
If true, a warning will be issued if this device is
overwriting a previous device with the same name.

"""
# Determine how to register the device
Expand All @@ -552,7 +558,8 @@ def register(
# Ignore any instances with the same name as a previous component
# (Needed for some sub-components that are just readback
# values of the parent)
# Check that we're not adding a duplicate component name
# Check if we're adding a duplicate component name
is_duplicate = False
if name in self._objects_by_name.keys():
old_obj = self._objects_by_name[name]
is_readback = component in [
Expand All @@ -563,11 +570,15 @@ def register(
if is_readback:
msg = f"Ignoring readback with duplicate name: '{name}'"
log.debug(msg)
return component
else:
msg = f"Ignoring component with duplicate name: '{name}'"
log.warning(msg)
warnings.warn(msg)
return component
is_duplicate = True
if warn_duplicates:
log.warning(msg)
warnings.warn(msg)
else:
log.debug(msg)
# Register this component
log.debug(f"Registering {name}")
# Create a set for this device name if it doesn't exist
Expand Down Expand Up @@ -599,5 +610,5 @@ def register(
else:
sub_signals = []
for cpt_name, cpt in sub_signals:
self.register(cpt)
self.register(cpt, warn_duplicates=not is_duplicate and warn_duplicates)
return component
21 changes: 15 additions & 6 deletions src/ophydregistry/tests/test_instrument_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@

import pytest
from ophyd import Device, EpicsMotor, sim
from ophyd_async.core import Device as AsyncDevice, soft_signal_rw
from ophyd_async.core import Device as AsyncDevice
from ophyd_async.core import soft_signal_rw

from ophydregistry import ComponentNotFound, MultipleComponentsFound, Registry

Expand Down Expand Up @@ -146,11 +147,12 @@ def test_find_component(registry):

def test_find_async_children(registry):
"""Check that the child components of an async device get registered."""

class MyDevice(AsyncDevice):
def __init__(self, name):
self.signal = soft_signal_rw()
super().__init__(name=name)

device = MyDevice(name="m1")
registry.register(device)
assert registry.find(device.signal.name) is device.signal
Expand Down Expand Up @@ -390,21 +392,28 @@ def test_getitem(registry):

def test_duplicate_device(caplog, registry):
"""Check that a device doesn't get added twice."""
motor = sim.motor
# Two devices with the same name
motor1 = sim.instantiate_fake_device(EpicsMotor, prefix="", name="motor")
motor2 = sim.instantiate_fake_device(EpicsMotor, prefix="", name="motor")
# Set up logging so that we can know what
caplog.clear()
with caplog.at_level(logging.DEBUG):
registry.register(motor)
registry.register(motor1)
# Check for the edge case where motor and motor.user_readback have the same name
assert "Ignoring component with duplicate name" not in caplog.text
assert "Ignoring readback with duplicate name" in caplog.text
# Check that truly duplicated entries get a warning
caplog.clear()
with caplog.at_level(logging.WARNING):
with pytest.warns(UserWarning):
registry.register(motor)
registry.register(motor2)
# Check for the edge case where motor and motor.user_readback have the same name
assert "Ignoring component with duplicate name" in caplog.text
print(caplog.text)
# Check that the warning is only issued for the top-level device, not all its children
assert "motor_user_setpoint" not in caplog.text
# Check that the correct second device is the one that wound up in the registry
assert registry["motor"] is motor2


def test_delete_by_name(registry):
Expand Down Expand Up @@ -472,7 +481,7 @@ def test_weak_references():

"""
motor = sim.SynAxis(name="weak_motor", labels={"motors"})
registry = Registry(keep_references=False)
registry = Registry(keep_references=False, auto_register=False)
registry.register(motor)
# Can we still find the object if the test has a reference?
assert registry.find("weak_motor") is motor
Expand Down
Loading