-
Notifications
You must be signed in to change notification settings - Fork 121
Fix Tesla/Powerwall data handling and improve robustness of control loop #3232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
e2d1824
15bae70
38fbc46
2d479e0
d5b572d
d0b3498
2346e01
5cfe1c7
71f4e41
2932b51
b4be565
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| #!/usr/bin/env python3 | ||
| import logging | ||
| import math | ||
| import time | ||
| from requests import HTTPError | ||
|
|
||
| from modules.common.abstract_device import AbstractCounter | ||
|
|
@@ -17,34 +19,187 @@ | |
| def __init__(self, component_config: TeslaCounterSetup) -> None: | ||
| self.component_config = component_config | ||
|
|
||
| # Throttle diagnostic logging (to avoid log spam in a 10s polling cycle) | ||
| self._last_energy_debug_log_ts: float = 0.0 | ||
|
|
||
| def initialize(self) -> None: | ||
| self.store = get_counter_value_store(self.component_config.id) | ||
| self.fault_state = FaultState(ComponentInfo.from_component_config(self.component_config)) | ||
|
|
||
| @staticmethod | ||
| def _safe_float(val, default: float = 0.0) -> float: | ||
| try: | ||
| if val is None: | ||
| return default | ||
| return float(val) | ||
| except (TypeError, ValueError): | ||
| return default | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Die Methode und die Aufrufe kann entfallen, das ist zentral umgesetzt. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Danke, ich entferne _safe_float() und verlasse mich auf die zentrale Behandlung. Mein ursprünglicher Gedanke war, unvollständige Tesla-Antworten robuster abzufangen. |
||
|
|
||
| @staticmethod | ||
| def _nearly_zero(x: float, eps: float = 1e-9) -> bool: | ||
| return abs(x) < eps | ||
|
|
||
| def _calc_currents_and_pf_from_pqu( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Diese Berechnung erfolgt zentral im CounterState, wenn keine Ströme gesetzt sind, aber Phasenspannungen und Phasenleistungen vorhanden sind. Die Berechnung soll nicht in jedem Modul erfolgen.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Im CounterState sind jedoch nur die Wirkleistungen vorhanden. Reale Ströme können dann nicht berechnet werden. Das sollte in einem anderen PR grundlegend überarbeitet werden.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Danke für den Hinweis. Dann würde ich für diesen PR die lokale Stromberechnung im Tesla-Counter beibehalten. In meinem Setup liefert Tesla/Neurio für die Phase currents nur 0-Werte, und ohne diese Ableitung funktioniert das Lastmanagement nicht zuverlässig. Den generischen Ansatz zur Berechnung realer currents im CounterState würde ich nicht in diesem PR lösen. Das wäre aus meiner Sicht ein separates Thema, wie von dir vorgeschlagen. Ich reduziere den PR daher weiter auf: reduzierte /api/status-Nutzung Ist das in eurem Interesse?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ich habe das bei mir umgesetzt und teste die Änderungen über Ostern. Anschließend würde ich den PR aktualisieren... |
||
| self, voltages: list[float], p_list: list[float], q_list: list[float] | ||
| ) -> tuple[list[float], list[float]]: | ||
| """ | ||
| Calculates signed currents (A) and signed power factors per phase from P/Q/U. | ||
| Convention: | ||
| - sign of current follows sign of active power P (import +, export -) | ||
| - PF = P / S (signed) | ||
| - S = sqrt(P^2 + Q^2) | ||
| - I = S / U (signed via P) | ||
| """ | ||
| currents: list[float] = [0.0, 0.0, 0.0] | ||
| pfs: list[float] = [0.0, 0.0, 0.0] | ||
|
|
||
| for i in range(3): | ||
| u = self._safe_float(voltages[i], 0.0) | ||
| p = self._safe_float(p_list[i], 0.0) | ||
| q = self._safe_float(q_list[i], 0.0) | ||
|
|
||
| if self._nearly_zero(u): | ||
| currents[i] = 0.0 | ||
| pfs[i] = 0.0 | ||
| continue | ||
|
|
||
| s = math.sqrt(p * p + q * q) # apparent power [VA] | ||
|
|
||
| if self._nearly_zero(s): | ||
| currents[i] = 0.0 | ||
| pfs[i] = 0.0 | ||
| continue | ||
|
|
||
| # signed PF: magnitude P/S, sign from P | ||
| pf = p / s | ||
| pfs[i] = pf | ||
|
|
||
| # signed current: magnitude S/U, sign from P | ||
| i_mag = s / u | ||
| currents[i] = i_mag if p >= 0 else -i_mag | ||
|
|
||
| return currents, pfs | ||
|
|
||
| def update(self, client: PowerwallHttpClient, aggregate): | ||
| # read firmware version | ||
| status = client.get_json("/api/status") | ||
| log.debug('Firmware: ' + status["version"]) | ||
| # read firmware version only at startup or after cookie renewal | ||
| if getattr(client, "cookie_renewed", False) or not getattr(self.store, "firmware", None): | ||
| try: | ||
| status = client.get_json("/api/status", fail_fast=False) | ||
| if isinstance(status, dict): | ||
| self.store.firmware = status.get("version", "") | ||
| log.debug("Firmware: %s", self.store.firmware) | ||
| except Exception: | ||
| # Non-critical: ignore status retrieval errors | ||
| pass | ||
| try: | ||
| # read additional info if firmware supports | ||
| meters_site = client.get_json("/api/meters/site") | ||
| cached = meters_site[0]["Cached_readings"] | ||
|
|
||
| # --- voltages / powers / reactive powers (per phase) --- | ||
| voltages = [self._safe_float(cached.get(f"v_l{phase}n")) for phase in range(1, 4)] | ||
| p_list = [self._safe_float(cached.get(f"real_power_{ph}")) for ph in ["a", "b", "c"]] | ||
| q_list = [self._safe_float(cached.get(f"reactive_power_{ph}")) for ph in ["a", "b", "c"]] | ||
|
|
||
| # --- currents from API (often all 0 on Neurio/Tesla) --- | ||
| api_currents = [self._safe_float(cached.get(f"i_{ph}_current")) for ph in ["a", "b", "c"]] | ||
|
|
||
| # --- energy counters --- | ||
| # IMPORTANT: | ||
| # We use ONLY aggregate["site"]["energy_imported"/"energy_exported"] as the source for | ||
| # imported/exported counters (same behaviour as the old/original module). | ||
| # | ||
| # We still *read* the per-phase energy fields (if available) ONLY for throttled diagnostic logging, | ||
| # to see whether per-phase sums diverge from aggregate counters. | ||
| def has_phase_energy(prefix: str) -> bool: | ||
| return all((prefix + s) in cached for s in ["_a", "_b", "_c"]) | ||
|
|
||
| def sum_phase_energy_wh(prefix: str) -> float: | ||
| # Keep as Wh (openWB uses Wh in many places; your logs match that) | ||
| return ( | ||
| self._safe_float(cached.get(prefix + "_a")) | ||
| + self._safe_float(cached.get(prefix + "_b")) | ||
| + self._safe_float(cached.get(prefix + "_c")) | ||
| ) | ||
|
|
||
| imported = self._safe_float(aggregate["site"]["energy_imported"]) | ||
| exported = self._safe_float(aggregate["site"]["energy_exported"]) | ||
|
|
||
| # --- throttled diagnostics (1x per hour) --- | ||
| # Log aggregate vs. per-phase sums to spot counter mismatches without affecting behaviour. | ||
| now = time.time() | ||
|
NicoP-codeing marked this conversation as resolved.
Outdated
|
||
| if (now - self._last_energy_debug_log_ts) >= 3600: | ||
| self._last_energy_debug_log_ts = now | ||
|
|
||
| phase_imported = None | ||
| phase_exported = None | ||
| if has_phase_energy("energy_imported"): | ||
| phase_imported = sum_phase_energy_wh("energy_imported") | ||
| if has_phase_energy("energy_exported"): | ||
| phase_exported = sum_phase_energy_wh("energy_exported") | ||
|
|
||
| # These totals are present in Cached_readings too, but in some firmwares they can be absurd. | ||
| cached_total_imported = cached.get("energy_imported") | ||
| cached_total_exported = cached.get("energy_exported") | ||
|
|
||
| # Deltas only make sense if units match; still useful to see large divergences. | ||
| delta_imported = (phase_imported - imported) if phase_imported is not None else None | ||
| delta_exported = (phase_exported - exported) if phase_exported is not None else None | ||
|
|
||
| log.info( | ||
| "Powerwall energy debug (1h): aggregate_imported=%s aggregate_exported=%s " | ||
| "phase_sum_imported=%s phase_sum_exported=%s delta_imported=%s delta_exported=%s " | ||
| "cached_total_imported=%s cached_total_exported=%s", | ||
| imported, | ||
| exported, | ||
| phase_imported, | ||
| phase_exported, | ||
| delta_imported, | ||
| delta_exported, | ||
| cached_total_imported, | ||
| cached_total_exported, | ||
| ) | ||
|
|
||
| # --- calculate PF + fallback currents if missing --- | ||
| calculated_currents, power_factors = self._calc_currents_and_pf_from_pqu( | ||
| voltages=voltages, p_list=p_list, q_list=q_list | ||
| ) | ||
|
|
||
| # If all API currents are 0 -> use calculated currents | ||
| if all(self._nearly_zero(i) for i in api_currents): | ||
| currents = calculated_currents | ||
| log.debug( | ||
| "Tesla/Neurio phase currents missing (all 0). Calculated currents from P/Q and U." | ||
| ) | ||
| else: | ||
| currents = api_currents | ||
| # PF still useful even if currents exist | ||
| log.debug("Using phase currents from Tesla/Neurio API.") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wenn es Modelle gibt, die keine Ströme liefern, dann dürfen diese im CounterState nicht gesetzt werden. Dann werden sie automatisch gesetzt. |
||
|
|
||
| powerwall_state = CounterState( | ||
| imported=aggregate["site"]["energy_imported"], | ||
| exported=aggregate["site"]["energy_exported"], | ||
| power=aggregate["site"]["instant_power"], | ||
| voltages=[meters_site[0]["Cached_readings"]["v_l" + str(phase) + "n"] for phase in range(1, 4)], | ||
| currents=[meters_site[0]["Cached_readings"]["i_" + phase + "_current"] for phase in ["a", "b", "c"]], | ||
| powers=[meters_site[0]["Cached_readings"]["real_power_" + phase] for phase in ["a", "b", "c"]] | ||
| imported=imported, | ||
| exported=exported, | ||
| power=self._safe_float(aggregate["site"]["instant_power"]), | ||
| voltages=voltages, | ||
| currents=currents, | ||
| powers=p_list, | ||
| power_factors=power_factors, | ||
| frequency=int(round(self._safe_float(aggregate["site"].get("frequency", 50)))), | ||
| serial_number=str(cached.get("serial_number", "")) if cached.get("serial_number") else "", | ||
| ) | ||
| except (KeyError, HTTPError): | ||
|
|
||
| except (KeyError, HTTPError, IndexError, TypeError) as e: | ||
| log.debug( | ||
| "Firmware seems not to provide detailed phase measurements. Fallback to total power only.") | ||
| "Firmware seems not to provide detailed phase measurements. Fallback to total power only. (%s)", | ||
| str(e), | ||
| ) | ||
| powerwall_state = CounterState( | ||
| imported=aggregate["site"]["energy_imported"], | ||
| exported=aggregate["site"]["energy_exported"], | ||
| power=aggregate["site"]["instant_power"] | ||
| imported=self._safe_float(aggregate["site"]["energy_imported"]), | ||
| exported=self._safe_float(aggregate["site"]["energy_exported"]), | ||
| power=self._safe_float(aggregate["site"]["instant_power"]), | ||
| ) | ||
|
|
||
| self.store.set(powerwall_state) | ||
|
|
||
|
|
||
| component_descriptor = ComponentDescriptor(configuration_factory=TeslaCounterSetup) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default Werte werden zentral im CounterState gesetzt, damit sie konsistent sind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bitte die Methode entfernen. Wenn Werte in einem Zyklus fehlen, wird eine Exception geworfen.
Es wird inkonsistent und der Code in jedem Modul sehr umfangreich, wenn überall default-Werte gesetzt werden.