Warn in the navbar when a hard drive runs hot (#470)

* Warn in the navbar when a hard drive runs hot.

The navbar already warns about high CPU temperature; add the equivalent
for drive temperature, since an overheating disk risks data loss and a
user running an off-grid library won't be watching for it.

- controller/lib/smart.py: build_smart_stats() returns the per-drive list
  plus warning thresholds (55C warn / 65C critical) so the frontend stays
  dumb. Reading SMART wakes a spun-down drive, so drives in standby are
  detected with `smartctl -n standby` (which works through the USB-SATA
  bridges where `hdparm -C` only reports "unknown") and NOT read; their
  last-known reading is reused instead.
- controller/lib/status_worker.py: collect SMART on a slow 60s cadence
  (cached on app.state) rather than the 5s status loop, so the poll does
  not keep USB drives awake. The result flows into the same cached payload
  via a new collect_all_status(smart_stats=...) kwarg.
- Frontend: useDriveTemperature() reports the hottest drive; Nav.js shows
  a yellow/red `hdd` icon with a popup, mirroring the CPU warning.

Verified on hardware: smart_stats reports real temps and SMART runs once
a minute, not every 5s.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Address Greptile review on HDD temperature warning.

- _drive_is_asleep also treats SLEEP mode as asleep, not just STANDBY.
  smartctl -n standby exits "Device is in SLEEP mode, exit(2)" for the
  deeper power mode; matching only "STANDBY" meant a SLEEP drive was read
  and woken — the exact failure the check prevents.  (TDD: added a failing
  SLEEP test, then fixed.)
- useDriveTemperature uses ?? instead of || for the temperature fallback
  to express "fall back only on null/undefined".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
lrnselfreliance 2026-06-03 18:46:29 -06:00 • committed by Roland
parent ef4512614a
commit 1bd102c768
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
8 changed files with 450 additions and 8 deletions

View file

@ -4,7 +4,15 @@ import {Dropdown, Icon as SIcon, Menu} from "semantic-ui-react";
import {Media, SettingsContext, StatusContext, ThemeContext} from "../contexts/contexts";
import {DarkModeToggle, HotspotStatusIcon, useLocalStorage} from "./Common";
import {ShareButton} from "./Share";
import {useCPUTemperature, useIOStats, useLoad, useMemoryStats, usePowerStats, useWROLMode} from "../hooks/customHooks";
import {
useCPUTemperature,
useDriveTemperature,
useIOStats,
useLoad,
useMemoryStats,
usePowerStats,
useWROLMode
} from "../hooks/customHooks";
import {useReorganizationStatus} from "../contexts/FileWorkerStatusContext";
import {SearchIconButton} from "./Search";
import {Icon, Popup} from "./Theme";
@ -174,6 +182,21 @@ export function NavBar() {
temperatureIcon = <Popup content={`CPU: ${temperature.toFixed()}°C`} trigger={link}/>;
}
// Hard-drive temperature. An overheating drive risks data loss.
const {
device: hotDrive,
temperature: driveTemperature,
highTemperature: driveHighTemperature,
criticalTemperature: driveCriticalTemperature,
} = useDriveTemperature();
let driveTemperatureIcon;
if (driveTemperature && driveTemperature >= driveHighTemperature) {
const color = driveTemperature >= driveCriticalTemperature ? highWarningColor : lowWarningColor;
const icon = <Icon data-testid='driveTemperatureIcon' name='hdd' size='large' color={color}/>
const link = <Link to='/admin/status'>{icon}</Link>;
driveTemperatureIcon = <Popup content={`${hotDrive}: ${driveTemperature.toFixed()}°C`} trigger={link}/>;
}
// Power issues, this is always displayed if detected.
const {underVoltage, overCurrent} = usePowerStats();
let powerIcon;
@ -189,7 +212,7 @@ export function NavBar() {
// Display the temperature icon first because it can cause the system to throttle. The rest are in order of effects
// that will slow down the system and the user should address. Generic load is last because it is probably not an
// issue.
const warningIcon = temperatureIcon || diskWaitIcon || memoryIcon || systemLoadIcon;
const warningIcon = temperatureIcon || driveTemperatureIcon || diskWaitIcon || memoryIcon || systemLoadIcon;
const {isReorganizing, taskType, collectionId, collectionKind} = useReorganizationStatus();

View file

@ -1335,6 +1335,28 @@ export const useCPUTemperature = () => {
return {temperature, highTemperature, criticalTemperature}
}
// Reports the hottest drive's SMART temperature so the navbar can warn about
// an overheating disk, mirroring useCPUTemperature. Drives without a
// temperature reading (USB flash, unsupported bridges, spun-down) are ignored.
export const useDriveTemperature = () => {
const {status} = React.useContext(StatusContext);
const smart = status?.smart_stats;
const highTemperature = smart?.high_temperature || 55;
const criticalTemperature = smart?.critical_temperature || 65;
const drives = Array.isArray(smart?.drives) ? smart.drives : [];
const hottest = drives
.filter((d) => typeof d.temperature === 'number')
.reduce((max, d) => (max === null || d.temperature > max.temperature ? d : max), null);
return {
device: hottest?.device || null,
temperature: hottest?.temperature ?? 0,
highTemperature,
criticalTemperature,
}
}
export const useLoad = () => {
const {status} = React.useContext(StatusContext);
let minute_1 = status?.load_stats?.minute_1;

View file

@ -1,8 +1,8 @@
import React from 'react';
import {act, renderHook} from '@testing-library/react';
import {render, screen} from '@testing-library/react';
import {usePages} from './customHooks';
import {QueryContext} from '../contexts/contexts';
import {usePages, useDriveTemperature} from './customHooks';
import {QueryContext, StatusContext} from '../contexts/contexts';
import {Paginator} from '../components/Common';
// Build a wrapper that provides a controlled QueryContext so usePages can read
@ -95,6 +95,73 @@ describe('usePages.setTotal -> totalPages', () => {
});
});
describe('useDriveTemperature', () => {
const makeStatusWrapper = (status) => ({children}) => (
<StatusContext.Provider value={{status}}>
{children}
</StatusContext.Provider>
);
test('selects the hottest drive and reads thresholds from the payload', () => {
const status = {
smart_stats: {
drives: [
{device: 'sda', temperature: 51},
{device: 'sdb', temperature: 60},
],
high_temperature: 55,
critical_temperature: 65,
},
};
const {result} = renderHook(() => useDriveTemperature(),
{wrapper: makeStatusWrapper(status)});
expect(result.current.device).toBe('sdb');
expect(result.current.temperature).toBe(60);
expect(result.current.highTemperature).toBe(55);
expect(result.current.criticalTemperature).toBe(65);
});
test('ignores drives without a numeric temperature', () => {
const status = {
smart_stats: {
drives: [
{device: 'sda', temperature: null},
{device: 'sdb', temperature: 48},
],
high_temperature: 55,
critical_temperature: 65,
},
};
const {result} = renderHook(() => useDriveTemperature(),
{wrapper: makeStatusWrapper(status)});
expect(result.current.device).toBe('sdb');
expect(result.current.temperature).toBe(48);
});
test('defaults to no warning and default thresholds when smart_stats is absent', () => {
const {result} = renderHook(() => useDriveTemperature(),
{wrapper: makeStatusWrapper({})});
expect(result.current.device).toBeNull();
expect(result.current.temperature).toBe(0);
expect(result.current.highTemperature).toBe(55);
expect(result.current.criticalTemperature).toBe(65);
});
test('returns no drive when none report a temperature', () => {
const status = {
smart_stats: {
drives: [{device: 'sda', temperature: null}],
high_temperature: 55,
critical_temperature: 65,
},
};
const {result} = renderHook(() => useDriveTemperature(),
{wrapper: makeStatusWrapper(status)});
expect(result.current.device).toBeNull();
expect(result.current.temperature).toBe(0);
});
});
describe('Paginator rendering', () => {
test('renders exactly totalPages numbered page links (no phantom trailing link)', () => {
// 48 results at 24 per page -> 2 pages. If totalPages is 3, semantic-ui

View file

@ -29,6 +29,7 @@ _FAKE_CACHED_STATUS = {
"processes_stats": [],
"disk_bandwidth_stats": {},
"uptime_stats": {"uptime_seconds": 0},
"smart_stats": {"drives": [], "high_temperature": 55, "critical_temperature": 65},
"last_status": "1970-01-01T00:00:00",
}

View file

@ -2,6 +2,7 @@
SMART disk health monitoring for WROLPi Controller.
"""
import subprocess
from typing import Optional
# Import pySMART only if available
@ -14,6 +15,16 @@ except ImportError:
from controller.lib.config import is_docker_mode
# Warning thresholds for hard-drive temperature, in Celsius. Unlike CPU
# temperature, neither SMART nor psutil reports a recommended limit, so we
# carry sensible spinning-disk defaults in the payload (warn 55, critical 65)
# and let the frontend stay dumb.
HDD_HIGH_TEMPERATURE = 55
HDD_CRITICAL_TEMPERATURE = 65
SMARTCTL_SCAN_TIMEOUT = 10
SMARTCTL_STANDBY_TIMEOUT = 5
def is_smart_available() -> bool:
"""Check if SMART monitoring is available."""
@ -90,3 +101,108 @@ def _get_attribute(device, name: str) -> Optional[int]:
except (ValueError, TypeError):
return None
return None
def _scan_devices() -> list[tuple[str, Optional[str]]]:
"""List SMART-capable devices via ``smartctl --scan``.
``--scan`` only enumerates devices; it does not read SMART data, so it
does not wake a drive that has spun down. Returns (path, interface)
tuples, where interface is the ``-d`` type smartctl detected (e.g.
"sat") or None.
"""
try:
result = subprocess.run(
["smartctl", "--scan"],
capture_output=True, text=True, timeout=SMARTCTL_SCAN_TIMEOUT,
)
except (subprocess.SubprocessError, OSError):
return []
devices = []
for line in result.stdout.splitlines():
line = line.strip()
if not line or line.startswith("#"):
continue
# Format: "/dev/sda -d sat # /dev/sda [SAT], ATA device"
parts = line.split()
path = parts[0]
if not path.startswith("/dev/"):
continue
interface = None
if "-d" in parts:
idx = parts.index("-d")
if idx + 1 < len(parts):
interface = parts[idx + 1]
devices.append((path, interface))
return devices
def _drive_is_asleep(path: str) -> bool:
"""Return True if the drive at ``path`` is in standby/sleep.
Uses ``smartctl -n standby``, which checks the drive's power mode and
exits WITHOUT waking it when it is in standby. This goes through the
same SAT translation smartctl already uses to read these drives, so it
works on the USB-SATA bridges where ``hdparm -C`` only reports
"unknown". On any error we return False so the caller falls back to a
normal SMART read (better to read an awake drive than to never read it).
"""
try:
result = subprocess.run(
["smartctl", "-n", "standby", "-i", path],
capture_output=True, text=True, timeout=SMARTCTL_STANDBY_TIMEOUT,
)
except (subprocess.SubprocessError, OSError):
return False
# In a low-power mode smartctl prints e.g. "Device is in STANDBY mode,
# exit(2)" (or "SLEEP" for the deeper mode) and skips the data read. An
# awake -i read mentions neither.
output = result.stdout.upper()
return "STANDBY" in output or "SLEEP" in output
def build_smart_stats(previous: Optional[dict] = None) -> dict:
"""Collect SMART status for the navbar/Disk Management warning.
Reading SMART wakes a sleeping drive, so any drive in standby is NOT
read: instead we reuse its last-known data from ``previous`` (matched by
path). A spun-down drive is not hot, so this is safe. The warning
thresholds travel in the payload alongside the per-drive list so the
frontend does not hardcode them.
Returns ``{"drives": [...], "high_temperature": int,
"critical_temperature": int}``. In Docker mode or without pySMART the
drive list is empty.
"""
stats = {
"drives": [],
"high_temperature": HDD_HIGH_TEMPERATURE,
"critical_temperature": HDD_CRITICAL_TEMPERATURE,
}
if not is_smart_available():
return stats
previous_by_path = {}
if previous:
for drive in previous.get("drives", []):
previous_by_path[drive.get("path")] = drive
try:
for path, interface in _scan_devices():
if _drive_is_asleep(path):
# Don't wake it; reuse the last reading if we have one.
prior = previous_by_path.get(path)
if prior is not None:
stats["drives"].append(prior)
continue
try:
device = Device(path, interface=interface)
except Exception:
continue
stats["drives"].append(_get_device_smart(device))
except Exception:
# Never let a status-collection failure blank the whole payload.
pass
return stats

View file

@ -13,6 +13,7 @@ from typing import Optional
logger = logging.getLogger(__name__)
from controller.lib.smart import build_smart_stats
from controller.lib.status import (
get_cpu_status,
get_disk_bandwidth_status,
@ -29,13 +30,21 @@ from controller.lib.status import (
# Type alias for cached status dict
CachedStatus = dict
# SMART reads can wake spun-down USB drives, so collect them far less often
# than the fast (5s) stats.
SMART_REFRESH_SECONDS = 60
async def collect_all_status() -> CachedStatus:
async def collect_all_status(smart_stats: Optional[dict] = None) -> CachedStatus:
"""
Collect all status information concurrently.
Returns a dict with all status types. Individual failures return None
for that stat type rather than failing the entire collection.
``smart_stats`` is collected on a slower cadence by the worker loop (see
``status_worker_loop``) and passed in so it lands in the same cached
payload as the fast 5s stats.
"""
# Run all status collections concurrently using asyncio.to_thread
# since the status functions are synchronous (use psutil)
@ -65,6 +74,7 @@ async def collect_all_status() -> CachedStatus:
"processes_stats": processes if not isinstance(processes, Exception) else [],
"disk_bandwidth_stats": disk_bandwidth if not isinstance(disk_bandwidth, Exception) else {},
"uptime_stats": uptime if not isinstance(uptime, Exception) else None,
"smart_stats": smart_stats,
"last_status": datetime.now().isoformat(),
}
@ -101,6 +111,30 @@ def get_adaptive_sleep(load_stats: Optional[dict], base_sleep: float = 5.0) -> f
return base_sleep
async def refresh_smart_stats(app, now: datetime) -> dict:
"""Collect SMART stats, but no more than once per SMART_REFRESH_SECONDS.
SMART reads can wake spun-down USB drives, so this runs on a slow
cadence independent of the fast status loop. The result and its
timestamp are cached on ``app.state``; between refreshes the cached
value is returned unchanged. ``build_smart_stats`` is given the prior
result so it can reuse readings for drives that are now asleep.
"""
last = getattr(app.state, "smart_last_collected", None)
cache = getattr(app.state, "smart_cache", None)
due = (
cache is None
or last is None
or (now - last).total_seconds() >= SMART_REFRESH_SECONDS
)
if due:
# build_smart_stats shells out to smartctl/hdparm; keep it off the loop.
cache = await asyncio.to_thread(build_smart_stats, cache)
app.state.smart_cache = cache
app.state.smart_last_collected = now
return cache
async def status_worker_loop(app, base_sleep: float = 5.0):
"""
Background loop that continuously collects status data.
@ -116,8 +150,11 @@ async def status_worker_loop(app, base_sleep: float = 5.0):
while True:
try:
# SMART on its own slow cadence; everything else every cycle.
smart_stats = await refresh_smart_stats(app, datetime.now())
# Collect all status data
status = await collect_all_status()
status = await collect_all_status(smart_stats=smart_stats)
# Store in app.state (thread-safe for reading in single-worker uvicorn)
app.state.cached_status = status

View file

@ -7,9 +7,13 @@ from unittest import mock
from controller.lib.smart import (
is_smart_available,
get_all_smart_status,
build_smart_stats,
_get_device_smart,
_get_temperature,
_get_attribute,
_drive_is_asleep,
HDD_HIGH_TEMPERATURE,
HDD_CRITICAL_TEMPERATURE,
)
@ -203,3 +207,95 @@ class TestGetAttribute:
result = _get_attribute(mock_device, "Power_On_Hours")
assert result is None
class TestDriveIsAsleep:
"""Tests for _drive_is_asleep function (smartctl -n standby)."""
def test_returns_true_for_standby(self):
completed = mock.Mock(stdout="Device is in STANDBY mode, exit(2)\n")
with mock.patch("controller.lib.smart.subprocess.run", return_value=completed):
assert _drive_is_asleep("/dev/sda") is True
def test_returns_true_for_sleep(self):
# smartctl -n standby also bails on the deeper SLEEP power mode;
# that drive must NOT be woken for a read either.
completed = mock.Mock(stdout="Device is in SLEEP mode, exit(2)\n")
with mock.patch("controller.lib.smart.subprocess.run", return_value=completed):
assert _drive_is_asleep("/dev/sda") is True
def test_returns_false_for_active(self):
# A normal `-i` read on an awake drive never mentions STANDBY.
completed = mock.Mock(stdout="Model Family: WDC\nDevice Model: WD100EMAZ\n")
with mock.patch("controller.lib.smart.subprocess.run", return_value=completed):
assert _drive_is_asleep("/dev/sda") is False
def test_returns_false_on_error(self):
"""smartctl missing / errors out → treat as awake (safe fallback)."""
with mock.patch("controller.lib.smart.subprocess.run",
side_effect=OSError("boom")):
assert _drive_is_asleep("/dev/sda") is False
class TestBuildSmartStats:
"""Tests for build_smart_stats — the navbar/Disk Management payload."""
def test_returns_thresholds_when_unavailable(self):
"""Docker / no pySMART → empty drive list but thresholds present."""
with mock.patch("controller.lib.smart.is_smart_available", return_value=False):
stats = build_smart_stats()
assert stats["drives"] == []
assert stats["high_temperature"] == HDD_HIGH_TEMPERATURE
assert stats["critical_temperature"] == HDD_CRITICAL_TEMPERATURE
def test_reads_awake_drives(self):
fake_device = mock.Mock()
fake_device.name = "sda"
fake_device.model = "WDC WD100EMAZ"
fake_device.serial = "S1"
fake_device.capacity = "10.0 TB"
fake_device.assessment = "PASS"
fake_device.smart_enabled = True
fake_device.temperature = 52
fake_device.attributes = []
with mock.patch("controller.lib.smart.is_smart_available", return_value=True), \
mock.patch("controller.lib.smart._scan_devices",
return_value=[("/dev/sda", "sat")]), \
mock.patch("controller.lib.smart._drive_is_asleep", return_value=False), \
mock.patch("controller.lib.smart.Device", return_value=fake_device) as Dev:
stats = build_smart_stats()
Dev.assert_called_once_with("/dev/sda", interface="sat")
assert len(stats["drives"]) == 1
assert stats["drives"][0]["device"] == "sda"
assert stats["drives"][0]["temperature"] == 52
def test_sleeping_drive_is_not_read_and_reuses_previous(self):
"""A standby drive must NOT be read (would wake it); reuse old data."""
previous = {
"drives": [{"device": "sda", "path": "/dev/sda", "temperature": 49}],
"high_temperature": HDD_HIGH_TEMPERATURE,
"critical_temperature": HDD_CRITICAL_TEMPERATURE,
}
with mock.patch("controller.lib.smart.is_smart_available", return_value=True), \
mock.patch("controller.lib.smart._scan_devices",
return_value=[("/dev/sda", "sat")]), \
mock.patch("controller.lib.smart._drive_is_asleep", return_value=True), \
mock.patch("controller.lib.smart.Device") as Dev:
stats = build_smart_stats(previous=previous)
Dev.assert_not_called()
assert stats["drives"] == [{"device": "sda", "path": "/dev/sda", "temperature": 49}]
def test_sleeping_drive_without_previous_is_skipped(self):
"""Standby drive with no prior reading is simply omitted (not hot)."""
with mock.patch("controller.lib.smart.is_smart_available", return_value=True), \
mock.patch("controller.lib.smart._scan_devices",
return_value=[("/dev/sda", "sat")]), \
mock.patch("controller.lib.smart._drive_is_asleep", return_value=True), \
mock.patch("controller.lib.smart.Device") as Dev:
stats = build_smart_stats(previous=None)
Dev.assert_not_called()
assert stats["drives"] == []

View file

@ -20,11 +20,29 @@ class TestCollectAllStatus:
for key in (
"cpu_stats", "memory_stats", "load_stats", "drives_stats",
"nic_bandwidth_stats", "power_stats", "last_status",
"nic_bandwidth_stats", "power_stats", "smart_stats", "last_status",
):
assert key in status
datetime.fromisoformat(status["last_status"])
@pytest.mark.asyncio
async def test_smart_stats_passed_through(self):
"""The smart_stats kwarg should land in the cached payload verbatim."""
from controller.lib.status_worker import collect_all_status
smart = {"drives": [{"device": "sda", "temperature": 52}],
"high_temperature": 55, "critical_temperature": 65}
status = await collect_all_status(smart_stats=smart)
assert status["smart_stats"] is smart
@pytest.mark.asyncio
async def test_smart_stats_defaults_to_none(self):
"""Without the kwarg, smart_stats is None (worker fills it in)."""
from controller.lib.status_worker import collect_all_status
status = await collect_all_status()
assert status["smart_stats"] is None
@pytest.mark.asyncio
@pytest.mark.parametrize("collector,result_key,failure_value", [
("get_cpu_status", "cpu_stats", None),
@ -48,6 +66,65 @@ class TestCollectAllStatus:
assert any(status[k] is not None for k in other_keys)
class TestRefreshSmartStats:
"""SMART must be collected on a slow cadence, not every status cycle."""
def _fake_app(self):
app = mock.Mock()
app.state = mock.Mock(spec=[]) # no smart_cache / smart_last_collected yet
return app
@pytest.mark.asyncio
async def test_collects_on_first_call(self):
from controller.lib.status_worker import refresh_smart_stats
sentinel = {"drives": [], "high_temperature": 55, "critical_temperature": 65}
with mock.patch("controller.lib.status_worker.build_smart_stats",
return_value=sentinel) as build:
app = self._fake_app()
now = datetime(2026, 6, 3, 12, 0, 0)
result = await refresh_smart_stats(app, now)
build.assert_called_once()
assert result is sentinel
assert app.state.smart_cache is sentinel
assert app.state.smart_last_collected == now
@pytest.mark.asyncio
async def test_reuses_cache_before_interval(self):
"""A second call a few seconds later must NOT re-read SMART."""
from controller.lib.status_worker import refresh_smart_stats
with mock.patch("controller.lib.status_worker.build_smart_stats",
side_effect=[{"n": 1}, {"n": 2}]) as build:
app = self._fake_app()
t0 = datetime(2026, 6, 3, 12, 0, 0)
first = await refresh_smart_stats(app, t0)
# 5s later — well within the 60s window.
second = await refresh_smart_stats(app, datetime(2026, 6, 3, 12, 0, 5))
assert build.call_count == 1
assert first == {"n": 1}
assert second == {"n": 1}
@pytest.mark.asyncio
async def test_refreshes_after_interval(self):
"""After >= 60s, SMART is re-read and the prior result is passed in."""
from controller.lib.status_worker import refresh_smart_stats
with mock.patch("controller.lib.status_worker.build_smart_stats",
side_effect=[{"n": 1}, {"n": 2}]) as build:
app = self._fake_app()
t0 = datetime(2026, 6, 3, 12, 0, 0)
await refresh_smart_stats(app, t0)
result = await refresh_smart_stats(app, datetime(2026, 6, 3, 12, 1, 0))
assert build.call_count == 2
# The prior cache is handed to build_smart_stats for spin-down reuse.
assert build.call_args_list[1].args[0] == {"n": 1}
assert result == {"n": 2}
class TestGetAdaptiveSleep:
"""Tests for adaptive sleep calculation."""
@ -121,11 +198,14 @@ class TestAggregatedStatsEndpoint:
for key in (
"cpu_stats", "memory_stats", "load_stats", "drives_stats",
"nic_bandwidth_stats", "power_stats", "last_status",
"nic_bandwidth_stats", "power_stats", "smart_stats", "last_status",
):
assert key in data
# last_status is an ISO timestamp.
datetime.fromisoformat(data["last_status"])
# SMART carries its thresholds for the frontend warning.
for field in ("drives", "high_temperature", "critical_temperature"):
assert field in data["smart_stats"]
# Sub-field shape spot-checks.
for field in ("percent", "cores"):
assert field in data["cpu_stats"]