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

feat: Add support for empty mounts in provisioning script #12961

Merged
merged 3 commits into from
Jun 23, 2023

Conversation

DerekMaggio
Copy link
Member

@DerekMaggio DerekMaggio commented Jun 22, 2023

Overview

Add support for empty pipette mounts.

Test Plan

  • Run configuration that has only a left pipette. Make sure that right pipette eeprom size is 0. Make sure left pipette eeprom size is not.

Changelog

  • Add logic to parse empty mount env var
  • Add tests

Review requests

None

Risk assessment

Low

@DerekMaggio DerekMaggio marked this pull request as ready for review June 22, 2023 17:45
@DerekMaggio DerekMaggio requested a review from a team as a code owner June 22, 2023 17:45
@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

Merging #12961 (42e6419) into edge (50efb8a) will increase coverage by 0.00%.
The diff coverage is 93.75%.

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #12961   +/-   ##
=======================================
  Coverage   72.91%   72.91%           
=======================================
  Files        2349     2349           
  Lines       64380    64392   +12     
  Branches     7167     7167           
=======================================
+ Hits        46943    46954   +11     
- Misses      15761    15762    +1     
  Partials     1676     1676           
Flag Coverage Δ
hardware 59.13% <93.75%> (+0.05%) ⬆️
notify-server 89.13% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ns_hardware/scripts/emulation_pipette_provision.py 93.47% <93.75%> (-0.64%) ⬇️

@@ -20,36 +19,81 @@
RIGHT_PIPETTE_ENV_VAR_NAME = "RIGHT_OT3_PIPETTE_DEFINITION"


@dataclass
class OT3PipetteEnvVar:
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't you just keep this a data class with defaults?

Copy link
Contributor

Choose a reason for hiding this comment

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

or keep it a dataclass and use a build classmethod

Copy link
Member Author

Choose a reason for hiding this comment

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

Stop making so much sense. I have no idea why I did it that way. I will go back and fix it.

If I remember the reason I did it that way, and the reason makes sense, I will drop a comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

I remembered my reasoning for doing it that way.
My reasoning was not good.

Here is the fix
262d21e

Copy link
Contributor

@fsinapi fsinapi left a comment

Choose a reason for hiding this comment

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

LGTM!

@DerekMaggio DerekMaggio merged commit f87e3a4 into edge Jun 23, 2023
16 checks passed
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.

None yet

3 participants