Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make useful scripts into a dodal utility #732

Closed
stan-dot opened this issue Aug 7, 2024 · 8 comments
Closed

Make useful scripts into a dodal utility #732

stan-dot opened this issue Aug 7, 2024 · 8 comments
Labels
enhancement New feature or request python Pull requests that update Python code

Comments

@stan-dot
Copy link
Contributor

stan-dot commented Aug 7, 2024

          Tagging @stan-dot as this is general good info. So to look up the type of a PV I do the following:
$ caget BL03I-EA-XSP3-01:ERASE.ZNAM BL03I-EA-XSP3-01:ERASE.ONAM
BL03I-EA-XSP3-01:ERASE.ZNAM    
BL03I-EA-XSP3-01:ERASE.ONAM    

We can see that both are blank compared to a PV of the same type I found elsewhere:

$ caget BL03I-MO-SGON-01:CS1:DeferMoves.ZNAM BL03I-MO-SGON-01:CS1:DeferMoves.ONAM
BL03I-MO-SGON-01:CS1:DeferMoves.ZNAM Defer Off
BL03I-MO-SGON-01:CS1:DeferMoves.ONAM Defer On
  • Given they're both blank this is why your string enum complains if they're anything else. Obviously this won't actually work in production as if you set blank on the PV how does it know which one you actually need?

So, we have some choices:

  1. Talk to controls and ask them to provide named states for the PV, or change the PV to a type that is more appropriate
  2. Use an int - This will work but is a bit of a shame as it's not super clear that writing 1 to the erase is what you want to do
  3. Use an IntEnum e.g.
class EraseEnum(int, Enum):
    ERASE = 1
    DONE = 0

But are you saying that didn't work? In which case we should probably open a discussion up about if this is a bug in ophyd_async

I prefer option 1 but it will be the most work. I can live with option 2 for now in the interests of just getting things done, maybe we can then put in a ticket with controls to improve it at a later date

Originally posted by @DominicOram in #524 (comment)

@stan-dot stan-dot added enhancement New feature or request python Pull requests that update Python code labels Aug 7, 2024
@stan-dot stan-dot self-assigned this Aug 7, 2024
@stan-dot
Copy link
Contributor Author

stan-dot commented Oct 1, 2024

@DominicOram what do you think?

@DominicOram
Copy link
Contributor

I think it needs wider discussion with controls about what they see the future of PV discover-ability being. find-pv has a number of issues currently that a future script would probably want to fix:

  • It's python 2
  • It depends on the current file system structure (I assume it will not discover any containerised IOCs running in k8s)
  • It doesn't pick up any python IOC PVs

I think trying to put it in dodal as is is wasted effort which we should instead put into a full solution. However, I suspect that full solution will require some effort and is not yet a prioirity

@stan-dot stan-dot removed their assignment Oct 1, 2024
@stan-dot
Copy link
Contributor Author

stan-dot commented Oct 1, 2024

@marcelldls , @coretl what do you think? since we already have device detection in dodal with dodal-conenct, the channel-specific logic sounds like could live quite near it.

OTOH maybe this is a non-issue, obsolesced by argocd?

still argocd only is set to go to 3 beamlines by the dark period, so there would be a stream of value from having the support for the existing non-containerized IOCs too

@coretl
Copy link
Collaborator

coretl commented Oct 7, 2024

I see no reason to touch find-pv at the moment, it will soon become obsolete when we have containerised all the IOCs. This will be solved in the future by https://channelfinder.github.io/ but I'm not sure as yet where or how it will run. @MJGaughran may have more ideas.

@stan-dot
Copy link
Contributor Author

stan-dot commented Oct 7, 2024

will soon become obsolete when we have containerised all the IOCs

if I remember right, last time I spoke with @gilesknap this was years away, that at the moment we're less than 1/3rd through

@coretl
Copy link
Collaborator

coretl commented Oct 7, 2024

will soon become obsolete when we have containerised all the IOCs

if I remember right, last time I spoke with @gilesknap this was years away, that at the moment we're less than 1/3rd through

Yes, and we have find-pv for the interim...

@stan-dot
Copy link
Contributor Author

stan-dot commented Oct 8, 2024

I see a script in dodal that is more powerful than that - performs the lookup of the various files too. Maybe I just make a quick PR to make the discussion more concrete

@stan-dot
Copy link
Contributor Author

as discussed with @coretl , deferred in favour of ChannelFinder exploration https://github.com/ChannelFinder/ChannelFinderService/

@stan-dot stan-dot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request python Pull requests that update Python code
Projects
None yet
Development

No branches or pull requests

3 participants