[lm-sensors] sensors-detect: unecessary suggested change to

[lm-sensors] sensors-detect: unecessary suggested change to

[Gary Funck]

sensors-detect, prints the following:

    To load everything that is needed, add this to some /etc/rc* file:

    #----cut here----
    # Chip drivers
    modprobe w83627hf
    # sleep 2 # optional
    /usr/bin/sensors -s # recommended
    #----cut here----

    If you have some drivers built into your kernel, the list above will
    contain too many modules. Skip the appropriate ones! You really
    should try these commands right now to make sure everything is
    working properly. Monitoring programs won't work until the needed
    modules are loaded.

    Do you want to overwrite /etc/sysconfig/lm_sensors? (YES/no):

But if the user replies 'yes' above, then appropriate
module name is written into /etc/sysconfig/lm_sensors,
_and_ /etc/init.d/lm_sensors will run the 'sensors -s'
command when lm_sensors starts up.

Suggestion: just print the module(s) that need to be loaded,
and if the user replies 'yes', take no further action.
Otherwise, print the suggestion to add the needed lines to an
/etc/rc* file.  Also, it would be helpful to suggest the
conventional name (such as /etc/rc.local) for the 'rc' file.

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] sensors-detect: unecessary suggested change to

[Jean Delvare]

Hi Gary,

On Sun, 16 Dec 2007 16:03:16 -0800, Gary Funck wrote:
> sensors-detect, prints the following:
> 
>     To load everything that is needed, add this to some /etc/rc* file:
> 
>     #----cut here----
>     # Chip drivers
>     modprobe w83627hf
>     # sleep 2 # optional
>     /usr/bin/sensors -s # recommended
>     #----cut here----
> 
>     If you have some drivers built into your kernel, the list above will
>     contain too many modules. Skip the appropriate ones! You really
>     should try these commands right now to make sure everything is
>     working properly. Monitoring programs won't work until the needed
>     modules are loaded.
> 
>     Do you want to overwrite /etc/sysconfig/lm_sensors? (YES/no):
> 
> But if the user replies 'yes' above, then appropriate
> module name is written into /etc/sysconfig/lm_sensors,
> _and_ /etc/init.d/lm_sensors will run the 'sensors -s'
> command when lm_sensors starts up.
> 
> Suggestion: just print the module(s) that need to be loaded,
> and if the user replies 'yes', take no further action.

Agreed. I wanted to do that some times ago already, but could never
find the time. This is done now, please give a try to the SVN version
of sensors-detect and let me know what you think about it:
http://www.lm-sensors.org/browser/lm-sensors/trunk/prog/detect/sensors-detect?format=txt

> Otherwise, print the suggestion to add the needed lines to an
> /etc/rc* file.  Also, it would be helpful to suggest the
> conventional name (such as /etc/rc.local) for the 'rc' file.

Assuming that you really meant /etc/rc.d/rc.local, I've done that too.

Thanks,
-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] sensors-detect: unecessary suggested change to

[Gary Funck]

On 12/18/07 14:13:40, Jean Delvare wrote:
> 
> Agreed. I wanted to do that some times ago already, but could never
> find the time. This is done now, please give a try to the SVN version
> of sensors-detect and let me know what you think about it:
> http://www.lm-sensors.org/browser/lm-sensors/trunk/prog/detect/sensors-detect?format=txt

Jean, thanks.  Tried it out on FC8.  Replying 'yes' to all, the
following is printed:

: Now follows a summary of the probes I have just done.
: Just press ENTER to continue: 
: Driver `w83627hf' (should be inserted):
:   Detects correctly:
:   * ISA bus, address 0x290
:     Chip `Winbond W83627HF/F/HG/G Super IO Sensors' (confidence: 9)
: 
: Do you want to overwrite /etc/sysconfig/lm_sensors? (YES/no): yes
: You should now start the lm_sensors service to load the required
: kernel modules.

Observations:

1. "(should be inserted):" may not be necessary, since either
sensors-detect will do that, or the user will be told to do that.

