Add a simple version to fill the unique needs of slit4

[stan-dot]

fixes #587

[DiamondJoseph]

Let's try it out on the beamline tomorrow, the signals are definitely currently wrong.

[callumforrester]

For signal correctness, try running the system tests on the Diamond network: https://github.com/DiamondLightSource/dodal?tab=readme-ov-file#testing-connectivity

[stan-dot]

connectivity check fails for this device AND panda1

[stan-dot]

will test on 30.07

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.44%. Comparing base (46132f0) to head (291050c).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #672   +/-   ##
=======================================
  Coverage   94.43%   94.44%           
=======================================
  Files         113      113           
  Lines        4581     4588    +7     
=======================================
+ Hits         4326     4333    +7     
  Misses        255      255           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[stan-dot]

not sure what is the desired behavior, which PVs do we want to expose?

the EDM screen shows:
X, Y, HDSO, VDSO, of the 2 latter each has a raw and 'real' variant.
There is an abscence of words known from other slits, such as x centre, or gap

@DiamondJoseph please advise

[DiamondJoseph]

Let's ask the beamline scientist tomorrow, but I assume that we want all of them.
Let's also keep them as Motors, same as the existing Slits, if they're not being moved that's fine but let's keep the option open

[stan-dot]

still not right, need to talk to the scientists

[stan-dot]

@DiamondJoseph this fits the Hyperion model of 'the changes fit our model of the beamline', and it passes the tests, can we merge this now?

[stan-dot]

dodal connect i22
Attempting connection to i22 (using dodal.beamlines.i22)
16 devices connected :
        oav
        fswitch
        slits_6
        slits_5
        slits_4
        slits_3
        slits_2
        slits_1
        undulator
        hfm
        vfm
        it
        i0
        waxs
        synchrotron
        saxs

this was successful, ready to merge

[DominicOram]

Should: As someone not involved in i22, it's not clear what dso means here. Could we have a comment or rename the variable to something more human readable

[stan-dot]

I do not either. that is how it was defined in EPICS. I did inquire on 07.08.2024 and received no response since.

[DominicOram]

Ok, well I don't feel comfortable us having code in dodal where no-one understands what it means. Could you please chase up? Have you asked Scientists+Controls?

[stan-dot]

I followed up again

[stan-dot]

fixed!

@DominicOram

[DominicOram]

So they stand for defining_slit_opening? Can we just use gap as that's what we use everywhere else

[DiamondJoseph]

I'm also assuming that "defining slit opening" -> "slit width" -> "gap"?

[DominicOram]

Yes, I think they are all equivalent, we use gap everywhere else, it's succinct (unlike defining_slit_opening) and makes more sense for x and y (unlike width which sort of implies just x). So I think we should stick to gap at the ophyd level

[stan-dot]

Ok I change it into x_gap etc, and moving the 'dso' into the class docstring

[DiamondJoseph]

class Slits(StandardReadable):
    def __init__(self, prefix: str, name: str = "") -> None:
        with self.add_children_as_readables():
            self.x_gap = Motor(prefix + "X:SIZE")
            self.y_gap = Motor(prefix + "Y:SIZE")
            self.x_centre = Motor(prefix + "X:CENTRE")
            self.y_centre = Motor(prefix + "Y:CENTRE")
        super().__init__(name)


class SpecialSlits(StandardReadable):
    def __init__(self, prefix: str, name: str = "") -> None:
        with self.add_children_as_readables():
            self.x = Motor(prefix + "X")
            self.y = Motor(prefix + "Y")
            self.x_gap = Motor(prefix + "HDSO")
            self.y_gap = Motor(prefix + "VDSO")
        super().__init__(name)

At which point we've got some very similar looking classes. It'd be nice if we could a. confirm that SpecialSlits.x is defining x centre, not e.g. x_min, and b. treat them all as some sort of generic slit with x_centre, x_gap, y_centre, y_gap

[stan-dot]

yes, it is centre. we can make both classes have the same properties but they do need to have different logic.

@DominicOram and @DiamondJoseph, how would you like this to be implemented?
the ways I see

• (default) keep 2 classes
• just 1 class with an override option for the channel names - that done either with a ternary inline logic or a big logic branch inside add_children_as_readables
• some interface that both classes implement that defines the same 4 properties

[DiamondJoseph]

* just 1 class with an override option for the channel names (in the constructor with the 5/6 case as the default and the 1/6 case as the special override)

would be my preference.

[DominicOram]

My preference would be:

1. Create an alias at the EPICs layer so that the PVs are the same - we have done this for a set on i03
2. If we're short on time ask for the alias and do the override of channel names

[stan-dot]

closing as this will be solved at the epics layer