Merge pull request #2932 from jimklimov/fix-ci-unmapped

jimklimov · web-flow · commit b03af2d4022a · 2025-05-01T02:48:48.000+02:00
Fix CI for "unmapped" data points, and update some docs
diff --git a/NEWS.adoc b/NEWS.adoc
@@ -73,7 +73,7 @@ https://github.com/networkupstools/nut/milestone/9
      and misfired on some platforms, and the way to print had a theoretical
      potential for buffer overflow. [#2915]
 
- - `usbhid-ups` updates:
+ - `usbhid-ups` driver updates:
    * The `cps-hid` subdriver's existing mechanism for fixing broken report
      descriptors was extended to cover a newly reported case of nominal UPS
      power being incorrectly reported due to an unrealistically low maximum
diff --git a/ci_build.sh b/ci_build.sh
@@ -99,6 +99,10 @@ if [ "$BUILD_TYPE" = fightwarn ]; then
 
     # Similarly for libusb implementations with varying support
     #[ -n "$NUT_USB_VARIANTS" ] || NUT_USB_VARIANTS=auto
+
+    # Similarly for testing builds with and without "unmapped" values
+    # (normally hidden by #ifdef blocks) in certain evolving drivers
+    #[ -n "$NUT_UNMAPPED_VARIANTS" ] || NUT_UNMAPPED_VARIANTS=auto
 fi
 
 # configure default is "no"; an "auto" value is "yes unless CFLAGS say something"
@@ -1449,7 +1453,7 @@ echo "Processing BUILD_TYPE='${BUILD_TYPE}' ..."
 
 ensure_CI_CCACHE_SYMLINKDIR_envvar
 echo "Build host settings:"
-set | grep -E '^(PATH|[^ ]*CCACHE[^ ]*|CI_[^ ]*|OS_[^ ]*|CANBUILD_[^ ]*|NODE_LABELS|MAKE|C[^ ]*FLAGS|LDFLAGS|ARCH[^ ]*|BITS[^ ]*|CC|CXX|CPP|DO_[^ ]*|BUILD_[^ ]*|[^ ]*_TGT)=' || true
+set | grep -E '^(PATH|[^ ]*CCACHE[^ ]*|CI_[^ ]*|OS_[^ ]*|CANBUILD_[^ ]*|NODE_LABELS|MAKE|C[^ ]*FLAGS|LDFLAGS|ARCH[^ ]*|BITS[^ ]*|CC|CXX|CPP|DO_[^ ]*|BUILD_[^ ]*|[^ ]*_TGT|INPLACE_RUNTIME)=' || true
 uname -a
 echo "LONG_BIT:`getconf LONG_BIT` WORD_BIT:`getconf WORD_BIT`" || true
 if command -v xxd >/dev/null ; then xxd -c 1 -l 6 | tail -1; else if command -v od >/dev/null; then od -N 1 -j 5 -b | head -1 ; else hexdump -s 5 -n 1 -C | head -1; fi; fi < /bin/ls 2>/dev/null | awk '($2 == 1){print "Endianness: LE"}; ($2 == 2){print "Endianness: BE"}' || true
@@ -1513,6 +1517,16 @@ default|default-alldrv|default-alldrv:no-distcheck|default-all-errors|default-sp
     CONFIG_OPTS+=("--with-devd-dir=${BUILD_PREFIX}/etc/devd")
     CONFIG_OPTS+=("--with-hotplug-dir=${BUILD_PREFIX}/etc/hotplug")
 
+    if [ "${BUILD_TYPE}" != "default-all-errors" ] ; then
+        case x"${WITH_UNMAPPED_DATAPOINTS-}" in
+            [Tt][Rr][Uu][Ee]|[Yy][Ee][Ss])
+                CONFIG_OPTS+=("--with-unmapped-data-points") ;;
+            [Ff][Aa][Ll][Ss][Ee]|[Nn][Oo])
+                CONFIG_OPTS+=("--without-unmapped-data-points") ;;
+            *)  ;; # Keep built-in default
+        esac
+    fi
+
     if [ x"${INPLACE_RUNTIME-}" = xtrue ]; then
         CONFIG_OPTS+=("--enable-inplace-runtime")
     fi
@@ -1967,6 +1981,17 @@ default|default-alldrv|default-alldrv:no-distcheck|default-all-errors|default-sp
                 fi
             fi
 
