search

ni / niveristand-communications-bus-template

Template custom device for using communications buses in VeriStand
http://www.ni.com
MIT License
0 stars 3 forks source link

Uniquify execution unit names #132

Closed buckd closed 2 years ago

buckd commented 2 years ago

What does this Pull Request accomplish?

Uniquifies name of each constructed Execution Unit by prepending the custom device name.

Why should this Pull Request be merged?

Fixes #128.

What testing has been done?

Verified through the compile action that each execution unit has the custom device name prepended.

niveristand-diff-bot commented 2 years ago

Bleep bloop!

LabVIEW Diff Robot here with some diffs served up hot for your pull request.

Notice something funny? Help fix me on my GitHub repo.

Communication Bus Template Engine.lvlib--Implementation.lvlib--Execution Unit Factory.lvclass--Create Execution Unit.vi.png ![capture](https://raw.githubusercontent.com/niveristand-diff-bot/diff-images/master/NI/niveristand-communications-bus-template/PR-132/2022-02-28/15%3A30%3A07/Communication%20Bus%20Template%20Engine.lvlib--Implementation.lvlib--Execution%20Unit%20Factory.lvclass--Create%20Execution%20Unit.vi.png)
Communication Bus Template Engine.lvlib--Implementation.lvlib--Execution Unit Factory.lvclass--Uniquify Execution Unit Name.vi.png ![capture](https://raw.githubusercontent.com/niveristand-diff-bot/diff-images/master/NI/niveristand-communications-bus-template/PR-132/2022-02-28/15%3A30%3A07/Communication%20Bus%20Template%20Engine.lvlib--Implementation.lvlib--Execution%20Unit%20Factory.lvclass--Uniquify%20Execution%20Unit%20Name.vi.png)
buckd commented 2 years ago

Technically this could fail for SLSC custom devices in the same slot but different SLSC chassis (or identically named T&S and regular CDs). We don't have any plans for irregular communications bus custom devices though, so I'm fine with this as-is.

The problem isn't really with the execution unit names. The problem is with the naming we provide to the channel groups of the Inline Async API, but we create those group names based on execution unit. I agree this could still fail to accomplish its goal in certain situations, but as you mentioned, we don't really have anything else planned. Using just the name is useful for tracking purposes though.

We could do something like inject a GUID into the channel group name if we wanted to. We could keep the device name for tracing purposes, but then add something globally unique. I just didn't feel like it was necessary at this point. The update to the channel group name is captured in the issue in case we need to refer to it later. I can add the GUID (or something like a fully sysdef path) now or we can defer it until it becomes an issue. Thoughts?

rtzoeller commented 2 years ago

I don't feel it necessary at this point.