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

Making formula pvs in xml file out of certain spreadsheet pvs #66

Merged
merged 4 commits into from
Feb 28, 2025

Conversation

KaushikMalapati
Copy link
Contributor

A replacement for #61, movivation from https://jira.slac.stanford.edu/browse/ECS-6818

Phoebus has support for different pv protocols, with channel acces being the the default.
https://control-system-studio.readthedocs.io/en/latest/core/pv/doc/#formulas

One of these protocols is for "formulas" which are very analogous to calc pvs in edm or the pydm calc plugin. It supports basic arithmetic, and also has a function that returns an alarm severity (major or minor) if a boolean condition evaluates to true - this boolean condition can contain other pvs.
https://control-system-studio.readthedocs.io/en/latest/core/formula/doc/#alarm

This needs changes to how we generate xmls to support - the code in this branch will interpret a pv formatted like minor://{expression} or major://{expression} as an attempt to create a formula based alarm, where the formula pv will go into either a minor or major alarm state based on whether the first five characters are minor or major respectively. It will then format the expression for what phoebus expects which generally looks like (I learned the backticks were not necessary while testing). I tried to make the spreadsheet syntax that nalms users use to make their formula alarms straightforward but I am open to any suggestions

This pull request has a demonstrative formula alarm added to the tmo configuration which I copied into the TST xml config on the dev branch
image
This is using a branch of slam which is being reviewed from slaclab/slac-alarm-manager#70 to make formula based pvs work with the salami tree.

@pcds-bot
Copy link

--- a/Spreadsheet/KFE/TMO-alarms.csv
+++ b/Spreadsheet/KFE/TMO-alarms.csv

@@#IndentBranchPVDescriptionLatchDelayFilterGuidance
+++major://(abs(SL1K0:POWER:ACTUAL_YWIDTH_RBV) * 1000) > AT1K0:GAS_MA_Y:MMS:1slits too wide

Copy link
Contributor

@tangkong tangkong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks pretty solid, thanks for doing all the legwork here and in Phoebus!

I assume this doesn't work until the SLAM PR is merged? To get this working for us we'll have to roll out an updated conda environment as well.

@tangkong
Copy link
Contributor

tangkong commented Feb 26, 2025

Perhaps in a future effort we should update the README here with more complete information on how to use these commands and what exactly the columns mean. The existing confluence pages are a bit more technical than we'd want for people simply contributing the the repository

In particular I'm curious as to what operators are available for these formula pvs

@KaushikMalapati
Copy link
Contributor Author

In particular I'm curious as to what operators are available for these formula pvs

This page has all the operators
https://control-system-studio.readthedocs.io/en/latest/core/formula/doc/#alarm

@KaushikMalapati
Copy link
Contributor Author

I assume this doesn't work until the SLAM PR is merged? To get this working for us we'll have to roll out an updated conda environment as well.

We could always make the slam scripts that the lucid screens use source dev_conda instead of pcds_conda? I should also mention that there's a slam bug that should be fixed before making a new version slaclab/slac-alarm-manager#71

@pcds-bot
Copy link

--- a/Spreadsheet/KFE/TMO-alarms.csv
+++ b/Spreadsheet/KFE/TMO-alarms.csv

@@#IndentBranchPVDescriptionLatchDelayFilterGuidance
+++major://(abs(SL1K0:POWER:ACTUAL_YWIDTH_RBV) * 1000) > AT1K0:GAS_MA_Y:MMS:1slits too wide

@tangkong
Copy link
Contributor

I won't rush this through until we see SLAM updated properly. I think having a dev slam is reasonable, once we can grab a stable one. We'll have to go around and change a bunch of scripts in various places though.

@KaushikMalapati
Copy link
Contributor Author

I won't rush this through until we see SLAM updated properly. I think having a dev slam is reasonable, once we can grab a stable one. We'll have to go around and change a bunch of scripts in various places though.

Sounds good. My branch has been merged, so I think we can release a new version after slaclab/slac-alarm-manager#71 is fixed.

Copy link
Contributor

@tangkong tangkong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fielding my questions. I think we can merge this without much problem, and make sure to get an updated slam version into dev_conda

@tangkong
Copy link
Contributor

Your last comment does raise the question about the PVA pvs in TMO's config, since they actually don't have any status/severity information. We should look into either removing them or making sure the daq people get those record fields implemented

$ caget PLC:TMO:MOTION:FFO:01:FaultHWO_RBV.SEVR
PLC:TMO:MOTION:FFO:01:FaultHWO_RBV.SEVR NO_ALARM
$ pvget DAQ:NEH:tmo:0:DeadFrac
DAQ:NEH:tmo:0:DeadFrac 2025-02-27 15:30:14.197  0 
$ pvget DAQ:NEH:tmo:0:DeadFrac.STAT
Timeout
DAQ:NEH:tmo:0:DeadFrac.STAT 

@KaushikMalapati
Copy link
Contributor Author

Your last comment does raise the question about the PVA pvs in TMO's config, since they actually don't have any status/severity information. We should look into either removing them or making sure the daq people get those record fields implemented

$ caget PLC:TMO:MOTION:FFO:01:FaultHWO_RBV.SEVR
PLC:TMO:MOTION:FFO:01:FaultHWO_RBV.SEVR NO_ALARM
$ pvget DAQ:NEH:tmo:0:DeadFrac
DAQ:NEH:tmo:0:DeadFrac 2025-02-27 15:30:14.197  0 
$ pvget DAQ:NEH:tmo:0:DeadFrac.STAT
Timeout
DAQ:NEH:tmo:0:DeadFrac.STAT 

Where do these even come from? I thought all record types had alarm fields like stat and sevr.

@tangkong
Copy link
Contributor

I assume they're some custom record type the daq group stood up? I'm really not sure myself.

@KaushikMalapati KaushikMalapati merged commit c956074 into pcdshub:master Feb 28, 2025
2 checks passed
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 this pull request may close these issues.

3 participants