+            if [ -z "$NUT_UNMAPPED_VARIANTS" ] || [ "$NUT_UNMAPPED_VARIANTS" = auto ] ; then
+                case x"${WITH_UNMAPPED_DATAPOINTS-}" in
+                    [Tt][Rr][Uu][Ee]|[Yy][Ee][Ss])
+                        NUT_UNMAPPED_VARIANTS="yes" ;;
+                    [Ff][Aa][Ll][Ss][Ee]|[Nn][Oo])
+                        NUT_UNMAPPED_VARIANTS="no" ;;
+                    *)
+                        NUT_UNMAPPED_VARIANTS="yes no" ;;
+                esac
+            fi
+
             # Count our expected build variants, so the last one gets the
             # "maintainer-clean" check and not a mere "distclean" check
             # NOTE: We count different dependency variations separately,
@@ -1981,6 +2006,15 @@ default|default-alldrv|default-alldrv:no-distcheck|default-all-errors|default-sp
                 BUILDSTODO_USB="`expr $BUILDSTODO_USB + 1`"
             done
 
+            BUILDSTODO_UNMAPPED=0
+            for NUT_UNMAPPED_VARIANT in $NUT_UNMAPPED_VARIANTS ; do
+                BUILDSTODO_UNMAPPED="`expr $BUILDSTODO_UNMAPPED + 1`"
+            done
+            # Use the only one requested right away
+            if [ "${BUILDSTODO_UNMAPPED}" -eq 1 ] ; then
+                CONFIG_OPTS+=("--with-unmapped-data-points=${NUT_UNMAPPED_VARIANT}")
+            fi
+
             if [ "${BUILDSTODO_SSL}" -gt 1 ] \
             && [ "${BUILDSTODO_USB}" -gt 1 ] \
             ; then
@@ -2012,10 +2046,17 @@ default|default-alldrv|default-alldrv:no-distcheck|default-all-errors|default-sp
                     echo "=== Only build USB scenario(s) picking whatever SSL is found"
                     BUILDSTODO="${BUILDSTODO_USB}"
                 fi
+
+            fi
+
+            # Tack a remaining yes/no in the end to whatever scenario is there;
+            # NOT a full matrix
+            if [ "${BUILDSTODO_UNMAPPED}" -gt 1 ] ; then
+                BUILDSTODO="`expr $BUILDSTODO + $BUILDSTODO_UNMAPPED - 1`"
             fi
 
             BUILDSTODO_INITIAL="$BUILDSTODO"
-            echo "=== Will loop now with $BUILDSTODO build variants: found ${BUILDSTODO_SSL} SSL ($NUT_SSL_VARIANTS) and ${BUILDSTODO_USB} USB ($NUT_USB_VARIANTS) variations..."
+            echo "=== Will loop now with $BUILDSTODO build variants: found ${BUILDSTODO_SSL} SSL ($NUT_SSL_VARIANTS) and ${BUILDSTODO_USB} USB ($NUT_USB_VARIANTS) and ${BUILDSTODO_UNMAPPED} UNMAPPED ($NUT_UNMAPPED_VARIANTS) variations..."
             # If we don't care about SSL implem and want to pick USB, go straight there
             ( [ "$NUT_SSL_VARIANTS" = "auto" ] && [ "${BUILDSTODO_USB}" -gt 0 ] ) || \
             for NUT_SSL_VARIANT in $NUT_SSL_VARIANTS ; do
@@ -2130,7 +2171,7 @@ default|default-alldrv|default-alldrv:no-distcheck|default-all-errors|default-sp
                         echo "=== SKIPPED sandbox cleanup because DO_CLEAN_CHECK=$DO_CLEAN_CHECK and $BUILDSTODO build variants remaining"
                     fi
                 fi
-            done
+            done	# end of "for NUT_SSL_VARIANT in $NUT_SSL_VARIANTS"
 
             # Effectively, whatever up to one version of LibUSB support
             # was detected (or not), was tested above among SSL builds.
@@ -2278,7 +2319,112 @@ default|default-alldrv|default-alldrv:no-distcheck|default-all-errors|default-sp
                         echo "=== SKIPPED sandbox cleanup because DO_CLEAN_CHECK=$DO_CLEAN_CHECK and $BUILDSTODO build variants remaining"
                     fi
                 fi
