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

Added NI TestStand code to example measurements using NI gRPC Device remoting #151

Merged
merged 8 commits into from
Dec 8, 2022

Conversation

tkrebes
Copy link
Contributor

@tkrebes tkrebes commented Dec 7, 2022

What does this Pull Request accomplish?

Makes instrument type ids more discoverable

  • Adds constants for instrument_type_id to session_management module
  • Updates examples to use INSTRUMENT_TYPE_ constants

Adds NI TestStand code to example measurements that use NI gRPC Device remoting

  • Copies _helpers.py, teststand_fixture.py, and NIDCPowerSourceDCVoltage.seq from NI-DCPower example to other examples
  • Updates teststand_fixture.py and sequence files to work with respective examples

Why should this Pull Request be merged?

This provides customers with more NI TestStand examples.

What testing has been done?

Checked that examples ran successfully in NI TestStand.

@tkrebes tkrebes marked this pull request as ready for review December 7, 2022 21:07
@bkeryan
Copy link
Collaborator

bkeryan commented Dec 7, 2022

I like how TestStand includes graphs in the report:
image

Copy link
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.

Looks good to me. I would like to get this committed and build a new release for this week's drop, and then worry about the remaining issues later.

@dixonjoel
Copy link
Collaborator

dixonjoel commented Dec 8, 2022

Looks good to me. I would like to get this committed and build a new release for this week's drop, and then worry about the remaining issues later.

Sure. I'll defer to Brad's review.

@tkrebes @bkeryan we had some failures on the checks if y'all hadn't noticed that yet.

@bkeryan
Copy link
Collaborator

bkeryan commented Dec 8, 2022

Looks good to me. I would like to get this committed and build a new release for this week's drop, and then worry about the remaining issues later.

Sure. I'll defer to Brad's review.

@tkrebes @bkeryan we had some failures on the checks if y'all hadn't noticed that yet.

This is the same failure I got with #155

  Command ['/Users/runner/work/measurement-services-python/measurement-services-python/.venv/bin/python', '-m', 'pip', 'install', '--disable-pip-version-check', '--prefix', '/Users/runner/work/measurement-services-python/measurement-services-python/.venv', '--no-deps', '/Users/runner/Library/Caches/pypoetry/artifacts/73/3c/45/33aec55a6ea8da23b87c418148e7bb9063754af7a548bbd4a4a5529733/black-22.10.0-1fixedarch-cp39-cp39-macosx_11_0_x86_64.whl'] errored with the following return code 1, and output: 
  ERROR: black-22.10.0-1fixedarch-cp39-cp39-macosx_11_0_x86_64.whl is not a supported wheel on this platform.

When black 22.10.0 was released, they accidentally uploaded amd64 packages as x86_64 packages. This was fixed, but this GitHub runner still has the wrong version cached in its local Poetry cache.

I can see two ways to get past this:

  • Change ni-python-styleguide to blacklist black 22.10.0 as @mshafer-NI suggested on the other PR.
  • Add poetry cache clear pypi:black:22.10.0 to our GitHub actions?

@bkeryan
Copy link
Collaborator

bkeryan commented Dec 8, 2022

Looks good to me. I would like to get this committed and build a new release for this week's drop, and then worry about the remaining issues later.

Sure. I'll defer to Brad's review.
@tkrebes @bkeryan we had some failures on the checks if y'all hadn't noticed that yet.

This is the same failure I got with #155

  Command ['/Users/runner/work/measurement-services-python/measurement-services-python/.venv/bin/python', '-m', 'pip', 'install', '--disable-pip-version-check', '--prefix', '/Users/runner/work/measurement-services-python/measurement-services-python/.venv', '--no-deps', '/Users/runner/Library/Caches/pypoetry/artifacts/73/3c/45/33aec55a6ea8da23b87c418148e7bb9063754af7a548bbd4a4a5529733/black-22.10.0-1fixedarch-cp39-cp39-macosx_11_0_x86_64.whl'] errored with the following return code 1, and output: 
  ERROR: black-22.10.0-1fixedarch-cp39-cp39-macosx_11_0_x86_64.whl is not a supported wheel on this platform.

When black 22.10.0 was released, they accidentally uploaded amd64 packages as x86_64 packages. This was fixed, but this GitHub runner still has the wrong version cached in its local Poetry cache.

I can see two ways to get past this:

  • Change ni-python-styleguide to blacklist black 22.10.0 as @mshafer-NI suggested on the other PR.
  • Add poetry cache clear pypi:black:22.10.0 to our GitHub actions?

I posted a PR to update ni-python-styleguide: ni/python-styleguide#95

We also need to update ni-python-styleguide to drop Python 3.6 before any PR builds will succeed, and @mshafer-NI is doing that: ni/python-styleguide#96

@tkrebes tkrebes force-pushed the users/tkrebes/teststand-examples branch from 19ee95d to 6d0881c Compare December 8, 2022 19:44
@tkrebes tkrebes merged commit 5c67f12 into main Dec 8, 2022
@dixonjoel dixonjoel deleted the users/tkrebes/teststand-examples branch December 9, 2022 14:02
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