Skip to content

Commit

Permalink
[modbus] Avoid unnecessary IllegalArgumentException on dispose (openh…
Browse files Browse the repository at this point in the history
…ab#8297)

[modbus] Do not raise exception on no-op operation of comms. interface

If communication interface is closed, you cannot naturally interact with
the device anymore. IllegalArgumentException is raised with all methods
that would need to interact with the device.

In this commit, close() and unregisterRegularPoll methods are not
raising the exception anymore since they are essentially no-ops with
closed communication interface. After all, close automatically
unregisters all registered regular polls. Thus, it should be considered
harmless to call these methods again on a closed instance, making them
mostly idempotent.

This change was motivated by seeing the IllegalStateException in the
wild, possibly triggered by certain dispose/initialize steps of things:
https://community.openhab.org/t/snip/102809/43

Signed-off-by: Sami Salonen <ssalonen@gmail.com>
Signed-off-by: Daan Meijer <daan@studioseptember.nl>
  • Loading branch information
ssalonen authored and DaanMeijer committed Sep 1, 2020
1 parent 2b07ef5 commit be7b507
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,11 @@ public PollTask registerRegularPoll(ModbusReadRequestBlueprint request, long pol
/**
* Unregister regularly polled task
*
* If this communication interface is closed already, the method returns immediately with false return value
*
* @param task poll task to unregister
* @return whether poll task was unregistered. Poll task is not unregistered in case of unexpected errors or
* in the case where the poll task is not registered in the first place
* @throws IllegalStateException when this communication has been closed already
*/
public boolean unregisterRegularPoll(PollTask task);

Expand All @@ -93,4 +94,16 @@ public PollTask registerRegularPoll(ModbusReadRequestBlueprint request, long pol
*/
public Future<?> submitOneTimeWrite(ModbusWriteRequestBlueprint request, ModbusWriteCallback resultCallback,
ModbusFailureCallback<ModbusWriteRequestBlueprint> failureCallback);

/**
* Close this communication interface and try to free all resources associated with it
*
* Upon close, all polling tasks registered by this instance are unregistered. In addition, connections are closed
* eagerly if this was the last connection interface pointing to the endpoint.
*
* After close, the communication interface cannot be used to communicate with the device.
*
*/
@Override
public void close() throws Exception;
}
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,8 @@ public PollTask registerRegularPoll(ModbusReadRequestBlueprint request, long pol
public boolean unregisterRegularPoll(PollTask task) {
synchronized (ModbusManagerImpl.this) {
if (closed) {
throw new IllegalStateException("Communication interface is closed already!");
// Closed already, nothing to unregister
return false;
}
pollTasksRegisteredByThisCommInterface.remove(task);
ModbusSlaveConnectionFactoryImpl localConnectionFactory = connectionFactory;
Expand Down Expand Up @@ -853,7 +854,8 @@ public Future<?> submitOneTimeWrite(ModbusWriteRequestBlueprint request, ModbusW
public void close() throws Exception {
synchronized (ModbusManagerImpl.this) {
if (closed) {
throw new IllegalStateException("Communication interface is closed already!");
// Closed already, nothing to unregister
return;
}
// Iterate over all tasks registered by this communication interface, and unregister those
// We copy pollTasksRegisteredByThisCommInterface temporarily so that unregisterRegularPoll can
Expand Down

0 comments on commit be7b507

Please sign in to comment.