squishykid / solax

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

Speedy discovery #126

Closed rnauber closed 6 months ago

rnauber commented 1 year ago

This PR builds on #119 and fixes linter errors, tests, and coverage (cf. https://github.com/squishykid/solax/pull/119#issuecomment-1646691601 )

Darsstar commented 1 year ago

The discover method still isn't asyncified enough. It still tries the different URL formats in sequence, instead of in parallel.

https://github.com/squishykid/solax/pull/115 was my attempt targeting a different branch.

I was (very irregulary) working on a version for the speedy-discovery branch, but the tests weren't co-operating.

Anyway, this is how it could look instead. (final code is not tested, the general approach is)

async def discover(host, port, pwd="") -> Inverter:
    failures: List = []
    clients = all_variations(host, port, pwd)

    pending = set()

    for delay, (client_name, client) in enumerate(clients.items()):
        # explicit delay, don't spam the inverter, the X1 Mini doesn't handle that well
        pending.add(asyncio.create_task(delayed(delay, client.request()), name=client_name))

    while pending:
        # do not switch to task groups at any point, no need to wait for a HTTP timeout
        # when a working URL has been found
        done, pending = await asyncio.wait(pending, return_when=asyncio.FIRST_COMPLETED)

        for task in done:
            if task.cancelled():
                continue

            ex = task.exception()

            if ex is not None:
                failures.append(
                    (
                        task.get_name(),
                        ex,
                    )
                )
                continue

            response = task.result()

            for inverter_class in REGISTRY:
                try:
                    inverter = inverter_class(clients[task.get_name()])
                    if inverter.identify(response):
                        for loser in pending:
                            loser.cancel()

                        return inverter
                    else:
                        failures.append(
                            (
                                task.get_name(),
                                inverter_class.__name__,
                                "did not identify",
                            )
                        )
                except InverterError as ex:
                    failures.append(
                        (
                            task.get_name(),
                            inverter_class.__name__,
                            ex,
                        )
                    )
    msg = (
        "Unable to connect to the inverter at "
        f"host={host} port={port}, or your inverter is not supported yet.\n"
        "Please see https://github.com/squishykid/solax/wiki/DiscoveryError\n"
        f"Failures={str(failures)}"
    )
    raise DiscoveryError(msg)

with delayed utility function being

async def delayed(delay: float, awaitable: Awaitable):
    if delay:
        await asyncio.sleep(delay)
    return await awaitable
Darsstar commented 6 months ago

This PR can probably be closed since #145 has been merged.