pymodbus-dev / pymodbus

A full modbus protocol written in python
Other
2.29k stars 938 forks source link

Add support for pyserial exclusive access mode. #1961

Closed jameshilliard closed 9 months ago

jameshilliard commented 9 months ago

Handy for preventing two applications from trying to access the same port simultaneously.

jameshilliard commented 9 months ago

We try to be platform independent, so "posix only" is not acceptable for a parameter.

I think other platforms may behave more like exclusive=True.

Secondly there are no need for a parameter, since it is never allowed to share the serial port

What's actually enforcing that?

if there is a need for code it should be built in fixed at the lowest level possible.

Isn't that what this is doing? It's passing the exclusive param to pyserial which implements this exclusivity using low level os features.

Furthermore I am not sure I understand the problem, currently 2 app cannot open the same device (in this case serial port), this is handled at OS level at least on my machine.

What platform are you using? I've seen issues with pyserial that this should reliably prevent, such as multiple applications instances trying to use the same serial device at the same time.

I think you are not setting the exclusive parameter for the sync client.

Oh, where is that one? It appears to me that there is sync client code I added it to.

janiversen commented 9 months ago

Normally the OS prevent a device from being opened multiple times in parallel, depending on the parameters in your open call.

You added the exclusive to the pyserial call in the async mode, but you did not add it in the sync mode.

I do not understand your question "Oh, where is that one?", Pyserial is sync, which is why we have a special async addon in transport.

I have no problem accepting exclusive=true in the pyserial call and leave it to pyserial to handle it on different platforms....but I do not see the need to make it a parameter in pymodbus, since we always demand exclusive access.

I work on different platforms, currently mostly on macos and raspian.

jameshilliard commented 9 months ago

Normally the OS prevent a device from being opened multiple times in parallel, depending on the parameters in your open call.

So that's from my understanding what passing exclusive to pyserial essentially tells the OS to enforce, by setting the exclusive lock here by calling fcntl.flock(self.fd, fcntl.LOCK_EX | fcntl.LOCK_NB) on the file descriptor.

You added the exclusive to the pyserial call in the async mode, but you did not add it in the sync mode.

Ok, I'll go through the code again and try to find where to add it for sync mode.

I have no problem accepting exclusive=true in the pyserial call and leave it to pyserial to handle it on different platforms

Isn't that what this is doing essentially? It's passing exclusive from pymodbus to pyserial. Note we can't unconditionally set exclusive=true in our pyserial call since pyserial will throw an exception if it is set on an unsupported platform.

but I do not see the need to make it a parameter in pymodbus, since we always demand exclusive access.

I'm confused, how would we pass the param from pymodbus to pyserial without making it also a pymodbus param? I also don't see how we currently demand exclusive access since we don't currently set the exclusive param in pyserial.

I work on different platforms, currently mostly on macos and raspian.

Hmm, so I recall seeing issues on linux based platforms where if you don't set the exclusive param in pyserial you can get multiple simultaneous connections to the same serial port. Maybe macos enforces exclusive access without a fcntl.flock(self.fd, fcntl.LOCK_EX | fcntl.LOCK_NB) on the file descriptor? The pr that added this exclusive param to pyserial also indicates that Linux by default will not enforce exclusive port access but windows always will.

jameshilliard commented 9 months ago

I checked again and it looks like setting exclusive=True unconditionally should work fine on all platforms, it appears the windows client will only throw an exception for exclusive=False(since port access on windows is always exclusive). I've opened #1964 as an alternative to this which will unconditionally pass exclusive=True to pyserial.