Skip to content

simple-system-monitor: fix GPU and CPU sensors, CPU usage and network rates - #1889

Open
chloecaffeinexo wants to merge 1 commit into
linuxmint:masterfrom
chloecaffeinexo:simple-system-monitor-sensor-and-accuracy-fixes
Open

simple-system-monitor: fix GPU and CPU sensors, CPU usage and network rates#1889
chloecaffeinexo wants to merge 1 commit into
linuxmint:masterfrom
chloecaffeinexo:simple-system-monitor-sensor-and-accuracy-fixes

Conversation

@chloecaffeinexo

Copy link
Copy Markdown

@arielandrade

Thanks for this desklet, it has been on my desktop for a while. While looking at
why the GPU row never seemed to move I found a few things worth fixing, all in
the 5.8 version.

The GPU temperature never read the GPU. customGPUPath and customCPUPath
both default to /sys/class/thermal/thermal_zone0, and that default is written
into the saved settings on first run, so the GPU row has always shown the same
motherboard sensor as the CPU row. On my machine the two rows sat on an identical
number permanently, which is what made me look.

The sensors are now detected instead. NVIDIA is read through nvidia-smi, since
the proprietary driver publishes no hwmon entry, and that call is asynchronous
and rate limited to once every 2 seconds so it cannot stall the shell. AMD, Intel
and nouveau are read from hwmon. The CPU prefers the on-die package sensor
(coretemp / k10temp) and falls back to x86_pkg_temp, then thermal_zone0.
A path the user has actually chosen still wins. Because the old default is sitting
in existing users' settings files, /sys/class/thermal/thermal_zone0 is treated
as "detect automatically" rather than as a deliberate choice, otherwise the fix
would not reach anyone who has already run the desklet.

CPU usage was measured wrongly in both directions. It used
(user + sys + iowait) / total, which counts iowait as busy, so a slow disk read
as CPU load, while nice, irq and softirq were not counted at all. It is now
(total - idle - iowait) / total.

Network traffic was counted more than once. update_connections sums every
activated NetworkManager device, which on a machine with a VPN includes both the
tunnel and the physical card carrying it, plus loopback. My figures were roughly
doubled whenever the VPN was up. Only interfaces with a device entry in sysfs
are counted now, with a fallback to the old list if that leaves nothing.

Network rate units. The rate divided bytes by milliseconds and labelled the
answer KB, which is about 2.4 % high, and the label had no per-second part, so a
speed read like a total. Now computed as bytes per second and shown as B/s,
KB/s or MB/s.

Refresh interval is now a setting, defaulting to the existing 1 second, range
0.5 to 30. This one is a feature rather than a fix, so I am happy to drop it from
the PR if you would rather keep the interval fixed.

Settings object was rebuilt on every change. setupUI created a new
DeskletSettings and rebound every property each time it ran, while the previous
bindings stayed live and also called setupUI, so each settings change cost more
than the one before. It is now built once, with the UI rebuild split into its own
function. The changed::desklet-decorations handler had the same problem.

Tested on Cinnamon 6.6.7, Fedora 44, on a laptop with an Intel UHD iGPU and an
NVIDIA RTX 2060, with Proton VPN and Tailscale both up. CPU and GPU temperatures
were cross checked against nvidia-smi and the coretemp hwmon entry, and the
network figures against the raw interface counters. Versions 3.0 and 4.0 are
untouched.


🤖 Generated with Claude Code

… rates

The GPU temperature was read from thermal_zone0, the same motherboard sensor
used for the CPU, so the GPU row showed the CPU value on every install. It is
now read from the GPU itself: nvidia-smi for NVIDIA, hwmon for AMD and Intel.
The CPU sensor is likewise detected rather than defaulting to thermal_zone0,
preferring the on-die package sensor.

CPU usage counted iowait as busy time and left out nice, irq and softirq, so a
slow disk showed as CPU load while real work went uncounted. It is now measured
as the share of time that is neither idle nor waiting on I/O.

Network traffic was summed over every activated interface including loopback and
any VPN, so traffic through a tunnel was counted twice, once on the tunnel and
again on the hardware carrying it. Only physical interfaces are counted now. The
rate itself divided bytes by milliseconds and labelled the result KB, and the
label was missing the per-second part.

The refresh interval becomes a setting, keeping 1 second as the default. GPU
temperature is polled no more than once every 2 seconds since it costs a
subprocess.

The settings object was rebuilt on every settings change while the previous
bindings stayed live, so each change cost more than the one before it.
@github-actions

Copy link
Copy Markdown

Best-practices scanner

This is a regex-based check for API usage that can pose security, performance or
maintainability issues, or that may already be provided by Cinnamon. Most findings
are advisory and do not automatically disqualify a pull request.

This check is not perfect and will not replace a normal review.


Found 6 potential issue(s):

⚠️ WARNING

⚠️ sync_file_get_contents

simple-system-monitor@ariel/files/simple-system-monitor@ariel/5.8/desklet.js:40

let [ok, contents] = GLib.file_get_contents(path);

Synchronous file_get_contents() blocks the main loop.
Use Gio.File.load_contents_async() instead.

⚠️ sync_file_test

simple-system-monitor@ariel/files/simple-system-monitor@ariel/5.8/desklet.js:59

if (!GLib.file_test(input, GLib.FileTest.EXISTS))

file_test() is a synchronous stat call that can block on slow/network filesystems.
Prefer attempting the operation and handling a Gio.IOErrorEnum.NOT_FOUND error instead.

simple-system-monitor@ariel/files/simple-system-monitor@ariel/5.8/desklet.js:94

if (!GLib.file_test(dir, GLib.FileTest.IS_DIR))

file_test() is a synchronous stat call that can block on slow/network filesystems.
Prefer attempting the operation and handling a Gio.IOErrorEnum.NOT_FOUND error instead.

simple-system-monitor@ariel/files/simple-system-monitor@ariel/5.8/desklet.js:97

if (GLib.file_test(candidate, GLib.FileTest.EXISTS))

file_test() is a synchronous stat call that can block on slow/network filesystems.
Prefer attempting the operation and handling a Gio.IOErrorEnum.NOT_FOUND error instead.

simple-system-monitor@ariel/files/simple-system-monitor@ariel/5.8/desklet.js:337

name && name != "lo" && GLib.file_test(`/sys/class/net/${name}/device`, GLib.FileTest.EXISTS));

file_test() is a synchronous stat call that can block on slow/network filesystems.
Prefer attempting the operation and handling a Gio.IOErrorEnum.NOT_FOUND error instead.

⚠️ lang_bind

simple-system-monitor@ariel/files/simple-system-monitor@ariel/5.8/desklet.js:427

'changed::desklet-decorations', Lang.bind(this, this.setupUI));

Lang.bind() is deprecated. Use arrow functions (() => {}) or Function.prototype.bind() instead.


Automated pattern check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant