More human friendly temperature sensor naming

[DianaNites]

Description

This changes how temperature sensors are displayed, in a way I believe to be much better.

This PR is based on top of and requires #805

Comparison

Before:

After:

Testing

Tested on my personal machine. This will likely need additional testing due to the apparent lack of a standard way to get better sensor names.

If this is a code change, please also indicate which platforms were tested:

• Windows
• macOS
• Linux

Checklist

If relevant, ensure the following have been met:

• Areas your change affects have been linted using rustfmt (cargo fmt)
• The change has been tested and doesn't appear to cause any unintended breakage
• Documentation has been added/updated if needed (README.md, help menu, etc.)
• The pull request passes the provided CI pipeline
• There are no merge conflicts
• If relevant, new tests were added (don't worry too much about coverage)

[codecov-commenter]

Codecov Report

Base: 23.59% // Head: 23.55% // Decreases project coverage by -0.03% ⚠️

Coverage data is based on head (65af8e6) compared to base (10efe75).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #807      +/-   ##
==========================================
- Coverage   23.59%   23.55%   -0.04%     
==========================================
  Files          60       60              
  Lines       13276    13298      +22     
==========================================
  Hits         3133     3133              
- Misses      10143    10165      +22     

Impacted Files	Coverage Δ
src/app/data_harvester/temperature/heim.rs	0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[DianaNites]

It now occurs to me this change would likely cause peoples temp_filter to no longer apply, if they were actually using that

on the upside, this change also makes it more practical to filter specific things, though.

[DianaNites]

Rebased onto master

[ClementTsang]

It now occurs to me this change would likely cause peoples temp_filter to no longer apply, if they were actually using that

on the upside, this change also makes it more practical to filter specific things, though.

I think it's fine, I'll mention the change and how it might break some existing configs when I release.

[DianaNites]

Awesome!

[ClementTsang]

Tested on my machine, and it generally works fine. One thing I did notice though is that for me, iwlwifi_1 becomes just thermal_zone0, which might actually be more unhelpful than it is now. Part of it might be that there isn't a label for this sensor, but I would imagine that just thermal_zone0 is kinda too ambiguous - for example, it's an entirely different thing for you than me, and it's not immediately obvious what the sensor possibly could be with a name like that.

Wondering if this specific case (thermal_zone<x>) should maybe display something like thermal_zone0: iwlwifi_1 instead, or if we should just skip it entirely and leave it as iwlwifi_1. I'm open to any suggestions.

[ClementTsang]

Generally looks fine, but I left a few comments to consider.

[DianaNites]

Tested on my machine, and it generally works fine. One thing I did notice though is that for me, iwlwifi_1 becomes just thermal_zone0, which might actually be more unhelpful than it is now. Part of it might be that there isn't a label for this sensor, but I would imagine that just thermal_zone0 is kinda too ambiguous - for example, it's an entirely different thing for you than me, and it's not immediately obvious what the sensor possibly could be with a name like that.

Wondering if this specific case (thermal_zone<x>) should maybe display something like thermal_zone0: iwlwifi_1 instead, or if we should just skip it entirely and leave it as iwlwifi_1. I'm open to any suggestions.

Nice catch! Thats why testing is good! And why the lack of more standard interfaces and/or naming schemes is annoying... My laptop has a MediaTek wifi card, which doesnt seem to report any sensors, so I didnt catch this.

The latest commit should address it, though at the cost of some clutter

It could be reduced, but it would require special casing certain names, which could be fragile. Might be worth it though, its a bit redundant especially for nvme

With this change, existing temp_filters will probably also continue to work now, since they'll still have the hwmon sensor names now.

[ClementTsang]

I think that looks good, it does add a bit more verbosity and is a bit funny in some cases like nvme, but I guess that's a bit better than expecting a certain sensor and not being sure why it seemingly doesn't exist when it's just under some really vague name.

[DianaNites]

Updated this to current master branch, otherwise should be no functional changes

[ClementTsang]

Looks good, thanks for your work on this!