openhab / jamod

A fork of Java Modbus Library (jamod) - http://jamod.sourceforge.net/
Apache License 2.0
17 stars 26 forks source link

Modbus TCP client to read full messages, even if fragmented/segmented #13

Closed ssalonen closed 3 years ago

ssalonen commented 3 years ago

Additional correction to PR #11. PR #11 Only fixed Modbus TCP server implementation (readRequest). This is not used in openHAB binding at all.

Pre-compiled modbus transport that uses this version of jamod: org.openhab.core.io.transport.modbus-3.1.0-2021-04-20-tcp-transport-fix-SNAPSHOT.zip

Intention to fix SMA inverter behaviour which seems to split the modbus response in multiple tcp packets. Apparently this seems to be rare otherwise. See https://community.openhab.org/t/modbus-binding-with-sma-inverter-missing-lower-byte/119467/29 and https://github.com/openhab/openhab-addons/issues/5347 .

TODO

Signed-off-by: Sami Salonen ssalonen@gmail.com

ssalonen commented 3 years ago

Note to reviewers. Same comment I had in PR #11 applies here:

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

ssalonen commented 3 years ago

'Integration' tests against temporary modbus tcp server in openhab-core pass with the modified jamod:

[INFO] Results: [INFO] [INFO] Tests run: 1074, Failures: 0, Errors: 0, Skipped: 0 [INFO]

This confirms that the basic functionality should not be broken with this fix.

ssalonen commented 3 years ago

Positive test result from the user https://community.openhab.org/t/modbus-binding-with-sma-inverter-missing-lower-byte/119467/35?u=ssalonen

This resolves long-standing issue with SMA inverters, see https://github.com/openhab/openhab-addons/issues/5347

ssalonen commented 3 years ago

@kaikreuzer could you please review? Perhaps we can get this to the next oh release as well.

kaikreuzer commented 3 years ago

@ssalonen Shall I do a new release or do you expect further changes in the next days/weeks that we could cumulate?

ssalonen commented 3 years ago

@kaikreuzer thanks. I'm not expecting any further changes in the short term... We have had exceptionally many issues identified now recently :)

kaikreuzer commented 3 years ago

@ssalonen Version 1.3.3.OH has been deployed to our Artifactory.