Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FSTemperatureMonitor needs some love #21

Closed
ntninja opened this issue Aug 14, 2015 · 5 comments
Closed

FSTemperatureMonitor needs some love #21

ntninja opened this issue Aug 14, 2015 · 5 comments

Comments

@ntninja
Copy link

ntninja commented Aug 14, 2015

As we've discussed in #18 the current implementation of FSTemperatureMonitor is somewhat confusing.

Some things I'd suggest doing:

  • Rename the whole thing to LinuxTemperatureMonitor. Neither does the hwmon work only on file systems (the current code ignores everything except CPUs), nor is it portable to anything except Linux.
  • Get rid of the configuration file: Hardware is dynamic and the code should reflect this. It's OK to check the list of available sensors (additionally to their values) every second, the additional overhead for you're usual <10 sensors is negligible.
  • If you do want to keep the configuration file around, use it only to allow the user to set additional temperature sensors and blacklist known bad ones instead of using as "temperature sensor law".

Please let me know what you think about these points.

@ntninja
Copy link
Author

ntninja commented Aug 15, 2015

You might already know this, but (for reference) here's the reference sheet for the hwmon sysfs interface:
https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface

Also wouldn't it make sense to measure the temperature of all available hardware devices (instead of just CPUs) and return the maximum temperature measured instead of the average one. If some process really stresses my GPU, for instance, I'd want my fan to start doing some extra cooling as well…

@hirschmann
Copy link
Owner

These are some pretty good suggestions and I agree with most of them.
I think a temperature monitoring plugin dedicated to Linux is a pretty good idea.

I have already read the sysfs-interface documentation and there is one sentence I want to highlight:

An alternative method that some programs use is to access the sysfs
files directly. [...]
That said, such programs will have to implement conversion, labeling and hiding of inputs. For
this reason, it is still not recommended to bypass the library.

I think the plugin should use libsensors to read temperatures from various temperature sensors instead of directly accessing the sysfs files.
Because there is nothing in the FSTemperatureMonitor code which could be reused, I prefer to create a new plugin project which will supersede FSTemperatureMonitor once it is finished.

@ntninja
Copy link
Author

ntninja commented Aug 15, 2015

I'll look into this, but looking at the search results I found I fear I'll have to create a libsensor C# binding (involving unsafe code and whatnot) myself. :-(
Please let me know if you know of bindings/reimplementations of libsensor for C#, I's not actually a language I use very often so your the expert here. ;-) Otherwise I'll get to work writing that binding tomorrow.

Note to self: Some demo code

@hirschmann
Copy link
Owner

I know of a C# wrapper for libsensors , which is exactly what you've probably been looking for.

If you want to implement the new plugin, please let me know, so that I don't work on the same stuff as you. ;)

@github-actions
Copy link

This issue is stale because it has been open more than 180 days with no activity. If nobody comments within 7 days, this issue will be closed

@github-actions github-actions bot added the Stale label Dec 13, 2019
FedorChervyakov pushed a commit to FedorChervyakov/nbfc that referenced this issue Jul 30, 2021
Create Add HP Spectre x360 Convertible 15-df1xxx config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants