[etherlab-dev] ethercat-1.5: Various issues

[Gavin Lambert]

Quoth Frank Heckenbach:
> Perhaps you can at least make sure they compile with the new version.
> Ultimately, Florian will have to integrate the patches into the
> development sources, or not.

Well, they won't all merge/compile cleanly without some changes.  (Also
there's a few cases where variables introduced in one patch aren't used
until a later patch, and the reverse, so some of the intermediate states
won't compile.)  But *most* of the changes are fairly minor.

> Indeed, it doesn't, but the development code as obtained by the "hg
> clone" command as listed on http://etherlab.org/de/ethercat/index.php
> does. Though it uses
> master->io_mutex instead of master->io_sem now. For what I can tell
> (seeing that 1.5.0 doesn't have io_mutex at all), an intermediate
> version introduced the code with io_sem (which I then took) and was
> later changed to io_mutex along with other changes. (This all might not
> be very relevant except to explain where I got the code from; otherwise
> you can treat it like it was one of my patches, I guess.)

I'm not really sure what's going on with the default branch, but as best I
can tell it's outdated and should be ignored.  All the new changes are on
the stable-1.5 branch.  (There hasn't been anything committed to "default"
since 2011.)

> I admit I'm not very proficient with hg, so I probably mixed up the
> commits. I'd have to read up on it and dig deeper, maybe you're faster
> at it. In any case, the cloned code (see above) does contain lock_cb and
> unlock_cb in place of send_cb and receive_cb as in 1.5.0 and 1.5.2. So I
> figure, Florian made this change, but hasn't pushed it into a release
> yet.

I haven't traced the history, but I suspect that (as this was on the default
branch), this was the code before send_cb and receive_cb were introduced in
the first place.  So changing it back would presumably be a regression.  (I
don't have particularly strong feelings about it though because I'm not
using RTAI.)

> I don't know about your application, but in my case, I only want to run
> with the correct (configured) number of slaves anyway.
> Therefore, in my cyclic code I always call ecrt_master_state and check
> that in the result link_up is set and slaves_responding is equal to the
> number I expect; otherwise I abort immediately because something's
> fishy.

Mine is a bit more flexible; I'm trying to get it to support hotplugging as
cleanly as possible if I can, along with a bit of autodetection.

> ec_fsm_coe_exec could return 0 in 1.5.0 already. The return value is
> only used by those callers that go on to do other things after calling
> it (to return early if 0); those that call it at their end don't (need
> to) check the result, and I think that's ok.
> 
> One of my changes was that ec_fsm_master_exec doesn't always return
> 1 after executing a state (it still does in 1.5.2). It's a consequence
> of my previous changes: If CoE is reserved by the slave FSM, the master
> FSM must wait. In order to do that I set
> datagram->state to EC_DATAGRAM_INVALID, and prevent it from being
> queued. Returning 0 from ec_fsm_master_exec is an easy way to achieve
> this (for both callers) since the 0 return (the first branch in
> ec_fsm_master_exec) was already there in case the FSM is still waiting
> for a reply and doesn't execute a state at all (and therefore obviously
> doesn't send a new datagram either).

But in 1.5.2 ec_fsm_coe_exec has "datagram_used" and could return 0 when the
datagram was in INIT, QUEUED, or SENT, which I assumed would occur in some
of the middle states while it was waiting for something to happen, which
didn't sound that different from what you were trying to do.  But you're
right that previously this didn't make any difference to what
ec_fsm_master_exec returned, which seems a little odd.  I wonder if this
might be why I sometimes see "datagram initialised" errors.