Skip to content

fix(api): try to open thermocycler lid before throwing an error#18255

Merged
caila-marashaj merged 4 commits intoedgefrom
EXEC-1435-try-home-tc-lid-b4-error
May 5, 2025
Merged

fix(api): try to open thermocycler lid before throwing an error#18255
caila-marashaj merged 4 commits intoedgefrom
EXEC-1435-try-home-tc-lid-b4-error

Conversation

@caila-marashaj
Copy link
Copy Markdown
Contributor

@caila-marashaj caila-marashaj commented May 5, 2025

Overview

Sometimes, in response to a lack of error-handling from the thermocyler firmware, and rarely from reasons yet unknown, it's possible for the thermocycler to lose track of its lid's position status. Most of the time this happens, the lid is already all the way open, and just barely backed off the limit switch. However, because it's backed off the limit switch, the robot server doesn't know the lid's position and throws a ThermocyclerLidNotOpenError. It's possible to fix this issue most if not all of the time by attempting to open the lid until we find the lid-open limit switch.

This code changes the robot-server protocol for this to check if the thermocycler lid is open. If the position is IN_BETWEEN or UNKNOWN, try to open the lid and check the status again. If the status at this point is still anything besides OPEN, throw a ThermocyclerLidNotOpenError.

Changelog

  • make ThermocyclerMovementFlagger take in an EquipmentHandler, so it can command the tc lid to move
  • change ThermocyclerMovementFlagger::raise_if_labware_in_non_open_thermocycler to ThermocyclerMovementFlagger::ensure_labware_in_open_thermocycler, and have it try to open the tc lid before throwing an error

Also closes RABR-736

@caila-marashaj caila-marashaj requested a review from a team as a code owner May 5, 2025 16:02
Copy link
Copy Markdown
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Makes sense!

Changes requested for:

  1. What I think is a logic bug, but double-check my thinking.
  2. Optional arguments muddying the waters of where these helpers and their dependencies ought to be constructed

Comment thread api/src/opentrons/protocol_engine/execution/labware_movement.py
Comment thread api/src/opentrons/protocol_engine/execution/thermocycler_movement_flagger.py Outdated
Comment thread api/src/opentrons/protocol_engine/execution/thermocycler_movement_flagger.py Outdated
Comment thread api/src/opentrons/protocol_engine/execution/thermocycler_movement_flagger.py Outdated
Comment thread api/src/opentrons/protocol_engine/execution/movement.py Outdated
@caila-marashaj caila-marashaj force-pushed the EXEC-1435-try-home-tc-lid-b4-error branch from 5d247a2 to 55fa1d1 Compare May 5, 2025 21:55
Copy link
Copy Markdown
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Thanks!

@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 23.62%. Comparing base (bfefd85) to head (55fa1d1).
Report is 3 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #18255      +/-   ##
==========================================
- Coverage   23.62%   23.62%   -0.01%     
==========================================
  Files        3055     3055              
  Lines      255734   255734              
  Branches    30114    30120       +6     
==========================================
- Hits        60419    60410       -9     
- Misses     195301   195310       +9     
  Partials       14       14              
Flag Coverage Δ
app 3.11% <ø> (-0.01%) ⬇️
protocol-designer 19.06% <ø> (ø)
step-generation 4.55% <ø> (-0.01%) ⬇️

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

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@caila-marashaj caila-marashaj merged commit 0a101c9 into edge May 5, 2025
55 checks passed
ddcc4 pushed a commit that referenced this pull request May 17, 2025
ddcc4 pushed a commit that referenced this pull request May 17, 2025
ddcc4 pushed a commit that referenced this pull request May 17, 2025
ddcc4 pushed a commit that referenced this pull request May 17, 2025
ddcc4 pushed a commit that referenced this pull request May 17, 2025
ddcc4 pushed a commit that referenced this pull request May 17, 2025
ddcc4 pushed a commit that referenced this pull request May 19, 2025
ddcc4 pushed a commit that referenced this pull request May 19, 2025
ddcc4 pushed a commit that referenced this pull request May 19, 2025
ddcc4 pushed a commit that referenced this pull request May 20, 2025
ddcc4 pushed a commit that referenced this pull request May 20, 2025
ddcc4 pushed a commit that referenced this pull request May 22, 2025
ddcc4 pushed a commit that referenced this pull request May 23, 2025
ddcc4 pushed a commit that referenced this pull request May 24, 2025
ddcc4 pushed a commit that referenced this pull request May 24, 2025
ddcc4 pushed a commit that referenced this pull request May 29, 2025
ddcc4 pushed a commit that referenced this pull request May 29, 2025
ddcc4 pushed a commit that referenced this pull request May 29, 2025
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