Re: COMEDI wrapper (was: Re: Data acquisition in Octave)

[John W. Eaton]

On 21-Nov-2008, Olaf Till wrote:

| On Thu, Nov 20, 2008 at 12:04:49PM -0500, John W. Eaton wrote:
| > I have the beginnings of a package here:
| > 
| >   ftp://velveeta.che.wisc.edu/pub/octave-comedi/comedi-0.1.tar.gz
| > 
| Some first remarks:
| 
| The crash:
| 
| octave-3.0.2:1> dev = comedi_open ("/dev/comedi0")
| dev = <comedi_t object>
| octave-3.0.2:2> comedi_close (dev)
| ans = 0
| octave-3.0.2:3> comedi_close (dev)
| panic: Segmentation fault -- stopping myself...
| attempting to save variables to `octave-core'...
| error: octave_base_value::save_binary(): wrong type argument `comedi_t'
| save to `octave-core' complete
| Segmentation fault
| address@hidden:~/devel/src/octave-comedi-0.1/src$ 
| 
| can probably be cured by something like this patch:
| 
| --- orig-comedi_open.cc       2008-11-20 17:16:50.000000000 +0100
| +++ comedi_open.cc    2008-11-21 10:45:39.000000000 +0100
| @@ -163,6 +163,8 @@
|  
|    bool print_as_scalar (void) const { return true; }
|  
| +  void comedi_t_set_null (void) {dev = NULL;}
| +
|  private:
|  
|    comedi_t *dev;
| @@ -222,6 +224,25 @@
|    return retval;
|  }
|  
| +static void
| +octave_comedi_t_set_null (octave_value& arg)
| +{
| +
| +  try
| +    {
| +      octave_base_value& rep = (octave_base_value&) arg.get_rep ();

You are casting away const with this C-style cast, so that may be
asking for trouble.

I think the real problem is that octave_value is not really the right
kind of object for wrapping a pointer like this.  One way around this
problem is to do something like what we currently use for files.
Instead of returning an object that wraps a C FILE pointer, we return
an index that can be used to look up the FILE pointer as needed.  So
maybe I should do that instead.  Properly implemnting an octave_value
type that can do the right thing here would probably be a lot more
work...

| comedilib allocates comedi_range's for fields within  comedi_t, frees
| the ranges in comedi_close and luckily sets the pointers to NULL after
| freeing, so it's probably not problematic to wrap comedi_range into an
| octave_value as you did.
| 
| However, one usually allocates some more with or for comedilib. I
| don't know if wrapping those into octave_value is feasible, since I do
| not know the Octave internals good enough:

OK, I think we may have the same kinds of problems with this as with
the comedi_t type.  The real trouble is that Octave just doesn't have
a pointer (or reference) type, so wrapping bare pointers the way I did
is probably not the right thing to do.  The reference counting in the
octave_value class is probably not going to do the right thing for us.

| - comedi_polynomial_t
| 
| for comedi_to/from_physical (comedi_to/from_phys is deprecated);
| allocated by user, filled in by comedilib, one potentially for each
| range in each channel in each device;

In the version of comedi I have, these are not marked as deprecated,
and the new functions that use the polynomial type are in the section
of the comedilib.h header file marked with

  #ifndef _COMEDILIB_STRICT_ABI
  /*
     The following prototypes are _NOT_ part of the Comedilib ABI, and
     may change in future versions without regard to source or binary
     compatibility.  In practice, this is a holding place for the next
     library ABI version change.
   */

I intentionally avoided functions declared in this section of the
header because they could change in future versions.  I'm not sure how
likely that is, but I wanted to try to get something working first.

jwe