[etherlab-dev] Support for multiple mailbox protocols

[Gavin Lambert]

Hi Knud,



I haven't reviewed or tested the whole patch yet, but I like the idea in
concept.  One thing that made me pause though is the way that the check_mbox
flag is handled.  You have it protected by a semaphore (implying you're
expecting it to get used concurrently), but you have non-atomic
test-and-then-set actions which will mean that if concurrent access is
attempted multiple users might call prepare_check and check_mbox_set
(causing some of them to be "lost", which may or may not be a problem), and
then on failure one might call check_mbox_clear while another is still
waiting for a check to occur (which seems like it would be a problem).

 

Again, I haven't followed the logic all the way through yet so possibly this
isn't a real problem, but it bothers me. J

 

If the intent is to have only one concurrent state machine trigger a check
datagram at a time (which it seems like it is), then you should probably be
using an atomic test-and-set operation instead.

 

Additionally it doesn't look like you have any protection against concurrent
CoE access (which TBH I'm not entirely sure whether this occurs, but Frank's
patch 27 suggests it does), and I'm definitely not a fan of
allocating/deallocating memory on each mailbox transfer, which is what it
looks like you're doing.

 

Regards,

Gavin Lambert

 

From: etherlab-dev-bounces at etherlab.org
[mailto:etherlab-dev-bounces at etherlab.org] On Behalf Of Knud Baastrup
Sent: Wednesday, 25 June 2014 01:13
To: etherlab-dev at etherlab.org
Subject: Re: [etherlab-dev] Support for multiple mailbox protocols

 

Hi !

 

I just discovered that the provided patch included a hardcoded mailbox size
that I have now replaced with a dynamic allocated buffer. I have attached a
new patch (ethercat_152_stable_mailbox_1.patch) that fully replaces the
prior patch (ethercat_152_stable_mailbox.patch).

 

Thanks,

 

Knud Baastrup

 

 

From: etherlab-dev-bounces at etherlab.org
[mailto:etherlab-dev-bounces at etherlab.org] On Behalf Of Knud Baastrup
Sent: 23. juni 2014 14:27
To: etherlab-dev at etherlab.org
Subject: [etherlab-dev] Support for multiple mailbox protocols

 

Hello Florian, Gavin, Frank (and others facing the lack of support for
multiple mailbox protocols)

 

 

I have like Frank Heckenbach and Gavin also struggled with the lack of
support for multiple mailbox protocols and came up with an alternative
solution to the one provided by Frank in patch 9-10-11. 

 

I have attached the patch that is based on the stable-1.5 branch. The patch
should support all the mailbox protocols, but has only been tested with CoE,
EoE and FoE.

 

I will in few lines try to summarize the patch:

In this patch I accept that a mailbox read request (e.g. FP-RD) for a given
mailbox protocol can return data from any other mailbox protocol running at
the same time. The data returned by a read datagram is therefore stored in a
separate buffer for each mailbox protocol instead of the datagram data
buffer. The mailbox state machines will check and fetch the data from their
own buffer instead of the datagram buffer (that is no longer used for
mailbox read data). A check_mbox flag is introduced to track when a given
slave has an ongoing mailbox read request. In normal case the mailbox state
machine will run as previously if no mailbox read request is ongoing, but if
a mailbox read-request is ongoing (check_mbox flag is set) it will check its
own mailbox buffer (as the ongoing mailbox read request might have returned
its data) and otherwise wait until the read request is done and it gets the
opportunity to reserve the mailbox for its own read request.

 


Venlig hilsen / Best regards, 

Knud Baastrup 
DEIF Wind Power Technology
SW Developer

Direct.: +45 9614 8458
E-mail:  <mailto:kba at deif.com> kba at deif.com
---------------------------------------------------------------

Retrofit your Vestas COTAS controller and optimize availability that will
improve your annual energy generation, reduce service cost and extend the
lifetime of your turbine.
V27 V39 V44 V47

 
<http://www.deifwindpower.com/retrofit.aspx?utm_source=Retrofit&utm_medium=e
mail%20signatur&utm_term=Retrofit%2BVestas%2BCOTAS&utm_content=textlink&utm_
campaign=Retrofit> Read more about DEIF's solutions to retrofit your
turbines on our website

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.etherlab.org/pipermail/etherlab-dev/attachments/20140625/482c4bd5/attachment-0002.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.png
Type: image/png
Size: 3536 bytes
Desc: not available
URL: <http://lists.etherlab.org/pipermail/etherlab-dev/attachments/20140625/482c4bd5/attachment-0008.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image002.png
Type: image/png
Size: 2940 bytes
Desc: not available
URL: <http://lists.etherlab.org/pipermail/etherlab-dev/attachments/20140625/482c4bd5/attachment-0009.png>