Update retry strategy, asyncio server and reactive servers. Test fixes #605
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is just to confirm it is running before adding lots of config (presumably with errors...).
They can be enabled in separate PRs that fix their issues.
flake8 supposedly works better here
Performance: Patch out time.time() with itertools.count() in both test_client_sync and test_transaction; client.timeout now will be the number of reads being made + 1, this also speeds up execution of test suite (from 10.8s to 4.3s in local testing, with test.test_client_sync going from 2.1s to 0.05s). Coverage, pymodbus/client/sync.py: Add tests of idle_time(), debug_enabled(), trace(...), _dump(...) to testBaseModbusClient. Add test of _recv(None) to testTlsClientRecv. Invoke send/recv instead of _send/_recv, the former of which invoke the latter, increasing coverage. Expand test of serial client instantiation to ModbusSocketFramer; while this code does not make sense it does exist, and it should therefore be tested or removed. Test is_socket_open methods. Add test of connecting to an RTU serial client. Add test of serial client with client.timeout set to None. Fix name of method for testing tcp client repr to testTcpClientRpr. Coverage, pymodbus/client/sync.py: Add test of broadcast and handle_local_echo with wrong response. Missed statements go from 861 to 830.
Raise an error when the modbus unit unexpectedly closes the connection during a receive data operation without returning any data, and log a warning if it does return some but not all data before closing the connection. Detecting and handling this here ensures that ModbusTcpClient._recv doesn't needlessly loop until it times out after the stream is closed by the peer, and also makes it certain that a response of b'' from ModbusTcpClient._recv means there was a timeout and cannot mean that the peer closed the stream, as it could mean before. The previous behavior was to not identify the remote end closing the socket, triggering a timeout, in turn causing the transaction manager to treat it as the modbus unit returning a response with errors in it (raising InvalidMessageReceivedException). This will now raise a ConnectionException, which falls through to the client, bypassing the retry mechanisms. Note that https://docs.python.org/3/library/socket.html does not contain a full description on the socket.recv method; see also pydoc for socket.socket.recv.
Change the _sync method so that when no specific size is requested it fetches up to 4096 bytes instead of 1 byte at a time from the operating system. It's not clear what the reason was for setting it 1 byte. It was introduced in pymodbus commit: 9d2c864 Patch: 3b490fe But there's no rationale there for setting it to 1 byte specifically. While it may have been to handle some obscure operating system with a poor network stack implementation this should have been documented, as it is written it comes across as that the implementer didn't understand how the TCP/stream socket networking API works. Note that https://docs.python.org/3/library/socket.html does not contain a full description on the socket.recv method; see also pydoc for socket.socket.recv.
Synchronized TCP client that performs detail logging of network activity, for diagnosis of network related issues.
For TCP/IP when no data is received at all before a timeout the phrasing "incomplete message received" in error messages in ModbusTransactionManager._recv is misleading. This wording assertively states that a message has been received from the modbus unit, and when no actual data has received this would imply that a message with an empty payload has been received by the application, which is not possible with TCP/IP. More likely it's just a timeout, without any data at all having been received from the modbus unit.
It's not just not receiving the right number of bytes when there is a known number of bytes to read that is an issue, but also when there is no response at all to a read of None bytes (_recv isn't invoked unless one expects a response).
Always close connection on no response/error response from modbus unit. This attempts to resolve issue #538, where the response of a previously timed out request over synchronized TCP is being returned as the response to the current request, by leaving the connection open only if there is no unanswered response. This commit only touches the code for synchronized clients, and only seeks to resolve it for TCP based clients, though the situation for UDP clients should improve as well.
This was referenced Feb 13, 2021
Closed
|
SonarCloud Quality Gate failed.
|
This was referenced Mar 2, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.