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

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')

hid_get_data_sub() takes care of the endianness, but for extra care i've also
tested the diff on macppc, and with an ups that returns RunTimeToEmpty on two
bytes (which returns a sensible value).

as returned by sysctl hw.sensors:
hw.sensors.upd0.timedelta0=4687.000000 secs (RunTimeToEmpty), OK
hw.sensors.upd1.timedelta0=3368.000000 secs (RunTimeToEmpty), OK

and systat sensors:
upd0.timedelta0      1.302 hr    OK    RunTimeToEmpty
upd1.timedelta0    56.133 min    OK    RunTimeToEmpty

bikeshedding on the comment welcome. i take okays and testing on other UPSses
too, EATON or not.

Index: dev/usb/upd.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/upd.c,v
diff -u -r1.33 upd.c
--- dev/usb/upd.c       1 Dec 2024 09:05:05 -0000       1.33
+++ dev/usb/upd.c       28 Feb 2025 17:35:41 -0000
@@ -428,6 +428,15 @@
        }
 
        hdata = hid_get_data(buf, len, &sensor->hitem.loc);
+       switch (HID_GET_USAGE(sensor->hitem.usage)) {
+       case HUB_RUNTIMETO_EMPTY:
+               /* if the value is on 4 bytes, the relevant bytes
+                 are the middle one. as a bonus, matches what nut says.
+                */
+               if (len == 4 && hdata > 0x10000)
+                       hdata = hdata >> 8;
+               break;
+       }
        if (sensor->ksensor.type == SENSOR_INDICATOR)
                sensor->ksensor.value = hdata ? 1 : 0;
        else