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

Open
wants to merge 62 commits into
base: edge
Choose a base branch
from

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 and others added 30 commits November 3, 2024 19:57
@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
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