Conversation
|
@hxu-bnl if you run |
|
@jwlodek It was tested and ready to review. |
There was a problem hiding this comment.
Pull request overview
Adds support for deploying a Beckhoff EK9000 + ELxxxx (ELIO) IOC, including module install metadata and a new elio device role, plus a small cleanup/consistency update for existing roles/modules.
Changes:
- Added new installable module definitions for
ek9000andelio, including dependency chaining tomodbus. - Introduced a new
eliodevice role (schema, example config, startup template, and Ansible task to install it). - Updated the
mdriverole substitution template to useSYS_IOC, and setdeploy_ioc_template_root_pathformdrive.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| roles/install_module/vars/modbus_bb9fa05.yml | Removes executable from the modbus module metadata. |
| roles/install_module/vars/elio_cc756ab.yml | Adds module metadata for the ELIO IOC module and its dependency on ek9000. |
| roles/install_module/vars/ek9000_60e62b5.yml | Adds module metadata for the EK9000 module and its dependency on modbus. |
| roles/device_roles/mdrive/templates/asynRecord.substitutions.j2 | Switches generated P macro to use SYS_IOC and aligns pattern formatting. |
| roles/device_roles/elio/templates/base.cmd.j2 | Adds ELIO IOC startup commands (EK9000 config + terminal loop). |
| roles/device_roles/elio/tasks/main.yml | Installs the ELIO base.cmd into the IOC boot directory. |
| roles/device_roles/elio/schema.yml | Adds initial schema for elio device role environment. |
| roles/device_roles/elio/example.yml | Adds a sample elio IOC configuration with terminals + environment. |
| roles/deploy_ioc/vars/mdrive.yml | Sets deploy_ioc_template_root_path to the required module path for mdrive. |
| roles/deploy_ioc/vars/elio.yml | Adds deploy defaults for the new elio role/module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| epicsEnvSet("POS",{{ elio[0] }}) | ||
| epicsEnvSet("MODEL","{{ elio[1] }}") | ||
| epicsEnvSet("Dev","{$(DEV)_$(POS)}") |
There was a problem hiding this comment.
epicsEnvSet("Dev","{$(DEV)_$(POS)}") sets the Dev env var with literal {} braces around the expanded value. Nothing else in the repo uses this {...} form for env var composition, and the resulting $(Dev) will include braces when passed into ek9000ConfigureTerminal / dbLoadTemplate, which is likely unintended and can break generated PV names/macros. Consider composing this as $(DEV)_$(POS) (no braces), or otherwise ensure the downstream templates explicitly expect braces.
| epicsEnvSet("Dev","{$(DEV)_$(POS)}") | |
| epicsEnvSet("Dev","$(DEV)_$(POS)") |
| HOST: str() | ||
| NUM_TERMS: int() | ||
| PREFIX: str() | ||
| ENGINEER: str() |
There was a problem hiding this comment.
The template iterates over ioc.terminals, but the role schema doesn't define/require a terminals field. This allows invalid configs to pass schema validation and then fail at render time when ioc.terminals is missing. Add a terminals entry to the schema (e.g., a non-empty list of 2-item tuples/lists: position + model) to make the role interface self-validating.
| ENGINEER: str() | |
| ENGINEER: str() | |
| terminals: list() |
No description provided.