Skip to content

refactor: fetch component data without using device uri#169

Merged
jasonacox merged 2 commits into
jasonacox:mainfrom
johncuthbertuk:refactor/device-query
Apr 26, 2025
Merged

refactor: fetch component data without using device uri#169
jasonacox merged 2 commits into
jasonacox:mainfrom
johncuthbertuk:refactor/device-query

Conversation

@johncuthbertuk
Copy link
Copy Markdown

Switches from using device specific uri to resolve 502 error on Powerwall 3

@jasonacox
Copy link
Copy Markdown
Owner

Test proxy image: jasonacox/pypowerwall:0.12.10t72-beta10

@jasonacox jasonacox requested a review from Copilot April 26, 2025 06:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the API calls for fetching Powerwall 3 component data by removing device-specific URIs and the sender DIN field to address a 502 error on some Powerwall 3 systems.

  • Removed assignment of sender.din from the Protobuf message in several locations.
  • Updated the endpoint URL from using a device-specific URI to a generic one and adjusted the tail value accordingly.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tools/tedapi/PW3_Vitals.py Removed sender DIN and updated tail value and URL to eliminate device-specific references.
tools/tedapi/PW3_Strings.py Similar removal of sender DIN and URL update to reflect the new endpoint.
tools/tedapi/ComponentsQuery.py Adjusted logging message, removed sender DIN, and updated the URL consistently.
pypowerwall/tedapi/init.py Removed sender DIN from two functions and updated tail value and URL accordingly.
RELEASE.md Updated release notes to document the change from a device-specific to a generic URI.
Comments suppressed due to low confidence (6)

tools/tedapi/PW3_Vitals.py:185

  • Removal of 'sender.din' aligns with the updated API endpoint, but please confirm that downstream processing does not rely on this field.
pb.message.sender.din = din  # DIN of Primary Powerwall 3 / System

tools/tedapi/PW3_Strings.py:156

  • Ensure that removing 'sender.din' in this context does not impact any internal routing or logging mechanisms.
pb.message.sender.din = din  # DIN of Primary Powerwall 3 / System

tools/tedapi/ComponentsQuery.py:148

  • Confirm that the elimination of the 'sender.din' assignment in this query is fully supported by the backend and does not affect identification.
pb.message.sender.din = din  # DIN of Primary Powerwall 3 / System

pypowerwall/tedapi/init.py:683

  • Verify that removing the 'sender.din' field here remains compatible with the expected message structure after the refactor.
pb.message.sender.din = din  # DIN of Primary Powerwall 3 / System

pypowerwall/tedapi/init.py:822

  • Ensure that the updated message without 'sender.din' preserves all necessary information for backend processing under the new URI.
pb.message.sender.din = self.din  # DIN of Primary Powerwall 3 / System

tools/tedapi/PW3_Vitals.py:192

  • Changing 'tail.value' from 2 to 1 appears to be required for the new endpoint; please verify that this value is consistent with the updated protocol requirements.
pb.tail.value = 1

@jasonacox jasonacox merged commit 0998eb7 into jasonacox:main Apr 26, 2025
10 checks passed
@jasonacox
Copy link
Copy Markdown
Owner

Pypi: https://pypi.org/project/pypowerwall/0.12.10/

Proxy container: jasonacox/pypowerwall:0.12.10t72

@jasonacox
Copy link
Copy Markdown
Owner

Hit this right away:


AttributeError: 'NoneType' object has no attribute 'get'

Traceback (most recent call last):
File "/usr/local/lib/python3.10/socketserver.py", line 683, in process_request_thread
self.finish_request(request, client_address)
File "/usr/local/lib/python3.10/socketserver.py", line 360, in finish_request
self.RequestHandlerClass(request, client_address, self)
File "/usr/local/lib/python3.10/socketserver.py", line 747, in init
self.handle()
File "/usr/local/lib/python3.10/http/server.py", line 433, in handle
self.handle_one_request()
File "/usr/local/lib/python3.10/http/server.py", line 421, in handle_one_request
method()
File "/app/server.py", line 501, in do_GET
vitals = pw.vitals() or {}
File "/app/pypowerwall/init.py", line 379, in vitals
output = self.client.vitals()
File "/app/pypowerwall/tedapi/pypowerwall_tedapi.py", line 562, in vitals
return self.tedapi.vitals()
File "/app/pypowerwall/tedapi/init.py", line 1078, in vitals
fan_speeds = self.extract_fan_speeds(status)
File "/app/pypowerwall/tedapi/init.py", line 946, in extract_fan_speeds
for component in components.get("msa", []):
AttributeError: 'NoneType' object has no attribute 'get'

@johncuthbertuk johncuthbertuk deleted the refactor/device-query branch April 26, 2025 07:05
@johncuthbertuk
Copy link
Copy Markdown
Author

Wonder how I didn't see that error, although no fan speeds on PW3 🤔

@jasonacox
Copy link
Copy Markdown
Owner

Easy fix.

@jasonacox
Copy link
Copy Markdown
Owner

It seems that this change is causing some issue for PW3 owners with multiple PW3s?

#172

@johncuthbertuk
Copy link
Copy Markdown
Author

It seems that this change is causing some issue for PW3 owners with multiple PW3s?

#172

It did cross my mind what the difference was between my configuration vs others who weren’t seeing any issues. Looks like that difference is 1x PW3 (my setup) vs 2+ PW3s. There could also be another slight difference in behaviour if they’re full PW3s vs expansion units (no inverter/MPPTs).

With the above in mind, a quick fix might be to have the logic handle if querying the additional batteries use the specific device URI. Although, looking over the code, I wonder if pb.tail.value = 1 has any significance and if that needs to iterate?

@heynorm
Copy link
Copy Markdown

heynorm commented Apr 30, 2025 via email

@jasonacox
Copy link
Copy Markdown
Owner

With the above in mind, a quick fix might be to have the logic handle if querying the additional batteries use the specific device URI.

Yes, I think that is what we need to do. I have no way to test it locally so doing this basic logic seems like the best approach that would work for both of your cases. Hopefully that generalizes for other users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants