[etherlab-dev] Multiple mailbox protocols and other issues

[Gavin Lambert]

On 27 February 2015 21:44, quoth Knud Baastrup:
> 
> Added one additional patch
(0015-Internal-SDO-requests-now-synchronized-with-
> external.patch) that solves an issue with input/output errors when
executing
> the ethercat sdos command (that now fetch directory) while configured SDO
> requests are executed from user application. Can also be observed with
> ethercat upload/download from EtherCAT Tool together with the execution of
> configured SDO requests. See the documentation in the patch it selves for
> more information.

I just noticed that patch "17_remove_prints_to_avoid_syslog_spam.patch" from
the 02022015 patch series appears to have vanished from later series.  I
assume this was intentional, as I don't recall seeing the spam it referred
to, but I thought I'd mention it just in case.

Also, some compiler warnings are still present from patch 0013:

master/fsm_slave_scan.c: In function 'ec_fsm_slave_scan_enter_attach_sii':
master/fsm_slave_scan.c:494:17: warning: format '%zu' expects argument of
type 'size_t', but argument 5 has type 'int' [-Wformat=]
                 EC_SLAVE_DBG(slave, 1, "Slave can re-use SII image data
stored."
                 ^
master/fsm_slave_scan.c:502:17: warning: format '%zu' expects argument of
type 'size_t', but argument 5 has type 'uint32_t' [-Wformat=]
                 EC_SLAVE_DBG(slave, 1, "Slave can re-use SII image data
stored."
                 ^
master/fsm_slave_scan.c:502:17: warning: format '%zu' expects argument of
type 'size_t', but argument 6 has type 'uint32_t' [-Wformat=]
master/fsm_slave_scan.c:502:17: warning: format '%zu' expects argument of
type 'size_t', but argument 7 has type 'uint32_t' [-Wformat=]
master/fsm_slave_scan.c: In function 'ec_fsm_slave_scan_state_sii_alias':
master/fsm_slave_scan.c:721:5: warning: format '%zu' expects argument of
type 'size_t', but argument 5 has type 'int' [-Wformat=]
     EC_SLAVE_DBG(slave, 1, "Alias: %zu\n", slave->effective_alias);
     ^
master/fsm_slave_scan.c: In function 'ec_fsm_slave_scan_state_sii_serial':
master/fsm_slave_scan.c:759:5: warning: format '%zu' expects argument of
type 'size_t', but argument 5 has type 'uint32_t' [-Wformat=]
     EC_SLAVE_DBG(slave, 1, "Serial Number: %zu\n",
slave->effective_serial_number);
     ^
master/fsm_slave_scan.c: In function 'ec_fsm_slave_scan_state_sii_vendor':
master/fsm_slave_scan.c:792:5: warning: format '%zu' expects argument of
type 'size_t', but argument 5 has type 'uint32_t' [-Wformat=]
     EC_SLAVE_DBG(slave, 1, "Vendor ID: %zu\n", slave->effective_vendor_id);
     ^
master/fsm_slave_scan.c: In function 'ec_fsm_slave_scan_state_sii_product':
master/fsm_slave_scan.c:825:5: warning: format '%zu' expects argument of
type 'size_t', but argument 5 has type 'uint32_t' [-Wformat=]
     EC_SLAVE_DBG(slave, 1, "Product code: %zu\n",
slave->effective_product_code);
     ^

The ones complaining about "int" probably need casts to "unsigned" (or
uint32_t if you prefer) due to default parameter extension, and the %zu
should just be %u for all of them.

Also vendor ids and product codes are usually printed in hex.  Not sure
about serial numbers, but "ethercat slaves -v" displays those in hex too, so
that seems reasonable.

On a somewhat related note, I'm not sure "effective_serial" etc are good
variable names.  "Effective alias" is phrased that way because there are
several different kinds of alias, but this contains the one that's currently
in use (eg. see the EC_REGALIAS code, which allows the effective alias to
come from a register rather than the SII); but that isn't really true for
the vendor/product/serial values.  This is just a minor quibble of course.
:)

Although speaking of the EC_REGALIAS code, if that's enabled and if the
register 0x0012 alias is different from the SII alias, then this patch might
malfunction (it should probably skip reading the SII alias and go straight
for the register).  Having said that, normally the two should be the same,
unless someone is in the process of changing the alias (in which case
rebooting the slave afterwards should "fix" everything).  There might be
some odd slaves out there though, which could be why EC_REGALIAS was added
in the first place..?

Finally, this is one of those "probably not strictly necessary but it makes
things tidier just in case" changes, but I recommend adding the following
hunk to patch 0005:

--- a/master/datagram.c
+++ b/master/datagram.c
@@ -586,6 +586,9 @@
         case EC_DATAGRAM_ERROR:
             printk("error");
             break;
+        case EC_DATAGRAM_INVALID:
+            printk("invalid");
+            break;
         default:
             printk("???");
     }