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

add cloud api support #12

Closed
wants to merge 15 commits into from
Closed

add cloud api support #12

wants to merge 15 commits into from

Conversation

drc38
Copy link

@drc38 drc38 commented Jan 27, 2023

To incorporate the cloud api, I have renamed __init__.py to local.api.py and imported it in the new __init__.py file.

Looks like the issue with the test files on Windows has meant my repo does not have them, so if you could add them back in that would be great.

Happy to take on board any other feedback.

@farmio
Copy link
Collaborator

farmio commented Jan 27, 2023

Hi 👋!
Since local and cloud api handler here are completely unrelated (eg. aiohttp vs. httpx) and also have different specific needs (local needs special error handling currently done in HA integration code) - wouldn't it make sense to have the Solar.web api library as an extra library on pypi instead of merging it with the local one? I think this would make it easier to understand, find and fix issues and maintain.

@nielstron
Copy link
Owner

@drc38 Great that this worked out.

Actually I did not think much about the consequences of adding cloud API. @drc38 are you using any of the definitions used for the local API? If not, I agree with @farmio that this should go to a separate package. Feel free to just copy the project skeleton, then there should be little to no extra work in separating the packages.

@drc38
Copy link
Author

drc38 commented Jan 27, 2023

@drc38 Great that this worked out.

Actually I did not think much about the consequences of adding cloud API. @drc38 are you using any of the definitions used for the local API? If not, I agree with @farmio that this should go to a separate package. Feel free to just copy the project skeleton, then there should be little to no extra work in separating the packages.

Originally I had thought the same functions could be repeated eg current_power_flow in the cloud api and data formatted to match, but perhaps that will be more difficult than simply creating a new pypi library and separate HACS repo for HA. I already have a separate repo setup so no issue taking that approach.

@farmio
Copy link
Collaborator

farmio commented Jan 27, 2023

You could always require and import both libraries from the HA integration if you want to ship it in HA Core instead of HACS.
In config flow a user could simply choose local / cloud first and then branch out to the individual config steps needed.

There are several integrations already doing this. So you should find working examples in HAs codebase.

@drc38
Copy link
Author

drc38 commented Jan 28, 2023

Closing as first draft of cloud api now added to Pypi https://pypi.org/project/fronius-solarweb/

@drc38 drc38 closed this Jan 28, 2023
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

Successfully merging this pull request may close these issues.

None yet

3 participants