search

squishykid / solax

🌞 Solax Inverter API Wrapper
MIT License
100 stars 57 forks source link

feat: Finish discovery right after finding one #110

Closed kdehairy closed 1 year ago

kdehairy commented 1 year ago

There is no need to wait for all inverters to finish, as long as one of them finished with success.

This patch significantly decrease the waiting time until successful discovery.

kdehairy commented 1 year ago

@squishykid can you have a look when you have time? Thnx

kdehairy commented 1 year ago

@squishykid still no time? :)

kdehairy commented 1 year ago

@squishykid it's taking long now. Is there something I can help with? Is the PR not acceptable as a concept?

Can we have a closure please? :)

juliangilbey commented 1 year ago

This turns out to be a very significant patch; I have a newer inverter (will submit a PR once I've been able to do a couple more tests/checks), but the simple code in the README fails on my system, because an earlier inverter in the discovery list hangs on the discover http request. This patch means that the discovery happens without that being a problem.

kdehairy commented 1 year ago

I'm not sure if this repo is maintained anymore. I can see clearly that no PR is reviewed/accepted since the beginning of this year (Feb 2023), including this PR :(

juliangilbey commented 1 year ago

I'm not sure if this repo is maintained anymore. I can see clearly that no PR is reviewed/accepted since the beginning of this year (Feb 2023), including this PR :(

You may well be right; perhaps someone else with the interest could fork it and make that the new "official" repo. Though they wouldn't have the rights to update the version on PyPI. @squishykid - are you intending to continue maintaining this really useful project?

squishykid commented 1 year ago

Apologies for the delay. I've been struggling to find time for this project recently- as is clearly evident. But I can merge this as-is and then make a new PyPi release

kdehairy commented 1 year ago

Welcome back @squishykid and Thnx :)

squishykid commented 1 year ago

perhaps you have a chance to look at https://github.com/squishykid/solax/tree/describe-inverters-with-json

The PR is doing two things, but one of them was to split the discovery into two phases:

Phase 1: run all the HTTP clients and get data from the inverter Phase 2: run this data through all of the relevant inverters to see which ones parsed it without throwing an error.

Perhaps you can have a go at re-formatting the PR, with just the discovery changes? That would require moving all of the HTTP clients out of the inverters, and making the discovery the two-phase process described above.

I haven't had a chance to finish the PR :(

kdehairy commented 1 year ago

Happy to help. I'll take it.