Merge pull request networkupstools#2565 from jimklimov/issue-2347

Address spurious LB+RB log flood on APC BXnnnnMI devices

NEWS.adoc

@@ -116,6 +116,10 @@ https://github.com/networkupstools/nut/milestone/11
  - usbhid-ups updates:
    * Support of the `onlinedischarge_log_throttle_hovercharge` in the NUT
      v2.8.2 release was found to be incomplete. [#2423, follow-up to #2215]
+   * Added support for `lbrb_log_delay_sec=N` setting to delay propagation of
+     `LB` or `LB+RB` state (buggy with APC BXnnnnMI devices circa 2023-2024).
+     This may work better with flags like `onlinedischarge_calibration` and
+     `lbrb_log_delay_without_calibrating` for some devices. [#2347]
    * General suggestion from `possibly_supported()` message method for devices
      with VendorID=`0x06da` (Phoenixtec), seen in some models supported by
      MGE HID or Liebert HID, updated to suggest trying `nutdrv_qx`. [#334]
@@ -222,6 +226,9 @@ during a NUT build.
      and values (or macros) in the NUT codebase. [issue #1176, issue #31]
    * custom `distcheck-something` targets did not inherit `DISTCHECK_FLAGS`
      properly. [#2541]
+   * added `status_get()` in NUT driver state API, to check if a status
+     token string had been set recently, and to avoid duplicate settings.
+     [PR #2565]
    * local socket/pipe protocol introduced a `LOGOUT` command for cleaner
      disconnection handling. [#2572]
    * codebase adapted to the liking of `clang-18` and newer revisions of

UPGRADING.adoc

@@ -50,6 +50,13 @@ Changes from 2.8.2 to 2.8.3
   devices; just in case a flag toggle `powercom_sdcmd_byte_order_fallback`
   was added to set the old behavior (if some devices do need it). [PR #2480]
 
+- Added support for `lbrb_log_delay_sec=N` setting to delay propagation of
+  `LB` or `LB+RB` state (buggy with APC BXnnnnMI devices/firmwares issued
+  circa 2023-2024 which flood the logs with spurious LOWBATT and REPLACEBATT
+  events). This may work better for some devices when combined with flags
+  like `onlinedischarge_calibration` and `lbrb_log_delay_without_calibrating`.
+  [#2347]
+
 - Enabled installation of built PDF and HTML (including man page renditions)
   files under the configured `docdir`. It seems previously they were only
   built (if requested) but not installed via `make`, unlike the common man

ci_build.sh

@@ -470,7 +470,7 @@ if [ -z "${CANBUILD_LIBGD_CGI-}" ]; then
 
     # NUT CI farm with Jenkins can build it; Travis could not
     [[ "$CI_OS_NAME" = "freebsd" ]] && CANBUILD_LIBGD_CGI=yes \
-    || [[ "$TRAVIS_OS_NAME" = "freebsd" ]] && CANBUILD_LIBGD_CGI=no
+    || { [[ "$TRAVIS_OS_NAME" = "freebsd" ]] && CANBUILD_LIBGD_CGI=no ; }
 
     # See also below for some compiler-dependent decisions
 fi
@@ -1294,7 +1294,11 @@ default|default-alldrv|default-alldrv:no-distcheck|default-all-errors|default-sp
                     CONFIG_OPTS+=("--with-cgi=auto")
                 fi
             else
-                CONFIG_OPTS+=("--with-cgi=auto")
+                if [ "${CANBUILD_LIBGD_CGI-}" = "no" ] ; then
+                    CONFIG_OPTS+=("--without-cgi")
+                else
+                    CONFIG_OPTS+=("--with-cgi=auto")
+                fi
             fi
             ;;
         "default-alldrv:no-distcheck")

data/driver.list.in

@@ -79,6 +79,7 @@
 "APC"	"ups"	"3"	"Back-UPS BF500"	"USB"	"usbhid-ups"
 "APC"	"ups"	"3"	"BACK-UPS XS LCD"	"USB"	"usbhid-ups"
 "APC"	"ups"	"3"	"Back-UPS XS 1000M (Back-UPS Pro 1000, Model BX1000M)"	"USB"	"usbhid-ups"	# https://github.com/networkupstools/nut/issues/139
+"APC"	"ups"	"3"	"Back-UPS BX****MI Series (may need tweaks since 2023)"	"USB"	"usbhid-ups lbrb_log_delay_sec=N lbrb_log_delay_without_calibrating onlinedischarge_calibration"	# https://github.com/networkupstools/nut/issues/2347
 "APC"	"ups"	"3"	"SMC2200BI-BR"	"USB"	"usbhid-ups"	# https://github.com/networkupstools/nut/issues/557
 "APC"	"ups"	"3"	"Smart-UPS (USB)"	"USB"	"usbhid-ups"
 "APC"	"ups"	"3"	"Smart-UPS 750 (SMT750I, USB)"	"USB"	"usbhid-ups"

docs/man/usbhid-ups.txt

@@ -160,6 +160,20 @@ not forcing it to be fully charged all the time.  As long as the current value
 of `battery.charge` remains at or above this threshold percentage (default 100),
 the `OL+DISCHRG` message logging is not triggered by variations of the charge.
 
+*lbrb_log_delay_sec*='num'::
+Set to delay status-setting (and log messages) about device entering `LB` or
+`LB+RB` state.
++
+Some APC BXnnnnMI device models or firmware versions (reportedly 2023-2024)
+frequently report low battery, replace battery, and all ok within a couple
+of seconds, sometimes but not always preceded by OL+DISCHRG (presumably
+calibration). This setting lets the driver ignore short-lived states and
+only pay attention if they persist longer than this setting (and the device
+power state is `OL`).
+
+*lbrb_log_delay_without_calibrating*::
+Set to apply `lbrb_log_delay_sec` even if device is not calibrating.
+
 *disable_fix_report_desc*::
 Set to disable fix-ups for broken USB encoding, etc. which we apply by default
 on certain models (vendors/products) which were reported as not following the

docs/new-drivers.txt

@@ -214,7 +214,11 @@ UPS status flags like on line (OL) and on battery (OB) live in
 ups.status.  Don't manipulate this by hand.  There are functions which
 will do this for you.
 
-	status_init() -- before doing anything else
+	status_init()   -- before doing anything else (clear internal buffers,
+	                   etc.)
+
+	status_get(val) -- optionally check if a status word had been set
+	                   since the most-recent status_init()
 
 	status_set(val) -- add a status word (OB, OL, etc)
 

docs/nut.dict

@@ -1,4 +1,4 @@
-personal_ws-1.1 en 3205 utf-8
+personal_ws-1.1 en 3208 utf-8
 AAC
 AAS
 ABI
@@ -100,6 +100,7 @@ BTS
 BTV
 BUFRD
 BUZ
+BXnnnnMI
 BYP
 BZ
 BZOFF
@@ -981,6 +982,7 @@ README
 REALPATH
 REDi
 REFREPO
+REPLACEBATT
 REPLBATT
 REQSSL
 RETPCT
@@ -2150,6 +2152,7 @@ labcd
 lan
 langid
 lasaine
+lbrb
 ld
 ldd
 le

drivers/dstate.c

@@ -1620,6 +1620,36 @@ void status_init(void)
 	memset(status_buf, 0, sizeof(status_buf));
 }
 
+/* check if a status element has been set, return 0 if not, 1 if yes
+ * (considering a whole-word token in temporary status_buf) */
+int status_get(const char *buf)
+{
+	char	*s = NULL;
+	size_t	offset = 0, buflen = 0;
+
+	if (!buf || !*buf || !*status_buf)
+		return 0;
+
+	s = strstr(status_buf, buf);
+	buflen = strlen(buf);
+
+	/* not found */
+	if (!s)
+		return 0;
+
+	offset = status_buf - s;
+	if (offset == 0 || status_buf[offset - 1] == ' ') {
+		/* We have hit the start of token */
+		if (s[buflen] == '\0' || s[buflen] == ' ') {
+			/* And we have hit the end of token */
+			return 1;
+		}
+	}
+
+	/* buf was a substring of some other token */
+	return 0;
+}
+
 /* add a status element */
 void status_set(const char *buf)
 {
@@ -1628,6 +1658,11 @@ void status_set(const char *buf)
 		return;
 	}
 
+	if (status_get(buf)) {
+		upsdebugx(2, "%s: status was already set: %s", __func__, buf);
+		return;
+	}
+
 	/* separate with a space if multiple elements are present */
 	if (strlen(status_buf) > 0) {
 		snprintfcat(status_buf, sizeof(status_buf), " %s", buf);

drivers/dstate.h

@@ -101,6 +101,10 @@ int dstate_is_stale(void);
 /* clean out the temp space for a new pass */
 void status_init(void);
 
+/* check if a status element has been set, return 0 if not, 1 if yes
+ * (considering a whole-word token in temporary status_buf) */
+int status_get(const char *buf);
+
 /* add a status element */
 void status_set(const char *buf);