Skip to content

Update the niscope example to have pin arrays#146

Merged
subash-suresh merged 2 commits intomainfrom
users/subash/update-examples
Dec 23, 2022
Merged

Update the niscope example to have pin arrays#146
subash-suresh merged 2 commits intomainfrom
users/subash/update-examples

Conversation

@subash-suresh
Copy link
Copy Markdown
Contributor

@subash-suresh subash-suresh commented Dec 7, 2022

What does this Pull Request accomplish?

  • Made updates to the niscope_acquire_waveform example to have pin arrays in place of pins with comma seperated strings.
  • Some minor name changes and UI updates

Why should this Pull Request be merged?

  • niscope_acquire_waveform
    Modified the name of the highlighted parameter from trigger_source to Source

image

What testing has been done?

Manually loaded the UI and verified the loading of the measurement UI and the population of pin values in the dropdown.
I ran the measurement and verified that the given values are taken properly.
NIScopeExample

@bkeryan
Copy link
Copy Markdown
Collaborator

bkeryan commented Dec 7, 2022

Modified the name of the highlighted parameter from trigger_source to Source

Thanks for catching that.

@dixonjoel
Copy link
Copy Markdown
Collaborator

The pin control in .measui still only supports selection of a single pin, correct? And we don't have multi-select. So what does it look like in the UI control for default values and how do you send in multiple pin values? Do you have to type in the comma-separated strings? And does our validation logic allow that? Maybe adding a couple screenshots with the default values or showing how the user picks a different set of pins would help.

Comment thread examples/niscope_acquire_waveform/measurement.py Outdated
Comment thread examples/niscope_acquire_waveform/NIScopeAcquireWaveform.measui
@bkeryan
Copy link
Copy Markdown
Collaborator

bkeryan commented Dec 7, 2022

The pin control in .measui still only supports selection of a single pin, correct? And we don't have multi-select. So what does it look like in the UI control for default values and how do you send in multiple pin values? Do you have to type in the comma-separated strings? And does our validation logic allow that? Maybe adding a couple screenshots with the default values or showing how the user picks a different set of pins would help.

The pin array causes an error. I posted a screenshot.
This is no longer reproduced , i have attached a gif in the description

@subash-suresh subash-suresh force-pushed the users/subash/update-examples branch from 8eda05e to 016dbe7 Compare December 20, 2022 14:45
@subash-suresh subash-suresh changed the base branch from main to users/bkeryan/proto-updates-4.21 December 20, 2022 14:46
Copy link
Copy Markdown
Collaborator

@bkeryan bkeryan left a comment

Choose a reason for hiding this comment

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

Approved with suggestions.

Now that this no longer affects the 12/15 drop and the corresponding C# code in https://ni.visualstudio.com/DevCentral/_git/ASW/pullrequest/407435 will be committed soon, this update is ok to me.

Also, please wait until protobuf-4.21, proto-updates-4.21, and proto-updates are committed and then commit this to main.

Comment thread ni_measurement_service/session_management.py
@subash-suresh subash-suresh changed the base branch from users/bkeryan/proto-updates-4.21 to main December 21, 2022 04:38
Comment thread examples/niscope_acquire_waveform/measurement.py Outdated
@subash-suresh subash-suresh self-assigned this Dec 22, 2022
@subash-suresh
Copy link
Copy Markdown
Contributor Author

Approved with suggestions.

Now that this no longer affects the 12/15 drop and the corresponding C# code in https://ni.visualstudio.com/DevCentral/_git/ASW/pullrequest/407435 will be committed soon, this update is ok to me.

Also, please wait until protobuf-4.21, proto-updates-4.21, and proto-updates are committed and then commit this to main.

These changes have gone in

The pin control in .measui still only supports selection of a single pin, correct? And we don't have multi-select. So what does it look like in the UI control for default values and how do you send in multiple pin values? Do you have to type in the comma-separated strings? And does our validation logic allow that? Maybe adding a couple screenshots with the default values or showing how the user picks a different set of pins would help.

This is no the case after the multiselect feature went in the ASW side

@subash-suresh subash-suresh merged commit 0e103b6 into main Dec 23, 2022
@dixonjoel dixonjoel deleted the users/subash/update-examples branch January 3, 2023 19:04
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.

6 participants