ZeroOne3010 / yetanotherhueapi

A Java library for controlling Philips Hue lights. Available from the Maven Central.
MIT License
68 stars 21 forks source link

Add support for Friends of Hue switches #23

Closed philipparndt closed 3 years ago

philipparndt commented 3 years ago

Hi @ZeroOne3010,

I've added support for Friends of Hue switches. fixes https://github.com/ZeroOne3010/yetanotherhueapi/issues/22

First I implemented this change using the DimmerSwitch classes, but as there is no Action Code for the FOH switches, this feels a bit strange and I changed it to custom sensors.

ZeroOne3010 commented 3 years ago

Very nice work, thank you for the quality submission! I'm not concerned about whether this code would work with the Senic switches you mentioned, but I am worried of how it will perform with other kinds of Friends of Hue products, such as the Lutron Aurora dimmer switch. Obviously that one is just a dimmer switch like the Philips one, but I don't know how it would appear in the API and I'm afraid it'd be misrepresented in the library if we go ahead with the implementation as it is. 😟

Another issue I just realized is that I actually recently acquired a Hue Tap switch but haven't had much time to play with it. It turns out that it is also represented as a "ZGPSwitch" -- so much for using only the type as the identifier of a sensor. 🤔

Based on what I just read online, your switch basically has two rocker keys, i.e. four individual buttons, and the "top" and "bottom" buttons are "virtual" buttons that activate when you push both rockers up/down simultaneously. Is that correct? I wonder how the top/down buttons are actually done in the API as the list of inputs that you provided only has four keys with two events (press & release) for each. Maybe there are some extra rules associated with the Senic switch as well? The Hue Tap switch, by the way, only has one event for each of the four inputs: the initial_press.

The versatility of the Friends of Hue brand also makes me concerned whether terms like "top", "left" and "bottom" apply to all of the different variations in the same way. I could easily imagine one manufacturer numbering the keys from left to right and the from top to bottom, but another one numbering them from bottom to top, or in some other permutation. Would it maybe be safer to call them "button 1", "button 2", etc? It is pretty dull, though, I admit... 😔

It'd be nice to hear more on what kind of an obstacle you ran into when trying to implement the change with the DimmerSwitch classes? I did not quite understand the part about the Senic switch not having an action code. I was thinking that maybe the dimmer switch classes could be extended to support these FoH switches after all. In your current implementation you parse event code 20 as just "LEFT_TOP", when actually it's the button "LEFT_TOP" with action "SHORT_RELEASED". It seems to me that event type 16 is currently ignored and should be parsed as button "LEFT_TOP" with action "INITIAL_PRESS". Both of these would map nicely to the current DimmerSwitchButtonEvent class, as long as the DimmerSwitchButton enum is amended with the two new buttons that do not exist in the official dimer switch but are present in the Senic one. I'm thinking the buttons should just be called "DIM_UP_2" and "DIM_DOWN_2"? ON and OFF could be mapped to "TOP" and "BOTTOM".

What do you think of all this? Sorry for the wall of text... 😅

philipparndt commented 3 years ago

Thanks for the answer. You are right, we cannot know for sure how the other FoH switches behave until someone is able to test this. The Lutron Aurora seems to be different than all other FoH switches.

I did some more tests with the Senic switch and we definitely need to do some changes in this PR. Most actions behave like the description in the capabilities/input. Senic provides an additional single rocker instead of the dual rockers. I use the dual rockers for regular use but implemented a Christmas Tree switch by pressing both top/bottom buttons.

As the hue API only supports polling and no SSE / websocket, it is nearly impossible to get the initial_press event. But sometimes it will raise.

The correct key map is:

Senic Button initial_press short_release
Left Top 0 16 20
Left Bottom 1 17 21
Right Top 2 19 23
Right Bottom 3 18 22
0+2 100 101
1+3 98 99

As 0+2 and 1+3 are not mentioned in the capabilities/input section we cannot support them by just interpreting this section.

Maybe it would be good to just use capabilities/input and add some kind of generic handling in case someone likes to do more. For example, the BasicSensor could provide more information, like manufacturerName, modelId and rawType and maybe a public method like this:

  public Map<String, Object> readRawState() {
    return stateProvider.get();
  }

with this, we would have a fallback to handle more/unknown sensors. What do you think? At the moment I use the released version of your API and do some reflection to access the readStateValue method but for obvious reasons, I don't like to use reflection ;) and this will only work by using the switch name to identify the type.

ZeroOne3010 commented 3 years ago

For one, I'm definitely thinking the current naive getDimmerSwitches() and SensorType.DIMMER_SWITCH probably need to go. A switch should be a Switch, much like a light is a Light.

I admit I haven't given enough thought to the concept of switches in the API. What's your use case with them anyway, if you don't mind me asking? It's just that it usually helps to understand the actual problem before trying to come up with some clever solution... 😅

philipparndt commented 3 years ago

You are right, there are not that many reasons for using Switches with the polling API provided by Hue. Polling makes it hard to get the event within an acceptable amount of time.

I use it as there is no other way for the Senic Switch to have two rockers (4 buttons) and assign actions when pressing both rockers at once (the button 0+2and 1+3 events). I use them to activate some scenes, that I do not use that much. For example, switching on/off the Christmas tree light or activating some party light for my kids.

What I think about doing with this is implement a toggle switch. There seems to be no way in the Hue app doing this. But the polling thing keeps me from doing this (latency of ~500-800ms just feels not right for everyday stuff).

ZeroOne3010 commented 3 years ago

Well, I've started working on revamping on how the switches are handled in the library. Stay tuned.

ZeroOne3010 commented 3 years ago

@philipparndt I'm almost done with my project, so if you don't mind I think I'm just going to go ahead and close this pull request, as the library is changing too much. Would you mind doing a code review for me, though, when I'm done? :)

ZeroOne3010 commented 3 years ago

I've made my own PR, #24.