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

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.cpp

@@ -19,7 +19,6 @@ struct masterMacsAxisImpl {
     std::bitset<16> axisStatus;
     std::bitset<16> axisError;
 
-    double lastSetSpeed;
     bool waitForHandshake;
     time_t timeAtHandshake;
     bool needInit = true;
@@ -122,7 +121,6 @@ masterMacsAxis::masterMacsAxis(masterMacsController *pC, int axisNo)
     pMasterMacsA_ = std::make_unique<masterMacsAxisImpl>((masterMacsAxisImpl){
         .axisStatus = std::bitset<16>(0),
         .axisError = std::bitset<16>(0),
-        .lastSetSpeed = 0.0,
         .waitForHandshake = false,
         .timeAtHandshake = 0,
         .targetReachedUninitialized = true,
@@ -262,11 +260,6 @@ asynStatus masterMacsAxis::init() {
                                           __PRETTY_FUNCTION__, __LINE__);
     }
 
-    // Cache the motor speed. If this value differs from the one in the motor
-    // record at the start of a movement, the motor record value is sent to
-    // MasterMACS.
-    pMasterMacsA_->lastSetSpeed = motorVelocity;
-
     // Store the motor position in the parameter library
     pl_status = setMotorPosition(motorPosition);
     if (pl_status != asynSuccess) {
@@ -439,184 +432,181 @@ asynStatus masterMacsAxis::doPoll(bool *moving) {
     Read out the error if either a fault condition status flag has been set or
     if a movement has just ended.
      */
+    if (faultConditionSet() || !(*moving)) {
+        rw_status = readAxisError();
+    }
+
     msgPrintControlKey keyError = msgPrintControlKey(
         pC_->portName, axisNo_, __PRETTY_FUNCTION__, __LINE__);
 
-    if (faultConditionSet() || !(*moving)) {
-        rw_status = readAxisError();
+    /*
+    A communication error is a special case. If a communication between
+    controller and axis error occurred, all subsequent errors are ignored,
+    since this information is not reliable.
+     */
+    if (communicationError()) {
+        if (pC_->getMsgPrintControl().shouldBePrinted(keyError, true,
+                                                      pC_->pasynUser())) {
+            asynPrint(pC_->pasynUser(), ASYN_TRACE_ERROR,
+                      "Controller \"%s\", axis %d  => %s, line "
+                      "%d\nCommunication error.%s\n",
+                      pC_->portName, axisNo_, __PRETTY_FUNCTION__, __LINE__,
+                      pC_->getMsgPrintControl().getSuffix());
+        }
+
+        setAxisParamChecked(this, motorMessageText,
+                            "Communication error between PC and motor "
+                            "controller. Please call the support.");
+
+        poll_status = asynError;
+    } else {
+
+        // This buffer must be initialized to zero because we build the
+        // error message by appending strings.
+        char errorMessage[pC_->MAXBUF_] = {0};
+        char shellMessage[pC_->MAXBUF_] = {0};
+
+        // Concatenate all other errors
+        if (shortCircuit()) {
+            appendErrorMessage(shellMessage, sizeof(shellMessage),
+                               "Short circuit fault.");
+            appendErrorMessage(errorMessage, sizeof(errorMessage),
+                               "Short circuit error. Please call the support.");
+
+            poll_status = asynError;
+        }
+
+        if (encoderError()) {
+            appendErrorMessage(shellMessage, sizeof(shellMessage),
+                               "Encoder error.");
+            appendErrorMessage(errorMessage, sizeof(errorMessage),
+                               "Encoder error. Please call the support.");
+
+            poll_status = asynError;
+        }
+
+        if (followingError()) {
+            appendErrorMessage(shellMessage, sizeof(shellMessage),
+                               "Maximum callowed following error exceeded.");
+            appendErrorMessage(
+                errorMessage, sizeof(errorMessage),
+                "Maximum allowed following error exceeded.Check if "
+                "movement range is blocked. Otherwise please call the "
+                "support.");
+
+            poll_status = asynError;
+        }
+
+        if (feedbackError()) {
+            appendErrorMessage(shellMessage, sizeof(shellMessage),
+                               "Feedback error.");
+            appendErrorMessage(errorMessage, sizeof(errorMessage),
+                               "Feedback error. Please call the support.");
+
+            poll_status = asynError;
+        }
 
         /*
-        A communication error is a special case. If a communication between
-        controller and axis error occurred, all subsequent errors are ignored,
-        since this information is not reliable.
-         */
-        if (communicationError()) {
+        Either the software limits or the end switches of the controller
+        have been hit. Since the EPICS limits are derived from the software
+        limits and are a little bit smaller, these error cases can only
+        happen if either the axis has an incremental encoder which is not
+        properly homed or if a bug occured.
+        */
+        if (positiveLimitSwitch() || negativeLimitSwitch() ||
+            positiveSoftwareLimit() || negativeSoftwareLimit()) {
+
+            // Distinction for developers
+            if (positiveLimitSwitch()) {
+                appendErrorMessage(shellMessage, sizeof(shellMessage),
+                                   "Positive limit switch.");
+            }
+            if (negativeLimitSwitch()) {
+                appendErrorMessage(shellMessage, sizeof(shellMessage),
+                                   "Negative limit switch.");
+            }
+            if (positiveSoftwareLimit()) {
+                appendErrorMessage(shellMessage, sizeof(shellMessage),
+                                   "Positive software limit.");
+            }
+            if (negativeSoftwareLimit()) {
+                appendErrorMessage(shellMessage, sizeof(shellMessage),
+                                   "Negative software limit.");
+            }
+
+            // Generic error message for user
+            appendErrorMessage(
+                errorMessage, sizeof(errorMessage),
+                "Software limits or end switch hit. Try homing the motor, "
+                "moving in the opposite direction or check the SPS for "
+                "errors (if available). Otherwise please call the "
+                "support.");
+
+            poll_status = asynError;
+        }
+
+        if (overCurrent()) {
+            appendErrorMessage(shellMessage, sizeof(shellMessage),
+                               "Overcurrent error.");
+            appendErrorMessage(errorMessage, sizeof(errorMessage),
+                               "Overcurrent error. Please call the support.");
+
+            poll_status = asynError;
+        }
+
+        if (overTemperature()) {
+            appendErrorMessage(shellMessage, sizeof(shellMessage),
+                               "Overtemperature error.");
+            appendErrorMessage(
+                errorMessage, sizeof(errorMessage),
+                "Overtemperature error. Please call the support.");
+
+            poll_status = asynError;
+        }
+
+        if (overVoltage()) {
+            appendErrorMessage(shellMessage, sizeof(shellMessage),
+                               "Overvoltage error.");
+            appendErrorMessage(errorMessage, sizeof(errorMessage),
+                               "Overvoltage error. Please call the support.");
+
+            poll_status = asynError;
+        }
+
+        if (underVoltage()) {
+            appendErrorMessage(shellMessage, sizeof(shellMessage),
+                               "Undervoltage error.");
+            appendErrorMessage(errorMessage, sizeof(errorMessage),
+                               "Undervoltage error. Please call the support.");
+
+            poll_status = asynError;
+        }
+
+        if (stoFault()) {
+            appendErrorMessage(shellMessage, sizeof(shellMessage),
+                               "STO input is on disable state.");
+            appendErrorMessage(errorMessage, sizeof(errorMessage),
+                               "STO fault. Please call the support.");
+
+            poll_status = asynError;
+        }
+
+        if (strlen(shellMessage) > 0) {
             if (pC_->getMsgPrintControl().shouldBePrinted(keyError, true,
                                                           pC_->pasynUser())) {
                 asynPrint(pC_->pasynUser(), ASYN_TRACE_ERROR,
                           "Controller \"%s\", axis %d  => %s, line "
-                          "%d\nCommunication error.%s\n",
+                          "%d\n%s%s\n",
                           pC_->portName, axisNo_, __PRETTY_FUNCTION__, __LINE__,
-                          pC_->getMsgPrintControl().getSuffix());
+                          shellMessage, pC_->getMsgPrintControl().getSuffix());
             }
-
-            setAxisParamChecked(this, motorMessageText,
-                                "Communication error between PC and motor "
-                                "controller. Please call the support.");
-
-            poll_status = asynError;
-        } else {
-
-            // This buffer must be initialized to zero because we build the
-            // error message by appending strings.
-            char errorMessage[pC_->MAXBUF_] = {0};
-            char shellMessage[pC_->MAXBUF_] = {0};
-
-            // Concatenate all other errors
-            if (shortCircuit()) {
-                appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                   "Short circuit fault.");
-                appendErrorMessage(
-                    errorMessage, sizeof(errorMessage),
-                    "Short circuit error. Please call the support.");
-
-                poll_status = asynError;
-            }
-
-            if (encoderError()) {
-                appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                   "Encoder error.");
-                appendErrorMessage(errorMessage, sizeof(errorMessage),
-                                   "Encoder error. Please call the support.");
-
-                poll_status = asynError;
-            }
-
-            if (followingError()) {
-                appendErrorMessage(
-                    shellMessage, sizeof(shellMessage),
-                    "Maximum callowed following error exceeded.");
-                appendErrorMessage(
-                    errorMessage, sizeof(errorMessage),
-                    "Maximum allowed following error exceeded.Check if "
-                    "movement range is blocked. Otherwise please call the "
-                    "support.");
-
-                poll_status = asynError;
-            }
-
-            if (feedbackError()) {
-                appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                   "Feedback error.");
-                appendErrorMessage(errorMessage, sizeof(errorMessage),
-                                   "Feedback error. Please call the support.");
-
-                poll_status = asynError;
-            }
-
-            /*
-            Either the software limits or the end switches of the controller
-            have been hit. Since the EPICS limits are derived from the software
-            limits and are a little bit smaller, these error cases can only
-            happen if either the axis has an incremental encoder which is not
-            properly homed or if a bug occured.
-            */
-            if (positiveLimitSwitch() || negativeLimitSwitch() ||
-                positiveSoftwareLimit() || negativeSoftwareLimit()) {
-
-                // Distinction for developers
-                if (positiveLimitSwitch()) {
-                    appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                       "Positive limit switch.");
-                }
-                if (negativeLimitSwitch()) {
-                    appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                       "Negative limit switch.");
-                }
-                if (positiveSoftwareLimit()) {
-                    appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                       "Positive software limit.");
-                }
-                if (negativeSoftwareLimit()) {
-                    appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                       "Negative software limit.");
-                }
-
-                // Generic error message for user
-                appendErrorMessage(
-                    errorMessage, sizeof(errorMessage),
-                    "Software limits or end switch hit. Try homing the motor, "
-                    "moving in the opposite direction or check the SPS for "
-                    "errors (if available). Otherwise please call the "
-                    "support.");
-
-                poll_status = asynError;
-            }
-
-            if (overCurrent()) {
-                appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                   "Overcurrent error.");
-                appendErrorMessage(
-                    errorMessage, sizeof(errorMessage),
-                    "Overcurrent error. Please call the support.");
-
-                poll_status = asynError;
-            }
-
-            if (overTemperature()) {
-                appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                   "Overtemperature error.");
-                appendErrorMessage(
-                    errorMessage, sizeof(errorMessage),
-                    "Overtemperature error. Please call the support.");
-
-                poll_status = asynError;
-            }
-
-            if (overVoltage()) {
-                appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                   "Overvoltage error.");
-                appendErrorMessage(
-                    errorMessage, sizeof(errorMessage),
-                    "Overvoltage error. Please call the support.");
-
-                poll_status = asynError;
-            }
-
-            if (underVoltage()) {
-                appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                   "Undervoltage error.");
-                appendErrorMessage(
-                    errorMessage, sizeof(errorMessage),
-                    "Undervoltage error. Please call the support.");
-
-                poll_status = asynError;
-            }
-
-            if (stoFault()) {
-                appendErrorMessage(shellMessage, sizeof(shellMessage),
-                                   "STO input is on disable state.");
-                appendErrorMessage(errorMessage, sizeof(errorMessage),
-                                   "STO fault. Please call the support.");
-
-                poll_status = asynError;
-            }
-
-            if (strlen(shellMessage) > 0) {
-                if (pC_->getMsgPrintControl().shouldBePrinted(
-                        keyError, true, pC_->pasynUser())) {
-                    asynPrint(pC_->pasynUser(), ASYN_TRACE_ERROR,
-                              "Controller \"%s\", axis %d  => %s, line "
-                              "%d\n%s%s\n",
-                              pC_->portName, axisNo_, __PRETTY_FUNCTION__,
-                              __LINE__, shellMessage,
-                              pC_->getMsgPrintControl().getSuffix());
-                }
-            }
-
-            setAxisParamChecked(this, motorMessageText, errorMessage);
         }
