Reading coil data bits is reversed

[brainelectronics]

Found during resolving #36

Setting up a coil with the following data and reading the coil states returns 1100 1101 instead of expected 1011 0011 as setup. This issue was covered by the limit of only reading up to 8 coils in a row. And due to the bad unittest, not covering this case.

{
    "COILS": {
        "MANY_COILS": {
            "register": 150,
            "len": 19,
            "val": [
                1, 0, 1, 1, 0, 0, 1, 1,
                1, 1, 0, 1, 0, 1, 1, 0,
                1, 0, 1
            ],
            "description": "Modbus_Application_Protocol_V1_1b3 Read Coils example"
        }
    }
}

b'\xCD\x6B\x05',
[
            # 20, 21,    22,   23,   24,    25,    26,   27
            True, False, True, True, False, False, True, True,
            # 28, 29,   30,    31,   32,    33,   34,   35
            True, True, False, True, False, True, True, False,
            # 36, 37,    38
            True, False, True
]

Reason/root cause
Not sure yet, but it might be due to

micropython-modbus/umodbus/functions.py

Lines 279 to 295 in eee18c2

	if function_code in [Const.READ_COILS, Const.READ_DISCRETE_INPUTS]:
	sectioned_list = [value_list[i:i + 8] for i in range(0, len(value_list), 8)] # noqa: E501
	output_value = []
	for index, byte in enumerate(sectioned_list):
	# see https://github.com/brainelectronics/micropython-modbus/issues/22
	# output = sum(v << i for i, v in enumerate(byte))
	output = 0
	for bit in byte:
	output = (output << 1) | bit
	output_value.append(output)
	fmt = 'B' * len(output_value)
	return struct.pack('>BB' + fmt,
	function_code,
	((len(value_list) - 1) // 8) + 1,
	*output_value)

which is of course the only untested function of functions.py. In particular the constructed output_value is mixing up LSB and MSB.

The status of outputs 27–20 is shown as the byte value CD hex, or binary 1100 1101. Output 27 is the MSB of this byte, and output 20 is the LSB.

Taken from Modbus Application Protocol, chapter 6.1 01 (0x01) Read Coils

[brainelectronics]

To clarify. This issue exists only, as labeled, in the client Implementation of this library. The issue does not exist when reading from a power meter or other device not running on this lib.

[beyonlo]

@brainelectronics I tested now in the 2.1.0-rc15.dev39 and now this is working on this client Modbus Implementation as well :)

register_definitions = {
    "COILS": {
        "RESET_REGISTER_DATA_COIL": {
            "register": 42,
            "len": 1,
            "val": 0
        },
        "RESET_REGISTER_DATA_COIL_2": {
            "register": 45,
            "len": 1,
            "val": 0
        },
        "EXAMPLE_COIL": {
            "register": 123,
            "len": 16,
            "val": [1,0,0,1,1,0,1,1,1,1,1,1,1,1,0,1]
        },
...

Slave:

$ mpremote run tcp_client_example.py
Waiting for WiFi connection...
Waiting for WiFi connection...
Waiting for WiFi connection...
Waiting for WiFi connection...
Connected to WiFi.
('192.168.1.4', '255.255.255.0', '192.168.1.1', '192.168.1.1')
Setting up registers ...
Register setup done
Running ModBus version: 2.1.0-rc15.dev39
Serving as TCP client on 192.168.1.4:502

Master:

>>> from umodbus.tcp import TCP as ModbusTCPMaster
>>> host = ModbusTCPMaster(slave_ip='192.168.1.4', slave_port=502, timeout=5)
>>> from umodbus import version
>>> version.__version__
'2.1.0-rc15.dev39'
>>> host.read_coils(slave_addr=10, starting_addr=123, coil_qty=16)
[True, False, False, True, True, False, True, True, True, True, True, True, True, True, False, True]

Look as was in Problem 2 for the same register_definitions:

Problem 2: COILs wrong values. The values read is different from the register_definitions set up.
Look that second value is True, and from bit 9 to bit 16 are all False, but in the register_definitions is different:

host.read_coils(slave_addr=10, starting_addr=123, coil_qty=16)
[True, True, False, True, True, False, False, True, False, False, False, False, False, False, False, False]

So, the problem Problem 2 is fixed in this version, just Problem 1 still have the problem.

[beyonlo]

Now I don't know that is the correct order, from left to right or vice-versa?

Because the register 45 is working in inverse way of the register 123 (above):

>>> host.read_coils(slave_addr=10, starting_addr=45, coil_qty=16)
[False, False, False, False, False, False, False, False]
>>> host.write_single_coil(slave_addr=10, output_address=45, output_value=1)
True
>>> host.read_coils(slave_addr=10, starting_addr=45, coil_qty=16)
[False, False, False, False, False, False, False, True]

If the set up is just only one COIL (like as in register 45), it is in the first position right? So "val": 1 is the same like as "val": [1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0] right?

[beyonlo]

To clarify. This issue exists only, as labeled, in the client Implementation of this library. The issue does not exist when reading from a power meter or other device not running on this lib.

I think that you are right, because this is not getting all 16 bits from the register 45. It get all bits just when are set all 16 bits in the register_definitions (like test done above with register 123):

Register 45:

>> host.read_coils(slave_addr=10, starting_addr=45, coil_qty=16)
[False, False, False, False, False, False, False, True] <--- Just 8 bits, missing more 8 bits

but if I use MODSCAN it get all bits that you want, look screenshot below:

Ps: register 45 in the register_definitions is the 46 in the MODSCAN

[beyonlo]

@brainelectronics I think that this bug is fixed, right?

[brainelectronics]

Unfortunately no 😔 but it's only a client Implementation issue. As mentioned the data should be returned in blocks of 8 starting with the highest address. Like 27, 26, ... 20. Right now it is 20, 21, ... 27

[beyonlo]

All right! Understood :)