> Date: Tue, 25 Aug 2020 21:19:19 +0200
> From: Paul de Weerd <we...@weirdnet.nl>
>
> Hi all,
>
> I've dug out my stash of weird usb devices and found another sensor (a
> uthum(4), with only temperature support). I have a few other sensors
> in live machines too (acpitz(4), cpu(4), admtemp(4), it(4), maybe some
> more) that I could look into.
>
> Is there any interest in adding support for setting the tv member for
> non-time sensitive sensors? Or should I drop this quest?
I don't understand the point. None of the sensor drivers set that
member except the timedelta sensors. I don't think adding code to do
so to all sensor drivers makes sense.
> uhidev6 at uhub3 port 1 configuration 1 interface 0 "Ten X Technology, Inc.
> PCsensor Temper" rev 1.10/1.50 addr 10
> uhidev6: iclass 3/1
> uthum0 at uhidev6
> uhidev7 at uhub3 port 1 configuration 1 interface 1 "Ten X Technology, Inc.
> PCsensor Temper" rev 1.10/1.50 addr 10
> uhidev7: iclass 3/0
> uthum1 at uhidev7
> uthum1: type ds75/12bit (temperature)
>
> Paul 'WEiRD' de Weerd
>
> (following diff has been compile tested only so far)
>
> Index: uthum.c
> ===================================================================
> RCS file: /home/OpenBSD/cvs/src/sys/dev/usb/uthum.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 uthum.c
> --- uthum.c 14 Feb 2020 14:55:30 -0000 1.34
> +++ uthum.c 25 Aug 2020 19:15:45 -0000
> @@ -662,6 +662,7 @@ uthum_refresh_temperhum(struct uthum_sof
> sc->sc_sensor[UTHUM_TEMPERHUM_TEMP].sensor.flags &= ~SENSOR_FINVALID;
> sc->sc_sensor[UTHUM_TEMPERHUM_HUM].sensor.value = rh;
> sc->sc_sensor[UTHUM_TEMPERHUM_HUM].sensor.flags &= ~SENSOR_FINVALID;
> + microtime(&sc->sc_sensor[UTHUM_TEMPERHUM_HUM].sensor.tv);
> }
>
> void
> @@ -699,6 +700,7 @@ uthum_refresh_temper(struct uthum_softc
>
> sc->sc_sensor[sensor].sensor.value = (temp * 10000) + 273150000;
> sc->sc_sensor[sensor].sensor.flags &= ~SENSOR_FINVALID;
> + microtime(&sc->sc_sensor[sensor].sensor.tv);
> }
>
> void
> @@ -733,6 +735,7 @@ uthum_refresh_temperntc(struct uthum_sof
> temp += sc->sc_sensor[sensor].cal_offset * 10000;
> sc->sc_sensor[sensor].sensor.value = temp;
> sc->sc_sensor[sensor].sensor.flags &= ~SENSOR_FINVALID;
> + microtime(&sc->sc_sensor[sensor].sensor.tv);
> }
> }
>
>
> On Sat, Aug 15, 2020 at 10:08:56AM +0200, Paul de Weerd wrote:
> | Thanks Hiltjo, that made me look at ugold.c.
> |
> | With the below diff, my simple test program shows a value for the tv
> | struct member.
> |
> | [weerd@pom] $ ./sensor_last_change
> | 1597477798.557355: 28.72
> |
> | However, given what Hiltjo showed, the tv member seems to only be used
> | for time-sensors, so it may be completely on purpose that other
> | sensors don't expose this. My rationale for inspecting tv is to
> | ensure that I only take 'unique' samples (so when tv of the new sample
> | differs from the previous one - while the actual value may remain the
> | same).
> |
> | Is this worth adding?
> |
> | Cheers,
> |
> | Paul 'WEiRD' de Weerd
> |
> | Index: ugold.c
> | ===================================================================
> | RCS file: /home/OpenBSD/cvs/src/sys/dev/usb/ugold.c,v
> | retrieving revision 1.14
> | diff -u -p -r1.14 ugold.c
> | --- ugold.c 5 Oct 2017 17:29:00 -0000 1.14
> | +++ ugold.c 15 Aug 2020 07:32:42 -0000
> | @@ -270,11 +270,13 @@ ugold_ds75_intr(struct uhidev *addr, voi
> | case 4:
> | temp = ugold_ds75_temp(buf[4], buf[5]);
> | sc->sc_sensor[UGOLD_OUTER].value = temp;
> | + microtime(&sc->sc_sensor[UGOLD_OUTER].tv);
> | sc->sc_sensor[UGOLD_OUTER].flags &= ~SENSOR_FINVALID;
> | /* FALLTHROUGH */
> | case 2:
> | temp = ugold_ds75_temp(buf[2], buf[3]);
> | sc->sc_sensor[UGOLD_INNER].value = temp;
> | + microtime(&sc->sc_sensor[UGOLD_INNER].tv);
> | sc->sc_sensor[UGOLD_INNER].flags &= ~SENSOR_FINVALID;
> | break;
> | default:
> | @@ -394,9 +396,11 @@ ugold_si700x_intr(struct uhidev *addr, v
> | sc->sc_hdev.sc_dev.dv_xname, buf[1]);
> | temp = ugold_si700x_temp(sc->sc_type, buf[2], buf[3]);
> | sc->sc_sensor[UGOLD_INNER].value = (temp * 1000) + 273150000;
> | + microtime(&sc->sc_sensor[UGOLD_INNER].tv);
> | sc->sc_sensor[UGOLD_INNER].flags &= ~SENSOR_FINVALID;
> | rhum = ugold_si700x_rhum(sc->sc_type, buf[4], buf[5], temp);
> | sc->sc_sensor[UGOLD_HUM].value = rhum;
> | + microtime(&sc->sc_sensor[UGOLD_HUM].tv);
> | sc->sc_sensor[UGOLD_HUM].flags &= ~SENSOR_FINVALID;
> | break;
> | default:
> |
> | On Fri, Aug 14, 2020 at 02:50:39PM +0200, Hiltjo Posthuma wrote:
> | | On Fri, Aug 14, 2020 at 01:46:57PM +0200, Paul de Weerd wrote:
> | | > Hi all,
> | | >
> | | > I'm trying to read temperature sensor values from my ugold(4) device.
> | | > Seems to work alright (I get the same temperature reading as sysctl(8)
> | | > returns for the sensor), but the 'sensor value last change time'
> | | > doesn't seem to be updated.
> | | >
> | | > [weerd@pom] $ cat sensor_last_change.c
> | | > #include <stdio.h>
> | | > #include <sys/time.h>
> | | > #include <sys/sensors.h>
> | | > #include <sys/sysctl.h>
> | | >
> | | > int
> | | > main()
> | | > {
> | | > int mib[5];
> | | > size_t sensorlen;
> | | > struct sensor sensor;
> | | >
> | | > mib[0] = CTL_HW;
> | | > mib[1] = HW_SENSORS;
> | | > mib[2] = 3; /* ugold0 on my machine */
> | | > mib[3] = SENSOR_TEMP;
> | | > mib[4] = 0;
> | | >
> | | > sensorlen = sizeof(sensor);
> | | > sysctl(mib, 5, &sensor, &sensorlen, NULL, 0);
> | | > printf("%lld.%06ld: %.2f\n",
> | | > sensor.tv.tv_sec,
> | | > sensor.tv.tv_usec,
> | | > ((sensor.value-273150000)/1000000.0));
> | | >
> | | > return 0;
> | | > }
> | | > [weerd@pom] $ make sensor_last_change
> | | > cc -O2 -pipe -MD -MP -o sensor_last_change sensor_last_change.c
> | | > [weerd@pom] $ ./sensor_last_change
> | | > 0.000000: 32.32
> | | > [weerd@pom] $ sysctl -n hw.sensors.ugold0.temp0
> | | > 32.32 degC (inner)
> | | >
> | | > The 'tv' member of struct sensor seems to always be 0.0. Am I doing
> | | > something wrong?
> | | >
> | | > Cluesticks very welcome...
> | | >
> | | > Thanks,
> | | >
> | | > Paul
> | | >
> | | > --
> | | > >++++++++[<++++++++++>-]<+++++++.>+++[<------>-]<.>+++[<+
> | | > +++++++++++>-]<.>++[<------------>-]<+.--------------.[-]
> | | > http://www.weirdnet.nl/
> | | >
> | |
> | | Hi,
> | |
> | | I don't think you're doing anything wrong, but I don't think its
> supported by
> | | the device and it's simply not set and so stays zero.
> | |
> | | A quick grep shows some files which do use it:
> | |
> | | /usr/src/sys $ grep -R sc_sensor\.tv .
> | | ./dev/gpio/gpiodcf.c: microtime(&sc->sc_sensor.tv);
> | | ./dev/pv/hypervic.c: microtime(&sc->sc_sensor.tv);
> | | ./dev/pv/vmt.c: struct timeval *guest = &sc->sc_sensor.tv;
> | | ./dev/pv/vmmci.c: struct timeval *guest =
> &sc->sc_sensor.tv;
> | | ./dev/usb/udcf.c: microtime(&sc->sc_sensor.tv);
> | |
> | | I have a ugold(4) device and will gladly test any improvements/patches.
> | |
> | | --
> | | Kind regards,
> | | Hiltjo
> |
> | --
> | >++++++++[<++++++++++>-]<+++++++.>+++[<------>-]<.>+++[<+
> | +++++++++++>-]<.>++[<------------>-]<+.--------------.[-]
> | http://www.weirdnet.nl/
> |
>
> --
> >++++++++[<++++++++++>-]<+++++++.>+++[<------>-]<.>+++[<+
> +++++++++++>-]<.>++[<------------>-]<+.--------------.[-]
> http://www.weirdnet.nl/
>
>
