HDW tests for PI 517 and 753 controllers + fixes:

* GCS comm fixes:
  - no param queries
  - case of replies (0X vs 0x)
* 753 fixes:
  -  get_voltage()
  - _get_pos()
  - get_hw_info()

CHANGELOG.md

@@ -57,6 +57,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
     - Improved management of current_configuration / default_configuration
 - Project: Remove `-conda` suffix from requirement files
 
+- PI E712 E753
+    - uniformization of communication and recorder.
+    - "wave" motion generator.
+
 ### Fixed
 
 - Flint

bliss/controllers/motors/pi_e51x.py

@@ -28,6 +28,21 @@ from bliss.common import event
 from . import pi_gcs
 
 
+"""
+Special commands, e.g. fast polling commands, consist only of one
+character. The 24th ASCII character e.g. is called #24. Note that
+these commands are not followed by a termination character (but the
+responses to them are).
+
+* #5: Request Motion Status
+* #6: Query If Position Has Changed Since Last POS? Command
+* #7: Request Controller Ready Status
+* #8: Query If Macro Is Running
+* #9: Get Wave Generator Status
+* #24: Stop All Motion
+"""
+
+
 class PI_E51X(Controller):
     """ Base class for E517 and E518
     """

bliss/controllers/motors/pi_e712.py

@@ -6,7 +6,6 @@
 # Distributed under the GNU LGPLv3. See LICENSE for more info.
 
 import time
-import re
 import numpy
 import weakref
 import gevent
@@ -17,7 +16,7 @@ from bliss.common.utils import add_property
 from bliss.common.axis import AxisState, Motion, CyclicTrajectory
 from bliss.config.channels import Cache
 from bliss.common.switch import Switch as BaseSwitch
-from bliss.common.logtools import *
+from bliss.common.logtools import log_info, log_debug, log_error
 
 from . import pi_gcs
 

bliss/controllers/motors/pi_e753.py

@@ -8,24 +8,31 @@ import contextlib
 import numpy
 
 from bliss.controllers.motor import Controller
-from bliss.common.utils import object_method
 from bliss.common.utils import object_attribute_get, object_attribute_set, object_method
 from bliss.common.axis import AxisState
 from bliss.common import axis as axis_module
-from bliss.common.logtools import log_debug, log_debug_data, log_info, log_warning
+from bliss.common.logtools import log_debug, log_info
 
 from . import pi_gcs
-from bliss.comm.util import TCP
 import gevent.lock
 
-import sys
-import time
 
 """
 Bliss controller for ethernet PI E753 piezo controller.
 Model PI E754 should be compatible.. to be tested.
 """
 
+"""
+Special commands, e.g. fast polling commands, consist only of one
+character. The 24th ASCII character e.g. is called #24. Note that
+these commands are not followed by a termination character (but the
+responses to them are).
+
+* #5: Request Motion Status
+* #9: Get Wave Generator Status
+* #24: Stop All Motion
+"""
+
 
 class PI_E753(pi_gcs.Communication, pi_gcs.Recorder, Controller):
     def __init__(self, *args, **kwargs):
@@ -109,7 +116,7 @@ class PI_E753(pi_gcs.Communication, pi_gcs.Recorder, Controller):
     """ STATE """
 
     def state(self, axis):
-        # check if WAV motion is active
+        # check if WAV motion is active  #9
         if self.sock.write_readline(chr(9).encode()) != b"0":
             return AxisState("MOVING")
 
@@ -149,10 +156,10 @@ class PI_E753(pi_gcs.Communication, pi_gcs.Recorder, Controller):
 
     def stop(self, axis):
         """
-        * HLT -> stop smoothly
         * STP -> stop asap
         * 24    -> stop asap
         * to check : copy of current position into target position ???
+        * NB: 'HLT' command does not exist for pi e-753
         """
         if self._get_closed_loop_status(axis):
             self.command("STP")
@@ -164,7 +171,7 @@ class PI_E753(pi_gcs.Communication, pi_gcs.Recorder, Controller):
     @object_method(types_info=("None", "float"))
     def get_voltage(self, axis):
         """ Return voltage read from controller."""
-        return float(self.command(axis, "SVA? 1"))
+        return float(self.command("SVA? 1"))
 
     @object_method(types_info=("None", "float"))
     def get_output_voltage(self, axis):
