nut icon indicating copy to clipboard operation
nut copied to clipboard

[BUG]: APC beeper.enable doesn't work. beeper.on does

Open madmic1314 opened this issue 3 years ago • 6 comments

beeper.disable works beeper.enable reports OK, but doens't turn beeper on beeper.on works

upsc apc:

Init SSL without certificate database
battery.charge: 50
battery.charge.low: 10
battery.charge.warning: 50
battery.date: 2013/08/21
battery.mfr.date: 2013/08/21
battery.runtime: 1249
battery.runtime.low: 120
battery.temperature: 29.2
battery.type: PbAc
battery.voltage: 13.2
battery.voltage.nominal: 12.0
device.mfr: American Power Conversion
device.model: Back-UPS CS 650
device.serial: 4B1334P09099
device.type: ups
driver.name: usbhid-ups
driver.parameter.pollfreq: 30
driver.parameter.pollinterval: 2
driver.parameter.port: auto
driver.parameter.synchronous: no
driver.version: 2.7.4
driver.version.data: APC HID 0.96
driver.version.internal: 0.41
input.sensitivity: medium
input.transfer.high: 266
input.transfer.low: 180
input.voltage: 246.0
input.voltage.nominal: 230
output.frequency: 50.0
output.voltage: 230.0
output.voltage.nominal: 230.0
ups.beeper.status: disabled
ups.delay.shutdown: 20
ups.delay.start: 30
ups.firmware: 817.v9.I
ups.firmware.aux: v9
ups.load: 19.0
ups.mfr: American Power Conversion
ups.mfr.date: 2013/08/21
ups.model: Back-UPS CS 650
ups.productid: 0002
ups.realpower.nominal: 400
ups.serial: 4B1334P09099
ups.status: OL CHRG
ups.test.result: No test initiated
ups.timer.reboot: 0
ups.timer.shutdown: -1
ups.timer.start: 0
ups.vendorid: 051d

upscmd -l apc

Instant commands supported on UPS [apc]:
beeper.disable - Disable the UPS beeper
beeper.enable - Enable the UPS beeper
beeper.mute - Temporarily mute the UPS beeper
beeper.off - Obsolete (use beeper.disable or beeper.mute)
beeper.on - Obsolete (use beeper.enable)
load.off - Turn off the load immediately
load.off.delay - Turn off the load with a delay (seconds)
load.on - Turn on the load immediately
load.on.delay - Turn on 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
shutdown.stop - Stop a shutdown in progress
test.battery.start.deep - Start a deep battery test
test.battery.start.quick - Start a quick battery test
test.battery.stop - Stop the battery test
test.panel.start - Start testing the UPS panel
test.panel.stop - Stop a UPS panel test

Testing

upscmd apc beeper.enable Username (root): Password: OK

upsc apc: [Truncated] ups.beeper.status: disabled

upscmd apc beeper.on Username (root): Password: OK

upsc apc: [Truncated] ups.beeper.status: enabled

madmic1314 avatar Jun 01 '22 09:06 madmic1314

I'm a bit out of context lately, but doesn't an enable/disable command (supportive) UPSes to allow beeping alarms or stay quiet even in trouble, respectively?

jimklimov avatar Jun 02 '22 07:06 jimklimov

Yes it's for when there is an abnormal event (power loss, fault, etc).

The issue I found is that beeper.enable doesn't turn the beeper option on. Beeper.disable does turn it off though

Beeper.on and beeper.off are legacy commands and work. Worried they might be removed in a future update.

madmic1314 avatar Jun 02 '22 08:06 madmic1314

So I stand corrected, thanks. According to source code search like https://github.com/networkupstools/nut/search?q=beeper+enable&type=code there was a renaming (and aliasing) spree, which maybe was not applied coherently to all driver efforts especially as PRs come from older branches and forks over time.

jimklimov avatar Jun 02 '22 08:06 jimklimov

All instant commands are basically handled by the NUT drivers themselves.

Ideally, NUT would have implemented a global alias in the upper layer so beeper.off and beeper.on get automatically translated to beeper.disable and beeper.enable. But because it doesn't, a single typo in the individual driver is enough to cause this bug. I'll try to take a look at the driver.

Update: This is incorrect. While it's true that individual driver still handles their own instant commands without an upper layer, but the HID subsystem does handle this alias.

biergaizi avatar Jun 20 '22 18:06 biergaizi

You're using an old NUT version and driver. Please upgrading to NUT 2.8.0 and retest (please also set the NUT driver's loglevel to LOG_DEBUG). Because of some difficulties on maintaining the project, NUT has not made even a single stable version release in the past 5 years. NUT 2.7.4 was released back in 2016, and NUT 2.8.0 has just been released in 2022. There are literally thousands of changes made during this period. Without testing the latest version, it's difficult to tell the nature of this bug.

This problem should not been able to occur in this manner using the latest version. If there's still a problem, it should at least show itself in a different form - it should either work with both command, or doesn't work with both command.

In the latest version, in usbhid-ups.c, the command beeper.enable is automatically replaced with beeper.on before it's executed. Thus, it should not be possible that one command works while the other one doesn't (unless there's an additional bug somewhere).

As you see:

/* process instant command and take action. */
int instcmd(const char *cmdname, const char *extradata)
{
	hid_info_t	*hidups_item;
	const char	*val;
	double		value;

	if (!strcasecmp(cmdname, "beeper.off")) {
		/* compatibility mode for old command */
		upslogx(LOG_WARNING,
			"The 'beeper.off' command has been "
			"renamed to 'beeper.disable'");
		return instcmd("beeper.disable", NULL);
	}

	if (!strcasecmp(cmdname, "beeper.on")) {
		/* compatibility mode for old command */
		upslogx(LOG_WARNING,
			"The 'beeper.on' command has been "
			"renamed to 'beeper.enable'");
		return instcmd("beeper.enable", NULL);
	}

biergaizi avatar Jun 20 '22 19:06 biergaizi

Also, all UPS drivers have a polling/update rate. A change made to the UPS settings won't be reflected instantaneously until the next update. Your driver has a polling frequency of 30 seconds.

driver.parameter.pollfreq: 30
driver.parameter.pollinterval: 2

If you didn't wait for a minute or so after running beeper.enable before rechecking the UPS status, you would be still seeing the old status. And when you execute beeper.on, 30 seconds have already passed and the driver updates its status, creating an illusion that the former command didn't work but the new command did.

Please also retest to rule out this possibility. It's possible that there was no bug to begin with, if so, this issue can be closed (well, one may argue that the NUT driver should force a status update immediately for better user experience - good point, but it should be handled in a separate bug report).

biergaizi avatar Jun 20 '22 19:06 biergaizi