[LIFX] Add optional host configuration parameter, support new products and improvements

[wborn]

• Add host configuration parameter for lights that do not respond to UDP broadcasts (fixes [Lifx] direct IP address support #3913)
• Add support for new products (most notably the new Downlights)
• Add deviceId configuration parameter pattern to prevent mistakes/exceptions
• Fix static code analysis findings
• Makes the LIFX Binding periodically update network interface information. This for instance resolves the issue that network interfaces are not (or still) being used for broadcasts after they go up (or down).
• The code for this information was duplicated across the LifxLightDiscovery and LifxLightCommunicationHandler classes and has now been extracted into LifxNetworkUtil.
• All utility classes have now been grouped in the .util package.
• Update Thing status to configuration error when Device ID and Host are null
• Introduce logId property so logging identifies light depending on MAC/IP configuration
• Add logId where missing to simplify debugging
• Use lambdas for listener registration and iteratating over listeners
• Resolve @nonnull warnings
• Fix properties not always being updated
• Use a LifxLightContext for sharing common variables between handler helper objects
• Improve exception handling by catching specific exceptions and using more specific error messages

[kaikreuzer]

I am not clear on how the deviceId and the host parameter relate to each other. Which one is required for the handler in order to identify and communicate with a certain bulb? One of both? If so, what happens if both values are set (as in your example in the README), but do not correspond to the same light?

[wborn]

The binding always uses the deviceId to identify the light. The host parameter is only required for communications when the binding can not automatically discover the light using UDP broadcasts.

If so, what happens if both values are set (as in your example in the README), but do not correspond to the same light?

The binding ignores incoming packets that do not match the light MACAddress (deviceId). So in that scenario the light will remain offline. The binding will still send packets resulting from commands to the light.

Maybe we should add a helpful status description for this scenario? It should be possible to detect this.

[kaikreuzer]

The binding ignores incoming packets that do not match the light MACAddress (deviceId).

Do we know from which IP the packets originate from? If so, are there any real cases where the packet is NOT from the bulb at that IP and thus must be ignored? I know that in early times, LIFX bulbs were meant to act as a router to 6LoWPAN networks, but afaik this isn't true anymore and thus every bulb should have a unique IP - so if this is the case, the IP would indeed be the better configuration parameter (while the mac could be fully omitted).

[wborn]

Do we know from which IP the packets originate from? If so, are there any real cases where the packet is NOT from the bulb at that IP and thus must be ignored?

The IP of the light from which the packet originates is always known. Using just the IP as a configuration parameter only makes sense when people have a static IP configured for their lights. Otherwise the broadcast is needed to find the light and match it with the MAC.

LIFX bulbs were meant to act as a router to 6LoWPAN networks, but afaik this isn't true anymore and thus every bulb should have a unique IP

Lights no longer act as routers for other lights. It was not stable and thus removed. At the same time the V2 LAN protocol was introduced.

while the mac could be fully omitted

In theory the MAC could be removed as a configuration parameter for those people who run their lights with static IPs. The only issue with that is that most of the current code assumes the MAC is readily available and uses it for instance with logging and locking.

Another issue could be that the user should then configure either the MAC or the IP. Is it possible to define such a constraint within the <config-description>?

[kaikreuzer]

Another issue could be that the user should then configure either the MAC or the IP. Is it possible to define such a constraint within the ?

I think giving the choice (and not always requiring the MAC) would be the best option. Unfortunately, there isn't such a constraint for config descriptions. Probably the easiest way would be to declare both as optional and set the Thing status to CONFIGURATION_ERROR, if none of the two parameters is provided. Wdyt?

[wborn]

set the Thing status to CONFIGURATION_ERROR

Yes that would solve this issue.

After some more thought I think there will be other issues when there will be 2 properties (MAC or IP) that identify a Thing.

E.g. the <representation-property> can no longer be set to the MAC. The DiscoveryService will also not be able to tell if a Thing has a static or dynamic IP and thus which property should identify the Thing.

[kaikreuzer]

E.g. the can no longer be set to the MAC.

Why not? The MAC is the invariable identifier, so even if you find the bulb at 5 IP addresses, you can notice that it is the very same. So the MAC should stay as the representation-property.

DiscoveryService will also not be able to tell if a Thing has a static or dynamic IP and thus which property should identify the Thing.

The discovery can decide, which config parameter to use - and if it easily gets hold of the MAC, it should use it. Where is the problem?

[wborn]

Where is the problem?

Wouldn't discovery then create new discovery results (with MAC configuration parameters) when you have added all your lights using only IP configuration parameters?

A workaround could be to automatically update an empty MAC configuration parameter when a packet is received from the light.

[kaikreuzer]

Hm, good point. I was actually thinking that the MAC would be added as a property and thus the "representation-property" info would do the trick, but you are right that this isn't possible as the MAC is already a config param. So yes, setting this param by the handler might solve the issue.

only reviewed one of two commits

[wborn]

With the latest commit the deviceId (MAC) is now an optional property for lights @kaikreuzer. Since a lot of code (logging, throttling) was depending on the MAC, there are quite some code changes. I also made some additional improvements while I was at it.

[kaikreuzer]

Thanks, indeed huge code changes again (do you reimplement this binding every month? ;-)).
Hope you have thoroughly tested it, code looks good.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/lifx-binding-with-multiple-networks-scanning-wrong-subnet/66665/2