From: Hilbrand Bouwkamp Date: Fri, 8 Jan 2021 22:16:33 +0000 (+0100) Subject: [dsmr] Fix handling problem reading gasmeter (#9736) X-Git-Url: https://git.basschouten.com/?a=commitdiff_plain;h=3f0c0ebf1fccff17f8aad0354d7849ac7fa8fd0d;p=openhab-addons.git [dsmr] Fix handling problem reading gasmeter (#9736) * [dsmr] Fix handling problem reading gasmeter The gasmeter seem to return an invalid date value in some cases. Possible problems with the meter. This however caused the whole meter readings to be ignored, while it should only ignore the single problem value. Signed-off-by: Hilbrand Bouwkamp * [dsmr] Make eclipse happy again. Don't ask... Signed-off-by: Hilbrand Bouwkamp * [dsmr] Removed sat warnings Removed usage of apache library and added NonNullByDefault to test classes.. Signed-off-by: Hilbrand Bouwkamp --- diff --git a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMRDeviceConfiguration.java b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMRDeviceConfiguration.java index c59a348279..d75b11d34e 100644 --- a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMRDeviceConfiguration.java +++ b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMRDeviceConfiguration.java @@ -12,8 +12,6 @@ */ package org.openhab.binding.dsmr.internal.device; -import org.apache.commons.lang.StringUtils; - /** * Class describing the DSMR bridge user configuration * @@ -60,7 +58,8 @@ public class DSMRDeviceConfiguration { * @return true if serial port settings are all set. */ public boolean isSerialFixedSettings() { - return baudrate > 0 && databits > 0 && !StringUtils.isBlank(parity) && !StringUtils.isBlank(stopbits); + return baudrate > 0 && databits > 0 && !(parity != null && parity.isBlank()) + && !(stopbits != null && stopbits.isBlank()); } @Override diff --git a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/cosem/CosemDate.java b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/cosem/CosemDate.java index 711fe70203..77f3a52a44 100644 --- a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/cosem/CosemDate.java +++ b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/cosem/CosemDate.java @@ -17,6 +17,7 @@ import java.time.LocalDateTime; import java.time.ZoneId; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeParseException; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -35,6 +36,10 @@ import org.slf4j.LoggerFactory; class CosemDate extends CosemValueDescriptor { public static final CosemDate INSTANCE = new CosemDate("timestamp"); + /* + * Some meters can return the following value when something is wrong. + */ + public static final String INVALID_METER_VALUE = "632525252525W"; private final Logger logger = LoggerFactory.getLogger(CosemDate.class); @@ -102,8 +107,16 @@ class CosemDate extends CosemValueDescriptor { if (m.matches()) { logger.trace("{} matches pattern: {}", cosemValue, cosemDateFormat.pattern); - LocalDateTime localDateTime = LocalDateTime.parse(m.group(1), cosemDateFormat.formatter); - return new DateTimeType(ZonedDateTime.of(localDateTime, ZoneId.systemDefault())); + try { + LocalDateTime localDateTime = LocalDateTime.parse(m.group(1), cosemDateFormat.formatter); + return new DateTimeType(ZonedDateTime.of(localDateTime, ZoneId.systemDefault())); + } catch (DateTimeParseException e) { + if (INVALID_METER_VALUE.equals(cosemValue)) { + throw new ParseException( + "Cosem value: '" + cosemValue + "' might indicate something is wrong with the meter.", + 0); + } + } } } throw new ParseException("Cosem value: '" + cosemValue + "' is not a known CosemDate string", 0); diff --git a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryService.java b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryService.java index d19702c3c8..8c3f6716ca 100644 --- a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryService.java +++ b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryService.java @@ -12,8 +12,12 @@ */ package org.openhab.binding.dsmr.internal.discovery; -import java.util.*; +import java.util.Collection; +import java.util.List; +import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -143,10 +147,9 @@ public class DSMRMeterDiscoveryService extends DSMRDiscoveryService implements P .map(Thing::getHandler) .filter(DSMRMeterHandler.class::isInstance) .map(DSMRMeterHandler.class::cast) - .map(h -> h == null ? null : h.getMeterDescriptor()) - .map(d -> Optional.ofNullable(d == null ? null : d.getMeterType())) - .filter(Optional::isPresent) - .map(Optional::get) + .map(DSMRMeterHandler::getMeterDescriptor) + .filter(Objects::nonNull) + .map(d -> d.getMeterType()) .collect(Collectors.toSet()); // @formatter:on // Create list of all configured meters that are not in the detected list. If not empty meters might not be diff --git a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/TelegramReaderUtil.java b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/TelegramReaderUtil.java index 1bd968da47..a27edc4ae2 100644 --- a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/TelegramReaderUtil.java +++ b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/TelegramReaderUtil.java @@ -18,7 +18,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.concurrent.atomic.AtomicReference; -import org.apache.commons.io.IOUtils; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.binding.dsmr.internal.device.p1telegram.P1Telegram; import org.openhab.binding.dsmr.internal.device.p1telegram.P1Telegram.TelegramState; import org.openhab.binding.dsmr.internal.device.p1telegram.P1TelegramParser; @@ -28,6 +28,7 @@ import org.openhab.binding.dsmr.internal.device.p1telegram.P1TelegramParser; * * @author Hilbrand Bouwkamp - Initial contribution */ +@NonNullByDefault public final class TelegramReaderUtil { private static final String TELEGRAM_EXT = ".telegram"; @@ -43,7 +44,10 @@ public final class TelegramReaderUtil { */ public static byte[] readRawTelegram(String telegramName) { try (InputStream is = TelegramReaderUtil.class.getResourceAsStream(telegramName + TELEGRAM_EXT)) { - return IOUtils.toByteArray(is); + if (is == null) { + fail("Could not find telegram file with name:" + telegramName + TELEGRAM_EXT); + } + return is.readAllBytes(); } catch (IOException e) { throw new AssertionError("IOException reading telegram data: ", e); } diff --git a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/device/DSMRSerialAutoDeviceTest.java b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/device/DSMRSerialAutoDeviceTest.java index fe79fa1afc..8738c1d0b7 100644 --- a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/device/DSMRSerialAutoDeviceTest.java +++ b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/device/DSMRSerialAutoDeviceTest.java @@ -24,6 +24,8 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -47,18 +49,19 @@ import org.openhab.core.io.transport.serial.SerialPortManager; * @author Hilbrand Bouwkamp - Initial contribution */ @ExtendWith(MockitoExtension.class) +@NonNullByDefault public class DSMRSerialAutoDeviceTest { private static final String DUMMY_PORTNAME = "/dev/dummy-serial"; private static final String TELEGRAM_NAME = "dsmr_50"; - private @Mock SerialPortIdentifier mockIdentifier; - private @Mock ScheduledExecutorService scheduler; - private @Mock SerialPort mockSerialPort; + private @NonNullByDefault({}) @Mock SerialPortIdentifier mockIdentifier; + private @NonNullByDefault({}) @Mock ScheduledExecutorService scheduler; + private @NonNullByDefault({}) @Mock SerialPort mockSerialPort; - private SerialPortManager serialPortManager = new SerialPortManager() { + private final SerialPortManager serialPortManager = new SerialPortManager() { @Override - public SerialPortIdentifier getIdentifier(String name) { + public @Nullable SerialPortIdentifier getIdentifier(String name) { assertEquals(DUMMY_PORTNAME, name, "Expect the passed serial port name"); return mockIdentifier; } @@ -68,7 +71,7 @@ public class DSMRSerialAutoDeviceTest { return Stream.empty(); } }; - private SerialPortEventListener serialPortEventListener; + private @NonNullByDefault({}) SerialPortEventListener serialPortEventListener; @BeforeEach public void setUp() throws PortInUseException, TooManyListenersException { @@ -81,7 +84,7 @@ public class DSMRSerialAutoDeviceTest { @Test public void testHandlingDataAndRestart() throws IOException, PortInUseException { mockValidSerialPort(); - AtomicReference telegramRef = new AtomicReference<>(null); + AtomicReference<@Nullable P1Telegram> telegramRef = new AtomicReference<>(null); DSMREventListener listener = new DSMREventListener() { @Override public void handleTelegramReceived(P1Telegram telegram) { @@ -114,7 +117,7 @@ public class DSMRSerialAutoDeviceTest { @Test public void testHandleError() throws IOException, PortInUseException { - AtomicReference eventRef = new AtomicReference<>(null); + AtomicReference<@Nullable DSMRConnectorErrorEvent> eventRef = new AtomicReference<>(null); DSMREventListener listener = new DSMREventListener() { @Override public void handleTelegramReceived(P1Telegram telegram) { diff --git a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/device/p1telegram/P1TelegramParserTest.java b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/device/p1telegram/P1TelegramParserTest.java index 3839f379ef..c0ed627351 100644 --- a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/device/p1telegram/P1TelegramParserTest.java +++ b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/device/p1telegram/P1TelegramParserTest.java @@ -17,6 +17,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import java.util.Arrays; import java.util.List; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import org.openhab.binding.dsmr.internal.TelegramReaderUtil; @@ -27,31 +28,34 @@ import org.openhab.binding.dsmr.internal.device.p1telegram.P1Telegram.TelegramSt * * @author Hilbrand Bouwkamp - Initial contribution */ +@NonNullByDefault public class P1TelegramParserTest { // @formatter:off public static final List data() { return Arrays.asList(new Object[][] { - { "ace4000", 59, }, - { "dsmr_40", 39, }, - { "dsmr_42", 39, }, - { "dsmr_50", 41, }, - { "flu5", 21, }, - { "Iskra_AM550", 41, }, - { "Landis_Gyr_E350", 10, }, - { "Landis_Gyr_ZCF110", 25, }, - { "Sagemcom_XS210", 41, }, - { "smarty", 28, }, - { "smarty_with_units", 23, }, + { "ace4000", 59, 0}, + { "dsmr_40", 39, 0}, + { "dsmr_42", 39, 0}, + { "dsmr_50", 41, 0}, + { "flu5_invalid_gasmeter", 19, 1}, + { "flu5", 21, 0}, + { "Iskra_AM550", 41, 0}, + { "Landis_Gyr_E350", 10, 0}, + { "Landis_Gyr_ZCF110", 25, 0}, + { "Sagemcom_XS210", 41, 0}, + { "smarty", 28, 0}, + { "smarty_with_units", 23, 0}, }); } // @formatter:on @ParameterizedTest @MethodSource("data") - public void testParsing(final String telegramName, final int numberOfCosemObjects) { + public void testParsing(final String telegramName, final int numberOfCosemObjects, final int unknownObjects) { P1Telegram telegram = TelegramReaderUtil.readTelegram(telegramName, TelegramState.OK); - assertEquals(0, telegram.getUnknownCosemObjects().size(), "Should not have any unknown cosem objects"); + assertEquals(unknownObjects, telegram.getUnknownCosemObjects().size(), + "Should not have other than " + unknownObjects + " unknown cosem objects"); assertEquals(numberOfCosemObjects, telegram.getCosemObjects().stream().mapToInt(co -> co.getCosemValues().size()).sum(), "Expected number of objects"); diff --git a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDetectorTest.java b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDetectorTest.java index 7c3b7b3c36..06b51caeb0 100644 --- a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDetectorTest.java +++ b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDetectorTest.java @@ -24,6 +24,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import org.openhab.binding.dsmr.internal.TelegramReaderUtil; @@ -39,6 +40,7 @@ import org.openhab.binding.dsmr.internal.meter.DSMRMeterType; * * @author Hilbrand Bouwkamp - Initial contribution */ +@NonNullByDefault public class DSMRMeterDetectorTest { // @formatter:off diff --git a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryServiceTest.java b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryServiceTest.java index a140c9c473..25344d1bbb 100644 --- a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryServiceTest.java +++ b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryServiceTest.java @@ -24,6 +24,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Answers; @@ -45,14 +46,15 @@ import org.openhab.core.thing.ThingUID; * @author Hilbrand Bouwkamp - Initial contribution */ @ExtendWith(MockitoExtension.class) +@NonNullByDefault public class DSMRMeterDiscoveryServiceTest { private static final String EXPECTED_CONFIGURED_TELEGRAM = "dsmr_50"; private static final String UNREGISTERED_METER_TELEGRAM = "unregistered_meter"; - private @Mock(answer = Answers.RETURNS_DEEP_STUBS) DSMRBridgeHandler bridge; - private @Mock Thing thing; - private @Mock DSMRMeterHandler meterHandler; + private @NonNullByDefault({}) @Mock(answer = Answers.RETURNS_DEEP_STUBS) DSMRBridgeHandler bridge; + private @NonNullByDefault({}) @Mock Thing thing; + private @NonNullByDefault({}) @Mock DSMRMeterHandler meterHandler; /** * Test if discovery reports when the user has incorrectly configured the binding with the wrong meter types. diff --git a/bundles/org.openhab.binding.dsmr/src/test/resources/org/openhab/binding/dsmr/internal/flu5_invalid_gasmeter.telegram b/bundles/org.openhab.binding.dsmr/src/test/resources/org/openhab/binding/dsmr/internal/flu5_invalid_gasmeter.telegram new file mode 100644 index 0000000000..fd8d50f940 --- /dev/null +++ b/bundles/org.openhab.binding.dsmr/src/test/resources/org/openhab/binding/dsmr/internal/flu5_invalid_gasmeter.telegram @@ -0,0 +1,23 @@ +/FLU5\123456789_A + +0-0:96.1.4(50213) +0-0:96.1.1(1234567890123456789012345678) +0-0:1.0.0(191128000000W) +1-0:1.8.1(000123.456kWh) +1-0:1.8.2(000234.567kWh) +1-0:2.8.1(000345.678kWh) +1-0:2.8.2(000456.789kWh) +0-0:96.14.0(0001) +1-0:1.7.0(01.234kW) +1-0:2.7.0(00.000kW) +1-0:32.7.0(234.5V) +1-0:31.7.0(005A) +0-0:96.3.10(1) +0-0:17.0.0(999.9kW) +1-0:31.4.0(999A) +0-0:96.13.0() +0-1:24.1.0(003) +0-1:96.1.1(1234567890123456789012345678) +0-1:24.4.0(0) +0-1:24.2.3(632525252525W)(00000.000) +!70AC