fix(api): Make labware loads and moves slightly more atomic

[SyntaxColoring]

Overview

This tries to address one of the errors in RESC-227.

Suppose you tried to catch a labware load exception, like this:

hs = protocol.load_module("heaterShakerModuleV1",1)
try:
    # Error: conflicts with the adjacent Heater-Shaker.
    lw = protocol.load_labware("opentrons_24_tuberack_eppendorf_1.5ml_safelock_snapcap", 2)
except Exception:
    # ...

When you catch the exception, the system is currently left in an inconsistent state. According to the Protocol Engine layer, the labware load succeeded. But according to the PAPI layer, the labware load failed. This is a longstanding known problem that has caused user-facing confusion before. In RESC-227, a customer tried to clear the slot by doing del protocol.deck[slot], but the inconsistent state caused that to raise an internal KeyError.

Changelog

In ProtocolContext.load_labware(), ProtocolContext.load_adapter(), and ProtocolContext.move_labware(): if we detect a conflict, try to keep the two layers in sync by "undoing" the load in Protocol Engine, by running a Protocol Engine moveLabware offDeck command.

This is not a real solution because:

• It does not work if the conflict happens in ProtocolContext.load_module() (because we can't unload modules).
• The failed labware will still show up in the run and protocol analysis, which means it will still show up in the Opentrons App's setup steps.
• The offDeck labware move will show up in the run log.

Review requests

Is this worth the complexity? Instead of doing this, should we push towards solving this the right way, as described in the # todo comment?

opentrons/api/src/opentrons/protocol_api/core/engine/protocol.py

Lines 244 to 257 in 8cfa337

	# We should do some combination of these:
	#
	# 1. Make the conflict checks fine-grained and geometry-based (what will
	# *actually* collide), instead of coarse-grained and heuristic-based
	# (like "no tall labware next to a Heater-Shaker unless it's one of
	# these specific definitions"). That way, if the checks start applying
	# to JSON and HTTP protocols, they wouldn't be overconservative and take
	# meaningful flexibility away from those platforms.
	#
	# 2. Make the conflict checks warnings instead of errors, for the same
	# reason.
	#
	# 3. Move the checks from PAPI down into ProtocolEngine so they can happen
	# atomically with the load.

Test Plan

To do.

Risk assessment

To do.

[sfoster1]

I don't think this version of it is a good idea. I think you still end up in a generally incorrect state - the labware is loaded but OFF_DECK - and I think you end up with an even more incorrect run log that could confuse clients that rely only on the engine output for protocol state, as we encourage clients to do.

I think the Right Solution to the problem of "we want to enforce certain rules in the python api that we do not want to enforce in the engine" is enforce those rules before involving the engine, and only involve the engine if the enforcement passes. We could do that in two ways:

1. We could reimplement the checks to take (engine state, proposed change) instead of just (engine state with the change already made). I think this is more fundamentally sound, because the question of "is this state that exists valid" is a really awkward one because it allows the concept that invalid states can exist. Instead, we should be arranging our code so that a) as many invalid states as possible have invalidity encoded into language semantics (i.e. , "it's invalid for a value to be True and False" is something we encode into language semantics by saying "this value is a bool") and b) the rest of them are checked before entering the state.
2. Make a new engine with a copy of the state and an analysis backend, supply the action to that engine, and run the check on that engine's state. This sounds like a joke but like, the whole point of the engine is that it's transient, that it can run with and without hardware, that at the end of the day it's just data, and data can be copied.

I'd prefer 1, though.

[sanni-t]

Agree with @sfoster1. Also prefer option 1.
It also does look possible to do the deck conflict check without loading the labware into the engine first.