Fix AsyncioModbusSerialClient (serial options and create_serial_connection usage) #268
Conversation
[FIX] usage of create_serial_connection
| try: | ||
| yield from self.loop.create_connection(self._create_protocol) | ||
| _logger.info('Connected to %s:%s.' % (self.host, self.port)) | ||
| from serial_asyncio import create_serial_connection |
There was a problem hiding this comment.
Good catch. Please move this change to serial factory. https://github.com/cbergmiller/pymodbus/blob/fix/serial_async_client/pymodbus/client/async/factory/serial.py#L82.
There was a problem hiding this comment.
Thanks for the review. I updated the pull request.
|
@cbergmiller the tests are failing on python3 , please have a look. |
[FIX] testSerialAsyncioClient
| :return: | ||
| """ | ||
| if self._connected: | ||
| if self._connected.is_set(): |
There was a problem hiding this comment.
Should this be if self._connected: as you are returning self._connected_event.is_set() from the property _connected?
| _logger.info('Protocol made connection.') | ||
| if not self._connected: | ||
| self._connected = True | ||
| if not self._connected_event.is_set(): |
There was a problem hiding this comment.
You can directly use the property _connected here.
There was a problem hiding this comment.
I didn't want to break compatibility with possible subclasses or mixins. So i created the _connected property that returns the state of the connected event. But you are right it is cleaner to use the _property inside the class as well. I will update the pull request.
2 participants
The API of serial_asyncio.create_serial_connection is different from the async socket API.
Also, the serial kwargs (baudrate etc.) were missing in AsyncioModbusSerialClient.init Method.