Reconcile I22 and I03 DCMs about dodal HOT 8 OPEN

@olliesilvester Somewhere, I can't find it, you outlined a nice compositional idea where you build a DCM object out of a bunch of modules, so we can cope with design differences etc.

@coretl RE: finding a better solution than relying on DAQ to tell you if the PVs are correct. I think you should be relying on CI rather than DAQ, I think any solution that relies on CI to tell DAQ and/or controls if the PVs are wrong is a huge improvement on the status quo. We were hoping #507 would provide at least some of that, but open to other ideas.

from dodal.

Hi @callumforrester

I have a broader question - is the expectation, with the current developments to dodal, ophyd, and blusky, etc, that PV names across Diamond will adopt common naming conventions for devices?

I'm not criticising, but just highlighting that there is currently no action plan on the controls side that I'm aware of to make this happen.

Including @coretl on this thread.

Thanks,

James

from dodal.

Hi James, currently we've been doing it ad-hoc. When we encounter a discrepancy between two beamlines, we try asking the relevant controls engineer to change the PV names. This is quite nice and agile, and it means we don't need a giant work package to rename all PVs across Diamond simultaneously, just that the level of standardisation that is needed for DAQ purposes should creep in over time. I'll defer to @coretl though as I know he has opinions.

from dodal.

I'm not a big fan of the current approach of "make som PV names, and rely on DAQ to tell us if they're right", so I'm keen to find a better solution.

One possibility discussed with @gilesknap was that we make some DLS record alias templates that live in a single support module that we can include for existing beamlines. This could point to dodal as the canonical description of what PV names are required for which type of device.

from dodal.

As of #652, we also have an i24 DCM. So the DCM base class now should contain common PV's between i03, i22 and i24. And I guess any PV's which only differ in syntax should be renamed to a common name

from dodal.

@olliesilvester Somewhere, I can't find it, you outlined a nice compositional idea where you build a DCM object out of a bunch of modules, so we can cope with design differences etc.

I think I found the email you were referring to
'we have decided to decompose our current bluesky plans into common building blocks for any MX beamline to use. A beamline can then construct its specific implementation by building up these smaller generic plans. These generic plans will be stored in the mx_bluesky repo. We will also look into using bluesky's 'prepare' command for device setup.'

So yeah, same idea works for devices

from dodal.

Hello, I'm not familiar with dodal and friends at all so the following suggestion is with full ignorance:

Could the code not hold variables which are populated with the particular naming implementation?

eg for a DCM:

base_pv = BL03I-MO-DCM-01
bragg_suffix = :BRAGG
first_xtal_roll = :ROLL
second_xtal_roll = None
first_crystal_pitch = :PITCH
second_crystal_pitch = None
etc

from dodal.

@JamesOHeaDLS Yes, good suggestion, I think this covers at least some of the use cases. It requires there to be a consistent superstate of all DCMs, i.e. something that could hold all of the variables without conflicts. That does make it harder to tell how the DCM works just from reading the code though. For instance, if I03's DCM has a second_crystal_pitch = Motor("BL03I-MO-ETC") and I22's has second_crystal_pitch = None, you have to look it up somewhere. And every piece of code that access it has to check if it's None or not.

It would be nicer if I22's DCM simply didn't have a second_crystal_pitch attribute in the first place. Then it's immediately obvious and the code that uses it gets simpler.

Maybe something more like this?

# i22.py
def dcm() -> DCMBase:
    return DCMBase(
        "BL22I-MO-DCM-01", 
        crystals=[
            DCMCrystalRollOnly("BL22I-MO-DCM-01:XTSAL1"),
            DCMCrystalRollAndPitch("BL22I-MO-DCM-01:XTSAL2"),
        ]
    )


# i03.py
def dcm() -> DCMBase:
    return DCMBase(
        "BL03I-MO-DCM-01", 
        crystals=[
            DCMCrystalRollAndPitch("BL03I-MO-DCM-01:XTSAL1"),
            DCMCrystalRollAndPitch("BL03I-MO-DCM-01:XTSAL2"),
        ]
    )

from dodal.