From 43b0de07c145ddc7cb3de301be45946690f18c53 Mon Sep 17 00:00:00 2001 From: Yifeng Li Date: Mon, 31 Aug 2026 20:33:09 +0000 Subject: [PATCH 1/6] huawei-ups2000: fix broken instant command invocations, close #3603. A serious regression was introduced during the "fightwarn" campaign that completely broke bypass.start, shutdown.return, shutdown.reboot, and shutdown.reboot.graceful. Any attempt to use them will fail, with an error logged to `syslog`: huawei-ups2000: instcmd: command [bypass.start] reg1 is negative huawei-ups2000: instcmd: command [shutdown.return] reg1 is negative huawei-ups2000: instcmd: command [shutdown.reboot] reg1 is negative huawei-ups2000: instcmd: command [shutdown.reboot.graceful] reg1 is negative Because the regression was introduced shortly after the initial driver was merged into the upstream, these instant commands do no work in any NUT version (with the exception of the earliest development build). This problem was not discovered during pre-merge test for the same reason: it's a regression that occurred after the Pull Request was merged. In huawei-ups2000, all instant commands are driven by a lookup table to consolidate the logic to a central dispatcher. Some commands are dispatched to a (*handler_func)(uint16_t reg1) function, which is used to handle commands that needs additional processing. Not all *handler_func actually reads "reg1", it's an optional argument. Because declaring the function to accept a variadic or a (void *) would be over-engineering, a simple unsigned 16-bit value is used, *handler_func may or may not use it. During the project-wide "fightwarn" refactor campaign, it was noticed that "reg1" is stored in the lookup table as "int16_t" (-1 indicates an unneeded register), but *handler_func accepts a "uint16_t". In case that *handler_func doesn't need the register ID from the lookup table, "reg1" is implicitly converted from -1 to 65535. This is harmless, since "reg1" is not actually used in this case. Unfortunately, in an attempt to make the code "safe" by suppressing compiler warning, an incorrect range check was added to the function, which refuses to invoke *handler_func if "reg1" is negative. As a result, all instant commands that don't use "reg1" are broken. These commands include bypass.start, shutdown.return, shutdown.reboot, and shutdown.reboot.graceful. To fix this problem, fix the implicit type conversion correctly by change the "reg1" data type in the lookup table from "int16_t" to "uint16_t", and remove the "negative reg1" check. As defensive programming. all *handler_func that actually need "reg1" would check and abort if "reg1" is 0. Fixes: e9f02e242acf ("drivers/huawei-ups2000.c: instcmd(): range-check and cast for ups2000_write_register() and handler_func()") Signed-off-by: Yifeng Li --- drivers/huawei-ups2000.c | 41 +++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/drivers/huawei-ups2000.c b/drivers/huawei-ups2000.c index b04c873a29..4f71f5b333 100644 --- a/drivers/huawei-ups2000.c +++ b/drivers/huawei-ups2000.c @@ -1446,14 +1446,17 @@ static int ups2000_delay_set(const char *var, const char *string) * * 3. Calling "*handler_func" and passing "reg1". This is * used to handle commands that needs additional processing. - * If "reg1" is not necessary or unsuitable, "-1" is used. + * If "reg1" is not necessary or unsuitable, "0" is used. */ -#define REG_NULL -1, -1 +#define REG_NULL 0, -1 #define FUNC_NULL NULL static struct ups2000_cmd_t { const char *cmd; - const int16_t reg1, val1, reg2, val2; + const uint16_t reg1; + const int16_t val1; + const uint16_t reg2; + const int16_t val2; int (*const handler_func)(const uint16_t); } ups2000_cmd[] = { @@ -1471,7 +1474,7 @@ static struct ups2000_cmd_t { { "shutdown.return", REG_NULL, REG_NULL, ups2000_instcmd_shutdown_return }, { "shutdown.reboot", REG_NULL, REG_NULL, ups2000_instcmd_shutdown_reboot }, { "shutdown.reboot.graceful", REG_NULL, REG_NULL, ups2000_instcmd_shutdown_reboot_graceful }, - { NULL, -1, -1, -1, -1, NULL }, + { NULL, 0, -1, 0, -1, NULL }, }; @@ -1510,15 +1513,9 @@ static int instcmd(const char *cmd, const char *extra) if (cmd_action->handler_func) { /* handled by a function */ - if (cmd_action->reg1 < 0) { - /* FIXME: ...INSTCMD_CONVERSION_FAILED ? */ - upslogx(LOG_INSTCMD_UNKNOWN, "instcmd: command [%s] reg1 is negative", cmd); - return STAT_INSTCMD_UNKNOWN; - } else { - status = cmd_action->handler_func((uint16_t)cmd_action->reg1); - } + status = cmd_action->handler_func(cmd_action->reg1); } - else if (cmd_action->reg1 >= 0 && cmd_action->val1 >= 0) { + else if (cmd_action->reg1 > 0 && cmd_action->val1 >= 0) { /* handled by a register write */ int r = ups2000_write_register(modbus_ctx, 10000 + cmd_action->reg1, @@ -1532,7 +1529,7 @@ static int instcmd(const char *cmd, const char *extra) * if the previous write succeeds and there is an additional * register to write. */ - if (r == 1 && cmd_action->reg2 >= 0 && cmd_action->val2 >= 0) { + if (r == 1 && cmd_action->reg2 > 0 && cmd_action->val2 >= 0) { r = ups2000_write_register(modbus_ctx, 10000 + cmd_action->reg2, (uint16_t)cmd_action->val2); @@ -1559,6 +1556,12 @@ static int ups2000_instcmd_load_on(const uint16_t reg) int r; const char *status; + if (reg == 0) { + upslogx(LOG_INSTCMD_FAILED, + "invalid register in LUT, please file a bug report!"); + return STAT_INSTCMD_FAILED; + } + /* force refresh UPS status */ status_init(); r = ups2000_update_status(); @@ -1650,6 +1653,12 @@ static int ups2000_instcmd_beeper_toggle(const uint16_t reg) int r; const char *string; + if (reg == 0) { + upslogx(LOG_INSTCMD_FAILED, + "invalid register in LUT, please file a bug report!"); + return STAT_INSTCMD_FAILED; + } + r = ups2000_beeper_get(reg); if (r != 0) return STAT_INSTCMD_FAILED; @@ -1678,6 +1687,12 @@ static int ups2000_instcmd_shutdown_stayoff(const uint16_t reg) uint16_t val; int r; + if (reg == 0) { + upslogx(LOG_INSTCMD_FAILED, + "invalid register in LUT, please file a bug report!"); + return STAT_INSTCMD_FAILED; + } + r = setvar("ups.start.auto", "no"); if (r != STAT_SET_HANDLED) return STAT_INSTCMD_FAILED; From d1ddfb3e9f2335ffa89a685612ff40de310f6783 Mon Sep 17 00:00:00 2001 From: Yifeng Li Date: Mon, 31 Aug 2026 22:29:48 +0000 Subject: [PATCH 2/6] huawei-ups2000: use modbus_write_register, not modbus_write_registers, fix #3593. On some Huawei UPS2000 models, no instant commands or variable writes are possible, because modbus_write_registers() doesn't work correctly. Upon investigation, Huawei UPS2000 doesn't support multiple-register writes at all (Modbus command 0x10), the official datasheet only supports single- register writes (Modbus command 0x06). Multiple-register writes worked on some models, but this turned out to be undefined behavior, it doesn't work with all models. This commit switches modbus_write_registers() to modbus_write_register() to fix the problem. Signed-off-by: Yifeng Li --- drivers/huawei-ups2000.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/drivers/huawei-ups2000.c b/drivers/huawei-ups2000.c index 4f71f5b333..10442be6c1 100644 --- a/drivers/huawei-ups2000.c +++ b/drivers/huawei-ups2000.c @@ -162,7 +162,6 @@ static void ups2000_device_identification(void); static size_t ups2000_read_serial(uint8_t *buf, size_t buf_len); static int ups2000_read_registers(modbus_t *ctx, int addr, int nb, uint16_t *dest); static int ups2000_write_register(modbus_t *ctx, int addr, uint16_t val); -static int ups2000_write_registers(modbus_t *ctx, int addr, int nb, uint16_t *src); static uint16_t crc16(uint8_t *buffer, size_t buffer_length); static time_t time_seek(time_t t, int seconds); @@ -1731,8 +1730,12 @@ static int ups2000_shutdown_guaranteed_return(uint16_t offdelay, uint16_t ondela val[0] = (offdelay * 10) / 60; val[1] = ondelay / 60; - r = ups2000_write_registers(modbus_ctx, 1047 + 10000, 2, val); - if (r != 2) + r = ups2000_write_register(modbus_ctx, 1047 + 10000, val[0]); + if (r != 1) + return STAT_INSTCMD_FAILED; + + r = ups2000_write_register(modbus_ctx, 1048 + 10000, val[1]); + if (r != 1) return STAT_INSTCMD_FAILED; return STAT_INSTCMD_HANDLED; @@ -2061,7 +2064,14 @@ static int ups2000_read_registers(modbus_t *ctx, int addr, int nb, uint16_t *des } -static int ups2000_write_registers(modbus_t *ctx, int addr, int nb, uint16_t *src) +/* + * Huawei UPS2000 doesn't officially support multiple-register writes (Modbus + * command 0x10), the official datasheet only supports single-register writes + * (Modbus command 0x06). Multiple-register writes worked on some models, but + * this turned out to be undefined behavior, it doesn't work with all models. + * This is why this function is not symmetric to ups2000_read_registers(). + */ +static int ups2000_write_register(modbus_t *ctx, int addr, uint16_t val) { int i; int r = -1; @@ -2072,24 +2082,24 @@ static int ups2000_write_registers(modbus_t *ctx, int addr, int nb, uint16_t *sr "Please file a bug report!", addr); for (i = 0; i < 3; i++) { - r = modbus_write_registers(ctx, addr, nb, src); + r = modbus_write_register(ctx, addr, val); /* generic retry for modbus write failures. */ - if (retry_status == RETRY_ENABLE && r != nb) { - upslogx(LOG_WARNING, "modbus_write_registers() failed (%d, errno %d): %s", + if (retry_status == RETRY_ENABLE && r != 1) { + upslogx(LOG_WARNING, "modbus_write_register() failed (%d, errno %d): %s", r, errno, modbus_strerror(errno)); upslogx(LOG_WARNING, "Register %04d has a write failure. Retrying...", addr); sleep(1); continue; } - else if (r == nb) + else if (r == 1) retry_status = RETRY_ENABLE; return r; } /* Give up */ - upslogx(LOG_ERR, "modbus_write_registers() failed (%d, errno %d): %s", + upslogx(LOG_ERR, "modbus_write_register() failed (%d, errno %d): %s", r, errno, modbus_strerror(errno)); upslogx(LOG_ERR, "Register %04d has a fatal write failure.", addr); retry_status = RETRY_DISABLE_TEMPORARY; @@ -2097,12 +2107,6 @@ static int ups2000_write_registers(modbus_t *ctx, int addr, int nb, uint16_t *sr } -static int ups2000_write_register(modbus_t *ctx, int addr, uint16_t val) -{ - return ups2000_write_registers(ctx, addr, 1, &val); -} - - /* * The following CRC-16 code was copied from libmodbus. * From b35e14df8c9303ed2e2194d19c166fad3af636af Mon Sep 17 00:00:00 2001 From: Yifeng Li Date: Mon, 31 Aug 2026 22:37:48 +0000 Subject: [PATCH 3/6] huawei-ups2000: bump driver version to v0.14. Signed-off-by: Yifeng Li --- drivers/huawei-ups2000.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/huawei-ups2000.c b/drivers/huawei-ups2000.c index 10442be6c1..6de21ea9e9 100644 --- a/drivers/huawei-ups2000.c +++ b/drivers/huawei-ups2000.c @@ -55,7 +55,7 @@ #endif #define DRIVER_NAME "NUT Huawei UPS2000 (1kVA-3kVA) RS-232 Modbus driver (libmodbus link type: " NUT_MODBUS_LINKTYPE_STR ")" -#define DRIVER_VERSION "0.13" +#define DRIVER_VERSION "0.14" #define CHECK_BIT(var,pos) ((var) & (1<<(pos))) #define MODBUS_SLAVE_ID 1 From 2377d8388f5808f3bdb44d2a7e9f18ff104454f3 Mon Sep 17 00:00:00 2001 From: Yifeng Li Date: Mon, 31 Aug 2026 22:58:28 +0000 Subject: [PATCH 4/6] docs/man/huawei-ups2000.txt: document bug #3593 and bug #3603. Two bugs #3593, #3603 can prevent users from sending any instant commands or changing any variables at all. Document these known bugs in the man page. Signed-off-by: Yifeng Li --- docs/man/huawei-ups2000.txt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/man/huawei-ups2000.txt b/docs/man/huawei-ups2000.txt index 72a6238a26..c294fbdbf1 100644 --- a/docs/man/huawei-ups2000.txt +++ b/docs/man/huawei-ups2000.txt @@ -375,6 +375,16 @@ to endless read failures and permanent data stalls. Driver v0.12 (NUT v2.8.5) fixed support for Microsoft Windows, Windows was previously unsupported due to a file locking bug. +Driver v0.14 (NUT v2.8.6) fixed two serious problems that affected all +previous NUT versions, upgrading is strongly recommended. Due to a broken +instant command handler, it was impossible to send some instant commands +to the UPS, including *bypass.start*, *shutdown.return*, *shutdown.reboot*, +and *shutdown.reboot.graceful*, any attempt to use them will fail with an +"reg1 is negative" error in the syslog. Due to a separate Modbus compatibility +problem, the negative impact is worse on some UPS models: it was not possible +to *write to any registers at all*, meaning that changing any *variable* or +sending any *instant command* is impossible. + See *Historical bugs* at the bottom of this manual for links. Battery status has a non-fatal read failure @@ -492,6 +502,12 @@ https://github.com/networkupstools/nut/issues/3244 * Commit: call ser_close() before modbus_connect(), fix #3244: https://github.com/networkupstools/nut/commit/db2e148503d654726d5520d68e20e71117335813 +* Issue 3593: huawei-ups2000: shutdown commands never power off the UPS on firmware V2R1C1SPC50 - the unit ignores Modbus function 0x10 writes; only 0x06 works +https://github.com/networkupstools/nut/issues/3593 + +* Issue 3603: huawei-ups2000: bypass.start, shutdown.return, shutdown.reboot, and shutdown.reboot.graceful are broken in all NUT versions +https://github.com/networkupstools/nut/issues/3603 + Windows Drivers: ~~~~~~~~~~~~~~~~ From 8656922e08a910921490aedca2d4d865186bcb21 Mon Sep 17 00:00:00 2001 From: Yifeng Li Date: Tue, 1 Sep 2026 08:28:10 +0000 Subject: [PATCH 5/6] NEWS.adoc: huawei-ups2000 instant commands and Modbus fixes [#3593, #3603] Signed-off-by: Yifeng Li --- NEWS.adoc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/NEWS.adoc b/NEWS.adoc index 8da073ce8e..8ab6ff846d 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -117,6 +117,21 @@ https://github.com/networkupstools/nut/milestone/13 authentication configuration discovery. It accepts `default`, `none`, or a specific authconf file path. [issue #3329] + - `huawei-ups2000` driver updates: + * Fixed a Modbus compatibility problem that made changing any variables + or sending any instant commands impossible with some UPS models. This + was caused by the use of `modbus_write_registers()` (Modbus command + `0x10`), which is not officially supported, making any register write + impossible on some models. All uses of `modbus_write_registers()` have + been changed to `modbus_write_register()` (Modbus command `0x06`) to + avoid this problem. [issue #3593, PR #3604]. + + * Fixed a serious problem previously existed in all NUT versions that + made many instant commands unusable, including `bypass.start`, + `shutdown.return`, `shutdown.reboot`, and `shutdown.reboot.graceful`. + Any attempt to use them would fail, with an error logged to `syslog`: + `reg1 is negative`. [issue #3603, PR #3604]. + - `nhs_ser` driver updates: * Modernized serial communication and added validated settings for baud rate, data bits, parity, stop bits, flow control, per-byte read timeout, From 37c545c9fb77ea3113ed8d98a7b9e3fbb511936d Mon Sep 17 00:00:00 2001 From: Yifeng Li Date: Tue, 1 Sep 2026 08:29:20 +0000 Subject: [PATCH 6/6] UPGRADING.adoc: huawei-ups2000 instant commands and Modbus fixes [#3593, #3603] Signed-off-by: Yifeng Li --- UPGRADING.adoc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/UPGRADING.adoc b/UPGRADING.adoc index 9a01aee6cf..0485efadcb 100644 --- a/UPGRADING.adoc +++ b/UPGRADING.adoc @@ -87,6 +87,16 @@ The new `-A filename` option defaults to trying to use a `nutauth.conf` file device, please add `cypress_drain_quirk = 0` in your `ups.conf` and let us know. [issue #2534, PR #3563] +- The `huawei-ups2000` driver fixed two serious problems that affected all + previous NUT versions. All `huawei-ups2000` users are strongly recommended + to upgrade to NUT 2.8.6, all previous versions are considered too unreliable + to use. Due to a Modbus compatibility problem, it was not possible to *write + to any registers at all* on some UPS models, meaning that changing any + *variable* or sending any *instant command* was impossible. Furthermore, due + to a broken instant command handler, some instant commands would always fail + with a `reg1 is negative` error (even with a compatible UPS), including + `bypass.start`, `shutdown.return`, `shutdown.reboot`, and + `shutdown.reboot.graceful`. [issue #3593, #3603, PR #3604]. Changes from 2.8.4 to 2.8.5 ---------------------------