pcdshub / lcls-twincat-motion

TwinCAT 3 Motion Control Utilities - PLC Motion Library for all PCDS Applications
https://pcdshub.github.io/lcls-twincat-motion
Other
35 stars 20 forks source link

Configure default names for DUT_PositionState instances of in/out positioners #144

Closed klauer closed 2 years ago

klauer commented 2 years ago

FB_PositionStateInOut https://github.com/pcdshub/lcls-twincat-motion/blob/b06601abf7178475651e279f94becbe4ec9926bf/lcls-twincat-motion/Library/POUs/Motion/States/Examples/FB_PositionStateInOut.TcPOU#L17-L20

Projects which do not configure these have invalid names:

$ caget -S AT2L0:XTES:MMS:02:STATE:01:NAME_RBV AT2L0:XTES:MMS:02:STATE:02:NAME_RBV  AT2L0:XTES:MMS:02:STATE:03:NAME_RBV
AT2L0:XTES:MMS:02:STATE:01:NAME_RBV Invalid
AT2L0:XTES:MMS:02:STATE:02:NAME_RBV Invalid
AT2L0:XTES:MMS:02:STATE:03:NAME_RBV Invalid

Arguably the project should be specific about the state names, but I think we can probably agree that it would be sensible for the motion library to provide sensible defaults.

This matters more now that pcdsdevices (and typhos) will rely on these PVs for drop-down menus in typhos and such.

cc @ZLLentz @ZryletTC

ZLLentz commented 2 years ago

I think this is already handled for in/out? Am I missing something? https://github.com/pcdshub/lcls-twincat-motion/blob/b06601abf7178475651e279f94becbe4ec9926bf/lcls-twincat-motion/Library/POUs/Deprecated/FB_EpicsInOut.TcPOU#L57-L58

ZLLentz commented 2 years ago

Well this is actually a bit confusing because there are two versions of in/out: the "Deprecated" one linked here and the newer one at: https://github.com/pcdshub/lcls-twincat-motion/blob/master/lcls-twincat-motion/Library/POUs/Motion/States/Examples/FB_PositionStateInOut.TcPOU

ZLLentz commented 2 years ago

It looks like it is this newer version that fails to mandate some basic default name values

klauer commented 2 years ago

The non-deprecated version could be considered in the wrong spot, though the comments do note:

https://github.com/pcdshub/lcls-twincat-motion/blob/b06601abf7178475651e279f94becbe4ec9926bf/lcls-twincat-motion/Library/POUs/Motion/States/Examples/FB_PositionStateInOut.TcPOU#L7

ZLLentz commented 2 years ago

The non-deprecated version definitely needs a fix as per this issue, but the link in the issue text is to the wrong file

klauer commented 2 years ago

My apologies - I'll fix that now.