@@ -183,10 +190,12 @@ class PI_E753(pi_gcs.Communication, pi_gcs.Recorder, Controller):
 
     def _get_pos(self):
         """
-        - no axis parameter as _get_pos is used by encoder.... can be a problem???
-        - Return a 'float': real position read by capacitive sensor.
+        - no axis parameter as _get_pos() is also used by encoder object.
+
+        Returns : float'
+            Real position of axis read by capacitive sensor.
         """
-        return float(self.command("POS?"))
+        return float(self.command("POS? 1"))
 
     def _get_target_pos(self, axis):
         """
@@ -309,13 +318,12 @@ class PI_E753(pi_gcs.Communication, pi_gcs.Recorder, Controller):
 
     def get_hw_info(self):
         """
-        Return a set of usefull information about controller.
-        Helpful to tune the device.
+        Helpful parameter to tune the device.
 
         Args:
             None
-        Return:
-            None
+        Return: str
+            information about controller.
 
         IDN? for e753:
              Physik Instrumente, E-753.1CD, 111166712, 08.00.02.00
@@ -324,29 +332,26 @@ class PI_E753(pi_gcs.Communication, pi_gcs.Recorder, Controller):
              (c)2016 Physik Instrumente (PI) GmbH & Co. KG, E-754.1CD, 117045756, 1.01
 
         0xffff000* parameters are not valid for 754
-
         """
 
         _infos = [
             ("Identifier                 ", "*IDN?"),
             ("Com level                  ", "CCL?"),
-            ("Real Position              ", "POS?"),
-            ("Setpoint Position          ", "MOV?"),
-            ("Position low limit         ", "SPA? 1 0x07000000"),
-            ("Position High limit        ", "SPA? 1 0x07000001"),
-            ("Velocity                   ", "VEL?"),
-            ("On target                  ", "ONT?"),
-            ("On target window           ", "SPA? 1 0x07000900"),
+            ("Real Position              ", "POS? 1"),
+            ("Setpoint Position          ", "MOV? 1"),
+            ("Position low limit         ", "SPA? 1 0X07000000"),
+            ("Position High limit        ", "SPA? 1 0X07000001"),
+            ("Velocity                   ", "VEL? 1"),
+            ("On target                  ", "ONT? 1"),
             ("Target tolerance           ", "SPA? 1 0X07000900"),
             ("Settling time              ", "SPA? 1 0X07000901"),
-            ("Sensor Offset              ", "SPA? 1 0x02000200"),
-            ("Sensor Gain                ", "SPA? 1 0x02000300"),
-            ("Motion status              ", "#5"),
-            ("Closed loop status         ", "SVO?"),
-            ("Auto Zero Calibration ?    ", "ATZ?"),
-            ("Analog input setpoint      ", "AOS?"),
-            ("Low  Voltage Limit         ", "SPA? 1 0x07000A00"),
-            ("High Voltage Limit         ", "SPA? 1 0x07000A01"),
+            ("Sensor Offset              ", "SPA? 1 0X02000200"),
+            ("Sensor Gain                ", "SPA? 1 0X02000300"),
+            ("Closed loop status         ", "SVO? 1"),
+            ("Auto Zero Calibration ?    ", "ATZ? 1"),
+            ("Analog input setpoint      ", "AOS? 1"),
+            ("Voltage Low Limit          ", "SPA? 1 0X07000A00"),
+            ("Voltage High Limit         ", "SPA? 1 0X07000A01"),
         ]
 
         if self.model == "E-753":
@@ -361,7 +366,7 @@ class PI_E753(pi_gcs.Communication, pi_gcs.Recorder, Controller):
 
         # Reads pre-defined infos (1-line answers only)
         for i in _infos:
-            _ans = self.sock.write_readline(f"{i[1]}\n".encode()).decode()
+            _ans = self.command(i[1])
             _txt += f"        {i[0]} {_ans} \n"
 
         # Reads multi-lines infos.
@@ -408,6 +413,7 @@ class PI_E753(pi_gcs.Communication, pi_gcs.Recorder, Controller):
         Start a simple wav trajectory,
         -- wavetype can be LIN for a Linear  or
            SIN for a sinusoidal.
+        -- offset: 
         -- amplitude motor displacement
         -- nb_cycles the number of time the motion is repeated.
         -- wavelen the time in second that should last the motion

bliss/controllers/motors/pi_gcs.py

@@ -6,10 +6,13 @@
 # Distributed under the GNU LGPLv3. See LICENSE for more info.
 # Distributed under the GNU LGPLv3. See LICENSE.txt for more info.
 
-# PI GCS
-import numpy
+"""
+This module is a common base for PI controllers:
+* for communication
+* fro Wave generator
+"""
 
-from warnings import warn
+import numpy
 
 from bliss.comm.util import get_comm, get_comm_type, TCP
 from bliss.common.event import connect, disconnect
@@ -428,6 +431,8 @@ class Communication:
     def com_initialize(self):
         self.sock = get_pi_comm(self.config, TCP)
         global_map.register(self, children_list=[self.sock])
+
+        # ???
         connect(self.sock, "connect", self._clear_error)
 
     def finalize(self):
@@ -457,8 +462,26 @@ class Communication:
 
         Read answer if needed (ie. `cmd` contains a `?`).
 
-        - Encode `cmd` string.
-        - Add `\\n` terminator.
+        Parameters:
+            <cmd>: str
+                Command. Not encoded; Without terminator character.
+
+            [<nb_line>]: int
+                Number of lines expected in answer.
+                For multi-lines commands (ex: IFC?) or multiple commands.
+
+        Returns: str  ;  list of str  ; tuple of str
+
+        Usage:
+            * id = self.command("*IDN?")
+            * ont = self.command("ONT? 1")
+            * ans = self.command("SPA? 1 0x07000A00")
+            * com_pars_list = self.command("IFC?", 5)
+            * pos, vel = self.command("POS? 1\nVEL? 1", 2)
+
+        Note:
+            Does not work for single char commands (#5 #9 #24 etc.)
+
         """
 
         with self.sock.lock:
@@ -474,35 +497,53 @@ class Communication:
                 if not reply:  # it's an error
                     errors = [self.name] + list(self.get_error())
                     raise RuntimeError(
-                        "Device {0} error nb {1} => ({2})".format(*errors)
+                        "PI Device {0} error nb {1} => ({2})".format(*errors)
                     )
 
                 if nb_line > 1:
+                    # Multi-lines answer or multiple commands
                     parsed_reply = list()
                     commands = cmd.split(b"\n")
-                    if len(commands) == nb_line:  # one reply per command
+                    if len(commands) == nb_line:
+                        # Many queries, one reply per query
+                        # Return a tuple of str
                         for cmd, rep in zip(commands, reply):
                             space_pos = cmd.find(b" ")
                             if space_pos > -1:
                                 args = cmd[space_pos + 1 :]
-                                parsed_reply.append(self._parse_reply(rep, args))
+                                parsed_reply.append(self._parse_reply(rep, args, cmd))
                             else:
+                                # No space in cmd => no param to parse. ex: "*IDN?" "CCL?"
                                 parsed_reply.append(rep)
-                    else:  # a command with several replies
+                    else:
+                        # One command with reply in several lines
+                        # Return a list of str
                         space_pos = cmd.find(b" ")
                         if space_pos > -1:
                             args = cmd[space_pos + 1 :]
                             for arg, rep in zip(args.split(), reply):
-                                parsed_reply.append(self._parse_reply(rep, arg))
+                                parsed_reply.append(self._parse_reply(rep, arg, cmd))
+                        else:
+                            # TSP? TAD? IFC? etc.
+                            for ans in reply:
+                                parsed_reply.append(ans.decode())
                     reply = parsed_reply
                 else:
+                    # Single line answer.
+
+                    # Example: cmd = "VEL? 1"
                     space_pos = cmd.find(b" ")
+                    # print(f"cmd={cmd}   space_pos={space_pos}  reply={reply} ")
                     if space_pos > -1:
-                        reply = self._parse_reply(reply, cmd[space_pos + 1 :])
+                        axes_arg = cmd[
+                            space_pos + 1 :
+                        ]  # 2nd part of the command -> axes id.
+                        reply = self._parse_reply(reply, axes_arg, cmd)
                     else:
                         reply = reply.decode()
                 return reply
             else:
+                # no reply expected.
                 self.sock.write(cmd + b"\n")
                 errno, error_message = self.get_error()
                 if errno:
@@ -519,13 +560,31 @@ class Communication:
         com = com.encode()
         return self.sock.write_readline(b"%s\n" % com)
 
-    def _parse_reply(self, reply, args):
+    def _parse_reply(self, reply, args, cmd):
+        """
+        Extract pertinent value in controller's answer.
+        <reply>: answer of the controller.
+        <args>: arguments of the command (axes numbers)
+                example: "1"    # can be "1 2" "A B" ??
+
+        Examples of commands / answers:
+        * VEL? 1              ->  1=11.0000
+        * SVO? 1              ->  1=1
+        * SPA? 1 0X07000000   ->  1 0x07000000=-3.00000000e+1  # NB: PI replies with '0x' in lower case.
+        * SPA? 1 0X07000A00   ->  1 0x07000A00=0.00000000e+0
+        """
+        u_reply = reply.upper()
+        u_args = args.upper()
         args_pos = reply.find(b"=")
