Hi
I have been thinking how the Helios use case would look like with the API in reality.
Helios has some specialties that might need some work on the binding side, especially the fact that one “Helios transaction” consists of many modbus requests (e.g. write followed by read). Say you have two transactions, both consisting of write followed by read: write1-read1 & write2-read2. We probably want to ensure the nice order (write1 -> read1 -> write2 -> read2) instead of mixed order (write1 -> write2 -> read1 -> read2). The safest in this case would be to “synchronize transactions” on the binding side.
In the below example (implemented as unit test), Striped from Guava library is used to “lock” the endpoint for single transaction at once. Note that the ModbusSlaveEndpoint interface provided by the modbus transport guarantees that equals and hashing is implemented correctly as required by Striped.
In case you want to abort ongoing requests, you can fire off a thread and interrupt it (see below how interruption has been handled).
I have been experimenting with CompletableFuture as well but I think it might be overengineering and the below code is actually pretty readable.
import com.google.common.util.concurrent.Striped;
// locks for "Helios transactions"
Striped<Lock> locks = Striped.lazyWeakLock(5);
@Test
public void test() throws InterruptedException {
Manager modbusManager = new Manager();
ModbusTCPSlaveEndpoint endpoint = new ModbusTCPSlaveEndpoint("localhost", 55502);
Lock endpointLock = locks.get(endpoint);
try {
endpointLock.lock();
// submit first
{
final AtomicBoolean success = new AtomicBoolean();
final CountDownLatch completed = new CountDownLatch(1);
WriteTask write = new WriteTaskImpl(endpoint,
new ModbusWriteRegisterRequestBlueprintImpl(0, 0, new ModbusRegisterArrayImpl(2), true, 15),
new ModbusWriteCallback() {
@Override
public void onWriteResponse(ModbusWriteRequestBlueprint request, ModbusResponse response) {
completed.countDown();
success.set(Boolean.TRUE);
}
@Override
public void onError(ModbusWriteRequestBlueprint request, Exception error) {
completed.countDown();
success.set(Boolean.FALSE);
}
});
ScheduledFuture<?> writeFuture = modbusManager.submitOneTimeWrite(write);
try {
completed.await();
} catch (InterruptedException e) {
// TODO Auto-generated catch block
e.printStackTrace();
writeFuture.cancel(true);
throw e;
}
if (!Boolean.TRUE.equals(success)) {
// failure, abort?
// TODO: log
return;
}
}
// submit second
{
final AtomicBoolean success = new AtomicBoolean();
final CountDownLatch completed = new CountDownLatch(1);
// Proceed with next query
}
} finally {
endpointLock.unlock();
}
}
Do you think something like this would work for you?
Btw, please note that helios-based-on-modbus2-transport is already obsolete. The latest Modbus binding and transport code can be found from modbus-openhab2-native-binding branch. I suggest you build Helios binding on top of that branch, using the transport bundle. Prepare to rebase your changes on top of my changes if necessary.
Please note that the new transport contains the implementations for requests, previously my example included implementation for WriteTask. Check all the public implementations and interfaces from source code: java/org/openhab/io/transport/modbus.
Finally, should you post a “WIP” PR (see here for motivation) to Github?
Best,
Sami