[lm-sensors] Question about the new sysfs alarm interface

[khali@linux-fr.org (Jean Delvare)]

Hi Hans,

> I've finally made the time to convert the abituguru driver to the new
> alarm sysfs interface however the uguru has 2 alarms for each voltage
> input, a volt low and a volt low alarm, currently I create the following
> for these:
> in0_alarm_low
> in0_alarm_high

Not correct, these should be in0_min_alarm and in0_max_alarm,
respectively, according to the proposal we discussed some months ago.

Rudolf Marek is working on a documentation update covering this new
interface, so hopefully it'll be official soon:
http://lists.lm-sensors.org/pipermail/lm-sensors/2006-May/016131.html

> I was thinking that for compatibility with apps which just expect an
> alarm file as documented in the new standard to add:
> in0_alarm
> 
> Which will contain an alarm if either of the 2 above alarms happens. I
> personally find this a good idea of mine, but i just wanted to check to
> make sure.

No, I don't think this is a good idea. No software application actually
expects this file, as it is part of an interface specification which
was just defined, isn't even properly documented yet, and isn't
implemented by any driver.

I do agree that emulating a single alarm flag for channels with more
than one alarm bit makes sense, as some applications may not want to
bother with the exact alarm cause. However, implementing this emulation
in every driver will be a lot of work and will also increase the
drivers size. Instead, this would be the job of libsensors (current or
future) to offer this facility to applications, so that we only have to
write the code once for all drivers and all applications.

> Also the uguru has the ability do disable alarms on a certain input.
> This way you can silence a certain input and make it not report any
> alarms which might upset monitoring scripts etc.
> 
> I currently have added the following enntries for these:
> in0_alarm_low_enable
> in0_alarm_high_enable
> temp1_alarm_enable
> fan1_alarm_enable

Looks fine, except that alarm_{low,high} should be {min,max}_alarm.

This will need to be documented too (on top of Rudolf Marek's pending
change.)

> And I could, but I'm not so sure about that one, seems overkill add a:
> in0_alarm_enable
> which then can be used to disable/enable both alarms at once and will
> show alarms enabled if one of or both the alarms are enabled.

No, this would make some sense on write, but becomes very confusing on
read. And here again this would be libsensors' job to offer shortcuts
if they seem to be needed.

> I personally think this is ugly, but it would be somewhat consistent,
> then again a monitor app may expect in0_alarm, so I really think I
> should add that. But I think that generic apps shouldn't touch the
> _enable files and leave that to the sysadmin.

Doesn't make much sense to me. What are "generic apps"? Runing the
application as root, you should be allowed to write to all files (ala
"sensors -s".) Running as a regular user, you shouldn't. It's that
simple, and doesn't depend on the interface decisions anyway.

-- 
Jean Delvare