Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

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

Merged
merged 5 commits into from Sep 25, 2017
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
412d008
[LIFX] Add optional host configuration parameter, support new products
wborn Sep 9, 2017
37a425b
Make broadcastEnabled final
wborn Sep 10, 2017
6cf433e
Periodically update available network interface information
wborn Sep 22, 2017
2a173e0
Make Device ID an optional configuration paramater, use lambdas and f…
wborn Sep 23, 2017
c789bc9
Don't set host property on discovered lights
wborn Sep 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion extensions/binding/org.eclipse.smarthome.binding.lifx/ESH-INF/config/config.xml
View file
Open in desktop
Expand Up @@ -4,8 +4,14 @@
xsi:schemaLocation="http://eclipse.org/smarthome/schemas/config-description/v1.0.0 http://eclipse.org/smarthome/schemas/config-description-1.0.0.xsd">

<config-description uri="thing-type:lifx:light">
<parameter name="deviceId" type="text" required="true">
<parameter name="deviceId" type="text" pattern="([0-9A-Fa-f]{12})" required="true">
<label>LIFX device ID</label>
<description>Identifies the light e.g. "D073D5A1A1A1"</description>
</parameter>
<parameter name="host" type="text" required="false">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

<label>Host</label>
<description>Hostname or IP address of the light. Use empty value for automatic discovery.</description>
<context>network-address</context>
</parameter>
<parameter name="fadetime" type="integer" required="false">
<label>Fade time</label>
Expand Down
3 changes: 3 additions & 0 deletions extensions/binding/org.eclipse.smarthome.binding.lifx/ESH-INF/i18n/lifx.properties
View file
Open in desktop
Expand Up @@ -10,6 +10,9 @@ thing-type.lifx.whitelight.label = LIFX White Light

# thing type configuration
thing-type.config.lifx.light.deviceId.label = LIFX device ID
thing-type.config.lifx.light.deviceId.description = Identifies the light, e.g. "D073D5A1A1A1"
thing-type.config.lifx.light.host.label = Host
thing-type.config.lifx.light.host.description = Hostname or IP address of the light. Use empty value for automatic discovery.
thing-type.config.lifx.light.fadetime.label = Fade time
thing-type.config.lifx.light.fadetime.description = The time to fade to the new color value (in ms).

Expand Down
3 changes: 3 additions & 0 deletions extensions/binding/org.eclipse.smarthome.binding.lifx/ESH-INF/i18n/lifx_nl.properties
View file
Open in desktop
Expand Up @@ -10,6 +10,9 @@ thing-type.lifx.whitelight.label = LIFX Wittinten Lamp

# thing type configuration
thing-type.config.lifx.light.deviceId.label = LIFX apparaat ID
thing-type.config.lifx.light.deviceId.description = Identificeert de lamp, bv. "D073D5A1A1A1"
thing-type.config.lifx.light.host.label = Host
thing-type.config.lifx.light.host.description = Hostnaam of IP adres van de lamp. Gebruik lege waarde voor automatische detectie.
thing-type.config.lifx.light.fadetime.label = Vervagingsduur
thing-type.config.lifx.light.fadetime.description = De tijdsduur van het vervagen naar een nieuwe kleur (in ms).

Expand Down
26 changes: 16 additions & 10 deletions extensions/binding/org.eclipse.smarthome.binding.lifx/README.md
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@ The following table lists the thing types of the supported LIFX devices:
| Color 1000 BR30 | colorlight |
| LIFX A19 | colorlight |
| LIFX BR30 | colorlight |
| LIFX Downlight | colorlight |
| | |
| LIFX+ A19 | colorirlight |
| LIFX+ BR30 | colorirlight |
Expand Down Expand Up @@ -53,20 +54,25 @@ Thing lifx:colorlight:living [ deviceId="D073D5A1A1A1", fadetime=200 ]

The *fadetime* is an optional thing configuration parameter which configures the time to fade to a new color value (in ms). When the *fadetime* is not configured, the binding uses 300ms as default.

You can optionally also configure a fixed hostname or IP address when lights are in a different subnet and are not discovered.

