Skip to content

Conversation

@gwbischof
Copy link
Collaborator

@gwbischof gwbischof commented Nov 4, 2025

Migrated SmarAct role from ioc-deploy-roles to nsls2.ioc_deploy

Copilot AI review requested due to automatic review settings November 4, 2025 19:28
Copy link
Contributor

Copilot AI left a 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 pull request adds support for two new device roles in an Ansible-based EPICS IOC deployment system:

  1. Wiener Crate - for SNMP-based monitoring and control of Wiener power supply crates
  2. SmarAct Motor Controllers - for MCS and MCS2 piezo motor positioning systems

Key Changes

  • Added complete role implementations for wienercrate and smaract device types with templates, tasks, schemas, and examples
  • Introduced two new EPICS module configurations: snmp_nscl_3cd2804 and motorsmaract_ab4dfeb
  • Included database files, MIB definitions, and configuration files for Wiener crate monitoring
  • Created deployment variable files linking device roles to their required EPICS modules

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
roles/install_module/vars/snmp_nscl_3cd2804.yml Module definition for SNMP support with NSCL variant
roles/install_module/vars/motorsmaract_ab4dfeb.yml Module definition for SmarAct motor driver
roles/device_roles/wienercrate/* Complete device role for Wiener crate SNMP monitoring
roles/device_roles/smaract/* Complete device role for SmarAct motor controllers
roles/deploy_ioc/vars/wienercrate.yml IOC deployment configuration for Wiener crates
roles/deploy_ioc/vars/smaract.yml IOC deployment configuration for SmarAct controllers

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

{{ device_roles_smaract_idle_poll_period }},
{{ device_roles_smaract_disable_speed }})
{% for i in range(ioc.num_axes) %}
smaraActMCSCreateAxis("$(PORT)", {{ i }}, {{ i }})
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'smaraActMCSCreateAxis' to 'smarActMCSCreateAxis'. The function name has a typo with 'smara' instead of 'smarAct'.

Suggested change
smaraActMCSCreateAxis("$(PORT)", {{ i }}, {{ i }})
smarActMCSCreateAxis("$(PORT)", {{ i }}, {{ i }})

Copilot uses AI. Check for mistakes.
drvAsynIPPortConfigure("$(PORT)_ETH", "$(CONTROLLER_IP):55551", 0, 0, 0)
MCS2CreateController("$(PORT)", "$(PORT)_ETH", "$(NUM_AXES)",
{{ device_roles_smaract_moving_poll_period * 1000 }},
{{ device_roles_smaract_moving_poll_period * 1000 }},
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 10 uses device_roles_smaract_moving_poll_period * 1000 but should likely use device_roles_smaract_idle_poll_period * 1000 based on the parameter naming convention and the non-MCS2 branch which uses separate moving and idle periods.

Suggested change
{{ device_roles_smaract_moving_poll_period * 1000 }},
{{ device_roles_smaract_idle_poll_period * 1000 }},

Copilot uses AI. Check for mistakes.
@thopkins32
Copy link
Contributor

Substitution files need work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants