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

Joshc slac/ecseng 565 #44

Merged
merged 3 commits into from
Dec 13, 2024
Merged

Joshc slac/ecseng 565 #44

merged 3 commits into from
Dec 13, 2024

Conversation

joshc-slac
Copy link

Description

By adding a veto device as specified in scenario 1 from the docs such that we can logically ignore the faults when L1 is not receiving beam.

Motivation and Context

We are currently bypassing a fault that is not meaningful. Adding this as a veto device allows us to keep our faults meaningful and the system safer.

How Has This Been Tested?

Ideally today

Where Has This Been Documented?

Not yet

Screenshots (if appropriate):

Pre-merge checklist

  • Code works interactively
  • Code contains descriptive comments
  • Test suite passes locally
  • Libraries are set to fixed versions and not Always Newest
  • Code committed with pre-commit (alternatively pre-commit run --all-files)

@joshc-slac joshc-slac requested a review from ghalym February 28, 2024 16:07
@@ -307,7 +307,7 @@ XPP_DS_Gauge.rPRESS := XPP_Downstream_Gauge.rPRESS;//XPP_Modbus_Gauge.VG.rPRESS;
i_sDevName:= 'BT2L0:PLEG:VGC:01',
iq_stValve=> ,
xMPS_OK=> ,
io_fbFFHWO:= g_FastFaultOutput1,
io_fbFFHWO:= g_FastFaultOutput1, // TODO(josh): ask maggie if this is sufficient because we have connected the FF
Copy link
Author

Choose a reason for hiding this comment

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

{attribute 'pytmc' := '
pv: PLC:LFE:VAC:FFO:04
'}
g_FastFaultOutput4 : FB_HardwareFFOutput :=(i_sNetID:='172.21.88.66.1.1'); //FFO for ST1L0_XTES's Downstream Components
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment is confusing. The new FFO is for the valves in the section that is downstream of the optics and upstream of ST1L1 on the L1 line.

@@ -352,7 +352,7 @@ XPP_DS_Gauge.rPRESS := XPP_Downstream_Gauge.rPRESS;//XPP_Modbus_Gauge.VG.rPRESS;
i_sDevName:= 'TV1L1:VGC:01',
iq_stValve=> ,
xMPS_OK=> ,
io_fbFFHWO:= g_FastFaultOutput1,
io_fbFFHWO:= g_FastFaultOutput1, // TODO(josh): ask maggie if this is sufficient because we have connected the FF
Copy link
Contributor

Choose a reason for hiding this comment

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

This one should be linked to the new FFO that has the optics veto evaluation

@@ -103,7 +99,7 @@
<Resolution>LCLS Vacuum, 2.3.0 (SLAC - LCLS)</Resolution>
</PlaceholderResolution>
<PlaceholderResolution Include="PMPS">
<Resolution>PMPS, 3.0.14 (SLAC - LCLS)</Resolution>
<Resolution>PMPS, * (SLAC - LCLS)</Resolution>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make sure to set the effective version on the libraries. it has to point to a specific version and not set to newest.

Copy link
Author

Choose a reason for hiding this comment

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

image

Copy link
Author

Choose a reason for hiding this comment

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

<LibraryReference Include="PMPS,3.2.1,SLAC - LCLS">
<Namespace>PMPS</Namespace>
</LibraryReference>

Copy link
Author

Choose a reason for hiding this comment

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

https://docs.google.com/presentation/d/1moIDgnQtO2f7veBoTfxUGaALhq8NSwQulQrYihWgLtE/edit?usp=sharing

From slides 39 and 40 I believe I did it correctly, though I see what you are calling out. Let me know if theres a way not enumerated in the documentation to do this

Copy link
Contributor

Choose a reason for hiding this comment

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

The Library shouldn't have been upgraded to the latest. Should have remained the same. I am seeing issues because of this now. You mentioned that these changes are deployed, But now i see that they aren't.

@joshc-slac joshc-slac force-pushed the joshc-slac/ECSENG-565 branch from bea2b68 to d101fdc Compare March 12, 2024 20:34
@@ -307,7 +307,7 @@ XPP_DS_Gauge.rPRESS := XPP_Downstream_Gauge.rPRESS;//XPP_Modbus_Gauge.VG.rPRESS;
i_sDevName:= 'BT2L0:PLEG:VGC:01',
iq_stValve=> ,
xMPS_OK=> ,
io_fbFFHWO:= g_FastFaultOutput1,
io_fbFFHWO:= g_FastFaultOutput4, // TODO(josh): ask maggie if this is sufficient because we have connected the FF
Copy link
Author

@joshc-slac joshc-slac Mar 12, 2024

Choose a reason for hiding this comment

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

@ghalym you called out the one below, my assumption is this one also needed to be updated.

Copy link
Contributor

@ghalym ghalym left a comment

Choose a reason for hiding this comment

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

As per @joshc-slac changes are tested and running on PLC.

@joshc-slac joshc-slac merged commit acb709c into master Dec 13, 2024
20 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.

2 participants