diff --git a/changelog.md b/changelog.md index 7ac479b..a8c8b62 100644 --- a/changelog.md +++ b/changelog.md @@ -15,6 +15,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## 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) -[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 diff --git a/fakes/machine.py b/fakes/machine.py index 1053f66..46fc358 100755 --- a/fakes/machine.py +++ b/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: """ diff --git a/umodbus/serial.py b/umodbus/serial.py index d24b981..5f60896 100644 --- a/umodbus/serial.py +++ b/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)