Merge pull request #335 from bluesky/325-configuration_attrs

Resolve problem with configuration attributes of Diffractometer class.

File new issue(s) should problems appear.

.github/scripts/README.md

@@ -0,0 +1,12 @@
+# GitHub Workflow Scripts
+
+To avoid using `wget` in the CI process, the bash shell script to manage the
+IOCs has been copied from the vendor repository:
+
+```bash
+cd ~/bin
+wget https://raw.githubusercontent.com/prjemian/epics-docker/main/resources/iocmgr.sh
+chmod +x iocmgr.sh
+iocmgr.sh start GP gp
+iocmgr.sh start ADSIM ad
+```

.github/scripts/iocmgr.sh

@@ -0,0 +1,180 @@
+#!/bin/bash
+
+# iocmgr.sh -- Manage IOCs in docker containers
+
+# Usage: ${0} ACTION IOC PRE
+# ACTION    console|restart|run|start|status|stop|caqtdm|medm|usage
+# IOC       "gp", "adsim", (as provided by image)
+# PRE       User's choice.  No trailing colon!
+
+# make all arguments lower case
+ACTION=$(echo "${1}" | tr '[:upper:]' '[:lower:]')
+IOC=$(echo "${2}" | tr '[:upper:]' '[:lower:]')
+PRE=$(echo "${3}" | tr '[:upper:]' '[:lower:]')
+
+# -------------------------------------------
+
+# docker image
+IMAGE=prjemian/synapps:latest
+
+# IOC prefix
+if [ "${PRE:(-1)}" == ":" ]; then
+    # remove the trailing colon
+    PRE="${PRE:0:-1}"
+fi
+PREFIX=${PRE}:
+
+# name of docker container
+CONTAINER=ioc${PRE}
+
+# pass the IOC PREFIX to the container at boot time
+ENVIRONMENT="PREFIX=${PREFIX}"
+
+# convenience definitions
+RUN="docker exec ${CONTAINER}"
+TMP_ROOT=/tmp/docker_ioc
+HOST_IOC_ROOT=${TMP_ROOT}/${CONTAINER}
+HOST_TMP_SHARE="${HOST_IOC_ROOT}/tmp"
+IOC_SCRIPT="/root/bin/${IOC}.sh"
+
+get_docker_container_process() {
+    process=$(docker ps -a | grep ${CONTAINER})
+}
+
+get_docker_container_id() {
+    get_docker_container_process
+    cid=$(echo ${process} | head -n1 | awk '{print $1;}')
+}
+
+start_container(){
+    echo -n "starting container '${CONTAINER}' with PREFIX='${PREFIX}' ... "
+    docker \
+        run -it -d --rm \
+        --name "${CONTAINER}" \
+        -e "${ENVIRONMENT}" \
+        --net=host \
+        -v "${HOST_TMP_SHARE}":/tmp \
+        "${IMAGE}" \
+        bash
+}
+
+start_ioc_in_container(){
+    get_docker_container_process
+    if [ "" != "${process}" ]; then
+        docker exec "${CONTAINER}" bash "${IOC_SCRIPT}" start
+    fi
+}
+
+restart(){
+    get_docker_container_id
+    if [ "" != "${cid}" ]; then
+        stop
+    fi
+    start
+}
+
+status(){
+    get_docker_container_process
+    if [ "" == "${process}" ]; then
+        echo "Not found: ${CONTAINER}"
+    else
+        echo "docker container status"
+        echo "${process}"
+        echo ""
+        echo "processes in docker container"
+        docker top "${CONTAINER}"
+        echo ""
+        echo "IOC status"
+        docker exec "${CONTAINER}" bash "${IOC_SCRIPT}" status
+    fi
+}
+
+start(){
+    get_docker_container_process
+    if [ "" != "${process}" ]; then
+        echo "Found existing ${CONTAINER}, cannot start new one."
+        echo "${process}"
+    else
+        start_container
+        start_ioc_in_container
+    fi
+}
+
+stop(){
+    get_docker_container_id
+    if [ "" != "${cid}" ]; then
+        echo -n "stopping container '${CONTAINER}' ... "
+        docker stop "${CONTAINER}"
+        get_docker_container_id
+        if [ "" != "${cid}" ]; then
+            echo -n "removing container '${CONTAINER}' ... "
+            docker rm ${CONTAINER}
+        fi
+    fi
+}
+
+symbols(){
+    # for diagnostic purposes
+    get_docker_container_id
+    echo "cid=${cid}"
+    echo "process=${process}"
+    echo "ACTION=${ACTION}"
+    echo "CONTAINER=${CONTAINER}"
+    echo "ENVIRONMENT=${ENVIRONMENT}"
+    echo "HOST_IOC_ROOT=${HOST_IOC_ROOT}"
+    echo "HOST_TMP_SHARE=${HOST_TMP_SHARE}"
+    echo "IMAGE=${IMAGE}"
+    echo "IOC=${IOC}"
+    echo "PRE=${PRE}"
+    echo "PREFIX=${PREFIX}"
+    echo "RUN=${RUN}"
+    echo "TMP_ROOT=${TMP_ROOT}"
+}
+
+caqtdm(){
+    if [ "gp" == "${IOC}" ]; then
+        custom_screen="ioc${PRE}.ui"
+    fi
+    # echo "custom_screen=${custom_screen}"
+    "${HOST_TMP_SHARE}/start_caQtDM_${PRE}" "${custom_screen}"
+}
+
+medm(){
+    if [ "gp" == "${IOC}" ]; then
+        custom_screen="ioc${PRE}.ui"
+    fi
+    "${HOST_TMP_SHARE}/start_MEDM_${PRE}" "${custom_screen}"
+}
+
+usage(){
+    echo "Usage: ${0} ACTION IOC PRE"
+    echo "  where:"
+    echo "  ACTION  choices: start stop restart status caqtdm medm usage"
+    echo "  IOC     choices: gp adsim"
+    echo "  PRE     User's choice.  No trailing colon!"
+    echo ""
+    echo "Received: ${0} ${ACTION} ${IOC} ${PRE}"
+    exit 1
+}
+
+# check the inputs
+if [ "3" -ne "$#" ]; then
+    usage
+fi
+
+case "${IOC}" in
+    adsim)  ;;
+    gp)  ;;
+    *) usage
+esac
+
+case "${ACTION}" in
+  start) start ;;
+  stop) stop ;;
+  restart) restart ;;
+  status) status ;;
+  symbols) symbols ;;
+  caqtdm) caqtdm ;;
+  medm) medm ;;
+  *) usage
+esac

.github/workflows/conda_unit_test.yml

@@ -54,6 +54,48 @@ jobs:
         micromamba env list
         python --version
 
+    - name: Directories before Docker
+      run: ls -lAFghrt ~/
+
+    - name: Start EPICS IOCs in Docker
+      run: |
+        set -vxeuo pipefail
+        bash ./.github/scripts/iocmgr.sh start ADSIM ad
+        bash ./.github/scripts/iocmgr.sh start GP gp
+        docker ps -a
+        ls -lAFgh /tmp/docker_ioc/iocad/
+        ls -lAFgh /tmp/docker_ioc/iocgp/
+
+    - name: Directories after Docker
+      run: ls -lAFghrt ~/
+
+    - name: Confirm EPICS IOC is available via caget
+      shell: bash -l {0}
+      run: |
+        set -vxeuo pipefail
+        docker exec iocad /opt/base/bin/linux-x86_64/caget ad:cam1:Acquire_RBV
+        docker exec iocgp grep float1 /home/iocgp/dbl-all.txt
+        docker exec iocgp /opt/base/bin/linux-x86_64/caget gp:UPTIME gp:gp:float1
+        which caget
+        caget ad:cam1:Acquire_RBV
+        caget gp:UPTIME
+        caget gp:gp:float1
+
+    - name: Confirm EPICS IOC is available via PyEpics
+      shell: bash -l {0}
+      run: |
+        python -c "import epics; print(epics.caget('gp:UPTIME'))"
+
+    - name: Confirm EPICS IOC is available via ophyd
+      shell: bash -l {0}
+      run: |
+        CMD="import ophyd"
+        CMD+="; up = ophyd.EpicsSignalRO('gp:UPTIME', name='up')"
+        CMD+="; pv = ophyd.EpicsSignalRO('gp:gp:float1', name='pv')"
+        CMD+="; up.wait_for_connection()"
+        CMD+="; print(up.get(), pv.get())"
+        python -c "${CMD}"
+
     - name: Install the hklpy package
       shell: bash -l {0}
       run: |

.github/workflows/pre-commit.yml

@@ -16,6 +16,8 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - uses: actions/setup-python@v5
+        with:
+          python-version: '3.10'
       - uses: pre-commit/[email protected]
         with:
           extra_args: --all-files

RELEASE_NOTES.rst

@@ -21,10 +21,17 @@ Release History
     v1.1.1 (released -tba-)
     ======================================
 
+    Fixes
+    -----------
+
+    * Resolve problem with configuration attributes of Diffractometer class.
+
     Maintenance
     -----------
 
-    Reorganized the web site docs to be easier to navigate.
+    * Add EPICS IOCs to GitHub workflow unit test suite.
+    * Drop Python 3.7 from unit test suite matrix.
+    * Reorganized the web site docs to be easier to navigate.
 
 v1.1.0 (released 2024-01-13)
 ======================================

docs/source/configuration.rst

@@ -183,7 +183,6 @@ API
     .. autoattribute:: real_axes
     .. autoattribute:: reciprocal_axes
     .. autoattribute:: samples
-    .. autoattribute:: samples
     .. autoattribute:: name
     .. autoattribute:: datetime
     .. autoattribute:: wavelength_angstrom

hkl/diffract.py

@@ -315,20 +315,17 @@ def __init__(
             # fmt: off
             prefix,
             read_attrs=read_attrs,
-            configuration_attrs=configuration_attrs,
+            configuration_attrs=configuration_attrs,  # write xtal info to descriptor
             **kwargs,
             # fmt: on
         )
 
-        # write the crystal orientation information in descriptor doc
-        self.configuration_attrs += self._orientation_attrs
+        for attr in self.orientation_attrs.get():
+            getattr(self, attr).kind = "config"
+        self.energy_update_calc_flag.kind = "config"
+        self.orientation_attrs.kind = "config"  # orientation written as descriptors
         self._constraints_stack = []
 
-        if read_attrs is None:
-            # if unspecified, set the read attrs to the pseudo/real motor
-            # positions once known
-            self.read_attrs = list(self.PseudoPosition._fields) + list(self.RealPosition._fields)
-
         self.energy.subscribe(self._energy_changed, event_type=Signal.SUB_VALUE)
         self.energy_offset.subscribe(self._energy_offset_changed, event_type=Signal.SUB_VALUE)
         self.energy_units.subscribe(self._energy_units_changed, event_type=Signal.SUB_VALUE)

hkl/tests/common.py

@@ -0,0 +1 @@
+IOC_PV_PREFIX_GP = "gp:"

hkl/tests/test_i325_motor_config_attrs.py

@@ -0,0 +1,81 @@
+import pytest
+from ophyd import Component
+from ophyd import Device
+from ophyd import EpicsMotor
+from ophyd import EpicsSignal
+from ophyd import Kind
+
+import hkl
+
+from .common import IOC_PV_PREFIX_GP
+
+OMEGA_PV = f"{IOC_PV_PREFIX_GP}m1"
+CHI_PV = f"{IOC_PV_PREFIX_GP}m2"
+PHI_PV = f"{IOC_PV_PREFIX_GP}m3"
+TTH_PV = f"{IOC_PV_PREFIX_GP}m4"
+
+MOTOR_RECORD_CONFIG_ATTRS = sorted(
+    """
+        acceleration
+        motor_egu
+        user_offset
+        user_offset_dir
+        velocity
+    """.split()
+)
+
+
+class FourC(hkl.SimMixin, hkl.E4CV):
+    omega = Component(EpicsMotor, OMEGA_PV)
+    chi = Component(EpicsMotor, CHI_PV)
+    phi = Component(EpicsMotor, PHI_PV)
+    tth = Component(EpicsMotor, TTH_PV)
+
+
+class MyDevice(Device):
+    omega = Component(EpicsMotor, OMEGA_PV)
+
+
+device = MyDevice("", name="device")
+fourc = FourC("", name="fourc")
+motor = EpicsMotor(OMEGA_PV, name="motor")
+
+
+@pytest.mark.parametrize("attr", MOTOR_RECORD_CONFIG_ATTRS)
+@pytest.mark.parametrize(
+    "motor, parent",
+    [
+        [motor, motor],
+        [device.omega, device],
+        [fourc.omega, fourc],
+        [fourc.chi, fourc],
+        [fourc.phi, fourc],
+        [fourc.tth, fourc],
+    ],
+)
+def test_i325(attr, motor, parent):
+    """
+    Check the configuration_attrs for EpicsMotor itself and as Component.
+
+    https://github.com/bluesky/hklpy/issues/325
+
+    EPICS IOC and motor records are necessary to demonstrate this problem. We
+    prove the problem is NOT in the ophyd Devices by including them in the
+    tests.
+
+    Comparison of device's ``.read_configuration()`` output can show this
+    problem. Compare EpicsMotor with any/all Diffractometer motors.
+    """
+    parent.wait_for_connection()
+    motor.wait_for_connection()
+
+    device_configuration = parent.read_configuration()
+    device_configuration_keys = sorted(list(device_configuration))
+    expected_key = f"{motor.name}_{attr}"
+    assert attr in motor.component_names, f"{expected_key=!r}"
+    component = getattr(motor, attr)
+    assert isinstance(component, EpicsSignal)
+    assert component.kind == Kind.config, f"{component.kind=!r}"
+
+    assert expected_key in device_configuration_keys, f"{expected_key=!r}"
+    assert sorted(motor.configuration_attrs) == MOTOR_RECORD_CONFIG_ATTRS, f"{motor.name=!r}"