On 28/02/20(Fri) 10:02, Boudewijn Dijkstra wrote: > Op Thu, 27 Feb 2020 17:30:34 +0100 schreef Martin Pieuchot > <m...@openbsd.org>: > > On 27/02/20(Thu) 16:58, boudew...@indes.com wrote: > > > >Synopsis: boolean indicators in sensorsd.conf(5) are too > > > >cumbersome > > > >Category: system > > > >Environment: > > > System : OpenBSD 6.6 > > > Details : OpenBSD 6.6 (GENERIC.MP) #372: Sat Oct 12 10:56:27 > > > MDT 2019 > > > dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP > > > > > > Architecture: OpenBSD.amd64 > > > Machine : amd64 > > > >Description: > > > Some upd(4) devices use -1 for "On" and some use 1. sysctl(8) and > > > sensorsd(8) hide this detail from the user, which makes it difficult > > > to define low and high values in sensorsd.conf(5). > > > > Which device reports "-1" for which usage? Is this from any > > specification or is it a workaround for your device? > > In the misc@ thread I linked it was reported that different devices use > different values. My device happens to report -1 for "On". Given how > sensorsd.conf currently works, it would be most convenient if 0 and 1 were > the only possible values.
You're rephrasing your diff in words. My question is: can there be any drawback to this approach? Did you check the spec? Why is your UPS returning -1 and not 1 in this case? Is this the right place to fix the bug? > > Diff looks fine, although we could do simpler, see below. > > > > Index: upd.c > > =================================================================== > > RCS file: /cvs/src/sys/dev/usb/upd.c,v > > retrieving revision 1.26 > > diff -u -p -r1.26 upd.c > > --- upd.c 8 Apr 2017 02:57:25 -0000 1.26 > > +++ upd.c 27 Feb 2020 16:25:24 -0000 > > @@ -425,7 +425,10 @@ upd_sensor_update(struct upd_softc *sc, > > } > > hdata = hid_get_data(buf, len, &sensor->hitem.loc); > > - sensor->ksensor.value = hdata * adjust; > > + if (sensor->ksensor.type == SENSOR_INDICATOR) > > + sensor->ksensor.value = hdata ? 1 : 0; > > + else > > + sensor->ksensor.value = hdata * adjust; > > sensor->ksensor.status = SENSOR_S_OK; > > sensor->ksensor.flags &= ~SENSOR_FINVALID; > > Your diff is indeed simpler, but I thought it would be cleaner to not assign > 'adjust' when it's not needed. That's an improvement indeed, but it isn't related to the bug you're trying to fix ;)
