bluesky / ophyd-async

Hardware abstraction for bluesky written using asyncio
https://blueskyproject.io/ophyd-async
BSD 3-Clause "New" or "Revised" License
7 stars 21 forks source link

Clearer documentation on epics demo #193

Open callumforrester opened 2 months ago

callumforrester commented 2 months ago

Currently, the epics demo module includes facility to start a Python IOC with all of the PVs that the demo devices point at. It can be tested and played with via ipython -i docs/user/examples/epics_demo.py. There is an expectation that a developer makes sure the Python soft IOC actually contains all the needed PVs when they add new demo objects.

This process needs to be documented somewhere and possibly made easier to test. Some suggestions:

The one thing we can't automate is instantiation of the demo devices, since that's manually embedded inside with DeviceCollector in docs/user/examples/epics_demo.py, unless we can also think of a way to do that. It would be very nice to have the CI automatically flag up if someone has broken the demo or left it incomplete.

callumforrester commented 2 months ago

Teach me to read before I write, it's already documented in https://blueskyproject.io/ophyd-async/main/user/tutorials/using-existing-devices.html

However I think those docs are slightly out of date, it seems to be easier to start the demo than the tutorial suggests now. The docs build also runs the demo file so should at least break if it breaks. Things that I still think would be nice:

callumforrester commented 2 months ago

@coretl Possibly a separate issue but should we also make the tests for the demo run against the actual IOC? Since it is already packaged the cost is low

coretl commented 2 months ago

However I think those docs are slightly out of date, it seems to be easier to start the demo than the tutorial suggests now.

What do you do now that is easier than in the docs?

* Some way to test that all devices in `ophyd-async.epics.demo` are instantiated

Yes, this strays into dodal territory, which is why I skirted round the issue...

We can either:

The first is closer to what we do in production, but either needs manual checking (like now), or an extra compound Demo device.

The second is neat, but people might ask whether they have to decorate every Device with @demo_epics_database("myrecords.template")...

callumforrester commented 2 months ago

What do you do now that is easier than in the docs?

The docs talk you through making your own demo file, but we ship ophyd-async with one.

For keeping in sync, how about this: we have a demo file that says:

with DeviceCollector(...):
    sensor = Sensor(...)
    stage = Stage(...)
    ...

We then have a test that runs it and makes sure that every class in epics.demo is used at some point, doesn't have to be at the top level. This test would run up the IOC and make sure all devices connect to it as a side effect.