jasonacox / pypowerwall

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

Allow customization of the cachefile location and name #74

Closed emptywee closed 6 months ago

emptywee commented 6 months ago

Hi,

I am using your library in my home stackstorm-based automation actions/workflows and needed to move the cache file outside the "current directory" of the action runners for safety and security. Had to extend your class and override the init, but then I thought to try to get this little change to upstream, so here it is. Hope you will accept this PR. Thanks!

emptywee commented 6 months ago

@jasonacox btw, ever thought of refactoring the library a bit and make some abstractions for local and cloud mode operations? It would probably make the code nicer and cleaner without checks for cloud mode here and there...

jasonacox commented 6 months ago

Thanks @emptywee / @igcherkaev ! Excellent addition. 🙏

ever thought of refactoring the library a bit and make some abstractions for local and cloud mode operations

I would love that! Would you be willing to take a stab at it and submit a PR?

emptywee commented 6 months ago

Thank you! We may want to update README.md too, there's usage examples and now it's a little off :)

emptywee commented 6 months ago

I would love that! Would you be willing to take a stab at it and submit a PR?

Hmm, I could try :)

jasonacox commented 6 months ago

update README.md too

good call.. working on it

jasonacox commented 6 months ago

Hmm, I could try :)

No pressure or rush, but a good suggestion. We can just create an issue to track the enhancement. I won't have time to work on it for a while. I'm working with some others on adding features to help with the Tesla removal of vitals data.

emptywee commented 6 months ago

No pressure or rush, but a good suggestion. We can just create an issue to track the enhancement. I won't have time to work on it for a while. I'm working with some others on adding features to help with the Tesla removal of vitals data.

I understand. I'll see what I can do.

emptywee commented 6 months ago

@jasonacox do you have any code styling requirements? Didn't see any guidelines or anything about contributing to the project.

jasonacox commented 6 months ago

This PR was included in v0.7.12 which has been pushed to PyPI.

The Proxy container: jasonacox/pypowerwall:0.7.12t43 - If you use Powerwall-Dashboard, you can edit the powerwall.yml to use that and then run ./compose-dash.sh up -d.

Didn't see any guidelines or anything about contributing to the project.

Yes, that's about right. We are a bit informal here. 😂

@mcbirse and/or I will review the PR and make suggestions. High level, we target as simple and readable as possible over clever and cryptic. As you have probably seen, with the dependency on unofficial Tesla APIs, it can be a bit of a moving target and we optimize for flexibility and fast releases to ensure dependent projects (mostly the Powerwall-Dashboard) can stay current. Most important, it needs to be fun! 😉

emptywee commented 6 months ago

This PR was included in v0.7.12 which has been pushed to PyPI.

Thanks! I'll update my requirements.txt and remove my extra code to redefine the cache file location :)

Yes, that's about right. We are a bit informal here. 😂

@mcbirse and/or I will review the PR and make suggestions. High level, we target as simple and readable as possible over clever and cryptic. As you have probably seen, with the dependency on unofficial Tesla APIs, it can be a bit of a moving target and we optimize for flexibility and fast releases to ensure dependent projects (mostly the Powerwall-Dashboard) can stay current. Most important, it needs to be fun! 😉

Totally get it :) I think the way I've done it for now is pretty simple, But in my opinion it'll open the way to further break it down to simple functions for each piece of data we need and maybe even we'll allow making modifications to powerwall settings via cloud. It'll be an all-in-one library for simple use :)