-
Notifications
You must be signed in to change notification settings - Fork 1
Add a zebra device #11
base: main
Are you sure you want to change the base?
Conversation
@coretl how would I go about setting a connection timeout for a PV with v2? This is something Dom's zebra device currently uses, so would be good to maybe expose this in the class. |
The timeout problem is being investigated/expanded upon: bluesky/ophyd#1101 Tests are failing, going to have a look as to why. I thought I made my test devices have sim=True so that it wouldn't matter if they were actually connected to real underlying PVs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments, mostly looks good
src/ophyd_epics_devices/zebra.py
Outdated
class Zebra(StandardReadable): | ||
def __init__(self, prefix: str, name: str = ""): | ||
self.pc = PositionCompare("") | ||
self.output = ZebraOutputPanel("") | ||
self.logic_gates = LogicGateConfigurer("") | ||
self.sys = System("") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I have a preference for mirroring the structure of the blocks on the Zebra opi screens, but I think this should be a wider discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy for it to mirror the OPI more closely, I think it makes things easier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will have a crack at doing this...
I'm assuming opi screens == Epics screens when you open up a zebra
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. There is a heirarchy implied in both the screen layout and the record names. E.g.
PREFIX:PC_ARM
could map tozebra.pc.arm: SignalX
PREFIX:PC_GATE_START
could map tozebra.pc.gate.start: SignalRW[float]
PREFIX:DIV2_DIV
could map tozebra.div[2].div: SignalRW[int]
There are also bitmap registers that correspond to many blocks:
- Div1 "Trigger on" is
PREFIX:POLARITY:B8
which could map tozebra.div[1].trigger_on: SignalRW[bool]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
structure now (hopefully) mirrors epics screens better
I agree that users will find it more intuitive if the ophyd support mirrors
the GUI they see.
…On Tue, Mar 21, 2023, 7:58 AM Dominic Oram ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/ophyd_epics_devices/zebra.py
<#11 (comment)>
:
> +class Zebra(StandardReadable):
+ def __init__(self, prefix: str, name: str = ""):
+ self.pc = PositionCompare("")
+ self.output = ZebraOutputPanel("")
+ self.logic_gates = LogicGateConfigurer("")
+ self.sys = System("")
I'm happy for it to mirror the OPI more closely, I think it makes things
easier
—
Reply to this email directly, view it on GitHub
<#11 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AARMUMFDY3HHIUIZKPMJBODW5GQYFANCNFSM6AAAAAAVJW2HTQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Another source of PVs or fields is the autosave requests files in the
`somethingApp/Db/` directory. In addition to the screen description files
(.opi), the files in the Db directory are informative about the information
is used.
…On Thu, Mar 23, 2023, 10:41 AM RAYemelyanova ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/ophyd_epics_devices/zebra.py
<#11 (comment)>
:
> +class Zebra(StandardReadable):
+ def __init__(self, prefix: str, name: str = ""):
+ self.pc = PositionCompare("")
+ self.output = ZebraOutputPanel("")
+ self.logic_gates = LogicGateConfigurer("")
+ self.sys = System("")
I will have a crack at doing this...
I'm assuming opi screens == Epics screens when you open up a zebra
—
Reply to this email directly, view it on GitHub
<#11 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AARMUMCKRB5WDYCJE3OGMXTW5RVKPANCNFSM6AAAAAAVJW2HTQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Thanks for all the comments. At the moment the tests are failing with:
I have copied the test format from test_motor, yet I get this error. I'm unsure why this is happening, but I wonder if it's to do with the DeviceCollector not working properly with fixtures? I'm a bit confused. But at this point, this is a problem for another day. Would appreciate any help. Thanks |
This is bluesky/ophyd#1093 If you update pyproject.toml to point at ophyd 215bd0a43c1240788e176a67f30b9746321e2023 (current master) then it should work. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #11 +/- ##
==========================================
+ Coverage 98.60% 99.13% +0.52%
==========================================
Files 5 6 +1
Lines 359 575 +216
==========================================
+ Hits 354 570 +216
Misses 5 5
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks better, thanks! Some additional minor suggestions
Working on this again; I've just rebased and forced push. Got rid of earlier commits as they weren't that clean. |
One test failing now due to #32 which is currently being investigated. |
# RE = RunEngine() | ||
|
||
|
||
# async def somefunc(): | ||
# async with DeviceCollector(): | ||
# # I think I'd like to do and_screen.and[1].inp[1]... | ||
# # so let's start with inp[1]... | ||
|
||
# # and_gates[1].inp[1] | ||
|
||
# # want to do, inp[1].use for example... | ||
# setup_cap = Zebra("BL03S-EA-ZEBRA-01") | ||
# return setup_cap | ||
|
||
|
||
# zebra = asyncio.run(somefunc()) | ||
# print("aha") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll get rid of these lines in a future commit... whoops
This PR is intended to include a zebra device inside the suite of ophyd v2 devices. Some of this, especially the logic gates, is adapted from https://github.com/DiamondLightSource/python-artemis, and the other parts of this have been incorporated from my own work on making a v2 Zebra device.
A few things I'm unsure about;
This PR is part of a larger work, https://jira.diamond.ac.uk/browse/DAQ-4527, looking at putting my v2 Zebra device onto here and in dodal (https://github.com/DiamondLightSource/dodal).