Moved error handling out of error read condition.

Previously, error messaging was only done after the error has been read.
This means that cached errors were simply ignored, if e.g. the motor was
moving. This commit now messages an error as long as it exists in the
cache "masterMacsAxisImpl->axisError".

.gitea/workflows/action.yaml

@@ -0,0 +1,24 @@
+name: Test And Build
+on: [push]
+
+jobs:
+  Lint:
+    runs-on: linepics
+    steps:
+      - name: checkout repo
+        uses: actions/checkout@v4
+      - name: cppcheck
+        run: cppcheck --std=c++17 --addon=cert --addon=misc --error-exitcode=1 src/*.cpp
+      - name: formatting
+        run: clang-format --style=file --Werror --dry-run src/*.cpp
+  Build:
+    runs-on: linepics
+    steps:
+      - name: checkout repo
+        uses: actions/checkout@v4
+        with:
+          submodules: 'true'
+      - run: |
+          sed -i 's/ARCH_FILTER=.*/ARCH_FILTER=linux%/' Makefile
+          echo -e "\nIGNORE_SUBMODULES += sinqmotor" >> Makefile
+          make install

.gitlab-ci.yml

@@ -1,48 +0,0 @@
-default:
-  image: docker.psi.ch:5000/sinqdev/sinqepics:latest
-
-stages:
-  - lint
-  - build
-  - test
-
-cppcheck:
-  stage: lint
-  script:
-    - cppcheck --std=c++17 --addon=cert --addon=misc --suppress=cert-STR07-C --error-exitcode=1 src/*.cpp
-  artifacts:
-    expire_in: 1 week
-  tags:
-    - sinq
-
-formatting:
-  stage: lint
-  script:
-    - clang-format --style=file --Werror --dry-run src/*.cpp
-  artifacts:
-    expire_in: 1 week
-  tags:
-    - sinq
-
-build_module:
-  stage: build
-  script:
-    - export SINQMOTOR_VERSION="$(grep 'sinqMotor_VERSION=' Makefile | cut -d= -f2)"
-    - git clone --depth 1 --branch "${SINQMOTOR_VERSION}" https://gitlab-ci-token:${CI_JOB_TOKEN}@git.psi.ch/sinq-epics-modules/sinqmotor.git
-    - pushd sinqmotor
-    - sed -i 's/ARCH_FILTER=.*/ARCH_FILTER=linux%/' Makefile
-    - echo "LIBVERSION=${SINQMOTOR_VERSION}" >> Makefile
-    - make install
-    - popd
-    - sed -i 's/ARCH_FILTER=.*/ARCH_FILTER=linux%/' Makefile
-    - echo "LIBVERSION=${CI_COMMIT_TAG:-0.0.1}" >> Makefile
-    - make install
-    - cp -rT "/ioc/modules/masterMacs/$(ls -U /ioc/modules/masterMacs/ | head -1)" "./masterMacs-${CI_COMMIT_TAG:-$CI_COMMIT_SHORT_SHA}"
-  artifacts:
-    name: "masterMacs-${CI_COMMIT_TAG:-$CI_COMMIT_SHORT_SHA}"
-    paths:
-      - "masterMacs-${CI_COMMIT_TAG:-$CI_COMMIT_SHORT_SHA}/*"
-    expire_in: 1 week
-  when: always
-  tags:
-    - sinq

Makefile

@@ -6,9 +6,6 @@ BUILDCLASSES=Linux
 EPICS_VERSIONS=7.0.7
 ARCH_FILTER=RHEL%
 
-# Additional module dependencies
-REQUIRED+=sinqMotor
-
 # Specify the version of asynMotor we want to build against
 motorBase_VERSION=7.2.2
 

README.md

@@ -55,11 +55,11 @@ setMaxSubsequentTimeouts("$(NAME)", 20);
 setThresholdComTimeout("$(NAME)", 100, 1);
 
 # Parametrize the EPICS record database with the substitution file named after the MCU.
-epicsEnvSet("SINQDBPATH","$(sinqMotor_DB)/sinqMotor.db")
+epicsEnvSet("SINQDBPATH","$(masterMacs_DB)/sinqMotor.db")
 dbLoadTemplate("$(TOP)/$(NAME).substitutions", "INSTR=$(INSTR)$(NAME):,CONTROLLER=$(NAME)")
 epicsEnvSet("SINQDBPATH","$(masterMacs_DB)/masterMacs.db")
 dbLoadTemplate("$(TOP)/$(NAME).substitutions", "INSTR=$(INSTR)$(NAME):,CONTROLLER=$(NAME)")
