-
Notifications
You must be signed in to change notification settings - Fork 9
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
adding new comparison #68
Conversation
--- a/Spreadsheet/KFE/TMO-alarms.csv
|
--- a/Spreadsheet/KFE/TMO-alarms.csv
|
Spreadsheet/KFE/TMO-alarms.csv
Outdated
@@ -88,7 +88,8 @@ | |||
1,STOPPER,,,,,, | |||
2,ST1K4,,,,,, | |||
,,ST1K4:TEST:MMS:STATE:ERR_RBV,ST1K4 error state,,,, | |||
,,ST1K4:TEST:MMS:STATE:GET_RBV,ST1K4 state,,,PPS:NEH1:1:ST3K4INSUM!=0, | |||
,,minor://(ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&PPS:NEH1:1:ST3K4INSUM==0),ST1K4 state,,,, |
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.
,,minor://(ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&PPS:NEH1:1:ST3K4INSUM==0),ST1K4 state,,,, | |
,,minor://(ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&(PPS:NEH1:1:ST3K4INSUM==0),ST1K4 state,,,, |
missing an opening parentheses
Spreadsheet/KFE/TMO-alarms.csv
Outdated
@@ -88,7 +88,8 @@ | |||
1,STOPPER,,,,,, | |||
2,ST1K4,,,,,, | |||
,,ST1K4:TEST:MMS:STATE:ERR_RBV,ST1K4 error state,,,, | |||
,,ST1K4:TEST:MMS:STATE:GET_RBV,ST1K4 state,,,PPS:NEH1:1:ST3K4INSUM!=0, | |||
,,minor://(ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&PPS:NEH1:1:ST3K4INSUM==0),ST1K4 state,,,, | |||
,,major://(ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&PPS:NEH1:1:ST3K4INSUM==1),ST1K4 state,,,, |
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.
,,major://(ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&PPS:NEH1:1:ST3K4INSUM==1),ST1K4 state,,,, | |
,,major://(ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&(PPS:NEH1:1:ST3K4INSUM==1),ST1K4 state,,,, |
also missing an opening parentheses
Spreadsheet/KFE/TMO-alarms.csv
Outdated
@@ -7,7 +7,7 @@ | |||
,,MR1K4:SOMS:PRSM:1_RBV,Cool pressure,,,, | |||
,,MR1K4:SOMS:PIP:01:PRESS_RBV,MR1K4 ion pump,,,, | |||
,,MR1K4:SOMS:GCC:1:PRESS_RBV,GCC,,,, | |||
,,MR1K4:SOMS:COATING:STATE:GET_RBV,MR1K4 Coating,,,SIOC:SYS0:ML00:AO628 <= 400 & MR1K1:BEND:COATING:STATE:GET_RBV==B4C, | |||
,,major://SIOC:SYS0:ML00:AO628 <= 450 & MR1K4:SOMS:COATING:STATE:GET_RBV==2,MR1K4 Coating,,,, |
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.
Is this supposed to be 450 eV or 400eV?
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.
Also I notice that we switched from the string to int representation of the coating enum. I'm actually not sure if phoebus cares which one we use
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.
It is 450 Ev not 400 EV,I double check with James. @KaushikMalapati
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.
I tried the string representation and the int representation with a condition that should have evaluated true. The int condition went into an alarm state, but the string one did not. I assume it was comparing the int value of the pv to the string "B4C" and evaluated to false.
Spreadsheet/KFE/TMO-alarms.csv
Outdated
,,PLC:LAS:OPCPA:01:LPS:AMPHOS_RELAY:ENABLE_RBV,AMPHOS TRIP,,,, | ||
0,KFE BEAMLINES,,,,,, | ||
1,SL1K0,,,,,, | ||
,,major://(SL1K0:POWER:ACTUAL_YWIDTH_RBV>2 OR SL1K0:POWER:ACTUAL_XWIDTH_RBV>2)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==1,SL1K0 depending on AT1K0,,,, |
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.
,,major://(SL1K0:POWER:ACTUAL_YWIDTH_RBV>2 OR SL1K0:POWER:ACTUAL_XWIDTH_RBV>2)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==1,SL1K0 depending on AT1K0,,,, | |
,,major://(SL1K0:POWER:ACTUAL_YWIDTH_RBV>2 || SL1K0:POWER:ACTUAL_XWIDTH_RBV>2)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==1,SL1K0 depending on AT1K0,,,, |
need to use || instead of OR
Spreadsheet/KFE/TMO-alarms.csv
Outdated
,,major://SL1K0:POWER:ACTUAL_YWIDTH_RBV>6.8 OR SL1K0:POWER:ACTUAL_XWIDTH_RBV>6.8)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==2,SL1K0 depending on AT1K0,,,, | ||
,,major://SL1K0:POWER:ACTUAL_YWIDTH_RBV>8 OR SL1K0:POWER:ACTUAL_XWIDTH_RBV>8)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==3,SL1K0 depending on AT1K0,,,, | ||
,,major://SL1K0:POWER:ACTUAL_YWIDTH_RBV>8.5 OR SL1K0:POWER:ACTUAL_XWIDTH_RBV>8.5)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==4,SL1K0 depending on AT1K0,,,, |
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.
,,major://SL1K0:POWER:ACTUAL_YWIDTH_RBV>6.8 OR SL1K0:POWER:ACTUAL_XWIDTH_RBV>6.8)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==2,SL1K0 depending on AT1K0,,,, | |
,,major://SL1K0:POWER:ACTUAL_YWIDTH_RBV>8 OR SL1K0:POWER:ACTUAL_XWIDTH_RBV>8)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==3,SL1K0 depending on AT1K0,,,, | |
,,major://SL1K0:POWER:ACTUAL_YWIDTH_RBV>8.5 OR SL1K0:POWER:ACTUAL_XWIDTH_RBV>8.5)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==4,SL1K0 depending on AT1K0,,,, | |
,,major://(SL1K0:POWER:ACTUAL_YWIDTH_RBV>6.8 || SL1K0:POWER:ACTUAL_XWIDTH_RBV>6.8)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==2,SL1K0 depending on AT1K0,,,, | |
,,major://(SL1K0:POWER:ACTUAL_YWIDTH_RBV>8 || SL1K0:POWER:ACTUAL_XWIDTH_RBV>8)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==3,SL1K0 depending on AT1K0,,,, | |
,,major://(SL1K0:POWER:ACTUAL_YWIDTH_RBV>8.5 || SL1K0:POWER:ACTUAL_XWIDTH_RBV>8.5)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==4,SL1K0 depending on AT1K0,,,, |
Using wrong OR and missing opening parentheses
Spreadsheet/KFE/TMO-alarms.csv
Outdated
,,PLC:LAS:OPCPA:01:LPS:AMPHOS_RELAY:ENABLE_RBV,AMPHOS TRIP,,,, | ||
0,KFE BEAMLINES,,,,,, | ||
1,SL1K0,,,,,, | ||
,,major://(SL1K0:POWER:ACTUAL_YWIDTH_RBV>2 OR SL1K0:POWER:ACTUAL_XWIDTH_RBV>2)&AT1K0:GAS_MAA:01:Y:STATE:GET_RBV==1,SL1K0 depending on AT1K0,,,, |
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.
Just want to double check, is it supposed to be (X OR Y) or (X AND Y)?
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.
X or Y
Spreadsheet/KFE/TMO-alarms.csv
Outdated
,,minor://(ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&PPS:NEH1:1:ST3K4INSUM==0),ST1K4 state,,,, | ||
,,major://(ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&PPS:NEH1:1:ST3K4INSUM==1),ST1K4 state,,,, |
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 inequality clause in both of these need to be changed because the enums are different. When ST3K4 is IN it has a value of 2, but when ST1K4 is IN it has a value of 1.
[kaushikm@psbuild-rhel7-03 pcds-python-migration-tools]$ caget ST1K4:TEST:MMS:STATE:GET_RBV -d31
ST1K4:TEST:MMS:STATE:GET_RBV
Native data type: DBF_ENUM
Request type: DBR_CTRL_ENUM
Element count: 1
Value: IN
Status: NO_ALARM
Severity: NO_ALARM
Enums: ( 3)
[ 0] UNKNOWN
[ 1] OUT
[ 2] IN
[kaushikm@psbuild-rhel7-03 pcds-python-migration-tools]$ caget PPS:NEH1:1:ST3K4INSUM -d 31
PPS:NEH1:1:ST3K4INSUM
Native data type: DBF_ENUM
Request type: DBR_CTRL_ENUM
Element count: 1
Value: IN
Status: LINK
Severity: MAJOR
Enums: ( 2)
[ 0] NOT_IN
[ 1] IN
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.
ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&PPS:NEH1:1:ST3K4INSUM==0. Conditions : ST1K4 != ST3K4 Minor Alarm if ST3K4 is OUT
Major Alarm if ST3K4 is IN
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.
,,minor://((ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&PPS:NEH1:1:ST3K4INSUM==0),ST1K4 state,,,,
,,major://((ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&PPS:NEH1:1:ST3K4INSUM==1),ST1K4 state,,,,
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.
@KaushikMalapati I do not know why it is wrong here, confused
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.
Kaushik hit the big points, I left some questions (mostly for him, the CSS expert now)
There are a lot of SLAM/phoebus changes to enable this, but I think at this point Kaushik has pushed them all through (thanks Kaushik!)
Spreadsheet/KFE/TMO-alarms.csv
Outdated
@@ -7,7 +7,7 @@ | |||
,,MR1K4:SOMS:PRSM:1_RBV,Cool pressure,,,, | |||
,,MR1K4:SOMS:PIP:01:PRESS_RBV,MR1K4 ion pump,,,, | |||
,,MR1K4:SOMS:GCC:1:PRESS_RBV,GCC,,,, | |||
,,MR1K4:SOMS:COATING:STATE:GET_RBV,MR1K4 Coating,,,SIOC:SYS0:ML00:AO628 <= 400 & MR1K1:BEND:COATING:STATE:GET_RBV==B4C, | |||
,,major://SIOC:SYS0:ML00:AO628 <= 450 & MR1K4:SOMS:COATING:STATE:GET_RBV==2,MR1K4 Coating,,,, |
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.
Also I notice that we switched from the string to int representation of the coating enum. I'm actually not sure if phoebus cares which one we use
Spreadsheet/KFE/TMO-alarms.csv
Outdated
,,MR2K4:KBO:COATING:STATE:PMPS:ARB:ENABLE_RBV,MR2K4 arbiter status,,,, | ||
2,MR3K4,,,,,, | ||
,,MR3K4:KBO:FWM:1_RBV,Flow sensor,,,, | ||
,,MR3K4:KBO:RTD:CHIN:R:TEMP_RBV,Temperature,,,, | ||
,,MR3K4:KBO:RTD:CHIN:L:TEMP_RBV,Temperature,,,, | ||
,,MR3K4:KBO:COATING:STATE:GET_RBV,MR3K4 Coating,,,SIOC:SYS0:ML00:AO628 <= 400 & PPS:NEH1:1:ST3K4INSUM!=1, | ||
,,major://SIOC:SYS0:ML00:AO628 <= 450 & MR3K4:KBO:COATING:STATE:GET_RBV==2,MR3K4 Coating,,,, |
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.
I feel like I asked @KaushikMalapati this before, but can't find the thread: Is there an operation order to these operators? More specifically, are the comparison operators (<, >, >= etc) executed before the logical operators (&, |)? If they're not, we should make sure to group these properly.
Also we've used single &
instead of &&
. It seems like CSS doesn't differentiate here, but I'd love to know for sure
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.
I think I found the order here
https://github.com/ControlSystemStudio/phoebus/blob/5508dfc472bdf9d3c760a74d5ff41597068a9aa5/core/formula/src/main/java/org/csstudio/apputil/formula/Formula.java#L44-L53
Seems to be pretty normal
According to that documentation and testing on the tst configuration, & and && are identical. I assume that everything on https://control-system-studio.readthedocs.io/en/latest/core/formula/doc/ is true since it's the official phoebus documentation
--- a/Spreadsheet/KFE/TMO-alarms.csv
|
--- a/Spreadsheet/KFE/TMO-alarms.csv
|
--- a/Spreadsheet/KFE/TMO-alarms.csv
|
--- a/Spreadsheet/KFE/TMO-alarms.csv
|
Add new comparison : All mirrors + ST1K4 + SL1K0 vs AT1K0
Add AMPOUS (pump laser)trip PV