Add PMIC temperature reading (vcgencmd measure_temp pmic)

README.md

@@ -14,8 +14,8 @@ automation systems like openHAB). Runs as a systemd service.
   active `cpufreq` scaling governor
 - **Load average** - 1/5/15 minute values shown as gauges, scaled to CPU
   core count
-- **CPU temperature** - auto-detected thermal zone, with an optional
-  `vcgencmd`-sourced GPU temperature if available
+- **CPU temperature** - auto-detected thermal zone, with optional
+  `vcgencmd`-sourced GPU and PMIC (Pi 4/5) temperatures if available
 - **Memory & swap** usage
 - **Filesystem usage** - per mounted filesystem, pseudo filesystems
   (tmpfs, proc, overlay, ...) excluded by default

docs/API.md

@@ -128,6 +128,7 @@ extract the fields you need (e.g. via JSONPath in openHAB's HTTP binding).
   "cpu_count": 4,
   "temperature": { "zone": "cpu-thermal", "celsius": 48.6 },
   "gpu_temperature": { "celsius": 47.8 },
+  "pmic_temperature": { "celsius": 52.1 },
   "throttled": {
     "under_voltage_now": false,
     "frequency_capped_now": false,
@@ -261,7 +262,16 @@ Notes:
   and a core that is offline or whose driver doesn't expose both files is
   simply left out rather than failing the whole reading.
 - `gpu_temperature` is only present if `vcgencmd` is installed and
-  responded successfully; otherwise the field is omitted.
+  responded successfully; otherwise the field is omitted. Note that it is
+  not a second physical sensor: CPU and GPU share the SoC die, so this
+  reads the same sensor as `temperature`, by a different route.
+- `pmic_temperature` is the Power-Management IC's own sensor — genuinely
+  separate silicon from the SoC, and useful for spotting power-delivery or
+  board-level heat distinct from CPU load. It exists only on the Raspberry
+  Pi 4 and 5, is read via `vcgencmd measure_temp pmic`, and is omitted
+  (exactly like `gpu_temperature`) whenever `vcgencmd` is unavailable or
+  the board has no PMIC sensor. It is not exposed through sysfs hwmon on
+  Raspberry Pi OS, so it never appears in `sensors` either.
 - `throttled` decodes the Raspberry Pi `vcgencmd get_throttled` bitmask.
   The `*_now` flags reflect the current state; the `*_since_boot` flags
   latch whether the condition has occurred at any point since boot. A set
@@ -317,7 +327,8 @@ Notes:
 - These are additive to `v1`: `GET /api/v1/metrics` is unchanged, and
   keeps returning every field, including the ones with no endpoint of
   their own (`timestamp`, `uptime_seconds`, `load_average`, `cpu_count`,
-  `cpu_frequency`, `swap`, `gpu_temperature`, `throttled`, `system`,
+  `cpu_frequency`, `swap`, `gpu_temperature`, `pmic_temperature`,
+  `throttled`, `system`,
   `disk_io`, `wireless`, `sensors`).
   Poll the full snapshot if you need several metrics at once — six narrow
   requests cost more than one full one.
@@ -718,6 +729,7 @@ Metrics exposed (all gauges, prefixed `pimonitor_`):
 | `cpu_core_usage_percent` | `core` (0-based index) | Omitted entirely on platforms without per-core data |
 | `temperature_celsius` | `zone` | Omitted entirely — the whole family is skipped — whenever the most recent temperature collection failed (e.g. no readable thermal zone) or hasn't completed yet; a `0` reading is never fabricated for a missing sensor |
 | `gpu_temperature_celsius` | — | Only present when `vcgencmd` responded, like `gpu_temperature` in `GET /api/v1/metrics` |
+| `pmic_temperature_celsius` | — | Only present when `vcgencmd measure_temp pmic` responded (Raspberry Pi 4/5), like `pmic_temperature` in `GET /api/v1/metrics` |
 | `memory_total_bytes`, `memory_available_bytes`, `memory_used_percent` | — | |
 | `swap_total_bytes`, `swap_used_bytes`, `swap_used_percent` | — | |
 | `disk_total_bytes`, `disk_used_bytes`, `disk_used_percent` | `mount` | One series per mounted filesystem, same set as `disks` in `GET /api/v1/metrics` (pseudo-filesystems and network filesystems already excluded) |

docs/ARCHITECTURE.md

@@ -337,8 +337,9 @@ Left at its default (`healthz_max_staleness_seconds: 0`), that bound is `3 *
 poll_interval_seconds + 2 * collector.WorstCaseTickOverhead`, not just the poll
 interval alone: `Collector.fastTick` (`collector.go`) publishes `latest.Timestamp`
 only when a tick *completes*, sequentially running collectors that themselves
-degrade via timeout rather than fail fast — `TemperatureCollector`/`ThrottledCollector`
-each bound a hung `vcgencmd` call at `vcgencmdTimeout`, `DiskCollector` bounds a
+degrade via timeout rather than fail fast — `TemperatureCollector` (twice, for
+`measure_temp` and `measure_temp pmic`) and `ThrottledCollector` each bound a hung
+`vcgencmd` call at `vcgencmdTimeout`, `DiskCollector` bounds a
 stalled `statfs` at `defaultStatfsTimeout` — so a single legitimately slow tick can
 already take `collector.WorstCaseTickOverhead`, and the timestamp visible right
 before the *next* tick publishes can lag by up to twice that. Ignoring this would
@@ -536,7 +537,7 @@ apt package cache (`apt-get update`) requires root while reading its result
   world-readable files under `/proc`, `/sys/class/thermal`,
   `/sys/devices/system/cpu/*/cpufreq`, `/sys/class/hwmon`, `/etc/os-release`, and the
   existing apt cache, plus the read-only `apt list --upgradable` command and the optional
-  `vcgencmd measure_temp` / `vcgencmd get_throttled` commands — all invoked with fixed
+  `vcgencmd measure_temp` / `vcgencmd measure_temp pmic` / `vcgencmd get_throttled` commands — all invoked with fixed
   argument lists (never user input interpolated into a shell command), and further
   sandboxed via systemd unit hardening directives.
 - **`pimonitor-apt-update.timer`** runs as root, on a schedule (every 6h), and performs

internal/collector/collector.go

@@ -149,9 +149,13 @@ func dropPrefix(points []HistoryPoint, keep func(HistoryPoint) bool) []HistoryPo
 
 // WorstCaseTickOverhead is the most a single fastTick may legitimately run
 // over instant /proc-style reads before c.latest is updated. Within
-// collectFastTickSamples, TemperatureCollector and ThrottledCollector each
-// shell out to vcgencmd (bounded by vcgencmdTimeout), and these run
-// sequentially, not concurrently, so their worst case is additive.
+// collectFastTickSamples, TemperatureCollector shells out to vcgencmd twice
+// (`measure_temp` and `measure_temp pmic`) and ThrottledCollector once
+// (`get_throttled`), each bounded by vcgencmdTimeout, and these run
+// sequentially, not concurrently, so their worst case is additive — hence
+// the factor of three below, which must be kept in step with the number of
+// vcgencmd invocations a tick makes (see
+// TestWorstCaseTickOverhead_CoversEveryVcgencmdInvocation).
 // DiskCollector bounds a stalled statfs at defaultStatfsTimeout — counted
 // once here, which is the common case: a single dying device or
 // unresponsive network mount. Several mounts stalling at the same time cost
@@ -162,7 +166,7 @@ func dropPrefix(points []HistoryPoint, keep func(HistoryPoint) bool) []HistoryPo
 // firmware call or an unresponsive mount, both of which the collector
 // deliberately degrades rather than dies on — isn't mistaken for a stalled
 // collector.
-const WorstCaseTickOverhead = 2*vcgencmdTimeout + defaultStatfsTimeout
+const WorstCaseTickOverhead = 3*vcgencmdTimeout + defaultStatfsTimeout
 
 // Collector periodically samples every metric source and keeps the latest
 // snapshot plus a bounded in-memory history per metric.
@@ -454,6 +458,7 @@ type fastTickSamples struct {
 	load       LoadAverage
 	temp       Temperature
 	gpuTemp    *GPUTemperature
+	pmicTemp   *PMICTemperature
 	tempErr    error
 	throttled  *Throttled
 	mem        Memory
@@ -491,7 +496,7 @@ func (c *Collector) collectFastTickSamples(ctx context.Context) fastTickSamples
 	if err != nil {
 		c.log.Warn("load average collection failed", "error", err)
 	}
-	s.temp, s.gpuTemp, s.tempErr = c.temp.Collect(ctx)
+	s.temp, s.gpuTemp, s.pmicTemp, s.tempErr = c.temp.Collect(ctx)
 	if s.tempErr != nil {
 		c.log.Warn("temperature collection failed", "error", s.tempErr)
 	}
@@ -548,6 +553,7 @@ func (c *Collector) fastTick(ctx context.Context) {
 	c.latest.Temperature = s.temp
 	c.latest.TemperatureValid = s.tempErr == nil
 	c.latest.GPUTemperature = s.gpuTemp
+	c.latest.PMICTemperature = s.pmicTemp
 	c.latest.Throttled = s.throttled
 	c.latest.Memory = s.mem
 	c.latest.Swap = s.swap

internal/collector/collector_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -337,6 +338,78 @@ func TestCollector_FastTick_Hwmon(t *testing.T) {
 	}
 }
 
+// fakeVcgencmdCollector points a Collector's two vcgencmd-backed
+// collectors at one fake binary that logs every invocation to countFile,
+// so a test can assert both what a tick reads and how many firmware calls
+// it costs. The fake answers get_throttled, measure_temp and measure_temp
+// pmic the way a Pi 4/5 would.
+func fakeVcgencmdCollector(t *testing.T, c *Collector) (countFile string) {
+	t.Helper()
+	dir := t.TempDir()
+	countFile = filepath.Join(dir, "invocations")
+	path := writeFakeVcgencmd(t, dir, "fake-vcgencmd", `echo "$@" >> `+countFile+`
+if [ "$1" = "get_throttled" ]; then
+  echo "throttled=0x0"
+elif [ "$2" = "pmic" ]; then
+  echo "temp=52.1'C"
+else
+  echo "temp=42.8'C"
+fi`)
+	vcg := &vcgencmdRunner{detected: true, path: path}
+
+	zoneRoot := t.TempDir()
+	writeThermalZone(t, zoneRoot, "thermal_zone0", "cpu-thermal", "50000")
+	c.temp = &TemperatureCollector{
+		zonePath: filepath.Join(zoneRoot, "thermal_zone0"),
+		zoneType: "cpu-thermal",
+		vcg:      vcg,
+	}
+	c.throttled = &ThrottledCollector{vcg: vcg}
+	return countFile
+}
+
+// TestCollector_FastTick_PMICTemperature covers issue #56's wiring: the
+// PMIC reading collected alongside the GPU/SoC one must reach the
+// published snapshot.
+func TestCollector_FastTick_PMICTemperature(t *testing.T) {
+	c := newTestCollector()
+	fakeVcgencmdCollector(t, c)
+
+	c.fastTick(context.Background())
+
+	snap := c.Snapshot()
+	if snap.GPUTemperature == nil || snap.GPUTemperature.Celsius != 42.8 {
+		t.Fatalf("GPUTemperature = %+v, want Celsius=42.8", snap.GPUTemperature)
+	}
+	if snap.PMICTemperature == nil || snap.PMICTemperature.Celsius != 52.1 {
+		t.Fatalf("PMICTemperature = %+v, want Celsius=52.1", snap.PMICTemperature)
+	}
+}
+
+// TestWorstCaseTickOverhead_CoversEveryVcgencmdInvocation keeps the
+// /healthz staleness budget honest: every vcgencmd call a fast tick makes
+// runs sequentially and can each stall for up to vcgencmdTimeout, so
+// WorstCaseTickOverhead must budget for all of them. Adding a firmware
+// call (issue #56 added `measure_temp pmic`) without widening the constant
+// would make /healthz flap on a Pi whose only problem is slow firmware, so
+// this counts the invocations rather than trusting the constant's comment.
+func TestWorstCaseTickOverhead_CoversEveryVcgencmdInvocation(t *testing.T) {
+	c := newTestCollector()
+	countFile := fakeVcgencmdCollector(t, c)
+
+	c.fastTick(context.Background())
+
+	logged, err := os.ReadFile(countFile)
+	if err != nil {
+		t.Fatalf("read vcgencmd invocation log: %v", err)
+	}
+	invocations := len(strings.Split(strings.TrimSpace(string(logged)), "\n"))
+	if want := time.Duration(invocations)*vcgencmdTimeout + defaultStatfsTimeout; WorstCaseTickOverhead < want {
+		t.Fatalf("WorstCaseTickOverhead = %v, want at least %v: a fast tick makes %d vcgencmd invocations (%s), each bounded by %v\ninvocations:\n%s",
+			WorstCaseTickOverhead, want, invocations, countFile, vcgencmdTimeout, logged)
+	}
+}
+
 // TestCollector_FastTick_HwmonDisabled verifies that HwmonEnabled: false
 // skips hwmon collection entirely, mirroring the network disabled test
 // above.

internal/collector/temperature.go

@@ -73,8 +73,8 @@ func readThermalZoneMilliC(zonePath string) (float64, error) {
 	return float64(milliC) / 1000, nil
 }
 
-// TemperatureCollector reads CPU temperature from sysfs, with an optional
-// vcgencmd-sourced GPU/SoC reading on Raspberry Pi OS.
+// TemperatureCollector reads CPU temperature from sysfs, with optional
+// vcgencmd-sourced GPU/SoC and PMIC readings on Raspberry Pi OS.
 //
 // The thermal zone is resolved lazily and re-resolved (throttled) when it
 // is still missing, so a sensor or driver that appears after the process
@@ -90,7 +90,18 @@ type TemperatureCollector struct {
 	zonePath       string
 	zoneType       string
 	lastZoneDetect time.Time
-	vcg            *vcgencmdRunner // nil disables the GPU/SoC reading
+	vcg            *vcgencmdRunner // nil disables the GPU/SoC and PMIC readings
+
+	// pmicUnsupported latches true once vcgencmd has run successfully but
+	// answered `measure_temp pmic` with something other than a temperature
+	// reading (see errVcgencmdUnsupportedOutput) — in practice, the board
+	// has no PMIC sensor, a Pi 3 and earlier. Unlike a failed exec or a
+	// timeout, which are transient and worth retrying, that outcome can
+	// never become false at runtime, so latching it avoids paying a
+	// pointless vcgencmd invocation on every fast tick for the rest of the
+	// process's life. A real exec/timeout failure does not set this flag
+	// and keeps retrying, same as the GPU/SoC reading.
+	pmicUnsupported bool
 }
 
 // NewTemperatureCollector auto-detects the CPU thermal zone. Detection
@@ -99,7 +110,8 @@ type TemperatureCollector struct {
 // development off-Pi). If the zone is missing at construction, Collect
 // re-attempts detection at most once every detectRetryInterval, so a sensor
 // that shows up later is used automatically. vcg is the vcgencmd runner
-// shared with ThrottledCollector; pass nil to disable the GPU/SoC reading.
+// shared with ThrottledCollector; pass nil to disable the GPU/SoC and PMIC
+// readings.
 func NewTemperatureCollector(vcg *vcgencmdRunner) *TemperatureCollector {
 	c := &TemperatureCollector{zoneGlob: thermalZoneGlob, now: time.Now, vcg: vcg}
 	c.redetectZoneLocked()
@@ -125,8 +137,8 @@ func (c *TemperatureCollector) redetectZoneLocked() {
 }
 
 // Collect returns the current CPU temperature and, if vcgencmd is
-// available, the GPU/SoC temperature as a secondary reading.
-func (c *TemperatureCollector) Collect(ctx context.Context) (Temperature, *GPUTemperature, error) {
+// available, the GPU/SoC and PMIC temperatures as secondary readings.
+func (c *TemperatureCollector) Collect(ctx context.Context) (Temperature, *GPUTemperature, *PMICTemperature, error) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
@@ -137,7 +149,7 @@ func (c *TemperatureCollector) Collect(ctx context.Context) (Temperature, *GPUTe
 
 	c.redetectZoneLocked()
 	if c.zonePath == "" {
-		return Temperature{}, nil, fmt.Errorf("no CPU thermal zone detected")
+		return Temperature{}, nil, nil, fmt.Errorf("no CPU thermal zone detected")
 	}
 	celsius, err := readThermalZoneMilliC(c.zonePath)
 	if err != nil {
@@ -151,44 +163,75 @@ func (c *TemperatureCollector) Collect(ctx context.Context) (Temperature, *GPUTe
 			}
 		}
 		if err != nil {
-			return Temperature{}, nil, err
+			return Temperature{}, nil, nil, err
 		}
 	}
 	temp := Temperature{Zone: c.zoneType, Celsius: celsius}
 
-	gpuTemp, err := c.readVcgencmdTemp(ctx)
-	if err != nil {
-		// vcgencmd is an optional extra data point; its unavailability or
-		// failure should not fail the whole collection.
-		return temp, nil, nil
+	// The vcgencmd readings are optional extra data points: unavailability
+	// or failure must fail neither the whole collection nor each other. The
+	// PMIC sensor in particular exists only on the Pi 4/5, so on older
+	// boards `measure_temp` succeeds while `measure_temp pmic` does not.
+	var gpuTemp *GPUTemperature
+	if gpuC, err := c.readVcgencmdTemp(ctx); err == nil {
+		gpuTemp = &GPUTemperature{Celsius: gpuC}
+	}
+	var pmicTemp *PMICTemperature
+	if !c.pmicUnsupported {
+		pmicC, err := c.readVcgencmdTemp(ctx, "pmic")
+		switch {
+		case err == nil:
+			pmicTemp = &PMICTemperature{Celsius: pmicC}
+		case errors.Is(err, errVcgencmdUnsupportedOutput):
+			// vcgencmd ran and answered, just not with a PMIC reading:
+			// this board has no PMIC sensor, which cannot change at
+			// runtime. Stop asking.
+			c.pmicUnsupported = true
+		}
 	}
-	return temp, &gpuTemp, nil
+	return temp, gpuTemp, pmicTemp, nil
 }
 
-// readVcgencmdTemp runs `vcgencmd measure_temp` (via the shared vcg runner)
-// and parses output of the form "temp=42.8'C".
-func (c *TemperatureCollector) readVcgencmdTemp(ctx context.Context) (GPUTemperature, error) {
+// readVcgencmdTemp runs `vcgencmd measure_temp [args...]` (via the shared
+// vcg runner) and parses output of the form "temp=42.8'C". args carries the
+// subcommand's own arguments: none for the GPU/SoC die reading, "pmic" for
+// the Power-Management IC's own sensor.
+func (c *TemperatureCollector) readVcgencmdTemp(ctx context.Context, args ...string) (float64, error) {
 	if c.vcg == nil {
-		return GPUTemperature{}, errVcgencmdUnavailable
+		return 0, errVcgencmdUnavailable
 	}
-	out, err := c.vcg.run(ctx, "measure_temp")
+	out, err := c.vcg.run(ctx, "measure_temp", args...)
 	if err != nil {
-		return GPUTemperature{}, err
+		return 0, err
 	}
 	return parseVcgencmdTemp(out)
 }
 
-func parseVcgencmdTemp(output string) (GPUTemperature, error) {
+// errVcgencmdUnsupportedOutput indicates vcgencmd executed successfully but
+// answered with something other than a "temp=NN.N'C" line — its
+// "error=1 error_msg=..." response, or any other output parseVcgencmdTemp
+// doesn't recognize. For a sensor-specific subcommand (measure_temp pmic)
+// this means the requested sensor does not exist on this board: a
+// permanent condition for the life of the process, unlike a failed exec or
+// a timeout, which are transient and worth retrying. Callers that want to
+// tell the two apart (see TemperatureCollector.pmicUnsupported) check for
+// this with errors.Is.
+var errVcgencmdUnsupportedOutput = errors.New("vcgencmd: output is not a temperature reading")
+
+// parseVcgencmdTemp decodes vcgencmd's "temp=NN.N'C" output into degrees
+// Celsius. The form is identical for every measure_temp variant, so the
+// GPU/SoC and PMIC readings share this parser.
+func parseVcgencmdTemp(output string) (float64, error) {
 	output = strings.TrimSpace(output)
 	const prefix = "temp="
 	if !strings.HasPrefix(output, prefix) {
-		return GPUTemperature{}, fmt.Errorf("unexpected vcgencmd output: %q", output)
+		return 0, fmt.Errorf("%w: unexpected vcgencmd output: %q", errVcgencmdUnsupportedOutput, output)
 	}
 	rest := strings.TrimPrefix(output, prefix)
 	rest = strings.TrimSuffix(rest, "'C")
 	celsius, err := strconv.ParseFloat(rest, 64)
 	if err != nil {
-		return GPUTemperature{}, fmt.Errorf("parse vcgencmd temp %q: %w", output, err)
+		return 0, fmt.Errorf("%w: parse vcgencmd temp %q: %w", errVcgencmdUnsupportedOutput, output, err)
 	}
-	return GPUTemperature{Celsius: celsius}, nil
+	return celsius, nil
 }