-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add PVI integration #132
Add PVI integration #132
Conversation
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.
no issues with the code changes!
now trying it out ...
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## main #132 +/- ##
==========================================
+ Coverage 68.15% 71.31% +3.15%
==========================================
Files 16 16
Lines 804 868 +64
==========================================
+ Hits 548 619 +71
+ Misses 256 249 -7
📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
ouch! If only I had done the pydantic conversion! Do I need an Asyn definition? The top of my asynNDArrayDriver.pvi.device.yaml looks like this label: asynNDArrayDriver
parent: asynPortDriver
children:
- type: Group
name: ADSetup
layout:
|
One more question - do we have a schema for device.yaml. If so I can use vscode to debug the yaml itself. UPDATE: I found the schema and all device YAMLS are passing. |
ooops |
If there is no schema error, it is probably the name case thing. Check This may be the bit out of that mess that is useful:
So the name of Group 6 (not sure if 0-indexed) needs a capital? |
OK that did it - no more schema errors - but I needed to change quite a few places. I assume we can fix the generator to not make this mistake (and maybe include the restriction in the schema). I'm now getting
which is correct - will try and generate a AsynPortDriver |
This should start with a lower case to match the driver name. I generated the pvi yamls here. |
Thanks - but that is the AsynPortDriver itself from Asyn I assume. Your link does not have one of these and I can't convince PVI to build one. (not sure which DB I should use but its the .h file it doesn't like I think.
|
We have lift-off!! I removed the ref to AsynPortDriver that I mentioned above #132 (comment) |
5abc43e
to
417f130
Compare
These files were renamed in `5a1ee025 fix naming of subst files`
Make generate_links take a specific support module to link rather than doing glob of all directories.
Add PVI_DEFS to list of assets to be exported for runtime stage Change use of Dict to Mapping to make mypy happy; ``` src/ibek/runtime_cmds/commands.py:158: error: Argument "args" to "Database" has incompatible type "dict[str, str]"; expected "dict[str, str | None]" [arg-type] src/ibek/runtime_cmds/commands.py:158: note: "Dict" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance src/ibek/runtime_cmds/commands.py:158: note: Consider using "Mapping" instead, which is covariant in the value type ```
This PR has highlighted an issue that CI needs to know where to find the ibek support YAML - but ibek already knows Added --ibek-defs for use in CI to generate schema without breaking each time we move the /epics/ibek-defs folder. This is the default. The regenerate_samples.sh script can use --no-ibek-defs to only include the given support yaml definitions.
@gilesknap I added the last tweaks, visible in the compare changes here. I know you said it should avoid using I will leave you to press the merge button in case there are any issues with this. |
Yep I'm happy with the if definitions thing. |
Closes #119