public/openhab-addons
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[knx] Improve thread safety, null-analysis (#14509)

Browse Source 
Carryover from smarthomej/addons#13 and smarthomej/addons#46, smarthomej/addons#60.

Also-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
This commit is contained in:
Holger Friedrich 2023-03-06 22:34:16 +01:00 committed by GitHub
parent 539f49e4ec
commit 7ec7de55e3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 83 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/KNXTypeMapper.java
View File

@ -38,7 +38,7 @@ public interface KNXTypeMapper {
* @return datapoint value as a string
*/
@Nullable
public String toDPTValue(Type type, @Nullable String dpt);
String toDPTValue(Type type, @Nullable String dpt);
/**
* maps a datapoint value to an openHAB command or state
@ -49,8 +49,8 @@ public interface KNXTypeMapper {
* @return a command or state of openHAB
*/
@Nullable
public Type toType(Datapoint datapoint, byte[] data);
Type toType(Datapoint datapoint, byte[] data);
@Nullable
public Class<? extends Type> toTypeClass(@Nullable String dpt);
Class<? extends Type> toTypeClass(@Nullable String dpt);
}

4
bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeDimmer.java
View File

@ -12,12 +12,10 @@
*/
package org.openhab.binding.knx.internal.channel;
import static java.util.stream.Collectors.toSet;
import static org.openhab.binding.knx.internal.KNXBindingConstants.*;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;
import org.eclipse.jdt.annotation.NonNullByDefault;
@ -40,7 +38,7 @@ class TypeDimmer extends KNXChannelType {
@Override
protected Set<String> getAllGAKeys() {
return Stream.of(SWITCH_GA, POSITION_GA, INCREASE_DECREASE_GA).collect(toSet());
return Set.of(SWITCH_GA, POSITION_GA, INCREASE_DECREASE_GA);
}
@Override

4
bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/channel/TypeRollershutter.java
View File

@ -12,12 +12,10 @@
*/
package org.openhab.binding.knx.internal.channel;
import static java.util.stream.Collectors.toSet;
import static org.openhab.binding.knx.internal.KNXBindingConstants.*;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;
import org.eclipse.jdt.annotation.NonNullByDefault;
@ -53,6 +51,6 @@ class TypeRollershutter extends KNXChannelType {
@Override
protected Set<String> getAllGAKeys() {
return Stream.of(UP_DOWN_GA, STOP_MOVE_GA, POSITION_GA).collect(toSet());
return Set.of(UP_DOWN_GA, STOP_MOVE_GA, POSITION_GA);
}
}

5
bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/client/AbstractKNXClient.java
View File

@ -297,12 +297,11 @@ public abstract class AbstractKNXClient implements NetworkLinkListener, KNXClien
}
}
@SuppressWarnings("null")
protected void releaseConnection() {
logger.debug("Bridge {} is disconnecting from KNX bus", thingUID);
var tmplink = link;
if (tmplink != null) {
link.removeLinkListener(this);
tmplink.removeLinkListener(this);
}
busJob = nullify(busJob, j -> j.cancel(true));
readDatapoints.clear();
@ -321,7 +320,7 @@ public abstract class AbstractKNXClient implements NetworkLinkListener, KNXClien
logger.trace("Bridge {} disconnected from KNX bus", thingUID);
}
private <T> @Nullable T nullify(T target, @Nullable Consumer<T> lastWill) {
private <T> @Nullable T nullify(@Nullable T target, @Nullable Consumer<T> lastWill) {
if (target != null && lastWill != null) {
lastWill.accept(target);
}

4
bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/dpt/KNXCoreTypeMapper.java
View File

@ -977,7 +977,7 @@ public class KNXCoreTypeMapper implements KNXTypeMapper {
if (typeClass.equals(DateTimeType.class)) {
String date = formatDateTime(value, datapoint.getDPT());
if (date.isEmpty()) {
logger.debug("toType: KNX clock msg ignored: date object null or empty {}.", date);
logger.debug("toType: KNX clock msg ignored: date object empty {}.", date);
return null;
} else {
return DateTimeType.valueOf(date);
@ -1047,7 +1047,7 @@ public class KNXCoreTypeMapper implements KNXTypeMapper {
* @return a formatted String like </code>yyyy-MM-dd'T'HH:mm:ss</code> which
* is target format of the {@link DateTimeType}
*/
private String formatDateTime(String value, String dpt) {
private String formatDateTime(String value, @Nullable String dpt) {
Date date = null;
try {

21
bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/factory/KNXHandlerFactory.java
View File

@ -14,8 +14,8 @@ package org.openhab.binding.knx.internal.factory;
import static org.openhab.binding.knx.internal.KNXBindingConstants.*;
import java.util.Arrays;
import java.util.Collection;
import java.util.Set;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
@ -50,7 +50,7 @@ import org.osgi.service.component.annotations.Reference;
@Component(service = ThingHandlerFactory.class, configurationPid = "binding.knx")
public class KNXHandlerFactory extends BaseThingHandlerFactory {
public static final Collection<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Arrays.asList(THING_TYPE_DEVICE,
public static final Collection<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Set.of(THING_TYPE_DEVICE,
THING_TYPE_IP_BRIDGE, THING_TYPE_SERIAL_BRIDGE);
@Nullable
@ -58,9 +58,11 @@ public class KNXHandlerFactory extends BaseThingHandlerFactory {
private final SerialPortManager serialPortManager;
@Activate
public KNXHandlerFactory(final @Reference TranslationProvider translationProvider,
final @Reference LocaleProvider localeProvider, final @Reference SerialPortManager serialPortManager) {
public KNXHandlerFactory(final @Reference NetworkAddressService networkAddressService,
final @Reference TranslationProvider translationProvider, final @Reference LocaleProvider localeProvider,
final @Reference SerialPortManager serialPortManager) {
KNXTranslationProvider.I18N.setProvider(localeProvider, translationProvider);
this.networkAddressService = networkAddressService;
this.serialPortManager = serialPortManager;
SerialTransportAdapter.setSerialPortManager(serialPortManager);
}
@ -84,7 +86,7 @@ public class KNXHandlerFactory extends BaseThingHandlerFactory {
if (THING_TYPE_DEVICE.equals(thingTypeUID)) {
return super.createThing(thingTypeUID, configuration, thingUID, bridgeUID);
}
throw new IllegalArgumentException("The thing type " + thingTypeUID + " is not supported by the KNX binding.");
return null;
}
@Override
@ -116,13 +118,4 @@ public class KNXHandlerFactory extends BaseThingHandlerFactory {
String serialPort = (String) configuration.get(SERIAL_PORT);
return new ThingUID(thingTypeUID, serialPort);
}
@Reference
protected void setNetworkAddressService(NetworkAddressService networkAddressService) {
this.networkAddressService = networkAddressService;
}
protected void unsetNetworkAddressService(NetworkAddressService networkAddressService) {
this.networkAddressService = null;
}
}

2
bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/AbstractKNXThingHandler.java
View File

@ -52,7 +52,7 @@ public abstract class AbstractKNXThingHandler extends BaseThingHandler implement
private final Logger logger = LoggerFactory.getLogger(AbstractKNXThingHandler.class);
protected @Nullable IndividualAddress address;
private @Nullable Future<?> descriptionJob;
private @Nullable ScheduledFuture<?> descriptionJob;
private boolean filledDescription = false;
private final Random random = new Random();

63
bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java
View File

@ -15,13 +15,12 @@ package org.openhab.binding.knx.internal.handler;
import static org.openhab.binding.knx.internal.KNXBindingConstants.*;
import java.math.BigDecimal;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
@ -69,11 +68,11 @@ public class DeviceThingHandler extends AbstractKNXThingHandler {
private final Logger logger = LoggerFactory.getLogger(DeviceThingHandler.class);
private final KNXTypeMapper typeHelper = new KNXCoreTypeMapper();
private final Set<GroupAddress> groupAddresses = new HashSet<>();
private final Set<GroupAddress> groupAddressesWriteBlockedOnce = new HashSet<>();
private final Set<OutboundSpec> groupAddressesRespondingSpec = new HashSet<>();
private final Map<GroupAddress, ScheduledFuture<?>> readFutures = new HashMap<>();
private final Map<ChannelUID, ScheduledFuture<?>> channelFutures = new HashMap<>();
private final Set<GroupAddress> groupAddresses = ConcurrentHashMap.newKeySet();
private final Set<GroupAddress> groupAddressesWriteBlockedOnce = ConcurrentHashMap.newKeySet();
private final Set<OutboundSpec> groupAddressesRespondingSpec = ConcurrentHashMap.newKeySet();
private final Map<GroupAddress, ScheduledFuture<?>> readFutures = new ConcurrentHashMap<>();
private final Map<ChannelUID, ScheduledFuture<?>> channelFutures = new ConcurrentHashMap<>();
private int readInterval;
public DeviceThingHandler(Thing thing) {
@ -99,20 +98,20 @@ public class DeviceThingHandler extends AbstractKNXThingHandler {
@Override
public void dispose() {
cancelChannelFutures();
freeGroupAdresses();
freeGroupAddresses();
super.dispose();
}
private void cancelChannelFutures() {
for (ScheduledFuture<?> future : channelFutures.values()) {
if (!future.isDone()) {
future.cancel(true);
}
for (ChannelUID channelUID : channelFutures.keySet()) {
channelFutures.computeIfPresent(channelUID, (k, v) -> {
v.cancel(true);
return null;
});
}
channelFutures.clear();
}
private void freeGroupAdresses() {
private void freeGroupAddresses() {
groupAddresses.clear();
groupAddressesWriteBlockedOnce.clear();
groupAddressesRespondingSpec.clear();
@ -120,12 +119,12 @@ public class DeviceThingHandler extends AbstractKNXThingHandler {
@Override
protected void cancelReadFutures() {
for (ScheduledFuture<?> future : readFutures.values()) {
if (!future.isDone()) {
future.cancel(true);
}
for (GroupAddress groupAddress : readFutures.keySet()) {
readFutures.computeIfPresent(groupAddress, (k, v) -> {
v.cancel(true);
return null;
});
}
readFutures.clear();
}
@FunctionalInterface
@ -220,7 +219,9 @@ public class DeviceThingHandler extends AbstractKNXThingHandler {
@SuppressWarnings("null")
private void rememberRespondingSpec(OutboundSpec commandSpec, boolean add) {
GroupAddress ga = commandSpec.getGroupAddress();
groupAddressesRespondingSpec.removeIf(spec -> spec.getGroupAddress().equals(ga));
if (ga != null) {
groupAddressesRespondingSpec.removeIf(spec -> spec.getGroupAddress().equals(ga));
}
if (add) {
groupAddressesRespondingSpec.add(commandSpec);
}
@ -393,18 +394,18 @@ public class DeviceThingHandler extends AbstractKNXThingHandler {
&& (type instanceof UnDefType || type instanceof IncreaseDecreaseType) && frequency > 0) {
// continuous dimming by the binding
if (UnDefType.UNDEF.equals(type)) {
ScheduledFuture<?> future = channelFutures.remove(channelUID);
if (future != null) {
future.cancel(false);
}
channelFutures.computeIfPresent(channelUID, (k, v) -> {
v.cancel(false);
return null;
});
} else if (type instanceof IncreaseDecreaseType) {
ScheduledFuture<?> future = scheduler.scheduleWithFixedDelay(() -> {
postCommand(channelUID, (Command) type);
}, 0, frequency, TimeUnit.MILLISECONDS);
ScheduledFuture<?> previousFuture = channelFutures.put(channelUID, future);
if (previousFuture != null) {
previousFuture.cancel(true);
}
channelFutures.compute(channelUID, (k, v) -> {
if (v != null) {
v.cancel(true);
}
return scheduler.scheduleWithFixedDelay(() -> postCommand(channelUID, (Command) type), 0,
frequency, TimeUnit.MILLISECONDS);
});
}
} else {
if (type instanceof Command) {

3
bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/IPBridgeThingHandler.java
View File

@ -175,8 +175,9 @@ public class IPBridgeThingHandler extends KNXBridgeBaseThingHandler {
logger.debug("NetworkAddressService not available, cannot create bridge {}", thing.getUID());
updateStatus(ThingStatus.OFFLINE);
return;
} else {
localEndPoint = new InetSocketAddress(networkAddressService.getPrimaryIpv4HostAddress(), 0);
}
localEndPoint = new InetSocketAddress(networkAddressService.getPrimaryIpv4HostAddress(), 0);
}
updateStatus(ThingStatus.UNKNOWN);

38
bundles/org.openhab.binding.knx/src/test/java/org/openhab/binding/knx/internal/channel/KNXChannelTypeTest.java
View File

@ -36,11 +36,15 @@ class KNXChannelTypeTest {
ct = new MyKNXChannelType("");
}
@SuppressWarnings("null")
@Test
void testParseWithDptMultipleWithRead() {
ChannelConfiguration res = ct.parse("5.001:<1/3/22+0/3/22+<0/8/15");
if (res == null) {
fail();
return;
}
assertEquals("5.001", res.getDPT());
assertEquals("1/3/22", res.getMainGA().getGA());
assertTrue(res.getMainGA().isRead());
@ -48,11 +52,15 @@ class KNXChannelTypeTest {
assertEquals(2, res.getReadGAs().size());
}
@SuppressWarnings("null")
@Test
void testParseWithDptMultipleWithoutRead() {
ChannelConfiguration res = ct.parse("5.001:1/3/22+0/3/22+0/8/15");
if (res == null) {
fail();
return;
}
assertEquals("5.001", res.getDPT());
assertEquals("1/3/22", res.getMainGA().getGA());
assertFalse(res.getMainGA().isRead());
@ -60,11 +68,15 @@ class KNXChannelTypeTest {
assertEquals(0, res.getReadGAs().size());
}
@SuppressWarnings("null")
@Test
void testParseWithoutDptSingleWithoutRead() {
ChannelConfiguration res = ct.parse("1/3/22");
if (res == null) {
fail();
return;
}
assertNull(res.getDPT());
assertEquals("1/3/22", res.getMainGA().getGA());
assertFalse(res.getMainGA().isRead());
@ -72,11 +84,15 @@ class KNXChannelTypeTest {
assertEquals(0, res.getReadGAs().size());
}
@SuppressWarnings("null")
@Test
void testParseWithoutDptSingleWitRead() {
ChannelConfiguration res = ct.parse("<1/3/22");
if (res == null) {
fail();
return;
}
assertNull(res.getDPT());
assertEquals("1/3/22", res.getMainGA().getGA());
assertTrue(res.getMainGA().isRead());
@ -84,19 +100,29 @@ class KNXChannelTypeTest {
assertEquals(1, res.getReadGAs().size());
}
@SuppressWarnings("null")
@Test
void testParseTwoLevel() {
ChannelConfiguration res = ct.parse("5.001:<3/1024+<4/1025");
if (res == null) {
fail();
return;
}
assertEquals("3/1024", res.getMainGA().getGA());
assertEquals(2, res.getListenGAs().size());
assertEquals(2, res.getReadGAs().size());
}
@SuppressWarnings("null")
@Test
void testParseFreeLevel() {
ChannelConfiguration res = ct.parse("5.001:<4610+<4611");
if (res == null) {
fail();
return;
}
assertEquals("4610", res.getMainGA().getGA());
assertEquals(2, res.getListenGAs().size());
assertEquals(2, res.getReadGAs().size());