Skip to content
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

Added support for Energy Sites #219

Merged
merged 8 commits into from
Oct 9, 2021
Merged

Added support for Energy Sites #219

merged 8 commits into from
Oct 9, 2021

Conversation

giachello
Copy link
Contributor

I added support for Energy Sites (Solar Panels). This has been working for my installation for a week now and feeds into my Energy panel on Home Assistant. I know you wanted to restructure the entire API in a separate thread on this topic, but I'm not going to do that work (don't have the time).

This goes together with a pull request on the Tesla custom component.

Copy link
Collaborator

@alandtse alandtse left a comment

Choose a reason for hiding this comment

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

Thanks. Please address the version/changelog reversion and lint errors and we'll get this merged in this weekend.

CHANGELOG.md Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
teslajsonpy/__version__.py Outdated Show resolved Hide resolved
@giachello
Copy link
Contributor Author

Ok, I think I took care of all the comments you left. Let me know!

@alandtse alandtse merged commit e9798b7 into zabuldon:dev Oct 9, 2021
alandtse added a commit that referenced this pull request Oct 9, 2021
feat: added support for energy sites  (#219)
@shred86
Copy link
Contributor

shred86 commented Aug 18, 2022

Hey @giachello, I'm helping with the rewrite and had a question on some changes in this pull request (thanks for adding this by the way!). I was wondering what the thought was behind mapping the site_id to energysite_id?

        self.__id_energysiteid_map = {}
        self.__energysiteid_id_map = {}

It looks like all the power data is stored in self.__power which is organized by energysite_id, so even someone that has two energy sites, I can still just pull the data I need using energysite_id. I'm sure there's a good reason, I'm just a novice programmer so I'm trying to understand before I make any major changes. Thanks!

@giachello
Copy link
Contributor Author

Actually, I think I've just done that for completeness, to conform with the way that the Car entities were written at the time. Not sure if there is a real need to do this.

@alandtse
Copy link
Collaborator

CarIDs are subject to change so it's possible they may change within a session so we had to translate it. If the energy sites cannot change, then you wouldn't need it.

@shred86
Copy link
Contributor

shred86 commented Aug 19, 2022

Ahh okay, that makes sense. I'm not 100% certain but I have not noticed the energy_site_id change. I haven't really looked closely at just the "id", but I'll keep an eye on it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants