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

Speedy discovery #126

Closed
wants to merge 19 commits into from
Closed

Conversation

rnauber
Copy link
Contributor

@rnauber rnauber commented Jul 22, 2023

This PR builds on #119 and fixes linter errors, tests, and coverage (cf. #119 (comment) )

@Darsstar
Copy link
Contributor

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

#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
Copy link
Contributor

Darsstar commented Apr 2, 2024

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

@squishykid squishykid closed this Apr 2, 2024
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.

3 participants