From 28cb1897cb8be80b35b1ce7573066fb36b01d2d7 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Wed, 31 Aug 2022 21:13:53 -0700 Subject: [PATCH] USB power mode cleanup (#25619) * first pass at usb power mode cleanup * fix build * a sneaky one * little more * fix build * bump pnada * remove that * power monitoring cleanup * fix tests * bump submodules --- cereal | 2 +- panda | 2 +- selfdrive/boardd/boardd.cc | 32 ++----------------- selfdrive/boardd/panda.cc | 4 --- selfdrive/boardd/panda.h | 1 - selfdrive/thermald/power_monitoring.py | 29 +++++------------ .../thermald/tests/test_power_monitoring.py | 20 ++++++------ selfdrive/thermald/thermald.py | 9 +----- system/hardware/base.py | 4 --- system/hardware/pc/hardware.py | 5 +-- system/hardware/tici/hardware.py | 1 - 11 files changed, 24 insertions(+), 85 deletions(-) diff --git a/cereal b/cereal index d05d3cbd0e..d3a943ef66 160000 --- a/cereal +++ b/cereal @@ -1 +1 @@ -Subproject commit d05d3cbd0e1243c3fef63c58ab15aeb7508159a7 +Subproject commit d3a943ef660dd29f73700806ee0baf1d5aff6834 diff --git a/panda b/panda index 6d2e2bde86..13d64d4cc3 160000 --- a/panda +++ b/panda @@ -1 +1 @@ -Subproject commit 6d2e2bde861a237593740526b466d17d43849a17 +Subproject commit 13d64d4cc38295401ff1ffcf6a233a4b9625fd9f diff --git a/selfdrive/boardd/boardd.cc b/selfdrive/boardd/boardd.cc index 59579a7fcf..47bff1c5b6 100644 --- a/selfdrive/boardd/boardd.cc +++ b/selfdrive/boardd/boardd.cc @@ -197,12 +197,6 @@ Panda *usb_connect(std::string serial="", uint32_t index=0) { } //panda->enable_deepsleep(); - // power on charging, only the first time. Panda can also change mode and it causes a brief disconneciton -#ifndef __x86_64__ - static std::once_flag connected_once; - std::call_once(connected_once, &Panda::set_usb_power_mode, panda, cereal::PeripheralState::UsbPowerMode::CDP); -#endif - sync_time(panda.get(), SyncTimeDir::FROM_PANDA); return panda.release(); } @@ -391,13 +385,6 @@ std::optional send_panda_states(PubMaster *pm, const std::vector } void send_peripheral_state(PubMaster *pm, Panda *panda) { - auto pandaState_opt = panda->get_state(); - if (!pandaState_opt) { - return; - } - - health_t pandaState = *pandaState_opt; - // build msg MessageBuilder msg; auto evt = msg.initEvent(); @@ -415,7 +402,6 @@ void send_peripheral_state(PubMaster *pm, Panda *panda) { } uint16_t fan_speed_rpm = panda->get_fan_speed(); - ps.setUsbPowerMode(cereal::PeripheralState::UsbPowerMode(pandaState.usb_power_mode_pkt)); ps.setFanSpeedRpm(fan_speed_rpm); pm->send("peripheralState", msg); @@ -491,7 +477,6 @@ void peripheral_control_thread(Panda *panda) { uint16_t prev_fan_speed = 999; uint16_t ir_pwr = 0; uint16_t prev_ir_pwr = 999; - bool prev_charging_disabled = false; unsigned int cnt = 0; FirstOrderFilter integ_lines_filter(0, 30.0, 0.05); @@ -500,23 +485,9 @@ void peripheral_control_thread(Panda *panda) { cnt++; sm.update(1000); // TODO: what happens if EINTR is sent while in sm.update? - if (!Hardware::PC() && sm.updated("deviceState")) { - // Charging mode - bool charging_disabled = sm["deviceState"].getDeviceState().getChargingDisabled(); - if (charging_disabled != prev_charging_disabled) { - if (charging_disabled) { - panda->set_usb_power_mode(cereal::PeripheralState::UsbPowerMode::CLIENT); - LOGW("TURN OFF CHARGING!\n"); - } else { - panda->set_usb_power_mode(cereal::PeripheralState::UsbPowerMode::CDP); - LOGW("TURN ON CHARGING!\n"); - } - prev_charging_disabled = charging_disabled; - } - } - // Other pandas don't have fan/IR to control if (panda->hw_type != cereal::PandaState::PandaType::UNO && panda->hw_type != cereal::PandaState::PandaType::DOS) continue; + if (sm.updated("deviceState")) { // Fan speed uint16_t fan_speed = sm["deviceState"].getDeviceState().getFanSpeedPercentDesired(); @@ -541,6 +512,7 @@ void peripheral_control_thread(Panda *panda) { ir_pwr = 100.0 * (MIN_IR_POWER + ((cur_integ_lines - CUTOFF_IL) * (MAX_IR_POWER - MIN_IR_POWER) / (SATURATE_IL - CUTOFF_IL))); } } + // Disable ir_pwr on front frame timeout uint64_t cur_t = nanos_since_boot(); if (cur_t - last_front_frame_t > 1e9) { diff --git a/selfdrive/boardd/panda.cc b/selfdrive/boardd/panda.cc index d90c4cdab2..685dabd873 100644 --- a/selfdrive/boardd/panda.cc +++ b/selfdrive/boardd/panda.cc @@ -342,10 +342,6 @@ void Panda::enable_deepsleep() { usb_write(0xfb, 0, 0); } -void Panda::set_usb_power_mode(cereal::PeripheralState::UsbPowerMode power_mode) { - usb_write(0xe6, (uint16_t)power_mode, 0); -} - void Panda::send_heartbeat(bool engaged) { usb_write(0xf3, engaged, 0); } diff --git a/selfdrive/boardd/panda.h b/selfdrive/boardd/panda.h index 23a10d5850..1cefc3cb4d 100644 --- a/selfdrive/boardd/panda.h +++ b/selfdrive/boardd/panda.h @@ -86,7 +86,6 @@ class Panda { std::optional get_serial(); void set_power_saving(bool power_saving); void enable_deepsleep(); - void set_usb_power_mode(cereal::PeripheralState::UsbPowerMode power_mode); void send_heartbeat(bool engaged); void set_can_speed_kbps(uint16_t bus, uint16_t speed); void set_data_speed_kbps(uint16_t bus, uint16_t speed); diff --git a/selfdrive/thermald/power_monitoring.py b/selfdrive/thermald/power_monitoring.py index 9f009d3265..7834569088 100644 --- a/selfdrive/thermald/power_monitoring.py +++ b/selfdrive/thermald/power_monitoring.py @@ -108,32 +108,19 @@ class PowerMonitoring: def get_car_battery_capacity(self) -> int: return int(self.car_battery_capacity_uWh) - # See if we need to disable charging - def should_disable_charging(self, ignition: bool, in_car: bool, offroad_timestamp: Optional[float]) -> bool: - if offroad_timestamp is None: - return False - - now = sec_since_boot() - disable_charging = False - disable_charging |= (now - offroad_timestamp) > MAX_TIME_OFFROAD_S - disable_charging |= (self.car_voltage_mV < (VBATT_PAUSE_CHARGING * 1e3)) and (self.car_voltage_instant_mV > (VBATT_INSTANT_PAUSE_CHARGING * 1e3)) - disable_charging |= (self.car_battery_capacity_uWh <= 0) - disable_charging &= not ignition - disable_charging &= (not self.params.get_bool("DisablePowerDown")) - disable_charging &= in_car - disable_charging |= self.params.get_bool("ForcePowerDown") - return disable_charging - # See if we need to shutdown - def should_shutdown(self, peripheralState, ignition, in_car, offroad_timestamp, started_seen): + def should_shutdown(self, ignition: bool, in_car: bool, offroad_timestamp: Optional[float], started_seen: bool): if offroad_timestamp is None: return False now = sec_since_boot() - panda_charging = (peripheralState.usbPowerMode != log.PeripheralState.UsbPowerMode.client) - should_shutdown = False - # Wait until we have shut down charging before powering down - should_shutdown |= (not panda_charging and self.should_disable_charging(ignition, in_car, offroad_timestamp)) + should_shutdown |= (now - offroad_timestamp) > MAX_TIME_OFFROAD_S + should_shutdown |= (self.car_voltage_mV < (VBATT_PAUSE_CHARGING * 1e3)) and (self.car_voltage_instant_mV > (VBATT_INSTANT_PAUSE_CHARGING * 1e3)) + should_shutdown |= (self.car_battery_capacity_uWh <= 0) + should_shutdown &= not ignition + should_shutdown &= (not self.params.get_bool("DisablePowerDown")) + should_shutdown &= in_car + should_shutdown |= self.params.get_bool("ForcePowerDown") should_shutdown &= started_seen or (now > MIN_ON_TIME_S) return should_shutdown diff --git a/selfdrive/thermald/tests/test_power_monitoring.py b/selfdrive/thermald/tests/test_power_monitoring.py index adcdaf427d..5d7463d455 100755 --- a/selfdrive/thermald/tests/test_power_monitoring.py +++ b/selfdrive/thermald/tests/test_power_monitoring.py @@ -129,8 +129,8 @@ class TestPowerMonitoring(unittest.TestCase): while ssb <= start_time + MOCKED_MAX_OFFROAD_TIME: pm.calculate(peripheralState, ignition) if (ssb - start_time) % 1000 == 0 and ssb < start_time + MOCKED_MAX_OFFROAD_TIME: - self.assertFalse(pm.should_disable_charging(ignition, True, start_time)) - self.assertTrue(pm.should_disable_charging(ignition, True, start_time)) + self.assertFalse(pm.should_shutdown(ignition, True, start_time, False)) + self.assertTrue(pm.should_shutdown(ignition, True, start_time, False)) # Test to check policy of stopping charging when the car voltage is too low @parameterized.expand(ALL_PANDA_TYPES) @@ -145,8 +145,8 @@ class TestPowerMonitoring(unittest.TestCase): for i in range(TEST_TIME): pm.calculate(peripheralState, ignition) if i % 10 == 0: - self.assertEqual(pm.should_disable_charging(ignition, True, ssb), (pm.car_voltage_mV < VBATT_PAUSE_CHARGING*1e3)) - self.assertTrue(pm.should_disable_charging(ignition, True, ssb)) + self.assertEqual(pm.should_shutdown(ignition, True, ssb, True), (pm.car_voltage_mV < VBATT_PAUSE_CHARGING*1e3)) + self.assertTrue(pm.should_shutdown(ignition, True, ssb, True)) # Test to check policy of not stopping charging when DisablePowerDown is set def test_disable_power_down(self): @@ -161,8 +161,8 @@ class TestPowerMonitoring(unittest.TestCase): for i in range(TEST_TIME): pm.calculate(peripheralState, ignition) if i % 10 == 0: - self.assertFalse(pm.should_disable_charging(ignition, True, ssb)) - self.assertFalse(pm.should_disable_charging(ignition, True, ssb)) + self.assertFalse(pm.should_shutdown(ignition, True, ssb, False)) + self.assertFalse(pm.should_shutdown(ignition, True, ssb, False)) # Test to check policy of not stopping charging when ignition def test_ignition(self): @@ -176,8 +176,8 @@ class TestPowerMonitoring(unittest.TestCase): for i in range(TEST_TIME): pm.calculate(peripheralState, ignition) if i % 10 == 0: - self.assertFalse(pm.should_disable_charging(ignition, True, ssb)) - self.assertFalse(pm.should_disable_charging(ignition, True, ssb)) + self.assertFalse(pm.should_shutdown(ignition, True, ssb, False)) + self.assertFalse(pm.should_shutdown(ignition, True, ssb, False)) # Test to check policy of not stopping charging when harness is not connected def test_harness_connection(self): @@ -192,8 +192,8 @@ class TestPowerMonitoring(unittest.TestCase): for i in range(TEST_TIME): pm.calculate(peripheralState, ignition) if i % 10 == 0: - self.assertFalse(pm.should_disable_charging(ignition, False,ssb)) - self.assertFalse(pm.should_disable_charging(ignition, False, ssb)) + self.assertFalse(pm.should_shutdown(ignition, False, ssb, False)) + self.assertFalse(pm.should_shutdown(ignition, False, ssb, False)) if __name__ == "__main__": unittest.main() diff --git a/selfdrive/thermald/thermald.py b/selfdrive/thermald/thermald.py index c765af664f..5c2fbd6825 100755 --- a/selfdrive/thermald/thermald.py +++ b/selfdrive/thermald/thermald.py @@ -177,7 +177,6 @@ def thermald_thread(end_event, hw_queue): modem_temps=[], ) - current_filter = FirstOrderFilter(0., CURRENT_TAU, DT_TRML) temp_filter = FirstOrderFilter(0., TEMP_TAU, DT_TRML) should_start_prev = False in_car = False @@ -239,8 +238,6 @@ def thermald_thread(end_event, hw_queue): msg.deviceState.modemTempC = last_hw_state.modem_temps msg.deviceState.screenBrightnessPercent = HARDWARE.get_screen_brightness() - msg.deviceState.usbOnline = HARDWARE.get_usb_present() - current_filter.update(msg.deviceState.batteryCurrent / 1e6) max_comp_temp = temp_filter.update( max(max(msg.deviceState.cpuTempC), msg.deviceState.memoryTempC, max(msg.deviceState.gpuTempC)) @@ -350,15 +347,11 @@ def thermald_thread(end_event, hw_queue): statlog.sample("som_power_draw", som_power_draw) msg.deviceState.somPowerDrawW = som_power_draw - # Check if we need to disable charging (handled by boardd) - msg.deviceState.chargingDisabled = power_monitor.should_disable_charging(onroad_conditions["ignition"], in_car, off_ts) - # Check if we need to shut down - if power_monitor.should_shutdown(peripheralState, onroad_conditions["ignition"], in_car, off_ts, started_seen): + if power_monitor.should_shutdown(onroad_conditions["ignition"], in_car, off_ts, started_seen): cloudlog.warning(f"shutting device down, offroad since {off_ts}") params.put_bool("DoShutdown", True) - msg.deviceState.chargingError = current_filter.x > 0. and msg.deviceState.batteryPercent < 90 # if current is positive, then battery is being discharged msg.deviceState.started = started_ts is not None msg.deviceState.startedMonoTime = int(1e9*(started_ts or 0)) diff --git a/system/hardware/base.py b/system/hardware/base.py index c9c02f3a59..31df1babe0 100644 --- a/system/hardware/base.py +++ b/system/hardware/base.py @@ -78,10 +78,6 @@ class HardwareBase(ABC): def set_bandwidth_limit(upload_speed_kbps: int, download_speed_kbps: int) -> None: pass - @abstractmethod - def get_usb_present(self): - pass - @abstractmethod def get_current_power_draw(self): pass diff --git a/system/hardware/pc/hardware.py b/system/hardware/pc/hardware.py index 60d14e4a6b..564f9e483a 100644 --- a/system/hardware/pc/hardware.py +++ b/system/hardware/pc/hardware.py @@ -50,12 +50,9 @@ class Pc(HardwareBase): def get_network_strength(self, network_type): return NetworkStrength.unknown - def get_usb_present(self): - return False - def get_current_power_draw(self): return 0 - + def get_som_power_draw(self): return 0 diff --git a/system/hardware/tici/hardware.py b/system/hardware/tici/hardware.py index 14a7101c61..340093b604 100644 --- a/system/hardware/tici/hardware.py +++ b/system/hardware/tici/hardware.py @@ -372,7 +372,6 @@ class Tici(HardwareBase): return (self.read_param_file("/sys/class/power_supply/bms/voltage_now", int) * self.read_param_file("/sys/class/power_supply/bms/current_now", int) / 1e12) def shutdown(self): - # Note that for this to work and have the device stay powered off, the panda needs to be in UsbPowerMode::CLIENT! os.system("sudo poweroff") def get_thermal_config(self):