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

Simplify server implementation. #1071

Merged
merged 2 commits into from
Sep 8, 2022
Merged

Simplify server implementation. #1071

merged 2 commits into from
Sep 8, 2022

Conversation

janiversen
Copy link
Collaborator

Remove sync. server implementation but keep StartServer as well as StartAsyncServer to allow sync. application to spin up a server.

Changed Async StartServer to StartAsyncServer to differentiate.

Moved all external symbols to init.py to allow consistent implementation independent access for applications.

@janiversen
Copy link
Collaborator Author

@dhoomakethu you might want to check this with your setup before it gets merged.

@dhoomakethu
Copy link
Contributor

Will do it today

@dhoomakethu
Copy link
Contributor

Not sure if this is being handled in a different branch but can't run the serial version of client

python3 examples/client_sync_basic_calls.py --comm serial --framer rtu --log debug
--port /tmp/ttyp0
usage: client_sync_basic_calls.py [-h] [--comm {tcp,udp,serial,tls}] [--framer {ascii,binary,rtu,socket,tls}]
                                  [--log {critical,error,warning,info,debug}] [--port PORT]
client_sync_basic_calls.py: error: argument --port: invalid int value: '/tmp/ttyp0'

@dhoomakethu
Copy link
Contributor

REPL serial client also seems to be having issues.

(pymodbus) ➜  pymodbus git:(dev_server) ✗ pymodbus.console serial --method rtu --port /tmp/ttyp0 --baudrate 9600 --timeout 3

----------------------------------------------------------------------------
__________          _____             .___  __________              .__
\______   \___.__. /     \   ____   __| _/  \______   \ ____ ______ |  |
 |     ___<   |  |/  \ /  \ /  _ \ / __ |    |       _// __ \\____ \|  |
 |    |    \___  /    Y    (  <_> ) /_/ |    |    |   \  ___/|  |_> >  |__
 |____|    / ____\____|__  /\____/\____ | /\ |____|_  /\___  >   __/|____/
           \/            \/            \/ \/        \/     \/|__|
                                        v1.3.0 - [pymodbus, version 3.0.0.dev5]
----------------------------------------------------------------------------

> client.read_holding_registers address=0 count=1 unit=4
TypeError("__init__() got multiple values for argument 'unit'")
>

@janiversen
Copy link
Collaborator Author

Interesting, let me have a look.

@janiversen
Copy link
Collaborator Author

The latter is because you should use slave= not unit= as pr documentation

@dhoomakethu
Copy link
Contributor

dhoomakethu commented Aug 29, 2022

I could get the serial client example working

diff --git a/examples/client_sync.py b/examples/client_sync.py
index c9a257d3..a5801da0 100755
--- a/examples/client_sync.py
+++ b/examples/client_sync.py
@@ -158,7 +158,7 @@ def get_commandline():
     parser.add_argument(
         "--port",
         help="the port to use",
-        type=int,
+        type=str,
     )
     args = parser.parse_args()

however the server threw this error

(pymodbus) ➜  pymodbus git:(dev_server) ✗ python3 examples/server_sync.py --comm serial --framer rtu --log debug --port /tmp/ptyp0 --slaves 2 --store sequential
19:08:30 INFO  server_sync:72 ### Create datastore
19:08:30 INFO  server_sync:149 ### start server, listening on /tmp/ptyp0 - serial
19:08:30 DEBUG selector_events:59 Using selector: KqueueSelector
19:08:30 DEBUG async_io:118 Serial connection opened on port: /tmp/ptyp0
19:08:30 DEBUG async_io:444 Serial connection established
19:08:37 DEBUG async_io:210 Handling data: 0x1 0x1 0x0 0x1 0x0 0x1 0xac 0xa
19:08:37 DEBUG rtu_framer:193 Getting Frame - 0x1 0x0 0x1 0x0 0x1
19:08:37 DEBUG factory:220 Factory Request[ReadCoilsRequest': 1]
19:08:37 DEBUG rtu_framer:116 Frame advanced, resetting header!!
19:08:37 ERROR async_io:268 Datastore unable to fulfill request: 'dict' object has no attribute 'validate'; Traceback (most recent call last):
  File "/Users/sanjay/repo/pymodbus/pymodbus/server/async_io.py", line 257, in execute
    response = request.execute(context)
  File "/Users/sanjay/repo/pymodbus/pymodbus/bit_read_message.py", line 164, in execute
    if not context.validate(self.function_code, self.address, self.count):
AttributeError: 'dict' object has no attribute 'validate'

19:08:37 ERROR pdu:118 Exception Response(129, 1, SlaveFailure)
19:08:37 DEBUG async_io:285 send: [Exception Response(129, 1, SlaveFailure)]- b'0181044193'

When run against basic sync client example

(pymodbus) ➜  pymodbus git:(dev_server) ✗ python3 examples/client_sync_basic_calls.py --comm serial --framer rtu --log debug
--port /tmp/ttyp0
19:08:37 INFO  client_sync:50 ### Create client object
19:08:37 INFO  client_sync:119 ### Client starting
19:08:37 INFO  client_sync_basic_calls:17 ### Reading Coil
19:08:37 DEBUG transaction:130 Current transaction state - IDLE
19:08:37 DEBUG transaction:134 Running transaction 1
19:08:37 DEBUG transaction:291 SEND: 0x1 0x1 0x0 0x1 0x0 0x1 0xac 0xa
19:08:37 DEBUG base:194 New Transaction state "SENDING"
19:08:37 DEBUG transaction:312 Changing transaction state from "SENDING" to "WAITING FOR REPLY"
19:08:37 DEBUG transaction:428 Changing transaction state from "WAITING FOR REPLY" to "PROCESSING REPLY"
19:08:37 DEBUG transaction:327 RECV: 0x1 0x81 0x4 0x41 0x93
19:08:37 DEBUG rtu_framer:193 Getting Frame - 0x81 0x4
19:08:37 DEBUG factory:352 Factory Response[129]
19:08:37 DEBUG rtu_framer:116 Frame advanced, resetting header!!
19:08:37 DEBUG transaction:516 Adding transaction 1
19:08:37 DEBUG transaction:528 Getting transaction 1
19:08:37 DEBUG transaction:234 Changing transaction state from "PROCESSING REPLY" to "TRANSACTION_COMPLETE"
Traceback (most recent call last):
  File "examples/client_sync_basic_calls.py", line 121, in <module>
    run_client(testclient, modbus_calls=demonstrate_calls)
  File "/Users/sanjay/repo/pymodbus/examples/client_sync.py", line 122, in run_client
    modbus_calls(client)
  File "examples/client_sync_basic_calls.py", line 113, in demonstrate_calls
    handle_coils(client)
  File "examples/client_sync_basic_calls.py", line 19, in handle_coils
    assert not rr.isError()  # test that call was OK
AssertionError

@dhoomakethu
Copy link
Contributor

The latter is because you should use slave= not unit= as pr documentation

I will fix this once you merge this branch to dev

@janiversen
Copy link
Collaborator Author

I am trying to debug the problems. I saw the validate error quite a while ago, but thought it was gone.

@janiversen
Copy link
Collaborator Author

And let me see if I can add an Exception if unit= is used.

@dhoomakethu
Copy link
Contributor

dhoomakethu commented Aug 29, 2022

And let me see if I can add an Exception if unit= is used.

How about we implicitly use slave when unit is passed to be backward compatible ?
We can also provide a deprecation warning for the time being and remove it later in one of the major 3.x release.

@dhoomakethu
Copy link
Contributor

Async client errors

 ✗ python3 examples/client_async_basic_calls.py --comm serial --framer rtu --log debug --port /tmp/ttyp0
19:14:06 INFO  client_async:51 ### Create client object
19:14:06 DEBUG selector_events:59 Using selector: KqueueSelector
19:14:06 DEBUG selector_events:59 Using selector: KqueueSelector
19:14:06 INFO  client_async:121 ### Client starting
19:14:06 DEBUG serial:94 Starting serial connection
19:14:06 WARNING serial:111 Failed to connect: 'ModbusRtuFramer' object is not callable
Traceback (most recent call last):
  File "examples/client_async_basic_calls.py", line 123, in <module>
    asyncio.run(run_client(testclient, modbus_calls=demonstrate_calls))
  File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.8/lib/python3.8/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.8/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/Users/sanjay/repo/pymodbus/examples/client_async.py", line 123, in run_client
    assert client.protocol
AssertionError

@janiversen
Copy link
Collaborator Author

And let me see if I can add an Exception if unit= is used.

How about we implicitly use slave when unit is passed to be backward compatible ?

that is another possibility, let me see what I can come up with. I will add some commits here and request a new review when ready.

@janiversen
Copy link
Collaborator Author

The "validate" is very interesting it happens only serial comm. But I can see that tcp have a problem with the rtuframer, which should be ok.

But at least I can reproduce the problems now, I will fix it and get back to you.

@janiversen janiversen marked this pull request as draft August 29, 2022 15:25
@janiversen janiversen force-pushed the dev_server branch 15 times, most recently from 7a90083 to 80bf4c3 Compare September 3, 2022 16:22
@janiversen janiversen marked this pull request as ready for review September 7, 2022 18:00
@janiversen
Copy link
Collaborator Author

@dhoomakethu I think I have solved all the problems you saw and a lot more, there was quite a number of small errors in the library code.

Furthermore I rewrote text_examples.py to give a better coverage.

@dhoomakethu
Copy link
Contributor

@janiversen I will give this a try over the weekend. Please do not wait for me if you are comfortable with the changes, we can capture any new findings as separate github issues or enhancements.

@janiversen
Copy link
Collaborator Author

Sounds like a good plan, looking forward to hear about your findings. I am working on getting TLS to work in test (adding self-signed certificates) and after that I will write a mock to simulate from serial_asyncio.
.

@janiversen janiversen merged commit ff6341f into dev Sep 8, 2022
@janiversen janiversen deleted the dev_server branch September 8, 2022 06:35
@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
Development

Successfully merging this pull request may close these issues.

2 participants