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(api): add retract_axis to OT-2 and FLEX hardware-control api #13001

Merged
merged 6 commits into from
Jul 7, 2023

Conversation

ahiuchingau
Copy link
Contributor

@ahiuchingau ahiuchingau commented Jun 27, 2023

Addresses hardware-control side of RQA-997
Protocol engine & app changes will follow in #13006

Overview

Adds per-axis retraction feature to hardware controller API for OT2 & Flex

Test Plan

Test on OT2 & Flex and make sure the axes retract correctly. Also make sure mount retraction remains unchanged.

Changelog

  • added retract_axis HC api methods and updated mount retraction to delegate to the new method

Review requests

Risk assessment

Low

@codecov
Copy link

codecov bot commented Jun 27, 2023

Codecov Report

Merging #13001 (1f77633) into edge (9d0c60f) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #13001      +/-   ##
==========================================
  Coverage   72.28%   72.28%              
==========================================
  Files        2373     1552     -821     
  Lines       65064    50630   -14434     
  Branches     7289     3256    -4033     
==========================================
- Hits        47032    36600   -10432     
+ Misses      16328    13548    -2780     
+ Partials     1704      482    -1222     
Flag Coverage Δ
app 43.69% <ø> (-27.58%) ⬇️
components 64.42% <ø> (ø)
g-code-testing 96.44% <ø> (ø)
labware-library 49.61% <ø> (ø)
notify-server 89.13% <ø> (ø)
protocol-designer 44.06% <ø> (ø)
step-generation 84.28% <ø> (ø)

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

Impacted Files Coverage Δ
api/src/opentrons/hardware_control/api.py 82.47% <ø> (-0.04%) ⬇️
api/src/opentrons/hardware_control/ot3api.py 80.15% <ø> (ø)

... and 823 files with indirect coverage changes

Copy link
Member

@sfoster1 sfoster1 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, typo tho

@@ -1291,8 +1291,33 @@ async def retract(

Works regardless of critical point or home status.
"""
machine_ax = OT3Axis.by_mount(mount)
await self._home((machine_ax,))
await self.restract_axis(OT3Axis.by_mount(mount))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
await self.restract_axis(OT3Axis.by_mount(mount))
await self.retract_axis(OT3Axis.by_mount(mount))

Copy link
Member

@sanni-t sanni-t left a comment

Choose a reason for hiding this comment

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

Tested this along with #13006 (without app changes) on an OT3 and confirmed that axis retraction works. The only issue observed was that motion planning doesn't seem to know that the axis is already at its highest and doesn't need to move to a different point to clear all labware on deck. Pretty sure that's on the engine so will confirm and get back to you so we can merge this PR in.

@sanni-t
Copy link
Member

sanni-t commented Jul 6, 2023

The only issue observed was that motion planning doesn't seem to know that the axis is already at its highest and doesn't need to move to a different point to clear all labware on deck. Pretty sure that's on the engine so will confirm and get back to you so we can merge this PR in.

Alright, verified that fixing this issue required only a protocol engine change.
So @andySigler or @ahiuchingau you can undraft this PR and merge in edge. I'll rebase my branch on top of this one.

@andySigler andySigler marked this pull request as ready for review July 6, 2023 18:56
@andySigler andySigler requested a review from a team as a code owner July 6, 2023 18:56
@andySigler andySigler merged commit 434168a into edge Jul 7, 2023
@andySigler andySigler deleted the api_add-retract-axis-func branch July 7, 2023 13:23
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