Various small improvements to documentation, error messages etc.

Also moved the initialization of some parameters to sinqMotor

Makefile

@@ -11,7 +11,7 @@ REQUIRED+=asynMotor
 REQUIRED+=sinqMotor
 
 # Specify the version of sinqMotor we want to build against
-sinqMotor_VERSION=0.3.0
+sinqMotor_VERSION=0.6.2
 
 # These headers allow to depend on this library for derived drivers.
 HEADERS += src/pmacv3Axis.h

README.md

@@ -4,50 +4,43 @@
 
 This is a driver for the pmacV3 motion controller with the SINQ communication protocol. It is based on the sinqMotor shared library (https://git.psi.ch/sinq-epics-modules/sinqmotor). The header files contain detailed documentation for all public functions. The headers themselves are exported when building the library to allow other drivers to depend on this one.
 
-## Usage in IOC shell
+## User guide
+
+This driver is a standard sinqMotor-derived driver and does not need any specific configuration. For the general configuration, please see https://git.psi.ch/sinq-epics-modules/sinqmotor/-/blob/main/README.md.
+
+The folder "utils" contains utility scripts for working with pmac motor controllers:
+- writeRead.py: Allows sending commands to and receiving commands from a pmac controller over an ethernet connection.
+- analyzeTcpDump: This script takes a tcpdump as an input and 
+
+## Developer guide
+
+### Usage in IOC shell
 
 pmacv3 exposes the following IOC shell functions (all in pmacv3Controller.cpp):
 - `pmacv3Controller`: Create a new controller object.
 - `pmacv3Axis`: Create a new axis object.
-The function arguments are documented directly within the source code or are available from the help function of the IOC shell.
-
-## Database
-
-The pmacV3 module provides additional PVs in the database template  db/pmacv3.db. It can be parametrized with the `dbLoadTemplate` function from the IOC shell:
-
+These functions are parametrized as follows:
 ```
-require sinqMotor, y.y.y # The sinqMotor module is needed for the pmacv3 module. The version y.y.y is defined in the Makefile (line sinqMotor_VERSION=x.x.x)
-require pmacv3, x.x.x # This is the three-digit version number of the pmacv3 module
-dbLoadTemplate "motor.substitutions"
+pmacv3Controller(
+    "$(NAME)", # Name of the MCU, e.g. mcu1. This parameter should be provided by an environment variable.
+    "$(ASYN_PORT)", # IP-Port of the MCU. This parameter should be provided by an environment variable.
+    8, # Maximum number of axes
+    0.05, # Busy poll period in seconds
+    1, # Idle poll period in seconds
+    0.05 # Communication timeout in seconds
+    );
+```
+```
+pmacv3Axis(
+    "$(NAME)", # Name of the associated MCU, e.g. mcu1. This parameter should be provided by an environment variable.
+    1 # Index of the axis.
+    );
 ```
 
-The substitutions file can be concatenated with that of sinqMotor:
-```
-file "$(sinqMotor_DB)/sinqMotor.db"
-{
-pattern
-...
-}
-
-file "$(pmacv3_DB)/pmacv3.db"
-{
-pattern
-{ AXIS,     M}
-{ 1,   "lin1"}
-{ 2,   "rot1"}
-}
-```
-The sinqMotor pattern "..." is documented in https://git.psi.ch/sinq-epics-modules/sinqmotor/-/blob/main/README.md.
-The other parameters have the following meaning:
-- `AXIS`: Index of the axis, corresponds to the physical connection of the axis to the MCU
-- `M`: Name of the motor as shown in EPICS
-
-The axis name should correspond to that of the sinqMotor pattern with the same respective index.
-
-## Versioning
+### Versioning
 
 Please see the documentation for the module sinqMotor: https://git.psi.ch/sinq-epics-modules/sinqmotor/-/blob/main/README.md.
 
-## How to build it
+### How to build it
 
 Please see the documentation for the module sinqMotor: https://git.psi.ch/sinq-epics-modules/sinqmotor/-/blob/main/README.md.

db/pmacv3.db

@@ -1,59 +1,19 @@
-# Encoder type
-record(waveform, "$(P)$(M):Encoder_Type") {
-    field(DTYP, "asynOctetRead")
-    field(INP, "@asyn($(CONTROLLER),$(AXIS),1) ENCODER_TYPE")
-    field(FTVL, "CHAR")
-    field(NELM, "80")
-    field(SCAN, "I/O Intr")
-}
-
-# reread encoder
-record(longout, "$(P)$(M):Reread_Encoder") {
+# Trigger a rereading of the encoder. This action is sometimes necessary for 
+# absolute encoders after enabling them. For incremental encoders, this is a no-op.
+# This record is coupled to the parameter library via rereadEncoderPosition_ -> REREAD_ENCODER_POSITION.
+record(longout, "$(INSTR)$(M):RereadEncoder") {
     field(DTYP, "asynInt32")
     field(OUT, "@asyn($(CONTROLLER),$(AXIS),1) REREAD_ENCODER_POSITION")
     field(PINI, "NO")
 }
 
-# reread encoder
-record(longout, "$(P)$(M):Read_Config") {
+# The pmacV3 driver reads certain configuration parameters (such as the velocity
+# and the acceleration) directly from the MCU. This reading procedure is performed
+# once at IOC startup during atFirstPoll. However, it can be triggered manually
+# by setting this record value to 1.
+# This record is coupled to the parameter library via readConfig_ -> READ_CONFIG.
+record(longout, "$(INSTR)$(M):ReadConfig") {
     field(DTYP, "asynInt32")
     field(OUT, "@asyn($(CONTROLLER),$(AXIS),1) READ_CONFIG")
     field(PINI, "NO")
-}
-
-# ===================================================================
-# The following records read acceleration and velocity from the driver and 
-# copy those values into the corresponding fields of the main motor record.
-# This strategy is described here: https://epics.anl.gov/tech-talk/2022/msg00464.php
-
-# Helper record for the high limit which is filled in by the driver
-record(ai, "$(P)$(M):MOTOR_VELOCITY-RBV")
-{
-    field(DTYP, "asynFloat64")
-    field(INP,  "@asyn($(CONTROLLER),$(AXIS)) MOTOR_VELOCITY_FROM_DRIVER")
-    field(SCAN, "I/O Intr")
-    field(FLNK, "$(P)$(M):PUSH_VELO_TO_FIELD")
-}
-
-# Push the value into the field of the main motor record
-record(ao, "$(P)$(M):PUSH_VELO_TO_FIELD") {
-    field(DOL,  "$(P)$(M):MOTOR_VELOCITY-RBV CP")
-    field(OUT,  "$(P)$(M).VELO")
-    field(OMSL, "closed_loop") # This configuration keeps the PV and the field in sync
-}
-
-# Helper record for the low limit which is filled in by the driver
-record(ai, "$(P)$(M):MOTOR_ACCL-RBV")
-{
-    field(DTYP, "asynFloat64")
-    field(INP,  "@asyn($(CONTROLLER),$(AXIS)) MOTOR_ACCEL_FROM_DRIVER")
-    field(SCAN, "I/O Intr")
-    field(FLNK, "$(P)$(M):PUSH_ACCL_TO_FIELD")
-}
-
-# Push the value into the field of the main motor record
-record(ao, "$(P)$(M):PUSH_ACCL_TO_FIELD") {
-    field(DOL,  "$(P)$(M):MOTOR_ACCL-RBV CP")
-    field(OUT,  "$(P)$(M).ACCL")
-    field(OMSL, "closed_loop") # This configuration keeps the PV and the field in sync
 }

src/pmacv3Axis.cpp

@@ -50,7 +50,8 @@ pmacv3Axis::pmacv3Axis(pmacv3Controller *pC, int axisNo)
         exit(-1);
     }
 
-    status = pC_->setDoubleParam(axisNo_, pC_->motorPosition_, 0.0);
+    // pmacv3 motors can always be disabled
+    status = pC_->setIntegerParam(axisNo_, pC_->motorCanDisable_, 1);
     if (status != asynSuccess) {
         asynPrint(
             pC_->pasynUserSelf, ASYN_TRACE_ERROR,
@@ -73,12 +74,7 @@ pmacv3Axis::~pmacv3Axis(void) {
 /**
 Read the configuration at the first poll
  */
-asynStatus pmacv3Axis::atFirstPoll() { return readConfig(); }
-
-/*
-Read the configuration from the motor control unit and the parameter library.
-*/
-asynStatus pmacv3Axis::readConfig() {
+asynStatus pmacv3Axis::atFirstPoll() {
 
     // Local variable declaration
     asynStatus status = asynSuccess;
@@ -87,6 +83,7 @@ asynStatus pmacv3Axis::readConfig() {
     double motorRecResolution = 0.0;
     double motorPosition = 0.0;
     double motorVelocity = 0.0;
+    double motorVmax = 0.0;
     double motorAccel = 0.0;
     int acoDelay = 0.0; // Offset time for the movement watchdog caused by
                         // the air cushions in milliseconds.
@@ -101,16 +98,19 @@ asynStatus pmacv3Axis::readConfig() {
                                          __PRETTY_FUNCTION__, __LINE__);
     }
 
-    // Software limits and current position
+    /*
+    Read out the axis status, the current position, current and maximum speed,
+    acceleration and the air cushion delay.
+    */
     snprintf(command, sizeof(command),
-             "P%2.2d00 Q%2.2d10 Q%2.2d04 Q%2.2d06 P%2.2d22", axisNo_, axisNo_,
-             axisNo_, axisNo_, axisNo_);
-    status = pC_->writeRead(axisNo_, command, response, 5);
+             "P%2.2d00 Q%2.2d10 Q%2.2d03 Q%2.2d04 Q%2.2d06 P%2.2d22", axisNo_,
+             axisNo_, axisNo_, axisNo_, axisNo_, axisNo_);
+    status = pC_->writeRead(axisNo_, command, response, 6);
     if (status != asynSuccess) {
         return status;
     }
-    nvals = sscanf(response, "%d %lf %lf %lf %d", &axStatus, &motorPosition,
-                   &motorVelocity, &motorAccel, &acoDelay);
+    nvals = sscanf(response, "%d %lf %lf %lf %lf %d", &axStatus, &motorPosition,
+                   &motorVmax, &motorVelocity, &motorAccel, &acoDelay);
 
     // The acoDelay is given in milliseconds -> Convert to seconds, rounded up
     offsetMovTimeout_ = std::ceil(acoDelay / 1000.0);
@@ -120,7 +120,7 @@ asynStatus pmacv3Axis::readConfig() {
     // here to mm/s^2.
     motorAccel = motorAccel * 1000;
 
-    if (nvals != 5) {
+    if (nvals != 6) {
         return pC_->errMsgCouldNotParseResponse(command, response, axisNo_,
                                                 __PRETTY_FUNCTION__, __LINE__);
     }
@@ -135,25 +135,14 @@ asynStatus pmacv3Axis::readConfig() {
                                          __PRETTY_FUNCTION__, __LINE__);
     }
 
-    status =
-        pC_->setDoubleParam(axisNo_, pC_->motorVelocityRBV_, motorVelocity);
+    // Write to the motor record fields
+    status = setVeloFields(motorVelocity, 0.0, motorVmax);
     if (status != asynSuccess) {
-        return pC_->paramLibAccessFailed(status, "motorVelocityRBV_",
-                                         __PRETTY_FUNCTION__, __LINE__);
+        return status;
     }
-
-    status = pC_->setDoubleParam(axisNo_, pC_->motorAccelRBV_, motorAccel);
+    status = setAcclField(motorAccel);
     if (status != asynSuccess) {
-        return pC_->paramLibAccessFailed(status, "motorAccelRBV_",
-                                         __PRETTY_FUNCTION__, __LINE__);
-    }
-
-    // Set the initial enable based on the motor status value
-    status =
-        setIntegerParam(pC_->enableMotor_, (axStatus != -3 && axStatus != -5));
-    if (status != asynSuccess) {
-        return pC_->paramLibAccessFailed(status, "enableMotor_",
-                                         __PRETTY_FUNCTION__, __LINE__);
+        return status;
     }
 
     // Update the parameter library immediately
@@ -251,7 +240,7 @@ asynStatus pmacv3Axis::doPoll(bool *moving) {
     }
 
     // Transform from EPICS to motor coordinates (see comment in
-    // pmacv3Axis::readConfig())
+    // pmacv3Axis::atFirstPoll)
     previousPosition = previousPosition * motorRecResolution;
 
     // Query the axis status
@@ -274,13 +263,13 @@ asynStatus pmacv3Axis::doPoll(bool *moving) {
     The axis limits are set as: ({[]})
     where [] are the positive and negative limits set in EPICS/NICOS, {} are the
     software limits set on the MCU and () are the hardware limit switches. In
-    other words, the EPICS/NICOS limits must be stricter than the software
+    other words, the EPICS/NICOS limits should be stricter than the software
     limits on the MCU which in turn should be stricter than the hardware limit
     switches. For example, if the hardware limit switches are at [-10, 10], the
     software limits could be at [-9, 9] and the EPICS / NICOS limits could be at
     [-8, 8]. Therefore, we cannot use the software limits read from the MCU
     directly, but need to shrink them a bit. In this case, we're shrinking them
-    by 0.1 mm or 0.1 degree (depending on the axis type) on both sides.
+    by limitsOffset on both sides.
     */
     pl_status =
         pC_->getDoubleParam(axisNo_, pC_->motorLimitsOffset_, &limitsOffset);
@@ -294,6 +283,14 @@ asynStatus pmacv3Axis::doPoll(bool *moving) {
     // Store the axis status
     axisStatus_ = axStatus;
 
+    // Update the enablement PV
+    pl_status = setIntegerParam(pC_->motorEnableRBV_,
+                                (axStatus != -3 && axStatus != -5));
+    if (pl_status != asynSuccess) {
+        return pC_->paramLibAccessFailed(pl_status, "motorEnableRBV_",
+                                         __PRETTY_FUNCTION__, __LINE__);
+    }
+
     // Intepret the status
     switch (axStatus) {
     case -6:
@@ -524,7 +521,7 @@ asynStatus pmacv3Axis::doPoll(bool *moving) {
 
         snprintf(command, sizeof(command),
                  "Maximum allowed following error exceeded (P%2.2d01 = %d). "
-                 "Check if movement range is blocked."
+                 "Check if movement range is blocked. "
                  "Otherwise please call the support.",
                  axisNo_, error);
         pl_status = setStringParam(pC_->motorMessageText_, command);
@@ -618,13 +615,6 @@ asynStatus pmacv3Axis::doPoll(bool *moving) {
         }
     }
 
-    pl_status = setIntegerParam(pC_->enableMotorRBV_,
-                                (axStatus != -3 && axStatus != -5));
-    if (pl_status != asynSuccess) {
-        return pC_->paramLibAccessFailed(pl_status, "enableMotorRBV_",
-                                         __PRETTY_FUNCTION__, __LINE__);
-    }
-
     pl_status = setIntegerParam(pC_->motorStatusMoving_, *moving);
     if (pl_status != asynSuccess) {
         return pC_->paramLibAccessFailed(pl_status, "motorStatusMoving_",
@@ -658,7 +648,7 @@ asynStatus pmacv3Axis::doPoll(bool *moving) {
     }
 
     // Transform from motor to EPICS coordinates (see comment in
-    // pmacv3Axis::readConfig())
+    // pmacv3Axis::atFirstPoll())
     currentPosition = currentPosition / motorRecResolution;
 
     pl_status = setDoubleParam(pC_->motorPosition_, currentPosition);
@@ -681,12 +671,15 @@ asynStatus pmacv3Axis::doMove(double position, int relative, double minVelocity,
 
     char command[pC_->MAXBUF_], response[pC_->MAXBUF_];
     double motorCoordinatesPosition = 0.0;
-    int enabled = 0;
     double motorRecResolution = 0.0;
+    double motorVelocity = 0.0;
+    int enabled = 0;
+    int motorCanSetSpeed = 0;
+    int writeOffset = 0;
 
     // =========================================================================
 
-    pl_status = pC_->getIntegerParam(axisNo_, pC_->enableMotorRBV_, &enabled);
+    pl_status = pC_->getIntegerParam(axisNo_, pC_->motorEnableRBV_, &enabled);
     if (pl_status != asynSuccess) {
         return pC_->paramLibAccessFailed(pl_status, "enableMotorRBV_",
                                          __PRETTY_FUNCTION__, __LINE__);
@@ -708,18 +701,43 @@ asynStatus pmacv3Axis::doMove(double position, int relative, double minVelocity,
 
     // Convert from EPICS to user / motor units
     motorCoordinatesPosition = position * motorRecResolution;
+    motorVelocity = maxVelocity * motorRecResolution;
 
     asynPrint(pC_->pasynUserSelf, ASYN_TRACE_FLOW,
               "%s => line %d:\nStart of axis %d to position %lf.\n",
               __PRETTY_FUNCTION__, __LINE__, axisNo_, position);
 
-    // Perform handshake, Set target position and start the move command
+    // Check if the speed is allowed to be changed
+    pl_status = pC_->getIntegerParam(axisNo_, pC_->motorCanSetSpeed_,
+                                     &motorCanSetSpeed);
+    if (pl_status != asynSuccess) {
+        return pC_->paramLibAccessFailed(pl_status, "motorCanSetSpeed_",
+                                         __PRETTY_FUNCTION__, __LINE__);
+    }
+
+    // Prepend the new motor speed, if the user is allowed to set the speed.
+    // Mind the " " (space) before the closing "", as the command created here
+    // is prepended to the one down below.
+    if (motorCanSetSpeed != 0) {
+        snprintf(command, sizeof(command), "Q%2.2d04=%lf ", axisNo_,
+                 motorVelocity);
+        writeOffset = strlen(command);
+
+        asynPrint(pC_->pasynUserSelf, ASYN_TRACE_FLOW,
+                  "%s => line %d:\nSetting speed of axis %d to %lf.\n",
+                  __PRETTY_FUNCTION__, __LINE__, axisNo_, motorVelocity);
+    }
+
+    // Perform handshake, Set target position (and speed, if allowed) and start
+    // the move command
     if (relative) {
-        snprintf(command, sizeof(command), "P%2.2d23=0 Q%2.2d02=%lf M%2.2d=2",
-                 axisNo_, axisNo_, motorCoordinatesPosition, axisNo_);
+        snprintf(&command[writeOffset], sizeof(command) - writeOffset,
+                 "P%2.2d23=0 Q%2.2d02=%lf M%2.2d=2", axisNo_, axisNo_,
+                 motorCoordinatesPosition, axisNo_);
     } else {
-        snprintf(command, sizeof(command), "P%2.2d23=0 Q%2.2d01=%lf M%2.2d=1",
-                 axisNo_, axisNo_, motorCoordinatesPosition, axisNo_);
+        snprintf(&command[writeOffset], sizeof(command) - writeOffset,
+                 "P%2.2d23=0 Q%2.2d01=%lf M%2.2d=1", axisNo_, axisNo_,
+                 motorCoordinatesPosition, axisNo_);
     }
 
     // We don't expect an answer
@@ -864,12 +882,8 @@ asynStatus pmacv3Axis::readEncoderType() {
 
     int reponse_length = strlen(response);
     if (reponse_length < 3) {
-        asynPrint(
-            pC_->pasynUserSelf, ASYN_TRACE_ERROR,
-            "%s => line %d:\nUnexpected reponse '%s' from axis %d on "
-            "controller %s while reading the encoder type. Aborting....\n",
-            __PRETTY_FUNCTION__, __LINE__, response, axisNo_, pC_->portName);
-        return asynError;
+        return pC_->errMsgCouldNotParseResponse(command, response, axisNo_,
+                                                __PRETTY_FUNCTION__, __LINE__);
     }
 
     // We are only interested in the last two digits and the last value in
@@ -936,25 +950,18 @@ asynStatus pmacv3Axis::rereadEncoder() {
     }
 
     // Abort if the axis is incremental
-    if (strcmp(encoderType, IncrementalEncoder) == 1) {
-        asynPrint(pC_->pasynUserSelf, ASYN_TRACE_WARNING,
-                  "%s => line %d:\nTrying to reread absolute encoder of "
-                  "axis %d on controller %s, but it is a relative encoder.\n",
-                  __PRETTY_FUNCTION__, __LINE__, axisNo_, pC_->portName);
-        pl_status = setStringParam(pC_->motorMessageText_,
-                                   "Cannot reread an incremental encoder.");
-        if (pl_status != asynSuccess) {
-            return pC_->paramLibAccessFailed(pl_status, "motorMessageText_",
-                                             __PRETTY_FUNCTION__, __LINE__);
-        }
-
-        return asynError;
+    if (strcmp(encoderType, IncrementalEncoder) == 0) {
+        asynPrint(pC_->pasynUserSelf, ASYN_TRACE_FLOW,
+                  "%s => line %d:\nEncoder of axis %d is not reread because it "
+                  "is incremental.\n",
+                  __PRETTY_FUNCTION__, __LINE__, axisNo_);
+        return asynSuccess;
     }
 
     // Check if the axis is disabled. If not, inform the user that this
     // is necessary
     int enabled = 0;
-    pl_status = pC_->getIntegerParam(axisNo_, pC_->enableMotorRBV_, &enabled);
+    pl_status = pC_->getIntegerParam(axisNo_, pC_->motorEnableRBV_, &enabled);
     if (pl_status != asynSuccess) {
         return pC_->paramLibAccessFailed(pl_status, "enableMotorRBV_",
                                          __PRETTY_FUNCTION__, __LINE__);
@@ -1035,13 +1042,6 @@ asynStatus pmacv3Axis::enable(bool on) {
                                              __PRETTY_FUNCTION__, __LINE__);
         }
 
-        // Reset the value in the param lib.
-        pl_status = setIntegerParam(pC_->enableMotor_, 1);
-        if (pl_status != asynSuccess) {
-            return pC_->paramLibAccessFailed(pl_status, "enableMotor_",
-                                             __PRETTY_FUNCTION__, __LINE__);
-        }
-
         return asynError;
     }
 
@@ -1055,6 +1055,14 @@ asynStatus pmacv3Axis::enable(bool on) {
         return asynSuccess;
     }
 
+    // Reread the encoder, if the axis is going to be enabled
+    if (on != 0) {
+        rw_status = rereadEncoder();
+        if (rw_status != asynSuccess) {
+            return rw_status;
+        }
+    }
+
     // Enable / disable the axis if it is not moving
     snprintf(command, sizeof(command), "M%2.2d14=%d", axisNo_, on);
     asynPrint(pC_->pasynUserSelf, ASYN_TRACE_FLOW,
@@ -1070,7 +1078,6 @@ asynStatus pmacv3Axis::enable(bool on) {
         return pC_->paramLibAccessFailed(pl_status, "motorMessageText_",
                                          __PRETTY_FUNCTION__, __LINE__);
     }
-
     rw_status = pC_->writeRead(axisNo_, command, response, 0);
     if (rw_status != asynSuccess) {
         return rw_status;
@@ -1120,8 +1127,3 @@ asynStatus pmacv3Axis::enable(bool on) {
     }
     return asynError;
 }
-
-asynStatus pmacv3Axis::isEnabled(bool *on) {
-    *on = (axisStatus_ != -3 && axisStatus_ != -5);
-    return asynSuccess;
-}

src/pmacv3Axis.h

@@ -88,15 +88,6 @@ class pmacv3Axis : public sinqAxis {
      */
     asynStatus enable(bool on);
 
-    /**
-     * @brief EThis function sets "on" to true, if the motor is enabled, and to
-     * false otherwise
-     *
-     * @param on
-     * @return asynStatus
-     */
-    asynStatus isEnabled(bool *on);
-
     /**
      * @brief Read the encoder type (incremental or absolute) for this axis from
      * the MCU and store the information in the PV ENCODER_TYPE.
@@ -115,7 +106,6 @@ class pmacv3Axis : public sinqAxis {
   protected:
     pmacv3Controller *pC_;
 
-    asynStatus readConfig();
     bool initial_poll_;
     bool waitForHandshake_;
     time_t timeAtHandshake_;

src/pmacv3Controller.cpp

@@ -1,4 +1,3 @@
-
 #include "pmacv3Controller.h"
 #include "asynMotorController.h"
 #include "asynOctetSyncIO.h"
@@ -11,6 +10,24 @@
 #include <string.h>
 #include <unistd.h>
 
+/**
+ * @brief Copy src into dst and replace all carriage returns with spaces. This
+ * allows to print *dst with asynPrint.
+ *
+ *
+ * @param dst                 Buffer for the modified string
+ * @param src                 Original string
+ */
+void adjustResponseForPrint(char *dst, const char *src, size_t buf_length) {
+    for (size_t i = 0; i < buf_length; i++) {
+        if (src[i] == '\r') {
+            dst[i] = ' ';
+        } else {
+            dst[i] = src[i];
+        }
+    }
+}
+
 /**
  * @brief Construct a new pmacv3Controller::pmacv3Controller object
  *
@@ -31,13 +48,10 @@ pmacv3Controller::pmacv3Controller(const char *portName,
           portName, ipPortConfigName, numAxes, movingPollPeriod, idlePollPeriod,
           /*
           The following parameter library entries are added in this driver:
-          - ENCODER_TYPE
           - REREAD_ENCODER_POSITION
           - READ_CONFIG
-          - MOTOR_VELOCITY_FROM_DRIVER
-          - MOTOR_ACCEL_FROM_DRIVER
           */
-          5)
+          NUM_PMACV3_DRIVER_PARAMS)
 
 {
 
@@ -66,15 +80,6 @@ pmacv3Controller::pmacv3Controller(const char *portName,
     // =========================================================================
     // Create additional parameter library entries
 
-    status = createParam("ENCODER_TYPE", asynParamOctet, &encoderType_);
-    if (status != asynSuccess) {
-        asynPrint(this->pasynUserSelf, ASYN_TRACE_ERROR,
-                  "%s => line %d:\nFATAL ERROR (creating a parameter failed "
-                  "with %s).\nTerminating IOC",
-                  __PRETTY_FUNCTION__, __LINE__, stringifyAsynStatus(status));
-        exit(-1);
-    }
-
     status = createParam("REREAD_ENCODER_POSITION", asynParamInt32,
                          &rereadEncoderPosition_);
     if (status != asynSuccess) {
@@ -94,26 +99,6 @@ pmacv3Controller::pmacv3Controller(const char *portName,
         exit(-1);
     }
 
-    status = createParam("MOTOR_VELOCITY_FROM_DRIVER", asynParamFloat64,
-                         &motorVelocityRBV_);
-    if (status != asynSuccess) {
-        asynPrint(this->pasynUserSelf, ASYN_TRACE_ERROR,
-                  "%s => line %d:\nFATAL ERROR (creating a parameter failed "
-                  "with %s).\nTerminating IOC",
-                  __PRETTY_FUNCTION__, __LINE__, stringifyAsynStatus(status));
-        exit(-1);
-    }
-
-    status = createParam("MOTOR_ACCEL_FROM_DRIVER", asynParamFloat64,
-                         &motorAccelRBV_);
-    if (status != asynSuccess) {
-        asynPrint(this->pasynUserSelf, ASYN_TRACE_ERROR,
-                  "%s => line %d:\nFATAL ERROR (creating a parameter failed "
-                  "with %s).\nTerminating IOC",
-                  __PRETTY_FUNCTION__, __LINE__, stringifyAsynStatus(status));
-        exit(-1);
-    }
-
     /*
     Define the end-of-string of a message coming from the device to EPICS.
     It is not necessary to append a terminator to outgoing messages, since
@@ -192,8 +177,9 @@ asynStatus pmacv3Controller::writeRead(int axisNo, const char *command,
     // Definition of local variables.
     asynStatus status = asynSuccess;
     asynStatus pl_status = asynSuccess;
-    char full_command[MAXBUF_] = {0};
-    char user_message[MAXBUF_] = {0};
+    char fullCommand[MAXBUF_] = {0};
+    char drvMessageText[MAXBUF_] = {0};
+    char modResponse[MAXBUF_] = {0};
     int motorStatusProblem = 0;
     int numReceivedResponses = 0;
 
@@ -232,7 +218,7 @@ asynStatus pmacv3Controller::writeRead(int axisNo, const char *command,
     this protocol encodes the message length at the beginning. See Turbo PMAC
     User Manual, page 418 in VR_PMAC_GETRESPONSE
 
-    The message has to be build manually into the buffer full_command, since it
+    The message has to be build manually into the buffer fullCommand, since it
     contains NULL terminators in its middle, therefore the string manipulation
     methods of C don't work.
     */
@@ -242,20 +228,20 @@ asynStatus pmacv3Controller::writeRead(int axisNo, const char *command,
         strlen(command) + 1; // +1 because of the appended /r
     const int offset = 8;
 
-    // Positions 2 to 6 must have the value 0. Since full_command is initialized
+    // Positions 2 to 6 must have the value 0. Since fullCommand is initialized
     // as an array of zeros, we don't need to set these bits manually.
-    full_command[0] = '\x40';
-    full_command[1] = '\xBF';
-    full_command[7] = commandLength;
+    fullCommand[0] = '\x40';
+    fullCommand[1] = '\xBF';
+    fullCommand[7] = commandLength;
 
-    snprintf((char *)full_command + offset, MAXBUF_ - offset, "%s\r", command);
+    snprintf((char *)fullCommand + offset, MAXBUF_ - offset, "%s\r", command);
     asynPrint(this->pasynUserSelf, ASYN_TRACEIO_DRIVER,
               "%s => line %d:\nSending command %s", __PRETTY_FUNCTION__,
-              __LINE__, full_command);
+              __LINE__, fullCommand);
 
     // Perform the actual writeRead
     status = pasynOctetSyncIO->writeRead(
-        lowLevelPortUser_, full_command, commandLength + offset, response,
+        lowLevelPortUser_, fullCommand, commandLength + offset, response,
         MAXBUF_, comTimeout_, &nbytesOut, &nbytesIn, &eomReason);
 
     /*
@@ -291,7 +277,7 @@ asynStatus pmacv3Controller::writeRead(int axisNo, const char *command,
         if (status == asynSuccess) {
             // If flushing the MCU succeded, try to send the command again
             status = pasynOctetSyncIO->writeRead(
-                lowLevelPortUser_, full_command, commandLength + offset,
+                lowLevelPortUser_, fullCommand, commandLength + offset,
                 response, MAXBUF_, comTimeout_, &nbytesOut, &nbytesIn,
                 &eomReason);
 
@@ -306,15 +292,18 @@ asynStatus pmacv3Controller::writeRead(int axisNo, const char *command,
 
             // Second check: If this fails, give up and propagate the error.
             if (numExpectedResponses != numReceivedResponses) {
-                asynPrint(
-                    this->pasynUserSelf, ASYN_TRACE_ERROR,
-                    "%s => line %d:\nUnexpected response %s for command %s\n",
-                    __PRETTY_FUNCTION__, __LINE__, response, command);
-                snprintf(user_message, sizeof(user_message),
-                         "Received unexpected response %s for command %s. "
+
+                adjustResponseForPrint(modResponse, response, MAXBUF_);
+                asynPrint(this->pasynUserSelf, ASYN_TRACE_ERROR,
+                          "%s => line %d:\nUnexpected response '%s' (carriage "
+                          "returns are replaced with spaces) for command %s\n",
+                          __PRETTY_FUNCTION__, __LINE__, modResponse, command);
+                snprintf(drvMessageText, sizeof(drvMessageText),
+                         "Received unexpected response '%s' (carriage returns "
+                         "are replaced with spaces) for command %s. "
                          "Please call the support",
-                         response, command);
-                pl_status = setStringParam(motorMessageText_, user_message);
+                         modResponse, command);
+                pl_status = setStringParam(motorMessageText_, drvMessageText);
                 if (pl_status != asynSuccess) {
                     return paramLibAccessFailed(pl_status, "motorMessageText_",
                                                 __PRETTY_FUNCTION__, __LINE__);
@@ -330,34 +319,47 @@ asynStatus pmacv3Controller::writeRead(int axisNo, const char *command,
     }
 
     // Create custom error messages for different failure modes
-    if (strlen(user_message) == 0) {
+    if (strlen(drvMessageText) == 0) {
         switch (status) {
         case asynSuccess:
             break; // Communicate nothing
         case asynTimeout:
-            snprintf(user_message, sizeof(user_message),
+            snprintf(drvMessageText, sizeof(drvMessageText),
                      "connection timeout for axis %d", axisNo);
             break;
         case asynDisconnected:
-            snprintf(user_message, sizeof(user_message),
+            snprintf(drvMessageText, sizeof(drvMessageText),
                      "axis is not connected");
             break;
         case asynDisabled:
-            snprintf(user_message, sizeof(user_message), "axis is disabled");
+            snprintf(drvMessageText, sizeof(drvMessageText),
+                     "axis is disabled");
             break;
         default:
-            snprintf(user_message, sizeof(user_message),
+            snprintf(drvMessageText, sizeof(drvMessageText),
                      "Communication failed (%s)", stringifyAsynStatus(status));
             break;
         }
     }
 
-    if (status != asynSuccess) {
+    // Log the overall status (communication successfull or not)
+    if (status == asynSuccess) {
+
+        asynPrint(lowLevelPortUser_, ASYN_TRACEIO_DRIVER,
+                  "%s => line %d:\nDevice response: %s (_ are "
+                  "carriage returns)\n",
+                  __PRETTY_FUNCTION__, __LINE__, modResponse);
+        pl_status = axis->setIntegerParam(this->motorStatusCommsError_, 0);
+    } else {
+        asynPrint(lowLevelPortUser_, ASYN_TRACE_ERROR,
+                  "%s => line %d:\nCommunication failed for command %s (%s)\n",
+                  __PRETTY_FUNCTION__, __LINE__, fullCommand,
+                  stringifyAsynStatus(status));
+
         // Check if the axis already is in an error communication mode. If it is
         // not, upstream the error. This is done to avoid "flooding" the user
         // with different error messages if more than one error ocurred before
         // an error-free communication
-
         pl_status =
             getIntegerParam(axisNo, motorStatusProblem_, &motorStatusProblem);
         if (pl_status != asynSuccess) {
@@ -366,37 +368,26 @@ asynStatus pmacv3Controller::writeRead(int axisNo, const char *command,
         }
 
         if (motorStatusProblem == 0) {
-            pl_status =
-                axis->setStringParam(this->motorMessageText_, user_message);
+            pl_status = axis->setStringParam(motorMessageText_, drvMessageText);
             if (pl_status != asynSuccess) {
                 return paramLibAccessFailed(pl_status, "motorMessageText_",
                                             __PRETTY_FUNCTION__, __LINE__);
             }
+
+            pl_status = axis->setIntegerParam(motorStatusProblem_, 1);
+            if (pl_status != asynSuccess) {
+                return paramLibAccessFailed(pl_status, "motorStatusProblem",
+                                            __PRETTY_FUNCTION__, __LINE__);
+            }
+
+            pl_status = axis->setIntegerParam(motorStatusProblem_, 1);
+            if (pl_status != asynSuccess) {
+                return paramLibAccessFailed(pl_status, "motorStatusCommsError_",
+                                            __PRETTY_FUNCTION__, __LINE__);
+            }
         }
     }
-
-    // Log the overall status (communication successfull or not)
-    if (status == asynSuccess) {
-        asynPrint(lowLevelPortUser_, ASYN_TRACEIO_DRIVER,
-                  "%s => line %d:\nDevice response: %s\n", __PRETTY_FUNCTION__,
-                  __LINE__, response);
-        pl_status = axis->setIntegerParam(this->motorStatusCommsError_, 0);
-    } else {
-        if (status == asynSuccess) {
-            asynPrint(
-                lowLevelPortUser_, ASYN_TRACE_ERROR,
-                "%s => line %d:\nCommunication failed for command %s (%s)\n",
-                __PRETTY_FUNCTION__, __LINE__, full_command,
-                stringifyAsynStatus(status));
-            pl_status = axis->setIntegerParam(this->motorStatusCommsError_, 1);
-        }
-
-        if (pl_status != asynSuccess) {
-            return paramLibAccessFailed(pl_status, "motorStatusCommsError_",
-                                        __PRETTY_FUNCTION__, __LINE__);
-        }
-    }
-    return asynSuccess;
+    return status;
 }
 
 asynStatus pmacv3Controller::writeInt32(asynUser *pasynUser, epicsInt32 value) {
@@ -414,12 +405,31 @@ asynStatus pmacv3Controller::writeInt32(asynUser *pasynUser, epicsInt32 value) {
     if (function == rereadEncoderPosition_) {
         return axis->rereadEncoder();
     } else if (function == readConfig_) {
-        return axis->readConfig();
+        return axis->atFirstPoll();
     } else {
         return sinqController::writeInt32(pasynUser, value);
     }
 }
 
+asynStatus sinqController::readInt32(asynUser *pasynUser, epicsInt32 *value) {
+    // PMACs can be disabled
+    if (pasynUser->reason == motorCanDisable_) {
+        *value = 1;
+        return asynSuccess;
+    } else {
+        return asynMotorController::readInt32(pasynUser, value);
+    }
+}
+
+asynStatus pmacv3Controller::errMsgCouldNotParseResponse(
+    const char *command, const char *response, int axisNo,
+    const char *functionName, int lineNumber) {
+    char modifiedResponse[MAXBUF_] = {0};
+    adjustResponseForPrint(modifiedResponse, response, MAXBUF_);
+    return sinqController::errMsgCouldNotParseResponse(
+        command, modifiedResponse, axisNo, functionName, lineNumber);
+}
+
 /*************************************************************************************/
 /** The following functions are C-wrappers, and can be called directly from
  * iocsh */
@@ -431,7 +441,7 @@ C wrapper for the controller constructor. Please refer to the pmacv3Controller
 constructor documentation.
 */
 asynStatus pmacv3CreateController(const char *portName,
-                                  const char *lowLevelPortName, int numAxes,
+                                  const char *ipPortConfigName, int numAxes,
                                   double movingPollPeriod,
                                   double idlePollPeriod, double comTimeout) {
     /*
@@ -446,7 +456,7 @@ asynStatus pmacv3CreateController(const char *portName,
 #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
 #pragma GCC diagnostic ignored "-Wunused-variable"
     pmacv3Controller *pController =
-        new pmacv3Controller(portName, lowLevelPortName, numAxes,
+        new pmacv3Controller(portName, ipPortConfigName, numAxes,
                              movingPollPeriod, idlePollPeriod, comTimeout);
 
     return asynSuccess;
@@ -533,9 +543,9 @@ types and then providing "factory" functions
 (configCreateControllerCallFunc). These factory functions are used to
 register the constructors during compilation.
 */
-static const iocshArg CreateControllerArg0 = {"Controller port name",
+static const iocshArg CreateControllerArg0 = {"Controller name (e.g. mcu1)",
                                               iocshArgString};
-static const iocshArg CreateControllerArg1 = {"Low level port name",
+static const iocshArg CreateControllerArg1 = {"Asyn IP port name (e.g. pmcu1)",
                                               iocshArgString};
 static const iocshArg CreateControllerArg2 = {"Number of axes", iocshArgInt};
 static const iocshArg CreateControllerArg3 = {"Moving poll rate (s)",
@@ -558,7 +568,8 @@ static void configPmacV3CreateControllerCallFunc(const iocshArgBuf *args) {
 Same procedure as for the CreateController function, but for the axis
 itself.
 */
-static const iocshArg CreateAxisArg0 = {"Controller port name", iocshArgString};
+static const iocshArg CreateAxisArg0 = {"Controller name (e.g. mcu1)",
+                                        iocshArgString};
 static const iocshArg CreateAxisArg1 = {"Axis number", iocshArgInt};
 static const iocshArg *const CreateAxisArgs[] = {&CreateAxisArg0,
                                                  &CreateAxisArg1};

src/pmacv3Controller.h

@@ -12,9 +12,6 @@
 #include "sinqAxis.h"
 #include "sinqController.h"
 
-#define IncrementalEncoder "Incremental encoder"
-#define AbsoluteEncoder "Absolute encoder"
-
 class pmacv3Controller : public sinqController {
 
   public:
@@ -51,10 +48,9 @@ class pmacv3Controller : public sinqController {
     pmacv3Axis *getAxis(int axisNo);
 
     /**
-     * @brief Overloaded function of asynMotorController
+     * @brief Overloaded function of sinqController
      *
-     * The function is overloaded to allow enabling / disabling the motor and
-     * rereading the encoder.
+     * The function is overloaded to allow rereading the encoder and config.
      *
      * @param pasynUser           Specify the axis via the asynUser
      * @param value               New value
@@ -93,6 +89,29 @@ class pmacv3Controller : public sinqController {
      */
     pmacv3Axis *castToAxis(asynMotorAxis *asynAxis);
 
+    /**
+     * @brief Specialized version of sinqController::errMsgCouldNotParseResponse
+     * for pmacv3
+     *
+     * This is an overloaded version of
+     * sinqController::errMsgCouldNotParseResponse which calls
+     * adjustResponseForLogging on response before handing it over to
+     * sinqController::errMsgCouldNotParseResponse.
+     *
+     * @param command             Command which led to the unparseable message
+     * @param response            Response which wasn't parseable
+     * @param axisNo_             Axis where the problem occurred
+     * @param functionName        Name of the caller function. It is recommended
+    to use a macro, e.g. __func__ or __PRETTY_FUNCTION__.
+     * @param lineNumber          Source code line where this function is
+    called. It is recommended to use a macro, e.g. __LINE__.
+     * @return asynStatus         Returns asynError.
+     */
+    asynStatus errMsgCouldNotParseResponse(const char *command,
+                                           const char *response, int axisNo_,
+                                           const char *functionName,
+                                           int lineNumber);
+
   private:
     // Set the maximum buffer size. This is an empirical value which must be
     // large enough to avoid overflows for all commands to the device /
@@ -105,18 +124,13 @@ class pmacv3Controller : public sinqController {
     double comTimeout_;
 
     // Indices of additional PVs
+#define FIRST_PMACV3_PARAM rereadEncoderPosition_
     int rereadEncoderPosition_;
     int readConfig_;
-    int encoderType_;
-
-    /*
-    Same strategy as with the limits in sinqController -> Use additional PVs to
-    write speed and acceleration from the driver to the record.
-     */
-    int motorVelocityRBV_;
-    int motorAccelRBV_;
+#define LAST_PMACV3_PARAM readConfig_
 
     friend class pmacv3Axis;
 };
+#define NUM_PMACV3_DRIVER_PARAMS (&LAST_PMACV3_PARAM - &FIRST_PMACV3_PARAM + 1)
 
 #endif /* pmacv3Controller_H */