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

Add support for the new SKY firmware effect available on LIFX Ceiling devices #132

Merged
merged 3 commits into from
Jun 14, 2024

Conversation

Djelibeybi
Copy link
Collaborator

@Djelibeybi Djelibeybi commented Jun 13, 2024

Note that the TileEffectSkyType enum is currently missing from the published protocol and needs to be added manually until LIFX add it themselves.

The TileEffectSkyType enum has been added upstream.

Signed-off-by: Avi Miller me@dje.li

Copy link
Owner

@delfick delfick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me :)

@Djelibeybi Djelibeybi marked this pull request as ready for review June 14, 2024 02:01
@Djelibeybi Djelibeybi requested a review from delfick June 14, 2024 02:01
@Djelibeybi
Copy link
Collaborator Author

Upstream have fixed the protocol specification so this is ready for review/merge.

@delfick
Copy link
Owner

delfick commented Jun 14, 2024

@Djelibeybi are you able to clean up the commit history a little please ? :)

@Djelibeybi
Copy link
Collaborator Author

@Djelibeybi are you able to clean up the commit history a little please ? :)

I haven't squashed any of the commits, but I have cleaned up the commit messages. Hope that's what you meant. ;)

@delfick
Copy link
Owner

delfick commented Jun 14, 2024

@Djelibeybi as in it feels this PR only really needs three commits

  • deps: fix bootstrap script for making protocol
  • deps: update public-protocol and regenerate protocol
  • control: Add support for sky firmware effect

Signed-off-by: Avi Miller <me@dje.li>
Signed-off-by: Avi Miller <me@dje.li>
Signed-off-by: Avi Miller <me@dje.li>
@Djelibeybi
Copy link
Collaborator Author

Done. I also switched the field names to snake case to match everything else and updated the help text just a little bit. :)

Copy link
Owner

@delfick delfick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks, do you want me to make a release too?

@delfick delfick merged commit 9b6eccf into delfick:main Jun 14, 2024
9 checks passed
@Djelibeybi Djelibeybi deleted the add-sky-effect branch June 14, 2024 05:12
@Djelibeybi
Copy link
Collaborator Author

Looks good, thanks, do you want me to make a release too?

Not super urgent, but getting Interactor support would be nifty. I'm not sure if that's automagic these days or not.

@delfick
Copy link
Owner

delfick commented Jun 14, 2024

I'm not sure if that's automagic these days or not.

not overly. Technically interactor only has a lower bound on photons, but the docker container needs to be rebuilt for home assistant

Anyway I have released new photons core and interactor and the docker builds here https://github.com/delfick/photons/actions/runs/9512926226

@Djelibeybi

@Djelibeybi
Copy link
Collaborator Author

I'll add a reminder to take the container for a spin over the weekend. I've spent enough time staring at a screen today. :)

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.

None yet

2 participants