search

tcunit / TcUnit

An unit testing framework for Beckhoff's TwinCAT 3
Other
273 stars 75 forks source link

Too many ADSLOGSTR() causes some to become lost #35

Closed sagatowski closed 5 years ago

sagatowski commented 5 years ago

As discovered in issue #26, doing many ADSLOGSTR(), it seems that the AMS-router looses some of them. For instance, running the TcUnit-Verifier makes it report different results (in terms of how many error-messages are shown) with every run!

This can (hopefully) be solved by creating a queue out of each ADSLOGSTR()-message, and consume them one-by-one with a timer inbetween, basically a buffer.

Aliazzzz commented 5 years ago

I have the following concern, as I am affraid that your suggested workaround is not going to tackle the real issue, it is just going to obfuscate the underlying problem further.

The AMS router acts as a buffer, so why add a new buffer next to it? The only thing that this causes is extra overhead and flow-control in project software, which makes using a buffer pointless. Yes, the message drop effect has been mitigated but maybe there is another less costly resolution to this issue?

We can monitor the ADS Router Message Qeueue buffersize with the following function; https://infosys.beckhoff.com/english.php?content=../content/1033/tcplclib_tc2_utilities/9007199290109451.html&id= If it is possible to increase the ADS Router Message Qeueue buffersize, we should focus on that first.

sagatowski commented 5 years ago

Increasing the router memory will do zero difference. The default size is 32mb large, which is quite big. I did a test by running the TcUnit-Verifier, and even with all those 136 ADSLOGSTR-messages, I didn't even use 15kb of that, including the actual program. Even if it did a difference, this would require all users of their libraries/plc-code to increase the router memory just for the unit tests which would be far from optimal.

You don't need to use any code to do the monitoring by the way, this is available in the window "AMS Router information".

I'm all for a simpler solution to the problem, and I'm open for suggestions that are working.

sagatowski commented 5 years ago

Solved in commit 420275d.