Skip to content

nirfsg: Enable complex number support for gRPC and update system tests#2172

Open
rahur-NI wants to merge 23 commits intoni:masterfrom
rahur-NI:nirfsgGrpcComplexNumberSupport
Open

nirfsg: Enable complex number support for gRPC and update system tests#2172
rahur-NI wants to merge 23 commits intoni:masterfrom
rahur-NI:nirfsgGrpcComplexNumberSupport

Conversation

@rahur-NI
Copy link
Copy Markdown
Contributor

@rahur-NI rahur-NI commented Mar 23, 2026

  • This contribution adheres to CONTRIBUTING.md.
  • I've updated CHANGELOG.md if applicable.
  • I've added tests applicable for this pull request

What does this Pull Request accomplish?

  • Updated numpy_read_method.py.mako template with snippets copied over from default_method.py.mako and also included gRPC generation support for numpy complex methods.
  • Updated _grpc_stub_interpreter.py.mako template to import nidevice_pb2 as grpc_complex_types only if complex numbers are used in the module.
  • Updated 'included_in_proto' in functions.py of niFake to generate the gRPC changes in functions which has complex numpy values. Also updated nifake.proto file to generate the necessary changes to include in the nifake unit tests.
  • Added grpc_server_config.json file for nirfsg to include the address and port for server listening.

List issues fixed by this Pull Request below, if any.

NA

What testing has been done?

  • Ensured the codegen modifications are as expected.
  • Added a new TestGrpc class in test_system_nirfsg.py to include gRPC-based system test session setup.
  • As the fix for a defect in nirfsg grpc-device metadata was merged recently, test_get_all_named_waveform_names test will be only passing when the next release version of grpc-device server is used for nimi-python system tests. So this test case was moved from common path to TestLibrary class in test_system_nirfsg.py file to skip being tested against grpc. This movement will be reverted once we update the grpc-device version being used later.
  • Existing system tests are passing when tested against TestLibrary and TestGrpc class included in simulated session.
  • Existing system tests are tested against hardware by keeping use_simulated_session option as False, all the nirfsg test cases are passing as expected.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 92.30769% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.76%. Comparing base (448825e) to head (feb959a).

Files with missing lines Patch % Lines
generated/nifake/nifake/_grpc_stub_interpreter.py 92.30% 1 Missing ⚠️
generated/nirfsg/nirfsg/_grpc_stub_interpreter.py 92.30% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2172      +/-   ##
==========================================
+ Coverage   88.76%   89.76%   +0.99%     
==========================================
  Files          73       73              
  Lines       18986    19004      +18     
==========================================
+ Hits        16853    17059     +206     
+ Misses       2133     1945     -188     
Flag Coverage Δ
codegenunittests 84.90% <ø> (ø)
nidcpowersystemtests 94.65% <ø> (+0.04%) ⬆️
nidcpowerunittests 89.53% <ø> (ø)
nidigitalsystemtests 92.26% <ø> (ø)
nidigitalunittests 68.44% <ø> (ø)
nidmmsystemtests 92.72% <ø> (ø)
nifakeunittests 86.01% <92.30%> (+0.33%) ⬆️
nifgensystemtests 94.61% <ø> (ø)
nimodinstsystemtests 73.85% <ø> (ø)
nimodinstunittests 94.20% <ø> (ø)
nirfsgsystemtests 80.43% <92.30%> (+7.41%) ⬆️
niscopesystemtests 92.94% <ø> (ø)
niscopeunittests 43.20% <ø> (ø)
nisesystemtests 91.50% <ø> (ø)
niswitchsystemtests 82.03% <ø> (ø)
nitclksystemtests 94.87% <ø> (ø)
nitclkunittests 98.26% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
generated/nifake/nifake/_grpc_stub_interpreter.py 85.11% <92.30%> (+1.71%) ⬆️
generated/nirfsg/nirfsg/_grpc_stub_interpreter.py 78.62% <92.30%> (+78.62%) ⬆️

... and 5 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 448825e...feb959a. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ni-jfitzger
Copy link
Copy Markdown
Collaborator

You should add new tests to src\nifake\unit_tests\test_grpc.py

@ni-jfitzger
Copy link
Copy Markdown
Collaborator

I'm worried about the system test results. We seem to have hung as soon as we reached the first nirfsg gRPC test for 32-bit. You may want to investigate on a local system.

def session_creation_kwargs(self):
return {}

def test_get_all_named_waveform_names(self, rfsg_device_session):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you add a comment about why this test was moved to TestLibrary and when it can be moved back?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tried to move this back within the common path to check whether the failure in system test is anyway related to this. But it doesnt seems to be. Will investigate further regarding the nirfsg system test failure and update again.

Comment on lines +626 to +636
# Re-enabled for 32-bit testing after gRPC device update
@pytest.mark.skipif(use_simulated_session is True, reason="Needs Updated gRPC device that supports get_all_named_waveform_names bug fix")
def test_get_all_named_waveform_names(self, rfsg_device_session):
rfsg_device_session.generation_mode = nirfsg.GenerationMode.ARB_WAVEFORM
waveform_data1 = np.full(1000, 1 + 0j, dtype=np.complex128)
waveform_data2 = np.full(800, 1 + 0j, dtype=np.complex128)
rfsg_device_session.write_arb_waveform('waveform1', waveform_data1, False)
rfsg_device_session.write_arb_waveform('waveform2', waveform_data2, False)
names = rfsg_device_session.get_all_named_waveform_names()
assert 'waveform1' in names
assert 'waveform2' in names
Copy link
Copy Markdown
Collaborator

@ni-jfitzger ni-jfitzger Mar 29, 2026

Choose a reason for hiding this comment

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

Unless I'm misinterpreting this, now you're skipping this for non-gRPC cases, too?

@ni-jfitzger
Copy link
Copy Markdown
Collaborator

travis says you need to run codegen again.

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.

4 participants