-            done
+            done	# end of "for NUT_USB_VARIANT in $NUT_USB_VARIANTS"
+
+            # Tack a remaining yes/no in the end to whatever scenario is there;
+            # NOT a full matrix
+            if [ "${BUILDSTODO_UNMAPPED}" -gt 1 ] ; then
+                for NUT_UNMAPPED_VARIANT in $NUT_UNMAPPED_VARIANTS ; do
+                    case "${NUT_UNMAPPED_VARIANT}" in
+                        no) ;;	# we already did the default ("no") implicitly
+                        *)	# Try this variant
+                            echo "=== Starting 'NUT_UNMAPPED_VARIANT=$NUT_UNMAPPED_VARIANT', $BUILDSTODO build variants remaining..."
+                            ( if [ "$NUT_SSL_VARIANTS" != "auto" ] ; then
+                                  CONFIG_OPTS+=("--without-all")
+                                  CONFIG_OPTS+=("--without-ssl")
+                              fi
+                              CONFIG_OPTS+=("--with-serial=auto")
+                              if [ "$NUT_USB_VARIANTS" != "no" ] ; then
+                                  CONFIG_OPTS+=("--with-usb=auto")
+                              fi
+                              CONFIG_OPTS+=("--with-unmapped-data-points=${NUT_UNMAPPED_VARIANT}")
+                              configure_nut
+                            ) || {
+                                RES_ALLERRORS=$?
+                                FAILED="${FAILED} NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}[configure]"
+                                # TOTHINK: Do we want to try clean-up if we likely have no Makefile?
+                                if [ "$CI_FAILFAST" = true ]; then
+                                    echo "===== Aborting because CI_FAILFAST=$CI_FAILFAST" >&2
+                                    break
+                                fi
+                                BUILDSTODO="`expr $BUILDSTODO - 1`" || [ "$BUILDSTODO" = "0" ] || break
+                                continue
+                            }
+
+                            echo "=== Configured 'NUT_UNMAPPED_VARIANT=$NUT_UNMAPPED_VARIANT', $BUILDSTODO build variants (including this one) remaining to complete; trying to build..."
+                            cd "${CI_BUILDDIR}"
+                            # Use default target e.g. "all":
+                            build_to_only_catch_errors_target && {
+                                SUCCEEDED="${SUCCEEDED} NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}[build]"
+                            } || {
+                                RES_ALLERRORS=$?
+                                FAILED="${FAILED} NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}[build]"
+                                # Help find end of build (before cleanup noise) in logs:
+                                echo "=== FAILED 'NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}' build"
+                                if [ "$CI_FAILFAST" = true ]; then
+                                    echo "===== Aborting because CI_FAILFAST=$CI_FAILFAST" >&2
+                                    break
+                                fi
+                            }
+
+                            build_to_only_catch_errors_check && {
+                                SUCCEEDED="${SUCCEEDED} NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}[check]"
+                            } || {
+                                RES_ALLERRORS=$?
+                                FAILED="${FAILED} NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}[check]"
+                                # Help find end of build (before cleanup noise) in logs:
+                                echo "=== FAILED 'NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}' check"
+                                if [ "$CI_FAILFAST" = true ]; then
+                                    echo "===== Aborting because CI_FAILFAST=$CI_FAILFAST" >&2
+                                    break
+                                fi
+                            }
+
+                            # Note: when `expr` calculates a zero value below, it returns
+                            # an "erroneous" `1` as exit code. Notes above.
+                            BUILDSTODO="`expr $BUILDSTODO - 1`" || [ "$BUILDSTODO" = "0" ] || break
+
+                            if [ "$BUILDSTODO" -gt 0 ] && [ "${DO_CLEAN_CHECK-}" != no ]; then
+                                # For last iteration with DO_CLEAN_CHECK=no,
+                                # we would leave built products in place
+                                echo "=== Clean the sandbox, $BUILDSTODO build variants remaining..."
+                            fi
+
+                            if can_clean_check ; then
+                                if [ $BUILDSTODO -gt 0 ]; then
+                                    ### Avoid having to re-autogen in a loop:
+                                    optional_dist_clean_check && {
+                                        if [ "${DO_DIST_CLEAN_CHECK-}" != "no" ] ; then
+                                            SUCCEEDED="${SUCCEEDED} NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}[dist_clean]"
+                                        fi
+                                    } || {
+                                        RES_ALLERRORS=$?
+                                        FAILED="${FAILED} NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}[dist_clean]"
+                                    }
+                                else
+                                    optional_maintainer_clean_check && {
+                                        if [ "${DO_MAINTAINER_CLEAN_CHECK-}" != no ] ; then
+                                            SUCCEEDED="${SUCCEEDED} NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}[maintainer_clean]"
+                                        fi
+                                    } || {
+                                        RES_ALLERRORS=$?
+                                        FAILED="${FAILED} NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}[maintainer_clean]"
+                                    }
+                                fi
+                                echo "=== Completed sandbox cleanup-check after NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}, $BUILDSTODO build variants remaining"
+                            else
+                                if [ "$BUILDSTODO" -gt 0 ] && [ "${DO_CLEAN_CHECK-}" != no ]; then
+                                    $MAKE distclean $MAKE_FLAGS_CLEAN -k \
+                                    || echo "WARNING: 'make distclean' FAILED: $? ... proceeding" >&2
+                                    echo "=== Completed sandbox cleanup after NUT_UNMAPPED_VARIANT=${NUT_UNMAPPED_VARIANT}, $BUILDSTODO build variants remaining"
+                                else
+                                    echo "=== SKIPPED sandbox cleanup because DO_CLEAN_CHECK=$DO_CLEAN_CHECK and $BUILDSTODO build variants remaining"
+                                fi
+                            fi
+                            ;;	# end of "Try this variant"
+                    esac
+                done	# end of "for NUT_UNMAPPED_VARIANT in $NUT_UNMAPPED_VARIANTS"
+            fi
 
             # TODO: Similar loops for other variations like TESTING,
             # MGE SHUT vs. other serial protocols...
@@ -2461,6 +2607,21 @@ bindings)
         CONFIG_OPTS+=("--disable-silent-rules")
     fi
 
+    if [ -z "${WITH_UNMAPPED_DATAPOINTS-}" ] ; then
+        if [ x"${INPLACE_RUNTIME-}" = xtrue ]; then
+            WITH_UNMAPPED_DATAPOINTS=false
+        else
+            WITH_UNMAPPED_DATAPOINTS=true
+        fi
+    fi
+
+    if [ x"${WITH_UNMAPPED_DATAPOINTS-}" = xtrue ] ; then
+        # This is assumed for non-production builds to avoid confusion
+        # for end-users (not dev/testers).
+        # See above for defaulting of this vs. inplace builds.
+        CONFIG_OPTS+=("--with-unmapped-data-points")
+    fi
+
     if [ -n "${BUILD_DEBUGINFO-}" ]; then
         CONFIG_OPTS+=("--with-debuginfo=${BUILD_DEBUGINFO}")
     else
diff --git a/docs/Makefile.am b/docs/Makefile.am
@@ -433,6 +433,7 @@ MAINTAINER_ASCIIDOCS_CHANGELOG_DEBUG = no
 		-e 's%\(PR\|pull request\) *networkupstools/\([^ ][^ ]*\)\#\([1-9][0-9]*\)\([^0-9]\|$$\)%link:https://github.com/networkupstools/\2/pull/\3[\1 \2\#\#\3]\4%g' \
 		-e 's%\([[ ,]\)networkupstools/\([^ ][^ ]*\)\#\([1-9][0-9]*\)\([^0-9]\|$$\)%\1link:https://github.com/networkupstools/\2/issues/\3[\2\#\#\3]\4%g' \
 		-e 's%\#\(\#[1-9][0-9]*\)%\1%g' \
+		-e 's,\(https*://[^ \+]*\)[\]*[+],\1%2B,g' \
 	 ; } > "$@.tmp.$$$$" \
 	 && mv -f "$@.tmp.$$$$" "$@"
 	@if [ x'$@' = x'$(top_builddir)/ChangeLog.adoc-parsed' ] ; then \
diff --git a/docs/config-prereqs.txt b/docs/config-prereqs.txt
@@ -106,8 +106,14 @@ recommended for build agents with immutable system areas, etc.
 Build prerequisites to make NUT from scratch on various Operating Systems
 -------------------------------------------------------------------------
 
-Debian 10/11/12/13
-~~~~~~~~~~~~~~~~~~
+Debian, Ubuntu and derivatives
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+NOTE: Documented according to NUT CI farm build agents and development
+workstation running Debian 10/11/12/13 (should at least partially apply
+as far back as Debian 7/8), Ubuntu 14.x-25.x, and Termux revisions circa
+2023-2025. This section generally covers a very wide family of APT-based
+Debian-derived Linux operating system distributions.
 
 Being a popular baseline among Linux distributions, Debian is an
 important build target. Related common operating systems include
diff --git a/docs/developers.txt b/docs/developers.txt
@@ -186,6 +186,38 @@ function do_stuff(void)
 }
 --------------------------------------------------------------------------------
 
