diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/command/DebugCommand.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/command/DebugCommand.java index d35e495b61c30..4e899fed3b204 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/command/DebugCommand.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/command/DebugCommand.java @@ -33,10 +33,10 @@ import org.openhab.binding.insteon.internal.device.X10Address; import org.openhab.binding.insteon.internal.device.X10Device; import org.openhab.binding.insteon.internal.transport.PortListener; +import org.openhab.binding.insteon.internal.transport.message.Direction; import org.openhab.binding.insteon.internal.transport.message.FieldException; import org.openhab.binding.insteon.internal.transport.message.InvalidMessageTypeException; import org.openhab.binding.insteon.internal.transport.message.Msg; -import org.openhab.binding.insteon.internal.transport.message.Msg.Direction; import org.openhab.binding.insteon.internal.transport.message.MsgDefinitionRegistry; import org.openhab.binding.insteon.internal.utils.HexUtils; import org.openhab.core.io.console.Console; diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonModem.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonModem.java index b41212d553a21..d3328d8cd8e74 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonModem.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonModem.java @@ -245,7 +245,7 @@ private void handleModemInfo(Msg msg) throws FieldException { int deviceCategory = msg.getInt("DeviceCategory"); int subCategory = msg.getInt("DeviceSubCategory"); - ProductData productData = ProductDataRegistry.getInstance().getProductData(deviceCategory, subCategory); + ProductData productData = ProductData.makeInsteonProduct(deviceCategory, subCategory); productData.setFirmwareVersion(msg.getInt("FirmwareVersion")); DeviceType deviceType = productData.getDeviceType(); diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonScene.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonScene.java index 33c19aab860e8..e7810eeca7f0b 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonScene.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonScene.java @@ -87,9 +87,9 @@ public List getFeatures(InsteonAddress address) { } public State getState() { - return getEntries().stream().allMatch(entry -> entry.getState() == UnDefType.NULL) ? UnDefType.NULL - : OnOffType.from(getEntries().stream().filter(entry -> entry.getState() != UnDefType.NULL) - .allMatch(entry -> entry.getState().equals(entry.getOnState()))); + return getEntries().stream().noneMatch(SceneEntry::isStateDefined) ? UnDefType.NULL + : OnOffType + .from(getEntries().stream().filter(SceneEntry::isStateDefined).allMatch(SceneEntry::isStateOn)); } public boolean hasEntry(InsteonAddress address) { @@ -354,6 +354,14 @@ public State getState() { return state; } + public boolean isStateDefined() { + return !UnDefType.NULL.equals(state); + } + + public boolean isStateOn() { + return getOnState().equals(state); + } + public void setState(State state) { this.state = state; } diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/ProductData.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/ProductData.java index 358c08328b670..39baf5d856724 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/ProductData.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/ProductData.java @@ -231,21 +231,9 @@ public static ProductData makeInsteonProduct(int deviceCategory, int subCategory ProductData productData = new ProductData(); productData.setDeviceCategory(deviceCategory); productData.setSubCategory(subCategory); - return productData; - } - - /** - * Factory method for creating a ProductData for an Insteon product - * - * @param deviceCategory the Insteon device category - * @param subCategory the Insteon device subcategory - * @param srcData the source product data to use - * @return the product data - */ - public static ProductData makeInsteonProduct(int deviceCategory, int subCategory, @Nullable ProductData srcData) { - ProductData productData = makeInsteonProduct(deviceCategory, subCategory); - if (srcData != null) { - productData.update(srcData); + ProductData resourceData = ProductDataRegistry.getInstance().getProductData(deviceCategory, subCategory); + if (resourceData != null) { + productData.update(resourceData); } return productData; } diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/ProductDataRegistry.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/ProductDataRegistry.java index 6c2a7e1837073..ef201b4230efa 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/ProductDataRegistry.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/ProductDataRegistry.java @@ -45,9 +45,9 @@ private ProductDataRegistry() { * * @param deviceCategory device category to match * @param subCategory device subcategory to match - * @return product data matching provided parameters + * @return product data if matching provided parameters, otherwise null */ - public ProductData getProductData(int deviceCategory, int subCategory) { + public @Nullable ProductData getProductData(int deviceCategory, int subCategory) { int productId = getProductId(deviceCategory, subCategory); if (!products.containsKey(productId)) { logger.warn("unknown product for devCat:{} subCat:{} in device products xml file", @@ -55,19 +55,7 @@ public ProductData getProductData(int deviceCategory, int subCategory) { // fallback to matching product id using device category only productId = getProductId(deviceCategory, ProductData.SUB_CATEGORY_UNKNOWN); } - - return ProductData.makeInsteonProduct(deviceCategory, subCategory, products.get(productId)); - } - - /** - * Returns the device type for a given dev/sub category - * - * @param deviceCategory device category to match - * @param subCategory device subcategory to match - * @return device type matching provided parameters - */ - public @Nullable DeviceType getDeviceType(int deviceCategory, int subCategory) { - return getProductData(deviceCategory, subCategory).getDeviceType(); + return products.get(productId); } /** @@ -143,7 +131,9 @@ private void parseProduct(Element element) throws SAXException { logger.warn("overwriting previous definition of product {}", products.get(productId)); } - ProductData productData = ProductData.makeInsteonProduct(deviceCategory, subCategory); + ProductData productData = new ProductData(); + productData.setDeviceCategory(deviceCategory); + productData.setSubCategory(subCategory); productData.setProductKey(productKey); productData.setFirstRecordLocation(firstRecord); diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/RampRate.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/RampRate.java index 7a9079ae1d6d4..d622162b70901 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/RampRate.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/RampRate.java @@ -17,6 +17,7 @@ import java.text.DecimalFormat; import java.util.Arrays; import java.util.Comparator; +import java.util.List; import java.util.Map; import java.util.function.Function; import java.util.stream.Collectors; @@ -64,6 +65,8 @@ public enum RampRate { SEC_0_2(0x1E, 0.2), INSTANT(0x1F, 0.1); + private static final List SUPPORTED_FEATURE_TYPES = List.of(FEATURE_TYPE_GENERIC_DIMMER); + private static final Map VALUE_MAP = Arrays.stream(values()) .collect(Collectors.toUnmodifiableMap(rate -> rate.value, Function.identity())); @@ -105,7 +108,7 @@ public String toString() { * @return true if supported */ public static boolean supportsFeatureType(String featureType) { - return FEATURE_TYPE_GENERIC_DIMMER.equals(featureType); + return SUPPORTED_FEATURE_TYPES.contains(featureType); } /** diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/database/ModemDBReader.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/database/ModemDBReader.java index e748c24cb604d..07c9e4aa6cd7b 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/database/ModemDBReader.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/database/ModemDBReader.java @@ -24,7 +24,6 @@ import org.openhab.binding.insteon.internal.device.InsteonAddress; import org.openhab.binding.insteon.internal.device.InsteonModem; import org.openhab.binding.insteon.internal.device.ProductData; -import org.openhab.binding.insteon.internal.device.ProductDataRegistry; import org.openhab.binding.insteon.internal.transport.PortListener; import org.openhab.binding.insteon.internal.transport.message.FieldException; import org.openhab.binding.insteon.internal.transport.message.InvalidMessageTypeException; @@ -290,7 +289,7 @@ private void handleProductData(Msg msg) throws FieldException { int subCategory = Byte.toUnsignedInt(toAddr.getMiddleByte()); int firmware = Byte.toUnsignedInt(toAddr.getLowByte()); int hardware = msg.getInt("command2"); - ProductData productData = ProductDataRegistry.getInstance().getProductData(deviceCategory, subCategory); + ProductData productData = ProductData.makeInsteonProduct(deviceCategory, subCategory); productData.setFirmwareVersion(firmware); productData.setHardwareVersion(hardware); // set product data if in modem db diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/discovery/InsteonDiscoveryService.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/discovery/InsteonDiscoveryService.java index b3af9b20fa348..8009e9dab416b 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/discovery/InsteonDiscoveryService.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/discovery/InsteonDiscoveryService.java @@ -62,8 +62,7 @@ protected void startScan() { public void discoverInsteonDevice(InsteonAddress address, @Nullable ProductData productData) { InsteonModem modem = handler.getModem(); - if (handler.isDeviceDiscoveryEnabled() && modem != null && modem.getDB().hasEntry(address) - && !modem.hasDevice(address)) { + if (modem != null && modem.getDB().hasEntry(address) && !modem.hasDevice(address)) { addInsteonDevice(address, productData); } else { removeInsteonDevice(address); @@ -72,8 +71,7 @@ public void discoverInsteonDevice(InsteonAddress address, @Nullable ProductData public void discoverInsteonScene(int group) { InsteonModem modem = handler.getModem(); - if (handler.isSceneDiscoveryEnabled() && modem != null && modem.getDB().hasBroadcastGroup(group) - && !modem.hasScene(group)) { + if (modem != null && modem.getDB().hasBroadcastGroup(group) && !modem.hasScene(group)) { addInsteonScene(group); } else { removeInsteonScene(group); diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonBridgeHandler.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonBridgeHandler.java index 6e8f63e29203d..e3975e9ceb908 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonBridgeHandler.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/handler/InsteonBridgeHandler.java @@ -284,14 +284,14 @@ private void cleanUpStorage(InsteonAddress address) { protected void discoverInsteonDevice(InsteonAddress address, @Nullable ProductData productData) { InsteonDiscoveryService discoveryService = getDiscoveryService(); - if (discoveryService != null) { + if (discoveryService != null && isDeviceDiscoveryEnabled()) { scheduler.execute(() -> discoveryService.discoverInsteonDevice(address, productData)); } } protected void discoverInsteonScene(int group) { InsteonDiscoveryService discoveryService = getDiscoveryService(); - if (discoveryService != null) { + if (discoveryService != null && isSceneDiscoveryEnabled()) { scheduler.execute(() -> discoveryService.discoverInsteonScene(group)); } } diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/DataType.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/DataType.java index 3b6d753debe82..c0333e763a73c 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/DataType.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/DataType.java @@ -18,6 +18,7 @@ import java.util.stream.Collectors; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; /** * Defines the data types that can be used in the fields of a message. @@ -29,8 +30,7 @@ @NonNullByDefault public enum DataType { BYTE("byte", 1), - ADDRESS("address", 3), - INVALID("invalid", -1); + ADDRESS("address", 3); private static final Map NAME_MAP = Arrays.stream(values()) .collect(Collectors.toUnmodifiableMap(type -> type.name, Function.identity())); @@ -55,9 +55,9 @@ public int getSize() { * Factory method for getting a DataType from the data type name * * @param name the data type name - * @return the data type + * @return the data type if defined, otherwise null */ - public static DataType get(String name) { - return NAME_MAP.getOrDefault(name, DataType.INVALID); + public static @Nullable DataType get(String name) { + return NAME_MAP.get(name); } } diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Direction.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Direction.java new file mode 100644 index 0000000000000..b27c43b460fd9 --- /dev/null +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Direction.java @@ -0,0 +1,46 @@ +/** + * Copyright (c) 2010-2024 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.binding.insteon.internal.transport.message; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; + +/** + * The {@link Direction} represents a message direction + * + * @author Jeremy Setton - Initial contribution + */ +@NonNullByDefault +public enum Direction { + FROM_MODEM("IN"), + TO_MODEM("OUT"); + + private final String label; + + private Direction(String label) { + this.label = label; + } + + @Override + public String toString() { + return label; + } + + public static @Nullable Direction get(String value) { + try { + return Direction.valueOf(value); + } catch (IllegalArgumentException e) { + return null; + } + } +} diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Field.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Field.java index 2fbf6ee28f27d..447a0dd082777 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Field.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Field.java @@ -34,6 +34,12 @@ public final class Field { private final int offset; private final DataType type; + public Field(String name, int offset, DataType type) { + this.name = name; + this.offset = offset; + this.type = type; + } + public String getName() { return name; } @@ -42,16 +48,6 @@ public int getOffset() { return offset; } - public DataType getType() { - return type; - } - - public Field(String name, DataType type, int offset) { - this.name = name; - this.type = type; - this.offset = offset; - } - private void check(int len, DataType t) throws FieldException { if (offset + type.getSize() > len) { throw new FieldException("field write beyond end of msg"); @@ -61,16 +57,16 @@ private void check(int len, DataType t) throws FieldException { } } - public void set(byte[] data, Object o) throws FieldException { + public void set(byte[] data, String value) throws FieldException, IllegalArgumentException { switch (type) { case BYTE: - setByte(data, (Byte) o); + byte b = value.isEmpty() ? 0x00 : (byte) HexUtils.toInteger(value); + setByte(data, b); break; case ADDRESS: - setAddress(data, (InsteonAddress) o); + InsteonAddress address = value.isEmpty() ? InsteonAddress.UNKNOWN : new InsteonAddress(value); + setAddress(data, address); break; - default: - throw new FieldException("field data type unknown"); } } @@ -141,8 +137,6 @@ public String toString(byte[] data) { case ADDRESS: s += getAddress(data).toString(); break; - default: - throw new FieldException("field data type unknown"); } } catch (FieldException e) { s += "NULL"; diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Msg.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Msg.java index 4ca24bce91d35..25d64a2561962 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Msg.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/Msg.java @@ -38,38 +38,21 @@ */ @NonNullByDefault public class Msg { - - public static enum Direction { - TO_MODEM, - FROM_MODEM - } - private final Logger logger = LoggerFactory.getLogger(Msg.class); - private byte[] data; - private int headerLength; - private Direction direction; - private MsgDefinition definition = new MsgDefinition(); + private final byte[] data; + private final MsgDefinition definition; private long quietTime = 0; private boolean replayed = false; private long timestamp = System.currentTimeMillis(); - public Msg(int headerLength, int dataLength, Direction direction) { - this.data = new byte[dataLength]; - this.headerLength = headerLength; - this.direction = direction; - } - - public Msg(Msg msg, byte[] data, int dataLength) { - this.data = Arrays.copyOf(data, dataLength); - this.headerLength = msg.headerLength; - this.direction = msg.direction; - // the message definition usually doesn't change, but just to be sure... - this.definition = new MsgDefinition(msg.definition); + public Msg(byte[] data, MsgDefinition definition) { + this.data = Arrays.copyOf(data, definition.getLength()); + this.definition = definition; } - public Msg(Msg msg) { - this(msg, msg.data, msg.data.length); + public Msg(MsgDefinition definition) { + this(definition.getData(), definition); } public byte[] getData() { @@ -81,15 +64,11 @@ public int getLength() { } public int getHeaderLength() { - return headerLength; + return definition.getHeaderLength(); } public Direction getDirection() { - return direction; - } - - public MsgDefinition getDefinition() { - return definition; + return definition.getDirection(); } public long getQuietTime() { @@ -129,11 +108,11 @@ public boolean isFromAddress(@Nullable InsteonAddress address) { } public boolean isInbound() { - return direction == Direction.FROM_MODEM; + return getDirection() == Direction.FROM_MODEM; } public boolean isOutbound() { - return direction == Direction.TO_MODEM; + return getDirection() == Direction.TO_MODEM; } public boolean isEcho() { @@ -244,10 +223,6 @@ public boolean isReplayed() { return replayed; } - public void setDefinition(MsgDefinition definition) { - this.definition = definition; - } - public void setQuietTime(long quietTime) { this.quietTime = quietTime; } @@ -256,10 +231,6 @@ public void setIsReplayed(boolean replayed) { this.replayed = replayed; } - public void addField(Field f) { - definition.addField(f); - } - public boolean containsField(String key) { return definition.containsField(key); } @@ -283,12 +254,11 @@ public int getMaxHops() { /** * Sets a byte at a specific field * - * @param key the string key in the message definition + * @param key the name of the field * @param value the byte to put */ public void setByte(String key, byte value) throws FieldException { - Field field = definition.getField(key); - field.setByte(data, value); + definition.getField(key).setByte(data, value); } /** @@ -443,27 +413,6 @@ public int getInt32(String key) throws FieldException { return getInt(key, 4); } - /** - * Returns a byte as a hex string - * - * @param key the name of the field - * @return the hex string - */ - public String getHexString(String key) throws FieldException { - return HexUtils.getHexString(getByte(key)); - } - - /** - * Returns a byte array starting from a certain field as a hex string - * - * @param key the name of the field - * @param numBytes number of bytes to get - * @return the hex string - */ - public String getHexString(String key, int numBytes) throws FieldException { - return HexUtils.getHexString(getBytes(key, numBytes), numBytes); - } - /** * Returns group based on specific message characteristics * @@ -644,7 +593,7 @@ public int hashCode() { @Override public String toString() { - String s = (direction == Direction.TO_MODEM) ? "OUT:" : "IN:"; + String s = definition.getDirection() + ":"; for (Field field : definition.getFields()) { if ("messageFlags".equals(field.getName())) { s += field.toString(data) + "=" + getType() + ":" + getHopsLeft() + ":" + getMaxHops() + "|"; @@ -668,8 +617,8 @@ public String toString() { return null; } return Optional - .ofNullable(MsgDefinitionRegistry.getInstance().getTemplate(buf[1], isExtended, Direction.FROM_MODEM)) - .filter(template -> template.getLength() == msgLen).map(template -> new Msg(template, buf, msgLen)) + .ofNullable(MsgDefinitionRegistry.getInstance().getDefinition(buf[1], isExtended, Direction.FROM_MODEM)) + .filter(definition -> definition.getLength() == msgLen).map(definition -> new Msg(buf, definition)) .orElse(null); } @@ -678,10 +627,12 @@ public String toString() { * * @param cmd the message command received * @return the length of the header to expect + * @throws InvalidMessageTypeException */ - public static int getHeaderLength(byte cmd) { - return Optional.ofNullable(MsgDefinitionRegistry.getInstance().getTemplate(cmd, Direction.FROM_MODEM)) - .map(Msg::getHeaderLength).orElse(-1); + public static int getHeaderLength(byte cmd) throws InvalidMessageTypeException { + return Optional.ofNullable(MsgDefinitionRegistry.getInstance().getDefinition(cmd, Direction.FROM_MODEM)) + .map(MsgDefinition::getHeaderLength) + .orElseThrow(() -> new InvalidMessageTypeException("unknown message command")); } /** @@ -690,32 +641,27 @@ public static int getHeaderLength(byte cmd) { * @param cmd the message command received * @param isExtended if is an extended message * @return message length, or -1 if length cannot be determined + * @throws InvalidMessageTypeException */ - public static int getMessageLength(byte cmd, boolean isExtended) { + public static int getMessageLength(byte cmd, boolean isExtended) throws InvalidMessageTypeException { return Optional - .ofNullable(MsgDefinitionRegistry.getInstance().getTemplate(cmd, isExtended, Direction.FROM_MODEM)) - .map(Msg::getLength).orElse(-1); + .ofNullable(MsgDefinitionRegistry.getInstance().getDefinition(cmd, isExtended, Direction.FROM_MODEM)) + .map(MsgDefinition::getLength) + .orElseThrow(() -> new InvalidMessageTypeException("unknown message command")); } /** * Factory method to determine if a message is extended * * @param buf the received bytes - * @param len the number of bytes received so far - * @param headerLength the known length of the header - * @return true if it is definitely extended, false if cannot be - * determined or if it is a standard message + * @param headerLength the header length + * @return true if is extended */ - public static boolean isExtended(byte[] buf, int len, int headerLength) { - if (headerLength <= 2) { + public static boolean isExtended(byte[] buf, int headerLength) { + if (headerLength <= 2 || buf.length < headerLength) { return false; - } // extended messages are longer - if (len < headerLength) { - return false; - } // not enough data to tell if extended - byte flags = buf[headerLength - 1]; // last byte says flags - boolean isExtended = BinaryUtils.isBitSet(flags & 0xFF, 4); - return isExtended; + } + return BinaryUtils.isBitSet(buf[headerLength - 1] & 0xFF, 4); } /** @@ -726,7 +672,7 @@ public static boolean isExtended(byte[] buf, int len, int headerLength) { * @throws InvalidMessageTypeException */ public static Msg makeMessage(byte cmd) throws InvalidMessageTypeException { - return Optional.ofNullable(MsgDefinitionRegistry.getInstance().getTemplate(cmd, Direction.TO_MODEM)) + return Optional.ofNullable(MsgDefinitionRegistry.getInstance().getDefinition(cmd, Direction.TO_MODEM)) .map(Msg::new).orElseThrow(() -> new InvalidMessageTypeException( "unknown message command: " + HexUtils.getHexString(cmd))); } @@ -739,7 +685,7 @@ public static Msg makeMessage(byte cmd) throws InvalidMessageTypeException { * @throws InvalidMessageTypeException */ public static Msg makeMessage(String type) throws InvalidMessageTypeException { - return Optional.ofNullable(MsgDefinitionRegistry.getInstance().getTemplate(type)).map(Msg::new) + return Optional.ofNullable(MsgDefinitionRegistry.getInstance().getDefinition(type)).map(Msg::new) .orElseThrow(() -> new InvalidMessageTypeException("unknown message type: " + type)); } diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgDefinition.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgDefinition.java index 2d295e4063fc4..75a51cc08816d 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgDefinition.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgDefinition.java @@ -12,12 +12,11 @@ */ package org.openhab.binding.insteon.internal.transport.message; -import java.util.Comparator; -import java.util.HashMap; import java.util.List; import java.util.Map; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.openhab.binding.insteon.internal.utils.BinaryUtils; /** * Definition (layout) of an Insteon message. Says which bytes go where. @@ -30,34 +29,34 @@ */ @NonNullByDefault public class MsgDefinition { - private Map fields = new HashMap<>(); + private final byte[] data; + private final int headerLength; + private final Direction direction; + private final Map fields; - MsgDefinition() { + public MsgDefinition(byte[] data, int headerLength, Direction direction, Map fields) { + this.data = data; + this.headerLength = headerLength; + this.direction = direction; + this.fields = fields; } - MsgDefinition(MsgDefinition definition) { - fields = new HashMap<>(definition.fields); + public byte[] getData() { + return data; } - public List getFields() { - return fields.values().stream().sorted(Comparator.comparing(Field::getOffset)).toList(); + public int getLength() { + return data.length; } - public boolean containsField(String name) { - return fields.containsKey(name); + public int getHeaderLength() { + return headerLength; } - public void addField(Field field) { - fields.put(field.getName(), field); + public Direction getDirection() { + return direction; } - /** - * Finds field of a given name - * - * @param name name of the field to search for - * @return reference to field - * @throws FieldException if no such field can be found - */ public Field getField(String name) throws FieldException { Field field = fields.get(name); if (field == null) { @@ -65,4 +64,41 @@ public Field getField(String name) throws FieldException { } return field; } + + public List getFields() { + return fields.values().stream().toList(); + } + + public boolean containsField(String name) { + return fields.containsKey(name); + } + + public byte getByte(String name) throws FieldException { + return getField(name).getByte(data); + } + + public byte getCommand() { + try { + return getByte("Cmd"); + } catch (FieldException e) { + return (byte) 0xFF; + } + } + + public boolean isExtended() { + try { + return BinaryUtils.isBitSet(getByte("messageFlags"), 4); + } catch (FieldException e) { + return false; + } + } + + @Override + public String toString() { + String s = direction + ":"; + for (Field field : getFields()) { + s += field.toString(data) + "|"; + } + return s; + } } diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgDefinitionRegistry.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgDefinitionRegistry.java index 445b2c9a824ec..107bc2b2d3774 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgDefinitionRegistry.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgDefinitionRegistry.java @@ -14,14 +14,10 @@ import java.util.LinkedHashMap; import java.util.Map; -import java.util.Map.Entry; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.insteon.internal.InsteonResourceLoader; -import org.openhab.binding.insteon.internal.device.InsteonAddress; -import org.openhab.binding.insteon.internal.transport.message.Msg.Direction; -import org.openhab.binding.insteon.internal.utils.HexUtils; import org.w3c.dom.Element; import org.w3c.dom.Node; import org.w3c.dom.NodeList; @@ -39,44 +35,46 @@ public class MsgDefinitionRegistry extends InsteonResourceLoader { private static final MsgDefinitionRegistry MSG_DEFINITION_REGISTRY = new MsgDefinitionRegistry(); private static final String RESOURCE_NAME = "/msg-definitions.xml"; - private Map definitions = new LinkedHashMap<>(); + private Map definitions = new LinkedHashMap<>(); private MsgDefinitionRegistry() { super(RESOURCE_NAME); } /** - * Returns message template for a given type + * Returns message definition for a given type * * @param type message type to match - * @return message template if found, otherwise null + * @return message definition if found, otherwise null */ - public @Nullable Msg getTemplate(String type) { + public @Nullable MsgDefinition getDefinition(String type) { return definitions.get(type); } /** - * Returns message template for a given command and direction + * Returns message definition for a given command and direction * * @param cmd message command to match * @param direction message direction to match - * @return message template if found, otherwise null + * @return message definition if found, otherwise null */ - public @Nullable Msg getTemplate(byte cmd, Direction direction) { - return getTemplate(cmd, null, direction); + public @Nullable MsgDefinition getDefinition(byte cmd, Direction direction) { + return getDefinition(cmd, null, direction); } /** - * Returns message template for a given command, extended flag and direction + * Returns message definition for a given command, extended flag and direction * * @param cmd message command to match * @param isExtended if message is extended * @param direction message direction to match - * @return message template if found, otherwise null + * @return message definition if found, otherwise null */ - public @Nullable Msg getTemplate(byte cmd, @Nullable Boolean isExtended, Direction direction) { - return definitions.values().stream().filter(msg -> msg.getCommand() == cmd && msg.getDirection() == direction - && (isExtended == null || msg.isExtended() == isExtended)).findFirst().orElse(null); + public @Nullable MsgDefinition getDefinition(byte cmd, @Nullable Boolean isExtended, Direction direction) { + return definitions.values().stream() + .filter(definition -> definition.getCommand() == cmd && definition.getDirection() == direction + && (isExtended == null || definition.isExtended() == isExtended)) + .findFirst().orElse(null); } /** @@ -84,7 +82,7 @@ private MsgDefinitionRegistry() { * * @return currently known message definitions */ - public Map getDefinitions() { + public Map getDefinitions() { return definitions; } @@ -131,12 +129,22 @@ protected void parseDocument(Element element) throws SAXException { * @throws SAXException */ private void parseMsgDefinition(Element element) throws SAXException { - LinkedHashMap fields = new LinkedHashMap<>(); String name = element.getAttribute("name"); - Direction direction = Direction.valueOf(element.getAttribute("direction")); - int length = element.hasAttribute("length") ? Integer.parseInt(element.getAttribute("length")) : 0; + if (name.isEmpty()) { + throw new SAXException("undefined message definition name"); + } + Direction direction = Direction.get(element.getAttribute("direction")); + if (direction == null) { + throw new SAXException("invalid direction for message definition " + name); + } + int length = getAttributeAsInteger(element, "length", 0); + if (length == 0) { + throw new SAXException("undefined length for message definition " + name); + } int headerLength = 0; int offset = 0; + byte[] data = new byte[length]; + Map fields = new LinkedHashMap<>(); NodeList nodes = element.getChildNodes(); for (int i = 0; i < nodes.getLength(); i++) { @@ -144,27 +152,26 @@ private void parseMsgDefinition(Element element) throws SAXException { if (node.getNodeType() == Node.ELEMENT_NODE) { Element child = (Element) node; String nodeName = child.getNodeName(); - if ("header".equals(nodeName)) { - headerLength = parseHeader(child, fields); - // Increment the offset by the header length - offset += headerLength; - } else { + if (!"header".equals(nodeName)) { // Increment the offset by the field data type length - offset += parseField(child, offset, fields); + offset += parseField(child, offset, data, fields); + } else if (offset == 0) { + headerLength = parseHeader(child, data, fields); + // Set the offset to the header length + offset = headerLength; } } } - if (length == 0) { - length = offset; - } else if (offset != length) { - throw new SAXException("actual msg length " + offset + " differs from given msg length " + length); + if (headerLength == 0) { + throw new SAXException("undefined header for message definition " + name); + } + if (offset != length) { + throw new SAXException("actual msg length " + offset + " differs from given length " + length); } - try { - Msg msg = makeMsgTemplate(fields, headerLength, length, direction); - definitions.put(name, msg); - } catch (FieldException e) { - throw new SAXException("failed to create message definition " + name + ":", e); + MsgDefinition definition = new MsgDefinition(data, headerLength, direction, fields); + if (definitions.put(name, definition) != null) { + logger.warn("duplicate message definition {}", name); } } @@ -172,12 +179,16 @@ private void parseMsgDefinition(Element element) throws SAXException { * Parses header node * * @param element element to parse + * @param data msg data to update * @param fields fields map to update * @return header length * @throws SAXException */ - private int parseHeader(Element element, LinkedHashMap fields) throws SAXException { - int length = Integer.parseInt(element.getAttribute("length")); + private int parseHeader(Element element, byte[] data, Map fields) throws SAXException { + int length = getAttributeAsInteger(element, "length", 0); + if (length == 0) { + throw new SAXException("undefined header length"); + } int offset = 0; NodeList nodes = element.getChildNodes(); @@ -186,10 +197,10 @@ private int parseHeader(Element element, LinkedHashMap fields) th if (node.getNodeType() == Node.ELEMENT_NODE) { Element child = (Element) node; // Increment the offset by the field data type length - offset += parseField(child, offset, fields); + offset += parseField(child, offset, data, fields); } } - if (length != offset) { + if (offset != length) { throw new SAXException("actual header length " + offset + " differs from given length " + length); } return length; @@ -200,93 +211,27 @@ private int parseHeader(Element element, LinkedHashMap fields) th * * @param element element to parse * @param offset msg offset + * @param data msg data to update * @param fields fields map to update * @return field data type length * @throws SAXException */ - private int parseField(Element element, int offset, LinkedHashMap fields) throws SAXException { + private int parseField(Element element, int offset, byte[] data, Map fields) throws SAXException { String name = element.getAttribute("name"); - if (name == null) { - throw new SAXException("undefined field name"); - } DataType dataType = DataType.get(element.getNodeName()); - Field field = new Field(name, dataType, offset); - Object value = getFieldValue(dataType, element.getTextContent().trim()); - fields.put(field, value); - return dataType.getSize(); - } - - /** - * Returns field value - * - * @param dataType field data type - * @param value value to convert - * @return field value - * @throws SAXException - */ - private Object getFieldValue(DataType dataType, String value) throws SAXException { - switch (dataType) { - case BYTE: - return getByteValue(value); - case ADDRESS: - return getAddressValue(value); - default: - throw new SAXException("invalid field data type"); - } - } - - /** - * Returns field value as a byte - * - * @param value value to convert - * @return byte - * @throws SAXException - */ - private byte getByteValue(String value) throws SAXException { - try { - return value.isEmpty() ? 0x00 : (byte) HexUtils.toInteger(value); - } catch (NumberFormatException e) { - throw new SAXException("invalid field byte value: " + value); + if (dataType == null) { + throw new SAXException("invalid field data type"); } - } - - /** - * Returns field value as an insteon address - * - * @param value value to convert - * @return insteon address - * @throws SAXException - */ - private InsteonAddress getAddressValue(String value) throws SAXException { + Field field = new Field(name, offset, dataType); try { - return value.isEmpty() ? InsteonAddress.UNKNOWN : new InsteonAddress(value); - } catch (IllegalArgumentException e) { - throw new SAXException("invalid field address value: " + value); + field.set(data, element.getTextContent()); + } catch (FieldException | IllegalArgumentException e) { + throw new SAXException("failed to set field data:", e); } - } - - /** - * Returns new message template - * - * @param fields msg fields - * @param length msg length - * @param headerLength header length - * @param direction msg direction - * @return new msg template - * @throws FieldException - */ - private Msg makeMsgTemplate(Map fields, int headerLength, int length, Direction direction) - throws FieldException { - Msg msg = new Msg(headerLength, length, direction); - for (Entry entry : fields.entrySet()) { - Field field = entry.getKey(); - byte[] data = msg.getData(); - field.set(data, entry.getValue()); - if (!field.getName().isEmpty()) { - msg.addField(field); - } + if (!name.isEmpty()) { + fields.put(name, field); } - return msg; + return dataType.getSize(); } /** diff --git a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgFactory.java b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgFactory.java index 5394ec3cdb0e0..0555e04cce30f 100644 --- a/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgFactory.java +++ b/bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/transport/message/MsgFactory.java @@ -97,8 +97,7 @@ public void addData(byte[] data, int len) { logger.trace("got pure nack!"); removeFromBuffer(1); try { - msg = Msg.makeMessage("PureNACK"); - return msg; + return Msg.makeMessage("PureNACK"); } catch (InvalidMessageTypeException e) { return null; } @@ -112,39 +111,23 @@ public void addData(byte[] data, int len) { // If not, we return null, and expect this method to be called again // when more data has come in. if (end > 1) { - // we have some data, but do we have enough to read the entire header? - int headerLength = Msg.getHeaderLength(buf[1]); - boolean isExtended = Msg.isExtended(buf, end, headerLength); - logger.trace("header length expected: {} extended: {}", headerLength, isExtended); - if (headerLength < 0) { - if (logger.isDebugEnabled()) { - logger.debug("got unknown command code: {}", HexUtils.getHexString(buf[1])); - } - removeFromBuffer(1); // get rid of the leading 0x02 so draining works - bail(); - } else if (headerLength >= 2) { + try { + int headerLength = Msg.getHeaderLength(buf[1]); + logger.trace("header length expected: {}", headerLength); if (end >= headerLength) { - // only when the header is complete do we know that isExtended is correct! + boolean isExtended = Msg.isExtended(buf, headerLength); int msgLen = Msg.getMessageLength(buf[1], isExtended); - logger.trace("msgLen expected: {}", msgLen); - if (msgLen < 0) { - // Cannot make sense out of the combined command code & isExtended flag. - if (logger.isDebugEnabled()) { - logger.debug("got unknown command code/ext flag: {}", HexUtils.getHexString(buf[1])); - } - removeFromBuffer(1); - bail(); - } else if (msgLen > 0) { - if (end >= msgLen) { - msg = Msg.createMessage(buf, msgLen, isExtended); - removeFromBuffer(msgLen); - } - } else { // should never happen - logger.warn("invalid message length, internal error!"); + logger.trace("msgLen expected: {} extended: {}", msgLen, isExtended); + if (end >= msgLen) { + msg = Msg.createMessage(buf, msgLen, isExtended); + removeFromBuffer(msgLen); } } - } else { // should never happen - logger.warn("invalid header length, internal error!"); + } catch (InvalidMessageTypeException e) { + if (logger.isDebugEnabled()) { + logger.debug("got unknown command code: {}", HexUtils.getHexString(buf[1])); + } + bail(); } } // indicate no more messages available in buffer if empty or undefined message @@ -159,14 +142,11 @@ public void addData(byte[] data, int len) { } private void bail() throws IOException { - drainBuffer(); // this will drain until end or it finds the next message start - throw new IOException("bad data received"); - } - - private void drainBuffer() { - while (end > 0 && buf[0] != 0x02) { + // drain buffer until end or the next message start + do { removeFromBuffer(1); - } + } while (end > 0 && buf[0] != 0x02); + throw new IOException("bad data received"); } private void removeFromBuffer(int len) { @@ -174,7 +154,9 @@ private void removeFromBuffer(int len) { if (l > end) { l = end; } - System.arraycopy(buf, l, buf, 0, end + 1 - l); - end -= l; + if (l > 0) { + System.arraycopy(buf, l, buf, 0, end + 1 - l); + end -= l; + } } } diff --git a/bundles/org.openhab.binding.insteon/src/main/resources/msg-definitions.xml b/bundles/org.openhab.binding.insteon/src/main/resources/msg-definitions.xml index dad8f26207fdb..81736a02a765a 100644 --- a/bundles/org.openhab.binding.insteon/src/main/resources/msg-definitions.xml +++ b/bundles/org.openhab.binding.insteon/src/main/resources/msg-definitions.xml @@ -163,7 +163,7 @@ - 0x00 +
@@ -205,20 +205,20 @@
- 0x00 - 0x00 - 0x00 - 0x00 - 0x00 - 0x00 - 0x00 - 0x00 - 0x00 - 0x00 - 0x00 - 0x00 - 0x00 - 0x00 + + + + + + + + + + + + + +
@@ -250,16 +250,16 @@ 0x02 0x63
- - 0x00 + +
0x02 0x63
- - 0x00 + + @@ -267,16 +267,16 @@ 0x02 0x64 - - 0x00 + +
0x02 0x64
- - 0x00 + + @@ -550,9 +550,9 @@
- 0x00 - 0x00 - 0x00 + + +
@@ -603,9 +603,9 @@ 0x02 0x79
- 0x00 - 0x00 - 0x00 + + +