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,app): Allow blowout and droptip when unhomed #15816

Merged
merged 8 commits into from
Jul 30, 2024

Conversation

sfoster1
Copy link
Member

@sfoster1 sfoster1 commented Jul 26, 2024

We have this flow called the drop tip wizard, which happens after a protocol fails and there are still tips. It offers the user a way to get rid of the liquid stuck in the tips and the tips stuck on the pipette, which is very important on the well-sealed Flex pipette and the 96 channel.

However, if the protocol failed in the middle of a plunger motion, you can't actually do any of that because the plunger stopped abruptly and lost its position estimation.

The nice thing is that we have encoders! So let's add some new code that lets us use those to do things like blowout and drop tip even if we don't know the position of the plunger.

It's important not to just generally expose the ability to update from the encoders, because critically the encoders have a coarser position resolution than our internal position accumulators. Updating from the encoder position will introduce a small offset into the position of the axis, and that could be a real problem if it's being used for scientific tasks. So while we're adding a hardware control method to reset the position estimators of axes, we're not adding a command.

That's not a problem for quick commands that are going to fix up the state of the system, so we can make new commands in an unsafe command domain (just made up terminology, but it's the stuff before the slash) to indicate that they're not generally useful that will refresh the position before doing whatever it is.

Also, these will work generally on a flex, so we can just unconditionally use them if it's a flex in DTWiz and not mess around with dealing with partial tip configurations or whatever.

Unfortunately we also move to positions. We could make an unsafe equivalent of every movement command, but that seems like much especially since the thing they're doing lasts beyond their call. Instead, let's add unsafe/updatePositionEstimators, which we can call once at the beginning of DTWiz. In the future, we should add software tracking of the position estimators, and fail safe commands if the position comes from the encoders while letting the unsafe commands through.

Testing

  • Can you actually use this stuff in the DTwiz after a failed aspirate
    • on an LT pipette
    • on a 96channel

Closes EXEC-401

Copy link
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.

Sounds good. I'll take a closer look on Monday, but here are some questions before I forget.

pipette_location = self._state_view.motion.get_pipette_location(
params.pipetteId
)
await ot3_hardware_api.update_axis_position_estimations(
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. So if you use one of these unsafe commands, does that have the side-effect of poisoning the position estimation until the next plunger home? Is there a way to limit the encoder-sourced estimation to just this command, so if someone tries to follow it up with an aspirate or whatever, the system will error instead of moving wrongly?

Copy link
Member Author

Choose a reason for hiding this comment

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

I could see that being something we want. But,

  • Encoders are a hardware concern today and don't know about "commands". We would have to add some API for like "temporary position setting", or more probably track position estimator in the engine or something
  • When you update the estimator from the encoder, you'll introduce some fixed offset of order 0.1mm thereafter. But then you're using the internal fixed estimator, and so you're accurate with just that offset. But were you to keep doing it, every time you'd add a new 0.1mm offset. So we wouldn't want to actually reset the estimator every time, we'd want some kind of software emulation. That feels like it's getting a bit much.

Copy link
Contributor

@TamarZanzouri TamarZanzouri left a comment

Choose a reason for hiding this comment

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

I like this! and its very clear that its unsafe :-)

@sfoster1
Copy link
Member Author

To the point of that unsafe/updatePositionEstimators thing. I really don't want to do it but I'm realizing that ultimately, there needs to be a version of this for each command that might be the first one the DTWiz uses to touch a given axis.

Base automatically changed from exec-613-command-schema-9 to edge July 29, 2024 17:27
sfoster1 added 8 commits July 29, 2024 16:35
We have encoders, at least on the flex; we now need to use them in
actually reasonably public APIs.

Add the minimal API to take advantage of the encoders with a wrapper
around the already-existing backend update_position_estimators function
that takes a list of axes to update the estimators for. This will cause
these axes to update their open-loop estimators from the encoders. We
don't want to do this frequently because the encoders have a coarser
resolution than the internal position accumulators, so this will be a
net accuracy loss; these axes should be homed before being used for
something important. However, the encoder is plenty accurate for blowing
out or dropping tips to make the pipette safe to home, and that's what
it's for.

Also slightly refactor the hardware protocols so this and the other
encoder stuff is in its own protocol facet.
Adds a new command that is capable (on Flex) of blowing out a pipette
even when the plunger position is not known, i.e. after motion suddenly
stopped during a plunger move.

This is part of a new command domain called unsafe/ that will contain
commands that are generally not good ideas to run during protocols for
one reason or another. The reason this one's in there is that the
encoder doesn't have the resolution of the plunger's internal position
accumulator, so after calling this there will be some static position
offset until the plunger is homed. You wouldn't want to do this during a
protocol.

Commands in the unsafe/ domain will not ever get Python API bindings;
they should probably not be used by anything but clients handling error cases.
This is a special command that (on Flex) will make the machine reload
its plunger position from its encoder and then drop tip. This makes this
command always runnable even if there's no position known for the
plunger axis, as there wouldn't be after an error during a liquid
handling command. This lets us call it unconditionally during the drop
tip wizard.
By using the new unsafe/blowOutInPlace and unsafe/dropTipInPlace
commands in the drop tip wizard's blowout and drop tip modals, we can
make sure that it will work on a flex even if the machine was
interrupted while executing a liquid handling command.

Closes EXEC-401
Adds a command that will update all specified axis position estimators from their
encoders. This is something that a client can use when it knows it's
about to be moving multiple axes. As with the other unsafe commands,
it's best to home after doing this before doing something that requires accuracy.
This will let you move around and blowout and drop tips even if the
robot has lost positioning.
When we reload the estimators, we need to update our local cache of
their state.
@sfoster1 sfoster1 force-pushed the exec-401-blow-out-if-plunger-unhomed branch from 3649e56 to a211690 Compare July 29, 2024 20:35
Copy link
Contributor

@mjhuff mjhuff 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 to me!

Copy link
Contributor

@TamarZanzouri TamarZanzouri left a comment

Choose a reason for hiding this comment

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

Still looks great!

Copy link
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, thanks.

If we need the catch-all unsafe/updatePositionEstimators, I'd prefer to make clients compose that with the existing blowOutInPlace and dropTipInPlace commands, instead of retaining the "combination commands" of unsafe/blowOutInPlace and unsafe/dropTipInPlace. Basically to reduce ongoing maintenance.

@sfoster1 sfoster1 merged commit 2383bf8 into edge Jul 30, 2024
67 checks passed
@sfoster1
Copy link
Member Author

sfoster1 commented Jul 30, 2024

Makes sense, thanks.

If we need the catch-all unsafe/updatePositionEstimators, I'd prefer to make clients compose that with the existing blowOutInPlace and dropTipInPlace commands, instead of retaining the "combination commands" of unsafe/blowOutInPlace and unsafe/dropTipInPlace. Basically to reduce ongoing maintenance.

I think that those commands have a future if we implement a nicer software system for managing position estimator state, something like

  • The engine, probably, has a cache of "is position state ok or unknown or from-encoder"
  • commands outside of unsafe will raise MustHomeError whenever they interact with a position state other than ok
  • commands inside of unsafe will raise MustHomeError whenever they interact with a position that is unknown only

That lets them keep a place that is "you use this when things are messed up"

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.

5 participants