```
Thing lifx:colorirlight:porch [ deviceId="D073D5B2B2B2", host="10.120.130.4", fadetime=0 ]
```

## Channels

All devices support some of the following channels:

| Channel Type ID | Item Type | Description | Thing Types |
|-----------------|-----------|--------------------------------------------------------------------------------------|----------------------------------------|
| brightness | Dimmer | This channel supports adjusting the brightness value. | whitelight |
| color | Color | This channel supports full color control with hue, saturation and brightness values. | colorlight, colorirlight, colormzlight |
| colorzone | Color | This channel supports full zone color control with hue, saturation and brightness values. | colormzlight |
| infrared | Dimmer | This channel supports adjusting the infrared value. *Note:* IR capable lights only activate their infrared LEDs when the brightness drops below a certain level. | colorirlight |
| signalstrength | Number | This channel represents signal strength with values 0, 1, 2, 3 or 4; 0 being worst strength and 4 being best strength. | colorlight, colorirlight, colormzlight, whitelight |
| temperature | Dimmer | This channel supports adjusting the color temperature from cold (0%) to warm (100%). | colorlight, colorirlight, colormzlight, whitelight |
| temperaturezone | Dimmer | This channel supports adjusting the zone color temperature from cold (0%) to warm (100%). | colormzlight |
| Channel Type ID | Item Type | Description | Thing Types |
|-----------------|-----------|------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------|
| brightness | Dimmer | This channel supports adjusting the brightness value. | whitelight |
| color | Color | This channel supports full color control with hue, saturation and brightness values. | colorlight, colorirlight, colormzlight |
| colorzone | Color | This channel supports full zone color control with hue, saturation and brightness values. | colormzlight |
| infrared | Dimmer | This channel supports adjusting the infrared value. *Note:* IR capable lights only activate their infrared LEDs when the brightness drops below a certain level. | colorirlight |
| signalstrength | Number | This channel represents signal strength with values 0, 1, 2, 3 or 4; 0 being worst strength and 4 being best strength. | colorlight, colorirlight, colormzlight, whitelight |
| temperature | Dimmer | This channel supports adjusting the color temperature from cold (0%) to warm (100%). | colorlight, colorirlight, colormzlight, whitelight |
| temperaturezone | Dimmer | This channel supports adjusting the zone color temperature from cold (0%) to warm (100%). | colormzlight |

The *color* and *brightness* channels have a "Power on brightness" configuration option that is used to determine the brightness when a light is switched on. When it is left empty, the brightness of a light remains unchanged when a light is switched on or off.

Expand Down Expand Up @@ -97,7 +103,7 @@ Thing lifx:colorlight:living2 [ deviceId="D073D5A2A2A2" ] {
Type color : color [ powerOnBrightness=50 ]
}

