[etherlab-dev] [PATCH] My patchset roundup July 2015

Tue Aug 11 11:52:23 CEST 2015

Hi Gavin,

I have read ETG1000.6 6.4.1.4 and agree with you. I have checked with wireshark and can see that the Master do requests the PREOP and slave response with INIT + ERROR.

Thanks, Knud

-----Original Message-----
From: Gavin Lambert [mailto:gavinl at compacsort.com] 
Sent: 10. august 2015 00:58
To: Knud Baastrup
Cc: etherlab-dev at etherlab.org
Subject: RE: [etherlab-dev] [PATCH] My patchset roundup July 2015

I'm not sure that's correct either.  Generally the error indication is to provide information about a refusal to enter a higher state, and can be acknowledged.  So the "correct" flow would be:

1. Slave is in BOOT and has received invalid firmware; if possible, signal an FoE error or log a Diagnostics message at this time.
2. Master requests INIT.
3. Slave acknowledges INIT with no error.
4. Master requests PREOP.
5. Slave refuses PREOP by replying with INIT + ERROR.
6. Master acknowledges ERROR.
7. Slave returns to INIT (clears ERROR).
8. Repeat 4-7 if master re-requests PREOP.

Again, have a look at ETG1000.6 6.4.1.4.

> -----Original Message-----
> From: Knud Baastrup [mailto:kba at deif.com]
> Sent: Thursday, 6 August 2015 20:26
> To: Gavin Lambert <gavin.lambert at compacsort.com>
> Cc: etherlab-dev at etherlab.org
> Subject: RE: [etherlab-dev] [PATCH] My patchset roundup July 2015
> 
> Hi Gavin,
> 
> We do enter state INIT, but with the Error indication flag set (due to
invalid
> firmware) and that is why I wrote "prevented to enter state INIT", 
> which I agree could be misunderstood.
> 
> BR, Knud
> 
> 
> -----Original Message-----
> From: Gavin Lambert [mailto:gavinl at compacsort.com]
> Sent: 6. august 2015 09:42
> To: Knud Baastrup
> Cc: etherlab-dev at etherlab.org
> Subject: RE: [etherlab-dev] [PATCH] My patchset roundup July 2015
> 
> Hi Knud,
> 
> Thanks for sharing the updates.
> 
> Regarding the description of the update for patch 0008, though, this
puzzles
> me a little.
> 
> According to ETG1000.5 (specifically 6.2.1.3.8 "Stop Bootstrap Mode"
> service, and also ETG1000.6 6.4.1.4 #51 and #57), a slave is not 
> permitted
to
> refuse the transition from BOOT to INIT (this generally applies to all 
> of
the
> other "upwards" transitions as well).
> 
> Of course the master still needs to be able to cope with it anyway (a
crashed
> slave may fail to respond to all transitions until power cycled, 
> although
this is
> not the same as a refusal), but it's not something that's supposed to
happen.
> 
> I assume this is one of your own slaves?  My reading of the standards 
> suggests that in the case of invalid firmware a more appropriate 
> response would be to allow the transition to INIT (and back to BOOT), 
> but refuse transition to PREOP with AL status 0x0014 ("no valid firmware").
> 
> Having said that, there's very little information in the standards (at
least that
> I've found) about how firmware updates are recommended to work 
> (presumably because this is very hardware-specific), so it wouldn't 
> hurt
to
> ask about it on the ETG forums, if you haven't already.
> 
> Regards,
> Gavin Lambert
> 
> > -----Original Message-----
> > From: Knud Baastrup [mailto:kba at deif.com]
> > Sent: Thursday, 6 August 2015 19:05
> > To: Gavin Lambert
> > Cc: etherlab-dev at etherlab.org
> > Subject: RE: [etherlab-dev] [PATCH] My patchset roundup July 2015
> >
> > Hi Gavin,
> >
> > I look forward to try out the complete access support that you have 
> > implemented. We have several cases where we believe this feature can 
> > be very useful.
> >
> >
> > I have done a few updates on some of the knud-xxxxxxx patches as 
> > well, which includes:
> >
> > Correction to 0003-Eoe-mac-address-now-derived-from-unique-
> mac.patch:
> > Added new line to EC_SLAVE_INFO
> >
> > Correction to 0008-Clear-slave-mailboxes-after-a-re-scan.patch:
> > Now assuming that the slaves mailbox data is valid even if the slave
> scanning
> > skipped the clear mailbox state, e.g. if the slave refused to enter 
> > state
> INIT.
> > This correct an introduced bug where invalid application firmware 
> > (that prevented the slave module to enter state INIT) could prevent 
> > mailbox communication (FoE) in state BOOT.
> >
> > New 0017-EoE-processing-is-now-only-allowed-in-state-PREOP-SA.patch
> > The patch ensure that EoE fragments only is forwarded in state 
> > PREOP, SAFEOP and OP and not in state BOOT that is mainly (only?) 
> > intended for bootstrapping using FoE. The patch corrects an issue 
> > where EoE fragments can interrupt a successful FoE firmware upgrade 
> > in state BOOT despite that the slave code is restricted to only use 
> > FoE in state BOOT. In our case we
> use
> > a common bootloader for all I/O modules that only supports FoE. The 
> > bootloader will however still use mailbox buffer to receive EoE 
> > fragments (and additional to inform the Master that EoE is not 
> > supported), which in some cases can prevent the execution of a FoE
> request.
> >
> > Mvh. Knud
> >
> >
> > -----Original Message-----
> > From: etherlab-dev [mailto:etherlab-dev-bounces at etherlab.org] On 
> > Behalf Of Gavin Lambert
> > Sent: 10. juli 2015 09:14
> > To: etherlab-dev at etherlab.org; Florian Pose
> > Subject: [etherlab-dev] [PATCH] My patchset roundup July 2015
> >
> > Hi all,
> >
> > It's been a while since I last posted my current patchset for 
> > Etherlab, so
> it
> > seemed like a good time to share the newest ones.
> >
> > As with the last set, this is based on stable-1.5 (specifically
> > 4b0b906df1b40a1b5610282117b2c22581890575) and contains both my own 
> > patches and patches from others in the community, and I'm sharing 
> > them in case people find them useful and that hopefully they'll make 
> > it into
> mainline.
> >
> > Having said that, it appears that some of the patches in here have 
> > made it into the default branch already; I haven't had time yet to 
> > inspect them
> and
> > rebase my patchset onto that branch, but it's on my TODO list. :)
> >
> > The series file included in the archive shows the intended 
> > application
> order
> > (though you'll probably want to skip some of them if you're on 
> > default already).
> >
> > Short descriptions (commit notes) are at the top of each patch; 
> > detailed descriptions for the older patches can be found in previous 
> > emails, but
> I'll
> > give a bit more background on the newer patches below.  The older 
> > patches are unchanged except for possible defuzzing.
> >
> > Before that though, just a reminder that patch 0006
> "dc_sync_vs_sys_time"
> > is only a partial fix; it works for slaves that have AssignActivate 
> > bits
> 0x3000
> > set, and that don't mind the offset between SM events and SYNC 
> > events changing, but won't help other slaves -- they'll need a more 
> > comprehensive fix (fully recalculating the Sync Start Time).  It's 
> > probably not
> something I will
> > do because my slaves do work with this, and I'm not sufficiently 
> > familiar
> with
> > the way DC sync is calculated in Etherlab.
> >
> > The new patches are the gavinl-2000 series, as follows:
> >
> > gavinl-2001-reg_readwrite:
> > 	This adds an API "ecrt_reg_request_readwrite" and tool command 
> > "reg_readwrite" which will write register data and read back the 
> > values before the write in a single FPRW datagram.  This can be 
> > useful to ensure that the data is coherent and you don't miss an 
> > update.  It assumes Knud's
> rt-
> > mutex patch has been applied; if not you'll have to switch the 
> > locking
> calls
> > back.
> >
> > gavinl-2002-reg_req_info:
> > 	This enhances some of the debug-level logging for register requests.
> >
> > gavinl-2003-sdo_complete_upload:
> > 	This adds an API "ecrt_master_sdo_upload_complete" and extends
> tool
> > command "upload" to support reading entire objects from a slave via 
> > SDO Complete Access (where supported by the slave).  As with the 
> > existing download functionality only Complete Access from subindex 0 
> > is supported; future extension might be to support access from index 
> > 1 as well.  Note
> that
> > when using the tool "upload" it will default to the "octet_string"
> > data
> type, so
> > you can use something like "ethercat upload -p0 INDEX | hd" to see 
> > the
> data
> > formatted nicely.  It also assumes use of Knud's rt-mutex patch.
> >
> > gavinl-2004-sdo_write_size:
> > 	This might be a little more controversial. :)  This modifies the 
> > "ecrt_sdo_request_write" API (so it's a breaking change) to also 
> > include
> the
> > size of data to be written (this must be less than or equal to the 
> > size
> specified
> > at the time the request was created).  Combined with the existing 
> > "ecrt_sdo_request_index" API, this allows you to re-use one request 
> > object for any SDO on the same slave, provided the initial size is 
> > large
> enough.
> > Previously you had to use at least one object per different object 
> > size,
> as
> > there was no way to set the write size other than at creation time, 
> > and
> the
> > write size was also altered whenever you did a read.  (Possible
> non-breaking
> > alternative would be to make "ecrt_sdo_request_set_data_size" or 
> > something, but that seems messier.)
> >
> > gavinl-2005-sdo_async_complete:
> > 	This is another breaking API change, which adds a boolean
> "complete"
> > parameter to "ecrt_slave_config_create_sdo_request" and 
> > "ecrt_sdo_request_index".  This allows realtime requests to use 
> > Complete Access; prior to this patch this was only available to the 
> > non-realtime
> API.
> > (If this is judged to be *too* breaking, one possible breakage 
> > reduction
> is to
> > omit the change to the create function since that is more common 
> > than use of the index function.  However this will require anyone 
> > that wants to
> create
> > a Complete Access request to make two API calls specifying the index 
> > and subindex twice, which seems uglier.  Similar for adding a 
> > separate "ecrt_sdo_request_complete" API, which I also considered.)
> >
> > As before, note that these patches do not include a change to 
> > EC_IOCTL_VERSION_MAGIC, but you should change this if you apply any 
> > of them to reduce the risk of incompatibility (assuming that your 
> > app checks
> the
> > versions).
> >
> > Regards,
> > Gavin Lambert
> 