Skip to content

Conversation

@jameshilliard
Copy link
Contributor

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

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.

We try to be platform independent, so "posix only" is not acceptable for a parameter. Secondly there are no need for a parameter, since it is never allowed to share the serial port...if there is a need for code it should be built in fixed at the lowest level possible.

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.

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.

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

@jameshilliard
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

jameshilliard commented Feb 3, 2024

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.

stopbits=self.comm_params.stopbits,
baudrate=self.comm_params.baudrate,
parity=self.comm_params.parity,
exclusive=self.comm_params.exclusive,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't this the part of the sync client where I'm already passing the exclusive param to pyserial? Or are you referring to something different?

Handy for preventing two applications from trying to access
the same port simultaneously.
@jameshilliard
Copy link
Contributor Author

jameshilliard commented Feb 3, 2024

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.

@janiversen janiversen closed this Feb 4, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2024
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