Move thermald hardware calls into HW abstraction layer (#2630)

* abstracted away hardware calls

* oopsie

* remove bugs

* remove bugs #2

* fix unit test

* removed print

Co-authored-by: Comma Device <device@comma.ai>
old-commit-hash: e64484aecd

common/hardware.py

@@ -48,6 +48,30 @@ class Pc(HardwareBase):
   def get_network_strength(self, network_type):
     return NetworkStrength.unknown
 
+  def get_battery_capacity(self):
+    return 100
+
+  def get_battery_status(self):
+    return ""
+
+  def get_battery_current(self):
+    return 0
+
+  def get_battery_voltage(self):
+    return 0
+
+  def get_battery_charging(self):
+    return True
+
+  def set_battery_charging(self, on):
+    pass
+
+  def get_usb_present(self):
+    return False
+
+  def get_current_power_draw(self):
+    return 0
+
 
 if EON:
   HARDWARE = cast(HardwareBase, Android())

common/hardware_android.py

@@ -300,3 +300,32 @@ class Android(HardwareBase):
           network_strength = max(network_strength, ns)
 
       return network_strength
+
+  def get_battery_capacity(self):
+    return self.read_param_file("/sys/class/power_supply/battery/capacity", int, 100)
+
+  def get_battery_status(self):
+    # This does not correspond with actual charging or not.
+    # If a USB cable is plugged in, it responds with 'Charging', even when charging is disabled
+    return self.read_param_file("/sys/class/power_supply/battery/status", lambda x: x.strip(), '')
+
+  def get_battery_current(self):
+    return self.read_param_file("/sys/class/power_supply/battery/current_now", int)
+
+  def get_battery_voltage(self):
+    return self.read_param_file("/sys/class/power_supply/battery/voltage_now", int)
+
+  def get_battery_charging(self):
+    # This does correspond with actually charging
+    return self.read_param_file("/sys/class/power_supply/battery/charge_type", lambda x: x.strip() != "N/A", True)
+
+  def set_battery_charging(self, on):
+    with open('/sys/class/power_supply/battery/charging_enabled', 'w') as f:
+      f.write(f"{1 if on else 0}\n")
+
+  def get_usb_present(self):
+    return self.read_param_file("/sys/class/power_supply/usb/present", lambda x: bool(int(x)), False)
+
+  def get_current_power_draw(self):
+    # We don't have a good direct way to measure this on android
+    return None

common/hardware_base.py

@@ -8,6 +8,14 @@ class HardwareBase:
       cmdline = f.read()
     return {kv[0]: kv[1] for kv in [s.split('=') for s in cmdline.split(' ')] if len(kv) == 2}
 
+  @staticmethod
+  def read_param_file(path, parser, default=0):
+    try:
+      with open(path) as f:
+        return parser(f.read())
+    except Exception:
+      return default
+
   @abstractmethod
   def get_sound_card_online(self):
     pass
@@ -39,3 +47,35 @@ class HardwareBase:
   @abstractmethod
   def get_network_strength(self, network_type):
     pass
+
+  @abstractmethod
+  def get_battery_capacity(self):
+    pass
+
+  @abstractmethod
+  def get_battery_status(self):
+    pass
+
+  @abstractmethod
+  def get_battery_current(self):
+    pass
+
+  @abstractmethod
+  def get_battery_voltage(self):
+    pass
+
+  @abstractmethod
+  def get_battery_charging(self):
+    pass
+
+  @abstractmethod
+  def set_battery_charging(self, on):
+    pass
+
+  @abstractmethod
+  def get_usb_present(self):
+    pass
+
+  @abstractmethod
+  def get_current_power_draw(self):
+    pass

common/hardware_tici.py

@@ -57,3 +57,29 @@ class Tici(HardwareBase):
 
   def get_network_strength(self, network_type):
     return NetworkStrength.unknown
+
+  # We don't have a battery, so let's use some sane constants
+  def get_battery_capacity(self):
+    return 100
+
+  def get_battery_status(self):
+    return ""
+
+  def get_battery_current(self):
+    return 0
+
+  def get_battery_voltage(self):
+    return 0
+
+  def get_battery_charging(self):
+    return True
+
+  def set_battery_charging(self, on):
+    pass
+
+  def get_usb_present(self):
+    # Not sure if relevant on tici, but the file exists
+    return self.read_param_file("/sys/class/power_supply/usb/present", lambda x: bool(int(x)), False)
+
+  def get_current_power_draw(self):
+    return (self.read_param_file("/sys/class/hwmon/hwmon1/power1_input", int) / 1e6)

selfdrive/thermald/power_monitoring.py

@@ -6,7 +6,7 @@ from statistics import mean
 from cereal import log
 from common.realtime import sec_since_boot
 from common.params import Params, put_nonblocking
-from common.hardware import TICI
+from common.hardware import HARDWARE
 from selfdrive.swaglog import cloudlog
 
 CAR_VOLTAGE_LOW_PASS_K = 0.091 # LPF gain for 5s tau (dt/tau / (dt/tau + 1))
@@ -19,48 +19,6 @@ CAR_CHARGING_RATE_W = 45
 VBATT_PAUSE_CHARGING = 11.0
 MAX_TIME_OFFROAD_S = 30*3600
 
-# Parameters
-def get_battery_capacity():
-  return _read_param("/sys/class/power_supply/battery/capacity", int)
-
-
-def get_battery_status():
-  # This does not correspond with actual charging or not.
-  # If a USB cable is plugged in, it responds with 'Charging', even when charging is disabled
-  return _read_param("/sys/class/power_supply/battery/status", lambda x: x.strip(), '')
-
-
-def get_battery_current():
-  return _read_param("/sys/class/power_supply/battery/current_now", int)
-
-
-def get_battery_voltage():
-  return _read_param("/sys/class/power_supply/battery/voltage_now", int)
-
-
-def get_usb_present():
-  return _read_param("/sys/class/power_supply/usb/present", lambda x: bool(int(x)), False)
-
-
-def get_battery_charging():
-  # This does correspond with actually charging
-  return _read_param("/sys/class/power_supply/battery/charge_type", lambda x: x.strip() != "N/A", True)
-
-
-def set_battery_charging(on):
-  with open('/sys/class/power_supply/battery/charging_enabled', 'w') as f:
-    f.write(f"{1 if on else 0}\n")
-
-
-# Helpers
-def _read_param(path, parser, default=0):
-  try:
-    with open(path) as f:
-      return parser(f.read())
-  except Exception:
-    return default
-
-
 class PowerMonitoring:
   def __init__(self):
     self.params = Params()
@@ -121,14 +79,13 @@ class PowerMonitoring:
         # No ignition, we integrate the offroad power used by the device
         is_uno = health.health.hwType == log.HealthData.HwType.uno
         # Get current power draw somehow
-        current_power = 0
+        current_power = HARDWARE.get_current_power_draw()
-        if TICI:
+        if current_power is not None:
-          with open("/sys/class/hwmon/hwmon1/power1_input") as f:
+          pass
-            current_power = int(f.read()) / 1e6
+        elif HARDWARE.get_battery_status() == 'Discharging':
-        elif get_battery_status() == 'Discharging':
           # If the battery is discharging, we can use this measurement
           # On C2: this is low by about 10-15%, probably mostly due to UNO draw not being factored in
-          current_power = ((get_battery_voltage() / 1000000) * (get_battery_current() / 1000000))
+          current_power = ((HARDWARE.get_battery_voltage() / 1000000) * (HARDWARE.get_battery_current() / 1000000))
         elif (self.next_pulsed_measurement_time is not None) and (self.next_pulsed_measurement_time <= now):
           # TODO: Figure out why this is off by a factor of 3/4???
           FUDGE_FACTOR = 1.33
@@ -136,22 +93,22 @@ class PowerMonitoring:
           # Turn off charging for about 10 sec in a thread that does not get killed on SIGINT, and perform measurement here to avoid blocking thermal
           def perform_pulse_measurement(now):
             try:
-              set_battery_charging(False)
+              HARDWARE.set_battery_charging(False)
               time.sleep(5)
 
               # Measure for a few sec to get a good average
               voltages = []
               currents = []
               for _ in range(6):
-                voltages.append(get_battery_voltage())
+                voltages.append(HARDWARE.get_battery_voltage())
-                currents.append(get_battery_current())
+                currents.append(HARDWARE.get_battery_current())
                 time.sleep(1)
               current_power = ((mean(voltages) / 1000000) * (mean(currents) / 1000000))
 
               self._perform_integration(now, current_power * FUDGE_FACTOR)
 
               # Enable charging again
-              set_battery_charging(True)
+              HARDWARE.set_battery_charging(True)
             except Exception:
               cloudlog.exception("Pulsed power measurement failed")
 
@@ -222,6 +179,6 @@ class PowerMonitoring:
     should_shutdown = False
     # Wait until we have shut down charging before powering down
     should_shutdown |= (not panda_charging and self.should_disable_charging(health, offroad_timestamp))
-    should_shutdown |= ((get_battery_capacity() < BATT_PERC_OFF) and (not get_battery_charging()) and ((now - offroad_timestamp) > 60))
+    should_shutdown |= ((HARDWARE.get_battery_capacity() < BATT_PERC_OFF) and (not HARDWARE.get_battery_charging()) and ((now - offroad_timestamp) > 60))
     should_shutdown &= started_seen
     return should_shutdown

selfdrive/thermald/tests/test_power_monitoring.py

@@ -70,8 +70,8 @@ class TestPowerMonitoring(unittest.TestCase):
   def test_offroad_integration_discharging(self, hw_type):
     BATT_VOLTAGE = 4
     BATT_CURRENT = 1
-    with pm_patch("get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("get_battery_current", BATT_CURRENT * 1e6), \
+    with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \
-    pm_patch("get_battery_status", "Discharging"):
+    pm_patch("HARDWARE.get_battery_status", "Discharging"), pm_patch("HARDWARE.get_current_power_draw", None):
       pm = PowerMonitoring()
       for _ in range(TEST_DURATION_S + 1):
         pm.calculate(self.mock_health(False, hw_type))
@@ -83,8 +83,8 @@ class TestPowerMonitoring(unittest.TestCase):
   def test_car_battery_integration_onroad(self, hw_type):
     BATT_VOLTAGE = 4
     BATT_CURRENT = 1
-    with pm_patch("get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("get_battery_current", BATT_CURRENT * 1e6), \
+    with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \
-    pm_patch("get_battery_status", "Discharging"):
+    pm_patch("HARDWARE.get_battery_status", "Discharging"), pm_patch("HARDWARE.get_current_power_draw", None):
       pm = PowerMonitoring()
       pm.car_battery_capacity_uWh = 0
       for _ in range(TEST_DURATION_S + 1):
@@ -97,8 +97,8 @@ class TestPowerMonitoring(unittest.TestCase):
   def test_car_battery_integration_upper_limit(self, hw_type):
     BATT_VOLTAGE = 4
     BATT_CURRENT = 1
-    with pm_patch("get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("get_battery_current", BATT_CURRENT * 1e6), \
+    with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \
-    pm_patch("get_battery_status", "Discharging"):
+    pm_patch("HARDWARE.get_battery_status", "Discharging"), pm_patch("HARDWARE.get_current_power_draw", None):
       pm = PowerMonitoring()
       pm.car_battery_capacity_uWh = CAR_BATTERY_CAPACITY_uWh - 1000
       for _ in range(TEST_DURATION_S + 1):
@@ -111,8 +111,8 @@ class TestPowerMonitoring(unittest.TestCase):
   def test_car_battery_integration_offroad(self, hw_type):
     BATT_VOLTAGE = 4
     BATT_CURRENT = 1
-    with pm_patch("get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("get_battery_current", BATT_CURRENT * 1e6), \
+    with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \
-    pm_patch("get_battery_status", "Discharging"):
+    pm_patch("HARDWARE.get_battery_status", "Discharging"), pm_patch("HARDWARE.get_current_power_draw", None):
       pm = PowerMonitoring()
       pm.car_battery_capacity_uWh = CAR_BATTERY_CAPACITY_uWh
       for _ in range(TEST_DURATION_S + 1):
@@ -125,8 +125,8 @@ class TestPowerMonitoring(unittest.TestCase):
   def test_car_battery_integration_lower_limit(self, hw_type):
     BATT_VOLTAGE = 4
     BATT_CURRENT = 1
-    with pm_patch("get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("get_battery_current", BATT_CURRENT * 1e6), \
+    with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \
-    pm_patch("get_battery_status", "Discharging"):
+    pm_patch("HARDWARE.get_battery_status", "Discharging"), pm_patch("HARDWARE.get_current_power_draw", None):
       pm = PowerMonitoring()
       pm.car_battery_capacity_uWh = 1000
       for _ in range(TEST_DURATION_S + 1):
@@ -141,8 +141,9 @@ class TestPowerMonitoring(unittest.TestCase):
     BATT_VOLTAGE = 4
     BATT_CURRENT = 0 # To stop shutting down for other reasons
     MOCKED_MAX_OFFROAD_TIME = 3600
-    with pm_patch("get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("get_battery_current", BATT_CURRENT * 1e6), \
+    with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \
-    pm_patch("get_battery_status", "Discharging"), pm_patch("MAX_TIME_OFFROAD_S", MOCKED_MAX_OFFROAD_TIME, constant=True):
+    pm_patch("HARDWARE.get_battery_status", "Discharging"), pm_patch("MAX_TIME_OFFROAD_S", MOCKED_MAX_OFFROAD_TIME, constant=True), \
+    pm_patch("HARDWARE.get_current_power_draw", None):
       pm = PowerMonitoring()
       pm.car_battery_capacity_uWh = CAR_BATTERY_CAPACITY_uWh
       start_time = ssb
@@ -160,8 +161,8 @@ class TestPowerMonitoring(unittest.TestCase):
     BATT_VOLTAGE = 4
     BATT_CURRENT = 0 # To stop shutting down for other reasons
     TEST_TIME = 100
-    with pm_patch("get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("get_battery_current", BATT_CURRENT * 1e6), \
+    with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \
-    pm_patch("get_battery_status", "Discharging"):
+    pm_patch("HARDWARE.get_battery_status", "Discharging"), pm_patch("HARDWARE.get_current_power_draw", None):
       pm = PowerMonitoring()
       pm.car_battery_capacity_uWh = CAR_BATTERY_CAPACITY_uWh
       health = self.mock_health(False, hw_type, car_voltage=(VBATT_PAUSE_CHARGING - 1))
@@ -178,8 +179,8 @@ class TestPowerMonitoring(unittest.TestCase):
     BATT_CURRENT = 0 # To stop shutting down for other reasons
     TEST_TIME = 100
     params.put("DisablePowerDown", b"1")
-    with pm_patch("get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("get_battery_current", BATT_CURRENT * 1e6), \
+    with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \
-    pm_patch("get_battery_status", "Discharging"):
+    pm_patch("HARDWARE.get_battery_status", "Discharging"), pm_patch("HARDWARE.get_current_power_draw", None):
       pm = PowerMonitoring()
       pm.car_battery_capacity_uWh = CAR_BATTERY_CAPACITY_uWh
       health = self.mock_health(False, log.HealthData.HwType.uno, car_voltage=(VBATT_PAUSE_CHARGING - 1))
@@ -195,8 +196,8 @@ class TestPowerMonitoring(unittest.TestCase):
     BATT_VOLTAGE = 4
     BATT_CURRENT = 0 # To stop shutting down for other reasons
     TEST_TIME = 100
-    with pm_patch("get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("get_battery_current", BATT_CURRENT * 1e6), \
+    with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \
-    pm_patch("get_battery_status", "Discharging"):
+    pm_patch("HARDWARE.get_battery_status", "Discharging"), pm_patch("HARDWARE.get_current_power_draw", None):
       pm = PowerMonitoring()
       pm.car_battery_capacity_uWh = CAR_BATTERY_CAPACITY_uWh
       health = self.mock_health(True, log.HealthData.HwType.uno, car_voltage=(VBATT_PAUSE_CHARGING - 1))

selfdrive/thermald/thermald.py

@@ -19,12 +19,7 @@ from selfdrive.controls.lib.alertmanager import set_offroad_alert
 from selfdrive.loggerd.config import get_available_percent
 from selfdrive.pandad import get_expected_signature
 from selfdrive.swaglog import cloudlog
-from selfdrive.thermald.power_monitoring import (PowerMonitoring,
+from selfdrive.thermald.power_monitoring import PowerMonitoring
-                                                 get_battery_capacity,
-                                                 get_battery_current,
-                                                 get_battery_status,
-                                                 get_battery_voltage,
-                                                 get_usb_present)
 from selfdrive.version import get_git_branch, terms_version, training_version
 
 ThermalConfig = namedtuple('ThermalConfig', ['cpu', 'gpu', 'mem', 'bat', 'ambient'])
@@ -257,11 +252,11 @@ def thermald_thread():
     msg.thermal.cpuPerc = int(round(psutil.cpu_percent()))
     msg.thermal.networkType = network_type
     msg.thermal.networkStrength = network_strength
-    msg.thermal.batteryPercent = get_battery_capacity()
+    msg.thermal.batteryPercent = HARDWARE.get_battery_capacity()
-    msg.thermal.batteryStatus = get_battery_status()
+    msg.thermal.batteryStatus = HARDWARE.get_battery_status()
-    msg.thermal.batteryCurrent = get_battery_current()
+    msg.thermal.batteryCurrent = HARDWARE.get_battery_current()
-    msg.thermal.batteryVoltage = get_battery_voltage()
+    msg.thermal.batteryVoltage = HARDWARE.get_battery_voltage()
-    msg.thermal.usbOnline = get_usb_present()
+    msg.thermal.usbOnline = HARDWARE.get_usb_present()
 
     # Fake battery levels on uno for frame
     if (not EON) or is_uno: