Moved version definition in Makefile

The expected version can now be set in the Makefile via USR_CXXFLAGS.
Additionally, the README.md has received documentation regarding the
version check. Lastly, the version check can now be disabled by omitting
the flags or setting one of them to a negative value.

Makefile

@@ -30,3 +30,7 @@ DBDS += sinqMotor/src/sinqMotor.dbd
 DBDS += src/masterMacs.dbd
 
 USR_CFLAGS += -Wall -Wextra -Weffc++ -Wunused-result -Wextra -Werror
+
+# These flags define the expected firmware version. See README.md, section
+# "Firmware version checking" for details.
+USR_CXXFLAGS += -DFIRMWARE_MAJOR_VERSION=2 -DFIRMWARE_MINOR_VERSION=2

README.md

@@ -66,6 +66,32 @@ dbLoadRecords("$(masterMacs_DB)/asynRecord.db","P=$(INSTR)$(NAME),PORT=$(ASYN_PO
 
 Please see the documentation for the module sinqMotor: https://git.psi.ch/sinq-epics-modules/sinqmotor/-/blob/main/README.md.
 
+### Firmware version checking
+
+This driver expects a certain version of the firmware running on the controller itself.
+This is checked at IOC startup by reading the version directly from the hardware.
+If the firmware version is incompatible to the driver, the IOC will be shut down.
+If the firmware version cannot be read (e.g. because the variable used to do so
+does not exist yet on old firmware versions), the firmware is assumed to be compatible
+to the driver.
+
+The version check is separated into a check of the major and the minor firmware
+version against expected values. The firmware is seen as compatible if the following conditions hold:
+- Read-out major version == Expected major version
+- Read-out read major version >= Expected minor version
+
+The expected versions are defined via compiler flags in `Makefile`:
+
+```
+USR_CXXFLAGS += -DFIRMWARE_MAJOR_VERSION=1 -DFIRMWARE_MINOR_VERSION=0
+```
+Be aware that these flags are only used to compile C++-files (.cpp, .cxx) and not
+C-files (.c). For C-files, the Makefile variable `USR_CFLAGS` must be used.
+
+In order to disable the checks, the flags can be set to -1 or just be removed
+entirely. If one of the flags is not given, both the major and the minor version 
+checks are deactivated.
+
 ### How to build it
 
 Please see the documentation for the module sinqMotor: https://git.psi.ch/sinq-epics-modules/sinqmotor/-/blob/main/README.md.

src/masterMacsController.cpp

@@ -13,16 +13,26 @@
 #include <unistd.h>
 
 /*
-These two constants store the major and the minor firmware version this driver
+These functions are used to read out the compiler flags defining the major and
-needs. The following constraints need to be fulfilled:
+minor versions. See README.md, section "Firmware version checking" for
-Actual major version == FIRMWARE_MAJOR_VERSION
+details. If these flags are not given, a default value of -1 is used, which
-Actual minor version >= FIRMWARE_MINOR_VERSION
+disables the version checks (it suffices to have one of these at -1 to disable
-If one of these constraints is not fulfilled, the driver will shut down the IOC.
+both major and minor version check)
-If the firmware version cannot be read (e.g. because the variable used to do so
-does not exist yet), the conditions outlined above are seen as fulfilled.
  */
-const int FIRMWARE_MAJOR_VERSION = 2;
+constexpr int firmware_major_version() {
-const int FIRMWARE_MINOR_VERSION = 2;
+#ifdef FIRMWARE_MAJOR_VERSION
+    return FIRMWARE_MAJOR_VERSION;
+#else
+    return -1;
+#endif
+}
+constexpr int firmware_minor_version() {
+#ifdef FIRMWARE_MINOR_VERSION
+    return FIRMWARE_MINOR_VERSION;
+#else
+    return -1;
+#endif
+}
 
 struct masterMacsControllerImpl {
     double comTimeout;
@@ -132,51 +142,56 @@ masterMacsController::masterMacsController(const char *portName,
 
     // =========================================================================
 
-    // Check the firmware version according to the conditions outlined in the
+    if (firmware_major_version() >= 0 && firmware_minor_version() >= 0) {
-    // comment for FIRMWARE_MAJOR_VERSION and FIRMWARE_MINOR_VERSION
+        // Check the firmware version according to the conditions outlined in
-    status = read(0, 99, response);
+        // README.md
+        status = read(0, 99, response);
 
-    if (status == asynSuccess) {
+        if (status == asynSuccess) {
 
-        // Just interpret the version if the variable already exists
+            // Just interpret the version if the variable already exists
-        double versionRaw = 0.0;
+            double versionRaw = 0.0;
-        int nvals = sscanf(response, "%lf", &versionRaw);
+            int nvals = sscanf(response, "%lf", &versionRaw);
-        if (nvals == 1 && versionRaw != 0.0) {
+            if (nvals == 1 && versionRaw != 0.0) {
-            // Discard decimal part
+                // Discard decimal part
-            long long versionInt = (long long)versionRaw;
+                long long versionInt = (long long)versionRaw;
 
-            // Extract bugfix (last 3 digits)
+                // Extract bugfix (last 3 digits)
-            // Currently not used, just here for completions sake
+                // Currently not used, just here for completions sake
-            // int bugfix = versionInt % 1000;
+                // int bugfix = versionInt % 1000;
-            versionInt /= 1000;
+                versionInt /= 1000;
 
-            // Extract minor (next 3 digits)
+                // Extract minor (next 3 digits)
-            int minor = versionInt % 1000;
+                int minor = versionInt % 1000;
-            versionInt /= 1000;
+                versionInt /= 1000;
 
-            // Remaining is major
+                // Remaining is major
-            int major = (int)versionInt;
+                int major = (int)versionInt;
 
-            // Compare to target values
+                // Compare to target values
-            if (FIRMWARE_MAJOR_VERSION != major ||
+                if (firmware_major_version() != major ||
-                FIRMWARE_MINOR_VERSION > minor) {
+                    firmware_minor_version() > minor) {
-                asynPrint(
+                    asynPrint(this->pasynUser(), ASYN_TRACE_ERROR,
-                    this->pasynUser(), ASYN_TRACE_ERROR,
+                              "Controller \"%s\" => %s, line %d\nFATAL ERROR "
-                    "Controller \"%s\" => %s, line %d\nFATAL ERROR (Incorrect "
+                              "(Incorrect "
-                    "version number of firmware: Expected major version equal "
+                              "version number of firmware: Expected major "
-                    "to %d, got %d. Expected minor version equal to or larger "
+                              "version equal "
-                    "than %d, got %d).\nTerminating IOC",
+                              "to %d, got %d. Expected minor version equal to "
-                    portName, __PRETTY_FUNCTION__, __LINE__,
+                              "or larger "
-                    FIRMWARE_MAJOR_VERSION, major, FIRMWARE_MINOR_VERSION,
+                              "than %d, got %d).\nTerminating IOC",
-                    minor);
+                              portName, __PRETTY_FUNCTION__, __LINE__,
-                exit(-1);
+                              firmware_major_version(), major,
+                              firmware_minor_version(), minor);
+                    exit(-1);
+                }
             }
+        } else {
+            asynPrint(
+                this->pasynUser(), ASYN_TRACE_ERROR,
+                "Controller \"%s\" => %s, line %d\nCould not read firmware "
+                "version\n",
+                portName, __PRETTY_FUNCTION__, __LINE__);
         }
-    } else {
-        asynPrint(this->pasynUser(), ASYN_TRACE_ERROR,
-                  "Controller \"%s\" => %s, line %d\nCould not read firmware "
-                  "version\n",
-                  portName, __PRETTY_FUNCTION__, __LINE__);
     }
 }