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

Innr RB 250C does not support enhancedHue #1040

Merged
merged 4 commits into from
Mar 7, 2020
Merged

Conversation

sjorge
Copy link
Sponsor Contributor

@sjorge sjorge commented Mar 6, 2020

I noticed a while ago that the Innr RB 250C did not support the enhancedHue and friends command.

It does however support the generic moveToHue, moveToSaturation, and moveToHueAndSaturation commands. I never figured out how to till the converter how to deal with this until I saw your options.applyRedFix.

This PR introduces options.enhancedHue, I set this to false for the Innr RB 250C (probably other Innr bulbs would need it too, but I don't own those so I couldn't test).

I thought this would be less disruptive than setting options.enhancedHue to true for all existing bulbs. I then added an extra branch to the if and case statements in light_color toZigbee converter to make it do the right thing.

@sjorge
Copy link
Sponsor Contributor Author

sjorge commented Mar 6, 2020

zigbee2mqtt:debug 2020-03-06 17:11:29: Received MQTT message on 'zigbee2mqtt/0x14b457fffe2bd760/set' with data '{"color":{ "hue": 150, "saturation": 100}}'
zigbee2mqtt:debug 2020-03-06 17:11:29: Publishing 'set' 'color' to '0x14b457fffe2bd760'
zigbee2mqtt:info  2020-03-06 17:11:29: MQTT publish: topic 'zigbee2mqtt/0x14b457fffe2bd760', payload '{"state":"ON","linkquality":55,"brightness":254,"color":{"saturation":100,"hue":150,"x":0.6763,"y":0.3102},"color_temp":555,"color_mode":1}'
zigbee2mqtt:debug 2020-03-06 17:11:29: Received Zigbee message from '0x14b457fffe2bd760', type 'attributeReport', cluster 'lightingColorCtrl', data '{"currentHue":106,"currentSaturation":254,"currentX":7903,"currentY":31455,"colorTemperature":153}' from endpoint 1 with groupID 0
zigbee2mqtt:info  2020-03-06 17:11:30: MQTT publish: topic 'zigbee2mqtt/0x14b457fffe2bd760', payload '{"state":"ON","linkquality":55,"brightness":254,"color":{"saturation":100,"hue":150,"x":0.1206,"y":0.48},"color_temp":153,"color_mode":1}'
zigbee2mqtt:debug 2020-03-06 17:11:32: Received MQTT message on 'zigbee2mqtt/0x14b457fffe2bd760/set' with data '{"color":{ "hue": 0, "saturation": 100}}'
zigbee2mqtt:debug 2020-03-06 17:11:32: Publishing 'set' 'color' to '0x14b457fffe2bd760'
zigbee2mqtt:info  2020-03-06 17:11:32: MQTT publish: topic 'zigbee2mqtt/0x14b457fffe2bd760', payload '{"state":"ON","linkquality":55,"brightness":254,"color":{"saturation":100,"hue":0,"x":0.1206,"y":0.48},"color_temp":153,"color_mode":1}'
zigbee2mqtt:debug 2020-03-06 17:11:32: Received Zigbee message from '0x14b457fffe2bd760', type 'attributeReport', cluster 'lightingColorCtrl', data '{"currentHue":0,"currentSaturation":254}' from endpoint 1 with groupID 0
zigbee2mqtt:debug 2020-03-06 17:11:32: Received Zigbee message from '0x14b457fffe2bd760', type 'attributeReport', cluster 'lightingColorCtrl', data '{"currentX":44323,"currentY":20329,"colorTemperature":555}' from endpoint 1 with groupID 0
zigbee2mqtt:info  2020-03-06 17:11:33: MQTT publish: topic 'zigbee2mqtt/0x14b457fffe2bd760', payload '{"state":"ON","linkquality":55,"brightness":254,"color":{"saturation":100,"hue":0,"x":0.6763,"y":0.3102},"color_temp":555,"color_mode":1}'

Not all device support enhancedMoveToHue, enhancedMoveToHueAndSaturation... we add a new option to indicate support.
We assume the default is yet.
@sjorge
Copy link
Sponsor Contributor Author

sjorge commented Mar 6, 2020

forced pushed a lint fix

@sjorge
Copy link
Sponsor Contributor Author

sjorge commented Mar 6, 2020

@Koenkk do we want to read currentHue and currentSaturation in the tz.light_color ? As we are setting those. They should return anything in colorMode 1, but should return in colorMode 0.

Perhaps we should also query colorMode itself too.

@Koenkk
Copy link
Owner

Koenkk commented Mar 6, 2020

PR looks fine to me,

Which read do you mean?

@sjorge
Copy link
Sponsor Contributor Author

sjorge commented Mar 6, 2020

The one in convertGet

@Koenkk
Copy link
Owner

Koenkk commented Mar 7, 2020

Yes, it the same logic should also be added there.

@sjorge
Copy link
Sponsor Contributor Author

sjorge commented Mar 7, 2020

Done, I added currentHue and currentSaturation to the convertGet

@Koenkk
Copy link
Owner

Koenkk commented Mar 7, 2020

Thanks!

@Koenkk Koenkk merged commit 9047828 into Koenkk:master Mar 7, 2020
@sjorge sjorge deleted the enhancedhue branch March 7, 2020 22:34
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