From 1d88494bd075021ac0c962a9bf0dad20db9d109c Mon Sep 17 00:00:00 2001 From: TomZanna Date: Tue, 29 Sep 2026 09:04:34 +0200 Subject: [PATCH 01/13] docs: Document optional value argument for upscmd Add the optional 'value' argument to the upscmd man page synopsis and describe both the 'command' and 'value' parameters. Signed-off-by: TomZanna --- docs/man/upscmd.txt | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/man/upscmd.txt b/docs/man/upscmd.txt index 4c74a628dc..8244dac304 100644 --- a/docs/man/upscmd.txt +++ b/docs/man/upscmd.txt @@ -13,7 +13,7 @@ SYNOPSIS *upscmd* -l 'ups' -*upscmd* [-A 'authconf'] [-u 'username'] [-p 'password'] [-w] [-t ] 'ups' 'command' +*upscmd* [-A 'authconf'] [-u 'username'] [-p 'password'] [-w] [-t ] 'ups' 'command' ['value'] DESCRIPTION ----------- @@ -68,6 +68,14 @@ Set a timeout when using *-w*. Defaults to 10 seconds. Connect to this UPS. The format is `upsname[@hostname[:port]]`. The default hostname is "localhost". +'command':: +Valid instant command, for example 'test.panel.start'. + +'value':: +Optional additional data for the command, such as a number of seconds taken +by commands like 'shutdown.return' or 'load.off.delay'. Not all drivers and +commands support a value; those which do not would ignore it. + COMMON OPTIONS -------------- From 695e82c6af92d46f81490579937407845e084026 Mon Sep 17 00:00:00 2001 From: TomZanna Date: Tue, 29 Sep 2026 12:46:04 +0200 Subject: [PATCH 02/13] feat(tripplitesu): add outlet count, watchdog status and delay options Document and implement command_delay automatic timeout handling, plus offdelay and startdelay parameters, and expose outlet.count and ups.watchdog.status variables. Signed-off-by: TomZanna --- NEWS.adoc | 37 ++ data/cmdvartab | 2 + docs/man/tripplitesu.txt | 244 ++++++++- docs/nut.dict | 3 +- drivers/tripplitesu.c | 1049 ++++++++++++++++++++++++++++++++++---- 5 files changed, 1227 insertions(+), 108 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index f04a9a2e77..aa4e68799b 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -360,6 +360,43 @@ https://github.com/networkupstools/nut/milestone/13 - `riello_usb`, `riello_ser` driver updates: * riello_usb, riello_ser: report NAK on RE as failure [issue #3546] + - `tripplitesu` driver updates: + * Added the `offdelay` and `startdelay` settings in `ups.conf` (in + seconds and minutes respectively, the units of the two device + commands) and support for an optional delay (in seconds), passed as + the `value` argument of the `shutdown.return`, `shutdown.reboot` and + `shutdown.reboot.graceful` instant commands, so that the timer can be + chosen per invocation without restarting the driver or changing its + configuration. + * Added control of the individual outlet banks through the + `outlet.n.load.off` and `outlet.n.load.on` instant commands, and the + matching `outlet.count`, `outlet.n.status` and `outlet.n.switchable` + variables. + * Added the `shutdown.stayoff` instant command, and made the + shutdown-related commands keep the device return delay consistent with + what they mean: the two reboot commands ask for the load to come back + right behind the shutdown (the device only restarts the load when the + restart delay is longer than the shutdown delay), while + `shutdown.return` and `shutdown.stayoff` clear it, so that the load + comes back only when mains is (re)applied, or not at all. + * Added the read/write variables `ups.beeper.status`, `ups.delay.start`, + `ups.start.auto`, `ups.watchdog.status`, `outlet.delay.start` and + `input.eco.switchable`, the `battery.date` variable, the `beeper.mute` + and `clear.fault.record` instant commands, and validation of + `input.transfer.low`/`.high` against the ranges reported by the device + (published as `input.transfer.low.min`/`.max` and + `input.transfer.high.min`/`.max`). + * Instant commands and variable writes now report a failure when the + device rejects or does not answer one of the operations they perform, + instead of always reporting success. + * Added the `command_delay` setting in `ups.conf` as a minimum interval + between two commands sent to the UPS, in milliseconds, to work around + units that do not answer a command sent too soon after the previous + one. The interval is measured from the previous command, so an idle + link is not penalized, and the special value `-1` makes the driver + start without waiting and pace commands by one second only if one of + them times out. [PR #3273] + - `usbhid-ups` driver updates: * When reconnecting, report success more visibly (not only in debug), and do not reset driver state to "quiet" when it will loop trying. [PR #3423] diff --git a/data/cmdvartab b/data/cmdvartab index e13c1649f7..4a1a0ce48c 100644 --- a/data/cmdvartab +++ b/data/cmdvartab @@ -43,6 +43,7 @@ VARDESC ups.start.auto "UPS starts when mains is (re)applied" VARDESC ups.start.battery "Allow to start UPS from battery" VARDESC ups.start.reboot "UPS reboots when power returns during shutdown delay" VARDESC ups.shutdown "Enable or disable UPS shutdown ability (poweroff)" +VARDESC ups.watchdog.status "UPS watchdog status (enabled or disabled)" VARDESC input.voltage "Input voltage (V)" VARDESC input.voltage.extended "Extended input voltage range" @@ -168,6 +169,7 @@ VARDESC ambient.contacts.2.status "State of the dry contact sensor 2" # # VARDESC outlet.[[:digit:]]+.id "Outlet system identifier ()" +VARDESC outlet.count "Total number of outlets" VARDESC outlet.id "Outlet system identifier" VARDESC outlet.desc "Outlet description" VARDESC outlet.switch "Outlet switch control" diff --git a/docs/man/tripplitesu.txt b/docs/man/tripplitesu.txt index ae2ec8f4d5..e26884b070 100644 --- a/docs/man/tripplitesu.txt +++ b/docs/man/tripplitesu.txt @@ -34,13 +34,245 @@ initiated by linkman:upsmon[8]. By default, the UPS may not report low battery until there are only a few seconds left. Common values are around 25--30. *command_delay*='milliseconds':: -Set a delay in milliseconds to be applied before each command sent to the UPS. -The default value is 0 (disabled). If experiencing intermittent communication -timeouts when multiple commands are sent in rapid succession, set this to -1000 (1 second) or adjust as needed. +Set the minimum interval to keep between two commands sent to the UPS, in +milliseconds. The default is 0, which sends a command as soon as the +previous answer has been read. ++ +Some units are slow to digest a command: one sent too soon after the previous +one is not answered, and the driver reports a read timeout. Setting this to +1000 (one second) paces the driver; smaller values such as 100 may be enough. +The interval is measured from the previous command, so the first command of a +poll burst, and any command sent after a slow exchange, do not wait. The +parameter as configured is published as *driver.parameter.command_delay*. ++ +Note that the driver sends close to thirty commands while initializing +(including the first poll cycle), so a large interval also delays the first +published values: half a minute with 1000. ++ +A value of -1 selects the automatic behaviour: the driver starts without +waiting and, when a command times out, raises the interval to 1000 +milliseconds once for the rest of the run, logging a warning. An explicit +value is never changed, and 0 (or leaving the parameter unset) disables the +feature. ++ +For reference, the vendor software paces its own commands by 1.5 to 3 +seconds, so values in that range are known to be safe on this hardware. -This delay helps prevent "read timeout" errors when the hardware buffer is -not ready to accept new commands immediately after completing the previous one. +*offdelay*='num':: +Set the delay, in seconds, used by the shutdown-related instant commands +when no value is passed to them. The default value is 10. ++ +The value may be overridden for a single invocation with the optional +'value' argument of linkman:upscmd[8], for example: ++ + upscmd -u admin -p secret myups shutdown.return 45 ++ +The value passed on the command line takes precedence over this setting, +which in turn takes precedence over the built-in default. ++ +The driver shutdown handling (the 'shutdown.default' instant command and +linkman:upsdrvctl[8] 'shutdown') uses this setting as well, but keeps the +historical default of 5 seconds when it is not set in linkman:ups.conf[5]. + +*startdelay*='minutes':: +Set the return (restart) delay, in minutes, used by the driver shutdown +handling as the delay before the load is started again. The default value +is 1. ++ +The parameter uses the unit of the device command ('SDR', whole minutes); +values outside the range are rejected. The same delay is also exposed, in +seconds, as the read/write variable *ups.delay.start* (see VARIABLES below), +where a value that is not a whole number of minutes is rounded up when it is +sent to the device. ++ +The 'shutdown.return' and 'shutdown.stayoff' commands do not use this +setting: they clear the return delay instead (see the INSTANT COMMANDS +section), so that the load comes back only when mains is (re)applied, or not +at all. + +INSTANT COMMANDS +---------------- + +The following instant commands switch the outlet banks. They take no value: +a delayed shutdown of the whole UPS is what the 'shutdown.*' commands below +are for. + +'load.off':: + Switch off every controllable outlet bank immediately. + +'load.on':: + Switch on every controllable outlet bank immediately. + +The following instant commands accept an optional delay, in seconds, passed +as the 'value' argument of linkman:upscmd[8]: + +'shutdown.return' 'seconds':: + Turn off the load after the given number of seconds, so that the + computer can finish shutting down first. The load is started again + when mains is (re)applied, not after a delay of its own (the return + delay of any earlier command is cleared first); use + 'shutdown.reboot' for a timed restart. Defaults to the 'offdelay' + setting. + +'shutdown.reboot' 'seconds':: + Shut down the load after the given number of seconds and start it + again once it has been switched off (see the note below). Defaults to + the 'offdelay' setting. + +'shutdown.reboot.graceful' 'seconds':: + Same as 'shutdown.reboot', but with a default of 60 seconds, which is + not affected by the 'offdelay' setting. + +'shutdown.stayoff' 'seconds':: + Turn off the load after the given number of seconds and leave it off + even if mains returns (the return delay of any earlier command is + cleared first). Defaults to the 'offdelay' setting. + +The accepted range is 1 through 3600 seconds. An invalid value makes the +command fail without sending anything to the UPS. + +All other instant commands ignore any value passed to them. + +NOTE: The device takes the shutdown delay in seconds (its 'SDA' command) and +the restart delay in whole minutes ('SDR'), and it only starts the load +again when the restart delay is longer than the shutdown delay. The two +reboot commands therefore add one second to the shutdown delay before +rounding it up to whole minutes, so that the restart always falls behind +the off, while 'shutdown.return' and 'shutdown.stayoff' clear the restart +delay, so that the load does not come back on a timer of its own. The +driver shutdown handling sends the configured 'startdelay' instead: as the +device only restarts the load when that delay is longer than the shutdown +delay, choose a 'startdelay' longer than the 'offdelay' used there (or than +the historical 5 seconds, when 'offdelay' is not set). + +A command that is reported as failed had at least one of the operations it +performs rejected by the device (or unanswered), so a partially applied +command is possible and worth re-checking with linkman:upsc[8]. + +The following instant commands control a single outlet bank, where 'n' +ranges from 1 to the number of banks reported by the device (see +*outlet.count* below). They take no value: + +'outlet.n.load.off':: + Switch off outlet bank n immediately. + +'outlet.n.load.on':: + Switch on outlet bank n immediately. + +The following instant commands control the device buzzer. They take no +value; enabling/disabling the buzzer for good is done through the +*ups.beeper.status* variable instead: + +'beeper.mute':: + Silence the alarm that is currently beeping, without disabling the + buzzer for future alarms. This requires the alarm to be active: on a + quiet device it has no audible effect. + +'clear.fault.record':: + Clear the bypass reason recorded by the device. This is a one-shot + action, so the recorded reason is not published as a variable. + +VARIABLES +--------- + +The driver publishes the usual battery, input and output variables, plus +the following ones: + +*outlet.count*:: + Total number of controllable outlet banks. + +*outlet.n.status*:: + Status of outlet bank n, either 'on' or 'off', as reported by the + device relay status. This is the state of the bank relay, not a + measure of the voltage present on the outlet: with the whole UPS + output off, the reported value can still be 'on'. + +*outlet.n.switchable*:: + Set to 'yes' for every controllable outlet bank. + +*outlet.delay.start*:: + Read/write power-on (boot) delay, in seconds: how long the device waits + before it starts delivering voltage to its outputs (turning the + inverter on and closing the load relays), both when it is powered up + and when it is asked to switch the outputs on. This is a device + setting, not an action, so it has no matching instant command and it is + only read and written with linkman:upsrw[8]. The accepted range is + 0 through 999 seconds and, as this is the 'main outlet' form of the + variable, it applies to all outlets. Note that this is the delay used + by the hardware itself, which is different from *ups.delay.start* (the + return delay of the shutdown sequence, in seconds) and from the + 'startdelay' setting (the same return delay, in minutes). + +*battery.date*:: + Battery installation date reported by the device, in YYYYMMDD + format. + +*ups.beeper.status*:: + Read/write buzzer setting, either 'enabled' or 'disabled'. This is a + device setting, so it has no matching instant command; the + 'beeper.mute' command instead silences the alarm that is currently + sounding (a transient state the device does not report, which is why + it never appears here). The vendor software only ever writes this + setting, so the device may not report it back: when it does not, the + published value is the last one written by the driver, starting from + the 'enabled' assumption. + +*ups.delay.start*:: + Read/write return delay, in seconds, used by the driver shutdown + handling (the 'shutdown.default' instant command and + linkman:upsdrvctl[8] 'shutdown'). Values are rounded up to whole + minutes when they are sent to the device and, since the corresponding + device command is write-only, the variable is not read back from the + UPS: after a driver restart it reflects the 'startdelay' setting. + +*ups.start.auto*:: + Read/write auto-reboot setting, either 'yes' or 'no': when it is 'yes' + the device starts again by itself once mains is (re)applied, while + 'no' leaves the outputs off. As required by the NUT naming scheme, + the shutdown-related instant commands set this value to what they + need before running: 'shutdown.return', 'shutdown.reboot' and + 'shutdown.reboot.graceful' (and the driver shutdown handler) switch it + to 'yes', while 'shutdown.stayoff' switches it to 'no'. ++ +WARNING: The device stores this setting, so such a change outlives the +shutdown that needed it and also applies to the following sessions. The +driver logs every change it makes (with the previous value); use +linkman:upsrw[8] to restore your preferred value, for example: ++ + upsrw -s ups.start.auto=no myups + +*ups.watchdog.status*:: + Read/write watchdog setting, either 'enabled' or 'disabled': the + device reboots the load when it does not hear from the driver for the + watchdog timeout. A timeout of 0 seconds means 'disabled', and + setting the variable to 'enabled' arms the watchdog with the last + timeout reported by the device (255 seconds when none was ever + reported), keeping the restart-alarm behaviour the device uses. Note + that the timeout itself is not exposed as a separate variable, + because there is no name for it in the NUT naming scheme. ++ +WARNING: Enabling this setting arms a timer the device uses to power cycle +the load when the driver stops talking to it, so a driver that hangs (or a +serial link that breaks) while the watchdog is armed can switch the load off +and on again. ++ +The device reports the timeout and the restart-alarm flag, not a watchdog +status: 'enabled' and 'disabled' are inferred from a non-zero timeout, which +has not been verified on hardware. + +*input.eco.switchable*:: + Read/write high efficiency (ECO) mode selection, either 'ECO' or + 'normal'. The vendor software only ever writes this setting, so the + device may not report it back: when it does not, the published value + is the last one written by the driver, starting from the 'normal' + assumption. + +*input.transfer.low*, *input.transfer.high*:: + Read/write low and high voltage transfer points, in V. Values are + validated against the range reported by the device (see + *input.transfer.low.min*, *input.transfer.low.max*, + *input.transfer.high.min* and *input.transfer.high.max*, published + when the device reports them) and are read back after each change. AUTHOR ------ diff --git a/docs/nut.dict b/docs/nut.dict index ce96e4546d..b1eb1ac22e 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3878 utf-8 +personal_ws-1.1 en 3879 utf-8 AAC AAS ABI @@ -1634,6 +1634,7 @@ YQ YV YY YYYY +YYYYMMDD YZ Ygor Yifeng diff --git a/drivers/tripplitesu.c b/drivers/tripplitesu.c index 3fd1828fae..1700fb5607 100644 --- a/drivers/tripplitesu.c +++ b/drivers/tripplitesu.c @@ -47,64 +47,89 @@ company (if either) originated the protocol. Picking one of the companies arbitrarily to name it after just seems wrong. - There are a few things still to add. Primarily there's control of - individual outlet banks, using (I presume) outlet.n.x variables. - I'll probably wait for an example of that to show up in some other - driver before adding that, to try to make sure I do it in the way - that Russell Kroll envisioned. It also might be nice to give the - user control over the delays before shutdown and restart, probably - with additional driver parameters. Letting the user turn the buzzer - off might be nice too; for that I'd have to investigate whether the - command to do that disables it entirely, or just turns it off during - the existing alarm condition, to determine whether it would be better - implemented through variables or instant commands, and then of course - request that those get added to the list of known names. Finally, - there are a number of other alarm conditions that can be reported - that would be nice to pass on to the user; these would require new - variables or new status values. + There are a few things still to add. There are a number of other alarm + conditions that can be reported that would be nice to pass on to the + user; these would require new variables or new status values. The following parameters (ups.conf) are supported: lowbatt command_delay + offdelay + startdelay The following variables are supported (RW = read/write): ambient.humidity (1) ambient.temperature (1) battery.charge battery.current (1) + battery.date battery.temperature battery.voltage battery.voltage.nominal input.frequency + input.eco.switchable (RW) input.sensitivity (RW) (1) input.transfer.high (RW) input.transfer.low (RW) input.voltage input.voltage.nominal + outlet.count + outlet.delay.start (RW) + outlet.n.status + outlet.n.switchable output.current (1) output.frequency output.voltage output.voltage.nominal + ups.beeper.status (RW) + ups.delay.start (RW) ups.firmware ups.id (RW) (1) ups.load ups.mfr ups.model ups.status + ups.start.auto (RW) ups.test.result + ups.watchdog.status (RW) ups.contacts (1) The following instant commands are supported: load.off load.on + outlet.n.load.off (one per outlet bank) + outlet.n.load.on (one per outlet bank) + beeper.mute + clear.fault.record shutdown.reboot shutdown.reboot.graceful shutdown.return + shutdown.stayoff shutdown.stop test.battery.start test.battery.stop + The shutdown-related commands above accept an optional delay, in seconds, + passed as the "value" argument of upscmd; it takes precedence over the + "offdelay" setting from ups.conf, which in turn takes precedence over + the built-in default. The "load.off", "load.on" and "outlet.n.load.*" + commands do not take a value: they only switch the outlet banks. + + "shutdown.return" and "shutdown.stayoff" clear the device return delay + (SDR), so the load comes back only when mains is (re)applied or not at + all, while the two reboot commands ask for the load to come back right + behind the shutdown (just behind it, see reboot_start_delay()). The + "startdelay" setting is used by the driver shutdown handler only. + + Because that is what the commands mean on this hardware, the shutdown + commands change the device auto-reboot setting ("ups.start.auto", ARB); + the device stores it, so the change outlives the shutdown that needs it, + and the driver logs it (restore it with upsrw if needed). The buzzer is + a device setting as well: enabling/disabling it is done through the + read/write "ups.beeper.status" variable, while the "beeper.mute" command + only silences the alarm that is currently sounding. + The following ups.status values are supported: BOOST (1) BYPASS @@ -127,7 +152,7 @@ #include "nut_stdint.h" #define DRIVER_NAME "Tripp Lite SmartOnline driver" -#define DRIVER_VERSION "0.12" +#define DRIVER_VERSION "0.13" /* driver description structure */ upsdrv_info_t upsdrv_info = { @@ -140,6 +165,64 @@ upsdrv_info_t upsdrv_info = { #define MAX_RESPONSE_LENGTH 256 +/* Delay, in seconds, used by shutdown-related instant commands when the + * caller does not pass a value and 'offdelay' is not set in ups.conf. + * Note: a shutdown action value of 0 is used to cancel a scheduled + * shutdown (see "shutdown.stop"), so it is not accepted as a delay. */ +#define DEFAULT_OFFDELAY 10U +#define MIN_OFFDELAY 1U +#define MAX_OFFDELAY 3600U + +/* Default delay, in seconds, of the graceful reboot command; unlike the + * other shutdown commands it is not affected by the 'offdelay' setting. */ +#define DEFAULT_GRACEFUL_DELAY 60U + +/* Delay, in seconds, before the load is returned (restarted) after a + * shutdown. The device command (SDR) takes minutes, so values are rounded + * up when sent; the default of one minute matches the value this driver + * used before it became configurable. Unlike the shutdown action (SDA), + * a value of 0 is meaningful here and means "return without delay", so it + * is accepted. The ups.conf parameter 'startdelay' is expressed in + * minutes (the device unit), while the ups.delay.start variable uses + * seconds, as usual for delays in NUT. */ +#define DEFAULT_STARTDELAY 60U +#define MIN_STARTDELAY 0U +#define MAX_STARTDELAY 3600U +#define MAX_STARTDELAY_MINUTES (MAX_STARTDELAY / 60U) + +/* Power-on (boot) delay: how many seconds the device waits before it starts + * delivering voltage to its outputs (inverter on, load relays closed) when + * it is powered up or asked to switch them on. This is a device setting, + * not an action, so it is only exposed as a read/write variable (no instant + * command); 0 means "no delay" and the device field is three digits wide. */ +#define DEFAULT_BOOTDELAY 0U +#define MAX_BOOTDELAY 999U + +/* Watchdog setting (WDG): the device stores a timeout in seconds (up to + * three digits, 0 disables it) plus a flag selecting whether the restart + * alarm is enabled (1) or disabled (2). The timeout and the flag are + * preserved when the watchdog is (dis)armed from NUT. */ +#define DEFAULT_WATCHDOG 255U +#define MAX_WATCHDOG 255U + +/* Sanity limit on the outlet bank count reported by the device (LET), so + * that a garbled answer cannot make the driver publish, register and poll + * an unbounded number of banks. */ +#define MAX_OUTLET_BANKS 16U + +/* Command pacing: the device needs time to digest a command before it can + * accept the next one, so a burst of commands sent back to back (as the + * driver does while initializing and polling) can make it miss answers. + * The ups.conf parameter 'command_delay' is the minimum interval to keep + * between two commands, in milliseconds: 0 disables the pacing (the + * historical behaviour), a positive value is the interval itself, and -1 + * asks the driver to work it out by itself (see note_command_timeout()). + * The vendor software paces its own commands by 1.5 to 3 seconds. */ +#define COMMAND_DELAY_AUTO (-1) +#define DEFAULT_COMMAND_DELAY 0 +#define DEFAULT_AUTO_COMMAND_DELAY 1000U +#define MAX_COMMAND_DELAY 60000U + static const char *test_result_names[] = { "No test performed", "Passed", @@ -164,10 +247,19 @@ static struct { unsigned long commands_available; } ups; -static long command_delay = 0; /* delay in milliseconds before each command, 0 = no delay by default */ - -/* bits in commands_available */ -#define WDG_AVAILABLE (1UL << 1) +static long command_delay_conf = DEFAULT_COMMAND_DELAY; /* 'command_delay' as set in ups.conf */ +static unsigned int command_delay = 0; /* minimum interval kept between commands, in milliseconds (0 = no waiting) */ +static int command_delay_auto = 0; /* 1 when 'command_delay' asked for automatic pacing (-1) */ +static int command_sent = 0; /* 1 once a command has been sent (see pace_before_command()) */ +static struct timeval command_sent_at; /* when the last command was sent */ +static unsigned int offdelay = DEFAULT_OFFDELAY; /* delay in seconds before shutdown */ +static int offdelay_from_conf = 0; /* set if 'offdelay' was provided in ups.conf */ +static unsigned int startdelay = DEFAULT_STARTDELAY; /* delay in seconds before return */ +static unsigned int bootdelay = DEFAULT_BOOTDELAY; /* power-on delay in seconds */ +static unsigned int watchdog_seconds = 0; /* watchdog timeout last reported by the device */ +static unsigned int watchdog_alarm = 2; /* watchdog restart-alarm flag last reported */ +static unsigned int min_low_transfer, max_low_transfer; /* reported transfer voltage ranges */ +static unsigned int min_high_transfer, max_high_transfer; /* message types */ #define POLL 'P' @@ -185,7 +277,8 @@ static long command_delay = 0; /* delay in milliseconds before each command, 0 = #define BUZZER_TEST "BTT" /* set */ #define BATTERY_TEST "BTV" /* poll/set */ #define BUZZER "BUZ" /* set */ -#define ECONOMIC_MODE "ECO" /* set */ +#define BYPASS_REASON "BPA" /* poll/set */ +#define ECONOMIC_MODE "ECO" /* set; poll is unproven */ #define ENABLE_BUZZER "EDB" /* set */ #define ENVIRONMENT_INFORMATION "ENV" /* poll */ #define OUTLET_RELAYS "LET" /* poll */ @@ -219,19 +312,76 @@ static long command_delay = 0; /* delay in milliseconds before each command, 0 = #define WATCHDOG "WDG" /* poll/set */ +/* Milliseconds elapsed since '*start', as a signed value ("now" is expected + * to be later than the reference time). */ +static long elapsed_milliseconds(const struct timeval *start) +{ + struct timeval now; + + gettimeofday(&now, NULL); + return (long)(now.tv_sec - start->tv_sec) * 1000L + + (long)(now.tv_usec - start->tv_usec) / 1000L; +} + +/* Wait until at least 'command_delay' milliseconds have passed since the + * previous command was sent, so that the device has time to get ready. + * Nothing is waited for on an idle link, nor when the driver work between + * two commands (a slow answer, for instance) already took that long: what + * the device needs is a minimum spacing between commands, not a pause + * before every single one of them. */ +static void pace_before_command(void) +{ + long remaining; + + if (!command_delay || !command_sent) + return; + + remaining = (long)command_delay - elapsed_milliseconds(&command_sent_at); + if (remaining <= 0) + return; + + upsdebugx(3, "%s: waiting %ld ms for the device to digest the " + "previous command", __func__, remaining); + usleep((useconds_t)(remaining * 1000L)); +} + +/* Remember when a command was sent, for pace_before_command(). */ +static void note_command_sent(void) +{ + gettimeofday(&command_sent_at, NULL); + command_sent = 1; +} + +/* The device did not answer a command in time. In automatic mode the + * interval is raised, once: a device that is slow to digest a command does + * not get faster again for the rest of the run (command_delay is 0 until + * this function raises it, so this happens only once). An explicitly + * configured value - including 0 - is never touched. Only POLL commands + * count: a SET command that goes unanswered may have been carried out all + * the same, so it says nothing about how fast the device can be driven. */ +static void note_command_timeout(char type) +{ + if (type != POLL || !command_delay_auto || command_delay) + return; + + command_delay = DEFAULT_AUTO_COMMAND_DELAY; + upslogx(LOG_WARNING, "Read timeout: waiting %u ms between commands " + "from now on (command_delay=auto); adjust 'command_delay' in " + "ups.conf to change this", command_delay); +} + + static ssize_t do_command(char type, const char *command, const char *parameters, char *response) { char buffer[SMALLBUF]; size_t count; ssize_t ret; + /* let the device get ready for the next command, then drop whatever + * it may have sent in the meantime */ + pace_before_command(); ser_flush_io(upsfd); - /* Apply configurable delay if enabled (> 0) to prevent communication timeouts */ - if (command_delay > 0) { - usleep((useconds_t)command_delay*1000); - } - if (response) { *response = '\0'; } @@ -244,6 +394,7 @@ static ssize_t do_command(char type, const char *command, const char *parameters return -1; } + note_command_sent(); upsdebugx(3, "do_command: %" PRIiSIZE " bytes sent [%s] -> OK", ret, buffer); ret = ser_get_buf_len(upsfd, (unsigned char *)buffer, 4, 3, 0); @@ -253,6 +404,7 @@ static ssize_t do_command(char type, const char *command, const char *parameters } if (ret == 0) { upsdebugx(3, "do_command: read -> TIMEOUT"); + note_command_timeout(type); return -1; } @@ -269,6 +421,7 @@ static ssize_t do_command(char type, const char *command, const char *parameters } if (ret == 0) { upsdebugx(3, "do_command: read -> TIMEOUT"); + note_command_timeout(type); return -1; } @@ -303,6 +456,7 @@ static ssize_t do_command(char type, const char *command, const char *parameters } if (ret == 0) { upsdebugx(3, "do_command: read -> TIMEOUT"); + note_command_timeout(type); return -1; } @@ -325,6 +479,24 @@ static ssize_t do_command(char type, const char *command, const char *parameters return -1; } +/* Send a SET command and tell whether the device accepted it: it answers + * with a plain ACK (~00A) for accepted operations and with a rejection + * (~00R) for ones it does not support, while a timeout or a bad answer + * makes do_command() return a negative value in all those cases. */ +static int send_set_command(const char *command, const char *parameters) { + return do_command(SET, command, parameters, NULL) >= 0; +} + +/* Same, but log the failure and return the matching instant command status. */ +static int send_set_or_fail(const char *command, const char *parameters) { + if (send_set_command(command, parameters)) + return STAT_INSTCMD_HANDLED; + + upslogx(LOG_ERR, "device rejected or did not answer [%s%s]", + command, NUT_STRARG(parameters)); + return STAT_INSTCMD_FAILED; +} + static char *field(char *str, int fieldnum) { @@ -358,7 +530,10 @@ static void set_identification(const char *val) { if (strcmp(val, response)) { strncpy(response, val, MAX_RESPONSE_LENGTH); response[MAX_RESPONSE_LENGTH - 1] = '\0'; - do_command(SET, IDENTIFICATION, response, NULL); + if (!send_set_command(IDENTIFICATION, response)) + upslogx(LOG_ERR, "%s: device rejected or did not " + "answer [%s%s]", __func__, IDENTIFICATION, + response); } } @@ -391,7 +566,9 @@ static void set_transfer_voltage_low(int val) { return; high = atoi(ptr); snprintf(response, sizeof(response), "%d;%d", val, high); - do_command(SET, TRANSFER_VOLTAGE, response, NULL); + if (!send_set_command(TRANSFER_VOLTAGE, response)) + upslogx(LOG_ERR, "%s: device rejected or did not answer " + "[%s%s]", __func__, TRANSFER_VOLTAGE, response); } static int get_transfer_voltage_high(void) { @@ -423,7 +600,9 @@ static void set_transfer_voltage_high(int val) { if (!ptr || val == atoi(ptr)) return; snprintf(response, sizeof(response), "%d;%d", low, val); - do_command(SET, TRANSFER_VOLTAGE, response, NULL); + if (!send_set_command(TRANSFER_VOLTAGE, response)) + upslogx(LOG_ERR, "%s: device rejected or did not answer " + "[%s%s]", __func__, TRANSFER_VOLTAGE, response); } static int get_sensitivity(void) { @@ -450,99 +629,515 @@ static void set_sensitivity(const char *val) { for (i = 0; i < SIZEOF_ARRAY(sensitivity); i++) { if (!strcasecmp(val, sensitivity[i].name)) { snprintf(parm, sizeof(parm), "%u", i); - do_command(SET, VOLTAGE_SENSITIVITY, parm, NULL); + if (!send_set_command(VOLTAGE_SENSITIVITY, parm)) + upslogx(LOG_ERR, "%s: device rejected or did " + "not answer [%s%s]", __func__, + VOLTAGE_SENSITIVITY, parm); break; } } } -static void auto_reboot(int enable) { +/* Name the device auto-reboot mode for logging and variable values. */ +static const char *auto_reboot_name(int mode) { + switch (mode) { + case 1: + return "yes"; + case 2: + return "no"; + default: + return "unknown"; + } +} + +static int auto_reboot(int enable) { char parm[20]; char response[MAX_RESPONSE_LENGTH]; char *ptr; - int mode; + int mode, oldmode; if (enable) mode = 1; else mode = 2; + if (do_command(POLL, AUTO_REBOOT, "", response) <= 0) { + /* the current setting is unknown (some devices do not report + * it), so leave it alone and let the caller decide based on + * its own commands */ + upsdebugx(2, "%s: no answer for [%s], leaving it unchanged", + __func__, AUTO_REBOOT); + return 1; + } + ptr = field(response, 0); + oldmode = ptr ? atoi(ptr) : 0; + if (oldmode != mode) { + snprintf(parm, sizeof(parm), "%d", mode); + if (!send_set_command(AUTO_REBOOT, parm)) { + upslogx(LOG_ERR, "%s: device rejected or did not answer " + "[%s%s]", __func__, AUTO_REBOOT, parm); + return 0; + } + /* the device stores this setting, so the change outlives the + * shutdown that needs it: make it (and how to undo it) visible */ + upslogx(LOG_INFO, "%s: ups.start.auto changed from '%s' to " + "'%s' (device setting; restore with " + "\"upsrw -s ups.start.auto=yes|no\" if needed)", + __func__, auto_reboot_name(oldmode), + auto_reboot_name(mode)); + } + /* keep the published setting in sync with the device */ + dstate_setinfo("ups.start.auto", "%s", auto_reboot_name(mode)); + return 1; +} + +/* Read the auto-reboot setting (whether the device starts again when mains + * is (re)applied) and publish it as ups.start.auto; 1 = yes, 2 = no. + * This is auxiliary information, so a missing or unexpected answer must not + * mark the whole UPS as stale. */ +static int get_auto_reboot(void) { + char response[MAX_RESPONSE_LENGTH]; + if (do_command(POLL, AUTO_REBOOT, "", response) <= 0) + return 0; + + switch (atoi(response)) { + case 1: + dstate_setinfo("ups.start.auto", "%s", "yes"); + return 1; + case 2: + dstate_setinfo("ups.start.auto", "%s", "no"); + return 1; + default: + upsdebugx(2, "%s: unexpected [%s] response [%s]", + __func__, AUTO_REBOOT, response); + return 0; + } +} + +/* Read the battery installation date (BRD field 0, formatted as YYYYMMDD). + * The second field looks like a scheduled replacement date, but its meaning + * has not been confirmed for this protocol, so it is only reported in debug + * output instead of being published as a standard variable. */ +static void get_battery_date(void) { + char response[MAX_RESPONSE_LENGTH]; + char *ptr, *sep; + + if (do_command(POLL, BATTERY_REPLACEMENT_DATE, "", response) <= 0) return; + ptr = field(response, 0); - if (!ptr || atoi(ptr) != mode) { - snprintf(parm, sizeof(parm), "%d", mode); - do_command(SET, AUTO_REBOOT, parm, NULL); + if (ptr) { + sep = strchr(ptr, ';'); + if (sep) + *sep = '\0'; + if (*ptr) + dstate_setinfo("battery.date", "%s", ptr); } + + ptr = field(response, 1); + if (ptr) + upsdebugx(2, "%s: unmapped second field [%s]", + __func__, ptr); +} + +/* Read the power-on (boot) delay from the device. This is auxiliary + * information: a missing or unexpected answer must not mark the whole UPS + * as stale, it only leaves the previously known value in place. */ +static int get_boot_delay(void) { + char response[MAX_RESPONSE_LENGTH]; + unsigned int delay; + + if (do_command(POLL, BOOT_DELAY, "", response) <= 0) + return 0; + + if (!str_to_uint_strict(response, &delay, 10) || delay > MAX_BOOTDELAY) { + upsdebugx(2, "%s: unexpected [%s] response [%s]", + __func__, BOOT_DELAY, response); + return 0; + } + + bootdelay = delay; + dstate_setinfo("outlet.delay.start", "%u", bootdelay); + return 1; +} + +/* Write the power-on (boot) delay to the device; the field is three + * digits wide, so it is zero padded like the vendor software does. */ +static void set_boot_delay(unsigned int delay) { + char parm[20]; + + snprintf(parm, sizeof(parm), "%03u", delay); + if (!send_set_command(BOOT_DELAY, parm)) + upslogx(LOG_ERR, "%s: device rejected or did not answer " + "[%s%s]", __func__, BOOT_DELAY, parm); +} + +/* Read the watchdog setting and publish ups.watchdog.status. The device + * reports ";", where a timeout of 0 + * means that the watchdog is disabled. This is auxiliary information, so a + * missing or unexpected answer must not mark the whole UPS as stale. */ +static int get_watchdog(void) { + char response[MAX_RESPONSE_LENGTH]; + char *ptr; + int timeout, alarm; + + if (do_command(POLL, WATCHDOG, "", response) <= 0) + return 0; + + ptr = field(response, 0); + if (!ptr) + return 0; + timeout = atoi(ptr); + if (timeout < 0 || timeout > (int)MAX_WATCHDOG) { + upsdebugx(2, "%s: unexpected [%s] response [%s]", + __func__, WATCHDOG, response); + return 0; + } + watchdog_seconds = (unsigned int)timeout; + + ptr = field(response, 1); + if (ptr) { + alarm = atoi(ptr); + if (alarm == 1 || alarm == 2) + watchdog_alarm = (unsigned int)alarm; + } + + dstate_setinfo("ups.watchdog.status", "%s", + watchdog_seconds ? "enabled" : "disabled"); + return 1; +} + +/* Arm or disarm the watchdog, keeping the timeout (255 seconds when none + * was ever reported) and the restart-alarm flag that the device uses. */ +static void set_watchdog(int enable) { + char parm[20]; + unsigned int timeout = 0; + + if (enable) + timeout = watchdog_seconds ? watchdog_seconds : DEFAULT_WATCHDOG; + + snprintf(parm, sizeof(parm), "%u;%u", timeout, watchdog_alarm); + if (!send_set_command(WATCHDOG, parm)) + upslogx(LOG_ERR, "%s: device rejected or did not answer " + "[%s%s]", __func__, WATCHDOG, parm); +} + +/* Read the ECO (high efficiency) setting and publish it as + * input.eco.switchable; 1 = high efficiency, 2 = high quality. + * The vendor software only ever writes this setting, so a device may not + * answer the poll: in that case the published value is left alone. */ +static int get_eco_mode(void) { + char response[MAX_RESPONSE_LENGTH]; + + if (do_command(POLL, ECONOMIC_MODE, "", response) <= 0) + return 0; + + switch (atoi(response)) { + case 1: + dstate_setinfo("input.eco.switchable", "%s", "ECO"); + return 1; + case 2: + dstate_setinfo("input.eco.switchable", "%s", "normal"); + return 1; + default: + upsdebugx(2, "%s: unexpected [%s] response [%s]", + __func__, ECONOMIC_MODE, response); + return 0; + } +} + +/* Switch between high efficiency (1) and high quality (2) operation. The + * published value is only updated when the device accepted the setting. */ +static void set_eco_mode(int mode) { + char parm[20]; + + snprintf(parm, sizeof(parm), "%d", mode); + if (!send_set_command(ECONOMIC_MODE, parm)) { + upslogx(LOG_ERR, "%s: device rejected or did not answer " + "[%s%s]", __func__, ECONOMIC_MODE, parm); + return; + } + dstate_setinfo("input.eco.switchable", "%s", mode == 1 ? "ECO" : "normal"); +} + +/* Read the buzzer status (EDB: 1 = enabled, 2 = disabled) and publish it as + * ups.beeper.status. The vendor software only ever writes this setting, so + * a device may not answer the poll: in that case the published value is left + * alone. Note that "muted" (the one-shot action performed by the + * beeper.mute command) is a transient state that the device does not + * report. */ +static int get_beeper_status(void) { + char response[MAX_RESPONSE_LENGTH]; + + if (do_command(POLL, ENABLE_BUZZER, "", response) <= 0) + return 0; + + switch (atoi(response)) { + case 1: + dstate_setinfo("ups.beeper.status", "%s", "enabled"); + return 1; + case 2: + dstate_setinfo("ups.beeper.status", "%s", "disabled"); + return 1; + default: + upsdebugx(2, "%s: unexpected [%s] response [%s]", + __func__, ENABLE_BUZZER, response); + return 0; + } +} + +/* Enable (1) or disable (2) the buzzer; this is a device setting, unlike + * the beeper.mute command, which only silences the current alarm. */ +static void set_beeper_status(int enable) { + char parm[20]; + + snprintf(parm, sizeof(parm), "%d", enable ? 1 : 2); + if (!send_set_command(ENABLE_BUZZER, parm)) { + upslogx(LOG_ERR, "%s: device rejected or did not answer " + "[%s%s]", __func__, ENABLE_BUZZER, parm); + return; + } + dstate_setinfo("ups.beeper.status", "%s", enable ? "enabled" : "disabled"); +} + +/* Read the relay status of each outlet bank (SOL: 0 = powered, 1 = off) + * and publish it as outlet.n.status. Errors are not fatal: this is + * auxiliary information, so a missing answer must not mark the whole UPS + * as stale. */ +static void get_outlet_status(void) { + char command[8]; + char response[MAX_RESPONSE_LENGTH]; + char varname[32]; + unsigned int i; + + for (i = 1; i <= (unsigned int)ups.outlet_banks; i++) { + snprintf(command, sizeof(command), "%s%u", RELAY_STATUS, i); + if (do_command(POLL, command, "", response) <= 0) { + upsdebugx(2, "%s: no response for [%s]", + __func__, command); + continue; + } + snprintf(varname, sizeof(varname), "outlet.%u.status", i); + if (!strcmp(response, "0")) + dstate_setinfo(varname, "%s", "on"); + else if (!strcmp(response, "1")) + dstate_setinfo(varname, "%s", "off"); + else + upsdebugx(2, "%s: unexpected [%s] response [%s]", + __func__, command, response); + } +} + +/* Send the restart (return) delay to the UPS. The SDR value is expressed + * in minutes, so it is rounded up to never return the load earlier than + * requested. Returns 1 when the device accepted the setting. */ +static int set_start_delay(unsigned int delay) { + char parm[20]; + + snprintf(parm, sizeof(parm), "%u", (delay + 59U) / 60U); + return send_set_command(TSU_SHUTDOWN_RESTART, parm); +} + +/* Restart delay (in seconds) to ask for when the load has to come back + * after a shutdown: one second more than the shutdown delay, so that + * rounding up to whole minutes (set_start_delay(): the device counts its + * restart delay in minutes) always lands behind the off. The device does + * not restart the load when the restart delay is 0 - an online device + * switches its outputs off and leaves them off, an offline one is only + * started up (tested on an SUINT1500RTXL2Ua) - hence it stays 0 then. */ +static unsigned int reboot_start_delay(unsigned int delay) +{ + return delay ? delay + 1U : 0U; +} + +/* Parse an unsigned integer bounded to the given inclusive range. Returns + * 1 on success and stores the result in *result, 0 otherwise. */ +static int parse_uint_range(const char *val, unsigned int minval, + unsigned int maxval, unsigned int *result) +{ + unsigned int tmp; + + if (!val || !*val) + return 0; + + if (!str_to_uint_strict(val, &tmp, 10)) + return 0; + + if (tmp < minval || tmp > maxval) + return 0; + + *result = tmp; + return 1; +} + +/* Parse a transfer voltage setting: it must be numeric and, when the device + * reported a settable range, fall inside it. */ +static int parse_transfer_voltage(const char *val, unsigned int minval, + unsigned int maxval, unsigned int *voltage) +{ + unsigned int tmp; + + /* sanity bounds, also used when the device reported no range */ + if (!parse_uint_range(val, 1U, 999U, &tmp)) + return 0; + + if (maxval && (tmp < minval || tmp > maxval)) + return 0; + + *voltage = tmp; + return 1; +} + +/* Parse the delay (in seconds) before the load is returned. A value of 0 + * is meaningful here and means "return without delay", so it is accepted, + * unlike the shutdown delay. */ +static int parse_return_delay(const char *val, unsigned int *delay) +{ + return parse_uint_range(val, MIN_STARTDELAY, MAX_STARTDELAY, delay); +} + +/* Resolve the delay (in seconds) to use for a shutdown-related command: + * the value passed by the caller takes precedence over the given fallback + * (the 'offdelay' setting from ups.conf, or the command's own default). + * Returns 1 on success and stores the result in *delay, 0 otherwise. */ +static int get_shutdown_delay(const char *cmdname, const char *extra, + unsigned int fallback, unsigned int *delay) +{ + *delay = fallback; + + if (extra && *extra) { + if (!parse_uint_range(extra, MIN_OFFDELAY, MAX_OFFDELAY, + delay)) { + upslogx(LOG_ERR, "instcmd(%s): invalid delay value '%s' " + "(expected %u..%u seconds)", + cmdname, extra, MIN_OFFDELAY, MAX_OFFDELAY); + return 0; + } + } + + return 1; } static int instcmd(const char *cmdname, const char *extra) { + static const struct { + const char *suffix; /* command suffix */ + const char *relay; /* protocol RELAY_* command */ + int maybepower; /* 1: only possibly affects power state */ + } outlet_commands[] = { + { "load.off", RELAY_OFF, 0 }, + { "load.on", RELAY_ON, 1 } + }; + /* the shutdown commands share the same skeleton: set the auto-reboot + * setting ("ups.start.auto") to what the shutdown needs, set the + * restart (SDR) delay and schedule the shutdown action (SDA) */ + static const struct { + const char *name; /* instant command name */ + int autoreboot; /* ARB value to set: 1 = yes, 0 = no */ + int use_offdelay; /* 1: default value is the offdelay setting */ + int timed_return; /* 1: the load restarts right behind the off */ + } shutdown_commands[] = { + { "shutdown.reboot", 1, 1, 1 }, + { "shutdown.reboot.graceful", 1, 0, 1 }, + { "shutdown.return", 1, 1, 0 }, + { "shutdown.stayoff", 0, 1, 0 } + }; int i; char parm[20]; + char command[32]; /* "outlet..load.on" and friends */ + unsigned int bank, cmd_index; + unsigned int delay; + unsigned int fallback; + int result = STAT_INSTCMD_HANDLED; - /* May be used in logging below, but not as a command argument */ - NUT_UNUSED_VARIABLE(extra); upsdebug_INSTCMD_STARTING(cmdname, extra); if (!strcasecmp(cmdname, "load.off")) { upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); for (i = 0; i < ups.outlet_banks; i++) { snprintf(parm, sizeof(parm), "%d;1", i + 1); - do_command(SET, RELAY_OFF, parm, NULL); + if (send_set_or_fail(RELAY_OFF, parm) != STAT_INSTCMD_HANDLED) + result = STAT_INSTCMD_FAILED; } - return STAT_INSTCMD_HANDLED; + return result; } if (!strcasecmp(cmdname, "load.on")) { upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); for (i = 0; i < ups.outlet_banks; i++) { snprintf(parm, sizeof(parm), "%d;1", i + 1); - do_command(SET, RELAY_ON, parm, NULL); + if (send_set_or_fail(RELAY_ON, parm) != STAT_INSTCMD_HANDLED) + result = STAT_INSTCMD_FAILED; } - return STAT_INSTCMD_HANDLED; + return result; } - if (!strcasecmp(cmdname, "shutdown.reboot")) { - upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - auto_reboot(1); - do_command(SET, TSU_SHUTDOWN_RESTART, "1", NULL); - do_command(SET, TSU_SHUTDOWN_ACTION, "10", NULL); - return STAT_INSTCMD_HANDLED; + /* one set of commands per outlet bank; these take no value */ + for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(outlet_commands); cmd_index++) { + for (bank = 1; bank <= (unsigned int)ups.outlet_banks; bank++) { + snprintf(command, sizeof(command), "outlet.%u.%s", + bank, outlet_commands[cmd_index].suffix); + if (strcasecmp(cmdname, command)) + continue; + + if (outlet_commands[cmd_index].maybepower) + upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); + else + upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); + + snprintf(parm, sizeof(parm), "%u;1", bank); + return send_set_or_fail(outlet_commands[cmd_index].relay, + parm); + } } - if (!strcasecmp(cmdname, "shutdown.reboot.graceful")) { - upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - auto_reboot(1); - do_command(SET, TSU_SHUTDOWN_RESTART, "1", NULL); - do_command(SET, TSU_SHUTDOWN_ACTION, "60", NULL); - return STAT_INSTCMD_HANDLED; + /* buzzer: BUZ 2 silences the alarm that is currently sounding. + * Enabling/disabling the buzzer is a device setting (it survives + * restarts), so it is exposed as the read/write ups.beeper.status + * variable instead of a pair of instant commands. */ + if (!strcasecmp(cmdname, "beeper.mute")) { + return send_set_or_fail(BUZZER, "2"); } - if (!strcasecmp(cmdname, "shutdown.return")) { - upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - auto_reboot(1); - do_command(SET, TSU_SHUTDOWN_RESTART, "1", NULL); - do_command(SET, TSU_SHUTDOWN_ACTION, "10", NULL); - return STAT_INSTCMD_HANDLED; + if (!strcasecmp(cmdname, "clear.fault.record")) { + /* reset the bypass reason recorded by the device */ + return send_set_or_fail(BYPASS_REASON, "0"); } -#if 0 /* doesn't seem to work */ - if (!strcasecmp(cmdname, "shutdown.stayoff")) { + for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(shutdown_commands); + cmd_index++) { + if (strcasecmp(cmdname, shutdown_commands[cmd_index].name)) + continue; + + fallback = shutdown_commands[cmd_index].use_offdelay + ? offdelay : DEFAULT_GRACEFUL_DELAY; + if (!get_shutdown_delay(cmdname, extra, fallback, &delay)) + return STAT_INSTCMD_FAILED; + upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - auto_reboot(0); - do_command(SET, TSU_SHUTDOWN_ACTION, "10", NULL); - return STAT_INSTCMD_HANDLED; + if (!auto_reboot(shutdown_commands[cmd_index].autoreboot)) + result = STAT_INSTCMD_FAILED; + /* the reboot commands ask the load to come back right behind + * the shutdown; the others clear any return delay left over + * from an earlier command, so that the load comes back only + * when mains is (re)applied, or not at all */ + if (!set_start_delay(shutdown_commands[cmd_index].timed_return + ? reboot_start_delay(delay) : 0U)) + result = STAT_INSTCMD_FAILED; + snprintf(parm, sizeof(parm), "%u", delay); + if (send_set_or_fail(TSU_SHUTDOWN_ACTION, parm) + != STAT_INSTCMD_HANDLED) + result = STAT_INSTCMD_FAILED; + return result; } -#endif if (!strcasecmp(cmdname, "shutdown.stop")) { upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); - do_command(SET, TSU_SHUTDOWN_ACTION, "0", NULL); - return STAT_INSTCMD_HANDLED; + /* SDA 0 cancels a scheduled shutdown; the auto-reboot + * setting is left as it is */ + return send_set_or_fail(TSU_SHUTDOWN_ACTION, "0"); } if (!strcasecmp(cmdname, "test.battery.start")) { upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); - do_command(SET, TEST, "3", NULL); - return STAT_INSTCMD_HANDLED; + return send_set_or_fail(TEST, "3"); } if (!strcasecmp(cmdname, "test.battery.stop")) { upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); - do_command(SET, TEST, "0", NULL); - return STAT_INSTCMD_HANDLED; + return send_set_or_fail(TEST, "0"); } upslog_INSTCMD_UNKNOWN(cmdname, extra); @@ -558,13 +1153,119 @@ static int setvar(const char *varname, const char *val) get_identification(); return STAT_SET_HANDLED; } + if (!strcasecmp(varname, "ups.delay.start")) { + unsigned int delay; + + if (!parse_return_delay(val, &delay)) { + upslogx(LOG_ERR, "setvar(%s): invalid return delay " + "value '%s' (expected %u..%u seconds)", + varname, val, MIN_STARTDELAY, MAX_STARTDELAY); + return STAT_SET_FAILED; + } + if (!set_start_delay(delay)) { + upslogx(LOG_ERR, "setvar(%s): device rejected or did " + "not answer the return delay", varname); + return STAT_SET_FAILED; + } + startdelay = delay; + dstate_setinfo("ups.delay.start", "%u", startdelay); + return STAT_SET_HANDLED; + } + if (!strcasecmp(varname, "outlet.delay.start")) { + unsigned int delay; + + if (!parse_uint_range(val, 0U, MAX_BOOTDELAY, &delay)) { + upslogx(LOG_ERR, "setvar(%s): invalid power-on delay " + "value '%s' (expected 0..%u seconds)", + varname, val, MAX_BOOTDELAY); + return STAT_SET_FAILED; + } + set_boot_delay(delay); + /* read the value back, so that the published variable + * reflects what the device actually accepted */ + get_boot_delay(); + return STAT_SET_HANDLED; + } + if (!strcasecmp(varname, "ups.start.auto")) { + if (!strcasecmp(val, "yes")) + auto_reboot(1); + else if (!strcasecmp(val, "no")) + auto_reboot(0); + else { + upslogx(LOG_ERR, "setvar(%s): invalid value '%s' " + "(expected yes or no)", varname, val); + return STAT_SET_FAILED; + } + return STAT_SET_HANDLED; + } + if (!strcasecmp(varname, "ups.beeper.status")) { + if (!strcasecmp(val, "enabled")) + set_beeper_status(1); + else if (!strcasecmp(val, "disabled")) + set_beeper_status(0); + else { + upslogx(LOG_ERR, "setvar(%s): invalid value '%s' " + "(expected enabled or disabled)", varname, val); + return STAT_SET_FAILED; + } + /* read the value back, so that the published variable + * reflects what the device actually accepted */ + get_beeper_status(); + return STAT_SET_HANDLED; + } + if (!strcasecmp(varname, "ups.watchdog.status")) { + if (!strcasecmp(val, "enabled")) + set_watchdog(1); + else if (!strcasecmp(val, "disabled")) + set_watchdog(0); + else { + upslogx(LOG_ERR, "setvar(%s): invalid value '%s' " + "(expected enabled or disabled)", varname, val); + return STAT_SET_FAILED; + } + /* read the value back, so that the published variable + * reflects what the device actually accepted */ + get_watchdog(); + return STAT_SET_HANDLED; + } + if (!strcasecmp(varname, "input.eco.switchable")) { + if (!strcasecmp(val, "ECO")) + set_eco_mode(1); + else if (!strcasecmp(val, "normal")) + set_eco_mode(2); + else { + upslogx(LOG_ERR, "setvar(%s): invalid value '%s' " + "(expected ECO or normal)", varname, val); + return STAT_SET_FAILED; + } + get_eco_mode(); + return STAT_SET_HANDLED; + } if (!strcasecmp(varname, "input.transfer.low")) { - set_transfer_voltage_low(atoi(val)); + unsigned int voltage; + + if (!parse_transfer_voltage(val, min_low_transfer, + max_low_transfer, &voltage)) { + upslogx(LOG_ERR, "setvar(%s): invalid value '%s' " + "(expected %u..%u)", varname, val, + min_low_transfer, max_low_transfer); + return STAT_SET_FAILED; + } + set_transfer_voltage_low((int)voltage); get_transfer_voltage_low(); return STAT_SET_HANDLED; } if (!strcasecmp(varname, "input.transfer.high")) { - set_transfer_voltage_high(atoi(val)); + unsigned int voltage; + + if (!parse_transfer_voltage(val, min_high_transfer, + max_high_transfer, &voltage)) { + upslogx(LOG_ERR, "setvar(%s): invalid value '%s' " + "(expected %u..%u)", varname, val, + min_high_transfer, max_high_transfer); + return STAT_SET_FAILED; + } + set_transfer_voltage_high((int)voltage); get_transfer_voltage_high(); return STAT_SET_HANDLED; } @@ -607,8 +1308,7 @@ static int init_comm(void) void upsdrv_initinfo(void) { char response[MAX_RESPONSE_LENGTH]; - unsigned int min_low_transfer, max_low_transfer; - unsigned int min_high_transfer, max_high_transfer; + char buf[32]; /* "outlet..load.on" and friends */ unsigned int i; char *ptr; @@ -664,9 +1364,55 @@ void upsdrv_initinfo(void) max_high_transfer = (unsigned int)ipv; } } - if (do_command(POLL, OUTLET_RELAYS, "", response) > 0) - ups.outlet_banks = atoi(response); + if (do_command(POLL, OUTLET_RELAYS, "", response) > 0) { + int banks = atoi(response); + + if (banks > 0 && banks <= (int)MAX_OUTLET_BANKS) + ups.outlet_banks = banks; + else if (banks != 0) + upslogx(LOG_WARNING, "%s: ignoring implausible " + "outlet bank count [%s]", __func__, response); + } + if (ups.outlet_banks > 0) { + dstate_setinfo("outlet.count", "%d", ups.outlet_banks); + for (i = 1; i <= (unsigned int)ups.outlet_banks; i++) { + snprintf(buf, sizeof(buf), "outlet.%u.switchable", i); + dstate_setinfo(buf, "%s", "yes"); + } + } + get_battery_date(); /* define things that are settable */ + dstate_setinfo("ups.delay.start", "%u", startdelay); + dstate_setflags("ups.delay.start", ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("ups.delay.start", 4); + /* the power-on delay is a device setting (get/set), so it is published + * with a default and refreshed from the device right away */ + dstate_setinfo("outlet.delay.start", "%u", bootdelay); + dstate_setflags("outlet.delay.start", ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("outlet.delay.start", 3); + get_boot_delay(); + /* the auto-reboot setting is a device setting (get/set) as well */ + dstate_setinfo("ups.start.auto", "%s", "yes"); + dstate_setflags("ups.start.auto", ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("ups.start.auto", 3); + get_auto_reboot(); + /* the watchdog is a device setting too: 0 seconds means disabled */ + dstate_setinfo("ups.watchdog.status", "%s", "disabled"); + dstate_setflags("ups.watchdog.status", ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("ups.watchdog.status", 8); + get_watchdog(); + /* the ECO (high efficiency) mode selection is a device setting too */ + dstate_setinfo("input.eco.switchable", "%s", "normal"); + dstate_setflags("input.eco.switchable", ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("input.eco.switchable", 6); + get_eco_mode(); + /* the buzzer is a device setting: enabling/disabling it is done with + * upsrw, while the beeper.mute command only silences the current + * alarm */ + dstate_setinfo("ups.beeper.status", "%s", "enabled"); + dstate_setflags("ups.beeper.status", ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("ups.beeper.status", 8); + get_beeper_status(); if (get_identification()) { dstate_setflags("ups.id", ST_FLAG_RW | ST_FLAG_STRING); dstate_setaux("ups.id", 100); @@ -676,27 +1422,45 @@ void upsdrv_initinfo(void) for (i = min_low_transfer; i <= max_low_transfer; i++) dstate_addenum("input.transfer.low", "%u", i); } - if (get_transfer_voltage_high() && max_low_transfer) { + if (get_transfer_voltage_high() && max_high_transfer) { dstate_setflags("input.transfer.high", ST_FLAG_RW); for (i = min_high_transfer; i <= max_high_transfer; i++) dstate_addenum("input.transfer.high", "%u", i); } + /* report the settable ranges, so that the values accepted by setvar() + * are visible to clients */ + if (max_low_transfer) { + dstate_setinfo("input.transfer.low.min", "%u", min_low_transfer); + dstate_setinfo("input.transfer.low.max", "%u", max_low_transfer); + } + if (max_high_transfer) { + dstate_setinfo("input.transfer.high.min", "%u", min_high_transfer); + dstate_setinfo("input.transfer.high.max", "%u", max_high_transfer); + } if (get_sensitivity()) { dstate_setflags("input.sensitivity", ST_FLAG_RW); for (i = 0; i < SIZEOF_ARRAY(sensitivity); i++) dstate_addenum("input.sensitivity", "%s", sensitivity[i].name); } + /* load.off, load.on and the commands below switch individual outlet + * banks, so they are only registered when the device reports any */ if (ups.outlet_banks) { dstate_addcmd("load.off"); dstate_addcmd("load.on"); + for (i = 1; i <= (unsigned int)ups.outlet_banks; i++) { + snprintf(buf, sizeof(buf), "outlet.%u.load.off", i); + dstate_addcmd(buf); + snprintf(buf, sizeof(buf), "outlet.%u.load.on", i); + dstate_addcmd(buf); + } } + dstate_addcmd("beeper.mute"); + dstate_addcmd("clear.fault.record"); dstate_addcmd("shutdown.reboot"); dstate_addcmd("shutdown.reboot.graceful"); dstate_addcmd("shutdown.return"); -#if 0 /* doesn't work */ dstate_addcmd("shutdown.stayoff"); -#endif dstate_addcmd("shutdown.stop"); dstate_addcmd("test.battery.start"); dstate_addcmd("test.battery.stop"); @@ -868,6 +1632,8 @@ void upsdrv_updateinfo(void) dstate_setinfo("ups.contacts", "%02X", (unsigned int)flags); } + get_outlet_status(); + /* if we are here, status is valid */ status_commit(); dstate_dataok(); @@ -882,13 +1648,18 @@ void upsdrv_shutdown(void) if (!init_comm()) printf("Status failed. Assuming it's on battery and trying a shutdown anyway.\n"); - auto_reboot(1); /* in case the power is on, tell it to automatically reboot. if * it is off, this has no effect. */ - snprintf(parm, sizeof(parm), "%d", 1); /* delay before reboot, in minutes */ - do_command(SET, TSU_SHUTDOWN_RESTART, parm, NULL); - snprintf(parm, sizeof(parm), "%d", 5); /* delay before shutdown, in seconds */ - do_command(SET, TSU_SHUTDOWN_ACTION, parm, NULL); + if (!auto_reboot(1)) + upslogx(LOG_ERR, "%s: could not set ups.start.auto", __func__); + if (!set_start_delay(startdelay)) + upslogx(LOG_ERR, "%s: could not set the return delay", __func__); + /* delay before shutdown, in seconds: honor 'offdelay' if the user set + * it in ups.conf, otherwise keep the historical default */ + snprintf(parm, sizeof(parm), "%u", + offdelay_from_conf ? offdelay : 5U); + if (!send_set_command(TSU_SHUTDOWN_ACTION, parm)) + upslogx(LOG_ERR, "%s: could not schedule the shutdown", __func__); } void upsdrv_help(void) @@ -903,10 +1674,26 @@ void upsdrv_tweak_prognames(void) /* list flags and values that you want to receive via -x or ups.conf */ void upsdrv_makevartable(void) { + char msg[256]; + addvar(VAR_VALUE, "lowbatt", "Set low battery level, in percent"); - addvar(VAR_VALUE, "command_delay", - "Delay in milliseconds before each command (default: 0 = no delay; " - "set to 1000ms if experiencing communication timeouts)"); + addvar(VAR_VALUE, "command_delay", + "Minimum interval between commands, in milliseconds " + "(default: 0 = no wait; -1 = automatic)"); + + snprintf(msg, sizeof msg, "Set shutdown delay, in seconds (default=%u, range %u..%u).", + DEFAULT_OFFDELAY, MIN_OFFDELAY, MAX_OFFDELAY); + addvar(VAR_VALUE, "offdelay", msg); + + /* The return delay is configured in minutes because that is the unit + * used by the device (SDR), while it is exposed in seconds through + * the ups.delay.start variable, as usual for delays in NUT. */ + snprintf(msg, sizeof msg, "Set return (restart) delay, in minutes " + "(default=%u, range %u..%u).", + DEFAULT_STARTDELAY / 60U, + MIN_STARTDELAY / 60U, + MAX_STARTDELAY_MINUTES); + addvar(VAR_VALUE, "startdelay", msg); } void upsdrv_initups(void) @@ -916,22 +1703,82 @@ void upsdrv_initups(void) upsfd = ser_open(device_path); ser_set_speed(upsfd, device_path, B2400); - /* Initialize command_delay from configuration */ + /* Initialize command_delay from configuration. The value is in + * milliseconds: 0 disables the pacing (commands follow each other as + * soon as the previous answer is read), -1 asks the driver to pace + * them by itself (start without waiting, raise the interval once if a + * command times out) and a positive value is the minimum interval to + * keep between two commands. */ + command_delay_conf = DEFAULT_COMMAND_DELAY; val = getval("command_delay"); - if (val) { - long temp = atol(val); - /* 0 (no delay) or positive values */ - if (temp < 0) { - fatalx(EXIT_FAILURE, "Invalid command_delay parameter: %s (must be >= 0)", val); - } - command_delay = temp; - if (command_delay == 0) { - upsdebugx(2, "command_delay is explicitly set to 0 (no delay)"); - } else { - upsdebugx(2, "Setting command_delay to %ld milliseconds", command_delay); + if (val && *val) { + long temp; + + if (!str_to_long_strict(val, &temp, 10) || + temp < (long)COMMAND_DELAY_AUTO || + temp > (long)MAX_COMMAND_DELAY) { + fatalx(EXIT_FAILURE, "Invalid command_delay parameter: %s " + "(expected -1 (auto), 0 (no wait) or 1..%u " + "milliseconds)", val, MAX_COMMAND_DELAY); + } + command_delay_conf = temp; + } else { + upsdebugx(2, "Using default command_delay of %ld milliseconds", + command_delay_conf); + } + + if (command_delay_conf == (long)COMMAND_DELAY_AUTO) { + command_delay_auto = 1; + command_delay = 0; + upsdebugx(2, "command_delay is automatic: commands are paced by " + "%u milliseconds once one of them times out", + DEFAULT_AUTO_COMMAND_DELAY); + } else { + command_delay_auto = 0; + command_delay = (unsigned int)command_delay_conf; + if (command_delay) + upsdebugx(2, "Setting command_delay to %u milliseconds", + command_delay); + else + upsdebugx(2, "command_delay is 0: no wait between commands"); + } + + /* Initialize offdelay from configuration */ + val = getval("offdelay"); + if (val && *val) { + unsigned int temp; + if (!str_to_uint_strict(val, &temp, 10) || + temp < MIN_OFFDELAY || temp > MAX_OFFDELAY) { + fatalx(EXIT_FAILURE, "Invalid offdelay parameter: %s " + "(expected %u..%u seconds)", + val, MIN_OFFDELAY, MAX_OFFDELAY); + } + offdelay = temp; + offdelay_from_conf = 1; + upsdebugx(2, "Setting offdelay to %u seconds", offdelay); + } else { + upsdebugx(2, "Using default offdelay of %u seconds", offdelay); + } + + /* Initialize startdelay from configuration. The parameter is in + * minutes (the unit used by the device), the internal value is in + * seconds (as used by ups.delay.start). */ + val = getval("startdelay"); + if (val && *val) { + unsigned int temp; + if (!str_to_uint_strict(val, &temp, 10) || + temp > MAX_STARTDELAY_MINUTES) { + fatalx(EXIT_FAILURE, "Invalid startdelay parameter: %s " + "(expected %u..%u minutes)", + val, MIN_STARTDELAY / 60U, + MAX_STARTDELAY_MINUTES); } + startdelay = temp * 60U; + upsdebugx(2, "Setting startdelay to %u seconds (%u minutes)", + startdelay, temp); } else { - upsdebugx(2, "Using default command_delay of %ld (no delay)", command_delay); + upsdebugx(2, "Using default startdelay of %u seconds", + startdelay); } } From 3be3ad24f2e06e7bdc90a821b507ece71d4fccf4 Mon Sep 17 00:00:00 2001 From: TomZanna Date: Thu, 1 Oct 2026 14:52:08 +0200 Subject: [PATCH 03/13] feat(tripplitesu): publish transfer reason and only report known settings Add read-only input.transfer.reason variable from the device's recorded bypass code, refreshed each poll and cleared by clear.fault.record. Stop publishing device settings (outlet.delay.start, ups.start.auto, ups.watchdog.status, input.eco.switchable, ups.beeper.status) with assumed defaults; expose them only once the device reports them, retaining last known values with a warning when the device stops responding. --- NEWS.adoc | 14 +++ docs/man/tripplitesu.txt | 39 +++++--- drivers/tripplitesu.c | 194 +++++++++++++++++++++++++++------------ 3 files changed, 176 insertions(+), 71 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index aa4e68799b..6b98f2e63a 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -173,6 +173,20 @@ https://github.com/networkupstools/nut/milestone/13 same usage `ups.beeper.status` reads, not a temporary mute/re-arm action. + - `tripplitesu` driver updates: + * The bypass (transfer) reason recorded by the device is now published + as the read-only `input.transfer.reason` variable, refreshed on every + poll, and the existing `clear.fault.record` instant command clears + the recorded reason on the device. + * Device settings (`outlet.delay.start`, `ups.start.auto`, + `ups.watchdog.status`, `input.eco.switchable` and + `ups.beeper.status`) are no longer published with assumed values when + the device does not report them: they appear, and become settable, + only once the device has reported them. A setting that was read + before keeps its last known value when the device stops answering, + and a warning is logged instead of silently falling back to a + default. + - `apc_modbus` driver updates: * Fixed string join not doing zero termination. [PR #3413] * Decode `RunTimeCalibrationStatus_BF` into diff --git a/docs/man/tripplitesu.txt b/docs/man/tripplitesu.txt index e26884b070..44740671ae 100644 --- a/docs/man/tripplitesu.txt +++ b/docs/man/tripplitesu.txt @@ -169,14 +169,23 @@ value; enabling/disabling the buzzer for good is done through the quiet device it has no audible effect. 'clear.fault.record':: - Clear the bypass reason recorded by the device. This is a one-shot - action, so the recorded reason is not published as a variable. + Clear the bypass reason recorded by the device. The recorded reason + itself is published, read-only, as *input.transfer.reason*, which is + refreshed on every poll cycle. VARIABLES --------- The driver publishes the usual battery, input and output variables, plus the following ones: ++ +Read/write variables that describe a device setting (*outlet.delay.start*, +*ups.start.auto*, *ups.watchdog.status*, *input.eco.switchable* and +*ups.beeper.status*) are published, and made settable, only once the device +has reported the setting: when it does not answer, the variable is absent +(no value is assumed) and a warning is logged. A setting that was read +before keeps its last known value when the device stops answering, again +with a warning for every failed read. *outlet.count*:: Total number of controllable outlet banks. @@ -213,9 +222,8 @@ the following ones: 'beeper.mute' command instead silences the alarm that is currently sounding (a transient state the device does not report, which is why it never appears here). The vendor software only ever writes this - setting, so the device may not report it back: when it does not, the - published value is the last one written by the driver, starting from - the 'enabled' assumption. + setting, so some devices may not report it back; see the note on device + settings above. *ups.delay.start*:: Read/write return delay, in seconds, used by the driver shutdown @@ -245,11 +253,11 @@ linkman:upsrw[8] to restore your preferred value, for example: Read/write watchdog setting, either 'enabled' or 'disabled': the device reboots the load when it does not hear from the driver for the watchdog timeout. A timeout of 0 seconds means 'disabled', and - setting the variable to 'enabled' arms the watchdog with the last - timeout reported by the device (255 seconds when none was ever - reported), keeping the restart-alarm behaviour the device uses. Note - that the timeout itself is not exposed as a separate variable, - because there is no name for it in the NUT naming scheme. + setting the variable to 'enabled' arms the watchdog with the timeout + last reported by the device, keeping the restart-alarm behaviour the + device uses. Note that the timeout itself is not exposed as a + separate variable, because there is no name for it in the NUT naming + scheme. + WARNING: Enabling this setting arms a timer the device uses to power cycle the load when the driver stops talking to it, so a driver that hangs (or a @@ -260,12 +268,17 @@ The device reports the timeout and the restart-alarm flag, not a watchdog status: 'enabled' and 'disabled' are inferred from a non-zero timeout, which has not been verified on hardware. +*input.transfer.reason*:: + Read-only reason for the last transfer recorded by the device, + published as the raw two-digit code it reports: the meaning of the + codes is not documented, so they are not translated. The value is + refreshed on every poll cycle, and cleared on the device by the + 'clear.fault.record' instant command. + *input.eco.switchable*:: Read/write high efficiency (ECO) mode selection, either 'ECO' or 'normal'. The vendor software only ever writes this setting, so the - device may not report it back: when it does not, the published value - is the last one written by the driver, starting from the 'normal' - assumption. + device may not report it back; see the note on device settings above. *input.transfer.low*, *input.transfer.high*:: Read/write low and high voltage transfer points, in V. Values are diff --git a/drivers/tripplitesu.c b/drivers/tripplitesu.c index 1700fb5607..13d4fcfe4d 100644 --- a/drivers/tripplitesu.c +++ b/drivers/tripplitesu.c @@ -72,6 +72,7 @@ input.sensitivity (RW) (1) input.transfer.high (RW) input.transfer.low (RW) + input.transfer.reason input.voltage input.voltage.nominal outlet.count @@ -195,7 +196,6 @@ upsdrv_info_t upsdrv_info = { * it is powered up or asked to switch them on. This is a device setting, * not an action, so it is only exposed as a read/write variable (no instant * command); 0 means "no delay" and the device field is three digits wide. */ -#define DEFAULT_BOOTDELAY 0U #define MAX_BOOTDELAY 999U /* Watchdog setting (WDG): the device stores a timeout in seconds (up to @@ -255,9 +255,9 @@ static struct timeval command_sent_at; /* when the last command was sent */ static unsigned int offdelay = DEFAULT_OFFDELAY; /* delay in seconds before shutdown */ static int offdelay_from_conf = 0; /* set if 'offdelay' was provided in ups.conf */ static unsigned int startdelay = DEFAULT_STARTDELAY; /* delay in seconds before return */ -static unsigned int bootdelay = DEFAULT_BOOTDELAY; /* power-on delay in seconds */ static unsigned int watchdog_seconds = 0; /* watchdog timeout last reported by the device */ static unsigned int watchdog_alarm = 2; /* watchdog restart-alarm flag last reported */ +static int watchdog_known = 0; /* set once the device reported its watchdog setting */ static unsigned int min_low_transfer, max_low_transfer; /* reported transfer voltage ranges */ static unsigned int min_high_transfer, max_high_transfer; @@ -685,20 +685,28 @@ static int auto_reboot(int enable) { __func__, auto_reboot_name(oldmode), auto_reboot_name(mode)); } - /* keep the published setting in sync with the device */ + /* keep the published setting in sync with the device, and make it + * settable again in case it was not read at startup */ dstate_setinfo("ups.start.auto", "%s", auto_reboot_name(mode)); + dstate_setflags("ups.start.auto", ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("ups.start.auto", 3); return 1; } /* Read the auto-reboot setting (whether the device starts again when mains * is (re)applied) and publish it as ups.start.auto; 1 = yes, 2 = no. * This is auxiliary information, so a missing or unexpected answer must not - * mark the whole UPS as stale. */ + * mark the whole UPS as stale: a warning is logged and the previously + * published value is left in place. */ static int get_auto_reboot(void) { char response[MAX_RESPONSE_LENGTH]; - if (do_command(POLL, AUTO_REBOOT, "", response) <= 0) + if (do_command(POLL, AUTO_REBOOT, "", response) <= 0) { + upslogx(LOG_WARNING, "%s: could not read the auto-reboot " + "setting [%s]; keeping the last known value", __func__, + AUTO_REBOOT); return 0; + } switch (atoi(response)) { case 1: @@ -708,8 +716,9 @@ static int get_auto_reboot(void) { dstate_setinfo("ups.start.auto", "%s", "no"); return 1; default: - upsdebugx(2, "%s: unexpected [%s] response [%s]", - __func__, AUTO_REBOOT, response); + upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " + "keeping the last known value", __func__, AUTO_REBOOT, + response); return 0; } } @@ -742,22 +751,27 @@ static void get_battery_date(void) { /* Read the power-on (boot) delay from the device. This is auxiliary * information: a missing or unexpected answer must not mark the whole UPS - * as stale, it only leaves the previously known value in place. */ + * as stale, it logs a warning and only leaves the previously known value in + * place. */ static int get_boot_delay(void) { char response[MAX_RESPONSE_LENGTH]; unsigned int delay; - if (do_command(POLL, BOOT_DELAY, "", response) <= 0) + if (do_command(POLL, BOOT_DELAY, "", response) <= 0) { + upslogx(LOG_WARNING, "%s: could not read the power-on delay " + "[%s]; keeping the last known value", __func__, + BOOT_DELAY); return 0; + } if (!str_to_uint_strict(response, &delay, 10) || delay > MAX_BOOTDELAY) { - upsdebugx(2, "%s: unexpected [%s] response [%s]", - __func__, BOOT_DELAY, response); + upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " + "keeping the last known value", __func__, BOOT_DELAY, + response); return 0; } - bootdelay = delay; - dstate_setinfo("outlet.delay.start", "%u", bootdelay); + dstate_setinfo("outlet.delay.start", "%u", delay); return 1; } @@ -775,25 +789,36 @@ static void set_boot_delay(unsigned int delay) { /* Read the watchdog setting and publish ups.watchdog.status. The device * reports ";", where a timeout of 0 * means that the watchdog is disabled. This is auxiliary information, so a - * missing or unexpected answer must not mark the whole UPS as stale. */ + * missing or unexpected answer must not mark the whole UPS as stale: a + * warning is logged and the previously published value is left in place. */ static int get_watchdog(void) { char response[MAX_RESPONSE_LENGTH]; char *ptr; int timeout, alarm; - if (do_command(POLL, WATCHDOG, "", response) <= 0) + if (do_command(POLL, WATCHDOG, "", response) <= 0) { + upslogx(LOG_WARNING, "%s: could not read the watchdog " + "setting [%s]; keeping the last known value", + __func__, WATCHDOG); return 0; + } ptr = field(response, 0); - if (!ptr) + if (!ptr) { + upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " + "keeping the last known value", __func__, WATCHDOG, + response); return 0; + } timeout = atoi(ptr); if (timeout < 0 || timeout > (int)MAX_WATCHDOG) { - upsdebugx(2, "%s: unexpected [%s] response [%s]", - __func__, WATCHDOG, response); + upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " + "keeping the last known value", __func__, WATCHDOG, + response); return 0; } watchdog_seconds = (unsigned int)timeout; + watchdog_known = 1; ptr = field(response, 1); if (ptr) { @@ -813,8 +838,16 @@ static void set_watchdog(int enable) { char parm[20]; unsigned int timeout = 0; - if (enable) + if (enable) { + /* arming without a reading uses assumed values, which is + * worth telling about: the timeout is not the one the device + * would have reported */ + if (!watchdog_known) + upslogx(LOG_WARNING, "%s: arming the watchdog with an " + "assumed timeout, the device never reported " + "its [%s] setting", __func__, WATCHDOG); timeout = watchdog_seconds ? watchdog_seconds : DEFAULT_WATCHDOG; + } snprintf(parm, sizeof(parm), "%u;%u", timeout, watchdog_alarm); if (!send_set_command(WATCHDOG, parm)) @@ -822,15 +855,49 @@ static void set_watchdog(int enable) { "[%s%s]", __func__, WATCHDOG, parm); } +/* Read the bypass reason recorded by the device (BPA) and publish it as + * input.transfer.reason. This is a live value, so it is refreshed on every + * update cycle; a missing or unexpected answer must not mark the whole UPS + * as stale: a warning is logged and the previously published reason is left + * in place. The device value is a two-digit code whose meaning is not + * documented, so it is published as it comes. */ +static int get_bypass_reason(void) { + char response[MAX_RESPONSE_LENGTH]; + char *ptr; + + if (do_command(POLL, BYPASS_REASON, "", response) <= 0) { + upslogx(LOG_WARNING, "%s: could not read the bypass reason " + "[%s]; keeping the last known value", __func__, + BYPASS_REASON); + return 0; + } + + ptr = field(response, 0); + if (!ptr || !*ptr) { + upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " + "keeping the last known value", __func__, BYPASS_REASON, + response); + return 0; + } + + dstate_setinfo("input.transfer.reason", "%s", ptr); + return 1; +} + /* Read the ECO (high efficiency) setting and publish it as * input.eco.switchable; 1 = high efficiency, 2 = high quality. * The vendor software only ever writes this setting, so a device may not - * answer the poll: in that case the published value is left alone. */ + * answer the poll: in that case a warning is logged and the published value + * is left alone. */ static int get_eco_mode(void) { char response[MAX_RESPONSE_LENGTH]; - if (do_command(POLL, ECONOMIC_MODE, "", response) <= 0) + if (do_command(POLL, ECONOMIC_MODE, "", response) <= 0) { + upslogx(LOG_WARNING, "%s: could not read the ECO setting " + "[%s]; keeping the last known value", __func__, + ECONOMIC_MODE); return 0; + } switch (atoi(response)) { case 1: @@ -840,8 +907,9 @@ static int get_eco_mode(void) { dstate_setinfo("input.eco.switchable", "%s", "normal"); return 1; default: - upsdebugx(2, "%s: unexpected [%s] response [%s]", - __func__, ECONOMIC_MODE, response); + upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " + "keeping the last known value", __func__, + ECONOMIC_MODE, response); return 0; } } @@ -862,15 +930,19 @@ static void set_eco_mode(int mode) { /* Read the buzzer status (EDB: 1 = enabled, 2 = disabled) and publish it as * ups.beeper.status. The vendor software only ever writes this setting, so - * a device may not answer the poll: in that case the published value is left - * alone. Note that "muted" (the one-shot action performed by the - * beeper.mute command) is a transient state that the device does not - * report. */ + * a device may not answer the poll: in that case a warning is logged and the + * published value is left alone. Note that "muted" (the one-shot action + * performed by the beeper.mute command) is a transient state that the device + * does not report. */ static int get_beeper_status(void) { char response[MAX_RESPONSE_LENGTH]; - if (do_command(POLL, ENABLE_BUZZER, "", response) <= 0) + if (do_command(POLL, ENABLE_BUZZER, "", response) <= 0) { + upslogx(LOG_WARNING, "%s: could not read the buzzer setting " + "[%s]; keeping the last known value", __func__, + ENABLE_BUZZER); return 0; + } switch (atoi(response)) { case 1: @@ -880,8 +952,9 @@ static int get_beeper_status(void) { dstate_setinfo("ups.beeper.status", "%s", "disabled"); return 1; default: - upsdebugx(2, "%s: unexpected [%s] response [%s]", - __func__, ENABLE_BUZZER, response); + upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " + "keeping the last known value", __func__, ENABLE_BUZZER, + response); return 0; } } @@ -1385,34 +1458,34 @@ void upsdrv_initinfo(void) dstate_setinfo("ups.delay.start", "%u", startdelay); dstate_setflags("ups.delay.start", ST_FLAG_RW | ST_FLAG_STRING); dstate_setaux("ups.delay.start", 4); - /* the power-on delay is a device setting (get/set), so it is published - * with a default and refreshed from the device right away */ - dstate_setinfo("outlet.delay.start", "%u", bootdelay); - dstate_setflags("outlet.delay.start", ST_FLAG_RW | ST_FLAG_STRING); - dstate_setaux("outlet.delay.start", 3); - get_boot_delay(); - /* the auto-reboot setting is a device setting (get/set) as well */ - dstate_setinfo("ups.start.auto", "%s", "yes"); - dstate_setflags("ups.start.auto", ST_FLAG_RW | ST_FLAG_STRING); - dstate_setaux("ups.start.auto", 3); - get_auto_reboot(); - /* the watchdog is a device setting too: 0 seconds means disabled */ - dstate_setinfo("ups.watchdog.status", "%s", "disabled"); - dstate_setflags("ups.watchdog.status", ST_FLAG_RW | ST_FLAG_STRING); - dstate_setaux("ups.watchdog.status", 8); - get_watchdog(); - /* the ECO (high efficiency) mode selection is a device setting too */ - dstate_setinfo("input.eco.switchable", "%s", "normal"); - dstate_setflags("input.eco.switchable", ST_FLAG_RW | ST_FLAG_STRING); - dstate_setaux("input.eco.switchable", 6); - get_eco_mode(); - /* the buzzer is a device setting: enabling/disabling it is done with - * upsrw, while the beeper.mute command only silences the current - * alarm */ - dstate_setinfo("ups.beeper.status", "%s", "enabled"); - dstate_setflags("ups.beeper.status", ST_FLAG_RW | ST_FLAG_STRING); - dstate_setaux("ups.beeper.status", 8); - get_beeper_status(); + /* the device settings below (get/set) are published, and made + * settable, only when the device actually reports them: an unreadable + * setting must not appear with an assumed value, and each get_*() + * already warns when it cannot read one */ + if (get_boot_delay()) { + dstate_setflags("outlet.delay.start", + ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("outlet.delay.start", 3); + } + if (get_auto_reboot()) { + dstate_setflags("ups.start.auto", ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("ups.start.auto", 3); + } + if (get_watchdog()) { + dstate_setflags("ups.watchdog.status", + ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("ups.watchdog.status", 8); + } + if (get_eco_mode()) { + dstate_setflags("input.eco.switchable", + ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("input.eco.switchable", 6); + } + if (get_beeper_status()) { + dstate_setflags("ups.beeper.status", + ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("ups.beeper.status", 8); + } if (get_identification()) { dstate_setflags("ups.id", ST_FLAG_RW | ST_FLAG_STRING); dstate_setaux("ups.id", 100); @@ -1632,6 +1705,11 @@ void upsdrv_updateinfo(void) dstate_setinfo("ups.contacts", "%02X", (unsigned int)flags); } + /* the bypass (transfer) reason is a live value: refresh it on every + * pass, without marking the whole UPS stale when the device does not + * report it */ + get_bypass_reason(); + get_outlet_status(); /* if we are here, status is valid */ From 034f4d75fddece4e4e73b8dc2a06244075e60fa2 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 2 Oct 2026 15:23:01 +0200 Subject: [PATCH 04/13] NEWS.adoc: group together the tripplitesu driver updates [#3722] Signed-off-by: Jim Klimov --- NEWS.adoc | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index 6b98f2e63a..fab8dac463 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -173,20 +173,6 @@ https://github.com/networkupstools/nut/milestone/13 same usage `ups.beeper.status` reads, not a temporary mute/re-arm action. - - `tripplitesu` driver updates: - * The bypass (transfer) reason recorded by the device is now published - as the read-only `input.transfer.reason` variable, refreshed on every - poll, and the existing `clear.fault.record` instant command clears - the recorded reason on the device. - * Device settings (`outlet.delay.start`, `ups.start.auto`, - `ups.watchdog.status`, `input.eco.switchable` and - `ups.beeper.status`) are no longer published with assumed values when - the device does not report them: they appear, and become settable, - only once the device has reported them. A setting that was read - before keeps its last known value when the device stops answering, - and a warning is logged instead of silently falling back to a - default. - - `apc_modbus` driver updates: * Fixed string join not doing zero termination. [PR #3413] * Decode `RunTimeCalibrationStatus_BF` into @@ -372,9 +358,21 @@ https://github.com/networkupstools/nut/milestone/13 some problems possible when iterating devices. [issue #1768, PR #3477] - `riello_usb`, `riello_ser` driver updates: - * riello_usb, riello_ser: report NAK on RE as failure [issue #3546] + * riello_usb, riello_ser: report NAK on RE as failure. [issue #3546] - - `tripplitesu` driver updates: + - `tripplitesu` driver updates [PR #3273]: + * The bypass (transfer) reason recorded by the device is now published + as the read-only `input.transfer.reason` variable, refreshed on every + poll, and the existing `clear.fault.record` instant command clears + the recorded reason on the device. + * Device settings (`outlet.delay.start`, `ups.start.auto`, + `ups.watchdog.status`, `input.eco.switchable` and + `ups.beeper.status`) are no longer published with assumed values when + the device does not report them: they appear, and become settable, + only once the device has reported them. A setting that was read + before keeps its last known value when the device stops answering, + and a warning is logged instead of silently falling back to a + default. * Added the `offdelay` and `startdelay` settings in `ups.conf` (in seconds and minutes respectively, the units of the two device commands) and support for an optional delay (in seconds), passed as @@ -409,7 +407,7 @@ https://github.com/networkupstools/nut/milestone/13 one. The interval is measured from the previous command, so an idle link is not penalized, and the special value `-1` makes the driver start without waiting and pace commands by one second only if one of - them times out. [PR #3273] + them times out. - `usbhid-ups` driver updates: * When reconnecting, report success more visibly (not only in debug), and From e506a3abdbdc6f2789318618628f91feb89b93b9 Mon Sep 17 00:00:00 2001 From: TomZanna Date: Sat, 3 Oct 2026 10:20:30 +0200 Subject: [PATCH 05/13] feat(tripplitesu): exit on intermediate errors and clarify terminology in man page Improve tripplitesu driver error handling to exit upon intermediate errors rather than continuing. Update the man page to consistently use "outlet" terminology instead of "outlet bank", and expand documentation of the watchdog setting and command failure behavior for clarity. Signed-off-by: TomZanna --- docs/man/tripplitesu.txt | 55 ++++++++------- drivers/tripplitesu.c | 143 +++++++++++++++++++++------------------ 2 files changed, 107 insertions(+), 91 deletions(-) diff --git a/docs/man/tripplitesu.txt b/docs/man/tripplitesu.txt index 44740671ae..d70144a95f 100644 --- a/docs/man/tripplitesu.txt +++ b/docs/man/tripplitesu.txt @@ -93,15 +93,15 @@ at all. INSTANT COMMANDS ---------------- -The following instant commands switch the outlet banks. They take no value: +The following instant commands switch the outlets. They take no value: a delayed shutdown of the whole UPS is what the 'shutdown.*' commands below are for. 'load.off':: - Switch off every controllable outlet bank immediately. + Switch off every controllable outlet immediately. 'load.on':: - Switch on every controllable outlet bank immediately. + Switch on every controllable outlet immediately. The following instant commands accept an optional delay, in seconds, passed as the 'value' argument of linkman:upscmd[8]: @@ -146,18 +146,20 @@ delay, choose a 'startdelay' longer than the 'offdelay' used there (or than the historical 5 seconds, when 'offdelay' is not set). A command that is reported as failed had at least one of the operations it -performs rejected by the device (or unanswered), so a partially applied -command is possible and worth re-checking with linkman:upsc[8]. +performs rejected by the device (or left unanswered), so it may have been +applied only partially. The driver logs which operation was rejected; the +published state (for example *outlet.n.status*) reflects what the device +actually did, as reported on the next poll. -The following instant commands control a single outlet bank, where 'n' -ranges from 1 to the number of banks reported by the device (see +The following instant commands control a single outlet, where 'n' +ranges from 1 to the number of outlets reported by the device (see *outlet.count* below). They take no value: 'outlet.n.load.off':: - Switch off outlet bank n immediately. + Switch off outlet n immediately. 'outlet.n.load.on':: - Switch on outlet bank n immediately. + Switch on outlet n immediately. The following instant commands control the device buzzer. They take no value; enabling/disabling the buzzer for good is done through the @@ -188,16 +190,16 @@ before keeps its last known value when the device stops answering, again with a warning for every failed read. *outlet.count*:: - Total number of controllable outlet banks. + Total number of controllable outlets. *outlet.n.status*:: - Status of outlet bank n, either 'on' or 'off', as reported by the - device relay status. This is the state of the bank relay, not a + Status of outlet n, either 'on' or 'off', as reported by the + device relay status. This is the state of the outlet relay, not a measure of the voltage present on the outlet: with the whole UPS output off, the reported value can still be 'on'. *outlet.n.switchable*:: - Set to 'yes' for every controllable outlet bank. + Set to 'yes' for every controllable outlet. *outlet.delay.start*:: Read/write power-on (boot) delay, in seconds: how long the device waits @@ -250,19 +252,22 @@ linkman:upsrw[8] to restore your preferred value, for example: upsrw -s ups.start.auto=no myups *ups.watchdog.status*:: - Read/write watchdog setting, either 'enabled' or 'disabled': the - device reboots the load when it does not hear from the driver for the - watchdog timeout. A timeout of 0 seconds means 'disabled', and - setting the variable to 'enabled' arms the watchdog with the timeout - last reported by the device, keeping the restart-alarm behaviour the - device uses. Note that the timeout itself is not exposed as a - separate variable, because there is no name for it in the NUT naming - scheme. + Read/write watchdog setting, either 'enabled' or 'disabled'. This is + an external (hardware) watchdog: the device expects the driver to keep + talking to it and, when contact is lost for longer than the watchdog + timeout, it power cycles the load, switching the outputs off and back + on. A timeout of 0 seconds means 'disabled', and setting the variable + to 'enabled' arms the watchdog with the timeout last reported by the + device, keeping the restart-alarm behaviour the device uses. Note that + the timeout itself is not exposed as a separate variable, because there + is no name for it in the NUT naming scheme. + -WARNING: Enabling this setting arms a timer the device uses to power cycle -the load when the driver stops talking to it, so a driver that hangs (or a -serial link that breaks) while the watchdog is armed can switch the load off -and on again. +WARNING: While the watchdog is armed, a driver that hangs or a serial link +that breaks is enough to switch the load off and on again, and the load is +powered only as long as the driver keeps polling the device. If you do not +need this protection, disable it with: ++ + upsrw -s ups.watchdog.status=disabled myups + The device reports the timeout and the restart-alarm flag, not a watchdog status: 'enabled' and 'disabled' are inferred from a non-zero timeout, which diff --git a/drivers/tripplitesu.c b/drivers/tripplitesu.c index 13d4fcfe4d..9f8611067b 100644 --- a/drivers/tripplitesu.c +++ b/drivers/tripplitesu.c @@ -99,8 +99,8 @@ The following instant commands are supported: load.off load.on - outlet.n.load.off (one per outlet bank) - outlet.n.load.on (one per outlet bank) + outlet.n.load.off (one per outlet) + outlet.n.load.on (one per outlet) beeper.mute clear.fault.record shutdown.reboot @@ -115,7 +115,7 @@ passed as the "value" argument of upscmd; it takes precedence over the "offdelay" setting from ups.conf, which in turn takes precedence over the built-in default. The "load.off", "load.on" and "outlet.n.load.*" - commands do not take a value: they only switch the outlet banks. + commands do not take a value: they only switch the outlets. "shutdown.return" and "shutdown.stayoff" clear the device return delay (SDR), so the load comes back only when mains is (re)applied or not at @@ -205,10 +205,10 @@ upsdrv_info_t upsdrv_info = { #define DEFAULT_WATCHDOG 255U #define MAX_WATCHDOG 255U -/* Sanity limit on the outlet bank count reported by the device (LET), so +/* Sanity limit on the outlet count reported by the device (LET), so * that a garbled answer cannot make the driver publish, register and poll - * an unbounded number of banks. */ -#define MAX_OUTLET_BANKS 16U + * an unbounded number of outlets. */ +#define MAX_OUTLETS 16U /* Command pacing: the device needs time to digest a command before it can * accept the next one, so a burst of commands sent back to back (as the @@ -243,7 +243,7 @@ static struct { }; static struct { - int outlet_banks; + int outlets; unsigned long commands_available; } ups; @@ -312,17 +312,6 @@ static unsigned int min_high_transfer, max_high_transfer; #define WATCHDOG "WDG" /* poll/set */ -/* Milliseconds elapsed since '*start', as a signed value ("now" is expected - * to be later than the reference time). */ -static long elapsed_milliseconds(const struct timeval *start) -{ - struct timeval now; - - gettimeofday(&now, NULL); - return (long)(now.tv_sec - start->tv_sec) * 1000L - + (long)(now.tv_usec - start->tv_usec) / 1000L; -} - /* Wait until at least 'command_delay' milliseconds have passed since the * previous command was sent, so that the device has time to get ready. * Nothing is waited for on an idle link, nor when the driver work between @@ -336,7 +325,7 @@ static void pace_before_command(void) if (!command_delay || !command_sent) return; - remaining = (long)command_delay - elapsed_milliseconds(&command_sent_at); + remaining = (long)command_delay - elapsed_since_timeval(&command_sent_at); if (remaining <= 0) return; @@ -492,7 +481,7 @@ static int send_set_or_fail(const char *command, const char *parameters) { if (send_set_command(command, parameters)) return STAT_INSTCMD_HANDLED; - upslogx(LOG_ERR, "device rejected or did not answer [%s%s]", + upslogx(LOG_ERR, "device rejected or did not answer [%s %s]", command, NUT_STRARG(parameters)); return STAT_INSTCMD_FAILED; } @@ -532,7 +521,7 @@ static void set_identification(const char *val) { response[MAX_RESPONSE_LENGTH - 1] = '\0'; if (!send_set_command(IDENTIFICATION, response)) upslogx(LOG_ERR, "%s: device rejected or did not " - "answer [%s%s]", __func__, IDENTIFICATION, + "answer [%s %s]", __func__, IDENTIFICATION, response); } } @@ -568,7 +557,7 @@ static void set_transfer_voltage_low(int val) { snprintf(response, sizeof(response), "%d;%d", val, high); if (!send_set_command(TRANSFER_VOLTAGE, response)) upslogx(LOG_ERR, "%s: device rejected or did not answer " - "[%s%s]", __func__, TRANSFER_VOLTAGE, response); + "[%s %s]", __func__, TRANSFER_VOLTAGE, response); } static int get_transfer_voltage_high(void) { @@ -602,7 +591,7 @@ static void set_transfer_voltage_high(int val) { snprintf(response, sizeof(response), "%d;%d", low, val); if (!send_set_command(TRANSFER_VOLTAGE, response)) upslogx(LOG_ERR, "%s: device rejected or did not answer " - "[%s%s]", __func__, TRANSFER_VOLTAGE, response); + "[%s %s]", __func__, TRANSFER_VOLTAGE, response); } static int get_sensitivity(void) { @@ -631,7 +620,7 @@ static void set_sensitivity(const char *val) { snprintf(parm, sizeof(parm), "%u", i); if (!send_set_command(VOLTAGE_SENSITIVITY, parm)) upslogx(LOG_ERR, "%s: device rejected or did " - "not answer [%s%s]", __func__, + "not answer [%s %s]", __func__, VOLTAGE_SENSITIVITY, parm); break; } @@ -662,11 +651,11 @@ static int auto_reboot(int enable) { mode = 2; if (do_command(POLL, AUTO_REBOOT, "", response) <= 0) { /* the current setting is unknown (some devices do not report - * it), so leave it alone and let the caller decide based on - * its own commands */ - upsdebugx(2, "%s: no answer for [%s], leaving it unchanged", - __func__, AUTO_REBOOT); - return 1; + * it), so it cannot be confirmed: report a failure so the + * caller aborts instead of powering the load off on a guess */ + upslogx(LOG_ERR, "%s: no answer for [%s], cannot set or " + "confirm ups.start.auto", __func__, AUTO_REBOOT); + return 0; } ptr = field(response, 0); oldmode = ptr ? atoi(ptr) : 0; @@ -674,7 +663,7 @@ static int auto_reboot(int enable) { snprintf(parm, sizeof(parm), "%d", mode); if (!send_set_command(AUTO_REBOOT, parm)) { upslogx(LOG_ERR, "%s: device rejected or did not answer " - "[%s%s]", __func__, AUTO_REBOOT, parm); + "[%s %s]", __func__, AUTO_REBOOT, parm); return 0; } /* the device stores this setting, so the change outlives the @@ -783,7 +772,7 @@ static void set_boot_delay(unsigned int delay) { snprintf(parm, sizeof(parm), "%03u", delay); if (!send_set_command(BOOT_DELAY, parm)) upslogx(LOG_ERR, "%s: device rejected or did not answer " - "[%s%s]", __func__, BOOT_DELAY, parm); + "[%s %s]", __func__, BOOT_DELAY, parm); } /* Read the watchdog setting and publish ups.watchdog.status. The device @@ -852,7 +841,7 @@ static void set_watchdog(int enable) { snprintf(parm, sizeof(parm), "%u;%u", timeout, watchdog_alarm); if (!send_set_command(WATCHDOG, parm)) upslogx(LOG_ERR, "%s: device rejected or did not answer " - "[%s%s]", __func__, WATCHDOG, parm); + "[%s %s]", __func__, WATCHDOG, parm); } /* Read the bypass reason recorded by the device (BPA) and publish it as @@ -922,7 +911,7 @@ static void set_eco_mode(int mode) { snprintf(parm, sizeof(parm), "%d", mode); if (!send_set_command(ECONOMIC_MODE, parm)) { upslogx(LOG_ERR, "%s: device rejected or did not answer " - "[%s%s]", __func__, ECONOMIC_MODE, parm); + "[%s %s]", __func__, ECONOMIC_MODE, parm); return; } dstate_setinfo("input.eco.switchable", "%s", mode == 1 ? "ECO" : "normal"); @@ -967,13 +956,13 @@ static void set_beeper_status(int enable) { snprintf(parm, sizeof(parm), "%d", enable ? 1 : 2); if (!send_set_command(ENABLE_BUZZER, parm)) { upslogx(LOG_ERR, "%s: device rejected or did not answer " - "[%s%s]", __func__, ENABLE_BUZZER, parm); + "[%s %s]", __func__, ENABLE_BUZZER, parm); return; } dstate_setinfo("ups.beeper.status", "%s", enable ? "enabled" : "disabled"); } -/* Read the relay status of each outlet bank (SOL: 0 = powered, 1 = off) +/* Read the relay status of each outlet (SOL: 0 = powered, 1 = off) * and publish it as outlet.n.status. Errors are not fatal: this is * auxiliary information, so a missing answer must not mark the whole UPS * as stale. */ @@ -983,7 +972,7 @@ static void get_outlet_status(void) { char varname[32]; unsigned int i; - for (i = 1; i <= (unsigned int)ups.outlet_banks; i++) { + for (i = 1; i <= (unsigned int)ups.outlets; i++) { snprintf(command, sizeof(command), "%s%u", RELAY_STATUS, i); if (do_command(POLL, command, "", response) <= 0) { upsdebugx(2, "%s: no response for [%s]", @@ -1118,7 +1107,7 @@ static int instcmd(const char *cmdname, const char *extra) int i; char parm[20]; char command[32]; /* "outlet..load.on" and friends */ - unsigned int bank, cmd_index; + unsigned int outlet, cmd_index; unsigned int delay; unsigned int fallback; int result = STAT_INSTCMD_HANDLED; @@ -1127,7 +1116,7 @@ static int instcmd(const char *cmdname, const char *extra) if (!strcasecmp(cmdname, "load.off")) { upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - for (i = 0; i < ups.outlet_banks; i++) { + for (i = 0; i < ups.outlets; i++) { snprintf(parm, sizeof(parm), "%d;1", i + 1); if (send_set_or_fail(RELAY_OFF, parm) != STAT_INSTCMD_HANDLED) result = STAT_INSTCMD_FAILED; @@ -1136,18 +1125,18 @@ static int instcmd(const char *cmdname, const char *extra) } if (!strcasecmp(cmdname, "load.on")) { upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); - for (i = 0; i < ups.outlet_banks; i++) { + for (i = 0; i < ups.outlets; i++) { snprintf(parm, sizeof(parm), "%d;1", i + 1); if (send_set_or_fail(RELAY_ON, parm) != STAT_INSTCMD_HANDLED) result = STAT_INSTCMD_FAILED; } return result; } - /* one set of commands per outlet bank; these take no value */ + /* one set of commands per outlet; these take no value */ for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(outlet_commands); cmd_index++) { - for (bank = 1; bank <= (unsigned int)ups.outlet_banks; bank++) { + for (outlet = 1; outlet <= (unsigned int)ups.outlets; outlet++) { snprintf(command, sizeof(command), "outlet.%u.%s", - bank, outlet_commands[cmd_index].suffix); + outlet, outlet_commands[cmd_index].suffix); if (strcasecmp(cmdname, command)) continue; @@ -1156,7 +1145,7 @@ static int instcmd(const char *cmdname, const char *extra) else upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - snprintf(parm, sizeof(parm), "%u;1", bank); + snprintf(parm, sizeof(parm), "%u;1", outlet); return send_set_or_fail(outlet_commands[cmd_index].relay, parm); } @@ -1183,20 +1172,30 @@ static int instcmd(const char *cmdname, const char *extra) return STAT_INSTCMD_FAILED; upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - if (!auto_reboot(shutdown_commands[cmd_index].autoreboot)) - result = STAT_INSTCMD_FAILED; + /* set the auto-reboot and return-delay prerequisites before + * scheduling the shutdown: if either cannot be confirmed, + * stop here and leave the load powered rather than power it + * off with no guaranteed way to come back on */ + if (!auto_reboot(shutdown_commands[cmd_index].autoreboot)) { + upslogx(LOG_ERR, "%s: could not set or confirm " + "ups.start.auto, not powering the load off", cmdname); + return STAT_INSTCMD_FAILED; + } /* the reboot commands ask the load to come back right behind * the shutdown; the others clear any return delay left over * from an earlier command, so that the load comes back only * when mains is (re)applied, or not at all */ if (!set_start_delay(shutdown_commands[cmd_index].timed_return - ? reboot_start_delay(delay) : 0U)) - result = STAT_INSTCMD_FAILED; + ? reboot_start_delay(delay) : 0U)) { + upslogx(LOG_ERR, "%s: could not set the return " + "delay, not powering the load off", cmdname); + return STAT_INSTCMD_FAILED; + } snprintf(parm, sizeof(parm), "%u", delay); if (send_set_or_fail(TSU_SHUTDOWN_ACTION, parm) != STAT_INSTCMD_HANDLED) - result = STAT_INSTCMD_FAILED; - return result; + return STAT_INSTCMD_FAILED; + return STAT_INSTCMD_HANDLED; } if (!strcasecmp(cmdname, "shutdown.stop")) { upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); @@ -1260,15 +1259,21 @@ static int setvar(const char *varname, const char *val) return STAT_SET_HANDLED; } if (!strcasecmp(varname, "ups.start.auto")) { + int enable; + if (!strcasecmp(val, "yes")) - auto_reboot(1); + enable = 1; else if (!strcasecmp(val, "no")) - auto_reboot(0); + enable = 0; else { upslogx(LOG_ERR, "setvar(%s): invalid value '%s' " "(expected yes or no)", varname, val); return STAT_SET_FAILED; } + /* auto_reboot() logs the reason on failure: a device that + * does not report the setting cannot confirm it */ + if (!auto_reboot(enable)) + return STAT_SET_FAILED; return STAT_SET_HANDLED; } if (!strcasecmp(varname, "ups.beeper.status")) { @@ -1438,17 +1443,17 @@ void upsdrv_initinfo(void) } } if (do_command(POLL, OUTLET_RELAYS, "", response) > 0) { - int banks = atoi(response); + int outlet_count = atoi(response); - if (banks > 0 && banks <= (int)MAX_OUTLET_BANKS) - ups.outlet_banks = banks; - else if (banks != 0) + if (outlet_count > 0 && outlet_count <= (int)MAX_OUTLETS) + ups.outlets = outlet_count; + else if (outlet_count != 0) upslogx(LOG_WARNING, "%s: ignoring implausible " - "outlet bank count [%s]", __func__, response); + "outlet count [%s]", __func__, response); } - if (ups.outlet_banks > 0) { - dstate_setinfo("outlet.count", "%d", ups.outlet_banks); - for (i = 1; i <= (unsigned int)ups.outlet_banks; i++) { + if (ups.outlets > 0) { + dstate_setinfo("outlet.count", "%d", ups.outlets); + for (i = 1; i <= (unsigned int)ups.outlets; i++) { snprintf(buf, sizeof(buf), "outlet.%u.switchable", i); dstate_setinfo(buf, "%s", "yes"); } @@ -1517,11 +1522,11 @@ void upsdrv_initinfo(void) sensitivity[i].name); } /* load.off, load.on and the commands below switch individual outlet - * banks, so they are only registered when the device reports any */ - if (ups.outlet_banks) { + * outlets, so they are only registered when the device reports any */ + if (ups.outlets) { dstate_addcmd("load.off"); dstate_addcmd("load.on"); - for (i = 1; i <= (unsigned int)ups.outlet_banks; i++) { + for (i = 1; i <= (unsigned int)ups.outlets; i++) { snprintf(buf, sizeof(buf), "outlet.%u.load.off", i); dstate_addcmd(buf); snprintf(buf, sizeof(buf), "outlet.%u.load.on", i); @@ -1728,10 +1733,16 @@ void upsdrv_shutdown(void) printf("Status failed. Assuming it's on battery and trying a shutdown anyway.\n"); /* in case the power is on, tell it to automatically reboot. if * it is off, this has no effect. */ - if (!auto_reboot(1)) - upslogx(LOG_ERR, "%s: could not set ups.start.auto", __func__); - if (!set_start_delay(startdelay)) - upslogx(LOG_ERR, "%s: could not set the return delay", __func__); + if (!auto_reboot(1)) { + upslogx(LOG_ERR, "%s: could not set or confirm ups.start.auto, " + "not powering the load off", __func__); + return; + } + if (!set_start_delay(startdelay)) { + upslogx(LOG_ERR, "%s: could not set the return delay, " + "not powering the load off", __func__); + return; + } /* delay before shutdown, in seconds: honor 'offdelay' if the user set * it in ups.conf, otherwise keep the historical default */ snprintf(parm, sizeof(parm), "%u", From 068c4cee28fb0932234811c4b8adfd90b48ee626 Mon Sep 17 00:00:00 2001 From: TomZanna Date: Sun, 11 Oct 2026 20:00:53 +0000 Subject: [PATCH 06/13] feat(tripplitesu): add watchdog variables and harden shutdown handling Expose the watchdog timeout and restart-alarm fields as the read/write ups.watchdog.timeout and ups.watchdog.alarm variables, add a reset.watchdog instant command, and disarm an armed watchdog when the driver exits on purpose so that a restart (e.g. during a package upgrade) does not power cycle the load. Watchdogs the driver never enabled or that are already disabled are left untouched, and a crash or SIGKILL does not disarm them. Make shutdown.default always turn auto-reboot on so every outlet comes back when mains returns, and add the shutdown_mode setting to choose between stopping at the first failed or unanswered operation (abort) and continuing (best effort, default). It no longer sets a return delay of its own, so startdelay only seeds ups.delay.start. The load.off/load.on multi-frame commands now also stop at the first rejected or unanswered frame. Signed-off-by: TomZanna --- NEWS.adoc | 26 ++++ data/cmdvartab | 2 + docs/man/tripplitesu.txt | 155 +++++++++++++----- docs/nut-names.txt | 4 + drivers/tripplitesu.c | 328 ++++++++++++++++++++++++++++++++------- 5 files changed, 418 insertions(+), 97 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index fab8dac463..b1c8b47a9e 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -408,6 +408,32 @@ https://github.com/networkupstools/nut/milestone/13 link is not penalized, and the special value `-1` makes the driver start without waiting and pace commands by one second only if one of them times out. + * The watchdog is disarmed when the driver exits on purpose (a handled + signal, the `driver.exit` instant command or the end of a data dump), + so that a driver restart - for example during a package upgrade - does + not leave an armed watchdog that would power cycle the load. A + watchdog the driver never saw enabled, or one already disabled, is left + untouched, and a crash or a SIGKILL does not disarm it. + * Added the read/write `ups.watchdog.timeout` and `ups.watchdog.alarm` + variables, which expose the two fields the device stores for its + watchdog - the timeout in seconds, 0 disabling the watchdog, and the + restart-alarm flag - instead of only the derived + `ups.watchdog.status`, and the `reset.watchdog` instant command, which + sends the watchdog setting the driver has again to restart its timer. + * The driver shutdown handling (`shutdown.default`, which + `upsdrvctl shutdown` runs) always asks the device to start the load + again when mains is (re)applied (the auto-reboot setting is turned on), + so that every outlet is powered again after the outage, and the new + `shutdown_mode` setting decides whether it stops at the first operation + the device rejects or does not answer (`abort`, fail safe) or keeps + sending the rest (`continue`, best effort, the default). + It also sets no return delay of its own any more, so that it does not + schedule a restart on a timer, and the `startdelay` setting now only + seeds the `ups.delay.start` variable. + * The multi-frame commands (`load.off` and `load.on`, which walk the + outlet banks) stop at the first frame the device rejects or does not + answer, as the shutdown commands already did, instead of sending the + remaining frames. - `usbhid-ups` driver updates: * When reconnecting, report success more visibly (not only in debug), and diff --git a/data/cmdvartab b/data/cmdvartab index 4a1a0ce48c..de8e135512 100644 --- a/data/cmdvartab +++ b/data/cmdvartab @@ -44,6 +44,8 @@ VARDESC ups.start.battery "Allow to start UPS from battery" VARDESC ups.start.reboot "UPS reboots when power returns during shutdown delay" VARDESC ups.shutdown "Enable or disable UPS shutdown ability (poweroff)" VARDESC ups.watchdog.status "UPS watchdog status (enabled or disabled)" +VARDESC ups.watchdog.timeout "UPS watchdog timeout (seconds, 0 disables)" +VARDESC ups.watchdog.alarm "UPS watchdog restart alarm (enabled or disabled)" VARDESC input.voltage "Input voltage (V)" VARDESC input.voltage.extended "Extended input voltage range" diff --git a/docs/man/tripplitesu.txt b/docs/man/tripplitesu.txt index d70144a95f..e613dc488e 100644 --- a/docs/man/tripplitesu.txt +++ b/docs/man/tripplitesu.txt @@ -75,20 +75,39 @@ linkman:upsdrvctl[8] 'shutdown') uses this setting as well, but keeps the historical default of 5 seconds when it is not set in linkman:ups.conf[5]. *startdelay*='minutes':: -Set the return (restart) delay, in minutes, used by the driver shutdown -handling as the delay before the load is started again. The default value -is 1. +Set the return (restart) delay, in minutes, that the driver publishes as the +read/write variable *ups.delay.start* (see VARIABLES below). The default +value is 1. + The parameter uses the unit of the device command ('SDR', whole minutes); -values outside the range are rejected. The same delay is also exposed, in -seconds, as the read/write variable *ups.delay.start* (see VARIABLES below), -where a value that is not a whole number of minutes is rounded up when it is -sent to the device. +values outside the range are rejected. The device command is write-only, so +the value is not read back from the UPS: after a driver restart the variable +reflects this setting, and a value written with linkman:upsrw[8] is published +as it was sent (a value that is not a whole number of minutes is rounded up +when it is sent to the device). + -The 'shutdown.return' and 'shutdown.stayoff' commands do not use this -setting: they clear the return delay instead (see the INSTANT COMMANDS -section), so that the load comes back only when mains is (re)applied, or not -at all. +The driver shutdown handling does not use this setting: it turns the +auto-reboot setting on and sets the shutdown delay, and leaves the return delay +stored on the device as it is (see the note in the INSTANT COMMANDS section). + +*shutdown_mode*='continue'|'abort':: +Set how the driver shutdown handling reacts when the device rejects, or does +not answer, one of the operations that make up a shutdown, that is switching +the auto-reboot setting on and scheduling the shutdown itself. The default +value is 'continue'. ++ +'continue' (best effort) keeps going, so a device that does not confirm the +auto-reboot setting is still asked to switch the load off: the shutdown +happens even when part of it could not be confirmed. 'abort' (fail safe) +stops at the first failure and leaves the load powered instead, because a +shutdown that is not known to be able to bring the load back is worse than no +shutdown. + +WARNING: With 'abort', a shutdown the device does not confirm leaves the load +powered: the computer may already be going down at that point, so an outage +then ends in a hard power loss rather than in an orderly shutdown. Use +'abort' only with a device that answers reliably and a load that must not be +switched off without a confirmed way back. INSTANT COMMANDS ---------------- @@ -131,6 +150,12 @@ as the 'value' argument of linkman:upscmd[8]: The accepted range is 1 through 3600 seconds. An invalid value makes the command fail without sending anything to the UPS. +The driver shutdown handling is available as the 'shutdown.default' instant +command, which is what linkman:upsdrvctl[8] 'shutdown' runs. It takes no +value: before it switches the load off, after the 'offdelay' setting (5 +seconds when it is not set in linkman:ups.conf[5]), it asks the device to +start the load again once mains is (re)applied - see the note below. + All other instant commands ignore any value passed to them. NOTE: The device takes the shutdown delay in seconds (its 'SDA' command) and @@ -139,17 +164,22 @@ again when the restart delay is longer than the shutdown delay. The two reboot commands therefore add one second to the shutdown delay before rounding it up to whole minutes, so that the restart always falls behind the off, while 'shutdown.return' and 'shutdown.stayoff' clear the restart -delay, so that the load does not come back on a timer of its own. The -driver shutdown handling sends the configured 'startdelay' instead: as the -device only restarts the load when that delay is longer than the shutdown -delay, choose a 'startdelay' longer than the 'offdelay' used there (or than -the historical 5 seconds, when 'offdelay' is not set). - -A command that is reported as failed had at least one of the operations it -performs rejected by the device (or left unanswered), so it may have been -applied only partially. The driver logs which operation was rejected; the -published state (for example *outlet.n.status*) reflects what the device -actually did, as reported on the next poll. +delay, so that the load does not come back on a timer of its own. ++ +The driver shutdown handling (the 'shutdown.default' instant command, which +is what linkman:upsdrvctl[8] 'shutdown' runs) turns the auto-reboot setting +on and sets the shutdown delay, and sends no return delay at all: it does not +clear a return delay stored on the device either, so a value left behind by an +earlier command (or written to *ups.delay.start* with linkman:upsrw[8]) still +applies and the device starts the load again on that timer when it is longer +than the shutdown delay. Write 0 to *ups.delay.start* if the load has to +stay off until mains is (re)applied. + +A command that is reported as failed met an operation the device rejected (or +left unanswered). The driver logs which operation failed and stops there, so +the rest of the command is not attempted and it may have been applied only +partially. The published state (for example *outlet.n.status*) reflects what +the device actually did, as reported on the next poll. The following instant commands control a single outlet, where 'n' ranges from 1 to the number of outlets reported by the device (see @@ -175,6 +205,18 @@ value; enabling/disabling the buzzer for good is done through the itself is published, read-only, as *input.transfer.reason*, which is refreshed on every poll cycle. +'reset.watchdog':: + Restart the watchdog timer of the device, which is the "kick" a + hardware watchdog needs to stay quiet. The command takes no value: it + sends the watchdog setting the driver has, so the timeout and the + restart-alarm behaviour are the ones the device last reported or + accepted (use *ups.watchdog.timeout* and *ups.watchdog.alarm* to change + them, and *ups.watchdog.status* to arm or disarm the watchdog). It + fails when the device never reported its watchdog setting, so that a + kick cannot arm a watchdog with assumed values; a watchdog that is + disabled is written again with a timeout of 0, which leaves it as it + is. + VARIABLES --------- @@ -182,12 +224,12 @@ The driver publishes the usual battery, input and output variables, plus the following ones: + Read/write variables that describe a device setting (*outlet.delay.start*, -*ups.start.auto*, *ups.watchdog.status*, *input.eco.switchable* and -*ups.beeper.status*) are published, and made settable, only once the device -has reported the setting: when it does not answer, the variable is absent -(no value is assumed) and a warning is logged. A setting that was read -before keeps its last known value when the device stops answering, again -with a warning for every failed read. +*ups.start.auto*, *ups.watchdog.status*, *ups.watchdog.timeout*, +*ups.watchdog.alarm*, *input.eco.switchable* and *ups.beeper.status*) are +published, and made settable, only once the device has reported the setting: +when it does not answer, the variable is absent (no value is assumed) and a +warning is logged. A setting that was read before keeps its last known value +when the device stops answering, again with a warning for every failed read. *outlet.count*:: Total number of controllable outlets. @@ -228,12 +270,18 @@ with a warning for every failed read. settings above. *ups.delay.start*:: - Read/write return delay, in seconds, used by the driver shutdown - handling (the 'shutdown.default' instant command and - linkman:upsdrvctl[8] 'shutdown'). Values are rounded up to whole - minutes when they are sent to the device and, since the corresponding - device command is write-only, the variable is not read back from the - UPS: after a driver restart it reflects the 'startdelay' setting. + Read/write return (restart) delay of the device, in seconds: how long + the device leaves the load switched off before it starts it again by + itself. Values are rounded up to whole minutes when they are sent to + the device ('SDR') and, since that command is write-only, the variable + is not read back from the UPS: after a driver restart it reflects the + 'startdelay' setting. ++ +The device only starts the load again when this delay is longer than the +shutdown delay of the command that switched it off, so it has to be chosen +together with the delay passed to the shutdown command (see the note in the +INSTANT COMMANDS section). The driver shutdown handling does not change it, +while 'shutdown.return' and 'shutdown.stayoff' clear it. *ups.start.auto*:: Read/write auto-reboot setting, either 'yes' or 'no': when it is 'yes' @@ -257,10 +305,16 @@ linkman:upsrw[8] to restore your preferred value, for example: talking to it and, when contact is lost for longer than the watchdog timeout, it power cycles the load, switching the outputs off and back on. A timeout of 0 seconds means 'disabled', and setting the variable - to 'enabled' arms the watchdog with the timeout last reported by the - device, keeping the restart-alarm behaviour the device uses. Note that - the timeout itself is not exposed as a separate variable, because there - is no name for it in the NUT naming scheme. + to 'enabled' arms the watchdog with the timeout reported by the device + (255 seconds, the value the vendor software used, when the device + reports no timeout), keeping the restart-alarm behaviour the device + uses. Use *ups.watchdog.timeout* and *ups.watchdog.alarm* to choose + those two fields, and the 'reset.watchdog' instant command to only + restart the timer. ++ +The device stores the timeout and the restart-alarm flag together rather than +a status, so the three variables are published from the same setting and a +change to one of them writes the whole pair to the device. + WARNING: While the watchdog is armed, a driver that hangs or a serial link that breaks is enough to switch the load off and on again, and the load is @@ -272,6 +326,31 @@ need this protection, disable it with: The device reports the timeout and the restart-alarm flag, not a watchdog status: 'enabled' and 'disabled' are inferred from a non-zero timeout, which has not been verified on hardware. ++ +When the driver exits on purpose, it disarms the watchdog before closing the +port: a watchdog left armed would power cycle the load as soon as the device +stops hearing from the driver, turning a driver restart (for example during a +package upgrade) into an unexpected outage. This covers the exits the driver +can handle, that is the signals used to stop it, the 'driver.exit' instant +command and the end of a data dump. A watchdog that is already disabled, or +that the device never reported, is left alone, and a driver that crashes or is +killed without unwinding does not get there, so the watchdog keeps covering +that case. + +*ups.watchdog.timeout*:: + Read/write watchdog timeout, in seconds: the time the device waits + without hearing from the driver before it power cycles the load (see + *ups.watchdog.status* above). The accepted range is 0 through 255: 0 + disables the watchdog and a non-zero value enables it, which is the + value the 'enabled'/'disabled' status is derived from. + +*ups.watchdog.alarm*:: + Read/write restart-alarm flag, either 'enabled' or 'disabled': it tells + the device whether to sound its alarm when the watchdog restarts the + load. The device stores the flag as a number, 1 for 'enabled' and 2 + for 'disabled', next to the timeout; changing it leaves the timeout + alone, so it can be set while the watchdog is disabled. What exactly + triggers that alarm has not been verified on hardware. *input.transfer.reason*:: Read-only reason for the last transfer recorded by the device, diff --git a/docs/nut-names.txt b/docs/nut-names.txt index c93e887c1e..061acf8337 100644 --- a/docs/nut-names.txt +++ b/docs/nut-names.txt @@ -280,6 +280,10 @@ ups: General unit information see the note below) | vendor:eaton:ECO | ups.watchdog.status | UPS watchdog status (enabled or disabled) | disabled +| ups.watchdog.timeout | UPS watchdog timeout + (seconds, 0 disables) | 255 +| ups.watchdog.alarm | UPS watchdog restart alarm + (enabled or disabled) | disabled | ups.start.auto | UPS starts when mains is (re)applied | yes | ups.start.battery | Allow to start UPS from diff --git a/drivers/tripplitesu.c b/drivers/tripplitesu.c index 9f8611067b..6e42a3dda6 100644 --- a/drivers/tripplitesu.c +++ b/drivers/tripplitesu.c @@ -57,6 +57,7 @@ command_delay offdelay startdelay + shutdown_mode The following variables are supported (RW = read/write): ambient.humidity (1) @@ -93,7 +94,9 @@ ups.status ups.start.auto (RW) ups.test.result + ups.watchdog.alarm (RW) ups.watchdog.status (RW) + ups.watchdog.timeout (RW) ups.contacts (1) The following instant commands are supported: @@ -103,6 +106,7 @@ outlet.n.load.on (one per outlet) beeper.mute clear.fault.record + reset.watchdog shutdown.reboot shutdown.reboot.graceful shutdown.return @@ -114,14 +118,19 @@ The shutdown-related commands above accept an optional delay, in seconds, passed as the "value" argument of upscmd; it takes precedence over the "offdelay" setting from ups.conf, which in turn takes precedence over - the built-in default. The "load.off", "load.on" and "outlet.n.load.*" - commands do not take a value: they only switch the outlets. + the built-in default. The "load.off", "load.on", "outlet.n.load.*" and + "reset.watchdog" commands do not take a value. + + Commands that send more than one frame stop at the first operation the + device rejects or does not answer, so a failed command is never applied + partially: the driver logs the operation that failed and sends nothing + more. "load.off" and "load.on", for instance, leave the outlets they did + not reach untouched. "shutdown.return" and "shutdown.stayoff" clear the device return delay (SDR), so the load comes back only when mains is (re)applied or not at all, while the two reboot commands ask for the load to come back right - behind the shutdown (just behind it, see reboot_start_delay()). The - "startdelay" setting is used by the driver shutdown handler only. + behind the shutdown (just behind it, see reboot_start_delay()). Because that is what the commands mean on this hardware, the shutdown commands change the device auto-reboot setting ("ups.start.auto", ARB); @@ -129,7 +138,20 @@ and the driver logs it (restore it with upsrw if needed). The buzzer is a device setting as well: enabling/disabling it is done through the read/write "ups.beeper.status" variable, while the "beeper.mute" command - only silences the alarm that is currently sounding. + only silences the alarm that is currently sounding. The watchdog is the + third device setting of that kind: its timeout and restart-alarm flag are + the read/write "ups.watchdog.timeout" and "ups.watchdog.alarm" + variables, "ups.watchdog.status" is their enabled/disabled view, and the + "reset.watchdog" command sends the stored setting again. + + The driver shutdown handler (the "shutdown.default" instant command, run + by "upsdrvctl shutdown" and by the "driver.killpower" request) always + turns the auto-reboot setting on (ARB) and then schedules the shutdown + (SDA), so that the load is powered again when mains is (re)applied; + unlike the commands above it does not touch the return delay (SDR). The + "shutdown_mode" setting chooses whether a failure of one of those two + operations stops the handler there ("abort", fail safe) or not + ("continue", best effort, the default). The following ups.status values are supported: BOOST (1) @@ -200,8 +222,10 @@ upsdrv_info_t upsdrv_info = { /* Watchdog setting (WDG): the device stores a timeout in seconds (up to * three digits, 0 disables it) plus a flag selecting whether the restart - * alarm is enabled (1) or disabled (2). The timeout and the flag are - * preserved when the watchdog is (dis)armed from NUT. */ + * alarm is enabled (1) or disabled (2). Both fields are written together, + * so the driver keeps them apart in the read/write variables + * ups.watchdog.timeout and ups.watchdog.alarm, and reloads the pair it has + * when the watchdog is (dis)armed or kicked from NUT. */ #define DEFAULT_WATCHDOG 255U #define MAX_WATCHDOG 255U @@ -255,9 +279,10 @@ static struct timeval command_sent_at; /* when the last command was sent */ static unsigned int offdelay = DEFAULT_OFFDELAY; /* delay in seconds before shutdown */ static int offdelay_from_conf = 0; /* set if 'offdelay' was provided in ups.conf */ static unsigned int startdelay = DEFAULT_STARTDELAY; /* delay in seconds before return */ -static unsigned int watchdog_seconds = 0; /* watchdog timeout last reported by the device */ -static unsigned int watchdog_alarm = 2; /* watchdog restart-alarm flag last reported */ -static int watchdog_known = 0; /* set once the device reported its watchdog setting */ +static unsigned int watchdog_seconds = 0; /* watchdog timeout currently set on the device */ +static unsigned int watchdog_alarm = 2; /* watchdog restart-alarm flag currently set on the device */ +static int watchdog_known = 0; /* set once the device reported or accepted its watchdog setting */ +static int shutdown_abort_on_error = 0; /* 1 when 'shutdown_mode' asks the driver shutdown handler to stop at the first failure */ static unsigned int min_low_transfer, max_low_transfer; /* reported transfer voltage ranges */ static unsigned int min_high_transfer, max_high_transfer; @@ -775,11 +800,28 @@ static void set_boot_delay(unsigned int delay) { "[%s %s]", __func__, BOOT_DELAY, parm); } -/* Read the watchdog setting and publish ups.watchdog.status. The device - * reports ";", where a timeout of 0 - * means that the watchdog is disabled. This is auxiliary information, so a - * missing or unexpected answer must not mark the whole UPS as stale: a - * warning is logged and the previously published value is left in place. */ +/* Publish the watchdog variables from the setting the driver has: the one + * the device reported, or the one it accepted. The device stores a timeout + * and a restart-alarm flag, not a status, so 'enabled' and 'disabled' are + * derived from a non-zero timeout. */ +static void publish_watchdog(void) { + dstate_setinfo("ups.watchdog.status", "%s", + watchdog_seconds ? "enabled" : "disabled"); + dstate_setinfo("ups.watchdog.timeout", "%u", watchdog_seconds); + dstate_setinfo("ups.watchdog.alarm", "%s", + watchdog_alarm == 1 ? "enabled" : "disabled"); +} + +/* Read the watchdog setting (WDG) and publish it as ups.watchdog.status + * (enabled/disabled), ups.watchdog.timeout (seconds, 0 = disabled) and + * ups.watchdog.alarm (enabled/disabled; the device stores 1 = enabled and + * 2 = disabled). The device reports ";", where a timeout of 0 means that the watchdog is disabled. This is + * auxiliary information, so a missing or unexpected answer must not mark the + * whole UPS as stale: a warning is logged and the setting the driver has is + * left in place. Both fields are kept because the device stores them + * together: whenever one of them changes, the driver has to send the other + * one along. */ static int get_watchdog(void) { char response[MAX_RESPONSE_LENGTH]; char *ptr; @@ -816,32 +858,101 @@ static int get_watchdog(void) { watchdog_alarm = (unsigned int)alarm; } - dstate_setinfo("ups.watchdog.status", "%s", - watchdog_seconds ? "enabled" : "disabled"); + publish_watchdog(); return 1; } -/* Arm or disarm the watchdog, keeping the timeout (255 seconds when none - * was ever reported) and the restart-alarm flag that the device uses. */ -static void set_watchdog(int enable) { +/* Send the watchdog setting (WDG ";"): the + * device stores the two fields together, so both are always written. The + * values it accepts are remembered as the setting the driver has, so that + * the watchdog variables can be published from them without another round + * trip and disable_watchdog_on_exit() knows whether it has to act even when + * reading the value back fails. Returns 1 when the device accepted the + * setting. */ +static int send_watchdog_setting(unsigned int timeout, unsigned int alarm) +{ char parm[20]; - unsigned int timeout = 0; - if (enable) { - /* arming without a reading uses assumed values, which is - * worth telling about: the timeout is not the one the device - * would have reported */ - if (!watchdog_known) - upslogx(LOG_WARNING, "%s: arming the watchdog with an " - "assumed timeout, the device never reported " - "its [%s] setting", __func__, WATCHDOG); - timeout = watchdog_seconds ? watchdog_seconds : DEFAULT_WATCHDOG; + if (timeout > MAX_WATCHDOG || (alarm != 1 && alarm != 2)) { + upslogx(LOG_ERR, "%s: refusing to write the watchdog setting " + "[%u;%u]", __func__, timeout, alarm); + return 0; } - snprintf(parm, sizeof(parm), "%u;%u", timeout, watchdog_alarm); - if (!send_set_command(WATCHDOG, parm)) + snprintf(parm, sizeof(parm), "%u;%u", timeout, alarm); + if (!send_set_command(WATCHDOG, parm)) { upslogx(LOG_ERR, "%s: device rejected or did not answer " "[%s %s]", __func__, WATCHDOG, parm); + return 0; + } + + watchdog_seconds = timeout; + watchdog_alarm = alarm; + watchdog_known = 1; + publish_watchdog(); + return 1; +} + +/* Arm or disarm the watchdog, keeping the timeout (255 seconds when the + * device never reported one) and the restart-alarm flag that the device + * uses: arming a watchdog whose restart alarm is disabled must not enable + * that alarm, and the other way round. Arming a watchdog that was never + * read uses a timeout that is assumed, which is worth telling about. */ +static int set_watchdog(int enable) { + if (enable && !watchdog_known) + upslogx(LOG_WARNING, "%s: arming the watchdog with an " + "assumed timeout, the device never reported " + "its [%s] setting", __func__, WATCHDOG); + + return send_watchdog_setting(enable + ? (watchdog_seconds ? watchdog_seconds : DEFAULT_WATCHDOG) + : 0U, watchdog_alarm); +} + +/* The "reset.watchdog" instant command: send the setting the device last + * reported (or accepted) again, which restarts its timer. The command + * takes no value, so the watchdog it kicks is the one the driver is + * configured with. A watchdog that was never read is left alone, so that a + * kick cannot arm it with assumed values; a watchdog that is disabled is + * simply refreshed (0 is sent again) so that its stored timeout is left as + * the device reported it. */ +static int reset_watchdog(void) { + if (!watchdog_known) { + upslogx(LOG_ERR, "%s: the device never reported its [%s] " + "setting, not resetting the watchdog", + __func__, WATCHDOG); + return STAT_INSTCMD_FAILED; + } + + if (!watchdog_seconds) + upslogx(LOG_INFO, "%s: the watchdog is disabled (timeout " + "0), the kick only refreshes that", __func__); + + if (!send_watchdog_setting(watchdog_seconds, watchdog_alarm)) + return STAT_INSTCMD_FAILED; + + return STAT_INSTCMD_HANDLED; +} + +/* A watchdog left armed power cycles the load as soon as the device stops + * hearing from the driver, so disarm it when the driver goes away on + * purpose: a signal handled in main.c (SIGTERM, SIGINT, SIGQUIT), the + * 'driver.exit' instant command and the end of a data dump all run + * upsdrv_cleanup(), which calls this function before closing the port. A + * driver restart (for example during a package upgrade) then does not turn + * into an unexpected outage. Only a watchdog the driver believes to be + * armed is touched, so a device whose watchdog is already disabled is left + * alone and nothing is sent at all. A driver that crashes or is killed + * without unwinding never gets here, so the watchdog keeps covering that + * case. */ +static void disable_watchdog_on_exit(void) +{ + if (!watchdog_seconds) + return; + + upslogx(LOG_INFO, "%s: disabling the watchdog before exiting", + __func__); + set_watchdog(0); } /* Read the bypass reason recorded by the device (BPA) and publish it as @@ -1104,33 +1215,37 @@ static int instcmd(const char *cmdname, const char *extra) { "shutdown.return", 1, 1, 0 }, { "shutdown.stayoff", 0, 1, 0 } }; - int i; char parm[20]; char command[32]; /* "outlet..load.on" and friends */ unsigned int outlet, cmd_index; unsigned int delay; unsigned int fallback; - int result = STAT_INSTCMD_HANDLED; upsdebug_INSTCMD_STARTING(cmdname, extra); + /* Switching the outlets is a multi-frame command: stop at the first + * frame the device rejects or does not answer, so that a failure is + * not reported as a success (the outlets that were not reached are + * left as they were). */ if (!strcasecmp(cmdname, "load.off")) { upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - for (i = 0; i < ups.outlets; i++) { - snprintf(parm, sizeof(parm), "%d;1", i + 1); - if (send_set_or_fail(RELAY_OFF, parm) != STAT_INSTCMD_HANDLED) - result = STAT_INSTCMD_FAILED; + for (outlet = 1; outlet <= (unsigned int)ups.outlets; outlet++) { + snprintf(parm, sizeof(parm), "%u;1", outlet); + if (send_set_or_fail(RELAY_OFF, parm) + != STAT_INSTCMD_HANDLED) + return STAT_INSTCMD_FAILED; } - return result; + return STAT_INSTCMD_HANDLED; } if (!strcasecmp(cmdname, "load.on")) { upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); - for (i = 0; i < ups.outlets; i++) { - snprintf(parm, sizeof(parm), "%d;1", i + 1); - if (send_set_or_fail(RELAY_ON, parm) != STAT_INSTCMD_HANDLED) - result = STAT_INSTCMD_FAILED; + for (outlet = 1; outlet <= (unsigned int)ups.outlets; outlet++) { + snprintf(parm, sizeof(parm), "%u;1", outlet); + if (send_set_or_fail(RELAY_ON, parm) + != STAT_INSTCMD_HANDLED) + return STAT_INSTCMD_FAILED; } - return result; + return STAT_INSTCMD_HANDLED; } /* one set of commands per outlet; these take no value */ for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(outlet_commands); cmd_index++) { @@ -1161,6 +1276,11 @@ static int instcmd(const char *cmdname, const char *extra) /* reset the bypass reason recorded by the device */ return send_set_or_fail(BYPASS_REASON, "0"); } + if (!strcasecmp(cmdname, "reset.watchdog")) { + /* kick the watchdog: this takes no value, it sends the + * setting the driver has, which restarts the device timer */ + return reset_watchdog(); + } for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(shutdown_commands); cmd_index++) { if (strcasecmp(cmdname, shutdown_commands[cmd_index].name)) @@ -1292,17 +1412,53 @@ static int setvar(const char *varname, const char *val) return STAT_SET_HANDLED; } if (!strcasecmp(varname, "ups.watchdog.status")) { + if (!strcasecmp(val, "enabled")) { + if (!set_watchdog(1)) + return STAT_SET_FAILED; + } else if (!strcasecmp(val, "disabled")) { + if (!set_watchdog(0)) + return STAT_SET_FAILED; + } else { + upslogx(LOG_ERR, "setvar(%s): invalid value '%s' " + "(expected enabled or disabled)", varname, val); + return STAT_SET_FAILED; + } + return STAT_SET_HANDLED; + } + if (!strcasecmp(varname, "ups.watchdog.timeout")) { + unsigned int timeout; + + /* 0 disables the watchdog; the restart-alarm flag the + * device stores with it is kept as it is */ + if (!parse_uint_range(val, 0U, MAX_WATCHDOG, &timeout)) { + upslogx(LOG_ERR, "setvar(%s): invalid timeout value " + "'%s' (expected 0..%u seconds, 0 disables)", + varname, val, MAX_WATCHDOG); + return STAT_SET_FAILED; + } + if (!send_watchdog_setting(timeout, watchdog_alarm)) + return STAT_SET_FAILED; + /* read the value back, so that the published variables + * reflect what the device actually accepted */ + get_watchdog(); + return STAT_SET_HANDLED; + } + if (!strcasecmp(varname, "ups.watchdog.alarm")) { + unsigned int alarm; + if (!strcasecmp(val, "enabled")) - set_watchdog(1); + alarm = 1; else if (!strcasecmp(val, "disabled")) - set_watchdog(0); + alarm = 2; else { upslogx(LOG_ERR, "setvar(%s): invalid value '%s' " "(expected enabled or disabled)", varname, val); return STAT_SET_FAILED; } - /* read the value back, so that the published variable - * reflects what the device actually accepted */ + if (!send_watchdog_setting(watchdog_seconds, alarm)) + return STAT_SET_FAILED; + /* read the value back, so that the published variables + * reflect what the device actually accepted */ get_watchdog(); return STAT_SET_HANDLED; } @@ -1480,6 +1636,12 @@ void upsdrv_initinfo(void) dstate_setflags("ups.watchdog.status", ST_FLAG_RW | ST_FLAG_STRING); dstate_setaux("ups.watchdog.status", 8); + dstate_setflags("ups.watchdog.timeout", + ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("ups.watchdog.timeout", 3); + dstate_setflags("ups.watchdog.alarm", + ST_FLAG_RW | ST_FLAG_STRING); + dstate_setaux("ups.watchdog.alarm", 8); } if (get_eco_mode()) { dstate_setflags("input.eco.switchable", @@ -1535,6 +1697,7 @@ void upsdrv_initinfo(void) } dstate_addcmd("beeper.mute"); dstate_addcmd("clear.fault.record"); + dstate_addcmd("reset.watchdog"); dstate_addcmd("shutdown.reboot"); dstate_addcmd("shutdown.reboot.graceful"); dstate_addcmd("shutdown.return"); @@ -1731,20 +1894,28 @@ void upsdrv_shutdown(void) if (!init_comm()) printf("Status failed. Assuming it's on battery and trying a shutdown anyway.\n"); - /* in case the power is on, tell it to automatically reboot. if - * it is off, this has no effect. */ + /* Turn the device auto-reboot setting (ARB) on, unconditionally: + * it asks the device to start the load again once mains is + * (re)applied, so that every outlet is powered again after the + * outage that caused the shutdown. In case the power is on, this + * also tells the device to start again; if it is off, it has no + * effect. A device that does not confirm the setting is a device + * the driver cannot tell anything about, so 'shutdown_mode' decides + * whether the shutdown is scheduled anyway (best effort) or not + * (fail safe). */ if (!auto_reboot(1)) { - upslogx(LOG_ERR, "%s: could not set or confirm ups.start.auto, " - "not powering the load off", __func__); - return; - } - if (!set_start_delay(startdelay)) { - upslogx(LOG_ERR, "%s: could not set the return delay, " - "not powering the load off", __func__); - return; + upslogx(LOG_ERR, "%s: could not set or confirm " + "ups.start.auto", __func__); + if (shutdown_abort_on_error) { + upslogx(LOG_ERR, "%s: shutdown_mode=abort, not " + "powering the load off", __func__); + return; + } } /* delay before shutdown, in seconds: honor 'offdelay' if the user set - * it in ups.conf, otherwise keep the historical default */ + * it in ups.conf, otherwise keep the historical default. No return + * delay (SDR) is sent: whether the load comes back is decided by the + * auto-reboot setting above, not by a delay of its own. */ snprintf(parm, sizeof(parm), "%u", offdelay_from_conf ? offdelay : 5U); if (!send_set_command(TSU_SHUTDOWN_ACTION, parm)) @@ -1783,6 +1954,23 @@ void upsdrv_makevartable(void) MIN_STARTDELAY / 60U, MAX_STARTDELAY_MINUTES); addvar(VAR_VALUE, "startdelay", msg); + + /* The driver shutdown handler ("shutdown.default", run by + * upsdrvctl shutdown) always turns the device auto-reboot setting + * (ARB) on before it schedules the shutdown, so that the load is + * powered again when mains is (re)applied. The device stores the + * setting, so it also affects "ups.start.auto"; the shutdown + * commands (shutdown.return and friends) set it for their own + * needs. The 'shutdown_mode' setting decides how the handler + * reacts to a failed operation: keep sending the remaining ones + * (best effort) or stop at the first failure (fail safe). The + * operations are the auto-reboot setting (ARB) and the shutdown + * action (SDA). */ + snprintf(msg, sizeof msg, "How the shutdown handler reacts to a " + "failed operation: continue = keep sending the remaining " + "ones (best effort), abort = stop at the first failure " + "(fail safe, default=continue)."); + addvar(VAR_VALUE, "shutdown_mode", msg); } void upsdrv_initups(void) @@ -1869,9 +2057,31 @@ void upsdrv_initups(void) upsdebugx(2, "Using default startdelay of %u seconds", startdelay); } + + /* Initialize shutdown_mode from configuration: whether a failure of + * an operation stops the shutdown handler there (fail safe) or not + * (best effort, the default). */ + shutdown_abort_on_error = 0; + val = getval("shutdown_mode"); + if (val && *val) { + if (!strcasecmp(val, "continue")) { + shutdown_abort_on_error = 0; + } else if (!strcasecmp(val, "abort")) { + shutdown_abort_on_error = 1; + } else { + fatalx(EXIT_FAILURE, "Invalid shutdown_mode " + "parameter: %s (expected continue or abort)", + val); + } + upsdebugx(2, "Setting shutdown_mode to %s", + shutdown_abort_on_error ? "abort" : "continue"); + } else { + upsdebugx(2, "Using default shutdown_mode of continue"); + } } void upsdrv_cleanup(void) { + disable_watchdog_on_exit(); ser_close(upsfd, device_path); } From 0657051ec4a2f11c2a7a7abf926622e5802f661b Mon Sep 17 00:00:00 2001 From: TomZanna Date: Wed, 21 Oct 2026 08:45:57 +0200 Subject: [PATCH 07/13] refactor(tripplitesu): update driver parameters Rework the tripplitesu manual to match the driver's current behavior: document the accepted ranges for command_delay, offdelay and startdelay, clarify which instant commands use each delay, and drop the removed -1 automatic mode of command_delay along with the historical shutdown delay default that no longer applies. Signed-off-by: TomZanna --- NEWS.adoc | 49 ++- docs/man/tripplitesu.txt | 188 ++++----- drivers/tripplitesu.c | 875 ++++++++++++++++++++------------------- 3 files changed, 567 insertions(+), 545 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index 9b91c59e14..1f55b62a02 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -512,25 +512,28 @@ https://github.com/networkupstools/nut/milestone/13 and a warning is logged instead of silently falling back to a default. * Added the `offdelay` and `startdelay` settings in `ups.conf` (in - seconds and minutes respectively, the units of the two device - commands) and support for an optional delay (in seconds), passed as - the `value` argument of the `shutdown.return`, `shutdown.reboot` and - `shutdown.reboot.graceful` instant commands, so that the timer can be - chosen per invocation without restarting the driver or changing its - configuration. + seconds and whole minutes respectively, the units of the two device + commands). The `shutdown.return` and `shutdown.stayoff` instant + commands accept an optional shutdown delay in seconds, while + `shutdown.reboot` and `shutdown.reboot.graceful` accept + `off[:on]` - a mandatory shutdown delay in seconds and an optional + restart delay in minutes - so that the timers can be chosen per + invocation without restarting the driver or changing its + configuration. Invalid values are rejected before anything is sent. * Added control of the individual outlet banks through the `outlet.n.load.off` and `outlet.n.load.on` instant commands, and the matching `outlet.count`, `outlet.n.status` and `outlet.n.switchable` variables. * Added the `shutdown.stayoff` instant command, and made the shutdown-related commands keep the device return delay consistent with - what they mean: the two reboot commands ask for the load to come back - right behind the shutdown (the device only restarts the load when the - restart delay is longer than the shutdown delay), while - `shutdown.return` and `shutdown.stayoff` clear it, so that the load - comes back only when mains is (re)applied, or not at all. - * Added the read/write variables `ups.beeper.status`, `ups.delay.start`, - `ups.start.auto`, `ups.watchdog.status`, `outlet.delay.start` and + what they mean: the two reboot commands set it to the requested + restart delay, while `shutdown.return` and `shutdown.stayoff` clear it, + so that the load comes back only when mains is (re)applied, or not at + all. The driver no longer adjusts the return delay against the + shutdown delay or rounds it: both are sent in the units the device + uses and the firmware decides how they interact. + * Added the read/write variables `ups.beeper.status`, `ups.start.auto`, + `ups.watchdog.status`, `outlet.delay.start` and `input.eco.switchable`, the `battery.date` variable, the `beeper.mute` and `clear.fault.record` instant commands, and validation of `input.transfer.low`/`.high` against the ranges reported by the device @@ -543,9 +546,9 @@ https://github.com/networkupstools/nut/milestone/13 between two commands sent to the UPS, in milliseconds, to work around units that do not answer a command sent too soon after the previous one. The interval is measured from the previous command, so an idle - link is not penalized, and the special value `-1` makes the driver - start without waiting and pace commands by one second only if one of - them times out. + link is not penalized, and the configured value is used unchanged: + there is no automatic adjustment after a timeout. The former `-1` + automatic mode is no longer accepted; use `0` or an explicit interval. * The watchdog is disarmed when the driver exits on purpose (a handled signal, the `driver.exit` instant command or the end of a data dump), so that a driver restart - for example during a package upgrade - does @@ -558,6 +561,10 @@ https://github.com/networkupstools/nut/milestone/13 restart-alarm flag - instead of only the derived `ups.watchdog.status`, and the `reset.watchdog` instant command, which sends the watchdog setting the driver has again to restart its timer. + The watchdog is disabled by default, has to be armed explicitly, and + returns to disabled after it trips; `reset.watchdog` now fails when the + timeout is 0 instead of reporting a successful kick of a watchdog that + is not running. * The driver shutdown handling (`shutdown.default`, which `upsdrvctl shutdown` runs) always asks the device to start the load again when mains is (re)applied (the auto-reboot setting is turned on), @@ -565,13 +572,17 @@ https://github.com/networkupstools/nut/milestone/13 `shutdown_mode` setting decides whether it stops at the first operation the device rejects or does not answer (`abort`, fail safe) or keeps sending the rest (`continue`, best effort, the default). - It also sets no return delay of its own any more, so that it does not - schedule a restart on a timer, and the `startdelay` setting now only - seeds the `ups.delay.start` variable. + It also clears the return delay (0 minutes), so that the load does not + come back on a timer left over from an earlier command, and schedules + the shutdown after `offdelay`. * The multi-frame commands (`load.off` and `load.on`, which walk the outlet banks) stop at the first frame the device rejects or does not answer, as the shutdown commands already did, instead of sending the remaining frames. + * Hardened protocol handling: requests are length-checked against the + protocol and buffer limits, replies must arrive complete and exactly + as announced, and semicolon-separated fields are copied with a bound + instead of being used in place. - `usbhid-ups` driver updates: * When reconnecting, report success more visibly (not only in debug), and diff --git a/docs/man/tripplitesu.txt b/docs/man/tripplitesu.txt index e613dc488e..10c4379cfa 100644 --- a/docs/man/tripplitesu.txt +++ b/docs/man/tripplitesu.txt @@ -35,8 +35,8 @@ until there are only a few seconds left. Common values are around 25--30. *command_delay*='milliseconds':: Set the minimum interval to keep between two commands sent to the UPS, in -milliseconds. The default is 0, which sends a command as soon as the -previous answer has been read. +milliseconds. The default is 0 and the accepted range is 0 through 60000, +where 0 sends a command as soon as the previous answer has been read. + Some units are slow to digest a command: one sent too soon after the previous one is not answered, and the driver reports a read timeout. Setting this to @@ -49,18 +49,19 @@ Note that the driver sends close to thirty commands while initializing (including the first poll cycle), so a large interval also delays the first published values: half a minute with 1000. + -A value of -1 selects the automatic behaviour: the driver starts without -waiting and, when a command times out, raises the interval to 1000 -milliseconds once for the rest of the run, logging a warning. An explicit -value is never changed, and 0 (or leaving the parameter unset) disables the -feature. -+ For reference, the vendor software paces its own commands by 1.5 to 3 -seconds, so values in that range are known to be safe on this hardware. +seconds, so values in that range are known to be safe on this hardware. The +driver does not adjust this interval on its own: a timeout is reported as a +timeout, and the configured value is used unchanged (0 or unset means no +wait). The former -1 automatic mode is no longer accepted; use 0 or an +explicit interval instead. *offdelay*='num':: -Set the delay, in seconds, used by the shutdown-related instant commands -when no value is passed to them. The default value is 10. +Set the shutdown delay, in seconds, used by 'shutdown.return', +'shutdown.reboot', 'shutdown.stayoff' and the driver shutdown handler when no +value is passed to them. The default value is 10 and the accepted range is 1 +through 3600. 'shutdown.reboot.graceful' uses 60 seconds instead when it is +called without a value. + The value may be overridden for a single invocation with the optional 'value' argument of linkman:upscmd[8], for example: @@ -70,31 +71,21 @@ The value may be overridden for a single invocation with the optional The value passed on the command line takes precedence over this setting, which in turn takes precedence over the built-in default. + -The driver shutdown handling (the 'shutdown.default' instant command and -linkman:upsdrvctl[8] 'shutdown') uses this setting as well, but keeps the -historical default of 5 seconds when it is not set in linkman:ups.conf[5]. *startdelay*='minutes':: -Set the return (restart) delay, in minutes, that the driver publishes as the -read/write variable *ups.delay.start* (see VARIABLES below). The default -value is 1. -+ -The parameter uses the unit of the device command ('SDR', whole minutes); -values outside the range are rejected. The device command is write-only, so -the value is not read back from the UPS: after a driver restart the variable -reflects this setting, and a value written with linkman:upsrw[8] is published -as it was sent (a value that is not a whole number of minutes is rounded up -when it is sent to the device). -+ -The driver shutdown handling does not use this setting: it turns the -auto-reboot setting on and sets the shutdown delay, and leaves the return delay -stored on the device as it is (see the note in the INSTANT COMMANDS section). +Set the restart (return) delay, in whole minutes, used by 'shutdown.reboot' +and 'shutdown.reboot.graceful' when their value does not specify one. The +default value is 1 and the accepted range is 0 through 60. The driver sends +this value to the device unchanged: there is no conversion or rounding. A +value of 0 asks for no timed restart, so the load comes back only when mains +is (re)applied. + *shutdown_mode*='continue'|'abort':: Set how the driver shutdown handling reacts when the device rejects, or does -not answer, one of the operations that make up a shutdown, that is switching -the auto-reboot setting on and scheduling the shutdown itself. The default -value is 'continue'. +not answer, one of the operations that make up a shutdown: switching the +auto-reboot setting on, clearing the return delay and scheduling the shutdown +itself. The default value is 'continue'. + 'continue' (best effort) keeps going, so a device that does not confirm the auto-reboot setting is still asked to switch the load off: the shutdown @@ -122,58 +113,53 @@ are for. 'load.on':: Switch on every controllable outlet immediately. -The following instant commands accept an optional delay, in seconds, passed -as the 'value' argument of linkman:upscmd[8]: +The following instant commands accept an optional value, passed as the +'value' argument of linkman:upscmd[8]: 'shutdown.return' 'seconds':: Turn off the load after the given number of seconds, so that the computer can finish shutting down first. The load is started again - when mains is (re)applied, not after a delay of its own (the return - delay of any earlier command is cleared first); use - 'shutdown.reboot' for a timed restart. Defaults to the 'offdelay' - setting. - -'shutdown.reboot' 'seconds':: - Shut down the load after the given number of seconds and start it - again once it has been switched off (see the note below). Defaults to - the 'offdelay' setting. - -'shutdown.reboot.graceful' 'seconds':: - Same as 'shutdown.reboot', but with a default of 60 seconds, which is - not affected by the 'offdelay' setting. + when mains is (re)applied; use 'shutdown.reboot' for a timed restart. + Defaults to the 'offdelay' setting. + +'shutdown.reboot' 'off[:on]':: + Shut down the load after 'off' seconds and start it again after the + restart delay 'on', in minutes. 'off' is mandatory when a colon is + used; without a colon the value is just 'off'. 'off' defaults to the + 'offdelay' setting and 'on' to the 'startdelay' setting, so + 'shutdown.reboot 45' uses the configured restart delay and + 'shutdown.reboot 45:2' asks for a 45-second shutdown followed by a + 2-minute restart delay. + +'shutdown.reboot.graceful' 'off[:on]':: + Same as 'shutdown.reboot', but the default for 'off' is 60 seconds, + which is not affected by the 'offdelay' setting. 'on' still defaults + to the 'startdelay' setting. 'shutdown.stayoff' 'seconds':: Turn off the load after the given number of seconds and leave it off - even if mains returns (the return delay of any earlier command is - cleared first). Defaults to the 'offdelay' setting. + even if mains returns. Defaults to the 'offdelay' setting. -The accepted range is 1 through 3600 seconds. An invalid value makes the -command fail without sending anything to the UPS. +The shutdown delay ('seconds' and 'off') accepts 1 through 3600 seconds and +the restart delay ('on') accepts 0 through 60 whole minutes. Both fields +must be decimal numbers. An empty field, a ';', a '.', a ',', more than one +colon or an out-of-range value makes the command fail before anything is +sent to the UPS. A bare value for 'shutdown.return' or 'shutdown.stayoff' +is a shutdown delay only; those commands clear the restart delay. The driver shutdown handling is available as the 'shutdown.default' instant command, which is what linkman:upsdrvctl[8] 'shutdown' runs. It takes no -value: before it switches the load off, after the 'offdelay' setting (5 -seconds when it is not set in linkman:ups.conf[5]), it asks the device to -start the load again once mains is (re)applied - see the note below. +value: it turns the auto-reboot setting on, clears the return delay (0 +minutes), then asks the device to switch the load off after the 'offdelay' +setting. The load is therefore started again only when mains is +(re)applied. All other instant commands ignore any value passed to them. NOTE: The device takes the shutdown delay in seconds (its 'SDA' command) and -the restart delay in whole minutes ('SDR'), and it only starts the load -again when the restart delay is longer than the shutdown delay. The two -reboot commands therefore add one second to the shutdown delay before -rounding it up to whole minutes, so that the restart always falls behind -the off, while 'shutdown.return' and 'shutdown.stayoff' clear the restart -delay, so that the load does not come back on a timer of its own. -+ -The driver shutdown handling (the 'shutdown.default' instant command, which -is what linkman:upsdrvctl[8] 'shutdown' runs) turns the auto-reboot setting -on and sets the shutdown delay, and sends no return delay at all: it does not -clear a return delay stored on the device either, so a value left behind by an -earlier command (or written to *ups.delay.start* with linkman:upsrw[8]) still -applies and the device starts the load again on that timer when it is longer -than the shutdown delay. Write 0 to *ups.delay.start* if the load has to -stay off until mains is (re)applied. +the restart delay in whole minutes ('SDR'). The driver sends both values as +requested; it does not adjust one against the other, because the firmware +decides how they interact. A command that is reported as failed met an operation the device rejected (or left unanswered). The driver logs which operation failed and stops there, so @@ -211,11 +197,12 @@ value; enabling/disabling the buzzer for good is done through the sends the watchdog setting the driver has, so the timeout and the restart-alarm behaviour are the ones the device last reported or accepted (use *ups.watchdog.timeout* and *ups.watchdog.alarm* to change - them, and *ups.watchdog.status* to arm or disarm the watchdog). It - fails when the device never reported its watchdog setting, so that a - kick cannot arm a watchdog with assumed values; a watchdog that is - disabled is written again with a timeout of 0, which leaves it as it - is. + them, and *ups.watchdog.status* to arm or disarm the watchdog). + It fails when the device never reported its watchdog setting, so that + a kick cannot arm a watchdog with assumed values. It also fails when + the timeout is 0: a disabled watchdog has no timer to restart, and + reporting a successful kick would hide a monitoring or configuration + error. VARIABLES --------- @@ -251,10 +238,9 @@ when the device stops answering, again with a warning for every failed read. setting, not an action, so it has no matching instant command and it is only read and written with linkman:upsrw[8]. The accepted range is 0 through 999 seconds and, as this is the 'main outlet' form of the - variable, it applies to all outlets. Note that this is the delay used - by the hardware itself, which is different from *ups.delay.start* (the - return delay of the shutdown sequence, in seconds) and from the - 'startdelay' setting (the same return delay, in minutes). + variable, it applies to all outlets. This is the delay used by the + hardware itself, not the shutdown restart delay configured by the + 'startdelay' setting. *battery.date*:: Battery installation date reported by the device, in YYYYMMDD @@ -269,20 +255,6 @@ when the device stops answering, again with a warning for every failed read. setting, so some devices may not report it back; see the note on device settings above. -*ups.delay.start*:: - Read/write return (restart) delay of the device, in seconds: how long - the device leaves the load switched off before it starts it again by - itself. Values are rounded up to whole minutes when they are sent to - the device ('SDR') and, since that command is write-only, the variable - is not read back from the UPS: after a driver restart it reflects the - 'startdelay' setting. -+ -The device only starts the load again when this delay is longer than the -shutdown delay of the command that switched it off, so it has to be chosen -together with the delay passed to the shutdown command (see the note in the -INSTANT COMMANDS section). The driver shutdown handling does not change it, -while 'shutdown.return' and 'shutdown.stayoff' clear it. - *ups.start.auto*:: Read/write auto-reboot setting, either 'yes' or 'no': when it is 'yes' the device starts again by itself once mains is (re)applied, while @@ -304,18 +276,27 @@ linkman:upsrw[8] to restore your preferred value, for example: an external (hardware) watchdog: the device expects the driver to keep talking to it and, when contact is lost for longer than the watchdog timeout, it power cycles the load, switching the outputs off and back - on. A timeout of 0 seconds means 'disabled', and setting the variable - to 'enabled' arms the watchdog with the timeout reported by the device - (255 seconds, the value the vendor software used, when the device - reports no timeout), keeping the restart-alarm behaviour the device - uses. Use *ups.watchdog.timeout* and *ups.watchdog.alarm* to choose - those two fields, and the 'reset.watchdog' instant command to only - restart the timer. + on. A timeout of 0 seconds means 'disabled'. The watchdog is disabled + by default and has to be armed explicitly, either by setting this + variable to 'enabled' or by writing a non-zero + *ups.watchdog.timeout*. Setting the variable to 'enabled' arms the + watchdog with the timeout reported by the device (255 seconds, the + value the vendor software used, when the device reports no timeout), + keeping the restart-alarm behaviour the device uses. Use + *ups.watchdog.timeout* and *ups.watchdog.alarm* to choose those two + fields, and the 'reset.watchdog' instant command to only restart the + timer. + The device stores the timeout and the restart-alarm flag together rather than a status, so the three variables are published from the same setting and a change to one of them writes the whole pair to the device. + +Once armed, the watchdog must be kicked continuously with the +'reset.watchdog' instant command for as long as it is meant to protect the +load, or explicitly disabled with *ups.watchdog.status*. When it trips, the +device power cycles the load and the watchdog returns to 'disabled' until it +is armed again; it does not stay armed by itself. ++ WARNING: While the watchdog is armed, a driver that hangs or a serial link that breaks is enough to switch the load off and on again, and the load is powered only as long as the driver keeps polling the device. If you do not @@ -346,11 +327,12 @@ that case. *ups.watchdog.alarm*:: Read/write restart-alarm flag, either 'enabled' or 'disabled': it tells - the device whether to sound its alarm when the watchdog restarts the - load. The device stores the flag as a number, 1 for 'enabled' and 2 - for 'disabled', next to the timeout; changing it leaves the timeout - alone, so it can be set while the watchdog is disabled. What exactly - triggers that alarm has not been verified on hardware. + the device whether to sound its buzzer when the watchdog trips and + restarts the load. The device stores the flag as a number, 1 for + 'enabled' and 2 for 'disabled', next to the timeout; changing it leaves + the timeout alone, so it can be set while the watchdog is disabled. + The driver does not decode the alarm status reported by the device, so + the flag describes the configured behavior rather than a live alarm. *input.transfer.reason*:: Read-only reason for the last transfer recorded by the device, diff --git a/drivers/tripplitesu.c b/drivers/tripplitesu.c index 6e42a3dda6..7bee1c20de 100644 --- a/drivers/tripplitesu.c +++ b/drivers/tripplitesu.c @@ -55,8 +55,8 @@ The following parameters (ups.conf) are supported: lowbatt command_delay - offdelay - startdelay + offdelay (seconds) + startdelay (minutes) shutdown_mode The following variables are supported (RW = read/write): @@ -85,7 +85,6 @@ output.voltage output.voltage.nominal ups.beeper.status (RW) - ups.delay.start (RW) ups.firmware ups.id (RW) (1) ups.load @@ -115,11 +114,19 @@ test.battery.start test.battery.stop - The shutdown-related commands above accept an optional delay, in seconds, - passed as the "value" argument of upscmd; it takes precedence over the - "offdelay" setting from ups.conf, which in turn takes precedence over - the built-in default. The "load.off", "load.on", "outlet.n.load.*" and - "reset.watchdog" commands do not take a value. + "shutdown.return" and "shutdown.stayoff" accept an optional delay, in + seconds, passed as the "value" argument of upscmd; it takes precedence + over the "offdelay" setting from ups.conf, which in turn takes precedence + over the built-in default. "shutdown.reboot" and + "shutdown.reboot.graceful" accept "off[:on]": the shutdown delay in + seconds and an optional restart delay in minutes, separated by one colon. + Without a colon, only the shutdown delay is changed and the restart delay + defaults to "startdelay"; with no value at all, the shutdown delay + defaults to "offdelay" (60 seconds for the graceful variant) as well. + Fields that are empty, non-numeric, use ';', '.' or ',', or are out of + range make the command fail before anything is sent. The "load.off", + "load.on", "outlet.n.load.*" and "reset.watchdog" commands do not take a + value. Commands that send more than one frame stop at the first operation the device rejects or does not answer, so a failed command is never applied @@ -128,9 +135,10 @@ not reach untouched. "shutdown.return" and "shutdown.stayoff" clear the device return delay - (SDR), so the load comes back only when mains is (re)applied or not at - all, while the two reboot commands ask for the load to come back right - behind the shutdown (just behind it, see reboot_start_delay()). + (SDR 0), so the load comes back only when mains is (re)applied or not at + all, while the two reboot commands ask for the load to come back after + the restart delay. The firmware decides how the shutdown and restart + delays relate; the driver sends both as requested. Because that is what the commands mean on this hardware, the shutdown commands change the device auto-reboot setting ("ups.start.auto", ARB); @@ -142,16 +150,17 @@ third device setting of that kind: its timeout and restart-alarm flag are the read/write "ups.watchdog.timeout" and "ups.watchdog.alarm" variables, "ups.watchdog.status" is their enabled/disabled view, and the - "reset.watchdog" command sends the stored setting again. + "reset.watchdog" command sends the stored setting again. A disabled + watchdog has no timer to kick, so "reset.watchdog" fails then. The driver shutdown handler (the "shutdown.default" instant command, run - by "upsdrvctl shutdown" and by the "driver.killpower" request) always - turns the auto-reboot setting on (ARB) and then schedules the shutdown - (SDA), so that the load is powered again when mains is (re)applied; - unlike the commands above it does not touch the return delay (SDR). The - "shutdown_mode" setting chooses whether a failure of one of those two - operations stops the handler there ("abort", fail safe) or not - ("continue", best effort, the default). + by "upsdrvctl shutdown" and by the "driver.killpower" request) turns the + auto-reboot setting on (ARB), clears the return delay (SDR 0) and then + schedules the shutdown (SDA) after "offdelay", so that the load is + powered again only when mains is (re)applied. The "shutdown_mode" + setting chooses whether a failure of one of those operations stops the + handler there ("abort", fail safe) or not ("continue", best effort, the + default). The following ups.status values are supported: BOOST (1) @@ -170,6 +179,8 @@ */ +#include + #include "main.h" #include "serial.h" #include "nut_stdint.h" @@ -200,18 +211,16 @@ upsdrv_info_t upsdrv_info = { * other shutdown commands it is not affected by the 'offdelay' setting. */ #define DEFAULT_GRACEFUL_DELAY 60U -/* Delay, in seconds, before the load is returned (restarted) after a - * shutdown. The device command (SDR) takes minutes, so values are rounded - * up when sent; the default of one minute matches the value this driver - * used before it became configurable. Unlike the shutdown action (SDA), - * a value of 0 is meaningful here and means "return without delay", so it - * is accepted. The ups.conf parameter 'startdelay' is expressed in - * minutes (the device unit), while the ups.delay.start variable uses - * seconds, as usual for delays in NUT. */ -#define DEFAULT_STARTDELAY 60U -#define MIN_STARTDELAY 0U -#define MAX_STARTDELAY 3600U -#define MAX_STARTDELAY_MINUTES (MAX_STARTDELAY / 60U) +/* Delay, in minutes, before the load is returned (restarted) after a + * shutdown. The device command (SDR) takes whole minutes, so this value is + * used as is, without any rounding or relation to the shutdown delay: the + * firmware decides how the two delays interact. Unlike the shutdown action + * (SDA), a value of 0 is meaningful here and asks the load to return as soon + * as mains is available. The ups.conf parameter 'startdelay' and the value + * used by the reboot commands are both expressed in minutes. */ +#define DEFAULT_STARTDELAY_MINUTES 1U +#define MIN_STARTDELAY_MINUTES 0U +#define MAX_STARTDELAY_MINUTES 60U /* Power-on (boot) delay: how many seconds the device waits before it starts * delivering voltage to its outputs (inverter on, load relays closed) when @@ -239,12 +248,9 @@ upsdrv_info_t upsdrv_info = { * driver does while initializing and polling) can make it miss answers. * The ups.conf parameter 'command_delay' is the minimum interval to keep * between two commands, in milliseconds: 0 disables the pacing (the - * historical behaviour), a positive value is the interval itself, and -1 - * asks the driver to work it out by itself (see note_command_timeout()). - * The vendor software paces its own commands by 1.5 to 3 seconds. */ -#define COMMAND_DELAY_AUTO (-1) -#define DEFAULT_COMMAND_DELAY 0 -#define DEFAULT_AUTO_COMMAND_DELAY 1000U + * historical behaviour) and a positive value is the interval itself. The + * vendor software paces its own commands by 1.5 to 3 seconds. */ +#define DEFAULT_COMMAND_DELAY 0U #define MAX_COMMAND_DELAY 60000U static const char *test_result_names[] = { @@ -271,14 +277,11 @@ static struct { unsigned long commands_available; } ups; -static long command_delay_conf = DEFAULT_COMMAND_DELAY; /* 'command_delay' as set in ups.conf */ -static unsigned int command_delay = 0; /* minimum interval kept between commands, in milliseconds (0 = no waiting) */ -static int command_delay_auto = 0; /* 1 when 'command_delay' asked for automatic pacing (-1) */ +static unsigned int command_delay = DEFAULT_COMMAND_DELAY; /* minimum interval kept between commands, in milliseconds (0 = no waiting) */ static int command_sent = 0; /* 1 once a command has been sent (see pace_before_command()) */ static struct timeval command_sent_at; /* when the last command was sent */ static unsigned int offdelay = DEFAULT_OFFDELAY; /* delay in seconds before shutdown */ -static int offdelay_from_conf = 0; /* set if 'offdelay' was provided in ups.conf */ -static unsigned int startdelay = DEFAULT_STARTDELAY; /* delay in seconds before return */ +static unsigned int startdelay = DEFAULT_STARTDELAY_MINUTES; /* delay in minutes before return */ static unsigned int watchdog_seconds = 0; /* watchdog timeout currently set on the device */ static unsigned int watchdog_alarm = 2; /* watchdog restart-alarm flag currently set on the device */ static int watchdog_known = 0; /* set once the device reported or accepted its watchdog setting */ @@ -366,131 +369,147 @@ static void note_command_sent(void) command_sent = 1; } -/* The device did not answer a command in time. In automatic mode the - * interval is raised, once: a device that is slow to digest a command does - * not get faster again for the rest of the run (command_delay is 0 until - * this function raises it, so this happens only once). An explicitly - * configured value - including 0 - is never touched. Only POLL commands - * count: a SET command that goes unanswered may have been carried out all - * the same, so it says nothing about how fast the device can be driven. */ -static void note_command_timeout(char type) -{ - if (type != POLL || !command_delay_auto || command_delay) - return; - - command_delay = DEFAULT_AUTO_COMMAND_DELAY; - upslogx(LOG_WARNING, "Read timeout: waiting %u ms between commands " - "from now on (command_delay=auto); adjust 'command_delay' in " - "ups.conf to change this", command_delay); -} - - -static ssize_t do_command(char type, const char *command, const char *parameters, char *response) +/* Send a command and, when 'response' is not NULL, read its reply. The + * frame length is bounded by the protocol's three-digit field and by the + * destination buffer, and the reply is only accepted when it is complete: + * a short or oversized answer is a failure, never a truncated value. + * Returns the number of payload bytes read, or -1 on failure. */ +static ssize_t do_command(char type, const char *command, const char *parameters, + char *response, size_t response_size) { char buffer[SMALLBUF]; - size_t count; + size_t command_len, parameters_len, payload_len, count, i; ssize_t ret; + int frame_len; /* let the device get ready for the next command, then drop whatever * it may have sent in the meantime */ pace_before_command(); ser_flush_io(upsfd); - if (response) { + if (response && response_size) *response = '\0'; + + if (!command) + return -1; + if (!parameters) + parameters = ""; + + command_len = strlen(command); + parameters_len = strlen(parameters); + if (command_len > 999U || parameters_len > 999U + || command_len + parameters_len > 999U) { + upsdebugx(3, "do_command: request too long [%s%s]", + command, parameters); + return -1; } + payload_len = command_len + parameters_len; - snprintf(buffer, sizeof(buffer), "~00%c%03d%s%s", type, (int)(strlen(command) + strlen(parameters)), command, parameters); + frame_len = snprintf(buffer, sizeof(buffer), "~00%c%03u%s%s", + type, (unsigned int)payload_len, command, parameters); + if (frame_len < 0 || (size_t)frame_len >= sizeof(buffer) + || (size_t)frame_len != 7U + payload_len) { + upsdebugx(3, "do_command: request does not fit the buffer"); + return -1; + } ret = ser_send_pace(upsfd, 10000, "%s", buffer); - if (ret <= 0) { + if (ret != (ssize_t)frame_len) { upsdebug_with_errno(3, "do_command: send [%s]", buffer); return -1; } note_command_sent(); - upsdebugx(3, "do_command: %" PRIiSIZE " bytes sent [%s] -> OK", ret, buffer); + upsdebugx(3, "do_command: %" PRIiSIZE " bytes sent [%s] -> OK", ret, + buffer); ret = ser_get_buf_len(upsfd, (unsigned char *)buffer, 4, 3, 0); - if (ret < 0) { - upsdebug_with_errno(3, "do_command: read"); + if (ret != 4) { + if (ret < 0) + upsdebug_with_errno(3, "do_command: read"); + else + upsdebugx(3, "do_command: short or missing reply"); return -1; } - if (ret == 0) { - upsdebugx(3, "do_command: read -> TIMEOUT"); - note_command_timeout(type); + buffer[4] = '\0'; + upsdebugx(3, "do_command: %" PRIiSIZE " bytes read [%s]", ret, buffer); + + if (buffer[0] != '~' || buffer[1] != '0' || buffer[2] != '0') { + upsdebugx(3, "do_command: unexpected reply [%s]", buffer); return -1; } - buffer[ret] = '\0'; - upsdebugx(3, "do_command: %" PRIiSIZE " byted read [%s]", ret, buffer); - - if (!strcmp(buffer, "~00D")) { - int c; + /* SET commands are acknowledged with ~00A; a reply to a POLL + * command is a ~00D data frame. A rejection (~00R) and a reply of + * the wrong kind both mean the command failed. */ + if (buffer[3] == ACCEPT) + return response ? -1 : 0; + if (buffer[3] != DATA) { + upsdebugx(3, "do_command: unexpected reply [%s]", buffer); + return -1; + } + if (!response) { + upsdebugx(3, "do_command: response not expected!"); + return -1; + } + if (!response_size) { + upsdebugx(3, "do_command: no room for the response"); + return -1; + } - ret = ser_get_buf_len(upsfd, (unsigned char *)buffer, 3, 3, 0); - if (ret < 0) { + ret = ser_get_buf_len(upsfd, (unsigned char *)buffer, 3, 3, 0); + if (ret != 3) { + if (ret < 0) upsdebug_with_errno(3, "do_command: read"); - return -1; - } - if (ret == 0) { - upsdebugx(3, "do_command: read -> TIMEOUT"); - note_command_timeout(type); - return -1; - } - - buffer[ret] = '\0'; - upsdebugx(3, "do_command: %" PRIiSIZE " bytes read [%s]", ret, buffer); - - c = atoi(buffer); - if (c < 0) { - upsdebugx(3, "do_command: response not expected to be a negative count!"); - return -1; - } + else + upsdebugx(3, "do_command: short or missing reply"); + return -1; + } + buffer[3] = '\0'; + upsdebugx(3, "do_command: %" PRIiSIZE " bytes read [%s]", ret, buffer); - count = (size_t)c; - if (count >= MAX_RESPONSE_LENGTH) { - upsdebugx(3, "do_command: response exceeds expected size!"); + for (i = 0; i < 3; i++) { + if (buffer[i] < '0' || buffer[i] > '9') { + upsdebugx(3, "do_command: invalid response length [%s]", + buffer); return -1; } + } + count = (size_t)(buffer[0] - '0') * 100U + + (size_t)(buffer[1] - '0') * 10U + + (size_t)(buffer[2] - '0'); - if (count && !response) { - upsdebugx(3, "do_command: response not expected!"); - return -1; - } + if (count >= response_size) { + upsdebugx(3, "do_command: response of %" PRIuSIZE " bytes " + "does not fit the %" PRIuSIZE " byte buffer", + count, response_size); + return -1; + } - if (count == 0) { - return 0; - } + if (count == 0) + return 0; - ret = ser_get_buf_len(upsfd, (unsigned char *)response, count, 3, 0); - if (ret < 0) { + ret = ser_get_buf_len(upsfd, (unsigned char *)response, count, 3, 0); + if (ret != (ssize_t)count) { + if (ret < 0) upsdebug_with_errno(3, "do_command: read"); - return -1; - } - if (ret == 0) { - upsdebugx(3, "do_command: read -> TIMEOUT"); - note_command_timeout(type); - return -1; - } - - response[ret] = '\0'; - upsdebugx(3, "do_command: %" PRIiSIZE " bytes read [%s]", ret, response); - - /* Tripp Lite pads their string responses with spaces. - * I don't like that, so I remove them. This is safe to - * do with all responses for this protocol, so I just - * do that here. */ - str_rtrim(response, ' '); - - return ret; + else + upsdebugx(3, "do_command: short reply (%" PRIiSIZE + " of %" PRIuSIZE " bytes)", ret, count); + response[0] = '\0'; + return -1; } + response[count] = '\0'; + upsdebugx(3, "do_command: %" PRIiSIZE " bytes read [%s]", ret, + response); - if (!strcmp(buffer, "~00A")) { - return 0; - } + /* Tripp Lite pads their string responses with spaces. + * I don't like that, so I remove them. This is safe to + * do with all responses for this protocol, so I just + * do that here. */ + str_rtrim(response, ' '); - return -1; + return ret; } /* Send a SET command and tell whether the device accepted it: it answers @@ -498,7 +517,7 @@ static ssize_t do_command(char type, const char *command, const char *parameters * (~00R) for ones it does not support, while a timeout or a bad answer * makes do_command() return a negative value in all those cases. */ static int send_set_command(const char *command, const char *parameters) { - return do_command(SET, command, parameters, NULL) >= 0; + return do_command(SET, command, parameters, NULL, 0) >= 0; } /* Same, but log the failure and return the matching instant command status. */ @@ -511,24 +530,38 @@ static int send_set_or_fail(const char *command, const char *parameters) { return STAT_INSTCMD_FAILED; } -static char *field(char *str, int fieldnum) +/* Copy field number 'fieldnum' (starting at 0) of a semicolon-separated + * reply into 'value'. Returns 0 when the field does not exist or does not + * fit, so that a malformed reply is rejected instead of being truncated. */ +static int get_field(const char *str, unsigned int fieldnum, char *value, + size_t valuesize) { + const char *start = str; + size_t len; + + if (!str || !value || !valuesize) + return 0; - while (str && fieldnum--) { - str = strchr(str, ';'); - if (str) - str++; + while (fieldnum--) { + start = strchr(start, ';'); + if (!start) + return 0; + start++; } - if (str && *str == ';') - return NULL; - return str; + len = strcspn(start, ";"); + if (len == 0 || len >= valuesize) + return 0; + + memcpy(value, start, len); + value[len] = '\0'; + return 1; } static int get_identification(void) { char response[MAX_RESPONSE_LENGTH]; - if (do_command(POLL, IDENTIFICATION, "", response) >= 0) { + if (do_command(POLL, IDENTIFICATION, "", response, sizeof(response)) >= 0) { dstate_setinfo("ups.id", "%s", response); return 1; } @@ -539,11 +572,16 @@ static int get_identification(void) { static void set_identification(const char *val) { char response[MAX_RESPONSE_LENGTH]; - if (do_command(POLL, IDENTIFICATION, "", response) < 0) + if (do_command(POLL, IDENTIFICATION, "", response, sizeof(response)) < 0) return; if (strcmp(val, response)) { - strncpy(response, val, MAX_RESPONSE_LENGTH); - response[MAX_RESPONSE_LENGTH - 1] = '\0'; + if (strlen(val) >= sizeof(response)) { + upslogx(LOG_ERR, "%s: value is too long (maximum %" + PRIuSIZE " characters)", __func__, + sizeof(response) - 1U); + return; + } + snprintf(response, sizeof(response), "%s", val); if (!send_set_command(IDENTIFICATION, response)) upslogx(LOG_ERR, "%s: device rejected or did not " "answer [%s %s]", __func__, IDENTIFICATION, @@ -553,12 +591,11 @@ static void set_identification(const char *val) { static int get_transfer_voltage_low(void) { char response[MAX_RESPONSE_LENGTH]; - char *ptr; + char low[32]; - if (do_command(POLL, TRANSFER_VOLTAGE, "", response) > 0) { - ptr = field(response, 0); - if (ptr) - dstate_setinfo("input.transfer.low", "%d", atoi(ptr)); + if (do_command(POLL, TRANSFER_VOLTAGE, "", response, sizeof(response)) > 0 + && get_field(response, 0, low, sizeof(low))) { + dstate_setinfo("input.transfer.low", "%d", atoi(low)); return 1; } @@ -567,19 +604,16 @@ static int get_transfer_voltage_low(void) { static void set_transfer_voltage_low(int val) { char response[MAX_RESPONSE_LENGTH]; - char *ptr; - int high; + char low[32], high[32]; - if (do_command(POLL, TRANSFER_VOLTAGE, "", response) <= 0) + if (do_command(POLL, TRANSFER_VOLTAGE, "", response, sizeof(response)) <= 0) return; - ptr = field(response, 0); - if (!ptr || val == atoi(ptr)) + if (!get_field(response, 0, low, sizeof(low)) + || !get_field(response, 1, high, sizeof(high))) return; - ptr = field(response, 1); - if (!ptr) + if (val == atoi(low)) return; - high = atoi(ptr); - snprintf(response, sizeof(response), "%d;%d", val, high); + snprintf(response, sizeof(response), "%d;%d", val, atoi(high)); if (!send_set_command(TRANSFER_VOLTAGE, response)) upslogx(LOG_ERR, "%s: device rejected or did not answer " "[%s %s]", __func__, TRANSFER_VOLTAGE, response); @@ -587,12 +621,11 @@ static void set_transfer_voltage_low(int val) { static int get_transfer_voltage_high(void) { char response[MAX_RESPONSE_LENGTH]; - char *ptr; + char high[32]; - if (do_command(POLL, TRANSFER_VOLTAGE, "", response) > 0) { - ptr = field(response, 1); - if (ptr) - dstate_setinfo("input.transfer.high", "%d", atoi(ptr)); + if (do_command(POLL, TRANSFER_VOLTAGE, "", response, sizeof(response)) > 0 + && get_field(response, 1, high, sizeof(high))) { + dstate_setinfo("input.transfer.high", "%d", atoi(high)); return 1; } @@ -601,19 +634,16 @@ static int get_transfer_voltage_high(void) { static void set_transfer_voltage_high(int val) { char response[MAX_RESPONSE_LENGTH]; - char *ptr; - int low; + char low[32], high[32]; - if (do_command(POLL, TRANSFER_VOLTAGE, "", response) <= 0) + if (do_command(POLL, TRANSFER_VOLTAGE, "", response, sizeof(response)) <= 0) return; - ptr = field(response, 0); - if (!ptr) + if (!get_field(response, 0, low, sizeof(low)) + || !get_field(response, 1, high, sizeof(high))) return; - low = atoi(ptr); - ptr = field(response, 1); - if (!ptr || val == atoi(ptr)) + if (val == atoi(high)) return; - snprintf(response, sizeof(response), "%d;%d", low, val); + snprintf(response, sizeof(response), "%d;%d", atoi(low), val); if (!send_set_command(TRANSFER_VOLTAGE, response)) upslogx(LOG_ERR, "%s: device rejected or did not answer " "[%s %s]", __func__, TRANSFER_VOLTAGE, response); @@ -621,12 +651,14 @@ static void set_transfer_voltage_high(int val) { static int get_sensitivity(void) { char response[MAX_RESPONSE_LENGTH]; - unsigned int i; + unsigned int i, code; - if (do_command(POLL, VOLTAGE_SENSITIVITY, "", response) <= 0) + if (do_command(POLL, VOLTAGE_SENSITIVITY, "", response, sizeof(response)) <= 0) + return 0; + if (!str_to_uint_strict(response, &code, 10)) return 0; for (i = 0; i < SIZEOF_ARRAY(sensitivity); i++) { - if (sensitivity[i].code == atoi(response)) { + if ((unsigned int)sensitivity[i].code == code) { dstate_setinfo("input.sensitivity", "%s", sensitivity[i].name); return 1; @@ -667,14 +699,14 @@ static const char *auto_reboot_name(int mode) { static int auto_reboot(int enable) { char parm[20]; char response[MAX_RESPONSE_LENGTH]; - char *ptr; + char value[32]; int mode, oldmode; if (enable) mode = 1; else mode = 2; - if (do_command(POLL, AUTO_REBOOT, "", response) <= 0) { + if (do_command(POLL, AUTO_REBOOT, "", response, sizeof(response)) <= 0) { /* the current setting is unknown (some devices do not report * it), so it cannot be confirmed: report a failure so the * caller aborts instead of powering the load off on a guess */ @@ -682,8 +714,7 @@ static int auto_reboot(int enable) { "confirm ups.start.auto", __func__, AUTO_REBOOT); return 0; } - ptr = field(response, 0); - oldmode = ptr ? atoi(ptr) : 0; + oldmode = get_field(response, 0, value, sizeof(value)) ? atoi(value) : 0; if (oldmode != mode) { snprintf(parm, sizeof(parm), "%d", mode); if (!send_set_command(AUTO_REBOOT, parm)) { @@ -714,15 +745,22 @@ static int auto_reboot(int enable) { * published value is left in place. */ static int get_auto_reboot(void) { char response[MAX_RESPONSE_LENGTH]; + unsigned int mode; - if (do_command(POLL, AUTO_REBOOT, "", response) <= 0) { + if (do_command(POLL, AUTO_REBOOT, "", response, sizeof(response)) <= 0) { upslogx(LOG_WARNING, "%s: could not read the auto-reboot " "setting [%s]; keeping the last known value", __func__, AUTO_REBOOT); return 0; } + if (!str_to_uint_strict(response, &mode, 10)) { + upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " + "keeping the last known value", __func__, AUTO_REBOOT, + response); + return 0; + } - switch (atoi(response)) { + switch (mode) { case 1: dstate_setinfo("ups.start.auto", "%s", "yes"); return 1; @@ -743,24 +781,17 @@ static int get_auto_reboot(void) { * output instead of being published as a standard variable. */ static void get_battery_date(void) { char response[MAX_RESPONSE_LENGTH]; - char *ptr, *sep; + char date[32], replacement[32]; - if (do_command(POLL, BATTERY_REPLACEMENT_DATE, "", response) <= 0) + if (do_command(POLL, BATTERY_REPLACEMENT_DATE, "", response, sizeof(response)) <= 0) return; - ptr = field(response, 0); - if (ptr) { - sep = strchr(ptr, ';'); - if (sep) - *sep = '\0'; - if (*ptr) - dstate_setinfo("battery.date", "%s", ptr); - } + if (get_field(response, 0, date, sizeof(date))) + dstate_setinfo("battery.date", "%s", date); - ptr = field(response, 1); - if (ptr) + if (get_field(response, 1, replacement, sizeof(replacement))) upsdebugx(2, "%s: unmapped second field [%s]", - __func__, ptr); + __func__, replacement); } /* Read the power-on (boot) delay from the device. This is auxiliary @@ -771,7 +802,7 @@ static int get_boot_delay(void) { char response[MAX_RESPONSE_LENGTH]; unsigned int delay; - if (do_command(POLL, BOOT_DELAY, "", response) <= 0) { + if (do_command(POLL, BOOT_DELAY, "", response, sizeof(response)) <= 0) { upslogx(LOG_WARNING, "%s: could not read the power-on delay " "[%s]; keeping the last known value", __func__, BOOT_DELAY); @@ -824,39 +855,31 @@ static void publish_watchdog(void) { * one along. */ static int get_watchdog(void) { char response[MAX_RESPONSE_LENGTH]; - char *ptr; - int timeout, alarm; + char timeout_value[32], alarm_value[32]; + unsigned int timeout, alarm; - if (do_command(POLL, WATCHDOG, "", response) <= 0) { + if (do_command(POLL, WATCHDOG, "", response, sizeof(response)) <= 0) { upslogx(LOG_WARNING, "%s: could not read the watchdog " "setting [%s]; keeping the last known value", __func__, WATCHDOG); return 0; } - ptr = field(response, 0); - if (!ptr) { + if (!get_field(response, 0, timeout_value, sizeof(timeout_value)) + || !str_to_uint_strict(timeout_value, &timeout, 10) + || timeout > MAX_WATCHDOG + || !get_field(response, 1, alarm_value, sizeof(alarm_value)) + || !str_to_uint_strict(alarm_value, &alarm, 10) + || (alarm != 1 && alarm != 2)) { upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " "keeping the last known value", __func__, WATCHDOG, response); return 0; } - timeout = atoi(ptr); - if (timeout < 0 || timeout > (int)MAX_WATCHDOG) { - upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " - "keeping the last known value", __func__, WATCHDOG, - response); - return 0; - } - watchdog_seconds = (unsigned int)timeout; - watchdog_known = 1; - ptr = field(response, 1); - if (ptr) { - alarm = atoi(ptr); - if (alarm == 1 || alarm == 2) - watchdog_alarm = (unsigned int)alarm; - } + watchdog_seconds = timeout; + watchdog_alarm = alarm; + watchdog_known = 1; publish_watchdog(); return 1; @@ -913,9 +936,9 @@ static int set_watchdog(int enable) { * reported (or accepted) again, which restarts its timer. The command * takes no value, so the watchdog it kicks is the one the driver is * configured with. A watchdog that was never read is left alone, so that a - * kick cannot arm it with assumed values; a watchdog that is disabled is - * simply refreshed (0 is sent again) so that its stored timeout is left as - * the device reported it. */ + * kick cannot arm it with assumed values, and a disabled watchdog is not + * kicked either: there is no timer to reset, and pretending otherwise would + * report a watchdog that does not protect the load. */ static int reset_watchdog(void) { if (!watchdog_known) { upslogx(LOG_ERR, "%s: the device never reported its [%s] " @@ -924,9 +947,11 @@ static int reset_watchdog(void) { return STAT_INSTCMD_FAILED; } - if (!watchdog_seconds) - upslogx(LOG_INFO, "%s: the watchdog is disabled (timeout " - "0), the kick only refreshes that", __func__); + if (!watchdog_seconds) { + upslogx(LOG_ERR, "%s: the watchdog is disabled (timeout " + "0), not resetting it", __func__); + return STAT_INSTCMD_FAILED; + } if (!send_watchdog_setting(watchdog_seconds, watchdog_alarm)) return STAT_INSTCMD_FAILED; @@ -963,24 +988,23 @@ static void disable_watchdog_on_exit(void) * documented, so it is published as it comes. */ static int get_bypass_reason(void) { char response[MAX_RESPONSE_LENGTH]; - char *ptr; + char reason[32]; - if (do_command(POLL, BYPASS_REASON, "", response) <= 0) { + if (do_command(POLL, BYPASS_REASON, "", response, sizeof(response)) <= 0) { upslogx(LOG_WARNING, "%s: could not read the bypass reason " "[%s]; keeping the last known value", __func__, BYPASS_REASON); return 0; } - ptr = field(response, 0); - if (!ptr || !*ptr) { + if (!get_field(response, 0, reason, sizeof(reason))) { upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " "keeping the last known value", __func__, BYPASS_REASON, response); return 0; } - dstate_setinfo("input.transfer.reason", "%s", ptr); + dstate_setinfo("input.transfer.reason", "%s", reason); return 1; } @@ -991,15 +1015,22 @@ static int get_bypass_reason(void) { * is left alone. */ static int get_eco_mode(void) { char response[MAX_RESPONSE_LENGTH]; + unsigned int mode; - if (do_command(POLL, ECONOMIC_MODE, "", response) <= 0) { + if (do_command(POLL, ECONOMIC_MODE, "", response, sizeof(response)) <= 0) { upslogx(LOG_WARNING, "%s: could not read the ECO setting " "[%s]; keeping the last known value", __func__, ECONOMIC_MODE); return 0; } + if (!str_to_uint_strict(response, &mode, 10)) { + upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " + "keeping the last known value", __func__, ECONOMIC_MODE, + response); + return 0; + } - switch (atoi(response)) { + switch (mode) { case 1: dstate_setinfo("input.eco.switchable", "%s", "ECO"); return 1; @@ -1036,15 +1067,22 @@ static void set_eco_mode(int mode) { * does not report. */ static int get_beeper_status(void) { char response[MAX_RESPONSE_LENGTH]; + unsigned int status; - if (do_command(POLL, ENABLE_BUZZER, "", response) <= 0) { + if (do_command(POLL, ENABLE_BUZZER, "", response, sizeof(response)) <= 0) { upslogx(LOG_WARNING, "%s: could not read the buzzer setting " "[%s]; keeping the last known value", __func__, ENABLE_BUZZER); return 0; } + if (!str_to_uint_strict(response, &status, 10)) { + upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " + "keeping the last known value", __func__, ENABLE_BUZZER, + response); + return 0; + } - switch (atoi(response)) { + switch (status) { case 1: dstate_setinfo("ups.beeper.status", "%s", "enabled"); return 1; @@ -1085,7 +1123,7 @@ static void get_outlet_status(void) { for (i = 1; i <= (unsigned int)ups.outlets; i++) { snprintf(command, sizeof(command), "%s%u", RELAY_STATUS, i); - if (do_command(POLL, command, "", response) <= 0) { + if (do_command(POLL, command, "", response, sizeof(response)) <= 0) { upsdebugx(2, "%s: no response for [%s]", __func__, command); continue; @@ -1101,28 +1139,16 @@ static void get_outlet_status(void) { } } -/* Send the restart (return) delay to the UPS. The SDR value is expressed - * in minutes, so it is rounded up to never return the load earlier than - * requested. Returns 1 when the device accepted the setting. */ -static int set_start_delay(unsigned int delay) { +/* Send the restart (return) delay to the UPS. SDR takes whole minutes and + * this value is already expressed in that unit, so it is sent unchanged. + * Returns 1 when the device accepted the setting. */ +static int set_start_delay(unsigned int minutes) { char parm[20]; - snprintf(parm, sizeof(parm), "%u", (delay + 59U) / 60U); + snprintf(parm, sizeof(parm), "%u", minutes); return send_set_command(TSU_SHUTDOWN_RESTART, parm); } -/* Restart delay (in seconds) to ask for when the load has to come back - * after a shutdown: one second more than the shutdown delay, so that - * rounding up to whole minutes (set_start_delay(): the device counts its - * restart delay in minutes) always lands behind the off. The device does - * not restart the load when the restart delay is 0 - an online device - * switches its outputs off and leaves them off, an offline one is only - * started up (tested on an SUINT1500RTXL2Ua) - hence it stays 0 then. */ -static unsigned int reboot_start_delay(unsigned int delay) -{ - return delay ? delay + 1U : 0U; -} - /* Parse an unsigned integer bounded to the given inclusive range. Returns * 1 on success and stores the result in *result, 0 otherwise. */ static int parse_uint_range(const char *val, unsigned int minval, @@ -1161,14 +1187,6 @@ static int parse_transfer_voltage(const char *val, unsigned int minval, return 1; } -/* Parse the delay (in seconds) before the load is returned. A value of 0 - * is meaningful here and means "return without delay", so it is accepted, - * unlike the shutdown delay. */ -static int parse_return_delay(const char *val, unsigned int *delay) -{ - return parse_uint_range(val, MIN_STARTDELAY, MAX_STARTDELAY, delay); -} - /* Resolve the delay (in seconds) to use for a shutdown-related command: * the value passed by the caller takes precedence over the given fallback * (the 'offdelay' setting from ups.conf, or the command's own default). @@ -1191,6 +1209,58 @@ static int get_shutdown_delay(const char *cmdname, const char *extra, return 1; } +/* Parse the value of the reboot commands, "off[:on]": an optional + * shutdown delay in seconds and, after a colon, an optional restart delay + * in minutes. An empty value means both defaults (the command's default + * shutdown delay and startdelay). A bare number only sets the shutdown + * delay. Anything that + * is not made of decimal digits and at most one colon is rejected, so ';' + * cannot start a second command field and '.' or ',' cannot look like + * fractions of a second. Returns 1 on success. */ +static int parse_reboot_delay(const char *cmdname, const char *extra, + unsigned int default_off, unsigned int *off, unsigned int *on) +{ + const char *sep; + char off_value[20]; + size_t off_len; + + *off = default_off; + *on = startdelay; + + if (!extra || !*extra) + return 1; + + sep = strchr(extra, ':'); + if (!sep) { + if (!parse_uint_range(extra, MIN_OFFDELAY, MAX_OFFDELAY, off)) + goto invalid; + return 1; + } + + /* the shutdown delay is mandatory when a restart delay is given, + * and only one colon is accepted */ + off_len = (size_t)(sep - extra); + if (off_len == 0 || off_len >= sizeof(off_value) + || sep[1] == '\0' || strchr(sep + 1, ':') != NULL) + goto invalid; + + memcpy(off_value, extra, off_len); + off_value[off_len] = '\0'; + if (!parse_uint_range(off_value, MIN_OFFDELAY, MAX_OFFDELAY, off) + || !parse_uint_range(sep + 1, MIN_STARTDELAY_MINUTES, + MAX_STARTDELAY_MINUTES, on)) + goto invalid; + + return 1; + +invalid: + upslogx(LOG_ERR, "instcmd(%s): invalid value '%s' (expected " + "off or off:on, with off in %u..%u seconds and on in " + "%u..%u minutes)", cmdname, extra, MIN_OFFDELAY, MAX_OFFDELAY, + MIN_STARTDELAY_MINUTES, MAX_STARTDELAY_MINUTES); + return 0; +} + static int instcmd(const char *cmdname, const char *extra) { static const struct { @@ -1218,7 +1288,7 @@ static int instcmd(const char *cmdname, const char *extra) char parm[20]; char command[32]; /* "outlet..load.on" and friends */ unsigned int outlet, cmd_index; - unsigned int delay; + unsigned int delay, return_delay; unsigned int fallback; upsdebug_INSTCMD_STARTING(cmdname, extra); @@ -1288,8 +1358,18 @@ static int instcmd(const char *cmdname, const char *extra) fallback = shutdown_commands[cmd_index].use_offdelay ? offdelay : DEFAULT_GRACEFUL_DELAY; - if (!get_shutdown_delay(cmdname, extra, fallback, &delay)) - return STAT_INSTCMD_FAILED; + if (shutdown_commands[cmd_index].timed_return) { + /* "off[:on]", both in the units the device uses */ + if (!parse_reboot_delay(cmdname, extra, fallback, + &delay, &return_delay)) + return STAT_INSTCMD_FAILED; + } else { + if (!get_shutdown_delay(cmdname, extra, fallback, &delay)) + return STAT_INSTCMD_FAILED; + /* no timed return: only mains coming back may start + * the load again */ + return_delay = 0; + } upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); /* set the auto-reboot and return-delay prerequisites before @@ -1301,12 +1381,7 @@ static int instcmd(const char *cmdname, const char *extra) "ups.start.auto, not powering the load off", cmdname); return STAT_INSTCMD_FAILED; } - /* the reboot commands ask the load to come back right behind - * the shutdown; the others clear any return delay left over - * from an earlier command, so that the load comes back only - * when mains is (re)applied, or not at all */ - if (!set_start_delay(shutdown_commands[cmd_index].timed_return - ? reboot_start_delay(delay) : 0U)) { + if (!set_start_delay(return_delay)) { upslogx(LOG_ERR, "%s: could not set the return " "delay, not powering the load off", cmdname); return STAT_INSTCMD_FAILED; @@ -1345,24 +1420,6 @@ static int setvar(const char *varname, const char *val) get_identification(); return STAT_SET_HANDLED; } - if (!strcasecmp(varname, "ups.delay.start")) { - unsigned int delay; - - if (!parse_return_delay(val, &delay)) { - upslogx(LOG_ERR, "setvar(%s): invalid return delay " - "value '%s' (expected %u..%u seconds)", - varname, val, MIN_STARTDELAY, MAX_STARTDELAY); - return STAT_SET_FAILED; - } - if (!set_start_delay(delay)) { - upslogx(LOG_ERR, "setvar(%s): device rejected or did " - "not answer the return delay", varname); - return STAT_SET_FAILED; - } - startdelay = delay; - dstate_setinfo("ups.delay.start", "%u", startdelay); - return STAT_SET_HANDLED; - } if (!strcasecmp(varname, "outlet.delay.start")) { unsigned int delay; @@ -1520,18 +1577,25 @@ static int init_comm(void) ups.commands_available = 0; /* Repeat enumerate command 2x, firmware bug on some units garbles 1st response */ - if (do_command(POLL, AVAILABLE, "", response) <= 0){ + if (do_command(POLL, AVAILABLE, "", response, sizeof(response)) <= 0){ upslogx(LOG_NOTICE, "init_comm: Initial response malformed, retrying in 300ms"); usleep(3E5); } - if (do_command(POLL, AVAILABLE, "", response) <= 0) + if (do_command(POLL, AVAILABLE, "", response, sizeof(response)) <= 0) return 0; i = strlen(response); + if (i > sizeof(ups.commands_available) * CHAR_BIT) { + upslogx(LOG_WARNING, "%s: [%s] response has %" PRIuSIZE + " bits, only the last %" PRIuSIZE " are used", + __func__, AVAILABLE, i, + sizeof(ups.commands_available) * CHAR_BIT); + i = sizeof(ups.commands_available) * CHAR_BIT; + } for (bit = 0; bit < i; bit++) if (response[i - bit - 1] == '1') ups.commands_available |= (1UL << bit); - if (do_command(POLL, MANUFACTURER, "", response) <= 0) + if (do_command(POLL, MANUFACTURER, "", response, sizeof(response)) <= 0) return 0; if (strcmp(response, "Tripp Lite")) return 0; @@ -1543,8 +1607,8 @@ void upsdrv_initinfo(void) { char response[MAX_RESPONSE_LENGTH]; char buf[32]; /* "outlet..load.on" and friends */ + char value[32]; unsigned int i; - char *ptr; if (!init_comm()) fatalx(EXIT_FAILURE, "Unable to detect Tripp Lite SmartOnline UPS on port %s\n", @@ -1553,52 +1617,44 @@ void upsdrv_initinfo(void) min_high_transfer = max_high_transfer = 0; /* get all the read-only fields here */ - if (do_command(POLL, MANUFACTURER, "", response) > 0) + if (do_command(POLL, MANUFACTURER, "", response, sizeof(response)) > 0) dstate_setinfo("ups.mfr", "%s", response); - if (do_command(POLL, MODEL, "", response) > 0) + if (do_command(POLL, MODEL, "", response, sizeof(response)) > 0) dstate_setinfo("ups.model", "%s", response); - if (do_command(POLL, VERSION_CMD, "", response) > 0) + if (do_command(POLL, VERSION_CMD, "", response, sizeof(response)) > 0) dstate_setinfo("ups.firmware", "%s", response); - if (do_command(POLL, RATINGS, "", response) > 0) { - ptr = field(response, 0); - if (ptr) + if (do_command(POLL, RATINGS, "", response, sizeof(response)) > 0) { + if (get_field(response, 0, value, sizeof(value))) dstate_setinfo("input.voltage.nominal", "%d", - atoi(ptr)); - ptr = field(response, 2); - if (ptr) { + atoi(value)); + if (get_field(response, 2, value, sizeof(value))) dstate_setinfo("output.voltage.nominal", "%d", - atoi(ptr)); - } - ptr = field(response, 14); - if (ptr) + atoi(value)); + if (get_field(response, 14, value, sizeof(value))) dstate_setinfo("battery.voltage.nominal", "%d", - atoi(ptr)); - ptr = field(response, 10); - if (ptr) { - int ipv = atoi(ptr); + atoi(value)); + if (get_field(response, 10, value, sizeof(value))) { + int ipv = atoi(value); if (ipv >= 0) min_low_transfer = (unsigned int)ipv; } - ptr = field(response, 9); - if (ptr) { - int ipv = atoi(ptr); + if (get_field(response, 9, value, sizeof(value))) { + int ipv = atoi(value); if (ipv >= 0) max_low_transfer = (unsigned int)ipv; } - ptr = field(response, 12); - if (ptr) { - int ipv = atoi(ptr); + if (get_field(response, 12, value, sizeof(value))) { + int ipv = atoi(value); if (ipv >= 0) min_high_transfer = (unsigned int)ipv; } - ptr = field(response, 11); - if (ptr) { - int ipv = atoi(ptr); + if (get_field(response, 11, value, sizeof(value))) { + int ipv = atoi(value); if (ipv >= 0) max_high_transfer = (unsigned int)ipv; } } - if (do_command(POLL, OUTLET_RELAYS, "", response) > 0) { + if (do_command(POLL, OUTLET_RELAYS, "", response, sizeof(response)) > 0) { int outlet_count = atoi(response); if (outlet_count > 0 && outlet_count <= (int)MAX_OUTLETS) @@ -1615,10 +1671,6 @@ void upsdrv_initinfo(void) } } get_battery_date(); - /* define things that are settable */ - dstate_setinfo("ups.delay.start", "%u", startdelay); - dstate_setflags("ups.delay.start", ST_FLAG_RW | ST_FLAG_STRING); - dstate_setaux("ups.delay.start", 4); /* the device settings below (get/set) are published, and made * settable, only when the device actually reports them: an unreadable * setting must not appear with an assumed value, and each get_*() @@ -1721,24 +1773,24 @@ void upsdrv_initinfo(void) void upsdrv_updateinfo(void) { char response[MAX_RESPONSE_LENGTH]; - char *ptr, *ptr2; + char value[32]; + char *ptr2; int i; int flags; int contacts_set; int low_battery; status_init(); - if (do_command(POLL, STATUS_OUTPUT, "", response) <= 0) { + if (do_command(POLL, STATUS_OUTPUT, "", response, sizeof(response)) <= 0) { dstate_datastale(); return; } - ptr = field(response, 0); /* require output status field to exist */ - if (!ptr) { + if (!get_field(response, 0, value, sizeof(value))) { dstate_datastale(); return; } - switch (atoi(ptr)) { + switch (atoi(value)) { case 0: status_set("OL"); break; @@ -1767,79 +1819,69 @@ void upsdrv_updateinfo(void) default: break; } - ptr = field(response, 6); - if (ptr) - dstate_setinfo("ups.load", "%d", atoi(ptr)); - ptr = field(response, 3); - if (ptr) + if (get_field(response, 6, value, sizeof(value))) + dstate_setinfo("ups.load", "%d", atoi(value)); + if (get_field(response, 3, value, sizeof(value))) dstate_setinfo("output.voltage", "%03.1f", - (double) (atoi(ptr)) / 10.0); - ptr = field(response, 1); - if (ptr) + (double) (atoi(value)) / 10.0); + if (get_field(response, 1, value, sizeof(value))) dstate_setinfo("output.frequency", "%03.1f", - (double) (atoi(ptr)) / 10.0); - ptr = field(response, 4); - if (ptr) + (double) (atoi(value)) / 10.0); + if (get_field(response, 4, value, sizeof(value))) dstate_setinfo("output.current", "%03.1f", - (double) (atoi(ptr)) / 10.0); + (double) (atoi(value)) / 10.0); low_battery = 0; - if (do_command(POLL, STATUS_BATTERY, "", response) <= 0) { + if (do_command(POLL, STATUS_BATTERY, "", response, sizeof(response)) <= 0) { dstate_datastale(); return; } - ptr = field(response, 0); - if (ptr && atoi(ptr) == 2) + if (get_field(response, 0, value, sizeof(value)) + && atoi(value) == 2) status_set("RB"); - ptr = field(response, 1); - if (ptr && atoi(ptr)) + if (get_field(response, 1, value, sizeof(value)) + && atoi(value)) low_battery = 1; - ptr = field(response, 8); - if (ptr) - dstate_setinfo("battery.temperature", "%d", atoi(ptr)); - ptr = field(response, 9); - if (ptr) { - dstate_setinfo("battery.charge", "%d", atoi(ptr)); + if (get_field(response, 8, value, sizeof(value))) + dstate_setinfo("battery.temperature", "%d", atoi(value)); + if (get_field(response, 9, value, sizeof(value))) { + dstate_setinfo("battery.charge", "%d", atoi(value)); ptr2 = getval("lowbatt"); if (ptr2 && atoi(ptr2) > 0 && atoi(ptr2) <= 99 - && atoi(ptr) <= atoi(ptr2) + && atoi(value) <= atoi(ptr2) ) { low_battery = 1; } } - ptr = field(response, 6); - if (ptr) + if (get_field(response, 6, value, sizeof(value))) dstate_setinfo("battery.voltage", "%03.1f", - (double) (atoi(ptr)) / 10.0); - ptr = field(response, 7); - if (ptr) + (double) (atoi(value)) / 10.0); + if (get_field(response, 7, value, sizeof(value))) dstate_setinfo("battery.current", "%03.1f", - (double) (atoi(ptr)) / 10.0); + (double) (atoi(value)) / 10.0); if (low_battery) status_set("LB"); - if (do_command(POLL, STATUS_ALARM, "", response) <= 0) { + if (do_command(POLL, STATUS_ALARM, "", response, sizeof(response)) <= 0) { dstate_datastale(); return; } - ptr = field(response, 3); - if (ptr && atoi(ptr)) + if (get_field(response, 3, value, sizeof(value)) + && atoi(value)) status_set("OVER"); - if (do_command(POLL, STATUS_INPUT, "", response) > 0) { - ptr = field(response, 2); - if (ptr) + if (do_command(POLL, STATUS_INPUT, "", response, sizeof(response)) > 0) { + if (get_field(response, 2, value, sizeof(value))) dstate_setinfo("input.voltage", "%03.1f", - (double) (atoi(ptr)) / 10.0); - ptr = field(response, 1); - if (ptr) + (double) (atoi(value)) / 10.0); + if (get_field(response, 1, value, sizeof(value))) dstate_setinfo("input.frequency", "%03.1f", - (double) (atoi(ptr)) / 10.0); + (double) (atoi(value)) / 10.0); } - if (do_command(POLL, TEST_RESULT, "", response) > 0) { + if (do_command(POLL, TEST_RESULT, "", response, sizeof(response)) > 0) { int r; size_t trsize; @@ -1852,20 +1894,17 @@ void upsdrv_updateinfo(void) dstate_setinfo("ups.test.result", "%s", test_result_names[r]); } - if (do_command(POLL, ENVIRONMENT_INFORMATION, "", response) > 0) { - ptr = field(response, 0); - if (ptr) - dstate_setinfo("ambient.temperature", "%d", atoi(ptr)); - ptr = field(response, 1); - if (ptr) - dstate_setinfo("ambient.humidity", "%d", atoi(ptr)); + if (do_command(POLL, ENVIRONMENT_INFORMATION, "", response, sizeof(response)) > 0) { + if (get_field(response, 0, value, sizeof(value))) + dstate_setinfo("ambient.temperature", "%d", atoi(value)); + if (get_field(response, 1, value, sizeof(value))) + dstate_setinfo("ambient.humidity", "%d", atoi(value)); flags = 0; contacts_set = 0; for (i = 0; i < 4; i++) { - ptr = field(response, 2 + i); - if (ptr) { + if (get_field(response, 2 + i, value, sizeof(value))) { contacts_set = 1; - if (*ptr == '1') + if (value[0] == '1') flags |= 1 << i; } } @@ -1912,12 +1951,20 @@ void upsdrv_shutdown(void) return; } } - /* delay before shutdown, in seconds: honor 'offdelay' if the user set - * it in ups.conf, otherwise keep the historical default. No return - * delay (SDR) is sent: whether the load comes back is decided by the - * auto-reboot setting above, not by a delay of its own. */ - snprintf(parm, sizeof(parm), "%u", - offdelay_from_conf ? offdelay : 5U); + /* clear the return delay (SDR 0), so that the load comes back only + * when mains is (re)applied, as asked by the auto-reboot setting + * above, and not on a timer left over from an earlier command */ + if (!set_start_delay(0)) { + upslogx(LOG_ERR, "%s: could not clear the return delay", + __func__); + if (shutdown_abort_on_error) { + upslogx(LOG_ERR, "%s: shutdown_mode=abort, not " + "powering the load off", __func__); + return; + } + } + /* delay before shutdown, in seconds, from 'offdelay' */ + snprintf(parm, sizeof(parm), "%u", offdelay); if (!send_set_command(TSU_SHUTDOWN_ACTION, parm)) upslogx(LOG_ERR, "%s: could not schedule the shutdown", __func__); } @@ -1939,19 +1986,17 @@ void upsdrv_makevartable(void) addvar(VAR_VALUE, "lowbatt", "Set low battery level, in percent"); addvar(VAR_VALUE, "command_delay", "Minimum interval between commands, in milliseconds " - "(default: 0 = no wait; -1 = automatic)"); + "(default: 0 = no wait)"); snprintf(msg, sizeof msg, "Set shutdown delay, in seconds (default=%u, range %u..%u).", DEFAULT_OFFDELAY, MIN_OFFDELAY, MAX_OFFDELAY); addvar(VAR_VALUE, "offdelay", msg); /* The return delay is configured in minutes because that is the unit - * used by the device (SDR), while it is exposed in seconds through - * the ups.delay.start variable, as usual for delays in NUT. */ + * used by the device (SDR). */ snprintf(msg, sizeof msg, "Set return (restart) delay, in minutes " "(default=%u, range %u..%u).", - DEFAULT_STARTDELAY / 60U, - MIN_STARTDELAY / 60U, + DEFAULT_STARTDELAY_MINUTES, MIN_STARTDELAY_MINUTES, MAX_STARTDELAY_MINUTES); addvar(VAR_VALUE, "startdelay", msg); @@ -1982,43 +2027,30 @@ void upsdrv_initups(void) /* Initialize command_delay from configuration. The value is in * milliseconds: 0 disables the pacing (commands follow each other as - * soon as the previous answer is read), -1 asks the driver to pace - * them by itself (start without waiting, raise the interval once if a - * command times out) and a positive value is the minimum interval to - * keep between two commands. */ - command_delay_conf = DEFAULT_COMMAND_DELAY; + * soon as the previous answer is read), a positive value is the + * minimum interval to keep between two commands. */ + command_delay = DEFAULT_COMMAND_DELAY; val = getval("command_delay"); if (val && *val) { - long temp; + unsigned int temp; - if (!str_to_long_strict(val, &temp, 10) || - temp < (long)COMMAND_DELAY_AUTO || - temp > (long)MAX_COMMAND_DELAY) { + if (!str_to_uint_strict(val, &temp, 10) + || temp > MAX_COMMAND_DELAY) { fatalx(EXIT_FAILURE, "Invalid command_delay parameter: %s " - "(expected -1 (auto), 0 (no wait) or 1..%u " - "milliseconds)", val, MAX_COMMAND_DELAY); + "(expected 0 (no wait) or 1..%u milliseconds)", + val, MAX_COMMAND_DELAY); } - command_delay_conf = temp; + command_delay = temp; } else { - upsdebugx(2, "Using default command_delay of %ld milliseconds", - command_delay_conf); + upsdebugx(2, "Using default command_delay of %u milliseconds", + command_delay); } - if (command_delay_conf == (long)COMMAND_DELAY_AUTO) { - command_delay_auto = 1; - command_delay = 0; - upsdebugx(2, "command_delay is automatic: commands are paced by " - "%u milliseconds once one of them times out", - DEFAULT_AUTO_COMMAND_DELAY); - } else { - command_delay_auto = 0; - command_delay = (unsigned int)command_delay_conf; - if (command_delay) - upsdebugx(2, "Setting command_delay to %u milliseconds", - command_delay); - else - upsdebugx(2, "command_delay is 0: no wait between commands"); - } + if (command_delay) + upsdebugx(2, "Setting command_delay to %u milliseconds", + command_delay); + else + upsdebugx(2, "command_delay is 0: no wait between commands"); /* Initialize offdelay from configuration */ val = getval("offdelay"); @@ -2031,30 +2063,27 @@ void upsdrv_initups(void) val, MIN_OFFDELAY, MAX_OFFDELAY); } offdelay = temp; - offdelay_from_conf = 1; upsdebugx(2, "Setting offdelay to %u seconds", offdelay); } else { upsdebugx(2, "Using default offdelay of %u seconds", offdelay); } - /* Initialize startdelay from configuration. The parameter is in - * minutes (the unit used by the device), the internal value is in - * seconds (as used by ups.delay.start). */ + /* Initialize startdelay from configuration, in whole minutes (the + * unit used by the device). */ val = getval("startdelay"); if (val && *val) { unsigned int temp; - if (!str_to_uint_strict(val, &temp, 10) || - temp > MAX_STARTDELAY_MINUTES) { + if (!str_to_uint_strict(val, &temp, 10) + || temp > MAX_STARTDELAY_MINUTES) { fatalx(EXIT_FAILURE, "Invalid startdelay parameter: %s " "(expected %u..%u minutes)", - val, MIN_STARTDELAY / 60U, + val, MIN_STARTDELAY_MINUTES, MAX_STARTDELAY_MINUTES); } - startdelay = temp * 60U; - upsdebugx(2, "Setting startdelay to %u seconds (%u minutes)", - startdelay, temp); + startdelay = temp; + upsdebugx(2, "Setting startdelay to %u minutes", startdelay); } else { - upsdebugx(2, "Using default startdelay of %u seconds", + upsdebugx(2, "Using default startdelay of %u minutes", startdelay); } From 4f60acf8f91cb2f722a66b5c2e555d46949856a2 Mon Sep 17 00:00:00 2001 From: TomZanna Date: Wed, 21 Oct 2026 22:52:22 +0200 Subject: [PATCH 08/13] refactor(tripplitesu): share two-state setting reader Extend the auto-reboot reader into a generic helper driven by a table of protocol command, variable name, log description and on/off names. Use it for auto-reboot, ECO mode and buzzer settings, removing the duplicated polling, parsing and warning code from each getter. Signed-off-by: TomZanna --- drivers/tripplitesu.c | 311 +++++++++++++++++++++--------------------- 1 file changed, 159 insertions(+), 152 deletions(-) diff --git a/drivers/tripplitesu.c b/drivers/tripplitesu.c index 7bee1c20de..36248cc16d 100644 --- a/drivers/tripplitesu.c +++ b/drivers/tripplitesu.c @@ -738,43 +738,77 @@ static int auto_reboot(int enable) { return 1; } -/* Read the auto-reboot setting (whether the device starts again when mains - * is (re)applied) and publish it as ups.start.auto; 1 = yes, 2 = no. - * This is auxiliary information, so a missing or unexpected answer must not - * mark the whole UPS as stale: a warning is logged and the previously - * published value is left in place. */ -static int get_auto_reboot(void) { +/* A device setting that the protocol reports as a single number: 1 selects + * the first name published in the variable, 2 the second one. The variable + * name and both names live in the table, so that reading any of these + * settings is the same code. The table order must match the identifiers + * below, which are used as its indexes. */ +enum two_state_setting_id { + TWO_STATE_AUTO_REBOOT, + TWO_STATE_ECO_MODE, + TWO_STATE_BEEPER +}; + +struct two_state_setting { + const char *command; /* protocol POLL command */ + const char *varname; /* variable the setting is published as */ + const char *what; /* description used in the log messages */ + const char *on_name; /* name published for value 1 */ + const char *off_name; /* name published for value 2 */ +}; + +static const struct two_state_setting two_state_settings[] = { + { AUTO_REBOOT, "ups.start.auto", "auto-reboot setting", + "yes", "no" }, + { ECONOMIC_MODE, "input.eco.switchable", "ECO setting", + "ECO", "normal" }, + { ENABLE_BUZZER, "ups.beeper.status", "buzzer setting", + "enabled", "disabled" } +}; + +/* Read a two-state setting (see above) and publish it. This is auxiliary + * information, so a missing or unexpected answer must not mark the whole + * UPS as stale: a warning is logged and the previously published value is + * left in place. Returns 1 when the setting was published. */ +static int get_two_state_setting(enum two_state_setting_id id) { + const struct two_state_setting *setting = &two_state_settings[id]; char response[MAX_RESPONSE_LENGTH]; - unsigned int mode; + unsigned int value; - if (do_command(POLL, AUTO_REBOOT, "", response, sizeof(response)) <= 0) { - upslogx(LOG_WARNING, "%s: could not read the auto-reboot " - "setting [%s]; keeping the last known value", __func__, - AUTO_REBOOT); + if (do_command(POLL, setting->command, "", response, sizeof(response)) <= 0) { + upslogx(LOG_WARNING, "%s: could not read the %s [%s]; " + "keeping the last known value", __func__, setting->what, + setting->command); return 0; } - if (!str_to_uint_strict(response, &mode, 10)) { + if (!str_to_uint_strict(response, &value, 10)) { upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " - "keeping the last known value", __func__, AUTO_REBOOT, - response); + "keeping the last known value", __func__, + setting->command, response); return 0; } - switch (mode) { + switch (value) { case 1: - dstate_setinfo("ups.start.auto", "%s", "yes"); + dstate_setinfo(setting->varname, "%s", setting->on_name); return 1; case 2: - dstate_setinfo("ups.start.auto", "%s", "no"); + dstate_setinfo(setting->varname, "%s", setting->off_name); return 1; default: upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " - "keeping the last known value", __func__, AUTO_REBOOT, - response); + "keeping the last known value", __func__, + setting->command, response); return 0; } } +/* Read the auto-reboot setting (whether the device starts again when mains + * is (re)applied) and publish it as ups.start.auto; 1 = yes, 2 = no. */ +static int get_auto_reboot(void) { + return get_two_state_setting(TWO_STATE_AUTO_REBOOT); +} + /* Read the battery installation date (BRD field 0, formatted as YYYYMMDD). * The second field looks like a scheduled replacement date, but its meaning * has not been confirmed for this protocol, so it is only reported in debug @@ -1014,35 +1048,7 @@ static int get_bypass_reason(void) { * answer the poll: in that case a warning is logged and the published value * is left alone. */ static int get_eco_mode(void) { - char response[MAX_RESPONSE_LENGTH]; - unsigned int mode; - - if (do_command(POLL, ECONOMIC_MODE, "", response, sizeof(response)) <= 0) { - upslogx(LOG_WARNING, "%s: could not read the ECO setting " - "[%s]; keeping the last known value", __func__, - ECONOMIC_MODE); - return 0; - } - if (!str_to_uint_strict(response, &mode, 10)) { - upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " - "keeping the last known value", __func__, ECONOMIC_MODE, - response); - return 0; - } - - switch (mode) { - case 1: - dstate_setinfo("input.eco.switchable", "%s", "ECO"); - return 1; - case 2: - dstate_setinfo("input.eco.switchable", "%s", "normal"); - return 1; - default: - upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " - "keeping the last known value", __func__, - ECONOMIC_MODE, response); - return 0; - } + return get_two_state_setting(TWO_STATE_ECO_MODE); } /* Switch between high efficiency (1) and high quality (2) operation. The @@ -1066,35 +1072,7 @@ static void set_eco_mode(int mode) { * performed by the beeper.mute command) is a transient state that the device * does not report. */ static int get_beeper_status(void) { - char response[MAX_RESPONSE_LENGTH]; - unsigned int status; - - if (do_command(POLL, ENABLE_BUZZER, "", response, sizeof(response)) <= 0) { - upslogx(LOG_WARNING, "%s: could not read the buzzer setting " - "[%s]; keeping the last known value", __func__, - ENABLE_BUZZER); - return 0; - } - if (!str_to_uint_strict(response, &status, 10)) { - upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " - "keeping the last known value", __func__, ENABLE_BUZZER, - response); - return 0; - } - - switch (status) { - case 1: - dstate_setinfo("ups.beeper.status", "%s", "enabled"); - return 1; - case 2: - dstate_setinfo("ups.beeper.status", "%s", "disabled"); - return 1; - default: - upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " - "keeping the last known value", __func__, ENABLE_BUZZER, - response); - return 0; - } + return get_two_state_setting(TWO_STATE_BEEPER); } /* Enable (1) or disable (2) the buzzer; this is a device setting, unlike @@ -1261,6 +1239,100 @@ static int parse_reboot_delay(const char *cmdname, const char *extra, return 0; } +/* One shutdown-related instant command: the auto-reboot setting (ARB) it + * needs, where its default shutdown delay comes from and whether the load is + * expected to restart right behind the off (timed return) or only when mains + * is (re)applied. */ +struct shutdown_command { + const char *name; /* instant command name */ + int autoreboot; /* ARB value to set: 1 = yes, 0 = no */ + int use_offdelay; /* 1: default value is the offdelay setting */ + int timed_return; /* 1: the load restarts right behind the off */ +}; + +static const struct shutdown_command shutdown_commands[] = { + { "shutdown.reboot", 1, 1, 1 }, + { "shutdown.reboot.graceful", 1, 0, 1 }, + { "shutdown.return", 1, 1, 0 }, + { "shutdown.stayoff", 0, 1, 0 } +}; + +/* Switch one outlet bank: the device addresses the banks with ";1", + * starting at 1, and the RELAY_* command says what to do with it. */ +static int switch_outlet(unsigned int outlet, const char *relay) { + char parm[20]; + + snprintf(parm, sizeof(parm), "%u;1", outlet); + return send_set_or_fail(relay, parm); +} + +/* Send 'relay' to every outlet bank. This is a multi-frame command: stop + * at the first frame the device rejects or does not answer, so that a + * failure is not reported as a success (the banks that were not reached are + * left as they were). */ +static int switch_outlets(const char *relay) { + unsigned int outlet; + + for (outlet = 1; outlet <= (unsigned int)ups.outlets; outlet++) { + if (switch_outlet(outlet, relay) != STAT_INSTCMD_HANDLED) { + return STAT_INSTCMD_FAILED; + } + } + + return STAT_INSTCMD_HANDLED; +} + +/* The shutdown commands share the same skeleton: set the auto-reboot + * setting ("ups.start.auto") to what the shutdown needs, set the restart + * (SDR) delay and schedule the shutdown action (SDA). */ +static int shutdown_command(const char *cmdname, const char *extra, + const struct shutdown_command *spec) +{ + char parm[20]; + unsigned int delay, return_delay; + unsigned int fallback = spec->use_offdelay + ? offdelay : DEFAULT_GRACEFUL_DELAY; + + if (spec->timed_return) { + /* "off[:on]", both in the units the device uses */ + if (!parse_reboot_delay(cmdname, extra, fallback, + &delay, &return_delay)) { + return STAT_INSTCMD_FAILED; + } + } else { + if (!get_shutdown_delay(cmdname, extra, fallback, &delay)) { + return STAT_INSTCMD_FAILED; + } + /* no timed return: only mains coming back may start the + * load again */ + return_delay = 0; + } + + upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); + /* set the auto-reboot and return-delay prerequisites before + * scheduling the shutdown: if either cannot be confirmed, stop here + * and leave the load powered rather than power it off with no + * guaranteed way to come back on */ + if (!auto_reboot(spec->autoreboot)) { + upslogx(LOG_ERR, "%s: could not set or confirm " + "ups.start.auto, not powering the load off", cmdname); + return STAT_INSTCMD_FAILED; + } + if (!set_start_delay(return_delay)) { + upslogx(LOG_ERR, "%s: could not set the return delay, " + "not powering the load off", cmdname); + return STAT_INSTCMD_FAILED; + } + + snprintf(parm, sizeof(parm), "%u", delay); + if (send_set_or_fail(TSU_SHUTDOWN_ACTION, parm) + != STAT_INSTCMD_HANDLED) { + return STAT_INSTCMD_FAILED; + } + + return STAT_INSTCMD_HANDLED; +} + static int instcmd(const char *cmdname, const char *extra) { static const struct { @@ -1271,51 +1343,21 @@ static int instcmd(const char *cmdname, const char *extra) { "load.off", RELAY_OFF, 0 }, { "load.on", RELAY_ON, 1 } }; - /* the shutdown commands share the same skeleton: set the auto-reboot - * setting ("ups.start.auto") to what the shutdown needs, set the - * restart (SDR) delay and schedule the shutdown action (SDA) */ - static const struct { - const char *name; /* instant command name */ - int autoreboot; /* ARB value to set: 1 = yes, 0 = no */ - int use_offdelay; /* 1: default value is the offdelay setting */ - int timed_return; /* 1: the load restarts right behind the off */ - } shutdown_commands[] = { - { "shutdown.reboot", 1, 1, 1 }, - { "shutdown.reboot.graceful", 1, 0, 1 }, - { "shutdown.return", 1, 1, 0 }, - { "shutdown.stayoff", 0, 1, 0 } - }; - char parm[20]; char command[32]; /* "outlet..load.on" and friends */ unsigned int outlet, cmd_index; - unsigned int delay, return_delay; - unsigned int fallback; upsdebug_INSTCMD_STARTING(cmdname, extra); - /* Switching the outlets is a multi-frame command: stop at the first - * frame the device rejects or does not answer, so that a failure is - * not reported as a success (the outlets that were not reached are - * left as they were). */ + /* Switching the outlets is a multi-frame command: switch_outlets() + * stops at the first frame the device rejects or does not answer, so + * that a failure is not reported as a success. */ if (!strcasecmp(cmdname, "load.off")) { upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - for (outlet = 1; outlet <= (unsigned int)ups.outlets; outlet++) { - snprintf(parm, sizeof(parm), "%u;1", outlet); - if (send_set_or_fail(RELAY_OFF, parm) - != STAT_INSTCMD_HANDLED) - return STAT_INSTCMD_FAILED; - } - return STAT_INSTCMD_HANDLED; + return switch_outlets(RELAY_OFF); } if (!strcasecmp(cmdname, "load.on")) { upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); - for (outlet = 1; outlet <= (unsigned int)ups.outlets; outlet++) { - snprintf(parm, sizeof(parm), "%u;1", outlet); - if (send_set_or_fail(RELAY_ON, parm) - != STAT_INSTCMD_HANDLED) - return STAT_INSTCMD_FAILED; - } - return STAT_INSTCMD_HANDLED; + return switch_outlets(RELAY_ON); } /* one set of commands per outlet; these take no value */ for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(outlet_commands); cmd_index++) { @@ -1330,9 +1372,8 @@ static int instcmd(const char *cmdname, const char *extra) else upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - snprintf(parm, sizeof(parm), "%u;1", outlet); - return send_set_or_fail(outlet_commands[cmd_index].relay, - parm); + return switch_outlet(outlet, + outlet_commands[cmd_index].relay); } } /* buzzer: BUZ 2 silences the alarm that is currently sounding. @@ -1353,44 +1394,10 @@ static int instcmd(const char *cmdname, const char *extra) } for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(shutdown_commands); cmd_index++) { - if (strcasecmp(cmdname, shutdown_commands[cmd_index].name)) - continue; - - fallback = shutdown_commands[cmd_index].use_offdelay - ? offdelay : DEFAULT_GRACEFUL_DELAY; - if (shutdown_commands[cmd_index].timed_return) { - /* "off[:on]", both in the units the device uses */ - if (!parse_reboot_delay(cmdname, extra, fallback, - &delay, &return_delay)) - return STAT_INSTCMD_FAILED; - } else { - if (!get_shutdown_delay(cmdname, extra, fallback, &delay)) - return STAT_INSTCMD_FAILED; - /* no timed return: only mains coming back may start - * the load again */ - return_delay = 0; - } - - upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - /* set the auto-reboot and return-delay prerequisites before - * scheduling the shutdown: if either cannot be confirmed, - * stop here and leave the load powered rather than power it - * off with no guaranteed way to come back on */ - if (!auto_reboot(shutdown_commands[cmd_index].autoreboot)) { - upslogx(LOG_ERR, "%s: could not set or confirm " - "ups.start.auto, not powering the load off", cmdname); - return STAT_INSTCMD_FAILED; + if (!strcasecmp(cmdname, shutdown_commands[cmd_index].name)) { + return shutdown_command(cmdname, extra, + &shutdown_commands[cmd_index]); } - if (!set_start_delay(return_delay)) { - upslogx(LOG_ERR, "%s: could not set the return " - "delay, not powering the load off", cmdname); - return STAT_INSTCMD_FAILED; - } - snprintf(parm, sizeof(parm), "%u", delay); - if (send_set_or_fail(TSU_SHUTDOWN_ACTION, parm) - != STAT_INSTCMD_HANDLED) - return STAT_INSTCMD_FAILED; - return STAT_INSTCMD_HANDLED; } if (!strcasecmp(cmdname, "shutdown.stop")) { upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); From 6b33deba0ef800eec42375d9300568795596dac3 Mon Sep 17 00:00:00 2001 From: TomZanna Date: Wed, 21 Oct 2026 22:53:19 +0200 Subject: [PATCH 09/13] refactor(tripplitesu): generalize transfer voltage handling Replace the separate low/high transfer voltage getters and setters with table-driven, index-based helpers backed by a transfer_voltage_id enum. Each entry carries the dstate variable name and its field in the TXV reply, so field parsing, comparison and the combined device write share one code path instead of duplicated logic. Store reported transfer limits per voltage in arrays keyed by the same enum, replacing the four min/max globals. Signed-off-by: TomZanna --- drivers/tripplitesu.c | 401 ++++++++++++++++++++++++------------------ 1 file changed, 228 insertions(+), 173 deletions(-) diff --git a/drivers/tripplitesu.c b/drivers/tripplitesu.c index 36248cc16d..c4e421aa29 100644 --- a/drivers/tripplitesu.c +++ b/drivers/tripplitesu.c @@ -286,8 +286,25 @@ static unsigned int watchdog_seconds = 0; /* watchdog timeout currently set on t static unsigned int watchdog_alarm = 2; /* watchdog restart-alarm flag currently set on the device */ static int watchdog_known = 0; /* set once the device reported or accepted its watchdog setting */ static int shutdown_abort_on_error = 0; /* 1 when 'shutdown_mode' asks the driver shutdown handler to stop at the first failure */ -static unsigned int min_low_transfer, max_low_transfer; /* reported transfer voltage ranges */ -static unsigned int min_high_transfer, max_high_transfer; +/* The two settable transfer voltages: the variable each one is published as + * and the field it occupies in the TXV reply. The table order must match the + * transfer_voltage_id values, which are used as its indexes. */ +enum transfer_voltage_id { + TRANSFER_VOLTAGE_LOW, + TRANSFER_VOLTAGE_HIGH, + TRANSFER_VOLTAGE_COUNT +}; + +static const struct transfer_voltage_setting { + const char *varname; + unsigned int field; +} transfer_voltage_settings[TRANSFER_VOLTAGE_COUNT] = { + { "input.transfer.low", 0 }, + { "input.transfer.high", 1 } +}; + +static unsigned int min_transfer[TRANSFER_VOLTAGE_COUNT]; /* reported minimum */ +static unsigned int max_transfer[TRANSFER_VOLTAGE_COUNT]; /* reported maximum */ /* message types */ #define POLL 'P' @@ -589,64 +606,65 @@ static void set_identification(const char *val) { } } -static int get_transfer_voltage_low(void) { +static int get_transfer_voltage(enum transfer_voltage_id id) { + const struct transfer_voltage_setting *setting = + &transfer_voltage_settings[id]; char response[MAX_RESPONSE_LENGTH]; - char low[32]; + char value[32]; if (do_command(POLL, TRANSFER_VOLTAGE, "", response, sizeof(response)) > 0 - && get_field(response, 0, low, sizeof(low))) { - dstate_setinfo("input.transfer.low", "%d", atoi(low)); + && get_field(response, setting->field, value, sizeof(value))) { + dstate_setinfo(setting->varname, "%d", atoi(value)); return 1; } return 0; } -static void set_transfer_voltage_low(int val) { +static void set_transfer_voltage(enum transfer_voltage_id id, int val) { + const struct transfer_voltage_setting *setting = + &transfer_voltage_settings[id]; char response[MAX_RESPONSE_LENGTH]; - char low[32], high[32]; + int voltages[TRANSFER_VOLTAGE_COUNT]; + unsigned int i; if (do_command(POLL, TRANSFER_VOLTAGE, "", response, sizeof(response)) <= 0) return; - if (!get_field(response, 0, low, sizeof(low)) - || !get_field(response, 1, high, sizeof(high))) - return; - if (val == atoi(low)) + /* the device takes both voltages at once, and this driver must not + * change the other one: leave the setting alone unless both are + * readable */ + for (i = 0; i < TRANSFER_VOLTAGE_COUNT; i++) { + char value[32]; + + if (!get_field(response, transfer_voltage_settings[i].field, + value, sizeof(value))) + return; + voltages[i] = atoi(value); + } + if (val == voltages[id]) return; - snprintf(response, sizeof(response), "%d;%d", val, atoi(high)); + voltages[id] = val; + snprintf(response, sizeof(response), "%d;%d", + voltages[TRANSFER_VOLTAGE_LOW], + voltages[TRANSFER_VOLTAGE_HIGH]); if (!send_set_command(TRANSFER_VOLTAGE, response)) upslogx(LOG_ERR, "%s: device rejected or did not answer " - "[%s %s]", __func__, TRANSFER_VOLTAGE, response); -} - -static int get_transfer_voltage_high(void) { - char response[MAX_RESPONSE_LENGTH]; - char high[32]; - - if (do_command(POLL, TRANSFER_VOLTAGE, "", response, sizeof(response)) > 0 - && get_field(response, 1, high, sizeof(high))) { - dstate_setinfo("input.transfer.high", "%d", atoi(high)); - return 1; - } - - return 0; + "[%s %s] for %s", __func__, TRANSFER_VOLTAGE, response, + setting->varname); } -static void set_transfer_voltage_high(int val) { - char response[MAX_RESPONSE_LENGTH]; - char low[32], high[32]; +/* Store one of the transfer voltage range fields of a RAT reply, leaving the + * current value alone when the device does not report a usable one. */ +static void set_transfer_range(const char *response, unsigned int field, + unsigned int *target) { + char value[32]; + int parsed; - if (do_command(POLL, TRANSFER_VOLTAGE, "", response, sizeof(response)) <= 0) + if (!get_field(response, field, value, sizeof(value))) return; - if (!get_field(response, 0, low, sizeof(low)) - || !get_field(response, 1, high, sizeof(high))) - return; - if (val == atoi(high)) - return; - snprintf(response, sizeof(response), "%d;%d", atoi(low), val); - if (!send_set_command(TRANSFER_VOLTAGE, response)) - upslogx(LOG_ERR, "%s: device rejected or did not answer " - "[%s %s]", __func__, TRANSFER_VOLTAGE, response); + parsed = atoi(value); + if (parsed >= 0) + *target = (unsigned int)parsed; } static int get_sensitivity(void) { @@ -1257,6 +1275,51 @@ static const struct shutdown_command shutdown_commands[] = { { "shutdown.stayoff", 0, 1, 0 } }; +/* How an instant command affects the power state, for the upshandler.h + * logging macros: some change it, some may, and many do not touch it. */ +#define INSTCMD_POWERSTATE_NONE 0 +#define INSTCMD_POWERSTATE_MAYBE 1 +#define INSTCMD_POWERSTATE_CHANGE 2 + +/* Log an instant command before it is sent, using the classification kept in + * the command tables below. This is a macro so that the logging macros still + * report instcmd() as the function that sent the command. */ +#define log_instcmd_powerstate(powerstate, cmdname, extra) do { \ + if ((powerstate) == INSTCMD_POWERSTATE_CHANGE) \ + upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); \ + else if ((powerstate) == INSTCMD_POWERSTATE_MAYBE) \ + upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); \ + } while (0) + +/* Commands that switch every outlet bank ("load.off", "load.on") and, with + * the same name after "outlet..", a single bank. */ +static const struct relay_command { + const char *name; /* instant command name */ + const char *relay; /* protocol RELAY_* command */ + int powerstate; /* INSTCMD_POWERSTATE_* value */ +} relay_commands[] = { + { "load.off", RELAY_OFF, INSTCMD_POWERSTATE_CHANGE }, + { "load.on", RELAY_ON, INSTCMD_POWERSTATE_MAYBE } +}; + +/* Commands that are a single SET frame with a fixed parameter. The buzzer + * entry only silences the alarm that is currently sounding: enabling and + * disabling the buzzer is a device setting (it survives restarts), so it is + * exposed as the read/write ups.beeper.status variable instead of a pair of + * instant commands. */ +static const struct simple_command { + const char *name; /* instant command name */ + const char *command; /* protocol SET command */ + const char *parameters; /* parameter sent with it */ + int powerstate; /* INSTCMD_POWERSTATE_* value */ +} simple_commands[] = { + { "beeper.mute", BUZZER, "2", INSTCMD_POWERSTATE_NONE }, + { "clear.fault.record", BYPASS_REASON, "0", INSTCMD_POWERSTATE_NONE }, + { "shutdown.stop", TSU_SHUTDOWN_ACTION, "0", INSTCMD_POWERSTATE_MAYBE }, + { "test.battery.start", TEST, "3", INSTCMD_POWERSTATE_MAYBE }, + { "test.battery.stop", TEST, "0", INSTCMD_POWERSTATE_MAYBE } +}; + /* Switch one outlet bank: the device addresses the banks with ";1", * starting at 1, and the RELAY_* command says what to do with it. */ static int switch_outlet(unsigned int outlet, const char *relay) { @@ -1333,16 +1396,12 @@ static int shutdown_command(const char *cmdname, const char *extra, return STAT_INSTCMD_HANDLED; } +/* Dispatch an instant command. The command names, their frames and how they + * affect the power state all come from the tables above, which are also used + * to register the commands (see add_instant_commands()), so a command cannot + * be advertised without being handled or handled without being advertised. */ static int instcmd(const char *cmdname, const char *extra) { - static const struct { - const char *suffix; /* command suffix */ - const char *relay; /* protocol RELAY_* command */ - int maybepower; /* 1: only possibly affects power state */ - } outlet_commands[] = { - { "load.off", RELAY_OFF, 0 }, - { "load.on", RELAY_ON, 1 } - }; char command[32]; /* "outlet..load.on" and friends */ unsigned int outlet, cmd_index; @@ -1351,47 +1410,46 @@ static int instcmd(const char *cmdname, const char *extra) /* Switching the outlets is a multi-frame command: switch_outlets() * stops at the first frame the device rejects or does not answer, so * that a failure is not reported as a success. */ - if (!strcasecmp(cmdname, "load.off")) { - upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - return switch_outlets(RELAY_OFF); - } - if (!strcasecmp(cmdname, "load.on")) { - upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); - return switch_outlets(RELAY_ON); + for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(relay_commands); + cmd_index++) { + if (strcasecmp(cmdname, relay_commands[cmd_index].name)) + continue; + + log_instcmd_powerstate(relay_commands[cmd_index].powerstate, + cmdname, extra); + return switch_outlets(relay_commands[cmd_index].relay); } /* one set of commands per outlet; these take no value */ - for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(outlet_commands); cmd_index++) { - for (outlet = 1; outlet <= (unsigned int)ups.outlets; outlet++) { + for (outlet = 1; outlet <= (unsigned int)ups.outlets; outlet++) { + for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(relay_commands); + cmd_index++) { snprintf(command, sizeof(command), "outlet.%u.%s", - outlet, outlet_commands[cmd_index].suffix); + outlet, relay_commands[cmd_index].name); if (strcasecmp(cmdname, command)) continue; - if (outlet_commands[cmd_index].maybepower) - upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); - else - upslog_INSTCMD_POWERSTATE_CHANGE(cmdname, extra); - + log_instcmd_powerstate( + relay_commands[cmd_index].powerstate, + cmdname, extra); return switch_outlet(outlet, - outlet_commands[cmd_index].relay); + relay_commands[cmd_index].relay); } } - /* buzzer: BUZ 2 silences the alarm that is currently sounding. - * Enabling/disabling the buzzer is a device setting (it survives - * restarts), so it is exposed as the read/write ups.beeper.status - * variable instead of a pair of instant commands. */ - if (!strcasecmp(cmdname, "beeper.mute")) { - return send_set_or_fail(BUZZER, "2"); - } - if (!strcasecmp(cmdname, "clear.fault.record")) { - /* reset the bypass reason recorded by the device */ - return send_set_or_fail(BYPASS_REASON, "0"); + /* commands that are a single SET frame */ + for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(simple_commands); + cmd_index++) { + if (strcasecmp(cmdname, simple_commands[cmd_index].name)) + continue; + + log_instcmd_powerstate( + simple_commands[cmd_index].powerstate, cmdname, extra); + return send_set_or_fail(simple_commands[cmd_index].command, + simple_commands[cmd_index].parameters); } - if (!strcasecmp(cmdname, "reset.watchdog")) { - /* kick the watchdog: this takes no value, it sends the - * setting the driver has, which restarts the device timer */ + /* kick the watchdog: this takes no value, it sends the setting the + * driver has, which restarts the device timer */ + if (!strcasecmp(cmdname, "reset.watchdog")) return reset_watchdog(); - } for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(shutdown_commands); cmd_index++) { if (!strcasecmp(cmdname, shutdown_commands[cmd_index].name)) { @@ -1399,27 +1457,52 @@ static int instcmd(const char *cmdname, const char *extra) &shutdown_commands[cmd_index]); } } - if (!strcasecmp(cmdname, "shutdown.stop")) { - upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); - /* SDA 0 cancels a scheduled shutdown; the auto-reboot - * setting is left as it is */ - return send_set_or_fail(TSU_SHUTDOWN_ACTION, "0"); - } - if (!strcasecmp(cmdname, "test.battery.start")) { - upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); - return send_set_or_fail(TEST, "3"); - } - if (!strcasecmp(cmdname, "test.battery.stop")) { - upslog_INSTCMD_POWERSTATE_MAYBE(cmdname, extra); - return send_set_or_fail(TEST, "0"); - } upslog_INSTCMD_UNKNOWN(cmdname, extra); return STAT_INSTCMD_UNKNOWN; } +/* Register every instant command the driver supports. The names come from + * the same tables instcmd() dispatches on, so the two cannot drift apart. */ +static void add_instant_commands(void) +{ + char command[32]; /* "outlet..load.on" and friends */ + unsigned int outlet, cmd_index; + + for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(simple_commands); + cmd_index++) { + dstate_addcmd(simple_commands[cmd_index].name); + } + dstate_addcmd("reset.watchdog"); + for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(shutdown_commands); + cmd_index++) { + dstate_addcmd(shutdown_commands[cmd_index].name); + } + + /* the relay commands switch outlets (load.off and load.on all of + * them, outlet.n.load.* a single bank), so they are only registered + * when the device reports any */ + if (!ups.outlets) + return; + + for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(relay_commands); + cmd_index++) { + dstate_addcmd(relay_commands[cmd_index].name); + } + for (outlet = 1; outlet <= (unsigned int)ups.outlets; outlet++) { + for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(relay_commands); + cmd_index++) { + snprintf(command, sizeof(command), "outlet.%u.%s", + outlet, relay_commands[cmd_index].name); + dstate_addcmd(command); + } + } +} + static int setvar(const char *varname, const char *val) { + unsigned int i, voltage; + upsdebug_SET_STARTING(varname, val); if (!strcasecmp(varname, "ups.id")) { @@ -1539,32 +1622,21 @@ static int setvar(const char *varname, const char *val) get_eco_mode(); return STAT_SET_HANDLED; } - if (!strcasecmp(varname, "input.transfer.low")) { - unsigned int voltage; + for (i = 0; i < TRANSFER_VOLTAGE_COUNT; i++) { + const struct transfer_voltage_setting *setting = + &transfer_voltage_settings[i]; - if (!parse_transfer_voltage(val, min_low_transfer, - max_low_transfer, &voltage)) { - upslogx(LOG_ERR, "setvar(%s): invalid value '%s' " - "(expected %u..%u)", varname, val, - min_low_transfer, max_low_transfer); - return STAT_SET_FAILED; - } - set_transfer_voltage_low((int)voltage); - get_transfer_voltage_low(); - return STAT_SET_HANDLED; - } - if (!strcasecmp(varname, "input.transfer.high")) { - unsigned int voltage; - - if (!parse_transfer_voltage(val, min_high_transfer, - max_high_transfer, &voltage)) { + if (strcasecmp(varname, setting->varname)) + continue; + if (!parse_transfer_voltage(val, min_transfer[i], + max_transfer[i], &voltage)) { upslogx(LOG_ERR, "setvar(%s): invalid value '%s' " "(expected %u..%u)", varname, val, - min_high_transfer, max_high_transfer); + min_transfer[i], max_transfer[i]); return STAT_SET_FAILED; } - set_transfer_voltage_high((int)voltage); - get_transfer_voltage_high(); + set_transfer_voltage((enum transfer_voltage_id)i, (int)voltage); + get_transfer_voltage((enum transfer_voltage_id)i); return STAT_SET_HANDLED; } if (!strcasecmp(varname, "input.sensitivity")) { @@ -1614,14 +1686,15 @@ void upsdrv_initinfo(void) { char response[MAX_RESPONSE_LENGTH]; char buf[32]; /* "outlet..load.on" and friends */ + char rangevar[48]; /* ".min" and ".max" */ char value[32]; unsigned int i; if (!init_comm()) fatalx(EXIT_FAILURE, "Unable to detect Tripp Lite SmartOnline UPS on port %s\n", device_path); - min_low_transfer = max_low_transfer = 0; - min_high_transfer = max_high_transfer = 0; + for (i = 0; i < TRANSFER_VOLTAGE_COUNT; i++) + min_transfer[i] = max_transfer[i] = 0; /* get all the read-only fields here */ if (do_command(POLL, MANUFACTURER, "", response, sizeof(response)) > 0) @@ -1640,26 +1713,18 @@ void upsdrv_initinfo(void) if (get_field(response, 14, value, sizeof(value))) dstate_setinfo("battery.voltage.nominal", "%d", atoi(value)); - if (get_field(response, 10, value, sizeof(value))) { - int ipv = atoi(value); - if (ipv >= 0) - min_low_transfer = (unsigned int)ipv; - } - if (get_field(response, 9, value, sizeof(value))) { - int ipv = atoi(value); - if (ipv >= 0) - max_low_transfer = (unsigned int)ipv; - } - if (get_field(response, 12, value, sizeof(value))) { - int ipv = atoi(value); - if (ipv >= 0) - min_high_transfer = (unsigned int)ipv; - } - if (get_field(response, 11, value, sizeof(value))) { - int ipv = atoi(value); - if (ipv >= 0) - max_high_transfer = (unsigned int)ipv; - } + /* the transfer voltage ranges: field 10 is the lowest settable + * low transfer, 9 the highest settable low transfer, 12 the + * lowest settable high transfer and 11 the highest settable + * high transfer */ + set_transfer_range(response, 10, + &min_transfer[TRANSFER_VOLTAGE_LOW]); + set_transfer_range(response, 9, + &max_transfer[TRANSFER_VOLTAGE_LOW]); + set_transfer_range(response, 12, + &min_transfer[TRANSFER_VOLTAGE_HIGH]); + set_transfer_range(response, 11, + &max_transfer[TRANSFER_VOLTAGE_HIGH]); } if (do_command(POLL, OUTLET_RELAYS, "", response, sizeof(response)) > 0) { int outlet_count = atoi(response); @@ -1716,25 +1781,33 @@ void upsdrv_initinfo(void) dstate_setflags("ups.id", ST_FLAG_RW | ST_FLAG_STRING); dstate_setaux("ups.id", 100); } - if (get_transfer_voltage_low() && max_low_transfer) { - dstate_setflags("input.transfer.low", ST_FLAG_RW); - for (i = min_low_transfer; i <= max_low_transfer; i++) - dstate_addenum("input.transfer.low", "%u", i); - } - if (get_transfer_voltage_high() && max_high_transfer) { - dstate_setflags("input.transfer.high", ST_FLAG_RW); - for (i = min_high_transfer; i <= max_high_transfer; i++) - dstate_addenum("input.transfer.high", "%u", i); + for (i = 0; i < TRANSFER_VOLTAGE_COUNT; i++) { + const struct transfer_voltage_setting *setting = + &transfer_voltage_settings[i]; + unsigned int voltage; + + if (!get_transfer_voltage((enum transfer_voltage_id)i)) + continue; + if (!max_transfer[i]) + continue; + dstate_setflags(setting->varname, ST_FLAG_RW); + for (voltage = min_transfer[i]; + voltage <= max_transfer[i]; voltage++) { + dstate_addenum(setting->varname, "%u", voltage); + } } /* report the settable ranges, so that the values accepted by setvar() * are visible to clients */ - if (max_low_transfer) { - dstate_setinfo("input.transfer.low.min", "%u", min_low_transfer); - dstate_setinfo("input.transfer.low.max", "%u", max_low_transfer); - } - if (max_high_transfer) { - dstate_setinfo("input.transfer.high.min", "%u", min_high_transfer); - dstate_setinfo("input.transfer.high.max", "%u", max_high_transfer); + for (i = 0; i < TRANSFER_VOLTAGE_COUNT; i++) { + const struct transfer_voltage_setting *setting = + &transfer_voltage_settings[i]; + + if (!max_transfer[i]) + continue; + snprintf(rangevar, sizeof(rangevar), "%s.min", setting->varname); + dstate_setinfo(rangevar, "%u", min_transfer[i]); + snprintf(rangevar, sizeof(rangevar), "%s.max", setting->varname); + dstate_setinfo(rangevar, "%u", max_transfer[i]); } if (get_sensitivity()) { dstate_setflags("input.sensitivity", ST_FLAG_RW); @@ -1742,28 +1815,10 @@ void upsdrv_initinfo(void) dstate_addenum("input.sensitivity", "%s", sensitivity[i].name); } - /* load.off, load.on and the commands below switch individual outlet - * outlets, so they are only registered when the device reports any */ - if (ups.outlets) { - dstate_addcmd("load.off"); - dstate_addcmd("load.on"); - for (i = 1; i <= (unsigned int)ups.outlets; i++) { - snprintf(buf, sizeof(buf), "outlet.%u.load.off", i); - dstate_addcmd(buf); - snprintf(buf, sizeof(buf), "outlet.%u.load.on", i); - dstate_addcmd(buf); - } - } - dstate_addcmd("beeper.mute"); - dstate_addcmd("clear.fault.record"); - dstate_addcmd("reset.watchdog"); - dstate_addcmd("shutdown.reboot"); - dstate_addcmd("shutdown.reboot.graceful"); - dstate_addcmd("shutdown.return"); - dstate_addcmd("shutdown.stayoff"); - dstate_addcmd("shutdown.stop"); - dstate_addcmd("test.battery.start"); - dstate_addcmd("test.battery.stop"); + /* register every command, taking the names from the same tables that + * instcmd() dispatches on (load.off, load.on and the outlet commands + * are only added when the device reports outlet banks) */ + add_instant_commands(); /* add all the variables that change regularly */ upsdrv_updateinfo(); From 565b9139fe82871853bbd8f15f0260ff3a12ca0a Mon Sep 17 00:00:00 2001 From: TomZanna Date: Thu, 22 Oct 2026 14:38:38 +0000 Subject: [PATCH 10/13] refactor(tripplitesu): reorder instcmd lookup and trim comments Move the shutdown command lookup to the top of instcmd() so these commands are matched before the outlet and simple command paths. Guard the outlet command walk and the shutdown lookup with a case-insensitive prefix check, so commands that cannot match skip those loops entirely. Also shorten several header and function comments and drop the verbose restore hint from the ups.start.auto change log line, keeping only the old and new setting names. Signed-off-by: TomZanna --- drivers/tripplitesu.c | 64 +++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/drivers/tripplitesu.c b/drivers/tripplitesu.c index c4e421aa29..860d14e526 100644 --- a/drivers/tripplitesu.c +++ b/drivers/tripplitesu.c @@ -123,10 +123,8 @@ Without a colon, only the shutdown delay is changed and the restart delay defaults to "startdelay"; with no value at all, the shutdown delay defaults to "offdelay" (60 seconds for the graceful variant) as well. - Fields that are empty, non-numeric, use ';', '.' or ',', or are out of - range make the command fail before anything is sent. The "load.off", - "load.on", "outlet.n.load.*" and "reset.watchdog" commands do not take a - value. + The "load.off", "load.on", "outlet.n.load.*" and "reset.watchdog" commands + do not take a value. Commands that send more than one frame stop at the first operation the device rejects or does not answer, so a failed command is never applied @@ -137,8 +135,7 @@ "shutdown.return" and "shutdown.stayoff" clear the device return delay (SDR 0), so the load comes back only when mains is (re)applied or not at all, while the two reboot commands ask for the load to come back after - the restart delay. The firmware decides how the shutdown and restart - delays relate; the driver sends both as requested. + the restart delay. Because that is what the commands mean on this hardware, the shutdown commands change the device auto-reboot setting ("ups.start.auto", ARB); @@ -740,11 +737,8 @@ static int auto_reboot(int enable) { "[%s %s]", __func__, AUTO_REBOOT, parm); return 0; } - /* the device stores this setting, so the change outlives the - * shutdown that needs it: make it (and how to undo it) visible */ - upslogx(LOG_INFO, "%s: ups.start.auto changed from '%s' to " - "'%s' (device setting; restore with " - "\"upsrw -s ups.start.auto=yes|no\" if needed)", + + upslogx(LOG_INFO, "%s: ups.start.auto changed from '%s' to '%s'", __func__, auto_reboot_name(oldmode), auto_reboot_name(mode)); } @@ -1407,6 +1401,15 @@ static int instcmd(const char *cmdname, const char *extra) upsdebug_INSTCMD_STARTING(cmdname, extra); + if (!strncasecmp(cmdname, "shutdown.", 9)) { + for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(shutdown_commands); + cmd_index++) { + if (!strcasecmp(cmdname, shutdown_commands[cmd_index].name)) { + return shutdown_command(cmdname, extra, + &shutdown_commands[cmd_index]); + } + } + } /* Switching the outlets is a multi-frame command: switch_outlets() * stops at the first frame the device rejects or does not answer, so * that a failure is not reported as a success. */ @@ -1419,20 +1422,24 @@ static int instcmd(const char *cmdname, const char *extra) cmdname, extra); return switch_outlets(relay_commands[cmd_index].relay); } - /* one set of commands per outlet; these take no value */ - for (outlet = 1; outlet <= (unsigned int)ups.outlets; outlet++) { - for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(relay_commands); - cmd_index++) { - snprintf(command, sizeof(command), "outlet.%u.%s", - outlet, relay_commands[cmd_index].name); - if (strcasecmp(cmdname, command)) - continue; - - log_instcmd_powerstate( - relay_commands[cmd_index].powerstate, - cmdname, extra); - return switch_outlet(outlet, - relay_commands[cmd_index].relay); + /* one set of commands per outlet; these take no value. Only the + * names that start with "outlet." can match one, so every other + * command skips the walk over the outlet list. */ + if (!strncasecmp(cmdname, "outlet.", 7)) { + for (outlet = 1; outlet <= (unsigned int)ups.outlets; outlet++) { + for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(relay_commands); + cmd_index++) { + snprintf(command, sizeof(command), "outlet.%u.%s", + outlet, relay_commands[cmd_index].name); + if (strcasecmp(cmdname, command)) + continue; + + log_instcmd_powerstate( + relay_commands[cmd_index].powerstate, + cmdname, extra); + return switch_outlet(outlet, + relay_commands[cmd_index].relay); + } } } /* commands that are a single SET frame */ @@ -1450,13 +1457,6 @@ static int instcmd(const char *cmdname, const char *extra) * driver has, which restarts the device timer */ if (!strcasecmp(cmdname, "reset.watchdog")) return reset_watchdog(); - for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(shutdown_commands); - cmd_index++) { - if (!strcasecmp(cmdname, shutdown_commands[cmd_index].name)) { - return shutdown_command(cmdname, extra, - &shutdown_commands[cmd_index]); - } - } upslog_INSTCMD_UNKNOWN(cmdname, extra); return STAT_INSTCMD_UNKNOWN; From 3617c2394fde313a3f0805dea87572c5d24cc55a Mon Sep 17 00:00:00 2001 From: TomZanna Date: Thu, 22 Oct 2026 20:04:56 +0000 Subject: [PATCH 11/13] fix(tripplitesu): re-read watchdog setting each poll cycle The tripplitesu device restarts its watchdog timer on every command, not only on an explicit kick, so the driver's periodic polls usually keep an armed watchdog quiet. However, a watchdog that trips, power cycles the load and comes back disabled could previously go unnoticed, leaving stale values in `ups.watchdog.status`, `ups.watchdog.timeout` and `ups.watchdog.alarm`. - Re-read the watchdog setting on every update cycle while the watchdog is believed to be armed, so a trip is published even when the driver keeps running across the restart (e.g. host not powered by the UPS). - Log repeated failed reads at debug level to avoid flooding the log, and log other failed setting reads instead of silently using defaults. - Document that polls restart the timer as an implementation detail, why an explicit `reset.watchdog` is still recommended, and that `reset.watchdog` fails after a trip until the watchdog is re-armed. - Add corresponding `NEWS.adoc` entries for the upcoming release. Signed-off-by: TomZanna --- NEWS.adoc | 12 ++++++ docs/man/tripplitesu.txt | 42 +++++++++++++++----- drivers/tripplitesu.c | 86 ++++++++++++++++++++++++++++++---------- 3 files changed, 111 insertions(+), 29 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index e342738cdf..be3e1afe46 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -580,6 +580,18 @@ https://github.com/networkupstools/nut/milestone/13 returns to disabled after it trips; `reset.watchdog` now fails when the timeout is 0 instead of reporting a successful kick of a watchdog that is not running. + * The device restarts its watchdog timer on any command it receives, not + only on a kick, so the driver's own polls usually keep an armed watchdog + quiet as well. The documentation still tells the user to kick it + explicitly with `reset.watchdog`, because the polling cadence is an + implementation detail (`pollinterval` can be longer than the watchdog + timeout) and not a guarantee. + * The watchdog setting is re-read on every update cycle while the watchdog + is armed, so a watchdog that trips, power cycles the load and comes back + disabled shows up as `ups.watchdog.status` = `disabled`, with the + timeout and restart-alarm flag that come with it, even when the driver + keeps running across the restart, for example on a machine that is not + powered by the UPS. * The driver shutdown handling (`shutdown.default`, which `upsdrvctl shutdown` runs) always asks the device to start the load again when mains is (re)applied (the auto-reboot setting is turned on), diff --git a/docs/man/tripplitesu.txt b/docs/man/tripplitesu.txt index 10c4379cfa..640564aeec 100644 --- a/docs/man/tripplitesu.txt +++ b/docs/man/tripplitesu.txt @@ -203,6 +203,16 @@ value; enabling/disabling the buzzer for good is done through the the timeout is 0: a disabled watchdog has no timer to restart, and reporting a successful kick would hide a monitoring or configuration error. ++ +The device restarts its watchdog timer whenever it receives a command, not +only on a kick, so the periodic polls the driver sends already keep an armed +watchdog quiet as long as they follow each other within the timeout. Do not +rely on that: kick the watchdog explicitly, for example from a monitoring +script, for as long as it is meant to protect the load. How often the driver +polls is an implementation detail that can change, the common 'pollinterval' +setting could be raised above the watchdog timeout, and a device that answers +slowly or drops commands may not get every poll. A kick every few seconds +costs nothing; an unexpected power cycle of the load does not. VARIABLES --------- @@ -216,7 +226,10 @@ Read/write variables that describe a device setting (*outlet.delay.start*, published, and made settable, only once the device has reported the setting: when it does not answer, the variable is absent (no value is assumed) and a warning is logged. A setting that was read before keeps its last known value -when the device stops answering, again with a warning for every failed read. +when the device stops answering, and the failure is logged instead of falling +back to a default. The watchdog setting is also re-read on every update cycle +while the watchdog is armed, and those repeated failures are logged at debug +level (see *ups.watchdog.status* below). *outlet.count*:: Total number of controllable outlets. @@ -291,16 +304,27 @@ The device stores the timeout and the restart-alarm flag together rather than a status, so the three variables are published from the same setting and a change to one of them writes the whole pair to the device. + -Once armed, the watchdog must be kicked continuously with the -'reset.watchdog' instant command for as long as it is meant to protect the -load, or explicitly disabled with *ups.watchdog.status*. When it trips, the -device power cycles the load and the watchdog returns to 'disabled' until it -is armed again; it does not stay armed by itself. +Once armed, the watchdog has to be kicked for as long as it is meant to +protect the load, or explicitly disabled with *ups.watchdog.status*. The +device restarts its timer on every command it receives, so the polls of a +driver that keeps talking usually keep it quiet, but an explicit kick with the +'reset.watchdog' instant command is strongly recommended: see that command +above, and keep the timeout well above 'pollinterval'. ++ +When the watchdog trips, the device power cycles the load and returns to +'disabled' until it is armed again: it does not stay armed by itself. The +driver re-reads the setting on every update cycle while it believes the +watchdog is armed, so such a change is published in *ups.watchdog.status*, +*ups.watchdog.timeout* and *ups.watchdog.alarm* even when the driver keeps +running across the restart, for example when the computer driving it is not +powered by the UPS. A 'reset.watchdog' issued after the trip fails, because +the watchdog is reported as disabled until it is armed again. + WARNING: While the watchdog is armed, a driver that hangs or a serial link -that breaks is enough to switch the load off and on again, and the load is -powered only as long as the driver keeps polling the device. If you do not -need this protection, disable it with: +that breaks for longer than the watchdog timeout is enough to switch the load +off and on again: the load is protected only as long as the device keeps +hearing from the driver. If you do not need this protection, disable it +with: + upsrw -s ups.watchdog.status=disabled myups + diff --git a/drivers/tripplitesu.c b/drivers/tripplitesu.c index 860d14e526..3fadd0e552 100644 --- a/drivers/tripplitesu.c +++ b/drivers/tripplitesu.c @@ -147,8 +147,13 @@ third device setting of that kind: its timeout and restart-alarm flag are the read/write "ups.watchdog.timeout" and "ups.watchdog.alarm" variables, "ups.watchdog.status" is their enabled/disabled view, and the - "reset.watchdog" command sends the stored setting again. A disabled - watchdog has no timer to kick, so "reset.watchdog" fails then. + "reset.watchdog" command sends the stored setting again. The device + restarts its watchdog timer on any command it receives, so the driver's + own polls already do it; the command is still the explicit kick to rely + on, because the polling cadence is not a guarantee. A disabled watchdog + has no timer to kick, so "reset.watchdog" fails then, and the driver + re-reads the setting on every poll while it is armed, so a watchdog that + tripped (and came back disabled) is published as such. The driver shutdown handler (the "shutdown.default" instant command, run by "upsdrvctl shutdown" and by the "driver.killpower" request) turns the @@ -282,6 +287,7 @@ static unsigned int startdelay = DEFAULT_STARTDELAY_MINUTES; /* delay in minutes static unsigned int watchdog_seconds = 0; /* watchdog timeout currently set on the device */ static unsigned int watchdog_alarm = 2; /* watchdog restart-alarm flag currently set on the device */ static int watchdog_known = 0; /* set once the device reported or accepted its watchdog setting */ +static int watchdog_cycle_ok = 1; /* 1 while the watchdog reads succeed */ static int shutdown_abort_on_error = 0; /* 1 when 'shutdown_mode' asks the driver shutdown handler to stop at the first failure */ /* The two settable transfer voltages: the variable each one is published as * and the field it occupies in the TXV reply. The table order must match the @@ -895,34 +901,59 @@ static void publish_watchdog(void) { * 2 = disabled). The device reports ";", where a timeout of 0 means that the watchdog is disabled. This is * auxiliary information, so a missing or unexpected answer must not mark the - * whole UPS as stale: a warning is logged and the setting the driver has is + * whole UPS as stale: the failure is logged and the setting the driver has is * left in place. Both fields are kept because the device stores them * together: whenever one of them changes, the driver has to send the other - * one along. */ -static int get_watchdog(void) { + * one along. + * 'from_cycle' is set by the caller in upsdrv_updateinfo(), which re-reads + * the setting while the driver believes the watchdog is armed: the device + * disables its watchdog when it trips, and the driver may well keep running + * across the restart of the load (it is often not powered by the UPS). A + * caller that reads the setting once (the startup and the reads that follow + * a variable write) gets a warning for every failure. The update cycle gets + * a warning for the first failure of a series and debug messages for the + * following ones, so that a device that stops answering does not fill the log + * at every poll interval; the first failure of a series is the one after a + * read that worked, which also covers the first cycle after the startup. */ +static int get_watchdog(int from_cycle) { char response[MAX_RESPONSE_LENGTH]; char timeout_value[32], alarm_value[32]; + char msg[MAX_RESPONSE_LENGTH + 96]; + const char *failure = NULL; unsigned int timeout, alarm; if (do_command(POLL, WATCHDOG, "", response, sizeof(response)) <= 0) { - upslogx(LOG_WARNING, "%s: could not read the watchdog " + snprintf(msg, sizeof(msg), "could not read the watchdog " "setting [%s]; keeping the last known value", - __func__, WATCHDOG); - return 0; - } - - if (!get_field(response, 0, timeout_value, sizeof(timeout_value)) + WATCHDOG); + failure = msg; + } else if (!get_field(response, 0, timeout_value, sizeof(timeout_value)) || !str_to_uint_strict(timeout_value, &timeout, 10) || timeout > MAX_WATCHDOG || !get_field(response, 1, alarm_value, sizeof(alarm_value)) || !str_to_uint_strict(alarm_value, &alarm, 10) || (alarm != 1 && alarm != 2)) { - upslogx(LOG_WARNING, "%s: unexpected [%s] response [%s]; " - "keeping the last known value", __func__, WATCHDOG, + snprintf(msg, sizeof(msg), "unexpected [%s] response " + "[%s]; keeping the last known value", WATCHDOG, response); + failure = msg; + } + + if (failure) { + if (!from_cycle || watchdog_cycle_ok) + upslogx(LOG_WARNING, "%s: %s", __func__, failure); + else + upsdebugx(2, "%s: %s", __func__, failure); + + if (from_cycle) + watchdog_cycle_ok = 0; return 0; } + /* a read that worked starts a new series of failures, so the next + * failure is warned about again */ + watchdog_cycle_ok = 1; + watchdog_seconds = timeout; watchdog_alarm = alarm; watchdog_known = 1; @@ -979,10 +1010,13 @@ static int set_watchdog(int enable) { } /* The "reset.watchdog" instant command: send the setting the device last - * reported (or accepted) again, which restarts its timer. The command - * takes no value, so the watchdog it kicks is the one the driver is - * configured with. A watchdog that was never read is left alone, so that a - * kick cannot arm it with assumed values, and a disabled watchdog is not + * reported (or accepted) again, which restarts its timer. The device + * restarts that timer on any command it receives, so the driver's own polls + * already do it in practice; this is the explicit kick the documentation + * tells the user to rely on, because the polling cadence is not a guarantee. + * The command takes no value, so the watchdog it kicks is the one the driver + * is configured with. A watchdog that was never read is left alone, so that + * a kick cannot arm it with assumed values, and a disabled watchdog is not * kicked either: there is no timer to reset, and pretending otherwise would * report a watchdog that does not protect the load. */ static int reset_watchdog(void) { @@ -1587,7 +1621,7 @@ static int setvar(const char *varname, const char *val) return STAT_SET_FAILED; /* read the value back, so that the published variables * reflect what the device actually accepted */ - get_watchdog(); + get_watchdog(0); return STAT_SET_HANDLED; } if (!strcasecmp(varname, "ups.watchdog.alarm")) { @@ -1606,7 +1640,7 @@ static int setvar(const char *varname, const char *val) return STAT_SET_FAILED; /* read the value back, so that the published variables * reflect what the device actually accepted */ - get_watchdog(); + get_watchdog(0); return STAT_SET_HANDLED; } if (!strcasecmp(varname, "input.eco.switchable")) { @@ -1756,7 +1790,7 @@ void upsdrv_initinfo(void) dstate_setflags("ups.start.auto", ST_FLAG_RW | ST_FLAG_STRING); dstate_setaux("ups.start.auto", 3); } - if (get_watchdog()) { + if (get_watchdog(0)) { dstate_setflags("ups.watchdog.status", ST_FLAG_RW | ST_FLAG_STRING); dstate_setaux("ups.watchdog.status", 8); @@ -1981,6 +2015,18 @@ void upsdrv_updateinfo(void) get_outlet_status(); + /* A watchdog that trips power cycles the load and comes back + * disabled. The driver often runs on a machine that is not powered + * by the UPS and survives that restart, so re-read the setting while + * the driver believes the watchdog is armed: the first read after the + * trip publishes 'disabled' (and the timeout and restart-alarm flag + * that came with it), which also stops these reads. Reading the + * setting is a command like any other and therefore restarts the + * device timer as well, but the documentation still tells the user to + * kick the watchdog explicitly instead of relying on the polls. */ + if (watchdog_seconds) + get_watchdog(1); + /* if we are here, status is valid */ status_commit(); dstate_dataok(); From cce98cf7deb49cc354c6a3dbc39243216c59d8fc Mon Sep 17 00:00:00 2001 From: TomZanna Date: Thu, 22 Oct 2026 22:53:50 +0200 Subject: [PATCH 12/13] fix --- drivers/tripplitesu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tripplitesu.c b/drivers/tripplitesu.c index 3fadd0e552..294d56abf8 100644 --- a/drivers/tripplitesu.c +++ b/drivers/tripplitesu.c @@ -920,7 +920,7 @@ static int get_watchdog(int from_cycle) { char timeout_value[32], alarm_value[32]; char msg[MAX_RESPONSE_LENGTH + 96]; const char *failure = NULL; - unsigned int timeout, alarm; + unsigned int timeout = 0, alarm = 1; if (do_command(POLL, WATCHDOG, "", response, sizeof(response)) <= 0) { snprintf(msg, sizeof(msg), "could not read the watchdog " From 43105aaba6b0d0c977544d5b0d377df76556c832 Mon Sep 17 00:00:00 2001 From: TomZanna Date: Sun, 25 Oct 2026 19:33:01 +0000 Subject: [PATCH 13/13] feat(tripplitesu): add ups.start instant command Add a `ups.start` instant command so a device that has been switched off can be started again. It enables the auto-reboot setting and writes the restart delay, which is what makes the firmware start its outputs. The delay is passed as the command value in whole minutes (0-60) and defaults to the `startdelay` setting when no value is given. A device that is already running is not switched off; only the delay is stored, and a scheduled shutdown is left untouched. The command fails when the device does not confirm the auto-reboot setting, before anything that could change the power state is sent. Document the command in the tripplitesu(8) man page, add its entry to `data/cmdvartab`, and note it in NEWS.adoc. Signed-off-by: TomZanna --- NEWS.adoc | 7 +++++ data/cmdvartab | 1 + docs/man/tripplitesu.txt | 44 ++++++++++++++++++++++++-------- docs/nut-names.txt | 2 ++ drivers/tripplitesu.c | 55 +++++++++++++++++++++++++++++++++++++++- 5 files changed, 97 insertions(+), 12 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index be3e1afe46..28d8c227fe 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -610,6 +610,13 @@ https://github.com/networkupstools/nut/milestone/13 protocol and buffer limits, replies must arrive complete and exactly as announced, and semicolon-separated fields are copied with a bound instead of being used in place. + * Added the `ups.start` instant command, which starts the load of a + device that is off: it turns the auto-reboot setting on and writes the + restart delay, which is what makes the device start its outputs. The + delay is passed as the command value, in minutes, and defaults to the + `startdelay` setting (`ups.start 0` starts as soon as the device can); + a device that is already running is not switched off, it only gets the + delay stored. - `usbhid-ups` driver updates: * When reconnecting, report success more visibly (not only in debug), and diff --git a/data/cmdvartab b/data/cmdvartab index de8e135512..b5b55296b0 100644 --- a/data/cmdvartab +++ b/data/cmdvartab @@ -244,6 +244,7 @@ CMDDESC driver.reload-or-exit "Reload running driver configuration from the file CMDDESC load.off "Turn off the load immediately" CMDDESC load.on "Turn on the load immediately" CMDDESC load.cycle "Power-cycle the load (off then back on)" +CMDDESC ups.start "Start the load of a UPS that is off, possibly after a delay" CMDDESC shutdown.default "Run the driver-defined UPS shutdown sequence (as opposed to user-configured 'sdcommands')" CMDDESC shutdown.return "Turn off the load and return when power is back" CMDDESC shutdown.stayoff "Turn off the load and remain off" diff --git a/docs/man/tripplitesu.txt b/docs/man/tripplitesu.txt index 640564aeec..55968d2abb 100644 --- a/docs/man/tripplitesu.txt +++ b/docs/man/tripplitesu.txt @@ -74,11 +74,12 @@ which in turn takes precedence over the built-in default. *startdelay*='minutes':: Set the restart (return) delay, in whole minutes, used by 'shutdown.reboot' -and 'shutdown.reboot.graceful' when their value does not specify one. The -default value is 1 and the accepted range is 0 through 60. The driver sends -this value to the device unchanged: there is no conversion or rounding. A -value of 0 asks for no timed restart, so the load comes back only when mains -is (re)applied. +and 'shutdown.reboot.graceful' when their value does not specify one, and by +'ups.start' when it is called without a value. The default value is 1 and +the accepted range is 0 through 60. The driver sends this value to the +device unchanged: there is no conversion or rounding. A value of 0 asks for +no timed restart, so the load comes back only when mains is (re)applied; +for 'ups.start' it asks the device to start the load as soon as it can. *shutdown_mode*='continue'|'abort':: @@ -113,6 +114,24 @@ are for. 'load.on':: Switch on every controllable outlet immediately. +The following instant command starts a device that was switched off, and +accepts an optional restart delay, in minutes: + +'ups.start' ['minutes']:: + Start the load of a device that is off, without switching off a + device that is running. Unlike 'load.on', which closes the outlet + relays, this starts the device itself: it turns the auto-reboot + setting on and writes the restart delay, which is what makes the + firmware start its outputs. 'minutes' (0 through 60) is how long + the device waits before starting, so 0 starts the load as soon as + the device can; without a value the 'startdelay' setting is used + (1 minute by default). On a device that is already running the + delay is only stored, for a restart the device may do later: + nothing is switched off and a shutdown that is already scheduled + is not cancelled (use 'shutdown.stop' for that). It fails when the + device does not confirm the auto-reboot setting, before anything + that could change the power state is sent. + The following instant commands accept an optional value, passed as the 'value' argument of linkman:upscmd[8]: @@ -141,11 +160,12 @@ The following instant commands accept an optional value, passed as the even if mains returns. Defaults to the 'offdelay' setting. The shutdown delay ('seconds' and 'off') accepts 1 through 3600 seconds and -the restart delay ('on') accepts 0 through 60 whole minutes. Both fields -must be decimal numbers. An empty field, a ';', a '.', a ',', more than one -colon or an out-of-range value makes the command fail before anything is -sent to the UPS. A bare value for 'shutdown.return' or 'shutdown.stayoff' -is a shutdown delay only; those commands clear the restart delay. +the restart delay ('on' and the 'ups.start' value) accepts 0 through 60 whole +minutes. Both fields must be decimal numbers. An empty field, a ';', a +'.', a ',', more than one colon or an out-of-range value makes the command +fail before anything is sent to the UPS. A bare value for 'shutdown.return' +or 'shutdown.stayoff' is a shutdown delay only; those commands clear the +restart delay. The driver shutdown handling is available as the 'shutdown.default' instant command, which is what linkman:upsdrvctl[8] 'shutdown' runs. It takes no @@ -275,7 +295,9 @@ level (see *ups.watchdog.status* below). the shutdown-related instant commands set this value to what they need before running: 'shutdown.return', 'shutdown.reboot' and 'shutdown.reboot.graceful' (and the driver shutdown handler) switch it - to 'yes', while 'shutdown.stayoff' switches it to 'no'. + to 'yes', while 'shutdown.stayoff' switches it to 'no'. 'ups.start' + does the same, as a load that has to start must be allowed to come + back. + WARNING: The device stores this setting, so such a change outlives the shutdown that needed it and also applies to the following sessions. The diff --git a/docs/nut-names.txt b/docs/nut-names.txt index 061acf8337..7fd9143de6 100644 --- a/docs/nut-names.txt +++ b/docs/nut-names.txt @@ -1031,6 +1031,8 @@ Instant commands | load.on | Turn on the load immediately | load.off.delay | Turn off the load possibly after a delay | load.on.delay | Turn on the load possibly after a delay +| ups.start | Start the load of a UPS that is off, + possibly after a delay | shutdown.default | Run default driver-defined (device-specific) routine, primarily intended for emergency poweroff performed as part of FSD handling; diff --git a/drivers/tripplitesu.c b/drivers/tripplitesu.c index 294d56abf8..0664736e57 100644 --- a/drivers/tripplitesu.c +++ b/drivers/tripplitesu.c @@ -113,6 +113,7 @@ shutdown.stop test.battery.start test.battery.stop + ups.start "shutdown.return" and "shutdown.stayoff" accept an optional delay, in seconds, passed as the "value" argument of upscmd; it takes precedence @@ -123,6 +124,8 @@ Without a colon, only the shutdown delay is changed and the restart delay defaults to "startdelay"; with no value at all, the shutdown delay defaults to "offdelay" (60 seconds for the graceful variant) as well. + "ups.start" accepts an optional restart delay, in minutes, and defaults + to the "startdelay" setting like the "on" part of the reboot commands. The "load.off", "load.on", "outlet.n.load.*" and "reset.watchdog" commands do not take a value. @@ -130,7 +133,9 @@ device rejects or does not answer, so a failed command is never applied partially: the driver logs the operation that failed and sends nothing more. "load.off" and "load.on", for instance, leave the outlets they did - not reach untouched. + not reach untouched. "ups.start" writes the restart delay only once the + auto-reboot setting is confirmed, so a device that cannot start the load + is left as it was. "shutdown.return" and "shutdown.stayoff" clear the device return delay (SDR 0), so the load comes back only when mains is (re)applied or not at @@ -1424,6 +1429,47 @@ static int shutdown_command(const char *cmdname, const char *extra, return STAT_INSTCMD_HANDLED; } +/* Start the load of a device that is off, and leave a running one alone. + * The firmware starts its outputs when the restart delay (SDR) is written + * while they are off, so the value is how long it waits before doing so: + * 0 asks for a start as soon as the device can, and without a value the + * 'startdelay' setting is used, like the "on" part of the reboot commands. + * On a device that is already running the delay is only stored, for a + * restart it may do later; nothing is switched off. The auto-reboot + * setting (ARB) is turned on first, so that the load is powered again when + * mains is (re)applied. No shutdown action (SDA) is sent: this command + * does not switch the load off, and it does not cancel a shutdown that is + * already scheduled ("shutdown.stop" does). */ +static int start_ups(const char *cmdname, const char *extra) { + unsigned int delay = startdelay; + + if (extra && *extra) { + if (!parse_uint_range(extra, MIN_STARTDELAY_MINUTES, + MAX_STARTDELAY_MINUTES, &delay)) { + upslogx(LOG_ERR, "instcmd(%s): invalid delay value " + "'%s' (expected %u..%u minutes)", + cmdname, extra, MIN_STARTDELAY_MINUTES, + MAX_STARTDELAY_MINUTES); + return STAT_INSTCMD_FAILED; + } + } + + log_instcmd_powerstate(INSTCMD_POWERSTATE_MAYBE, cmdname, extra); + if (!auto_reboot(1)) { + upslogx(LOG_ERR, "%s: could not set or confirm " + "ups.start.auto, not setting the restart delay", + cmdname); + return STAT_INSTCMD_FAILED; + } + if (!set_start_delay(delay)) { + upslogx(LOG_ERR, "%s: could not set the restart delay, " + "the device may not start", cmdname); + return STAT_INSTCMD_FAILED; + } + + return STAT_INSTCMD_HANDLED; +} + /* Dispatch an instant command. The command names, their frames and how they * affect the power state all come from the tables above, which are also used * to register the commands (see add_instant_commands()), so a command cannot @@ -1487,6 +1533,12 @@ static int instcmd(const char *cmdname, const char *extra) return send_set_or_fail(simple_commands[cmd_index].command, simple_commands[cmd_index].parameters); } + /* start the load of a device that is off: the restart delay (SDR) + * is what makes the firmware start its outputs, so this is not a + * plain SET and the frame is always sent */ + if (!strcasecmp(cmdname, "ups.start")) + return start_ups(cmdname, extra); + /* kick the watchdog: this takes no value, it sends the setting the * driver has, which restarts the device timer */ if (!strcasecmp(cmdname, "reset.watchdog")) @@ -1508,6 +1560,7 @@ static void add_instant_commands(void) dstate_addcmd(simple_commands[cmd_index].name); } dstate_addcmd("reset.watchdog"); + dstate_addcmd("ups.start"); for (cmd_index = 0; cmd_index < SIZEOF_ARRAY(shutdown_commands); cmd_index++) { dstate_addcmd(shutdown_commands[cmd_index].name);