-
Notifications
You must be signed in to change notification settings - Fork 13
Fix pi e621 role #133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix pi e621 role #133
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the pi_e621 role to fix configuration issues and add proper motor substitution handling. The role is being migrated from the hyphenated naming (pi-e621) to underscore naming (pi_e621).
Key changes:
- Restructured motor configuration to use a list-based schema instead of hardcoded environment variables
- Added verification configuration for automated testing
- Separated HOST and HOST_PORT in schema for better validation
Reviewed changes
Copilot reviewed 9 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| roles/device_roles/pi_e621/templates/base.cmd.j2 | New template for IOC startup commands with updated variable references |
| roles/device_roles/pi_e621/templates/asyn_motor.substitutions.j2 | Modified to use motor list iteration instead of hardcoded values |
| roles/device_roles/pi_e621/tasks/main.yml | New Ansible tasks for deploying templates |
| roles/device_roles/pi_e621/schema.yml | Updated schema with motor list support and improved HOST validation |
| roles/device_roles/pi_e621/examples/pi-e621-test/verify.yml | New verification configuration with skip_compilation flag |
| roles/device_roles/pi_e621/examples/pi-e621-test/config.yml | Example configuration demonstrating motor list usage |
| roles/device_roles/pi_e621/README.md | Added basic documentation |
| roles/device_roles/pi-e621/* | Removed old hyphenated directory files |
| roles/deploy_ioc/vars/pi_e621.yml | New deployment variables for the role |
| .github/workflows/test-roles.yml | Comment clarification about excluding deleted roles |
Comments suppressed due to low confidence (1)
roles/device_roles/pi_e621/schema.yml:49
- The motor schema defines fields (egu, twv, velo, vbas, accl, bdst, bvel, bacc, mres, prec, dhlm, dllm, init) that are not referenced in the asyn_motor.substitutions.j2 template. The template uses hardcoded environment variable references like "$(EGU)", "$(TWV)", etc. instead of motor.egu, motor.twv, etc. Either remove these unused schema fields or update the template to use per-motor values.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Jakub Wlodek <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.