Thing lifx:colorirlight:porch [ deviceId="D073D5B2B2B2", fadetime=0 ] {
Thing lifx:colorirlight:porch [ deviceId="D073D5B2B2B2", host="10.120.130.4", fadetime=0 ] {
Channels:
Type color : color [ powerOnBrightness=75 ]
}
Expand Down
2 changes: 1 addition & 1 deletion extensions/binding/org.eclipse.smarthome.binding.lifx/about.html
View file
Open in desktop
Expand Up @@ -25,4 +25,4 @@ <h3>License</h3>
and such source code may be obtained at <a href="http://www.eclipse.org/">http://www.eclipse.org</a>.</p>

</body>
</html>
</html>
4 changes: 4 additions & 0 deletions ...e.binding.lifx/src/main/java/org/eclipse/smarthome/binding/lifx/LifxBindingConstants.java
View file
Open in desktop
Expand Up @@ -33,6 +33,10 @@ public class LifxBindingConstants {
// The LIFX LAN Protocol Specification states that lights can process up to 20 messages per second, not more.
public static final long PACKET_INTERVAL = 50;

// Port constants
public static final int BROADCAST_PORT = 56700;
public static final int UNICAST_PORT = 56700;

// Minimum and maximum of MultiZone light indices
public static final int MIN_ZONE_INDEX = 0;
public static final int MAX_ZONE_INDEX = 255;
Expand Down
23 changes: 10 additions & 13 deletions ...nding.lifx/src/main/java/org/eclipse/smarthome/binding/lifx/handler/LifxLightHandler.java
View file
Open in desktop
Expand Up @@ -10,7 +10,6 @@
import static org.eclipse.smarthome.binding.lifx.LifxBindingConstants.*;
import static org.eclipse.smarthome.binding.lifx.internal.LifxUtils.increaseDecreasePercentType;

import java.math.BigDecimal;
import java.time.Duration;
import java.time.LocalDateTime;
import java.util.ArrayList;
Expand All @@ -23,6 +22,7 @@
import org.eclipse.smarthome.binding.lifx.LifxBindingConstants;
import org.eclipse.smarthome.binding.lifx.internal.LifxChannelFactory;
import org.eclipse.smarthome.binding.lifx.internal.LifxLightCommunicationHandler;
import org.eclipse.smarthome.binding.lifx.internal.LifxLightConfig;
import org.eclipse.smarthome.binding.lifx.internal.LifxLightCurrentStateUpdater;
import org.eclipse.smarthome.binding.lifx.internal.LifxLightOnlineStateUpdater;
import org.eclipse.smarthome.binding.lifx.internal.LifxLightPropertiesUpdater;
Expand Down Expand Up @@ -72,14 +72,13 @@ public class LifxLightHandler extends BaseThingHandler implements LifxProperties

private final Logger logger = LoggerFactory.getLogger(LifxLightHandler.class);

private static final Duration FADE_TIME_DEFAULT = Duration.ofMillis(300);
private static final Duration MIN_STATUS_INFO_UPDATE_INTERVAL = Duration.ofSeconds(1);
private static final Duration MAX_STATE_CHANGE_DURATION = Duration.ofSeconds(4);

private final LifxChannelFactory channelFactory;
private Products product;

private Duration fadeTime = FADE_TIME_DEFAULT;
private Duration fadeTime;
private PercentType powerOnBrightness;

private MACAddress macAddress;
Expand Down Expand Up @@ -211,20 +210,25 @@ public void initialize() {
try {
lock.lock();

LifxLightConfig configuration = getConfigAs(LifxLightConfig.class);

product = getProduct();
macAddress = new MACAddress((String) getConfig().get(LifxBindingConstants.CONFIG_PROPERTY_DEVICE_ID), true);
macAddress = configuration.getMACAddress();
macAsHex = this.macAddress.getHex();

logger.debug("Initializing the LIFX handler for light '{}'.", macAsHex);

fadeTime = getFadeTime();
fadeTime = configuration.getFadeTime();
powerOnBrightness = getPowerOnBrightness();

channelStates = new HashMap<>();
currentLightState = new CurrentLightState();
pendingLightState = new LifxLightState();

communicationHandler = new LifxLightCommunicationHandler(macAddress, scheduler, currentLightState);
communicationHandler = configuration.hasHost()
? new LifxLightCommunicationHandler(macAddress, configuration.getHost(), scheduler,
currentLightState)
: new LifxLightCommunicationHandler(macAddress, scheduler, currentLightState);
currentStateUpdater = new LifxLightCurrentStateUpdater(macAddress, scheduler, currentLightState,
communicationHandler, product);
onlineStateUpdater = new LifxLightOnlineStateUpdater(macAddress, scheduler, currentLightState,
Expand Down Expand Up @@ -282,17 +286,11 @@ public void dispose() {

currentLightState = null;
pendingLightState = null;

} finally {
lock.unlock();
}
}

private Duration getFadeTime() {
BigDecimal fadeCfg = (BigDecimal) getConfig().get(LifxBindingConstants.CONFIG_PROPERTY_FADETIME);
return fadeCfg == null ? FADE_TIME_DEFAULT : Duration.ofMillis(fadeCfg.longValue());
}

private PercentType getPowerOnBrightness() {
Channel channel = null;

Expand Down Expand Up @@ -368,7 +366,6 @@ private void sendPacket(Packet packet) {

@Override
public void handleCommand(ChannelUID channelUID, Command command) {

if (command instanceof RefreshType) {
try {
switch (channelUID.getId()) {
Expand Down