ophyd
ophyd copied to clipboard
Ophyd v2 core
Add:
- Device baseclass
- Signal baseclass
- EpicsSignal with CA and Sim backends
- Demo EPICS Device and IOC to drive it
- Magics for some simple plan stubs
- Docs and a tutorial
- Reasonable test coverage
What I'd like reviewed most is the docs. Downlowd and unzip https://github.com/bluesky/ophyd/suites/8898144421/artifacts/407113849 then point a browser at it and look at the User Guide v2 tutorials and how-tos.
For some reason GitHub won't let me ask @tacaswell @whs92 and @callumforrester for a review...
A couple of tests that worked on 3.9 but don't work on 3.8, will look at this next week...
Since Channel Access PVs remain part of EPICS 7, change EPICS V3 PVs
to EPICS Channel Access (V3) PVs
I'll give the docs a readthrough, first impressions are very positive
Since Channel Access PVs remain part of EPICS 7, change EPICS V3 PVs to EPICS Channel Access (V3) PVs
The top half of the main index that includes this text is actually the existing README of this repo- maybe the README needs a look-over. Not sure if that's in scope for v2 docs.
I agree with the general flow of the documentation shown here. Very readable.
We're also being asked to comment on code?
I'm very concerned that the complexity shown in the example class Sensor(Configurable, Readable, Device):
will add a barrier to further adoption of Bluesky at APS. How much of this can be hidden behind a much simpler interface, such as without type declarations?
Since Channel Access PVs remain part of EPICS 7, change EPICS V3 PVs to EPICS Channel Access (V3) PVs
The top half of the main index that includes this text is actually the existing README of this repo- maybe the README needs a look-over. Not sure if that's in scope for v2 docs.
We're planning PVA support too, so I'd prefer to exclude this README from the scope of the review and come back to it when that's done
We're also being asked to comment on code?
Yes please, I was aware that people had limited time, so I wanted people to focus on docs and the example user code that was being presented in the docs rather than the "behind-the-scenes" implementation of EpicsSignal for instance, but feel free to comment on anything in the PR.
I'm very concerned that the complexity shown in the example
class Sensor(Configurable, Readable, Device):
will add a barrier to further adoption of Bluesky at APS. How much of this can be hidden behind a much simpler interface, such as without type declarations?
I've made the Sensor
demo Device use the same baseclass as the Mover
device that gets rid of the boilerplate read
, describe
etc. methods. I didn't use it originally as I wanted to build up to it, but I can work the other way (start with the simple class using the helper mix-in, then show how you can implement those methods yourself in a more advanced How-To).
@prjemian please can you comment on the specific lines of ophyd/v2/epicsdemo/__init__.py
that you'd like to hide away? Is it the connect()
method? Or the explicit types in the EpicsSignal
constructor? Or was it the Readable
baseclass that has now gone?
Converting to Draft until I've actioned all the comments...
I'd like to discuss these changes in the community call on Thursday, can people make it to that?
@ZLLentz and @prjemian I've simplified the process of making a Device
by introducing a SimpleDevice
helper class that does a lot of the boilerplate code for you. I also discussed magics with @danielballan and @tacaswell and I think we need a wider discussion on this. Dan was going to try and find a good time. In the spirit of "not inventing a third thing for users to learn", I've converted the tutorial to only use plans through the RunEngine (with a few less characters courtesy of bluesky.utils.register_transform
)
Please could you have another look at the tutorial and how-to on the docs and see if this is any better? https://github.com/bluesky/ophyd/suites/8968599012/artifacts/412343865