Conversation
|
Hi, a few initial comments:
|
|
Hi, Thank you for the quick feedback.
|
647fcad to
eab4a2a
Compare
…tify / thaw_notify
757ed66 to
37f2334
Compare
…der (for ease to use & it fixes core order in coretemp)
1791bee to
a38614b
Compare
|
I think this is now ready for review.
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 :
@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 |
|
Found a small edge case related to the "pwm" type : the file naming does not match the other types of sensors (no "_input" suffix). |
…THER, we implement all useful sensor types. Group POWER sensors close to FREQ
|
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") "pwm1".replace("", "_label") # → "_labelp_labelw_labelm_label1_label" 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. |
|
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. |
|
It looks really good now @pilatomic. I fixed the bugs also and added quite a few refinements.
I can merge this from here. I'll squash your commits and squash mine so it only takes two of them. |
|
I pushed here temporarily https://github.com/linuxmint/mintreport/tree/sensors. |
|
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 ? |
|
Shouldnt the CURRENT type rather use the "xsi-physics-amps-symbolic" icon, rather than the generic ""xsi-physics-wave-symbolic" ? |




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).
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 ?