-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cjg/usb 6002 #8
Cjg/usb 6002 #8
Conversation
test/runtests.jl
Outdated
@@ -90,12 +90,16 @@ else | |||
@test start(t) == nothing | |||
@test length(read(t, UInt8, 6)) == 12 | |||
@test stop(t) == nothing | |||
@test NIDAQ.CfgSampClkTiming(t.th, convert(Ref{UInt8},b""), 100.0, NIDAQ.Val_Rising, | |||
rslt = Ref(UInt32(0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ref{UIn32}(0)
is the preferred syntax i believe. see https://docs.julialang.org/en/latest/manual/calling-c-and-fortran-code/#Passing-Pointers-for-Modifying-Inputs-1
test/runtests.jl
Outdated
@test NIDAQ.CfgSampClkTiming(t.th, convert(Ref{UInt8},b""), 100.0, NIDAQ.Val_Rising, | ||
rslt = Ref(UInt32(0)) | ||
NIDAQ.DAQmxGetBufInputOnbrdBufSize(t.th, rslt) | ||
if rslt.x != 0 #If the device supports buffered digital input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similarly, rslt[]
is preferred.
test/runtests.jl
Outdated
@test length(read(t, UInt32)) == 20 | ||
@test stop(t) == nothing | ||
@test clear(t) == nothing | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be nice to add an elseif
here stating that device does not support clocked digital input.
looks great. thanks for this. just a couple of small suggestions. also, could you please shorten up the commit messages so they aren't wrapped/truncated? |
Thanks for the review! I think I've addressed everything in the amended commits, but I don't have access to a device today to test. I'll test asap and let you know. |
test/runtests.jl
Outdated
@@ -112,18 +118,24 @@ else | |||
@test start(t) == nothing | |||
@test write(t, round.(UInt8, [1 0; 0 0; 1 0; 0 1; 1 1; 0 1])) == 6 | |||
@test stop(t) == nothing | |||
@test NIDAQ.CfgSampClkTiming(t.th, convert(Ref{UInt8},b""), 100.0, NIDAQ.Val_Rising, | |||
rslt = Ref{UInt32}(0) | |||
NIDAQ.DAQmxGetBufInputOnbrdBufSize(t.th, rslt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be DAQmxGetBufOutputOnbrdBufSize
, not ...Input...
. otherwise, this works on my box. did you get a chance to test on yours?
Sorry, I've been traveling and hardware access has been difficult. I'll
change according to your suggestion and test on my box tomorrow.
…On Wed, Jul 19, 2017 at 7:46 AM, Ben Arthur ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In test/runtests.jl
<#8 (comment)>:
> @@ -112,18 +118,24 @@ else
@test start(t) == nothing
@test write(t, round.(UInt8, [1 0; 0 0; 1 0; 0 1; 1 1; 0 1])) == 6
@test stop(t) == nothing
- @test NIDAQ.CfgSampClkTiming(t.th, convert(Ref{UInt8},b""), 100.0, NIDAQ.Val_Rising,
+ rslt = Ref{UInt32}(0)
+ NIDAQ.DAQmxGetBufInputOnbrdBufSize(t.th, rslt)
should be DAQmxGetBufOutputOnbrdBufSize, not ...Input.... otherwise, this
works on my box. did you get a chance to test on yours?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#8 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AE78hD1bdZxOjOJtviiQA1RrQtF3eTMgks5sPfqsgaJpZM4OELO0>
.
|
would be great if you could also test #9 with your apps before i merge it. thanks. |
Okay this should be good to go! I also rebased your work in #9 on this (only locally), installed NIDAQmx v17.1, and ran the tests with my device. Tests passed! |
Modifies the test script so that devices without buffered digital input / output and counter output can pass.