-
-
Notifications
You must be signed in to change notification settings - Fork 11
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: basic support for hello fairy #56
base: main
Are you sure you want to change the base?
Conversation
src/led_ble/led_ble.py
Outdated
model_num = 0 | ||
if self._is_hello_fairy(): | ||
if data[0] == 0xAA: | ||
if data[1] == 0x00: # hw info | ||
if len(data) > 7: | ||
version_string = data[3:8].decode("ascii") | ||
_LOGGER.debug("version %s", version_string) | ||
self._state = replace( | ||
self._state, | ||
version_num=(data[3] - 48) * 100 | ||
+ (data[5] - 48) * 10 | ||
+ (data[7] - 48), | ||
) | ||
if len(data) > 12: | ||
model = data[8:13].decode("ascii") | ||
_LOGGER.debug("model %s", model) | ||
if len(data) > 24: | ||
lights = data[24] # guessing | ||
_LOGGER.debug("lights %d", lights) | ||
if len(data) > 33: | ||
effects = data[33] # guessing | ||
_LOGGER.debug("effects %d", effects) | ||
|
||
if data[1] == 0x01: # state info | ||
if len(data) > 6: | ||
self._state = replace(self._state, power=data[6] > 0) | ||
else: | ||
if len(data) == 4 and data[0] == 0xCC: | ||
on = data[1] == 0x23 | ||
self._state = replace(self._state, power=on) | ||
return | ||
if len(data) < 11: | ||
return | ||
model_num = data[1] | ||
on = data[2] == 0x23 | ||
preset_pattern = data[3] | ||
mode = data[4] | ||
speed = data[5] | ||
r = data[6] | ||
g = data[7] | ||
b = data[8] | ||
w = data[9] | ||
version = data[10] | ||
self._state = LEDBLEState( | ||
on, (r, g, b), w, model_num, preset_pattern, mode, speed, version | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to abstract this into a new method in the protocol so each protocol can handle this different
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. We don't control the ProtocolBase class though, it comes from flux_led. One option would be to make a device class to encapsulate the differences between the flux_led devices and the hello fairy devices. This would include the (send) protocol, this notification_handler, and (to your other comments), the status command, and protocol_for_version_num.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its no problem to merge a PR to flux_led
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cleaned up flux_led for Python 3.9+ and released 1.1.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want the new protocol method to return LEDBLEState (so move that class into flux-led)? or a generic dict?
await self._send_command_while_connected([STATE_COMMAND]) | ||
if self._is_hello_fairy(): | ||
await self._send_command_while_connected( | ||
[b"\xaa\x00\x00\xaa"] | ||
) # get version and capabilities | ||
else: | ||
await self._send_command_while_connected([STATE_COMMAND]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a new method to the protocol to get the state command instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that method is implemented but the protocol is not resolved yet at this point
I'm about to have family in town for the week so I'll likely miss GitHub notifications. If I don't respond for a day or so, feel free to ping me on discord (same handle) |
Support for addressable LED strings that use the 'hello fairy' app. The protocol is different but pretty analogous to the existing protocols.
I've tested this with HA. Very minimal changes are needed there to use these changes, as I'm sure you know.
I don't know if you're interested in expanding your library in this way. If not, feel free to close this and I'll just work with the fork (and I'll have to make a new HA integration essentially the same as led-ble, in that case).
This code would be cleaner if some refactoring were done to generalize some things and encapsulate device differences, e.g. response protocol, status request command, parameter scaling. I didn't do any of that because I didn't want to change more than necessary without coordination, but I could look at that.