-        if reply[:args_pos] != args:  # weird
-            print("Weird thing happens with connection of %s" % self.name)
-            return reply.decode()
+        if u_reply[:args_pos] != u_args:  # weird
+            print("@ ---------------------------------------------------------")
+            print("@ Weird thing happens with connection of %s" % self.name)
+            print(f"@ command={cmd}")
+            print(f"@   reply={reply} args={args} reply[:args_pos]={reply[:args_pos]}")
+            print("@ ---------------------------------------------------------")
+            return u_reply.decode()
         else:
-            return reply[args_pos + 1 :].decode()
+            return u_reply[args_pos + 1 :].decode()
 
 
 class Recorder:

doc/docs/config_pi_piezo.md

@@ -13,7 +13,7 @@ This configuration should be common to the following models:
 
 Encoder | Shutter | Trajectories
 ------- | ------- | ------------
-YES	| NO      | YES (E-712)  
+YES	    | NO      | YES (E-712)
 
 ### YAML configuration file example
 ```yaml
@@ -42,7 +42,7 @@ controller:
 !!! note
 If `port` is not specified in `url`, e753 uses by default port `50000`.
 
-!!! warning 
+!!! warning
     The PI controller only accepts a single tcp connection
 
 If a controller needs to be accessible in multiple sessions simultaneously
@@ -55,3 +55,125 @@ controller:
     tcp:
        url: e754id42:50000
 ```
+
+
+## Recorder
+
+For 712 753.
+
+Real-time data recorder is able to record several input and output signals
+(e.g. current position, sensor input, output voltage) from different data
+sources (e.g. controller axes or input and output channels).
+
+POSSIBLE DATA RECORDER TYPE:
+
+* `TARGET_POSITION_OF_AXIS`
+* `CURRENT_POSITION_OF_AXIS`
+* `POSITION_ERROR_OF_AXIS`
+* `CONTROL_VOLTAGE_OF_OUTPUT_CHAN`
+* `DDL_OUTPUT_OF_AXIS`
+* `OPEN_LOOP_CONTROL_OF_AXIS`
+* `CONTROL_OUTPUT_OF_AXIS`
+* `VOLTAGE_OF_OUTPUT_CHAN`
+* `SENSOR_NORMALIZED_OF_INPUT_CHAN`
+* `SENSOR_FILTERED_OF_INPUT_CHAN`
+* `SENSOR_ELECLINEAR_OF_INPUT_CHAN`
+* `SENSOR_MECHLINEAR_OF_INPUT_CHAN`
+* `SLOWED_TARGET_OF_AXIS`
+
+The gathered data is stored (temporarily) in "data recorder tables".
+
+* `set_recorder_data_type(*motor_data_type)`: Configure the data recorder
+
+    `motor_data_type` should be a list of tuple with motor and datatype
+     i.e: motor_data_type=[px, px.CURRENT_POSITION_OF_AXIS,
+                           py, py.CURRENT_POSITION_OF_AXIS]
+
+Example:
+```python
+
+mot.controller.set_recorder_data_type(motor, motor.VOLTAGE_OF_OUTPUT_CHAN)
+
+mot.controller.start_recording(mot.controller.WAVEFORM, recorder_rate)
+```
+
+## Wave motion generator
+
+For 753.
+
+A "wave generator" allows to produce "waveforms" ie.user-specified patterns.
+
+This feature is especially important in dynamic applications which require
+periodic, synchronous motion of the axes. The waveforms to be output are stored
+in "wave tables" in the controllers volatile memory—one waveform per wave
+table. Waveforms can be created based on predefined "curve" shapes. This can be
+sine, ramp or single scan line curves. Additionally you can freely define curve
+shapes.
+
+During the wave generator output, data is recorded in "record tables" on the
+controller.
+
+Example:
+```python
+mot.run_wave(wavetype, offset, amplitude, nb_cycles, wavelen)
+```
+
+* `wavetype`: str: `"LIN"` or `"SIN"`
+* `offset`: float: motor displacement offset
+* `amplitude`: float: motor displacement amplitude
+* `nb_cycles`: int: the number of times the motion is repeated
+* `wavelen`: float: time in second that should last the motion
+
+
+## Trajectories
+
+For E712.
+
+## Switch
+
+For E712.
+
+
+## Examples
+
+ID11 example for stress-rig motor combining wave generator and recorder.
+
+```python
+def stress_run_display(wavetype, offset, amplitude, nb_cycles, wavelen, recorder_rate=None):
+    if STRESS_RIG_MOTOR is None:
+        raise RuntimeError("First call init")
+
+    mot = STRESS_RIG_MOTOR
+    # Starts the recorder when the trajectory starts
+    mot.controller.start_recording(mot.controller.WAVEFORM, recorder_rate=recorder_rate)
+
+    def refresh_display(data):
+        current_index = len(data) if data is not None else 0
+        ldata = mot.controller.get_data(from_event_id=current_index)
+        if ldata is None or len(ldata) == 0:
+            return data
+        if data is not None:
+            data = numpy.append(data,ldata)
+        else:
+            data = ldata
+
+        x,y,y2 = (data[name] for name in data.dtype.names)
+        #p.plot(data=y,x=x)
+        p.plot(data={'target':y,'current':y2},x=x)
+        return data
+
+    # Get flint
+    f = flint()
+    #Create the plot
+    p = f.get_plot(plot_class="plot1d", name="stress", unique_name="pi_stress")
+    data=None
+    with mot.run_wave(wavetype, offset, amplitude, nb_cycles, wavelen):
+        while not mot.state.READY:
+            current_index = len(data) if data is not None else 0
+            data = refresh_display(data)
+            if current_index == len(data):
+                gevent.sleep(1)
+
+    #last display
+    refresh_display(data)
+```

tests/controllers_hw/conftest.py

@@ -13,13 +13,18 @@ from bliss.config.conductor import client
 
 @pytest.fixture
 def beacon_beamline():
-    static.CONFIG = None
+    """
+    Fixture to retreive configuration objects from beamline beacon
+    and not from test beacon.
+    """
+
+    static.Config.instance = None
     client._default_connection = None
     config = static.get_config()
     yield config
     config.close()
+    client._default_connection.close()
     client._default_connection = None
-    static.CONFIG = None
 
 
 def pytest_collection_modifyitems(config, items):

tests/controllers_hw/test_PI_E517.py

@@ -12,29 +12,39 @@ Run with:
     $ pytest --axis-name <axis-name> ..../test_PI_E517.py
 
 """
-import pytest
 import time
-import gevent
-import os
+import pytest
 
 
 @pytest.fixture
 def axis(request, beacon_beamline):
+    """
+    Function to access axis given as parameter in test command line.
+    """
     axis_name = request.config.getoption("--axis-name")
-    axis = beacon_beamline.get(axis_name)
+    test_axis = beacon_beamline.get(axis_name)
     try:
-        yield axis
+        yield test_axis
     finally:
-        axis.close()
+        test_axis.close()
 
 
 def test_hw_axis_init(axis):
+    """
+    Hardware initialization
+    Device must be present.
+    Use axis fixture.
+    """
     axis.sync_hard()
     axis.controller._initialize_axis(axis)
 
 
 def test_hw_read_position(axis):
+    """
+    Read position (cache ?)
+    """
     pos = axis.position
+    assert pos
 
 
 # called at end of each test

tests/controllers_hw/test_PI_E753.py

@@ -12,32 +12,112 @@ Run with:
     $ pytest --axis-name <axis-name> ..../test_PI_E753.py
 
 """
-import pytest
 import time
-import gevent
-import os
+import pytest
 
 
 @pytest.fixture
 def axis(request, beacon_beamline):
+    """
+    Function to access axis given as parameter in test command line.
+    """
     axis_name = request.config.getoption("--axis-name")
-    axis = beacon_beamline.get(axis_name)
+    test_axis = beacon_beamline.get(axis_name)
     try:
-        yield axis
+        yield test_axis
     finally:
-        axis.close()
+        test_axis.controller.finalize()
+        # time.sleep(1)
 
 
 def test_hw_axis_init(axis):
+    """
+    Hardware initialization.
+    Device must be present.
+    Use axis fixture.
+    """
+    axis.sync_hard()
     axis.controller._initialize_axis(axis)
 
 
-def test_hw_read_position(axis):
+def test_hw_read_values(axis):
+    """
+    Read position (cache ?)
+    """
     pos = axis.position
+    # print(f"{axis.name} POSITION={pos}")
+    # must be in closed-loop ?
+
+    assert pos >= 0