pcdshub / pytmc

Generate EPICS IOCs and records from TwinCAT projects - along with many TwinCAT project tools
https://pcdshub.github.io/pytmc/
Other
10 stars 11 forks source link

ENH: partial array pragmas #240

Closed klauer closed 3 years ago

klauer commented 3 years ago

Partially addresses #223

klauer commented 3 years ago

slaclab Travis CI is beyond its quota. Local proof:

 247 passed, 1 skipped, 28 xfailed in 94.51s (0:01:34) 

Docs are building correctly as well.

ZLLentz commented 3 years ago

This doesn't seem to work for the states shortening like I expected- all of my records are still generated. I'm having trouble debugging because my chains aren't showing up in the debug UI.

klauer commented 3 years ago

Happy to take a look at the project - did you push the (built) branch somewhere?

ZLLentz commented 3 years ago

I'm getting a lot of: ERROR:pytmc.bin.db:Error creating record: Symbol GVL_Logger.fbRootLogger failure: Type with GUID {18071995-0000-0000-0000-000000000018} not in TMC Which breaks the debug GUI but does not break the db generator (???). This happens on master and on my branch. I modified my checkout to work around this and I see a bunch of this now, so in a better position to start debug: image

My branch is chilling at https://github.com/ZLLentz/lcls-plc-kfe-motion/tree/pytmc-test-arr

klauer commented 3 years ago

I can confirm all you're seeing - unless you have a strong interest to, don't bother diving too deep here. (It's dark down in the parsing/record-generating internals, and I may be the only one with a candlelight...)

Must be some differences in the type lookups in the GUI versus the db generation, but I can't fathom as to why.

Will get this sorted...

ZLLentz commented 3 years ago

Alright, I'll work on something else- deploying pcds-envs maybe

For context on that branch, what has been done is:

  1. Update lcls2-cc-lib to add array: 1..4 etc. as applicable like so:
    {attribute 'pytmc' := '
        pv: MMS:STATE
        io: io
        array: 1..4
    '}
    fbStates: FB_XPIM_States;
  2. Install to prog node as v0.0.0
  3. Rebuild kfe plc with cclib v0.0.0
klauer commented 3 years ago

Found the primary difference in the debug tool errors versus the db generation. Debug tool goes down a more detailed path, looking at every symbol chain: even those where one or more pragmas are missing somewhere along the chain (allow_no_pragma=True). Hmm

klauer commented 3 years ago

OK, I think I see the confusion here. The pragma is on fbStates, not arrStates which I had erroneously assumed. Based on the helpful screenshot, it should have been obvious to me before I started poking around with pytmc summary --debug's IPython console - but I digress.

This sadly falls under bullet point (3) of the PR - the thing I haven't figured out how to do just yet. The TMC file is laid out as follows:

The TLDR of the above is: the structure of the TMC file appears to make impossible for the same structure member to be pragma'd differently on a per-instance basis.

I think we'll have no choice but to extend pytmc's interpretation of pragmas to be more like the TcLinkTo syntax we frequently use.

ZLLentz commented 3 years ago

Hmm, is there a reason why the io can cascade down and the pv can accumulate suffixes, but the array cannot?

klauer commented 3 years ago

I finally realized last night that you were assuming it would cascade down to the subitems, based on your example.

There is no technical reason it could not. I'm a bit worried that we'll be shooting ourselves in the foot - the cascading would fail when there are two arrays of differing sizes in a DUT, or any such arrays in a structure of that structure. Once we got the "extended" pragma definition working, I'd expect to remove support for that cascading.

In my opinion, the above is enough to avoid cascading entirely. Or do you think the above is too alarmist?

ZLLentz commented 3 years ago

The IOCs all loaded without this last night and without increasing PLC load too much, so I think it's fine to not support the cascading

klauer commented 3 years ago

OK, but whether the IOC boots with increased PLC load or not is a bit of a separate topic... isn't it?

ZLLentz commented 3 years ago

Sure. In general I agree with your logic that something bad can happen if we let the array pragma cascade down. I would be ok with overlooking this if we really needed the feature and that was an easy way to skip over this. Since the feature is a "very want" and not a "category 9 emergency" I think we take the time to do it right.

klauer commented 3 years ago

Ah, I see - understood. Then let's move forward with this part of the pragma support, and circle back soonish to sub-item pragmas.