u9n / dlms-cosem

A Python library for DLMS/COSEM
Other
80 stars 41 forks source link

Socket recv method may return only partial data #35

Closed Mapomme78 closed 3 years ago

Mapomme78 commented 3 years ago

In one of my tests, I have a DLMS server (over TCP) which responds slowly. This results in the following error message: Traceback (most recent call last): File "", line 1, in File "C:\Program Files (x86)\Python38-32\lib\site-packages\dlms_cosem\clients\dlms_client.py", line 185, in get self.send( File "C:\Program Files (x86)\Python38-32\lib\site-packages\dlms_cosem\clients\dlms_client.py", line 327, in send response_bytes = self.io_interface.send(data) File "C:\Program Files (x86)\Python38-32\lib\site-packages\dlms_cosem\clients\blocking_tcp_transport.py", line 92, in send return self.recv() File "C:\Program Files (x86)\Python38-32\lib\site-packages\dlms_cosem\clients\blocking_tcp_transport.py", line 101, in recv header = WrapperHeader.from_bytes(self.tcp_socket.recv(8)) File "C:\Program Files (x86)\Python38-32\lib\site-packages\dlms_cosem\protocol\wrappers.py", line 62, in from_bytes raise ValueError( ValueError: Wrapper Header can only consists of 8 bytes and got 1

I found a way to fix this in blocking_tcp_transport.py: def recv(self) -> bytes: """ Receives a whole DLMS APDU. Gets the total length from the DLMS IP Wrapper. """ if not self.tcp_socket: raise RuntimeError("TCP transport not connected.") try: header = WrapperHeader.from_bytes(self.read_fixed_length(self.tcp_socket, 8)) data = self.read_fixed_length(self.tcp_socket, header.length) except (OSError, IOError, socket.timeout, socket.error) as e: raise exceptions.CommunicationError("Could not receive data") from e return data

def read_fixed_length(self, sckt, length):
    data=b''
    while len(data) < length:
        data += sckt.recv(length-len(data))
    return data
Krolken commented 3 years ago

Hi.

Sorry for not responding earlier, the notification must have been lost in all other emails.

How long does it take to read the whole message? You could try and increase the socket timeout.

I have also seen that the default timeout can be a bit short for some meters in bad coverage.

Mapomme78 commented 3 years ago

Hello,

In fact, in my use case the DLMS server is implemented in java on a Linux system. My guess is that the contract in socket API (either in JVM or Linux) allows sending partial messages over the socket, leading to the issue I faced.

Since I implemented the modification mentioned in my comment, everything works fine though :)

Krolken commented 3 years ago

Ok, great.

I do think it might be a good feature to add so it doesn't occur on other meters. And I guess you don't really need an extra timeout for the whole read since you are always interested in getting the whole message.

I'm planning some other features right now but I could maybe sneak in a solution on this.