Fix RemoteSlaveContext (#1599)

Co-authored-by: fchastagnol <frederic.chastagnol@pi-system.ch>
Co-authored-by: jan iversen <jancasacondor@gmail.com>

Author: 3 people

pymodbus/bit_read_message.py

@@ -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
 
 
@@ -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)
 
 
@@ -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)
 
 

pymodbus/bit_write_message.py

@@ -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
 
 
@@ -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):
@@ -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):

pymodbus/datastore/remote.py

@@ -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.

pymodbus/register_read_message.py

@@ -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):
@@ -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)
 
 
@@ -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)
 
 
@@ -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
         )

pymodbus/register_write_message.py

@@ -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):
@@ -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])
 
@@ -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):
@@ -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)
 
 

test/test_remote_datastore.py

@@ -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"""
@@ -64,4 +68,4 @@ def test_remote_slave_validate_values(self):
         assert result
 
         result = context.validate(3, 0, 10)
-        assert not result
+        assert result