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

Modify measurement template to have a PPL for the Measurement UI #511

Merged
merged 16 commits into from
May 9, 2024
Merged

Modify measurement template to have a PPL for the Measurement UI #511

merged 16 commits into from
May 9, 2024

Conversation

jarnoldNI
Copy link
Contributor

@jarnoldNI jarnoldNI commented Apr 25, 2024

Changed the template to be better suited for UIs which use anything other than LabVIEW primitives. This template separates the UI VI into a different lvlib. The generator adds a PPL build spec, builds the PPL, and updates the Get UI Details.vi with the appropriate path.

Here's what the output of the new template is:
image

Why should this Pull Request be merged?

Adds a template with a UI users can use with subVIs, type defs, etc. as opposed to the original template UI which only works with LabVIEW primitives.

What testing has been done?

Built into a local VI package, installed and tested creating templates with and without the new flag toggled. Ran the new template in the development environment. Also, built the exe and deployed as well. Tested that the plug-in works in InstrumentStudio.

Copy link
Contributor

@jasonmreding jasonmreding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't done a detailed review yet since viewing diffs is very difficult with the current structure of the change. Rather than making a copy of the current template library in source and adding a new library for the UI, I would prefer to just maintain the existing template as is. Then as part of the scripting when creating the service, remove the UI VI from the template project and add it to a new UI library that is created on the fly. That should eliminate having to maintain two copies of what appear to be the exact same files in source. As long as you have existing VIs to work from, I don't think the scripting to create new libraries and modifying the copy of the template within the user's project should be too bad.

Once that structure is in place, the real changes should be more apparent, and I may have additional feedback at that time.

Another thing you could consider while doing this is simply adding a second scripting pass over what gets created from the existing template. That might allow for adding a second command into the tools menu for creating a UI template stand alone which might be nice for cases where the measurement is written in another language. Or it could be used to upgrade an existing measurement from a stand alone UI VI to an advanced UI with its own build. The logic for these commands would be identical to the logic you are adding here for creating the measurement with the advanced UI checkbox checked. They would just need to be wrapped into a UI that is specific to the needs of that command.

@jarnoldNI
Copy link
Contributor Author

Another option which would get rid of having to maintain two templates would be to just keep one and change it to be the 'advanced' one that I've added here. No more Meas UI in the same lvlib and I could get rid of the checkbox option on the UI. Just do a full switch. I added the switch in case someone really wanted to the old behavior.

@jasonmreding
Copy link
Contributor

Another option which...

I would prefer that over maintaining two templates as is the case in the current state of the PR. I'm not sure how clients would feel about that though. If scripting the changes needed to support both use cases from a single template is a lot of work, we can discuss this further. We should probably run it by some of our internal clients first though to gauge their reaction.

@jarnoldNI
Copy link
Contributor Author

Another option which...

I would prefer that over maintaining...

The scripting work feels like a large lift and potentially error prone compared to the value of keeping the original template around. I'll survey internally then we can gauge the response and determine the next steps.

…orts UIs; removed the option for 'advanced UI'
@jarnoldNI
Copy link
Contributor Author

Latest commit removes the additional template and updates the original one to the new pattern shown above which better supports UI development.

@dixonjoel
Copy link
Collaborator

dixonjoel commented May 3, 2024

Please update the PR title and descripton since we don't have the check box anymore.

@dixonjoel dixonjoel changed the title Add 'advanced UI' option to the template generator Modify measurement template to have a PPL for the Measurement UI May 9, 2024
@jasonmreding jasonmreding merged commit 97e6046 into ni:main May 9, 2024
6 checks passed
@jarnoldNI jarnoldNI deleted the users/jarnold/add-adv-ui-to-generator branch May 9, 2024 22:03
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.

3 participants