-    } else {
+
+        setAxisParamChecked(this, motorMessageText, errorMessage);
+    }
+
+    // No error has been detected -> Reset the error count
+    if (poll_status == asynSuccess) {
         pC_->getMsgPrintControl().resetCount(keyError, pC_->pasynUser());
     }
 
@@ -741,9 +731,7 @@ asynStatus masterMacsAxis::doMove(double position, int relative,
 
     // Set the new motor speed, if the user is allowed to do so and if the
     // motor speed changed since the last move command.
-    if (motorCanSetSpeed != 0 && pMasterMacsA_->lastSetSpeed != motorVelocity) {
-
-        pMasterMacsA_->lastSetSpeed = motorVelocity;
+    if (motorCanSetSpeed != 0) {
 
         snprintf(value, sizeof(value), "%lf", motorVelocity);
         status = pC_->write(axisNo_, 05, value);
@@ -815,16 +803,31 @@ asynStatus masterMacsAxis::stop(double acceleration) {
 
 asynStatus masterMacsAxis::doReset() {
 
-    asynStatus status = pC_->write(axisNo_, 17, "");
+    asynStatus status = asynSuccess;
+
+    // Reset the controller ("node reset")
+    status = pC_->write(axisNo_, 16, "");
     if (status != asynSuccess) {
         setAxisParamChecked(this, motorStatusProblem, true);
     }
 
-    // Reset the driver to idle state and move out of the handshake wait loop,
-    // if we're currently inside it.
+    // Reset any errors in the controller. Since the node reset results in a
+    // power cycle, we use the corresponding timeout.
+    status = pC_->write(axisNo_, 17, "", PowerCycleTimeout);
+    if (status != asynSuccess) {
+        setAxisParamChecked(this, motorStatusProblem, true);
+    }
+
+    // Move out of the handshake wait loop, if we're currently inside it.
     pMasterMacsA_->waitForHandshake = false;
 
-    return status;
+    // Reinitialize the axis
+    status = masterMacsAxis::init();
+    if (status != asynSuccess) {
+        return status;
+    }
+    bool moving = false;
+    return forcedPoll(&moving);
 }
 
 /*
@@ -990,7 +993,7 @@ asynStatus masterMacsAxis::enable(bool on) {
         if (switchedOn() == on) {
             bool moving = false;
             // Perform a poll to update the parameter library
-            poll(&moving);
+            forcedPoll(&moving);
             return asynSuccess;
         }
     }