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

Pick up new metadata for mioDAQ configurable digital voltage feature #653

Merged

Conversation

SiuFong-NI
Copy link
Contributor

What does this Pull Request accomplish?

Pickup new DAQmx metadata, updated attributes.py and enums.py.

Why should this Pull Request be merged?

To support configurable digital voltage feature for mioDAQ.

What testing has been done?

Ran poetry run pytest against 2 NI MAX configuration files specified in CONTRIBUTING.md.
image

@SiuFong-NI
Copy link
Contributor Author

My first PR from github, just curious how do we complete a PR here? Do we have an auto-complete option?

Copy link
Collaborator

@zhindes zhindes left a comment

Choose a reason for hiding this comment

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

Let's filter out the Power stuff since its not fully cooked. See Brad/my comments on the metadata PR.

@zhindes
Copy link
Collaborator

zhindes commented Nov 5, 2024

My first PR from github, just curious how do we complete a PR here? Do we have an auto-complete option?

There is, but you might need permission. I see this at the bottom:
image

What do you see?

@SiuFong-NI
Copy link
Contributor Author

Let's filter out the Power stuff since its not fully cooked. See Brad/my comments on the metadata PR.

Updated in latest commit, verified that generated files excluded CalculatedPower related stuff.

My first PR from github, just curious how do we complete a PR here? Do we have an auto-complete option?

There is, but you might need permission. I see this at the bottom: image

What do you see?

image

Just saw the statement mentioned "Merging can be performed automatically once the requested changes are addressed.", so I guess it does but just pending the condition.

@SiuFong-NI SiuFong-NI requested a review from zhindes November 7, 2024 02:32
@SiuFong-NI
Copy link
Contributor Author

My latest commit was to pull latest nidaqmx.proto from my grpc-device PR: ni/grpc-device#1115

@SiuFong-NI
Copy link
Contributor Author

SiuFong-NI commented Nov 8, 2024

Pending grpc-device-scrapigen https://github.com/ni/grpc-device-scrapigen/pull/251 to be merged before merging this PR.

image

Looks like I do not have permission to complete the merge in this repo, @zhindes probably need your help later.

Or probably someone can grant me a write access?

@charitylxy charitylxy merged commit 3151edf into ni:master Nov 8, 2024
18 checks passed
@charitylxy
Copy link
Collaborator

@SiuFong-NI completed the PR for you

@SiuFong-NI
Copy link
Contributor Author

Thanks @charitylxy for the help. Initially I was waiting for the https://github.com/ni/grpc-device-scrapigen/pull/251 to be merged first, I guess this change is independent so probably safe to merge first.

@SiuFong-NI SiuFong-NI deleted the users/siutan/pickupPortLogicFamilyMetadata branch November 8, 2024 02:33
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.

4 participants