Changed read to readFully in TCP transports to get rid of fragmented packets

[denis-ftc-denisov]

As discussed here https://github.com/openhab/jamod/pull/10 i've checked if we change read to readFully when appropriate and let socket handle the timeout. And without surprises, it works.

Also, i've had a look at SerialTransport and it seems enableReceiveTimeout is enough for there. As stated in docs https://docs.oracle.com/cd/E17802_01/products/products/javacomm/reference/api/javax/comm/CommPort.html#enableReceiveTimeout(int) timeout makes read call return, not throw an exception, so changing to readFully could lead to unwanted hangup.

[ssalonen]

Excellent that this is finally fixed, this will really help with the crc errors some people have been seeing.

LGTM.

Methods now raise EOFException. Should be fine as the modbus transport in openhab-core already handles IOException which is parent of EOFException.

[ssalonen]

@kaikreuzer do you want to do the honours and merge and release?

[LeviPieroni]

Hello, @denis-ftc-denisov is there a way that i can test the fix?

[denis-ftc-denisov]

Hello @LeviPieroni ! My setup was like that: arduino as modbus slave on serial port. Then, serial port was exported as TCP port via ser2net and then OpenHAB was configured to use that exported TCP port as modbus RTU-over-TCP endpoint. Without the fix reads from arduino were failing because not full packet was received by OpenHAB. With the fix reads work correctly.

[LeviPieroni]

Do you have a jar file that i can test in openhab?

[kaikreuzer]

@kaikreuzer do you want to do the honours and merge and release?

Done! New version is 1.3.2.OH. As Bintray is about to be shut down, I have changed the distribution location to now be https://openhab.jfrog.io/artifactory/libs-3rdparty-local/net/wimpi/jamod/1.3.2.OH/. This should be directly available to the openHAB build, so feel free to create according PRs for openhab-addons!

[ssalonen]

Thanks @kaikreuzer

@denis-ftc-denisov can you please introduce PR for openhab-core, bumping up the version? See https://github.com/openhab/openhab-core/pull/1965 for an example

[ssalonen]

@LeviPieroni bundle:install https://openhab.jfrog.io/artifactory/libs-3rdparty-local/net/wimpi/jamod/1.3.2.OH/jamod-1.3.2.OH.jar should probably work out, just ensure the right version is activated

[LeviPieroni]

@ssalonen the bundel does not install like that?I also cant find any bundel that is named jamod in the console?I also tried to place the jar in the addons folder but its not loading.How can i ensure the right version is activated if i can not find a bundel named jamod?

[ssalonen]

@LeviPieroni it might show up as net.wimpi.modbus. Any luck?

bundle:list -s|grep wimpi

[LeviPieroni]

There is no bundle with that name? Bundle IDs: Error executing command: Error installing bundles: Unable to install bundle https://openhab.jfrog.io/artifactory/libs-3rdparty-local/net/wimpi/jamod/1.3.2.OH/jamod-1.3.2.OH.jar: org.osgi.framework.BundleException: OSGi R3 bundle not supported

[ssalonen]

@LeviPieroni I just checked how this modbus library is used in modbus transport bundle. It looks like it is embedded to the transport bundle, and it might not be possible to update it separately.

I think you actually have to wait for new version of modbus transport (Re https://github.com/openhab/jamod/pull/11#issuecomment-817252606), one that bundles-in this fix.

[LeviPieroni]

Ok thanks i wait for the fix.

[denis-ftc-denisov]

@LeviPieroni you can try this version (this is one i'm using now). org.openhab.core.io.transport.modbus-3.1.0-SNAPSHOT.jar.zip

To install it you should remove modbus transport bundle from OpenHAB distribution and put this jar into addons folder.

[denis-ftc-denisov]

@denis-ftc-denisov can you please introduce PR for openhab-core, bumping up the version? See openhab/openhab-core#1965 for an example

@ssalonen done that https://github.com/openhab/openhab-core/pull/2284

[ssalonen]

I have noticed that wrong method was patched in ModbusTCPTransport -- now the fix was applied to readRequest (used with Modbus servers but not in client) although it should have been getResponse.

I will introduce a fix for this