Clean New Log File Configuration VI

[oscarfonloz]

• [x] This contribution adheres to CONTRIBUTING.md.

TODO: Check the above box with an 'x' indicating you've read and followed CONTRIBUTING.md.

What does this Pull Request accomplish?

Change the New Log File Configuration VI into a sequencer that is easier to read and complies with the diagram size requirement from Systems R&D

Why should this Pull Request be merged?

I did this cleanup while doing the INI config feature. However, I realized this was a major change, and a potential risk to have in one PR. Therefore, I am submitting in a separate one.

What testing has been done?

Manual testing.

[oscarfonloz]

Enum to Array of Enums.vim is included in the PR because it was introduced to vi.lib until 2020, so we need to have the file in the repo for backwards compatibility.

[niveristand-diff-bot]

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.

Embedded Data Logger System Explorer.lvlib--New Log File Configuration.vi.png


Enum to Array of Enums.vim.png


[oscarfonloz]

I would argue that this change makes the diagram more difficult to read. By hiding the sequence ordering in an enum constant, I can't easily trace the execution ordering.

This also makes me have to scroll through several cases in order to see what's happening. This could more simply be improved by creating subVIs for each of these cases, since this sequence only needs to execute once. I think this change is unnecessary and recommend leaving the existing code as is.

You have good points. I can explain my rationale, an both of you can decide if you want to complete or abandon/cancel the PR. (It doesn't impact our project, so we don't have a strong feeling about it).

Regarding the enum:

1. Sequencers are a common LabVIEW pattern, and (as explained in the PR) the VIM that we're using to create the sequencer is now shipping in vi.lib as of 2020.
2. The enum array will always follow the steps in the enum, in the order in which they are in the enum. I presume we can solve this confusion, or clarify further, with a comment to that VI or the case structure itself.

Regarding scrolling horizontally instead of through case structures:

1. Tip: you can press the control key and use the mouse wheel to scroll through the cases of the structure. It's way easier than moving horizontally from my point of view
2. Large block diagrams generally are something we prefer to avoid because it makes it difficult to follow the dataflow for wires that cross all over the diagram. It also blends all the functionality into the "train tracks" that requires more time for someone arriving to the code to understand what is going on. The case structure helps explain what each phase of the sequence is doing.
3. I agree we could encapsulate the functionality of each step in a subVI (and G code purists would argue that the enum itself should be a Type Def. However, I went against those because I did not see a need to pollute the library with more files than those required, especially for functionality that is just breaking down code and not intended for code reuse.

[oscarfonloz]

@debryant do you have any thoughts?

[niveristand-diff-bot]

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.

Embedded Data Logger System Explorer.lvlib--New Log File Configuration.vi.png


Enum to Array of Enums.vim.png


[Karl-G1]

We're moving forward with a 20.5 release from the current branch build. Given the lack of desire from our tech leads to merge this PR, I'm going to close it for now. We can revisit any refactor to this VI if it is modified at a later time.