nidaqmx-gRPC Local Thread Memory (GetExtendedErrorInfo)

[IPetersNI]

What does this Pull Request accomplish?

This Pull Request acts on the Issue 684 (#684) removing the use of GetExtendedErrorInfo to get errors as they use thread local storage and instead utilizing grpc error codes.

Breaking changes

• GetExtendedErrorInfo: In (source/codegen/metadata/nidaqmx/functions.py) Changed codegen method from 'public' to 'private' thus removing use within services and clients .cpp files.
• nidaqmx.proto: Removes generated method creating a breaking change for the GetExtendedErrorInfo method.
• Error message retrieval: When a call is made that fails, the gRPC status object now includes the error message.

Why should this Pull Request be merged?

This should be merged to make GetExtendedErrorInfo unusable by the users as it will generate unwanted error responses.

What testing has been done?

Local builds have been done to ensure building is functional and testing of getting a error locally was done as well.
AB#2085088

[IPetersNI]

NOTE: A pull was done to update my local main branch and it looks as if GetExtendedErrorInfo is not incorporated in the examples for nidaqmx. It is seen however in some generated services and clients. Due to this I chose to insert a "codegen":"private"

[dmondrik]

@reckenro any idea why the python validation is failing? It's not clear to me what's even causing it.

[reckenro]

@reckenro any idea why the python validation is failing? It's not clear to me what's even causing it.

Yeah, think one of our dependencies latest versions updated yesterday which is causing issues. I have a fix in PR 752 where I was also hitting this: https://github.com/ni/grpc-device/pull/752/files#diff-de4bc5702b4e1bd6b0ed30ebefbd2d657f568a8dd2a845f39a352fefe3de9f1d

[reckenro]

Looks good to me. One thing that I'm less familiar with is updating it in scrapigen: https://github.com/ni/grpc-device-scrapigen It's currently what produces this grpc-device metadata for nidaqmx.

We can get @DavidCurtiss or @astarche 's thoughts on if we should update scrapigen for this (I'm guessing yes) and possibly more info on the how.

[DavidCurtiss]

... thoughts on if we should update scrapigen for this (I'm guessing yes) and possibly more info on the how.

Yes, we should. If it would work to just remove the function, then the best path forward is to copy the function from DAQmx.h into DAQmx_exlcude.h. But if it's important to keep it present but mark it private, we'd write special code to do that in src/scrapigen/daqapi/filterfunctions.py.

[IPetersNI]

@harsha-bhushan If you have some time I was wondering if I can have your thoughts on this PR. Thanks!

[harsha-bhushan]

Sorry, I got this question after I approved the PR.
The function documentation says this function is valid for the last function that failed. So isnt the bug reported an expected behavior?

Wouldn't there be a similar behavior in C API as well when the function is used the way shown in the bug? What am I missing?

[dmondrik]

Sorry, I got this question after I approved the PR. The function documentation says this function is valid for the last function that failed. So isnt the bug reported an expected behavior?

Wouldn't there be a similar behavior in C API as well when the function is used the way shown in the bug? What am I missing?

@harsha-bhushan what you're missing is that for gRPC-device the server makes calls on unpredictable threads. In the C API it is not a problem because one's code always executes sequentially on the same thread. So you get back an error, and you check for the details on the same thread. But that's not possible on gRPC-device because even if in your client-side code you make the calls on the same thread, that's not the same where the code actually calls the driver.

[IPetersNI]

@bkeryan Brad I was wondering if you could take some time to look at this bug and be able to close it out on the GitHub side. Right now it is still pending validation.