Beckhoff / ADS

Beckhoff protocol to communicate with TwinCAT devices.
MIT License
502 stars 194 forks source link

Multithreading #86

Closed bogdanul2003 closed 5 years ago

bogdanul2003 commented 5 years ago

Hi,

I'm trying to post sync requests (read/write) from two different threads using the same ADS port. I've notced that I get this warning message "Port: 30000 already in use as 00000065E91FF960" in AmsConnection::Reserve() function.

From what I could understand by looking at the library code, there seems to be an array that stores requests/responses for each port and if I'll try to post another request on the same port that is still waiting for a response, it will fail with the previous message.

Do you plan to make this threadsafe or should I use one ADS port per thread ?

Thanks.

pbruenn commented 5 years ago

If you come up with a use case were the optimal solution would require ports to allow multiple pending requests, I would agree and say it's worth to implement this.

But at the moment I would say AmsPorts are very different from normal ports. I would expect an AmsPort to connect to a specific AmsPort at a specific AmsNetId, just like TCP. But look at the reference library. You just open AmsPorts to request some resources. You never bind them to specific targets. So in that philosophy I would argue AmsPorts are for resource allocation. So if you need to wait for 1000 pending AmsRequests in parallel, just open 1000 AmsPorts for that.

The current synchronization mechanism is a single atomic pointer exchange. Increasing the number of allowed pending requests per port above 1 would significantly increase the management overhead.

Regards, Patrick

bogdanul2003 commented 5 years ago

Thanks for the explanation, I was not aware of this philosophy, kind of new to beckhoff environment. Does this mean that you should request a new port for every AmsRequest ? My use case, and I think this use case is common, is that from one thread I'm reading some PLC variables like commands that can come from the PLC and from another thread I write some PLC variables that are the result of some calculations done in threadpool on the PC side. I think that using one port per thread should solve my problem if each thread uses sync requests. Will come back with feedback.

pbruenn commented 5 years ago

No, don't request a port per request. I would say request a port per thread.

In case you use notifications, I think they are already concurrent per notification handle. If not you could implement your notification callback like an interrupt handler to achieve better concurrency.