From a56281bf2691f5b3f940ce7294c1db03aa9c3436 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Thu, 16 May 2024 15:41:01 +0200 Subject: [PATCH 01/65] bicker_ser: Bicker UPSes based on PSZ-1063 Add a new driver for Bicker DC UPS systems based on the PSZ-1063 extension module. This includes UPSIC-1205, UPSIC-2403 and DC2412-UPS(LD) models. Signed-off-by: Nicola Fontana --- drivers/Makefile.am | 4 +- drivers/bicker_ser.c | 306 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 309 insertions(+), 1 deletion(-) create mode 100644 drivers/bicker_ser.c diff --git a/drivers/Makefile.am b/drivers/Makefile.am index 670e6116ed..d8db1d3661 100644 --- a/drivers/Makefile.am +++ b/drivers/Makefile.am @@ -57,7 +57,7 @@ SERIAL_DRIVERLIST = al175 bcmxcp belkin belkinunv bestfcom \ gamatronic genericups isbmex liebert liebert-esp2 masterguard metasys \ mge-utalk microdowell microsol-apc mge-shut oneac optiups powercom rhino \ safenet nutdrv_siemens-sitop solis tripplite tripplitesu upscode2 victronups powerpanel \ - blazer_ser ivtscd apcsmart apcsmart-old riello_ser sms_ser + blazer_ser ivtscd apcsmart apcsmart-old riello_ser sms_ser bicker_ser SNMP_DRIVERLIST = snmp-ups USB_LIBUSB_DRIVERLIST = usbhid-ups bcmxcp_usb tripplite_usb \ blazer_usb richcomm_usb riello_usb \ @@ -186,6 +186,8 @@ riello_ser_SOURCES = riello.c riello_ser.c riello_ser_LDADD = $(LDADD) -lm sms_ser_SOURCES = sms_ser.c sms_ser_LDADD = $(LDADD) -lm +bicker_ser_SOURCES = bicker_ser.c +bicker_ser_LDADD = $(LDADD) -lm # non-serial drivers: these use custom LDADD and/or CFLAGS diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c new file mode 100644 index 0000000000..5be95bdc79 --- /dev/null +++ b/drivers/bicker_ser.c @@ -0,0 +1,306 @@ +/* + * bicker_ser.c: support for Bicker SuperCapacitors DC UPSes + * + * Copyright (C) 2024 - Nicola Fontana + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +/* The protocol is detailed in section 14 of the DC/UPSIC user's manual, + * downloadable somewhere from the Bicker website. + * + * Basically, this is a binary protocol without checksums: + * + * 1 byte 1 byte 1 byte 1 byte 0..252 bytes 1 byte + * +-------+-------+-------+-------+--- - - - ---+-------+ + * | SOH | Size | Index | CMD | Data | EOT | + * +-------+-------+-------+-------+--- - - - ---+-------+ + * + * where: + * - `SOH` is the start signal ('\x01') + * - `Size` is the length of the `Data` field (in bytes) plus 3 + * - `Index` is the command index (always '\x03') + * - `CMD` is the command code to execute + * - `Data` is the (optional) argument of the command + * - `EOT` is the end signal ('\x04') + * + * The same format is used for incoming and outcoming packets. Although + * not explicitly documented, the data returned by the `Data` field + * seems to be in little-endian order. + */ + +#include "config.h" +#include "main.h" +#include "attribute.h" + +#include "serial.h" + +#define DRIVER_NAME "Bicker serial protocol" +#define DRIVER_VERSION "0.01" + +#define BICKER_PACKET (1+1+1+1+252+1) +#define BICKER_SOH '\x01' +#define BICKER_EOT '\x04' + +upsdrv_info_t upsdrv_info = { + DRIVER_NAME, + DRIVER_VERSION, + "Nicola Fontana ", + DRV_EXPERIMENTAL, + { NULL } +}; + +static ssize_t bicker_send(char cmd, const char *data) +{ + char buf[BICKER_PACKET]; + size_t datalen, buflen; + ssize_t ret; + + ser_flush_io(upsfd); + + if (data != NULL) { + datalen = strlen(data); + if (datalen > 252) { + upslogx(LOG_ERR, "Data size exceeded: %d > 252", + (int)datalen); + return 0; + } + memcpy(buf + 4, data, datalen); + } else { + datalen = 0; + } + + buf[0] = BICKER_SOH; + buf[1] = datalen + 3; /* Packet size must include the header */ + buf[2] = '\x03'; /* Command index is always 3 */ + buf[3] = cmd; + buf[4 + datalen] = BICKER_EOT; + + /* The full packet includes SOH and EOT bytes too */ + buflen = datalen + 3 + 2; + + ret = ser_send_buf(upsfd, buf, buflen); + if (ret < 0) { + upslog_with_errno(LOG_WARNING, "ser_send_buf failed"); + return ret; + } else if ((size_t) ret != buflen) { + upslogx(LOG_WARNING, "ser_send_buf returned %d instead of %d", + (int)ret, (int)buflen); + return 0; + } + + upsdebug_hex(3, "bicker_send", buf, buflen); + return ret; +} + +static ssize_t bicker_receive_buf(void *dst, size_t dstlen) +{ + ssize_t ret; + + ret = ser_get_buf(upsfd, dst, dstlen, 1, 1); + if (ret < 0) { + upslog_with_errno(LOG_WARNING, "ser_get_buf failed"); + return ret; + } else if ((size_t) ret != dstlen) { + upslogx(LOG_WARNING, "ser_get_buf returned %d instead of %d", + (int)ret, (int)dstlen); + return 0; + } + + upsdebug_hex(3, "bicker_receive_buf", dst, dstlen); + return ret; +} + +static ssize_t bicker_receive(char cmd, void *dst, size_t dstlen) +{ + ssize_t ret; + size_t datalen; + char buf[BICKER_PACKET]; + + ret = bicker_receive_buf(buf, 1); + if (ret <= 0) { + return ret; + } else if (buf[0] != BICKER_SOH) { + upslogx(LOG_WARNING, "Received 0x%02X instead of SOH (0x%02X)", + (unsigned)buf[0], (unsigned)BICKER_SOH); + return 0; + } + + ret = bicker_receive_buf(buf + 1, 1); + if (ret <= 0) { + return ret; + } + + datalen = buf[1] - 3; + + ret = bicker_receive_buf(buf + 2, datalen + 3); + if (ret <= 0) { + return ret; + } else if (buf[datalen + 4] != BICKER_EOT) { + upslogx(LOG_WARNING, "Received 0x%02X instead of EOT (0x%02X)", + (unsigned)buf[datalen + 4], (unsigned)BICKER_EOT); + return 0; + } else if (buf[3] != cmd) { + upslogx(LOG_WARNING, "Commands do not match: sent 0x%02X, received 0x%02X", + (unsigned)cmd, (unsigned)buf[1]); + return 0; + } else if (dstlen < datalen) { + upslogx(LOG_ERR, "Not enough space for the payload: %d < %d", + (int)dstlen, (int)datalen); + return 0; + } + + memcpy(dst, buf + 4, datalen); + + upsdebug_hex(3, "bicker_receive", buf, datalen + 5); + return datalen; +} + +static ssize_t bicker_read_int16(char cmd, int16_t *dst) +{ + ssize_t ret; + + ret = bicker_send(cmd, NULL); + if (ret <= 0) { + return ret; + } + + /* XXX: by default, data is stored in little-endian so, + * on big-endian platforms, a byte swap should be needed */ + return bicker_receive(cmd, dst, 2); +} + +void upsdrv_initinfo(void) +{ + dstate_setinfo("device.type", "ups"); + dstate_setinfo("device.mfr", "Bicker Elektronik GmbH"); + /* No way to detect the UPS model within the protocol but the + * serial communication is provided by the PSZ-1063 extension + * module, so it seems correct to put that into the model */ + dstate_setinfo("device.model", "PSZ-1063"); +} + +void upsdrv_updateinfo(void) +{ + int16_t data, charge_current; + ssize_t ret; + + ret = bicker_read_int16('\x25', &data); + if (ret <= 0) { + dstate_datastale(); + return; + } + dstate_setinfo("input.voltage", "%.1f", (double) data / 1000); + + ret = bicker_read_int16('\x28', &data); + if (ret <= 0) { + dstate_datastale(); + return; + } + dstate_setinfo("input.current", "%.3f", (double) data / 1000); + + ret = bicker_read_int16('\x27', &data); + if (ret <= 0) { + dstate_datastale(); + return; + } + dstate_setinfo("output.voltage", "%.3f", (double) data / 1000); + + /* This is a supercap UPS so, in this context, + * the "battery" is the supercap stack */ + ret = bicker_read_int16('\x26', &data); + if (ret <= 0) { + dstate_datastale(); + return; + } + dstate_setinfo("battery.voltage", "%.3f", (double) data / 1000); + + ret = bicker_read_int16('\x29', &charge_current); + if (ret <= 0) { + dstate_datastale(); + return; + } + dstate_setinfo("battery.current", "%.3f", (double) charge_current / 1000); + + /* GetChargeStaturRegister returns a 16 bit register: + * + * 0. SD Shows that the device is in step-down (charging) mode. + * 1. SU Shows that the device is in step-up (backup) mode. + * 2. CV Shows that the charger is in constant voltage mode. + * 3. UV Shows that the charger is in undervoltage lockout. + * 4. CL Shows that the device is in input current limit. + * 5. CG Shows that the capacitor voltage is above power good threshold. + * 6. CS Shows that the capacitor manager is shunting. + * 7. CB Shows that the capacitor manager is balancing. + * 8. CD Shows that the charger is temporarily disabled for capacitance measurement. + * 9. CC Shows that the charger is in constant current mode. + * 10. RV Reserved Bit + * 11. PF Shows that the input voltage is below the Power Fail Input (PFI) threshold. + * 12. RV Reserved Bit + * 13. RV Reserved Bit + * 14. RV Reserved Bit + * 15. RV Reserved Bit + */ + ret = bicker_read_int16('\x1B', &data); + if (ret <= 0) { + dstate_datastale(); + return; + } + + status_init(); + + /* Check PF (bit 11) to know if the UPS is on line/battery */ + status_set((data & 0x0800) > 0 ? "OB" : "OL"); + + /* Check CG (bit 5) to know if the battery is low */ + if ((data & 0x0020) == 0) { + status_set("LB"); + } + + /* If there is a current of more than 1 A flowing towards + * the supercaps, consider the battery in charging mode */ + if (charge_current > 1000) { + status_set("CHRG"); + } + + status_commit(); + + dstate_dataok(); +} + +void upsdrv_shutdown(void) +{ + upslogx(LOG_ERR, "shutdown not supported"); + set_exit_flag(-1); +} + +void upsdrv_help(void) +{ +} + +void upsdrv_makevartable(void) +{ +} + +void upsdrv_initups(void) +{ + upsfd = ser_open(device_path); + ser_set_speed(upsfd, device_path, B38400); +} + +void upsdrv_cleanup(void) +{ + ser_close(upsfd, device_path); +} From 021472d53055a3e0ebd35c470a84f1c7d2b06548 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Fri, 17 May 2024 13:26:25 +0200 Subject: [PATCH 02/65] bicker_ser: add shutdown support The protocol supports shutdown after a custom delay settable between 0 and 255 seconds. Unfortunately the real device (UPSIC-2403D) seems to always enforce a 2 seconds delay. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 85 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 79 insertions(+), 6 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 5be95bdc79..1d46ca6cb8 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -53,6 +53,8 @@ #define BICKER_PACKET (1+1+1+1+252+1) #define BICKER_SOH '\x01' #define BICKER_EOT '\x04' +#define BICKER_DELAY 20 +#define BICKER_RETRIES 3 upsdrv_info_t upsdrv_info = { DRIVER_NAME, @@ -62,16 +64,15 @@ upsdrv_info_t upsdrv_info = { { NULL } }; -static ssize_t bicker_send(char cmd, const char *data) +static ssize_t bicker_send(char cmd, const void *data, size_t datalen) { char buf[BICKER_PACKET]; - size_t datalen, buflen; + size_t buflen; ssize_t ret; ser_flush_io(upsfd); if (data != NULL) { - datalen = strlen(data); if (datalen > 252) { upslogx(LOG_ERR, "Data size exceeded: %d > 252", (int)datalen); @@ -162,7 +163,9 @@ static ssize_t bicker_receive(char cmd, void *dst, size_t dstlen) return 0; } - memcpy(dst, buf + 4, datalen); + if (dst != NULL) { + memcpy(dst, buf + 4, datalen); + } upsdebug_hex(3, "bicker_receive", buf, datalen + 5); return datalen; @@ -172,7 +175,7 @@ static ssize_t bicker_read_int16(char cmd, int16_t *dst) { ssize_t ret; - ret = bicker_send(cmd, NULL); + ret = bicker_send(cmd, NULL, 0); if (ret <= 0) { return ret; } @@ -182,6 +185,60 @@ static ssize_t bicker_read_int16(char cmd, int16_t *dst) return bicker_receive(cmd, dst, 2); } +/* For some reason the `seconds` delay (at least on my UPSIC-2403D) + * is not honored: the shutdown is always delayed by 2 seconds. This + * fixed delay seems to be independent from the state of the UPS (on + * line or on battery) and from the dip switches setting. + * + * As response I get the same command with `0xE0` in the data field. + */ +static ssize_t bicker_delayed_shutdown(uint8_t seconds) +{ + ssize_t ret; + uint8_t response; + + ret = bicker_send('\x32', &seconds, 1); + if (ret <= 0) { + return ret; + } + + ret = bicker_receive('\x32', &response, 1); + if (ret > 0) { + upslogx(LOG_INFO, "Shutting down in %d seconds: response = 0x%02X", + (int)seconds, (unsigned)response); + } + + return ret; +} + +static ssize_t bicker_shutdown(void) +{ + const char *str; + int delay; + + str = dstate_getinfo("ups.delay.shutdown"); + delay = atoi(str); + if (delay > 255) { + upslogx(LOG_WARNING, "Shutdown delay too big: %d > 255", + delay); + delay = 255; + } + + return bicker_delayed_shutdown(delay); +} + +static int bicker_instcmd(const char *cmdname, const char *extra) +{ + NUT_UNUSED_VARIABLE(extra); + + if (!strcasecmp(cmdname, "shutdown.return")) { + bicker_shutdown(); + } + + upslogx(LOG_NOTICE, "instcmd: unknown command [%s]", cmdname); + return STAT_INSTCMD_UNKNOWN; +} + void upsdrv_initinfo(void) { dstate_setinfo("device.type", "ups"); @@ -190,6 +247,8 @@ void upsdrv_initinfo(void) * serial communication is provided by the PSZ-1063 extension * module, so it seems correct to put that into the model */ dstate_setinfo("device.model", "PSZ-1063"); + + upsh.instcmd = bicker_instcmd; } void upsdrv_updateinfo(void) @@ -282,7 +341,16 @@ void upsdrv_updateinfo(void) void upsdrv_shutdown(void) { - upslogx(LOG_ERR, "shutdown not supported"); + int retry; + + for (retry = 1; retry <= BICKER_RETRIES; retry++) { + if (bicker_shutdown() > 0) { + set_exit_flag(-2); /* EXIT_SUCCESS */ + return; + } + } + + upslogx(LOG_ERR, "Shutdown failed!"); set_exit_flag(-1); } @@ -298,6 +366,11 @@ void upsdrv_initups(void) { upsfd = ser_open(device_path); ser_set_speed(upsfd, device_path, B38400); + + /* Adding this variable here because upsdrv_initinfo() is not + * called when triggering a forced shutdown and + * "ups.delay.shutdown" is needed right there */ + dstate_setinfo("ups.delay.shutdown", "%u", BICKER_DELAY); } void upsdrv_cleanup(void) From 3ae6b50f74495087a31e346beffc81aedfc32c53 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Fri, 17 May 2024 14:05:43 +0200 Subject: [PATCH 03/65] data/driver.list.in: add bicker_ser based UPSes Signed-off-by: Nicola Fontana --- data/driver.list.in | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/data/driver.list.in b/data/driver.list.in index 7543a1fc02..2480f23247 100644 --- a/data/driver.list.in +++ b/data/driver.list.in @@ -196,6 +196,11 @@ "Best Power" "ups" "1" "Micro-Ferrups" "" "bestuferrups" "Best Power" "ups" "1" "Fortress/Ferrups" "f-command support" "bestfcom" +"Bicker" "ups" "3" "UPSIC-1205" "" "bicker_ser" +"Bicker" "ups" "3" "UPSIC-2403" "" "bicker_ser" +"Bicker" "ups" "3" "DC2412-UPS" "" "bicker_ser" +"Bicker" "ups" "3" "DC2412-UPS-LD" "" "bicker_ser" + "Borri" "ups" "2" "B400-010-B/B400-020-B/B400-030-B/B400-010-C/B400-020-C/B400-030-C" "USB" "blazer_usb" "Borri" "ups" "2" "B400-R010-B/B400-R020-B/B400-R030-B/B400-R010-C/B400-R020-C/B400-R030-C" "USB" "blazer_usb" "Borri" "ups" "2" "B500-060-B/B500-100-B/B500-060-C/B500-100-C" "USB" "blazer_usb" From 0b53475254fb216d1643de0d7e1c5a571b018132 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Fri, 17 May 2024 20:26:17 +0200 Subject: [PATCH 04/65] bicker_ser: add big-endian platform support Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 1d46ca6cb8..c6828c27c8 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -55,6 +55,7 @@ #define BICKER_EOT '\x04' #define BICKER_DELAY 20 #define BICKER_RETRIES 3 +#define BYTESWAP(in) ((((uint16_t)(in) & 0x00FF) << 8) + (((uint16_t)(in) & 0xFF00) >> 8)) upsdrv_info_t upsdrv_info = { DRIVER_NAME, @@ -180,9 +181,18 @@ static ssize_t bicker_read_int16(char cmd, int16_t *dst) return ret; } - /* XXX: by default, data is stored in little-endian so, - * on big-endian platforms, a byte swap should be needed */ - return bicker_receive(cmd, dst, 2); + ret = bicker_receive(cmd, dst, 2); + if (ret <= 0) { + return ret; + } + +#ifdef WORDS_BIGENDIAN + /* By default data is stored in little-endian so, on big-endian + * platforms, a byte swap must be performed */ + *dst = BYTESWAP(*dst); +#endif + + return ret; } /* For some reason the `seconds` delay (at least on my UPSIC-2403D) From 6538f2f4ce89654dfb78d5fdb2a73d50728adb44 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Fri, 17 May 2024 20:46:01 +0200 Subject: [PATCH 05/65] bicker_ser: use length macros to increase expressiveness Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index c6828c27c8..dadd9f1d27 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -27,10 +27,11 @@ * +-------+-------+-------+-------+--- - - - ---+-------+ * | SOH | Size | Index | CMD | Data | EOT | * +-------+-------+-------+-------+--- - - - ---+-------+ + * | HEADER | * * where: * - `SOH` is the start signal ('\x01') - * - `Size` is the length of the `Data` field (in bytes) plus 3 + * - `Size` is the length (in bytes) of the header and the data field * - `Index` is the command index (always '\x03') * - `CMD` is the command code to execute * - `Data` is the (optional) argument of the command @@ -50,13 +51,17 @@ #define DRIVER_NAME "Bicker serial protocol" #define DRIVER_VERSION "0.01" -#define BICKER_PACKET (1+1+1+1+252+1) #define BICKER_SOH '\x01' #define BICKER_EOT '\x04' #define BICKER_DELAY 20 #define BICKER_RETRIES 3 #define BYTESWAP(in) ((((uint16_t)(in) & 0x00FF) << 8) + (((uint16_t)(in) & 0xFF00) >> 8)) +/* Protocol fixed lengths */ +#define BICKER_HEADER 3 +#define BICKER_MAXDATA (255 - BICKER_HEADER) +#define BICKER_PACKET (1 + BICKER_HEADER + BICKER_MAXDATA + 1) + upsdrv_info_t upsdrv_info = { DRIVER_NAME, DRIVER_VERSION, @@ -74,9 +79,9 @@ static ssize_t bicker_send(char cmd, const void *data, size_t datalen) ser_flush_io(upsfd); if (data != NULL) { - if (datalen > 252) { - upslogx(LOG_ERR, "Data size exceeded: %d > 252", - (int)datalen); + if (datalen > BICKER_MAXDATA) { + upslogx(LOG_ERR, "Data size exceeded: %d > %d", + (int)datalen, BICKER_MAXDATA); return 0; } memcpy(buf + 4, data, datalen); @@ -85,13 +90,11 @@ static ssize_t bicker_send(char cmd, const void *data, size_t datalen) } buf[0] = BICKER_SOH; - buf[1] = datalen + 3; /* Packet size must include the header */ + buf[1] = datalen + BICKER_HEADER; buf[2] = '\x03'; /* Command index is always 3 */ buf[3] = cmd; - buf[4 + datalen] = BICKER_EOT; - - /* The full packet includes SOH and EOT bytes too */ - buflen = datalen + 3 + 2; + buf[1 + BICKER_HEADER + datalen] = BICKER_EOT; + buflen = 1 + BICKER_HEADER + datalen + 1; ret = ser_send_buf(upsfd, buf, buflen); if (ret < 0) { @@ -145,8 +148,10 @@ static ssize_t bicker_receive(char cmd, void *dst, size_t dstlen) return ret; } - datalen = buf[1] - 3; + datalen = buf[1] - BICKER_HEADER; + /* Still to be read: command index (1 byte), command (1 byte), data + * (datalen bytes) and EOT (1 byte), i.e. `datalen + 3` bytes */ ret = bicker_receive_buf(buf + 2, datalen + 3); if (ret <= 0) { return ret; From 67ffe25b363d651c4b41b311a0767e987a8207b1 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Fri, 17 May 2024 21:47:43 +0200 Subject: [PATCH 06/65] bicker_ser: add manpage Signed-off-by: Nicola Fontana --- docs/man/Makefile.am | 3 ++ docs/man/bicker_ser.txt | 66 +++++++++++++++++++++++++++++++++++++++++ docs/nut.dict | 5 +++- 3 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 docs/man/bicker_ser.txt diff --git a/docs/man/Makefile.am b/docs/man/Makefile.am index 921cc1f9d3..52669c1cd4 100644 --- a/docs/man/Makefile.am +++ b/docs/man/Makefile.am @@ -430,6 +430,7 @@ SRC_SERIAL_PAGES = \ bestuferrups.txt \ bestups.txt \ bestfcom.txt \ + bicker_ser.txt \ blazer-common.txt \ blazer_ser.txt \ clone.txt \ @@ -477,6 +478,7 @@ MAN_SERIAL_PAGES = \ bestuferrups.8 \ bestups.8 \ bestfcom.8 \ + bicker_ser.8 \ blazer_ser.8 \ clone.8 \ dummy-ups.8 \ @@ -526,6 +528,7 @@ HTML_SERIAL_MANS = \ bestuferrups.html \ bestups.html \ bestfcom.html \ + bicker_ser.html \ blazer_ser.html \ clone.html \ dummy-ups.html \ diff --git a/docs/man/bicker_ser.txt b/docs/man/bicker_ser.txt new file mode 100644 index 0000000000..f5122cf819 --- /dev/null +++ b/docs/man/bicker_ser.txt @@ -0,0 +1,66 @@ +BICKER_SER(8) +============= + +NAME +---- + +bicker_ser - Driver for Bicker DC UPS via serial port connections + +SYNOPSIS +-------- + +*bicker_ser* -h + +*bicker_ser* -a 'UPS_NAME' ['OPTIONS'] + +NOTE: This man page only documents the hardware-specific features of the +*bicker_ser* driver. For information about the core driver, see +linkman:nutupsdrv[8]. + +SUPPORTED HARDWARE +------------------ + +*bicker_ser* supports all Bicker UPSes shipped with the PSZ-1053 extension +module such as UPSIC-1205, UPSIC-2403, DC2412-UPS and DC2412-UPS-LD. + +CABLING +------- + +The needed cable is a standard pin-to-pin serial cable with pins 2, 3 and 5 +(on DB9 connector) connected. + +EXTRA ARGUMENTS +--------------- + +This driver supports no extra arguments from linkman:ups.conf[5]. + +INSTANT COMMANDS +---------------- + +*shutdown.return*:: +Turn off the load and return when power is back. + +KNOWN ISSUES AND BUGS +--------------------- + +*ups.delay.shutdown is not honored*:: +Although that delay is properly set when sending the shutdown command, it seems +some UPS ignore it and use a fixed 2 seconds delay instead. + +AUTHOR +------ + +Nicola Fontana + +SEE ALSO +-------- + +The core driver: +~~~~~~~~~~~~~~~~ + +linkman:nutupsdrv[8] + +Internet resources: +~~~~~~~~~~~~~~~~~~~ + +The NUT (Network UPS Tools) home page: https://www.networkupstools.org/ diff --git a/docs/nut.dict b/docs/nut.dict index 7f99a82718..b0ad18af66 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3149 utf-8 +personal_ws-1.1 en 3151 utf-8 AAC AAS ABI @@ -373,6 +373,7 @@ Fideltronik Filipozzi Fiskars FlossMetrics +Fontana Forza Fosshost Frama @@ -858,6 +859,7 @@ PSSENTR PSUs PSW PSX +PSZ PThreads PULS PV @@ -1245,6 +1247,7 @@ UPS's UPSCONN UPSDESC UPSHOST +UPSIC UPSIMAGEPATH UPSLC UPSNOTIFY From ffd42d16024f230aa1e5b7891c9884762c842f84 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Sat, 18 May 2024 09:07:38 +0200 Subject: [PATCH 07/65] NEWS.adoc: announce bicker_ser driver [#2318] Signed-off-by: Nicola Fontana --- NEWS.adoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.adoc b/NEWS.adoc index 0313d740df..ee34f0eb3e 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -146,6 +146,10 @@ https://github.com/networkupstools/nut/milestone/11 respective warnings issued by the new generations of analysis tools. [#823, #2437, link:https://github.com/networkupstools/nut-website/issues/52[nut-website issue #52]] + - bicker_ser: added new driver for Bicker 12/24Vdc UPS via RS-232 serial + communication protocol, which supports any UPS shipped with the PSZ-1053 + extension module. [PR #2448] + Release notes for NUT 2.8.2 - what's new since 2.8.1 ---------------------------------------------------- From d5e7ce5c5198a7e64fd808cde63df30c4c187716 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Tue, 21 May 2024 11:13:40 +0200 Subject: [PATCH 08/65] bicker_ser: update protocol documentation Bicker told me to refer to the UPS Gen software's user manual for more details on their protocol, and in fact that manual contains much more info. Added a summary of available commands to the comments, as a quick reference for feature implementation. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 72 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 7 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index dadd9f1d27..00d49163ba 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -18,8 +18,10 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -/* The protocol is detailed in section 14 of the DC/UPSIC user's manual, - * downloadable somewhere from the Bicker website. +/* The protocol is reported in many Bicker's manuals but (according to + * Bicker itself) the best source is the UPS Gen Software's user manual: + * + * https://www.bicker.de/media/pdf/ff/cc/fe/en_user_manual_ups-gen2-configuration-software.pdf * * Basically, this is a binary protocol without checksums: * @@ -32,14 +34,70 @@ * where: * - `SOH` is the start signal ('\x01') * - `Size` is the length (in bytes) of the header and the data field - * - `Index` is the command index (always '\x03') - * - `CMD` is the command code to execute + * - `Index` is the command index: see AVAILABLE COMMANDS + * - `CMD` is the command code to execute: see AVAILABLE COMMANDS * - `Data` is the (optional) argument of the command * - `EOT` is the end signal ('\x04') * - * The same format is used for incoming and outcoming packets. Although - * not explicitly documented, the data returned by the `Data` field - * seems to be in little-endian order. + * The same format is used for incoming and outcoming packets. The data + * returned in the `Data` field is always in little-endian order. + * + * AVAILABLE COMMANDS + * ------------------ + * + * - Index = 0x01 (GENERIC) + * - CMD = 0x40 (status flags) + * - CMD = 0x41 (input voltage) + * - CMD = 0x42 (input current) + * - CMD = 0x43 (output voltage) + * - CMD = 0x44 (output current) + * - CMD = 0x45 (battery voltage) + * - CMD = 0x46 (battery current) + * - CMD = 0x47 (battery state of charge) + * - CMD = 0x48 (battery state of health) + * - CMD = 0x49 (battery cycles) + * - CMD = 0x4A (battery temperature) + * - CMD = 0x60 (manufacturer) + * - CMD = 0x61 (serial number) + * - CMD = 0x62 (device name) + * - CMD = 0x63 (firmware version) + * - CMD = 0x64 (battery pack) + * - CMD = 0x65 (firmware core version) + * - CMD = 0x66 (CPU temperature) + * - CMD = 0x67 (hardware revision) + * - CMD = 0x21 (UPS output) + * - CMD = 0x2F (shutdown flag) + * - CMD = 0x7A (reset parameter settings) + * + * - Index = 0x07 (PARAMETER) + * - CMD = 0x00 (get/set dummy entry: do not use!) + * - CMD = 0x01 (get/set load sensor) + * - CMD = 0x02 (get/set maximum backup time) + * - CMD = 0x03 (get/set os shutdown by timer) + * - CMD = 0x04 (get/set restart delay timer) + * - CMD = 0x05 (get/set minimum capacity to start) + * - CMD = 0x06 (get/set maximum backup time by in-1) + * - CMD = 0x07 (get/set os shutdown by soc) + * - CMD = 0x08 (get/set battery soc low threshold) + * - CMD = 0x09 (get/set relay event configuration) + * - CMD = 0x0A (get/set RS232 port configuration: place holder!) + * + * - Index = 0x03 (COMMANDS GOT FROM UPSIC MANUAL) + * - CMD = 0x1B (GetChargeStatusRegister) + * - CMD = 0x1C (GetMonitorStatusRegister) + * - CMD = 0x1E (GetCapacity) + * - CMD = 0x1F (GetEsr) + * - CMD = 0x20 (GetVCap1Voltage) + * - CMD = 0x21 (GetVCap2Voltage) + * - CMD = 0x22 (GetVCap3Voltage) + * - CMD = 0x23 (GetVCap4Voltage) + * - CMD = 0x25 (GetInputVoltage) + * - CMD = 0x26 (GetCapStackVoltage) + * - CMD = 0x27 (GetOutputVoltage) + * - CMD = 0x28 (GetInputCurrent) + * - CMD = 0x29 (GetChargeCurrent) + * - CMD = 0x31 (StartCapEsrMeasurement) + * - CMD = 0x32 (SetTimeToShutdown) */ #include "config.h" From 162e39ee316018f3b495e476ca2bbdc863a035b8 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Tue, 21 May 2024 14:18:24 +0200 Subject: [PATCH 09/65] bicker_ser: explicitly pass the command index In the previous implementation the command index was always '\x03', as stated by the UPSIC manual. After new evidence has been found (UPS-Gen2 software user manual), it came out there are much more commands grouped in different indexes. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 00d49163ba..c8b09bf18d 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -128,7 +128,7 @@ upsdrv_info_t upsdrv_info = { { NULL } }; -static ssize_t bicker_send(char cmd, const void *data, size_t datalen) +static ssize_t bicker_send(char idx, char cmd, const void *data, size_t datalen) { char buf[BICKER_PACKET]; size_t buflen; @@ -149,7 +149,7 @@ static ssize_t bicker_send(char cmd, const void *data, size_t datalen) buf[0] = BICKER_SOH; buf[1] = datalen + BICKER_HEADER; - buf[2] = '\x03'; /* Command index is always 3 */ + buf[2] = idx; buf[3] = cmd; buf[1 + BICKER_HEADER + datalen] = BICKER_EOT; buflen = 1 + BICKER_HEADER + datalen + 1; @@ -186,7 +186,7 @@ static ssize_t bicker_receive_buf(void *dst, size_t dstlen) return ret; } -static ssize_t bicker_receive(char cmd, void *dst, size_t dstlen) +static ssize_t bicker_receive(char idx, char cmd, void *dst, size_t dstlen) { ssize_t ret; size_t datalen; @@ -217,9 +217,13 @@ static ssize_t bicker_receive(char cmd, void *dst, size_t dstlen) upslogx(LOG_WARNING, "Received 0x%02X instead of EOT (0x%02X)", (unsigned)buf[datalen + 4], (unsigned)BICKER_EOT); return 0; + } else if (buf[2] != idx) { + upslogx(LOG_WARNING, "Command indexes do not match: sent 0x%02X, received 0x%02X", + (unsigned)idx, (unsigned)buf[2]); + return 0; } else if (buf[3] != cmd) { upslogx(LOG_WARNING, "Commands do not match: sent 0x%02X, received 0x%02X", - (unsigned)cmd, (unsigned)buf[1]); + (unsigned)cmd, (unsigned)buf[3]); return 0; } else if (dstlen < datalen) { upslogx(LOG_ERR, "Not enough space for the payload: %d < %d", @@ -235,16 +239,16 @@ static ssize_t bicker_receive(char cmd, void *dst, size_t dstlen) return datalen; } -static ssize_t bicker_read_int16(char cmd, int16_t *dst) +static ssize_t bicker_read_int16(char idx, char cmd, int16_t *dst) { ssize_t ret; - ret = bicker_send(cmd, NULL, 0); + ret = bicker_send(idx, cmd, NULL, 0); if (ret <= 0) { return ret; } - ret = bicker_receive(cmd, dst, 2); + ret = bicker_receive(idx, cmd, dst, 2); if (ret <= 0) { return ret; } @@ -270,12 +274,12 @@ static ssize_t bicker_delayed_shutdown(uint8_t seconds) ssize_t ret; uint8_t response; - ret = bicker_send('\x32', &seconds, 1); + ret = bicker_send('\x03', '\x32', &seconds, 1); if (ret <= 0) { return ret; } - ret = bicker_receive('\x32', &response, 1); + ret = bicker_receive('\x03', '\x32', &response, 1); if (ret > 0) { upslogx(LOG_INFO, "Shutting down in %d seconds: response = 0x%02X", (int)seconds, (unsigned)response); @@ -329,21 +333,21 @@ void upsdrv_updateinfo(void) int16_t data, charge_current; ssize_t ret; - ret = bicker_read_int16('\x25', &data); + ret = bicker_read_int16('\x03', '\x25', &data); if (ret <= 0) { dstate_datastale(); return; } dstate_setinfo("input.voltage", "%.1f", (double) data / 1000); - ret = bicker_read_int16('\x28', &data); + ret = bicker_read_int16('\x03', '\x28', &data); if (ret <= 0) { dstate_datastale(); return; } dstate_setinfo("input.current", "%.3f", (double) data / 1000); - ret = bicker_read_int16('\x27', &data); + ret = bicker_read_int16('\x03', '\x27', &data); if (ret <= 0) { dstate_datastale(); return; @@ -352,14 +356,14 @@ void upsdrv_updateinfo(void) /* This is a supercap UPS so, in this context, * the "battery" is the supercap stack */ - ret = bicker_read_int16('\x26', &data); + ret = bicker_read_int16('\x03', '\x26', &data); if (ret <= 0) { dstate_datastale(); return; } dstate_setinfo("battery.voltage", "%.3f", (double) data / 1000); - ret = bicker_read_int16('\x29', &charge_current); + ret = bicker_read_int16('\x03', '\x29', &charge_current); if (ret <= 0) { dstate_datastale(); return; @@ -385,7 +389,7 @@ void upsdrv_updateinfo(void) * 14. RV Reserved Bit * 15. RV Reserved Bit */ - ret = bicker_read_int16('\x1B', &data); + ret = bicker_read_int16('\x03', '\x1B', &data); if (ret <= 0) { dstate_datastale(); return; From 8abde1a7cbe93c32ef2713c70fba6af6d67993d2 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Tue, 21 May 2024 15:54:24 +0200 Subject: [PATCH 10/65] bicker_ser: refactor to allow more complex packet handling I'm planning to add functions for reading strings, i.e. without knowing beforehand the size of the response packets. This required some refactoring of the receiving functions. Also, while at it, added docblocks to many Bicker functions, hopefully easing the maintenance in the future. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 160 +++++++++++++++++++++++++++---------------- 1 file changed, 102 insertions(+), 58 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index c8b09bf18d..2ca288a23e 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -111,6 +111,7 @@ #define BICKER_SOH '\x01' #define BICKER_EOT '\x04' +#define BICKER_TIMEOUT 1 #define BICKER_DELAY 20 #define BICKER_RETRIES 3 #define BYTESWAP(in) ((((uint16_t)(in) & 0x00FF) << 8) + (((uint16_t)(in) & 0xFF00) >> 8)) @@ -128,6 +129,14 @@ upsdrv_info_t upsdrv_info = { { NULL } }; +/** + * Send a packet. + * @param idx Command index + * @param cmd Command + * @param data Source data or NULL for no data field + * @param datalen Size of the source data field or 0 + * @return `datalen` on success or -1 on errors. + */ static ssize_t bicker_send(char idx, char cmd, const void *data, size_t datalen) { char buf[BICKER_PACKET]; @@ -140,7 +149,7 @@ static ssize_t bicker_send(char idx, char cmd, const void *data, size_t datalen) if (datalen > BICKER_MAXDATA) { upslogx(LOG_ERR, "Data size exceeded: %d > %d", (int)datalen, BICKER_MAXDATA); - return 0; + return -1; } memcpy(buf + 4, data, datalen); } else { @@ -157,99 +166,134 @@ static ssize_t bicker_send(char idx, char cmd, const void *data, size_t datalen) ret = ser_send_buf(upsfd, buf, buflen); if (ret < 0) { upslog_with_errno(LOG_WARNING, "ser_send_buf failed"); - return ret; + return -1; } else if ((size_t) ret != buflen) { upslogx(LOG_WARNING, "ser_send_buf returned %d instead of %d", (int)ret, (int)buflen); - return 0; + return -1; } upsdebug_hex(3, "bicker_send", buf, buflen); - return ret; -} - -static ssize_t bicker_receive_buf(void *dst, size_t dstlen) -{ - ssize_t ret; - - ret = ser_get_buf(upsfd, dst, dstlen, 1, 1); - if (ret < 0) { - upslog_with_errno(LOG_WARNING, "ser_get_buf failed"); - return ret; - } else if ((size_t) ret != dstlen) { - upslogx(LOG_WARNING, "ser_get_buf returned %d instead of %d", - (int)ret, (int)dstlen); - return 0; - } - - upsdebug_hex(3, "bicker_receive_buf", dst, dstlen); - return ret; + return datalen; } -static ssize_t bicker_receive(char idx, char cmd, void *dst, size_t dstlen) +/** + * Receive a packet with a data field of unknown size. + * @param idx Command index + * @param cmd Command + * @param data Destination buffer or NULL to discard the data field + * @return The size of the data field on success or -1 on errors. + * + * The data field is stored directly in the destination buffer. `data`, + * if not NULL, must have at least BICKER_MAXDATA bytes. + */ +static ssize_t bicker_receive(char idx, char cmd, void *data) { ssize_t ret; size_t datalen; char buf[BICKER_PACKET]; - ret = bicker_receive_buf(buf, 1); - if (ret <= 0) { - return ret; + /* Read first two bytes (SOH + size) */ + ret = ser_get_buf(upsfd, buf, 2, BICKER_TIMEOUT, 0); + if (ret < 0) { + upslog_with_errno(LOG_WARNING, "Initial ser_get_buf failed"); + return -1; + } else if (ret < 2) { + upslogx(LOG_WARNING, "Timeout waiting for response packet"); + return -1; } else if (buf[0] != BICKER_SOH) { upslogx(LOG_WARNING, "Received 0x%02X instead of SOH (0x%02X)", (unsigned)buf[0], (unsigned)BICKER_SOH); - return 0; - } - - ret = bicker_receive_buf(buf + 1, 1); - if (ret <= 0) { - return ret; + return -1; } datalen = buf[1] - BICKER_HEADER; - /* Still to be read: command index (1 byte), command (1 byte), data - * (datalen bytes) and EOT (1 byte), i.e. `datalen + 3` bytes */ - ret = bicker_receive_buf(buf + 2, datalen + 3); - if (ret <= 0) { - return ret; + /* Read the rest: command index (1 byte), command (1 byte), data + * (datalen bytes) and EOT (1 byte), i.e. `datalen + 3` bytes */ + ret = ser_get_buf(upsfd, buf + 2, datalen + 3, BICKER_TIMEOUT, 0); + if (ret < 0) { + upslog_with_errno(LOG_WARNING, "ser_get_buf failed"); + return -1; + } else if ((size_t)ret < datalen + 3) { + upslogx(LOG_WARNING, "Timeout waiting for the end of the packet"); + return -1; } else if (buf[datalen + 4] != BICKER_EOT) { upslogx(LOG_WARNING, "Received 0x%02X instead of EOT (0x%02X)", (unsigned)buf[datalen + 4], (unsigned)BICKER_EOT); - return 0; + return -1; } else if (buf[2] != idx) { upslogx(LOG_WARNING, "Command indexes do not match: sent 0x%02X, received 0x%02X", (unsigned)idx, (unsigned)buf[2]); - return 0; + return -1; } else if (buf[3] != cmd) { upslogx(LOG_WARNING, "Commands do not match: sent 0x%02X, received 0x%02X", (unsigned)cmd, (unsigned)buf[3]); - return 0; - } else if (dstlen < datalen) { - upslogx(LOG_ERR, "Not enough space for the payload: %d < %d", - (int)dstlen, (int)datalen); - return 0; + return -1; } - if (dst != NULL) { - memcpy(dst, buf + 4, datalen); + if (data != NULL) { + memcpy(data, buf + 4, datalen); } upsdebug_hex(3, "bicker_receive", buf, datalen + 5); return datalen; } +/** + * Receive a packet with a data field of known size. + * @param idx Command index + * @param cmd Command + * @param data Destination buffer or NULL to discard the data field + * @param datalen The expected size of the data field + * @return The size of the data field on success or -1 on errors. + * + * `data`, if specified, must have at least `datalen` bytes. If + * `datalen` is less than the received data size, an error is thrown. + */ +static ssize_t bicker_receive_known(char idx, char cmd, void *data, size_t datalen) +{ + ssize_t ret; + size_t real_datalen; + char real_data[BICKER_MAXDATA]; + + ret = bicker_receive(idx, cmd, real_data); + if (ret < 0) { + return ret; + } + + real_datalen = (size_t)ret; + + if (datalen < real_datalen) { + upslogx(LOG_ERR, "Not enough space for the payload: %d < %d", + (int)datalen, (int)real_datalen); + return -1; + } + + if (data != NULL) { + memcpy(data, real_data, real_datalen); + } + + return real_datalen; +} + +/** + * Execute a command that returns an int16_t value. + * @param idx Command index + * @param cmd Command + * @param dst Destination for the value + */ static ssize_t bicker_read_int16(char idx, char cmd, int16_t *dst) { ssize_t ret; ret = bicker_send(idx, cmd, NULL, 0); - if (ret <= 0) { + if (ret < 0) { return ret; } - ret = bicker_receive(idx, cmd, dst, 2); - if (ret <= 0) { + ret = bicker_receive_known(idx, cmd, dst, 2); + if (ret < 0) { return ret; } @@ -275,12 +319,12 @@ static ssize_t bicker_delayed_shutdown(uint8_t seconds) uint8_t response; ret = bicker_send('\x03', '\x32', &seconds, 1); - if (ret <= 0) { + if (ret < 0) { return ret; } - ret = bicker_receive('\x03', '\x32', &response, 1); - if (ret > 0) { + ret = bicker_receive_known('\x03', '\x32', &response, 1); + if (ret >= 0) { upslogx(LOG_INFO, "Shutting down in %d seconds: response = 0x%02X", (int)seconds, (unsigned)response); } @@ -334,21 +378,21 @@ void upsdrv_updateinfo(void) ssize_t ret; ret = bicker_read_int16('\x03', '\x25', &data); - if (ret <= 0) { + if (ret < 0) { dstate_datastale(); return; } dstate_setinfo("input.voltage", "%.1f", (double) data / 1000); ret = bicker_read_int16('\x03', '\x28', &data); - if (ret <= 0) { + if (ret < 0) { dstate_datastale(); return; } dstate_setinfo("input.current", "%.3f", (double) data / 1000); ret = bicker_read_int16('\x03', '\x27', &data); - if (ret <= 0) { + if (ret < 0) { dstate_datastale(); return; } @@ -357,14 +401,14 @@ void upsdrv_updateinfo(void) /* This is a supercap UPS so, in this context, * the "battery" is the supercap stack */ ret = bicker_read_int16('\x03', '\x26', &data); - if (ret <= 0) { + if (ret < 0) { dstate_datastale(); return; } dstate_setinfo("battery.voltage", "%.3f", (double) data / 1000); ret = bicker_read_int16('\x03', '\x29', &charge_current); - if (ret <= 0) { + if (ret < 0) { dstate_datastale(); return; } @@ -390,7 +434,7 @@ void upsdrv_updateinfo(void) * 15. RV Reserved Bit */ ret = bicker_read_int16('\x03', '\x1B', &data); - if (ret <= 0) { + if (ret < 0) { dstate_datastale(); return; } From eaddb347bf3da3525ba0110d69508687106c88f9 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Tue, 21 May 2024 19:09:58 +0200 Subject: [PATCH 11/65] bicker_ser: add more state variables Identification fields are now queried from the device. Also "battery.charge" is now set properly. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 223 ++++++++++++++++++++++++++++++++----------- 1 file changed, 166 insertions(+), 57 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 2ca288a23e..8306102bcd 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -114,13 +114,15 @@ #define BICKER_TIMEOUT 1 #define BICKER_DELAY 20 #define BICKER_RETRIES 3 -#define BYTESWAP(in) ((((uint16_t)(in) & 0x00FF) << 8) + (((uint16_t)(in) & 0xFF00) >> 8)) /* Protocol fixed lengths */ #define BICKER_HEADER 3 #define BICKER_MAXDATA (255 - BICKER_HEADER) #define BICKER_PACKET (1 + BICKER_HEADER + BICKER_MAXDATA + 1) +#define TOUINT(ch) ((unsigned)(uint8_t)(ch)) +#define BYTESWAP(in) ((((uint16_t)(in) & 0x00FF) << 8) + (((uint16_t)(in) & 0xFF00) >> 8)) + upsdrv_info_t upsdrv_info = { DRIVER_NAME, DRIVER_VERSION, @@ -194,16 +196,16 @@ static ssize_t bicker_receive(char idx, char cmd, void *data) char buf[BICKER_PACKET]; /* Read first two bytes (SOH + size) */ - ret = ser_get_buf(upsfd, buf, 2, BICKER_TIMEOUT, 0); + ret = ser_get_buf_len(upsfd, buf, 2, BICKER_TIMEOUT, 0); if (ret < 0) { - upslog_with_errno(LOG_WARNING, "Initial ser_get_buf failed"); + upslog_with_errno(LOG_WARNING, "Initial ser_get_buf_len failed"); return -1; } else if (ret < 2) { upslogx(LOG_WARNING, "Timeout waiting for response packet"); return -1; } else if (buf[0] != BICKER_SOH) { upslogx(LOG_WARNING, "Received 0x%02X instead of SOH (0x%02X)", - (unsigned)buf[0], (unsigned)BICKER_SOH); + TOUINT(buf[0]), TOUINT(BICKER_SOH)); return -1; } @@ -211,24 +213,31 @@ static ssize_t bicker_receive(char idx, char cmd, void *data) /* Read the rest: command index (1 byte), command (1 byte), data * (datalen bytes) and EOT (1 byte), i.e. `datalen + 3` bytes */ - ret = ser_get_buf(upsfd, buf + 2, datalen + 3, BICKER_TIMEOUT, 0); + ret = ser_get_buf_len(upsfd, buf + 2, datalen + 3, BICKER_TIMEOUT, 0); if (ret < 0) { - upslog_with_errno(LOG_WARNING, "ser_get_buf failed"); + upslog_with_errno(LOG_WARNING, "ser_get_buf_len failed"); return -1; - } else if ((size_t)ret < datalen + 3) { + } + + upsdebug_hex(3, "bicker_receive", buf, ret + 2); + + if ((size_t)ret < datalen + 3) { upslogx(LOG_WARNING, "Timeout waiting for the end of the packet"); return -1; } else if (buf[datalen + 4] != BICKER_EOT) { upslogx(LOG_WARNING, "Received 0x%02X instead of EOT (0x%02X)", - (unsigned)buf[datalen + 4], (unsigned)BICKER_EOT); + TOUINT(buf[datalen + 4]), TOUINT(BICKER_EOT)); return -1; } else if (buf[2] != idx) { - upslogx(LOG_WARNING, "Command indexes do not match: sent 0x%02X, received 0x%02X", - (unsigned)idx, (unsigned)buf[2]); + /* This probably suggests the command is correct but not + * supported, so no logging performed here */ + upsdebugx(2, "Indexes do not match, maybe the command is not supported?" + " Sent 0x%02X, received 0x%02X", + TOUINT(idx), TOUINT(buf[2])); return -1; } else if (buf[3] != cmd) { upslogx(LOG_WARNING, "Commands do not match: sent 0x%02X, received 0x%02X", - (unsigned)cmd, (unsigned)buf[3]); + TOUINT(cmd), TOUINT(buf[3])); return -1; } @@ -236,7 +245,6 @@ static ssize_t bicker_receive(char idx, char cmd, void *data) memcpy(data, buf + 4, datalen); } - upsdebug_hex(3, "bicker_receive", buf, datalen + 5); return datalen; } @@ -277,11 +285,31 @@ static ssize_t bicker_receive_known(char idx, char cmd, void *data, size_t datal return real_datalen; } +/** + * Execute a command that returns an uint8_t value. + * @param idx Command index + * @param cmd Command + * @param dst Destination for the value + * @return The size of the data field on success or -1 on errors. + */ +static ssize_t bicker_read_uint8(char idx, char cmd, uint8_t *dst) +{ + ssize_t ret; + + ret = bicker_send(idx, cmd, NULL, 0); + if (ret < 0) { + return ret; + } + + return bicker_receive_known(idx, cmd, dst, 1); +} + /** * Execute a command that returns an int16_t value. * @param idx Command index * @param cmd Command * @param dst Destination for the value + * @return The size of the data field on success or -1 on errors. */ static ssize_t bicker_read_int16(char idx, char cmd, int16_t *dst) { @@ -306,6 +334,38 @@ static ssize_t bicker_read_int16(char idx, char cmd, int16_t *dst) return ret; } +static ssize_t bicker_read_uint16(char idx, char cmd, uint16_t *dst) +{ + return bicker_read_int16(idx, cmd, (int16_t *) dst); +} + +/** + * Execute a command that returns a string. + * @param idx Command index + * @param cmd Command + * @param dst Destination for the string + * + * `dst` must have at least BICKER_MAXDATA+1 bytes, the additional byte + * needed to accomodate the ending '\0'. + */ +static ssize_t bicker_read_string(char idx, char cmd, char *dst) +{ + ssize_t ret; + + ret = bicker_send(idx, cmd, NULL, 0); + if (ret < 0) { + return ret; + } + + ret = bicker_receive(idx, cmd, dst); + if (ret < 0) { + return ret; + } + + dst[ret] = '\0'; + return ret; +} + /* For some reason the `seconds` delay (at least on my UPSIC-2403D) * is not honored: the shutdown is always delayed by 2 seconds. This * fixed delay seems to be independent from the state of the UPS (on @@ -362,98 +422,131 @@ static int bicker_instcmd(const char *cmdname, const char *extra) void upsdrv_initinfo(void) { + char string[BICKER_MAXDATA + 1]; + dstate_setinfo("device.type", "ups"); - dstate_setinfo("device.mfr", "Bicker Elektronik GmbH"); - /* No way to detect the UPS model within the protocol but the - * serial communication is provided by the PSZ-1063 extension - * module, so it seems correct to put that into the model */ - dstate_setinfo("device.model", "PSZ-1063"); + + if (bicker_read_string('\x01', '\x60', string) >= 0) { + dstate_setinfo("device.mfr", string); + } + + if (bicker_read_string('\x01', '\x61', string) >= 0) { + dstate_setinfo("device.serial", string); + } + + if (bicker_read_string('\x01', '\x62', string) >= 0) { + dstate_setinfo("device.model", string); + } upsh.instcmd = bicker_instcmd; } void upsdrv_updateinfo(void) { - int16_t data, charge_current; + uint8_t u8; + uint16_t u16; + int16_t i16; ssize_t ret; - ret = bicker_read_int16('\x03', '\x25', &data); + ret = bicker_read_uint16('\x01', '\x41', &u16); + if (ret < 0) { + dstate_datastale(); + return; + } + dstate_setinfo("input.voltage", "%.1f", (double) u16 / 1000); + + ret = bicker_read_uint16('\x01', '\x42', &u16); if (ret < 0) { dstate_datastale(); return; } - dstate_setinfo("input.voltage", "%.1f", (double) data / 1000); + dstate_setinfo("input.current", "%.3f", (double) u16 / 1000); - ret = bicker_read_int16('\x03', '\x28', &data); + ret = bicker_read_uint16('\x01', '\x43', &u16); if (ret < 0) { dstate_datastale(); return; } - dstate_setinfo("input.current", "%.3f", (double) data / 1000); + dstate_setinfo("output.voltage", "%.3f", (double) u16 / 1000); - ret = bicker_read_int16('\x03', '\x27', &data); + ret = bicker_read_uint16('\x01', '\x44', &u16); if (ret < 0) { dstate_datastale(); return; } - dstate_setinfo("output.voltage", "%.3f", (double) data / 1000); + dstate_setinfo("output.current", "%.3f", (double) u16 / 1000); /* This is a supercap UPS so, in this context, * the "battery" is the supercap stack */ - ret = bicker_read_int16('\x03', '\x26', &data); + ret = bicker_read_uint16('\x01', '\x45', &u16); if (ret < 0) { dstate_datastale(); return; } - dstate_setinfo("battery.voltage", "%.3f", (double) data / 1000); + dstate_setinfo("battery.voltage", "%.3f", (double) u16 / 1000); - ret = bicker_read_int16('\x03', '\x29', &charge_current); + ret = bicker_read_int16('\x01', '\x46', &i16); if (ret < 0) { dstate_datastale(); return; } - dstate_setinfo("battery.current", "%.3f", (double) charge_current / 1000); - - /* GetChargeStaturRegister returns a 16 bit register: - * - * 0. SD Shows that the device is in step-down (charging) mode. - * 1. SU Shows that the device is in step-up (backup) mode. - * 2. CV Shows that the charger is in constant voltage mode. - * 3. UV Shows that the charger is in undervoltage lockout. - * 4. CL Shows that the device is in input current limit. - * 5. CG Shows that the capacitor voltage is above power good threshold. - * 6. CS Shows that the capacitor manager is shunting. - * 7. CB Shows that the capacitor manager is balancing. - * 8. CD Shows that the charger is temporarily disabled for capacitance measurement. - * 9. CC Shows that the charger is in constant current mode. - * 10. RV Reserved Bit - * 11. PF Shows that the input voltage is below the Power Fail Input (PFI) threshold. - * 12. RV Reserved Bit - * 13. RV Reserved Bit - * 14. RV Reserved Bit - * 15. RV Reserved Bit - */ - ret = bicker_read_int16('\x03', '\x1B', &data); + dstate_setinfo("battery.current", "%.3f", (double) i16 / 1000); + + /* Not implemented for all energy packs: failure acceptable */ + if (bicker_read_uint16('\x01', '\x4A', &u16) >= 0) { + dstate_setinfo("battery.temperature", "%.1f", (double) u16 - 273.16); + } + + /* Not implemented for all energy packs: failure acceptable */ + if (bicker_read_uint8('\x01', '\x48', &u8) >= 0) { + dstate_setinfo("battery.status", "%d%%", u8); + } + + ret = bicker_read_uint8('\x01', '\x47', &u8); if (ret < 0) { dstate_datastale(); return; } + dstate_setinfo("battery.charge", "%d", u8); status_init(); - /* Check PF (bit 11) to know if the UPS is on line/battery */ - status_set((data & 0x0800) > 0 ? "OB" : "OL"); - - /* Check CG (bit 5) to know if the battery is low */ - if ((data & 0x0020) == 0) { + /* Consider the battery low when its charge is < 30% */ + if (u8 < 30) { status_set("LB"); } - /* If there is a current of more than 1 A flowing towards - * the supercaps, consider the battery in charging mode */ - if (charge_current > 1000) { + /* StatusFlags() returns an 8 bit register: + * 0. Charging + * 1. Discharging + * 2. Power present + * 3. Battery present + * 4. Shutdown received + * 5. Overcurrent + * 6. --- + * 7. --- + */ + ret = bicker_read_uint8('\x01', '\x40', &u8); + if (ret < 0) { + dstate_datastale(); + return; + } + + if ((u8 & 0x01) > 0) { status_set("CHRG"); } + if ((u8 & 0x02) > 0) { + status_set("DISCHRG"); + } + dstate_setinfo("battery.charger.status", + (u8 & 0x01) > 0 ? "charging" : + (u8 & 0x02) > 0 ? "discharging" : + "resting"); + + status_set((u8 & 0x04) > 0 ? "OL" : "OB"); + if ((u8 & 0x20) > 0) { + status_set("OVER"); + } status_commit(); @@ -485,13 +578,29 @@ void upsdrv_makevartable(void) void upsdrv_initups(void) { + char string[BICKER_MAXDATA + 1]; + upsfd = ser_open(device_path); ser_set_speed(upsfd, device_path, B38400); + ser_set_dtr(upsfd, 1); /* Adding this variable here because upsdrv_initinfo() is not * called when triggering a forced shutdown and * "ups.delay.shutdown" is needed right there */ dstate_setinfo("ups.delay.shutdown", "%u", BICKER_DELAY); + + if (bicker_read_string('\x01', '\x63', string) >= 0) { + dstate_setinfo("ups.firmware", string); + } + + if (bicker_read_string('\x01', '\x64', string) >= 0) { + dstate_setinfo("battery.type", string); + } + + /* Not implemented on all UPSes */ + if (bicker_read_string('\x01', '\x65', string) >= 0) { + dstate_setinfo("ups.firmware.aux", string); + } } void upsdrv_cleanup(void) From c659e314c00910d7bc9a69ab05b486650ec0b305 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Tue, 21 May 2024 20:39:29 +0200 Subject: [PATCH 12/65] bicker_ser: use variable for low battery limit Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 8306102bcd..0fe07fb9d9 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -443,6 +443,7 @@ void upsdrv_initinfo(void) void upsdrv_updateinfo(void) { + const char *str; uint8_t u8; uint16_t u16; int16_t i16; @@ -512,7 +513,8 @@ void upsdrv_updateinfo(void) status_init(); /* Consider the battery low when its charge is < 30% */ - if (u8 < 30) { + str = dstate_getinfo("battery.charge.low"); + if (u8 < atoi(str)) { status_set("LB"); } @@ -597,6 +599,8 @@ void upsdrv_initups(void) dstate_setinfo("battery.type", string); } + dstate_setinfo("battery.charge.low", "%d", 30); + /* Not implemented on all UPSes */ if (bicker_read_string('\x01', '\x65', string) >= 0) { dstate_setinfo("ups.firmware.aux", string); From a4c3767202516bd2f5ec2a36a357df86479bf14e Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Tue, 21 May 2024 22:38:28 +0200 Subject: [PATCH 13/65] bicker_ser: add parameter support Added the needed infrastructure for getting and setting parameters. Actually the only parameter that is surely working is "ups.delay.start". Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 135 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 0fe07fb9d9..2b31b11302 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -123,6 +123,12 @@ #define TOUINT(ch) ((unsigned)(uint8_t)(ch)) #define BYTESWAP(in) ((((uint16_t)(in) & 0x00FF) << 8) + (((uint16_t)(in) & 0xFF00) >> 8)) +#ifdef WORDS_BIGENDIAN +#define SWAPONBE(in) BYTESWAP(in) +#else +#define SWAPONBE(in) (in) +#endif + upsdrv_info_t upsdrv_info = { DRIVER_NAME, DRIVER_VERSION, @@ -131,6 +137,15 @@ upsdrv_info_t upsdrv_info = { { NULL } }; +typedef struct { + uint8_t id; + uint16_t min; + uint16_t max; + uint16_t std; + uint8_t enabled; + uint16_t val; +} BickerParameter; + /** * Send a packet. * @param idx Command index @@ -366,6 +381,88 @@ static ssize_t bicker_read_string(char idx, char cmd, char *dst) return ret; } +static ssize_t bicker_receive_parameter(BickerParameter *parameter) +{ + ssize_t ret; + uint8_t data[10]; + + ret = bicker_receive_known(0x07, parameter->id, data, 10); + if (ret < 0) { + return ret; + } + + /* The returned `data` is in the format: + * [AA] [bbBB] [ccCC] [ddDD] [EE] [ffFF] + * where: + * [AA] = parameter ID (Byte) + * [BBbb] = minimum value (UInt16) + * [CCcc] = maximum value (UInt16) + * [DDdd] = standard value (UInt16) + * [EE] = enabled (Bool) + * [FFff] = value (UInt16) + */ + parameter->id = data[0]; + parameter->min = SWAPONBE(* (uint16_t *) &data[1]); + parameter->max = SWAPONBE(* (uint16_t *) &data[3]); + parameter->std = SWAPONBE(* (uint16_t *) &data[5]); + parameter->enabled = data[7]; + parameter->val = SWAPONBE(* (uint16_t *) &data[8]); + + upsdebugx(3, "Parameter %d = %d (%s, min = %d, max = %d, std = %d)", + parameter->id, parameter->val, + parameter->enabled ? "enabled" : "disabled", + parameter->min, parameter->max, parameter->std); + + return ret; +} + +/** + * Get a Bicker parameter. + * @param parameter In/out parameter struct + * @return The size of the data field on success or -1 on errors. + * + * You must fill at least the `parameter->id` field. + */ +static ssize_t bicker_get(BickerParameter *parameter) +{ + ssize_t ret; + + ret = bicker_send(0x07, parameter->id, NULL, 0); + if (ret < 0) { + return ret; + } + + return bicker_receive_parameter(parameter); +} + +/** + * Set a Bicker parameter. + * @param parameter In parameter struct + * @return The size of the data field on success or -1 on errors. + * + * You must fill at least the `parameter->id`, `parameter->enabled` and + * `parameter->val` fields. + */ +static ssize_t bicker_set(BickerParameter *parameter) +{ + ssize_t ret; + uint8_t data[3]; + + /* Format of `data` is "[EE] [ffFF]" + * where: + * [EE] = enabled (Bool) + * [FFff] = value (UInt16) + */ + data[0] = parameter->enabled; + * (uint16_t *) &data[1] = SWAPONBE(parameter->val); + ret = bicker_send(0x07, parameter->id, data, 3); + if (ret < 0) { + return ret; + } + + return bicker_receive_parameter(parameter); +} + /* For some reason the `seconds` delay (at least on my UPSIC-2403D) * is not honored: the shutdown is always delayed by 2 seconds. This * fixed delay seems to be independent from the state of the UPS (on @@ -420,6 +517,29 @@ static int bicker_instcmd(const char *cmdname, const char *extra) return STAT_INSTCMD_UNKNOWN; } +static int bicker_setvar(const char *varname, const char *val) +{ + if (!strcasecmp(varname, "battery.charge.restart")) { + BickerParameter parameter; + parameter.id = 0x05; + parameter.enabled = 1; + parameter.val = atoi(val); + dstate_setinfo("battery.charge.restart", "%u", parameter.val); + } else if (!strcasecmp(varname, "ups.delay.start")) { + BickerParameter parameter; + parameter.id = 0x04; + parameter.enabled = 1; + parameter.val = atoi(val); + if (bicker_set(¶meter) >= 0) { + dstate_setinfo("ups.delay.start", "%u", parameter.val); + } + return STAT_SET_HANDLED; + } + + upslogx(LOG_NOTICE, "setvar: unknown variable [%s]", varname); + return STAT_SET_UNKNOWN; +} + void upsdrv_initinfo(void) { char string[BICKER_MAXDATA + 1]; @@ -439,6 +559,7 @@ void upsdrv_initinfo(void) } upsh.instcmd = bicker_instcmd; + upsh.setvar = bicker_setvar; } void upsdrv_updateinfo(void) @@ -581,6 +702,7 @@ void upsdrv_makevartable(void) void upsdrv_initups(void) { char string[BICKER_MAXDATA + 1]; + BickerParameter parameter; upsfd = ser_open(device_path); ser_set_speed(upsfd, device_path, B38400); @@ -605,6 +727,19 @@ void upsdrv_initups(void) if (bicker_read_string('\x01', '\x65', string) >= 0) { dstate_setinfo("ups.firmware.aux", string); } + + parameter.id = 0x05; + if (bicker_get(¶meter) >= 0) { + /* XXX: it seems to not work */ + dstate_setinfo("battery.charge.restart", "%u", + parameter.enabled ? parameter.val : 0); + } + + parameter.id = 0x04; + if (bicker_get(¶meter) >= 0) { + dstate_setinfo("ups.delay.start", "%u", + parameter.enabled ? parameter.val : 0); + } } void upsdrv_cleanup(void) From ee71353209123a046f77809b3255a473b50e780a Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Wed, 22 May 2024 10:12:37 +0200 Subject: [PATCH 14/65] bicker_ser: differentiate between error and unsupported When a command is unsupported, the "\x01\x03\xEE\x07\x04" response packet is returned. This is different from a response packet with a wrong command index. WARNING: the "unsupported" packet has been found sperimentally and it is an undocumented feature. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 2b31b11302..141e5dc075 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -243,12 +243,15 @@ static ssize_t bicker_receive(char idx, char cmd, void *data) upslogx(LOG_WARNING, "Received 0x%02X instead of EOT (0x%02X)", TOUINT(buf[datalen + 4]), TOUINT(BICKER_EOT)); return -1; + } else if (idx != '\xEE' && buf[2] == '\xEE') { + /* I found sperimentally that, when the syntax is + * formally correct but a feature is not supported, + * the device returns "\x01\x03\xEE\x07\x04". */ + upsdebugx(2, "Command is not supported"); + return -1; } else if (buf[2] != idx) { - /* This probably suggests the command is correct but not - * supported, so no logging performed here */ - upsdebugx(2, "Indexes do not match, maybe the command is not supported?" - " Sent 0x%02X, received 0x%02X", - TOUINT(idx), TOUINT(buf[2])); + upslogx(LOG_WARNING, "Indexes do not match: sent 0x%02X, received 0x%02X", + TOUINT(idx), TOUINT(buf[2])); return -1; } else if (buf[3] != cmd) { upslogx(LOG_WARNING, "Commands do not match: sent 0x%02X, received 0x%02X", From c9fed627197ab43f62da675d26ae3c75b8d63f77 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Wed, 22 May 2024 10:21:42 +0200 Subject: [PATCH 15/65] bicker_ser: fix string dstate setting User provided strings could contain printf special sequences and as such they should never be used in the format argument of dstate_setinfo(). Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 141e5dc075..758c133d41 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -550,15 +550,15 @@ void upsdrv_initinfo(void) dstate_setinfo("device.type", "ups"); if (bicker_read_string('\x01', '\x60', string) >= 0) { - dstate_setinfo("device.mfr", string); + dstate_setinfo("device.mfr", "%s", string); } if (bicker_read_string('\x01', '\x61', string) >= 0) { - dstate_setinfo("device.serial", string); + dstate_setinfo("device.serial", "%s", string); } if (bicker_read_string('\x01', '\x62', string) >= 0) { - dstate_setinfo("device.model", string); + dstate_setinfo("device.model", "%s", string); } upsh.instcmd = bicker_instcmd; @@ -717,18 +717,18 @@ void upsdrv_initups(void) dstate_setinfo("ups.delay.shutdown", "%u", BICKER_DELAY); if (bicker_read_string('\x01', '\x63', string) >= 0) { - dstate_setinfo("ups.firmware", string); + dstate_setinfo("ups.firmware", "%s", string); } if (bicker_read_string('\x01', '\x64', string) >= 0) { - dstate_setinfo("battery.type", string); + dstate_setinfo("battery.type", "%s", string); } dstate_setinfo("battery.charge.low", "%d", 30); /* Not implemented on all UPSes */ if (bicker_read_string('\x01', '\x65', string) >= 0) { - dstate_setinfo("ups.firmware.aux", string); + dstate_setinfo("ups.firmware.aux", "%s", string); } parameter.id = 0x05; From c9ba796d336b9af44bb2c79665283328a6997bae Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Wed, 22 May 2024 11:34:46 +0200 Subject: [PATCH 16/65] bicker_ser: fix alignment and endianness problems Accessing an uint16_t on odd addresses is not portable. Avoid having to deal with that issue by always constructing words mathematically from single bytes. This also solves any endianness difference. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 49 +++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 758c133d41..2c534dbc42 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -121,13 +121,9 @@ #define BICKER_PACKET (1 + BICKER_HEADER + BICKER_MAXDATA + 1) #define TOUINT(ch) ((unsigned)(uint8_t)(ch)) -#define BYTESWAP(in) ((((uint16_t)(in) & 0x00FF) << 8) + (((uint16_t)(in) & 0xFF00) >> 8)) - -#ifdef WORDS_BIGENDIAN -#define SWAPONBE(in) BYTESWAP(in) -#else -#define SWAPONBE(in) (in) -#endif +#define LOWBYTE(w) ((uint8_t)((uint16_t)(w) & 0x00FF)) +#define HIGHBYTE(w) ((uint8_t)(((uint16_t)(w) & 0xFF00) >> 8)) +#define WORDLH(l,h) ((uint16_t)((l) + ((h) << 8))) upsdrv_info_t upsdrv_info = { DRIVER_NAME, @@ -323,38 +319,44 @@ static ssize_t bicker_read_uint8(char idx, char cmd, uint8_t *dst) } /** - * Execute a command that returns an int16_t value. + * Execute a command that returns an uint16_t value. * @param idx Command index * @param cmd Command - * @param dst Destination for the value + * @param dst Destination for the value of NULL to discard * @return The size of the data field on success or -1 on errors. */ -static ssize_t bicker_read_int16(char idx, char cmd, int16_t *dst) +static ssize_t bicker_read_uint16(char idx, char cmd, uint16_t *dst) { ssize_t ret; + uint8_t data[2]; ret = bicker_send(idx, cmd, NULL, 0); if (ret < 0) { return ret; } - ret = bicker_receive_known(idx, cmd, dst, 2); + ret = bicker_receive_known(idx, cmd, data, 2); if (ret < 0) { return ret; } -#ifdef WORDS_BIGENDIAN - /* By default data is stored in little-endian so, on big-endian - * platforms, a byte swap must be performed */ - *dst = BYTESWAP(*dst); -#endif + if (dst != NULL) { + *dst = WORDLH(data[0], data[1]); + } return ret; } -static ssize_t bicker_read_uint16(char idx, char cmd, uint16_t *dst) +/** + * Execute a command that returns an int16_t value. + * @param idx Command index + * @param cmd Command + * @param dst Destination for the value or NULL to discard + * @return The size of the data field on success or -1 on errors. + */ +static ssize_t bicker_read_int16(char idx, char cmd, int16_t *dst) { - return bicker_read_int16(idx, cmd, (int16_t *) dst); + return bicker_read_uint16(idx, cmd, (uint16_t *) dst); } /** @@ -405,11 +407,11 @@ static ssize_t bicker_receive_parameter(BickerParameter *parameter) * [FFff] = value (UInt16) */ parameter->id = data[0]; - parameter->min = SWAPONBE(* (uint16_t *) &data[1]); - parameter->max = SWAPONBE(* (uint16_t *) &data[3]); - parameter->std = SWAPONBE(* (uint16_t *) &data[5]); + parameter->min = WORDLH(data[1], data[2]); + parameter->max = WORDLH(data[3], data[4]); + parameter->std = WORDLH(data[5], data[6]); parameter->enabled = data[7]; - parameter->val = SWAPONBE(* (uint16_t *) &data[8]); + parameter->val = WORDLH(data[8], data[9]); upsdebugx(3, "Parameter %d = %d (%s, min = %d, max = %d, std = %d)", parameter->id, parameter->val, @@ -457,7 +459,8 @@ static ssize_t bicker_set(BickerParameter *parameter) * [FFff] = value (UInt16) */ data[0] = parameter->enabled; - * (uint16_t *) &data[1] = SWAPONBE(parameter->val); + data[1] = LOWBYTE(parameter->val); + data[2] = HIGHBYTE(parameter->val); ret = bicker_send(0x07, parameter->id, data, 3); if (ret < 0) { return ret; From beb26a68472e98992dec22db26044fbd747311d7 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Wed, 22 May 2024 11:57:44 +0200 Subject: [PATCH 17/65] bicker_ser: cosmetic fixes Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 2c534dbc42..1c59135ada 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -240,7 +240,7 @@ static ssize_t bicker_receive(char idx, char cmd, void *data) TOUINT(buf[datalen + 4]), TOUINT(BICKER_EOT)); return -1; } else if (idx != '\xEE' && buf[2] == '\xEE') { - /* I found sperimentally that, when the syntax is + /* I found experimentally that, when the syntax is * formally correct but a feature is not supported, * the device returns "\x01\x03\xEE\x07\x04". */ upsdebugx(2, "Command is not supported"); @@ -267,10 +267,10 @@ static ssize_t bicker_receive(char idx, char cmd, void *data) * @param idx Command index * @param cmd Command * @param data Destination buffer or NULL to discard the data field - * @param datalen The expected size of the data field + * @param datalen The expected size of the data field * @return The size of the data field on success or -1 on errors. * - * `data`, if specified, must have at least `datalen` bytes. If + * `data`, if not NULL, must have at least `datalen` bytes. If * `datalen` is less than the received data size, an error is thrown. */ static ssize_t bicker_receive_known(char idx, char cmd, void *data, size_t datalen) @@ -363,10 +363,11 @@ static ssize_t bicker_read_int16(char idx, char cmd, int16_t *dst) * Execute a command that returns a string. * @param idx Command index * @param cmd Command - * @param dst Destination for the string + * @param dst Destination for the string or NULL to discard + * @return The size of the data field on success or -1 on errors. * - * `dst` must have at least BICKER_MAXDATA+1 bytes, the additional byte - * needed to accomodate the ending '\0'. + * `dst`, if not NULL, must have at least BICKER_MAXDATA+1 bytes, the + * additional byte needed to accomodate the ending '\0'. */ static ssize_t bicker_read_string(char idx, char cmd, char *dst) { @@ -472,7 +473,7 @@ static ssize_t bicker_set(BickerParameter *parameter) /* For some reason the `seconds` delay (at least on my UPSIC-2403D) * is not honored: the shutdown is always delayed by 2 seconds. This * fixed delay seems to be independent from the state of the UPS (on - * line or on battery) and from the dip switches setting. + * line or on battery) and from the DIP switches setting. * * As response I get the same command with `0xE0` in the data field. */ From 0c80ece5927dcbab5e5609ba34aed38ca6853a1a Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Wed, 22 May 2024 14:58:35 +0200 Subject: [PATCH 18/65] bicker_ser: use uint8_t for bytes instead of char `char` is by default signed and (as the name implies) designed to access string characters. For accessing bytes, the `uint8_t` type is more appropriate. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 68 ++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 1c59135ada..6b31c1c590 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -32,12 +32,12 @@ * | HEADER | * * where: - * - `SOH` is the start signal ('\x01') + * - `SOH` is the start signal (0x01) * - `Size` is the length (in bytes) of the header and the data field * - `Index` is the command index: see AVAILABLE COMMANDS * - `CMD` is the command code to execute: see AVAILABLE COMMANDS * - `Data` is the (optional) argument of the command - * - `EOT` is the end signal ('\x04') + * - `EOT` is the end signal (0x04) * * The same format is used for incoming and outcoming packets. The data * returned in the `Data` field is always in little-endian order. @@ -109,8 +109,8 @@ #define DRIVER_NAME "Bicker serial protocol" #define DRIVER_VERSION "0.01" -#define BICKER_SOH '\x01' -#define BICKER_EOT '\x04' +#define BICKER_SOH 0x01 +#define BICKER_EOT 0x04 #define BICKER_TIMEOUT 1 #define BICKER_DELAY 20 #define BICKER_RETRIES 3 @@ -150,9 +150,9 @@ typedef struct { * @param datalen Size of the source data field or 0 * @return `datalen` on success or -1 on errors. */ -static ssize_t bicker_send(char idx, char cmd, const void *data, size_t datalen) +static ssize_t bicker_send(uint8_t idx, uint8_t cmd, const void *data, size_t datalen) { - char buf[BICKER_PACKET]; + uint8_t buf[BICKER_PACKET]; size_t buflen; ssize_t ret; @@ -200,11 +200,11 @@ static ssize_t bicker_send(char idx, char cmd, const void *data, size_t datalen) * The data field is stored directly in the destination buffer. `data`, * if not NULL, must have at least BICKER_MAXDATA bytes. */ -static ssize_t bicker_receive(char idx, char cmd, void *data) +static ssize_t bicker_receive(uint8_t idx, uint8_t cmd, void *data) { ssize_t ret; size_t datalen; - char buf[BICKER_PACKET]; + uint8_t buf[BICKER_PACKET]; /* Read first two bytes (SOH + size) */ ret = ser_get_buf_len(upsfd, buf, 2, BICKER_TIMEOUT, 0); @@ -239,10 +239,10 @@ static ssize_t bicker_receive(char idx, char cmd, void *data) upslogx(LOG_WARNING, "Received 0x%02X instead of EOT (0x%02X)", TOUINT(buf[datalen + 4]), TOUINT(BICKER_EOT)); return -1; - } else if (idx != '\xEE' && buf[2] == '\xEE') { + } else if (idx != 0xEE && buf[2] == 0xEE) { /* I found experimentally that, when the syntax is * formally correct but a feature is not supported, - * the device returns "\x01\x03\xEE\x07\x04". */ + * the device returns 0x01 0x03 0xEE 0x07 0x04. */ upsdebugx(2, "Command is not supported"); return -1; } else if (buf[2] != idx) { @@ -273,11 +273,11 @@ static ssize_t bicker_receive(char idx, char cmd, void *data) * `data`, if not NULL, must have at least `datalen` bytes. If * `datalen` is less than the received data size, an error is thrown. */ -static ssize_t bicker_receive_known(char idx, char cmd, void *data, size_t datalen) +static ssize_t bicker_receive_known(uint8_t idx, uint8_t cmd, void *data, size_t datalen) { ssize_t ret; size_t real_datalen; - char real_data[BICKER_MAXDATA]; + uint8_t real_data[BICKER_MAXDATA]; ret = bicker_receive(idx, cmd, real_data); if (ret < 0) { @@ -306,7 +306,7 @@ static ssize_t bicker_receive_known(char idx, char cmd, void *data, size_t datal * @param dst Destination for the value * @return The size of the data field on success or -1 on errors. */ -static ssize_t bicker_read_uint8(char idx, char cmd, uint8_t *dst) +static ssize_t bicker_read_uint8(uint8_t idx, uint8_t cmd, uint8_t *dst) { ssize_t ret; @@ -325,7 +325,7 @@ static ssize_t bicker_read_uint8(char idx, char cmd, uint8_t *dst) * @param dst Destination for the value of NULL to discard * @return The size of the data field on success or -1 on errors. */ -static ssize_t bicker_read_uint16(char idx, char cmd, uint16_t *dst) +static ssize_t bicker_read_uint16(uint8_t idx, uint8_t cmd, uint16_t *dst) { ssize_t ret; uint8_t data[2]; @@ -354,7 +354,7 @@ static ssize_t bicker_read_uint16(char idx, char cmd, uint16_t *dst) * @param dst Destination for the value or NULL to discard * @return The size of the data field on success or -1 on errors. */ -static ssize_t bicker_read_int16(char idx, char cmd, int16_t *dst) +static ssize_t bicker_read_int16(uint8_t idx, uint8_t cmd, int16_t *dst) { return bicker_read_uint16(idx, cmd, (uint16_t *) dst); } @@ -369,7 +369,7 @@ static ssize_t bicker_read_int16(char idx, char cmd, int16_t *dst) * `dst`, if not NULL, must have at least BICKER_MAXDATA+1 bytes, the * additional byte needed to accomodate the ending '\0'. */ -static ssize_t bicker_read_string(char idx, char cmd, char *dst) +static ssize_t bicker_read_string(uint8_t idx, uint8_t cmd, char *dst) { ssize_t ret; @@ -482,12 +482,12 @@ static ssize_t bicker_delayed_shutdown(uint8_t seconds) ssize_t ret; uint8_t response; - ret = bicker_send('\x03', '\x32', &seconds, 1); + ret = bicker_send(0x03, 0x32, &seconds, 1); if (ret < 0) { return ret; } - ret = bicker_receive_known('\x03', '\x32', &response, 1); + ret = bicker_receive_known(0x03, 0x32, &response, 1); if (ret >= 0) { upslogx(LOG_INFO, "Shutting down in %d seconds: response = 0x%02X", (int)seconds, (unsigned)response); @@ -553,15 +553,15 @@ void upsdrv_initinfo(void) dstate_setinfo("device.type", "ups"); - if (bicker_read_string('\x01', '\x60', string) >= 0) { + if (bicker_read_string(0x01, 0x60, string) >= 0) { dstate_setinfo("device.mfr", "%s", string); } - if (bicker_read_string('\x01', '\x61', string) >= 0) { + if (bicker_read_string(0x01, 0x61, string) >= 0) { dstate_setinfo("device.serial", "%s", string); } - if (bicker_read_string('\x01', '\x62', string) >= 0) { + if (bicker_read_string(0x01, 0x62, string) >= 0) { dstate_setinfo("device.model", "%s", string); } @@ -577,28 +577,28 @@ void upsdrv_updateinfo(void) int16_t i16; ssize_t ret; - ret = bicker_read_uint16('\x01', '\x41', &u16); + ret = bicker_read_uint16(0x01, 0x41, &u16); if (ret < 0) { dstate_datastale(); return; } dstate_setinfo("input.voltage", "%.1f", (double) u16 / 1000); - ret = bicker_read_uint16('\x01', '\x42', &u16); + ret = bicker_read_uint16(0x01, 0x42, &u16); if (ret < 0) { dstate_datastale(); return; } dstate_setinfo("input.current", "%.3f", (double) u16 / 1000); - ret = bicker_read_uint16('\x01', '\x43', &u16); + ret = bicker_read_uint16(0x01, 0x43, &u16); if (ret < 0) { dstate_datastale(); return; } dstate_setinfo("output.voltage", "%.3f", (double) u16 / 1000); - ret = bicker_read_uint16('\x01', '\x44', &u16); + ret = bicker_read_uint16(0x01, 0x44, &u16); if (ret < 0) { dstate_datastale(); return; @@ -607,14 +607,14 @@ void upsdrv_updateinfo(void) /* This is a supercap UPS so, in this context, * the "battery" is the supercap stack */ - ret = bicker_read_uint16('\x01', '\x45', &u16); + ret = bicker_read_uint16(0x01, 0x45, &u16); if (ret < 0) { dstate_datastale(); return; } dstate_setinfo("battery.voltage", "%.3f", (double) u16 / 1000); - ret = bicker_read_int16('\x01', '\x46', &i16); + ret = bicker_read_int16(0x01, 0x46, &i16); if (ret < 0) { dstate_datastale(); return; @@ -622,16 +622,16 @@ void upsdrv_updateinfo(void) dstate_setinfo("battery.current", "%.3f", (double) i16 / 1000); /* Not implemented for all energy packs: failure acceptable */ - if (bicker_read_uint16('\x01', '\x4A', &u16) >= 0) { + if (bicker_read_uint16(0x01, 0x4A, &u16) >= 0) { dstate_setinfo("battery.temperature", "%.1f", (double) u16 - 273.16); } /* Not implemented for all energy packs: failure acceptable */ - if (bicker_read_uint8('\x01', '\x48', &u8) >= 0) { + if (bicker_read_uint8(0x01, 0x48, &u8) >= 0) { dstate_setinfo("battery.status", "%d%%", u8); } - ret = bicker_read_uint8('\x01', '\x47', &u8); + ret = bicker_read_uint8(0x01, 0x47, &u8); if (ret < 0) { dstate_datastale(); return; @@ -656,7 +656,7 @@ void upsdrv_updateinfo(void) * 6. --- * 7. --- */ - ret = bicker_read_uint8('\x01', '\x40', &u8); + ret = bicker_read_uint8(0x01, 0x40, &u8); if (ret < 0) { dstate_datastale(); return; @@ -720,18 +720,18 @@ void upsdrv_initups(void) * "ups.delay.shutdown" is needed right there */ dstate_setinfo("ups.delay.shutdown", "%u", BICKER_DELAY); - if (bicker_read_string('\x01', '\x63', string) >= 0) { + if (bicker_read_string(0x01, 0x63, string) >= 0) { dstate_setinfo("ups.firmware", "%s", string); } - if (bicker_read_string('\x01', '\x64', string) >= 0) { + if (bicker_read_string(0x01, 0x64, string) >= 0) { dstate_setinfo("battery.type", "%s", string); } dstate_setinfo("battery.charge.low", "%d", 30); /* Not implemented on all UPSes */ - if (bicker_read_string('\x01', '\x65', string) >= 0) { + if (bicker_read_string(0x01, 0x65, string) >= 0) { dstate_setinfo("ups.firmware.aux", "%s", string); } From 4a33b3a7c0a723868f81f380b869a6f2a7933636 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Wed, 22 May 2024 15:20:07 +0200 Subject: [PATCH 19/65] bicker_ser: use proper format modifiers in logs Ensure the format string in logging messages is the correct one without subcasting. Furthermore, due to the promotion rules of the C language (where any given type, even unsigned, is promoted to int when possible), explicitly cast to unsigned where required. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 6b31c1c590..b99fe97b3d 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -103,6 +103,7 @@ #include "config.h" #include "main.h" #include "attribute.h" +#include "nut_stdint.h" #include "serial.h" @@ -160,8 +161,9 @@ static ssize_t bicker_send(uint8_t idx, uint8_t cmd, const void *data, size_t da if (data != NULL) { if (datalen > BICKER_MAXDATA) { - upslogx(LOG_ERR, "Data size exceeded: %d > %d", - (int)datalen, BICKER_MAXDATA); + upslogx(LOG_ERR, + "Data size exceeded: %" PRIuSIZE " > %d", + datalen, BICKER_MAXDATA); return -1; } memcpy(buf + 4, data, datalen); @@ -181,8 +183,9 @@ static ssize_t bicker_send(uint8_t idx, uint8_t cmd, const void *data, size_t da upslog_with_errno(LOG_WARNING, "ser_send_buf failed"); return -1; } else if ((size_t) ret != buflen) { - upslogx(LOG_WARNING, "ser_send_buf returned %d instead of %d", - (int)ret, (int)buflen); + upslogx(LOG_WARNING, "ser_send_buf returned %" + PRIiSIZE " instead of %" PRIuSIZE, + ret, buflen); return -1; } @@ -216,7 +219,7 @@ static ssize_t bicker_receive(uint8_t idx, uint8_t cmd, void *data) return -1; } else if (buf[0] != BICKER_SOH) { upslogx(LOG_WARNING, "Received 0x%02X instead of SOH (0x%02X)", - TOUINT(buf[0]), TOUINT(BICKER_SOH)); + (unsigned)buf[0], (unsigned)BICKER_SOH); return -1; } @@ -237,7 +240,7 @@ static ssize_t bicker_receive(uint8_t idx, uint8_t cmd, void *data) return -1; } else if (buf[datalen + 4] != BICKER_EOT) { upslogx(LOG_WARNING, "Received 0x%02X instead of EOT (0x%02X)", - TOUINT(buf[datalen + 4]), TOUINT(BICKER_EOT)); + (unsigned)buf[datalen + 4], (unsigned)BICKER_EOT); return -1; } else if (idx != 0xEE && buf[2] == 0xEE) { /* I found experimentally that, when the syntax is @@ -247,11 +250,11 @@ static ssize_t bicker_receive(uint8_t idx, uint8_t cmd, void *data) return -1; } else if (buf[2] != idx) { upslogx(LOG_WARNING, "Indexes do not match: sent 0x%02X, received 0x%02X", - TOUINT(idx), TOUINT(buf[2])); + (unsigned)idx, (unsigned)buf[2]); return -1; } else if (buf[3] != cmd) { upslogx(LOG_WARNING, "Commands do not match: sent 0x%02X, received 0x%02X", - TOUINT(cmd), TOUINT(buf[3])); + (unsigned)cmd, (unsigned)buf[3]); return -1; } @@ -287,8 +290,9 @@ static ssize_t bicker_receive_known(uint8_t idx, uint8_t cmd, void *data, size_t real_datalen = (size_t)ret; if (datalen < real_datalen) { - upslogx(LOG_ERR, "Not enough space for the payload: %d < %d", - (int)datalen, (int)real_datalen); + upslogx(LOG_ERR, "Not enough space for the payload: %" + PRIuSIZE " < %" PRIuSIZE, + datalen, real_datalen); return -1; } @@ -414,10 +418,11 @@ static ssize_t bicker_receive_parameter(BickerParameter *parameter) parameter->enabled = data[7]; parameter->val = WORDLH(data[8], data[9]); - upsdebugx(3, "Parameter %d = %d (%s, min = %d, max = %d, std = %d)", - parameter->id, parameter->val, + upsdebugx(3, "Parameter %u = %u (%s, min = %u, max = %u, std = %u)", + (unsigned)parameter->id, (unsigned)parameter->val, parameter->enabled ? "enabled" : "disabled", - parameter->min, parameter->max, parameter->std); + (unsigned)parameter->min, (unsigned)parameter->max, + (unsigned)parameter->std); return ret; } @@ -490,7 +495,7 @@ static ssize_t bicker_delayed_shutdown(uint8_t seconds) ret = bicker_receive_known(0x03, 0x32, &response, 1); if (ret >= 0) { upslogx(LOG_INFO, "Shutting down in %d seconds: response = 0x%02X", - (int)seconds, (unsigned)response); + seconds, (unsigned)response); } return ret; From c78524c3f99b50ce3c08eedafb5bb0ac2eb0411e Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Wed, 22 May 2024 15:56:12 +0200 Subject: [PATCH 20/65] bicker_ser: refactor to clarify packet manipulation Change BICKER_PACKET from a constant value to a macro that depends on the data length. This should make clear that the packet size depends directly from the data size. Refactored some code here and there trying to minimize the presence of magic numbers and clarify the intentions. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index b99fe97b3d..85fe8e03e6 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -116,10 +116,10 @@ #define BICKER_DELAY 20 #define BICKER_RETRIES 3 -/* Protocol fixed lengths */ -#define BICKER_HEADER 3 -#define BICKER_MAXDATA (255 - BICKER_HEADER) -#define BICKER_PACKET (1 + BICKER_HEADER + BICKER_MAXDATA + 1) +/* Protocol lengths */ +#define BICKER_HEADER 3 +#define BICKER_MAXDATA (255 - BICKER_HEADER) +#define BICKER_PACKET(datalen) (1 + BICKER_HEADER + (datalen) + 1) #define TOUINT(ch) ((unsigned)(uint8_t)(ch)) #define LOWBYTE(w) ((uint8_t)((uint16_t)(w) & 0x00FF)) @@ -153,12 +153,10 @@ typedef struct { */ static ssize_t bicker_send(uint8_t idx, uint8_t cmd, const void *data, size_t datalen) { - uint8_t buf[BICKER_PACKET]; + uint8_t buf[BICKER_PACKET(BICKER_MAXDATA)]; size_t buflen; ssize_t ret; - ser_flush_io(upsfd); - if (data != NULL) { if (datalen > BICKER_MAXDATA) { upslogx(LOG_ERR, @@ -166,17 +164,19 @@ static ssize_t bicker_send(uint8_t idx, uint8_t cmd, const void *data, size_t da datalen, BICKER_MAXDATA); return -1; } - memcpy(buf + 4, data, datalen); + memcpy(&buf[1 + BICKER_HEADER], data, datalen); } else { datalen = 0; } + ser_flush_io(upsfd); + + buflen = BICKER_PACKET(datalen); buf[0] = BICKER_SOH; - buf[1] = datalen + BICKER_HEADER; + buf[1] = BICKER_HEADER + datalen; buf[2] = idx; buf[3] = cmd; - buf[1 + BICKER_HEADER + datalen] = BICKER_EOT; - buflen = 1 + BICKER_HEADER + datalen + 1; + buf[buflen - 1] = BICKER_EOT; ret = ser_send_buf(upsfd, buf, buflen); if (ret < 0) { @@ -206,8 +206,8 @@ static ssize_t bicker_send(uint8_t idx, uint8_t cmd, const void *data, size_t da static ssize_t bicker_receive(uint8_t idx, uint8_t cmd, void *data) { ssize_t ret; - size_t datalen; - uint8_t buf[BICKER_PACKET]; + size_t buflen, datalen; + uint8_t buf[BICKER_PACKET(BICKER_MAXDATA)]; /* Read first two bytes (SOH + size) */ ret = ser_get_buf_len(upsfd, buf, 2, BICKER_TIMEOUT, 0); @@ -223,11 +223,12 @@ static ssize_t bicker_receive(uint8_t idx, uint8_t cmd, void *data) return -1; } + /* buf[1] (the size field) is BICKER_HEADER + data length, so */ datalen = buf[1] - BICKER_HEADER; - /* Read the rest: command index (1 byte), command (1 byte), data - * (datalen bytes) and EOT (1 byte), i.e. `datalen + 3` bytes */ - ret = ser_get_buf_len(upsfd, buf + 2, datalen + 3, BICKER_TIMEOUT, 0); + /* Read the rest of the packet */ + buflen = BICKER_PACKET(datalen); + ret = ser_get_buf_len(upsfd, buf + 2, buflen - 2, BICKER_TIMEOUT, 0); if (ret < 0) { upslog_with_errno(LOG_WARNING, "ser_get_buf_len failed"); return -1; @@ -235,12 +236,12 @@ static ssize_t bicker_receive(uint8_t idx, uint8_t cmd, void *data) upsdebug_hex(3, "bicker_receive", buf, ret + 2); - if ((size_t)ret < datalen + 3) { + if ((size_t)ret < buflen - 2) { upslogx(LOG_WARNING, "Timeout waiting for the end of the packet"); return -1; - } else if (buf[datalen + 4] != BICKER_EOT) { + } else if (buf[buflen - 1] != BICKER_EOT) { upslogx(LOG_WARNING, "Received 0x%02X instead of EOT (0x%02X)", - (unsigned)buf[datalen + 4], (unsigned)BICKER_EOT); + (unsigned)buf[buflen - 1], (unsigned)BICKER_EOT); return -1; } else if (idx != 0xEE && buf[2] == 0xEE) { /* I found experimentally that, when the syntax is @@ -259,7 +260,7 @@ static ssize_t bicker_receive(uint8_t idx, uint8_t cmd, void *data) } if (data != NULL) { - memcpy(data, buf + 4, datalen); + memcpy(data, &buf[1 + BICKER_HEADER], datalen); } return datalen; @@ -326,7 +327,7 @@ static ssize_t bicker_read_uint8(uint8_t idx, uint8_t cmd, uint8_t *dst) * Execute a command that returns an uint16_t value. * @param idx Command index * @param cmd Command - * @param dst Destination for the value of NULL to discard + * @param dst Destination for the value or NULL to discard * @return The size of the data field on success or -1 on errors. */ static ssize_t bicker_read_uint16(uint8_t idx, uint8_t cmd, uint16_t *dst) From 27bdc8751ce567b1ba516344967664cf29f182ed Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Thu, 23 May 2024 08:33:49 +0200 Subject: [PATCH 21/65] bicker_ser: make parameter handling less error prone Using an in/out struct and making some specific field mandatory is cumbersome and error prone. Now the data needed for setting or getting parameters is explicitly required by the arguments. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 106 +++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 54 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 85fe8e03e6..96fc67b688 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -140,7 +140,7 @@ typedef struct { uint16_t max; uint16_t std; uint8_t enabled; - uint16_t val; + uint16_t value; } BickerParameter; /** @@ -402,44 +402,45 @@ static ssize_t bicker_receive_parameter(BickerParameter *parameter) return ret; } - /* The returned `data` is in the format: - * [AA] [bbBB] [ccCC] [ddDD] [EE] [ffFF] - * where: - * [AA] = parameter ID (Byte) - * [BBbb] = minimum value (UInt16) - * [CCcc] = maximum value (UInt16) - * [DDdd] = standard value (UInt16) - * [EE] = enabled (Bool) - * [FFff] = value (UInt16) - */ - parameter->id = data[0]; - parameter->min = WORDLH(data[1], data[2]); - parameter->max = WORDLH(data[3], data[4]); - parameter->std = WORDLH(data[5], data[6]); - parameter->enabled = data[7]; - parameter->val = WORDLH(data[8], data[9]); - - upsdebugx(3, "Parameter %u = %u (%s, min = %u, max = %u, std = %u)", - (unsigned)parameter->id, (unsigned)parameter->val, - parameter->enabled ? "enabled" : "disabled", - (unsigned)parameter->min, (unsigned)parameter->max, - (unsigned)parameter->std); + if (parameter != NULL) { + /* The returned `data` is in the format: + * [AA] [bbBB] [ccCC] [ddDD] [EE] [ffFF] + * where: + * [AA] = parameter ID (Byte) + * [BBbb] = minimum value (UInt16) + * [CCcc] = maximum value (UInt16) + * [DDdd] = standard value (UInt16) + * [EE] = enabled (Bool) + * [FFff] = value (UInt16) + */ + parameter->id = data[0]; + parameter->min = WORDLH(data[1], data[2]); + parameter->max = WORDLH(data[3], data[4]); + parameter->std = WORDLH(data[5], data[6]); + parameter->enabled = data[7]; + parameter->value = WORDLH(data[8], data[9]); + + upsdebugx(3, "Parameter %u = %u (%s, min = %u, max = %u, std = %u)", + (unsigned)parameter->id, (unsigned)parameter->value, + parameter->enabled ? "enabled" : "disabled", + (unsigned)parameter->min, (unsigned)parameter->max, + (unsigned)parameter->std); + } return ret; } /** * Get a Bicker parameter. - * @param parameter In/out parameter struct + * @param id ID of the parameter (0x01..0x0A) + * @param parameter Where to store the response or NULL to discard * @return The size of the data field on success or -1 on errors. - * - * You must fill at least the `parameter->id` field. */ -static ssize_t bicker_get(BickerParameter *parameter) +static ssize_t bicker_get(uint8_t id, BickerParameter *parameter) { ssize_t ret; - ret = bicker_send(0x07, parameter->id, NULL, 0); + ret = bicker_send(0x07, id, NULL, 0); if (ret < 0) { return ret; } @@ -449,13 +450,13 @@ static ssize_t bicker_get(BickerParameter *parameter) /** * Set a Bicker parameter. - * @param parameter In parameter struct + * @param id ID of the parameter (0x01..0x0A) + * @param enabled 0 to disable, 1 to enable + * @param value + * @param parameter Where to store the response or NULL to discard * @return The size of the data field on success or -1 on errors. - * - * You must fill at least the `parameter->id`, `parameter->enabled` and - * `parameter->val` fields. */ -static ssize_t bicker_set(BickerParameter *parameter) +static ssize_t bicker_set(uint8_t id, uint8_t enabled, uint16_t value, BickerParameter *parameter) { ssize_t ret; uint8_t data[3]; @@ -465,10 +466,10 @@ static ssize_t bicker_set(BickerParameter *parameter) * [EE] = enabled (Bool) * [FFff] = value (UInt16) */ - data[0] = parameter->enabled; - data[1] = LOWBYTE(parameter->val); - data[2] = HIGHBYTE(parameter->val); - ret = bicker_send(0x07, parameter->id, data, 3); + data[0] = enabled; + data[1] = LOWBYTE(value); + data[2] = HIGHBYTE(value); + ret = bicker_send(0x07, id, data, 3); if (ret < 0) { return ret; } @@ -533,19 +534,18 @@ static int bicker_instcmd(const char *cmdname, const char *extra) static int bicker_setvar(const char *varname, const char *val) { if (!strcasecmp(varname, "battery.charge.restart")) { - BickerParameter parameter; - parameter.id = 0x05; - parameter.enabled = 1; - parameter.val = atoi(val); - dstate_setinfo("battery.charge.restart", "%u", parameter.val); + int value = (uint16_t)atoi(val); + if (bicker_set(0x05, 1, value, NULL) < 0) { + return STAT_SET_FAILED; + } + dstate_setinfo("battery.charge.restart", "%u", value); + return STAT_SET_HANDLED; } else if (!strcasecmp(varname, "ups.delay.start")) { - BickerParameter parameter; - parameter.id = 0x04; - parameter.enabled = 1; - parameter.val = atoi(val); - if (bicker_set(¶meter) >= 0) { - dstate_setinfo("ups.delay.start", "%u", parameter.val); + int value = (uint16_t)atoi(val); + if (bicker_set(0x04, 1, value, NULL) < 0) { + return STAT_SET_FAILED; } + dstate_setinfo("ups.delay.start", "%u", value); return STAT_SET_HANDLED; } @@ -741,17 +741,15 @@ void upsdrv_initups(void) dstate_setinfo("ups.firmware.aux", "%s", string); } - parameter.id = 0x05; - if (bicker_get(¶meter) >= 0) { + if (bicker_get(0x05, ¶meter) >= 0) { /* XXX: it seems to not work */ dstate_setinfo("battery.charge.restart", "%u", - parameter.enabled ? parameter.val : 0); + parameter.enabled ? parameter.value : 0); } - parameter.id = 0x04; - if (bicker_get(¶meter) >= 0) { + if (bicker_get(0x04, ¶meter) >= 0) { dstate_setinfo("ups.delay.start", "%u", - parameter.enabled ? parameter.val : 0); + parameter.enabled ? parameter.value : 0); } } From fca995785b94fe1d87692daef2fe88b160bd26cb Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Thu, 23 May 2024 08:52:16 +0200 Subject: [PATCH 22/65] bicker_ser: add sanity checks to bicker_set These devices seem to be picky on what you use to set parameters. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 96fc67b688..37aaede920 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -115,6 +115,7 @@ #define BICKER_TIMEOUT 1 #define BICKER_DELAY 20 #define BICKER_RETRIES 3 +#define BICKER_MAXID 0x0A /* Max parameter ID */ /* Protocol lengths */ #define BICKER_HEADER 3 @@ -461,6 +462,17 @@ static ssize_t bicker_set(uint8_t id, uint8_t enabled, uint16_t value, BickerPar ssize_t ret; uint8_t data[3]; + if (id < 1 || id > BICKER_MAXID) { + upslogx(LOG_ERR, "bicker_set(0x%02X, %d, %u): id out of range (0x01..0x%02X)", + (unsigned)id, enabled, (unsigned)value, + (unsigned)BICKER_MAXID); + return -1; + } else if (enabled > 1) { + upslogx(LOG_ERR, "bicker_set(0x%02X, %d, %u): enabled must be 0 or 1", + (unsigned)id, enabled, (unsigned)value); + return -1; + } + /* Format of `data` is "[EE] [ffFF]" * where: * [EE] = enabled (Bool) From 4cbe12dce899aad4290f07afbe04ec31efeae745 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Thu, 23 May 2024 10:29:13 +0200 Subject: [PATCH 23/65] bicker_ser: expose mappable parameters Every Bicker parameter that has a correspondence in nut-names.txt has been mapped. If the parameter is disabled on the device, no NUT variable is created. If the NUT variable is set to an empty string, the parameter is reset and disabled on the device and that NUT variable is removed. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 108 ++++++++++++++++++++++++++++++------------- 1 file changed, 76 insertions(+), 32 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 37aaede920..fbbe68c444 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -144,6 +144,18 @@ typedef struct { uint16_t value; } BickerParameter; +typedef struct { + uint8_t bicker_id; + const char *nut_name; +} BickerMapping; + +static const BickerMapping bicker_mappings[] = { + { 0x02, "ups.delay.shutdown" }, + { 0x04, "ups.delay.start" }, + { 0x05, "battery.charge.restart" }, + { 0x07, "battery.charge.low" }, +}; + /** * Send a packet. * @param idx Command index @@ -489,6 +501,38 @@ static ssize_t bicker_set(uint8_t id, uint8_t enabled, uint16_t value, BickerPar return bicker_receive_parameter(parameter); } +/** + * Write to a Bicker parameter. + * @param id ID of the parameter (0x01..0x0A) + * @param val A string with the value to write + * @param parameter Where to store the response (required!) + * @return The size of the data field on success or -1 on errors. + * + * This function is similar to bicker_set() but accepts string values. + * If `val` is NULL or empty, the underlying Bicker parameter is + * disabled and reset to its standard value. + */ +static int bicker_write(uint8_t id, const char *val, BickerParameter *parameter) +{ + ssize_t ret; + uint8_t enabled; + uint16_t value; + + if (val == NULL || val[0] == '\0') { + ret = bicker_get(id, parameter); + if (ret < 0) { + return ret; + } + enabled = 0; + value = parameter->std; + } else { + enabled = 1; + value = atoi(val); + } + + return bicker_set(id, enabled, value, parameter); +} + /* For some reason the `seconds` delay (at least on my UPSIC-2403D) * is not honored: the shutdown is always delayed by 2 seconds. This * fixed delay seems to be independent from the state of the UPS (on @@ -521,7 +565,7 @@ static ssize_t bicker_shutdown(void) int delay; str = dstate_getinfo("ups.delay.shutdown"); - delay = atoi(str); + delay = str != NULL ? atoi(str) : 0; if (delay > 255) { upslogx(LOG_WARNING, "Shutdown delay too big: %d > 255", delay); @@ -545,20 +589,26 @@ static int bicker_instcmd(const char *cmdname, const char *extra) static int bicker_setvar(const char *varname, const char *val) { - if (!strcasecmp(varname, "battery.charge.restart")) { - int value = (uint16_t)atoi(val); - if (bicker_set(0x05, 1, value, NULL) < 0) { - return STAT_SET_FAILED; - } - dstate_setinfo("battery.charge.restart", "%u", value); - return STAT_SET_HANDLED; - } else if (!strcasecmp(varname, "ups.delay.start")) { - int value = (uint16_t)atoi(val); - if (bicker_set(0x04, 1, value, NULL) < 0) { - return STAT_SET_FAILED; + const BickerMapping *mapping; + unsigned i; + BickerParameter parameter; + + /* Handle mapped parameters */ + for (i = 0; i < SIZEOF_ARRAY(bicker_mappings); ++i) { + mapping = &bicker_mappings[i]; + if (!strcasecmp(varname, mapping->nut_name)) { + if (bicker_write(mapping->bicker_id, val, ¶meter) < 0) { + return STAT_SET_FAILED; + } + + if (parameter.enabled) { + dstate_setinfo(varname, "%u", parameter.value); + } else { + /* Disabled parameters are removed from NUT */ + dstate_delinfo(varname); + } + return STAT_SET_HANDLED; } - dstate_setinfo("ups.delay.start", "%u", value); - return STAT_SET_HANDLED; } upslogx(LOG_NOTICE, "setvar: unknown variable [%s]", varname); @@ -658,9 +708,9 @@ void upsdrv_updateinfo(void) status_init(); - /* Consider the battery low when its charge is < 30% */ + /* On no "battery.charge.low" variable, use 30% */ str = dstate_getinfo("battery.charge.low"); - if (u8 < atoi(str)) { + if (u8 < (str != NULL ? atoi(str) : 30)) { status_set("LB"); } @@ -728,16 +778,13 @@ void upsdrv_initups(void) { char string[BICKER_MAXDATA + 1]; BickerParameter parameter; + const BickerMapping *mapping; + unsigned i; upsfd = ser_open(device_path); ser_set_speed(upsfd, device_path, B38400); ser_set_dtr(upsfd, 1); - /* Adding this variable here because upsdrv_initinfo() is not - * called when triggering a forced shutdown and - * "ups.delay.shutdown" is needed right there */ - dstate_setinfo("ups.delay.shutdown", "%u", BICKER_DELAY); - if (bicker_read_string(0x01, 0x63, string) >= 0) { dstate_setinfo("ups.firmware", "%s", string); } @@ -746,22 +793,19 @@ void upsdrv_initups(void) dstate_setinfo("battery.type", "%s", string); } - dstate_setinfo("battery.charge.low", "%d", 30); - /* Not implemented on all UPSes */ if (bicker_read_string(0x01, 0x65, string) >= 0) { dstate_setinfo("ups.firmware.aux", "%s", string); } - if (bicker_get(0x05, ¶meter) >= 0) { - /* XXX: it seems to not work */ - dstate_setinfo("battery.charge.restart", "%u", - parameter.enabled ? parameter.value : 0); - } - - if (bicker_get(0x04, ¶meter) >= 0) { - dstate_setinfo("ups.delay.start", "%u", - parameter.enabled ? parameter.value : 0); + /* Initialize mapped parameters */ + for (i = 0; i < SIZEOF_ARRAY(bicker_mappings); ++i) { + mapping = &bicker_mappings[i]; + if (bicker_get(mapping->bicker_id, ¶meter) >= 0 && + parameter.enabled) { + dstate_setinfo(mapping->nut_name, "%u", + (unsigned)parameter.value); + } } } From d4962f28ec3d959e8a0ffbebf2e07540db7f797d Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 24 May 2024 03:29:50 +0200 Subject: [PATCH 24/65] docs/man/nut.exe.txt: mention `net start/stop` to manage Windows services [#2446, #2455] Signed-off-by: Jim Klimov --- docs/man/nut.exe.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/man/nut.exe.txt b/docs/man/nut.exe.txt index 6109629a85..0100a3dbe6 100644 --- a/docs/man/nut.exe.txt +++ b/docs/man/nut.exe.txt @@ -26,6 +26,16 @@ UPS shutdown command in case of FSD handling, or for mere 'netclient' systems it would run just the 'upsmon' client to monitor remote UPS device(s) and initiate the OS shut down on the local Windows system as applicable. +Beside launching or stopping a set of the NUT programs in certain cases, +this helper program also allows to register (or un-register) itself as a +Windows service. To actually manage the service from command line you can +execute the Windows `net` command, e.g.: + +---- +net stop "Network UPS Tools" +net start "Network UPS Tools" +---- + OPTIONS ------- From 29b7e7a6e27560f3c99e096a5400f590dfbacc3b Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 24 May 2024 03:33:23 +0200 Subject: [PATCH 25/65] scripts/Windows/wininit.c: clarify that this is what becomes "nut.exe" normally [#2446] Signed-off-by: Jim Klimov --- scripts/Windows/wininit.c | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/Windows/wininit.c b/scripts/Windows/wininit.c index f4c29b3bf0..92659bc11a 100644 --- a/scripts/Windows/wininit.c +++ b/scripts/Windows/wininit.c @@ -1,4 +1,5 @@ /* wininit.c - MS Windows service which replace the init script + (compiled as "nut.exe") Copyright (C) 2010 Frederic Bohe From 8541d6a41608e6a5a9fb791f4e00f9c1bea8e8f2 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 24 May 2024 04:09:00 +0200 Subject: [PATCH 26/65] scripts/Windows/wininit.c: introduce an SvcExists() method [#2455] Signed-off-by: Jim Klimov --- scripts/Windows/wininit.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/scripts/Windows/wininit.c b/scripts/Windows/wininit.c index 92659bc11a..3c66cc1ec0 100644 --- a/scripts/Windows/wininit.c +++ b/scripts/Windows/wininit.c @@ -424,6 +424,45 @@ static int SvcInstall(const char * SvcName, const char * args) return EXIT_SUCCESS; } +/* Returns a positive value if the service name exists + * -2 if we can not open the service manager + * -1 if we can not open the service itself + * +1 SvcName exists + */ +static int SvcExists(const char * SvcName) +{ + SC_HANDLE SCManager; + SC_HANDLE Service; + + SCManager = OpenSCManager( + NULL, /* local computer */ + NULL, /* ServicesActive database */ + SC_MANAGER_ALL_ACCESS); /* full access rights */ + + if (NULL == SCManager) { + upsdebugx(1, "OpenSCManager failed (%d)\n", (int)GetLastError()); + return -2; + } + + Service = OpenService( + SCManager, /* SCM database */ + SvcName, /* name of service */ + DELETE); /* need delete access */ + + if (Service == NULL) { + upsdebugx(1, "OpenService failed (%d) for \"%s\"\n", + (int)GetLastError(), SvcName); + CloseServiceHandle(SCManager); + return -1; + } + + CloseServiceHandle(Service); + CloseServiceHandle(SCManager); + + upsdebugx(1, "Service \"%s\" seems to exist", SvcName); + return 1; +} + static int SvcUninstall(const char * SvcName) { SC_HANDLE SCManager; From 1eb7a61f66671919d1d87969cfc77e598b361aa4 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 24 May 2024 04:09:38 +0200 Subject: [PATCH 27/65] scripts/Windows/wininit.c, docs/man/nut.exe.txt: introduce "nut.exe start/stop" wrappings [#2455] Signed-off-by: Jim Klimov --- docs/man/nut.exe.txt | 11 +++++++++++ scripts/Windows/wininit.c | 41 ++++++++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/docs/man/nut.exe.txt b/docs/man/nut.exe.txt index 0100a3dbe6..a1434302d3 100644 --- a/docs/man/nut.exe.txt +++ b/docs/man/nut.exe.txt @@ -36,6 +36,10 @@ net stop "Network UPS Tools" net start "Network UPS Tools" ---- +You can also execute `nut start` to automatically register the service +(if not yet registered) and start it, and `nut stop` to stop the service +if registered. + OPTIONS ------- @@ -66,6 +70,13 @@ Uninstall the Windows service. *-N*:: Run once in non-service mode (for troubleshooting). +*start*:: +Install as a Windows service called "Network UPS Tools" (if not yet done), +and try to start this service. + +*stop*:: +Try to stop a Windows service called "Network UPS Tools". + DIAGNOSTICS ----------- diff --git a/scripts/Windows/wininit.c b/scripts/Windows/wininit.c index 3c66cc1ec0..6c81929f8f 100644 --- a/scripts/Windows/wininit.c +++ b/scripts/Windows/wininit.c @@ -706,9 +706,14 @@ static void help(const char *arg_progname) printf("NUT for Windows all-in-one wrapper for driver(s), data server and monitoring client\n"); printf("including shutdown and power-off handling (where supported). All together they rely\n"); - printf("on nut.conf and other files in %s\n\n", confpath()); + printf("on nut.conf and other files in %s\n", confpath()); - printf("Usage: %s [OPTION]\n\n", arg_progname); + printf("\nUsage: %s {start | stop}\n\n", arg_progname); + printf(" start Install as a service (%s) if not yet done, then `net start` it\n", SVCNAME); + printf(" stop If the service (%s) is installed, command it to `net stop`\n", SVCNAME); + printf("Note you may have to run this in an elevated privilege command shell, or use `runas`\n"); + + printf("\nUsage: %s [OPTION]\n\n", arg_progname); printf("Options (note minus not slash as the control character), one of:\n"); printf(" -I Install as a service (%s)\n", SVCNAME); @@ -724,9 +729,35 @@ int main(int argc, char **argv) int i, default_opterr = opterr; const char *progname = xbasename(argc > 0 ? argv[0] : "nut.exe"); - if (argc > 1 && !strcmp(argv[1], "/?")) { - help(progname); - return EXIT_SUCCESS; + if (argc > 1) { + if (!strcmp(argv[1], "/?")) { + help(progname); + return EXIT_SUCCESS; + } + + if (!strcmp(argv[1], "stop")) { + int ret; + if (SvcExists(SVCNAME) < 0) + fprintf(stderr, "WARNING: Can not access service \"%s\"", SVCNAME); + + ret = system("net stop \"" SVCNAME "\""); + if (ret == 0) + return EXIT_SUCCESS; + fatalx(EXIT_FAILURE, "FAILED stopping %s: %i", SVCNAME, ret); + } + + if (!strcmp(argv[1], "start")) { + int ret; + if (SvcExists(SVCNAME) < 0) { + fprintf(stderr, "WARNING: Can not access service \"%s\", registering first", SVCNAME); + SvcInstall(SVCNAME, NULL); + } + + ret = system("net start \"" SVCNAME "\""); + if (ret == 0) + return EXIT_SUCCESS; + fatalx(EXIT_FAILURE, "FAILED starting %s: %i", SVCNAME, ret); + } } /* TODO: Do not warn about unknown args - pass them to SvcMain() From 9f06f461989063d996f5da18fc2f14fec0b2a40f Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 24 May 2024 10:32:46 +0200 Subject: [PATCH 28/65] docs/man/nut.exe.txt, docs/nut.dict: suggest some PowerShell magic to prepare Windows Firewall for upsd [#2446, #2455] Signed-off-by: Jim Klimov --- docs/man/nut.exe.txt | 28 +++++++++++++++++++++++++++- docs/nut.dict | 15 ++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/docs/man/nut.exe.txt b/docs/man/nut.exe.txt index a1434302d3..c176a89d33 100644 --- a/docs/man/nut.exe.txt +++ b/docs/man/nut.exe.txt @@ -38,7 +38,33 @@ net start "Network UPS Tools" You can also execute `nut start` to automatically register the service (if not yet registered) and start it, and `nut stop` to stop the service -if registered. +(if registered and running). + +Note that for a Windows machine to act as a NUT data server for further +clients, you may have to add Windows Firewall rules to allow incoming +connections (by default to port `3493/tcp`), e.g. using PowerShell to +elevate (alternately right-click a "Command Prompt" shortcut and select +"Run as administrator"), and execute `netsh` to actually configure the +needed "Advanced Firewall" rule: + +---- +REM Elevate to administrator status then run netsh to add firewall rule. +REM Recommended to adapt "LocalIP" to expected listeners of this server, +REM and "RemoteIP" to your single or comma-separated subnet(s) in CIDR +REM notation, specific client IP address(es), or ranges of address(es) +REM (dash-separated, as IP1-IP2). + +REM The following goes as one long command line: + +powershell.exe -Command "Start-Process netsh.exe -ArgumentList + \"advfirewall firewall add rule name=NUT-upsd-data-server + dir=in action=allow localip=ANY remoteip=ANY + program=%ProgramFiles%\NUT\sbin\upsd.exe + localport=3493 protocol=tcp\" -Verb RunAs" +---- + +Keep in mind that by default NUT `upsd` only listens on `localhost`, so +you would need to add some `LISTEN` directives in `upsd.conf` as well. OPTIONS ------- diff --git a/docs/nut.dict b/docs/nut.dict index b0ad18af66..f6feb857ec 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3151 utf-8 +personal_ws-1.1 en 3165 utf-8 AAC AAS ABI @@ -60,6 +60,7 @@ Antonino Apodaca AppData AppVeyor +ArgumentList Arjen Arkadiusz Armin @@ -611,6 +612,7 @@ LineB Lintian ListClients Lite's +LocalIP LogMax LogMin LowBatt @@ -907,6 +909,7 @@ PowerPS PowerPal PowerPanel PowerShare +PowerShell PowerShield PowerSure PowerTech @@ -924,6 +927,7 @@ PresentStatus Priv ProductID Progra +ProgramFiles Proxmox Prynych Pulizzi @@ -997,6 +1001,7 @@ Redhat Regados Reinholdtsen Remi +RemoteIP Remoting Rene René @@ -1014,6 +1019,7 @@ Rodríguez Rouben Rozman Rucelf +RunAs RunUPSCommand RxD Ryabov @@ -1405,6 +1411,7 @@ adm admin's adminbox adoc +advfirewall advorder ae aec @@ -2204,6 +2211,8 @@ lnetsnmp loadPercentage localcalculation localhost +localip +localport localtime lockf logfacility @@ -2350,6 +2359,7 @@ nds netcat netclient netserver +netsh netsnmp netvision networkupstools @@ -2530,6 +2540,7 @@ powernet poweroff powerpal powerpanel +powershell powerup powervalue powerware @@ -2610,6 +2621,7 @@ regtype relatime releasekeyring relicensing +remoteip renderer renderers repindex @@ -2823,6 +2835,7 @@ sublicense sublicenses submodule submodules +subnet subtree sudo suid From eac5a52ac69af221d41b88e0dcc167fdeaee1e26 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Mon, 27 May 2024 11:38:25 +0200 Subject: [PATCH 29/65] bicker_ser: rename `parameter` to `dst` This should make clear that argument is just used for destination. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 60 ++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index fbbe68c444..22b9b99f05 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -405,17 +405,17 @@ static ssize_t bicker_read_string(uint8_t idx, uint8_t cmd, char *dst) return ret; } -static ssize_t bicker_receive_parameter(BickerParameter *parameter) +static ssize_t bicker_receive_parameter(BickerParameter *dst) { ssize_t ret; uint8_t data[10]; - ret = bicker_receive_known(0x07, parameter->id, data, 10); + ret = bicker_receive_known(0x07, dst->id, data, 10); if (ret < 0) { return ret; } - if (parameter != NULL) { + if (dst != NULL) { /* The returned `data` is in the format: * [AA] [bbBB] [ccCC] [ddDD] [EE] [ffFF] * where: @@ -426,18 +426,18 @@ static ssize_t bicker_receive_parameter(BickerParameter *parameter) * [EE] = enabled (Bool) * [FFff] = value (UInt16) */ - parameter->id = data[0]; - parameter->min = WORDLH(data[1], data[2]); - parameter->max = WORDLH(data[3], data[4]); - parameter->std = WORDLH(data[5], data[6]); - parameter->enabled = data[7]; - parameter->value = WORDLH(data[8], data[9]); + dst->id = data[0]; + dst->min = WORDLH(data[1], data[2]); + dst->max = WORDLH(data[3], data[4]); + dst->std = WORDLH(data[5], data[6]); + dst->enabled = data[7]; + dst->value = WORDLH(data[8], data[9]); upsdebugx(3, "Parameter %u = %u (%s, min = %u, max = %u, std = %u)", - (unsigned)parameter->id, (unsigned)parameter->value, - parameter->enabled ? "enabled" : "disabled", - (unsigned)parameter->min, (unsigned)parameter->max, - (unsigned)parameter->std); + (unsigned)dst->id, (unsigned)dst->value, + dst->enabled ? "enabled" : "disabled", + (unsigned)dst->min, (unsigned)dst->max, + (unsigned)dst->std); } return ret; @@ -445,11 +445,11 @@ static ssize_t bicker_receive_parameter(BickerParameter *parameter) /** * Get a Bicker parameter. - * @param id ID of the parameter (0x01..0x0A) - * @param parameter Where to store the response or NULL to discard + * @param id ID of the parameter (0x01..0x0A) + * @param dst Where to store the response or NULL to discard * @return The size of the data field on success or -1 on errors. */ -static ssize_t bicker_get(uint8_t id, BickerParameter *parameter) +static ssize_t bicker_get(uint8_t id, BickerParameter *dst) { ssize_t ret; @@ -458,18 +458,18 @@ static ssize_t bicker_get(uint8_t id, BickerParameter *parameter) return ret; } - return bicker_receive_parameter(parameter); + return bicker_receive_parameter(dst); } /** * Set a Bicker parameter. - * @param id ID of the parameter (0x01..0x0A) - * @param enabled 0 to disable, 1 to enable - * @param value - * @param parameter Where to store the response or NULL to discard + * @param id ID of the parameter (0x01..0x0A) + * @param enabled 0 to disable, 1 to enable + * @param value What to set in the value field + * @param dst Where to store the response or NULL to discard * @return The size of the data field on success or -1 on errors. */ -static ssize_t bicker_set(uint8_t id, uint8_t enabled, uint16_t value, BickerParameter *parameter) +static ssize_t bicker_set(uint8_t id, uint8_t enabled, uint16_t value, BickerParameter *dst) { ssize_t ret; uint8_t data[3]; @@ -498,39 +498,39 @@ static ssize_t bicker_set(uint8_t id, uint8_t enabled, uint16_t value, BickerPar return ret; } - return bicker_receive_parameter(parameter); + return bicker_receive_parameter(dst); } /** * Write to a Bicker parameter. - * @param id ID of the parameter (0x01..0x0A) - * @param val A string with the value to write - * @param parameter Where to store the response (required!) + * @param id ID of the parameter (0x01..0x0A) + * @param val A string with the value to write + * @param dst Where to store the response (required!) * @return The size of the data field on success or -1 on errors. * * This function is similar to bicker_set() but accepts string values. * If `val` is NULL or empty, the underlying Bicker parameter is * disabled and reset to its standard value. */ -static int bicker_write(uint8_t id, const char *val, BickerParameter *parameter) +static int bicker_write(uint8_t id, const char *val, BickerParameter *dst) { ssize_t ret; uint8_t enabled; uint16_t value; if (val == NULL || val[0] == '\0') { - ret = bicker_get(id, parameter); + ret = bicker_get(id, dst); if (ret < 0) { return ret; } enabled = 0; - value = parameter->std; + value = dst->std; } else { enabled = 1; value = atoi(val); } - return bicker_set(id, enabled, value, parameter); + return bicker_set(id, enabled, value, dst); } /* For some reason the `seconds` delay (at least on my UPSIC-2403D) From 86d4d0ee5d69cda633bfd59e4208b65282238e2d Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Mon, 27 May 2024 11:41:42 +0200 Subject: [PATCH 30/65] bicker_ser: make `dst` optional in bicker_write The `dst` argument is optional in every other function, so be consistent and make it optional also in this one. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 22b9b99f05..5dd2d18879 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -505,7 +505,7 @@ static ssize_t bicker_set(uint8_t id, uint8_t enabled, uint16_t value, BickerPar * Write to a Bicker parameter. * @param id ID of the parameter (0x01..0x0A) * @param val A string with the value to write - * @param dst Where to store the response (required!) + * @param dst Where to store the response or NULL to discard * @return The size of the data field on success or -1 on errors. * * This function is similar to bicker_set() but accepts string values. @@ -515,22 +515,32 @@ static ssize_t bicker_set(uint8_t id, uint8_t enabled, uint16_t value, BickerPar static int bicker_write(uint8_t id, const char *val, BickerParameter *dst) { ssize_t ret; + BickerParameter parameter; uint8_t enabled; uint16_t value; if (val == NULL || val[0] == '\0') { - ret = bicker_get(id, dst); + ret = bicker_get(id, ¶meter); if (ret < 0) { return ret; } enabled = 0; - value = dst->std; + value = parameter.std; } else { enabled = 1; value = atoi(val); } - return bicker_set(id, enabled, value, dst); + ret = bicker_set(id, enabled, value, ¶meter); + if (ret < 0) { + return ret; + } + + if (dst != NULL) { + memcpy(dst, ¶meter, sizeof(parameter)); + } + + return ret; } /* For some reason the `seconds` delay (at least on my UPSIC-2403D) From 1964a2fdeba97d9c6f26f12dc73eecac782750f9 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Mon, 27 May 2024 11:57:06 +0200 Subject: [PATCH 31/65] bicker_ser: add id validation to bicker_get() Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 5dd2d18879..d3eb31872d 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -156,6 +156,24 @@ static const BickerMapping bicker_mappings[] = { { 0x07, "battery.charge.low" }, }; +/** + * Parameter id validation. + * @param id Id of the parameter + * @param context Description of the calling code for the log message + * @return 1 on valid id, 0 on errors. + * + * The id is valid if within the 0x01..BICKER_MAXID range (inclusive). + */ +static int bicker_valid_id(uint8_t id, const char *context) +{ + if (id < 1 || id > BICKER_MAXID) { + upslogx(LOG_ERR, "%s: parameter id 0x%02X is out of range (0x01..0x%02X)", + context, (unsigned)id, (unsigned)BICKER_MAXID); + return 0; + } + return 1; +} + /** * Send a packet. * @param idx Command index @@ -419,7 +437,7 @@ static ssize_t bicker_receive_parameter(BickerParameter *dst) /* The returned `data` is in the format: * [AA] [bbBB] [ccCC] [ddDD] [EE] [ffFF] * where: - * [AA] = parameter ID (Byte) + * [AA] = parameter id (Byte) * [BBbb] = minimum value (UInt16) * [CCcc] = maximum value (UInt16) * [DDdd] = standard value (UInt16) @@ -445,7 +463,7 @@ static ssize_t bicker_receive_parameter(BickerParameter *dst) /** * Get a Bicker parameter. - * @param id ID of the parameter (0x01..0x0A) + * @param id Id of the parameter * @param dst Where to store the response or NULL to discard * @return The size of the data field on success or -1 on errors. */ @@ -453,6 +471,10 @@ static ssize_t bicker_get(uint8_t id, BickerParameter *dst) { ssize_t ret; + if (!bicker_valid_id(id, "bicker_get")) { + return -1; + } + ret = bicker_send(0x07, id, NULL, 0); if (ret < 0) { return ret; @@ -463,7 +485,7 @@ static ssize_t bicker_get(uint8_t id, BickerParameter *dst) /** * Set a Bicker parameter. - * @param id ID of the parameter (0x01..0x0A) + * @param id Id of the parameter * @param enabled 0 to disable, 1 to enable * @param value What to set in the value field * @param dst Where to store the response or NULL to discard @@ -474,10 +496,7 @@ static ssize_t bicker_set(uint8_t id, uint8_t enabled, uint16_t value, BickerPar ssize_t ret; uint8_t data[3]; - if (id < 1 || id > BICKER_MAXID) { - upslogx(LOG_ERR, "bicker_set(0x%02X, %d, %u): id out of range (0x01..0x%02X)", - (unsigned)id, enabled, (unsigned)value, - (unsigned)BICKER_MAXID); + if (!bicker_valid_id(id, "bicker_set")) { return -1; } else if (enabled > 1) { upslogx(LOG_ERR, "bicker_set(0x%02X, %d, %u): enabled must be 0 or 1", @@ -503,7 +522,7 @@ static ssize_t bicker_set(uint8_t id, uint8_t enabled, uint16_t value, BickerPar /** * Write to a Bicker parameter. - * @param id ID of the parameter (0x01..0x0A) + * @param id Id of the parameter * @param val A string with the value to write * @param dst Where to store the response or NULL to discard * @return The size of the data field on success or -1 on errors. From c602b9ddb9259127d9723212cf50d746b9b15e78 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Mon, 27 May 2024 16:56:15 +0200 Subject: [PATCH 32/65] bicker_ser: allow NULL dst in parameter handling The comments say to use `NULL` for discarding the results but bicker_receive_parameter() was accessing `dst->id`, effectively preventing it. Fix that bug and improve the code: now bicker_receive_parameter() validates the parameter id (so it can be used freely) and it always logs debugging info about the parameter. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 93 +++++++++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 40 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index d3eb31872d..0cc47aaf81 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -335,6 +335,57 @@ static ssize_t bicker_receive_known(uint8_t idx, uint8_t cmd, void *data, size_t return real_datalen; } +/** + * Receive the response of a set/get parameter command. + * @param id Id of the parameter + * @param dst Where to store the response or NULL to discard + * @return The size of the data field on success or -1 on errors. + */ +static ssize_t bicker_receive_parameter(uint8_t id, BickerParameter *dst) +{ + ssize_t ret; + uint8_t data[10]; + BickerParameter parameter; + + if (!bicker_valid_id(id, "bicker_receive_parameter")) { + return -1; + } + + ret = bicker_receive_known(0x07, id, data, sizeof(data)); + if (ret < 0) { + return ret; + } + + /* The returned `data` is in the format: + * [AA] [bbBB] [ccCC] [ddDD] [EE] [ffFF] + * where: + * [AA] = parameter id (Byte) + * [BBbb] = minimum value (UInt16) + * [CCcc] = maximum value (UInt16) + * [DDdd] = standard value (UInt16) + * [EE] = enabled (Bool) + * [FFff] = value (UInt16) + */ + parameter.id = data[0]; + parameter.min = WORDLH(data[1], data[2]); + parameter.max = WORDLH(data[3], data[4]); + parameter.std = WORDLH(data[5], data[6]); + parameter.enabled = data[7]; + parameter.value = WORDLH(data[8], data[9]); + + upsdebugx(3, "Parameter %u = %u (%s, min = %u, max = %u, std = %u)", + (unsigned)parameter.id, (unsigned)parameter.value, + parameter.enabled ? "enabled" : "disabled", + (unsigned)parameter.min, (unsigned)parameter.max, + (unsigned)parameter.std); + + if (dst != NULL) { + memcpy(dst, ¶meter, sizeof(parameter)); + } + + return ret; +} + /** * Execute a command that returns an uint8_t value. * @param idx Command index @@ -423,44 +474,6 @@ static ssize_t bicker_read_string(uint8_t idx, uint8_t cmd, char *dst) return ret; } -static ssize_t bicker_receive_parameter(BickerParameter *dst) -{ - ssize_t ret; - uint8_t data[10]; - - ret = bicker_receive_known(0x07, dst->id, data, 10); - if (ret < 0) { - return ret; - } - - if (dst != NULL) { - /* The returned `data` is in the format: - * [AA] [bbBB] [ccCC] [ddDD] [EE] [ffFF] - * where: - * [AA] = parameter id (Byte) - * [BBbb] = minimum value (UInt16) - * [CCcc] = maximum value (UInt16) - * [DDdd] = standard value (UInt16) - * [EE] = enabled (Bool) - * [FFff] = value (UInt16) - */ - dst->id = data[0]; - dst->min = WORDLH(data[1], data[2]); - dst->max = WORDLH(data[3], data[4]); - dst->std = WORDLH(data[5], data[6]); - dst->enabled = data[7]; - dst->value = WORDLH(data[8], data[9]); - - upsdebugx(3, "Parameter %u = %u (%s, min = %u, max = %u, std = %u)", - (unsigned)dst->id, (unsigned)dst->value, - dst->enabled ? "enabled" : "disabled", - (unsigned)dst->min, (unsigned)dst->max, - (unsigned)dst->std); - } - - return ret; -} - /** * Get a Bicker parameter. * @param id Id of the parameter @@ -480,7 +493,7 @@ static ssize_t bicker_get(uint8_t id, BickerParameter *dst) return ret; } - return bicker_receive_parameter(dst); + return bicker_receive_parameter(id, dst); } /** @@ -517,7 +530,7 @@ static ssize_t bicker_set(uint8_t id, uint8_t enabled, uint16_t value, BickerPar return ret; } - return bicker_receive_parameter(dst); + return bicker_receive_parameter(id, dst); } /** From 2241b253caab47f0200f0409071873a4caf86679 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Mon, 27 May 2024 22:50:48 +0200 Subject: [PATCH 33/65] bicker_ser: ensure battery.charge.low is defined According to RFC-9271, that variable is part of the "Desktop PC Variables" set so it is expected to be present and its default value is 20 seconds. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 0cc47aaf81..2b40638b24 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -681,7 +681,6 @@ void upsdrv_initinfo(void) void upsdrv_updateinfo(void) { - const char *str; uint8_t u8; uint16_t u16; int16_t i16; @@ -750,9 +749,8 @@ void upsdrv_updateinfo(void) status_init(); - /* On no "battery.charge.low" variable, use 30% */ - str = dstate_getinfo("battery.charge.low"); - if (u8 < (str != NULL ? atoi(str) : 30)) { + /* In `u8` we already have the battery charge */ + if (u8 < atoi(dstate_getinfo("battery.charge.low"))) { status_set("LB"); } @@ -849,6 +847,11 @@ void upsdrv_initups(void) (unsigned)parameter.value); } } + + /* Ensure "battery.charge.low" variable is defined */ + if (dstate_getinfo("battery.charge.low") == NULL) { + dstate_setinfo("battery.charge.low", "%d", 20); + } } void upsdrv_cleanup(void) From afee54f3b0327550aa4d0338798a20e3807790b7 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Tue, 28 May 2024 13:49:34 +0200 Subject: [PATCH 34/65] bicker_ser: allow writing to Bicker parameters All Bicker parameters (at least the supported ones) can be changed, so enable writing and set proper ranges. Expose all parameters, even the non-standard ones. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 72 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 9 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 2b40638b24..76afcdeb88 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -116,6 +116,7 @@ #define BICKER_DELAY 20 #define BICKER_RETRIES 3 #define BICKER_MAXID 0x0A /* Max parameter ID */ +#define BICKER_MAXVAL 0xFFFF /* Max parameter value */ /* Protocol lengths */ #define BICKER_HEADER 3 @@ -147,13 +148,31 @@ typedef struct { typedef struct { uint8_t bicker_id; const char *nut_name; + const char *description; } BickerMapping; static const BickerMapping bicker_mappings[] = { - { 0x02, "ups.delay.shutdown" }, - { 0x04, "ups.delay.start" }, - { 0x05, "battery.charge.restart" }, - { 0x07, "battery.charge.low" }, + /* In docs/nut-names.txt: names and descriptions from there */ + { 0x02, "ups.delay.shutdown", + "Interval to wait after shutdown with delay command (seconds)" }, + { 0x04, "ups.delay.start", + "Interval to wait before restarting the load (seconds)" }, + { 0x05, "battery.charge.restart", + "Minimum battery level for UPS restart after power-off" }, + { 0x07, "battery.charge.low", + "Remaining battery level when UPS switches to LB (percent)" }, + + /* Not in docs/nut-names.txt: crafted names and descriptions */ + { 0x01, "output.current.low", + "Current threshold under which the power will be cut (mA)" }, + { 0x03, "ups.delay.shutdown.signal", + "Interval to wait before sending the shutdown signal (seconds)" }, + { 0x06, "ups.delay.shutdown.in1", + "Interval to wait with IN1 high before sending the shutdown signal (seconds)" }, + { 0x08, "battery.charge.low.empty", + "Battery level threshold for the empty signal (percent)" }, + { 0x09, "ups.relay.mode", + "Behavior of the relay" }, }; /** @@ -474,6 +493,42 @@ static ssize_t bicker_read_string(uint8_t idx, uint8_t cmd, char *dst) return ret; } +/** + * Create a read-write Bicker parameter. + * @param parameter Source information + * @param mapping How that parameter is mapped to NUT + */ +static void bicker_new(const BickerParameter *parameter, const BickerMapping *mapping) +{ + const char *varname; + + varname = mapping->nut_name; + if (parameter->enabled) { + dstate_setinfo(varname, "%u", (unsigned)parameter->value); + } else { + /* dstate_setinfo(varname, "") triggers a GCC warning */ + dstate_setinfo(varname, "%s", ""); + } + + /* Using ST_FLAG_STRING so an empty string can be used + * to identify a disabled parameter */ + dstate_setflags(varname, ST_FLAG_RW | ST_FLAG_STRING); + + /* Just tested it: setting a range does not hinder setting + * an empty string with `dstate_setinfo(varname, "")` */ + if (parameter->min == BICKER_MAXVAL) { + /* The device here is likely corrupt: + * apply a standard range to try using it anyway */ + upslogx(LOG_WARNING, "Parameter %s is corrupt", varname); + dstate_addrange(varname, 0, BICKER_MAXVAL); + } else { + dstate_addrange(varname, parameter->min, parameter->max); + } + + /* Maximum value for an uint16_t is 65535, i.e. 5 digits */ + dstate_setaux(varname, 5); +} + /** * Get a Bicker parameter. * @param id Id of the parameter @@ -644,7 +699,8 @@ static int bicker_setvar(const char *varname, const char *val) } if (parameter.enabled) { - dstate_setinfo(varname, "%u", parameter.value); + dstate_setinfo(varname, "%u", + (unsigned)parameter.value); } else { /* Disabled parameters are removed from NUT */ dstate_delinfo(varname); @@ -841,10 +897,8 @@ void upsdrv_initups(void) /* Initialize mapped parameters */ for (i = 0; i < SIZEOF_ARRAY(bicker_mappings); ++i) { mapping = &bicker_mappings[i]; - if (bicker_get(mapping->bicker_id, ¶meter) >= 0 && - parameter.enabled) { - dstate_setinfo(mapping->nut_name, "%u", - (unsigned)parameter.value); + if (bicker_get(mapping->bicker_id, ¶meter) >= 0) { + bicker_new(¶meter, mapping); } } From 1678f209415fc9f2928ec0ae0e2c0df781b7d705 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Tue, 28 May 2024 14:58:51 +0200 Subject: [PATCH 35/65] bicker_ser: add variables info to manpage Variables are mapped 1:1 to Bicker parameters, so the descriptions have been borrowed and adapted directly from the UPS Gen2 software manual. Signed-off-by: Nicola Fontana --- docs/man/bicker_ser.txt | 54 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/docs/man/bicker_ser.txt b/docs/man/bicker_ser.txt index f5122cf819..8ddc1cf6cf 100644 --- a/docs/man/bicker_ser.txt +++ b/docs/man/bicker_ser.txt @@ -34,6 +34,60 @@ EXTRA ARGUMENTS This driver supports no extra arguments from linkman:ups.conf[5]. +VARIABLES +--------- + +Depending on the type of your UPS unit, some of the following variables may +be changed with linkman:upsrw[8]. If the driver can't read a variable from the +UPS, it will not be made available. Whenever not explicitly stated, any variable +can be disabled, in which case the action it performs will not be executed. To +disable a variable, set it to an empty value. + +*output.current.low* (in mA, default `200`):: +If activated and the UPS is in battery mode and the current drops below the set +value, the output of the UPS will shut down and disconnect the energy storage to +prevent self-discharge. + +*ups.delay.shutdown* (in seconds, default disabled):: +If activated and the UPS is in battery mode and the set time has expired, the +output will be disabled, and the UPS and energy storage will be disconnected. + +*ups.delay.shutdown.signal* (in seconds, default disabled):: +If activated and the UPS is in battery mode and the set time has elapsed, a +shutdown command via USB or relay (relay event) is signaled. + +*ups.delay.start* (in seconds, default disabled):: +If activated and a restart condition switches the UPS output off and on again, +the set time is the delay between switching on and off. The time should cause a +defined off time so that capacities in the application can be discharged. + +*battery.charge.restart* (in percent, default disabled):: +If activated and the UPS is off or restarts, the UPS output will not be released +until the energy storage device has the set charge state. The energy storage +device is charged in the meantime. + +*ups.delay.shutdown.in1* (in seconds, default disabled):: +If activated and the UPS is in battery mode and the signal at the IN-1 input is +high and the set time has expired, a shutdown command via relay event is +signaled. + +*battery.charge.low* (in percent, default `20`):: +If activated and the UPS is in battery mode and the battery level drops below +the set value, a shutdown command via relay event is signaled. + +*battery.charge.low.empty* (in percent, default `20`):: +This parameter stores the threshold value for the "Battery Empty" signal. +Currently this setting is only valid for relay signaling. Cannot be disabled. + +*ups.relay.mode* (default `0x01`):: +This parameter controls the behavior of the relay in case of different events. +Cannot be disabled. +- `0x01` On power fail (normally closed) +- `0x02` On power fail (normally opened) +- `0x03` Shutdown impulse (1 second) +- `0x04` Battery low signal (normally closed) +- `0x05` Battery defect signal (normally closed) + INSTANT COMMANDS ---------------- From e8203b1ef1e7cceaef5380d9e84ddfed7bb7f7c8 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Tue, 28 May 2024 15:08:54 +0200 Subject: [PATCH 36/65] bicker_ser: bump driver version Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 76afcdeb88..51f13bbf29 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -108,7 +108,7 @@ #include "serial.h" #define DRIVER_NAME "Bicker serial protocol" -#define DRIVER_VERSION "0.01" +#define DRIVER_VERSION "0.02" #define BICKER_SOH 0x01 #define BICKER_EOT 0x04 From 727969538afb205f52a51886b0304456f62087bd Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Thu, 30 May 2024 08:14:22 +0200 Subject: [PATCH 37/65] bicker_ser: enforce expected data size When receiving a packet of known data size, ensure that data size is exactly what expected. That should avoid later surprises, e.g. fiddling with bytes never received. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 51f13bbf29..80409779ff 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -320,38 +320,35 @@ static ssize_t bicker_receive(uint8_t idx, uint8_t cmd, void *data) * Receive a packet with a data field of known size. * @param idx Command index * @param cmd Command - * @param data Destination buffer or NULL to discard the data field + * @param dst Destination buffer or NULL to discard the data field * @param datalen The expected size of the data field - * @return The size of the data field on success or -1 on errors. + * @return `datalen` on success or -1 on errors. * - * `data`, if not NULL, must have at least `datalen` bytes. If - * `datalen` is less than the received data size, an error is thrown. + * `dst`, if not NULL, must have at least `datalen` bytes. If the data + * is not exactly `datalen` bytes, an error is thrown. */ -static ssize_t bicker_receive_known(uint8_t idx, uint8_t cmd, void *data, size_t datalen) +static ssize_t bicker_receive_known(uint8_t idx, uint8_t cmd, void *dst, size_t datalen) { ssize_t ret; - size_t real_datalen; - uint8_t real_data[BICKER_MAXDATA]; + uint8_t data[BICKER_MAXDATA]; - ret = bicker_receive(idx, cmd, real_data); + ret = bicker_receive(idx, cmd, data); if (ret < 0) { return ret; } - real_datalen = (size_t)ret; - - if (datalen < real_datalen) { - upslogx(LOG_ERR, "Not enough space for the payload: %" - PRIuSIZE " < %" PRIuSIZE, - datalen, real_datalen); + if (datalen != (size_t)ret) { + upslogx(LOG_ERR, "Data size does not match: expected %" + PRIuSIZE " but got %" PRIiSIZE " bytes", + datalen, ret); return -1; } - if (data != NULL) { - memcpy(data, real_data, real_datalen); + if (dst != NULL) { + memcpy(dst, data, datalen); } - return real_datalen; + return datalen; } /** From 0f0c4dc40587ce798cf398126b79286cefe187bb Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Tue, 28 May 2024 15:50:39 +0200 Subject: [PATCH 38/65] bicker_ser: avoid GCC warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Initialize the `parameter` struct to avoid GCC complains about "‘parameter.value’ may be used uninitialized in this function". This is not a real error because, if `bicker_write` is successful, that struct is populated properly. Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 80409779ff..674cd51128 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -687,6 +687,18 @@ static int bicker_setvar(const char *varname, const char *val) unsigned i; BickerParameter parameter; + /* This should not be needed because when `bicker_write()` is + * successful the `parameter` struct is populated but gcc seems + * not to be smart enough to realize that and errors out with + * "error: ‘parameter...’ may be used uninitialized in this function" + */ + parameter.id = 0; + parameter.min = 0; + parameter.max = BICKER_MAXVAL; + parameter.std = 0; + parameter.enabled = 0; + parameter.value = 0; + /* Handle mapped parameters */ for (i = 0; i < SIZEOF_ARRAY(bicker_mappings); ++i) { mapping = &bicker_mappings[i]; From 7e5bc6f8a7fc2ae49d7ab885694576e0a3602b6f Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Thu, 30 May 2024 15:27:39 +0200 Subject: [PATCH 39/65] bicker_ser: register the shutdown.return instcmd Signed-off-by: Nicola Fontana --- drivers/bicker_ser.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 674cd51128..76ff307237 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -740,6 +740,8 @@ void upsdrv_initinfo(void) dstate_setinfo("device.model", "%s", string); } + dstate_addcmd("shutdown.return"); + upsh.instcmd = bicker_instcmd; upsh.setvar = bicker_setvar; } From 804323ea38578d7cd25045478225c7d9397aec5d Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Thu, 30 May 2024 15:49:18 +0200 Subject: [PATCH 40/65] bicker_ser: move unofficial variables into "experimental" namespace Signed-off-by: Nicola Fontana --- docs/man/bicker_ser.txt | 32 ++++++++++++++++---------------- drivers/bicker_ser.c | 14 +++++++------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/docs/man/bicker_ser.txt b/docs/man/bicker_ser.txt index 8ddc1cf6cf..97ef705fca 100644 --- a/docs/man/bicker_ser.txt +++ b/docs/man/bicker_ser.txt @@ -43,19 +43,10 @@ UPS, it will not be made available. Whenever not explicitly stated, any variable can be disabled, in which case the action it performs will not be executed. To disable a variable, set it to an empty value. -*output.current.low* (in mA, default `200`):: -If activated and the UPS is in battery mode and the current drops below the set -value, the output of the UPS will shut down and disconnect the energy storage to -prevent self-discharge. - *ups.delay.shutdown* (in seconds, default disabled):: If activated and the UPS is in battery mode and the set time has expired, the output will be disabled, and the UPS and energy storage will be disconnected. -*ups.delay.shutdown.signal* (in seconds, default disabled):: -If activated and the UPS is in battery mode and the set time has elapsed, a -shutdown command via USB or relay (relay event) is signaled. - *ups.delay.start* (in seconds, default disabled):: If activated and a restart condition switches the UPS output off and on again, the set time is the delay between switching on and off. The time should cause a @@ -66,20 +57,29 @@ If activated and the UPS is off or restarts, the UPS output will not be released until the energy storage device has the set charge state. The energy storage device is charged in the meantime. -*ups.delay.shutdown.in1* (in seconds, default disabled):: -If activated and the UPS is in battery mode and the signal at the IN-1 input is -high and the set time has expired, a shutdown command via relay event is -signaled. - *battery.charge.low* (in percent, default `20`):: If activated and the UPS is in battery mode and the battery level drops below the set value, a shutdown command via relay event is signaled. -*battery.charge.low.empty* (in percent, default `20`):: +*experimental.output.current.low* (in mA, default `200`):: +If activated and the UPS is in battery mode and the current drops below the set +value, the output of the UPS will shut down and disconnect the energy storage to +prevent self-discharge. + +*experimental.ups.delay.shutdown.signal* (in seconds, default disabled):: +If activated and the UPS is in battery mode and the set time has elapsed, a +shutdown command via relay event is signaled. + +*experimental.ups.delay.shutdown.signal.masked* (in seconds, default disabled):: +If activated and the UPS is in battery mode and the signal at the IN-1 input is +high and the set time has expired, a shutdown command via relay event is +signaled. + +*experimental.battery.charge.low.empty* (in percent, default `20`):: This parameter stores the threshold value for the "Battery Empty" signal. Currently this setting is only valid for relay signaling. Cannot be disabled. -*ups.relay.mode* (default `0x01`):: +*experimental.ups.relay.mode* (default `0x01`):: This parameter controls the behavior of the relay in case of different events. Cannot be disabled. - `0x01` On power fail (normally closed) diff --git a/drivers/bicker_ser.c b/drivers/bicker_ser.c index 76ff307237..a5c2c4db49 100644 --- a/drivers/bicker_ser.c +++ b/drivers/bicker_ser.c @@ -152,7 +152,7 @@ typedef struct { } BickerMapping; static const BickerMapping bicker_mappings[] = { - /* In docs/nut-names.txt: names and descriptions from there */ + /* Official variables present in docs/nut-names.txt */ { 0x02, "ups.delay.shutdown", "Interval to wait after shutdown with delay command (seconds)" }, { 0x04, "ups.delay.start", @@ -162,16 +162,16 @@ static const BickerMapping bicker_mappings[] = { { 0x07, "battery.charge.low", "Remaining battery level when UPS switches to LB (percent)" }, - /* Not in docs/nut-names.txt: crafted names and descriptions */ - { 0x01, "output.current.low", + /* Unofficial variables under the "experimental" namespace */ + { 0x01, "experimental.output.current.low", "Current threshold under which the power will be cut (mA)" }, - { 0x03, "ups.delay.shutdown.signal", + { 0x03, "experimental.ups.delay.shutdown.signal", "Interval to wait before sending the shutdown signal (seconds)" }, - { 0x06, "ups.delay.shutdown.in1", + { 0x06, "experimental.ups.delay.shutdown.signal.masked", "Interval to wait with IN1 high before sending the shutdown signal (seconds)" }, - { 0x08, "battery.charge.low.empty", + { 0x08, "experimental.battery.charge.low.empty", "Battery level threshold for the empty signal (percent)" }, - { 0x09, "ups.relay.mode", + { 0x09, "experimental.ups.relay.mode", "Behavior of the relay" }, }; From 9c3dedff16d96cbc79e26e839a9747a0dec51241 Mon Sep 17 00:00:00 2001 From: Nicola Fontana Date: Thu, 30 May 2024 17:21:41 +0200 Subject: [PATCH 41/65] bicker_ser: improve styling of relay modes doc Signed-off-by: Nicola Fontana --- docs/man/bicker_ser.txt | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/man/bicker_ser.txt b/docs/man/bicker_ser.txt index 97ef705fca..bebecceef7 100644 --- a/docs/man/bicker_ser.txt +++ b/docs/man/bicker_ser.txt @@ -82,11 +82,14 @@ Currently this setting is only valid for relay signaling. Cannot be disabled. *experimental.ups.relay.mode* (default `0x01`):: This parameter controls the behavior of the relay in case of different events. Cannot be disabled. -- `0x01` On power fail (normally closed) -- `0x02` On power fail (normally opened) -- `0x03` Shutdown impulse (1 second) -- `0x04` Battery low signal (normally closed) -- `0x05` Battery defect signal (normally closed) ++ +Available relay modes: +[horizontal] +`0x01`::: On power fail (normally closed) +`0x02`::: On power fail (normally opened) +`0x03`::: Shutdown impulse (1 second) +`0x04`::: Battery low signal (normally closed) +`0x05`::: Battery defect signal (normally closed) INSTANT COMMANDS ---------------- From 45ac7f17a7961f51be965160974c6a9870b8aa8b Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 3 Jun 2024 12:27:04 +0200 Subject: [PATCH 42/65] docs/documentation.txt: update URL for Configuration Examples to directly fetch the latest PDF book, not go to the GH release page Signed-off-by: Jim Klimov --- docs/documentation.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/documentation.txt b/docs/documentation.txt index ddc2d1f425..9f22dbb212 100644 --- a/docs/documentation.txt +++ b/docs/documentation.txt @@ -23,7 +23,7 @@ ifndef::website[] - link:https://www.networkupstools.org/ddl/index.html#_supported_devices[Devices Dumps Library (DDL)]: Provides information on how devices are supported; see also link:https://www.networkupstools.org/stable-hcl.html[the HCL] - link:../solaris-usb.html[Notes on NUT monitoring of USB devices in Solaris and related operating systems] endif::website[] -- link:https://github.com/networkupstools/ConfigExamples/releases/latest[NUT Configuration Examples] book maintained by Roger Price +- link:https://github.com/networkupstools/ConfigExamples/releases/latest/downloads/ConfigExamples.pdf[NUT Configuration Examples] book maintained by Roger Price - link:https://github.com/networkupstools/nut/wiki[NUT GitHub Wiki] Developer Documentation From a6053d58e7ce43118845d9c7fbf7648013088416 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 3 Jun 2024 12:27:36 +0200 Subject: [PATCH 43/65] docs/documentation.txt: update links for Roger Price original page and replicas in NUT GitHub org Signed-off-by: Jim Klimov --- docs/documentation.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/documentation.txt b/docs/documentation.txt index 9f22dbb212..d29c4b08ef 100644 --- a/docs/documentation.txt +++ b/docs/documentation.txt @@ -96,7 +96,11 @@ These are general information about UPS, PDU, ATS, PSU and SCD: These are writeups by users of the software. -- link:http://rogerprice.org/NUT.html[NUT Setup with openSUSE] '(Roger Price)' +- link:http://rogerprice.org/NUT[NUT Configuration Examples and helper scripts] + '(Roger Price)' (sources replicated in NUT GitHub organization as + link:https://github.com/networkupstools/ConfigExamples[ConfigExamples], + link:https://github.com/networkupstools/TLS-UPSmon[TLS-UPSmon], + and link:https://github.com/networkupstools/TLS-Shims[TLS-Shims]) - link:http://www.dimat.unina2.it/LCS/MonitoraggioUpsNutUbuntu10-eng.htm[Deploying NUT on an Ubuntu 10.04 cluster] '(Stefano Angelone)' - link:http://blog.shadypixel.com/monitoring-a-ups-with-nut-on-debian-or-ubuntu-linux[Monitoring a UPS with nut on Debian or Ubuntu Linux] '(Avery Fay)' - link:http://linux.developpez.com/cours/upsusb/[Installation et gestion d'un UPS USB en réseau sous linux] '(Olivier Van Hoof, french)' From 4f66f94b849cd08564a685374d5057cbec8116d2 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 3 Jun 2024 15:25:15 +0200 Subject: [PATCH 44/65] docs/documentation.txt: update URL for Configuration Examples to directly fetch the latest PDF book, not go to the GH release page (fix typo) Signed-off-by: Jim Klimov --- docs/documentation.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/documentation.txt b/docs/documentation.txt index d29c4b08ef..27abb1b76b 100644 --- a/docs/documentation.txt +++ b/docs/documentation.txt @@ -23,7 +23,7 @@ ifndef::website[] - link:https://www.networkupstools.org/ddl/index.html#_supported_devices[Devices Dumps Library (DDL)]: Provides information on how devices are supported; see also link:https://www.networkupstools.org/stable-hcl.html[the HCL] - link:../solaris-usb.html[Notes on NUT monitoring of USB devices in Solaris and related operating systems] endif::website[] -- link:https://github.com/networkupstools/ConfigExamples/releases/latest/downloads/ConfigExamples.pdf[NUT Configuration Examples] book maintained by Roger Price +- link:https://github.com/networkupstools/ConfigExamples/releases/latest/download/ConfigExamples.pdf[NUT Configuration Examples] book maintained by Roger Price - link:https://github.com/networkupstools/nut/wiki[NUT GitHub Wiki] Developer Documentation From ed74bc536b6742a6036a0b8674e14ef365fb267d Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 4 Jun 2024 15:42:47 +0200 Subject: [PATCH 45/65] Introduce a `NUT_DEBUG_SYSLOG` environment variable [#2394] Signed-off-by: Jim Klimov --- NEWS.adoc | 6 ++ common/common.c | 200 +++++++++++++++++++++++++++++++----------- conf/nut.conf.sample | 10 +++ docs/man/nut.conf.txt | 17 ++++ include/common.h | 12 +++ 5 files changed, 195 insertions(+), 50 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index ee34f0eb3e..dee697851d 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -116,6 +116,12 @@ https://github.com/networkupstools/nut/milestone/11 exiting (and/or start-up has aborted due to configuration or run-time issues). Warning about "world readable" files clarified. [#2417] + - common code: introduced a `NUT_DEBUG_SYSLOG` environment variable to tweak + activation of syslog message emission (and related detachment of `stderr` + when backgrounding), primarily useful for NIT and perhaps systemd. Most + methods relied on logging bits being set, so this change aims to be minimally + invasive to impact setting of those bits (or not) in the first place. [#2394] + - common code: root-owned daemons now use not the hard-coded `PIDPATH` value set by the `configure` script during build, but can override it with a `NUT_PIDPATH` environment variable in certain use-cases (such as tests). diff --git a/common/common.c b/common/common.c index 72a8f03ef3..d8558cbec6 100644 --- a/common/common.c +++ b/common/common.c @@ -146,11 +146,38 @@ static int xbit_test(int val, int flag) return ((val & flag) == flag); } +int syslog_is_disabled(void) +{ + static int value = -1; + + if (value < 0) { + char *s = getenv("NUT_DEBUG_SYSLOG"); + /* Not set or not disabled by the setting: default is enabled (inversed per method name) */ + value = 0; + if (s) { + if (!strcmp(s, "stderr")) { + value = 1; + } else if (!strcmp(s, "none") || !strcmp(s, "false")) { + value = 2; + } else if (!strcmp(s, "syslog") || !strcmp(s, "true") || !strcmp(s, "default")) { + /* Just reserve a value to quietly do the default */ + value = 0; + } else { + upsdebugx(0, "%s: unknown NUT_DEBUG_SYSLOG='%s' value, ignored (assuming enabled)", + __func__, s); + } + } + } + + return value; +} + /* enable writing upslog_with_errno() and upslogx() type messages to the syslog */ void syslogbit_set(void) { - xbit_set(&upslog_flags, UPSLOG_SYSLOG); + if (!syslog_is_disabled()) + xbit_set(&upslog_flags, UPSLOG_SYSLOG); } /* get the syslog ready for us */ @@ -159,12 +186,15 @@ void open_syslog(const char *progname) #ifndef WIN32 int opt; + if (syslog_is_disabled()) + return; + opt = LOG_PID; /* we need this to grab /dev/log before chroot */ -#ifdef LOG_NDELAY +# ifdef LOG_NDELAY opt |= LOG_NDELAY; -#endif +# endif /* LOG_NDELAY */ openlog(progname, opt, LOG_FACILITY); @@ -197,31 +227,43 @@ void open_syslog(const char *progname) break; default: fatalx(EXIT_FAILURE, "Invalid log level threshold"); -#else +# else case 0: break; default: upslogx(LOG_INFO, "Changing log level threshold not possible"); break; -#endif +# endif /* HAVE_SETLOGMASK && HAVE_DECL_LOG_UPTO */ } #else EventLogName = progname; -#endif +#endif /* WIND32 */ } /* close ttys and become a daemon */ void background(void) { + /* Normally we enable SYSLOG and disable STDERR, + * unless NUT_DEBUG_SYSLOG envvar interferes as + * interpreted in syslog_is_disabled() method: */ + int syslog_disabled = syslog_is_disabled(), + stderr_disabled = (syslog_disabled == 0 || syslog_disabled == 2); + #ifndef WIN32 int pid; if ((pid = fork()) < 0) fatal_with_errno(EXIT_FAILURE, "Unable to enter background"); +#endif - xbit_set(&upslog_flags, UPSLOG_SYSLOG); - xbit_clear(&upslog_flags, UPSLOG_STDERR); + if (!syslog_disabled) + /* not disabled: NUT_DEBUG_SYSLOG is unset or invalid */ + xbit_set(&upslog_flags, UPSLOG_SYSLOG); + if (stderr_disabled) + /* NUT_DEBUG_SYSLOG="none" or unset/invalid */ + xbit_clear(&upslog_flags, UPSLOG_STDERR); +#ifndef WIN32 if (pid != 0) { /* parent */ /* these are typically fds 0-2: */ @@ -234,7 +276,7 @@ void background(void) /* child */ /* make fds 0-2 (typically) point somewhere defined */ -#ifdef HAVE_DUP2 +# ifdef HAVE_DUP2 /* system can close (if needed) and (re-)open a specific FD number */ if (1) { /* scoping */ TYPE_FD devnull = open("/dev/null", O_RDWR); @@ -245,13 +287,15 @@ void background(void) fatal_with_errno(EXIT_FAILURE, "re-open /dev/null as STDIN"); if (dup2(devnull, STDOUT_FILENO) != STDOUT_FILENO) fatal_with_errno(EXIT_FAILURE, "re-open /dev/null as STDOUT"); - if (dup2(devnull, STDERR_FILENO) != STDERR_FILENO) - fatal_with_errno(EXIT_FAILURE, "re-open /dev/null as STDERR"); + if (stderr_disabled) { + if (dup2(devnull, STDERR_FILENO) != STDERR_FILENO) + fatal_with_errno(EXIT_FAILURE, "re-open /dev/null as STDERR"); + } close(devnull); } -#else -# ifdef HAVE_DUP +# else +# ifdef HAVE_DUP /* opportunistically duplicate to the "lowest-available" FD number */ close(STDIN_FILENO); if (open("/dev/null", O_RDWR) != STDIN_FILENO) @@ -261,10 +305,12 @@ void background(void) if (dup(STDIN_FILENO) != STDOUT_FILENO) fatal_with_errno(EXIT_FAILURE, "dup /dev/null as STDOUT"); - close(STDERR_FILENO); - if (dup(STDIN_FILENO) != STDERR_FILENO) - fatal_with_errno(EXIT_FAILURE, "dup /dev/null as STDERR"); -# else + if (stderr_disabled) { + close(STDERR_FILENO); + if (dup(STDIN_FILENO) != STDERR_FILENO) + fatal_with_errno(EXIT_FAILURE, "dup /dev/null as STDERR"); + } +# else close(STDIN_FILENO); if (open("/dev/null", O_RDWR) != STDIN_FILENO) fatal_with_errno(EXIT_FAILURE, "re-open /dev/null as STDIN"); @@ -273,20 +319,18 @@ void background(void) if (open("/dev/null", O_RDWR) != STDOUT_FILENO) fatal_with_errno(EXIT_FAILURE, "re-open /dev/null as STDOUT"); - close(STDERR_FILENO); - if (open("/dev/null", O_RDWR) != STDERR_FILENO) - fatal_with_errno(EXIT_FAILURE, "re-open /dev/null as STDERR"); + if (stderr_disabled) { + close(STDERR_FILENO); + if (open("/dev/null", O_RDWR) != STDERR_FILENO) + fatal_with_errno(EXIT_FAILURE, "re-open /dev/null as STDERR"); + } +# endif # endif -#endif -#ifdef HAVE_SETSID +# ifdef HAVE_SETSID setsid(); /* make a new session to dodge signals */ -#endif - -#else /* WIN32 */ - xbit_set(&upslog_flags, UPSLOG_SYSLOG); - xbit_clear(&upslog_flags, UPSLOG_STDERR); -#endif +# endif +#endif /* not WIN32 */ upslogx(LOG_INFO, "Startup successful"); } @@ -876,9 +920,15 @@ int upsnotify(upsnotify_state_t state, const char *fmt, ...) va_end(va); if ((ret < 0) || (ret >= (int) sizeof(msgbuf))) { - syslog(LOG_WARNING, - "%s (%s:%d): vsnprintf needed more than %" PRIuSIZE " bytes: %d", - __func__, __FILE__, __LINE__, sizeof(msgbuf), ret); + if (syslog_is_disabled()) { + fprintf(stderr, + "%s (%s:%d): vsnprintf needed more than %" PRIuSIZE " bytes: %d", + __func__, __FILE__, __LINE__, sizeof(msgbuf), ret); + } else { + syslog(LOG_WARNING, + "%s (%s:%d): vsnprintf needed more than %" PRIuSIZE " bytes: %d", + __func__, __FILE__, __LINE__, sizeof(msgbuf), ret); + } } else { msglen = strlen(msgbuf); } @@ -914,9 +964,15 @@ int upsnotify(upsnotify_state_t state, const char *fmt, ...) usec_t monots = timespec_load(&monoclock_ts); ret = snprintf(monoclock_str + 1, sizeof(monoclock_str) - 1, "MONOTONIC_USEC=%" PRI_USEC, monots); if ((ret < 0) || (ret >= (int) sizeof(monoclock_str) - 1)) { - syslog(LOG_WARNING, - "%s (%s:%d): snprintf needed more than %" PRIuSIZE " bytes: %d", - __func__, __FILE__, __LINE__, sizeof(monoclock_str), ret); + if (syslog_is_disabled()) { + fprintf(stderr, + "%s (%s:%d): snprintf needed more than %" PRIuSIZE " bytes: %d", + __func__, __FILE__, __LINE__, sizeof(monoclock_str), ret); + } else { + syslog(LOG_WARNING, + "%s (%s:%d): snprintf needed more than %" PRIuSIZE " bytes: %d", + __func__, __FILE__, __LINE__, sizeof(monoclock_str), ret); + } msglen = 0; } else { monoclock_str[0] = '\n'; @@ -933,9 +989,15 @@ int upsnotify(upsnotify_state_t state, const char *fmt, ...) if (msglen) { ret = snprintf(buf, sizeof(buf), "STATUS=%s", msgbuf); if ((ret < 0) || (ret >= (int) sizeof(buf))) { - syslog(LOG_WARNING, - "%s (%s:%d): snprintf needed more than %" PRIuSIZE " bytes: %d", - __func__, __FILE__, __LINE__, sizeof(buf), ret); + if (syslog_is_disabled()) { + fprintf(stderr, + "%s (%s:%d): snprintf needed more than %" PRIuSIZE " bytes: %d", + __func__, __FILE__, __LINE__, sizeof(buf), ret); + } else { + syslog(LOG_WARNING, + "%s (%s:%d): snprintf needed more than %" PRIuSIZE " bytes: %d", + __func__, __FILE__, __LINE__, sizeof(buf), ret); + } msglen = 0; } else { msglen = (size_t)ret; @@ -1128,9 +1190,15 @@ int upsnotify(upsnotify_state_t state, const char *fmt, ...) if ((ret < 0) || (ret >= (int) sizeof(buf))) { /* Refusal to send the watchdog ping is not an error to report */ if ( !(ret == -126 && (state == NOTIFY_STATE_WATCHDOG)) ) { - syslog(LOG_WARNING, - "%s (%s:%d): snprintf needed more than %" PRIuSIZE " bytes: %d", - __func__, __FILE__, __LINE__, sizeof(buf), ret); + if (syslog_is_disabled()) { + fprintf(stderr, + "%s (%s:%d): snprintf needed more than %" PRIuSIZE " bytes: %d", + __func__, __FILE__, __LINE__, sizeof(buf), ret); + } else { + syslog(LOG_WARNING, + "%s (%s:%d): snprintf needed more than %" PRIuSIZE " bytes: %d", + __func__, __FILE__, __LINE__, sizeof(buf), ret); + } } ret = -1; } else { @@ -1365,10 +1433,17 @@ static void vupslog(int priority, const char *fmt, va_list va, int use_strerror) /* Arbitrary limit, gotta stop somewhere */ if (bufsize > LARGEBUF * 64) { vupslog_too_long: - syslog(LOG_WARNING, "vupslog: vsnprintf needed " - "more than %" PRIuSIZE " bytes; logged " - "output can be truncated", - bufsize); + if (syslog_is_disabled()) { + fprintf(stderr, "vupslog: vsnprintf needed " + "more than %" PRIuSIZE " bytes; logged " + "output can be truncated", + bufsize); + } else { + syslog(LOG_WARNING, "vupslog: vsnprintf needed " + "more than %" PRIuSIZE " bytes; logged " + "output can be truncated", + bufsize); + } break; } } while(1); @@ -1672,8 +1747,13 @@ void s_upsdebug_with_errno(int level, const char *fmt, ...) ret = snprintf(fmt2, sizeof(fmt2), "[D%d] %s", level, fmt); } if ((ret < 0) || (ret >= (int) sizeof(fmt2))) { - syslog(LOG_WARNING, "upsdebug_with_errno: snprintf needed more than %d bytes", - LARGEBUF); + if (syslog_is_disabled()) { + fprintf(stderr, "upsdebug_with_errno: snprintf needed more than %d bytes", + LARGEBUF); + } else { + syslog(LOG_WARNING, "upsdebug_with_errno: snprintf needed more than %d bytes", + LARGEBUF); + } } else { fmt = (const char *)fmt2; } @@ -1722,8 +1802,13 @@ void s_upsdebugx(int level, const char *fmt, ...) } if ((ret < 0) || (ret >= (int) sizeof(fmt2))) { - syslog(LOG_WARNING, "upsdebugx: snprintf needed more than %d bytes", - LARGEBUF); + if (syslog_is_disabled()) { + fprintf(stderr, "upsdebugx: snprintf needed more than %d bytes", + LARGEBUF); + } else { + syslog(LOG_WARNING, "upsdebugx: snprintf needed more than %d bytes", + LARGEBUF); + } } else { fmt = (const char *)fmt2; } @@ -1850,10 +1935,25 @@ void s_upsdebug_ascii(int level, const char *msg, const void *buf, size_t len) static void vfatal(const char *fmt, va_list va, int use_strerror) { + /* Normally we enable SYSLOG and disable STDERR, + * unless NUT_DEBUG_SYSLOG envvar interferes as + * interpreted in syslog_is_disabled() method: */ + int syslog_disabled = syslog_is_disabled(), + stderr_disabled = (syslog_disabled == 0 || syslog_disabled == 2); + if (xbit_test(upslog_flags, UPSLOG_STDERR_ON_FATAL)) xbit_set(&upslog_flags, UPSLOG_STDERR); - if (xbit_test(upslog_flags, UPSLOG_SYSLOG_ON_FATAL)) - xbit_set(&upslog_flags, UPSLOG_SYSLOG); + if (xbit_test(upslog_flags, UPSLOG_SYSLOG_ON_FATAL)) { + if (syslog_disabled) { + /* FIXME: Corner case... env asked for stderr + * instead of syslog - should we care about + * UPSLOG_STDERR_ON_FATAL being not set? */ + if (!stderr_disabled) + xbit_set(&upslog_flags, UPSLOG_STDERR); + } else { + xbit_set(&upslog_flags, UPSLOG_SYSLOG); + } + } #ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL #pragma GCC diagnostic push diff --git a/conf/nut.conf.sample b/conf/nut.conf.sample index 1c38f86892..3df4fcd4f8 100644 --- a/conf/nut.conf.sample +++ b/conf/nut.conf.sample @@ -91,3 +91,13 @@ MODE=none # verbosity passed to NUT daemons. As an environment variable, its priority sits # between that of 'DEBUG_MIN' setting of a driver and the command-line options. #NUT_DEBUG_LEVEL=0 + +# Normally NUT can (attempt to) use the syslog or Event Log (WIN32), but the +# environment variable 'NUT_DEBUG_SYSLOG' allows to bypass it, and perhaps keep +# the daemons logging to stderr (useful e.g. in NUT Integration Test suite to +# not pollute the OS logs, or in systemd where stderr and syslog both go into +# the same journal). Recognized values: +# `stderr` Disabled and background() keeps stderr attached +# `none` Disabled and background() detaches stderr as usual +# `default`/unset/other Not disabled +#NUT_DEBUG_SYSLOG=stderr diff --git a/docs/man/nut.conf.txt b/docs/man/nut.conf.txt index ac3b60654e..f61a756f77 100644 --- a/docs/man/nut.conf.txt +++ b/docs/man/nut.conf.txt @@ -117,6 +117,23 @@ Optional, defaults to `0`. This setting controls the default debugging message verbosity passed to NUT daemons. As an environment variable, its priority sits between that of 'DEBUG_MIN' setting of a driver and the command-line options. +*NUT_DEBUG_SYSLOG*:: +Optional, unset by default. +Normally NUT can (attempt to) use the syslog or Event Log (WIN32), but the +environment variable 'NUT_DEBUG_SYSLOG' allows to bypass it, and perhaps keep +the daemons logging to stderr (useful e.g. in NUT Integration Test suite to +not pollute the OS logs, or in systemd where stderr and syslog both go into +the same journal). Recognized values: ++ +[options="header"] +|=========================================================================== +| Value | Description +| `stderr` | Disabled and background() keeps stderr attached +| `none` | Disabled and background() detaches stderr as usual +| `default` | Not disabled +| unset/other | Not disabled +|=========================================================================== + EXAMPLE ------- diff --git a/include/common.h b/include/common.h index 32e0ee2041..f1d7c5c0a3 100644 --- a/include/common.h +++ b/include/common.h @@ -199,6 +199,18 @@ extern const char *UPS_VERSION; /** @brief Default timeout (in seconds) for retrieving the result of a `TRACKING`-enabled operation (e.g. `INSTCMD`, `SET VAR`). */ #define DEFAULT_TRACKING_TIMEOUT 10 +/* Normally we can (attempt to) use the syslog or Event Log (WIN32), + * but environment variable NUT_DEBUG_SYSLOG allows to bypass it, and + * perhaps keep daemons logging to stderr (e.g. in NUT Integration Test + * suite to not pollute the OS logs, or in systemd where stderr and + * syslog both go into the same journal). Returns: + * 0 Not disabled (NUT_DEBUG_SYSLOG not set to a value below; unset or "default" + * values are handled quietly, but others emit a warning) + * 1 Disabled and background() keeps stderr attached (NUT_DEBUG_SYSLOG="stderr") + * 2 Disabled and background() detaches stderr as usual (NUT_DEBUG_SYSLOG="none") + */ +int syslog_is_disabled(void); + /* get the syslog ready for us */ void open_syslog(const char *progname); From 60f1828395ce0b3a7b2ea066be9d12110db94b1b Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Tue, 4 Jun 2024 15:48:53 +0200 Subject: [PATCH 46/65] tests/NIT/nit.sh: make use of NUT_DEBUG_SYSLOG tweak setting Signed-off-by: Jim Klimov --- tests/NIT/nit.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/NIT/nit.sh b/tests/NIT/nit.sh index 2bb7742c30..41e53a9564 100755 --- a/tests/NIT/nit.sh +++ b/tests/NIT/nit.sh @@ -37,6 +37,10 @@ export NUT_QUIET_INIT_SSL NUT_QUIET_INIT_UPSNOTIFY="true" export NUT_QUIET_INIT_UPSNOTIFY +# Avoid noise in syslog and OS console +NUT_DEBUG_SYSLOG="stderr" +export NUT_DEBUG_SYSLOG + NUT_DEBUG_PID="true" export NUT_DEBUG_PID From fd68a5612a0f0f075d9e7bc1a2b8177f5d18f6bd Mon Sep 17 00:00:00 2001 From: Andriy Gapon Date: Wed, 5 Jun 2024 11:26:03 +0300 Subject: [PATCH 47/65] clients/upsmon: restore handling of comm failure while on battery Commit 2647f026f7f0 from #2108 updated is_ups_critical() such that a communication failure (commstate == 0) while on battery (linestate == 0) would be treated as immediately entering critical state. There was no way to disable that behavior or add any grace period. As such, that change made DEADTIME parameter ineffective as lastpoll being stuck should be equivalent to commstate == 0 because any lastpoll update sets commstate to 1. This change removes special handling of that condition from is_ups_critical() and reverts to the prior behavior where recalc() looks at lastpoll and deadtime to set LB flag after the configured communication timeout. Signed-off-by: Andriy Gapon --- clients/upsmon.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/clients/upsmon.c b/clients/upsmon.c index 1e64c564a7..52d4aac22b 100644 --- a/clients/upsmon.c +++ b/clients/upsmon.c @@ -1111,14 +1111,6 @@ static int is_ups_critical(utype_t *ups) ups->sys); return 1; } - - if (ups->linestate == 0) { - upslogx(LOG_WARNING, - "UPS [%s] was last known to be not fully online " - "and currently is not communicating, assuming dead", - ups->sys); - return 1; - } } /* administratively OFF (long enough, see OFFDURATION) */ From a9f734f9bfd1066d6c15a7023d18d40d1019c1e9 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Thu, 6 Jun 2024 18:09:16 +0200 Subject: [PATCH 48/65] data/driver.list.in: shorten HCL title for Cyber Energy devices Asked via mailing list to not deference the USB VID:PID in the HCL title Signed-off-by: Jim Klimov --- data/driver.list.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data/driver.list.in b/data/driver.list.in index 2480f23247..9e0cdf9f42 100644 --- a/data/driver.list.in +++ b/data/driver.list.in @@ -228,7 +228,7 @@ "Crown" "ups" "2" "CMU-SP1200IEC" "USB" "nutdrv_qx port=auto vendorid=0001 productid=0000 protocol=hunnox langid_fix=0x0409 novendor noscanlangid" # https://github.com/networkupstools/nut/pull/638 caveats at https://github.com/networkupstools/nut/issues/1014 -"Cyber Energy" "ups" "3" "Models with USB ID 0483:A430" "USB" "usbhid-ups" # https://alioth-lists.debian.net/pipermail/nut-upsdev/2024-February/007966.html +"Cyber Energy" "ups" "3" "Models with USB" "USB" "usbhid-ups" # https://alioth-lists.debian.net/pipermail/nut-upsdev/2024-February/007966.html https://alioth-lists.debian.net/pipermail/nut-upsdev/2024-June/008002.html "Cyber Power Systems" "ups" "1" "550SL" "" "genericups upstype=7" "Cyber Power Systems" "ups" "1" "725SL" "" "genericups upstype=7" From 0bea73be264067d8d3d353b48e374bc9832dcc93 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 7 Jun 2024 20:15:41 +0200 Subject: [PATCH 49/65] NEWS.adoc: rearrange entries Signed-off-by: Jim Klimov --- NEWS.adoc | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index dee697851d..c75e9c0908 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -52,7 +52,7 @@ https://github.com/networkupstools/nut/milestone/11 broke detection of (in-)ability to find and query "Old NUT" servers via `libupsclient.so` (internal flag got always enabled). [#2246] - - drivers, upsd, upsmon: reduce "scary noise" about failure to `fopen()` + - drivers, `upsd`, `upsmon`: reduce "scary noise" about failure to `fopen()` the PID file (which most of the time means that no previous instance of the daemon was running to potentially conflict with), especially useless since in recent NUT releases the verdicts from `sendsignal*()` methods @@ -87,6 +87,10 @@ https://github.com/networkupstools/nut/milestone/11 string values reported by devices via USB are welcome), so these devices would now report `battery.voltage` and `battery.voltage.nominal`. [#2380] + - bicker_ser: added new driver for Bicker 12/24Vdc UPS via RS-232 serial + communication protocol, which supports any UPS shipped with the PSZ-1053 + extension module. [PR #2448] + - USB drivers could log `(nut_)libusb_get_string: Success` due to either reading an empty string or getting a success code `0` from libusb. This difference should now be better logged, and not into syslog. [#2399] @@ -116,16 +120,21 @@ https://github.com/networkupstools/nut/milestone/11 exiting (and/or start-up has aborted due to configuration or run-time issues). Warning about "world readable" files clarified. [#2417] - - common code: introduced a `NUT_DEBUG_SYSLOG` environment variable to tweak - activation of syslog message emission (and related detachment of `stderr` - when backgrounding), primarily useful for NIT and perhaps systemd. Most - methods relied on logging bits being set, so this change aims to be minimally - invasive to impact setting of those bits (or not) in the first place. [#2394] + - common code: + * introduced a `NUT_DEBUG_SYSLOG` environment variable to tweak activation + of syslog message emission (and related detachment of `stderr` when + backgrounding), primarily useful for NIT and perhaps systemd. Most + methods relied on logging bits being set, so this change aims to be + minimally invasive to impact setting of those bits (or not) in the + first place. [#2394] + * `root`-owned daemons now use not the hard-coded `PIDPATH` value set + by the `configure` script during build, but can override it with a + `NUT_PIDPATH` environment variable in certain use-cases (such as + tests). [#2407] - - common code: root-owned daemons now use not the hard-coded `PIDPATH` value - set by the `configure` script during build, but can override it with a - `NUT_PIDPATH` environment variable in certain use-cases (such as tests). - [#2407] + - various recipe, documentation and source files were revised to address + respective warnings issued by the new generations of analysis tools. + [#823, #2437, link:https://github.com/networkupstools/nut-website/issues/52[nut-website issue #52]] - revised `nut.exe` (the NUT for Windows wrapper for all-in-one service) to be more helpful with command-line use (report that it failed to start @@ -148,14 +157,6 @@ https://github.com/networkupstools/nut/milestone/11 of more complex configurations (e.g. some line patterns that involve too many double-quote characters) which are valid for NUT proper. [#657] - - various recipe, documentation and source files were revised to address - respective warnings issued by the new generations of analysis tools. - [#823, #2437, link:https://github.com/networkupstools/nut-website/issues/52[nut-website issue #52]] - - - bicker_ser: added new driver for Bicker 12/24Vdc UPS via RS-232 serial - communication protocol, which supports any UPS shipped with the PSZ-1053 - extension module. [PR #2448] - Release notes for NUT 2.8.2 - what's new since 2.8.1 ---------------------------------------------------- From 7b51a591e63dff37420917bf451f0c07940a4ab8 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 7 Jun 2024 20:19:01 +0200 Subject: [PATCH 50/65] Introduce getprocname() and checkprocname() for suspicious PID values, and helper parseprogbasename() [#2463] Signed-off-by: Jim Klimov --- NEWS.adoc | 5 + common/common.c | 360 +++++++++++++++++++++++++++++++++++++++++++++++ configure.ac | 6 + include/common.h | 31 ++++ 4 files changed, 402 insertions(+) diff --git a/NEWS.adoc b/NEWS.adoc index c75e9c0908..e1fcf94194 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -131,6 +131,11 @@ https://github.com/networkupstools/nut/milestone/11 by the `configure` script during build, but can override it with a `NUT_PIDPATH` environment variable in certain use-cases (such as tests). [#2407] + * introduced a check for daemons working with PID files to double-check + that if they can resolve the program name of a running process with + this identifier, that such name matches the current program (avoid + failures to start NUT daemons if PID files are on persistent storage, + and some unrelated program got that PID after a reboot). [#2463] - various recipe, documentation and source files were revised to address respective warnings issued by the new generations of analysis tools. diff --git a/common/common.c b/common/common.c index d8558cbec6..75644f4a21 100644 --- a/common/common.c +++ b/common/common.c @@ -32,6 +32,10 @@ #include #endif +#ifdef HAVE_UNISTD_H +# include /* readlink */ +#endif + #include #if !HAVE_DECL_REALPATH # include @@ -433,6 +437,15 @@ void become_user(struct passwd *pw) #endif } +#if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP_BESIDEFUNC) && (!defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP_INSIDEFUNC) && ( (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS_BESIDEFUNC) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE_BESIDEFUNC) ) +# pragma GCC diagnostic push +#endif +#if (!defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP_INSIDEFUNC) && (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS_BESIDEFUNC) +# pragma GCC diagnostic ignored "-Wtype-limits" +#endif +#if (!defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP_INSIDEFUNC) && (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE_BESIDEFUNC) +# pragma GCC diagnostic ignored "-Wtautological-constant-out-of-range-compare" +#endif /* drop down into a directory and throw away pointers to the old path */ void chroot_start(const char *path) { @@ -455,6 +468,353 @@ void chroot_start(const char *path) #endif } +char * getprocname(pid_t pid) +{ + /* Try to identify process (program) name for the given PID, + * return NULL if we can not for any reason (does not run, + * no rights, do not know how to get it on current OS, etc.) + * If the returned value is not NULL, caller should free() it. + * Some implementation pieces borrowed from + * https://man7.org/linux/man-pages/man2/readlink.2.html and + * https://github.com/openbsd/src/blob/master/bin/ps/ps.c + * NOTE: Very much platform-dependent! + */ + char *procname = NULL; + size_t procnamelen = 0; +#ifdef UNIX_PATH_MAX + char pathname[UNIX_PATH_MAX]; +#else + char pathname[PATH_MAX]; +#endif + struct stat st; + +#ifdef WIN32 + /* Try Windows API calls, then fall through to /proc emulation in MinGW/MSYS2 + * https://stackoverflow.com/questions/1591342/c-how-to-determine-if-a-windows-process-is-running + * http://cppip.blogspot.com/2013/01/check-if-process-is-running.html + */ +#endif + + if (stat("/proc", &st) == 0 && ((st.st_mode & S_IFMT) == S_IFDIR)) { + upsdebugx(3, "%s: /proc is an accessible directory, investigating", __func__); +#if (defined HAVE_READLINK) && HAVE_READLINK + /* Linux-like */ + if (snprintf(pathname, sizeof(pathname), "/proc/%" PRIuMAX "/exe", (uintmax_t)pid) < 10) { + upsdebug_with_errno(3, "%s: failed to snprintf pathname: Linux-like", __func__); + goto finish; + } + + if (lstat(pathname, &st) == 0) { + goto process_stat_symlink; + } + + /* FreeBSD-like */ + if (snprintf(pathname, sizeof(pathname), "/proc/%" PRIuMAX "/file", (uintmax_t)pid) < 10) { + upsdebug_with_errno(3, "%s: failed to snprintf pathname: FreeBSD-like", __func__); + goto finish; + } + + if (lstat(pathname, &st) == 0) { + goto process_stat_symlink; + } + + goto process_parse_file; + +process_stat_symlink: + upsdebugx(3, "%s: located symlink for PID %" PRIuMAX " at: %s", + __func__, (uintmax_t)pid, pathname); + /* Some magic symlinks under (for example) /proc and /sys + * report 'st_size' as zero. In that case, take PATH_MAX + * or equivalent as a "good enough" estimate. */ + if (st.st_size) { + /* Add one for ending '\0' */ + procnamelen = st.st_size + 1; + } else { + procnamelen = sizeof(pathname); + } + + /* Not xcalloc() here, not too fatal if we fail */ + procname = (char*)calloc(procnamelen, sizeof(char)); + if (procname) { + int nbytes = readlink(pathname, procname, procnamelen); + if (nbytes < 0) { + upsdebug_with_errno(1, "%s: failed to readlink() from %s", + __func__, pathname); + free(procname); + procname = NULL; + goto process_parse_file; + } +#if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP) && ( (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE) ) +# pragma GCC diagnostic push +#endif +#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS +# pragma GCC diagnostic ignored "-Wtype-limits" +#endif +#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE +# pragma GCC diagnostic ignored "-Wtautological-constant-out-of-range-compare" +#endif + if ((unsigned int)nbytes > SIZE_MAX || procnamelen <= (size_t)nbytes) { +#if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP) && ( (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE) ) +# pragma GCC diagnostic pop +#endif + upsdebugx(1, "%s: failed to readlink() from %s: may have been truncated", + __func__, pathname); + free(procname); + procname = NULL; + goto process_parse_file; + } + + /* Got a useful reply */ + procname[nbytes] = '\0'; + goto finish; + } else { + upsdebug_with_errno(3, "%s: failed to allocate the procname string " + "to readlink() size %" PRIuSIZE, __func__, procnamelen); + goto finish; + } +#else + upsdebugx(3, "%s: this platform does not have readlink(), skipping this method", __func__); +#endif /* HAVE_READLINK */ + +process_parse_file: + upsdebugx(3, "%s: try to parse some files under /proc", __func__); + + /* TODO: Check /proc/NNN/cmdline (may start with a '-' to ignore, + * for a title string like "-bash" where programs edit their argv[0] */ + + /* TODO: Check /proc/NNN/stat (second token, in parentheses, may be truncated) + * see e.g. https://stackoverflow.com/a/12675103/4715872 */ + + /* TODO: Solaris/illumos: parse binary structure at /proc/NNN/psinfo */ + } else { + upsdebug_with_errno(3, "%s: /proc is not a directory or not accessible", __func__); + /* TODO: OpenBSD: no /proc; use API call, see ps.c link above */ + } + + goto finish; + +finish: + if (procname) { + if (strlen(procname) == 0) { + free(procname); + procname = NULL; + } else { + upsdebugx(1, "%s: determined process name for PID %" PRIuMAX ": %s", + __func__, (uintmax_t)pid, procname); + } + } + + if (!procname) { + upsdebugx(1, "%s: failed to determine process name for PID %" PRIuMAX, + __func__, (uintmax_t)pid); + } + + return procname; +} +#if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP_BESIDEFUNC) && (!defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP_INSIDEFUNC) && ( (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS_BESIDEFUNC) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE_BESIDEFUNC) ) +# pragma GCC diagnostic pop +#endif + +size_t parseprogbasename(char *buf, size_t buflen, const char *progname, size_t *pprogbasenamelen, size_t *pprogbasenamedot) +{ + size_t i, + progbasenamelen = 0, + progbasenamedot = 0; + + if (pprogbasenamelen) + *pprogbasenamelen = 0; + + if (pprogbasenamedot) + *pprogbasenamedot = 0; + + if (!buf || !progname || !buflen || progname[0] == '\0') + return 0; + + for (i = 0; i < buflen && progname[i] != '\0'; i++) { + if (progname[i] == '/' +#ifdef WIN32 + || progname[i] == '\\' +#endif + ) { + progbasenamelen = 0; + progbasenamedot = 0; + continue; + } + + if (progname[i] == '.') + progbasenamedot = progbasenamelen; + + buf[progbasenamelen++] = progname[i]; + } + buf[progbasenamelen] = '\0'; + buf[buflen - 1] = '\0'; + + if (pprogbasenamelen) + *pprogbasenamelen = progbasenamelen; + + if (pprogbasenamedot) + *pprogbasenamedot = progbasenamedot; + + return progbasenamelen; +} + +int checkprocname(pid_t pid, const char *progname) +{ + /* If we can determine the binary path name of the specified "pid", + * check if it matches the assumed name of the current program. + * Returns: + * -2 Could not parse a program name (ok to proceed, + * risky - but matches legacy behavior) + * -1 Could not identify a program name (ok to proceed, + * risky - but matches legacy behavior) + * 0 Process name identified, does not seem to match + * 1+ Process name identified, and seems to match with + * varying precision + * Generally speaking, if (checkprocname(...)) then ok to proceed + */ + char *procname = getprocname(pid); + int ret = -127; + size_t procbasenamelen = 0, progbasenamelen = 0; + /* Track where the last dot is in the basename; 0 means none */ + size_t procbasenamedot = 0, progbasenamedot = 0; +#ifdef UNIX_PATH_MAX + char procbasename[UNIX_PATH_MAX], progbasename[UNIX_PATH_MAX]; +#else + char procbasename[PATH_MAX], progbasename[PATH_MAX]; +#endif + + if (!procname || !progname) { + ret = -1; + goto finish; + } + + /* First quickly try for an exact hit (possible dir names included) */ + if (!strcmp(procname, progname)) { + ret = 1; + goto finish; + } + + /* Parse the basenames apart */ + if (!parseprogbasename(progbasename, sizeof(progbasename), progname, &progbasenamelen, &progbasenamedot) + || !parseprogbasename(procbasename, sizeof(procbasename), procname, &procbasenamelen, &procbasenamedot) + ) { + ret = -2; + goto finish; + } + + /* First quickly try for an exact hit of base names */ + if (progbasenamelen == procbasenamelen && progbasenamedot == procbasenamedot && !strcmp(procname, progname)) { + ret = 2; + goto finish; + } + + /* Check for executable program filename extensions and/or case-insensitive + * matching on some platforms */ +#ifdef WIN32 + if (!strcasecmp(procname, progname)) { + ret = 3; + goto finish; + } + + if (!strcasecmp(procbasename, progbasename)) { + ret = 4; + goto finish; + } + + if (progbasenamedot == procbasenamedot || !progbasenamedot || !procbasenamedot) { + /* Same base name before ext, maybe different casing or absence of ext in one of them */ + size_t dot = progbasenamedot ? progbasenamedot : procbasenamedot; + + if (!strncasecmp(progbasename, procbasename, dot - 1) && + ( (progbasenamedot && !strcasecmp(progbasename + progbasenamedot, ".exe")) + || (procbasenamedot && !strcasecmp(procbasename + procbasenamedot, ".exe")) ) + ) { + ret = 5; + goto finish; + } + } +#endif + + /* Nothing above has matched */ + ret = 0; + +finish: + switch (ret) { + case 5: + upsdebugx(1, + "%s: exact case-insensitive base name hit with " + "an executable program extension involved for " + "PID %" PRIuMAX " of '%s'=>'%s' and checked " + "'%s'=>'%s'", + __func__, (uintmax_t)pid, + procname, procbasename, + progname, progbasename); + break; + + case 4: + upsdebugx(1, + "%s: case-insensitive base name hit for PID %" + PRIuMAX " of '%s'=>'%s' and checked '%s'=>'%s'", + __func__, (uintmax_t)pid, + procname, procbasename, + progname, progbasename); + break; + + case 3: + upsdebugx(1, + "%s: case-insensitive full name hit for PID %" + PRIuMAX " of '%s' and checked '%s'", + __func__, (uintmax_t)pid, procname, progname); + break; + + case 2: + upsdebugx(1, + "%s: case-sensitive base name hit for PID %" + PRIuMAX " of '%s'=>'%s' and checked '%s'=>'%s'", + __func__, (uintmax_t)pid, + procname, procbasename, + progname, progbasename); + break; + + case 1: + upsdebugx(1, + "%s: exact case-sensitive full name hit for PID %" + PRIuMAX " of '%s' and checked '%s'", + __func__, (uintmax_t)pid, procname, progname); + break; + + case 0: + upsdebugx(1, + "%s: did not find any match of program names " + "for PID %" PRIuMAX " of '%s'=>'%s' and checked " + "'%s'=>'%s'", + __func__, (uintmax_t)pid, + procname, procbasename, + progname, progbasename); + break; + + case -1: + /* failed to getprocname(), logged above in it */ + break; + + case -2: + upsdebugx(1, + "%s: failed to parse base names of the programs", + __func__); + break; + + default: + upsdebugx(1, + "%s: unexpected result looking for process name " + "of PID %" PRIuMAX ": %d", + __func__, (uintmax_t)pid, ret); + ret = -127; + break; + } + + return ret; +} + #ifdef WIN32 /* In WIN32 all non binaries files (namely configuration and PID files) are retrieved relative to the path of the binary itself. diff --git a/configure.ac b/configure.ac index d7e0b05627..badbc92383 100644 --- a/configure.ac +++ b/configure.ac @@ -1047,6 +1047,12 @@ AC_CHECK_HEADER([sys/select.h], [AC_DEFINE([HAVE_SYS_SELECT_H], [1], [Define to 1 if you have .])]) +AC_CHECK_HEADER([unistd.h], + [AC_DEFINE([HAVE_UNISTD_H], [1], + [Define to 1 if you have .])]) + +AC_CHECK_FUNCS(readlink) + AC_CACHE_CHECK([for suseconds_t], [ac_cv_type_suseconds_t], [AC_COMPILE_IFELSE( diff --git a/include/common.h b/include/common.h index f1d7c5c0a3..83e01690a4 100644 --- a/include/common.h +++ b/include/common.h @@ -226,6 +226,37 @@ void become_user(struct passwd *pw); /* drop down into a directory and throw away pointers to the old path */ void chroot_start(const char *path); +/* Try to identify process (program) name for the given PID, + * return NULL if we can not for any reason (does not run, + * no rights, do not know how to get it on current OS, etc.) + * If the returned value is not NULL, caller should free() it. + * Some implementation pieces borrowed from + * https://man7.org/linux/man-pages/man2/readlink.2.html and + * https://github.com/openbsd/src/blob/master/bin/ps/ps.c + * NOTE: Very much platform-dependent! */ +char * getprocname(pid_t pid); + +/* Determine the base name of specified progname (may be full path) + * and the location of the last "." dot character in it for extension + * (caller's len and dot populated only if pointers are not NULL). + */ +size_t parseprogbasename(char *buf, size_t buflen, const char *progname, size_t *pprogbasenamelen, size_t *pprogbasenamedot); + +/* If we can determine the binary path name of the specified "pid", + * check if it matches the assumed name of the current program. + * Returns: + * -2 Could not parse a program name (ok to proceed, + * risky - but matches legacy behavior) + * -1 Could not identify a program name (ok to proceed, + * risky - but matches legacy behavior) + * 0 Process name identified, does not seem to match + * 1+ Process name identified, and seems to match with + * varying precision + * Generally speaking, if (checkprocname(...)) then ok to proceed + */ +int checkprocname(pid_t pid, const char *progname); + + /* write a pid file - is a full pathname *or* just the program name */ void writepid(const char *name); From eaf783f51596767e57d58f2e483dcb623ba169c6 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 8 Jun 2024 15:19:03 +0200 Subject: [PATCH 51/65] common/common.c: sendsignalpid(): comment about not-signaling PID<2 Signed-off-by: Jim Klimov --- common/common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/common.c b/common/common.c index 75644f4a21..8cc38f402b 100644 --- a/common/common.c +++ b/common/common.c @@ -879,6 +879,8 @@ int sendsignalpid(pid_t pid, int sig) #ifndef WIN32 int ret; + /* TOTHINK: What about containers where a NUT daemon *is* the only process + * and is the PID=1 of the container (recycle if dead)? */ if (pid < 2 || pid > get_max_pid_t()) { if (nut_debug_level > 0 || nut_sendsignal_debug_level > 0) upslogx(LOG_NOTICE, From 879fa483404c58aa867228389156ab95b526ce9e Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 8 Jun 2024 15:33:57 +0200 Subject: [PATCH 52/65] Pass "progname" through sendsignal*() methods to verify that we sendsignal*() via old PID only to same progname [#2463] Internal API change for common.c/h Signed-off-by: Jim Klimov --- UPGRADING.adoc | 6 ++++++ clients/upsmon.c | 2 +- common/common.c | 30 ++++++++++++++++++++++-------- docs/nut.dict | 5 ++++- drivers/main.c | 10 +++++----- drivers/upsdrvctl.c | 28 ++++++++++++++-------------- include/common.h | 13 ++++++++----- server/upsd.c | 4 ++-- 8 files changed, 62 insertions(+), 36 deletions(-) diff --git a/UPGRADING.adoc b/UPGRADING.adoc index 19a95b78e0..3ffd2d28a6 100644 --- a/UPGRADING.adoc +++ b/UPGRADING.adoc @@ -33,6 +33,12 @@ Changes from 2.8.2 to 2.8.3 pages which are delivered automatically. Packaging recipes can likely be simplified now. [#2445] +- Internal API change for `sendsignalpid()` and `sendsignalfn()` methods, + which can impact NUT forks which build using `libcommon.la` and similar + libraries. Added new last argument with `const char *progname` (may be + `NULL`) to check that we are signalling an expected program name when we + work with a PID. [#2463] + Changes from 2.8.1 to 2.8.2 --------------------------- diff --git a/clients/upsmon.c b/clients/upsmon.c index 1e64c564a7..28c7c1d1f5 100644 --- a/clients/upsmon.c +++ b/clients/upsmon.c @@ -3018,7 +3018,7 @@ int main(int argc, char *argv[]) if (oldpid < 0) { cmdret = sendsignal(prog, cmd); } else { - cmdret = sendsignalpid(oldpid, cmd); + cmdret = sendsignalpid(oldpid, cmd, prog); } #else /* WIN32 */ diff --git a/common/common.c b/common/common.c index 8cc38f402b..00c05de807 100644 --- a/common/common.c +++ b/common/common.c @@ -574,6 +574,7 @@ char * getprocname(pid_t pid) } #else upsdebugx(3, "%s: this platform does not have readlink(), skipping this method", __func__); + goto process_parse_file; #endif /* HAVE_READLINK */ process_parse_file: @@ -595,7 +596,8 @@ char * getprocname(pid_t pid) finish: if (procname) { - if (strlen(procname) == 0) { + procnamelen = strlen(procname); + if (procnamelen == 0) { free(procname); procname = NULL; } else { @@ -874,7 +876,7 @@ void writepid(const char *name) /* send sig to pid, returns -1 for error, or * zero for a successfully sent signal */ -int sendsignalpid(pid_t pid, int sig) +int sendsignalpid(pid_t pid, int sig, const char *progname) { #ifndef WIN32 int ret; @@ -889,7 +891,16 @@ int sendsignalpid(pid_t pid, int sig) return -1; } - /* see if this is going to work first - does the process exist? */ + if (progname && !checkprocname(pid, progname)) { + if (nut_debug_level > 0 || nut_sendsignal_debug_level > 1) + upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX + " which exists but is not of program '%s'", + (uintmax_t)pid, progname); + return -1; + } + + /* see if this is going to work first - does the process exist, + * and do we have permissions to signal it? */ ret = kill(pid, 0); if (ret < 0) { @@ -913,6 +924,7 @@ int sendsignalpid(pid_t pid, int sig) #else NUT_UNUSED_VARIABLE(pid); NUT_UNUSED_VARIABLE(sig); + NUT_UNUSED_VARIABLE(progname); upslogx(LOG_ERR, "%s: not implemented for Win32 and " "should not have been called directly!", @@ -953,7 +965,7 @@ pid_t parsepid(const char *buf) * zero for a successfully sent signal */ #ifndef WIN32 -int sendsignalfn(const char *pidfn, int sig) +int sendsignalfn(const char *pidfn, int sig, const char *progname) { char buf[SMALLBUF]; FILE *pidf; @@ -989,7 +1001,7 @@ int sendsignalfn(const char *pidfn, int sig) if (pid >= 0) { /* this method actively reports errors, if any */ - ret = sendsignalpid(pid, sig); + ret = sendsignalpid(pid, sig, progname); } fclose(pidf); @@ -998,9 +1010,10 @@ int sendsignalfn(const char *pidfn, int sig) #else /* => WIN32 */ -int sendsignalfn(const char *pidfn, const char * sig) +int sendsignalfn(const char *pidfn, const char * sig, const char *progname_ignored) { BOOL ret; + NUT_UNUSED_VARIABLE(progname_ignored); ret = send_to_named_pipe(pidfn, sig); @@ -1069,12 +1082,13 @@ int sendsignal(const char *progname, int sig) snprintf(fn, sizeof(fn), "%s/%s.pid", rootpidpath(), progname); - return sendsignalfn(fn, sig); + return sendsignalfn(fn, sig, progname); } #else int sendsignal(const char *progname, const char * sig) { - return sendsignalfn(progname, sig); + /* progname is used as the pipe name for WIN32 */ + return sendsignalfn(progname, sig, NULL); } #endif diff --git a/docs/nut.dict b/docs/nut.dict index f6feb857ec..61bb7efe84 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3165 utf-8 +personal_ws-1.1 en 3168 utf-8 AAC AAS ABI @@ -2569,6 +2569,7 @@ probu proc productid prog +progname prtconf ps psu @@ -2702,6 +2703,8 @@ sendback sendline sendmail sendsignal +sendsignalfn +sendsignalpid sequentialized ser seria diff --git a/drivers/main.c b/drivers/main.c index 89950d1ed7..67d853fd81 100644 --- a/drivers/main.c +++ b/drivers/main.c @@ -2223,9 +2223,9 @@ int main(int argc, char **argv) int cmdret = -1; /* Send a signal to older copy of the driver, if any */ if (oldpid < 0) { - cmdret = sendsignalfn(pidfnbuf, cmd); + cmdret = sendsignalfn(pidfnbuf, cmd, progname); } else { - cmdret = sendsignalpid(oldpid, cmd); + cmdret = sendsignalpid(oldpid, cmd, progname); } switch (cmdret) { @@ -2321,7 +2321,7 @@ int main(int argc, char **argv) upslogx(LOG_WARNING, "Duplicate driver instance detected (PID file %s exists)! Terminating other driver!", pidfnbuf); - if ((sigret = sendsignalfn(pidfnbuf, SIGTERM) != 0)) { + if ((sigret = sendsignalfn(pidfnbuf, SIGTERM, progname) != 0)) { upsdebugx(1, "Can't send signal to PID, assume invalid PID file %s; " "sendsignalfn() returned %d (errno=%d): %s", pidfnbuf, sigret, errno, strerror(errno)); @@ -2339,9 +2339,9 @@ int main(int argc, char **argv) struct stat st; if (stat(pidfnbuf, &st) == 0) { upslogx(LOG_WARNING, "Duplicate driver instance is still alive (PID file %s exists) after several termination attempts! Killing other driver!", pidfnbuf); - if (sendsignalfn(pidfnbuf, SIGKILL) == 0) { + if (sendsignalfn(pidfnbuf, SIGKILL, progname) == 0) { sleep(5); - if (sendsignalfn(pidfnbuf, 0) == 0) { + if (sendsignalfn(pidfnbuf, 0, progname) == 0) { upslogx(LOG_WARNING, "Duplicate driver instance is still alive (could signal the process)"); /* TODO: Should we writepid() below in this case? * Or if driver init fails, restore the old content diff --git a/drivers/upsdrvctl.c b/drivers/upsdrvctl.c index 84ca633791..3d9295f13e 100644 --- a/drivers/upsdrvctl.c +++ b/drivers/upsdrvctl.c @@ -304,9 +304,9 @@ static void signal_driver_cmd(const ups_t *ups, nut_sendsignal_debug_level = NUT_SENDSIGNAL_DEBUG_LEVEL_KILL_SIG0PING - 1; #ifndef WIN32 if (ups->pid == -1) { - ret = sendsignalfn(pidfn, cmd); + ret = sendsignalfn(pidfn, cmd, ups->driver); } else { - ret = sendsignalpid(ups->pid, cmd); + ret = sendsignalpid(ups->pid, cmd, ups->driver); /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { upslog_with_errno(LOG_WARNING, @@ -381,9 +381,9 @@ static void stop_driver(const ups_t *ups) #ifndef WIN32 if (ups->pid == -1) { - ret = sendsignalfn(pidfn, SIGTERM); + ret = sendsignalfn(pidfn, SIGTERM, ups->driver); } else { - ret = sendsignalpid(ups->pid, SIGTERM); + ret = sendsignalpid(ups->pid, SIGTERM, ups->driver); /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { goto clean_return; @@ -397,9 +397,9 @@ static void stop_driver(const ups_t *ups) #ifndef WIN32 upsdebugx(2, "SIGTERM to %s failed, retrying with SIGKILL", pidfn); if (ups->pid == -1) { - ret = sendsignalfn(pidfn, SIGKILL); + ret = sendsignalfn(pidfn, SIGKILL, ups->driver); } else { - ret = sendsignalpid(ups->pid, SIGKILL); + ret = sendsignalpid(ups->pid, SIGKILL, ups->driver); /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { goto clean_return; @@ -419,16 +419,16 @@ static void stop_driver(const ups_t *ups) for (i = 0; i < 5 ; i++) { #ifndef WIN32 if (ups->pid == -1) { - ret = sendsignalfn(pidfn, 0); + ret = sendsignalfn(pidfn, 0, ups->driver); } else { /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { goto clean_return; } - ret = sendsignalpid(ups->pid, 0); + ret = sendsignalpid(ups->pid, 0, ups->driver); } #else - ret = sendsignalfn(pidfn, 0); + ret = sendsignalfn(pidfn, 0, ups->driver); #endif if (ret != 0) { upsdebugx(2, "Sending signal to %s failed, driver is finally down or wrongly owned", pidfn); @@ -440,13 +440,13 @@ static void stop_driver(const ups_t *ups) #ifndef WIN32 upslog_with_errno(LOG_ERR, "Stopping %s failed, retrying harder", pidfn); if (ups->pid == -1) { - ret = sendsignalfn(pidfn, SIGKILL); + ret = sendsignalfn(pidfn, SIGKILL, ups->driver); } else { /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { goto clean_return; } - ret = sendsignalpid(ups->pid, SIGKILL); + ret = sendsignalpid(ups->pid, SIGKILL, ups->driver); } #else upslog_with_errno(LOG_ERR, "Stopping %s failed, retrying again", pidfn); @@ -456,16 +456,16 @@ static void stop_driver(const ups_t *ups) for (i = 0; i < 5 ; i++) { #ifndef WIN32 if (ups->pid == -1) { - ret = sendsignalfn(pidfn, 0); + ret = sendsignalfn(pidfn, 0, ups->driver); } else { /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { goto clean_return; } - ret = sendsignalpid(ups->pid, 0); + ret = sendsignalpid(ups->pid, 0, ups->driver); } #else - ret = sendsignalfn(pidfn, 0); + ret = sendsignalfn(pidfn, 0, ups->driver); #endif if (ret != 0) { upsdebugx(2, "Sending signal to %s failed, driver is finally down or wrongly owned", pidfn); diff --git a/include/common.h b/include/common.h index 83e01690a4..56dd5ea3ff 100644 --- a/include/common.h +++ b/include/common.h @@ -264,7 +264,7 @@ void writepid(const char *name); * a few sanity checks; returns -1 on error */ pid_t parsepid(const char *buf); -/* send a signal to another running process */ +/* send a signal to another running NUT process */ #ifndef WIN32 int sendsignal(const char *progname, int sig); #else @@ -279,7 +279,7 @@ pid_t get_max_pid_t(void); /* send sig to pid after some sanity checks, returns * -1 for error, or zero for a successfully sent signal */ -int sendsignalpid(pid_t pid, int sig); +int sendsignalpid(pid_t pid, int sig, const char *progname); /* open , get the pid, then send it * returns zero for successfully sent signal, @@ -289,10 +289,13 @@ int sendsignalpid(pid_t pid, int sig); * -1 Error sending signal */ #ifndef WIN32 -/* open , get the pid, then send it */ -int sendsignalfn(const char *pidfn, int sig); +/* open , get the pid, then send it + * if executable process with that pid has suitable progname + */ +int sendsignalfn(const char *pidfn, int sig, const char *progname); #else -int sendsignalfn(const char *pidfn, const char * sig); +/* No progname here - communications via named pipe */ +int sendsignalfn(const char *pidfn, const char * sig, const char *progname_ignored); #endif const char *xbasename(const char *file); diff --git a/server/upsd.c b/server/upsd.c index 9c0bcfcb04..63483f8d76 100644 --- a/server/upsd.c +++ b/server/upsd.c @@ -2049,9 +2049,9 @@ int main(int argc, char **argv) */ if (oldpid < 0) { - cmdret = sendsignalfn(pidfn, cmd); + cmdret = sendsignalfn(pidfn, cmd, progname); } else { - cmdret = sendsignalpid(oldpid, cmd); + cmdret = sendsignalpid(oldpid, cmd, progname); } #else /* if WIN32 */ if (cmd) { From 829dca21fd9d5bfc6bb7e435b38debe56dae00bd Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 8 Jun 2024 20:06:44 +0200 Subject: [PATCH 53/65] common/common.c: getprocname(): implement support for /proc/NNN/cmdline parsing [#2463] Signed-off-by: Jim Klimov --- common/common.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/common/common.c b/common/common.c index 00c05de807..d83c091e7a 100644 --- a/common/common.c +++ b/common/common.c @@ -580,8 +580,52 @@ char * getprocname(pid_t pid) process_parse_file: upsdebugx(3, "%s: try to parse some files under /proc", __func__); - /* TODO: Check /proc/NNN/cmdline (may start with a '-' to ignore, - * for a title string like "-bash" where programs edit their argv[0] */ + /* Check /proc/NNN/cmdline (may start with a '-' to ignore, for + * a title string like "-bash" where programs edit their argv[0] + * (Linux-like OSes at least). Inspired by + * https://gist.github.com/evanslai/30c6d588a80222f665f10b4577dadd61 + */ + if (snprintf(pathname, sizeof(pathname), "/proc/%" PRIuMAX "/cmdline", (uintmax_t)pid) < 10) { + upsdebug_with_errno(3, "%s: failed to snprintf pathname: Linux-like", __func__); + goto finish; + } + + if (stat(pathname, &st) == 0) { + FILE* fp = fopen(pathname, "r"); + if (fp) { + char buf[LARGEBUF]; + if (fgets(buf, sizeof(buf), fp) != NULL) { + /* check the first token in the file, the program name */ + char* first = strtok(buf, " "); + + fclose(fp); + if (first) { + if (*first == '-') + first++; + + /* Not xcalloc() here, not too fatal if we fail */ + if ((procnamelen = strlen(first))) { + upsdebugx(3, "%s: try to parse some files under /proc: processing %s", + __func__, pathname); + if ((procname = (char*)calloc(procnamelen + 1, sizeof(char)))) { + if (snprintf(procname, procnamelen + 1, "%s", first) < 1) { + upsdebug_with_errno(3, "%s: failed to snprintf pathname: Linux-like", __func__); + } + } else { + upsdebug_with_errno(3, "%s: failed to allocate the procname " + "string to store token from 'cmdline' size %" PRIuSIZE, + __func__, procnamelen); + } + + goto finish; + } + } + } else { + fclose(fp); + } + } + } + /* TODO: Check /proc/NNN/stat (second token, in parentheses, may be truncated) * see e.g. https://stackoverflow.com/a/12675103/4715872 */ From 9fcfcbeb1a136b72261c31b07f0d99b4d989d2a2 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sun, 9 Jun 2024 00:04:42 +0200 Subject: [PATCH 54/65] common/common.c: getprocname(): implement support for /proc/NNN/stat parsing [#2463] Signed-off-by: Jim Klimov --- common/common.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/common/common.c b/common/common.c index d83c091e7a..e4b7597058 100644 --- a/common/common.c +++ b/common/common.c @@ -497,6 +497,7 @@ char * getprocname(pid_t pid) if (stat("/proc", &st) == 0 && ((st.st_mode & S_IFMT) == S_IFDIR)) { upsdebugx(3, "%s: /proc is an accessible directory, investigating", __func__); + #if (defined HAVE_READLINK) && HAVE_READLINK /* Linux-like */ if (snprintf(pathname, sizeof(pathname), "/proc/%" PRIuMAX "/exe", (uintmax_t)pid) < 10) { @@ -593,7 +594,7 @@ char * getprocname(pid_t pid) if (stat(pathname, &st) == 0) { FILE* fp = fopen(pathname, "r"); if (fp) { - char buf[LARGEBUF]; + char buf[sizeof(pathname)]; if (fgets(buf, sizeof(buf), fp) != NULL) { /* check the first token in the file, the program name */ char* first = strtok(buf, " "); @@ -626,9 +627,56 @@ char * getprocname(pid_t pid) } } - - /* TODO: Check /proc/NNN/stat (second token, in parentheses, may be truncated) + /* Check /proc/NNN/stat (second token, in parentheses, may be truncated) * see e.g. https://stackoverflow.com/a/12675103/4715872 */ + if (snprintf(pathname, sizeof(pathname), "/proc/%" PRIuMAX "/stat", (uintmax_t)pid) < 10) { + upsdebug_with_errno(3, "%s: failed to snprintf pathname: Linux-like", __func__); + goto finish; + } + + if (stat(pathname, &st) == 0) { + FILE* fp = fopen(pathname, "r"); + if (fp) { + long spid; + char sstate; + char buf[sizeof(pathname)]; + + memset (buf, 0, sizeof(buf)); + if ( (fscanf(fp, "%ld (%[^)]) %c", &spid, buf, &sstate)) == 3 ) { + /* Some names can be pretty titles like "init(Ubuntu)" + * or "Relay(223)". Or truncated like "docker-desktop-". + * Tokenize by "(" " " and extract the first token to + * address the former "problem", not too much we can + * do about the latter except for keeping NUT program + * names concise. + */ + char* first = strtok(buf, "( "); + + fclose(fp); + if (first) { + /* Not xcalloc() here, not too fatal if we fail */ + if ((procnamelen = strlen(first))) { + upsdebugx(3, "%s: try to parse some files under /proc: processing %s " + "(WARNING: may be truncated)", + __func__, pathname); + if ((procname = (char*)calloc(procnamelen + 1, sizeof(char)))) { + if (snprintf(procname, procnamelen + 1, "%s", first) < 1) { + upsdebug_with_errno(3, "%s: failed to snprintf pathname: Linux-like", __func__); + } + } else { + upsdebug_with_errno(3, "%s: failed to allocate the procname " + "string to store token from 'cmdline' size %" PRIuSIZE, + __func__, procnamelen); + } + + goto finish; + } + } + } else { + fclose(fp); + } + } + } /* TODO: Solaris/illumos: parse binary structure at /proc/NNN/psinfo */ } else { From 684a0da378ae3a1e498f408f685a9988b2ee1432 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sun, 9 Jun 2024 00:40:02 +0200 Subject: [PATCH 55/65] common/common.c: getprocname(): implement support for (Open)BSD parsing without a /proc [#2463] Signed-off-by: Jim Klimov --- common/Makefile.am | 4 ++-- common/common.c | 58 +++++++++++++++++++++++++++++++++++++++++++++- configure.ac | 39 +++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 3 deletions(-) diff --git a/common/Makefile.am b/common/Makefile.am index 7957fd170f..a80dd14795 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -97,8 +97,8 @@ endif HAVE_WINDOWS # ensure inclusion of local implementation of missing systems functions # using LTLIBOBJS. Refer to configure.in/.ac -> AC_REPLACE_FUNCS -libcommon_la_LIBADD = libparseconf.la @LTLIBOBJS@ @NETLIBS@ -libcommonclient_la_LIBADD = libparseconf.la @LTLIBOBJS@ @NETLIBS@ +libcommon_la_LIBADD = libparseconf.la @LTLIBOBJS@ @NETLIBS@ @BSDKVMPROCLIBS@ +libcommonclient_la_LIBADD = libparseconf.la @LTLIBOBJS@ @NETLIBS@ @BSDKVMPROCLIBS@ libcommon_la_CFLAGS = $(AM_CFLAGS) libcommonclient_la_CFLAGS = $(AM_CFLAGS) diff --git a/common/common.c b/common/common.c index e4b7597058..9029ff768b 100644 --- a/common/common.c +++ b/common/common.c @@ -83,6 +83,13 @@ const char *UPS_VERSION = NUT_VERSION_MACRO; #include #include #include + +#if defined(HAVE_LIB_BSD_KVM_PROC) && HAVE_LIB_BSD_KVM_PROC +# include +# include +# include +#endif + pid_t get_max_pid_t(void) { #ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_UNREACHABLE_CODE @@ -681,9 +688,58 @@ char * getprocname(pid_t pid) /* TODO: Solaris/illumos: parse binary structure at /proc/NNN/psinfo */ } else { upsdebug_with_errno(3, "%s: /proc is not a directory or not accessible", __func__); - /* TODO: OpenBSD: no /proc; use API call, see ps.c link above */ } +#if defined(HAVE_LIB_BSD_KVM_PROC) && HAVE_LIB_BSD_KVM_PROC + /* OpenBSD, maybe other BSD: no /proc; use API call, see ps.c link above and + * https://kaashif.co.uk/2015/06/18/how-to-get-a-list-of-processes-on-openbsd-in-c/ + */ + if (!procname) { + char errbuf[_POSIX2_LINE_MAX]; + kvm_t *kd = kvm_openfiles(NULL, NULL, NULL, KVM_NO_FILES, errbuf); + + upsdebugx(3, "%s: try to parse BSD KVM process info snapsnot", __func__); + if (!kd) { + upsdebugx(3, "%s: try to parse BSD KVM process info snapsnot: " + "kvm_openfiles() returned NULL", __func__); + } else { + int nentries = 0; + struct kinfo_proc *kp = kvm_getprocs(kd, KERN_PROC_PID, pid, sizeof(*kp), &nentries); + + if (!kp) { + upsdebugx(3, "%s: try to parse BSD KVM process info snapsnot: " + "kvm_getprocs() returned NULL", __func__); + } else { + int i; + if (nentries != 1) + upsdebugx(3, "%s: expected to get 1 reply from BSD kvm_getprocs but got %d", + __func__, nentries); + for (i = 0; i < nentries; i++) { + upsdebugx(5, "%s: processing reply #%d from BSD" + " kvm_getprocs: pid=%" PRIuMAX " name='%s'", + __func__, i, (uintmax_t)kp[i].p_pid, kp[i].p_comm); + if ((uintmax_t)(kp[i].p_pid) == (uintmax_t)pid) { + /* Not xcalloc() here, not too fatal if we fail */ + if ((procnamelen = strlen(kp[i].p_comm))) { + if ((procname = (char*)calloc(procnamelen + 1, sizeof(char)))) { + if (snprintf(procname, procnamelen + 1, "%s", kp[i].p_comm) < 1) { + upsdebug_with_errno(3, "%s: failed to snprintf pathname: BSD-like", __func__); + } + } else { + upsdebug_with_errno(3, "%s: failed to allocate the procname " + "string to store token from 'cmdline' size %" PRIuSIZE, + __func__, procnamelen); + } + + goto finish; + } + } + } + } + } + } +#endif /* HAVE_LIB_BSD_KVM_PROC */ + goto finish; finish: diff --git a/configure.ac b/configure.ac index badbc92383..b17fc0b80b 100644 --- a/configure.ac +++ b/configure.ac @@ -1111,6 +1111,44 @@ AS_IF([test x"${ac_cv_func_usleep}" = xyes], [AC_MSG_WARN([Required C library routine usleep not found; try adding -D_POSIX_C_SOURCE=200112L])] ) +dnl OpenBSD (at least) methods to query process info, per +dnl https://github.com/openbsd/src/blob/master/bin/ps/ps.c +dnl https://kaashif.co.uk/2015/06/18/how-to-get-a-list-of-processes-on-openbsd-in-c/ +BSDKVMPROCLIBS="" +myLIBS="$LIBS" +LIBS="$LIBS -lkvm" +AC_CACHE_CHECK([for BSD KVM process info libs], + [ac_cv_lib_bsd_kvm_proc], + [AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[ +#include +#include +#include +#include +#include + ]], + [[ + char errbuf[_POSIX2_LINE_MAX]; + kvm_t *kernel = kvm_openfiles(NULL, NULL, NULL, KVM_NO_FILES, errbuf); + int nentries = 0; + struct kinfo_proc *kinfo = kvm_getprocs(kernel, KERN_PROC_ALL, 0, sizeof(struct kinfo_proc), &nentries); + int i; + for (i = 0; i < nentries; ++i) { + printf("%s\n", kinfo[i].p_comm); + } +/* autoconf adds ";return 0;" */ +/* we hope the code above fails if type is not defined or range is not sufficient */ +]])], + [ac_cv_lib_bsd_kvm_proc=yes + BSDKVMPROCLIBS="-lkvm" + ], [ac_cv_lib_bsd_kvm_proc=no] + )]) +LIBS="$myLIBS" + +AS_IF([test x"${ac_cv_lib_bsd_kvm_proc}" = xyes], + [AC_DEFINE([HAVE_LIB_BSD_KVM_PROC], 1, [defined if we have libs, includes and methods for BSD KVM process info])] + ) + AC_LANG_POP([C]) dnl These routines' arg types differ in strict C standard mode @@ -4329,6 +4367,7 @@ AC_SUBST(DRIVER_BUILD_LIST) AC_SUBST(DRIVER_MAN_LIST) AC_SUBST(DRIVER_MAN_LIST_PAGES) AC_SUBST(DRIVER_INSTALL_TARGET) +AC_SUBST(BSDKVMPROCLIBS) AC_SUBST(NETLIBS) AC_SUBST(SERLIBS) AC_SUBST(SEMLIBS) From 2126ce2d28c5be2cf8e0172e21b1df2ab5f20d45 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sun, 9 Jun 2024 00:38:45 +0000 Subject: [PATCH 56/65] common/common.c: getprocname(): revise printed messages [#2463] Signed-off-by: Jim Klimov --- common/common.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/common/common.c b/common/common.c index 9029ff768b..d933d08d5f 100644 --- a/common/common.c +++ b/common/common.c @@ -586,7 +586,7 @@ char * getprocname(pid_t pid) #endif /* HAVE_READLINK */ process_parse_file: - upsdebugx(3, "%s: try to parse some files under /proc", __func__); + upsdebugx(5, "%s: try to parse some files under /proc", __func__); /* Check /proc/NNN/cmdline (may start with a '-' to ignore, for * a title string like "-bash" where programs edit their argv[0] @@ -617,7 +617,7 @@ char * getprocname(pid_t pid) __func__, pathname); if ((procname = (char*)calloc(procnamelen + 1, sizeof(char)))) { if (snprintf(procname, procnamelen + 1, "%s", first) < 1) { - upsdebug_with_errno(3, "%s: failed to snprintf pathname: Linux-like", __func__); + upsdebug_with_errno(3, "%s: failed to snprintf procname: Linux-like", __func__); } } else { upsdebug_with_errno(3, "%s: failed to allocate the procname " @@ -668,11 +668,11 @@ char * getprocname(pid_t pid) __func__, pathname); if ((procname = (char*)calloc(procnamelen + 1, sizeof(char)))) { if (snprintf(procname, procnamelen + 1, "%s", first) < 1) { - upsdebug_with_errno(3, "%s: failed to snprintf pathname: Linux-like", __func__); + upsdebug_with_errno(3, "%s: failed to snprintf procname: Linux-like", __func__); } } else { upsdebug_with_errno(3, "%s: failed to allocate the procname " - "string to store token from 'cmdline' size %" PRIuSIZE, + "string to store token from 'stat' size %" PRIuSIZE, __func__, procnamelen); } @@ -723,11 +723,12 @@ char * getprocname(pid_t pid) if ((procnamelen = strlen(kp[i].p_comm))) { if ((procname = (char*)calloc(procnamelen + 1, sizeof(char)))) { if (snprintf(procname, procnamelen + 1, "%s", kp[i].p_comm) < 1) { - upsdebug_with_errno(3, "%s: failed to snprintf pathname: BSD-like", __func__); + upsdebug_with_errno(3, "%s: failed to snprintf procname: BSD-like", __func__); } } else { upsdebug_with_errno(3, "%s: failed to allocate the procname " - "string to store token from 'cmdline' size %" PRIuSIZE, + "string to store token from BSD KVM process info " + "snapsnot size %" PRIuSIZE, __func__, procnamelen); } From 4707e116afd4c04f088dd37ab26951d2a13c1b01 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sun, 9 Jun 2024 00:39:50 +0000 Subject: [PATCH 57/65] common/common.c: getprocname(): implement support for /proc/NNN/psinfo (Solaris/illumos) parsing [#2463] Signed-off-by: Jim Klimov --- common/common.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++- configure.ac | 24 ++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/common/common.c b/common/common.c index d933d08d5f..259e735f27 100644 --- a/common/common.c +++ b/common/common.c @@ -90,6 +90,10 @@ const char *UPS_VERSION = NUT_VERSION_MACRO; # include #endif +#if defined(HAVE_LIB_ILLUMOS_PROC) && HAVE_LIB_ILLUMOS_PROC +# include +#endif + pid_t get_max_pid_t(void) { #ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_UNREACHABLE_CODE @@ -685,7 +689,55 @@ char * getprocname(pid_t pid) } } - /* TODO: Solaris/illumos: parse binary structure at /proc/NNN/psinfo */ +#if defined(HAVE_LIB_ILLUMOS_PROC) && HAVE_LIB_ILLUMOS_PROC + /* Solaris/illumos: parse binary structure at /proc/NNN/psinfo */ + if (snprintf(pathname, sizeof(pathname), "/proc/%" PRIuMAX "/psinfo", (uintmax_t)pid) < 10) { + upsdebug_with_errno(3, "%s: failed to snprintf pathname: Solaris/illumos-like", __func__); + goto finish; + } + + if (stat(pathname, &st) == 0) { + FILE* fp = fopen(pathname, "r"); + if (!fp) { + upsdebug_with_errno(3, "%s: try to parse '%s':" + "fopen() returned NULL", __func__, pathname); + } else { + psinfo_t info; /* process information from /proc */ + size_t r; + + memset (&info, 0, sizeof(info)); + r = fread((char *)&info, sizeof (info), 1, fp); + if (r != 1) { + upsdebug_with_errno(3, "%s: try to parse '%s': " + "unexpected read size: got %" PRIuSIZE + " record(s) from file of size %" PRIuMAX + " vs. 1 piece of %" PRIuSIZE " struct size", + __func__, pathname, r, + (uintmax_t)st.st_size, sizeof (info)); + fclose(fp); + } else { + fclose(fp); + + /* Not xcalloc() here, not too fatal if we fail */ + if ((procnamelen = strlen(info.pr_fname))) { + upsdebugx(3, "%s: try to parse some files under /proc: processing %s", + __func__, pathname); + if ((procname = (char*)calloc(procnamelen + 1, sizeof(char)))) { + if (snprintf(procname, procnamelen + 1, "%s", info.pr_fname) < 1) { + upsdebug_with_errno(3, "%s: failed to snprintf pathname: Solaris/illumos-like", __func__); + } + } else { + upsdebug_with_errno(3, "%s: failed to allocate the procname " + "string to store token from 'psinfo' size %" PRIuSIZE, + __func__, procnamelen); + } + + goto finish; + } + } + } + } +#endif } else { upsdebug_with_errno(3, "%s: /proc is not a directory or not accessible", __func__); } diff --git a/configure.ac b/configure.ac index b17fc0b80b..414e26ba65 100644 --- a/configure.ac +++ b/configure.ac @@ -1149,6 +1149,30 @@ AS_IF([test x"${ac_cv_lib_bsd_kvm_proc}" = xyes], [AC_DEFINE([HAVE_LIB_BSD_KVM_PROC], 1, [defined if we have libs, includes and methods for BSD KVM process info])] ) +dnl https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/sys/procfs.h#L318 +dnl https://github.com/illumos/illumos-gate/blob/master/usr/src/cmd/ps/ps.c +AC_CACHE_CHECK([for Solaris/illumos process info libs], + [ac_cv_lib_illumos_proc], + [AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[ +#include +#include + ]], + [[ + psinfo_t info; + printf("%s", info.pr_fname) +/* autoconf adds ";return 0;" */ +/* we hope the code above fails if type is not defined or range is not sufficient */ +]])], + [ac_cv_lib_illumos_proc=yes + ], [ac_cv_lib_illumos_proc=no] + )]) +LIBS="$myLIBS" + +AS_IF([test x"${ac_cv_lib_illumos_proc}" = xyes], + [AC_DEFINE([HAVE_LIB_ILLUMOS_PROC], 1, [defined if we have libs, includes and methods for Solaris/illumos process info])] + ) + AC_LANG_POP([C]) dnl These routines' arg types differ in strict C standard mode From 2229880e251dfd568c5f3c220a4a50de580ec956 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sun, 9 Jun 2024 03:50:44 +0200 Subject: [PATCH 58/65] common/common.c: getprocname(): implement support for WIN32 parsing without a /proc [#2463] Signed-off-by: Jim Klimov --- common/common.c | 72 +++++++++++++++++++++++++++++++++++++++++++----- include/common.h | 1 + 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/common/common.c b/common/common.c index 259e735f27..542c97f931 100644 --- a/common/common.c +++ b/common/common.c @@ -1,7 +1,7 @@ /* common.c - common useful functions Copyright (C) 2000 Russell Kroll - Copyright (C) 2021-2022 Jim Klimov + Copyright (C) 2021-2024 Jim Klimov This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -23,13 +23,15 @@ #include #ifndef WIN32 -#include -#include -#include -#include -#include +# include +# include +# include +# include +# include #else -#include +# include +# include +# include #endif #ifdef HAVE_UNISTD_H @@ -504,6 +506,62 @@ char * getprocname(pid_t pid) * https://stackoverflow.com/questions/1591342/c-how-to-determine-if-a-windows-process-is-running * http://cppip.blogspot.com/2013/01/check-if-process-is-running.html */ + upsdebugx(5, "%s: begin to query WIN32 process info", __func__); + HANDLE process = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, (DWORD)pid); + if (process) { + DWORD ret = GetModuleFileNameExA( + process, /* hProcess */ + NULL, /* hModule */ + (LPSTR)pathname, + (DWORD)(sizeof(pathname)) + ); + CloseHandle(process); + pathname[sizeof(pathname) - 1] = '\0'; + + if (ret) { + /* length of the string copied to the buffer */ + procnamelen = strlen(pathname); + + upsdebugx(3, "%s: try to parse the name from WIN32 process info", + __func__); + if (ret != procnamelen) { + upsdebugx(3, "%s: length mismatch getting WIN32 process info: %" + PRIuMAX " vs. " PRIuSIZE, + __func__, (uintmax_t)ret, procnamelen); + } + + if ((procname = (char*)calloc(procnamelen + 1, sizeof(char)))) { + if (snprintf(procname, procnamelen + 1, "%s", pathname) < 1) { + upsdebug_with_errno(3, "%s: failed to snprintf procname: WIN32-like", __func__); + } else { + goto finish; + } + } else { + upsdebug_with_errno(3, "%s: failed to allocate the procname " + "string to store token from WIN32 size %" PRIuSIZE, + __func__, procnamelen); + } + + /* Fall through to try /proc etc. if available */ + } else { + LPVOID WinBuf; + DWORD WinErr = GetLastError(); + FormatMessage( + FORMAT_MESSAGE_MAX_WIDTH_MASK | + FORMAT_MESSAGE_ALLOCATE_BUFFER | + FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, + WinErr, + MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (LPTSTR) &WinBuf, + 0, NULL ); + + upsdebugx(3, "%s: failed to get WIN32 process info: %s", + __func__, (char *)WinBuf); + LocalFree(WinBuf); + } + } #endif if (stat("/proc", &st) == 0 && ((st.st_mode & S_IFMT) == S_IFDIR)) { diff --git a/include/common.h b/include/common.h index 56dd5ea3ff..94be6e4625 100644 --- a/include/common.h +++ b/include/common.h @@ -1,6 +1,7 @@ /* common.h - prototypes for the common useful functions Copyright (C) 2000 Russell Kroll + Copyright (C) 2021-2024 Jim Klimov This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by From 2ebc2846494a6086e9b361703e3ea1bde8570091 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 10 Jun 2024 09:11:35 +0200 Subject: [PATCH 59/65] common/common.c: checkprocname(): case #5 is not "exact", fix the message [#2463] Signed-off-by: Jim Klimov --- common/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/common.c b/common/common.c index 542c97f931..c8e08912c0 100644 --- a/common/common.c +++ b/common/common.c @@ -1003,7 +1003,7 @@ int checkprocname(pid_t pid, const char *progname) switch (ret) { case 5: upsdebugx(1, - "%s: exact case-insensitive base name hit with " + "%s: case-insensitive base name hit with " "an executable program extension involved for " "PID %" PRIuMAX " of '%s'=>'%s' and checked " "'%s'=>'%s'", From 16ff315e3919a15ba1e699850a3619bafc59e2a3 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 10 Jun 2024 11:46:39 +0200 Subject: [PATCH 60/65] NEWS.adoc: explain the fix for #2454 via #2462 Signed-off-by: Jim Klimov --- NEWS.adoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.adoc b/NEWS.adoc index ee34f0eb3e..81620d58c6 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -51,6 +51,10 @@ https://github.com/networkupstools/nut/milestone/11 * Addition of "NUT Simulated devices" support to `nut-scanner` in v2.8.2 broke detection of (in-)ability to find and query "Old NUT" servers via `libupsclient.so` (internal flag got always enabled). [#2246] + * A fix for `upsmon` v2.8.1 setting of `OFFDURATION` [PR #2108, issue #2104, + revisiting PR #2055, issue #2044] was overly zealous and impacted also + the `OB` state in cases where communications to the data server were + severed and `DEADTIME` setting was not honored. [PR #2462, issue #2454] - drivers, upsd, upsmon: reduce "scary noise" about failure to `fopen()` the PID file (which most of the time means that no previous instance of From 82e5932d9c4b589856746d31835ee50da3185ce8 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 10 Jun 2024 11:31:00 +0200 Subject: [PATCH 61/65] clients/upsmon.c: for OB UPS and communications failure, leave just the debug log message [#2454] Signed-off-by: Jim Klimov --- clients/upsmon.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/clients/upsmon.c b/clients/upsmon.c index 65b0234c88..6dd7c2b9ca 100644 --- a/clients/upsmon.c +++ b/clients/upsmon.c @@ -1111,6 +1111,20 @@ static int is_ups_critical(utype_t *ups) ups->sys); return 1; } + + if (ups->linestate == 0) { + /* Just a message for post-mortem troubleshooting: + * no flag flips, no return values issued just here + * (note the message is likely to appear on every + * cycle when the communications are down, to help + * track when this was the case; no log throttling). + */ + upsdebugx(1, + "UPS [%s] was last known to be not fully online " + "and currently is not communicating, just so you " + "know (waiting for DEADTIME to elapse)", + ups->sys); + } } /* administratively OFF (long enough, see OFFDURATION) */ From 55ae3e1b430e5295a5e1a419e8a7abbbec29bce4 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Thu, 13 Jun 2024 13:30:47 +0200 Subject: [PATCH 62/65] UPGRADING.adoc: warn that packaged program name mismatches can be a problem [#2463] Signed-off-by: Jim Klimov --- UPGRADING.adoc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/UPGRADING.adoc b/UPGRADING.adoc index 3ffd2d28a6..cf50556ad1 100644 --- a/UPGRADING.adoc +++ b/UPGRADING.adoc @@ -37,7 +37,14 @@ Changes from 2.8.2 to 2.8.3 which can impact NUT forks which build using `libcommon.la` and similar libraries. Added new last argument with `const char *progname` (may be `NULL`) to check that we are signalling an expected program name when we - work with a PID. [#2463] + work with a PID. With the same effort, NUT programs which deal with PID + files to send signals (`upsd`, `upsmon`, drivers and `upsdrvctl`) would + now default to a safety precaution -- checking that the running process + with that PID has the expected program name (on platforms where we can + determine one). This might introduce regressions for heavily customized + NUT builds (e.g. embedded in NAS or similar devices) whose binary file + names differ significantly from a `progname` defined in the respective + NUT source file. [issue #2463] Changes from 2.8.1 to 2.8.2 From 0904b931533fec20506e581deb1189a925df0c5a Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Thu, 13 Jun 2024 14:56:21 +0200 Subject: [PATCH 63/65] Introduce NUT_IGNORE_CHECKPROCNAME=true support to skip PID process name validation if it causes problems [#2463] Signed-off-by: Jim Klimov --- NEWS.adoc | 8 +++++++- UPGRADING.adoc | 4 +++- common/common.c | 17 ++++++++++++++++- conf/nut.conf.sample | 9 +++++++++ docs/man/nut.conf.txt | 12 ++++++++++++ docs/nut.dict | 3 ++- include/common.h | 1 + 7 files changed, 50 insertions(+), 4 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index d203b6b1cc..7de6de1172 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -139,7 +139,13 @@ https://github.com/networkupstools/nut/milestone/11 that if they can resolve the program name of a running process with this identifier, that such name matches the current program (avoid failures to start NUT daemons if PID files are on persistent storage, - and some unrelated program got that PID after a reboot). [#2463] + and some unrelated program got that PID after a reboot). This might + introduce regressions for heavily customized NUT builds (e.g. those + embedded in NAS or similar devices) where binary file names differ + significantly from a `progname` string defined in the respective NUT + source file, so a boolean `NUT_IGNORE_CHECKPROCNAME` environment + variable support was added to optionally disable this verification. + [issue #2463] - various recipe, documentation and source files were revised to address respective warnings issued by the new generations of analysis tools. diff --git a/UPGRADING.adoc b/UPGRADING.adoc index cf50556ad1..7f7877c09e 100644 --- a/UPGRADING.adoc +++ b/UPGRADING.adoc @@ -44,7 +44,9 @@ Changes from 2.8.2 to 2.8.3 determine one). This might introduce regressions for heavily customized NUT builds (e.g. embedded in NAS or similar devices) whose binary file names differ significantly from a `progname` defined in the respective - NUT source file. [issue #2463] + NUT source file, so a boolean `NUT_IGNORE_CHECKPROCNAME` environment + variable support was added to optionally disable this verification. + [issue #2463] Changes from 2.8.1 to 2.8.2 diff --git a/common/common.c b/common/common.c index c8e08912c0..86c67ac472 100644 --- a/common/common.c +++ b/common/common.c @@ -924,6 +924,7 @@ int checkprocname(pid_t pid, const char *progname) /* If we can determine the binary path name of the specified "pid", * check if it matches the assumed name of the current program. * Returns: + * -3 Skipped because NUT_IGNORE_CHECKPROCNAME is set * -2 Could not parse a program name (ok to proceed, * risky - but matches legacy behavior) * -1 Could not identify a program name (ok to proceed, @@ -933,7 +934,7 @@ int checkprocname(pid_t pid, const char *progname) * varying precision * Generally speaking, if (checkprocname(...)) then ok to proceed */ - char *procname = getprocname(pid); + char *procname = NULL, *s; int ret = -127; size_t procbasenamelen = 0, progbasenamelen = 0; /* Track where the last dot is in the basename; 0 means none */ @@ -944,6 +945,16 @@ int checkprocname(pid_t pid, const char *progname) char procbasename[PATH_MAX], progbasename[PATH_MAX]; #endif + if ((s = getenv("NUT_IGNORE_CHECKPROCNAME"))) { + /* FIXME: Make server/conf.c::parse_boolean() reusable */ + if ( (!strcasecmp(s, "true")) || (!strcasecmp(s, "on")) || (!strcasecmp(s, "yes")) || (!strcasecmp(s, "1"))) { + upsdebugx(1, "%s: skipping because caller set NUT_IGNORE_CHECKPROCNAME", __func__); + ret = -3; + goto finish; + } + } + + procname = getprocname(pid); if (!procname || !progname) { ret = -1; goto finish; @@ -1064,6 +1075,10 @@ int checkprocname(pid_t pid, const char *progname) __func__); break; + case -3: + /* skipped due to envvar, logged above */ + break; + default: upsdebugx(1, "%s: unexpected result looking for process name " diff --git a/conf/nut.conf.sample b/conf/nut.conf.sample index 3df4fcd4f8..eb7eea852c 100644 --- a/conf/nut.conf.sample +++ b/conf/nut.conf.sample @@ -101,3 +101,12 @@ MODE=none # `none` Disabled and background() detaches stderr as usual # `default`/unset/other Not disabled #NUT_DEBUG_SYSLOG=stderr + +# Normally NUT can (attempt to) verify that the program file name matches the +# name associated with a running process, when using PID files to send signals. +# The `NUT_IGNORE_CHECKPROCNAME` boolean toggle allows to quickly skip such +# verification, in case it causes problems (e.g. NUT programs were renamed and +# do not match built-in expectations). This environment variable can also be +# optionally set in init-scripts or service methods for `upsd`, `upsmon` and +# NUT drivers/`upsdrvctl`. +#NUT_IGNORE_CHECKPROCNAME=true diff --git a/docs/man/nut.conf.txt b/docs/man/nut.conf.txt index f61a756f77..44a90f64b9 100644 --- a/docs/man/nut.conf.txt +++ b/docs/man/nut.conf.txt @@ -134,6 +134,18 @@ the same journal). Recognized values: | unset/other | Not disabled |=========================================================================== +*NUT_IGNORE_CHECKPROCNAME*:: +Optional, defaults to `false`. Normally NUT can (attempt to) verify that +the program file name matches the name associated with a running process, +when using PID files to send signals. ++ +The `NUT_IGNORE_CHECKPROCNAME` boolean toggle allows to quickly skip such +verification, in case it causes problems (e.g. NUT programs were renamed +and do not match built-in expectations). ++ +This environment variable can also be optionally set in init-scripts or +service methods for `upsd`, `upsmon` and NUT drivers/`upsdrvctl`. + EXAMPLE ------- diff --git a/docs/nut.dict b/docs/nut.dict index 61bb7efe84..45856ce90b 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3168 utf-8 +personal_ws-1.1 en 3169 utf-8 AAC AAS ABI @@ -154,6 +154,7 @@ CERTIDENT CERTREQUEST CERTVERIF CEST +CHECKPROCNAME CHOST CHRG CL diff --git a/include/common.h b/include/common.h index 94be6e4625..1a1e693149 100644 --- a/include/common.h +++ b/include/common.h @@ -246,6 +246,7 @@ size_t parseprogbasename(char *buf, size_t buflen, const char *progname, size_t /* If we can determine the binary path name of the specified "pid", * check if it matches the assumed name of the current program. * Returns: + * -3 Skipped because NUT_IGNORE_CHECKPROCNAME is set * -2 Could not parse a program name (ok to proceed, * risky - but matches legacy behavior) * -1 Could not identify a program name (ok to proceed, From b08dccee31903dc9cc5b0357561991d49bce005b Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Thu, 13 Jun 2024 21:04:05 +0200 Subject: [PATCH 64/65] Extend sendsignal*() API some more to optionally fall back to checkprocname() vs current getpid() [#2463] Signed-off-by: Jim Klimov --- NEWS.adoc | 3 +- UPGRADING.adoc | 3 +- clients/upsmon.c | 6 +- common/common.c | 137 +++++++++++++++++++++++++++++++++----- drivers/main.c | 12 ++-- drivers/upsdrvctl.c | 36 +++++----- include/common.h | 14 ++-- scripts/Windows/wininit.c | 4 +- server/upsd.c | 6 +- 9 files changed, 167 insertions(+), 54 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index 7de6de1172..a7279a9e66 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -145,7 +145,8 @@ https://github.com/networkupstools/nut/milestone/11 significantly from a `progname` string defined in the respective NUT source file, so a boolean `NUT_IGNORE_CHECKPROCNAME` environment variable support was added to optionally disable this verification. - [issue #2463] + Also the NUT daemons should request to double-check against their + run-time process name (if it can be detected). [issue #2463] - various recipe, documentation and source files were revised to address respective warnings issued by the new generations of analysis tools. diff --git a/UPGRADING.adoc b/UPGRADING.adoc index 7f7877c09e..0c5a7ac125 100644 --- a/UPGRADING.adoc +++ b/UPGRADING.adoc @@ -46,7 +46,8 @@ Changes from 2.8.2 to 2.8.3 names differ significantly from a `progname` defined in the respective NUT source file, so a boolean `NUT_IGNORE_CHECKPROCNAME` environment variable support was added to optionally disable this verification. - [issue #2463] + Also the NUT daemons should request to double-check against their + run-time process name (if it can be detected). [issue #2463] Changes from 2.8.1 to 2.8.2 diff --git a/clients/upsmon.c b/clients/upsmon.c index 6dd7c2b9ca..72314267dc 100644 --- a/clients/upsmon.c +++ b/clients/upsmon.c @@ -3022,15 +3022,15 @@ int main(int argc, char *argv[]) * is running by sending signal '0' (i.e. 'kill 0' equivalent) */ if (oldpid < 0) { - cmdret = sendsignal(prog, cmd); + cmdret = sendsignal(prog, cmd, 1); } else { - cmdret = sendsignalpid(oldpid, cmd, prog); + cmdret = sendsignalpid(oldpid, cmd, prog, 1); } #else /* WIN32 */ if (cmd) { /* Command the running daemon, it should be there */ - cmdret = sendsignal(UPSMON_PIPE_NAME, cmd); + cmdret = sendsignal(UPSMON_PIPE_NAME, cmd, 1); } else { /* Starting new daemon, check for competition */ mutex = CreateMutex(NULL, TRUE, UPSMON_PIPE_NAME); diff --git a/common/common.c b/common/common.c index 86c67ac472..b27f464eab 100644 --- a/common/common.c +++ b/common/common.c @@ -1150,10 +1150,11 @@ void writepid(const char *name) /* send sig to pid, returns -1 for error, or * zero for a successfully sent signal */ -int sendsignalpid(pid_t pid, int sig, const char *progname) +int sendsignalpid(pid_t pid, int sig, const char *progname, int check_current_progname) { #ifndef WIN32 - int ret; + int ret, cpn1 = -10, cpn2 = -10; + char *current_progname = NULL; /* TOTHINK: What about containers where a NUT daemon *is* the only process * and is the PID=1 of the container (recycle if dead)? */ @@ -1165,13 +1166,114 @@ int sendsignalpid(pid_t pid, int sig, const char *progname) return -1; } - if (progname && !checkprocname(pid, progname)) { - if (nut_debug_level > 0 || nut_sendsignal_debug_level > 1) - upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX - " which exists but is not of program '%s'", - (uintmax_t)pid, progname); + ret = 0; + if (progname) { + /* Check against some expected (often built-in) name */ + if (!(cpn1 = checkprocname(pid, progname))) { + /* Did not match expected (often built-in) name */ + ret = -1; + } else { + if (cpn1 > 0) { + /* Matched expected name, ok to proceed */ + ret = 1; + } + /* ...else could not determine name of PID; think later */ + } + } + /* if (cpn1 == -3) => NUT_IGNORE_CHECKPROCNAME=true */ + /* if (cpn1 == -1) => could not determine name of PID... retry just in case? */ + if (ret <= 0 && check_current_progname && cpn1 != -3) { + /* NOTE: This could be optimized a bit by pre-finding the procname + * of "pid" and re-using it, but this is not a hot enough code path + * to bother much. + */ + current_progname = getprocname(getpid()); + if (current_progname && (cpn2 = checkprocname(pid, current_progname))) { + if (cpn2 > 0) { + /* Matched current process as asked, ok to proceed */ + ret = 2; + } + /* ...else could not determine name of PID; think later */ + } else { + if (current_progname) { + /* Did not match current process name */ + ret = -2; + } /* else just did not determine current process + * name, so did not actually check either + * // ret = -3; + */ + } + } + + /* if ret == 0, ok to proceed - not asked for any sanity checks; + * if ret > 0 we had some definitive match above + */ + if (ret < 0) { + upsdebugx(1, + "%s: ran at least one check, and all such checks " + "found a process name for PID %" PRIuMAX " and " + "failed to match: expected progname='%s' (res=%d), " + "current progname='%s' (res=%d)", + __func__, (uintmax_t)pid, + NUT_STRARG(progname), cpn1, + NUT_STRARG(current_progname), cpn2); + + if (nut_debug_level > 0 || nut_sendsignal_debug_level > 1) { + switch (ret) { + case -1: + upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX + " which exists but is not of" + " expected program '%s'; not asked" + " to cross-check current PID's name", + (uintmax_t)pid, progname); + break; + + /* Maybe we tried both data sources, maybe just current_progname */ + case -2: + /*case -3:*/ + if (progname && current_progname) { + /* Tried both, downgraded verdict further */ + upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX + " which exists but is not of expected" + " program '%s' nor current '%s'", + (uintmax_t)pid, progname, current_progname); + } else if (current_progname) { + /* Not asked for progname==NULL */ + upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX + " which exists but is not of" + " current program '%s'", + (uintmax_t)pid, current_progname); + } else if (progname) { + upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX + " which exists but is not of" + " expected program '%s'; could not" + " cross-check current PID's name", + (uintmax_t)pid, progname); + } else { + /* Both NULL; one not asked, another not detected; + * should not actually get here (wannabe `ret==-3`) + */ + upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX + " but could not cross-check current PID's" + " name: did not expect to get here", + (uintmax_t)pid); + } + break; + } + } + + if (current_progname) { + free(current_progname); + current_progname = NULL; + } + + /* Logged or not, sanity-check was requested and failed */ return -1; } + if (current_progname) { + free(current_progname); + current_progname = NULL; + } /* see if this is going to work first - does the process exist, * and do we have permissions to signal it? */ @@ -1199,6 +1301,8 @@ int sendsignalpid(pid_t pid, int sig, const char *progname) NUT_UNUSED_VARIABLE(pid); NUT_UNUSED_VARIABLE(sig); NUT_UNUSED_VARIABLE(progname); + NUT_UNUSED_VARIABLE(check_current_progname); + /* Windows builds use named pipes, not signals per se */ upslogx(LOG_ERR, "%s: not implemented for Win32 and " "should not have been called directly!", @@ -1239,7 +1343,7 @@ pid_t parsepid(const char *buf) * zero for a successfully sent signal */ #ifndef WIN32 -int sendsignalfn(const char *pidfn, int sig, const char *progname) +int sendsignalfn(const char *pidfn, int sig, const char *progname, int check_current_progname) { char buf[SMALLBUF]; FILE *pidf; @@ -1275,7 +1379,7 @@ int sendsignalfn(const char *pidfn, int sig, const char *progname) if (pid >= 0) { /* this method actively reports errors, if any */ - ret = sendsignalpid(pid, sig, progname); + ret = sendsignalpid(pid, sig, progname, check_current_progname); } fclose(pidf); @@ -1284,10 +1388,11 @@ int sendsignalfn(const char *pidfn, int sig, const char *progname) #else /* => WIN32 */ -int sendsignalfn(const char *pidfn, const char * sig, const char *progname_ignored) +int sendsignalfn(const char *pidfn, const char * sig, const char *progname_ignored, int check_current_progname_ignored) { BOOL ret; NUT_UNUSED_VARIABLE(progname_ignored); + NUT_UNUSED_VARIABLE(check_current_progname_ignored); ret = send_to_named_pipe(pidfn, sig); @@ -1350,19 +1455,21 @@ int snprintfcat(char *dst, size_t size, const char *fmt, ...) /* lazy way to send a signal if the program uses the PIDPATH */ #ifndef WIN32 -int sendsignal(const char *progname, int sig) +int sendsignal(const char *progname, int sig, int check_current_progname) { char fn[SMALLBUF]; snprintf(fn, sizeof(fn), "%s/%s.pid", rootpidpath(), progname); - return sendsignalfn(fn, sig, progname); + return sendsignalfn(fn, sig, progname, check_current_progname); } #else -int sendsignal(const char *progname, const char * sig) +int sendsignal(const char *progname, const char * sig, int check_current_progname) { - /* progname is used as the pipe name for WIN32 */ - return sendsignalfn(progname, sig, NULL); + /* progname is used as the pipe name for WIN32 + * check_current_progname is de-facto ignored + */ + return sendsignalfn(progname, sig, NULL, check_current_progname); } #endif diff --git a/drivers/main.c b/drivers/main.c index 67d853fd81..788a1aa163 100644 --- a/drivers/main.c +++ b/drivers/main.c @@ -2223,9 +2223,9 @@ int main(int argc, char **argv) int cmdret = -1; /* Send a signal to older copy of the driver, if any */ if (oldpid < 0) { - cmdret = sendsignalfn(pidfnbuf, cmd, progname); + cmdret = sendsignalfn(pidfnbuf, cmd, progname, 1); } else { - cmdret = sendsignalpid(oldpid, cmd, progname); + cmdret = sendsignalpid(oldpid, cmd, progname, 1); } switch (cmdret) { @@ -2321,7 +2321,7 @@ int main(int argc, char **argv) upslogx(LOG_WARNING, "Duplicate driver instance detected (PID file %s exists)! Terminating other driver!", pidfnbuf); - if ((sigret = sendsignalfn(pidfnbuf, SIGTERM, progname) != 0)) { + if ((sigret = sendsignalfn(pidfnbuf, SIGTERM, progname, 1) != 0)) { upsdebugx(1, "Can't send signal to PID, assume invalid PID file %s; " "sendsignalfn() returned %d (errno=%d): %s", pidfnbuf, sigret, errno, strerror(errno)); @@ -2339,9 +2339,9 @@ int main(int argc, char **argv) struct stat st; if (stat(pidfnbuf, &st) == 0) { upslogx(LOG_WARNING, "Duplicate driver instance is still alive (PID file %s exists) after several termination attempts! Killing other driver!", pidfnbuf); - if (sendsignalfn(pidfnbuf, SIGKILL, progname) == 0) { + if (sendsignalfn(pidfnbuf, SIGKILL, progname, 1) == 0) { sleep(5); - if (sendsignalfn(pidfnbuf, 0, progname) == 0) { + if (sendsignalfn(pidfnbuf, 0, progname, 1) == 0) { upslogx(LOG_WARNING, "Duplicate driver instance is still alive (could signal the process)"); /* TODO: Should we writepid() below in this case? * Or if driver init fails, restore the old content @@ -2385,7 +2385,7 @@ int main(int argc, char **argv) upslogx(LOG_WARNING, "Duplicate driver instance detected! Terminating other driver!"); for(i=0;i<10;i++) { DWORD res; - sendsignal(name, COMMAND_STOP); + sendsignal(name, COMMAND_STOP, 1); if(mutex != NULL ) { res = WaitForSingleObject(mutex,1000); if(res==WAIT_OBJECT_0) { diff --git a/drivers/upsdrvctl.c b/drivers/upsdrvctl.c index 3d9295f13e..5eb62f839c 100644 --- a/drivers/upsdrvctl.c +++ b/drivers/upsdrvctl.c @@ -304,9 +304,9 @@ static void signal_driver_cmd(const ups_t *ups, nut_sendsignal_debug_level = NUT_SENDSIGNAL_DEBUG_LEVEL_KILL_SIG0PING - 1; #ifndef WIN32 if (ups->pid == -1) { - ret = sendsignalfn(pidfn, cmd, ups->driver); + ret = sendsignalfn(pidfn, cmd, ups->driver, 0); } else { - ret = sendsignalpid(ups->pid, cmd, ups->driver); + ret = sendsignalpid(ups->pid, cmd, ups->driver, 0); /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { upslog_with_errno(LOG_WARNING, @@ -315,7 +315,7 @@ static void signal_driver_cmd(const ups_t *ups, } } #else - ret = sendsignal(pidfn, cmd); + ret = sendsignal(pidfn, cmd, 0); #endif /* Restore the signal errors verbosity */ nut_sendsignal_debug_level = NUT_SENDSIGNAL_DEBUG_LEVEL_DEFAULT; @@ -381,25 +381,25 @@ static void stop_driver(const ups_t *ups) #ifndef WIN32 if (ups->pid == -1) { - ret = sendsignalfn(pidfn, SIGTERM, ups->driver); + ret = sendsignalfn(pidfn, SIGTERM, ups->driver, 0); } else { - ret = sendsignalpid(ups->pid, SIGTERM, ups->driver); + ret = sendsignalpid(ups->pid, SIGTERM, ups->driver, 0); /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { goto clean_return; } } #else - ret = sendsignal(pidfn, COMMAND_STOP); + ret = sendsignal(pidfn, COMMAND_STOP, 0); #endif if (ret < 0) { #ifndef WIN32 upsdebugx(2, "SIGTERM to %s failed, retrying with SIGKILL", pidfn); if (ups->pid == -1) { - ret = sendsignalfn(pidfn, SIGKILL, ups->driver); + ret = sendsignalfn(pidfn, SIGKILL, ups->driver, 0); } else { - ret = sendsignalpid(ups->pid, SIGKILL, ups->driver); + ret = sendsignalpid(ups->pid, SIGKILL, ups->driver, 0); /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { goto clean_return; @@ -407,7 +407,7 @@ static void stop_driver(const ups_t *ups) } #else upsdebugx(2, "Stopping %s failed, retrying again", pidfn); - ret = sendsignal(pidfn, COMMAND_STOP); + ret = sendsignal(pidfn, COMMAND_STOP, 0); #endif if (ret < 0) { upslog_with_errno(LOG_ERR, "Stopping %s failed", pidfn); @@ -419,16 +419,16 @@ static void stop_driver(const ups_t *ups) for (i = 0; i < 5 ; i++) { #ifndef WIN32 if (ups->pid == -1) { - ret = sendsignalfn(pidfn, 0, ups->driver); + ret = sendsignalfn(pidfn, 0, ups->driver, 0); } else { /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { goto clean_return; } - ret = sendsignalpid(ups->pid, 0, ups->driver); + ret = sendsignalpid(ups->pid, 0, ups->driver, 0); } #else - ret = sendsignalfn(pidfn, 0, ups->driver); + ret = sendsignalfn(pidfn, 0, ups->driver, 0); #endif if (ret != 0) { upsdebugx(2, "Sending signal to %s failed, driver is finally down or wrongly owned", pidfn); @@ -440,32 +440,32 @@ static void stop_driver(const ups_t *ups) #ifndef WIN32 upslog_with_errno(LOG_ERR, "Stopping %s failed, retrying harder", pidfn); if (ups->pid == -1) { - ret = sendsignalfn(pidfn, SIGKILL, ups->driver); + ret = sendsignalfn(pidfn, SIGKILL, ups->driver, 0); } else { /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { goto clean_return; } - ret = sendsignalpid(ups->pid, SIGKILL, ups->driver); + ret = sendsignalpid(ups->pid, SIGKILL, ups->driver, 0); } #else upslog_with_errno(LOG_ERR, "Stopping %s failed, retrying again", pidfn); - ret = sendsignal(pidfn, COMMAND_STOP); + ret = sendsignal(pidfn, COMMAND_STOP, 0); #endif if (ret == 0) { for (i = 0; i < 5 ; i++) { #ifndef WIN32 if (ups->pid == -1) { - ret = sendsignalfn(pidfn, 0, ups->driver); + ret = sendsignalfn(pidfn, 0, ups->driver, 0); } else { /* reap zombie if this child died */ if (waitpid(ups->pid, NULL, WNOHANG) == ups->pid) { goto clean_return; } - ret = sendsignalpid(ups->pid, 0, ups->driver); + ret = sendsignalpid(ups->pid, 0, ups->driver, 0); } #else - ret = sendsignalfn(pidfn, 0, ups->driver); + ret = sendsignalfn(pidfn, 0, ups->driver, 0); #endif if (ret != 0) { upsdebugx(2, "Sending signal to %s failed, driver is finally down or wrongly owned", pidfn); diff --git a/include/common.h b/include/common.h index 1a1e693149..9313245d63 100644 --- a/include/common.h +++ b/include/common.h @@ -268,9 +268,9 @@ pid_t parsepid(const char *buf); /* send a signal to another running NUT process */ #ifndef WIN32 -int sendsignal(const char *progname, int sig); +int sendsignal(const char *progname, int sig, int check_current_progname); #else -int sendsignal(const char *progname, const char * sig); +int sendsignal(const char *progname, const char * sig, int check_current_progname); #endif int snprintfcat(char *dst, size_t size, const char *fmt, ...) @@ -281,7 +281,7 @@ pid_t get_max_pid_t(void); /* send sig to pid after some sanity checks, returns * -1 for error, or zero for a successfully sent signal */ -int sendsignalpid(pid_t pid, int sig, const char *progname); +int sendsignalpid(pid_t pid, int sig, const char *progname, int check_current_progname); /* open , get the pid, then send it * returns zero for successfully sent signal, @@ -293,11 +293,15 @@ int sendsignalpid(pid_t pid, int sig, const char *progname); #ifndef WIN32 /* open , get the pid, then send it * if executable process with that pid has suitable progname + * (specified or that of the current process, depending on args: + * most daemons request to check_current_progname for their other + * process instancees, but upsdrvctl which manages differently + * named driver programs does not request it) */ -int sendsignalfn(const char *pidfn, int sig, const char *progname); +int sendsignalfn(const char *pidfn, int sig, const char *progname, int check_current_progname); #else /* No progname here - communications via named pipe */ -int sendsignalfn(const char *pidfn, const char * sig, const char *progname_ignored); +int sendsignalfn(const char *pidfn, const char * sig, const char *progname_ignored, int check_current_progname_ignored); #endif const char *xbasename(const char *file); diff --git a/scripts/Windows/wininit.c b/scripts/Windows/wininit.c index 6c81929f8f..abb6643090 100644 --- a/scripts/Windows/wininit.c +++ b/scripts/Windows/wininit.c @@ -193,7 +193,7 @@ static void run_upsd(void) static void stop_upsd(void) { - if (sendsignal(UPSD_PIPE_NAME, COMMAND_STOP)) { + if (sendsignal(UPSD_PIPE_NAME, COMMAND_STOP, 0)) { print_event(LOG_ERR, "Error stopping upsd (%d)", GetLastError()); } } @@ -215,7 +215,7 @@ static void run_upsmon(void) static void stop_upsmon(void) { - if (sendsignal(UPSMON_PIPE_NAME, COMMAND_STOP)) { + if (sendsignal(UPSMON_PIPE_NAME, COMMAND_STOP, 0)) { print_event(LOG_ERR, "Error stopping upsmon (%d)", GetLastError()); } } diff --git a/server/upsd.c b/server/upsd.c index 63483f8d76..833b269395 100644 --- a/server/upsd.c +++ b/server/upsd.c @@ -2049,14 +2049,14 @@ int main(int argc, char **argv) */ if (oldpid < 0) { - cmdret = sendsignalfn(pidfn, cmd, progname); + cmdret = sendsignalfn(pidfn, cmd, progname, 1); } else { - cmdret = sendsignalpid(oldpid, cmd, progname); + cmdret = sendsignalpid(oldpid, cmd, progname, 1); } #else /* if WIN32 */ if (cmd) { /* Command the running daemon, it should be there */ - cmdret = sendsignal(UPSD_PIPE_NAME, cmd); + cmdret = sendsignal(UPSD_PIPE_NAME, cmd, 1); } else { /* Starting new daemon, check for competition */ mutex = CreateMutex(NULL, TRUE, UPSD_PIPE_NAME); From dc099e8e57729a221e8343fdf1449ee0466e06a3 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 15 Jun 2024 17:08:58 +0200 Subject: [PATCH 65/65] README.adoc: add a Star History Chart badge/image Signed-off-by: Jim Klimov --- README.adoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.adoc b/README.adoc index 2164db7f4a..c507647fd5 100644 --- a/README.adoc +++ b/README.adoc @@ -217,6 +217,8 @@ which the larger potential sponsors consider when choosing how to help FOSS projects. Keeping the lights shining in such a large non-regression build matrix is a big undertaking! +image:https://api.star-history.com/svg?repos=networkupstools/nut&type=Date[link="https://star-history.com/#networkupstools/nut&Date" alt="Star History Chart"] + See <> for an overview of the shared effort. =====