Skip to content

Add sensors page#116

Open
pilatomic wants to merge 10 commits intolinuxmint:masterfrom
pilatomic:AddSensorsPage
Open

Add sensors page#116
pilatomic wants to merge 10 commits intolinuxmint:masterfrom
pilatomic:AddSensorsPage

Conversation

@pilatomic
Copy link
Contributor

@pilatomic pilatomic commented Jan 13, 2026

Hello,

This PR is not meant to be merged at this point. It is purely a draft, to open the discussion.

I have been really enjoying the latest additions to Mintreport, most notably the USB and PCI pages, that make accessible those information to users unfamiliar with the command line.

Following that, I have been playing with the idea of adding a new tab showing the different hardware sensors (gathered through hwmon).

image

It has been written in an evening, using AI tooling, and is very crude and missing important bits, notably translatable strings, and a lot of polish. Some sensors types ("current" for example) and icons are missing, but I think it does a good job of show casing what could be.

Also it is based on my 2 previous PR (#113 and #114)

If this is a path work pursuing, I can invest the work to clean it up until it is mergeable.

What do you think ?

@mtwebster
Copy link
Member

Hi, a few initial comments:

  • Keep the PR targeted to sensors - you're changing some usb stuff, and more importantly you're voiding some existing translations. If you want to change things in another area, make a new PR.
  • Don't update the potfile or .po files, we manage translations thru Launchpad and will run makepot when necessary.
  • I'm not against AI-assisted code but you'd better have tested everything and know why things are being done, not just looking at the output of your code's execution and calling it good.
  • I'm not sure "W" and "%" need translated - some of these technical symbols are language-independent - you should confirm. But for any that might, you should put them in the form of _("%d RPM") % raw or whatever... to provide context for translators.

@pilatomic
Copy link
Contributor Author

Hi,

Thank you for the quick feedback.

  • The USB stuff comes from other PRs (USB 1.5 Mbps speed is not formatted correctly #111 and Fix usb speed display #114), which I assumed would go through first. I will get rid of it in this PR to keep it clean.
  • Potfiles : OK, will fix
  • AI code : OK. it will undergo more scrutiny from my part and actual testing before leaving the draft stage. (To be clear, I checked the way of accessing the sensors, and that is sane. The only part I'm not 100% comfortable with is the python way of doing things).
  • Translations : OK, will do.

…der (for ease to use & it fixes core order in coretemp)
@pilatomic
Copy link
Contributor Author

pilatomic commented Jan 20, 2026

I think this is now ready for review.

image

It shows all Hwmon sensors, and has been tested on a bunch a different hardware (modern Intel CPU, modern AMD CPU, older Ryzen, a 2008 iMac, and a Virtualbox VM), where it consistently shows all the available sensors.

Here are the most important changes since the initial version :

  • All the sensor specifications (unit, icon, value formatting) are now in a dictionary, easy to edit
  • Removed _() for any unit that will not be translated.
  • Stop refreshing when the page is not visible, to avoid wasting CPU cycles
  • Sensors are now sorted by natural order. This is especially important for Intel coretemp sensors, which come in a weird order if one simply follows the hwmon files order. (Package temperature right in the middle of unsorted core temperatures).

@mtwebster : I did not format the unit in the way you recommended "_("%d RPM")", as they are in their own column. This is open to discussion.

To look complete, it stills requires a bunch of new icons. I used the "xsi-cog-symbolic" icon as a placeholder in the meantime.

The most important would be one of a Fan (for the fan speed and maybe PWM) and one electrical symbol that could be used for Power / Voltage / Current / Energy.

Maybe also a clock for the Frequency ?

I'm unsure if I can try to find and submit icons, or if that is best left to somebody else.

I'm open to any comment

@pilatomic pilatomic marked this pull request as ready for review January 20, 2026 21:22
@pilatomic pilatomic marked this pull request as draft January 21, 2026 17:59
@pilatomic
Copy link
Contributor Author

Found a small edge case related to the "pwm" type : the file naming does not match the other types of sensors (no "_input" suffix).
Converting back to draft until I can fix that.

…THER, we implement all useful sensor types. Group POWER sensors close to FREQ
@pilatomic pilatomic marked this pull request as ready for review January 22, 2026 19:12
@clefebvre clefebvre self-assigned this Feb 6, 2026
@clefebvre
Copy link
Member

clefebvre commented Feb 24, 2026

Pretty cool. I ran some AI checks also to find issues. Here's the report:


Bug 1 — PWM false positives (line 90)

sensor_spec_from_filename checks filename.endswith(suffix). Since PWM's suffix is "", every string ends with it. This means files like pwm1_enable, pwm1_mode, pwm1_freq all match as PWM sensors. Their values (0, 1, 2...) get formatted as int(raw)*100/255, producing garbage readings in the UI.

The fix should require that PWM files match pwmN exactly (e.g. a regex ^pwm\d+$).

Bug 2 — PWM label path corruption (line 234)

labelname = fname.replace(spec["suffix"], "_label")
When spec["suffix"] is "", Python's str.replace("", ...) inserts the replacement between every character:

"pwm1".replace("", "_label") # → "_labelp_labelw_labelm_label1_label"
This produces a completely wrong path. It happens to not matter in practice because the file won't be found and the fallback kicks in, but it's broken code.

Bug 3 — name file read from wrong path (line 211)

When base_path is set to device_path (for modules like apple-smc), the name file is read from device_path/name. But hwmon's name file lives at hwmon_path/name, not inside device/. So the device name won't be found and falls back to the raw folder name (e.g. hwmon3).

Minor — comment typo (line 172)

Refresh existing tree or build it is does not exist yet

Should be if not is.


I'll have a look at the icons.

@pilatomic
Copy link
Contributor Author

Thank you for the feedback. I'll look into Bug 1, 2 and the typo.

Bug 3 is not a bug, this is by design : in the case of the applesmc device, sensors and name file are both in the hwmonX/device folder.

@clefebvre
Copy link
Member

@clefebvre
Copy link
Member

SENSOR_SPECS = {
    SensorType.TEMP: {
        "prefix":"temp",
        "suffix":"_input",
        "format":lambda raw: f"{int(raw)/1000:.1f}",
        "unit":"°C",
        "icon":"xsi-temperature-symbolic"
    },
    SensorType.FAN: {
        "prefix":"fan",
        "suffix":"_input",
        "format":lambda raw: raw.strip(),
        "unit":_("RPM"),
        "icon":"xsi-fan-symbolic"
    },
    SensorType.PWM: {
        "prefix":"pwm",
        "suffix":"", # no _input suffix for pwm
        "format":lambda raw: f"{int(raw)*100/255:.0f}",
        "unit":"%",
        "icon":"xsi-fan-symbolic"
    },
    SensorType.FREQ: {
        "prefix":"freq",
        "suffix":"_input",
        "format":lambda raw: f"{int(raw)/1_000_000_000:.3f}",
        "unit":"GHz",
        "icon":"xsi-physics-wavelength-symbolic"
    },
    SensorType.VOLTAGE: {
        "prefix":"in",
        "suffix":"_input",
        "format":lambda raw: f"{int(raw)/1000:.3f}",
        "unit":"V",
        "icon":"xsi-physics-volts-symbolic"
    },
    SensorType.CURRENT: {
        "prefix":"curr",
        "suffix":"_input",
        "format":lambda raw: f"{int(raw)/1000:.3f}",
        "unit":"A",
        "icon":"xsi-physics-wave-symbolic"
    },
    SensorType.POWER: {
        "prefix":"power",
        "suffix":"_input",
        "format":lambda raw: f"{int(raw)/1_000_000:.1f}",
        "unit":"W",
        "icon":"xsi-physics-watts-symbolic"
    },
    SensorType.ENERGY: {
        "prefix":"energy",
        "suffix":"_input",
        "format":lambda raw: f"{int(raw)/1_000_000:.3f}",
        "unit":"J",
        "icon":"xsi-power-symbolic"
    }
}
image

@clefebvre
Copy link
Member

It looks much better with cards:

image

@clefebvre
Copy link
Member

It looks really good now @pilatomic. I fixed the bugs also and added quite a few refinements.

image

I can merge this from here. I'll squash your commits and squash mine so it only takes two of them.

@clefebvre
Copy link
Member

I pushed here temporarily https://github.com/linuxmint/mintreport/tree/sensors.

@pilatomic
Copy link
Contributor Author

That looks really great, I'm all in !

I am going to try your branch on a bunch of different machines, and I'll report back in a few hours.

I would like to suggest an alternative fix for the PWM file name :

https://github.com/pilatomic/mintreport/tree/AddSensorPageFixPwm

Initially to keep the code as readable as possible, I did not want to use regexp to match the sensors file name, but if we're going to use one anyway, we can make the rest of the code cleaner.

What do you think ?

@pilatomic
Copy link
Contributor Author

Shouldnt the CURRENT type rather use the "xsi-physics-amps-symbolic" icon, rather than the generic ""xsi-physics-wave-symbolic" ?

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants