On 2 February 2015 20:18, you quoth: > > I will just update you on some additional pathes I have prepared for EtherCAT > Master. I have attached the complete set of patches that we currently use, > but only the below patches have been updated added.
I'm just having a more detailed look through these patches now, and there's a few niggles. ;) 1. There are several files that appear to have tabs in them; it's usually a good idea when sharing patches with others to use spaces only, as different people/editors use different tab sizes. 2. There's several diffs in various files (eg. 12_sdo_directory.patch's master/ioctl.h) that contain only whitespace changes on various lines for no readily apparent reason. These sorts of things can cause unnecessary conflicts and hide the true intent of the patch. This may have been the result of a space -> tab -> space conversion gone wrong. 3. This is optional, but I think it's good style to include a short text description at the top of each patch file, which can act as a commit message, and helps people reading the patch later without having to hunt down the original email. (If you're using HG MQ it does this automatically; I think git format-patch will also do this for you if you have a branch structured with one commit per patch, though it also adds quite a bit of extra email-header junk.) 4. Regarding 13_domain_lock.patch, I believe the original rationale of the master is that locking between concurrent application tasks is the responsibility of the application, not the master -- that's why in kernelspace it has send/receive callbacks (formerly lock/unlock callbacks) so that eg. RTAI locks can be substituted, or locking can be avoided if the application has some other way to schedule things to avoid actual concurrency (or if it's only running a single task). See the "Concurrent Master Access" section in the docs. I don't have any personal objections to this patch though. 5. Regarding 14_fix_string_handling.patch, I don't think this is the "right" fix. I've attached Frank's 04 patch which fixes this a different way. 6. Regarding 16_improved_ethercat_rescan_performance.patch, it looks like a stray temporary file was included in the patch. Also, I'm not sure it's safe to retrieve the data only by serial number. Serial numbers are not guaranteed unique between vendors, or even between product lines -- I think at minimum you should include the vendor id and product code in the index. Also, possibly this should have a #define config guard to disable this functionality in case the master will be used at a site with pathological slaves (eg. multiple slaves with identical non-zero vendor/product/serial triplets, since *technically* they're not guaranteed unique at all -- although any slave vendor who does this deserves a kick). I haven't had a chance to test things locally yet, but at least everything is compiling ok with these patches. :)
frank_04-string-download.patch
Description: Binary data
_______________________________________________ etherlab-dev mailing list etherlab-dev@etherlab.org http://lists.etherlab.org/mailman/listinfo/etherlab-dev
