Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix validate and setValues in RemoteSlaveContext #1599

Merged
merged 14 commits into from
Jun 18, 2023
6 changes: 5 additions & 1 deletion pymodbus/bit_read_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
import struct

from pymodbus.constants import Defaults
from pymodbus.pdu import ExceptionResponse, ModbusRequest, ModbusResponse
from pymodbus.pdu import ModbusExceptions as merror
from pymodbus.pdu import ModbusRequest, ModbusResponse
from pymodbus.utilities import pack_bitstring, unpack_bitstring


Expand Down Expand Up @@ -171,6 +171,8 @@ def execute(self, context):
if not context.validate(self.function_code, self.address, self.count):
return self.doException(merror.IllegalAddress)
values = context.getValues(self.function_code, self.address, self.count)
if isinstance(values, ExceptionResponse):
return values
return ReadCoilsResponse(values)


Expand Down Expand Up @@ -237,6 +239,8 @@ def execute(self, context):
if not context.validate(self.function_code, self.address, self.count):
return self.doException(merror.IllegalAddress)
values = context.getValues(self.function_code, self.address, self.count)
if isinstance(values, ExceptionResponse):
return values
return ReadDiscreteInputsResponse(values)


Expand Down
12 changes: 9 additions & 3 deletions pymodbus/bit_write_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
import struct

from pymodbus.constants import ModbusStatus
from pymodbus.pdu import ExceptionResponse, ModbusRequest, ModbusResponse
from pymodbus.pdu import ModbusExceptions as merror
from pymodbus.pdu import ModbusRequest, ModbusResponse
from pymodbus.utilities import pack_bitstring, unpack_bitstring


Expand Down Expand Up @@ -90,8 +90,12 @@ def execute(self, context):
if not context.validate(self.function_code, self.address, 1):
return self.doException(merror.IllegalAddress)

context.setValues(self.function_code, self.address, [self.value])
result = context.setValues(self.function_code, self.address, [self.value])
if isinstance(result, ExceptionResponse):
return result
values = context.getValues(self.function_code, self.address, 1)
if isinstance(values, ExceptionResponse):
return values
return WriteSingleCoilResponse(self.address, values[0])

def get_response_pdu_size(self):
Expand Down Expand Up @@ -222,7 +226,9 @@ def execute(self, context):
if not context.validate(self.function_code, self.address, count):
return self.doException(merror.IllegalAddress)

context.setValues(self.function_code, self.address, self.values)
result = context.setValues(self.function_code, self.address, self.values)
if isinstance(result, ExceptionResponse):
return result
return WriteMultipleCoilsResponse(self.address, count)

def __str__(self):
Expand Down
34 changes: 17 additions & 17 deletions pymodbus/datastore/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,34 +32,34 @@ def reset(self):
"""Reset all the datastores to their default values."""
raise NotImplementedException()

def validate(self, fc_as_hex, address, count=1):
def validate(self, _fc_as_hex, _address, _count):
"""Validate the request to make sure it is in range.

:param fc_as_hex: The function we are working with
:param address: The starting address
:param count: The number of values to test
:returns: True if the request in within range, False otherwise
:returns: True
"""
Log.debug("validate[{}] {}:{}", fc_as_hex, address, count)
group_fx = self.decode(fc_as_hex)
if fc_as_hex in self._write_fc:
func_fc = self.__set_callbacks[f"{group_fx}{fc_as_hex}"]
else:
func_fc = self.__get_callbacks[group_fx]
self.result = func_fc(address, count)
return not self.result.isError()
return True

def getValues(self, fc_as_hex, _address, _count=1):
"""Get values from real call in validate"""
if fc_as_hex in self._write_fc:
return [0]
group_fx = self.decode(fc_as_hex)
func_fc = self.__get_callbacks[group_fx]
self.result = func_fc(_address, _count)
return self.__extract_result(self.decode(fc_as_hex), self.result)

def setValues(self, fc_as_hex, address, values):
"""Set the datastore with the supplied values.

Already done in validate
"""
"""Set the datastore with the supplied values."""
group_fx = self.decode(fc_as_hex)
if fc_as_hex in self._write_fc:
func_fc = self.__set_callbacks[f"{group_fx}{fc_as_hex}"]
if fc_as_hex in {0x0F, 0x10}:
self.result = func_fc(address, values)
else:
self.result = func_fc(address, values[0])
if self.result.isError():
return self.result
return None

