Skip to content
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

Add Unit test for cfg_time_start_trig #488

Merged
merged 21 commits into from
Feb 20, 2024

Conversation

DeborahOoi96
Copy link
Collaborator

@DeborahOoi96 DeborahOoi96 commented Feb 5, 2024

What does this Pull Request accomplish?

Add unit test coverage for cfg_time_start_trig
Add unit test coverage for new property introduced in #481
Created a new file called test_triggers.py because the existing test_triggers.py seems to use legacy testing environment and I foresee we will be adding a lot of new test coverage for time trigger APIs.
Made fixes to get_trig_attribute_timestamp as we found some missing conversion there.
Updated conftest.py to actually pull the simulated device.

Why should this Pull Request be merged?

Adds unit test coverage

What testing has been done?

All tests passed including the new tests
image
image
image

tests/conftest.py Outdated Show resolved Hide resolved
tests/component/test_triggers.py Outdated Show resolved Hide resolved
tests/component/_task_modules/test_triggers_properties.py Outdated Show resolved Hide resolved
tests/component/_task_modules/test_triggers_properties.py Outdated Show resolved Hide resolved
tests/component/_task_modules/test_triggers_properties.py Outdated Show resolved Hide resolved
tests/component/test_triggers.py Outdated Show resolved Hide resolved
tests/component/test_triggers.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
@DeborahOoi96
Copy link
Collaborator Author

test___arguments_provided___cfg_time_start_trig___no_errors is failing because timestamp stays at HOST instead of I/O TIME. I suspect this is a simulated device issue, since I get the same behaviour in LabView. I don't have a physical device currently that supports time triggering that can prove my theory, so I will update this test case once I know if it's an expected behavior on simulated device or not.

@zhindes
Copy link
Collaborator

zhindes commented Feb 7, 2024

test___arguments_provided___cfg_time_start_trig___no_errors is failing because timestamp stays at HOST instead of I/O TIME. I suspect this is a simulated device issue, since I get the same behaviour in LabView. I don't have a physical device currently that supports time triggering that can prove my theory, so I will update this test case once I know if it's an expected behavior on simulated device or not.

That doesn't surprise me, but I'm having trouble finding specific driver code that would do this. If you validate in LV, then that's good. Another thing to validate is use NI I/O Trace to ensure the settings going to the driver from Python are what you expect.

@DeborahOoi96
Copy link
Collaborator Author

DeborahOoi96 commented Feb 8, 2024

@zhindes Got my hands on a physical cdaq-9189, yes it is able to set timescale to I/O time.
image

This is the result of IO Trace on a simulated FIeldDAQ device in Labview
image

This is the result of IO Trace on simulated FieldDAQ in python
image

From both it seems that the value being passed into C API is correct (IODevice). Just that simulated devices seem to return HOST instead of IO Device.

@DeborahOoi96 DeborahOoi96 requested a review from bkeryan February 8, 2024 08:39
generated/nidaqmx/_grpc_interpreter.py Outdated Show resolved Hide resolved
@DeborahOoi96 DeborahOoi96 force-pushed the users/deooi/AddCfgTimeStartFT branch from 938e8cc to 468dae5 Compare February 15, 2024 09:32
@DeborahOoi96 DeborahOoi96 requested a review from bkeryan February 15, 2024 09:41
generated/nidaqmx/_grpc_time.py Outdated Show resolved Hide resolved
generated/nidaqmx/_lib_time.py Outdated Show resolved Hide resolved
@DeborahOoi96 DeborahOoi96 force-pushed the users/deooi/AddCfgTimeStartFT branch from 75d8693 to 6ba9796 Compare February 19, 2024 09:54
@DeborahOoi96 DeborahOoi96 requested a review from bkeryan February 19, 2024 17:53
generated/nidaqmx/_grpc_time.py Show resolved Hide resolved
generated/nidaqmx/_grpc_time.py Outdated Show resolved Hide resolved
generated/nidaqmx/_lib_time.py Outdated Show resolved Hide resolved
@DeborahOoi96 DeborahOoi96 merged commit ba42d15 into ni:master Feb 20, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants