See my in-lined comments! BR, Knud
-----Original Message----- From: Gavin Lambert [mailto:gav...@compacsort.com] Sent: 12. februar 2015 23:50 To: Knud Baastrup Cc: etherlab-dev@etherlab.org Subject: RE: [etherlab-dev] Multiple mailbox protocols and other issues On 13 February 2015 03:30, quoth Knud Baastrup: > I have attached a new set of patches (now by using git format-patch, > which also imply that the patch names are given by the commit text). > > I believe that I have addressed the issues you highlighted in prior > mails including the alias support that might be relevant for us as > well sometime in > the future. Nice! Although there still seem to be some funny things going on with the whitespace, eg. see patch 0013's master/fsm_slave_config.c's second hunk (ec_fsm_slave_config_enter_mbox_sync). [Knud] I guess I need more help to figure this out. I cannot (with my current knowledge of patch management) see anything wrong in this specific hunk (line 374 to 476). Do you get some kind of warning when applying the patch or how do you observe the issue? > The locks that conflicts with RTAI could be removed with a define > guard, e.g. > re-use the EC_RTDM define already available? They're not conflicts, and I wasn't suggesting any specific changes (as I said, I don't use RTDM myself so I don't really know specifics). It was just a comment to indicate why the master originally didn't do any locking there, and that your original problem *could* theoretically have been solved by doing the locking in the application code instead, as it's only an issue with concurrent realtime tasks, which are likely to need some application-level locking anyway. It's a possible reason that Florian might not want to accept the patch, but that doesn't mean that you should modify or withdraw it -- that's something he can decide. [Knud] Agree, we will keep the locks and let Florian decide to what extend he will include the patch. I'll integrate your new patchset into my build and do a bit more testing; I'm hoping to post my full patch queue in a few days. This will include your patches as well -- I hope you don't mind? (They'll be clearly attributed, of course.) [Knud] That is fully ok. I believe it will just further improve the chances for getting the patches merged into trunk by Florian. Feel free to do any improvements on the patches as well. Just some further thoughts on patch 0010 (deferring the sdo dictionary fetch): one of the interesting things about fsm_slave over fsm_master is that the former can run in parallel while the latter only in series. In principle, this means that if someone issues "ethercat sdos" with no filters on a large network, the fetch time could be reduced considerably. (It won't reduce the time to that of a single slave, as it has caps on the number of slave FSMs it can run in parallel to prevent blowing out the number of frames and causing latency.) Currently your patch forces this to still run sequentially anyway, because the ioctl is blocking and it only does one slave at a time. I was thinking about having a go at trying to make that change myself, but having said that, given that running "ethercat sdos" on multiple slaves is not particularly useful (since networks usually contain many duplicates) and that this is generally only used during development or commissioning, I'm not sure whether it's really worth it. [Knud] I agree, it is tempting now to take next step and do this in parallel. It currently requires a patient user to wait several minutes for an ethercat sdos command (with no progress information) to complete. Today we as well only fetch the dictionary for debug and test purposes so we do not plan to further improve this for the moment. I was also wondering if it should do a more limited dictionary fetch by default (just of the PDOs), which could improve some of the logging, but even then those messages only appear when the debug level is increased, so most of the time it wouldn't be all that useful. And someone can just look at the slave docs (or a local dictionary scan) if they want to interpret logs to find out what a particular SDO entry is called from its index:subindex. _______________________________________________ etherlab-dev mailing list etherlab-dev@etherlab.org http://lists.etherlab.org/mailman/listinfo/etherlab-dev
