From 35ebc2c6c7d245b810591970769a1f28b97ced4f Mon Sep 17 00:00:00 2001 From: Robbe Date: Mon, 23 Mar 2020 14:40:12 -0700 Subject: [PATCH 1/6] Release lock after exceptions --- selfdrive/thermald/power_monitoring.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/selfdrive/thermald/power_monitoring.py b/selfdrive/thermald/power_monitoring.py index e03ef69855d705..0f8f4a4fae8c9d 100644 --- a/selfdrive/thermald/power_monitoring.py +++ b/selfdrive/thermald/power_monitoring.py @@ -120,6 +120,9 @@ def perform_pulse_measurement(now): set_battery_charging(True) except Exception: cloudlog.exception("Pulsed power measurement failed") + finally: + if self.integration_lock.locked(): + self.integration_lock.release() # Start pulsed measurement and return threading.Thread(target=perform_pulse_measurement, args=(now,)).start() @@ -139,7 +142,10 @@ def perform_pulse_measurement(now): # Do the integration self._perform_integration(now, current_power) except Exception: - cloudlog.exception("Power monitoring calculation failed:") + cloudlog.exception("Power monitoring calculation failed") + finally: + if self.integration_lock.locked(): + self.integration_lock.release() def _perform_integration(self, t, current_power): self.integration_lock.acquire() From 1a9f508a23083f95a8f36bd0740012010542a9f2 Mon Sep 17 00:00:00 2001 From: Robbe Date: Mon, 23 Mar 2020 14:42:11 -0700 Subject: [PATCH 2/6] No pulsed measurement on uno --- selfdrive/thermald/power_monitoring.py | 5 +++-- selfdrive/thermald/thermald.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/selfdrive/thermald/power_monitoring.py b/selfdrive/thermald/power_monitoring.py index 0f8f4a4fae8c9d..9f775621071807 100644 --- a/selfdrive/thermald/power_monitoring.py +++ b/selfdrive/thermald/power_monitoring.py @@ -58,11 +58,12 @@ def panda_current_to_actual_current(panda_current): class PowerMonitoring: - def __init__(self): + def __init__(self, is_uno): self.last_measurement_time = None # Used for integration delta self.power_used_uWh = 0 # Integrated power usage in uWh since going into offroad self.next_pulsed_measurement_time = None self.integration_lock = threading.Lock() + self.is_uno = is_uno # Calculation tick def calculate(self, health): @@ -129,7 +130,7 @@ def perform_pulse_measurement(now): self.next_pulsed_measurement_time = None return - elif self.next_pulsed_measurement_time is None: + elif self.next_pulsed_measurement_time is None and not self.is_uno: # On a charging EON with black panda, or drawing more than 400mA out of a white/grey one # Only way to get the power draw is to turn off charging for a few sec and check what the discharging rate is # We shouldn't do this very often, so make sure it has been some long-ish random time interval diff --git a/selfdrive/thermald/thermald.py b/selfdrive/thermald/thermald.py index 8870ffcc0965b5..f273a40b78bdc1 100755 --- a/selfdrive/thermald/thermald.py +++ b/selfdrive/thermald/thermald.py @@ -187,7 +187,7 @@ def thermald_thread(): handle_fan = handle_fan_eon params = Params() - pm = PowerMonitoring() + pm = PowerMonitoring(is_uno) while 1: health = messaging.recv_sock(health_sock, wait=True) From 095d70b0a9a3b3545afe248c4194911383c6227f Mon Sep 17 00:00:00 2001 From: Robbe Date: Mon, 23 Mar 2020 14:46:27 -0700 Subject: [PATCH 3/6] Fix last_measurement_time=None while integrating when going offroad --- selfdrive/thermald/power_monitoring.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/selfdrive/thermald/power_monitoring.py b/selfdrive/thermald/power_monitoring.py index 9f775621071807..e689c4a7f67a8e 100644 --- a/selfdrive/thermald/power_monitoring.py +++ b/selfdrive/thermald/power_monitoring.py @@ -77,8 +77,10 @@ def calculate(self, health): # Only integrate when there is no ignition # If health is None, we're probably not in a car, so we don't care if health is None or (health.health.ignitionLine or health.health.ignitionCan): + self.integration_lock.acquire() self.last_measurement_time = None self.power_used_uWh = 0 + self.integration_lock.release() return # First measurement, set integration time @@ -150,9 +152,10 @@ def perform_pulse_measurement(now): def _perform_integration(self, t, current_power): self.integration_lock.acquire() - integration_time_h = (t - self.last_measurement_time) / 3600 - self.power_used_uWh += (current_power * 1000000) * integration_time_h - self.last_measurement_time = t + if self.last_measurement_time: + integration_time_h = (t - self.last_measurement_time) / 3600 + self.power_used_uWh += (current_power * 1000000) * integration_time_h + self.last_measurement_time = t self.integration_lock.release() # Get the power usage From fa45ab19f28aed39eff69307e38740f51b3fbc61 Mon Sep 17 00:00:00 2001 From: Robbe Date: Mon, 23 Mar 2020 15:33:18 -0700 Subject: [PATCH 4/6] Also clear next pulsed measurement time --- selfdrive/thermald/power_monitoring.py | 1 + 1 file changed, 1 insertion(+) diff --git a/selfdrive/thermald/power_monitoring.py b/selfdrive/thermald/power_monitoring.py index e689c4a7f67a8e..6f49e35d11dff6 100644 --- a/selfdrive/thermald/power_monitoring.py +++ b/selfdrive/thermald/power_monitoring.py @@ -79,6 +79,7 @@ def calculate(self, health): if health is None or (health.health.ignitionLine or health.health.ignitionCan): self.integration_lock.acquire() self.last_measurement_time = None + self.next_pulsed_measurement_time = None self.power_used_uWh = 0 self.integration_lock.release() return From e42f74e399f599cfcb4b2fc186e43787e5fb172b Mon Sep 17 00:00:00 2001 From: Robbe Date: Mon, 23 Mar 2020 15:49:31 -0700 Subject: [PATCH 5/6] Move around locks --- selfdrive/thermald/power_monitoring.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/selfdrive/thermald/power_monitoring.py b/selfdrive/thermald/power_monitoring.py index 6f49e35d11dff6..e580b2cb7a6445 100644 --- a/selfdrive/thermald/power_monitoring.py +++ b/selfdrive/thermald/power_monitoring.py @@ -118,6 +118,8 @@ def perform_pulse_measurement(now): currents.append(get_battery_current()) time.sleep(1) current_power = ((mean(voltages) / 1000000) * (mean(currents) / 1000000)) + + self.integration_lock.acquire() self._perform_integration(now, current_power * FUDGE_FACTOR) # Enable charging again @@ -144,6 +146,7 @@ def perform_pulse_measurement(now): return # Do the integration + self.integration_lock.acquire() self._perform_integration(now, current_power) except Exception: cloudlog.exception("Power monitoring calculation failed") @@ -152,12 +155,10 @@ def perform_pulse_measurement(now): self.integration_lock.release() def _perform_integration(self, t, current_power): - self.integration_lock.acquire() if self.last_measurement_time: integration_time_h = (t - self.last_measurement_time) / 3600 self.power_used_uWh += (current_power * 1000000) * integration_time_h self.last_measurement_time = t - self.integration_lock.release() # Get the power usage def get_power_used(self): From 5a905367c33ebf678eeb38bae40c321b1f1cb10f Mon Sep 17 00:00:00 2001 From: Robbe Date: Mon, 23 Mar 2020 16:45:09 -0700 Subject: [PATCH 6/6] Locks are good now? --- selfdrive/thermald/power_monitoring.py | 36 ++++++++++++-------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/selfdrive/thermald/power_monitoring.py b/selfdrive/thermald/power_monitoring.py index e580b2cb7a6445..376ec0704d1123 100644 --- a/selfdrive/thermald/power_monitoring.py +++ b/selfdrive/thermald/power_monitoring.py @@ -77,17 +77,17 @@ def calculate(self, health): # Only integrate when there is no ignition # If health is None, we're probably not in a car, so we don't care if health is None or (health.health.ignitionLine or health.health.ignitionCan): - self.integration_lock.acquire() - self.last_measurement_time = None - self.next_pulsed_measurement_time = None - self.power_used_uWh = 0 - self.integration_lock.release() + with self.integration_lock: + self.last_measurement_time = None + self.next_pulsed_measurement_time = None + self.power_used_uWh = 0 return # First measurement, set integration time - if self.last_measurement_time is None: - self.last_measurement_time = now - return + with self.integration_lock: + if self.last_measurement_time is None: + self.last_measurement_time = now + return # Get current power draw somehow current_power = 0 @@ -119,16 +119,12 @@ def perform_pulse_measurement(now): time.sleep(1) current_power = ((mean(voltages) / 1000000) * (mean(currents) / 1000000)) - self.integration_lock.acquire() self._perform_integration(now, current_power * FUDGE_FACTOR) # Enable charging again set_battery_charging(True) except Exception: cloudlog.exception("Pulsed power measurement failed") - finally: - if self.integration_lock.locked(): - self.integration_lock.release() # Start pulsed measurement and return threading.Thread(target=perform_pulse_measurement, args=(now,)).start() @@ -146,19 +142,19 @@ def perform_pulse_measurement(now): return # Do the integration - self.integration_lock.acquire() self._perform_integration(now, current_power) except Exception: cloudlog.exception("Power monitoring calculation failed") - finally: - if self.integration_lock.locked(): - self.integration_lock.release() def _perform_integration(self, t, current_power): - if self.last_measurement_time: - integration_time_h = (t - self.last_measurement_time) / 3600 - self.power_used_uWh += (current_power * 1000000) * integration_time_h - self.last_measurement_time = t + with self.integration_lock: + try: + if self.last_measurement_time: + integration_time_h = (t - self.last_measurement_time) / 3600 + self.power_used_uWh += (current_power * 1000000) * integration_time_h + self.last_measurement_time = t + except Exception: + cloudlog.exception("Integration failed") # Get the power usage def get_power_used(self):