Index | Thread | Search

From:
Landry Breuil <landry@openbsd.org>
Subject:
Re: upd(4): fix RunTimeToEmpty for EATON upses
To:
tech@openbsd.org
Date:
Fri, 28 Feb 2025 21:35:44 +0100

Download raw body.

Thread
Le Fri, Feb 28, 2025 at 06:48:33PM +0100, Landry Breuil a écrit :
> Le Sun, Feb 09, 2025 at 10:17:56AM +0100, Landry Breuil a écrit :
> > hi,
> > 
> > comeback of a diff i still had around, and has been tested by various
> > ppl on 3 distinct models of eaton upses which report a broken (eg huge) value
> > for RunTimeToEmpty.
> > 
> > i dont remember the exact details of my reading of the spec and the
> > sysutils/nut code, but if we read 4 bytes for RunTimeToEmpty, then the actual
> > value is in the leftmost bytes, so right-shifting / dividing the adjust factor
> > by 256 gives me a value that matches the value reported by nut.
> > 
> > i pondered doing the right-shifting after reading the actual hdata to make sure
> > the raw value read was within 'sensible' bounds, but i wouldn't know how to
> > define those.. advices welcome. "If the value is above 3h then it doesnt make
> > sense and right-shift should apply" ?
> 
> new diff, after discussing it with miod. it makes more sense to do the
> right-shifting on the value given by hid_get_data() instead of the adjust
> factor. i'm also checking that the value wouldnt fit in two bits (eg 'is huge')

fixed diff, the hdata > 0x10000 check was stupid, eg if the remaining secs
drops below 2^16 then the adjusting doesn't happen anymore and we get the
insane value again. i've tested it by unplugging the UPS and letting the
battery drain, when the RemainingCapacity dropped below 4% RunTimeToEmpty went
through the roof.

Feb 28 21:10:29 /bsd: upd0: got 110346, adjusting to 431
Feb 28 21:10:35 /bsd: upd0: got 119818, adjusting to 468
Feb 28 21:10:41 /bsd: upd0: got 68618, adjusting to 268
Feb 28 21:10:47 /bsd: upd0: got 64522, not adjusting
Feb 28 21:10:53 /bsd: upd0: got 61705, not adjusting

the (much better) comment is courtesy of miod@.

oks now really welcome, i'd like to stop beating that dead horse :)

Index: upd.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/upd.c,v
diff -u -r1.33 upd.c
--- upd.c       1 Dec 2024 09:05:05 -0000       1.33
+++ upd.c       28 Feb 2025 20:25:18 -0000
@@ -428,6 +428,20 @@
        }
 
        hdata = hid_get_data(buf, len, &sensor->hitem.loc);
+       switch (HID_GET_USAGE(sensor->hitem.usage)) {
+       case HUB_RUNTIMETO_EMPTY:
+               /*
+                * If the value is reported as a 4-byte item,
+                * assume the lowest 8 bits of the value are
+                * extra, unnecessary, precision, and discard
+                * them.
+                * This happens to match what sysutils/nut
+                * reports.
+                */
+               if (len == 4)
+                       hdata = hdata >> 8;
+               break;
+       }
        if (sensor->ksensor.type == SENSOR_INDICATOR)
                sensor->ksensor.value = hdata ? 1 : 0;
        else