Update use pyserial RS485 #2205

samskiter · 2024-05-27T19:42:08Z

I think this is a fix of the tests?

samskiter · 2024-05-27T23:25:54Z

Can't get this passing - it seems that serial_for_url is used by this library, but there's no way to use that to produce instances of pyserial's RS485 class. Perhaps the solution is to implement some kind of custom URL scheme for this purpose?

janiversen

Seems your fix breaks quite a number of tests, please fix.

samskiter · 2024-05-28T06:42:36Z

Open to suggestions - I can't seem to fix it!

janiversen · 2024-05-28T08:01:21Z

I have no suggestions, the serial library is only used in serial transport and there are no other libraries being used.

serial_for_url is essential for pymodbus since it allows simulating serial communication on a socket, but it's strange if the rs485 do not supply it.

samskiter · 2024-05-31T13:04:17Z

OK, I think this is actually a bit of an awkward solution, but ultimately the correct thing to do.

The purpose of using the RS485 class is to be able to configure RS485Settings, and without injecting those, I'll have to reach inside pymodbus and grab things I shouldn't to set rs485_mode on the sync_serial client. So I opted to allow these settings to be injected along with us in the commparams - there is precedent for params in there to be unused for some transports (e.g. the port parameter is not used if you have a serial commtype)

I hope this is an acceptable fix

test/transport/test_serial.py

janiversen

Minor details, the only bigger thing is your use (or not use) of comm_params.

We are in the process of preparing v3.7.0 so if you hurry this PR can be part of 3.7.0

pymodbus/client/base.py

pymodbus/transport/serialtransport.py

pymodbus/transport/transport.py

test/transport/test_serial.py

janiversen · 2024-06-03T08:50:45Z

It is easiest to run locally "pytest --cov"

janiversen · 2024-06-09T16:20:29Z

pymodbus/client/serial.py

@@ -15,6 +15,7 @@

 try:
    import serial
+    from serial.rs485 import RS485Settings


It is already imported in the line above, do not import twice.

janiversen · 2024-06-09T16:29:35Z

pymodbus/transport/serialtransport.py

@@ -8,25 +8,36 @@

 with contextlib.suppress(ImportError):
    import serial
+    import serial.rs485


It is already imported in the line above.

janiversen · 2024-06-09T16:32:34Z

pymodbus/transport/transport.py

@@ -58,6 +58,8 @@
 from functools import partial
 from typing import Any

+from serial.rs485 import RS485Settings


What happens if serial is not installed ?? Please import as serial is imported in other places.

janiversen · 2024-06-09T16:33:53Z

test/transport/test_serial.py

@@ -7,6 +7,7 @@

 import pytest
 import serial
+from serial.rs485 import RS485Settings


Already imported in the line above, please do not import twice.

janiversen · 2024-06-14T11:58:53Z

We have for other reasons decided to make a v3.6.9 that will be done this weekend, so I need an updated PR from you if you want it included.

janiversen · 2024-07-18T09:19:24Z

Still working on this ?

yawor · 2024-07-25T13:19:04Z

I have a personal project where I write a new software for already existing hardware with RS485 transceiver, but it is based on an AllWinner H3 SBC, where hardware flow control doesn't work very well, but I need to control RTS pin to control the transceiver's direction. So I really would like to have this feature merged, but (with all due respect to @samskiter) I don't think this is implemented properly.

SerialTransport adapts a non-asyncio serial.Serial class by adding async readers and writers to monitor serial port's file descriptor and they invoke callbacks to either read or write the data in a non-blocking fashion using normally blocking read and write methods. But RS485.write method adds additional, blocking delays (for example, it always flushes the port after each write), which should not be executed in the async writer's callbacks, because it's going to block the loop.

Also there's no guarantee that the async writer is going to call the callback only once per SerialTransport.write call as it's possible that the writer would split larger data into smaller chunks. In that case there would be multiple calls to RS485.write method for a single SerialTransport.write call, causing RTS to be toggled (potentially multiple times) in the middle of a single modbus operation, causing it to fail.

That's why the optional software RTS control should be implemented directly in the SerialTransport class, which is actually aware when the whole frame has been sent and RTS can be switched from TX to RX. It would be even better if it could just call program provided callbacks (either sync or async) before transmission starts and after it ends. That way a program could even use any available generic GPIO pin to control the transceiver direction, not just RTS pin.

janiversen · 2024-07-25T15:35:51Z

In general I agree with your arguments, we have over time had many problems with serial, a lot because it is no longer actively maintained.

We have been thinking of changing serial to another library, but never got time to actually do it.

I would however like to see a good RS485 integration! and e.g. enhac3 the transport layer to control RTS would also be welcomed.

So in short, and since this PR looks abandoned, I hope you will submit your ideas as a PR.

janiversen · 2024-07-25T15:43:34Z

I think RTS control can be implemented in the current serial transport, with an extra parameter to turn it on.

The best way would be to make a PR that implement it in serial transport with test, and a second that add the parameter to comm_params and the client/server api.

samskiter · 2024-07-26T06:53:07Z

Sorry for my delays, I will try and get this pr over the line!

…

On Thu, 25 Jul 2024, 16:43 jan iversen, ***@***.***> wrote: I think RTS control can be implemented in the current serial transport, with an extra parameter to turn it on. The best way would be to make a PR that implement it in serial transport with test, and a second that add the parameter to comm_params and the client/server api. — Reply to this email directly, view it on GitHub <#2205 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJWOWZUT4AGRFLDJ7K6FO3ZOEML3AVCNFSM6AAAAABILVM376VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJQG4ZTINBYGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

janiversen · 2024-07-26T09:09:26Z

@samskiter good luck, please resolve the conflicts first and then have a close look at the comments from @yawor which I think have a lot of merit.

Ohh and do not forget the review comments I made.

github-actions · 2024-09-10T02:28:25Z

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions · 2024-09-20T02:30:24Z

This PR was closed because it has been stalled for 10 days with no activity.

jameshilliard · 2024-11-19T22:59:40Z

But RS485.write method adds additional, blocking delays (for example, it always flushes the port after each write), which should not be executed in the async writer's callbacks, because it's going to block the loop.

So it appears pyserial also has some native rs485 support that AFAIU would avoid these issues, reworked this a bit to use that instead in #2460.

samskiter added 9 commits May 27, 2024 20:41

Update use pyserial RS485

33fac59

Correct tests access to async_loop

d85f1e9

I think this is a fix of the tests?

Update test_serial.py

7010a32

Update test_serial.py

2e3e3b4

Update test_serial.py

36a11fb

Update serialtransport.py

eb7bfa0

Update test_serial.py

1c38542

Update test_serial.py

bc598a0

Update serialtransport.py

0893839

janiversen requested changes May 28, 2024

View reviewed changes

samskiter added 9 commits May 31, 2024 13:47

Update transport.py

577848a

Update transport.py

f7381e3

Update serialtransport.py

0dbf835

Update serialtransport.py

a90488c

Update serialtransport.py

61007d4

Update transport.py

1b6cd21

Update test_serial.py

0ee5b75

Update serialtransport.py

69622df

Update transport.py

13868fa

Merge branch 'dev' into patch-1

feb4da1

samskiter requested a review from janiversen May 31, 2024 13:04

samskiter commented May 31, 2024

View reviewed changes

test/transport/test_serial.py Show resolved Hide resolved

Update base.py

7209eb0

janiversen requested changes May 31, 2024

View reviewed changes

some fixes

e6eec1a

fix test

f2a8215

samskiter requested a review from janiversen June 6, 2024 17:37

janiversen requested changes Jun 9, 2024

View reviewed changes

Merge branch 'dev' into patch-1

11d7899

janiversen marked this pull request as draft July 20, 2024 06:49

github-actions bot added the no-pr-activity label Sep 10, 2024

github-actions bot closed this Sep 20, 2024

jameshilliard mentioned this pull request Nov 19, 2024

Allow configuring pyserial hardware RS485 settings #2460

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update use pyserial RS485 #2205

Update use pyserial RS485 #2205

samskiter commented May 27, 2024

samskiter commented May 27, 2024

janiversen left a comment

samskiter commented May 28, 2024

janiversen commented May 28, 2024

samskiter commented May 31, 2024

janiversen left a comment

janiversen commented Jun 3, 2024

janiversen Jun 9, 2024

janiversen Jun 9, 2024

janiversen Jun 9, 2024

janiversen Jun 9, 2024

janiversen commented Jun 14, 2024

janiversen commented Jul 18, 2024

yawor commented Jul 25, 2024 •

edited

Loading

janiversen commented Jul 25, 2024

janiversen commented Jul 25, 2024

samskiter commented Jul 26, 2024 via email

janiversen commented Jul 26, 2024

github-actions bot commented Sep 10, 2024

github-actions bot commented Sep 20, 2024

jameshilliard commented Nov 19, 2024

Update use pyserial RS485 #2205

Update use pyserial RS485 #2205

Conversation

samskiter commented May 27, 2024

samskiter commented May 27, 2024

janiversen left a comment

Choose a reason for hiding this comment

samskiter commented May 28, 2024

janiversen commented May 28, 2024

samskiter commented May 31, 2024

janiversen left a comment

Choose a reason for hiding this comment

janiversen commented Jun 3, 2024

janiversen Jun 9, 2024

Choose a reason for hiding this comment

janiversen Jun 9, 2024

Choose a reason for hiding this comment

janiversen Jun 9, 2024

Choose a reason for hiding this comment

janiversen Jun 9, 2024

Choose a reason for hiding this comment

janiversen commented Jun 14, 2024

janiversen commented Jul 18, 2024

yawor commented Jul 25, 2024 • edited Loading

janiversen commented Jul 25, 2024

janiversen commented Jul 25, 2024

samskiter commented Jul 26, 2024 via email

janiversen commented Jul 26, 2024

github-actions bot commented Sep 10, 2024

github-actions bot commented Sep 20, 2024

jameshilliard commented Nov 19, 2024

yawor commented Jul 25, 2024 •

edited

Loading