Merge pull request brainelectronics#75 from wpyoga/fix-timing

Fix timing issues with UART communication

Author: brainelectronics

changelog.md

@@ -15,6 +15,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 <!-- ## [Unreleased] -->
 
 ## Released
+## [2.3.5] - 2023-07-01
+### Fixed
+- Time between RS485 control pin raise and UART transmission reduced by 80% from 1000us to 200us
+- The RS485 control pin is lowered as fast as possible by using `time.sleep_us()` instead of `machine.idle()` which uses an IRQ on the order of milliseconds. This kept the control pin active longer than necessary, causing the response message to be missed at higher baud rates. This applies only to MicroPython firmwares below v1.20.0
+- The following fixes were provided by @wpyoga
+- RS485 control pin handling fixed by using UART `flush` function, see #68
+- Invalid CRC while reading multiple coils and fixed, see #50 and #52
+
 ## [2.3.4] - 2023-03-20
 ### Added
 - `package.json` for `mip` installation with MicroPython v1.19.1 or newer
@@ -282,8 +290,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - PEP8 style issues on all files of [`lib/uModbus`](lib/uModbus)
 
 <!-- Links -->
-[Unreleased]: https://github.com/brainelectronics/micropython-modbus/compare/2.3.4...develop
+[Unreleased]: https://github.com/brainelectronics/micropython-modbus/compare/2.3.5...develop
 
+[2.3.5]: https://github.com/brainelectronics/micropython-modbus/tree/2.3.5
 [2.3.4]: https://github.com/brainelectronics/micropython-modbus/tree/2.3.4
 [2.3.3]: https://github.com/brainelectronics/micropython-modbus/tree/2.3.3
 [2.3.2]: https://github.com/brainelectronics/micropython-modbus/tree/2.3.2

fakes/machine.py

@@ -425,6 +425,9 @@ def sendbreak(self) -> None:
         """Send a break condition on the bus"""
         raise MachineError('Not yet implemented')
 
+    '''
+    # flush introduced in MicroPython v1.20.0
+    # use manual timing calculation for testing
     def flush(self) -> None:
         """
         Waits until all data has been sent
@@ -434,6 +437,7 @@ def flush(self) -> None:
         Only available with newer versions than 1.19
         """
         raise MachineError('Not yet implemented')
+    '''
 
     def txdone(self) -> bool:
         """

umodbus/serial.py

@@ -13,7 +13,6 @@
 from machine import Pin
 import struct
 import time
-import machine
 
 # custom packages
 from . import const as Const
@@ -96,6 +95,8 @@ def __init__(self,
         :param      ctrl_pin:    The control pin
         :type       ctrl_pin:    int
         """