def __str__(self):
"""Return a string representation of the context.
Expand Down
13 changes: 11 additions & 2 deletions pymodbus/register_read_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
import struct

from pymodbus.constants import Defaults
from pymodbus.pdu import ExceptionResponse, ModbusRequest, ModbusResponse
from pymodbus.pdu import ModbusExceptions as merror
from pymodbus.pdu import ModbusRequest, ModbusResponse


class ReadRegistersRequestBase(ModbusRequest):
Expand Down Expand Up @@ -151,6 +151,9 @@ def execute(self, context):
if not context.validate(self.function_code, self.address, self.count):
return self.doException(merror.IllegalAddress)
values = context.getValues(self.function_code, self.address, self.count)
if isinstance(values, ExceptionResponse):
return values

return ReadHoldingRegistersResponse(values)


Expand Down Expand Up @@ -209,6 +212,8 @@ def execute(self, context):
if not context.validate(self.function_code, self.address, self.count):
return self.doException(merror.IllegalAddress)
values = context.getValues(self.function_code, self.address, self.count)
if isinstance(values, ExceptionResponse):
return values
return ReadInputRegistersResponse(values)


Expand Down Expand Up @@ -324,7 +329,11 @@ def execute(self, context):
return self.doException(merror.IllegalAddress)
if not context.validate(self.function_code, self.read_address, self.read_count):
return self.doException(merror.IllegalAddress)
context.setValues(self.function_code, self.write_address, self.write_registers)
result = context.setValues(
self.function_code, self.write_address, self.write_registers
)
if isinstance(result, ExceptionResponse):
return result
registers = context.getValues(
self.function_code, self.read_address, self.read_count
)
Expand Down
16 changes: 12 additions & 4 deletions pymodbus/register_write_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
# pylint: disable=missing-type-doc
import struct

from pymodbus.pdu import ExceptionResponse, ModbusRequest, ModbusResponse
from pymodbus.pdu import ModbusExceptions as merror
from pymodbus.pdu import ModbusRequest, ModbusResponse


class WriteSingleRegisterRequest(ModbusRequest):
Expand Down Expand Up @@ -68,7 +68,9 @@ def execute(self, context):
if not context.validate(self.function_code, self.address, 1):
return self.doException(merror.IllegalAddress)

context.setValues(self.function_code, self.address, [self.value])
result = context.setValues(self.function_code, self.address, [self.value])
if isinstance(result, ExceptionResponse):
return result
values = context.getValues(self.function_code, self.address, 1)
return WriteSingleRegisterResponse(self.address, values[0])

Expand Down Expand Up @@ -211,7 +213,9 @@ def execute(self, context):
if not context.validate(self.function_code, self.address, self.count):
return self.doException(merror.IllegalAddress)

context.setValues(self.function_code, self.address, self.values)
result = context.setValues(self.function_code, self.address, self.values)
if isinstance(result, ExceptionResponse):
return result
return WriteMultipleRegistersResponse(self.address, self.count)

def get_response_pdu_size(self):
Expand Down Expand Up @@ -330,8 +334,12 @@ def execute(self, context):
if not context.validate(self.function_code, self.address, 1):
return self.doException(merror.IllegalAddress)
values = context.getValues(self.function_code, self.address, 1)[0]
if isinstance(values, ExceptionResponse):
return values
values = (values & self.and_mask) | (self.or_mask & ~self.and_mask)
context.setValues(self.function_code, self.address, [values])
result = context.setValues(self.function_code, self.address, [values])
if isinstance(result, ExceptionResponse):
return result
return MaskWriteRegisterResponse(self.address, self.and_mask, self.or_mask)


Expand Down
8 changes: 6 additions & 2 deletions test/test_remote_datastore.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,13 @@ def test_remote_slave_set_values(self):
"""Test setting values against a remote slave context"""
client = mock.MagicMock()
client.write_coils = lambda a, b: WriteMultipleCoilsResponse()
client.write_registers = lambda a, b: ExceptionResponse(0x10, 0x02)

context = RemoteSlaveContext(client)
context.setValues(1, 0, [1])
context.setValues(0x0F, 0, [1])
result = context.setValues(0x10, 1, [1])
assert result.exception_code == 0x02
assert result.function_code == 0x90

def test_remote_slave_get_values(self):
"""Test getting values from a remote slave context"""
Expand Down Expand Up @@ -64,4 +68,4 @@ def test_remote_slave_validate_values(self):
assert result

result = context.validate(3, 0, 10)
assert not result
assert result