From 6404b6a5fc36f745875cd7a66a71d73ecbfc87e1 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 5 Nov 2025 18:47:21 +0000 Subject: [PATCH] Expand temperature sensor compatibility for SuperIO and AMD CPUs Users with NCT6687 SuperIO chips and AMD processors reporting only chiplet temperatures were unable to see CPU temperature data. Added support for Nuvoton/Winbond/Fintek SuperIO chips and AMD Tccd chiplet temperatures, with debug logging to aid troubleshooting unsupported sensor configurations. Related to discussion #586 --- internal/monitoring/temperature.go | 93 +++++++++++++++++ internal/monitoring/temperature_test.go | 128 ++++++++++++++++++++++++ 2 files changed, 221 insertions(+) diff --git a/internal/monitoring/temperature.go b/internal/monitoring/temperature.go index 544b505..b0043d2 100644 --- a/internal/monitoring/temperature.go +++ b/internal/monitoring/temperature.go @@ -326,9 +326,25 @@ func (tc *TemperatureCollector) parseSensorsJSON(jsonStr string) (*models.Temper strings.Contains(chipLower, "k8temp") || strings.Contains(chipLower, "acpitz") || strings.Contains(chipLower, "it87") || + strings.Contains(chipLower, "nct6687") || // Nuvoton NCT6687 SuperIO + strings.Contains(chipLower, "nct6775") || // Nuvoton NCT6775 SuperIO + strings.Contains(chipLower, "nct6776") || // Nuvoton NCT6776 SuperIO + strings.Contains(chipLower, "nct6779") || // Nuvoton NCT6779 SuperIO + strings.Contains(chipLower, "nct6791") || // Nuvoton NCT6791 SuperIO + strings.Contains(chipLower, "nct6792") || // Nuvoton NCT6792 SuperIO + strings.Contains(chipLower, "nct6793") || // Nuvoton NCT6793 SuperIO + strings.Contains(chipLower, "nct6795") || // Nuvoton NCT6795 SuperIO + strings.Contains(chipLower, "nct6796") || // Nuvoton NCT6796 SuperIO + strings.Contains(chipLower, "nct6797") || // Nuvoton NCT6797 SuperIO + strings.Contains(chipLower, "nct6798") || // Nuvoton NCT6798 SuperIO + strings.Contains(chipLower, "w83627") || // Winbond W83627 SuperIO series + strings.Contains(chipLower, "f71882") || // Fintek F71882 SuperIO strings.Contains(chipLower, "cpu_thermal") || // Raspberry Pi CPU temperature strings.Contains(chipLower, "rpitemp") { foundCPUChip = true + log.Debug(). + Str("chip", chipName). + Msg("Detected CPU temperature chip") tc.parseCPUTemps(chipMap, temp) } @@ -355,12 +371,34 @@ func (tc *TemperatureCollector) parseSensorsJSON(jsonStr string) (*models.Temper // Available means any temperature data exists (backward compatibility) temp.Available = temp.HasCPU || temp.HasNVMe + // Log summary of what was detected + if !foundCPUChip { + // List all chip names found for debugging + chipNames := make([]string, 0, len(sensorsData)) + for chipName := range sensorsData { + chipNames = append(chipNames, chipName) + } + log.Debug(). + Strs("chips", chipNames). + Msg("No recognized CPU temperature chip found in sensors output") + } else { + log.Debug(). + Bool("hasCPU", temp.HasCPU). + Bool("hasNVMe", temp.HasNVMe). + Float64("cpuPackage", temp.CPUPackage). + Float64("cpuMax", temp.CPUMax). + Int("coreCount", len(temp.Cores)). + Int("nvmeCount", len(temp.NVMe)). + Msg("Temperature data parsed successfully") + } + return temp, nil } // parseCPUTemps extracts CPU temperature data from a sensor chip func (tc *TemperatureCollector) parseCPUTemps(chipMap map[string]interface{}, temp *models.Temperature) { foundPackageTemp := false + var chipletTemps []float64 // Store AMD Tccd chiplet temps for fallback for sensorName, sensorData := range chipMap { sensorMap, ok := sensorData.(map[string]interface{}) @@ -380,6 +418,43 @@ func (tc *TemperatureCollector) parseCPUTemps(chipMap map[string]interface{}, te if tempVal > temp.CPUMax { temp.CPUMax = tempVal } + log.Debug(). + Str("sensor", sensorName). + Float64("temp", tempVal). + Msg("Found CPU package temperature") + } + } + + // Look for AMD chiplet temperatures (Tccd1, Tccd2, etc.) as fallback + if strings.HasPrefix(sensorName, "Tccd") { + if tempVal := extractTempInput(sensorMap); !math.IsNaN(tempVal) && tempVal > 0 { + chipletTemps = append(chipletTemps, tempVal) + if tempVal > temp.CPUMax { + temp.CPUMax = tempVal + } + log.Debug(). + Str("sensor", sensorName). + Float64("temp", tempVal). + Msg("Found AMD chiplet temperature") + } + } + + // Look for SuperIO chip CPU temperature fields (CPUTIN, CPU Temperature, etc.) + if strings.Contains(sensorNameLower, "cputin") || + strings.Contains(sensorNameLower, "cpu temperature") || + (strings.Contains(sensorNameLower, "temp") && strings.Contains(sensorNameLower, "cpu")) { + if tempVal := extractTempInput(sensorMap); !math.IsNaN(tempVal) && tempVal > 0 { + if !foundPackageTemp { + temp.CPUPackage = tempVal + foundPackageTemp = true + } + if tempVal > temp.CPUMax { + temp.CPUMax = tempVal + } + log.Debug(). + Str("sensor", sensorName). + Float64("temp", tempVal). + Msg("Found SuperIO CPU temperature") } } @@ -394,10 +469,28 @@ func (tc *TemperatureCollector) parseCPUTemps(chipMap map[string]interface{}, te if tempVal > temp.CPUMax { temp.CPUMax = tempVal } + log.Debug(). + Str("sensor", sensorName). + Int("core", coreNum). + Float64("temp", tempVal). + Msg("Found core temperature") } } } + // If no package temperature found, use highest chiplet temp (AMD Ryzen) + if !foundPackageTemp && len(chipletTemps) > 0 { + for _, chipletTemp := range chipletTemps { + if chipletTemp > temp.CPUPackage { + temp.CPUPackage = chipletTemp + } + } + foundPackageTemp = true + log.Debug(). + Float64("temp", temp.CPUPackage). + Msg("Using highest chiplet temperature as CPU package temperature") + } + // If no package temperature was found (e.g., Raspberry Pi), look for generic temp sensors if !foundPackageTemp { for sensorName, sensorData := range chipMap { diff --git a/internal/monitoring/temperature_test.go b/internal/monitoring/temperature_test.go index 926aa23..b43589a 100644 --- a/internal/monitoring/temperature_test.go +++ b/internal/monitoring/temperature_test.go @@ -582,3 +582,131 @@ func TestDisableLegacySSHOnAuthFailure(t *testing.T) { t.Fatalf("expected non-authentication errors to leave legacy SSH enabled") } } + +// TestParseSensorsJSON_NCT6687SuperIO tests NCT6687 SuperIO chip detection +func TestParseSensorsJSON_NCT6687SuperIO(t *testing.T) { + collector := &TemperatureCollector{} + + jsonStr := `{ + "nct6687-isa-0a20": { + "CPUTIN": {"temp1_input": 48.5}, + "SYSTIN": {"temp2_input": 35.0} + } + }` + + temp, err := collector.parseSensorsJSON(jsonStr) + if err != nil { + t.Fatalf("unexpected error parsing NCT6687 sensors output: %v", err) + } + if temp == nil { + t.Fatalf("expected temperature struct, got nil") + } + if !temp.Available { + t.Fatalf("expected temperature to be available when NCT6687 CPUTIN is present") + } + if !temp.HasCPU { + t.Fatalf("expected HasCPU to be true when NCT6687 chip is detected") + } + if temp.CPUPackage != 48.5 { + t.Fatalf("expected cpu package temperature 48.5 from CPUTIN, got %.2f", temp.CPUPackage) + } +} + +// TestParseSensorsJSON_AmdChipletTemps tests AMD Tccd chiplet temperature detection +func TestParseSensorsJSON_AmdChipletTemps(t *testing.T) { + collector := &TemperatureCollector{} + + jsonStr := `{ + "k10temp-pci-00c3": { + "Tccd1": {"temp3_input": 62.5}, + "Tccd2": {"temp4_input": 58.0} + } + }` + + temp, err := collector.parseSensorsJSON(jsonStr) + if err != nil { + t.Fatalf("unexpected error parsing AMD chiplet sensors output: %v", err) + } + if temp == nil { + t.Fatalf("expected temperature struct, got nil") + } + if !temp.Available { + t.Fatalf("expected temperature to be available when AMD chiplet temps are present") + } + if !temp.HasCPU { + t.Fatalf("expected HasCPU to be true when K10temp chip is detected") + } + // Should use highest chiplet temp as package temp + if temp.CPUPackage != 62.5 { + t.Fatalf("expected cpu package temperature to be highest chiplet temp (62.5), got %.2f", temp.CPUPackage) + } + // CPUMax should also be 62.5 + if temp.CPUMax != 62.5 { + t.Fatalf("expected cpu max temperature 62.5, got %.2f", temp.CPUMax) + } +} + +// TestParseSensorsJSON_AmdTctlAndChiplets tests AMD with both Tctl and chiplet temps +func TestParseSensorsJSON_AmdTctlAndChiplets(t *testing.T) { + collector := &TemperatureCollector{} + + jsonStr := `{ + "k10temp-pci-00c3": { + "Tctl": {"temp1_input": 65.0}, + "Tccd1": {"temp3_input": 62.5}, + "Tccd2": {"temp4_input": 58.0} + } + }` + + temp, err := collector.parseSensorsJSON(jsonStr) + if err != nil { + t.Fatalf("unexpected error parsing AMD full sensors output: %v", err) + } + if temp == nil { + t.Fatalf("expected temperature struct, got nil") + } + if !temp.Available { + t.Fatalf("expected temperature to be available") + } + if !temp.HasCPU { + t.Fatalf("expected HasCPU to be true") + } + // Tctl should take precedence over chiplet temps for package temperature + if temp.CPUPackage != 65.0 { + t.Fatalf("expected cpu package temperature from Tctl (65.0), got %.2f", temp.CPUPackage) + } + // CPUMax should be Tctl since it's highest + if temp.CPUMax != 65.0 { + t.Fatalf("expected cpu max temperature 65.0, got %.2f", temp.CPUMax) + } +} + +// TestParseSensorsJSON_MultipleSuperioCPUFields tests SuperIO chips with multiple CPU temp fields +func TestParseSensorsJSON_MultipleSuperioCPUFields(t *testing.T) { + collector := &TemperatureCollector{} + + jsonStr := `{ + "nct6775-isa-0290": { + "CPU Temperature": {"temp1_input": 52.0}, + "SYSTIN": {"temp2_input": 38.0}, + "AUXTIN0": {"temp3_input": 40.0} + } + }` + + temp, err := collector.parseSensorsJSON(jsonStr) + if err != nil { + t.Fatalf("unexpected error parsing NCT6775 sensors output: %v", err) + } + if temp == nil { + t.Fatalf("expected temperature struct, got nil") + } + if !temp.Available { + t.Fatalf("expected temperature to be available") + } + if !temp.HasCPU { + t.Fatalf("expected HasCPU to be true") + } + if temp.CPUPackage != 52.0 { + t.Fatalf("expected cpu package temperature from 'CPU Temperature' field (52.0), got %.2f", temp.CPUPackage) + } +}