[Bug 488] New: AnalogIn/OutInterface is an atrocity.

For more infomation about this bug, visit <https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=488>

           Summary: AnalogIn/OutInterface is an atrocity.
           Product: RTT
           Version: 1.4.0
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Operating System Abstraction - Portability
        AssignedTo: orocos-dev at lists [dot] mech [dot] kuleuven [dot] be
        ReportedBy: peter.soetens at fmtc [dot] be
                CC: orocos-dev at lists [dot] mech [dot] kuleuven [dot] be
   Estimated Hours: 0.0

In contrast to the DigitalIn/OutInterface classes, the AnalogIn/OutInterface
takes a template parameter. Furthermore, the AnalogOutput helper class takes
the same parameter. It was meant to allow the choice between unsigned/signed
int or even float or double as the interface to an IO card with analog in or
out channels.

In reality the drivers return an int, or an unsigned int and uses max 16 bits,
but 12 bits ADC/DAC are more common. The AnalogOutput object allows conversion
between doubles and this integer value. I believe RTT 1.0 should have used
'int' as the data type for communicating with the analog interfaces. This would
have allowed devices with 31 bit unsigned int ADC/DAC and 32bit signed ADC/DAC,
far more than necessary.

Usercode would have changed from:

 AnalogOutInterface<unsigned int>* maout = AnalogOutInterface<unsigned

int>::nameserver.getObject("Ana_Out");

 AnalogOutput<unsigned int> output(maout, ANA_CHANNEL);
 output.value( v ); // in volts.

to:

 AnalogOutInterface* maout =

AnalogOutInterface::nameserver.getObject("Ana_Out");

 AnalogOutput output(maout, ANA_CHANNEL);
 output.value( v ); // in volts.

Which is consistent with the DigitalIn/OutInterface.

I don't have any idea how to solve this with respect to backwards
compatibility, since these classes also appear in user code.


Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.

[Bug 488] AnalogIn/OutInterface is an atrocity.

For more infomation about this bug, visit <https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=488>

Peter Soetens <peter.soetens at fmtc [dot] be> changed:

           What    |Removed                     |Added
 --------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|orocos-                     |peter.soetens@fmtc.be
                   |dev@lists.mech.kuleuven.be  |
--- Comment #1 from Peter Soetens <peter.soetens at fmtc [dot] be>  2008-03-17 10:51:49 ---

Created an attachment (id=252)

 --> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=252)

removes the template parameter from RTT classes.

This patch removes the template parameter and assumes that 'int' is used to
read/write to channels.

Only the resolution is an unsigned int, all the other functions use 'int'.

This patch does not fix the OCL Comedi classes.


[Bug 488] AnalogIn/OutInterface is an atrocity.

For more infomation about this bug, visit <https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=488>

--- Comment #2 from Peter Soetens <peter.soetens at fmtc [dot] be>  2008-04-20 00:00:42 ---

Created an attachment (id=263)

 --> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=263)

Adapts OCL's AnalogIn/Out use to template-less RTT implementation

This patch fixes all ocl/hardware classes for using the new RTT API.
The template parameter has been dropped. The comedi implementations are
now far more efficient (caching range settings etc.).


[Bug 488] AnalogIn/OutInterface is an atrocity.

For more infomation about this bug, visit <https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=488>

Peter Soetens <peter.soetens at fmtc [dot] be> changed:

           What    |Removed                     |Added
 --------------------------------------------------------------------------
 Attachment #252 is|0                           |1
           obsolete|                            |
--- Comment #3 from Peter Soetens <peter.soetens at fmtc [dot] be>  2008-04-20 00:02:14 ---

Created an attachment (id=264)

 --> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=264)

Improved the RTT API as well.

Same as previous RTT patch but the API is overall better. Some methods
(read,write,...) have been deprecated and have been
given a new name (value, rawValue,...). The OCL patch works with this one.

Any objections ?


[Bug 488] AnalogIn/OutInterface is an atrocity.

For more infomation about this bug, visit <https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=488>

--- Comment #4 from Klaas Gadeyne <klaas.gadeyne at fmtc [dot] be>  2008-04-21 11:59:04 ---

(In reply to comment #3)
> Created an attachment (id=264)

 --> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=264) [details]

> Improved the RTT API as well.
>
> Same as previous RTT patch but the API is overall better. Some methods
> (read,write,...) have been deprecated and have been
> given a new name (value, rawValue,...). The OCL patch works with this one.
>
> Any objections ?

None. The new names are an improvement (although I would maybe prefer

  • valueGet() for the input channels, and valueSet() for the output channels to

avoid confusion between read/write inputs, but that's probably a matter of
taste), and backwards compatibility seems guaranteed with this patch.

Some small remarks (the whitespace fixes make it hard to read your patch though
:-(

Index: src/dev/AnalogInput.hpp

         /**
          * Read the raw value of this channel.
          */
-        InputType rawValue() const
+        int rawValue() const
         {
-            InputType r;
-            board->read(channel, r);
+            unsigned int r;
+            board->rawValue(channel, r);
             return r;
         }

Shouldn't the return type of this function be "unsigned int" too?

Similar remark for the rawValue of AnalogOutput.hpp

Furthermore, some methods now have a void return type, while a "real" return
value would make sense, e.g.

         /**
          * Write the raw value of this channel.
          */
-        void rawValue(OutputType i)
+        void rawValue(int i)
         {
-            if ( i < board->binaryLowest(channel ) )
-                i_cache = board->binaryLowest( channel );
-            else if ( i > board->binaryHighest( channel ) )
-                i_cache = board->binaryHighest( channel ) ;
-            else
-                i_cache = i;
-            board->write(channel, i);
+            board->rawValue(channel, i);
         }

where the last line does permit return values. From a first (and probably too
fast) test, it seems such void2type return modifications are even ABI
compatible?


[Bug 488] AnalogIn/OutInterface is an atrocity.

For more infomation about this bug, visit <https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=488>

Peter Soetens <peter.soetens at fmtc [dot] be> changed:

           What    |Removed                     |Added
 --------------------------------------------------------------------------
 Attachment #264 is|0                           |1
           obsolete|                            |
--- Comment #5 from Peter Soetens <peter.soetens at fmtc [dot] be>  2008-04-29 23:03:08 ---

Created an attachment (id=270)

 --> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=270)

user read/write again at Analog*Interface level + fix return values as well

This patch solves the issues pointed at by Klaas.
The return values are now 'int' (to indicate success/failure) in case of the
Analog*Interface classes and the value() names have been replaced by the
read/write equivalents again to be more clear.

AnalogInInterface and AnalogOutInterface are compatible (again) when you wish
to inherit from both classes.

The patch is not ABI compatible.


[Bug 488] AnalogIn/OutInterface is an atrocity.

For more infomation about this bug, visit <https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=488>

Peter Soetens <peter.soetens at fmtc [dot] be> changed:

           What    |Removed                     |Added
 --------------------------------------------------------------------------
 Attachment #263 is|0                           |1
           obsolete|                            |
--- Comment #6 from Peter Soetens <peter.soetens at fmtc [dot] be>  2008-04-29 23:06:42 ---

Created an attachment (id=271)

 --> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=271)

Adapt to cleanup3 from RTT.

This patch adapts to the final new Analog* API and also places Comedi in the
OCL namespace.


[Bug 488] AnalogIn/OutInterface is an atrocity.

For more infomation about this bug, visit <https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=488>

Peter Soetens <peter.soetens at fmtc [dot] be> changed:

           What    |Removed                     |Added
 --------------------------------------------------------------------------
   Target Milestone|---                         |1.6.0
--- Comment #7 from Peter Soetens <peter.soetens at fmtc [dot] be>  2008-04-29 23:29:37 ---

Applied on trunk/rtt:
$ svn ci -m"Fix bug #488: AnalogIn/OutInterface is an atrocity.
> Applied analog-interface-cleanup3.patch
> "
Sending rtt/src/dev/AnalogInInterface.hpp
Sending rtt/src/dev/AnalogInput.hpp
Sending rtt/src/dev/AnalogOutInterface.hpp
Sending rtt/src/dev/AnalogOutput.hpp
Sending rtt/src/dev/io.cpp
Sending rtt/tests/FakeAnalogDevice.hpp
Sending rtt/tests/dev_test.cpp
Transmitting file data .......
Committed revision 29219.