-dbLoadRecords("$(sinqMotor_DB)/asynRecord.db","P=$(INSTR)$(NAME),PORT=$(ASYN_PORT)")
+dbLoadRecords("$(masterMacs_DB)/asynRecord.db","P=$(INSTR)$(NAME),PORT=$(ASYN_PORT)")
 ```
 
 ### Versioning

src/masterMacsAxis.h

@@ -1,12 +1,10 @@
 #ifndef masterMacsAXIS_H
 #define masterMacsAXIS_H
+#include "masterMacsController.h"
 #include "sinqAxis.h"
-#include <bitset>
+#include <memory>
 
-// Forward declaration of the controller class to resolve the cyclic dependency
-// between the controller and the axis .h-file. See
-// https://en.cppreference.com/w/cpp/language/class.
-class masterMacsController;
+struct masterMacsAxisImpl;
 
 class masterMacsAxis : public sinqAxis {
   public:
@@ -105,22 +103,25 @@ class masterMacsAxis : public sinqAxis {
      */
     asynStatus readEncoderType();
 
-  protected:
-    masterMacsController *pC_;
-    double lastSetSpeed_;
-    bool waitForHandshake_;
-    time_t timeAtHandshake_;
+    /**
+     * @brief Check if the axis needs to run its initialization function
+     *
+     * @return true
+     * @return false
+     */
+    bool needInit();
 
-    bool targetReachedUninitialized_;
+    /**
+     * @brief Instruct the axis to run its init() function during the next poll
+     *
+     * @param needInit
+     */
+    void setNeedInit(bool needInit);
 
-    asynStatus readConfig();
-
-    /*
-    The axis status and axis error of MasterMACS are given as an integer from
-    the controller. The 16 individual bits contain the actual information.
-    */
-    std::bitset<16> axisStatus_;
-    std::bitset<16> axisError_;
+    /**
+     * @brief Return a pointer to the axis controller
+     */
+    virtual masterMacsController *pController() override { return pC_; };
 
     /**
      * @brief Read the Master MACS status with the xR10 command and store the
@@ -130,7 +131,7 @@ class masterMacsAxis : public sinqAxis {
     asynStatus readAxisStatus();
 
     /*
-    The functions below read the specified status bit from the axisStatus_
+    The functions below read the specified status bit from the axisStatus
     bitset. Since a bit can either be 0 or 1, the return value is given as a
     boolean.
     */
@@ -138,68 +139,68 @@ class masterMacsAxis : public sinqAxis {
     /**
      * @brief Read the property from axisStatus_
      */
-    bool readyToBeSwitchedOn() { return axisStatus_[0]; }
+    bool readyToBeSwitchedOn();
 
     /**
      * @brief Read the property from axisStatus_
      */
-    bool switchedOn() { return axisStatus_[1]; }
+    bool switchedOn();
 
     // Bit 2 is unused
 
     /**
      * @brief Read the property from axisStatus_
      */
-    bool faultConditionSet() { return axisStatus_[3]; }
+    bool faultConditionSet();
 
     /**
      * @brief Read the property from axisStatus_
      */
-    bool voltagePresent() { return axisStatus_[4]; }
+    bool voltagePresent();
 
     /**
      * @brief Read the property from axisStatus_
      */
-    bool quickStopping() { return axisStatus_[5] == 0; }
+    bool quickStopping();
 
     /**
      * @brief Read the property from axisStatus_
      */
-    bool switchOnDisabled() { return axisStatus_[6]; }
+    bool switchOnDisabled();
 
     /**
      * @brief Read the property from axisStatus_
      */
-    bool warning() { return axisStatus_[7]; }
+    bool warning();
 
     // Bit 8 is unused
 
     /**
      * @brief Read the property from axisStatus_
      */
-    bool remoteMode() { return axisStatus_[9]; }
+    bool remoteMode();
 
     /**
      * @brief Read the property from axisStatus_
      */
-    bool targetReached() { return axisStatus_[10]; }
+    bool targetReached();
 
     /**
      * @brief Read the property from axisStatus_
      */
-    bool internalLimitActive() { return axisStatus_[11]; }
+    bool internalLimitActive();
 
     // Bits 12 and 13 are unused
 
     /**
      * @brief Read the property from axisStatus_
      */
-    bool setEventHasOcurred() { return axisStatus_[14]; }
+    bool setEventHasOcurred();
 
     /**
      * @brief Read the property from axisStatus_
      */
-    bool powerEnabled() { return axisStatus_[15]; }
+    bool powerEnabled();
 
     /**
      * @brief Read the Master MACS status with the xR10 command and store the
@@ -217,72 +218,76 @@ class masterMacsAxis : public sinqAxis {
     /**
      * @brief Read the property from axisError_
      */
-    bool shortCircuit() { return axisError_[1]; }
+    bool shortCircuit();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool encoderError() { return axisError_[2]; }
+    bool encoderError();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool followingError() { return axisError_[3]; }
+    bool followingError();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool communicationError() { return axisError_[4]; }
+    bool communicationError();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool feedbackError() { return axisError_[5]; }
+    bool feedbackError();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool positiveLimitSwitch() { return axisError_[6]; }
+    bool positiveLimitSwitch();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool negativeLimitSwitch() { return axisError_[7]; }
+    bool negativeLimitSwitch();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool positiveSoftwareLimit() { return axisError_[8]; }
+    bool positiveSoftwareLimit();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool negativeSoftwareLimit() { return axisError_[9]; }
+    bool negativeSoftwareLimit();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool overCurrent() { return axisError_[10]; }
+    bool overCurrent();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool overTemperature() { return axisError_[11]; }
+    bool overTemperature();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool overVoltage() { return axisError_[12]; }
+    bool overVoltage();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool underVoltage() { return axisError_[13]; }
+    bool underVoltage();
 
     /**
      * @brief Read the property from axisError_
      */
-    bool stoFault() { return axisError_[15]; }
+    bool stoFault();
+
+  private:
+    masterMacsController *pC_;
+    std::unique_ptr<masterMacsAxisImpl> pMasterMacsA_;
 };
 
 #endif

src/masterMacsController.cpp

@@ -12,6 +12,10 @@
 #include <string>
 #include <unistd.h>
 
+struct masterMacsControllerImpl {
+    double comTimeout;
+};
+
 /**
  * @brief Copy src into dst and replace all NULL terminators up to the carriage
  * return with spaces. This allows to print *dst with asynPrint.
@@ -59,8 +63,10 @@ masterMacsController::masterMacsController(const char *portName,
     // Initialization of local variables
     asynStatus status = asynSuccess;
 
-    // Initialization of all member variables
-    comTimeout_ = comTimeout;
+    pMasterMacsC_ =
+        std::make_unique<masterMacsControllerImpl>((masterMacsControllerImpl){
+            .comTimeout = comTimeout,
+        });
 
     // =========================================================================
 
@@ -130,7 +136,6 @@ asynStatus masterMacsController::writeRead(int axisNo, int tcpCmd,
 
     // Definition of local variables.
     asynStatus status = asynSuccess;
-    asynStatus pl_status = asynSuccess;
     char fullCommand[MAXBUF_] = {0};
     char fullResponse[MAXBUF_] = {0};
     char drvMessageText[MAXBUF_] = {0};
@@ -158,7 +163,7 @@ asynStatus masterMacsController::writeRead(int axisNo, int tcpCmd,
 
     // Check if a custom timeout has been given
     if (comTimeout < 0.0) {
-        comTimeout = comTimeout_;
+        comTimeout = pMasterMacsC_->comTimeout;
     }
 
     masterMacsAxis *axis = getMasterMacsAxis(axisNo);
@@ -192,7 +197,7 @@ asynStatus masterMacsController::writeRead(int axisNo, int tcpCmd,
     msgPrintControlKey comKey =
         msgPrintControlKey(portName, axisNo, __PRETTY_FUNCTION__, __LINE__);
     if (status != asynSuccess) {
-        if (msgPrintControl_.shouldBePrinted(comKey, true, pasynUserSelf)) {
+        if (getMsgPrintControl().shouldBePrinted(comKey, true, pasynUserSelf)) {
             char printableCommand[MAXBUF_] = {0};
             adjustForPrint(printableCommand, fullCommand, MAXBUF_);
             asynPrint(this->pasynUserSelf, ASYN_TRACE_ERROR,
@@ -202,7 +207,7 @@ asynStatus masterMacsController::writeRead(int axisNo, int tcpCmd,
                       stringifyAsynStatus(status), printableCommand);
         }
     } else {
-        msgPrintControl_.resetCount(comKey, pasynUserSelf);
+        getMsgPrintControl().resetCount(comKey, pasynUserSelf);
     }
 
     // Create custom error messages for different failure modes
@@ -249,41 +254,31 @@ asynStatus masterMacsController::writeRead(int axisNo, int tcpCmd,
 
     // Log the overall status (communication successfull or not)
     if (status == asynSuccess) {
-        pl_status = axis->setIntegerParam(this->motorStatusCommsError_, 0);
+        setAxisParamChecked(axis, motorStatusCommsError, false);
     } else {
 
-        // 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) {
-            return paramLibAccessFailed(pl_status, "motorStatusProblem_",
-                                        axisNo, __PRETTY_FUNCTION__, __LINE__);
-        }
+        /*
+        Since the communication failed, there is the possibility that the
+        controller is not connected at all to the network. In that case, we
+        cannot be sure that the information read out in the init method of the
+        axis is still up-to-date the next time we get a connection. Therefore,
+        an info flag is set which the axis object can use at the start of its
+        poll method to try to initialize itself.
+        */
+        axis->setNeedInit(true);
+
+        /*
+        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
+        */
+        getAxisParamChecked(axis, motorStatusProblem, &motorStatusProblem);
 
         if (motorStatusProblem == 0) {
-            pl_status = axis->setStringParam(motorMessageText_, drvMessageText);
-            if (pl_status != asynSuccess) {
-                return paramLibAccessFailed(pl_status, "motorMessageText_",
-                                            axisNo, __PRETTY_FUNCTION__,
-                                            __LINE__);
-            }
-
-            pl_status = axis->setIntegerParam(motorStatusProblem_, 1);
-            if (pl_status != asynSuccess) {
-                return paramLibAccessFailed(pl_status, "motorStatusProblem",
-                                            axisNo, __PRETTY_FUNCTION__,
-                                            __LINE__);
-            }
-
-            pl_status = axis->setIntegerParam(motorStatusProblem_, 1);
-            if (pl_status != asynSuccess) {
-                return paramLibAccessFailed(pl_status, "motorStatusCommsError_",
-                                            axisNo, __PRETTY_FUNCTION__,
-                                            __LINE__);
-            }
+            setAxisParamChecked(axis, motorMessageText, drvMessageText);
+            setAxisParamChecked(axis, motorStatusProblem, true);
+            setAxisParamChecked(axis, motorStatusCommsError, false);
         }
     }
 
@@ -313,13 +308,14 @@ asynStatus masterMacsController::parseResponse(
     msgPrintControlKey parseKey =
         msgPrintControlKey(portName, axisNo, __PRETTY_FUNCTION__, __LINE__);
 
-    // Was the motor previously connected?
-    status = getIntegerParam(axisNo, motorConnected(), &prevConnected);
-    if (status != asynSuccess) {
-        return paramLibAccessFailed(status, "motorConnected", axisNo,
-                                    __PRETTY_FUNCTION__, __LINE__);
+    masterMacsAxis *axis = getMasterMacsAxis(axisNo);
+    if (axis == nullptr) {
+        return asynError;
     }
 
+    // Was the motor previously connected?
+    getAxisParamChecked(axis, motorConnected, &prevConnected);
+
     // We don't use strlen here since the C string terminator 0x00
     // occurs in the middle of the char array.
     for (uint32_t i = 0; i < MAXBUF_; i++) {
@@ -342,16 +338,7 @@ asynStatus masterMacsController::parseResponse(
                           "connected.\n",
                           portName, axisNo, __PRETTY_FUNCTION__, __LINE__);
 
-                masterMacsAxis *axis = getMasterMacsAxis(axisNo);
-                if (axis == nullptr) {
-                    return asynError;
-                }
-                status = axis->setIntegerParam(motorConnected(), 1);
-                if (status != asynSuccess) {
-                    return paramLibAccessFailed(status, "motorConnected",
-                                                axisNo, __PRETTY_FUNCTION__,
-                                                __LINE__);
-                }
+                setAxisParamChecked(axis, motorConnected, true);
                 status = callParamCallbacks();
                 if (status != asynSuccess) {
                     asynPrint(this->pasynUserSelf, ASYN_TRACE_ERROR,
@@ -379,16 +366,7 @@ asynStatus masterMacsController::parseResponse(
                           "disconnected.\n",
                           portName, axisNo, __PRETTY_FUNCTION__, __LINE__);
 
-                masterMacsAxis *axis = getMasterMacsAxis(axisNo);
-                if (axis == nullptr) {
-                    return asynError;
-                }
-                status = axis->setIntegerParam(motorConnected(), 0);
-                if (status != asynSuccess) {
-                    return paramLibAccessFailed(status, "motorConnected",
-                                                axisNo, __PRETTY_FUNCTION__,
-                                                __LINE__);
-                }
+                setAxisParamChecked(axis, motorConnected, false);
                 status = callParamCallbacks();
                 if (status != asynSuccess) {
                     asynPrint(this->pasynUserSelf, ASYN_TRACE_ERROR,
@@ -405,15 +383,15 @@ asynStatus masterMacsController::parseResponse(
                      "Tried to write with a read-only command. This is a "
                      "bug, please call the support.");
 
-            if (msgPrintControl_.shouldBePrinted(parseKey, true,
-                                                 pasynUserSelf)) {
+            if (getMsgPrintControl().shouldBePrinted(parseKey, true,
+                                                     pasynUserSelf)) {
                 adjustForPrint(printableCommand, fullCommand, MAXBUF_);
                 asynPrint(
                     this->pasynUserSelf, ASYN_TRACE_ERROR,
                     "Controller \"%s\", axis %d => %s, line %d:\nTried to "
                     "write with the read-only command %s.%s\n",
                     portName, axisNo, __PRETTY_FUNCTION__, __LINE__,
-                    printableCommand, msgPrintControl_.getSuffix());
+                    printableCommand, getMsgPrintControl().getSuffix());
             }
             responseValid = false;
             break;
@@ -421,7 +399,7 @@ asynStatus masterMacsController::parseResponse(
     }
 
     if (responseValid) {
-        msgPrintControl_.resetCount(parseKey, pasynUserSelf);
+        getMsgPrintControl().resetCount(parseKey, pasynUserSelf);
 
         // Check if the response matches the expectations. Each response
         // contains the string "axisNo R tcpCmd" (including the spaces)
@@ -445,15 +423,15 @@ asynStatus masterMacsController::parseResponse(
             adjustForPrint(printableCommand, fullCommand, MAXBUF_);
             adjustForPrint(printableResponse, fullResponse, MAXBUF_);
 
-            if (msgPrintControl_.shouldBePrinted(parseKey, true,
-                                                 pasynUserSelf)) {
+            if (getMsgPrintControl().shouldBePrinted(parseKey, true,
+                                                     pasynUserSelf)) {
                 asynPrint(this->pasynUserSelf, ASYN_TRACEIO_DRIVER,
                           "Controller \"%s\", axis %d => %s, line "
                           "%d:\nMismatched "
                           "response %s to command %s.%s\n",
                           portName, axisNo, __PRETTY_FUNCTION__, __LINE__,
                           printableResponse, printableCommand,
-                          msgPrintControl_.getSuffix());
+                          getMsgPrintControl().getSuffix());
             }
 
             snprintf(drvMessageText, MAXBUF_,
@@ -462,7 +440,7 @@ asynStatus masterMacsController::parseResponse(
                      printableResponse, printableCommand);
             return asynError;
         } else {
-            msgPrintControl_.resetCount(responseMatchKey, pasynUserSelf);
+            getMsgPrintControl().resetCount(responseMatchKey, pasynUserSelf);
         }
     }
     return asynSuccess;
@@ -471,7 +449,7 @@ asynStatus masterMacsController::parseResponse(
 asynStatus masterMacsController::readInt32(asynUser *pasynUser,
                                            epicsInt32 *value) {
     // masterMacs can be disabled
-    if (pasynUser->reason == motorCanDisable_) {
+    if (pasynUser->reason == motorCanDisable()) {
         *value = 1;
         return asynSuccess;
     } else {
@@ -479,6 +457,8 @@ asynStatus masterMacsController::readInt32(asynUser *pasynUser,
     }
 }
 
+double masterMacsController::comTimeout() { return pMasterMacsC_->comTimeout; }
+
 /***************************************************************************/
 /** The following functions are C-wrappers, and can be called directly from
  * iocsh */

src/masterMacsController.h

@@ -8,9 +8,16 @@
 
 #ifndef masterMacsController_H
 #define masterMacsController_H
-#include "masterMacsAxis.h"
 #include "sinqAxis.h"
 #include "sinqController.h"
+#include <memory>
+
+// Forward declaration of the controller class to resolve the cyclic dependency
+// between the controller and the axis .h-file. See
+// https://en.cppreference.com/w/cpp/language/class.
+class masterMacsAxis;
+
+struct masterMacsControllerImpl;
 
 class masterMacsController : public sinqController {
 
@@ -132,13 +139,10 @@ class masterMacsController : public sinqController {
      *
      * @return double Timeout in seconds
      */
-    double comTimeout() { return comTimeout_; }
+    double comTimeout();
 
   private:
-    /*
-    Stores the constructor input comTimeout
-    */
-    double comTimeout_;
+    std::unique_ptr<masterMacsControllerImpl> pMasterMacsC_;
 };
 
 #endif /* masterMacsController_H */