Arjen de Korte
2007-Jul-29 10:44 UTC
[Nut-upsdev] belkin-hid: UPS.PowerSummary.BelowRemainingCapacityLimit
Peter, Apparently, some Belkin UPSes still have problems with this. When we receive this report through the interrupt pipeline it is ignored (the special case in usbhid-ups.c) and even after that it looks like there are still some problems with it: http://lists.alioth.debian.org/pipermail/nut-upsuser/2007-July/003014.html Wouldn't it be an option to just to ignore this completely and instead use bit 2 from 'UPS.BELKINStatus.BELKINBatteryStatus'? It looks like the battery low status is duplicated there. Another thing that puzzles me, is why we use two ways to set the online status of the UPS (through 'UPS.PowerSummary.ACPresent' and bit 5 of 'UPS.BELKINStatus.BELKINBatteryStatus'). Is this because some models only support the latter and not the first? Could we also use bit 0 from 'UPS.BELKINStatus.BELKINPowerStatus' which indicates directly (according to the comments) the AC status, instead of inverting the discharging bit from 'UPS.BELKINStatus.BELKINBatteryStatus'? Best regards, Arjen
Arjen de Korte wrote:> > Peter, > > Apparently, some Belkin UPSes still have problems with this. When we > receive this report through the interrupt pipeline it is ignored (the > special case in usbhid-ups.c) and even after that it looks like there > are still some problems with it: > http://lists.alioth.debian.org/pipermail/nut-upsuser/2007-July/003014.htmlHi Arjen, here's the information I have. On my own Belkin (which is a 050D/0980, F6C800-UNV), I have observed the bug that is documented in the "special case" in usbhid-ups.c. Namely, report 3d (decimal 61) shows "low battery" when sent via the interrupt pipeline, but not when requested as a feature report. The same is true for the log files that Justin Piszcz sent. Viz: $ grep 3D nut-2.0.4-4-debug.log Report : (8 bytes) => 3D 00 00 00 00 00 00 00 Report : (8 bytes) => 3D 00 00 00 00 00 00 00 Report : (8 bytes) => 3D 00 04 00 00 00 00 00 Report : (8 bytes) => 3D 00 04 00 00 00 00 00 Notification: (2 bytes) => 3D 01 Notification: (2 bytes) => 3D 01 Report : (8 bytes) => 3D 00 04 00 00 00 00 00 Notification: (2 bytes) => 3D 01 Notification: (2 bytes) => 3D 01 $ grep 3d nut-2.0.5-3+b1-debug.log 8d b1 02 85 3c 09 d0 75 08 95 01 15 00 26 ff 00 b1 82 09 d0 81 82 85 3d 09 Report[r]: (2 bytes) => 3d 00 Notification: (2 bytes) => 3d 01 Report[i]: (2 bytes) => 3d 01 Notification: (2 bytes) => 3d 01 Report[i]: (2 bytes) => 3d 01 Report[r]: (2 bytes) => 3d 00 Notification: (2 bytes) => 3d 01 Report[i]: (2 bytes) => 3d 01 Notification: (2 bytes) => 3d 01 Report[i]: (2 bytes) => 3d 01 Report[r]: (2 bytes) => 3d 00 Note that, although the debug format has changed between versions, the substance remains: on the interrupt pipeline, the device sends "3d 01" instead of "3d 00", with the latter being the correct value. The real source of Justin's bug is probably that the logic of the "special case" treatment in usbhid-ups.c is broken. When an interrupt report arrives, it is filed to a report cache (in HIDGetEvents, which is called at usbhid-ups.c:562), and then afterwards the value is ignored in usbhid-ups.c:583. The problem might be that the incorrect value, while not used immediately, is still stored in the cache; so if a feature request for this value is done before the cache expires, the incorrect value might still be used. This would happen intermittently.> Wouldn't it be an option to just to ignore this completely and instead > use bit 2 from 'UPS.BELKINStatus.BELKINBatteryStatus'? It looks like the > battery low status is duplicated there.The simplest solution is to move the "special case" code to another place, ignoring the faulty report completely. Another possible solution is to have a separate cache for interrupt transfers than for regular transfers. Finally, one could ignore the buggy report altogether as you suggest, and use UPS.BELKINStatus.BELKINBatteryStatus instead (which doesn't have an interrupt transfer). None of these solutions are pretty. Before anyone starts complaining about how NUT is such a messy piece of software, I should preemptively emphasize that the real cause for this bug is Belkin's faulty firmware. We can only work around their bugs as best as we can.> Another thing that puzzles me, is why we use two ways to set the online > status of the UPS (through 'UPS.PowerSummary.ACPresent' and bit 5 of > 'UPS.BELKINStatus.BELKINBatteryStatus'). Is this because some models > only support the latter and not the first? Could we also use bit 0 from > 'UPS.BELKINStatus.BELKINPowerStatus' which indicates directly (according > to the comments) the AC status, instead of inverting the discharging bit > from 'UPS.BELKINStatus.BELKINBatteryStatus'?I have no idea why this is duplicated. I figure the reason is indeed that perhaps some models don't have both. Since this does not seem to break anything, I would be reluctant to change it without cause. -- Peter