From ea2ab714647ed7abb0ddd4f49d5ebce7af79b913 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 5 Dec 2023 12:08:05 +0100 Subject: [PATCH 1/3] drivers/usbhid-ups.c, docs/man/usbhid-ups.txt, NEWS.adoc: deprecate "onlinedischarge" in favor of "onlinedischarge_onbattery" option name [#2213] Signed-off-by: Jim Klimov --- NEWS.adoc | 3 +++ docs/man/usbhid-ups.txt | 9 +++++++-- docs/nut.dict | 3 ++- drivers/usbhid-ups.c | 19 ++++++++++++------- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index 6ff7edbfc6..d75d36c82b 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -107,6 +107,9 @@ https://github.com/networkupstools/nut/milestone/10 * `arduino-hid` subdriver was enhanced from "initial bare bones" experimental set of mapped data points to support some 20 more mappings to make it more useful as an UPS driver, not just a controller developer sandbox. [#2188] + * The `onlinedischarge` configuration flag name was too ambiguous and got + deprecated (will be supported but no longer promoted by documentation), + introducing `onlinedischarge_onbattery` as the meaningful alias. [#2213] Release notes for NUT 2.8.1 - what's new since 2.8.0 ---------------------------------------------------- diff --git a/docs/man/usbhid-ups.txt b/docs/man/usbhid-ups.txt index 9c7a8cef71..c82823995c 100644 --- a/docs/man/usbhid-ups.txt +++ b/docs/man/usbhid-ups.txt @@ -114,12 +114,17 @@ If this flag is set, the driver will not use Interrupt In transfers during the shorter "pollinterval" cycles (not recommended, but needed if these reports are broken on your UPS). -*onlinedischarge*:: -If this flag is set, the driver will treat `OL+DISCHRG` status as offline. +*onlinedischarge_battery*:: +If this flag is set, the driver will treat `OL+DISCHRG` status as +offline/on-battery. ++ For most devices this combination means calibration or similar maintenance; however some UPS models (e.g. CyberPower UT series) emit `OL+DISCHRG` when wall power is lost -- and need this option to handle shutdowns. +*onlinedischarge*:: +DEPRECATED, old name for `onlinedischarge_battery` described above. + *onlinedischarge_calibration*:: If this flag is set, the driver will treat `OL+DISCHRG` status as calibration. Some UPS models (e.g. APC were seen to do so) report `OL+DISCHRG` when they diff --git a/docs/nut.dict b/docs/nut.dict index 7653b5e3a0..c04169b07e 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3421 utf-8 +personal_ws-1.1 en 3422 utf-8 AAC AAS ABI @@ -2661,6 +2661,7 @@ oldmge oldnut omnios onbatt +onbattery onbattwarn onclick ondelay diff --git a/drivers/usbhid-ups.c b/drivers/usbhid-ups.c index 585b618982..c2f8bcbb86 100644 --- a/drivers/usbhid-ups.c +++ b/drivers/usbhid-ups.c @@ -145,8 +145,10 @@ hid_dev_handle_t udev = HID_DEV_HANDLE_CLOSED; * CyberPower UT series sometime need a bit of help deciding their online status. * This quirk is to enable the special handling of OL & DISCHRG at the same time * as being OB (on battery power/no mains power). Enabled by device config flag. + * NOTE: Also known by legacy alias "onlinedischarge", deprecated but tolerated + * since NUT v2.8.2. */ -static int onlinedischarge = 0; +static int onlinedischarge_onbattery = 0; /** * Some UPS models (e.g. APC were seen to do so) report OL & DISCHRG when they @@ -944,7 +946,10 @@ void upsdrv_makevartable(void) addvar(VAR_FLAG, "pollonly", "Don't use interrupt pipe, only use polling"); addvar(VAR_FLAG, "onlinedischarge", - "Set to treat discharging while online as being offline"); + "Set to treat discharging while online as being offline/on-battery (DEPRECATED, use onlinedischarge_onbattery)"); + + addvar(VAR_FLAG, "onlinedischarge_onbattery", + "Set to treat discharging while online as being offline/on-battery"); addvar(VAR_FLAG, "onlinedischarge_calibration", "Set to treat discharging while online as doing calibration"); @@ -1255,8 +1260,8 @@ void upsdrv_initups(void) } /* Activate Cyberpower tweaks */ - if (testvar("onlinedischarge")) { - onlinedischarge = 1; + if (testvar("onlinedischarge") || testvar("onlinedischarge_onbattery")) { + onlinedischarge_onbattery = 1; } if (testvar("onlinedischarge_calibration")) { @@ -1889,18 +1894,18 @@ static void ups_status_set(void) status_set("CAL"); /* calibration */ } - if (onlinedischarge) { + if (onlinedischarge_onbattery) { /* if we treat OL+DISCHRG as being offline */ status_set("OB"); /* on battery */ } - if (!onlinedischarge && !onlinedischarge_calibration) { + if (!onlinedischarge_onbattery && !onlinedischarge_calibration) { if (!(ups_status & STATUS(CALIB))) { /* if in OL+DISCHRG unknowingly, warn user */ upslogx(LOG_WARNING, "%s: seems that UPS [%s] is in OL+DISCHRG state now. " "Is it calibrating (perhaps you want to set 'onlinedischarge_calibration' option)? " "Note that some UPS models (e.g. CyberPower UT series) emit OL+DISCHRG when " - "in fact offline/on-battery (perhaps you want to set 'onlinedischarge' option).", + "in fact offline/on-battery (perhaps you want to set 'onlinedischarge_onbattery' option).", __func__, upsname); } /* if we're calibrating */ From 40a53dad51d1550d2264a66b7db19e275cf877b7 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 5 Dec 2023 15:27:40 +0100 Subject: [PATCH 2/3] drivers/usbhid-ups.c, docs/man/usbhid-ups.txt, NEWS.adoc: introduce "onlinedischarge_log_throttle_sec" setting and/or throttling by changes of battery.charge [#2214] Signed-off-by: Jim Klimov --- NEWS.adoc | 9 +++ docs/man/usbhid-ups.txt | 17 +++++ drivers/usbhid-ups.c | 160 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 178 insertions(+), 8 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index d75d36c82b..0ef077d12f 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -110,6 +110,15 @@ https://github.com/networkupstools/nut/milestone/10 * The `onlinedischarge` configuration flag name was too ambiguous and got deprecated (will be supported but no longer promoted by documentation), introducing `onlinedischarge_onbattery` as the meaningful alias. [#2213] + * Logged notifications about `OL+DISCHRG` state should now be throttled + (see the driver manual page for more details) [#2214]: + - If `battery.charge` is available, make the message when entering the + state and then only if the charge differs from that when we posted + the earlier message (e.g. really discharging); + - Also can throttle to a time frequency configurable by a new option + `onlinedischarge_log_throttle_sec`, by default 30 sec if `battery.charge` + is not reported by the device (should be frequent by default, in case + the UPS-reported state combination does reflect a bad power condition). Release notes for NUT 2.8.1 - what's new since 2.8.0 ---------------------------------------------------- diff --git a/docs/man/usbhid-ups.txt b/docs/man/usbhid-ups.txt index c82823995c..3e4f1a0a53 100644 --- a/docs/man/usbhid-ups.txt +++ b/docs/man/usbhid-ups.txt @@ -135,6 +135,23 @@ NOTE: If it takes so long on your device that a shutdown gets issued, you may want to look at `upsmon` option `OFFDURATION` used to filter out temporary values of "administrative OFF" as not a loss of a feed for the powered load. +*onlinedischarge_log_throttle_sec*='num':: +Set the minimum frequency (in seconds) at which warnings would be emitted +for an otherwise not handled `OL+DISCHRG` device status combination. +Negative values disable sequentially repeated messages (when this state +appears and persists). ++ +If the device does not report `battery.charge`, the default value is 30 seconds +(fairly frequent, in case the UPS-reported state combination does reflect a +bad power condition and so the situation is urgent). ++ +If it does report `battery.charge`, by default the repeated notifications +would only be logged if this charge is different from when the message was +emitted previously (e.g. when the battery is really discharging). ++ +If both this option is set, and `battery.charge` is correctly reported, +either of these rules allow the notification to be logged. + *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 diff --git a/drivers/usbhid-ups.c b/drivers/usbhid-ups.c index c2f8bcbb86..8f169b4690 100644 --- a/drivers/usbhid-ups.c +++ b/drivers/usbhid-ups.c @@ -157,6 +157,36 @@ static int onlinedischarge_onbattery = 0; */ static int onlinedischarge_calibration = 0; +/** + * If/when an UPS reports OL & DISCHRG and we do not use any other special + * settings (e.g. they do not match the actual device capabilities/behaviors), + * the driver logs messages about the unexpected situation - on every cycle + * if must be. This setting allows to throttle frequency of such messages. + * A value of 0 is small enough to log on every processing cycle (old noisy + * de-facto default before NUT v2.8.2 which this throttle alleviates). + * A value of -1 (any negative passed via configuration) disables repeated + * messages completely. + * Default (-2) below is not final: if this throttle variable is *not* set + * in the driver configuration section, and... + * - If the device reports a battery.charge, the driver would default to + * only repeat reports about OL & DISCHRG when this charge changes from + * what it was when the previous report was made, independent of time; + * - Otherwise 30 sec. + * If both the throttle is set and battery.charge is reported (and changes + * over time), then hitting either trigger allows the message to be logged. + */ +static int onlinedischarge_log_throttle_sec = -2; +/** + * When did we last emit the message? + */ +static time_t onlinedischarge_log_throttle_timestamp = 0; +/** + * Last known battery charge (rounded to whole percent units) + * as of when we last actually logged about OL & DISCHRG. + * Gets reset to -1 whenever this condition is not present. + */ +static int onlinedischarge_log_throttle_charge = -1; + /* support functions */ static hid_info_t *find_nut_info(const char *varname); static hid_info_t *find_hid_info(const HIDData_t *hiddata); @@ -954,6 +984,9 @@ void upsdrv_makevartable(void) addvar(VAR_FLAG, "onlinedischarge_calibration", "Set to treat discharging while online as doing calibration"); + addvar(VAR_VALUE, "onlinedischarge_log_throttle_sec", + "Set to throttle log messages about discharging while online (only so often)"); + addvar(VAR_FLAG, "disable_fix_report_desc", "Set to disable fix-ups for broken USB encoding, etc. which we apply by default on certain vendors/products"); @@ -1259,7 +1292,7 @@ void upsdrv_initups(void) interrupt_only = 1; } - /* Activate Cyberpower tweaks */ + /* Activate Cyberpower/APC tweaks */ if (testvar("onlinedischarge") || testvar("onlinedischarge_onbattery")) { onlinedischarge_onbattery = 1; } @@ -1268,6 +1301,26 @@ void upsdrv_initups(void) onlinedischarge_calibration = 1; } + val = getval("onlinedischarge_log_throttle_sec"); + if (val) { + int ipv = atoi(val); + if (ipv == 0 && strcmp("0", val)) { + onlinedischarge_log_throttle_sec = 30; + upslogx(LOG_WARNING, + "Warning: invalid value for " + "onlinedischarge_log_throttle_sec: %s, " + "defaulting to %d", + val, onlinedischarge_log_throttle_sec); + } else { + if (ipv < 0) { + /* Has a specific meaning: user said be quiet */ + onlinedischarge_log_throttle_sec = -1; + } else { + onlinedischarge_log_throttle_sec = ipv; + } + } + } + if (testvar("disable_fix_report_desc")) { disable_fix_report_desc = 1; } @@ -1885,9 +1938,23 @@ static void ups_status_set(void) status_set("CAL"); /* calibration */ } + if ((!(ups_status & STATUS(DISCHRG))) && ( + onlinedischarge_log_throttle_timestamp != 0 + || onlinedischarge_log_throttle_charge != -1 + )) { + upsdebugx(1, + "%s: seems that UPS [%s] was in OL+DISCHRG state, " + "but no longer is now.", + __func__, upsname); + onlinedischarge_log_throttle_timestamp = 0; + onlinedischarge_log_throttle_charge = -1; + } + if (!(ups_status & STATUS(ONLINE))) { status_set("OB"); /* on battery */ } else if ((ups_status & STATUS(DISCHRG))) { + int do_logmsg = 0, current_charge = 0; + /* if online but discharging */ if (onlinedischarge_calibration) { /* if we treat OL+DISCHRG as calibrating */ @@ -1900,17 +1967,94 @@ static void ups_status_set(void) } if (!onlinedischarge_onbattery && !onlinedischarge_calibration) { + /* Some situation not managed by end-user's hints */ if (!(ups_status & STATUS(CALIB))) { - /* if in OL+DISCHRG unknowingly, warn user */ - upslogx(LOG_WARNING, "%s: seems that UPS [%s] is in OL+DISCHRG state now. " - "Is it calibrating (perhaps you want to set 'onlinedischarge_calibration' option)? " - "Note that some UPS models (e.g. CyberPower UT series) emit OL+DISCHRG when " - "in fact offline/on-battery (perhaps you want to set 'onlinedischarge_onbattery' option).", - __func__, upsname); + /* if in OL+DISCHRG unknowingly, warn user, + * unless we throttle it this time - see below */ + do_logmsg = 1; } - /* if we're calibrating */ + /* if we're presumably calibrating */ status_set("OL"); /* on line */ } + + if (do_logmsg) { + /* Any throttling to apply? */ + const char *s; + + /* First disable, then enable if OK for noise*/ + do_logmsg = 0; + + /* Time or not, did the charge change since last log? */ + if ((s = dstate_getinfo("battery.charge"))) { + /* NOTE: "0" may mean a conversion error: */ + current_charge = atoi(s); + if (current_charge != onlinedischarge_log_throttle_charge) { + /* Charge has changed */ + do_logmsg = 1; + } + } else { + /* Should we default the time throttle? */ + if (onlinedischarge_log_throttle_sec == -2) { + onlinedischarge_log_throttle_sec = 30; + /* Report once, so almost loud */ + upsdebugx(1, "%s: seems battery.charge " + "is not served by this device " + "or subdriver; defaulting " + "onlinedischarge_log_throttle_sec " + "to %d", + __func__, + onlinedischarge_log_throttle_sec); + } + } + + /* Do we track and honor time since last log? */ + if (onlinedischarge_log_throttle_timestamp > 0 + && onlinedischarge_log_throttle_sec >= 0 + ) { + time_t now; + time(&now); + + if ((now - onlinedischarge_log_throttle_timestamp) + >= onlinedischarge_log_throttle_sec + ) { + /* Enough time elapsed */ + do_logmsg = 1; + } + } + } + + if (do_logmsg) { + char msg_charge[LARGEBUF]; + msg_charge[0] = '\0'; + + /* Remember when we last logged this message */ + time(&onlinedischarge_log_throttle_timestamp); + + if (current_charge > 0 + && current_charge != onlinedischarge_log_throttle_charge + ) { + /* Charge has changed, report and remember this */ + if (onlinedischarge_log_throttle_charge < 0) { + /* First sequential message like this */ + snprintf(msg_charge, sizeof(msg_charge), + "Battery charge is currently %d. ", + current_charge); + } else { + snprintf(msg_charge, sizeof(msg_charge), + "Battery charge changed from %d to %d " + "since last such report. ", + onlinedischarge_log_throttle_charge, + current_charge); + } + onlinedischarge_log_throttle_charge = current_charge; + } + + upslogx(LOG_WARNING, "%s: seems that UPS [%s] is in OL+DISCHRG state now. %s" + "Is it calibrating (perhaps you want to set 'onlinedischarge_calibration' option)? " + "Note that some UPS models (e.g. CyberPower UT series) emit OL+DISCHRG when " + "in fact offline/on-battery (perhaps you want to set 'onlinedischarge_onbattery' option).", + __func__, upsname, msg_charge); + } } else if ((ups_status & STATUS(ONLINE))) { /* if simply online */ status_set("OL"); From 8b7811251177ed3d8ea0503ee7d4ddc64e7dffa9 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 5 Dec 2023 16:21:56 +0100 Subject: [PATCH 3/3] drivers/usbhid-ups.c, docs/man/usbhid-ups.txt, NEWS.adoc: introduce "onlinedischarge_log_throttle_hovercharge" setting [#2215] Signed-off-by: Jim Klimov --- NEWS.adoc | 5 +++-- docs/man/usbhid-ups.txt | 8 ++++++++ docs/nut.dict | 3 ++- drivers/usbhid-ups.c | 39 ++++++++++++++++++++++++++++++++++++--- 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index 0ef077d12f..7439105447 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -111,10 +111,11 @@ https://github.com/networkupstools/nut/milestone/10 deprecated (will be supported but no longer promoted by documentation), introducing `onlinedischarge_onbattery` as the meaningful alias. [#2213] * Logged notifications about `OL+DISCHRG` state should now be throttled - (see the driver manual page for more details) [#2214]: + (see the driver manual page for more details) [#2214, #2215]: - If `battery.charge` is available, make the message when entering the state and then only if the charge differs from that when we posted - the earlier message (e.g. really discharging); + the earlier message (e.g. really discharging) and is under + `onlinedischarge_log_throttle_hovercharge` value (defaults to 100%); - Also can throttle to a time frequency configurable by a new option `onlinedischarge_log_throttle_sec`, by default 30 sec if `battery.charge` is not reported by the device (should be frequent by default, in case diff --git a/docs/man/usbhid-ups.txt b/docs/man/usbhid-ups.txt index 3e4f1a0a53..323fe53deb 100644 --- a/docs/man/usbhid-ups.txt +++ b/docs/man/usbhid-ups.txt @@ -152,6 +152,14 @@ emitted previously (e.g. when the battery is really discharging). If both this option is set, and `battery.charge` is correctly reported, either of these rules allow the notification to be logged. +*onlinedischarge_log_throttle_hovercharge*='num':: +See details in `onlinedischarge_log_throttle_sec` and `battery.charge` based +log message throttling description above. This option adds a concept of UPS +"hovering" a battery charge at some level deemed safe for its chemistry, and +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. + *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 diff --git a/docs/nut.dict b/docs/nut.dict index c04169b07e..c428c982f3 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3422 utf-8 +personal_ws-1.1 en 3423 utf-8 AAC AAS ABI @@ -2173,6 +2173,7 @@ hostnames hostsfile hotplug hotplugging +hovercharge hpe href htaccess diff --git a/drivers/usbhid-ups.c b/drivers/usbhid-ups.c index 8f169b4690..4775d38fe8 100644 --- a/drivers/usbhid-ups.c +++ b/drivers/usbhid-ups.c @@ -186,6 +186,14 @@ static time_t onlinedischarge_log_throttle_timestamp = 0; * Gets reset to -1 whenever this condition is not present. */ static int onlinedischarge_log_throttle_charge = -1; +/** + * If battery.charge is served and equals or exceeds this value, + * suppress logging about OL & DISCHRG if battery.charge varied + * since last logged message. Defaults to 100% as some devices + * only report this state combo when fully charged (probably + * they try to prolong battery life by not over-charging it). + */ +static int onlinedischarge_log_throttle_hovercharge = 100; /* support functions */ static hid_info_t *find_nut_info(const char *varname); @@ -986,6 +994,9 @@ void upsdrv_makevartable(void) addvar(VAR_VALUE, "onlinedischarge_log_throttle_sec", "Set to throttle log messages about discharging while online (only so often)"); + + addvar(VAR_VALUE, "onlinedischarge_log_throttle_hovercharge", + "Set to throttle log messages about discharging while online (only if battery.charge is under this value)"); addvar(VAR_FLAG, "disable_fix_report_desc", "Set to disable fix-ups for broken USB encoding, etc. which we apply by default on certain vendors/products"); @@ -1988,9 +1999,31 @@ static void ups_status_set(void) if ((s = dstate_getinfo("battery.charge"))) { /* NOTE: "0" may mean a conversion error: */ current_charge = atoi(s); - if (current_charge != onlinedischarge_log_throttle_charge) { - /* Charge has changed */ - do_logmsg = 1; + if (current_charge > 0 + && current_charge != onlinedischarge_log_throttle_charge + ) { + /* Charge has changed, but is it + * now low enough to worry? */ + if (onlinedischarge_log_throttle_hovercharge + < onlinedischarge_log_throttle_charge + ) { + upsdebugx(3, "%s: current " + "battery.charge=%d is under " + "onlinedischarge_log_throttle_charge=%d", + __func__, current_charge, + onlinedischarge_log_throttle_charge); + do_logmsg = 1; + } else { + /* All seems OK, don't spam log + * unless running at a really + * high debug verbosity */ + upsdebugx(5, "%s: current " + "battery.charge=%d " + "is okay compared to " + "onlinedischarge_log_throttle_charge=%d", + __func__, current_charge, + onlinedischarge_log_throttle_charge); + } } } else { /* Should we default the time throttle? */