search

ni / grpc-labview

gRPC client and server support for LabVIEW
MIT License
90 stars 62 forks source link

Fix gRPCid out when registering empty messages #383

Closed jasonmreding closed 2 months ago

jasonmreding commented 2 months ago

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 commented 2 months ago

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

dixonjoel commented 2 months ago

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

jasonmreding commented 2 months ago

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 commented 2 months ago

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.