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

Update grpc-device to have support for Add Offset Nulling Calibration #1032

Merged
merged 7 commits into from
Jan 20, 2024

Conversation

DeborahOoi96
Copy link
Contributor

@DeborahOoi96 DeborahOoi96 commented Jan 10, 2024

What does this Pull Request accomplish?

Shows the result of the changes in grpc-device-scrapigen pr
Codegen nidaqmx.proto file which will be used in nidaqmx-python PR
Expected error test coverage added for offset nulling calibration tests in grpc-device because the devices in grpc-device functional test do not support offset nulling calibration. Discussion thread here.

Why should this Pull Request be merged?

Adds support for Offset Nulling Calibration in grpc devices

What testing has been done?

Built successfully, files codegened are as expected.
All tests passed as seen in PR build.

@reckenro
Copy link
Collaborator

reckenro commented Jan 10, 2024

I merely copied the functions.py over from grpc-device-scrapigen export and pasted it into src/codegen/metadata in grpc-device. However, it seems like some functions such as Self-Cal also got updated in the functions.py file. It seems like it wasn't previously updated in grpc-device-scrapigen, may I know if that is expected? Or should I manually revert the changes? Many thanks.

That's fine. What happened was this scrapigen PR changed the Self-Cal functions.py values but it was tested then and since it didn't have any .proto changes in grpc-device it wasn't merged into this repo yet. It's fine that it's being brought over now.

@DeborahOoi96 DeborahOoi96 requested a review from bkeryan January 12, 2024 09:58
@DeborahOoi96 DeborahOoi96 requested a review from bkeryan January 16, 2024 06:44
source/tests/system/nidaqmx_driver_api_tests.cpp Outdated Show resolved Hide resolved
source/tests/system/nidaqmx_driver_api_tests.cpp Outdated Show resolved Hide resolved
source/tests/system/nidaqmx_driver_api_tests.cpp Outdated Show resolved Hide resolved
source/tests/system/nidaqmx_driver_api_tests.cpp Outdated Show resolved Hide resolved
@DeborahOoi96 DeborahOoi96 requested a review from bkeryan January 17, 2024 10:11
Copy link
Contributor

@bkeryan bkeryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with suggestions

source/tests/system/nidaqmx_driver_api_tests.cpp Outdated Show resolved Hide resolved
source/tests/system/nidaqmx_driver_api_tests.cpp Outdated Show resolved Hide resolved
@DeborahOoi96 DeborahOoi96 force-pushed the users/deooi/grpc-device branch from 365b29a to 54299cc Compare January 19, 2024 02:49
@DeborahOoi96 DeborahOoi96 merged commit cbf3f28 into ni:main Jan 20, 2024
10 of 11 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.

6 participants