-
Notifications
You must be signed in to change notification settings - Fork 66
Add ReadAnalogWaveforms to NiDAQmxService #1203
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
base: main
Are you sure you want to change the base?
Changes from 12 commits
5e53c36
fff7769
01ea5e9
1270fd7
836c408
b9cf67a
12ca5e5
8e5eb77
7b50a84
3d29cbc
9b26670
d117ec2
23a6e98
1577116
c03a50d
4c36ac8
e7055d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -289,6 +289,8 @@ get_filename_component(deviceid_restricted_proto "source/protobuf_restricted/dev | |
| get_filename_component(debugsessionproperties_restricted_proto "source/protobuf_restricted/debugsessionproperties_restricted.proto" ABSOLUTE) | ||
| get_filename_component(calibrationoperations_restricted_proto "source/protobuf_restricted/calibrationoperations_restricted.proto" ABSOLUTE) | ||
| get_filename_component(data_moniker_proto "imports/protobuf/data_moniker.proto" ABSOLUTE) | ||
| get_filename_component(precision_timestamp_proto "third_party/ni-apis/ni/protobuf/types/precision_timestamp.proto" ABSOLUTE) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to your last PR, I think you'll need to update stage_client_files.py to get these proto files included in GitHub releases and the AzDo consumed adhoc exports. |
||
| get_filename_component(waveform_proto "third_party/ni-apis/ni/protobuf/types/waveform.proto" ABSOLUTE) | ||
| get_filename_component(session_proto_path "${session_proto}" PATH) | ||
|
|
||
| #---------------------------------------------------------------------- | ||
|
|
@@ -303,8 +305,9 @@ function(GenerateGrpcSources) | |
| set(proto_file "${GENERATE_ARGS_PROTO}") | ||
| if(USE_SUBMODULE_LIBS) | ||
| set(protobuf_includes_arg | ||
| -I ${CMAKE_SOURCE_DIR}/third_party/grpc/third_party/protobuf/src/ | ||
| -I ${CMAKE_SOURCE_DIR}/third_party/ni-apis/ni/grpcdevice/v1/) # for session.proto | ||
| -I ${CMAKE_SOURCE_DIR}/third_party/grpc/third_party/protobuf/src/ | ||
| -I ${CMAKE_SOURCE_DIR}/third_party/ni-apis/ni/grpcdevice/v1/ # for session.proto | ||
| -I ${CMAKE_SOURCE_DIR}/third_party/ni-apis/) | ||
| endif() | ||
| get_filename_component(proto_name "${proto_file}" NAME) | ||
| get_filename_component(proto_path "${proto_file}" PATH) | ||
|
|
@@ -346,6 +349,34 @@ function(GenerateGrpcSources) | |
| endif() | ||
| endfunction() | ||
|
|
||
| #---------------------------------------------------------------------- | ||
| # Generate sources from ni-apis proto files | ||
| # Usage: GenerateNiApisProtoSources(PROTO_PATH <relative_path> OUTPUT_SRCS <var> OUTPUT_HDRS <var> [DEPENDS <dependency>...]) | ||
| #---------------------------------------------------------------------- | ||
| function(GenerateNiApisProtoSources) | ||
| set(oneValueArgs PROTO_PATH OUTPUT_SRCS OUTPUT_HDRS) | ||
| set(multiValueArgs DEPENDS) | ||
| cmake_parse_arguments(GEN_ARGS "" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) | ||
|
|
||
| set(proto_srcs "${proto_srcs_dir}/${GEN_ARGS_PROTO_PATH}.pb.cc") | ||
| set(proto_hdrs "${proto_srcs_dir}/${GEN_ARGS_PROTO_PATH}.pb.h") | ||
|
|
||
| add_custom_command( | ||
| OUTPUT "${proto_srcs}" "${proto_hdrs}" | ||
| COMMAND ${_PROTOBUF_PROTOC} | ||
| ARGS --cpp_out ${proto_srcs_dir} | ||
| -I ${CMAKE_SOURCE_DIR}/third_party/ni-apis/ | ||
| -I ${CMAKE_SOURCE_DIR}/third_party/grpc/third_party/protobuf/src/ | ||
| ${GEN_ARGS_PROTO_PATH}.proto | ||
| DEPENDS "${CMAKE_SOURCE_DIR}/third_party/ni-apis/${GEN_ARGS_PROTO_PATH}.proto" ${GEN_ARGS_DEPENDS} | ||
| WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/third_party/ni-apis/ | ||
| VERBATIM | ||
| ) | ||
|
|
||
| set(${GEN_ARGS_OUTPUT_SRCS} "${proto_srcs}" PARENT_SCOPE) | ||
| set(${GEN_ARGS_OUTPUT_HDRS} "${proto_hdrs}" PARENT_SCOPE) | ||
| endfunction() | ||
|
|
||
| set(session_proto_srcs "${proto_srcs_dir}/session.pb.cc") | ||
| set(session_proto_hdrs "${proto_srcs_dir}/session.pb.h") | ||
| set(session_grpc_srcs "${proto_srcs_dir}/session.grpc.pb.cc") | ||
|
|
@@ -372,6 +403,10 @@ set(data_moniker_proto_srcs "${proto_srcs_dir}/data_moniker.pb.cc") | |
| set(data_moniker_proto_hdrs "${proto_srcs_dir}/data_moniker.pb.h") | ||
| set(data_moniker_grpc_srcs "${proto_srcs_dir}/data_moniker.grpc.pb.cc") | ||
| set(data_moniker_grpc_hdrs "${proto_srcs_dir}/data_moniker.grpc.pb.h") | ||
| set(precision_timestamp_proto_srcs "${proto_srcs_dir}/ni/protobuf/types/precision_timestamp.pb.cc") | ||
| set(precision_timestamp_proto_hdrs "${proto_srcs_dir}/ni/protobuf/types/precision_timestamp.pb.h") | ||
| set(waveform_proto_srcs "${proto_srcs_dir}/ni/protobuf/types/waveform.pb.cc") | ||
| set(waveform_proto_hdrs "${proto_srcs_dir}/ni/protobuf/types/waveform.pb.h") | ||
|
|
||
| GenerateGrpcSources( | ||
| PROTO | ||
|
|
@@ -441,6 +476,19 @@ GenerateGrpcSources( | |
| "${data_moniker_grpc_hdrs}" | ||
| ) | ||
|
|
||
| GenerateNiApisProtoSources( | ||
| PROTO_PATH "ni/protobuf/types/precision_timestamp" | ||
| OUTPUT_SRCS precision_timestamp_proto_srcs | ||
| OUTPUT_HDRS precision_timestamp_proto_hdrs | ||
| ) | ||
|
|
||
| GenerateNiApisProtoSources( | ||
| PROTO_PATH "ni/protobuf/types/waveform" | ||
| OUTPUT_SRCS waveform_proto_srcs | ||
| OUTPUT_HDRS waveform_proto_hdrs | ||
| DEPENDS "${precision_timestamp_proto_hdrs}" | ||
| ) | ||
|
|
||
| set(nidriver_service_library_hdrs | ||
| ${nidriver_service_library_hdrs} | ||
| "${session_proto_hdrs}" | ||
|
|
@@ -454,6 +502,8 @@ set(nidriver_service_library_hdrs | |
| "${debugsessionproperties_restricted_grpc_hdrs}" | ||
| "${calibrationoperations_restricted_proto_hdrs}" | ||
| "${calibrationoperations_restricted_grpc_hdrs}" | ||
| "${precision_timestamp_proto_hdrs}" | ||
| "${waveform_proto_hdrs}" | ||
| ) | ||
|
|
||
| foreach(api ${nidrivers}) | ||
|
|
@@ -514,6 +564,8 @@ add_executable(ni_grpc_device_server | |
| ${calibrationoperations_restricted_grpc_srcs} | ||
| ${data_moniker_proto_srcs} | ||
| ${data_moniker_grpc_srcs} | ||
| ${precision_timestamp_proto_srcs} | ||
| ${waveform_proto_srcs} | ||
| ${nidriver_service_srcs}) | ||
|
|
||
| # Enable warnings only on source that we own, not generated code or dependencies | ||
|
|
@@ -655,6 +707,8 @@ add_executable(IntegrationTestsRunner | |
| ${calibrationoperations_restricted_grpc_srcs} | ||
| ${data_moniker_proto_srcs} | ||
| ${data_moniker_grpc_srcs} | ||
| ${precision_timestamp_proto_srcs} | ||
| ${waveform_proto_srcs} | ||
| ${nidriver_service_srcs} | ||
| "${proto_srcs_dir}/nifake.pb.cc" | ||
| "${proto_srcs_dir}/nifake.grpc.pb.cc" | ||
|
|
@@ -742,6 +796,8 @@ add_executable(UnitTestsRunner | |
| ${calibrationoperations_restricted_grpc_srcs} | ||
| ${data_moniker_proto_srcs} | ||
| ${data_moniker_grpc_srcs} | ||
| ${precision_timestamp_proto_srcs} | ||
| ${waveform_proto_srcs} | ||
| "${proto_srcs_dir}/nifake.pb.cc" | ||
| "${proto_srcs_dir}/nifake.grpc.pb.cc" | ||
| "${proto_srcs_dir}/nifake_extension.pb.cc" | ||
|
|
@@ -879,6 +935,8 @@ set(system_test_runner_sources | |
| ${calibrationoperations_restricted_grpc_srcs} | ||
| ${data_moniker_proto_srcs} | ||
| ${data_moniker_grpc_srcs} | ||
| ${precision_timestamp_proto_srcs} | ||
| ${waveform_proto_srcs} | ||
| ${nidriver_service_srcs} | ||
| ${nidriver_client_srcs} | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ package nidaqmx_grpc; | |
|
|
||
| import "session.proto"; | ||
| import "data_moniker.proto"; | ||
| import "ni/protobuf/types/waveform.proto"; | ||
| import "google/protobuf/timestamp.proto"; | ||
|
|
||
| service NiDAQmx { | ||
|
|
@@ -465,6 +466,7 @@ service NiDAQmx { | |
| rpc BeginWriteRaw(BeginWriteRawRequest) returns (BeginWriteRawResponse); | ||
| rpc WriteToTEDSFromArray(WriteToTEDSFromArrayRequest) returns (WriteToTEDSFromArrayResponse); | ||
| rpc WriteToTEDSFromFile(WriteToTEDSFromFileRequest) returns (WriteToTEDSFromFileResponse); | ||
| rpc ReadAnalogWaveforms(ReadAnalogWaveformsRequest) returns (ReadAnalogWaveformsResponse); | ||
| } | ||
|
|
||
| enum BufferUInt32Attribute { | ||
|
|
@@ -3558,6 +3560,12 @@ enum WriteBasicTEDSOptions { | |
| WRITE_BASIC_TEDS_OPTIONS_DO_NOT_WRITE = 12540; | ||
| } | ||
|
|
||
| enum WaveformAttributeMode { | ||
| WAVEFORM_ATTRIBUTE_MODE_NONE = 0; | ||
| WAVEFORM_ATTRIBUTE_MODE_TIMING = 1; | ||
| WAVEFORM_ATTRIBUTE_MODE_EXTENDED_PROPERTIES = 2; | ||
| } | ||
|
|
||
| enum ChannelInt32AttributeValues { | ||
| option allow_alias = true; | ||
| CHANNEL_INT32_UNSPECIFIED = 0; | ||
|
|
@@ -11511,3 +11519,18 @@ message WriteToTEDSFromFileResponse { | |
| int32 status = 1; | ||
| } | ||
|
|
||
| message ReadAnalogWaveformsRequest { | ||
| nidevice_grpc.Session task = 1; | ||
| int32 number_of_samples_per_channel = 2; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ReadAnalogF64 calls this num_samps_per_chan. Nothing in nidaqmx.proto spells out "number_of_samples". |
||
| double timeout = 3; | ||
| oneof waveform_attribute_mode_enum { | ||
| WaveformAttributeMode waveform_attribute_mode = 4; | ||
| int32 waveform_attribute_mode_raw = 5; | ||
| } | ||
| } | ||
|
|
||
| message ReadAnalogWaveformsResponse { | ||
| int32 status = 1; | ||
| repeated ni.protobuf.types.DoubleAnalogWaveform waveforms = 2; | ||
| } | ||
|
|
||
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.
I made these build config changes in order to get nidaqmx_library.cpp to successfully compile on Ubuntu. As far as I can tell, the problem is that after I added the new library functions in functions.py, the nidaqmx_library.cpp became too large to compile with the original settings. It was either timing out or running out of memory or something.
I'm not sure this is the best way to fix the issue, however. Changing the optimization from O2 to O1 means the grpc device server will be less optimized for everyone on linux, right? Another approach that might work is splitting nidaqmx_library.cpp into multiple smaller .cpp files somehow. What do the reviewers think of that?
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.
When you build it locally, does it print any more output, such as
In the past I've sometimes had to use
-fno-var-tracking-assignmentsto compile large code-generated files. I don't know if this option is still relevant or there is a different option nowadays.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.
Also, how long does it take to build locally with -O2? Does it eventually succeed?
Uh oh!
There was an error while loading. Please reload this page.
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.
I don't have a linux machine to build locally on, I've been relying on the CI build for this. All I can see is this, after 33 minutes:
https://github.com/ni/grpc-device/actions/runs/18659129011/job/53195445860
or, when I added --verbose:
https://github.com/ni/grpc-device/actions/runs/18663525332/job/53209586512
I did some googling and some guessing, and figured resource exhaustion was the most likely explanation for exit code 143. So I changed the build parameters to see if that would get the CI to build successfully, and it did. That's all I know at this point.
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.
VTA is a potential cause of resource exhaustion. This feature tracks variable locations for each line of code in order to produce debug info that accurately shows the variable values when you step through the function, even if it's inlined. However, when your code has hundreds of variable assignments in the same function, this feature can take a lot of CPU and memory. I don't know exactly why.
VTA only affects the debugging experience, not execution speed, so if turning it off solves your problem, then it's a better solution than using -O1 and turning off a bunch of optimizations.
Please try updating https://github.com/ni/grpc-device/blob/main/source/codegen/templates/service.cpp.mako to use
#pragma GCC optimizeto scope the-fno-var-tracking-assignmentsor-O1to*_library.cpp. You will need to guard this pragma with#ifdef __GNUC__or similar.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.
In this PR, I've reverted the changes to build_cmake and implemented your suggested changes to service.cpp.mako, and nidaqmx_library.cpp is still compiling without crashing.
As another data point, I tried a different strategy in a different PR, where I reverted the other changes in build_cmake, and just used
-j ${{ runner.os == 'Linux' && '2' || steps.cpu-cores.outputs.count }}, which also allwed nidaqmx_library.cpp to compile without crashing.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.
I checked out your branch on my laptop, commented out the
-fno-var-tracking-assignments, and rantime cmake --build . -j 16 --config RelWithDebInfoon WSL:Running
.ninja_logthrough https://github.com/nico/ninjatracing showsIt seems like all of the DAQmx files are problem files, taking >3 minutes to compile.