Re: NI PCI-6221 encoder (Was: JR3 force sensor driver)

Frank Mori Hess wrote:
> On Tuesday 26 June 2007 07:00, Anders Blomdell wrote:
>> Anders Blomdell wrote:
> 
>>> I have looked a little more on the comedi code. The functionality
>>> seems to be lacking (or I'm totally blind). Which of the following
>>> implementations is preferred:
>>>
>>> 1. Adding INSN_CONFIG_SET_ABZ_SRC, which accepts two arguments:
>>>      index [0=A, 1=B, 2=Z]
>>>      source [1-10=PFI0-PFI9, 21-26=PFI10-PFI15, 31=DISABLED, ...]
>>>
>>> 2. Adding INSN_CONFIG_SET_A_SRC, INSN_CONFIG_SET_B_SRC,
>>> INSN_CONFIG_SET_Z_SRC, which accepts 1 argument:
>>>     source [1-10=PFI0-PFI9, 21-26=PFI10-PFI15, 31=DISABLED, ...]
>>>
>>> 3. Modify INSN_CONFIG_SET_GATE_SRC to accept
>>>      index [2=A, 3=B, 4=Z]
>>>      source [1-10=PFI0-PFI9, 21-26=PFI10-PFI15, 31=DISABLED, ...]
>>>
>>> 4. None of the above, please fill in a rough sketch on dotted line
>>> below:
>>>
>>>       ...........................................
>> Selected #3 above, patches to comedi & comedilib are included. Have not
>> got Z signal to work, but at least A/B works (will get back if I find
>> what is missing)...
> 
> Nice work, thanks for getting the quadrature counting working and providing 
> a useful demo program.  I don't use quadrature counting, and it might have 
> been forever before I got around to testing it myself.
> 
> I'd prefer you add a different insn_config id, since the a/b/z inputs 
> aren't really gates.  
The reason I piggybacked it on INSN_CONFIG_SET_GATE_SRC, was because they are
defined in the same table in the users manual (page A-8, table A-2). That the
same encoding for the PFI pins was used as with the gate source was another
reason for this choice. To be honest the A/B/Z inputs are more like CLOCK_SRC
(except that the PFI coding is different).

> I'd go with a generic configure source, like 
> INSN_CONFIG_SET_SOURCE that is otherwise similar to 
> INSN_CONFIG_SET_GATE_SRC, that can be used as a catch-all for configuring 
> sources for things other than gates (or clocks). Also, you should define
> a new enum + inline functions for the possible a/b/z sources, even if they 
> are mostly the same as the gate sources, especially since other than the 
> PFI pins they are unknown (right?).
As far as I know, do you know where one can find register level docs for the
M-series (my information has been from the NI RTL DDK examples and various posts
on their forum).

> Some other suggestions: 
> 
> Your demo should use the same command line options as the other demos, as 
> far as possible, and not conflict with them (the -a option).  Probably the 
> easiest way to handle the generic options would be to split 
> a "parse_option" function you could use out of the inside of the while 
> loop in "parse_options" in demo/common.c.
Which suddenly starts to make the parse_options more complex to use than the
plain getopt function! I understand that it's a good idea that all demos share a
common set of basic parameters (f:s:c:), but I would much prefer if demos spit
on parameters not used in that demo. Perhaps long_options would be an alternative?
> It would be nice if your demo explicitly set the index phase bits to 
> something, just so it's clear to the reader that its something they might 
> need to think about.
Good point.

BTW: I'm still not sure if "set_gate_source(..., {0, 1},
NI_GPCT_DISABLED_GATE_SELECT)" is necessary when using encoders.

I'll will get back with a revised patch as soon as possible.

Regards

Anders Blomdell

-- 
Anders Blomdell                  Email: anders.blomdell_at_control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118                     Fax:      +46 46 138118
SE-221 00 Lund, Sweden

Received on 2007-06-27Z07:07:52