Skip to content

Commit

Permalink
pxi6733: refactor, allow multiple channels in one task, cancel any pr…
Browse files Browse the repository at this point in the history
…evious task
fallen committed Jun 5, 2015
1 parent 398940f commit c251601
Showing 2 changed files with 73 additions and 28 deletions.
88 changes: 68 additions & 20 deletions artiq/devices/pxi6733/driver.py
Original file line number Diff line number Diff line change
@@ -14,22 +14,38 @@ def close(self):
def ping(self):
return True

def string_to_bytes(string, name):

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

Again, just assume a single type. Since all the NI strings contain only ASCII characters, there is absolutely no use for bytes.

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

Additionally, if that function were justified it should go in artiq.tools.

if isinstance(string, str):
string = bytes(string, encoding="ascii")
elif not isinstance(string, bytes):
raise ValueError("{} must be of either str or bytes type".format(name))
return string

class DAQmx:
"""NI PXI6733 DAQ interface."""

def __init__(self, device, analog_output, clock):
def __init__(self, channels, clock):
"""
:param channels: List of channels as a string or bytes(), following
the physical channels lists and ranges NI-DAQmx syntax.
Example: Dev1/ao0, Dev1/ao1:ao3
:param clock: Clock source terminal as a string or bytes(), following
NI-DAQmx terminal names syntax.
Example: PFI5
"""

import PyDAQmx as daq

self.device = device
self.analog_output = analog_output
self.clock = clock
self.tasks = []
self.channels = string_to_bytes(channels, "channels")

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

self.channels = channels.encode()

This comment has been minimized.

Copy link
@fallen

fallen Jun 6, 2015

Author Contributor

So I assume this code is always called from the controller (i.e. with strings only)?

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

Why would anyone need to call it with anything else than strings?

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

And PyDAQmx is not an example of a good Python API. Their requirement of bytes is probably one of the many idiosyncrasies it has, similar to requiring user-supplied integer byrefs instead of returning the value...

This comment has been minimized.

Copy link
@fallen

fallen Jun 6, 2015

Author Contributor

all right then, let's do var.encode()

self.clock = string_to_bytes(clock, "clock")
self.task = None
self.daq = daq

def done_callback_py(self, taskhandle, status, callback_data):
self.daq.DAQmxClearTask(taskhandle)
self.tasks.remove(taskhandle)
if taskhandle == self.task:

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

Is there any situation where the callback is called for another task?

This comment has been minimized.

Copy link
@fallen

fallen Jun 6, 2015

Author Contributor

I was more thinking about a race condition where done_callback_py would be called after the previous task is canceled, and after line 118 is executed. I don't know if that can happen or not though.

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

So figure it out.

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

This kind of "locking" may break if you do not understand the concurrency situation.

This comment has been minimized.

Copy link
@fallen

fallen Jun 6, 2015

Author Contributor

