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

Support ai/ao records with prec=0 as Signal[int] #280

Closed
evalott100 opened this issue May 2, 2024 · 4 comments · Fixed by #509
Closed

Support ai/ao records with prec=0 as Signal[int] #280

evalott100 opened this issue May 2, 2024 · 4 comments · Fixed by #509
Assignees

Comments

@evalott100
Copy link
Contributor

In #276, we fixed the bug that the panda intialises an analogOut with precision 0 as an int. The signal is made (correctly) with an int, however make_converter fails

if datatype and not issubclass(typ, datatype):
raise TypeError(f"{pv} has type {typ.__name__} not {datatype.__name__}")

since float is not a subclass of int.

The fix in in #276 allows the signal to be made anyway. We should add the constraint that .PREC == 0.

@evalott100 evalott100 self-assigned this May 2, 2024
@evalott100
Copy link
Contributor Author

Once the following are merged we can drop the changes introduced in #276

PandABlocks/PandABlocks-ioc#112
DiamondLightSource/pythonSoftIOC#160

We've decided these kind of checks shouldn't be handled in ophyd-async - pandablocks-ioc should be using an int for the record if ophyd-async expects an int datatype.

@evalott100 evalott100 changed the title Make p4p backend allow using int datatypes for float PVs only if PREC is 0 Drop changes from #276 May 7, 2024
@coretl
Copy link
Collaborator

coretl commented Jun 4, 2024

This has since come up with the CA backend, so I think we need to support this anyway. I suggest we:

  • Make the changes from Fix p4p converter bug #276 in the CA converter too
  • Also assert get_unique({k: v["display"]["precision"] for k, v in values.items()}, "precision") == 0 in the PVA converter
  • Also assert get_unique({k: v.precision for k, v in values.items()}, "precision") == 0 in the CA converter
  • Add records in test_records.db and a test in test/epics/test_signals.py to check

@coretl coretl changed the title Drop changes from #276 Support ai/ao records with prec=0 as Signal[int] Jun 4, 2024
@DominicOram
Copy link
Contributor

@evalott100 are you doing this? Otherwise MX-DAQ would be happy to pick up

@evalott100
Copy link
Contributor Author

are you doing this? Otherwise MX-DAQ would be happy to pick up

That would be much appreciated, there are a few tickets I'll be doing in this sprint before I get round to this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants