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

[US2366055] Update "LabVIEW DataType to Message Element DataType.vi" to use gRPC "Enum Value" #263

Conversation

WesleyTangNationalInstruments
Copy link
Collaborator

Updated LabVIEW DataType to Message Element DataType.vi to convert Eum U8, Enum U16, Enum U32, and Enum U64 to use gRPC EnumValue Message Element DataType instead. Previously, the were converted to their corresponding UInt type.

image

AB#2366055

…um U8", "Enum U16", "Enum U32", and "Enum U64" to use gRPC "EnumValue" Message Element DataType instead.
@JasonC512
Copy link

Where and how is this VI used? Are all of the callers in the other PR you created? ie: ni/measurement-plugin-labview#213

The diff looks reasonable, but it is difficult to know if this would break any assumptions made by callers. Will caller VIs break with a compiler error because of this change? Or would it just be potential logic errors?

@WesleyTangNationalInstruments
Copy link
Collaborator Author

WesleyTangNationalInstruments commented May 24, 2023

@JasonC512

Where and how is this VI used?

This VI is called by MeasurementLink Measurement Server.lvlib:Get MetaData from Configuration.vi and MeasurementLink Measurement Server.lvlib:Get MetaData from Result.vi to populate the configuration and output parameters.

https://github.com/ni/measurementlink-labview/tree/main/Source/Runtime/MeasurementLink%20Measurement%20Server/Helpers

See usage:
image

Are all of the callers in the other PR you created? ie: ni/measurement-plugin-labview#213

The callers of this VI already exist in the MeasurementLink repo. My other PR adds no new callers. The change is that if this VI tells the caller "you have a UInt", the remaining code will treat it as a UInt, but if this VI tells the caller "you have an Enum", we can label the configuration/response correctly so that we interpret the data via the enum annotations (which is what that other PR adds support for).

The diff looks reasonable, but it is difficult to know if this would break any assumptions made by callers. Will caller VIs break with a compiler error because of this change? Or would it just be potential logic errors?

Based on previous discussions with Everett, we think that this should be pretty safe. The existing behavior is that LabVIEW enums get coerced to gRPC UInts and there is no LabVIEW types that converts to gRPC Enums. This change results in LabVIEW enums now being converting to gRPC Enums instead of gRPC UInts. The main issue I could see is that if an existing caller relies on LabVIEW enums being coerced to gRPC UInts they would need to update their handling, but I don't think there are any callers trying to use other than MeasurementLink. This would be more of a change in behavior rather and I'd lean towards this unlikely to cause compile errors.

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.

3 participants