From 25286652d527df7a5c65d97573bbd249e6488828 Mon Sep 17 00:00:00 2001 From: smathis Date: Fri, 22 Aug 2025 13:18:43 +0200 Subject: [PATCH] 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 | 4 ++ README.md | 26 +++++++++ src/masterMacsController.cpp | 107 ++++++++++++++++++++--------------- 3 files changed, 91 insertions(+), 46 deletions(-) diff --git a/Makefile b/Makefile index eb95961..c60550d 100644 --- a/Makefile +++ b/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 diff --git a/README.md b/README.md index 821422c..a7ebe18 100644 --- a/README.md +++ b/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. diff --git a/src/masterMacsController.cpp b/src/masterMacsController.cpp index 02b3fd1..8fb9365 100644 --- a/src/masterMacsController.cpp +++ b/src/masterMacsController.cpp @@ -13,16 +13,26 @@ #include /* -These two constants store the major and the minor firmware version this driver -needs. The following constraints need to be fulfilled: -Actual major version == FIRMWARE_MAJOR_VERSION -Actual minor version >= FIRMWARE_MINOR_VERSION -If one of these constraints is not fulfilled, the driver will shut down the IOC. -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. +These functions are used to read out the compiler flags defining the major and +minor versions. See README.md, section "Firmware version checking" for +details. If these flags are not given, a default value of -1 is used, which +disables the version checks (it suffices to have one of these at -1 to disable +both major and minor version check) */ -const int FIRMWARE_MAJOR_VERSION = 2; -const int FIRMWARE_MINOR_VERSION = 2; +constexpr int firmware_major_version() { +#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 - // comment for FIRMWARE_MAJOR_VERSION and FIRMWARE_MINOR_VERSION - status = read(0, 99, response); + if (firmware_major_version() >= 0 && firmware_minor_version() >= 0) { + // Check the firmware version according to the conditions outlined in + // README.md + status = read(0, 99, response); - if (status == asynSuccess) { + if (status == asynSuccess) { - // Just interpret the version if the variable already exists - double versionRaw = 0.0; - int nvals = sscanf(response, "%lf", &versionRaw); - if (nvals == 1 && versionRaw != 0.0) { - // Discard decimal part - long long versionInt = (long long)versionRaw; + // Just interpret the version if the variable already exists + double versionRaw = 0.0; + int nvals = sscanf(response, "%lf", &versionRaw); + if (nvals == 1 && versionRaw != 0.0) { + // Discard decimal part + long long versionInt = (long long)versionRaw; - // Extract bugfix (last 3 digits) - // Currently not used, just here for completions sake - // int bugfix = versionInt % 1000; - versionInt /= 1000; + // Extract bugfix (last 3 digits) + // Currently not used, just here for completions sake + // int bugfix = versionInt % 1000; + versionInt /= 1000; - // Extract minor (next 3 digits) - int minor = versionInt % 1000; - versionInt /= 1000; + // Extract minor (next 3 digits) + int minor = versionInt % 1000; + versionInt /= 1000; - // Remaining is major - int major = (int)versionInt; + // Remaining is major + int major = (int)versionInt; - // Compare to target values - if (FIRMWARE_MAJOR_VERSION != major || - FIRMWARE_MINOR_VERSION > minor) { - asynPrint( - this->pasynUser(), ASYN_TRACE_ERROR, - "Controller \"%s\" => %s, line %d\nFATAL ERROR (Incorrect " - "version number of firmware: Expected major version equal " - "to %d, got %d. Expected minor version equal to or larger " - "than %d, got %d).\nTerminating IOC", - portName, __PRETTY_FUNCTION__, __LINE__, - FIRMWARE_MAJOR_VERSION, major, FIRMWARE_MINOR_VERSION, - minor); - exit(-1); + // Compare to target values + if (firmware_major_version() != major || + firmware_minor_version() > minor) { + asynPrint(this->pasynUser(), ASYN_TRACE_ERROR, + "Controller \"%s\" => %s, line %d\nFATAL ERROR " + "(Incorrect " + "version number of firmware: Expected major " + "version equal " + "to %d, got %d. Expected minor version equal to " + "or larger " + "than %d, got %d).\nTerminating IOC", + portName, __PRETTY_FUNCTION__, __LINE__, + 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__); } }