2. The output notes that "Driver" `w83627hf' was chosen, but
doesn't clearly say that was the module name written to
/etc/sysconfig/lm_sensors.  And the module/driver teminology may
be somewhat confusing here (a minor issue)?

3. The advice to start the service is good.  Should the user
also be advised to enable it via chkconfig, or is it enabled
by default in most distro packages?

The 'no' output follows:

: Do you want to overwrite /etc/sysconfig/lm_sensors? (YES/no): no
: To load everything that is needed, add this to e.g. /etc/rc.d/rc.local:
: 
: #----cut here----
: # Chip drivers
: modprobe w83627hf
: # sleep 2 # optional
: /usr/bin/sensors -s # recommended
: #----cut here----
: 
: If you have some drivers built into your kernel, the list above will
: contain too many modules. Skip the appropriate ones! You really
: should try these commands right now to make sure everything is
: working properly. Monitoring programs won't work until the needed
: modules are loaded.

Observations:

1. Perhaps rephrase the suggestion as:

: To load everything that is needed, add this to
: the system init file (e.g. /etc/rc.d/rc.local):

2. If redundant modprobes are harmless, perhaps the suggestion
to check which modules are built into the kernel is unecessary
and might confuse a novice user/admin?

3. Not sure what to do with this line:
: # sleep 2 # optional
How does the user know if he/she should uncomment this line?
Is it necessary at all?

Thanks,
  - Gary

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] sensors-detect: unecessary suggested change to

[Jean Delvare]

Hi Gary,

On Tue, 18 Dec 2007 08:27:45 -0800, Gary Funck wrote:
> On 12/18/07 14:13:40, Jean Delvare wrote:
> > 
> > Agreed. I wanted to do that some times ago already, but could never
> > find the time. This is done now, please give a try to the SVN version
> > of sensors-detect and let me know what you think about it:
> > http://www.lm-sensors.org/browser/lm-sensors/trunk/prog/detect/sensors-detect?format=txt
> 
> Jean, thanks.  Tried it out on FC8.  Replying 'yes' to all, the
> following is printed:
> 
> : Now follows a summary of the probes I have just done.
> : Just press ENTER to continue: 
> : Driver `w83627hf' (should be inserted):
> :   Detects correctly:
> :   * ISA bus, address 0x290
> :     Chip `Winbond W83627HF/F/HG/G Super IO Sensors' (confidence: 9)
> : 
> : Do you want to overwrite /etc/sysconfig/lm_sensors? (YES/no): yes
> : You should now start the lm_sensors service to load the required
> : kernel modules.
> 
> Observations:
> 
> 1. "(should be inserted):" may not be necessary, since either
> sensors-detect will do that, or the user will be told to do that.

In some cases, the message in parentheses will be different, so the
message is valuable independently of what happens next.

> 
> 2. The output notes that "Driver" `w83627hf' was chosen, but
> doesn't clearly say that was the module name written to
> /etc/sysconfig/lm_sensors.  And the module/driver teminology may
> be somewhat confusing here (a minor issue)?

The driver name and the module name are, fortunately, always the same,
so there's not much room for confusion. And anyway, the user doesn't
really need to know what was written to /etc/sysconfig/lm_sensors.

> 
> 3. The advice to start the service is good.  Should the user
> also be advised to enable it via chkconfig, or is it enabled
> by default in most distro packages?

In openSuse, sensors-detect is modified to call /sbin/inserv at this
point. I don't know about the other distributions. Admittedly it would
be nice to automate this step as well, however I don't know for sure
how this can be done in a portable way. We could do:

    system("/sbin/insserv", "/etc/init.d/lm_sensors")
      if -x "/sbin/insserv" && -f "/etc/init.d/lm_sensors";

That would work for openSuse and presumably Fedora, but I don't know
about Debian. Aurelien?

> 
> The 'no' output follows:
> 
> : Do you want to overwrite /etc/sysconfig/lm_sensors? (YES/no): no
> : To load everything that is needed, add this to e.g. /etc/rc.d/rc.local:
> : 
> : #----cut here----
> : # Chip drivers
> : modprobe w83627hf
> : # sleep 2 # optional
> : /usr/bin/sensors -s # recommended
> : #----cut here----
> : 
> : If you have some drivers built into your kernel, the list above will
> : contain too many modules. Skip the appropriate ones! You really
> : should try these commands right now to make sure everything is
> : working properly. Monitoring programs won't work until the needed
> : modules are loaded.
> 
> Observations:
> 
> 1. Perhaps rephrase the suggestion as:
> 
> : To load everything that is needed, add this to
> : the system init file (e.g. /etc/rc.d/rc.local):

There is no such thing as "the system init file". There are many such
files, and they are better qualified as "scripts" than just files.

> 
> 2. If redundant modprobes are harmless, perhaps the suggestion
> to check which modules are built into the kernel is unecessary
> and might confuse a novice user/admin?

modprobe will fail and display an error message if told to load a
module that doesn't exist. And anyway, a novice user/admin will
certainly be using a distribution where /etc/sysconfig exists, so he/she
won't see this message at all.

> 
> 3. Not sure what to do with this line:
> : # sleep 2 # optional
> How does the user know if he/she should uncomment this line?
> Is it necessary at all?

I don't think it is necessary. At least I can't find any reason why
sleeping at this point could be of any help. Originally the line was
not commented out, I changed this back in July 2003. It's probably time
to remove it completely. If nobody objects, I'll do that.

Thanks,
-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] sensors-detect: unecessary suggested change to

[Aurelien Jarno]

