Repository: tiagocoutinho/modbus-proxy (language: python)

Issue:
Question: Forwarding slave to another with different indices
Hi

I'm wondering if it is possible to forward one slave index to another, or - in other words - to change its index? For example: ModBus server foo.com:502 has a slave on index 0, and I would like to proxy it, but forward slave **1** to foo.com:502 slave **0** instead of 0 to 0 or 1 to 1.

The reason I'm asking this is because some MODBUS/TCP servers, such as the Huawei SUN-2000L, have their slave on index 0, which - if I understood correctly - is not meant to be used as _actual_ slave index. As a result, some software (e.g. Loxone's MODBUS support) refuses to read from it and requires a minimum slave index of 1. Meanwhile, tools such as qModMaster work fine with slave index 0 to read the necessary data. Having this support would allow me to work around this problem with quirky MODBUS implementations, and link both.

Thanks!


Discussion / hints:
Hi,
So far we only support [modbus TCP protocol](https://en.wikipedia.org/wiki/Modbus#Modbus_TCP_frame_format). So when you say "slave index" are you talking about the "Unit identifier" ?

Assuming you are the answer is no. We do not tampering with any part of the Modbus TCP frame.

I do think it is an interesting feature so if anyone is willing it I would be honored to code review a PR.
> So far we only support [modbus TCP protocol](https://en.wikipedia.org/wiki/Modbus#Modbus_TCP_frame_format). So when you say "slave index" are you talking about the "Unit identifier" ?

Yes, I think that is indeed it, e.g. what is `253` here:

![image](https://user-images.githubusercontent.com/24809863/215318607-91457e8d-205e-4866-a5a8-2c5cf37f11a3.png)

(I got confused because there seems to be a bug on qModMaster where if you had it on "TCP" before, on startup it will still say "Slave Address" as is the case for RTU; it fixes itself when you go back and forth.)

> I do think it is an interesting feature so if anyone is willing it I would be honored to code review a PR.

:+1: I can't make any promises, but I might take a whack at this at some point. I'm also investigating alternative solutions simultaneously (i.e. getting Loxone/Huawei to fix their stuff in my case), but I'm pessimistic either will do so.

Fix patch:
diff --git a/README.md b/README.md
index a8541b7..ae42641 100644
--- a/README.md
+++ b/README.md
@@ -45,6 +45,8 @@ devices:
     connection_time: 0.1       # delay after connection (s) (optional, default: 0)
   listen:
     bind: 0:9000               # listening address (mandatory)
+  unit_id_remapping:           # remap/forward unit IDs (optional, empty by default)
+    1: 0
 ```
 
 Assuming you saved this file as `modbus-config.yml`, start the server with:
@@ -81,6 +83,23 @@ modbus-proxy -b tcp://0:9000 --modbus tcp://plc1.acme.org:502
 
 (hint: run `modbus-proxy --help` to see all available options)
 
+### Forwarding Unit Identifiers
+
+You can also forward one unit ID to another whilst proxying. This is handy if the target
+modbus server has a unit on an index that is not supported by one of your clients.
+
+```yaml
+devices:
+- modbus: ... # see above.
+  listen: ... # see above.
+  unit_id_remapping:
+    1: 0
+```
+
+The above forwards requests to unit ID 1 to your modbus-proxy server to unit ID 0 on the
+actual modbus server.
+
+Note that **the reverse also applies**: if you forward unit ID 1 to unit ID 0, **all** responses coming from unit 0 will look as if they are coming from 1, so this may pose problems if you want to use unit ID 0 for some clients and unit ID 1 for others (use unit ID 1 for all in that case).
 
 ## Running the examples
 
diff --git a/src/modbus_proxy.py b/src/modbus_proxy.py
index d4bd499..2187fe7 100644
--- a/src/modbus_proxy.py
+++ b/src/modbus_proxy.py
@@ -132,6 +132,7 @@ def __init__(self, config):
         self.modbus_port = url.port
         self.timeout = modbus.get("timeout", None)
         self.connection_time = modbus.get("connection_time", 0)
+        self.unit_id_remapping = config.get("unit_id_remapping") or {}
         self.server = None
         self.lock = asyncio.Lock()
 
@@ -171,16 +172,35 @@ async def _write_read(self, data):
         await self._write(data)
         return await self._read()
 
+    def _transform_request(self, request):
+        uid = request[6]
+        new_uid = self.unit_id_remapping.setdefault(uid, uid)
+        if uid != new_uid:
+            request = bytearray(request)
+            request[6] = new_uid
+            self.log.debug("remapping unit ID %s to %s in request", uid, new_uid)
+        return request
+
+    def _transform_reply(self, reply):
+        uid = reply[6]
+        inverse_unit_id_map = {v: k for k, v in self.unit_id_remapping.items()}
+        new_uid = inverse_unit_id_map.setdefault(uid, uid)
+        if uid != new_uid:
+            reply = bytearray(reply)
+            reply[6] = new_uid
+            self.log.debug("remapping unit ID %s to %s in reply", uid, new_uid)
+        return reply
+
     async def handle_client(self, reader, writer):
         async with Client(reader, writer) as client:
             while True:
                 request = await client.read()
                 if not request:
                     break
-                reply = await self.write_read(request)
+                reply = await self.write_read(self._transform_request(request))
                 if not reply:
                     break
-                result = await client.write(reply)
+                result = await client.write(self._transform_reply(reply))
                 if not result:
                     break
 


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 731ec1d..3db3ee0 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -18,6 +18,15 @@
 REP2 = b"m\xf5\x00\x00\x00\x09\x01\x03\x08\x00\x02\x00\x03\x00\x04"
 
 
+# read_holding_registers(unit=1, start=2, size=3)
+#       |tid | tcp   | size  |uni|cod|s .addr|nb. reg|
+REQ3_ORIGINAL = b"m\xf5\x00\x00\x00\x06\xFF\x03\x00\x02\x00\x03"
+REQ3_MODIFIED = b"m\xf5\x00\x00\x00\x06\xFE\x03\x00\x02\x00\x03"
+#       |tid | tcp   | size  |uni|cod|len|   2   |   3   |   4   |
+REP3_ORIGINAL = b"m\xf5\x00\x00\x00\x09\xFE\x03\x08\x00\x02\x00\x03\x00\x04"
+REP3_MODIFIED = b"m\xf5\x00\x00\x00\x09\xFF\x03\x08\x00\x02\x00\x03\x00\x04"
+
+
 @pytest.fixture
 async def modbus_device():
     async def cb(r, w):
@@ -29,6 +38,9 @@ async def cb(r, w):
                 reply = REP
             elif data == REQ2:
                 reply = REP2
+            elif data == REQ3_MODIFIED:
+                reply = REP3_ORIGINAL
+
             w.write(reply)
             await w.drain()
 
@@ -45,6 +57,7 @@ def modbus_config(modbus_device):
     return {
         "modbus": {"url": "{}:{}".format(*modbus_device.address)},
         "listen": {"bind": "127.0.0.1:0"},
+        "unit_id_remapping": {255: 254},
     }
 
 
diff --git a/tests/test_modbus_proxy.py b/tests/test_modbus_proxy.py
index 0a8f47a..db60b89 100644
--- a/tests/test_modbus_proxy.py
+++ b/tests/test_modbus_proxy.py
@@ -20,7 +20,7 @@
 
 from modbus_proxy import parse_url, parse_args, load_config, run
 
-from .conftest import REQ, REP, REQ2, REP2
+from .conftest import REQ, REP, REQ2, REP2, REQ3_ORIGINAL, REP3_MODIFIED
 
 
 Args = namedtuple(
@@ -155,8 +155,9 @@ async def make_requests(modbus, requests):
     [
         (REQ, REP),
         (REQ2, REP2),
+        (REQ3_ORIGINAL, REP3_MODIFIED),
     ],
-    ids=["req1", "req2"],
+    ids=["req1", "req2", "req3"],
 )
 @pytest.mark.asyncio
 async def test_modbus(modbus, req, rep):