gnome-shell-system-monitor-applet icon indicating copy to clipboard operation
gnome-shell-system-monitor-applet copied to clipboard

Use labels instead of file names for sensors (temperature, fan)

Open rschupp opened this issue 10 months ago • 2 comments

Suggested fix for #63.

The file names of sensor devices (e.g. /sys/class/hwmon/hwmon4/temp1_input) are not stable and may change on reboot (esp. when booting a different kernel). But the corresponding labels (e.g. /sys/class/hwmon/hwmon4/{name,temp1_label}) are stable. Hence we should not "remember" (ie. via settings) the file name, but the label.

This patch changes check_sensors() (in prefs.js) to return a mapping "label → file name" instead of two parallel lists of labels and files. pref.js stores the label in settings and extension.js will read them from settings. It needs check_sensors() to translate the label into a file name. Hence move check_sensors() in a new common module, common.js and import it from both prefs.js and extension.js. Add parse_bytearray() to common.js, too, as it's used both from extension.js and common.js.

What I tested

  • [x] Verified no regressions to existing functionality.
  • [x] Tested on the following Gnome Shell versions: 45, 46

This change is Reviewable

rschupp avatar Apr 25 '24 12:04 rschupp

@mgalgs Addressed your points and added a little cleanup.

rschupp avatar Apr 27 '24 14:04 rschupp

@mgalgs There's one problem left switching from sensor file paths to sensor labels: the corresponding schema entries:

    <key name="thermal-sensor-file" type="s">
      <default>'/sys/devices/virtual/thermal/thermal_zone0/temp'</default>
      <summary>Sensor File</summary>
      <description>Location of the sensor file for cpu temp</description>
    </key>
...
    <key name="fan-sensor-file" type="s">
      <default>'/sys/devices/virtual/thermal/cooling_device0/cur_state'</default>
      <summary>Sensor File</summary>
      <description>Location of the sensor file for fan</description>
    </key>

Summary and description are easily adapted, but what about the defaults? There's no reasonable default label. Note that the existing defaults are never listed by check_sensors() as it only looks for entries below /sys/class/hwmon/. So while the current default (before this PR) gave you something (if the files even exist), you didn't know what it is for. But once you used preferences you couldn't get back to this default setting (unless using gsettings reset or similar). So I tend to change defaults to the empty string (which means no values are shown until you make a choice in preferences).

rschupp avatar Apr 28 '24 12:04 rschupp

Looks like there is a missing ) in prefs.js https://github.com/mgalgs/gnome-shell-system-monitor-applet/issues/63#issuecomment-2082747843

And another one on line 515

SyntaxError: missing ) after argument list @ file:///home/ixevix/.local/share/gnome-shell/extensions/[email protected]/prefs.js:515:68

Stack trace:
  async*_loadPrefs@resource:///org/gnome/Shell/Extensions/js/extensionPrefsDialog.js:36:50
  _init@resource:///org/gnome/Shell/Extensions/js/extensionPrefsDialog.js:26:14
  ExtensionPrefsDialog@resource:///org/gnome/Shell/Extensions/js/extensionPrefsDialog.js:17:4
  OpenExtensionPrefsAsync@resource:///org/gnome/Shell/Extensions/js/extensionsService.js:139:33
  async*_handleMethodCall@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:373:35
  _wrapJSObject/<@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:408:34
  _init/GLib.MainLoop.prototype.runAsync/</<@resource:///org/gnome/gjs/modules/core/overrides/GLib.js:266:34

ixevix avatar Apr 30 '24 07:04 ixevix

@ixevix Thanks, fixed. Note that in 912e233 I "renamed" the geschema key thermal-sensor-file to thermal-sensor-label (same for fan...) and changed the defaults to ''. So if you test after that, the extension will not show any temperatur/fan items until you choose a sensor in the preferences.

rschupp avatar Apr 30 '24 10:04 rschupp

Thanks for the patch, @rschupp! Cutting a release now.

mgalgs avatar May 02 '24 22:05 mgalgs