pcdshub / lcls-plc-crixs-motion

Repository for code related to the LCLS NEH H2.2 ChemRIXS endstation motion system.
https://pcdshub.github.io/lcls-plc-crixs-motion/
Other
0 stars 7 forks source link

ENH: EPS bad state condition ,Motor EPS override, and Update Limit Position Values #72

Closed jozamudi closed 6 months ago

jozamudi commented 7 months ago

Description

Added a default case if the motor is not in a known state we do not allow the motor to move. Added an override function to ignore EPS in case the motor is in a bad state.

Updated EPS Position Values, SDS RY got an encoder and position values had to be updated. UPDATED_EPS_TABLE

Motivation and Context

We dont want motors to move if we do know what state the other motors are in. To prevent controls person logging in to PLC and forcing values when motor is in a bad state, an override button was added to ignore the current motors eps.

How Has This Been Tested?

DJ and I checkout the changes by moving the motors and checking if the EPS would stop the motors from moving past limits.

Where Has This Been Documented?

Screenshots (if appropriate):

image

Pre-merge checklist

jyotiphy commented 7 months ago

@jozamudi I'm trying to think if it's a good idea to bypass the implemented conditions in terms of motor/device protection ? Is there a specific motor which needs this override, unencoded ones ?
@ZLLentz what do you think about this override ?

ZLLentz commented 7 months ago

I'm not a fan of overrides in general, but they're a practical compromise that lets the user opt in to handling edge cases themselves and can save us a lot of off-hours hassle. The main dangers of an override are:

I can review this PR in detail later if you'd like.

jozamudi commented 7 months ago

I will add the override timer, and add a more visible override indicator in this PR as well?

https://github.com/pcdshub/pcdsdevices/pull/1204#discussion_r1528868207

ZLLentz commented 7 months ago

and add a more visible override indicator in this PR as well?

Your override PV here is defined specially for the CRIX PLC, so it can't be part of the generic BeckhoffAxisEPSCustom screen, right? Or maybe there's something specific you're planning here? Or maybe you mean that you'll put override indicators in the motor-specific EPS windows?

Whichever way ends up working, it's a good idea to show the indicators (and the countdown timeout if possible) edit: and probably needs the controls to do the override too, right? Or are those more hidden?