[Tradfri] Added support for remote controller and motion sensor devices #4373
[Tradfri] Added support for remote controller and motion sensor devices #4373
Conversation
@cweitkamp Sounds great! Would be very cool to integrate the battery levels in my existing monitoring! |
Hopefully I have time soon to add my Group support back into this release. Great work! |
I pushed my latest changes and I think this PR has left the [WIP] status. I am looking forward for review comments and any kind of feedback. I own a remote controller and a motion sensor. If anybody owns a TRÅDFRI wireless dimmer, please contact me for testing it. |
I have both, a wireless remote control and a wireless dimmer. They are both discovered successfully and added to the inbox. After having them approved, the dimmer stayed offline at first, but after a restart into debug mode it then also worked instantly. So I have no idea what initially went wrong. |
Sounds good. Question is which thing-type is used for each device? I assume |
Yes, they were discovered as I agree, according to the ZLL standard Here is what they are discovered with:
I also don't see any significant difference apart from the |
Thanks for testing. I added a suitable check for the |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/tradfri-remote-support/34695/8 |
@afuechsel I definitely need some help with this merge conflict 😓. What should I do? 😄 |
@cweitkamp What an interesting conflict. :) Looks like the only one. I leave it up to you to solve the author's comment conflict :D |
…nly, battery level) Restructuring and refactoring of classes Moved some config classes to internal package Added property for NTP server Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
All, I removed the NTP features in this PR, to get this approach merged soon. I want to make space for other necessary changes. Reading and updating the NTP server configuration from the gateway is possible using the COAP interface, but currently leads to some not very nice issues. My approach was not good enough. I will keep on working on it in the future. |
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.
Thanks, looks pretty good, just some minor comments.
| Colour Temperature Light | 0x0220 | 0220 | | ||
| Extended Colour Light | 0x0210 | 0210 | | ||
| Occupancy Sensor Unit | 0x0107 | 0107 | | ||
| Non-Color Control Unit | 0x0820 | 0820 | |
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.
According to the spec referenced above, the name should be "Non-Colour Controller"
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.
In relation to the above linked document we maybe can use "Dimmer Switch" (0x0104) for this device as well.
| Dimmable Light | 0x0100 | 0100 | | ||
| Colour Temperature Light | 0x0220 | 0220 | | ||
| Extended Colour Light | 0x0210 | 0210 | | ||
| Occupancy Sensor Unit | 0x0107 | 0107 | |
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.
Where did you get the 0x0107 from? I cannot find it in the spec referenced above.
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.
see https://www.nxp.com/docs/en/user-guide/JN-UG-3114.pdf (page 71ff)
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.
Might be worth to update the link above then as well!
| Extended Colour Light | 0x0210 | 0210 | | ||
| Occupancy Sensor Unit | 0x0107 | 0107 | | ||
| Non-Color Control Unit | 0x0820 | 0820 | | ||
| Non-Color Scene Control Unit | 0x0830 | 0830 | |
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.
-> "Non-Colour Scene Controller"
return; | ||
} | ||
|
||
logger.warn("The controller is a read-only device and cannot handle commands."); |
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.
reduce to debug
return; | ||
} | ||
|
||
logger.warn("The sensor is a read-only device and cannot handle commands."); |
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.
reduce to debug
super.bridgeStatusChanged(bridgeStatusInfo); | ||
|
||
if (bridgeStatusInfo.getStatus() == ThingStatus.ONLINE) { | ||
scheduler.schedule(() -> { |
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.
you can use "submit" instead
@@ -105,6 +112,19 @@ public void onUpdate(String instanceId, JsonObject data) { | |||
thingType = THING_TYPE_DIMMABLE_LIGHT; | |||
} | |||
thingId = new ThingUID(thingType, bridge, Integer.toString(id)); | |||
// } else if (TYPE_SWITCH.equals(type) && data.has(SWITCH)) { |
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.
This can be removed as you are handling it below, right?
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
<bridge-type-ref id="gateway" /> | ||
</supported-bridge-type-refs> | ||
|
||
<label>Occupancy Sensor Unit</label> |
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.
Could you please also update the Thing types to match the README? (remove "Unit" on the new types)
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.
Oh, yes. Done.
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
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.
Thanks!
Good job, been looking forward to this! Thanks! |
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.
Any particular reason the TradfriGatewayConfig.java
to be moved to the internal package ?
I think it should be publicly accessible as it contains constants directly related to the Gateway thing type.
Sounds reasonable, but we should move those public constants into |
The TRÅDFRI controller and sensor devices currently cannot be observed directly using the coap protocol. We assume that they are communicating directly with the bulbs or lamps without routing their commands through the gateway. This makes it nearly impossible to trigger events or redirect any kind of information to ESH/OH2. Except using workarounds like defining TRÅDFRI groups (see PR #3799).
My approach adds the possibility to include those devices using the binding to the users environment and retrieve read-only data like
presence status
orbattery level
. I am happy to hear any feedback.abstract TradfriThingHandler
forTradfriControllerHandler
,TradfriLightHandler
andTradfriSensorHandler
ControllerData
,LightData
andSensorData
from handlers, move to own package (e.g.o.e.s.b.tradfri.internal.model
) and implement a reusableabstract TradfriData
classSigned-off-by: Christoph Weitkamp github@christophweitkamp.de