Skip to content

dstate/dummy-ups: ALARM status does not clear after dirty entrance into state #2928

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.

Sign up for GitHub

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

Jump to bottom
Open
desertwitch opened this issue Apr 29, 2025 · 1 comment · May be fixed by #2934
Open

dstate/dummy-ups: ALARM status does not clear after dirty entrance into state #2928

desertwitch opened this issue Apr 29, 2025 · 1 comment · May be fixed by #2934
Labels
bug impacts-release-2.8.3 Issues reported against NUT release 2.8.3 (maybe vanilla or with minor packaging tweaks)
Milestone
2.8.4

Comments

@desertwitch
Copy link
Contributor

Stumbled upon this with dummy-ups and the 2.8.3 release version:

Setting ups.status to OL ALARM correctly registers the alarm state. (Note: I did not expose ups.alarm hence as N/A)
Apr 29 14:44:59 TowerOLD upsmon[3362741]: UPS ups@127.0.0.1: one or more active alarms: [[N/A]]
Setting ups.status back to OL does not clear the alarm state, upsc (still) reports ups.status: ALARM OL
Setting ups.status to OL TRIM in an attempt to clear the alarm state, upsc now reports ups.status: ALARM OL TRIM
Apr 29 14:48:49 TowerOLD upsmon[3362741]: UPS ups@127.0.0.1: trimming incoming voltage (nothing else)
Setting ups.status back to OL in another attempt to clear both states, upsc now reports ups.status: ALARM OL
Apr 29 14:52:34 TowerOLD upsmon[3362741]: UPS ups@127.0.0.1: no longer trimming incoming voltage (nothing else)

I'm not sure if this is limited to dummy-ups, I haven't had time to investigate further so far.
@desertwitch
Copy link
Contributor Author

desertwitch commented Apr 29, 2025 •
edited
Loading

On first (quick) glance this seems to result from the alarm getting set in the ups.status directly and not via the "proper" dstate alarm functions. There is fallback dstate code handling this "dirty" entering into an alarm state, but seemingly not clearing the alarm again after disappearance of the status (if it disappears without calling the proper dstate alarm functions to clear):

nut/drivers/dstate.c

Lines 1797 to 1809 in 72fbc58

if (!strcasecmp(token, "ALARM")) {
/* Drivers really should not raise alarms this way,
* but for the sake of third-party forks, we handle
* the possibility...
*/
upsdebugx(2, "%s: (almost) ignoring ALARM set as a status", __func__);
if (!alarm_status && !alarm_active && strlen(alarm_buf) == 0) {
alarm_init(); /* no-op currently, but better be proper about it */
alarm_set("[N/A]");
}
alarm_status++;
return 0;
}

nut/drivers/dstate.c

Lines 1872 to 1881 in 72fbc58

if (!alarm_active && alarm_status && !strcmp(alarm_buf, "[N/A]")) {
upsdebugx(2, "%s: Assume sloppy driver coding that ignored alarm methods and used status_set(\"ALARM\") instead: commit the injected N/A ups.alarm value", __func__);
alarm_commit();
}
if (alarm_active) {
dstate_setinfo("ups.status", "ALARM %s", status_buf);
} else {
dstate_setinfo("ups.status", "%s", status_buf);
}

After setting ALARM in the status:

Apr 29 15:10:01 TowerOLD dummy-ups[3388135]: [D3:3388135] parse_data_file: variable "ups.status" with 3 args
Apr 29 15:10:01 TowerOLD dummy-ups[3388135]: [D2:3388135] status_set_callback: (almost) ignoring ALARM set as a status
Apr 29 15:10:01 TowerOLD dummy-ups[3388135]: [D2:3388135] str_add_unique_token: skip token 'ALARM': due to callback_always()
Apr 29 15:10:01 TowerOLD dummy-ups[3388135]: [D2:3388135] status_commit: Assume sloppy driver coding that ignored alarm methods and used status_set("ALARM") instead: commit the injected N/A ups.alarm value
Apr 29 15:10:01 TowerOLD dummy-ups[3388135]: [D5:3388135] send_to_all: SETINFO ups.alarm "[N/A]"
Apr 29 15:10:01 TowerOLD dummy-ups[3388135]: [D6:3388135] send_to_all: write 26 bytes to socket 5 succeeded (ret=26): SETINFO ups.alarm "[N/A]"
Apr 29 15:10:01 TowerOLD dummy-ups[3388135]: [D5:3388135] send_to_all: SETINFO ups.status "ALARM OL"
Apr 29 15:10:01 TowerOLD dummy-ups[3388135]: [D6:3388135] send_to_all: write 30 bytes to socket 5 succeeded (ret=30): SETINFO ups.status "ALARM OL"

After clearing ALARM from the status:

Apr 29 15:10:21 TowerOLD dummy-ups[3388135]: [D3:3388135] parse_data_file: variable "ups.status" with 2 args
Apr 29 15:10:21 TowerOLD dummy-ups[3388135]: [D3:3388135] parse_data_file: variable "ups.test.interval" with 2 args
Apr 29 15:10:21 TowerOLD dummy-ups[3388135]: [D2:3388135] entering setvar(ups.test.interval, 2592000)
Apr 29 15:10:21 TowerOLD dummy-ups[3388135]: [D3:3388135] parse_data_file: added "ups.test.interval" with value "2592000"

From a "good to be safe" perspective respecting the sloppy driver coding makes sense, as described here:

nut/drivers/dstate.c

Lines 1851 to 1870 in 72fbc58

/* NOTE: Not sure if any clients rely on ALARM being first if raised,
* but note that if someone also uses status_set("ALARM") we can end
* up with a "[N/A]" alarm value injected (if no other alarm was set)
* and only add the token here so it remains first.
*
* NOTE: alarm_commit() must be executed before status_commit() for
* this report to work!
* * If a driver only called status_set("ALARM") and did not bother
* with alarm_commit(), the "ups.alarm" value queries would have
* returned NULL if not for the "sloppy driver" fix below, although
* the "ups.status" value would report an ALARM token.
* * If a driver properly used alarm_init() and alarm_set(), but then
* called status_commit() before alarm_commit(), the "ups.status"
* value would not know to report an ALARM token, as before.
* * If a driver used both status_set("ALARM") and alarm_set() later,
* the injected "[N/A]" value of the alarm (if that's its complete
* value) would be overwritten by the explicitly assigned contents,
* and an explicit alarm_commit() would be required for proper
* reporting from a non-sloppy driver.
*/

Just wondering if we should also clear such a sloppy status by calling the proper dstate alarm functions (to clear) after it disappears without proper cleanup, for sake of completeness. Maybe with a timeout or waiting X status cycles for proper cleanup to occur after the status disappears, if not - clear the alarm state?

As for dummy-ups, it probably also makes sense to catch ALARM (entering/clearing) and raise the proper dstate alarm functions from the driver-level instead of relying on this safeguard functionality in dstate?

@desertwitch desertwitch changed the title dummy-ups: ALARM status does not clear after status change(s) dstate/dummy-ups: ALARM status does not clear after dirty entrance into state Apr 29, 2025
@jimklimov jimklimov added bug impacts-release-2.8.3 Issues reported against NUT release 2.8.3 (maybe vanilla or with minor packaging tweaks) labels Apr 29, 2025
@jimklimov jimklimov added this to the 2.8.4 milestone Apr 29, 2025
This was referenced Apr 30, 2025
Closed
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug impacts-release-2.8.3 Issues reported against NUT release 2.8.3 (maybe vanilla or with minor packaging tweaks)
Projects
None yet
Milestone
2.8.4
2 participants
@jimklimov @desertwitch