And on trunk/ocl:
$ svn ci hardware/ -m"Fix bug #488: AnalogIn/OutInterface is an atrocity.
All OCL hardware components are adapted (and compile tested).
> "
Sending hardware/axes/dev/AnalogDrive.hpp
Sending hardware/axes/dev/AnalogSensor.hpp
Sending hardware/comedi/dev/ComediDevice.cpp
Sending hardware/comedi/dev/ComediDevice.hpp
Sending hardware/comedi/dev/ComediEncoder.cpp
Sending hardware/comedi/dev/ComediEncoder.hpp
Sending hardware/comedi/dev/ComediPulseTrainGenerator.cpp
Sending hardware/comedi/dev/ComediPulseTrainGenerator.hpp
Sending hardware/comedi/dev/ComediSubDeviceAIn.cpp
Sending hardware/comedi/dev/ComediSubDeviceAIn.hpp
Sending hardware/comedi/dev/ComediSubDeviceAOut.cpp
Sending hardware/comedi/dev/ComediSubDeviceAOut.hpp
Sending hardware/comedi/dev/ComediSubDeviceDIn.cpp
Sending hardware/comedi/dev/ComediSubDeviceDIn.hpp
Sending hardware/comedi/dev/ComediSubDeviceDOut.cpp
Sending hardware/comedi/dev/ComediSubDeviceDOut.hpp
Sending hardware/comedi/dev/ComediThreadScope.hpp
Sending hardware/comedi/dev/comedi_internal.h
Sending hardware/ethercat-demo/EthercatDemonAxesVelocityController.cpp
Sending hardware/ethercat-demo/EthercatDemonAxesVelocityController.hpp
Sending hardware/ethercat-demo/EthercatIO.hpp
Sending hardware/ethercat-demo/dev/AnalogEtherCATInputDevice.hpp
Sending hardware/ethercat-demo/dev/AnalogEtherCATOutputDevice.hpp
Sending hardware/io/IOComponent.hpp
Sending hardware/io/dev/FakeAnalogDevice.hpp
Sending hardware/kuka/Kuka160nAxesVelocityController.cpp
Sending hardware/kuka/Kuka160nAxesVelocityController.hpp
Sending hardware/kuka/Kuka361nAxesTorqueController.cpp
Sending hardware/kuka/Kuka361nAxesTorqueController.hpp
Sending hardware/kuka/Kuka361nAxesVelocityController.cpp
Sending hardware/kuka/Kuka361nAxesVelocityController.hpp
Sending hardware/laserdistance/LaserDistance.cpp
Sending hardware/laserdistance/LaserDistance.hpp
Sending hardware/lias/CRSnAxesVelocityController.cpp
Sending hardware/lias/CRSnAxesVelocityController.hpp
Sending hardware/lias/IP_FastDAC_AOutInterface.cpp
Sending hardware/lias/IP_FastDAC_AOutInterface.hpp
Sending hardware/performer_mk2/PerformernAxesVelocityController.cpp
Sending hardware/performer_mk2/PerformernAxesVelocityController.hpp
Sending hardware/staubli/StaubliRX130nAxesVelocityController.cpp
Sending hardware/staubli/StaubliRX130nAxesVelocityController.hpp
Sending hardware/xyPlatform/xyPlatformAxisHardware.cpp
Sending hardware/xyPlatform/xyPlatformAxisHardware.hpp
Transmitting file data ...........................................
Committed revision 29222.


[Bug 488] AnalogIn/OutInterface is an atrocity.

For more infomation about this bug, visit <https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=488>

--- Comment #8 from Peter Soetens <peter.soetens at fmtc [dot] be>  2008-05-04 22:18:00 ---

Created an attachment (id=272)

 --> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=272)

Make analog interface raw type signed

After much [some mental process], I've changed the 'raw' type of the analog
interfaces to 'int' instead of 'unsigned int'. This allows in practice 32bits
signed AD converters and 31bits unsigned AD converters (only returns positive
numbers). This change was done to support two's complement AD converters as
well. In case your 'unsigned' AD converter has 32bits (does that exist?) your
implementation needs to offset it to a signed raw representation. I'm sure I'm
not bullying anyone with this... but you're free to correct me if I'm wrong.

This patch is for RTT (in addition to patch 270)

Peter


[Bug 488] AnalogIn/OutInterface is an atrocity.

For more infomation about this bug, visit <https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=488>

--- Comment #9 from Peter Soetens <peter.soetens at fmtc [dot] be>  2008-05-04 22:18:44 ---

Created an attachment (id=273)

 --> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=273)

Updates OCL for signed raw types.

Patch for OCL in addition to patch 271.


[Bug 488] AnalogIn/OutInterface is an atrocity.

For more infomation about this bug, visit <https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=488>

--- Comment #10 from Peter Soetens <peter.soetens at fmtc [dot] be>  2008-05-04 22:20:15 ---

Applied on trunk.

$ svn ci rtt/src/ rtt/tests/ -m"Applying patch for bug #488:
AnalogIn/OutInterface is an atrocity.
> Make analog interface raw type signed
> "
Sending rtt/src/dev/AnalogInInterface.hpp
Sending rtt/src/dev/AnalogInput.hpp
Sending rtt/src/dev/AnalogOutInterface.hpp
Sending rtt/src/dev/AnalogOutput.hpp
Sending rtt/src/dev/AxisInterface.hpp
Sending rtt/tests/FakeAnalogDevice.hpp
Transmitting file data ......
Committed revision 29223.

+ sspr@lt00129:~/src/Orocos/trunk

$ svn ci ocl -m"Applying patch for bug #488: AnalogIn/OutInterface is an
atrocity.
> Updates OCL for signed raw types.
> "
Sending ocl/hardware/comedi/dev/ComediSubDeviceAIn.cpp
Sending ocl/hardware/comedi/dev/ComediSubDeviceAIn.hpp
Sending ocl/hardware/comedi/dev/ComediSubDeviceAOut.cpp
Sending ocl/hardware/comedi/dev/ComediSubDeviceAOut.hpp
Transmitting file data ....
Committed revision 29224.