Use Neurio for TEDAPI data when Tesla Remote Meter is not present #157
Conversation
`METER_X` and `METER_Y` are specific to a Tesla electrical meter. If your system doesn't have these, Tesla's documentation says to get data from the Neurio. References: - https://energylibrary.tesla.com/docs/Public/EnergyStorage/Powerwall/General/MeteringGuide/en-us/GUID-932E78BF-6DFF-4CCA-9741-E4793E9F4314.html - https://energylibrary.tesla.com/docs/Public/EnergyStorage/Powerwall/General/MeteringGuide/en-us/GUID-A46FB1D7-AEE5-4B66-99FF-39FE576F2B2D.html
Nexarian
force-pushed
the
use_neurio_for_tedapi_data
branch
from
0be8170 to
f5c331e
Compare
|
Thanks @Nexarian 🙏 - So far it looks good and I can imagine solar only owners will love this. I do want to spend a bit more time on the code review and separate this out as a separate release. Stand by... |
|
@jasonacox Take your time. The core revelation is that the Tesla meter reports as METER_X and METER_Y (see linked documentation), and the Neurio is reported differently. |
|
Sorry again here too... delayed more by personal things than the effort to test. I've uploaded and started a test of the container: |
|
@jasonacox Any further thoughts on this one? |
|
I'm going to add more references here for posterity on exactly what Tesla's definition of Meter X, Y, and Z are: https://energylibrary.tesla.com/docs/Public/EnergyStorage/Powerwall/2/TroubleshootingManual/SetupAndInstall/en-us/GUID-9BAC4636-9BB2-4403-88EA-DD45A6132141.html Meter X: Primary
|
|
I'm gun shy a bit. The last few changes broke some edge cases (e.g. impacting some PW3 owners). Do we have enough logic to handle cases where users w/o Neurio won't see issues? Can we get others to test this and report results / errors? jasonacox/pypowerwall:0.12.9t69-beta157 |
|
First, I apologize for the previous issues. From what I understand, it was edge cases resulting from unique configurations. I'm afraid that problem will continue to happen. Being extremely shy is one way. But let me propose another path: Assuming Tesla (or their power division) survives, different configurations and data edge cases will be a persistent thorn in our side. There is always a risk that a "should never be empty" dictionary will be empty, or a "never be None" field will be None, etc. So: How about I make you a deal -- If I implement some unit tests first, and then put guards in place around the relevant functions, we can use that as a way to expedite this and future changes vis-a-vis testing? |
|
Thanks @Nexarian - I'm back from a lot of travel and plan to look at this, this weekend. I think this may help jasonacox/Powerwall-Dashboard#629 or at least give us some insight (perhaps Meter Z: Backup Switch related). |
|
Welcome back! This PR (still in draft, I have more work to do on it) has the simplest possible unit test to get started on this. I think in the future we need to have "golden" examples of data generated from each type of Tesla system our userbase has encountered (the raw JSON), and then use that for our tests. This is still not perfect, because, as in the example with fans, we're adding NEW fields, and it's unclear how each model will output data. Still, it's a start. |
There was a problem hiding this comment.
Pull Request Overview
This PR updates the TEDAPI integration to use Neurio data when the Tesla Remote Meter is unavailable.
- Introduces a Neurio branch in get_api_meters_aggregates for aggregating current and voltage readings.
- Adds helper methods in init.py to derive meter configuration and aggregate Neurio data for vitals.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pypowerwall/tedapi/pypowerwall_tedapi.py | Modified meter reading aggregation logic to incorporate Neurio data instead of Tesla meter readings when necessary. |
| pypowerwall/tedapi/init.py | Added enhancements for deriving meter configuration and aggregating Neurio data to support vitals generation. |
Comments suppressed due to low confidence (1)
pypowerwall/tedapi/pypowerwall_tedapi.py:373
- Using the loop variable 'i' to decide whether to pass v3n into compute_LL_voltage may lead to unintended behavior if the last iteration does not correspond with a valid third CT reading. Consider tracking the count of valid CT readings with an explicit counter and use that for the decision.
vll_site = compute_LL_voltage(v1n = v1n, v2n = v2n, v3n = v3n if i == 3 else None)
| status_data=status, | ||
| meter_config_data=self.tedapi.derive_meter_config(config) | ||
| ) | ||
| for i, data in enumerate(neurio_data[1].values()): |
There was a problem hiding this comment.
The loop uses enumerate, which produces a 0-based index, yet the conditions below check for i == 1, i == 2, and i == 3. This mismatch can lead to skipping the first CT reading; please adjust the indexing to clearly reflect the intended one-based logic.
| for i, data in enumerate(neurio_data[1].values()): | |
| for i, data in enumerate(neurio_data[1].values(), start=1): |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
I wondered about this. Is the logic to skip CT0 and the line voltages are on CT1 - CT3 ?
There was a problem hiding this comment.
Nothing so complex: Turns out I noticed and addressed this in the branch I'm running but forgot to contribute back! Oops!
Because we skip CT not labeled "site" we have to only conditionally increment i, meaning slick solutions like enumerate don't work here. We have to do it the old-fashioned way!
METER_XandMETER_Yare specific to a Tesla electrical meter. If your system doesn't have these, Tesla's documentation says to get data from the Neurio. Solar-only installs tend to use Neurio, Powerwalls tend to use the Tesla meter.References:
Testing
This allowed me to get current data from my Neurios on my Tesla inverter solar-only setup.