Jean Delvare a écrit :
> Hi Gary,
> 
> On Tue, 18 Dec 2007 08:27:45 -0800, Gary Funck wrote:
>> On 12/18/07 14:13:40, Jean Delvare wrote:
>>> Agreed. I wanted to do that some times ago already, but could never
>>> find the time. This is done now, please give a try to the SVN version
>>> of sensors-detect and let me know what you think about it:
>>> http://www.lm-sensors.org/browser/lm-sensors/trunk/prog/detect/sensors-detect?format=txt
>> Jean, thanks.  Tried it out on FC8.  Replying 'yes' to all, the
>> following is printed:
>>
>> : Now follows a summary of the probes I have just done.
>> : Just press ENTER to continue: 
>> : Driver `w83627hf' (should be inserted):
>> :   Detects correctly:
>> :   * ISA bus, address 0x290
>> :     Chip `Winbond W83627HF/F/HG/G Super IO Sensors' (confidence: 9)
>> : 
>> : Do you want to overwrite /etc/sysconfig/lm_sensors? (YES/no): yes
>> : You should now start the lm_sensors service to load the required
>> : kernel modules.
>>
>> Observations:
>>
>> 1. "(should be inserted):" may not be necessary, since either
>> sensors-detect will do that, or the user will be told to do that.
> 
> In some cases, the message in parentheses will be different, so the
> message is valuable independently of what happens next.
> 
>> 2. The output notes that "Driver" `w83627hf' was chosen, but
>> doesn't clearly say that was the module name written to
>> /etc/sysconfig/lm_sensors.  And the module/driver teminology may
>> be somewhat confusing here (a minor issue)?
> 
> The driver name and the module name are, fortunately, always the same,
> so there's not much room for confusion. And anyway, the user doesn't
> really need to know what was written to /etc/sysconfig/lm_sensors.
> 
>> 3. The advice to start the service is good.  Should the user
>> also be advised to enable it via chkconfig, or is it enabled
>> by default in most distro packages?
> 
> In openSuse, sensors-detect is modified to call /sbin/inserv at this
> point. I don't know about the other distributions. Admittedly it would
> be nice to automate this step as well, however I don't know for sure
> how this can be done in a portable way. We could do:
> 
>     system("/sbin/insserv", "/etc/init.d/lm_sensors")
>       if -x "/sbin/insserv" && -f "/etc/init.d/lm_sensors";
> 
> That would work for openSuse and presumably Fedora, but I don't know
> about Debian. Aurelien?

On Debian, the end of sensors-detect is heavily modified, as the sensors
modules are written to /etc/modules instead, and the format is a bit
different. All modules from this file are loaded automatically at boot time.

Then the /etc/init.d/lm-sensors script corresponds to a simple script
that read the "sensors -s" twice, so that the alarms are set and
possibly cleared.

For the sensord daemon we have a separate package, and the daemon is
enabled by default (since the package is not installed by default). This
can be changed via /etc/default/sensord (usual path on Debian).

So in short I am not opposed to changed on this part, as anyway it is
already different :-)

Cheers,
Aurelien

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] sensors-detect: unecessary suggested change to

[Jean Delvare]

Hi Aurelien,

On Tue, 18 Dec 2007 19:16:30 +0100, Aurelien Jarno wrote:
> Jean Delvare a écrit :
> > On Tue, 18 Dec 2007 08:27:45 -0800, Gary Funck wrote:
> >> 3. The advice to start the service is good.  Should the user
> >> also be advised to enable it via chkconfig, or is it enabled
> >> by default in most distro packages?
> > 
> > In openSuse, sensors-detect is modified to call /sbin/inserv at this
> > point. I don't know about the other distributions. Admittedly it would
> > be nice to automate this step as well, however I don't know for sure
> > how this can be done in a portable way. We could do:
> > 
> >     system("/sbin/insserv", "/etc/init.d/lm_sensors")
> >       if -x "/sbin/insserv" && -f "/etc/init.d/lm_sensors";
> > 
> > That would work for openSuse and presumably Fedora, but I don't know
> > about Debian. Aurelien?
> 
> On Debian, the end of sensors-detect is heavily modified, as the sensors
> modules are written to /etc/modules instead, and the format is a bit
> different. All modules from this file are loaded automatically at boot time.
> 
> Then the /etc/init.d/lm-sensors script corresponds to a simple script
> that read the "sensors -s" twice, so that the alarms are set and
> possibly cleared.

You should not assume that "sensors -s" will clear the alarms. It does
for now because libsysfs reads all the attributes even when not told to,
however I fixed that in SVN last week, so starting with lm-sensors
3.0.1 (or whatever we number the next version) it won't be the case
anymore. You should run "sensors", not "sensors -s", if you want to
clear the alarms. You may need to wait for 2 seconds before you do if
the first "sensors -s" read the alarm registers (which is the case in
3.0.0, but should no longer be in 3.0.1.)

> For the sensord daemon we have a separate package, and the daemon is
> enabled by default (since the package is not installed by default). This
> can be changed via /etc/default/sensord (usual path on Debian).
> 
> So in short I am not opposed to changed on this part, as anyway it is
> already different :-)

OK, fair enough. I think I'll add the optional insserv call, I guess it
cannot hurt as it is protected with the relevant conditionals. But I
want to keep sensors-detect somewhat distribution-neutral. So, if you
feel that part of the changes done to this script in Debian are general
enough, I would be happy to merge them upstream, with whatever
conditionals are needed so that it won't cause trouble on other
distributions. But I'm not forcing you either, if you want to keep the
changes in Debian to have better control over them, that's equally fine
with me. It's really up to you.

Same apply to other popular distros, BTW: changes that are general
enough will be considered for upstream inclusion.

Thanks,
-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors