pymodbus-dev / pymodbus

A full modbus protocol written in python
Other
2.22k stars 907 forks source link

Update transport.py #1889

Closed garvan2021 closed 9 months ago

garvan2021 commented 9 months ago

transport.abort() is already contains transport.close(), and it would cause error when transport.close() occurs after transport.abort()

garvan2021 commented 9 months ago

Error:

AttributeError: 'NoneType' object has no attribute 'close'

Machine: ARM 64

How it occurs:

AsyncModbusSerialClient().async_execute(...)

janiversen commented 9 months ago

I think your problem is different, because the error states the self.transport is None !!

alexrudd2 commented 9 months ago
 async def test_transport_close(self, server, dummy_protocol):
        """Test transport_close()."""
        dummy_protocol.abort = mock.MagicMock()
        dummy_protocol.close = mock.MagicMock()
        server.connection_made(dummy_protocol())
        server.recv_buffer = b"abc"
        server.reconnect_task = mock.MagicMock()
        server.transport_close()
        dummy_protocol.abort.assert_called_once()
>       dummy_protocol.close.assert_called_once()

test/sub_transport/test_basic.py:198:  AssertionError: Expected 'mock' to have been called once. Called 0 times.

The test failure is straightforward. Because abort is mocked, it doesn't call close().

abort() was inherited from the async serial client and goes back to at least v.3.0.0. At that time it was different from close(), but now there is no difference. I would therefore suggest the entire conditional be removed, since close() can be called directly.

    # version 3.0.0
    def abort(self):
        """Close the transport immediately.

        Pending operations will not be given opportunity to complete,
        and buffered data will be lost. No more data will be received
        and further writes will be ignored.  The protocol's
        connection_lost() method will eventually be called.
        """
        self._abort(None)

    def close(self):
        """Close the transport gracefully.

        Any buffered data will be written asynchronously. No more data
        will be received and further writes will be silently ignored.
        After all buffered data is flushed, the protocol's
        connection_lost() method will be called with None as its
        argument.
        """
        if not self._closing:
            self._close(None)
alexrudd2 commented 9 months ago

I think your problem is different, because the error states the self.transport is None !!

If only there were some type checking tool that could find such problems with implicit optionals and None! :laughing::laughing:

alexrudd2 commented 9 months ago

Error:

AttributeError: 'NoneType' object has no attribute 'close'

Machine: ARM 64

How it occurs:

AsyncModbusSerialClient().async_execute(...)

Does this still occur for you if you use the most recent dev?

garvan2021 commented 9 months ago

I should be more carefully with my code, may there is another error I missed. So I close the commit temperary and do apologize for my mistake

alexrudd2 commented 9 months ago

I should be more carefully with my code, may there is another error I missed. So I close the commit temperary and do apologize for my mistake

It was OK. The test failure only said the code had changed, not that the code was wrong.

I tried to solve your problem a different way. https://github.com/pymodbus-dev/pymodbus/pull/1890.

Can you try installing from dev and seeing if this works for you?