Getting invalid data into EPICS

NickeZ commented 8 years ago

When we run the scanner with 1khz (1ms) we sometimes get invalid data into the EPICS layer. This is largely solved by lowering the frequency to 667hz (1.5ms) but in theory the issue remains.

I think that invalid data never should be propagated to the EPICS layer.

What do you think? Do you need any more specific logs/data?

cc: @alex-soderqvist

ronaldomercado commented 7 years ago

Hi @NickeZ, I had not seen this message until today. Do you still have a problem with this? If so, could you expand on "invalid data"? I have seen cases of the version of the scanner not matching the version of the client and have recently added a version for scanner and client. The client receives the scanner version to compare and stop if there's a mismatch. I will push this to github today. Best wishes, Ronaldo

NickeZ commented 7 years ago

Hi, as far as I can recall (I don't have a working setup right now) whenever ethercat loses a frame you get invalid data in EPICS instead of simply skipping a value.

ronaldomercado commented 7 years ago

Hi, Thanks for the comment. I will attempt to reproduce and refresh my understanding of error propagation.

I believe the epics record’s severity is updated in case of an error flag as determined by the etherlab library using the call ecrt_domain_process in scanner.c

Regards, Ronaldo From: Niklas Claesson [mailto:notifications@github.com] Sent: 15 December 2016 14:32 To: dls-controls/ethercat Cc: Mercado, Ronaldo (DLSLtd,RAL,TEC); Comment Subject: Re: [dls-controls/ethercat] Getting invalid data into EPICS (#4)

Hi, as far as I can recall (I don't have a working setup right now) whenever ethercat loses a frame you get invalid data in EPICS instead of simply skipping a value.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/dls-controls/ethercat/issues/4#issuecomment-267341292, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACXQv9T11lk3uMv6t7o6v7Qgg3w6uchXks5rIU91gaJpZM4Hw_1m.

-- This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail. Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd. Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message. Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom

NickeZ commented 7 years ago

It might be, but when users fetch values that are archived they get nonsensical ones and I think that is worse than getting no values.

klauer commented 6 years ago

We see this once every minute or two with a 1KHz scanner frequency. It appears the source of the invalid data is actually from here: https://github.com/dls-controls/ethercat/blob/9360ef3e92a98138d74a94d4476863fd46f6925b/ethercatApp/src/ecAsyn.cpp#L457

Looking back in the repository history, it seems like asyn may have had some issues back in 2011 leading to the current implementation. I believe it would make sense to use asynPortDriver::setParamStatus instead of setting it to an arbitrary value.

Is anyone relying on this magic minimum value to indicate alarm status?

ronaldomercado commented 6 years ago

Hi @klauer, thanks for the comment and the analysis. I don't know of anyone relying on this and this indeed seems to be the wrong way to indicate an alarm. Like you say, this section of code pre-dates the introduction of "auxStatus" (release 4-19). The release notes I am reading say that from asyn release 4-30 there are new "alarmStatus" and "alarmSeverity" fields. I think that we should use those instead of setting the value to INT32_MIN

dls-controls / ethercat

Getting invalid data into EPICS #4