jasonacox / pypowerwall

Python API for Tesla Powerwall and Solar Power Data
MIT License
134 stars 24 forks source link

0.8.0 Refactoring code #77

Closed emptywee closed 6 months ago

emptywee commented 6 months ago

@jasonacox I know it's a little too much to review, but what do you think if we go this way as the first round of refactoring? It's a little hard to sync it with upstream changes, but if we transition to this structure, it'll be easy to both continue maintaining the current code and refactoring it further.

I noticed that meaning of the jsonformat param was inconsistent across the board, so I fixed that as well, but it might break things for any users of the library if they used the methods directly. Mainly it was the poll() method, which treated the param the other way around.

I also asked about code style guide or anything along those lines, you didn't see it I guess :) So I made it look a bit more Python-ish, if you don't mind.

Please, let me know what you think.

jasonacox commented 6 months ago

post method to change battery reserve settings

Yes! ❤️ We have the external tools (scripts for reserve and mode) but there is no reason we couldn't include it in pypowerwall. It makes more sense with the cloud support now. Also, we can make it work via command line, expanding on your argparse work. ;)

emptywee commented 6 months ago

post method to change battery reserve settings

Yes! ❤️ We have the external tools (scripts for reserve and mode) but there is no reason we couldn't include it in pypowerwall. It makes more sense with the cloud support now. Also, we can make it work via command line, expanding on your argparse work. ;)

Sweet! But for now let's focus on merging this in and let folks test it in the wild :)

mcbirse commented 6 months ago

@mcbirse if you have time, check to see if jasonacox/pypowerwall:0.8.0t51-beta4 solves the string errors. I'll merge tomorrow if all goes well.

@emptywee and @jasonacox - great work on this! I have re-tested and no longer have an exception raised for strings or logging spam from that. Functionally all appears to be working well, in both local and cloud mode.

I did note the following though, and am wondering if it should be handled differently?

On my Powerwall, requests to /api/solar_powerwall return an empty response (not a 404 like requests to /api/devices/vitals). It seems only PW+ owners (and maybe PW3) can utilise /api/solar_powerwall. Strange that I do not get a 404 though.

Perhaps it should be treated like the "vitals" call and give an error once, then get disabled? Not sure.

I did also test the api call directly to the TEG installer WiFi, to see if I get any data, which you can see in the logs below (I have bridged 192.168.91.1 onto my local LAN, no headache of multi-homed host setup 😉 ).

03/25/2024 08:36:38 PM [proxy] [DEBUG] 172.18.0.1 "GET /api/solar_powerwall HTTP/1.1" 200 -
03/25/2024 08:36:38 PM [pypowerwall.local.pypowerwall_local] [DEBUG] Empty response from Powerwall at https://192.168.91.1/api/solar_powerwall
03/25/2024 08:36:48 PM [proxy] [DEBUG] 172.18.0.1 "GET /strings HTTP/1.1" 200 -
03/25/2024 08:36:48 PM [pypowerwall.local.pypowerwall_local] [DEBUG] Empty response from Powerwall at https://192.168.91.1/api/solar_powerwall
03/25/2024 08:36:58 PM [proxy] [DEBUG] 172.18.0.1 "GET /alerts/pw HTTP/1.1" 200 -
03/25/2024 08:36:58 PM [pypowerwall.local.pypowerwall_local] [DEBUG] Empty response from Powerwall at https://192.168.91.1/api/solar_powerwall

Not a big issue, but the calls telegraf makes to /strings and /alerts/pw means it will show in the logs all the time if DEBUG is enabled, example below. Kind of seems like an error/notification you would want to see only once like with vitals?

