sys-API icon indicating copy to clipboard operation
sys-API copied to clipboard

Select Temperature Sensor

Open fushi-5 opened this issue 11 months ago • 2 comments

Hello good. As you can change the temperature sensor, the one showing me the CPU temperature is incorrect. Thank you.

fushi-5 avatar Jan 27 '25 12:01 fushi-5

There's no support for configuring the temperature sensor today.

Can you tell me a bit more about your system.

  • What OS are you running
  • Have you been able to read the temperature from another program

Krillsson avatar Feb 07 '25 08:02 Krillsson

I'll add to this.

Running in Unraid with the Community Template. I am running with network mode Host. Getting the temperature of my motherboard.

Unraid it self can read the temperature fine.

hndrkx avatar Feb 19 '25 14:02 hndrkx

I'm afraid running into the same problem here. Motherboard temperature is showing in the CPU temp.

Also running it on unraid and installed through the community template.

I must say i just installed it just today and already very impressed. Nice work!👌

Mr-git-in-the-code avatar Feb 19 '25 22:02 Mr-git-in-the-code

This seems to be related to https://github.com/oshi/oshi/pull/2793

There's a chance that it will fix itself if I just bump the dependency.

Stay put 👍🏻

Krillsson avatar Feb 20 '25 05:02 Krillsson

I've pushed a build to the nightly tag. Could you test this one out and see if your issue is resolved?

Image

After you have tested, remove the :nightly again to go back to the main builds

Krillsson avatar Feb 22 '25 11:02 Krillsson

Does not work, do I need to remove and add the server in the app?

Edit: Tried readding it, still no change to the temp sensor.

hndrkx avatar Feb 22 '25 12:02 hndrkx

Same here! Tried it with the nightly build, but no show for the change of temp....

mygitcrazy avatar Feb 22 '25 18:02 mygitcrazy

I have managed to create a fix for this, build and tested on my Unraid. I will have to test it a little bit more on another system as well, but I think I have a fallback in place, unless creating a Unraid specific image is fine.

In short: Instead of using OSHI Sensors (which only uses and detects the motherboard temp, which I think is because it matches on "CPUTIN" from hwmon), we're dynamically looking up temperature sensors in /sys/class/hwmon/, then we are matching them with labels to corresponding temp*_label. We're using the "Package id 0" as a main sensor, but fallback is average core sensor. If all that fails, fallback to default OSHI.

I have to get some sleep, I might be able to make a pull request tomorrow.

hndrkx avatar Feb 23 '25 00:02 hndrkx

Sounds promising @hndrkx. Could you also have a look at https://github.com/oshi/oshi/pull/2793. If the changes in there matches with what you are building.

During my investigation I started off with trying out version 6.6.6 of OSHI, which has these changes linked in the PR above. But no matter how I configured the oshi.os.linux.sensors.cpuTemperature.types, I only got a static 27 degrees celsius. Which was worse from what I had before. On my unraid system, the CPU temperature is working even on 6.4.12.

The earlier release 6.6.5 also has changes related to finding the appropriate temperature: https://github.com/oshi/oshi/pull/2740. And that one got me the correct CPU temperature, which is why I pushed to :nightly and asked you to test.

From what I could understand from both of these changes in OSHI is that they seem to alter what to read from "class/thermal/thermal_zoneX", but this code is only invoked if reading from hwmon fails.

Krillsson avatar Feb 23 '25 07:02 Krillsson

Ah that PR really looks like it should solve the issue though, I'll have a go at trying to use that version and see what I can come up with. My approach was to edit DefaultCpuSensors.kt and incorporate a case where it took the temp from hwmon instead and fallback to OHSI, which works. However if we can get it to work with that version of OHSI it would be a lot better. I'll do some testing and get back if I find something.

hndrkx avatar Feb 23 '25 11:02 hndrkx

To better fit in with the architecture, could you make it so that DefaultCpuSensors stay as is. But there is a UnraidCpuSensors that is injected instead if unRAID is detected. You can control the injection from the SysApiConfiguration class.

If you are unfamiliar with spring, you can leave as is and I will provide further guidance when I have more time.

Krillsson avatar Feb 23 '25 12:02 Krillsson

Yes, but I have been investigating this a little bit and one problem is this line: https://github.com/oshi/oshi/blob/oshi-parent-6.6.6/oshi-core/src/main/java/oshi/hardware/platform/linux/LinuxSensors.java#L68

