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

Do not filter using unit id in the received response #1076

Merged
merged 1 commit into from
Sep 5, 2022
Merged

Do not filter using unit id in the received response #1076

merged 1 commit into from
Sep 5, 2022

Conversation

JonasNorling
Copy link
Contributor

processIncomingPacket() can be told to ignore unit ids other than the specified one or accept all packets by specifying id 0 or 0xff. The modbus client would extract the unit id from the incoming data packet and use that for filtering. At best this is a no-op: the incoming unit id will match itself. At worst it makes us drop responses: if the data is fragmented each fragment was incorrectly parsed to extract a unit id.

Remove the incoming unit id check at this stage. This change is behavior preserving if the response is not fragmented and fixes riptideio#688 if it is fragmented.

Note: I have not tested this with current dev because async RTU seems broken at the moment ("'ModbusRtuFramer' object is not callable").

processIncomingPacket() can be told to ignore unit ids other than the
specified one or accept all packets by specifying id 0 or 0xff. The
modbus client would extract the unit id from the incoming data packet
and use that for filtering. At best this is a no-op: the incoming unit
id will match itself. At worst it makes us drop responses: if the data
is fragmented each fragment was incorrectly parsed to extract a unit
id.

Remove the incoming unit id check at this stage. This change is
behavior preserving if the response is not fragmented and fixes
riptideio#688 if it is fragmented.
@janiversen
Copy link
Collaborator

Seems your dev is a bit old, client_async* and server_async* works:

(pymodbus) examples % ./client_async_basic_calls.py 
11:42:25 INFO  client_async:51 ### Create client object
11:42:25 INFO  client_async:121 ### Client starting
11:42:25 INFO  tcp:100 Protocol made connection.
11:42:25 INFO  client_async_basic_calls:20 ### Reading Coil
11:42:25 INFO  client_async_basic_calls:26 ### Reading Coils to get bit 5
11:42:25 INFO  client_async_basic_calls:32 ### Write true to coil bit 0 and read to verify
11:42:25 INFO  client_async_basic_calls:40 ### Write true to multiple coils 1-8
11:42:25 INFO  client_async_basic_calls:53 ### Write False to address 1-8 coils
11:42:25 INFO  client_async_basic_calls:64 ### Reading discrete input, Read address:0-7
11:42:25 INFO  client_async_basic_calls:73 ### write holding register and read holding registers
11:42:25 INFO  client_async_basic_calls:81 ### write holding registers and read holding registers
11:42:25 INFO  client_async_basic_calls:89 ### write read holding registers
11:42:25 INFO  client_async_basic_calls:107 ### read input registers
11:42:25 INFO  client_async:127 ### End of Program
11:42:25 INFO  tcp:109 Protocol lost connection.
(pymodbus) examples % 

Please show how you get the error.

Copy link
Collaborator

@janiversen janiversen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, this might solve a couple of problems I have seen.

Let's see if CI gives green light.

@janiversen
Copy link
Collaborator

The idea of processIncomingPacket() filtering unitId is not meant for the client, but for the server. If the server is connected as slave on a serial with multiple devices it needs to ignore packets for the other slaves. However there are a bug in that part as well.

@janiversen janiversen merged commit 8905979 into pymodbus-dev:dev Sep 5, 2022
@JonasNorling
Copy link
Contributor Author

Seems your dev is a bit old, client_async* and server_async* works:
[...]
Please show how you get the error.

This fails for me:

$ git describe 
dev2.x-280-g8905979

$ python3 -m examples.client_async_basic_calls --comm serial --port /dev/ttyUSB1 
12:01:23 INFO  client_async:51 ### Create client object
12:01:23 INFO  client_async:121 ### Client starting
12:01:23 WARNING serial:111 Failed to connect: 'ModbusRtuFramer' object is not callable
Traceback (most recent call last):
  File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/jonasn/src/pymodbus/examples/client_async_basic_calls.py", line 123, in <module>
    asyncio.run(run_client(testclient, modbus_calls=demonstrate_calls))
  File "/usr/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.10/asyncio/base_events.py", line 646, in run_until_complete
    return future.result()
  File "/home/jonasn/src/pymodbus/examples/client_async.py", line 123, in run_client
    assert client.protocol
AssertionError

Thanks for responding and merging uncannily quickly :-)

@janiversen
Copy link
Collaborator

you are behind:

(pymodbus) examples % git describe
dev2.x-279-g9304a92

and I just ran a test with the modbusRTUframer.

./client_async_basic_calls.py --framer rtu
12:08:23 INFO  client_async:51 ### Create client object
12:08:23 INFO  client_async:121 ### Client starting
12:08:23 INFO  tcp:100 Protocol made connection.
12:08:23 INFO  client_async_basic_calls:19 ### Reading Coil
12:08:23 INFO  client_async_basic_calls:25 ### Reading Coils to get bit 5
12:08:23 INFO  client_async_basic_calls:31 ### Write true to coil bit 0 and read to verify
12:08:23 INFO  client_async_basic_calls:39 ### Write true to multiple coils 1-8
12:08:23 INFO  client_async_basic_calls:52 ### Write False to address 1-8 coils
12:08:23 INFO  client_async_basic_calls:63 ### Reading discrete input, Read address:0-7
12:08:23 INFO  client_async_basic_calls:72 ### write holding register and read holding registers
12:08:23 INFO  client_async_basic_calls:80 ### write holding registers and read holding registers
12:08:23 INFO  client_async_basic_calls:88 ### write read holding registers
12:08:23 INFO  client_async_basic_calls:106 ### read input registers
12:08:23 INFO  client_async:127 ### End of Program
12:08:23 INFO  tcp:109 Protocol lost connection.
(pymodbus) examples % 

I do not have the serial available at the moment, due to the fact that I am updating the server side, and making some new tests that are more inclusive. However I expect to get around to do the serial testing later this week.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants