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

Combine with built in integration #278

Open
JCalvi opened this issue Jul 9, 2024 · 7 comments
Open

Combine with built in integration #278

JCalvi opened this issue Jul 9, 2024 · 7 comments

Comments

@JCalvi
Copy link

JCalvi commented Jul 9, 2024

I currently use the built in Fronius integration and it works well.
Occasionally though there may be an issue with something in the chain and my HA data is lost.
Examples are, Fronius data logger loses connection, Home assistant is off for a while etc.

Because the inverter stores data, when connection is restored Solar.web catches up this missing data.

A great feature IMHO, would be to combine the two. The Fronius integration for fast updating and local operation, and your integration for filling in missing data from solar.web.

Would it be possible to have you plug-in check for existing data and only add solar.web if missing?

Perhaps it already does, as I have not yet tried both integrations together and your documentation implies to run one or the other only?

@drc38
Copy link
Owner

drc38 commented Jul 10, 2024

Hi, prior to creating this repo I approached the existing repo owners suggesting the two be combined. They preferred keeping it separate. Happy for you to raise it again with them.

@JCalvi
Copy link
Author

JCalvi commented Jul 10, 2024

I had the same thought, yes it would be perfect if they were combined. I suspected they may not want to change theirs, so thought I'd ask if you would consider changing yours to only update any missing data?
This way the existing one is installed to get live data, and your integration if installed as well only adds missing data.

@drc38
Copy link
Owner

drc38 commented Jul 10, 2024

I don't know how that would work. Happy to review a PR if you've got a workable solution.

@farmio
Copy link

farmio commented Jul 10, 2024

Hi 👋!

They preferred keeping it separate.

If I remember correctly, my suggestion was to keep the communication libs (pyfronius and your python-fronius-web) separate as they used a very different tech stack (httpx vs aiohttp, pydantic vs custom parser). I still think this is a good idea so each can individually be updated, tested and be used in other projects.

I do not have any objections to import and use both libraries in the HA core Fronius integration.

That said, the local SolarAPI pyfronius uses does provide historical data as well, however using this endpoint is not implemented currently.

@drc38
Copy link
Owner

drc38 commented Jul 10, 2024

I'd be happy if it was incorporated into the core HA fronius integration using @farmio's approach, this repo could then be sunset.

@farmio
Copy link

farmio commented Jul 11, 2024

Great! @JCalvi are you planning on working on this? I'd be happy to do reviews or answer questions along the way (if I can 🙃).

@JCalvi
Copy link
Author

JCalvi commented Jul 11, 2024

Hi @farmio, no I was just hoping to see some merging of these two projects.
Happy to help test, but coding it was not in my plans.

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

No branches or pull requests

3 participants