-
Notifications
You must be signed in to change notification settings - Fork 3
Support for native BPM CS aggregator #118
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
Conversation
gubaidulinvadim
left a comment
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.
This looks fine to me. I think it will be good to specify in the docstring that by native backend we mean that the underlying Device / PV implement an array of positions for many BPMs. (If I understand this correctly). I would like if @gupichon will also confirm that it is ok to merge because we wanted to fix the version to be used at the workshop.
|
OK. It will be nice to update the native tango-backend before fixing release for the workshop otherwise orbit correction on the bessy VA will not be possible with main pyaml branch. |
gupichon
left a comment
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.
Small error in the yaml test file a "/" is missing.
tests/config/bpms.yaml
Outdated
| y_pos_index: 1 | ||
| x_pos: | ||
| type: tango.pyaml.attribute_read_only | ||
| attribute: srdiag/bpm/c01-04Position |
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.
Typo error
This PR adds support for native BPM aggregator.
It also include support for needed vector variable and bring an incompatibly with the tango native backend due to the new function
attach_array()to be implemented.pyaml_cs_oabackend is already up-to-date.Please merge #123 and tag to 0.2.1 before merging this PR.