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): RobotContext: Add pipette helper functions to convert volume and position type #16682

Merged
merged 41 commits into from
Nov 20, 2024

Conversation

Laura-Danielle
Copy link
Contributor

Overview

For the robot_context, it is important to be able to convert volume and named positions to an AxisMap so that it can be passed in to the robot_context move functions.

Test Plan and Hands on Testing

Tested on a flex to verify that the pipette moves to the correct locations.

Changelog

  • Expose plunger position and aspirate/dispense commands.
  • add the pipette conversion functions
  • expose plunger positions in the pipette dict

Review requests

Are we OK with some of the conversions living in the robot core?

Risk assessment

Low, unreleased API.

@Laura-Danielle Laura-Danielle requested a review from a team as a code owner November 4, 2024 19:50
@Laura-Danielle Laura-Danielle requested review from a team and removed request for a team November 4, 2024 19:51
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.

I see what we're getting at, but I think we should do it a little differently. Specifically,

  • We should not do this in the core. Doing this in the core makes it impossible to execute commands of this type outside python protocols; it also pushes logic, or in this case duplicates logic, pretty high up. Instead, what about making the engine commands that you would give the positions to take either volume or distance, and convert internally? And for the conversions, let's make the actual math and the actual detail lookup part of an engine state handler that the api drills into, so that the logic and the lookup doesn't live in the core files.
  • let's not duplicate things like ul per mm. They're data agnostic so let's make them shared utility functions.
  • if we're going to push stuff into the engine, let's push it all the way. we definitely should not have new things in core/engine that reach into the hardware controller directly - let's get that stuff from the engine.

@Laura-Danielle Laura-Danielle requested a review from a team as a code owner November 7, 2024 21:52
@Laura-Danielle Laura-Danielle force-pushed the PLAT-351-move-axes-to-position branch 4 times, most recently from 5271362 to 1fb5978 Compare November 8, 2024 22:00
Base automatically changed from PLAT-351-move-axes-to-position to edge November 8, 2024 22:11
@Laura-Danielle Laura-Danielle force-pushed the PLAT-453-PLAT-454-add-pipette-support-functions branch from 070d998 to 8011190 Compare November 9, 2024 22:15
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, nice!

@sfoster1 sfoster1 changed the title feat(robot_context): Add pipette helper functions to convert volume and position type feat(api): RobotContext: Add pipette helper functions to convert volume and position type Nov 13, 2024
@Laura-Danielle Laura-Danielle force-pushed the PLAT-453-PLAT-454-add-pipette-support-functions branch from d3ab5bf to 66842cb Compare November 20, 2024 13:30
@Laura-Danielle Laura-Danielle force-pushed the PLAT-453-PLAT-454-add-pipette-support-functions branch from 66842cb to 28e9328 Compare November 20, 2024 16:28
@Laura-Danielle Laura-Danielle merged commit db8b1e5 into edge Nov 20, 2024
75 checks passed
@Laura-Danielle Laura-Danielle deleted the PLAT-453-PLAT-454-add-pipette-support-functions branch November 20, 2024 17:10
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