Skip to content

Commit e2f3b39

Browse files
janiversenjaniversen
authored
Simplify response rejection. (#2657)
1 parent d37bbe9 commit e2f3b39

25 files changed

+191
-169
lines changed

‎examples/message_parser.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ def decode(self, message):
8080
print(f"{decoder.decoder.__class__.__name__}")
8181
print("-" * 80)
8282
try:
83-
_, pdu = decoder.processIncomingFrame(message)
83+
_, pdu = decoder.handleFrame(message, 0, 0)
8484
self.report(pdu)
8585
except Exception: # pylint: disable=broad-except
8686
self.check_errors(decoder, message)

‎pymodbus/client/serial.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ def send(self, request: bytes, addr: tuple | None = None) -> int:
270270
if waitingbytes := self._in_waiting():
271271
result = self.socket.read(waitingbytes)
272272
Log.warning("Cleanup recv buffer before send: {}", result, ":hex")
273-
if (size := self.socket.write(request)) is None:
273+
if (size := self.socket.write(request)) is None: # pragma: no cover
274274
size = 0
275275
return size
276276
return 0

‎pymodbus/client/tcp.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ def recv(self, size: int | None) -> bytes:
259259
return self._handle_abrupt_socket_close(
260260
size, data, time.time() - time_
261261
)
262-
except SSLWantReadError:
262+
except SSLWantReadError: # pragma: no cover
263263
continue
264264
data.append(recv_data)
265265
data_length += len(recv_data)

‎pymodbus/constants.py

-4
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,6 @@ class DeviceInformation(int, enum.Enum):
112112
EXTENDED = 0x03
113113
SPECIFIC = 0x04
114114

115-
def __str__(self):
116-
"""Override to force int representation for enum members."""
117-
return str(int(self))
118-
119115

120116
class MoreData(int, enum.Enum):
121117
"""Represents the more follows condition.

‎pymodbus/exceptions.py

-13
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
__all__ = [
77
"ConnectionException",
8-
"InvalidMessageReceivedException",
98
"MessageRegisterException",
109
"ModbusIOException",
1110
"NoSuchIdException",
@@ -95,18 +94,6 @@ def __init__(self, string=""):
9594
ModbusException.__init__(self, message)
9695

9796

98-
class InvalidMessageReceivedException(ModbusException):
99-
"""Error resulting from invalid response received or decoded."""
100-
101-
def __init__(self, string=""):
102-
"""Initialize the exception.
103-
104-
:param string: The message to append to the error
105-
"""
106-
message = f"[Invalid Message] {string}"
107-
ModbusException.__init__(self, message)
108-
109-
11097
class MessageRegisterException(ModbusException):
11198
"""Error resulting from failing to register a custom message request/response."""
11299

‎pymodbus/framer/base.py

+22-33
Original file line numberDiff line numberDiff line change
@@ -63,40 +63,29 @@ def buildFrame(self, message: ModbusPDU) -> bytes:
6363
frame = self.encode(data, message.dev_id, message.transaction_id)
6464
return frame
6565

66-
def processIncomingFrame(self, data: bytes) -> tuple[int, ModbusPDU | None]:
67-
"""Process new packet pattern.
68-
69-
This takes in a new request packet, adds it to the current
70-
packet stream, and performs framing on it. That is, checks
71-
for complete messages, and once found, will process all that
72-
exist.
73-
"""
66+
def handleFrame(self, data: bytes, exp_devid: int, exp_tid: int) -> tuple[int, ModbusPDU | None]:
67+
"""Process incoming data."""
7468
used_len = 0
7569
while True:
76-
data_len, pdu = self._processIncomingFrame(data[used_len:])
70+
if used_len >= len(data):
71+
return used_len, None
72+
Log.debug("Processing: {}", data, ":hex")
73+
data_len, dev_id, tid, frame_data = self.decode(data)
7774
used_len += data_len
78-
if not data_len:
75+
if not data_len or not frame_data:
7976
return used_len, None
80-
if pdu:
81-
return used_len, pdu
82-
83-
def _processIncomingFrame(self, data: bytes) -> tuple[int, ModbusPDU | None]:
84-
"""Process new packet pattern.
85-
86-
This takes in a new request packet, adds it to the current
87-
packet stream, and performs framing on it. That is, checks
88-
for complete messages, and once found, will process all that
89-
exist.
90-
"""
91-
Log.debug("Processing: {}", data, ":hex")
92-
if not data:
93-
return 0, None
94-
used_len, dev_id, tid, frame_data = self.decode(data)
95-
if not frame_data:
96-
return used_len, None
97-
if (result := self.decoder.decode(frame_data)) is None:
98-
raise ModbusIOException("Unable to decode request")
99-
result.dev_id = dev_id
100-
result.transaction_id = tid
101-
Log.debug("Frame advanced, resetting header!!")
102-
return used_len, result
77+
if exp_devid and dev_id != exp_devid:
78+
Log.error(
79+
f"ERROR: request ask for id={exp_devid} but got id={dev_id}, Skipping."
80+
)
81+
continue
82+
if exp_tid and tid and tid != exp_tid:
83+
Log.error(
84+
f"ERROR: request ask for transaction_id={exp_tid} but got id={tid}, Skipping."
85+
)
86+
continue
87+
if (pdu := self.decoder.decode(frame_data)) is None:
88+
raise ModbusIOException("Unable to decode request")
89+
pdu.dev_id = dev_id
90+
pdu.transaction_id = tid
91+
return used_len, pdu

‎pymodbus/logging.py

+2-3
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ def build_msg(cls, txt, *args):
8383
string_args.append(hexlify_packets(args[i]))
8484
elif args[i + 1] == ":str":
8585
string_args.append(str(args[i]))
86-
elif args[i + 1] == ":b2a":
86+
else: # args[i + 1] == ":b2a":
8787
string_args.append(b2a_hex(args[i]))
8888
skip = True
8989
else:
@@ -117,5 +117,4 @@ def error(cls, txt, *args):
117117
@classmethod
118118
def critical(cls, txt, *args):
119119
"""Log critical messages."""
120-
if cls._logger.isEnabledFor(logging.CRITICAL):
121-
cls._logger.critical(cls.build_msg(txt, *args), stacklevel=2)
120+
cls._logger.critical(cls.build_msg(txt, *args), stacklevel=2)

‎pymodbus/pdu/bit_message.py

-5
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
3838
values = await context.async_getValues(
3939
self.function_code, self.address, self.count
4040
)
41-
if isinstance(values, int):
42-
return ExceptionResponse(self.function_code, values)
4341
response_class = (ReadCoilsResponse if self.function_code == 1 else ReadDiscreteInputsResponse)
4442
return response_class(dev_id=self.dev_id, transaction_id=self.transaction_id, bits=cast(list[bool], values))
4543

@@ -98,9 +96,6 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
9896
if (rc := await context.async_setValues(self.function_code, self.address, self.bits)):
9997
return ExceptionResponse(self.function_code, rc)
10098
values = await context.async_getValues(self.function_code, self.address, 1)
101-
if isinstance(values, int):
102-
return ExceptionResponse(self.function_code, values)
103-
10499
return WriteSingleCoilResponse(address=self.address, bits=cast(list[bool], values), dev_id=self.dev_id, transaction_id=self.transaction_id)
105100

106101
def get_response_pdu_size(self) -> int:

‎pymodbus/pdu/device.py

+2-8
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
# pylint: disable=missing-type-doc
1919
from collections import OrderedDict
2020

21-
from pymodbus.constants import INTERNAL_ERROR, DeviceInformation
21+
from pymodbus.constants import DeviceInformation
2222
from pymodbus.pdu.events import ModbusEvent
2323
from pymodbus.utilities import dict_property
2424

@@ -293,11 +293,6 @@ def __gets(cls, identity, object_ids): # pylint: disable=unused-private-member
293293
"""
294294
return {oid: identity[oid] for oid in object_ids if identity[oid]}
295295

296-
def __init__(self):
297-
"""Prohibit objects."""
298-
raise RuntimeError(INTERNAL_ERROR)
299-
300-
301296
# ---------------------------------------------------------------------------#
302297
# Counters Handler
303298
# ---------------------------------------------------------------------------#
@@ -562,8 +557,7 @@ def setDiagnostic(self, mapping):
562557
:param mapping: Dictionary of key:value pairs to set
563558
"""
564559
for entry in iter(mapping.items()):
565-
if entry[0] >= 0 and entry[0] < len(self._diagnostic):
566-
self._diagnostic[entry[0]] = bool(entry[1])
560+
self._diagnostic[entry[0]] = bool(entry[1])
567561

568562
def getDiagnostic(self, bit):
569563
"""Get the value in the diagnostic register.

‎pymodbus/pdu/register_message.py

+2-9
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from __future__ import annotations
33

44
import struct
5+
from collections.abc import Sequence
56
from typing import cast
67

78
from pymodbus.datastore import ModbusDeviceContext
@@ -37,8 +38,6 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
3738
values = await context.async_getValues(
3839
self.function_code, self.address, self.count
3940
)
40-
if isinstance(values, int):
41-
return ExceptionResponse(self.function_code, values)
4241
response_class = (ReadHoldingRegistersResponse if self.function_code == 3 else ReadInputRegistersResponse)
4342
return response_class(registers=cast(list[int], values), dev_id=self.dev_id, transaction_id=self.transaction_id)
4443

@@ -147,8 +146,6 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
147146
registers = await context.async_getValues(
148147
self.function_code, self.read_address, self.read_count
149148
)
150-
if isinstance(registers, int):
151-
return ExceptionResponse(self.function_code, registers)
152149
return ReadWriteMultipleRegistersResponse(registers=cast(list[int], registers), dev_id=self.dev_id, transaction_id=self.transaction_id)
153150

154151
def get_response_pdu_size(self) -> int:
@@ -194,8 +191,6 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
194191
if rc:
195192
return ExceptionResponse(self.function_code, rc)
196193
values = await context.async_getValues(self.function_code, self.address, 1)
197-
if isinstance(values, int):
198-
return ExceptionResponse(self.function_code, values)
199194
return WriteSingleRegisterResponse(address=self.address, registers=cast(list[int], values))
200195

201196
def get_response_pdu_size(self) -> int:
@@ -288,9 +283,7 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU:
288283
if not 0x0000 <= self.or_mask <= 0xFFFF:
289284
return ExceptionResponse(self.function_code, ExceptionResponse.ILLEGAL_VALUE)
290285
values = await context.async_getValues(self.function_code, self.address, 1)
291-
if isinstance(values, int):
292-
return ExceptionResponse(self.function_code, values)
293-
values = (values[0] & self.and_mask) | (self.or_mask & ~self.and_mask)
286+
values = (cast(Sequence[int | bool], values)[0] & self.and_mask) | (self.or_mask & ~self.and_mask)
294287
rc = await context.async_setValues(
295288
self.function_code, self.address, cast(list[int], [values])
296289
)

‎pymodbus/transaction/transaction.py

+8-15
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def dummy_trace_connect(self, connect: bool) -> None:
7979
"""Do dummy trace."""
8080
_ = connect
8181

82-
def sync_get_response(self, dev_id) -> ModbusPDU:
82+
def sync_get_response(self, dev_id, tid) -> ModbusPDU:
8383
"""Receive until PDU is correct or timeout."""
8484
databuffer = b''
8585
while True:
@@ -108,13 +108,9 @@ def sync_get_response(self, dev_id) -> ModbusPDU:
108108
continue
109109

110110
databuffer += data
111-
used_len, pdu = self.framer.processIncomingFrame(self.trace_packet(False, databuffer))
111+
used_len, pdu = self.framer.handleFrame(self.trace_packet(False, databuffer), dev_id, tid)
112112
databuffer = databuffer[used_len:]
113113
if pdu:
114-
if pdu.dev_id != dev_id:
115-
raise ModbusIOException(
116-
f"ERROR: request ask for id={dev_id} but id={pdu.dev_id}, CLOSING CONNECTION."
117-
)
118114
return self.trace_pdu(False, pdu)
119115

120116
def sync_execute(self, no_response_expected: bool, request: ModbusPDU) -> ModbusPDU:
@@ -133,7 +129,7 @@ def sync_execute(self, no_response_expected: bool, request: ModbusPDU) -> Modbus
133129
if no_response_expected:
134130
return ExceptionResponse(0xff)
135131
try:
136-
return self.sync_get_response(request.dev_id)
132+
return self.sync_get_response(request.dev_id, request.transaction_id)
137133
except asyncio.exceptions.TimeoutError:
138134
count_retries += 1
139135
if self.count_until_disconnect < 0:
@@ -191,8 +187,9 @@ async def execute(self, no_response_expected: bool, request: ModbusPDU) -> Modbu
191187

192188
def pdu_send(self, pdu: ModbusPDU, addr: tuple | None = None) -> None:
193189
"""Build byte stream and send."""
194-
self.request_dev_id = pdu.dev_id
195-
self.request_transaction_id = pdu.transaction_id
190+
if not self.is_server:
191+
self.request_dev_id = pdu.dev_id
192+
self.request_transaction_id = pdu.transaction_id
196193
packet = self.framer.buildFrame(self.trace_pdu(True, pdu))
197194
if self.is_sync and self.comm_params.handle_local_echo:
198195
self.sent_buffer = packet
@@ -214,16 +211,12 @@ def callback_disconnected(self, exc: Exception | None) -> None:
214211
def callback_data(self, data: bytes, addr: tuple | None = None) -> int:
215212
"""Handle received data."""
216213
self.last_pdu = self.last_addr = None
217-
used_len, pdu = self.framer.processIncomingFrame(self.trace_packet(False, data))
214+
used_len, pdu = self.framer.handleFrame(self.trace_packet(False, data), self.request_dev_id, self.request_transaction_id)
218215
if pdu:
219216
self.last_pdu = self.trace_pdu(False, pdu)
220217
self.last_addr = addr
221218
if not self.is_server:
222-
if pdu.dev_id != self.request_dev_id:
223-
Log.warning(f"ERROR: expected id {self.request_dev_id} but got {pdu.dev_id}, IGNORING.")
224-
elif pdu.transaction_id != self.request_transaction_id:
225-
Log.warning(f"ERROR: expected transaction {self.request_transaction_id} but got {pdu.transaction_id}, IGNORING.")
226-
elif self.response_future.done():
219+
if self.response_future.done():
227220
Log.warning("ERROR: received pdu without a corresponding request, IGNORING")
228221
else:
229222
self.response_future.set_result(self.last_pdu)

‎test/client/test_client.py

+9
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,12 @@ def fake_execute(_self, _no_response_expected, request):
136136
[0xC009, 0x21FB, 0x5444, 0x2D11],
137137
None,
138138
),
139+
(
140+
ModbusClientMixin.DATATYPE.BITS,
141+
[True],
142+
[1], # 0x00 0x01
143+
None,
144+
),
139145
(
140146
ModbusClientMixin.DATATYPE.BITS,
141147
[True] + [False] * 15,
@@ -197,6 +203,9 @@ def test_client_mixin_convert(self, datatype, word_order, registers, value, stri
197203
regs = ModbusClientMixin.convert_to_registers(value, datatype, **kwargs)
198204
assert regs == registers
199205
result = ModbusClientMixin.convert_from_registers(registers, datatype, **kwargs)
206+
if datatype == ModbusClientMixin.DATATYPE.BITS:
207+
if (missing := len(value) % 16):
208+
value = value + [False] * (16 - missing)
200209
if datatype == ModbusClientMixin.DATATYPE.FLOAT32:
201210
result = round(result, 6)
202211
assert result == value

‎test/client/test_client_faulty_response.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,23 @@ def fixture_framer(self):
1919

2020
def test_ok_frame(self, framer):
2121
"""Test ok frame."""
22-
used_len, pdu = framer.processIncomingFrame(self.good_frame)
22+
used_len, pdu = framer.handleFrame(self.good_frame, 0, 0)
2323
assert pdu
2424
assert used_len == len(self.good_frame)
2525

2626
def test_1917_frame(self):
2727
"""Test invalid frame in issue 1917."""
2828
recv = b"\x01\x86\x02\x00\x01"
2929
framer = FramerRTU(DecodePDU(False))
30-
used_len, pdu = framer.processIncomingFrame(recv)
30+
used_len, pdu = framer.handleFrame(recv, 0, 0)
3131
assert not pdu
3232
assert not used_len
3333

3434
def test_faulty_frame1(self, framer):
3535
"""Test ok frame."""
3636
faulty_frame = b"\x00\x04\x00\x00\x00\x05\x00\x03\x0a\x00\x04"
3737
with pytest.raises(ModbusIOException):
38-
framer.processIncomingFrame(faulty_frame)
39-
used_len, pdu = framer.processIncomingFrame(self.good_frame)
38+
framer.handleFrame(faulty_frame, 0, 0)
39+
used_len, pdu = framer.handleFrame(self.good_frame, 0, 0)
4040
assert pdu
4141
assert used_len == len(self.good_frame)

‎test/client/test_client_sync.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
ModbusTlsClient,
1313
ModbusUdpClient,
1414
)
15-
from pymodbus.exceptions import ConnectionException
15+
from pymodbus.exceptions import ConnectionException, ModbusIOException
1616
from pymodbus.framer import (
1717
FramerAscii,
1818
FramerRTU,
@@ -82,8 +82,8 @@ def test_udp_client_recv_duplicate(self):
8282
client.socket.mock_prepare_receive(test_msg)
8383
reply_ok = client.read_input_registers(0x820, count=1, device_id=1)
8484
assert not reply_ok.isError()
85-
reply_ok = client.read_input_registers(0x40, count=10, device_id=1)
86-
assert not reply_ok.isError()
85+
with pytest.raises(ModbusIOException):
86+
client.read_input_registers(0x40, count=10, device_id=1)
8787
client.close()
8888

8989
def test_udp_client_repr(self):

0 commit comments

Comments
 (0)