[US2366055] Annotated Enum Support in LabVIEW MeasurementService

[WesleyTangNationalInstruments]

• This contribution adheres to CONTRIBUTING.md.

What does this Pull Request accomplish?

• Added the "Create Enum Type Specialization.vi" helper VI to allow users to convert enums to Type Specializations.
• Updated "Get MetaData from Results.vi" to handle annotations in the same way that "Get MetaData from Configuration.vi" already does.
• Updated "Type Specialization Key.ctl" and "Type Specialization.ctl" to contain entries for enum and enum.values.
• Updated documentation comments in "Get Type Specializations.vi" in examples to cover enums.

Why should this Pull Request be merged?

AB#2366055

What testing has been done?

• Manually tested by calling "Create Enum Type Specialization.vi" from "Get Type Specializations.vi" in both examples.

[WesleyTangNationalInstruments]

Added the "Create Enum Type Specialization.vi" helper VI to allow users to convert enums to Type Specializations.

Create Enum Type Specialization.vi

Updated "Get MetaData from Results.vi" to handle annotations in the same way that "Get MetaData from Configuration.vi" already does.

Get MetaData from Results.vi

Updated "Type Specialization Key.ctl" and "Type Specialization.ctl" to contain entries for enum and enum.values.

Type Specialization Key.ctl

Type Specialization.ctl

Updated documentation comments in "Get Type Specializations.vi" in examples to cover enums.

Get Type Specialization.vi.

[pbirkhol-ni]

• It looks like the VIs are saved in LabVIEW 2021. We still want to save them in LabVIEW 2020SP1.
• For the new VI.
  • You should make the Enum Reference input required.
  • Click Edit --> Clean up Panel to make the front panel look better.
  • It is currently community scoped within the library, which will make it difficult for customers to use. It should be Public scoped.
  • The Parameter Name in the type specialization cluster is hardcoded to enum_values. What if there are multiple enums? That seems like it would cause conflicts.
  • You trim the last comma and white space, but it isn't obvious what it is doing just by looking at the code. Can you document that? Also, the 2 constant should be an I32.
  • I assume you will update at least one example to use an enum? I think it would be useful to show how to use this VI. I assume you would call it directly from Get Type Specializations.vi, right?
  • I wonder if we should put this VI in the Get Type Specializations.vi template VI. It can be in a diagram disable structure. That way customers don't have to hunt for it when/if they need it.
• In Get Metadata from Results, you add a new input but never wire it in the caller. You should make that input required, which will break any caller that doesn't wire it. You should also pass the new input to an output terminal to avoid wire forks (and copies of the object).
• You should add the new comment about enum type specializations to the UIProgressUpdates example and also to the template VI: C:\dev\measurementlink-labview\Source\Generator\MeasurementLink Measurement Template\Get Type Specializations.vi.

[WesleyTangNationalInstruments]

Create Enum Type Specialization.vi

Get MetaData from Configuration.vi

Get MetaData from Results.vi

Type Specialization Key.ctl

Type Specialization.ctl

Get Type Specialization.vi
Note: The comments added (marked in red) were propagated to existing examples, but the disabled block was not as enums are not used in those examples.

[WesleyTangNationalInstruments]

• Did Get MetaData from Configurations.vi change? If not, I think you should revert it.

Yes, the change was adding an output terminal for MeasurementPluginService. You made an earlier comment on Get MetaData from Results.vi about the newly added MeasurementPluginService not having an associated output and that adding one would reduce wire forks in the caller and that it should be a required input. Since I had based my changes to Get MetaData from Results.vi off of the existing code in Get MetaData from Configurations.vi, I made corresponding changes by adding an output terminal for MeasurementPluginService to Get MetaData from Configurations.vi as well when addressing that comment.

See usage here:

It looks like the linking changed when you modified the template Get Type Specializations.vi. It expects its typedefs to be in vi.lib. You will have to copy the typedefs to vi.lib, then relink them, then resave.

I think I addressed this correctly by relinking against the installed versions of the VIs in vi.lib (rather than pointing them at the ones in my local git repo), but let me know if that's not the case.

Screenshots of VIs below:

Create Enum Type Specialization.vi

Get Type Specialization.vi Template

Get MetaData from Configuration.vi

Get MetaData from Results.vi

[JasonC512]

I'd like to see a demo of what you have. Maybe in standup or just give me a call. It is difficult to diff LV code.

[pbirkhol-ni]

This is looking good. A few more comments:

• The Get Type Specialization.vi for the CancellableMeasurement example and the DCPower example are no longer referencing vi.lib controls. You need to relink them.
• On the block diagram of Create Enum Type Specialization.vi, all of the inputs should be aligned on the left and all of the outputs aligned on the right. Also, I think we're inconsistent currently, but I've been trying to standardize on using not using icons for the block diagram terminals. Can you unselect View as Icon for the controls and indicators?
• This isn't an action item, but I don't know if putting this VI in the Measurementlink Measurement Server library is the best place. It is the best place at the moment, but I'm wondering if we shouldn't have a separate library for public API VIs. This is something I can follow up on later. FYI @jasonmreding

[WesleyTangNationalInstruments]

Unselected "View as Icon". Aligned output controls to the right.

[WesleyTangNationalInstruments]

Fixed alignment.

[jasonmreding]

The block diagram comment you updated in the template, you inserted the documentation about enums in the middle of the pin.instrument_type annotation documentation.

The last line that discusses custom instruments should be grouped with the information for the built in instrument types in the pin map. You might consider adding a blank line to denote paragraphs and a natural break in topic from previous text. Since each section begins with a bold heading followed by a colon, I think each section will be easily identifiable even if a section has multiple paragraphs.

[jasonmreding]

If I'm reading things correctly, we end up calling the same VIs (and measurement service plugin VI) to populate type specializations for both the configuration parameters and the output parameters. Is that correct? If so, how does the plugin implementation know whether to populate annotations for the configuration parameters vs. the output parameters? It seems like we either need to have a plugin VI for inputs vs. outputs, add an input to indicate whether we're asking for inputs or outputs, or update Parameter Type Specialization.ctl to include an indication that the specialization applies to an input parameter vs. an output parameter (and then update the server code accordingly). I don't think there's anything that would otherwise prevent an input and output parameter from having the same name. Whatever we do, we need to ensure it maintains compatibility with existing measurement services unless there is realistically no other option other than to break compatibility.

[jasonmreding]

Perhaps this would be better to consider as a follow on PR, but have we considered proving a general purpose VI that given the Measurement Configuration.ctl cluster and/or the Measurment Results.ctl cluster, would automatically generate the correct annotations for enum controls and path controls. Users would still have to manually add pin control annotations since we can't figure that out automatically from the control itself. However, I'm not sure why we're making the user manually do that for path controls or making them manually specify each enum control in the cluster vs. just looking through all of the top level cluster element ourselves for enum controls. It would also eliminate have to hard code control names in multiple places.

[pbirkhol-ni]

Perhaps this would be better to consider as a follow on PR, but have we considered proving a general purpose VI that given the Measurement Configuration.ctl cluster and/or the Measurment Results.ctl cluster, would automatically generate the correct annotations for enum controls and path controls. Users would still have to manually add pin control annotations since we can't figure that out automatically from the control itself. However, I'm not sure why we're making the user manually do that for path controls or making them manually specify each enum control in the cluster vs. just looking through all of the top level cluster element ourselves for enum controls. It would also eliminate have to hard code control names in multiple places.

I think this is a great idea and definitely possible. You can get a refnum for all controls in a cluster, then use the Class Name property to figure out if the control is an enum, then pass the refnum to the VI that Wes wrote to generate the type specialization. We can do all of this without requiring the customer to do anything.

I don't know that it is worth it to go back and do this for paths considering how rare that will probably be for LabVIEW code, but I could be convinced otherwise.

[pbirkhol-ni]

If I'm reading things correctly, we end up calling the same VIs (and measurement service plugin VI) to populate type specializations for both the configuration parameters and the output parameters. Is that correct? If so, how does the plugin implementation know whether to populate annotations for the configuration parameters vs. the output parameters? It seems like we either need to have a plugin VI for inputs vs. outputs, add an input to indicate whether we're asking for inputs or outputs, or update Parameter Type Specialization.ctl to include an indication that the specialization applies to an input parameter vs. an output parameter (and then update the server code accordingly). I don't think there's anything that would otherwise prevent an input and output parameter from having the same name. Whatever we do, we need to ensure it maintains compatibility with existing measurement services unless there is realistically no other option other than to break compatibility.

We could add an annotation that specifies whether the type specialization is an input or output. That would be easy to do from LabVIEW (especially if we generate the enum type specializations automatically). I don't know if that would work from the client side, though.

I'm curious how this would work for paths and pins. Do we just assume those are always inputs? This also doesn't seem like a solely LabVIEW problem. Python would have the same problem, right?

[bkeryan]

We could add an annotation that specifies whether the type specialization is an input or output. That would be easy to do from LabVIEW (especially if we generate the enum type specializations automatically). I don't know if that would work from the client side, though.

There is no need to add an annotation to indicate whether a parameter is an input or output. The parameter metadata is already split into configuration parameters and output parameters.

The LV APIs for specifying type specializations and other annotations should identify parameters by name + input/output, not name alone.

This sounds like AB#2392402, but in LabVIEW.

I'm curious how this would work for paths and pins. Do we just assume those are always inputs? This also doesn't seem like a solely LabVIEW problem. Python would have the same problem, right?

Paths and pins are only supported for inputs. This is why output parameters didn't support annotations until enums were added.

In Python, you specify type specializations and other annotations using decorator methods which already distinguish between configuration parameters and output parameters, so I don't think the Python API has the same problem.

[jasonmreding]

I don't know that it is worth it to go back and do this for paths considering how rare that will probably be for LabVIEW code, but I could be convinced otherwise.

Just because the service is written LV doesn't mean the UI will be a .vi file. Also, for auto generated UIs like in TS or was done in a Blazor prototype, the annotations might still be useful. I don't think it's necessarily a high priority, but I also don't think it's difficult so we should probably just do it for consistency.

We could add an annotation that specifies whether the type specialization is an input or output. That would be easy to do from LabVIEW (especially if we generate the enum type specializations automatically). I don't know if that would work from the client side, though.

There is no need to add an annotation to indicate whether a parameter is an input or output. The parameter metadata is already split into configuration parameters and output parameters.

The LV APIs for specifying type specializations and other annotations should identify parameters by name + input/output, not name alone.

To be explicit, the annotations at the measurement_service.proto level are a (string, string) dictionary attached to each parameter. Since they are already owned by the definition of the parameter, there is no need to disambiguate them as input/output or even which parameter they apply to. That is already self evident based on the data hierarchy.

However, the LV API we created as a wrapper around that to make it easier to specify the annotations exposed things as an array of annotations for all parameters with only a parameter name as a way to identify which annotation goes with which parameter. This is where we chose poorly since the parameter name alone isn't sufficient to uniquely identify a parameter since input and output parameters can have the same name. So I think we either need to add the direction to the annotations for mapping purposes in GetMetadata where we encode that list into a dictionary of annotations attached to each parameter, or we need to make the call indicate whether annotations for inputs or outputs are being asked for and then maintain two lists for later encoding in GetMetadata.

[bkeryan]

So I think we either need to add the direction to the annotations for mapping purposes in GetMetadata where we encode that list into a dictionary of annotations attached to each parameter,

I think we should avoid extending the annotation schema to work around bugs in the LV API.

or we need to make the call indicate whether annotations for inputs or outputs are being asked for and then maintain two lists for later encoding in GetMetadata.

I think we should do it this way.

[pbirkhol-ni]

So I think we either need to add the direction to the annotations for mapping purposes in GetMetadata where we encode that list into a dictionary of annotations attached to each parameter,

I think we should avoid extending the annotation schema to work around bugs in the LV API.

or we need to make the call indicate whether annotations for inputs or outputs are being asked for and then maintain two lists for later encoding in GetMetadata.

I think we should do it this way.

I have talked to Wes and Jason this morning. Here is the plan:

• Wes is going to fix this issue by automatically creating type annotations for enums. He can do that in a place where we already know whether we're dealing with inputs or outputs.
• I am going to file a bug about adding a way for customers to specify whether an annotation is for an input or output. I will make sure this bug gets fixed before release. As part of the bug fix, we will allow customers to override our automatic enum type annotation if they want.