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

feat (switch): add support for the lifx switch #50

Merged
merged 4 commits into from
Mar 19, 2023

Conversation

bradrees
Copy link
Contributor

@bradrees bradrees commented May 7, 2022

This PR adds support for the Lifx Switch.

I've added tests and tested on my switches at home, and it all seems to work.

Thanks to the work done by @Shakesbeard and @ristomatti

Resolves #38

@bradrees bradrees changed the title feat (switch): add support the lifx switch feat (switch): add support for the lifx switch May 7, 2022
@ristomatti
Copy link
Collaborator

Thank you for the contribution! I'm very busy at the moment (working on a huge and blocker code refactor at work) so I can't promise when I will be able to review this. I'll be snoozing the email notification until I get time to properly check this.

I did a quick glance though and it seemed the changes seem to align with the codebase. Only biggest issue is about how well this fits to the current API. A switch/relay cannot easily be seen as a light (even though it could be toggling a light).

It would help if you could either:

  • Update the README, while trying to figure out how to describe the new API in a way it doesn't sound awkward
  • Come up with a suggestion how we could adapt the code structure to have an object device which would then be inherited by light and switch (or relay).

The latter would be preferred but as it'd be a lot of work, I'm not sure if I want it to be a thing to prevent people from using the library with their switches.

@ristomatti
Copy link
Collaborator

Just to update - I've not forgot about this. 😬

@bradrees
Copy link
Contributor Author

bradrees commented Jan 29, 2023

Hi, I have added some documentation and have been using this with Homebridge for the last 7 or so months, it works really well and is a big improvement over the standard lifx app. It's all stable and works every time.

It would be great to get this merged in, it really improves my setup at home.

Thanks!

@ristomatti ristomatti self-requested a review February 27, 2023 08:27
Copy link
Collaborator

@ristomatti ristomatti left a comment

Choose a reason for hiding this comment

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

Can't test this but can't spot any issues either. The code and docs fit nicely in the existing code style 👍. I'll take a leap of faith and publish a new minor release. Thanks you for the contrib and sorry it took ages to review!

@ristomatti ristomatti force-pushed the feature/switch-support branch from 26e2a20 to 01a51cc Compare March 19, 2023 13:51
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.

[Feature Request] add support for LIFX Switch
2 participants