Wake up automation w/ sunrise effect #113

duvholt · 2025-04-20T18:22:01Z

Finally got around to rebasing and continuing development on #81

Most of the notes from the original PR is still valid with the exception that I implemented support for using the sunrise effect with Hue bulbs 🌅 No support for sunrise with groups since effects is currently only implemented for light updates and not grouped lights.

As I said in the original PR I think the names for the new concepts could be better. For example I don't really like "scheduler.rs", but I think it's easier to have a naming discussion with a complete implementation.

It previously started fade in when the fade in when the light should've been at full brightness

When the wake up schedule first begins it shouldn't be cancelled by doing changes

The previous method was buggy and brittle

…uped

This should prevent to light from using the previous configuration when starting up

chrivers

Hi @duvholt

Thanks for this amazing work! It's really cool to see this new feature taking shape.

I've finished my first pass, mostly dealing with minor issues.

It's really excellent to see how you have integrated with the existing infrastructure and styles.

Of course, given the size of the PR, there are some things we need to work on, but this is a great first step.

Really looking forward to seeing the next iteration 👍

src/resource.rs

src/routes/clip/behavior_instance.rs

src/server/scheduler.rs

crates/hue/src/effect_duration.rs

chrivers · 2025-04-21T10:58:23Z

crates/hue/src/effect_duration.rs

+        } else if seconds < RESOLUTION_05M_LIMIT {
+            (RESOLUTION_05M_BASE, RESOLUTION_05M)
+        } else {
+            return Self(0);


Uh, this seems a little too can-do? 😄

Shouldn't this be Option<Self>, maybe?

Also, could I ask you to implement to_seconds() as well? It seems weird to only be able to go in one direction, and for the upcoming web ui, we need to be able to display things. Then it would come in handy :)

This is a case that shouldn't be possible, but I admit that's a very lazy solution 😓Would Option + stop the request with error logging suffice?

I don't see the use for displaying this value in a web ui since it's only used for the Hue binary format, but it shouldn't be too hard to implement it.

I have a proof-of-concept "inspector" feature for the web ui going, where the user can "look into the engine room", so to speak.

But okay, it's not very critical.

However, I am strongly against this conversion "magically" succeeding, when it's not supposed to.

Everything else in the hue crate uses HueResult (i.e. Result<T, HueError>), so let's use that.

Feel free to introduce a new error variant, if there isn't a suitable one already.

Ah, sounds neat.
And yes I'll fix the error case :)

chrivers · 2025-04-21T10:59:21Z

src/backend/z2m/mod.rs

                                    LightEffect::Cosmos => EffectType::Cosmos,
                                    LightEffect::Sunbeam => EffectType::Sunbeam,
                                    LightEffect::Enchant => EffectType::Enchant,
+                                    LightEffect::Sunrise => EffectType::Sunrise,


Okay, so "Sunrise" is encoded as a real Hue effect?

This is fantastic work, @duvholt 👍

chrivers · 2025-04-21T11:01:37Z

src/server/scheduler.rs

+}
+
+impl WakeupJob {
+    fn start_datetime(&self, now: DateTime<Local>) -> Result<DateTime<Local>, &'static str> {


Hm, are we sure DateTime<Local> is the right way to do this?

We exclusively use DateTime<Utc> everywhere else?

It's because the time provided in the wake up configuration doesn't include any timezone information. So if we just used UTC the time would likely be wrong. Although when I think about it I forgot to look into using the configured time zone (bridge.timezone). I also noticed that the time zone is configurable in the Hue app which I think is something we should implement for this to work seamlessly.

Agreed. Unfortunately, I think it's hopeless to get this to work reliably, without fixing that.

Using Local time, just means the server has to be the "right" timezone. It would probably be pretty close in most cases, but we would get endless error reports about it.

I'm almost certain that the right way is using bridge.timezone - but I'm not sure anybody outside Philips actually knows for sure. We'll be the first 😆

Would it make sense to move timezone from the config to the state? That way we could use best_guess_timezone to set the initial timezone and let the user override it like normal using the app?

And yeah I can see how using server time will be a problem for some users. I run everything in Kubernetes so I just set TZ on the container.

That's an excellent idea! It's really only in the config, to kick-start the bridge setup.

We should definitely do that! ... in another PR 😄

chrivers · 2025-04-21T11:03:19Z

src/server/scheduler.rs

+}
+
+impl WakeupJob {
+    fn start_datetime(&self, now: DateTime<Local>) -> Result<DateTime<Local>, &'static str> {


An error type of &'static str, although convenient, is not very good.

Feel free to introduce either a local error enum, or new error variants to ApiError, which (yes, despite the name.. 😅) is broadly used in Bifrost.

Yeah, forgot to fix this. I used the string values while debugging, but it's probably nice to have proper errors if other users experiences errors in this part of the code.

Definitely. Just use ApiResult<> while in the main Bifrost crate 👍

(feel free to introduce new error variants if you feel it's needed)

chrivers · 2025-04-21T18:51:59Z

src/server/scheduler.rs

+            Self::Light(resource_link) => {
+                let payload = LightUpdate::default()
+                    .with_on(Some(On::new(true)))
+                    .with_brightness(Some(0.0))


Watch out - many lights do not like (function) with brightness == 0.0.

Usual safe value is 1.0/255.0, which is going to be very dim, even for lights with more than 8 bits of precision.

For instance, (many) hue lights have, at least internally, 11 bits of brightness precision, but 1.0/255.0 is still very dim. We could make this a const on Dimming (MINIMUM_SAFE_BRIGHTNESS or somesuch?)

chrivers · 2025-04-21T18:56:40Z

src/server/scheduler.rs

+        let reset_backend_request = match self {
+            Self::Light(resource_link) => {
+                let payload = LightUpdate::default()
+                    .with_on(Some(On::new(true)))


If you like you could put this under impl On { ... in crates/hue/api/light.rs:

impl From<bool> for On { fn from(value: bool) -> Self { Self::new(value) } }

Then we could write:

.with_on(Some(true))

chrivers · 2025-04-21T19:00:20Z

src/resource.rs

        self.state.insert(link.rid, obj);

-        self.state_updates.notify_one();
+        self.state_updates.notify_waiters();


Oh dear, how did I miss this!? :D

I'm surprised things have worked as relatively well as they have. Good catch (on all counts) 👍

Yeah, this one confused me quite a bit. I think the only reason it worked is because there was only one listener 😄

chrivers · 2025-04-30T12:24:52Z

@duvholt I'm very sorry for making your life difficult again 😅

I've merged a stack of outstanding PRs into master, that make the internals of Bifrost quite a bit simpler:

Eventstreams are now generated correctly, and without resource-type-specific code
All hue (clip) routes are now handled generically, with only real resource handling happening in submodules. This means half of your code in src/routes/clip is no longer needed :)
Backend events have been refactored, making handling them simpler

In general, this makes it much easier to maintain Bifrost in the long term, but unfortunately it means a bunch of merge conflicts for your PR. Sorry about that 😅

When you have time, ping me on discord, if you'd like to talk about how we approach this?

I think the majority of your code should be unaffected, but some rework is unavoidable.

I'm looking forward to seeing your next iteration, and I think it should be a smoother road from here :-)

duvholt · 2025-05-01T14:56:54Z

@duvholt I'm very sorry for making your life difficult again 😅

No problem, I haven't worked on this lately so I expected some conflicts 😁 I'll probably take a look at rebasing this later today so I'll ping you then.

duvholt added 21 commits April 20, 2025 14:37

Add CRUD support for setting wake up routine

3fcdc17

Proof of concept implementation of wake up automation

dc87fd8

Support wake up weekday selection

ab023a5

Implement wake up brightness and transition

c99342a

Properly implement choosing devices for wakeup automation

0ee02bf

Disable unimplemented wake up sunrise style

eddd947

Implement non repeating wakeup

fee447a

Implement turn off lights after wake up

da67172

Start wake up time when fade in should start

4b548c4

It previously started fade in when the fade in when the light should've been at full brightness

Spawn actual wake up task in the background to mimic Hue

8926811

When the wake up schedule first begins it shouldn't be cancelled by doing changes

Introduce WakeupJob and refactor

318fab2

Properly implement start time calculation

9d1344b

The previous method was buggy and brittle

Refactor wake up job error handling

86087f0

Get rid of Time struct

ace0fdb

Set wake up color temperature

33263af

Add WakeupRequest abstraction so that backend requests are better gro…

0c7079a

…uped

Reset wake up lights before fade in

342385b

This should prevent to light from using the previous configuration when starting up

Fix bug where wake up alarm was disabled before it completed

7de4da5

Clean up behavior config parsing

156d8ff

Initial support for sunrise effect with hardcoded transition time

be5dd0c

Calculate effect duration for sunrise effect

db04497

chrivers requested changes Apr 21, 2025

View reviewed changes

duvholt added 4 commits April 21, 2025 20:06

Move behavior script details to hue crate

a6621c4

Add behavior_instance/{id} GET endpoint

48dbbea

Unmerge some imports

23aa411

Improve behavior instance serde models

cda0c1e

chrivers reviewed Apr 21, 2025

View reviewed changes

Add wake-up style enum

5132724

chrivers reviewed Apr 21, 2025

View reviewed changes

duvholt mentioned this pull request May 8, 2025

Hue crate changes needed for implementing wake-up automation #133

Merged

Uh oh!

Wake up automation w/ sunrise effect #113

Are you sure you want to change the base?

Wake up automation w/ sunrise effect #113

Uh oh!

Conversation

duvholt commented Apr 20, 2025

Uh oh!

chrivers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrivers commented Apr 30, 2025

Uh oh!

duvholt commented May 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants