[modbus] fix defaults for tcp and serial things and some other minor cleanup (#10147)

* [modbus] More strict nullness. Remove apache.commons.lang from itests
* [modbus] Defaults for tcp and serial things according to docs
* [modbus] further explicit defaults
* [modbus] document default encoding for serial.
RTU is pretty much the only one used in the field.
Previous default was ascii implicitly.
* [modbus] verify defaults are used for undefined configuration parameters

Signed-off-by: Sami Salonen <ssalonen@gmail.com>
This commit is contained in:
Sami Salonen 2021-02-17 12:41:40 +02:00 committed by GitHub
parent fe496c7389
commit fbf55d5886
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 47 additions and 18 deletions

View File

@ -146,7 +146,7 @@ Basic parameters
| stopBits | text | ✓ | | Stop bits. Valid values are: `"1.0"`, `"1.5"`, `"2.0"`. | |
| parity | text | ✓ | | Parity. Valid values are: `"none"`, `"even"`, `"odd"`. | |
| dataBits | integer | ✓ | | Data bits. Valid values are: `5`, `6`, `7` and `8`. | |
| encoding | text | ✓ | | Encoding. Valid values are: `"ascii"`, `"rtu"`, `"bin"`. | |
| encoding | text | | `"rtu"` | Encoding. Valid values are: `"ascii"`, `"rtu"`, `"bin"`. | |
| echo | boolean | | `false` | Flag for setting the RS485 echo mode. This controls whether we should try to read back whatever we send on the line, before reading the response. Valid values are: `true`, `false`. | |
Advanced parameters

View File

@ -24,19 +24,19 @@ import org.eclipse.jdt.annotation.Nullable;
@NonNullByDefault
public class ModbusSerialConfiguration {
private @Nullable String port;
private int id;
private int id = 1;
private int baud;
private @Nullable String stopBits;
private @Nullable String parity;
private int dataBits;
private @Nullable String encoding;
private String encoding = "rtu";
private boolean echo;
private int receiveTimeoutMillis;
private @Nullable String flowControlIn;
private @Nullable String flowControlOut;
private int timeBetweenTransactionsMillis;
private int connectMaxTries;
private int connectTimeoutMillis;
private int receiveTimeoutMillis = 1500;
private String flowControlIn = "none";
private String flowControlOut = "none";
private int timeBetweenTransactionsMillis = 35;
private int connectMaxTries = 1;
private int connectTimeoutMillis = 10_000;
private boolean enableDiscovery;
public @Nullable String getPort() {

View File

@ -25,12 +25,12 @@ import org.eclipse.jdt.annotation.Nullable;
public class ModbusTcpConfiguration {
private @Nullable String host;
private int port;
private int id;
private int timeBetweenTransactionsMillis;
private int id = 1;
private int timeBetweenTransactionsMillis = 60;
private int timeBetweenReconnectMillis;
private int connectMaxTries;
private int connectMaxTries = 1;
private int reconnectAfterMillis;
private int connectTimeoutMillis;
private int connectTimeoutMillis = 10_000;
private boolean enableDiscovery;
private boolean rtuEncoded;

View File

@ -45,7 +45,7 @@ public abstract class AbstractModbusEndpointThingHandler<E extends ModbusSlaveEn
protected volatile @Nullable C config;
protected volatile @Nullable E endpoint;
protected ModbusManager modbusManager;
protected volatile @Nullable EndpointPoolConfiguration poolConfiguration;
protected volatile @NonNullByDefault({}) EndpointPoolConfiguration poolConfiguration;
private final Logger logger = LoggerFactory.getLogger(AbstractModbusEndpointThingHandler.class);
private @NonNullByDefault({}) ModbusCommunicationInterface comms;

View File

@ -27,8 +27,6 @@ Fragment-Host: org.openhab.binding.modbus
#
-runbundles: \
javax.measure.unit-api;version='[1.0.0,1.0.1)',\
org.apache.commons.io;version='[2.2.0,2.2.1)',\
org.apache.commons.lang;version='[2.6.0,2.6.1)',\
org.apache.felix.configadmin;version='[1.9.8,1.9.9)',\
org.apache.felix.scr;version='[2.1.10,2.1.11)',\
org.eclipse.equinox.event;version='[1.4.300,1.4.301)',\

View File

@ -107,13 +107,13 @@ public abstract class AbstractModbusOSGiTest extends JavaOSGiTest {
private final Logger logger = LoggerFactory.getLogger(AbstractModbusOSGiTest.class);
protected @Mock @NonNullByDefault({}) ModbusManager mockedModbusManager;
protected @NonNullByDefault({}) ModbusManager realModbusManager;
protected @NonNullByDefault({}) ManagedThingProvider thingProvider;
protected @NonNullByDefault({}) ManagedItemProvider itemProvider;
protected @NonNullByDefault({}) ManagedItemChannelLinkProvider itemChannelLinkProvider;
protected @NonNullByDefault({}) ItemRegistry itemRegistry;
protected @NonNullByDefault({}) CoreItemFactory coreItemFactory;
private @NonNullByDefault({}) ModbusManager realModbusManager;
private Set<Item> addedItems = new HashSet<>();
private Set<Thing> addedThings = new HashSet<>();
private Set<ItemChannelLink> addedLinks = new HashSet<>();

View File

@ -14,7 +14,7 @@ package org.openhab.binding.modbus.tests;
import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.*;
import java.util.Objects;
@ -46,6 +46,7 @@ public class ModbusTcpThingHandlerTest extends AbstractModbusOSGiTest {
@Test
public void testInitializeAndSlaveEndpoint() throws EndpointNotInitializedException {
// Using mocked modbus manager
Configuration thingConfig = new Configuration();
thingConfig.put("host", "thisishost");
thingConfig.put("port", 44);
@ -81,6 +82,8 @@ public class ModbusTcpThingHandlerTest extends AbstractModbusOSGiTest {
@Test
public void testTwoDifferentEndpointWithDifferentParameters() {
// Real implementation needed to validate this behaviour
swapModbusManagerToReal();
// thing1
{
Configuration thingConfig = new Configuration();
@ -95,6 +98,18 @@ public class ModbusTcpThingHandlerTest extends AbstractModbusOSGiTest {
ModbusTcpThingHandler thingHandler = (ModbusTcpThingHandler) thing.getHandler();
assertNotNull(thingHandler);
EndpointPoolConfiguration expectedPoolConfiguration = new EndpointPoolConfiguration();
expectedPoolConfiguration.setConnectMaxTries(1);
expectedPoolConfiguration.setInterTransactionDelayMillis(1);
// defaults
expectedPoolConfiguration.setConnectTimeoutMillis(10_000);
expectedPoolConfiguration.setInterConnectDelayMillis(0);
expectedPoolConfiguration.setReconnectAfterMillis(0);
assertEquals(expectedPoolConfiguration, realModbusManager
.getEndpointPoolConfiguration(new ModbusTCPSlaveEndpoint("thisishost", 44, false)));
}
{
Configuration thingConfig = new Configuration();
@ -145,6 +160,22 @@ public class ModbusTcpThingHandlerTest extends AbstractModbusOSGiTest {
assertThat(thing.getStatus(), is(equalTo(ThingStatus.OFFLINE)));
assertThat(thing.getStatusInfo().getStatusDetail(), is(equalTo(ThingStatusDetail.CONFIGURATION_ERROR)));
}
{
//
// Ensure the right EndpointPoolConfiguration is still in place
//
EndpointPoolConfiguration expectedPoolConfiguration = new EndpointPoolConfiguration();
expectedPoolConfiguration.setConnectMaxTries(1);
expectedPoolConfiguration.setInterTransactionDelayMillis(1); // Note: not 100
// defaults
expectedPoolConfiguration.setConnectTimeoutMillis(10_000);
expectedPoolConfiguration.setInterConnectDelayMillis(0);
expectedPoolConfiguration.setReconnectAfterMillis(0);
assertEquals(expectedPoolConfiguration, realModbusManager
.getEndpointPoolConfiguration(new ModbusTCPSlaveEndpoint("thisishost", 44, false)));
}
}
@Test