https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=578

--- Comment #9 from S Roderick <kiwi [dot] net [..] ...> 2009-07-10 20:20:33 ---
What can be done to get this patch accepted? Would really like to see it on the
mainline. Were all the objections dealt with?

https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=578

--- Comment #8 from S Roderick <kiwi [dot] net [..] ...> 2009-05-22 14:12:41 ---
Did we ever come to any conclusion on this patch? Would love to see this
functionality included in OCL. Are any further mod's to the patch requested?
S

For more infomation about this bug, visit

--- Comment #7 from Klaas Gadeyne <klaas [dot] gadeyne [..] ...> 2008-08-14 13:54:27 ---
(In reply to comment #6)
[...]
> We had this discussion of wether 1. a constructor should throw when receiving
> bad arguments (for example a null pointer or wrong configuration data) or 2.
> just let an 'invalid' object exist that does not respond to the method calls.
>
> The latter (#2) is 'dangerous' because if you don't check isValid() you're
> assuming the device/object works and get wrong or unsafe behaviour. A throw
> from a constructor (#1) avoids this (and is OO-speaking the best thing to do)
> but will not allow this code to be compiled with --fno-exceptions. A flag that
> gains considerable code size reductions.
>
> A third option is to have rule #2 for the RTT, such that the RTT can be used in
> exception-free applications, while the OCL, consisting of all optional,
> cherry-picked by the user components, may choose freely which method to
> implement, and preferably uses #1.
>
> Does this sound acceptable ?

Yep. Moreover, 99.99% [*] of the people using comedi won't compile their code
using -fno-exceptions, and in this case 1. really makes sense.

This is also in sync with
, the numerical
recipes in C between the c++-faqs.

[*] Not to say 100%. If you do belong to the 0.01%, please speak up :-)

For more infomation about this bug, visit

--- Comment #6 from Peter Soetens
<peter [dot] soetens [..] ...> 2008-08-14 13:24:17 ---
(In reply to comment #5)
> (In reply to comment #4)
> > Created an attachment (id=338)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=338) [details] [details]
> > Full featured patch
>
> Your IsValid method doesn't check validity of the subdevice, only of the device
> itself, e.g.

As the code shows, when the subdevice is invalid, myCard == 0, hence isValid()
returns false.

We had this discussion of wether 1. a constructor should throw when receiving
bad arguments (for example a null pointer or wrong configuration data) or 2.
just let an 'invalid' object exist that does not respond to the method calls.

The latter (#2) is 'dangerous' because if you don't check isValid() you're
assuming the device/object works and get wrong or unsafe behaviour. A throw
from a constructor (#1) avoids this (and is OO-speaking the best thing to do)
but will not allow this code to be compiled with --fno-exceptions. A flag that
gains considerable code size reductions.

A third option is to have rule #2 for the RTT, such that the RTT can be used in
exception-free applications, while the OCL, consisting of all optional,
cherry-picked by the user components, may choose freely which method to
implement, and preferably uses #1.

Does this sound acceptable ?

Peter

For more infomation about this bug, visit

--- Comment #5 from Klaas Gadeyne <klaas [dot] gadeyne [..] ...> 2008-08-13 15:13:54 ---
(In reply to comment #4)
> Created an attachment (id=338)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=338) [details]
> Full featured patch

Your IsValid method doesn't check validity of the subdevice, only of the device
itself, e.g.

The current constructors all use this kind of code

void ComediSubDeviceAIn::init()
{
if ( !myCard) {
log(Error) << "Error creating ComediSubDeviceAIn: null ComediDevice
given." < return;
}
if ( myCard->getSubDeviceType( _subDevice ) != COMEDI_SUBD_AI )
{
log(Error) << "comedi_get_subdevice_type failed" << endlog();
myCard = 0;
return;
}

AFAIS
- either we should throw an exception when this fails (but I know Peter doesn't
like that idea :-) instead of just returning.
- either I would suggest to replace your IsValid() code with

bool ComediSubdeviceAIn::isValid()
{
if ( !myCard) {
return false;
}
else
if ( myCard->getSubDeviceType( _subDevice ) != COMEDI_SUBD_AI )
{
return false;
}
else return true;
}

And use the isValid method in the ::init() methods in order to avoid code
duplication.

thoughts?

Klaas

For more infomation about this bug, visit

S Roderick <kiwi [dot] net [..] ...> changed:

What |Removed |Added
--------------------------------------------------------------------------
Attachment #334 is|0 |1
obsolete| |

--- Comment #4 from S Roderick <kiwi [dot] net [..] ...> 2008-08-13 14:51:22 ---
Created an attachment (id=338)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=338)
Full featured patch

For more infomation about this bug, visit

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

What |Removed |Added
--------------------------------------------------------------------------
CC| |peter [dot] soetens [..] ...

--- Comment #3 from Peter Soetens
<peter [dot] soetens [..] ...> 2008-08-13 13:56:02 ---
(In reply to comment #2)
> NB, our code to use these checks is ...
> {{{
> // create comedi devices
> comediDev = new OCL::ComediDevice(
> comediDev_prop.rvalue() );
> rc = (NULL != comediDev->getDevice()) &&
> (NULL != comediDev->getDevice()->it);
> if (rc)
> {
> comediSubdev = new OCL::ComediSubDeviceAIn(
> comediDev, getName(), comediSubdev_prop.rvalue());
> rc = comediSubdev->isValid();
> if (rc)
> {
> }}}
>

The reason comedi_internal.h is called like that is because it really is
required to be internal, especially for targets like RTAI/LXRT. Why don't you
use isValid() on the comediDevice itself (which checks in turn 'it') ? That
offers a more uniform API.
The isValid() function is a good extension, but it should be applied on all
comedi (sub)device classes (matter of copy/paste).

Peter

For more infomation about this bug, visit

--- Comment #2 from S Roderick <kiwi [dot] net [..] ...> 2008-08-11 14:30:25 ---
NB, our code to use these checks is ...
{{{
// create comedi devices
comediDev = new OCL::ComediDevice(
comediDev_prop.rvalue() );
rc = (NULL != comediDev->getDevice()) &&
(NULL != comediDev->getDevice()->it);
if (rc)
{
comediSubdev = new OCL::ComediSubDeviceAIn(
comediDev, getName(), comediSubdev_prop.rvalue());
rc = comediSubdev->isValid();
if (rc)
{
}}}

For more infomation about this bug, visit

--- Comment #1 from S Roderick <kiwi [dot] net [..] ...> 2008-08-11 14:28:06 ---
Created an attachment (id=334)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=334)
Provide device access and add validity check