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

fix(api): use smaller of max pipette volume and max tip volume for splitting large transfer volumes #17387

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

sanni-t
Copy link
Member

@sanni-t sanni-t commented Jan 30, 2025

Overview

Found a bug in the logic that splits large volume transfers into smaller volumes.
Bug is that we only check the transfer volume against max pipette volume. So if you use a 50uL pipette with a 200uL tip, everything works correctly, but if you use a 1000uL pipette with 50uL tip, it attempts to pipette upto 1000uL, instead of clipping to the max tip volume of 50uL.

This PR fixes that by taking the smaller of pipette volume and tip volume as the max volume a single transfer can take.

Test Plan and Hands on Testing

Unit and integration tests would be enough to catch the error and fix.

Review requests

Check for code sanity

Risk assessment

Low. A scope-limited bug fix

@sanni-t sanni-t requested a review from a team as a code owner January 30, 2025 20:59
@sanni-t sanni-t requested a review from jbleon95 January 30, 2025 20:59
Copy link
Contributor

@jbleon95 jbleon95 left a comment

Choose a reason for hiding this comment

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

Good catch

Copy link

codecov bot commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.43%. Comparing base (0e7b516) to head (69ee437).
Report is 74 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             edge   #17387       +/-   ##
===========================================
+ Coverage   73.84%   92.43%   +18.58%     
===========================================
  Files          43       77       +34     
  Lines        3304     1283     -2021     
===========================================
- Hits         2440     1186     -1254     
+ Misses        864       97      -767     
Flag Coverage Δ
g-code-testing 92.43% <ø> (?)
shared-data ?

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

see 120 files with indirect coverage changes

@sanni-t sanni-t merged commit 253c300 into edge Jan 30, 2025
22 of 26 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.

2 participants