Lock async_execute operations for concurrent task safety.

[jameshilliard]

I think this should make calling methods operations like read_holding_registers from concurrent tasks on the same client safe.

[alexrudd2]

I lean slightly toward implementing locking on serial code. The slight difference between TCP/serial usage would not bother me; arguably having a different API is per the spec, anyhow.

In any case, docs on this issue would be good - I think most users will want to implement application-level locking even for TCP because of the misbehaving servers. Then again, we're all using Python.... not the right language if you want error checking 😆

[jameshilliard]

I lean slightly toward implementing locking on serial code. The slight difference between TCP/serial usage would not bother me; arguably having a different API is per the spec, anyhow.

I think a queuing approach may be more universal, been experimenting with that sort of things in #1982 a little which seemed to work reasonably well for serial connections at least.

In any case, docs on this issue would be good - I think most users will want to implement application-level locking even for TCP because of the misbehaving servers.

I'm pretty skeptical application devs want to implement application side locking, I mean there may be cases where that's necessary but I think it should be possible to make the API much more safe so that locking wouldn't be required in most cases.

[alexrudd2]

I'm pretty skeptical application devs want to implement application side locking, I mean there may be cases where that's necessary but I think it should be possible to make the API much more safe so that locking wouldn't be required in most cases.

Fair enough. I'm hesitating to state a strong opinion because I don't feel qualified enough to make architecture decisions. :)

[janiversen]

I think adding queuing for the framers that do not support transaction ID: rtu_framer, ascii_framer, binary_framer (remark this is about to die, because it´s non standard

A queue would allow to simulate transactions for those framers, and would make the API consistent in respect of concurrency.

There are a lot of misbehaving slaves out there, BUT the library cannot and should not try to solve those problems. Pymodbus implement the modbus standard not deviations, that is a major blocker !

We have in the past rejected pull requests, trying to e.g. implement registers as being 32 bits (A special Dutch wish).

We should one day write a architecture document, but it is actually quite simple:

Client classes Mixin transaction framers transport

Server classes Datastore transaction framers transport

Server is solely async, client is sync/async. Note that midterm the internals of the client will be only async.

[jameshilliard]

We have in the past rejected pull requests, trying to e.g. implement registers as being 32 bits (A special Dutch wish).

I wonder...would someone be able to implement something like this in their application by providing a custom framer to pymodbus?

[janiversen]

We have in the past rejected pull requests, trying to e.g. implement registers as being 32 bits (A special Dutch wish).

I wonder...would someone be able to implement something like this in their application by providing a custom framer to pymodbus?

Possible is everything...but I will not start guessing what e.g. the assumption that a register is 32 bits, will do to the internals of pymodbus.

But really it is not a problem, because modbus defines a register to be 16 bit, and therefore pymodbus implements 16 bits. Pymodbus is not intended for all the variants out there ! pymodbus implement the standard not everything else.

[janiversen]

Still working on this one, or is it being replaced by your queue idea ?

I think adding a lock on async serial makes a lot of sense, and is a simple way to avoid problems.

[jameshilliard]

closing in favor of #2030