Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function code 0x15 fails because of incomplete receival of bytes #1183

Closed
banana-sun opened this issue Nov 10, 2022 · 8 comments · Fixed by #1185 or #1340
Closed

Function code 0x15 fails because of incomplete receival of bytes #1183

banana-sun opened this issue Nov 10, 2022 · 8 comments · Fixed by #1185 or #1340

Comments

@banana-sun
Copy link
Contributor

banana-sun commented Nov 10, 2022

Versions

  • Python:3.10.7
  • OS:Win 10
  • Pymodbus:3.0.2
  • Modbus Hardware (if used):USB to TTL(CH340)

Pymodbus Specific

  • Server: rtu - sync
  • Client: rtu - sync

Description

What were you trying, what has happened, what went wrong, and what did you expect?

We were trying to send many bytes data to slave by function code 0x15. Finally, we found that serial received data is incomplete, especially when the baud rate is low. In function recv of pymodbus/client/serial.py, when size if not None, serial does not wait data.

Code and Logs

# code and logs here.

from pymodbus.client import ModbusSerialClient as ModbusClient
from pymodbus.file_message import FileRecord, WriteFileRecordRequest

import os
import argparse

parser = argparse.ArgumentParser(description='Firmware update on MODBus RTU.')

parser.add_argument('-f',
                    action='store',
                    type=str,
                    metavar='',
                    dest='file',
                    help='Full path and filename of the file to be downloaded. '
                         'Example: -f C:\git\Firmware_Download\my_file.bin')

args = parser.parse_args()

client = ModbusClient(method='rtu',port ='COM8', parity = 'E', baudrate=9600, timeout=5)
client.connect()

file_len = os.path.getsize(args.file)
record_len = file_len // 2
record_len = record_len if record_len < 121 else 121

file = open(args.file, 'rb')
file_data = file.read(record_len * 2)

frame = [FileRecord(reference_type=0x06, file_number=0x01, record_number=0, 
                 record_data=file_data, record_length=record_len)]
rq =  WriteFileRecordRequest(records=frame, unit=1)
print(rq.encode())
rr = client.execute(rq)

client.close()

When I modify the recv in this way, there is no problem. I don't know if there will be any other problems with this change. I hope you can help me review. Thank you.

    def recv(self, size):
        """Read data from the underlying descriptor."""
        super().recv(size)
        if not self.socket:
            raise ConnectionException(
                self.__str__()  # pylint: disable=unnecessary-dunder-call
            )
        if size is None:
            size = self._wait_for_data()
        elif size > self._in_waiting():
            self._wait_for_data()
        result = self.socket.read(size)
        return result
@janiversen
Copy link
Collaborator

Looks like you found a problem, pull requests are welcome.

@banana-sun
Copy link
Contributor Author

This problem still seems to exist, but it can occur occasionally when transferring large files.
I was using 115200 or 9600 baud rate, and after several attempts, I found a 10ms delay in the following code. I think this delay should be set according to the baud rate, because depending on the PC performance, there may be 10ms of unscheduled execution time, resulting in incomplete packets received.
After changing the delay to 20ms, I tried 1200 baud rate without this problem. I wonder if there is a better solution. thank you

    def _wait_for_data(self):
        """Wait for data."""
        size = 0
        more_data = False
        if self.params.timeout is not None and self.params.timeout:
            condition = partial(
                lambda start, timeout: (time.time() - start) <= timeout,
                timeout=self.params.timeout,
            )
        else:
            condition = partial(lambda dummy1, dummy2: True, dummy2=None)
        start = time.time()
        while condition(start):
            available = self._in_waiting()
            if (more_data and not available) or (more_data and available == size):
                break
            if available and available != size:
                more_data = True
                size = available
            time.sleep(0.01)
        return size

@janiversen
Copy link
Collaborator

You do not mention which version you are testing with ? nor do you say which file you talk about,

It sounds like a good idea to have this delay depending on the baudrate, please submit a pull request, so we can review your suggested changes (you suggested another change originally).

@janiversen janiversen reopened this Jan 30, 2023
@janiversen janiversen added this to the Version 3.2 milestone Jan 30, 2023
@banana-sun
Copy link
Contributor Author

Sorry for the lack of clarity. I just tried the latest version, 3.1.2, and it's the same problem I described. Modify _wait_for_data in pymodbus/client/serial.py.

@janiversen
Copy link
Collaborator

Yeah I do not think we made any changes to that logic. As I wrote earlier please submit a pull request, since you believe you have a solution.

@banana-sun
Copy link
Contributor Author

1 Modbus rtu function unavailable
I think the latest changes don't solve the problem, but create new bugs.
pymodbus/client/serial.py

    def recv(self, size):
        """Read data from the underlying descriptor."""
        super().recv(size)
        if not self.socket:
            raise ConnectionException(
                self.__str__()  # pylint: disable=unnecessary-dunder-call
            )
        if size is None:
            size = self._wait_for_data()
        if size > self._in_waiting():
            size = self._wait_for_data()   ### can not change size
        result = self.socket.read(size)
        return result

And as I've indicated, if do that, here's what happens.

Modbus Error: [Invalid Message] Incomplete message received, expected at least 4 bytes (21 received)

2 The delay problem mentioned above
I transfer large files via modbus's 0x15 function code. Each packet size is 256 bytes. During long time transmission, there is a probability of incomplete packet receiving. As mentioned above, I wanted to eliminate this instability by setting the latency based on baud rate.
At present, I have made the modification, and I have done the stress test. No problem was found when baud rate was set to 1200 and above. I will propose a merge request later, and I hope you can help review it.

@janiversen janiversen reopened this Feb 24, 2023
@janiversen
Copy link
Collaborator

Please as pr documentation add some debug log, so we can see what happens just before the problem.

What do you mean be a RTU function unavailable ? which one ?

The stress test you made is that with the code you included ? it is not very clear to me, it so please submit a pull request.

janiversen added a commit that referenced this issue Mar 3, 2023
* fix: Serial changes the receiving interval by baudrate.(#1183)

Co-authored-by: sunlinjie <[email protected]>
Co-authored-by: Sun <[email protected]>
Co-authored-by: jan iversen <[email protected]>
@janiversen janiversen modified the milestones: Version 3.2, Version 3.3 Mar 3, 2023
@janiversen
Copy link
Collaborator

I suppose this can be closed.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants