-
Notifications
You must be signed in to change notification settings - Fork 0
Change autosave frequency command #52
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
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
In the ticket it said:
But this looks to be using the XML route - did you discuss this with Freddie? |
| </U32> | ||
| </Cluster> | ||
| """ | ||
|
|
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.
You should use the full XML herer as per https://github.com/ISISComputingGroup/EPICS-isisdae/blob/master/isisdaeApp/src/xml/update_settings_cluster.xml
| Args: | ||
| freq (float): frequency of autosave in Hz. | ||
| """ |
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.
The units are actually "Frames" (as in isis pulses) as opposed to Hz and are an integer
| func = self.api.set_pv_value | ||
|
|
||
| check_xml = ( | ||
| b"<Cluster>\n <Name>DAE Updates</Name>\n <NumElts>3</NumElts>\n" |
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.
rather than repeat the xml data structure, could you make UPDATE_SETTINGS_XML a template i.e. with placeholders for data and then use that both here and earlier?
Implements a change ICP autosave frequency command.
DAE:UPDATESETTINGSpvPVValuecan now be of subtypeBytesTo test: