Adding: polling, actions and feedbacks (issue #4)

bitfocus / companion-module-elgato-keylight

Controls the Elgato Keylight and Ringlight family of devices

MIT License

3 stars 5 forks source link

Adding: polling, actions and feedbacks (issue #4) #5

Closed estilles closed 3 years ago

estilles commented 3 years ago

Adding the following contributions, as suggested in issue #4

Cleanup

removing unused variables declarations
Fixing: debug declares/used but not initialized

Features

polling light for status (power, brightness and temperature)
- settings to allow enable/disable polling
- settings to change polling interval
new actions:
- Power cycle/toggle on/off
- Setting temperature in Kelvin (dropdown)
- Increasing/decreasing temperature
- increasing/decreasing brightness
new feedbacks
- change button color based on power status
- change button color based on brightness
- change button color based on temperature

krocheck commented 3 years ago

This all seems fine, but before I update the core my only question is why a color temperature action for Kelvin with a dropdown was added instead of converting the existing action to that mode? Seems to me the 'RAW' number is not a useful control point for most users anyway.

estilles commented 3 years ago

This all seems fine, but before I update the core my only question is why a color temperature action for Kelvin with a dropdown was added instead of converting the existing action to that mode? Seems to me the 'RAW' number is not a useful control point for most users anyway.

The only reason I added one instead of replacing the existing one was for backwards compatibility. If you prefer to just have one action I'd be more than happy to update my PR.

krocheck commented 3 years ago

The underlying value/id is the same though, is it not? I'd think the values would transpose in a upgrade, but worst case we can do an upgrade script to convert the int to string if the dropdown forces that (I forget if we fixed that or not).

estilles commented 3 years ago

The underlying value/id is the same though, is it not?

Yes. The values are exactly the same. I will refactor and update my PR. Thanks for the comments.

I'd think the values would transpose in a upgrade, but worst case we can do an upgrade script to convert the int to string if the dropdown forces that (I forget if we fixed that or not).

You're probably right. I don't think there'll be a need for an upgrade script.

krocheck commented 3 years ago

This PR was already merged, so I think it will need to be a new one

estilles commented 3 years ago

This PR was already merged, so I think it will need to be a new one

I just realized that. Submitting a new PR shortly.

estilles commented 3 years ago

Submitted PR #6