Skip to content

Commit d47dc65

Browse files
committed
Correct bit handling internally and in API. (#2627)
1 parent ad1310e commit d47dc65

File tree

11 files changed

+141
-97
lines changed

11 files changed

+141
-97
lines changed

pymodbus/client/mixin.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@
1414
import pymodbus.pdu.register_message as pdu_reg
1515
from pymodbus.constants import ModbusStatus
1616
from pymodbus.exceptions import ModbusException
17-
from pymodbus.pdu import ModbusPDU
18-
from pymodbus.utilities import pack_bitstring, unpack_bitstring
17+
from pymodbus.pdu.pdu import ModbusPDU, pack_bitstring, unpack_bitstring
1918

2019

2120
T = TypeVar("T", covariant=False)

pymodbus/events.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from abc import ABC, abstractmethod
99

1010
from pymodbus.exceptions import ParameterException
11-
from pymodbus.utilities import pack_bitstring, unpack_bitstring
11+
from pymodbus.pdu.pdu import pack_bitstring, unpack_bitstring
1212

1313

1414
class ModbusEvent(ABC):

pymodbus/payload.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
from pymodbus.constants import Endian
2020
from pymodbus.exceptions import ParameterException
2121
from pymodbus.logging import Log
22-
from pymodbus.utilities import (
22+
from pymodbus.pdu.pdu import (
2323
pack_bitstring,
2424
unpack_bitstring,
2525
)

pymodbus/pdu/bit_message.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@
55

66
from pymodbus.constants import ModbusStatus
77
from pymodbus.datastore import ModbusSlaveContext
8-
from pymodbus.pdu.pdu import ModbusPDU
9-
from pymodbus.utilities import pack_bitstring, unpack_bitstring
8+
from pymodbus.pdu.pdu import ModbusPDU, pack_bitstring, unpack_bitstring
109

1110

1211
class ReadCoilsRequest(ModbusPDU):

pymodbus/pdu/diag_message.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77
from pymodbus.constants import ModbusPlusOperation
88
from pymodbus.datastore import ModbusSlaveContext
99
from pymodbus.device import ModbusControlBlock
10-
from pymodbus.pdu.pdu import ModbusPDU
11-
from pymodbus.utilities import pack_bitstring
10+
from pymodbus.pdu.pdu import ModbusPDU, pack_bitstring
1211

1312

1413
_MCB = ModbusControlBlock()

pymodbus/pdu/pdu.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,3 +142,52 @@ def encode(self) -> bytes:
142142
def decode(self, data: bytes) -> None:
143143
"""Decode a modbus exception response."""
144144
self.exception_code = int(data[0])
145+
146+
147+
def pack_bitstring(bits: list[bool], align_byte=True) -> bytes:
148+
"""Create a bytestring out of a list of bits.
149+
150+
example::
151+
152+
bits = [True, False, False, False] +
153+
[False, False, False, True] +
154+
[True, False, True, False] +
155+
[False, False, False, False]
156+
result = pack_bitstring(bits)
157+
bytes 0x05 0x81
158+
"""
159+
ret = b""
160+
i = packed = 0
161+
t_bits = bits
162+
bits_extra = 8 if align_byte else 16
163+
if (extra := len(bits) % bits_extra):
164+
t_bits += [False] * (bits_extra - extra)
165+
for bit in reversed(t_bits):
166+
packed <<= 1
167+
if bit:
168+
packed += 1
169+
i += 1
170+
if i == 8:
171+
ret += struct.pack(">B", packed)
172+
i = packed = 0
173+
return ret
174+
175+
176+
def unpack_bitstring(data: bytes) -> list[bool]:
177+
"""Create bit list out of a bytestring.
178+
179+
example::
180+
181+
bytes 0x05 0x81
182+
result = unpack_bitstring(bytes)
183+
184+
[True, False, False, False] +
185+
[False, False, False, True] +
186+
[True, False, True, False] +
187+
[False, False, False, False]
188+
"""
189+
res = []
190+
for byte_index in range(len(data) -1, -1, -1):
191+
for bit in (1, 2, 4, 8, 16, 32, 64, 128):
192+
res.append(bool(data[byte_index] & bit))
193+
return res

pymodbus/utilities.py

Lines changed: 1 addition & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
"""
66
from __future__ import annotations
77

8+
89
# pylint: disable=missing-type-doc
9-
import struct
1010

1111

1212
def dict_property(store, index):
@@ -44,56 +44,6 @@ def dict_property(store, index):
4444
return property(getter, setter)
4545

4646

47-
# --------------------------------------------------------------------------- #
48-
# Bit packing functions
49-
# --------------------------------------------------------------------------- #
50-
def pack_bitstring(bits: list[bool]) -> bytes:
51-
"""Create a bytestring out of a list of bits.
52-
53-
:param bits: A list of bits
54-
55-
example::
56-
57-
bits = [False, True, False, True]
58-
result = pack_bitstring(bits)
59-
"""
60-
ret = b""
61-
i = packed = 0
62-
for bit in bits:
63-
if bit:
64-
packed += 128
65-
i += 1
66-
if i == 8:
67-
ret += struct.pack(">B", packed)
68-
i = packed = 0
69-
else:
70-
packed >>= 1
71-
if 0 < i < 8:
72-
packed >>= 7 - i
73-
ret += struct.pack(">B", packed)
74-
return ret
75-
76-
77-
def unpack_bitstring(data: bytes) -> list[bool]:
78-
"""Create bit list out of a bytestring.
79-
80-
:param data: The modbus data packet to decode
81-
82-
example::
83-
84-
bytes = "bytes to decode"
85-
result = unpack_bitstring(bytes)
86-
"""
87-
byte_count = len(data)
88-
bits = []
89-
for byte in range(byte_count):
90-
value = int(data[byte])
91-
for _ in range(8):
92-
bits.append((value & 1) == 1)
93-
value >>= 1
94-
return bits
95-
96-
9747
# --------------------------------------------------------------------------- #
9848
# Error Detection Functions
9949
# --------------------------------------------------------------------------- #

test/client/test_client.py

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -138,32 +138,50 @@ def fake_execute(_self, _no_response_expected, request):
138138
),
139139
(
140140
ModbusClientMixin.DATATYPE.BITS,
141-
[True],
142-
[256],
141+
[True] + [False] * 15,
142+
[1], # 0x00 0x01
143143
None,
144144
),
145145
(
146146
ModbusClientMixin.DATATYPE.BITS,
147-
[True, False, True],
148-
[1280],
147+
[False] * 8 + [True] + [False] * 7,
148+
[256], # 0x01 0x00
149149
None,
150150
),
151151
(
152152
ModbusClientMixin.DATATYPE.BITS,
153-
[True, False, True] + [False] * 5 + [True],
154-
[1281],
153+
[False] * 15 + [True],
154+
[32768], # 0x80 0x00
155155
None,
156156
),
157157
(
158158
ModbusClientMixin.DATATYPE.BITS,
159-
[True, False, True] + [False] * 5 + [True] + [False] * 6 + [True],
160-
[1409],
159+
[True] + [False] * 14 + [True],
160+
[32769], # 0x80 0x01
161161
None,
162162
),
163163
(
164164
ModbusClientMixin.DATATYPE.BITS,
165-
[True, False, True] + [False] * 5 + [True] + [False] * 6 + [True] * 2,
166-
[1409, 256],
165+
[False] * 8 + [True, False, True] + [False] * 5,
166+
[1280], # 0x05 0x00
167+
None,
168+
),
169+
(
170+
ModbusClientMixin.DATATYPE.BITS,
171+
[True] + [False] * 7 + [True, False, True] + [False] * 5,
172+
[1281], # 0x05 0x01
173+
None,
174+
),
175+
(
176+
ModbusClientMixin.DATATYPE.BITS,
177+
[True] + [False] * 6 + [True, True, False, True] + [False] * 5,
178+
[1409], # 0x05 0x81
179+
None,
180+
),
181+
(
182+
ModbusClientMixin.DATATYPE.BITS,
183+
[False] * 8 + [True] + [False] * 7 + [True] + [False] * 6 + [True, True, False, True] + [False] * 5,
184+
[1409, 256], # 92340480 = 0x05 0x81 0x01 0x00
167185
None,
168186
),
169187
],
@@ -181,9 +199,6 @@ def test_client_mixin_convert(self, datatype, word_order, registers, value, stri
181199
result = ModbusClientMixin.convert_from_registers(registers, datatype, **kwargs)
182200
if datatype == ModbusClientMixin.DATATYPE.FLOAT32:
183201
result = round(result, 6)
184-
if datatype == ModbusClientMixin.DATATYPE.BITS:
185-
if (missing := len(value) % 16):
186-
value = value + [False] * (16 - missing)
187202
assert result == value
188203

189204
@pytest.mark.parametrize(

test/not_updated/test_utilities.py

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,7 @@
11
"""Test utilities."""
22
import struct
33

4-
import pytest
5-
6-
from pymodbus.utilities import (
7-
dict_property,
8-
pack_bitstring,
9-
unpack_bitstring,
10-
)
4+
from pymodbus.utilities import dict_property
115

126

137
_test_master = {4: "d"}
@@ -64,18 +58,3 @@ def test_dict_property(self):
6458
assert result.s_1 == "x"
6559
assert result.s_2 == "x"
6660
assert result.g_1 == "x"
67-
68-
@pytest.mark.parametrize(
69-
("bytestream", "bitlist"),
70-
[
71-
(b"\x55", [True, False, True, False, True, False, True, False]),
72-
(b"\x80", [False] * 7 + [True]),
73-
(b"\x01", [True] + [False] * 7),
74-
(b"\x80\x00", [False] * 7 + [True] + [False] * 8),
75-
(b"\x01\x00", [True] + [False] * 15),
76-
]
77-
)
78-
def test_bit_packing(self, bytestream, bitlist):
79-
"""Test all string <=> bit packing functions."""
80-
assert pack_bitstring(bitlist) == bytestream
81-
assert unpack_bitstring(bytestream) == bitlist

test/pdu/test_bit.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ def test_bit_read_base_response_encoding(self):
1212
data = [True] * i
1313
pdu = bit_msg.ReadCoilsResponse(bits=data)
1414
pdu.decode(pdu.encode())
15-
if i % 8:
16-
data.extend([False] * (8 - (i % 8)))
1715
assert pdu.bits == data
1816

1917
def test_bit_read_base_requests(self):

test/pdu/test_pdu.py

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
ExceptionResponse,
1414
ModbusPDU,
1515
)
16+
from pymodbus.pdu.pdu import pack_bitstring, unpack_bitstring
1617

1718

1819
class TestPdu:
@@ -110,7 +111,7 @@ def test_calculate_frame_size(self):
110111
]
111112

112113
responses = [
113-
(bit_msg.ReadCoilsResponse, (), {"bits": [True, True], "address": 17}, b'\x01\x01\x03'),
114+
(bit_msg.ReadCoilsResponse, (), {"bits": [True, True] + [False] * 6, "address": 17}, b'\x01\x01\x03'),
114115
(bit_msg.ReadDiscreteInputsResponse, (), {"bits": [True, True], "address": 17}, b'\x02\x01\x03'),
115116
(bit_msg.WriteSingleCoilResponse, (), {"address": 117, "bits": [True]}, b'\x05\x00\x75\xff\x00'),
116117
(bit_msg.WriteMultipleCoilsResponse, (), {"address": 117, "count": 3}, b'\x0f\x00\x75\x00\x03'),
@@ -245,3 +246,58 @@ async def test_pdu_default_datastore(self, mock_context):
245246
pdu = ModbusPDU()
246247
context = mock_context()
247248
assert await pdu.update_datastore(context)
249+
250+
@pytest.mark.parametrize(
251+
("bytestream", "bitlist"),
252+
[
253+
(b"\x00\x01", [True] + [False] * 15),
254+
(b"\x01\x00", [False] * 8 + [True] + [False] * 7),
255+
(b"\x80\x00", [False] * 15 + [True]),
256+
(b"\x80\x01", [True] + [False] * 14 + [True]),
257+
(b"\x05\x00", [False] * 8 + [True, False, True] + [False] * 5),
258+
(b"\x05\x01", [True] + [False] * 7 + [True, False, True] + [False] * 5),
259+
(b"\x05\x81", [True] + [False] * 6 + [True, True, False, True] + [False] * 5),
260+
(b"\x05\x81\x01\x00", [False] * 8 + [True] + [False] * 7 + [True] + [False] * 6 + [True, True, False, True] + [False] * 5),
261+
262+
(b"\x00\x01", [True]),
263+
(b"\x01\x00", [False] * 8 + [True]),
264+
(b"\x05\x00", [False] * 8 + [True, False, True]),
265+
(b"\x05\x01", [True] + [False] * 7 + [True, False, True]),
266+
(b"\x05\x81", [True] + [False] * 6 + [True, True, False, True]),
267+
(b"\x05\x81\x01\x00", [False] * 8 + [True] + [False] * 7 + [True] + [False] * 6 + [True, True, False, True]),
268+
],
269+
)
270+
def test_bit_packing(self, bytestream, bitlist):
271+
"""Test all string <=> bit packing functions."""
272+
assert pack_bitstring(bitlist, align_byte=False) == bytestream
273+
274+
@pytest.mark.parametrize(
275+
("bytestream", "bitlist"),
276+
[
277+
(b"\x01", [True]),
278+
(b"\x01\x00", [False] * 8 + [True]),
279+
(b"\x05", [True, False, True]),
280+
(b"\x05\x01", [True] + [False] * 7 + [True, False, True]),
281+
],
282+
)
283+
def test_bit_packing8(self, bytestream, bitlist):
284+
"""Test all string <=> bit packing functions."""
285+
assert pack_bitstring(bitlist) == bytestream
286+
287+
@pytest.mark.parametrize(
288+
("bytestream", "bitlist"),
289+
[
290+
(b"\x01", [True] + [False] * 7),
291+
(b"\x00\x01", [True] + [False] * 15),
292+
(b"\x01\x00", [False] * 8 + [True] + [False] * 7),
293+
(b"\x80\x00", [False] * 15 + [True]),
294+
(b"\x80\x01", [True] + [False] * 14 + [True]),
295+
(b"\x05\x00", [False] * 8 + [True, False, True] + [False] * 5),
296+
(b"\x05\x01", [True] + [False] * 7 + [True, False, True] + [False] * 5),
297+
(b"\x05\x81", [True] + [False] * 6 + [True, True, False, True] + [False] * 5),
298+
(b"\x05\x81\x01\x00", [False] * 8 + [True] + [False] * 7 + [True] + [False] * 6 + [True, True, False, True] + [False] * 5),
299+
],
300+
)
301+
def test_bit_unpacking(self, bytestream, bitlist):
302+
"""Test all string <=> bit packing functions."""
303+
assert unpack_bitstring(bytestream) == bitlist

0 commit comments

Comments
 (0)