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

Adding private APIs to configure attenuation tables in RFmx Instr #1052

Merged

Conversation

paulolamas18
Copy link
Contributor

What does this Pull Request accomplish?

Expose, in grpc-device, the existing private APIs in RFmxInstr to create and configure attenuation tables.

Why should this Pull Request be merged?

We have a use case for using these private APIs, but they are currently not exposed through NI's gRPC server.

What testing has been done?

Simple program that calls these APIs throws a NotImplemented exception with the currently release server. No errors thrown after these changes.

@reckenro
Copy link
Collaborator

reckenro commented Apr 5, 2024

@ThangamV-NI , I added you since you edited these restricted proto files most recently but feel free to ignore this or foreword to another reviewer, whichever makes sense.

Copy link
Collaborator

@reckenro reckenro left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I emailed you about joining the NI GitHub organization so you can kick off the PR build yourself and view private NI GitHub repos in the future if needed.

@paulolamas18
Copy link
Contributor Author

Hi @reckenro, "CI/run_win_system_tests/Run Tests (pull_request)" has been queued for a while, do you know what might be going on? This is the first time I try to pull a change to this repo.

@reckenro
Copy link
Collaborator

reckenro commented Apr 9, 2024

Hi @reckenro, "CI/run_win_system_tests/Run Tests (pull_request)" has been queued for a while, do you know what might be going on? This is the first time I try to pull a change to this repo.

Oh yes, @paulolamas18 the pipeline that makes the Windows VMs that those system tests run against sometimes has to be kicked off again because this step stalls out. I kicked that off and will re-queue that step once that completes. Should wrap up by the afternoon and then if things look good I'll merge your change in.

@reckenro reckenro merged commit 62d4480 into ni:main Apr 9, 2024
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.

None yet

4 participants