[modbus] Modbus transport API simplification

bundles/org.openhab.binding.modbus/src/main/java/org/openhab/binding/modbus/handler/ModbusEndpointThingHandler.java

@@ -16,8 +16,7 @@
 import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.smarthome.core.common.registry.Identifiable;
 import org.eclipse.smarthome.core.thing.ThingUID;
-import org.openhab.io.transport.modbus.ModbusManager;
-import org.openhab.io.transport.modbus.endpoint.ModbusSlaveEndpoint;
+import org.openhab.io.transport.modbus.ModbusCommunicationInterface;
 
 /**
  * Base interface for thing handlers of endpoint things
@@ -29,13 +28,13 @@
 public interface ModbusEndpointThingHandler extends Identifiable<ThingUID> {
 
  /**
- * Gets the {@link ModbusSlaveEndpoint} represented by the thing
+ * Gets the {@link ModbusCommunicationInterface} represented by the thing
  *
- * Note that the endpoint can be <code>null</code> in case of incomplete initialization
+ * Note that this can be <code>null</code> in case of incomplete initialization
  *
- * @return endpoint represented by this thing handler
+ * @return communication interface represented by this thing handler
  */
- public @Nullable ModbusSlaveEndpoint asSlaveEndpoint();
+ public @Nullable ModbusCommunicationInterface getCommunicationInterface();
 
  /**
  * Get Slave ID, also called as unit id, represented by the thing
@@ -45,13 +44,6 @@ public interface ModbusEndpointThingHandler extends Identifiable<ThingUID> {
  */
  public int getSlaveId() throws EndpointNotInitializedException;
 
- /**
- * Get {@link ModbusManager}
- *
- * @return reference to ModbusManager
- */
- public ModbusManager getModbusManager();
-
  /**
  * Return true if auto discovery is enabled for this endpoint
  *

bundles/org.openhab.binding.modbus/src/main/java/org/openhab/binding/modbus/handler/ModbusPollerThingHandler.java

@@ -34,12 +34,11 @@
 import org.openhab.binding.modbus.internal.config.ModbusPollerConfiguration;
 import org.openhab.binding.modbus.internal.handler.ModbusDataThingHandler;
 import org.openhab.io.transport.modbus.AsyncModbusReadResult;
-import org.openhab.io.transport.modbus.ModbusManager;
+import org.openhab.io.transport.modbus.ModbusCommunicationInterface;
 import org.openhab.io.transport.modbus.ModbusReadCallback;
 import org.openhab.io.transport.modbus.ModbusReadFunctionCode;
 import org.openhab.io.transport.modbus.ModbusReadRequestBlueprint;
 import org.openhab.io.transport.modbus.PollTask;
-import org.openhab.io.transport.modbus.endpoint.ModbusSlaveEndpoint;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -159,17 +158,16 @@ public ModbusPollerReadRequest(ModbusPollerConfiguration config,
  private ModbusPollerConfiguration config;
  private long cacheMillis;
  private volatile @Nullable PollTask pollTask;
- private volatile @Nullable ModbusSlaveEndpoint endpoint;
  private volatile @Nullable ModbusReadRequestBlueprint request;
- private ModbusManager modbusManager;
  private volatile boolean disposed;
  private volatile List<ModbusReadCallback> childCallbacks = new CopyOnWriteArrayList<>();
+ @NonNullByDefault({})
+ private ModbusCommunicationInterface comms;
 
  private ReadCallbackDelegator callbackDelegator = new ReadCallbackDelegator();
 
- public ModbusPollerThingHandler(Bridge bridge, ModbusManager modbusManager) {
+ public ModbusPollerThingHandler(Bridge bridge) {
  super(bridge);
- this.modbusManager = modbusManager;
  }
 
  @Override
@@ -211,6 +209,7 @@ public synchronized void initialize() {
  updateStatus(ThingStatus.OFFLINE);
  }
  this.callbackDelegator.resetCache();
+ comms = null;
  disposed = false;
  logger.trace("Initializing {} from status {}", this.getThing().getUID(), this.getThing().getStatus());
  try {
@@ -233,6 +232,7 @@ public synchronized void dispose() {
  disposed = true;
  unregisterPollTask();
  this.callbackDelegator.resetCache();
+ comms = null;
  }
 
  /**
@@ -248,11 +248,11 @@ public synchronized void unregisterPollTask() {
  PollTask localPollTask = this.pollTask;
  if (localPollTask != null) {
  logger.debug("Unregistering polling from ModbusManager");
- modbusManager.unregisterRegularPoll(localPollTask);
+ comms.unregisterRegularPoll(localPollTask);
  }
  this.pollTask = null;
  request = null;
- endpoint = null;
+ comms = null;
  updateStatus(ThingStatus.OFFLINE);
  }
 
@@ -278,14 +278,14 @@ private synchronized void registerPollTask() throws EndpointNotInitializedExcept
  logger.debug("No bridge handler available -- aborting init for {}", this);
  return;
  }
- ModbusSlaveEndpoint localEndpoint = slaveEndpointThingHandler.asSlaveEndpoint();
- this.endpoint = localEndpoint;
- if (localEndpoint == null) {
+ ModbusCommunicationInterface localComms = slaveEndpointThingHandler.getCommunicationInterface();
+ if (localComms == null) {
  updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE, String.format(
  "Bridge '%s' not completely initialized", Optional.ofNullable(getBridge()).map(b -> b.getLabel())));
- logger.debug("Bridge not initialized fully (no endpoint) -- aborting init for {}", this);
+ logger.debug("Bridge not initialized fully (no communication interface) -- aborting init for {}", this);
  return;
  }
+ this.comms = localComms;
 
  ModbusReadRequestBlueprint localRequest = new ModbusPollerReadRequest(config, slaveEndpointThingHandler);
  this.request = localRequest;
@@ -295,8 +295,7 @@ private synchronized void registerPollTask() throws EndpointNotInitializedExcept
  updateStatus(ThingStatus.ONLINE, ThingStatusDetail.NONE, "Not polling");
  } else {
  logger.debug("Registering polling with ModbusManager");
- pollTask = modbusManager.registerRegularPoll(localEndpoint, localRequest, config.getRefresh(), 0,
- callbackDelegator);
+ pollTask = localComms.registerRegularPoll(localRequest, config.getRefresh(), 0, callbackDelegator);
  assert pollTask != null;
  updateStatus(ThingStatus.ONLINE);
  }
@@ -330,15 +329,6 @@ public void childHandlerDisposed(ThingHandler childHandler, Thing childThing) {
  }
  }
 
- /**
- * Get {@link ModbusManager}
- *
- * @return ModbusManger instance
- */
- public ModbusManager getModbusManager() {
- return modbusManager;
- }
-
  /**
  * Return {@link ModbusReadRequestBlueprint} represented by this thing.
  *
@@ -351,14 +341,12 @@ public ModbusManager getModbusManager() {
  }
 
  /**
- * Return {@link ModbusSlaveEndpoint} associated with this thing
- *
- * Note that endpoint might be <code>null</code> in case initialization is not complete.
+ * Get communication interface associated with this poller
  *
- * @return endpoint associated with this poller
+ * @return
  */
- public @Nullable ModbusSlaveEndpoint getEndpoint() {
- return endpoint;
+ public ModbusCommunicationInterface getCommunicationInterface() {
+ return comms;
  }
 
  /**
@@ -383,9 +371,9 @@ public void refresh() {
  // cache expired, poll new data
  logger.debug("Poller {} received refresh() but the cache is not applicable. Polling new data",
  getThing().getUID());
- ModbusSlaveEndpoint localEndpoint = endpoint;
- if (localEndpoint != null) {
- modbusManager.submitOneTimePoll(localEndpoint, localRequest, callbackDelegator);
+ ModbusCommunicationInterface localComms = comms;
+ if (localComms != null) {
+ localComms.submitOneTimePoll(localRequest, callbackDelegator);
  }
  }
  }

bundles/org.openhab.binding.modbus/src/main/java/org/openhab/binding/modbus/internal/ModbusHandlerFactory.java

@@ -74,7 +74,7 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
  return new ModbusSerialThingHandler((Bridge) thing, manager);
  } else if (thingTypeUID.equals(THING_TYPE_MODBUS_POLLER)) {
  logger.debug("createHandler Modbus poller");
- return new ModbusPollerThingHandler((Bridge) thing, manager);
+ return new ModbusPollerThingHandler((Bridge) thing);
  } else if (thingTypeUID.equals(THING_TYPE_MODBUS_DATA)) {
  logger.debug("createHandler data");
  return new ModbusDataThingHandler(thing);

bundles/org.openhab.binding.modbus/src/main/java/org/openhab/binding/modbus/internal/handler/AbstractModbusEndpointThingHandler.java

@@ -22,6 +22,7 @@
 import org.eclipse.smarthome.core.types.Command;
 import org.openhab.binding.modbus.handler.ModbusEndpointThingHandler;
 import org.openhab.binding.modbus.internal.ModbusConfigurationException;
+import org.openhab.io.transport.modbus.ModbusCommunicationInterface;
 import org.openhab.io.transport.modbus.ModbusManager;
 import org.openhab.io.transport.modbus.ModbusManagerListener;
 import org.openhab.io.transport.modbus.endpoint.EndpointPoolConfiguration;
@@ -49,6 +50,8 @@ public abstract class AbstractModbusEndpointThingHandler<E extends ModbusSlaveEn
  @Nullable
  protected volatile EndpointPoolConfiguration poolConfiguration;
  private final Logger logger = LoggerFactory.getLogger(AbstractModbusEndpointThingHandler.class);
+ @NonNullByDefault({})
+ private ModbusCommunicationInterface comms;
 
  public AbstractModbusEndpointThingHandler(Bridge bridge, ModbusManager modbusManager) {
  super(bridge);
@@ -76,7 +79,7 @@ public void initialize() {
  throw new IllegalArgumentException("endpoint null after configuration!");
  }
  modbusManager.addListener(this);
- modbusManager.setEndpointPoolConfiguration(endpoint, poolConfiguration);
+ comms = modbusManager.newModbusCommunicationInterface(endpoint, poolConfiguration);
  updateStatus(ThingStatus.ONLINE);
  } catch (ModbusConfigurationException e) {
  logger.debug("Exception during initialization", e);
@@ -91,16 +94,13 @@ public void initialize() {
  @Override
  public void dispose() {
  modbusManager.removeListener(this);
- }
-
- @Override
- public @Nullable ModbusSlaveEndpoint asSlaveEndpoint() {
- return endpoint;
- }
-
- @Override
- public ModbusManager getModbusManager() {
- return modbusManager;
+ try {
+ comms.close();
+ } catch (Exception e) {
+ logger.error("Error closing modbus communication interface", e);
+ } finally {
+ comms = null;
+ }
  }
 
  @Override
@@ -119,6 +119,11 @@ public void onEndpointPoolConfigurationSet(ModbusSlaveEndpoint otherEndpoint,
  }
  }
 
+ @Override
+ public @Nullable ModbusCommunicationInterface getCommunicationInterface() {
+ return comms;
+ }
+
  @Override
  public abstract int getSlaveId();
 

bundles/org.openhab.binding.modbus/src/main/java/org/openhab/binding/modbus/internal/handler/ModbusDataThingHandler.java

@@ -65,9 +65,9 @@
 import org.openhab.io.transport.modbus.AsyncModbusWriteResult;
 import org.openhab.io.transport.modbus.BitArray;
 import org.openhab.io.transport.modbus.ModbusBitUtilities;
+import org.openhab.io.transport.modbus.ModbusCommunicationInterface;
 import org.openhab.io.transport.modbus.ModbusConstants;
 import org.openhab.io.transport.modbus.ModbusConstants.ValueType;
-import org.openhab.io.transport.modbus.ModbusManager;
 import org.openhab.io.transport.modbus.ModbusReadCallback;
 import org.openhab.io.transport.modbus.ModbusReadFunctionCode;
 import org.openhab.io.transport.modbus.ModbusReadRequestBlueprint;
@@ -77,10 +77,8 @@
 import org.openhab.io.transport.modbus.ModbusWriteCoilRequestBlueprint;
 import org.openhab.io.transport.modbus.ModbusWriteRegisterRequestBlueprint;
 import org.openhab.io.transport.modbus.ModbusWriteRequestBlueprint;
-import org.openhab.io.transport.modbus.endpoint.ModbusSlaveEndpoint;
 import org.openhab.io.transport.modbus.exception.ModbusConnectionException;
 import org.openhab.io.transport.modbus.exception.ModbusTransportException;
-import org.openhab.io.transport.modbus.internal.BasicWriteTask;
 import org.openhab.io.transport.modbus.json.WriteRequestJsonUtilities;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.FrameworkUtil;
@@ -143,8 +141,8 @@ public class ModbusDataThingHandler extends BaseThingHandler implements ModbusRe
  private volatile @Nullable ModbusReadFunctionCode functionCode;
  private volatile @Nullable ModbusReadRequestBlueprint readRequest;
  private volatile long updateUnchangedValuesEveryMillis;
- private volatile @Nullable ModbusSlaveEndpoint slaveEndpoint;
- private volatile @Nullable ModbusManager modbusManager;
+ @NonNullByDefault({})
+ private volatile ModbusCommunicationInterface comms;
  private volatile boolean isWriteEnabled;
  private volatile boolean isReadEnabled;
  private volatile boolean writeParametersHavingTransformationOnly;
@@ -168,8 +166,7 @@ public synchronized void handleCommand(ChannelUID channelUID, Command command) {
  logger.trace("Thing {} '{}' received command '{}' to channel '{}'", getThing().getUID(), getThing().getLabel(),
  command, channelUID);
  ModbusDataConfiguration config = this.config;
- ModbusManager manager = this.modbusManager;
- if (config == null || manager == null) {
+ if (config == null) {
  return;
  }
 
@@ -223,14 +220,12 @@ public synchronized void handleCommand(ChannelUID channelUID, Command command) {
 
  ModbusWriteRequestBlueprint request = requestFromCommand(channelUID, command, config, transformedCommand.get(),
  writeStart);
- ModbusSlaveEndpoint slaveEndpoint = this.slaveEndpoint;
- if (request == null || slaveEndpoint == null) {
+ if (request == null) {
  return;
  }
 
- BasicWriteTask writeTask = new BasicWriteTask(slaveEndpoint, request, this);
- logger.trace("Submitting write task: {}", writeTask);
- manager.submitOneTimeWrite(writeTask);
+ logger.trace("Submitting write task {} to endpoint {}", request, comms.getEndpoint());
+ comms.submitOneTimeWrite(request, this);
  }
 
  /**
@@ -316,9 +311,8 @@ public synchronized void handleCommand(ChannelUID channelUID, Command command) {
  }
 
  private void processJsonTransform(Command command, String transformOutput) {
- ModbusSlaveEndpoint slaveEndpoint = this.slaveEndpoint;
- ModbusManager manager = this.modbusManager;
- if (slaveEndpoint == null || manager == null) {
+ ModbusCommunicationInterface localComms = this.comms;
+ if (localComms == null) {
  return;
  }
  Collection<ModbusWriteRequestBlueprint> requests;
@@ -331,9 +325,10 @@ private void processJsonTransform(Command command, String transformOutput) {
  return;
  }
 
- requests.stream().map(request -> new BasicWriteTask(slaveEndpoint, request, this)).forEach(writeTask -> {
- logger.trace("Submitting write task: {} (based from transformation {})", writeTask, transformOutput);
- manager.submitOneTimeWrite(writeTask);
+ requests.stream().forEach(request -> {
+ logger.trace("Submitting write request: {} to endpoint {} (based from transformation {})", request,
+ localComms.getEndpoint(), transformOutput);
+ localComms.submitOneTimeWrite(request, this);
  });
  }
 
@@ -362,16 +357,14 @@ public synchronized void initialize() {
  // Write-only thing, parent is endpoint
  ModbusEndpointThingHandler endpointHandler = (ModbusEndpointThingHandler) bridgeHandler;
  slaveId = endpointHandler.getSlaveId();
- slaveEndpoint = endpointHandler.asSlaveEndpoint();
- modbusManager = endpointHandler.getModbusManager();
+ comms = endpointHandler.getCommunicationInterface();
  childOfEndpoint = true;
  functionCode = null;
  readRequest = null;
  } else {
  ModbusPollerThingHandler localPollerHandler = (ModbusPollerThingHandler) bridgeHandler;
  pollerHandler = localPollerHandler;
  ModbusReadRequestBlueprint localReadRequest = localPollerHandler.getRequest();
- ModbusSlaveEndpoint localEndpoint = localPollerHandler.getEndpoint();
  if (localReadRequest == null) {
  logger.debug("Poller {} '{}' has no read request -- configuration is changing?", bridge.getUID(),
  bridge.getLabel());
@@ -381,9 +374,8 @@ public synchronized void initialize() {
  }
  readRequest = localReadRequest;
  slaveId = localReadRequest.getUnitID();
- slaveEndpoint = localEndpoint;
  functionCode = localReadRequest.getFunctionCode();
- modbusManager = localPollerHandler.getModbusManager();
+ comms = localPollerHandler.getCommunicationInterface();
  pollStart = localReadRequest.getReference();
  childOfEndpoint = false;
  }
@@ -413,8 +405,7 @@ public synchronized void dispose() {
  writeStart = null;
  pollStart = 0;
  slaveId = 0;
- slaveEndpoint = null;
- modbusManager = null;
+ comms = null;
  functionCode = null;
  readRequest = null;
  isWriteEnabled = false;

bundles/org.openhab.binding.modbus/src/main/java/org/openhab/binding/modbus/internal/handler/ModbusSerialThingHandler.java

@@ -15,7 +15,6 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Optional;
-import java.util.function.Supplier;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;