search

jasonacox / pypowerwall

Python API for Tesla Powerwall and Solar Power Data
MIT License
120 stars 21 forks source link

v0.7.7 - Battery Data and Network Scanner #67

Closed jasonacox closed 5 months ago

jasonacox commented 5 months ago

Proxy v0.7.7 t40 - Battery Vitals Update

This update will help address some of the data lost with the removal of the vitals data starting with Firmware 23.44.0 as raised in https://github.com/jasonacox/Powerwall-Dashboard/discussions/402.

pyPowerwall v0.7.7 - Network Scanner Update

mcbirse commented 5 months ago

Before merging would you be interested in a change to the network scanning?

I just did a quick hack/test making it multi-threaded, and using no more than 10 threads at a time it scans my entire network in a fraction of the time...

jasonacox commented 5 months ago

Yes please @mcbirse ! Can you merge it into this branch / PR?

mcbirse commented 5 months ago

Will do - it's a bit raw at the moment so I will cleanup/refine before committing.

I literally just spent the last 20mins seeing if it would work, so have not spent much time on the code/testing yet. And I may have a bug where it actually created 255 threads simultaneously. It sure got a result quickly! 🤣

mcbirse commented 5 months ago

Hmm no it was 10 threads only. Wow - the improvement to scanning speed this makes is amazing!

I had noted your comment about the slow scanning, and had noticed this before myself as well with a thought to make it multi-threaded.

Just need to add some options, to e.g. control the max number of threads etc. Stay tuned.

mcbirse commented 5 months ago

Added the changes for simultaneously scanning multiple hosts with the network scan using threads.

I was testing and trying to determine a sensible default max number of threads?

I've gone with 32 - it seems like a lot, but researching various network scan tools out there it actually seems reasonable. I found others with about 30 as a default, another with 64 (Angry IP scanner)... and generally more than 10 is common.

On Windows the scan is slow, so a larger number of threads reduces the overall scan time. On Linux it will be lightning fast to scan your whole subnet (depending on how many actual live hosts you have on the network).

Please test - happy to adjust defaults. I have not tested impact/load on system resources due to the multiple threads.

NOTE: Due to the multi-threading, how the network scan outputs results had to be modified...

mcbirse commented 5 months ago

TODO: I will review the "Battery Vitals Update" changes next - sorry for hijacking this PR with my changes! 😄

mcbirse commented 5 months ago

I've gone with 32

I may change this to 30 and set a built in max of 100. Found several other network scan tools that seem to use these numbers... not sure why but I assume they know better than I do.

mcbirse commented 5 months ago

Testing latest changes to Battery Data / Vitals successfully in both Local and Cloud mode.

Checked output of both /freq and /pod and looks good.

I am seeing the below error in pypowerall logs however (even in non-debug mode) - when in local mode - EDIT and cloud mode (have run out of time to test).

01/21/2024 06:32:35 PM [proxy] [ERROR] Missing key in payload [nominal_full_pack_energy]
01/21/2024 06:32:35 PM [proxy] [ERROR] Missing key in payload [nominal_energy_remaining]

Due to aggregated data now includes below and passes a null value to the get_value function.

  "nominal_full_pack_energy": null,
  "nominal_energy_remaining": null,

I wonder if we should change/suppress this? Check for null before calling the function?

mcbirse commented 5 months ago

I think this is bug - I should not be getting null values for this in either mode. Not sure why yet - will check later tonight if time.

  "nominal_full_pack_energy": null,
  "nominal_energy_remaining": null,
jasonacox commented 5 months ago

01/21/2024 06:32:35 PM [proxy] [ERROR] Missing key in payload [nominal_full_pack_energy] 01/21/2024 06:32:35 PM [proxy] [ERROR] Missing key in payload [nominal_energy_remaining]

Yes, that was the bug I caught where vitals() and system_status() were using the same variable. Try with this commit: https://github.com/jasonacox/pypowerwall/pull/67/commits/2da87131eaffec6b089aea2b9f1d7a8301614a50

mcbirse commented 5 months ago

I will re-test... I was sure I pulled that commit!! But very likely I have stuffed up with my test setup here - sorry!

mcbirse commented 5 months ago

All good - my mistake, I had not pulled the latest changes! 😕 

{
  "backup_reserve_percent": 30,
  "nominal_full_pack_energy": 13051,
  "nominal_energy_remaining": 7334.157894736843,
  "time_remaining_hours": 1.5568100604639636
}
jasonacox commented 5 months ago

No worries! I'm glad you saw the same thing I did! If you have time, feel free to merge. I'll push to PyPI later today unless we find anything else.

jasonacox commented 5 months ago

Pushed:

Thanks @mcbirse !