claytonjn / hass-circadian_lighting

Circadian Lighting custom component for Home Assistant
Apache License 2.0
756 stars 89 forks source link

Error on homeassistant/home-assistant:2023.6.0.dev20230430 #227

Closed enkama closed 10 months ago

enkama commented 1 year ago

Hey.

I have my Home-Assistant Instance updated to 2023.6.0.dev20230430 and I am suddenly getting this error in the logs.

Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/config/custom_components/circadian_lighting/switch.py", line 383, in _light_state_changed
    await self._force_update_switch(lights=[entity_id])
  File "/config/custom_components/circadian_lighting/switch.py", line 325, in _force_update_switch
    return await self._update_switch(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/circadian_lighting/switch.py", line 322, in _update_switch
    await self._adjust_lights(lights or self._lights, transition)
  File "/config/custom_components/circadian_lighting/switch.py", line 377, in _adjust_lights
    await asyncio.wait(tasks)
  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.
/usr/local/lib/python3.11/asyncio/base_events.py:1907: RuntimeWarning: coroutine 'ServiceRegistry.async_call' was never awaited
  handle = self._ready.popleft()
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
2023-04-30 22:01:50.246 ERROR (MainThread) [homeassistant] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/config/custom_components/circadian_lighting/switch.py", line 383, in _light_state_changed
    await self._force_update_switch(lights=[entity_id])
  File "/config/custom_components/circadian_lighting/switch.py", line 325, in _force_update_switch
    return await self._update_switch(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/circadian_lighting/switch.py", line 322, in _update_switch
    await self._adjust_lights(lights or self._lights, transition)
  File "/config/custom_components/circadian_lighting/switch.py", line 377, in _adjust_lights
    await asyncio.wait(tasks)
  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.
/usr/src/homeassistant/homeassistant/util/logging.py:156: RuntimeWarning: coroutine 'ServiceRegistry.async_call' was never awaited
  log_exception(format_err, *args)
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
2023-04-30 22:02:11.430 ERROR (MainThread) [homeassistant.util.logging] Exception in _update_switch when dispatching 'circadian_lighting_update': ()
Traceback (most recent call last):
  File "/config/custom_components/circadian_lighting/switch.py", line 322, in _update_switch
    await self._adjust_lights(lights or self._lights, transition)
  File "/config/custom_components/circadian_lighting/switch.py", line 377, in _adjust_lights
    await asyncio.wait(tasks)
  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.
enkama commented 1 year ago

Changing 371 to the following seems to do it for me.

            tasks.append(
                self.hass.async_create_task(
                    self.hass.services.async_call(
                        LIGHT_DOMAIN, SERVICE_TURN_ON, service_data
                    )
                )
            )
            await asyncio.wait(tasks)
acidkill commented 1 year ago

The same issue is in official 2023.6 release. And the fix is working there also.

8ctorres commented 1 year ago

Can confirm this fixes it. Thanks @Wibias

LukasdeBoer commented 1 year ago

Maybe @Wibias could make a PR with this change to save Clayton some time and we can all benefit from his fix? :D

goobags commented 1 year ago

That would be fantastic @Wibias as I also have this issue and don’t necessarily understand what I need to change.

goobags commented 1 year ago

Well I gave it a go, first PR so go easy on me.

8ctorres commented 1 year ago

Hi! I left a comment on your PR

enkama commented 1 year ago

Yup also left a comment.

I would have already made a PR but I dont really see that this custom component is really maintained by the amount of open PR`s so i rather wrote it down in here. Also like I wrote in the PR please keep "if tasks" in and dont remove it like I have done so we do not have any ValueErrors if we have an empty list.

So like this:

    tasks.append(
        self.hass.async_create_task(
            self.hass.services.async_call(
                LIGHT_DOMAIN, SERVICE_TURN_ON, service_data
            )
        )
    )
if tasks:
    await asyncio.wait(tasks)

Maybe my thought process is wrong so feel free to give any comments. I am more of an user than a developer.

enkama commented 1 year ago

I thought about forking this awesome custom component just to maintain it if there is no intention or no time to maintain it anymore. Or maybe just add some maintainers who can merge PR´s and change the code and stuff.

But maybe @claytonjn can say something about this.

DeBendeBurcht commented 1 year ago

For anyone who is not familiar with how to change the code to fix this issue, you use visual studio code server, under the folder custom components, you will find the folder circadian lighting, and in there you open switch.py, in there you replace the code from line 371 untill 377 with:

tasks.append(
    self.hass.async_create_task(
        self.hass.services.async_call(
            LIGHT_DOMAIN, SERVICE_TURN_ON, service_data
        )
    )
)

if tasks: await asyncio.wait(tasks)

Good luck! And hope this gets fixed soon!

claytonjn commented 1 year ago

Hey all, sorry for missing the issue originally! I was sick, then traveling for work, then my home server running Home Assistant (amongst other things) started having hardware issues so I've been trying to figure out an affordable way to make it highly available and more power friendly.

I'm not as active with this project as I'd like to be able to but it's definitely still being maintained! Thanks to everyone for the contributions - I'll try to get the fix released today or tomorrow and I'll skip the normal beta release cycle.

vanstinator commented 1 year ago

I'm not as active with this project as I'd like to be able to but it's definitely still being maintained! Thanks to everyone for the contributions - I'll try to get the fix released today or tomorrow and I'll skip the normal beta release cycle.

Would you be interested in having the work upstreamed into HA core? I maintain a few integrations, both custom and upstreamed, and I'd be happy to take on the work to migrate it to core along with a config flow, etc.

claytonjn commented 1 year ago

Would you be interested in having the work upstreamed into HA core? I maintain a few integrations, both custom and upstreamed, and I'd be happy to take on the work to migrate it to core along with a config flow, etc.

To be honest, I don't know.

I pursued adding it as a core integration about 5 years ago and there wasn't any interest from Paulus; I believe the feedback was that it was not differentiated enough from the Flux integration which would cause confusion to users about various use-cases and also at the time he did not like that Circadian Lighting (and Flux) were switch entities, because they really enable/disable automation-type behavior and not something external to Home Assistant.

Then about three years ago, before creating Adaptive Lighting, basnijholt made a bunch of contributions to prepare the integration for addition to core, and again that didn't go anywhere I believe based on pushback from the core team - I'm not sure what the reasoning was at that time.

While frustrating, I do agree with a lot of the points Paulus and others have made - the integration can be daunting to set up, and it isn't immediately clear to the user how it behaves (for example, many users don't even realize that switch entities are generated to control the behavior, or that multiple configurations can be added). Also, being switch entities does complicate auto-generated dashboards (for example, where a user wants a dashboard of all light switches). Some of this would be mitigated by finally migrating to config flow rather than yaml, but part of the reason I haven't done so is because there are still the fundamental design issue of relying on switch entity types, so I've been considering better alternatives. At one point I was even working towards creating a new circadian_lighting. entity type, but I believe the core team didn't like that idea either.

As of now, I actually think that the best option wrt including this functionality in HA core would be twofold:

  1. Augment the current sun.sun integration with some additional attributes required for natural Circadian Lighting calculations (e.g. solar noon, solar midnight)
  2. Add a built-in blueprint that handles the actual Circadian Lighting functionality - this means rather than turning on/off Circadian Lighting from switch entities users would turn on/off automations, and they would be populated alongside automations rather than switches, which are both significantly more intuitive. Also the initial configuration would be more intuitive as Circadian Lighting would show up as an option when users go to add automations, and it would be far more obvious that multiple instances can be created for different sets of lights.

Any thoughts on the above? As someone who already maintains some core integrations, do you have any idea what would be involved in me submitting and maintaining a core blueprint?