-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Tesla Solar Energy Monitoring #212
Conversation
Thanks. Approach looks fine. Please remember to add tests. We've been trying to get better test coverage for the library since it's getting more complex. |
Actually, one thing to note. One of the issues we have with this library is that we have so much embedded HA stuff down in the API. That makes it so if you want to make a change in HA, you have to make changes in 2 places: the api library and HA. You'll note we have a long open issue #24 to try to break the HA out. If we're starting from scratch here, I'd suggest you see if you can isolate the raw api calls so they're in this library and then build the entity logic in HA itself. So basically I wouldn't suggest keeping the homeassistant/* files. The downside of course is someone can't access entities in teslajsonpy, but I'm not sure who is actually using the library beyond HA. You may also want to see if we can make ourselves API compatible with the existing powerwall integration. That way you could just contribute to that integration directly too. I believe the main difference between that integration and what is happening here is the powerwall integration relies on local access while this would rely on cloud access. See #49 |
@alandtse yea I looked into local access unfortunately it doesn't have any. I have SolarCity panels with a Tesla gateway after the fact which seems to only use cloud. I'll look at all those prs you mentioned. I also wasn't a big fan of having to do code here and then pr to the HA integration 😞 |
Yah, so if you're able to just make the necessary API changes here, you can take your HA specific code into HA itself. The only thing we'd need to keep in this component would be the base solar site object so the controller knows it should be fetching and caching information for that device. |
If you're still working on this, I'd suggest seeing #216 to see if it simplifies your PR. I think that's a more robust way to define/access the underlying API. I'm planning to switch the vehicle related calls to that method and may also let us get the solar stuff for free. Please also note, that with Tesla being removed from core, any improvements should go to the custom_component I'm hosting now. |
Hi, did you end up merging this into the main branch. I have also started writing support for "energy_sites" (i.e. solar panels, not powerwalls) based off this plugin but if you are already working on it, wouldn't continue. |
@giachello i have not had time to continue this. go for it |
This is an initial attempt at adding Tesla Solar energy to the library.
Before I go further, I'd like to get a once over of the PR and the approach.
I have tesla solar working with HA via a Rest Sensor but would like to see it get more integrated with the offical plugin.