Sync with sensors applet #30

infirit · 2015-06-30T00:18:34Z

Sync up with most of sensors-applet. There is some code that adds GSettings to sensors-applet but this was not trivial so left for another time.

Compiles and runs nicely here (intel gfx) but needs testing on ati/nvidia hence this PR.

monsta · 2015-06-30T07:42:48Z

Wow, that's a big one. I have nvidia (with proprietary drivers), so I can test it later.
BTW, what mbmon is for?

monsta · 2015-06-30T07:43:35Z

@NiceandGently @flexiondotorg @stefano-k
please help with testing too if you can 😄

infirit · 2015-06-30T08:10:39Z

mbmon is just another monitoring tool like libsensors. It applied cleanly so I thought why not 😄

monsta · 2015-06-30T08:11:41Z

I actually wondered what it might monitor 😄

infirit · 2015-06-30T15:09:53Z

I actually wondered what it might monitor

I may misunderstand this but the chips/HW supported are listed in the readme linked.

monsta · 2015-07-03T11:48:26Z

About mbmon - are you sure it's needed? Looks like this tool isn't maintained since 2008 (at least in Debian). It might not even have any support for the motherboards released since then. I think lm-sensors pretty much obsoletes this.

monsta · 2015-07-03T12:05:23Z

About nvidia stuff: new "thermal" sensor shows exactly the same value as the old "core" one - is that by design?
Also I noticed that the fan speed is shown in RPMs - but nvidia-settings tool says reading RPMs is unsupported and the value is actually in percents:

infirit · 2015-07-03T16:30:41Z

I only brought the code in sync with the original applet so that new fixes and feature can be easily applied. The questions are in such a detail that I do not know without spending significant effort to figure out (which I have no time for).

re mbmon, it is self contained and does not break anything. Plus the original applet authors considered it useful so who are we to argue against it 😄. Do you care strongly?

raveit65 · 2015-07-04T09:38:37Z

hmm, last xmbmon is from 2005
http://www.nt.phys.kyushu-u.ac.jp/shimizu/download/xmbmon/
Is this really shipped by any distro?

raveit65 · 2015-07-04T10:11:38Z

gtk3 build failed with nvidia!

Making all in nvidia
make[2]: Entering directory '/builddir/build/BUILD/mate-sensors-applet-1.10.2/plugins/nvidia'
  CC       nvidia-plugin.lo
nvidia-plugin.c: In function 'nvidia_plugin_get_sensor_value':
nvidia-plugin.c:55:9: error: 'NV_CTRL_TARGET_TYPE_THERMAL_SENSOR' undeclared (first use in this function)
         NV_CTRL_TARGET_TYPE_THERMAL_SENSOR,
         ^
nvidia-plugin.c:55:9: note: each undeclared identifier is reported only once for each function it appears in
nvidia-plugin.c:58:9: error: 'NV_CTRL_THERMAL_SENSOR_READING' undeclared (first use in this function)
         NV_CTRL_THERMAL_SENSOR_READING,
         ^
nvidia-plugin.c:62:9: error: 'NV_CTRL_TARGET_TYPE_COOLER' undeclared (first use in this function)
         NV_CTRL_TARGET_TYPE_COOLER,
         ^
nvidia-plugin.c:65:9: error: 'NV_CTRL_THERMAL_COOLER_LEVEL' undeclared (first use in this function)
         NV_CTRL_THERMAL_COOLER_LEVEL,
         ^
nvidia-plugin.c: In function 'nvidia_plugin_init':
nvidia-plugin.c:117:10: error: 'NV_CTRL_TARGET_TYPE_THERMAL_SENSOR' undeclared (first use in this function)
          NV_CTRL_TARGET_TYPE_THERMAL_SENSOR,
          ^
nvidia-plugin.c:136:10: error: 'NV_CTRL_TARGET_TYPE_COOLER' undeclared (first use in this function)
          NV_CTRL_TARGET_TYPE_COOLER,
          ^
Makefile:502: recipe for target 'nvidia-plugin.lo' failed
make[2]: Leaving directory '/builddir/build/BUILD/mate-sensors-applet-1.10.2/plugins/nvidia'
Makefile:433: recipe for target 'all-recursive' failed
make[1]: Leaving directory '/builddir/build/BUILD/mate-sensors-applet-1.10.2/plugins'
Makefile:563: recipe for target 'all-recursive' failed
make[2]: *** [nvidia-plugin.lo] Error 1
make[1]: *** [all-recursive] Error 1
make: *** [all-recursive] Error 1

infirit · 2015-07-04T10:50:24Z

Yes distros schip mbmon on linux. And do note that MATE runs on other os's like *bsd where lmsensors is not available.

I will look at the build failure when I am back at home next week.

raveit65 · 2015-07-04T11:01:56Z

Yes distros schip mbmon on linux. And do note that MATE runs on other os's like *bsd where lmsensors is not available.

Ok, bsd is a reason.

monsta · 2015-07-04T12:22:47Z

gtk3 build failed with nvidia!

Probably it's some old version of libxnvctrl...

monsta · 2015-07-04T12:23:42Z

And do note that MATE runs on other os's like *bsd where lmsensors is not available.

Poor BSD users... how did they even use this applet then? lm-sensors is a significant part.

raveit65 · 2015-07-04T13:43:15Z

Probably it's some old version of libxnvctrl...

I can only use this version http://pkgs.fedoraproject.org/cgit/libXNVCtrl.git/log/
Since f14 (4.5 years) libXNVCtrl isn't update anymore, only rebuild.
But i maintain it for epel7 branch, so patches are welcome ;)

monsta · 2015-07-04T20:08:54Z

Damn, version 169 is ancient... Ubuntu 14.04 has 331 (it's in nvidia-settings package there).

raveit65 · 2015-07-04T22:17:25Z

......and nvidia-settings is nonfree

monsta · 2015-07-05T14:02:47Z

That's not a problem for Mint 😄
As for Debian/Ubuntu, libxnvctrl has been split into a separate (and free) package in recent releases.
But anyway, this is not the point here 😄

infirit · 2015-07-05T15:53:09Z

The nvidia plugin is for the non free driver anyway and will not work without it. While I sympathize with the old version in fedora the commit brings support for newer HW so it will need a newer version of libxnvctrl.

I'll probably merge and push this in a day or two.

raveit65 · 2015-07-05T17:46:26Z

It would be friendly to wait for merging it.
I need to contact main fedora maintainer to see if it's possible to update libxnvctrl for fedora.
Also mate-sensor-applet isn't the own package which depends on it, so another maintainer is involved too.

root@mother rave]# repoquery --whatrequires libXNVCtrl
hwloc-plugins-0:1.10.1-2.fc22.x86_64
mate-sensors-applet-0:1.10.2-1.fc22.x86_64

All this action is impossible to do in one or two days.
If it's not possible to update the package i can't build mate-sensor-applet with nvidia sensor, which is a regression imo.
The nvidia plugin from current release works here out of box for both gtk+ versions

PS: is this commit ever tested for gtk3 on bare metal from anyone other then me?

raveit65 · 2015-07-05T18:17:29Z

Which nvidia-settings version is needed?
Latest is nvidia-settings-352.21.tar.bz2
see ftp://download.nvidia.com/XFree86/nvidia-settings/

infirit · 2015-07-05T18:55:41Z

All this action is impossible to do in one or two days.

You have till the next MATE release to get this lib updated in fedora, this is not meant to go into 1.10 which should only get bugfixes now. Good enough?

Which nvidia-settings version is needed?

I build against 343.22 so and I think @monsta build against ~340 which is in jessie.

raveit65 · 2015-07-05T19:13:46Z

You have till the next MATE release to get this lib updated in fedora, this is not meant to go into 1.10 which should only get bugfixes now. Good enough?

good enough, so m-s-a needs branched.
Another question is does that work with all current legacy nvidia driver versions?
304 and 340.
....340 seems working
anyways i will try to update libXNVCtrl to latest 352, and than i will see if it works with legacy 340 which i use.

leigh123linux · 2015-07-05T20:42:08Z

@NiceandGently

This is going to cost you some more reviews 😆 https://dl.dropboxusercontent.com/u/6907158/libXNVCtrl-352.21-1.fc23.src.rpm

infirit · 2015-07-06T09:49:48Z

Another question is does that work with all current legacy nvidia driver versions

tbh, I don't know but.. nvidia-settings does not mention anything on this subject so I think it is likely backwards compatible.

raveit65 · 2015-07-06T10:06:46Z

RFE is filed out.
https://bugzilla.redhat.com/show_bug.cgi?id=1240234
Edit:
i forgot to say, using libXNVCtrl-352.21 fix the build.

monsta · 2015-07-06T10:50:26Z

I can say it builds fine in Mint 17.2 (based on Ubuntu 14.04) with nvidia-settings 331.20, and of course in Debian Jessie where we have libxnvctrl 340.46.

monsta · 2015-07-06T10:58:42Z

Ok, found out the exact versions where these two defines appeared:

NV_CTRL_THERMAL_COOLER_LEVEL - 190.32:
NVIDIA/nvidia-settings@29bde12#diff-c8d9162ce179db35fb13e3b489e36facR2577
NV_CTRL_THERMAL_SENSOR_READING - 256.25:
NVIDIA/nvidia-settings@9f27714#diff-c8d9162ce179db35fb13e3b489e36facR2916

5-6 years ago... 😄

monsta · 2015-07-06T11:45:36Z

Also I noticed that the fan speed is shown in RPMs - but nvidia-settings tool says reading RPMs is unsupported and the value is actually in percents

Turned out it's a limitation of the applet's own code.
All values received from fans are treated as RPMs:
https://github.com/mate-desktop/mate-sensors-applet/blob/master/sensors-applet/active-sensor.c#L625
https://github.com/mate-desktop/mate-sensors-applet/blob/master/sensors-applet/sensors-applet.c#L372
https://github.com/mate-desktop/mate-sensors-applet/blob/master/sensors-applet/sensors-applet.h#L57
Limits are also set for RPMs:
https://github.com/mate-desktop/mate-sensors-applet/blob/master/lib/sensors-applet-plugin.c#L89

raveit65 · 2015-07-07T06:49:46Z

OK, i tested again and added the new patch.
On my main box with proprietary nvidia driver:
GTK2/GTK3 build works well.
On my notebook with hybrid grapphics (intel + nvidia), but intel is in used and nouveau driver is installed.
Same gtk2 build failed, means i can't add the applet to the panel.

Taken from sensors-applet commit: c2596b4534f612ae890fac1719d4e6e6ef35ef91 and adapted for MATE by infirit@gmail.com

…orruption Make sure we don't write past the bounds of the output buffer - thanks to JeanFI for noticing this issue.

Changed() signal doesn't occur that often so instead poll each time to ensure we have the most up-to-date value

Based off sensors-applet commit c1777c9c58994d7eeec95893a5fdd1cbb7acb6eb from Alex Murray <murray.alex@gmail.com>

Adds support for NV_CTRL_THERMAL_SENSOR_READING as alternative to NV_CTRL_GPU_CORE_TEMPERATURE and NV_CTRL_THERMAL_COOLER_LEVEL for reading fan speeds on more modern NVIDIA GPUs.

Adapted for mate-sensors-applet by infirit@gmail.com

Passing NULL to strcmp could be unsafe or at the minimum result in undefined behaviour. g_strcmp0 handles NULL gracefully and returns 0.

raveit65 · 2015-07-07T10:35:02Z

commit 1 is good --> applet starts on my notebook
commit 2 breaks the start of the applet on my notebook
commit 3 didn't apply without commit 2
commit 4 --> good
commit 5 --> good
commit 6 --> good
commit 7 --> good
commit 8 --> good
commit 9 --> good
commit 10 --> good
commit 11 --> good
commit 12 --> good
commit 14 --> good
commit 14 --> good

Summary:
The applet starts fine on my notebook if i use all commits, exept 2 +3 !
2 looks like a general applet commit which was mentioned in my stacktrace from yesterday

infirit · 2015-07-07T13:39:55Z

Thanks for testing. I merged all the good commits and the 3rd. It looks like you are hitting one of the g_debug's that @monsta and I do not. I'll close this one now and try to figure out what is going on.

raveit65 · 2015-07-09T17:27:27Z

@infirit
libXNVCtrl is updated in fedora + epel7 branch (rhel7 family)
https://admin.fedoraproject.org/updates/libXNVCtrl-352.21-1.fc22?_csrf_token=bf4cb00b505fa46daa5f85dd006dd067b7481cfd
I agree if you want backport changes to 1.10 branch, would be nice to have new sensors.
Maybe you need to check the situation in other distros.

alexmurray and others added 14 commits July 7, 2015 11:27

Set icon name for dialog windows

89b136b

Taken from sensors-applet commit: c2596b4534f612ae890fac1719d4e6e6ef35ef91 and adapted for MATE by infirit@gmail.com

Cleanup hddtemp plugin socket reading code to avoid possible memory c…

80cae83

…orruption Make sure we don't write past the bounds of the output buffer - thanks to JeanFI for noticing this issue.

Cleanup debugging in libsensors plugin

0535c32

Cleanup and make udisks plugin poll for new value

cba9085

Changed() signal doesn't occur that often so instead poll each time to ensure we have the most up-to-date value

Cleanup debugging during plugin loading

42a6dd9

Cleanup debugging during Gtk signal handling

205dedc

hddtemp-plugin: close socket fd if connect fails to avoid leaking fd

8a240b0

Set custom mate-sensors-applet icon for About dialog

fa786b5

Based off sensors-applet commit c1777c9c58994d7eeec95893a5fdd1cbb7acb6eb from Alex Murray <murray.alex@gmail.com>

Add support for nvidia thermal and cooler sensors

a7e5afc

Adds support for NV_CTRL_THERMAL_SENSOR_READING as alternative to NV_CTRL_GPU_CORE_TEMPERATURE and NV_CTRL_THERMAL_COOLER_LEVEL for reading fan speeds on more modern NVIDIA GPUs.

Add new mbmon plugin

972e741

Adapted for mate-sensors-applet by infirit@gmail.com

Fix up string check errors

bf82b3d

Add automake silent rules

f37db04

Fix implicit declaration of function

83107ca

hddtemp: use g_strcmp0 instead of strcmp

2f84639

Passing NULL to strcmp could be unsafe or at the minimum result in undefined behaviour. g_strcmp0 handles NULL gracefully and returns 0.

infirit closed this Jul 7, 2015

monsta mentioned this pull request Aug 21, 2018

NVIDIA Fan Speed showing percentage speed instead of RPM #84

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sync with sensors applet #30

Sync with sensors applet #30

infirit commented Jun 30, 2015

monsta commented Jun 30, 2015

monsta commented Jun 30, 2015

infirit commented Jun 30, 2015

monsta commented Jun 30, 2015

infirit commented Jun 30, 2015

monsta commented Jul 3, 2015

monsta commented Jul 3, 2015

infirit commented Jul 3, 2015

raveit65 commented Jul 4, 2015

raveit65 commented Jul 4, 2015

infirit commented Jul 4, 2015

raveit65 commented Jul 4, 2015

monsta commented Jul 4, 2015

monsta commented Jul 4, 2015

raveit65 commented Jul 4, 2015

monsta commented Jul 4, 2015

raveit65 commented Jul 4, 2015

monsta commented Jul 5, 2015

infirit commented Jul 5, 2015

raveit65 commented Jul 5, 2015

raveit65 commented Jul 5, 2015

infirit commented Jul 5, 2015

raveit65 commented Jul 5, 2015

leigh123linux commented Jul 5, 2015

infirit commented Jul 6, 2015

raveit65 commented Jul 6, 2015

monsta commented Jul 6, 2015

monsta commented Jul 6, 2015

monsta commented Jul 6, 2015

raveit65 commented Jul 7, 2015

raveit65 commented Jul 7, 2015

infirit commented Jul 7, 2015

raveit65 commented Jul 9, 2015

Sync with sensors applet #30

Sync with sensors applet #30

Conversation

infirit commented Jun 30, 2015

monsta commented Jun 30, 2015

monsta commented Jun 30, 2015

infirit commented Jun 30, 2015

monsta commented Jun 30, 2015

infirit commented Jun 30, 2015

monsta commented Jul 3, 2015

monsta commented Jul 3, 2015

infirit commented Jul 3, 2015

raveit65 commented Jul 4, 2015

raveit65 commented Jul 4, 2015

infirit commented Jul 4, 2015

raveit65 commented Jul 4, 2015

monsta commented Jul 4, 2015

monsta commented Jul 4, 2015

raveit65 commented Jul 4, 2015

monsta commented Jul 4, 2015

raveit65 commented Jul 4, 2015

monsta commented Jul 5, 2015

infirit commented Jul 5, 2015

raveit65 commented Jul 5, 2015

raveit65 commented Jul 5, 2015

infirit commented Jul 5, 2015

raveit65 commented Jul 5, 2015

leigh123linux commented Jul 5, 2015

infirit commented Jul 6, 2015

raveit65 commented Jul 6, 2015

monsta commented Jul 6, 2015

monsta commented Jul 6, 2015

monsta commented Jul 6, 2015

raveit65 commented Jul 7, 2015

raveit65 commented Jul 7, 2015

infirit commented Jul 7, 2015

raveit65 commented Jul 9, 2015