search

claytonjn / hass-circadian_lighting

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

Fix the ability to specify sunrise/sunset time manually #206

Closed D-side closed 2 years ago

D-side commented 2 years ago

Background: https://github.com/claytonjn/hass-circadian_lighting/issues/204#issuecomment-1242364207

  • Since 2.1.0-beta the integration made a lot of methods asynchronous (if I'm interpreting this right) and replaced a plain call to date.replace with async_add_executor_job that should've called the same method asynchronously with the same arguments
  • The async_add_executor_job, as it exists right now, does not accept any keyword arguments, and this blows up on the above call that has them

So — since 2.1.0-beta manually specifying sunset/sunrise times should've been completely broken. I think that adds up with what I've experienced 🤔

Replacing components of a date doesn't sound like something that should require any asynchrony, as it's an operation entirely for a tiny amount of CPU and RAM, with no I/O. So the fix, I suppose, is to turn that particular bit back to synchronous?

Fixes #204 — tested by performing the same change in my local installation of 2.1.2 and restarting Home Assistant (2022.9.1) with the following configuration:

circadian_lighting:
  min_colortemp: 1700
  max_colortemp: 6500
  transition: 10
  sunrise_time: "04:00:00"
  sunset_time: "21:00:00"

⚠ While I have experience in other languages, I'm a relative newcomer to Python and may be overlooking something glaringly obvious.

D-side commented 2 years ago

@claytonjn is there anything else that needs to be done to get this merged, please? 🙂