Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

USB HID UPS / APC HID UPS shutdown command attempted order #2670

Open
Dummy0815 opened this issue Nov 6, 2024 · 5 comments · May be fixed by #2686
Open

USB HID UPS / APC HID UPS shutdown command attempted order #2670

Dummy0815 opened this issue Nov 6, 2024 · 5 comments · May be fixed by #2686
Labels
APC impacts-release-2.7.3-or-older Issues reported against NUT release 2.7.3 or older, packaged or custom builds of code from that era question Shutdowns and overrides and battery level triggers Issues and PRs about system shutdown, especially if battery charge/runtime remaining is involved USB
Milestone

Comments

@Dummy0815
Copy link

Hi we are using an APC USB UPS with an Try Contact I/O card (AP9613) with an external key switch, to switch the UPS and therefor the connected systems with this key ON/OFF.
The APC is "simulating" an LOW BAT with key position OFF and triggers a UPS ON with the other position.
Everything works fine with the 'old' generation of APC (SUA750RMI1U, SUA750I, ..) but with the newer generation (SMT1500RMI2U) we have the issue, that if key OFF (low bat) triggers the system shut down, but the last command "upsdrvctl shutdown -> driver -k" sends an "shutdown.reboot" to the UPS. This causes the UPS to turn the outlets delayed off (which is what we want), but issuing a restart, which causes the UPS to turn on the outlets again (as main power is on the the whole time).
Reason seems to be that the 'old' APC's support the shutdown commands:
"shutdown.reboot", "shutdown.return", "shutdown.stayoff" (and load.off.delay)
While the newer ones only support:
"shutdown.reboot" (and load.off.delay)

So according the NUT documentation the driver -k tries to issue in a fixed order "shutdown.return", "shutdown.reboot", "load.off.delay".
So old APC is shutdown with "shutdown.return" which is Ok as the UPS stays down as the main power doesn't change.
While the newer, as they down support the first command, are doing an "shutdown.reboot" causing the issue we have.

So my question is there any parameter which influences the attempted command sequence? So that we could configure try: 'load.off.delay' first than, "shutdown.return", than "shutdown.reboot"?
As an workaround is there any chance to send a 'load.off.delay' in the final system shutdown stage instead of "upsdrvctl shutdown"?

For reference the output of an old vs an newer APC:
########################
upsc shups
..
device.mfr: American Power Conversion
device.model: Smart-UPS 750
driver.name: usbhid-ups
driver.version.data: APC HID 0.95
driver.version.internal: 0.38

upscmd -l shups
..
load.off.delay - Turn off the load with a delay (seconds)
shutdown.reboot - Shut down the load briefly while rebooting the UPS
shutdown.return - Turn off the load and return when power is back
shutdown.stayoff - Turn off the load and remain off

########################
upsc shups
..
device.mfr: American Power Conversion
device.model: Smart-UPS 1500
driver.name: usbhid-ups
driver.version: 2.7.2
driver.version.data: APC HID 0.95

upscmd -l shups
..
load.off.delay - Turn off the load with a delay (seconds)
shutdown.reboot - Shut down the load briefly while rebooting the UPS

@jimklimov jimklimov added question APC impacts-release-2.7.3-or-older Issues reported against NUT release 2.7.3 or older, packaged or custom builds of code from that era Shutdowns and overrides and battery level triggers Issues and PRs about system shutdown, especially if battery charge/runtime remaining is involved labels Nov 6, 2024
@jimklimov
Copy link
Member

Hello, thanks for the interesting question.

First of all, please note that you seem to be running NUT v2.7.2 which is over 10 years old and would not be directly updated, at least by core project. You might build your own if required, though (e.g. to apply a custom patch). Current NUT codebase aims to be buildable wherever it worked before, so https://github.com/networkupstools/nut/wiki/Building-NUT-for-in%E2%80%90place-upgrades-or-non%E2%80%90disruptive-tests might help you follow that path.

Looking at even current code, the command sequence in

nut/drivers/usbhid-ups.c

Lines 969 to 993 in d244d73

void upsdrv_shutdown(void)
{
upsdebugx(1, "upsdrv_shutdown...");
/* Try to shutdown with delay */
if (instcmd("shutdown.return", NULL) == STAT_INSTCMD_HANDLED) {
/* Shutdown successful */
return;
}
/* If the above doesn't work, try shutdown.reboot */
if (instcmd("shutdown.reboot", NULL) == STAT_INSTCMD_HANDLED) {
/* Shutdown successful */
return;
}
/* If the above doesn't work, try load.off.delay */
if (instcmd("load.off.delay", NULL) == STAT_INSTCMD_HANDLED) {
/* Shutdown successful */
return;
}
upslogx(LOG_ERR, "Shutdown failed!");
set_exit_flag(-1);
}
is hard-coded. I suppose a new driver configuration variable to list the suitable shutdown commands in order can make sense here.

Note that normally NUT daemons (drivers, upsmon, upsd...) are killed as any other services in the system going down, before it gets to the part where it calls upsdrvctl shutdown -> driver -k, so you would not be able to simply upscmd the needed operation. Generally it depends on your OS and its NUT integration, in some cases you might be able to modify the shutdown sequence so that NUT daemons survive until power-cycling. In others, the OS kills off all remaining processes before the endgame.

@jimklimov jimklimov added this to the 2.8.3 milestone Nov 6, 2024
@jimklimov jimklimov added the USB label Nov 6, 2024
@jimklimov
Copy link
Member

Tinkering about this...

jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
…er), by default chain the originally used sequence in upsdrv_shutdown() [networkupstools#2670]

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
…XIT_FAILURE and EF_EXIT_SUCCESS for set_exit_flag(int) methods [networkupstools#2670]

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
…rv_shutdown flag to exit or not after upsdrv_shutdown() and individual power-state related INSTCMDs [networkupstools#2670]

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
…t and upsdrv_shutdown_default() shared method [networkupstools#2670]

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
…wn_sdcommands_or_default() less ambiguously [networkupstools#2670]

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
…default upsdrv_shutdown() as the implementation of forceshutdown() and "driver.killpower" INSTCMD (e.g. via `drivername -k`) [networkupstools#2670]

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
…() so drivers might implement their own "shutdown.default" at will [networkupstools#2670]

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
…tinfo() where we register INSTCMD handlers [networkupstools#2670]

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
…fallback() so drivers might implement their own "shutdown.default" at will [networkupstools#2670]"

This reverts commit 25250b0:
too messy in logs where driver.instcmd() think they are last in stack.

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this issue Nov 19, 2024
…" as upsdrv_shutdown() in main loop too [networkupstools#2670]

Signed-off-by: Jim Klimov <[email protected]>
@jimklimov
Copy link
Member

jimklimov commented Nov 19, 2024

@Dummy0815 : cheers, I've posted a PR with prospective change for this feature. Testing would be very welcome; see https://github.com/networkupstools/nut/wiki/Building-NUT-for-in%E2%80%90place-upgrades-or-non%E2%80%90disruptive-tests about making a custom build of NUT for experiments (can be run straight from build workspace).

@jimklimov
Copy link
Member

@Dummy0815 : gentle bump, do you intend to try this solution?

@Dummy0815
Copy link
Author

@jimklimov: Hi sorry, I've just returned from sick leave and still have to catch up.
Thank you very much for your efforts and I will try this as soon as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APC impacts-release-2.7.3-or-older Issues reported against NUT release 2.7.3 or older, packaged or custom builds of code from that era question Shutdowns and overrides and battery level triggers Issues and PRs about system shutdown, especially if battery charge/runtime remaining is involved USB
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants