diff --git a/openpilot/common/hardware/base.py b/openpilot/common/hardware/base.py index 5d8f7770dced51..5f8aeebed2a352 100644 --- a/openpilot/common/hardware/base.py +++ b/openpilot/common/hardware/base.py @@ -16,20 +16,22 @@ class ThermalZone: zone_number = -1 def read(self) -> float: - if self.zone_number < 0: - for n in os.listdir("/sys/devices/virtual/thermal"): - if not n.startswith("thermal_zone"): - continue - with open(os.path.join("/sys/devices/virtual/thermal", n, "type")) as f: - if f.read().strip() == self.name: - self.zone_number = int(n.removeprefix("thermal_zone")) - break - try: + if self.zone_number < 0: + for n in os.listdir("/sys/devices/virtual/thermal"): + if not n.startswith("thermal_zone"): + continue + with open(os.path.join("/sys/devices/virtual/thermal", n, "type")) as f: + if f.read().strip() == self.name: + self.zone_number = int(n.removeprefix("thermal_zone")) + break + with open(f"/sys/devices/virtual/thermal/thermal_zone{self.zone_number}/temp") as f: return int(f.read()) / self.scale - except FileNotFoundError: - return 0 + except (OSError, ValueError): + # the zone may not exist on this platform, and reads themselves can fail + # transiently (e.g. EIO when the sensor transaction fails) + return float("nan") @dataclass class ThermalConfig: diff --git a/openpilot/system/hardware/hardwared.py b/openpilot/system/hardware/hardwared.py index 69f118d337d4bf..782f2cd0dc22c0 100755 --- a/openpilot/system/hardware/hardwared.py +++ b/openpilot/system/hardware/hardwared.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 import fcntl +import math import os import queue import struct @@ -56,6 +57,19 @@ # Override to highest thermal band when offroad and above this temp OFFROAD_DANGER_TEMP = 85 if HARDWARE.get_device_type() == "mici" else 75 +# how long we tolerate zero valid thermal readings before treating the +# thermal state as unknown (sensor reads can fail transiently, e.g. EIO) +THERMAL_READINGS_STALE_TIMEOUT = 10. + + +def max_valid_temp(temps) -> float: + # empty means the platform doesn't have these sensors; all-NaN means the + # reads are failing and the failure must propagate, not read as 0 + if len(temps) == 0: + return 0. + valid = [t for t in temps if not math.isnan(t)] + return max(valid) if valid else float("nan") + prev_offroad_states: dict[str, tuple[bool, str | None]] = {} @@ -178,6 +192,8 @@ def hardware_thread(end_event, hw_queue) -> None: all_temp_filter = FirstOrderFilter(0., TEMP_TAU, DT_HW, initialized=False) offroad_temp_filter = FirstOrderFilter(0., TEMP_TAU, DT_HW, initialized=False) + last_valid_temp_ts = time.monotonic() + thermal_readings_stale = False should_start_prev = False in_car = False engaged_prev = False @@ -256,17 +272,32 @@ def hardware_thread(end_event, hw_queue) -> None: set_usb_state(msg.deviceState, last_hw_state.usb_state) - # this subset is only used for offroad + # this subset is only used for offroad. sensor reads can fail (NaN); + # aggregate over the valid readings and never let NaN into the filters temp_sources = [ msg.deviceState.memoryTempC, - max(msg.deviceState.cpuTempC, default=0.), - max(msg.deviceState.gpuTempC, default=0.), + max_valid_temp(msg.deviceState.cpuTempC), + max_valid_temp(msg.deviceState.gpuTempC), ] - offroad_comp_temp = offroad_temp_filter.update(max(temp_sources)) + valid_sources = [t for t in temp_sources if not math.isnan(t)] + if len(valid_sources) > 0: + offroad_comp_temp = offroad_temp_filter.update(max(valid_sources)) + else: + offroad_comp_temp = offroad_temp_filter.x # this drives the thermal status while onroad - temp_sources.append(max(msg.deviceState.pmicTempC, default=0.)) - all_comp_temp = all_temp_filter.update(max(temp_sources)) + temp_sources.append(max_valid_temp(msg.deviceState.pmicTempC)) + valid_sources = [t for t in temp_sources if not math.isnan(t)] + if len(valid_sources) > 0: + all_comp_temp = all_temp_filter.update(max(valid_sources)) + last_valid_temp_ts = time.monotonic() + else: + all_comp_temp = all_temp_filter.x + + stale_prev = thermal_readings_stale + thermal_readings_stale = time.monotonic() - last_valid_temp_ts > THERMAL_READINGS_STALE_TIMEOUT + if thermal_readings_stale and not stale_prev: + cloudlog.error("thermal readings stale, no valid sensor for %.0fs", THERMAL_READINGS_STALE_TIMEOUT) msg.deviceState.maxTempC = all_comp_temp msg.deviceState.fanSpeedPercentDesired = fan_controller.update(all_comp_temp, onroad_conditions["ignition"]) @@ -296,8 +327,11 @@ def hardware_thread(end_event, hw_queue) -> None: startup_conditions["completed_training"] = params.get("CompletedTrainingVersion") == training_version startup_conditions["not_driver_view"] = not params.get_bool("IsDriverViewEnabled") - # must be at an engageable thermal band to go onroad - startup_conditions["device_temp_engageable"] = thermal_status < ThermalStatus.overheated + # must be at an engageable thermal band to go onroad; if we can't verify + # the device is cool (prolonged sensor failure), don't start a drive. + # while already onroad, stale readings hold the last band and log instead + # of forcing a disengagement over a sensor failure. + startup_conditions["device_temp_engageable"] = thermal_status < ThermalStatus.overheated and not thermal_readings_stale # ensure device is fully booted startup_conditions["device_booted"] = startup_conditions.get("device_booted", False) or HARDWARE.booted() diff --git a/openpilot/system/hardware/tests/test_thermal_readings.py b/openpilot/system/hardware/tests/test_thermal_readings.py new file mode 100644 index 00000000000000..47a507078d7d2d --- /dev/null +++ b/openpilot/system/hardware/tests/test_thermal_readings.py @@ -0,0 +1,53 @@ +import errno +import math +from unittest import mock + +from openpilot.common.test import OpenpilotTestCase +from openpilot.common.hardware.base import ThermalZone +from openpilot.system.hardware.hardwared import max_valid_temp + + +class TestThermalZone(OpenpilotTestCase): + def setUp(self): + self.zone = ThermalZone(name="cpu0-silver-usr") + self.zone.zone_number = 1 # skip discovery + + def test_read_ok(self): + with mock.patch("builtins.open", mock.mock_open(read_data="45000")): + assert self.zone.read() == 45.0 + + def test_read_io_error(self): + # the failure mode from the issue: spmi transaction fails, read gets EIO + with mock.patch("builtins.open", side_effect=OSError(errno.EIO, "read failed")): + assert math.isnan(self.zone.read()) + + def test_read_garbage(self): + with mock.patch("builtins.open", mock.mock_open(read_data="not a number")): + assert math.isnan(self.zone.read()) + + def test_read_missing_zone(self): + with mock.patch("builtins.open", side_effect=FileNotFoundError): + assert math.isnan(self.zone.read()) + + +class TestMaxValidTemp(OpenpilotTestCase): + def test_no_sensors_configured(self): + # platforms without these sensors keep today's 0.0 behavior + assert max_valid_temp([]) == 0. + + def test_all_valid(self): + assert max_valid_temp([40., 55., 45.]) == 55. + + def test_ignores_failed_reads(self): + assert max_valid_temp([40., float("nan"), 55.]) == 55. + + def test_all_failed_propagates(self): + # configured-but-failing must not silently read as 0 + assert math.isnan(max_valid_temp([float("nan"), float("nan")])) + + def test_order_independent(self): + # python's builtin max() with NaN is order-dependent, which is the bug + # this helper exists to avoid: max(nan, 55) is nan but max(55, nan) is 55 + a = [float("nan"), 55.] + b = [55., float("nan")] + assert max_valid_temp(a) == max_valid_temp(b) == 55. diff --git a/scons.log b/scons.log new file mode 100644 index 00000000000000..2660b68ddf67a9 --- /dev/null +++ b/scons.log @@ -0,0 +1,24 @@ +scons: Reading SConscript files ... +kj/filesystem-disk-unix.c++:1734: warning: PWD environment variable doesn't match current directory; pwd = /home/jpli/Project/openpilot-thermal +scons: done reading SConscript files. +scons: Building targets ... +progress: 9.6 +progress: 19.2 +progress: 28.8 +progress: 38.5 +progress: 48.1 +progress: 57.7 +progress: 67.3 +capnpc --src-prefix=openpilot/cereal --src-prefix=opendbc_repo/opendbc/car --import-path=opendbc_repo/opendbc/car openpilot/cereal/log.capnp openpilot/cereal/deprecated.capnp openpilot/cereal/custom.capnp opendbc_repo/opendbc/car/car.capnp -o c++:openpilot/cereal/gen/cpp/ +progress: 76.9 +progress: 86.5 +progress: 96.2 +progress: 100.0 + [CXX] openpilot/common/swaglog.o + [CXX] openpilot/common/tests/test_swaglog.o + [CXX] openpilot/common/params.o + [AR] openpilot/common/libcommon.a + [RANLIB] openpilot/common/libcommon.a + [LINK] openpilot/common/tests/test_swaglog + [LINK] openpilot/common/libparams_c.so +scons: done building targets. diff --git a/scons2.log b/scons2.log new file mode 100644 index 00000000000000..cd0022e4801052 --- /dev/null +++ b/scons2.log @@ -0,0 +1,52 @@ +scons: Reading SConscript files ... +kj/filesystem-disk-unix.c++:1734: warning: PWD environment variable doesn't match current directory; pwd = /home/jpli/Project/openpilot-thermal +scons: done reading SConscript files. +scons: Building targets ... +progress: 4.5 +progress: 9.1 + [CXX] msgq_repo/msgq/ipc.os + [CXX] msgq_repo/msgq/event.os +progress: 13.6 + [CXX] msgq_repo/msgq/impl_msgq.os + [CXX] msgq_repo/msgq/impl_fake.os +progress: 18.2 + [CXX] msgq_repo/msgq/msgq.os +progress: 22.7 + [CXX] msgq_repo/msgq/visionipc/visionipc.os +progress: 27.3 + [CXX] msgq_repo/msgq/visionipc/visionipc_server.os + [CXX] msgq_repo/msgq/visionipc/visionipc_client.os +progress: 31.8 + [CXX] msgq_repo/msgq/visionipc/visionbuf.os +progress: 36.4 +progress: 40.9 +progress: 45.5 +cythonize msgq_repo/msgq/ipc_pyx.pyx +progress: 50.0 +progress: 54.5 +progress: 59.1 +progress: 63.6 +progress: 68.2 +progress: 72.7 +progress: 77.3 +progress: 81.8 +progress: 86.4 +progress: 90.9 +progress: 95.5 + [CXX] msgq_repo/msgq/msgq_tests.o +progress: 100.0 +cythonize msgq_repo/msgq/visionipc/visionipc_pyx.pyx + [AR] msgq_repo/libmsgq.a + [RANLIB] msgq_repo/libmsgq.a + [LINK] msgq_repo/msgq/test_runner +Compiling /home/jpli/Project/openpilot-thermal/msgq_repo/msgq/ipc_pyx.pyx because it changed. +[1/1] Cythonizing /home/jpli/Project/openpilot-thermal/msgq_repo/msgq/ipc_pyx.pyx + [AR] msgq_repo/libvisionipc.a + [RANLIB] msgq_repo/libvisionipc.a + [CXX] msgq_repo/msgq/ipc_pyx.o +Compiling /home/jpli/Project/openpilot-thermal/msgq_repo/msgq/visionipc/visionipc_pyx.pyx because it changed. +[1/1] Cythonizing /home/jpli/Project/openpilot-thermal/msgq_repo/msgq/visionipc/visionipc_pyx.pyx + [CXX] msgq_repo/msgq/visionipc/visionipc_pyx.o + [LINK] msgq_repo/msgq/ipc_pyx.so + [LINK] msgq_repo/msgq/visionipc/visionipc_pyx.so +scons: done building targets. diff --git a/uv_sync.log b/uv_sync.log new file mode 100644 index 00000000000000..9b89b002fbf5d5 --- /dev/null +++ b/uv_sync.log @@ -0,0 +1,129 @@ + Building openpilot @ file:///home/jpli/Project/openpilot-thermal + Building msgq @ file:///home/jpli/Project/openpilot-thermal/msgq_repo + Building opendbc @ file:///home/jpli/Project/openpilot-thermal/opendbc_repo + Building pandacan @ file:///home/jpli/Project/openpilot-thermal/panda + Building rednose @ file:///home/jpli/Project/openpilot-thermal/rednose_repo + Building teleoprtc @ file:///home/jpli/Project/openpilot-thermal/teleoprtc_repo + Building tinygrad @ file:///home/jpli/Project/openpilot-thermal/tinygrad_repo +Downloading cryptography (4.5MiB) +Downloading comma-deps-acados (12.5MiB) +Downloading kiwisolver (1.4MiB) +Downloading pycryptodome (2.2MiB) +Downloading fonttools (4.8MiB) +Downloading zstandard (5.3MiB) +Downloading ty (12.0MiB) +Downloading pillow (6.6MiB) +Downloading matplotlib (9.6MiB) +Downloading pycapnp (5.1MiB) +Downloading comma-deps-imgui (2.5MiB) +Downloading libdatachannel-py (1.7MiB) +Downloading comma-deps-eigen (2.2MiB) +Downloading ruff (10.9MiB) +Downloading cython (3.2MiB) +Downloading numpy (15.9MiB) +Downloading comma-deps-ffmpeg (4.5MiB) +Downloading comma-deps-gcc-arm-none-eabi (16.2MiB) +Downloading scons (3.9MiB) +Downloading comma-deps-capnproto (2.5MiB) +Downloading comma-deps-git-lfs (4.7MiB) +Downloading comma-deps-raylib (4.8MiB) + Building spidev==3.8 + Built rednose @ file:///home/jpli/Project/openpilot-thermal/rednose_repo + Built pandacan @ file:///home/jpli/Project/openpilot-thermal/panda + Built msgq @ file:///home/jpli/Project/openpilot-thermal/msgq_repo + Built teleoprtc @ file:///home/jpli/Project/openpilot-thermal/teleoprtc_repo + Built openpilot @ file:///home/jpli/Project/openpilot-thermal + Built tinygrad @ file:///home/jpli/Project/openpilot-thermal/tinygrad_repo + Built opendbc @ file:///home/jpli/Project/openpilot-thermal/opendbc_repo + Downloading kiwisolver + Downloading libdatachannel-py + Built spidev==3.8 + Downloading pycryptodome + Downloading comma-deps-imgui + Downloading comma-deps-capnproto + Downloading comma-deps-eigen + Downloading cython + Downloading scons + Downloading cryptography + Downloading comma-deps-ffmpeg + Downloading fonttools + Downloading pycapnp + Downloading zstandard + Downloading comma-deps-git-lfs + Downloading pillow + Downloading matplotlib + Downloading ruff + Downloading ty + Downloading comma-deps-acados + Downloading numpy + Downloading comma-deps-gcc-arm-none-eabi + Downloading comma-deps-raylib +Prepared 63 packages in 3.55s +Installed 67 packages in 115ms + + certifi==2026.7.22 + + cffi==2.1.0 + + charset-normalizer==3.4.9 + + codespell==2.4.3 + + comma-deps-acados==0.2.2.post98 + + comma-deps-bootstrap-icons==1.10.5.0.post98 + + comma-deps-capnproto==1.0.1.post98 + + comma-deps-eigen==3.4.0.post98 + + comma-deps-ffmpeg==7.1.0.post98 + + comma-deps-gcc-arm-none-eabi==13.2.1.post98 + + comma-deps-git-lfs==3.6.1.post98 + + comma-deps-imgui==1.92.7.post98 + + comma-deps-json11==20170411.0.post98 + + comma-deps-libusb==1.0.29.post98 + + comma-deps-ncurses==6.5.post98 + + comma-deps-raylib==6.0.0.1.post98 + + comma-deps-zeromq==4.3.5.post98 + + comma-deps-zstd==1.5.6.post98 + + contourpy==1.3.3 + + coverage==7.15.3 + + cryptography==50.0.0 + + cycler==0.12.1 + + cython==3.2.9 + + fonttools==4.63.0 + + idna==3.18 + + importlib-resources==7.1.0 + + inputs==0.5 + + jeepney==0.9.0 + + kiwisolver==1.5.0 + + libdatachannel-py==2026.1.0.dev2 + + libusb-package==1.0.30.0 + + libusb1==3.4.0 + + matplotlib==3.11.1 + + mpmath==1.3.0 + + msgq==0.0.1 (from file:///home/jpli/Project/openpilot-thermal/msgq_repo) + + numpy==2.5.1 + + opendbc==0.3.1 (from file:///home/jpli/Project/openpilot-thermal/opendbc_repo) + + openpilot==0.1.0 (from file:///home/jpli/Project/openpilot-thermal) + + packaging==26.2 + + pandacan==0.0.10 (from file:///home/jpli/Project/openpilot-thermal/panda) + + pillow==12.3.0 + + pycapnp==2.1.0 + + pycparser==3.0 + + pycryptodome==3.23.0 + + pyjwt==2.13.0 + + pyparsing==3.3.2 + + python-dateutil==2.9.0.post0 + + pyzmq==27.1.0 + + qrcode==8.2 + + rednose==0.0.1 (from file:///home/jpli/Project/openpilot-thermal/rednose_repo) + + requests==2.34.2 + + ruff==0.16.1 + + scons==4.10.1 + + sentry-sdk==2.66.1 + + setproctitle==1.3.7 + + setuptools==83.0.0 + + six==1.17.0 + + sounddevice==0.5.5 + + spidev==3.8 + + sympy==1.14.0 + + teleoprtc==1.0.1 (from file:///home/jpli/Project/openpilot-thermal/teleoprtc_repo) + + tinygrad==0.13.0 (from file:///home/jpli/Project/openpilot-thermal/tinygrad_repo) + + tqdm==4.70.0 + + ty==0.0.65 + + urllib3==2.7.0 + + websocket-client==1.9.0 + + zstandard==0.25.0