diff --git a/app/src/components/Nav.js b/app/src/components/Nav.js index fcda2b52..707c3300 100644 --- a/app/src/components/Nav.js +++ b/app/src/components/Nav.js @@ -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 = ; } + // 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 = + const link = {icon}; + driveTemperatureIcon = ; + } + // 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(); diff --git a/app/src/hooks/customHooks.js b/app/src/hooks/customHooks.js index 2ebc2c25..78777c56 100644 --- a/app/src/hooks/customHooks.js +++ b/app/src/hooks/customHooks.js @@ -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; diff --git a/app/src/hooks/customHooks.test.js b/app/src/hooks/customHooks.test.js index f176c3ba..96bd73f6 100644 --- a/app/src/hooks/customHooks.test.js +++ b/app/src/hooks/customHooks.test.js @@ -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}) => ( + + {children} + + ); + + 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 diff --git a/controller/conftest.py b/controller/conftest.py index 8e304885..05b1f983 100644 --- a/controller/conftest.py +++ b/controller/conftest.py @@ -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", } diff --git a/controller/lib/smart.py b/controller/lib/smart.py index b7227e91..9e989cba 100644 --- a/controller/lib/smart.py +++ b/controller/lib/smart.py @@ -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 diff --git a/controller/lib/status_worker.py b/controller/lib/status_worker.py index 934b0790..d9531b5e 100644 --- a/controller/lib/status_worker.py +++ b/controller/lib/status_worker.py @@ -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 diff --git a/controller/test/test_smart.py b/controller/test/test_smart.py index 1a77871d..358c1e55 100644 --- a/controller/test/test_smart.py +++ b/controller/test/test_smart.py @@ -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"] == [] diff --git a/controller/test/test_status_worker.py b/controller/test/test_status_worker.py index c6988fca..03fe8ec1 100644 --- a/controller/test/test_status_worker.py +++ b/controller/test/test_status_worker.py @@ -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"]