03/25/2024 09:09:08 PM [proxy] [INFO] Connected to Energy Gateway 192.168.91.1 (My home)
03/25/2024 09:09:10 PM [proxy] [DEBUG] 172.18.0.1 "GET /freq HTTP/1.1" 200 -
03/25/2024 09:09:10 PM [proxy] [DEBUG] 172.18.0.1 "GET /temps/pw HTTP/1.1" 200 -
03/25/2024 09:09:10 PM [proxy] [DEBUG] 172.18.0.1 "GET /strings HTTP/1.1" 200 -
03/25/2024 09:09:10 PM [proxy] [DEBUG] 172.18.0.1 "GET /soe HTTP/1.1" 200 -
03/25/2024 09:09:10 PM [proxy] [DEBUG] 172.18.0.1 "GET /aggregates HTTP/1.1" 200 -
03/25/2024 09:09:10 PM [proxy] [DEBUG] 172.18.0.1 "GET /alerts/pw HTTP/1.1" 200 -
03/25/2024 09:09:10 PM [proxy] [DEBUG] 172.18.0.1 "GET /pod HTTP/1.1" 200 -
03/25/2024 09:09:10 PM [pypowerwall.local.pypowerwall_local] [ERROR] 404 Powerwall API not found at https://192.168.91.1/api/devices/vitals
03/25/2024 09:09:10 PM [pypowerwall.local.pypowerwall_local] [ERROR] 404 Powerwall API not found at https://192.168.91.1/api/devices/vitals
03/25/2024 09:09:10 PM [pypowerwall.local.pypowerwall_local] [ERROR] 404 Powerwall API not found at https://192.168.91.1/api/devices/vitals
03/25/2024 09:09:10 PM [pypowerwall.local.pypowerwall_local] [ERROR] 404 Powerwall API not found at https://192.168.91.1/api/devices/vitals
03/25/2024 09:09:10 PM [pypowerwall.local.pypowerwall_local] [ERROR] 404 Powerwall API not found at https://192.168.91.1/api/devices/vitals
03/25/2024 09:09:10 PM [pypowerwall.local.pypowerwall_local] [ERROR] Firmware 240400 detected - Does not support vitals API - disabling.
03/25/2024 09:09:10 PM [pypowerwall.local.pypowerwall_local] [ERROR] Firmware 240400 detected - Does not support vitals API - disabling.
03/25/2024 09:09:10 PM [pypowerwall.local.pypowerwall_local] [ERROR] Firmware 240400 detected - Does not support vitals API - disabling.
03/25/2024 09:09:10 PM [pypowerwall.local.pypowerwall_local] [DEBUG] Empty response from Powerwall at https://192.168.91.1/api/solar_powerwall
03/25/2024 09:09:10 PM [pypowerwall.local.pypowerwall_local] [DEBUG] Empty response from Powerwall at https://192.168.91.1/api/solar_powerwall
03/25/2024 09:09:11 PM [pypowerwall.local.pypowerwall_local] [ERROR] Firmware 240400 detected - Does not support vitals API - disabling.
03/25/2024 09:09:11 PM [pypowerwall.local.pypowerwall_local] [ERROR] Firmware 240400 detected - Does not support vitals API - disabling.
03/25/2024 09:09:15 PM [proxy] [DEBUG] 172.18.0.1 "GET /freq HTTP/1.1" 200 -
03/25/2024 09:09:15 PM [proxy] [DEBUG] 172.18.0.1 "GET /temps/pw HTTP/1.1" 200 -
03/25/2024 09:09:15 PM [proxy] [DEBUG] 172.18.0.1 "GET /strings HTTP/1.1" 200 -
03/25/2024 09:09:15 PM [proxy] [DEBUG] 172.18.0.1 "GET /soe HTTP/1.1" 200 -
03/25/2024 09:09:15 PM [proxy] [DEBUG] 172.18.0.1 "GET /aggregates HTTP/1.1" 200 -
03/25/2024 09:09:15 PM [proxy] [DEBUG] 172.18.0.1 "GET /alerts/pw HTTP/1.1" 200 -
03/25/2024 09:09:15 PM [proxy] [DEBUG] 172.18.0.1 "GET /pod HTTP/1.1" 200 -
03/25/2024 09:09:15 PM [pypowerwall.local.pypowerwall_local] [DEBUG] Empty response from Powerwall at https://192.168.91.1/api/solar_powerwall
03/25/2024 09:09:15 PM [pypowerwall.local.pypowerwall_local] [DEBUG] Empty response from Powerwall at https://192.168.91.1/api/solar_powerwall
03/25/2024 09:09:20 PM [proxy] [DEBUG] 172.18.0.1 "GET /soe HTTP/1.1" 200 -
03/25/2024 09:09:20 PM [proxy] [DEBUG] 172.18.0.1 "GET /aggregates HTTP/1.1" 200 -
03/25/2024 09:09:20 PM [proxy] [DEBUG] 172.18.0.1 "GET /temps/pw HTTP/1.1" 200 -
03/25/2024 09:09:20 PM [proxy] [DEBUG] 172.18.0.1 "GET /strings HTTP/1.1" 200 -
03/25/2024 09:09:20 PM [proxy] [DEBUG] 172.18.0.1 "GET /alerts/pw HTTP/1.1" 200 -
03/25/2024 09:09:20 PM [proxy] [DEBUG] 172.18.0.1 "GET /freq HTTP/1.1" 200 -
03/25/2024 09:09:20 PM [proxy] [DEBUG] 172.18.0.1 "GET /pod HTTP/1.1" 200 -
03/25/2024 09:09:20 PM [pypowerwall.local.pypowerwall_local] [DEBUG] Empty response from Powerwall at https://192.168.91.1/api/solar_powerwall
03/25/2024 09:09:20 PM [pypowerwall.local.pypowerwall_local] [DEBUG] Empty response from Powerwall at https://192.168.91.1/api/solar_powerwall

Thoughts?

BTW because of the lack of the solar_powerwalls api, I do not get any alerts in local mode (not even mock alerts like I do in cloud mode such as the SystemConnectedToGrid).

All good to merge either way - seems to be working well. 👍

emptywee commented 6 months ago

@mcbirse your setup seems a lot like mine. I've got a tesla gateway, enphase micro inverters, powerwall2 batteries. I'm getting same empty responses here and there with 200 OK status codes.

I think we could discuss how the library should behave and treat them in the next iteration of changes.

Maybe even introduce a hybrid mode? Where it goes for data to cloud if not available in local, all being automatically and transparent for users?

Just a thought.

jasonacox commented 6 months ago

Thanks @mcbirse ! We could definitely add some Alerts. I started an issue https://github.com/jasonacox/Powerwall-Dashboard/issues/457 I almost added to this release but don't want to delay getting this out. Scope for the next dot release. ;-)

Also, in the past, I had a few versions with memory leaks. This release looks good.

image

Past 24 hours.

Here we go... 0.8.0 🚀

emptywee commented 6 months ago

Hooray! We finally did it! I'll drink to that!

jasonacox commented 6 months ago

Thanks again @emptywee !! Yes, Cheers 🍻

Thanks for the great work. 🙏