I read in the API "doc": A Done event does not occur when a task is stopped explicitly, such as by calling DAQmxStopTask http://zone.ni.com/reference/en-XX/help/370471Y-01/daqmxcfunc/daqmxregisterdoneevent/
And the DAQmxClearTask does stop the Task (Clears the task. Before clearing, this function aborts the task, if necessary, and releases any resources reserved by the task. cf http://zone.ni.com/reference/en-XX/help/370471Y-01/daqmxcfunc/daqmxcleartask/)
So in theory, according to the doc, there should be no problem, but who knows what will happen in reality.
I will need to find a way to find out.

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

assert or logger.warning would be more appropriate than silently ignoring any problem.

This comment has been minimized.

Copy link
@fallen

fallen Jun 6, 2015

Author Contributor
def done_callback_py(self, taskhandle, status, callback_data):
    if taskhandle != self.task:
        logger.warning("Callback was called on an already canceled task")
    else:
        self.clear_pending_task()

?

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

Do not assume "already cancelled". logger.warning("done callback called with unexpected task") or something like that.

self.clear_pending_task()

def ping(self):
try:
@@ -42,43 +58,75 @@ def ping(self):
def load_sample_values(self, sampling_freq, values):
"""Load sample values into PXI 6733 device.
This loads sample values into the PXI 6733 device and then
configures a task to output those samples at each clock rising
edge.
This loads sample values into the PXI 6733 device.
The device will output samples at each clock rising edge.
The first sample is output at the first clock rising edge.

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

Unclear.

This comment has been minimized.

Copy link
@fallen

fallen Jun 6, 2015

Author Contributor

"The device waits for a clock rising edge to output the first sample"?

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

ok

When using several channels simultaneously, you must concatenate the
values for the different channels in the ``values`` array.
The sample values for the same channel must be grouped together.
Example:
A callback is registered to clear the task (deallocate resources)
when the task has completed.
>>> values = np.array([ch0_samp0, ch0_samp1, ch1_samp0, ch1_samp1],
dtype=float)
In this example the first two samples will be output via the first
channel and the two following samples will be output via the second
channel.
Any call to this method will cancel any previous task even if it has
not yet completed.
:param sampling_freq: The sampling frequency in samples per second.
:param values: A numpy array of sample values to load in the device.
"""

self.clear_pending_task()

t = self.daq.Task()
t.CreateAOVoltageChan(self.device+b"/"+self.analog_output, b"",
t.CreateAOVoltageChan(self.channels, b"",
min(values), max(values),
self.daq.DAQmx_Val_Volts, None)

channel_number = self.daq.int32()
t.GetTaskNumChans(byref(channel_number))
nb_values = len(values)
if nb_values % channel_number.value > 0:

This comment has been minimized.

Copy link
@sbourdeauducq

sbourdeauducq Jun 6, 2015

Member

> 0 is useless

This comment has been minimized.

Copy link
@fallen

fallen Jun 6, 2015

Author Contributor

ack

self.daq.DAQmxClearTask(t.taskHandle)
raise ValueError("The size of the values array must be a multiple "
"of the number of channels ({})"
.format(channel_number.value))
samps_per_channel = nb_values // channel_number

t.CfgSampClkTiming(self.clock, sampling_freq,
self.daq.DAQmx_Val_Rising,
self.daq.DAQmx_Val_FiniteSamps, len(values))
self.daq.DAQmx_Val_FiniteSamps, samps_per_channel)
num_samps_written = self.daq.int32()
values = np.require(values, dtype=float,
requirements=["C_CONTIGUOUS", "WRITEABLE"])
ret = t.WriteAnalogF64(len(values), False, 0,
ret = t.WriteAnalogF64(samps_per_channel, False, 0,
self.daq.DAQmx_Val_GroupByChannel, values,
byref(num_samps_written), None)
if num_samps_written.value != len(values):
if num_samps_written.value != nb_values:
raise IOError("Error: only {} sample values were written"
.format(num_samps_written.value))
if ret:
raise IOError("Error while writing samples to the channel buffer")

done_cb = self.daq.DAQmxDoneEventCallbackPtr(self.done_callback_py)
self.tasks.append(t.taskHandle)
self.task = t.taskHandle
self.daq.DAQmxRegisterDoneEvent(t.taskHandle, 0, done_cb, None)
t.StartTask()

def clear_pending_task(self):
"""Clear any pending task."""

if self.task is not None:
self.daq.DAQmxClearTask(self.task)
self.task = None

def close(self):
"""Clear all pending tasks."""
"""Free any allocated resources."""

for t in self.tasks:
self.daq.DAQmxClearTask(t)
self.clear_pending_task()
13 changes: 5 additions & 8 deletions artiq/frontend/pxi6733_controller.py
Original file line number Diff line number Diff line change
@@ -11,13 +11,11 @@
def get_argparser():
parser = argparse.ArgumentParser(description="NI PXI 6733 controller")
simple_network_args(parser, 3256)
parser.add_argument("-d", "--device", default=None,
help="Device name (e.g. Dev1)."
parser.add_argument("-C", "--channels", default=None,
help="List of channels (e.g. Dev1/ao0, Dev1/ao1:3)."
" Omit for simulation mode.")
parser.add_argument("-c", "--clock", default="PFI5",
help="Input clock pin name (default: PFI5)")
parser.add_argument("-a", "--analog-output", default="ao0",
help="Analog output pin name (default: ao0)")
verbosity_args(parser)
return parser

@@ -26,12 +24,11 @@ def main():
args = get_argparser().parse_args()
init_logger(args)

if args.device is None:
if args.channels is None:
daq = DAQmxSim()
else:
daq = DAQmx(bytes(args.device, "ascii"),
bytes(args.analog_output, "ascii"),
bytes(args.clock, "ascii"))
daq = DAQmx(args.channels,
args.clock)

try:
simple_server_loop({"pxi6733": daq},

0 comments on commit c251601

Please sign in to comment.