pcdshub / lcls-twincat-optics

TwinCAT 3 Motion Control for LCLS Optics
https://pcdshub.github.io/lcls-twincat-optics/
Other
0 stars 6 forks source link

Generic optics library for use by individual optics plc projects #1

Closed jsheppard95 closed 4 years ago

jsheppard95 commented 4 years ago

Pull request to begin discussion/solicit feedback on common optics PLC code for use within specific optics PLC projects.

Summary: Added function block FB_RunHOMS to enable motors, assign encoder offsets, run the autocoupling function blocks defined in lcls-twincat-motion (https://github.com/pcdshub/lcls-twincat-motion/blob/master/lcls-twincat-motion/Library/POUs/Motion/Gantry/FB_GantryAutoCoupling.TcPOU), and monitor gantry differences.

Currently, Main in this project serves as an example of another optics project that would instantiate FB_RunHOMS for each system it controls. I.e, here I have axes, instances of fbMotionStage, and the necessary coupling/decoupling booleans defined for two systems (M1L0/M2L0).

I have also included serial communication function blocks copied/pasted from Axilon's projects.

Initial Thoughts/TODO

  1. Unclear if bExecuteCouple/related decoupling booleans need to be defined in Main (i.e in specific optics PLC projects. Could be entirely local to FB_RunHOMS since coupling should only be adjusted by experts and not from Epics.

  2. After input/output hierarchy is set in stone, can add pytmc pragmas.

klauer commented 4 years ago

I did a quick pytmc summary of this to help get an overview of what's included in the PR, see here: https://gist.github.com/klauer/30424869ca2682f2c78a3af24a5e2138

jsheppard95 commented 4 years ago

I did a quick pytmc summary of this to help get an overview of what's included in the PR, see here: https://gist.github.com/klauer/30424869ca2682f2c78a3af24a5e2138

Oh that's awesome, way cleaner to see what's going on than using GitHub Diff's. So the piezo/serial control stuff I just copied/pasted from Axilon's project but need to spend some time going through it in detail to understand what's going on before asking for review.

Though in terms of FB_RunHOMS, I came up with my I/O structure somewhat arbitrarily through some guess/check. Maybe its not the best to have gantry tolerances as inputs, bExecuteCouple/bExecuteDecouple should just be in VAR instead of VAR_IN_OUT, or maybe these details are not a big deal either way. Just some initial thoughts.

ZLLentz commented 4 years ago

I don't think there's any risk at all in having couple/decouple records so long as the gantry interlock FBs stop you from breaking the system. I think this is a case where more knobs might come in handy later.

I'll actually review this PR now

ZLLentz commented 4 years ago

Seems fine as long as you take Ken's suggestions

jsheppard95 commented 4 years ago

Thanks for the comments, definitely good changes. Those I left unresolved are mostly due to some ambiguity of what should be defined in the library project and what should be defined in Optics projects that use the library. Maybe at this point I should make a second project MR1L0-MR2L0 which could use this library, and then I can clean up the GVLs/remove Main and that will hopefully make the necessary pragmas/MPS FBs/other remaining work more obvious.

jsheppard95 commented 4 years ago

I made another PR (https://github.com/pcdshub/MR1L0-MR2L0/pull/1) where the mirror system-specific pieces now live, so now the scope of this PR can just include general optics functionality that will be common to all the Axilon systems. I can now develop both these in parallel and have a better sense of the interface, but I'm thinking the next big pieces to focus on are the pytmc pragma structure and MPS fast faults. More commits to follow with a possible approach.