[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: COMEDI wrapper (was: Re: Data acquisition in Octave)
From: |
John W. Eaton |
Subject: |
Re: COMEDI wrapper (was: Re: Data acquisition in Octave) |
Date: |
Fri, 21 Nov 2008 13:03:50 -0500 |
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