| 1994 1995 1996 1997 1998 1999 2000 2001 2002 2003 2004 2005 2006 2007 2008 2009 2010 2011 2012 2013 2014 2015 2016 2017 2018 <2019> 2020 2021 2022 2023 2024 | Index | 1994 1995 1996 1997 1998 1999 2000 2001 2002 2003 2004 2005 2006 2007 2008 2009 2010 2011 2012 2013 2014 2015 2016 2017 2018 <2019> 2020 2021 2022 2023 2024 |
| <== Date ==> | <== Thread ==> |
|---|
| Subject: | RE: Unexpected change in behaviour of ai/ao device support |
| From: | "michael.abbott--- via Tech-talk" <[email protected]> |
| To: | "'Johnson, Andrew N.'" <[email protected]>, "tech-talk ([email protected])" <[email protected]> |
| Date: | Wed, 12 Jun 2019 06:32:55 +0000 |
|
(Two meta comments: firstly, apologies for lack of proper e-mail quoting, my e-mailer makes it next to impossible, and the quoting in your e-mail is almost
unreadable (nearly impossible to tell which text is quoted), possibly for the same reason. Secondly, why on earth doesn’t “Reply All” work on this mailing list? Is this again my crappy e-mailer, or is this some strage tech-talk configuration?) In you anxiety to avoid the “undefined” wrapping that occurs with integer arithmetic (technically, undefined for signed, defined as modulo word size for unsigned),
you changed the semantics drastically. This doesn’t feel like a good trade off to me. Ignoring your boundary checking and the rounding, and with extra casts inserted to make the types involved clearer, the difference between the two versions
of the conversion code boils down to this: old: rval = (int32_t) (double) value - (int32_t) roff; new: rval = (int32_t) ((double) value - (double) (uint32_t) roff); Now let’s look at a value of 0 being adjusted with an roff of -512 (for whatever reason, this is what the Hy8402 driver does): old: rval = 512; new: rval = - (uint32_t) -512 = -4294966784; Oops. I do wonder if it is time to act on the proposal of 1998 (a trifle before my time in this field) and remove these conversions altogether. For my own driver
support I gave up on letting EPICS do the ai/ao conversion long ago and use the unconverted val field instead. Much simpler and more predictable! From: [email protected]
[mailto:[email protected]] On Behalf Of Johnson, Andrew N. via Tech-talk On 6/11/19 3:28 AM, Ralph Lange via Tech-talk wrote: Use of the ROFF field has been effectively deprecated since 1998, when its functionality was taken over by the cleaner ESLO/EOFF mechanism. ROFF was mainly kept for compatibility reasons.
See https://epics.anl.gov/tech-talk/1998/msg00075.php for the announcement on Tech-Talk. It got un-deprecated again later because there are things ROFF can do that EOFF can't (see below). The change that Michael A is objecting to happened as a result of the discussion thread starting at
https://epics.anl.gov/tech-talk/2013/msg00684.php where I pointed out that we were relying on Undefined Behaviour (UB) in some cases, so that code needed to be fixed. The underlying problem is that the driver support relies on being able to set the ROFF field to a negative value. Unfortunately this is no longer possible, and the result is that ao outputs drive into maximum negative output voltage (potentially very damaging), and the ai inputs can also produce nonsense values (haven't actually tried yet, but the driver code is also now clearly broken). In the ao record's convert() routine ROFF gets subtracted from the scaled (ESLO+EOFF) and adjusted (ASLO+AOFF) value before it gets rounded and cast to the epicsInt32 RVAL field. Subtraction
of a positive number is necessary here to avoid the UB that would occur if the code relied on value wrap-around to map part of the EGUL-EGUF value range into the signed int32 range of RVAL, which I suspect the Hytec driver may be doing. However, I think that not having many users being affected might be related to the ROFF deprecation 17 years before. ROFF is still used by many device support that talk directly to ADC/DAC registers, but all the device support I have seen that use ROFF set it to a positive number when the hardware has an offset zero — that's why I didn't see much risk
in making this code change back in 2015. -- Complexity comes for free, Simplicity you have to work for.
-- 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. |