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

[minor] update: remove wire branch for gRPC id out in Create Measurement.vi #619

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

varshini-senthil
Copy link
Contributor

  • This contribution adheres to CONTRIBUTING.md. (Required)
  • Automatically post PR comments with images for G code changes? (Recommended for small changes)

What does this Pull Request accomplish?

  • Removed the wire branch for gRPC id in Create Measurement.vi and rewired to use the gRPC id out of Register Enum Metadata.vi to complete the metadata registration.
  • Updated the version of ni_lib_labview_grpc_library to be >= 1.2.6.1 in the Measurement Plug-In SDK Generator.vipb build spec, as the client generator requires the fix included in the 1.2.6.1 version of gRPC library to work without LabVIEW crash.

Why should this Pull Request be merged?

  • To remove the wire branch and use the gRPC id out of Register Enum Metadata.vi to complete the metadata registration in Create Measurement.vi.
  • Update the ni_lib_labview_grpc_library version information in the Measurement Plug-In SDK Generator.vipb build spec.

What testing has been done?

  • Tested by generating and running the client with the latest ni_lib_labview_grpc_library (version 1.2.6.1) package.

@varshini-senthil
Copy link
Contributor Author

Create Measurement.vi
Diff image

Before:
image

After:
image

@Avinash2Suresh
Copy link
Contributor

@jasonmreding I tried installing the latest LabVIEW gRPC version 1.2.6.1, and I am still facing the LabVIEW crash after the client generation. When I debugged it, I noticed that the feature toggle is still set to True. I’ve verified this on multiple PCs, and it’s behaving the same way. It might be working on your PC because we manually set the feature toggle to False during our previous debugging. Given this, I don’t think we can proceed with checking in this PR.

Also, I feel we should do one of the following until your PR is checked in and a new release for gRPC LabVIEW is made:

  1. Document somewhere (perhaps in our repo README) that the LabVIEW generator should only work with gRPC LabVIEW version 1.0.1.1, or if they want to use the latest gRPC LabVIEW version, they need to manually set the feature toggle as false to make it work.
  2. Add LabVIEW 1.0.1.1 as a direct dependency for the LabVIEW client generator during installation. This way, during the client generator installation process, it will automatically install/upgrade/downgrade the LabVIEW gRPC version to 1.0.1.1.
    3.We should not recommend any LabVIEW gRPC version other than 1.0.1.1 for our measurement plugin in LabVIEW until the issue is fixed. However, I don’t think this is a feasible option since the latest gRPC version fixes many bugs, and we can’t compromise on that as per our previous discussions.

Of the three above, I would prefer going with the first option. Please share your thoughts.

cc: @dixonjoel

@dixonjoel
Copy link
Collaborator

@jasonmreding I tried installing the latest LabVIEW gRPC version 1.2.6.1, and I am still facing the LabVIEW crash after the client generation. When I debugged it, I noticed that the feature toggle is still set to True. I’ve verified this on multiple PCs, and it’s behaving the same way. It might be working on your PC because we manually set the feature toggle to False during our previous debugging. Given this, I don’t think we can proceed with checking in this PR.

Also, I feel we should do one of the following until your PR is checked in and a new release for gRPC LabVIEW is made:

  1. Document somewhere (perhaps in our repo README) that the LabVIEW generator should only work with gRPC LabVIEW version 1.0.1.1, or if they want to use the latest gRPC LabVIEW version, they need to manually set the feature toggle as false to make it work.
  2. Add LabVIEW 1.0.1.1 as a direct dependency for the LabVIEW client generator during installation. This way, during the client generator installation process, it will automatically install/upgrade/downgrade the LabVIEW gRPC version to 1.0.1.1.
    3.We should not recommend any LabVIEW gRPC version other than 1.0.1.1 for our measurement plugin in LabVIEW until the issue is fixed. However, I don’t think this is a feasible option since the latest gRPC version fixes many bugs, and we can’t compromise on that as per our previous discussions.

Of the three above, I would prefer going with the first option. Please share your thoughts.

cc: @dixonjoel

Are you sure you don't have a feature_config.ini file anywhere under C:\Program Files\National Instruments\LabVIEW 2024\vi.lib\gRPC? If you had one before, uninstall of the packages won't remove it.

@Avinash2Suresh
Copy link
Contributor

@jasonmreding I tried installing the latest LabVIEW gRPC version 1.2.6.1, and I am still facing the LabVIEW crash after the client generation. When I debugged it, I noticed that the feature toggle is still set to True. I’ve verified this on multiple PCs, and it’s behaving the same way. It might be working on your PC because we manually set the feature toggle to False during our previous debugging. Given this, I don’t think we can proceed with checking in this PR.
Also, I feel we should do one of the following until your PR is checked in and a new release for gRPC LabVIEW is made:

  1. Document somewhere (perhaps in our repo README) that the LabVIEW generator should only work with gRPC LabVIEW version 1.0.1.1, or if they want to use the latest gRPC LabVIEW version, they need to manually set the feature toggle as false to make it work.
  2. Add LabVIEW 1.0.1.1 as a direct dependency for the LabVIEW client generator during installation. This way, during the client generator installation process, it will automatically install/upgrade/downgrade the LabVIEW gRPC version to 1.0.1.1.
    3.We should not recommend any LabVIEW gRPC version other than 1.0.1.1 for our measurement plugin in LabVIEW until the issue is fixed. However, I don’t think this is a feasible option since the latest gRPC version fixes many bugs, and we can’t compromise on that as per our previous discussions.

Of the three above, I would prefer going with the first option. Please share your thoughts.
cc: @dixonjoel

Are you sure you don't have a feature_config.ini file anywhere under C:\Program Files\National Instruments\LabVIEW 2024\vi.lib\gRPC? If you had one before, uninstall of the packages won't remove it.

Yes. Here are the steps I have followed:

  • I took a PC where the gRPC library version 1.0.1.1 is already installed, and the PC has LabVIEW 2021 installed.
  • I opened C:\Program Files\National Instruments\LabVIEW 2021\vi.lib\gRPC\LabVIEW gRPC Library\Libraries\Win64\ and I don’t see the INI file.
  • I updated the gRPC library version to 1.2.6.1.
  • I went back to the same location, but I still don’t see the INI file.
  • I installed the client generator and tried creating a client for a measurement.
  • The client was created successfully without causing LabVIEW to crash.
  • Then, I went to the gRPC location, and I saw that the INI file has been generated, and the flag has been set to true.
  • Now, when I tried creating a client, LabVIEW crashed during client generation.

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.

5 participants