REF: use pytmc template to generate everything; fix lots of things #86
Conversation
klauer
changed the title
|
I'm going to review the code first, but I'd like to quickly ask: for testing the build, I just need to set up an environment with the pytmc PR branch, clone this repo, and point my IOC Makefile to this repo's clone? |
That's the right idea. I've been using the usual |
iocBoot/templates/Makefile.base
Outdated
| @echo "Creating IOC data directory" | ||
| install --mode 0775 --group ps-ioc --directory $(IOC_DATA_PATH)/$(IOC_NAME)/{iocInfo,archive,logs,autosave} | ||
| install --mode 2775 --group ps-ioc --directory $(IOC_DATA_PATH)/$(IOC_NAME)/{iocInfo,archive,logs,autosave} |
There was a problem hiding this comment.
I always have to look these up: this is user rwx, group rwx, any r-x, setgid bit so that any can run with the same permissions as group.
There was a problem hiding this comment.
Also importantly, setgid ensures that permissions for new files underneath that directory get set consistently to that of the parent directory
| {% endif %} | ||
| {% endfor %} | ||
| # | ||
| ################### AUTO-GENERATED DO NOT EDIT ################### |
There was a problem hiding this comment.
👍 for the verbose st.cmd comment header
iocBoot/templates/st.cmd.template
Outdated
|
|
||
| < envPaths | ||
|
|
||
| epicsEnvSet("ADS_IOC_TOP", "$(TOP)" ) | ||
|
|
||
| {% if add_plc_route | int != 0 %} | ||
| # Add a route to the PLC automatically: | ||
| system("${ADS_IOC_TOP}/scripts/add_route.sh {{ plc_host }} {{ ioc_host_ip_regex }}") |
There was a problem hiding this comment.
I'm excited to see how well this works. I suspect the only time it fails is when the IOC launches prior to the PLC launch?
One question: where exactly is the ioc_host_ip_regex set up? I see the Makefile variable (in ALL CAPS) but nothing else.
There was a problem hiding this comment.
I suspect the only time it fails is when the IOC launches prior to the PLC launch?
Yes, but the IOC will also restart if it can't communicate with the PLC, so it should work out 🤞
One question: where exactly is the ioc_host_ip_regex set up? I see the Makefile variable (in ALL CAPS) but nothing else.
It's macro-ified here, along with all of the rest of the ads-ioc-specific variables:
https://github.com/klauer/ads-ioc/blob/1bf04d99caf4661ae9ebdedd6961a4242109bda1/iocBoot/templates/Makefile.base#L96
|
|
||
| echo "Running ads-async to add a route to ${ioc_hostname} for IOC host ${ioc_hostname} (${ioc_host_ip})..." | ||
| set -x | ||
| $CONDA_BIN run ads-async route \ |
There was a problem hiding this comment.
Good application of this conda run option 👍
| @@ -54,10 +95,12 @@ adsAsynPortDriverConfigure("$(ASYN_PORT)", "$(IPADDR)", "$(AMSID)", "$(AMS_PORT) | |||
|
|
|||
| cd "$(ADS_IOC_TOP)/db" | |||
|
|
|||
| {% set motors = get_motors(plc) %} | |||
There was a problem hiding this comment.
Line item in PR description that I want to call out specifically: Removes motion axes from IOC without pragma (*)
Related change in pytmc: https://github.com/pcdshub/pytmc/pull/290/files#diff-6be6c30834f696fcba07dcfa6bc5e38c3caacea04cb25945a8f4f59233d55967R329
This requires that motion axes have a pytmc pragma for inclusion into the startup script.
I believe this would be a good change, but I'd like to confirm it before moving forward here.
There was a problem hiding this comment.
To be clear: this stops us from creating the placeholder PVs for when someone has left a pragma off of the DUT_MotionStage instance? But it should still index the pragma'd motors correctly, skipping over the gaps? I think it's a good change (and I don't think most people will notice)
There was a problem hiding this comment.
That's correct - indexing and other functionality remains. The records for that axis just do not get created.
| @@ -13,6 +13,7 @@ SHELL = /bin/bash | |||
|
|
|||
There was a problem hiding this comment.
Picking a random line for this to start a thread
Sample size 1 for running this on lcls-twincat-motion-example:
- the files created are reasonable
- error messages on make:
WARNING:pytmc.parser:Pytmc failed to figure out what to do with: /cds/group/pcds/epics-dev/zlentz/lcls-twincat-motion-example/lcls-twincat-motion-example/_Config/NC/NC.xti
Traceback (most recent call last):
File "/cds/home/z/zlentz/pydev/pytmc/parser.py", line 2054, in _pre_load_xti_files
key = get_key(xti_filename, xti)
File "/cds/home/z/zlentz/pydev/pytmc/parser.py", line 2037, in get_key
raise ValueError(f'Hmm: {candidates}')
ValueError: Hmm: [(<class 'pytmc.parser.Image'>, '1', 'nc.xti'), (<class 'pytmc.parser.Axis'>, '1', 'nc.xti'), (<class 'pytmc.parser.Axis'>, '2', 'nc.xti'), (<class 'pytmc.parser.Axis'>, '3', 'nc.xti')]
WARNING:pytmc.parser:Pytmc failed to figure out what to do with: /cds/group/pcds/epics-dev/zlentz/lcls-twincat-motion-example/lcls-twincat-motion-example/_Config/PLC/tc_mot_example.xti
Traceback (most recent call last):
File "/cds/home/z/zlentz/pydev/pytmc/parser.py", line 2054, in _pre_load_xti_files
key = get_key(xti_filename, xti)
File "/cds/home/z/zlentz/pydev/pytmc/parser.py", line 2037, in get_key
raise ValueError(f'Hmm: {candidates}')
ValueError: Hmm: [(<class 'pytmc.parser.Instance'>, '#x08502000', 'tc_mot_example.xti'), (<class 'pytmc.parser.POU'>, '{7b41e58a-08f0-4959-90b4-af08ef5cbf10}', 'tc_mot_example.xti')]
There was a problem hiding this comment.
Current runtime behavior is:
- nothing happens, stalls on failed connections forever (plc is off)
There was a problem hiding this comment.
I tried digging into those warnings a bit but came out empty-handed. My only guess is that the old project is saved slightly differently compared to our standards. At least it's not fatal...
|
@klauer We are just done with the liquid jet experiments at XPP and we will take the PLC back to XCS. Are we at a point where we could test this in a real situation? |
|
Yes, this is ready for testing (with the expectation that further work might be necessary, of course!) Also, I may have limited ability to help in debugging/deploying this, depending on the scheduling of laser work this week. |
|
Fixes above were after working with Vincent on
|
I think the rework you describe here is great
What exactly is this issue? I seem to have forgotten |
I know you saw this already, but for the sake of looking back on this PR, the relevant issue is: #90 |
|
Any objections to merging and tagging this? |
|
@klauer LGTM |
Changes
>=2.14.0now requiredpytmc templatefrom upcoming pytmc release to generate:load_plc_databases.cmd- everything is inst.cmdnow$()make buildshould no longer copy to iocData directories (that can be done withmake allormake iocdata)2775instead of0775) by convention (make it easier for us to edit files there)Issues / related PRs
Closes #85
Closes #83
Closes #81
Closes #79
Closes #80
Closes #70
Closes #20
Closes #87
Closes #88
Closes #89
Closes #90
Requires pcdshub/pytmc#290
Example header
st.cmd diff for ioc-lfe-motion:
TODO