toreamun / amshan-homeassistant

Home Assistant integrasjon for strømmålere (AMS/HAN/P1). Integrasjonen støter både streaming (serieport/TCP-IP) og MQTT (Tibber Pulse, energyintelligence.se etc)
MIT License
146 stars 9 forks source link

AMSHAN Stopped Working after update to Home Assistant 2023.6.0 #62

Closed roberthaugen73 closed 1 year ago

roberthaugen73 commented 1 year ago

AMSHAN Stopped Working after update to Home Assistant 2023.6.0

2023-06-08 14:17:46.806 ERROR (MainThread) [homeassistant] Error doing job: Task exception was never retrieved Traceback (most recent call last): File "/usr/local/lib/python3.11/site-packages/han/meter_connection.py", line 353, in connect_loop await wait( File "/usr/local/lib/python3.11/asyncio/tasks.py", line 415, in wait raise TypeError("Passing coroutines is forbidden, use tasks explicitly.") TypeError: Passing coroutines is forbidden, use tasks explicitly. 2023-06-08 14:17:46.817 WARNING (MainThread) [py.warnings] /usr/local/lib/python3.11/asyncio/base_events.py:1907: RuntimeWarning: coroutine 'Event.wait' was never awaited handle = self._ready.popleft() 2023-06-08 14:17:46.829 WARNING (MainThread) [py.warnings] /usr/local/lib/python3.11/asyncio/base_events.py:1907: RuntimeWarning: coroutine 'ConnectionManager._try_connect' was never awaited handle = self._ready.popleft()

kjeisa commented 1 year ago

Same here

ruant commented 1 year ago

python was updated to 3.11 in this HA release.

https://docs.python.org/3/library/asyncio-task.html#asyncio.wait Changed in version 3.11: Passing coroutine objects to wait() directly is forbidden.

ruant commented 1 year ago

https://github.com/toreamun/amshan/blob/master/han/meter_connection.py#L353-L356 and https://github.com/toreamun/amshan/blob/master/han/meter_connection.py#L360-L363

I guess the fix would be to create Tasks of these with asyncio.create_task outside wait, and pass the tasks in a list to wait. I'll test this out tomorrow.

ruant commented 1 year ago

I'm no python developer, so I didn't get it to work when testing. But that has to be the way to do it, just have to figure out the correct way to do the code 😂

m0bygit commented 1 year ago

Ok, so here is what I did. I'm not a python programmer and I do not really know what I'm doing. But, it seems like it fixed the problem.

in meter_connection.py in the beginning change:

from asyncio import (
    FIRST_COMPLETED,
    BaseTransport,
    CancelledError,
    Event,
    Future,
    Protocol,
    Queue,
    iscoroutinefunction,
    sleep,
    wait,
) 

to

from asyncio import (
    create_task,
    ensure_future,
    FIRST_COMPLETED,
    BaseTransport,
    CancelledError,
    Event,
    Future,
    Protocol,
    Queue,
    iscoroutinefunction,
    sleep,
    wait,
)

change the connect_loop method to:

    async def connect_loop(self) -> None:
        """
        Connect to meter using connection factory, and keep reconnecting if connection is lost.

        The connection is not reconnected on connection loss if close() was called on this instance.
        """
        while not self._is_closing.is_set():
            connect_task = create_task(self._try_connect())
            closing_task = create_task(self._is_closing.wait())
            await wait(
                (connect_task, closing_task),
                return_when=FIRST_COMPLETED,
            )

            if self._connection:
                _, protocol = self._connection
                done_task = ensure_future(protocol.done)
                closing_task2 = create_task(self._is_closing.wait())
                await wait(
                    (done_task, closing_task2),
                    return_when=FIRST_COMPLETED,
                )

                if not self._is_closing.is_set():
                    _LOGGER.warning("Connection lost")
                    self._update_connection_lost_circuit_breaker()

                self._connection = None

        # done closing if that was the case of connection loss
        self._is_closing.clear()

        _LOGGER.info("Connect loop done")

Use at your own risk and do keep a copy of the original file. Hope it helps someone. 😃

mariwing commented 1 year ago

It seems that @toreamun might have abandoned this project, no activity for a long time and no response to this critical issue. There is also (at least) another custom component for reading AMS data through MBus available so this might be a good time to switch over if you are using this component for that purpose. I am just writing this so people coming to this issue realizes that there are alternatives, not to be disrespectful to anyone :-)

roberthaugen73 commented 1 year ago

Ok, so here is what I did. I'm not a python programmer and I do not really know what I'm doing. But, it seems like it fixed the problem.

in meter_connection.py in the beginning change:

from asyncio import (
    FIRST_COMPLETED,
    BaseTransport,
    CancelledError,
    Event,
    Future,
    Protocol,
    Queue,
    iscoroutinefunction,
    sleep,
    wait,
) 

to

from asyncio import (
    create_task,
    ensure_future,
    FIRST_COMPLETED,
    BaseTransport,
    CancelledError,
    Event,
    Future,
    Protocol,
    Queue,
    iscoroutinefunction,
    sleep,
    wait,
)

change the connect_loop method to:

    async def connect_loop(self) -> None:
        """
        Connect to meter using connection factory, and keep reconnecting if connection is lost.

        The connection is not reconnected on connection loss if close() was called on this instance.
        """
        while not self._is_closing.is_set():
            connect_task = create_task(self._try_connect())
            closing_task = create_task(self._is_closing.wait())
            await wait(
                (connect_task, closing_task),
                return_when=FIRST_COMPLETED,
            )

            if self._connection:
                _, protocol = self._connection
                done_task = ensure_future(protocol.done)
                closing_task2 = create_task(self._is_closing.wait())
                await wait(
                    (done_task, closing_task2),
                    return_when=FIRST_COMPLETED,
                )

                if not self._is_closing.is_set():
                    _LOGGER.warning("Connection lost")
                    self._update_connection_lost_circuit_breaker()

                self._connection = None

        # done closing if that was the case of connection loss
        self._is_closing.clear()

        _LOGGER.info("Connect loop done")

Use at your own risk and do keep a copy of the original file. Hope it helps someone. 😃

I can confirm that your solution is working. Thank you!:-)

docker cp homeassistant:/usr/local/lib/python3.11/site-packages/han/meter_connection.py /tmp/meter_connection.py
Successfully copied 16.4kB to /tmp/meter_connection.py

Edited the  /tmp/meter_connection.py

docker cp /tmp/meter_connection.py homeassistant:/usr/local/lib/python3.11/site-packages/han/meter_connection.py
Successfully copied 16.9kB to homeassistant:/usr/local/lib/python3.11/site-packages/han/meter_connection.py
roberthaugen73 commented 1 year ago

Do we need to change the manifest.json to reflect the new version of the AMSHAN ?


{
  "domain": "amshan",
  "name": "AMS HAN meter",
  "version": "2022.10.2",
  "config_flow": true,
  "documentation": "https://github.com/toreamun/amshan-homeassistant",
  "issue_tracker": "https://github.com/toreamun/amshan-homeassistant/issues",
  "codeowners": [
    "@toreamun"
  ],
  "requirements": [
    "amshan[serial]==2.1.1"
  ],
  "after_dependencies": [
    "mqtt"
  ],
  "iot_class": "local_push"
}
toreamun commented 1 year ago

Sorry for long delay. Thanks for pointing out the error, and thanks to @ruant for fixing it. Pre release ready. Still some issues, but integrations should be running.

ruant commented 1 year ago

Credits for the fix goes to @m0bygit I just made the PR.