Which means we will have to do a workaround for Unraid, which should work for everyone. I will try adding a new UnraidCpuSensors as you asked.

hndrkx avatar Feb 23 '25 15:02 hndrkx

Alright I really tried to implement this on my own, but since I rarely write any Java and have never worked with spring I was defeated, and I am sure you'll do this a lot faster than me, I will provide the code below.

So what we need: Some way to know if we are running Unraid, this is hard since we're executing this inside a container, easiest way would probably be to introduce a new ENV variable, something simple like UNRAID true/false(default). We need to use UnraidCpuSensors instead of DefaultCpuSensors if UNRAID.

We need this code (This is what I used and replaced original code in DefaultCpuSensors):

import java.io.File

    open fun cpuTemperatures(): List<Double> {
        val thermalBasePath = "/sys/class/thermal"
        val thermalZones = File(thermalBasePath).listFiles { file ->
            file.isDirectory && file.name.startsWith("thermal_zone")
        }

        val temperatures = mutableListOf<Double>()

        if (thermalZones != null) {
            for (zone in thermalZones) {
                try {
                    val type = File(zone, "type").readText().trim()
                    val temp = File(zone, "temp").readText().trim().toDouble() / 1000.0

                    if (type == "x86_pkg_temp") {
                        return listOf(temp)
                    }

                    temperatures.add(temp)

                } catch (e: Exception) {
                    println("DEBUG: Error reading from ${zone.name}")
                }
            }
        }

        return temperatures.ifEmpty { listOf(0.0) }
    }

hndrkx avatar Feb 23 '25 19:02 hndrkx

I can have a look at implementing it. Thanks for the support with the investigation.

Knowing that we are running on unRAID is easy now, since I recently added that information to the API.

Something tells me that the mappings can look different on different systems and that it's not something unique to the distro. Since it works for me, but not for some of you.

Krillsson avatar Feb 24 '25 06:02 Krillsson

Well looking at /sys/class/thermal should work on most linux dists, and filtering on x86_pkg_temp should also work (since Unraid is x86 only and do not support running on other architecture). You could always fall back to OSHI if those files are not found.

I guess it works for you since you're motherboard does not have a dedicated temperature sensor that is exposed? What is your first found temp_input sensor under /sys/class/hwmon* ? For example checking my laptop I have: /sys/class/hwmon/hwmon1/temp_input1 And the name for that is: /sys/class/hwmon/hwmon1/name acpitz

if your first temp_input is your cpu temp, that is why it works for you.

hndrkx avatar Feb 24 '25 09:02 hndrkx

I have another :nightly build I'd like you to test out.

What I did was to mimic how the implementation works in oshi. It saves all possible sensors to an internal state during startup. Then when its time to query the sensor it selects it by a certain criteria.

this code will first see if the user configured a specific sensor name to query, if there is no configured it will fallback to a known set of sensor types.

During startup it will print out the available sensors on your system so that you can override it if you like. Here's mine:

Available Sensors:
ThermalZone:pch_skylake 37.0 C
ThermalZone:acpitz 27.8 C
ThermalZone:x86_pkg_temp 60.0 C
ThermalZone:acpitz 29.8 C
Hwmon:coretemp 58.8 C
Hwmon:acpitz 28.8 C
Hwmon:pch_skylake 37.0 C

configure it like this:

linux:
  overrideCpuTempSensor: "hwmon:coretemp"
  journalLogs:
    enabled: true
  systemDaemonServiceManagement:
    enabled: true
logReader:

this is a bit work in progress still, as there's no distinction between thermalzone and hwmon in the configurable value. And there could be multiple sensors with the same name/type

Krillsson avatar Feb 24 '25 20:02 Krillsson

This is actually quite clean implementation, nice done. It works well on my system without any override since x86_pkg_temp is one of the possibleCpuSensors.

I do not think you have to make a distinction between them, whichever thermal sensor you find from the list of possibleCpuSensors, would it be from the ThermalZone or Hwmon should be fine.

If you have multiple sensors with the same name, that should not cause a problem because all CPU sensors should report the same temperature, not like acpitz which in your system as an example probably references 3 different temperature zones on your motherboard. And your implementation of sensors.firstOrNull should be enough.

hndrkx avatar Feb 24 '25 20:02 hndrkx

Implementation is now done. Docker image should be updated shortly.

Krillsson avatar Feb 25 '25 20:02 Krillsson