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): correct logic for simming partial tips #13998

Merged
merged 1 commit into from
Nov 16, 2023
Merged

Conversation

sfoster1
Copy link
Member

When we simulate partial tip pickup we do it by emulating the hardware control logic in the virtual pipette data provider and then passing the result to a nozzle manager the way the hardware controller does. Unfortunately, the logic in the pipette virtual data handler did not match the hardware controller.

Until we get to a place where we can have an actual simualting hardware controller or something, we just have to keep these in sync.

Testing

This is an analysis-only change, and has no effect on execution. The best way to test it is to test #13972 since that's where I noticed the issue (we'll rebase that PR on this).

When we simulate partial tip pickup we do it by emulating the hardware
control logic in the virtual pipette data provider and then passing the
result to a nozzle manager the way the hardware controller does.
Unfortunately, the logic in the pipette virtual data handler did not
match the hardware controller.

Until we get to a place where we can have an actual simualting hardware
controller or something, we just have to keep these in sync.
@sfoster1 sfoster1 requested a review from a team as a code owner November 16, 2023 16:59
@sfoster1 sfoster1 requested review from Laura-Danielle and a team November 16, 2023 16:59
Copy link

codecov bot commented Nov 16, 2023

Codecov Report

Merging #13998 (d384f6b) into edge (892b0e7) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #13998      +/-   ##
==========================================
- Coverage   70.66%   70.66%   -0.01%     
==========================================
  Files        2505     2505              
  Lines       70869    70869              
  Branches     8751     8751              
==========================================
- Hits        50080    50078       -2     
- Misses      18648    18649       +1     
- Partials     2141     2142       +1     
Flag Coverage Δ
app 67.72% <ø> (-0.01%) ⬇️
g-code-testing 96.44% <ø> (ø)

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

see 1 file with indirect coverage changes

Copy link
Contributor

@Laura-Danielle Laura-Danielle left a comment

Choose a reason for hiding this comment

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

lgtm

@sfoster1 sfoster1 merged commit 3b59962 into edge Nov 16, 2023
@sfoster1 sfoster1 deleted the fix-partial-pickup-sim branch November 16, 2023 17:42
@sanni-t
Copy link
Member

sanni-t commented Nov 16, 2023

 Oh awesome! This was giving me trouble in deck conflict check testing too. Thanks!

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.

3 participants