Skip to content

Issue #162: add /pw/XXX endpoints to expose Powerwall() API methods#166

Merged
jasonacox merged 3 commits into
jasonacox:mainfrom
JohnJ9ml:main
Apr 18, 2025
Merged

Issue #162: add /pw/XXX endpoints to expose Powerwall() API methods#166
jasonacox merged 3 commits into
jasonacox:mainfrom
JohnJ9ml:main

Conversation

@JohnJ9ml
Copy link
Copy Markdown

Issue #162: add /pw/XXX endpoints to expose Powerwall() API methods as JSON-returning endpoints

@JohnJ9ml
Copy link
Copy Markdown
Author

My first attempt at any of this. consider it a learning exercise.

@jasonacox jasonacox requested a review from Copilot April 17, 2025 04:49
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.

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread proxy/server.py Outdated
message = json.dumps(pw.system_status(False))
elif self.path == '/pw/grid_status':
grid_status_str = pw.grid_status(type="json")
message = json.loads(grid_status_str)
Copy link

Copilot AI Apr 17, 2025

Choose a reason for hiding this comment

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

grid_status_str is being parsed with json.loads on line 807, then immediately overwritten on line 808, which could lead to redundancy or inconsistent JSON handling. Consider using the properly parsed JSON object or removing the unnecessary conversion.

Suggested change
message = json.loads(grid_status_str)

Copilot uses AI. Check for mistakes.
@jasonacox
Copy link
Copy Markdown
Owner

Great job @JohnJ9ml ! I have some suggestions but basics are here.

Comment thread proxy/server.py Outdated
message = json.dumps(fans)
else:
message = '{}'
elif self.path in PW_ALLOWLIST:
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Actually, we dont need this we can use:

elif self.path.startswith('/pw/'):

We can then eliminate the big array and I'm thinking about just using a lambda function map. I've been planning to do that for all the rest, but this would be a good start.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'll submit a commit with those adjustments.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I knew this check would suffice, but I didn't want to break consistency - if there was already one array, it would be a shame to impose some other style on something I was just enhancing. I anticipated there was something more table-driven in mind.

Thanks for making the adjustments - you figured that would save me weeks of trying to figure out how to do it myself, right?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Actually, I just had Copilot rewrite it. ;)

@jasonacox jasonacox merged commit 5ba8895 into jasonacox:main Apr 18, 2025
10 checks passed
@JohnJ9ml
Copy link
Copy Markdown
Author

I"ve been using a local copy of this for a week now, and it works well for me. The one anomaly I've noted is that occasionally the HTTP connection kicks up errors, and I end up getting empty data back. My suspicion is that this little box sitting right next to the Dashboard host combined is just a bit too much for the inverter to handle. I look forward to the next Dashboard update that includes this fix, at which time I'll shut my proxy off and see how everything handles the load of my process running against the same proxy that's supporting the Dashboard.

Odd note: when the HTTP connection seems to have woes, it's generally (but not always) at night - i.e. when the sun is down and one would think there's seemingly not as much data for the inverter to process. Amusing, but seems a general pattern at present. (Then again, the next 23 coin flips could all come up tails, at which point.....)

@jasonacox
Copy link
Copy Markdown
Owner

Hi @JohnJ9ml - What error do you see in the logs when this happens?

@JohnJ9ml
Copy link
Copy Markdown
Author

The errors are generally timeouts and no route to host errors.

I'm just accepting right now that two proxies together are likely abusing the poor little inverter antenna, and waiting out the next release.

To its credit, the proxy hangs out, waits its time, retries for the next request, and eventually succeeds. I'm pleased it stays on its feet. I have to do a bit of filtering on my end because the data comes back with 0 values (for the power() call, for example), but that's perfectly reasonable in my mind for a system meant to run for months at a time.

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.

3 participants