Add Philips Hue Sensor support #5996
Conversation
polling the states via the lights-api is not possible under version 1.11, and this change continues to use the config-api. For that purpose, a ApiVersion type is introduced Signed-off-by: Samuel Leisering <samuel@sleisering.de>
Added support for the Hue Presence Sensor (Includes a temperature sensor, a presence sensor and a light level sensor) and the Hue Dimmer Switch * Refactored class HueLights to HueObjects * Added class FullSensor * Added handlers for the sensors * Added sensor thing and channel xmls * Added sensor polling job and configuration Signed-off-by: Samuel Leisering <samuel@sleisering.de>
* add license header to FullHueObject and HueSensorHandler * fix null-warning in HueBridgeHandler * added new package to exported packages * removed debug sysout Signed-off-by: Samuel Leisering <samuel@sleisering.de>
* made toJson of ConfigUpdate public Signed-off-by: Samuel Leisering <samuel@sleisering.de>
* changed unit tests to reflect the polling changes * added lock to prevent parallel polling * fixed the polling interval for sensors Signed-off-by: Samuel Leisering <samuel@sleisering.de>
|
Does this support the remote control in any way also? |
|
@thedannymullen it supports the dimmer switch, but not the tap switch. Looking at the documentation the tap switch seems that it's very similar to the dimmer switch, so it should be easy to add. I don't have one of those though, so its not part of this |
| <representation-property>uniqueId</representation-property> | ||
|
|
||
| <config-description> | ||
| <parameter name="lightId" type="text"> |
There was a problem hiding this comment.
In the code you partially use sensorId. Why not in this case?
There was a problem hiding this comment.
the lightId and sensorId are basically the same thing on the bridge, and i didn't want to introduce a new id-parameter which adds nothing. Maybe the lightId should be changed to objectId or something, but i dont know whether this would be backwards compatible
There was a problem hiding this comment.
I know, but isn't it a little bit confusion to set a property lightId on a sensor device?
| <label>Sensor Polling Interval</label> | ||
| <description>Milliseconds between fetching sensor-values from the Bridge. A higher value means more delay for the sensor values, but a too low value can cause congestion on the bridge.</description> | ||
| <required>true</required> | ||
| <default>100</default> |
There was a problem hiding this comment.
100ms as default value seems a little bit too optimistic for me. Don't you expect concurrency issues? Do we have to prepare for those?
Does the sensor polling job run always? Independently if a sensor has been added to the Hue bridge or not?
There was a problem hiding this comment.
There was a problem hiding this comment.
Don't he use that interval to check updates from the bridge?
E.g. a remote receives a click and sends it to the hue bridge, in my opinion openhab won't get notified about this change till the sensor polling interval is elapsed. Am I wrong?
There was a problem hiding this comment.
@darkblackside you are correct. Currently that is the only way to do that.
choosing a hig value for the interval starts to sometimes drop events (the button pressed event for instance, if the button released event follows closely). 100 ms seemed like a reasonable default to receive all events and not overwork the bridge.
@cweitkamp the only race contidion i can see is if the sensor and light polling job run at the same time. For this reason, the polling job acquires a lock during its execution.
The Sensor polling job always polls and updates all sensors.
|
|
||
| @Override | ||
| protected String getVendor(String modelId) { | ||
| return "Philips"; |
There was a problem hiding this comment.
Hard-coded "Philips"? What about other Zigbee sensors?
There is a vendor map in the HueLightHandler.
There was a problem hiding this comment.
But is the dimmerswitch not a philips-specific device?
There was a problem hiding this comment.
The DimmerSwitch is a philips-specific device, but apparently conformes to the ZigBee Standard.
But currentl the hue bridge only fully supports philips-specific switches and sensors, so a mapping is not yet neccessary
There was a problem hiding this comment.
I'll try to include a tradfri remote to hue and edit this code here, or did you test it with the tradfri remote already?
There was a problem hiding this comment.
I did not, no. According to some people the tradfri remote would communicate with the lamps directly, and would not be accessible using the bridge.
I was referring to the iConnectHue Project that states that 3rd party switches and sensors are not yet fully supported by the hue bridge. I don't know if thats accurate though.
There was a problem hiding this comment.
There are several people which connect the tradfri remote directly to the lamp, but it is also possible to connect it first to the bridge via touchlink and then connect it to a lamp.
I have two of the big tradfri remotes at home and will try to include them.
| protected void doSensorStateChanged(@Nullable HueBridge bridge, @NonNull FullSensor sensor, Configuration config) { | ||
| Object tmp = sensor.getState().get("temperature"); | ||
| BigDecimal value = new BigDecimal(String.valueOf(tmp)); | ||
| DecimalType temperature = new DecimalType(value.divide(new BigDecimal(100))); |
There was a problem hiding this comment.
If I remember correctly you declared the temperature Channel as Number:Temperature. Why do you use a DecimalType to update its state instead of a QuantityType?
There was a problem hiding this comment.
This would introduce dependencies to javax.measure.quantity and org.eclipse.smarthome.core.library.unit, if that's okay
There was a problem hiding this comment.
That is not only okay. It is very much appreciated.
| * @param on | ||
| */ | ||
| public void setOn(boolean on) { | ||
| commands.add(new Command("on", on)); |
There was a problem hiding this comment.
Can we turn the sensor on and off with this method? Please complete the JavaDoc to clarify.
There was a problem hiding this comment.
Look two rows above, there is a clarifying comment
* enable/disable the sensor
There was a problem hiding this comment.
i added some clarification from the Hue API Documentation. It sounds like the sensor itself is not technically switched off, the bridge just stops reporting status changes
| </parameter> | ||
| </config-description> | ||
| </thing-type> | ||
| </thing:thing-descriptions> |
There was a problem hiding this comment.
There are missing some new lines at the end of several files throughout your code. Can you please add them?
| </channel-type> | ||
|
|
||
| <channel-type id="lightlevel"> | ||
| <item-type>Number</item-type> |
There was a problem hiding this comment.
Maybe Number:Illuminance and unit "Lux"? We are more familiar with that.
IIRC the formula for converting the sensors lightlevel value to lux is known (see https://community.openhab.org/t/howto-use-philips-hue-sensors-motion-sensor-dimmer-switch/39344/4).
There was a problem hiding this comment.
If I understand the QuantityType of Openhab like it's defined, maybe it is possible to use an implementation of the QuantityType to make the conversion between different units possible easily. Then there could be Lux and the hue-own unit from one sensor value.
There was a problem hiding this comment.
For me it is okay to have two different channels to represent the values. One for the lightlevel which is returned by the API and one for the illumination.
Maybe we can ask some questions to get an answer for a proper design:
- What is the default value according to the ZigBee
ZLLLightLevelsensor specification? - Which value do we need elsewhere (e.g. in the properties
tholddarkandtholdoffset)? - What is the most convenient value the people will understand / work with?
There was a problem hiding this comment.
To 3: I think Lux is the better value to display instead of the hue-specific format because that is a defined SI-unit. Therefore you can use it in formulas if you need to for your automation.
Another option would be using a unit like Candela or Lumen. But these are not optimal for our problem because they must be calculated using some unknown variables, like, for example, distance.
There was a problem hiding this comment.
-
The documentation of the ZigBee Cluster library on page 423f gives the illuminance in logarithmic form, same as the hue bridge.
-
the thresholds are based on the same logartithmic values.
I changed the illuminance to lux in the last commit, but i think having two channels providing both values would be best
There was a problem hiding this comment.
having two channels providing both values would be best
Yes, I second that. "illuminance" and "lightLevel" ("lightLevel" should be an advanced channel).
| properties.put(MODEL_ID, modelId); | ||
| properties.put(LIGHT_UNIQUE_ID, light.getUniqueID()); | ||
| String uniqueID = object.getUniqueID(); | ||
| if (uniqueID != null) { |
There was a problem hiding this comment.
In which case does this happen? The uniqueId is the representational property of each thing and it would be a mess if we cannot set it properly.
There was a problem hiding this comment.
There is a virtual daylight sensor on the bridge which does not have an unique id, because it is implemented by the bridge itself. therefore the @NonNull annotation on getUniqueID is technically wrong.
There was a problem hiding this comment.
I see what you are meaning. But we are not adding a thing for that one anyways, do we?
| @@ -47,6 +54,7 @@ | |||
| * added representationProperty to discovery result | |||
| * @author Thomas Höfer - Added representation | |||
| * @author Denis Dudnik - switched to internally integrated source of Jue library | |||
| * @author Samuel Leisering - Added sensor support | |||
| */ | |||
| @NonNullByDefault | |||
| public class HueLightDiscoveryService extends AbstractDiscoveryService implements LightStatusListener { | |||
There was a problem hiding this comment.
Maybe I have missed something: Should the HueLightDiscoveryService implement the SensorStatusListener interface as well?
|
|
||
| abstract class PollingRunnable implements Runnable { |
There was a problem hiding this comment.
While touching this anyways, do you think it is possible to get rid of the Runnable interface and find a more elegant solution?
There was a problem hiding this comment.
the polling runnables are scheduled using the BaseThingHandlers executor service, so i'm not sure how to improve this. Do you have something in mind?
There was a problem hiding this comment.
I think it's connected to this comment: #5996 (comment)
Maybe you should try to register at the hue bridge and get notified if the value on the hue bridge changed. I don't know if this is possible, but it would be an elegant way to solve the runnable-problem.
There was a problem hiding this comment.
I see. Unfortunately the bridge does not support registering for changes, so polling is the only way currently
There was a problem hiding this comment.
Do you have something in mind?
Yes, I do. Refactoring anonymous classes to Java 8 lambda expressions. This will make the code easier to maintain and more readable.
Currently we are using something like this:
// using an anonymous class
Runnable pollingRunnable = new Runnable() {
@Override
public void run() {
// do polling stuff
}
};
scheduler.scheduleWithFixedDelay(pollingRunnable, 1, 1, TimeUnit.SECONDS);Possible replacement:
// using a lambda expression
scheduler.scheduleWithFixedDelay(() -> {
// do polling stuff
}, 1, 1, TimeUnit.SECONDS);Or:
class myClass {
public void poll() {
// do polling stuff
}
public void myFunction() {
scheduler.scheduleWithFixedDelay(this::poll, 1, 1, TimeUnit.SECONDS);
}
}
|
@Simiil Thanks for all the diligence and hard work! I would be happy to test the software. How do I obtain a copy? I have not used github beyond clicking the download a zip file. |
* changed lightlevel channel type to illuminance * calculated lux value for lightlevel channel * added manifest dependency for quantity type * set temperature channel using quantity type Signed-off-by: Samuel Leisering <samuel@sleisering.de>
| BigDecimal lightlevel = new BigDecimal(String.valueOf(oLightLevel)); | ||
| double lux = Math.pow(10, (lightlevel.subtract(new BigDecimal(1)).divide(new BigDecimal(10000))).doubleValue()); | ||
|
|
||
| updateState(HueBindingConstants.CHANNEL_LIGHTLEVEL, new QuantityType<Illuminance>(lux, SmartHomeUnits.LUX)); |
There was a problem hiding this comment.
You used a QuantityType here. So this comment is outdated #5996 (comment)
There was a problem hiding this comment.
I ran some functional tests tonight. Here are some points I observed during my tests.
After I added a new discovered thing via Paper UI and linked the channels to items the state of those items are not updated immediately. I had to wait until a onSensorStateChanged() event was triggered to see the values.
The timestamp for the last_updated channel is wrong (see inline comment).
Some exceptions occurred after deactivating my motion sensor in the Hue App.
First:
2018-08-20 21:28:20.119 [DEBUG] [binding.hue.handler.HueBridgeHandler] - Status update for Hue sensor 12 detected: {presence=null, lastupdated=none}
2018-08-20 21:28:20.155 [ERROR] [binding.hue.handler.HueBridgeHandler] - An exception occurred while calling the BridgeHeartbeatListener
java.lang.StringIndexOutOfBoundsException: String index out of range: 10
at java.lang.String.substring(String.java:1963) [?:?]
at org.eclipse.smarthome.core.library.types.DateTimeType.<init>(DateTimeType.java:96) [94:org.eclipse.smarthome.core:0.10.0.201808011124]
at org.eclipse.smarthome.core.library.types.DateTimeType.valueOf(DateTimeType.java:120) [94:org.eclipse.smarthome.core:0.10.0.201808011124]
at org.eclipse.smarthome.binding.hue.handler.HueSensorHandler.onSensorStateChanged(HueSensorHandler.java:179) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler.notifySensorStatusListeners(HueBridgeHandler.java:656) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler.access$3(HueBridgeHandler.java:641) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler$1.doConnectedRun(HueBridgeHandler.java:191) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler$PollingRunnable.run(HueBridgeHandler.java:94) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) [?:?]
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308) [?:?]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180) [?:?]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294) [?:?]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:?]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:?]
at java.lang.Thread.run(Thread.java:748) [?:?]
Second:
2018-08-20 21:28:20.207 [DEBUG] [binding.hue.handler.HueBridgeHandler] - Status update for Hue sensor 13 detected: {lightlevel=null, dark=null, daylight=null, lastupdated=none}
2018-08-20 21:28:20.246 [ERROR] [binding.hue.handler.HueBridgeHandler] - An exception occurred while calling the BridgeHeartbeatListener
java.lang.NumberFormatException: null
at java.math.BigDecimal.<init>(BigDecimal.java:494) [?:?]
at java.math.BigDecimal.<init>(BigDecimal.java:383) [?:?]
at java.math.BigDecimal.<init>(BigDecimal.java:806) [?:?]
at org.eclipse.smarthome.binding.hue.handler.sensors.LightLevelHandler.doSensorStateChanged(LightLevelHandler.java:70) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueSensorHandler.onSensorStateChanged(HueSensorHandler.java:174) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler.notifySensorStatusListeners(HueBridgeHandler.java:656) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler.access$3(HueBridgeHandler.java:641) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler$1.doConnectedRun(HueBridgeHandler.java:191) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler$PollingRunnable.run(HueBridgeHandler.java:94) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) [?:?]
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308) [?:?]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180) [?:?]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294) [?:?]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:?]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:?]
at java.lang.Thread.run(Thread.java:748) [?:?]
Third:
2018-08-20 21:28:20.377 [DEBUG] [binding.hue.handler.HueBridgeHandler] - Status update for Hue sensor 11 detected: {temperature=null, lastupdated=none}
2018-08-20 21:28:20.388 [ERROR] [binding.hue.handler.HueBridgeHandler] - An exception occurred while calling the BridgeHeartbeatListener
java.lang.NumberFormatException: null
at java.math.BigDecimal.<init>(BigDecimal.java:494) [?:?]
at java.math.BigDecimal.<init>(BigDecimal.java:383) [?:?]
at java.math.BigDecimal.<init>(BigDecimal.java:806) [?:?]
at org.eclipse.smarthome.binding.hue.handler.sensors.TemperatureHandler.doSensorStateChanged(TemperatureHandler.java:53) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueSensorHandler.onSensorStateChanged(HueSensorHandler.java:174) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler.notifySensorStatusListeners(HueBridgeHandler.java:656) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler.access$3(HueBridgeHandler.java:641) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler$1.doConnectedRun(HueBridgeHandler.java:191) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at org.eclipse.smarthome.binding.hue.handler.HueBridgeHandler$PollingRunnable.run(HueBridgeHandler.java:94) [243:org.eclipse.smarthome.binding.hue:0.10.0.201808201722]
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) [?:?]
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308) [?:?]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180) [?:?]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294) [?:?]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:?]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:?]
at java.lang.Thread.run(Thread.java:748) [?:?]
Sry for my short comments inside the code.
| Object valueObject = sensor.getState().get("lastupdated"); | ||
| if (valueObject != null) { | ||
| // DateFormat.getDateTimeInstance().parse(String.valueOf(valueObject)); | ||
| DateTimeType type = DateTimeType.valueOf(String.valueOf(valueObject)); |
There was a problem hiding this comment.
We should consider the timezone for converting the DateTime values. I am living in CEST (UTC+2) whereas the the API returns UTC.
2018-08-20 20:45:23.394 [DEBUG] [binding.hue.handler.HueBridgeHandler] - Status update for Hue sensor 11 detected: {temperature=2008.0, lastupdated=2018-08-20T18:45:23}
| <tags> | ||
| <tag>Lighting</tag> | ||
| </tags> | ||
| <state> |
| <label>Last Updated</label> | ||
| <description> The date when the sensor was last updated | ||
| </description> | ||
| <tags> |
There was a problem hiding this comment.
Add <state readOnly="true" /> to all channel types.
There was a problem hiding this comment.
Remove <tags> on this channel type.
| <label>Light Level</label> | ||
| <description> The detected lightlevel in logarithmic lux | ||
| </description> | ||
| <tags> |
There was a problem hiding this comment.
Add <state readOnly="true" pattern="%.1f %unit%" />.
| <description> The Button that was last pressed on the dimmer switch | ||
| </description> | ||
| <category>Light</category> | ||
| <tags> |
There was a problem hiding this comment.
Remove <tags> on this channel type.
| <label>On</label> | ||
| <description>Indicates whether the sensor is on</description> | ||
| </parameter> | ||
| <parameter name="battery" type="integer" readOnly="true"> |
There was a problem hiding this comment.
Instead of using a configuration parameter for the battery level we should add two channels for it (see e.g. https://github.com/eclipse/smarthome/blob/master/extensions/binding/org.eclipse.smarthome.binding.tradfri/ESH-INF/thing/thing-types.xml#L95-L96).
|
|
||
| <channels> | ||
| <channel id="dimmer_switch" typeId="dimmer_switch" /> | ||
| <channel id="last_updated" typeId="last_updated" /> |
There was a problem hiding this comment.
Add the battery channels on this thing type too.
| channel-type.hue.temperature.label = Temperatur | ||
| channel-type.hue.temperature.description = Die Temperatur | ||
|
|
||
| channel-type.hue.dimmer_switch.options.option.1000 = An (Initial) |
There was a problem hiding this comment.
The translations for the state options do not work. The key is wrong. Sry for my misleading example.
channel-type.hue.dimmer_switch.state.option.1000 = ...
| <label>Battery level</label> | ||
| <description>The battery level of the sensor</description> | ||
| </parameter> | ||
| <parameter name="reachable" type="boolean" readOnly="true"> |
There was a problem hiding this comment.
IMHO we do not need this configuration parameter. The status of the thing should reflect the reachability.
| <description>The identifier that is used within the hue bridge.</description> | ||
| <required>true</required> | ||
| </parameter> | ||
| <parameter name="tholddark" type="integer"> |
There was a problem hiding this comment.
Is the user supposed to change these values? If so it does not work. It was not send to the Hue Bridge.
|
@Simiil Are you still around? Please let me know if you need any help on this. |
|
Definitely curious also! Love to see some of the updates I believe are included in this. It would be great if TRÅDFRI could be connected also. |
|
@cweitkamp I would like to help also, but I need help to setup my IDE. I've tried to set it up, but my compiled building is not working. Do you have any help for me? |
|
@darkblackside Maybe Yes, maybe No. But that is nothing to discuss here. Did you already asked for help in the community forum? If not, may I ask you to create a topic over there - or contact me via email (github@christophweitkamp.de) with details. |
|
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/howto-use-philips-hue-sensors-motion-sensor-dimmer-switch/39344/65 |
|
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/http-refresh-service-causing-high-cpu-load/56009/5 |
|
@Simiil Just a short heads up. I will start to work on this feature in the next couple of days and try to finish your great work. Please give me a hint if you are still interested in finalizing this PR on your own or not. Thanks in advance. |
|
Here it is: 1c82e84 @htreu @kaikreuzer How shall I continue? Create a new PR to supersed this one? |
|
Hey @cweitkamp, great that you are driving this forward! Yes, please create a new PR and we can close this one - you can keep the existing commits or alternatively simply create a new commit with an "Also-By" line. |
|
@cweitkamp Thank you, my time to work on this was very limited recently, it is nice to get this done. I'm closing this PR. |
Adds Sensor Support to the philips hue binding using periodic polling, fixes #2603
the polling interval is a new config property, separate from the polling interval of the light states.
This contribution adds support for the Hue Presence Sensor and the Dimmer Switch.