+        # UART flush function is introduced in Micropython v1.20.0
+        self._has_uart_flush = callable(getattr(UART, "flush", None))
         self._uart = UART(uart_id,
                           baudrate=baudrate,
                           bits=data_bits,
@@ -112,12 +113,16 @@ def __init__(self,
         else:
             self._ctrlPin = None
 
+        # timing of 1 character in microseconds (us)
         self._t1char = (1000000 * (data_bits + stop_bits + 2)) // baudrate
+
+        # inter-frame delay in microseconds (us)
+        # - <= 19200 bps: 3.5x timing of 1 character
+        # - > 19200 bps: 1750 us
         if baudrate <= 19200:
-            # 4010us (approx. 4ms) @ 9600 baud
-            self._t35chars = (3500000 * (data_bits + stop_bits + 2)) // baudrate
+            self._inter_frame_delay = (self._t1char * 3500) // 1000
         else:
-            self._t35chars = 1750   # 1750us (approx. 1.75ms)
+            self._inter_frame_delay = 1750
 
     def _calculate_crc16(self, data: bytearray) -> bytes:
         """
@@ -143,31 +148,35 @@ def _exit_read(self, response: bytearray) -> bool:
         :param      response:    The response
         :type       response:    bytearray
 
-        :returns:   State of basic read response evaluation
+        :returns:   State of basic read response evaluation,
+                    True if entire response has been read
         :rtype:     bool
         """
-        if response[1] >= Const.ERROR_BIAS:
-            if len(response) < Const.ERROR_RESP_LEN:
+        response_len = len(response)
+        if response_len >= 2 and response[1] >= Const.ERROR_BIAS:
+            if response_len < Const.ERROR_RESP_LEN:
                 return False
-        elif (Const.READ_COILS <= response[1] <= Const.READ_INPUT_REGISTER):
+        elif response_len >= 3 and (Const.READ_COILS <= response[1] <= Const.READ_INPUT_REGISTER):
             expected_len = Const.RESPONSE_HDR_LENGTH + 1 + response[2] + Const.CRC_LENGTH
-            if len(response) < expected_len:
+            if response_len < expected_len:
                 return False
-        elif len(response) < Const.FIXED_RESP_LEN:
+        elif response_len < Const.FIXED_RESP_LEN:
             return False
 
         return True
 
     def _uart_read(self) -> bytearray:
         """
-        Read up to 40 bytes from UART
+        Read incoming slave response from UART
 
         :returns:   Read content
         :rtype:     bytearray
         """
         response = bytearray()
 
-        for x in range(1, 40):
+        # TODO: use some kind of hint or user-configurable delay
+        #       to determine this loop counter
+        for x in range(1, 120):
             if self._uart.any():
                 # WiPy only
                 # response.extend(self._uart.readall())
@@ -178,7 +187,7 @@ def _uart_read(self) -> bytearray:
                     break
 
             # wait for the maximum time between two frames
-            time.sleep_us(self._t35chars)
+            time.sleep_us(self._inter_frame_delay)
 
         return response
 
@@ -194,10 +203,9 @@ def _uart_read_frame(self, timeout: Optional[int] = None) -> bytearray:
         """
         received_bytes = bytearray()
 
-        # set timeout to at least twice the time between two frames in case the
-        # timeout was set to zero or None
+        # set default timeout to at twice the inter-frame delay
         if timeout == 0 or timeout is None:
-            timeout = 2 * self._t35chars  # in milliseconds
+            timeout = 2 * self._inter_frame_delay  # in microseconds
 
         start_us = time.ticks_us()
 
@@ -210,13 +218,13 @@ def _uart_read_frame(self, timeout: Optional[int] = None) -> bytearray:
 
                 # do not stop reading and appending the result to the buffer
                 # until the time between two frames elapsed
-                while time.ticks_diff(time.ticks_us(), last_byte_ts) <= self._t35chars:
+                while time.ticks_diff(time.ticks_us(), last_byte_ts) <= self._inter_frame_delay:
                     # WiPy only
                     # r = self._uart.readall()
                     r = self._uart.read()
 
                     # if something has been read after the first iteration of
-                    # this inner while loop (during self._t35chars time)
+                    # this inner while loop (within self._inter_frame_delay)
                     if r is not None:
                         # append the new read stuff to the buffer
                         received_bytes.extend(r)
@@ -235,32 +243,49 @@ def _send(self, modbus_pdu: bytes, slave_addr: int) -> None:
         """
         Send Modbus frame via UART
 
-        If a flow control pin has been setup, it will be controller accordingly
+        If a flow control pin has been setup, it will be controlled accordingly
 
         :param      modbus_pdu:  The modbus Protocol Data Unit
         :type       modbus_pdu:  bytes
         :param      slave_addr:  The slave address
         :type       slave_addr:  int
         """
-        serial_pdu = bytearray()
-        serial_pdu.append(slave_addr)
-        serial_pdu.extend(modbus_pdu)
-
-        crc = self._calculate_crc16(serial_pdu)
-        serial_pdu.extend(crc)
+        # modbus_adu: Modbus Application Data Unit
+        # consists of the Modbus PDU, with slave address prepended and checksum appended
+        modbus_adu = bytearray()
+        modbus_adu.append(slave_addr)
+        modbus_adu.extend(modbus_pdu)
+        modbus_adu.extend(self._calculate_crc16(modbus_adu))
 
         if self._ctrlPin:
-            self._ctrlPin(1)
-            time.sleep_us(1000)     # wait until the control pin really changed
-            send_start_time = time.ticks_us()
-
-        self._uart.write(serial_pdu)
+            self._ctrlPin.on()
+            # wait until the control pin really changed
+            # 85-95us (ESP32 @ 160/240MHz)
+            time.sleep_us(200)
+
+        # the timing of this part is critical:
+        # - if we disable output too early,
+        #   the command will not be received in full
+        # - if we disable output too late,
+        #   the incoming response will lose some data at the beginning
+        # easiest to just wait for the bytes to be sent out on the wire
+
+        send_start_time = time.ticks_us()
+        # 360-400us @ 9600-115200 baud (measured) (ESP32 @ 160/240MHz)
+        self._uart.write(modbus_adu)
+        send_finish_time = time.ticks_us()
+        if self._has_uart_flush:
+            self._uart.flush()
+        else:
+            sleep_time_us = (
+                self._t1char * len(modbus_adu) -    # total frame time in us
+                time.ticks_diff(send_finish_time, send_start_time) +
+                100     # only required at baudrates above 57600, but hey 100us
+            )
+            time.sleep_us(sleep_time_us)
 
         if self._ctrlPin:
-            total_frame_time_us = self._t1char * len(serial_pdu)
-            while time.ticks_us() <= send_start_time + total_frame_time_us:
-                machine.idle()
-            self._ctrlPin(0)
+            self._ctrlPin.off()
 
     def _send_receive(self,
                       modbus_pdu: bytes,
@@ -279,7 +304,7 @@ def _send_receive(self,
         :returns:   Validated response content
         :rtype:     bytes
         """
-        # flush the Rx FIFO
+        # flush the Rx FIFO buffer
         self._uart.read()
 
         self._send(modbus_pdu=modbus_pdu, slave_addr=slave_addr)