DiamondLightSource / dodal

Ophyd devices and other utils that could be used across DLS beamlines
Apache License 2.0
2 stars 8 forks source link

Raise an error when wrong device name #735

Open stan-dot opened 1 month ago

stan-dot commented 1 month ago

Summarized and abridged comment from a different issue

# Factory name is detector_1, device name is detector_1
def detector_1() -> MyDetector:
    return MyDetector(name="detector_1")

# Factory name is detector_2, device name is pilatus_2
def detector_2() -> MyDetector:
    return MyDetector(name="pilatus_2")

There is no guarantee that the function name and device name will match up. The question is, if you want to use the second device in a plan, should you pass "detector_2" or "pilatus_2" as a parameter? We should consider the case of nested devices too. Do we want detector_2.exposure, pilatus_2.exposure or pilatus_2-exposure. The latter is the name automatically assigned to the signal on device connection. Raise an error, either in blueapi or dodal, if the names do not match up. Force them to be the same. This would be potentially annoying when writing devices but would make debugging easier.

It just includes hidden coupling, the knowledge is exposed to the user through an exception.

_Originally posted by @callumforrester in https://github.com/DiamondLightSource/dodal/pull/597#discussion_r1703958097_

The current behavior is that devices are keyed by name and it is ignored in blueapi.

callumforrester commented 1 month ago

Should we do this and/or automagically set the device name to the function name by default? I think @coretl and @DominicOram are now in favour of that.

DominicOram commented 1 month ago

Yes, I think setting them the same automatically is good.