basic infrastructure for simple sensors

[avg]

Summary

This is based on rS172631 and rS172632 with all relevant attributions.
Plus some small changes by me.

This review request is only to facilitate the discussion.
There are no commit plans at the moment.

Test Plan

I am using this locally with it(4) driver and modified jedec_ts.

[imp]

Thanks for bringing this in. I've done a super-duper quick pass and there's a number of items that I don't like which I'd like to offer constructive criticism on, but don't yet have a good way to do that, so I'll defer comment on them until I can.

There's generally a poor separation between what and why in the sensor lists (some things are special temperature, but others are just integer sensors, for example). I'll see if I can come up with a better way to distinguish these two types of things.

Still not sure sysctl is a good way to expose this data, but it is an easy way, though it lacks many features that would help it scale and react more quickly than the current polling interval will allow. That's the crux of what people objected to before.

So expect more comments to help refine things w/o creating undue burden on the drivers themselves. That model is somewhat OK, though not entirely perfect, as it seems to lack a notification mechanism for changes that would help the polling problem. And a number of sensors make themselves valid / invalid from time to time in a way that seems racy on first blush.

Again, this is all preliminary as a way to help improve the situation on this code review which isn't in the 'commit path' to help iron out contentious issues (or potentially contentious ones) in a smaller audience rather than an impossibly large one like the initial attempt suffered from so many years ago.

usr.sbin/sensorsd/sensorsd.c

169

The crux of the argument against this before was that we don't want a default daemon that wakes up every second and does a lot of work to determine that nothing really had changed. Having a stream of changes (perhaps in addition to sysctl presentation) seems a better fit, either ala the routing socket model where we announce the changes when they happen, or ala the kqueue model that just says that something changed with perhaps a hint about what changed. sensord still suffers from this, but I can spare some time to code up something that's more efficient.

[avg]

Some of my thoughts.

I don't see much value in sensord and I personally don't use it, so it can be excluded from the initial version and added later if needed.

I also don't like the unconditional periodic polling of all sensors that's done in the kernel. I think that we should read sensors only if some subscriber wants their values or if there is some notification from the hardware (where it is supported and configured).

I like the sysctl interface, but it certainly does not have to be the only one. I think that we could have some object + KPI, let's call it a sensor hub, with which all sensors are registered and which provides methods for consumers to discover and query sensors (and to register for notifications). The the sysctl interface could be just one of consumers on top of the sensor hub. Or, better to say, it would be just one of proxies between real consumers (like end-users) and the sensor hub.

[imp]

The other problem with this API is that it lacks the ability to communicate how often sensor data is available, how old the latest sensor data is, what the range of the sensor's operation is, and what accuracy the data is reported as (which may differ than the precision it's reported as). Other systems have these features, and it wouldn't take a lot to add them, though it would be incompatible with OpenBSD's ABI I suppose.

[wblock]

Remember that .Dd needs to updated on commit.

lib/libc/gen/sysctl.3

313

The third level comprises an array of

316

Does "may" mean "are allowed to" or just "might" here?

318

Use the serial comma: s/fourth/fourth,/

share/man/man4/it.4

78

No need for a possessive here: s/sensors'/sensor/

share/man/man5/rc.conf.5

321

s/sensors/sensor/

share/man/man9/sensor_attach.9

129

Avoid contractions: s/don't/do not/
Also, avoid possessives: s/its/the/

usr.bin/systat/systat.1

312

Use the lower window to display the current values of hardware sensors
in a format similar to
.Xr sysctl 8 .

usr.sbin/sensorsd/sensorsd.8

33

Use the serial comma: s/voltage/voltage,/

51

This is a long sentence that is not very clear:

High and low limits can be set for every sensor, regardless of whether the
sensor automatically provides a current state.
This allows a local notion of sensor status to be computed by
indicating whether the sensor is within or is exceeding the limits.

The second sentence is still somewhat unclear, the meaning of "local notion" particularly.

53

"may be" implies permission, "are" is probably better here.

59

s/ as follows//

65

s/will run/runs/

89

an additional delay in status reporting and command execution.
This can result in sensor state not being reported until one or two minutes
after starting
.Nm .

usr.sbin/sensorsd/sensorsd.conf.5

35

which can specify high and low limits``` (no trailing comma)

37

If the limits are crossed or driver status changes,

39

Break sentence.

alert functionality is triggered.
If a command was specified, it is executed.

54

This whole sentence is unclear. "An entry" meaning an entry in the file? "Not found" meaning not found in the actual detected hardware? How is "temp" parsed out of "hw.sensors.lm0.temp1", or is it?

60

These are clearer when not in the form of a sentence, but just the item description. For example:

Command to be executed on state change.

62

As above:

Upper limit.

64

As above:

Lower limit.

70

Better in shorter sentences:

Temperature sensor values can be given in degrees Celsius or Fahrenheit.
Voltage sensor values are in volts.
Fan speed sensor values take a

96

As above:

xname of the device the sensor sits on

("sits on"? why not "is attached to"?)

98

sensor type

100

sensor number

102

current value

104

low limit

106

high limit

138

Avoid possessives: s/its//

143

Avoid semicolons as sentence splices.

will be executed, with the sensor number passed to it.
However,

176

Avoid semicolons as sentence splices.

Alert functionality is triggered every time there is a change in sensor state.

177

For example, when

181

s/whatever it is/the present value/

182

This must be kept in mind when using commands