+Variable length arrays
+~~~~~~~~~~~~~~~~~~~~~~
+
+We also avoid initialization of arrays using a variable (e.g. a method
+argument) as poorly portable to older compilers, and prefer explicitly
+allocated and freed buffers, if variable size is needed.
+
+This should be avoided:
+--------------------------------------------------------------------------------
+function avoid_vla(int len)
+{
+	char	*buf[len];
+...
+}
+--------------------------------------------------------------------------------
+
+This should be sued instead (preferably with `size_t` arguments right away),
+be sure to `free()` anywhere you `return` from the function:
+--------------------------------------------------------------------------------
+int use_xcalloc(size_t len)
+{
+	char	*buf = xcalloc(len, sizeof(char));
+
+	if (!buf)
+		return -1;
+...
+	free(buf);
+	return 1;
+}
+--------------------------------------------------------------------------------
+
+
 Other hints
 ~~~~~~~~~~~
 
diff --git a/docs/man/nutdrv_qx.txt b/docs/man/nutdrv_qx.txt
@@ -137,7 +137,7 @@ Some devices "natively" report just report a part of the total battery voltage
 device or properly guessed by the driver otherwise).
 +
 If this flag is set, most of the subdrivers (except those which know about
-more  complicated device-specific nuances, currently: *ablerex*, *masterguard*
+more complicated device-specific nuances, currently: *ablerex*, *masterguard*
 and *voltronic-qs-hex*) adjust their ultimately reported *battery.voltage*
 value as a multiple of *battery.packs* and "native" *battery.voltage*).
 +
diff --git a/docs/nut.dict b/docs/nut.dict
@@ -1,4 +1,4 @@
-personal_ws-1.1 en 3470 utf-8
+personal_ws-1.1 en 3473 utf-8
 AAC
 AAS
 ABI
@@ -2244,6 +2244,7 @@ interruptonly
 interruptsize
 intltool
 inverter
+inverters
 invmode
 io
 ioLogik
@@ -2729,6 +2730,7 @@ pfy
 ph
 phoenixcontact
 phoenixtec
+photovoltaic
 picocom
 pid
 pidpath
@@ -3355,9 +3357,9 @@ varname
 varvalue
 vbat
 vbatt
-ve
 vc
 vda
+ve
 vendorid
 ver
 verifySourceSig
@@ -3375,6 +3377,7 @@ virtinst
 virtualization
 virtualized
 vivo
+vla
 vo
 volsize
 voltronic
diff --git a/docs/nutdrv_qx-subdrivers.txt b/docs/nutdrv_qx-subdrivers.txt
@@ -25,6 +25,13 @@ the main `nutdrv_qx.c` file.  However, in `ups.conf` this entity is referred
 to via the communication `protocol` keyword, if the end-user wants to pick
 one explicitly (bypassing auto-detection).
 
+The string value of each `protocol` setting is derived from the respective
+source module's self-identification, by picking the first space-separated token
+(its `NAME`, assuming a `NAME VERSION` pattern) and making a case-insensitive
+comparison.  The self-identification value is maintained as a string macro at
+the top of each module's C source file, and is passed to the main driver code
+as part of `subdriver_t` structure instance defined in the end of the file.
+
 Confusingly, there *is* also an optional USB `subdriver` setting (available
 when the driver is built with USB support), for "Serial-over-USB subdriver
 selection", corresponding to entries in the `usbsubdriver` array and several
diff --git a/drivers/nutdrv_qx_ablerex.c b/drivers/nutdrv_qx_ablerex.c
diff --git a/drivers/nutdrv_qx_ablerex.h b/drivers/nutdrv_qx_ablerex.h
diff --git a/drivers/powervar-hid.h b/drivers/powervar-hid.h

Original file line number	Diff line number	Diff line change
`@@ -137,7 +137,7 @@ Some devices "natively" report just report a part of the total battery voltage`
`137`	`137`	`device or properly guessed by the driver otherwise).`
`138`	`138`	`+`
`139`	`139`	`If this flag is set, most of the subdrivers (except those which know about`
`140`		`-more complicated device-specific nuances, currently: ablerex, masterguard`
	`140`	`+more complicated device-specific nuances, currently: ablerex, masterguard`
`141`	`141`	`and voltronic-qs-hex) adjust their ultimately reported battery.voltage`
`142`	`142`	`value as a multiple of battery.packs and "native" battery.voltage).`
`143`	`143`	`+`