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

Fix implementation of set_rgbw. #52

Closed
wants to merge 1 commit into from

Conversation

rpavlik
Copy link

@rpavlik rpavlik commented Dec 9, 2024

I tried to run the example with my "LD-0003" device from Five Below, and I got an exception in the set_rgbw call. Looks like it was trying to maybe pass None for both of the "white" params, when the rgbw expands to already include one of them?

@bdraco
Copy link
Member

bdraco commented Dec 9, 2024

Can you provide a link to where you purchased the device?

@rpavlik
Copy link
Author

rpavlik commented Dec 10, 2024

I got it quite a while ago, I don't think they sell it anymore. It's not on their web site except via search engine. "Lumin" Smart Light, FCC ID 2ADM5-LD-0003. https://www.fivebelow.com/products/lumin-bluetooth-smart-light-w-app-control

But regardless, the bug still exists in the underlying code, that's independent of what device it's used with. All you have to do is modify the example to specify your device's address and run it.

@bdraco
Copy link
Member

bdraco commented Dec 10, 2024

Found one on ebay. I'll test the change when it arrives

@rpavlik
Copy link
Author

rpavlik commented Dec 11, 2024

The device is rather immaterial to this fix. Any device would show this, it's literally a syntax error?

@bdraco
Copy link
Member

bdraco commented Dec 11, 2024

The device is rather immaterial to this fix. Any device would show this, it's literally a syntax error?

There are no tests for the library, and this PR doesn't add tests for this case so it should be independently tested manually instead.

@bdraco
Copy link
Member

bdraco commented Dec 11, 2024

Its a bit unexpected that the linters didn't catch the tuple unpack. I switched it to use kwargs instead #53 so it can't happen

@rpavlik
Copy link
Author

rpavlik commented Dec 11, 2024

oh even better, good idea.

@rpavlik rpavlik deleted the fix-rgbw branch December 11, 2024 21:47
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.

2 participants