From e64484aecdffbadbcc32c8bd316b579cdd3938ad Mon Sep 17 00:00:00 2001 From: robbederks Date: Wed, 2 Dec 2020 15:20:57 +0100 Subject: [PATCH] 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 --- common/hardware.py | 24 +++++++ common/hardware_android.py | 29 +++++++++ common/hardware_base.py | 40 ++++++++++++ common/hardware_tici.py | 26 ++++++++ selfdrive/thermald/power_monitoring.py | 65 ++++--------------- .../thermald/tests/test_power_monitoring.py | 37 ++++++----- selfdrive/thermald/thermald.py | 17 ++--- 7 files changed, 155 insertions(+), 83 deletions(-) diff --git a/common/hardware.py b/common/hardware.py index 67d0c44d19c52b..f4e502ea5ea5e0 100644 --- a/common/hardware.py +++ b/common/hardware.py @@ -48,6 +48,30 @@ def get_sim_info(self): 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()) diff --git a/common/hardware_android.py b/common/hardware_android.py index 91e1d1423e5f78..dc18d3292eecae 100644 --- a/common/hardware_android.py +++ b/common/hardware_android.py @@ -300,3 +300,32 @@ def get_cdma_level(cdmadbm, cdmaecio): 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 diff --git a/common/hardware_base.py b/common/hardware_base.py index 24bb52a5e668c0..79cff5305a81d1 100644 --- a/common/hardware_base.py +++ b/common/hardware_base.py @@ -8,6 +8,14 @@ def get_cmdline(): 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 @@ def get_sim_info(self): @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 diff --git a/common/hardware_tici.py b/common/hardware_tici.py index 2212edd48156d5..3c51a75fcd3d72 100644 --- a/common/hardware_tici.py +++ b/common/hardware_tici.py @@ -57,3 +57,29 @@ def get_sim_info(self): 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) diff --git a/selfdrive/thermald/power_monitoring.py b/selfdrive/thermald/power_monitoring.py index a6f2d4c4e9556e..477b834d7728c6 100644 --- a/selfdrive/thermald/power_monitoring.py +++ b/selfdrive/thermald/power_monitoring.py @@ -6,7 +6,7 @@ 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 @@ 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 @@ def calculate(self, health): # 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 - if TICI: - with open("/sys/class/hwmon/hwmon1/power1_input") as f: - current_power = int(f.read()) / 1e6 - elif get_battery_status() == 'Discharging': + current_power = HARDWARE.get_current_power_draw() + if current_power is not None: + pass + elif HARDWARE.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 @@ def calculate(self, health): # 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()) - currents.append(get_battery_current()) + voltages.append(HARDWARE.get_battery_voltage()) + 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 @@ def should_shutdown(self, health, offroad_timestamp, started_seen, LEON): 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 diff --git a/selfdrive/thermald/tests/test_power_monitoring.py b/selfdrive/thermald/tests/test_power_monitoring.py index fe91dce8e39763..32a3ea4c92cdd8 100755 --- a/selfdrive/thermald/tests/test_power_monitoring.py +++ b/selfdrive/thermald/tests/test_power_monitoring.py @@ -70,8 +70,8 @@ def test_offroad_ignition(self, hw_type): 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), \ - pm_patch("get_battery_status", "Discharging"): + with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \ + 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 @@ def test_offroad_integration_discharging(self, hw_type): 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), \ - pm_patch("get_battery_status", "Discharging"): + with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \ + 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 @@ def test_car_battery_integration_onroad(self, hw_type): 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), \ - pm_patch("get_battery_status", "Discharging"): + with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \ + 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 @@ def test_car_battery_integration_upper_limit(self, hw_type): 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), \ - pm_patch("get_battery_status", "Discharging"): + with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \ + 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 @@ def test_car_battery_integration_offroad(self, hw_type): 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), \ - pm_patch("get_battery_status", "Discharging"): + with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \ + 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 @@ def test_max_time_offroad(self, hw_type): 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), \ - pm_patch("get_battery_status", "Discharging"), pm_patch("MAX_TIME_OFFROAD_S", MOCKED_MAX_OFFROAD_TIME, constant=True): + with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \ + 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 @@ def test_car_voltage(self, hw_type): 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), \ - pm_patch("get_battery_status", "Discharging"): + with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \ + 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 @@ def test_disable_power_down(self): 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), \ - pm_patch("get_battery_status", "Discharging"): + with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \ + 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 @@ def test_ignition(self): 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), \ - pm_patch("get_battery_status", "Discharging"): + with pm_patch("HARDWARE.get_battery_voltage", BATT_VOLTAGE * 1e6), pm_patch("HARDWARE.get_battery_current", BATT_CURRENT * 1e6), \ + 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)) diff --git a/selfdrive/thermald/thermald.py b/selfdrive/thermald/thermald.py index c36e9cdbe07e3d..400d71860c1046 100755 --- a/selfdrive/thermald/thermald.py +++ b/selfdrive/thermald/thermald.py @@ -19,12 +19,7 @@ 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, - get_battery_capacity, - get_battery_current, - get_battery_status, - get_battery_voltage, - get_usb_present) +from selfdrive.thermald.power_monitoring import PowerMonitoring 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.batteryStatus = get_battery_status() - msg.thermal.batteryCurrent = get_battery_current() - msg.thermal.batteryVoltage = get_battery_voltage() - msg.thermal.usbOnline = get_usb_present() + msg.thermal.batteryPercent = HARDWARE.get_battery_capacity() + msg.thermal.batteryStatus = HARDWARE.get_battery_status() + msg.thermal.batteryCurrent = HARDWARE.get_battery_current() + msg.thermal.batteryVoltage = HARDWARE.get_battery_voltage() + msg.thermal.usbOnline = HARDWARE.get_usb_present() # Fake battery levels on uno for frame if (not EON) or is_uno: