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

Fix gRPCid out when registering empty messages #383

Merged
merged 3 commits into from
Sep 13, 2024

Conversation

jasonmreding
Copy link
Collaborator

Similar to #336. Convert tunnel to shift register to protect against zero iteration For Loops causing the gRPCid out from getting reset to zero.

image

@dixonjoel
Copy link
Collaborator

Change looks fine. Just curious what effects there are of the bug and how you encountered this problem.

@dixonjoel
Copy link
Collaborator

Also looks like you have a build error related to this VI

@jasonmreding
Copy link
Collaborator Author

Also looks like you have a build error related to this VI

For some reason LV ended up saving in 2024 even though the owning library has configured the source version as 2019. Should be fixed now.

@jasonmreding
Copy link
Collaborator Author

Change looks fine. Just curious what effects there are of the bug and how you encountered this problem.

The bug is that gRPCid in/out won't chain through correctly if the for loop executes zero times. It's a "classic" bug with LV code that continually pops up because it is not intuitive and it is easy to overlook in code reviews. I don't recall if their is a VI analyzer check for this, but there probably is. To my knowledge, nobody has been burned by this yet. There was a similar bug filed against Register Enum Metadata which is essentially the same block diagram as this (see PR comment). However, when that was fixed, we overlooked the same bug with this VI. It came to my attention in another code review when I noticed the code was forking the "in" wire rather than using the "out" terminal.

@jasonmreding jasonmreding merged commit 3da3b19 into ni:master Sep 13, 2024
5 checks passed
@jasonmreding jasonmreding deleted the users/jreding/RegistrationFix branch September 13, 2024 22:16
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.

2 participants