From 6c5820f9118ffd2cb905f9bae2bd93f9f1b9ad31 Mon Sep 17 00:00:00 2001 From: Jonas Scharpf Date: Fri, 6 Jan 2023 16:47:42 +0100 Subject: [PATCH 1/5] fix register definitions EXAMPLE_IREG length to 1 in TCP and RTU example --- examples/rtu_client_example.py | 2 +- examples/rtu_host_example.py | 2 +- examples/tcp_client_example.py | 2 +- examples/tcp_host_example.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/rtu_client_example.py b/examples/rtu_client_example.py index d30ced4..7b0ef9b 100644 --- a/examples/rtu_client_example.py +++ b/examples/rtu_client_example.py @@ -88,7 +88,7 @@ "IREGS": { "EXAMPLE_IREG": { "register": 10, - "len": 2, + "len": 1, "val": 60001 } } diff --git a/examples/rtu_host_example.py b/examples/rtu_host_example.py index e8d3106..c22923c 100644 --- a/examples/rtu_host_example.py +++ b/examples/rtu_host_example.py @@ -92,7 +92,7 @@ "IREGS": { "EXAMPLE_IREG": { "register": 10, - "len": 2, + "len": 1, "val": 60001 } } diff --git a/examples/tcp_client_example.py b/examples/tcp_client_example.py index 5a1cdbd..5125e17 100644 --- a/examples/tcp_client_example.py +++ b/examples/tcp_client_example.py @@ -157,7 +157,7 @@ def reset_data_registers_cb(reg_type, address, val): "IREGS": { "EXAMPLE_IREG": { "register": 10, - "len": 2, + "len": 1, "val": 60001 } } diff --git a/examples/tcp_host_example.py b/examples/tcp_host_example.py index 80c5577..98cc3ab 100644 --- a/examples/tcp_host_example.py +++ b/examples/tcp_host_example.py @@ -101,7 +101,7 @@ "IREGS": { "EXAMPLE_IREG": { "register": 10, - "len": 2, + "len": 1, "val": 60001 } } From bfdba1a9b88da4768d3796f6e696d678d98207c8 Mon Sep 17 00:00:00 2001 From: Jonas Scharpf Date: Fri, 6 Jan 2023 16:48:15 +0100 Subject: [PATCH 2/5] remove overlapping register positions in example JSON file, found during #35 --- registers/example.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/registers/example.json b/registers/example.json index a8b7c56..448a492 100644 --- a/registers/example.json +++ b/registers/example.json @@ -33,7 +33,7 @@ "unit": "" }, "ANOTHER_EXAMPLE_COIL": { - "register": 126, + "register": 127, "len": 3, "val": [0, 1, 0], "description": "Example COILS registers with length of 3, Coils (setter+getter) [0, 1]", @@ -97,7 +97,7 @@ "unit": "" }, "ANOTHER_EXAMPLE_ISTS": { - "register": 69, + "register": 70, "len": 3, "val": [0, 1, 0], "description": "Example ISTS registers with length of 3, Ists (only getter) [0, 1]", From 7ca7a301d5a3678b6faa3f3fbcdbaab69a604a44 Mon Sep 17 00:00:00 2001 From: Jonas Scharpf Date: Fri, 6 Jan 2023 16:48:45 +0100 Subject: [PATCH 3/5] enable setting and getting registers individually, even if setup as list of registers, resolve #35, #15, #24 --- umodbus/modbus.py | 119 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 104 insertions(+), 15 deletions(-) diff --git a/umodbus/modbus.py b/umodbus/modbus.py index e0e245c..f8e6416 100644 --- a/umodbus/modbus.py +++ b/umodbus/modbus.py @@ -119,14 +119,49 @@ def _create_response(self, :rtype: Union[List[bool], List[int]] """ data = [] - address = request.register_addr + default_value = {'val': 0} + reg_dict = self._register_dict[reg_type] - if type(self._register_dict[reg_type][address]['val']) is list: - data = self._register_dict[reg_type][address]['val'] - else: - data = [self._register_dict[reg_type][address]['val']] + if reg_type in ['COILS', 'ISTS']: + default_value = {'val': False} - return data[:request.quantity] + for addr in range(request.register_addr, + request.register_addr + request.quantity): + value = reg_dict.get(addr, default_value)['val'] + + if isinstance(value, (list, tuple)): + data.extend(value) + else: + data.append(value) + + # caution LSB vs MSB + # [ + # 1, 0, 1, 1, 0, 0, 1, 1, # 0xB3 + # 1, 1, 0, 1, 0, 1, 1, 0, # 0xD6 + # 1, 0, 1 # 0x5 + # ] + # but should be, documented at #38, see + # https://github.com/brainelectronics/micropython-modbus/issues/38 + # this is only an issue of data provisioning as client/slave, + # it has thereby NOT to be fixed in + # :py:function:`umodbus.functions.bytes_to_bool` + # [ + # 1, 1, 0, 0, 1, 1, 0, 1, # 0xCD + # 0, 1, 1, 0, 1, 0, 1, 1, # 0x6B + # 1, 0, 1 # 0x5 + # ] + # 27 .... 20 + # CD 1100 1101 + # + # 35 .... 28 + # 6B 0110 1011 + # + # 43 .... 36 + # 05 0000 0101 + # + # 1011 0011 1101 0110 1010 0000 + + return data def _process_read_access(self, request: Request, reg_type: str) -> None: """ @@ -166,6 +201,10 @@ def _process_write_access(self, request: Request, reg_type: str) -> None: valid_register = False if address in self._register_dict[reg_type]: + if request.data is None: + request.send_exception(Const.ILLEGAL_DATA_VALUE) + return + if reg_type == 'COILS': valid_register = True @@ -175,7 +214,7 @@ def _process_write_access(self, request: Request, reg_type: str) -> None: valid_register = False request.send_exception(Const.ILLEGAL_DATA_VALUE) else: - val = (val == 0xFF) + val = [(val == 0xFF)] elif request.function == Const.WRITE_MULTIPLE_COILS: tmp = int.from_bytes(request.data, "big") val = [ @@ -189,9 +228,8 @@ def _process_write_access(self, request: Request, reg_type: str) -> None: val = list(functions.to_short(byte_array=request.data, signed=False)) - if request.function == Const.WRITE_SINGLE_REGISTER: - self.set_hreg(address=address, value=val[0]) - elif request.function == Const.WRITE_MULTIPLE_REGISTERS: + if request.function in [Const.WRITE_SINGLE_REGISTER, + Const.WRITE_MULTIPLE_REGISTERS]: self.set_hreg(address=address, value=val) else: # nothing except holding registers or coils can be set @@ -519,7 +557,7 @@ def _set_reg_in_dict(self, :type reg_type: str :param address: The address (ID) of the register :type address: int - :param value: The default value + :param value: The value(s) of the register(s) :type value: Union[bool, int, List[bool], List[int]] :param on_set_cb: Callback on setting the register :type on_get_cb: Callable[ @@ -533,13 +571,59 @@ def _set_reg_in_dict(self, ] :raise KeyError: No register at specified address found - :returns: Register value - :rtype: Union[bool, int, List[bool], List[int]] """ if not self._check_valid_register(reg_type=reg_type): raise KeyError('{} is not a valid register type of {}'. format(reg_type, self._available_register_types)) + if isinstance(value, (list, tuple)): + # flatten the list and add single registers only + for idx, val in enumerate(value): + this_addr = address + idx + self._set_single_reg_in_dict(reg_type=reg_type, + address=this_addr, + value=val, + on_set_cb=on_set_cb, + on_get_cb=on_get_cb) + else: + self._set_single_reg_in_dict(reg_type=reg_type, + address=address, + value=value, + on_set_cb=on_set_cb, + on_get_cb=on_get_cb) + + def _set_single_reg_in_dict(self, + reg_type: str, + address: int, + value: Union[bool, int], + on_set_cb: Callable[ + [str, int, Union[List[bool], List[int]]], + None + ] = None, + on_get_cb: Callable[ + [str, int, Union[List[bool], List[int]]], + None + ] = None) -> None: + """ + Set a register value in the dictionary of registers. + + :param reg_type: The register type + :type reg_type: str + :param address: The address (ID) of the register + :type address: int + :param value: The value of the register + :type value: Union[bool, int] + :param on_set_cb: Callback on setting the register + :type on_get_cb: Callable[ + [str, int, Union[List[bool], List[int]]], + None + ] + :param on_get_cb: Callback on getting the register + :type on_get_cb: Callable[ + [str, int, Union[List[bool], List[int]]], + None + ] + """ data = {'val': value} # if the register exists already in the register dict a "set_*" @@ -687,8 +771,13 @@ def _set_changed_register(self, :raise KeyError: Register can not be changed externally """ if reg_type in self._changeable_register_types: - content = {'val': value, 'time': time.ticks_ms()} - self._changed_registers[reg_type][address] = content + if isinstance(value, (list, tuple)): + for idx, val in enumerate(value): + content = {'val': val, 'time': time.ticks_ms()} + self._changed_registers[reg_type][address + idx] = content + else: + content = {'val': value, 'time': time.ticks_ms()} + self._changed_registers[reg_type][address] = content else: raise KeyError('{} can not be changed externally'.format(reg_type)) From 6d6bc7af72dac2fef82f601c5d7809f7478241bb Mon Sep 17 00:00:00 2001 From: Jonas Scharpf Date: Fri, 6 Jan 2023 16:49:07 +0100 Subject: [PATCH 4/5] add unittests to verify fix of #35 for #15 and #24 --- tests/test_tcp_example.py | 177 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) diff --git a/tests/test_tcp_example.py b/tests/test_tcp_example.py index b5d12c8..62c9cae 100644 --- a/tests/test_tcp_example.py +++ b/tests/test_tcp_example.py @@ -327,6 +327,39 @@ def test_read_coils_partially(self) -> None: self.assertTrue(all(isinstance(x, bool) for x in coil_status)) self.assertEqual(coil_status, expectation_list_partial) + def test_read_coils_specific_of_multiple(self) -> None: + """Test reading specific coils of client defined as list""" + # the offset based on the specified register + # e.g. register = 150, offset = 3, qty = 5, the requested coils are + # 153-158 + base_coil_offset = 3 + coil_qty = 5 # read only 5 coils of multiple defined ones + + coil_address = ( + self._register_definitions['COILS']['MANY_COILS']['register'] + + base_coil_offset + ) + expectation_list_full = list( + map(bool, + self._register_definitions['COILS']['MANY_COILS']['val']) + ) + expectation_list = expectation_list_full[ + base_coil_offset:base_coil_offset + coil_qty + ] + + coil_status = self._host.read_coils( + slave_addr=self._client_addr, + starting_addr=coil_address, + coil_qty=coil_qty) + + self.test_logger.debug( + 'Status of COIL {} length {}: {}, expectation: {}'. + format(coil_address, coil_qty, coil_status, expectation_list)) + self.assertIsInstance(coil_status, list) + self.assertEqual(len(coil_status), coil_qty) + self.assertTrue(all(isinstance(x, bool) for x in coil_status)) + self.assertEqual(coil_status, expectation_list) + def test_read_discrete_inputs_single(self) -> None: """Test reading discrete inputs of client""" ist_address = \ @@ -748,6 +781,73 @@ def test_write_single_coil(self) -> None: self.assertTrue(all(isinstance(x, bool) for x in coil_status)) self.assertEqual(coil_status, expectation_list) + # test setting a coil in a list of coils + base_coil_offset = 3 + coil_qty = 1 + coil_address = ( + self._register_definitions['COILS']['MANY_COILS']['register'] + + base_coil_offset + ) + expectation_list_full = list( + map(bool, + self._register_definitions['COILS']['MANY_COILS']['val']) + ) + expectation_list = expectation_list_full[ + base_coil_offset:base_coil_offset + coil_qty + ] + + # + # Check clean system (client register data is as initially defined) + # + # verify current state by reading coil states + coil_status = self._host.read_coils( + slave_addr=self._client_addr, + starting_addr=coil_address, + coil_qty=coil_qty) + + self.test_logger.debug( + 'Initial status of COIL {}: {}, expectation: {}'.format( + coil_address, + coil_status, + expectation_list)) + self.assertIsInstance(coil_status, list) + self.assertEqual(len(coil_status), coil_qty) + self.assertTrue(all(isinstance(x, bool) for x in coil_status)) + self.assertEqual(coil_status, expectation_list) + + # + # Test setting coil to True + # + # update coil state of client with a different than the current state + new_coil_val = not expectation_list[0] + expectation_list[0] = new_coil_val + + operation_status = self._host.write_single_coil( + slave_addr=self._client_addr, + output_address=coil_address, + output_value=new_coil_val) + + self.test_logger.debug( + 'Result of setting COIL {} to {}: {}, expectation: {}'.format( + coil_address, new_coil_val, operation_status, True)) + self.assertIsInstance(operation_status, bool) + self.assertTrue(operation_status) + + # verify setting of state by reading data back again + coil_status = self._host.read_coils( + slave_addr=self._client_addr, + starting_addr=coil_address, + coil_qty=coil_qty) + + self.test_logger.debug('Status of COIL {}: {}, expectation: {}'. + format(coil_address, + coil_status, + expectation_list)) + self.assertIsInstance(coil_status, list) + self.assertEqual(len(coil_status), coil_qty) + self.assertTrue(all(isinstance(x, bool) for x in coil_status)) + self.assertEqual(coil_status, expectation_list) + def test_write_single_register(self) -> None: """Test updating single holding register of client""" hreg_address = \ @@ -934,6 +1034,83 @@ def test_write_multiple_coils(self) -> None: # https://github.com/brainelectronics/micropython-modbus/issues/38 # self.assertEqual(coil_status, expectation_list) + def test_write_multiple_coils_specific_of_multiple(self) -> None: + """Test updating specific coils of client defined as list""" + # test with more than 8 coils + coil_address = \ + self._register_definitions['COILS']['MANY_COILS']['register'] + coil_qty = \ + self._register_definitions['COILS']['MANY_COILS']['len'] + expectation_list = list( + map(bool, self._register_definitions['COILS']['MANY_COILS']['val']) + ) + + # + # Check clean system (client register data is as initially defined) + # + # verify current state by reading coil states + coil_status = self._host.read_coils( + slave_addr=self._client_addr, + starting_addr=coil_address, + coil_qty=coil_qty) + + self.test_logger.debug( + 'Initial status of COIL {} length {}: {}, expectation: {}'.format( + coil_address, coil_qty, coil_status, expectation_list)) + self.assertIsInstance(coil_status, list) + self.assertEqual(len(coil_status), coil_qty) + self.assertTrue(all(isinstance(x, bool) for x in coil_status)) + self.assertEqual(coil_status, expectation_list) + + # + # Test setting coils to inverted initial states + # + # update coil states of client with a different than the current state + new_coil_vals_full = [not val for val in expectation_list] + + # the offset based on the specified register + # e.g. register = 150, offset = 3, qty = 5, the requested coils are + # 153-158 + base_coil_offset = 3 + coil_qty = 5 # read only 5 coils of multiple defined ones + + new_coil_vals = new_coil_vals_full[ + base_coil_offset:(base_coil_offset + coil_qty) + ] + expectation_list = ( + expectation_list[:base_coil_offset] + + new_coil_vals + + expectation_list[base_coil_offset + coil_qty:] + ) + + operation_status = self._host.write_multiple_coils( + slave_addr=self._client_addr, + starting_address=coil_address, + output_values=new_coil_vals) + + self.test_logger.debug( + 'Result of setting COIL {} length {} to {}: {}, expectation: {}'. + format( + coil_address, coil_qty, new_coil_vals, operation_status, True)) + self.assertIsInstance(operation_status, bool) + self.assertTrue(operation_status) + + # verify setting of states by reading data back again + coil_status = self._host.read_coils( + slave_addr=self._client_addr, + starting_addr=coil_address, + coil_qty=coil_qty) + + self.test_logger.debug( + 'Status of COIL {} length {}: {}, expectation: {}'.format( + coil_address, coil_qty, coil_status, expectation_list)) + self.assertIsInstance(coil_status, list) + self.assertEqual(len(coil_status), coil_qty) + self.assertTrue(all(isinstance(x, bool) for x in coil_status)) + # Reading coil data bits is reversed, see #38 + # https://github.com/brainelectronics/micropython-modbus/issues/38 + # self.assertEqual(coil_status, expectation_list) + def test_write_multiple_registers(self) -> None: """Test updating multiple holding register of client""" hreg_address = \ From 11ff445f7d1203f8f75bc8c61cb8dd02aacf0ce0 Mon Sep 17 00:00:00 2001 From: Jonas Scharpf Date: Fri, 6 Jan 2023 16:49:14 +0100 Subject: [PATCH 5/5] update changelog --- changelog.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/changelog.md b/changelog.md index 9b76697..a635dc5 100644 --- a/changelog.md +++ b/changelog.md @@ -15,6 +15,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Released +## [2.3.1] - 2023-01-06 +### Added +- Unittest to read multiple coils at any location if defined as list, verifies #35 +- Unittests to write a single coil or multiple coils at any location if defined as list, verifies fix #15 and #24 + +### Fixed +- All configured register of a client can be accessed and modified individually, see #35 +- Resolved overlapping register positions in example JSON file +- Register length of `EXAMPLE_IREG` in TCP and RTU examples corrected to 1 instead of 2 + ## [2.3.0] - 2023-01-03 ### Added - Custom callback functions can be registered on client (ModbusRTU or ModbusTCP) side with new parameters `on_set_cb` and `on_get_cb` available from [modbus.py](umodbus/modbus.py) functions `add_coil` and `add_hreg`. Functions `add_ist` and `add_ireg` support only `on_get_cb`, see #31 @@ -233,8 +243,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.0...develop +[Unreleased]: https://github.com/brainelectronics/micropython-modbus/compare/2.3.1...develop +[2.3.1]: https://github.com/brainelectronics/micropython-modbus/tree/2.3.1 [2.3.0]: https://github.com/brainelectronics/micropython-modbus/tree/2.3.0 [2.2.0]: https://github.com/brainelectronics/micropython-modbus/tree/2.2.0 [2.1.3]: https://github.com/brainelectronics/micropython-modbus/tree/2.1.3