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

adding new comparison #68

Merged
merged 6 commits into from
Mar 1, 2025
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions Spreadsheet/KFE/TMO-alarms.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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,,,,
Copy link
Contributor

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?

Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor

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.

,,MR1K4:SOMS:GANTRY_X_RBV,Gantry X,,,,
,,MR1K4:SOMS:GANTRY_Y_RBV,Gantry Y,,,,
,,MR1K4:SOMS:COATING:STATE:PMPS:ARB:ENABLE_RBV,MR1K4 arbiter status,,,,
Expand All @@ -18,13 +18,13 @@
,,MR2K4:KBO:RTD:CHIN:R:TEMP_RBV,Temperature,,,,
,,MR2K4:KBO:RTD:CHIN:L:TEMP_RBV,Temperature,,,,
,,MR2K4:KBO:GCC:01:PRESS_RBV,GCC,,,,
,,MR2K4:KBO:COATING:STATE:GET_RBV,MR2K4 Coating,,,SIOC:SYS0:ML00:AO628 <= 400 & PPS:NEH1:1:ST3K4INSUM!=1,
,,major://SIOC:SYS0:ML00:AO628 <= 450 & MR2K4:KBO:COATING:STATE:GET_RBV==2,MR2K4 Coating,,,,
,,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,,,,
Copy link
Contributor

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

Copy link
Contributor

@KaushikMalapati KaushikMalapati Mar 1, 2025

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

,,MR3K4:KBO:COATING:STATE:PMPS:ARB:ENABLE_RBV,MR3K4 arbiter status,,,,
2,MR4K4,,,,,,
,,MR4K4:KBO:RTD:CHIN:L:TEMP_RBV,Temperature,,,,
Expand Down Expand Up @@ -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,,,,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
,,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

,,major://(ST1K4:TEST:MMS:STATE:GET_RBV!=PPS:NEH1:1:ST3K4INSUM)&&PPS:NEH1:1:ST3K4INSUM==1),ST1K4 state,,,,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
,,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

Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor Author

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,,,,

Copy link
Contributor Author

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

,,ST1K4:TEST:MMS:STATE:ST3K4_AUTO_RBV,ST1K4 follow ST3K4 status,,,,
1,IP1,,,,,,
2,Main Chamber Pressure,,,,,,
Expand Down Expand Up @@ -124,4 +125,10 @@
,,LM1K4:QADC:01:OUT0_EDGE2,QADC signal monitor,,,LM1K4:QADC:01:OUT0_EDGE2>0,
1,LASER PHASE MOTOR,,,,,,
,,LAS:LHN:LLG2:01:PHAS:ERROR,Laser phase feedback,,,,
,,major://(abs(SL1K0:POWER:ACTUAL_YWIDTH_RBV) * 1000) > AT1K0:GAS_MA_Y:MMS:1,slits too wide,,,,
,,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,,,,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
,,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

Copy link
Contributor

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)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

X or Y

,,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,,,,
Copy link
Contributor

@KaushikMalapati KaushikMalapati Feb 28, 2025

Choose a reason for hiding this comment

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

Suggested change
,,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

Loading