Skip to content

Commit

Permalink
[lifx] Fix all SAT findings (openhab#10307)
Browse files Browse the repository at this point in the history
* Remove dependency on commons-lang classes
* Use HexUtils
* Fix/add missing initial contribution author lines
* Add unit test for MACAddress
* Rename protocol package to dto

Signed-off-by: Wouter Born <github@maindrain.net>
  • Loading branch information
wborn authored and computergeek1507 committed Jul 13, 2021
1 parent a2454d7 commit 4bdb399
Show file tree
Hide file tree
Showing 96 changed files with 348 additions and 306 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
/**
* The {@link LifxChannelFactory} creates dynamic LIFX channels.
*
* @author Wouter Born - Add i18n support
* @author Wouter Born - Initial contribution
*/
@NonNullByDefault
public interface LifxChannelFactory {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
/**
* The {@link LifxChannelFactoryImpl} creates dynamic LIFX channels.
*
* @author Wouter Born - Add i18n support
* @author Wouter Born - Initial contribution
*/
@NonNullByDefault
@Component(service = LifxChannelFactory.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.lifx.internal.dto.GetServiceRequest;
import org.openhab.binding.lifx.internal.dto.Packet;
import org.openhab.binding.lifx.internal.dto.StateServiceResponse;
import org.openhab.binding.lifx.internal.fields.MACAddress;
import org.openhab.binding.lifx.internal.handler.LifxLightHandler.CurrentLightState;
import org.openhab.binding.lifx.internal.listener.LifxResponsePacketListener;
import org.openhab.binding.lifx.internal.protocol.GetServiceRequest;
import org.openhab.binding.lifx.internal.protocol.Packet;
import org.openhab.binding.lifx.internal.protocol.StateServiceResponse;
import org.openhab.binding.lifx.internal.util.LifxNetworkUtil;
import org.openhab.binding.lifx.internal.util.LifxSelectorUtil;
import org.slf4j.Logger;
Expand All @@ -46,7 +46,7 @@
/**
* The {@link LifxLightCommunicationHandler} is responsible for the communications with a light.
*
* @author Wouter Born - Extracted class from LifxLightHandler
* @author Wouter Born - Initial contribution
*/
@NonNullByDefault
public class LifxLightCommunicationHandler {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class LifxLightConfig {

public @Nullable MACAddress getMACAddress() {
String localDeviceId = deviceId;
return localDeviceId == null ? null : new MACAddress(localDeviceId, true);
return localDeviceId == null ? null : new MACAddress(localDeviceId);
}

public @Nullable InetSocketAddress getHost() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.binding.lifx.internal.handler.LifxLightHandler;
import org.openhab.binding.lifx.internal.handler.LifxLightHandler.CurrentLightState;
import org.openhab.binding.lifx.internal.protocol.Product;

/**
* The {@link LifxLightContext} shares the context of a light with {@link LifxLightHandler} helper objects.
*
* @author Wouter Born - Add optional host configuration parameter
* @author Wouter Born - Initial contribution
*/
@NonNullByDefault
public class LifxLightContext {
Expand All @@ -31,10 +30,10 @@ public class LifxLightContext {
private final LifxLightConfig configuration;
private final CurrentLightState currentLightState;
private final LifxLightState pendingLightState;
private final Product product;
private final LifxProduct product;
private final ScheduledExecutorService scheduler;

public LifxLightContext(String logId, Product product, LifxLightConfig configuration,
public LifxLightContext(String logId, LifxProduct product, LifxLightConfig configuration,
CurrentLightState currentLightState, LifxLightState pendingLightState, ScheduledExecutorService scheduler) {
this.logId = logId;
this.configuration = configuration;
Expand All @@ -52,7 +51,7 @@ public LifxLightConfig getConfiguration() {
return configuration;
}

public Product getProduct() {
public LifxProduct getProduct() {
return product;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
package org.openhab.binding.lifx.internal;

import static org.openhab.binding.lifx.internal.LifxBindingConstants.MIN_ZONE_INDEX;
import static org.openhab.binding.lifx.internal.protocol.Product.Feature.*;
import static org.openhab.binding.lifx.internal.LifxProduct.Feature.*;
import static org.openhab.binding.lifx.internal.util.LifxMessageUtil.infraredToPercentType;

import java.util.concurrent.ScheduledExecutorService;
Expand All @@ -23,22 +23,21 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.lifx.internal.dto.GetColorZonesRequest;
import org.openhab.binding.lifx.internal.dto.GetLightInfraredRequest;
import org.openhab.binding.lifx.internal.dto.GetRequest;
import org.openhab.binding.lifx.internal.dto.GetTileEffectRequest;
import org.openhab.binding.lifx.internal.dto.GetWifiInfoRequest;
import org.openhab.binding.lifx.internal.dto.Packet;
import org.openhab.binding.lifx.internal.dto.StateLightInfraredResponse;
import org.openhab.binding.lifx.internal.dto.StateLightPowerResponse;
import org.openhab.binding.lifx.internal.dto.StateMultiZoneResponse;
import org.openhab.binding.lifx.internal.dto.StatePowerResponse;
import org.openhab.binding.lifx.internal.dto.StateResponse;
import org.openhab.binding.lifx.internal.dto.StateTileEffectResponse;
import org.openhab.binding.lifx.internal.dto.StateWifiInfoResponse;
import org.openhab.binding.lifx.internal.fields.HSBK;
import org.openhab.binding.lifx.internal.handler.LifxLightHandler.CurrentLightState;
import org.openhab.binding.lifx.internal.protocol.GetColorZonesRequest;
import org.openhab.binding.lifx.internal.protocol.GetLightInfraredRequest;
import org.openhab.binding.lifx.internal.protocol.GetRequest;
import org.openhab.binding.lifx.internal.protocol.GetTileEffectRequest;
import org.openhab.binding.lifx.internal.protocol.GetWifiInfoRequest;
import org.openhab.binding.lifx.internal.protocol.Packet;
import org.openhab.binding.lifx.internal.protocol.Product;
import org.openhab.binding.lifx.internal.protocol.StateLightInfraredResponse;
import org.openhab.binding.lifx.internal.protocol.StateLightPowerResponse;
import org.openhab.binding.lifx.internal.protocol.StateMultiZoneResponse;
import org.openhab.binding.lifx.internal.protocol.StatePowerResponse;
import org.openhab.binding.lifx.internal.protocol.StateResponse;
import org.openhab.binding.lifx.internal.protocol.StateTileEffectResponse;
import org.openhab.binding.lifx.internal.protocol.StateWifiInfoResponse;
import org.openhab.core.library.types.PercentType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -47,7 +46,7 @@
* The {@link LifxLightCurrentStateUpdater} sends packets to a light in order to update the {@code currentLightState} to
* the actual light state.
*
* @author Wouter Born - Extracted class from LifxLightHandler
* @author Wouter Born - Initial contribution
*/
@NonNullByDefault
public class LifxLightCurrentStateUpdater {
Expand All @@ -57,7 +56,7 @@ public class LifxLightCurrentStateUpdater {
private final Logger logger = LoggerFactory.getLogger(LifxLightCurrentStateUpdater.class);

private final String logId;
private final Product product;
private final LifxProduct product;
private final CurrentLightState currentLightState;
private final ScheduledExecutorService scheduler;
private final LifxLightCommunicationHandler communicationHandler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,16 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;

import org.apache.commons.lang.StringUtils;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.lifx.internal.dto.GetLabelRequest;
import org.openhab.binding.lifx.internal.dto.GetServiceRequest;
import org.openhab.binding.lifx.internal.dto.GetVersionRequest;
import org.openhab.binding.lifx.internal.dto.Packet;
import org.openhab.binding.lifx.internal.dto.StateLabelResponse;
import org.openhab.binding.lifx.internal.dto.StateServiceResponse;
import org.openhab.binding.lifx.internal.dto.StateVersionResponse;
import org.openhab.binding.lifx.internal.fields.MACAddress;
import org.openhab.binding.lifx.internal.protocol.GetLabelRequest;
import org.openhab.binding.lifx.internal.protocol.GetServiceRequest;
import org.openhab.binding.lifx.internal.protocol.GetVersionRequest;
import org.openhab.binding.lifx.internal.protocol.Packet;
import org.openhab.binding.lifx.internal.protocol.Product;
import org.openhab.binding.lifx.internal.protocol.StateLabelResponse;
import org.openhab.binding.lifx.internal.protocol.StateServiceResponse;
import org.openhab.binding.lifx.internal.protocol.StateVersionResponse;
import org.openhab.binding.lifx.internal.util.LifxSelectorUtil;
import org.openhab.core.config.discovery.AbstractDiscoveryService;
import org.openhab.core.config.discovery.DiscoveryResult;
Expand Down Expand Up @@ -87,7 +85,7 @@ private class DiscoveredLight {
private InetSocketAddress socketAddress;
private String logId;
private @Nullable String label;
private @Nullable Product product;
private @Nullable LifxProduct product;
private long productVersion;
private boolean supportedProduct = true;
private LifxSelectorContext selectorContext;
Expand Down Expand Up @@ -300,7 +298,8 @@ private void handlePacket(Packet packet, InetSocketAddress address) {
light.label = ((StateLabelResponse) packet).getLabel().trim();
} else if (packet instanceof StateVersionResponse) {
try {
light.product = Product.getProductFromProductID(((StateVersionResponse) packet).getProduct());
light.product = LifxProduct
.getProductFromProductID(((StateVersionResponse) packet).getProduct());
light.productVersion = ((StateVersionResponse) packet).getVersion();
} catch (IllegalArgumentException e) {
logger.debug("Discovered an unsupported light ({}): {}", light.macAddress.getAsLabel(),
Expand All @@ -322,7 +321,7 @@ private void handlePacket(Packet packet, InetSocketAddress address) {
}

private DiscoveryResult createDiscoveryResult(DiscoveredLight light) throws IllegalArgumentException {
Product product = light.product;
LifxProduct product = light.product;
if (product == null) {
throw new IllegalArgumentException("Product of discovered light is null");
}
Expand All @@ -331,7 +330,7 @@ private DiscoveryResult createDiscoveryResult(DiscoveredLight light) throws Ille
ThingUID thingUID = new ThingUID(product.getThingTypeUID(), macAsLabel);

String label = light.label;
if (StringUtils.isBlank(label)) {
if (label == null || label.isBlank()) {
label = product.getName();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,17 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.lifx.internal.dto.GetEchoRequest;
import org.openhab.binding.lifx.internal.dto.GetServiceRequest;
import org.openhab.binding.lifx.internal.dto.Packet;
import org.openhab.binding.lifx.internal.handler.LifxLightHandler.CurrentLightState;
import org.openhab.binding.lifx.internal.protocol.GetEchoRequest;
import org.openhab.binding.lifx.internal.protocol.GetServiceRequest;
import org.openhab.binding.lifx.internal.protocol.Packet;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* The {@link LifxLightOnlineStateUpdater} sets the state of a light offline when it no longer responds to echo packets.
*
* @author Wouter Born - Extracted class from LifxLightHandler
* @author Wouter Born - Initial contribution
*/
@NonNullByDefault
public class LifxLightOnlineStateUpdater {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,24 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.lifx.internal.dto.GetHostFirmwareRequest;
import org.openhab.binding.lifx.internal.dto.GetVersionRequest;
import org.openhab.binding.lifx.internal.dto.GetWifiFirmwareRequest;
import org.openhab.binding.lifx.internal.dto.Packet;
import org.openhab.binding.lifx.internal.dto.StateHostFirmwareResponse;
import org.openhab.binding.lifx.internal.dto.StateVersionResponse;
import org.openhab.binding.lifx.internal.dto.StateWifiFirmwareResponse;
import org.openhab.binding.lifx.internal.fields.MACAddress;
import org.openhab.binding.lifx.internal.handler.LifxLightHandler.CurrentLightState;
import org.openhab.binding.lifx.internal.listener.LifxPropertiesUpdateListener;
import org.openhab.binding.lifx.internal.protocol.GetHostFirmwareRequest;
import org.openhab.binding.lifx.internal.protocol.GetVersionRequest;
import org.openhab.binding.lifx.internal.protocol.GetWifiFirmwareRequest;
import org.openhab.binding.lifx.internal.protocol.Packet;
import org.openhab.binding.lifx.internal.protocol.Product;
import org.openhab.binding.lifx.internal.protocol.StateHostFirmwareResponse;
import org.openhab.binding.lifx.internal.protocol.StateVersionResponse;
import org.openhab.binding.lifx.internal.protocol.StateWifiFirmwareResponse;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* The {@link LifxLightPropertiesUpdater} updates the light properties when a light goes online. When packets get lost
* the requests are resent when the {@code UPDATE_INTERVAL} elapses.
*
* @author Wouter Born - Update light properties when online
* @author Wouter Born - Initial contribution
*/
@NonNullByDefault
public class LifxLightPropertiesUpdater {
Expand Down Expand Up @@ -157,7 +156,7 @@ public void handleResponsePacket(Packet packet) {
properties.put(LifxBindingConstants.PROPERTY_PRODUCT_VERSION, Long.toString(productVersion));

try {
Product product = Product.getProductFromProductID(productId);
LifxProduct product = LifxProduct.getProductFromProductID(productId);
properties.put(LifxBindingConstants.PROPERTY_PRODUCT_NAME, product.getName());
properties.put(LifxBindingConstants.PROPERTY_VENDOR_ID, Long.toString(product.getVendor().getID()));
properties.put(LifxBindingConstants.PROPERTY_VENDOR_NAME, product.getVendor().getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,19 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.lifx.internal.dto.Effect;
import org.openhab.binding.lifx.internal.dto.PowerState;
import org.openhab.binding.lifx.internal.dto.SignalStrength;
import org.openhab.binding.lifx.internal.fields.HSBK;
import org.openhab.binding.lifx.internal.listener.LifxLightStateListener;
import org.openhab.binding.lifx.internal.protocol.Effect;
import org.openhab.binding.lifx.internal.protocol.PowerState;
import org.openhab.binding.lifx.internal.protocol.SignalStrength;
import org.openhab.core.library.types.HSBType;
import org.openhab.core.library.types.OnOffType;
import org.openhab.core.library.types.PercentType;

/**
* The {@link LifxLightState} stores the properties that represent the state of a light.
*
* @author Wouter Born - Extracted class from LifxLightHandler, added listener logic
* @author Wouter Born - Initial contribution
*/
@NonNullByDefault
public class LifxLightState {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
package org.openhab.binding.lifx.internal;

import static org.openhab.binding.lifx.internal.LifxBindingConstants.PACKET_INTERVAL;
import static org.openhab.binding.lifx.internal.protocol.Product.Feature.MULTIZONE;
import static org.openhab.binding.lifx.internal.LifxProduct.Feature.MULTIZONE;
import static org.openhab.binding.lifx.internal.util.LifxMessageUtil.*;

import java.time.Duration;
Expand All @@ -29,25 +29,24 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.lifx.internal.dto.AcknowledgementResponse;
import org.openhab.binding.lifx.internal.dto.ApplicationRequest;
import org.openhab.binding.lifx.internal.dto.Effect;
import org.openhab.binding.lifx.internal.dto.GetColorZonesRequest;
import org.openhab.binding.lifx.internal.dto.GetLightInfraredRequest;
import org.openhab.binding.lifx.internal.dto.GetLightPowerRequest;
import org.openhab.binding.lifx.internal.dto.GetRequest;
import org.openhab.binding.lifx.internal.dto.Packet;
import org.openhab.binding.lifx.internal.dto.PowerState;
import org.openhab.binding.lifx.internal.dto.SetColorRequest;
import org.openhab.binding.lifx.internal.dto.SetColorZonesRequest;
import org.openhab.binding.lifx.internal.dto.SetLightInfraredRequest;
import org.openhab.binding.lifx.internal.dto.SetLightPowerRequest;
import org.openhab.binding.lifx.internal.dto.SetPowerRequest;
import org.openhab.binding.lifx.internal.dto.SetTileEffectRequest;
import org.openhab.binding.lifx.internal.dto.SignalStrength;
import org.openhab.binding.lifx.internal.fields.HSBK;
import org.openhab.binding.lifx.internal.listener.LifxLightStateListener;
import org.openhab.binding.lifx.internal.protocol.AcknowledgementResponse;
import org.openhab.binding.lifx.internal.protocol.ApplicationRequest;
import org.openhab.binding.lifx.internal.protocol.Effect;
import org.openhab.binding.lifx.internal.protocol.GetColorZonesRequest;
import org.openhab.binding.lifx.internal.protocol.GetLightInfraredRequest;
import org.openhab.binding.lifx.internal.protocol.GetLightPowerRequest;
import org.openhab.binding.lifx.internal.protocol.GetRequest;
import org.openhab.binding.lifx.internal.protocol.Packet;
import org.openhab.binding.lifx.internal.protocol.PowerState;
import org.openhab.binding.lifx.internal.protocol.Product;
import org.openhab.binding.lifx.internal.protocol.SetColorRequest;
import org.openhab.binding.lifx.internal.protocol.SetColorZonesRequest;
import org.openhab.binding.lifx.internal.protocol.SetLightInfraredRequest;
import org.openhab.binding.lifx.internal.protocol.SetLightPowerRequest;
import org.openhab.binding.lifx.internal.protocol.SetPowerRequest;
import org.openhab.binding.lifx.internal.protocol.SetTileEffectRequest;
import org.openhab.binding.lifx.internal.protocol.SignalStrength;
import org.openhab.core.library.types.PercentType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -57,7 +56,7 @@
* light so the change the actual light state to that of the {@code pendingLightState}. When the light does not
* acknowledge a packet, it resends it (max 3 times).
*
* @author Wouter Born - Extracted class from LifxLightHandler, added logic for handling packet loss
* @author Wouter Born - Initial contribution
*/
@NonNullByDefault
public class LifxLightStateChanger implements LifxLightStateListener {
Expand All @@ -75,7 +74,7 @@ public class LifxLightStateChanger implements LifxLightStateListener {
private final Logger logger = LoggerFactory.getLogger(LifxLightStateChanger.class);

private final String logId;
private final Product product;
private final LifxProduct product;
private final Duration fadeTime;
private final LifxLightState pendingLightState;
private final ScheduledExecutorService scheduler;
Expand Down
Loading

0 comments on commit 4bdb399

Please sign in to comment.