Skip to content

Commit

Permalink
fix(app,api,robot-server): remove old sim flag (#15018)
Browse files Browse the repository at this point in the history
Remove the disableFastProtocolUpload flag.

This flag makes the simulation stack, when simulating older protocols,
use a hardware controller built in simulation mode and a full standard
protocol context and/or engine. This is... not something we've
significantly tested and has a lot of surprising problems, and at this
point it doesn't bring much value. So remove the feature flag.

The old behavior can still be accessed via opentrons_simulate, which
will only use the simulating hardware controller when simulating
pre-engine python and json protocols.

Closes EXEC-360
  • Loading branch information
sfoster1 authored Apr 29, 2024
1 parent a30f0f8 commit 114c9ba
Show file tree
Hide file tree
Showing 16 changed files with 23 additions and 256 deletions.
24 changes: 11 additions & 13 deletions api/src/opentrons/config/advanced_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,19 +159,6 @@ class Setting(NamedTuple):
robot_type=[RobotTypeEnum.OT2],
default_true_on_robot_types=[RobotTypeEnum.FLEX],
),
SettingDefinition(
_id="disableFastProtocolUpload",
title="Use older protocol analysis method",
description=(
"Use an older, slower method of analyzing uploaded protocols. "
"This changes how the OT-2 validates your protocol during the upload "
"step, but does not affect how your protocol actually runs. "
"Opentrons Support might ask you to change this setting if you encounter "
"problems with the newer, faster protocol analysis method."
),
restart_required=False,
robot_type=[RobotTypeEnum.OT2, RobotTypeEnum.FLEX],
),
SettingDefinition(
_id="enableOT3HardwareController",
title="Enable experimental OT-3 hardware controller",
Expand Down Expand Up @@ -729,6 +716,16 @@ def _migrate32to33(previous: SettingsMap) -> SettingsMap:
return newmap


def _migrate33to34(previous: SettingsMap) -> SettingsMap:
"""Migrate to version 34 of the feature flags file.
- Removes disableFastProtocolUpload
"""
removals = ["disableFastProtocolUpload"]
newmap = {k: v for k, v in previous.items() if k not in removals}
return newmap


_MIGRATIONS = [
_migrate0to1,
_migrate1to2,
Expand Down Expand Up @@ -763,6 +760,7 @@ def _migrate32to33(previous: SettingsMap) -> SettingsMap:
_migrate30to31,
_migrate31to32,
_migrate32to33,
_migrate33to34,
]
"""
List of all migrations to apply, indexed by (version - 1). See _migrate below
Expand Down
6 changes: 0 additions & 6 deletions api/src/opentrons/config/feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,6 @@ def enable_door_safety_switch(robot_type: RobotTypeEnum) -> bool:
return advs.get_setting_with_env_overload("enableDoorSafetySwitch", robot_type)


def disable_fast_protocol_upload() -> bool:
return advs.get_setting_with_env_overload(
"disableFastProtocolUpload", RobotTypeEnum.FLEX
)


def enable_ot3_hardware_controller() -> bool:
"""Get whether to use the OT-3 hardware controller."""

Expand Down
4 changes: 1 addition & 3 deletions api/src/opentrons/protocol_api/create_protocol_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

from opentrons_shared_data.labware.dev_types import LabwareDefinition

from opentrons.config import feature_flags
from opentrons.hardware_control import (
HardwareControlAPI,
ThreadManager,
Expand Down Expand Up @@ -123,8 +122,7 @@ def create_protocol_context(
sync_hardware=sync_hardware,
)

# TODO(mc, 2022-8-22): remove `disable_fast_protocol_upload`
elif use_simulating_core and not feature_flags.disable_fast_protocol_upload():
elif use_simulating_core:
legacy_deck = LegacyDeck(deck_type=deck_type)
core = LegacyProtocolCoreSimulator(
sync_hardware=sync_hardware,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"""Simulating AbstractRunner factory."""

from opentrons.config import feature_flags
from opentrons.hardware_control import API as OT2API, HardwareControlAPI
from opentrons.protocols.api_support import deck_type
from opentrons.protocols.api_support.deck_type import should_load_fixed_trash
Expand Down Expand Up @@ -58,7 +57,7 @@ async def create_simulating_runner(
use_virtual_modules=True,
use_virtual_gripper=True,
use_simulated_deck_config=True,
use_virtual_pipettes=(not feature_flags.disable_fast_protocol_upload()),
use_virtual_pipettes=True,
),
load_fixed_trash=should_load_fixed_trash(protocol_config),
)
Expand Down
13 changes: 10 additions & 3 deletions api/tests/opentrons/config/test_advanced_settings_migration.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

@pytest.fixture
def migrated_file_version() -> int:
return 33
return 34


# make sure to set a boolean value in default_file_settings only if
Expand All @@ -22,7 +22,6 @@ def default_file_settings() -> Dict[str, Any]:
"useOldAspirationFunctions": None,
"disableLogAggregation": None,
"enableDoorSafetySwitch": None,
"disableFastProtocolUpload": None,
"enableOT3HardwareController": None,
"rearPanelIntegration": True,
"disableStallDetection": None,
Expand Down Expand Up @@ -405,6 +404,14 @@ def v33_config(v32_config: Dict[str, Any]) -> Dict[str, Any]:
return r


@pytest.fixture
def v34_config(v33_config: Dict[str, Any]) -> Dict[str, Any]:
r = v33_config.copy()
r.pop("disableFastProtocolUpload")
r["_version"] = 34
return r


@pytest.fixture(
scope="session",
params=[
Expand Down Expand Up @@ -443,6 +450,7 @@ def v33_config(v32_config: Dict[str, Any]) -> Dict[str, Any]:
lazy_fixture("v31_config"),
lazy_fixture("v32_config"),
lazy_fixture("v33_config"),
lazy_fixture("v34_config"),
],
)
def old_settings(request: SubRequest) -> Dict[str, Any]:
Expand Down Expand Up @@ -527,7 +535,6 @@ def test_ensures_config() -> None:
"useOldAspirationFunctions": None,
"disableLogAggregation": True,
"enableDoorSafetySwitch": None,
"disableFastProtocolUpload": None,
"enableOT3HardwareController": None,
"rearPanelIntegration": None,
"disableStallDetection": None,
Expand Down
1 change: 0 additions & 1 deletion app/src/assets/localization/en/anonymous.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
"update_requires_restarting_app": "Updating requires restarting the app.",
"update_robot_software_description": "Bypass the auto-update process and update the robot software manually.",
"update_robot_software_link": "Launch software update page",
"use_older_protocol_analysis_method_description": "Use an older, slower method of analyzing uploaded protocols. This changes how the OT-2 validates your protocol during the upload step, but does not affect how your protocol actually runs. Support might ask you to change this setting if you encounter problems with the newer, faster protocol analysis method.",
"versions_sync": "Learn more about keeping the app and robot software in sync",
"want_to_help_out": "Want to help out?",
"welcome_title": "Welcome!",
Expand Down
1 change: 0 additions & 1 deletion app/src/assets/localization/en/branded.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
"update_requires_restarting_app": "Updating requires restarting the Opentrons App.",
"update_robot_software_description": "Bypass the Opentrons App auto-update process and update the robot software manually.",
"update_robot_software_link": "Launch Opentrons software update page",
"use_older_protocol_analysis_method_description": "Use an older, slower method of analyzing uploaded protocols. This changes how the OT-2 validates your protocol during the upload step, but does not affect how your protocol actually runs. Opentrons Support might ask you to change this setting if you encounter problems with the newer, faster protocol analysis method.",
"versions_sync": "Learn more about keeping the Opentrons App and robot software in sync",
"want_to_help_out": "Want to help out Opentrons?",
"welcome_title": "Welcome to your Opentrons Flex!",
Expand Down
1 change: 0 additions & 1 deletion app/src/assets/localization/en/device_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@
"usb_to_ethernet_description": "Looking for USB-to-Ethernet Adapter info?",
"use_older_aspirate": "Use older aspirate behavior",
"use_older_aspirate_description": "Aspirate with the less accurate volumetric calibrations that were used before version 3.7.0. Use this if you need consistency with pre-v3.7.0 results. This only affects GEN1 P10S, P10M, P50M, and P300S pipettes.",
"use_older_protocol_analysis_method": "Use older protocol analysis method",
"validating_software": "Validating software...",
"view_details": "View details",
"view_latest_release_notes_at": "View latest release notes at {{url}}",
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,3 @@ export * from './Troubleshooting'
export * from './UpdateRobotSoftware'
export * from './UsageSettings'
export * from './UseOlderAspirateBehavior'
export * from './UseOlderProtocol'
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {
UpdateRobotSoftware,
UsageSettings,
UseOlderAspirateBehavior,
UseOlderProtocol,
} from './AdvancedTab'
import {
updateSetting,
Expand Down Expand Up @@ -232,12 +231,6 @@ export function RobotSettingsAdvanced({
/>
{isFlex ? null : (
<>
<Divider marginY={SPACING.spacing16} />
<UseOlderProtocol
settings={findSettings('disableFastProtocolUpload')}
robotName={robotName}
isRobotBusy={isRobotBusy || isEstopNotDisengaged}
/>
<Divider marginY={SPACING.spacing16} />
<LegacySettings
settings={findSettings('deckCalibrationDots')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
UpdateRobotSoftware,
UsageSettings,
UseOlderAspirateBehavior,
UseOlderProtocol,
} from '../AdvancedTab'
import { RobotSettingsAdvanced } from '../RobotSettingsAdvanced'

Expand Down Expand Up @@ -53,7 +52,6 @@ vi.mock('../AdvancedTab/Troubleshooting')
vi.mock('../AdvancedTab/UpdateRobotSoftware')
vi.mock('../AdvancedTab/UsageSettings')
vi.mock('../AdvancedTab/UseOlderAspirateBehavior')
vi.mock('../AdvancedTab/UseOlderProtocol')

const mockUpdateRobotStatus = vi.fn()

Expand Down Expand Up @@ -110,9 +108,6 @@ describe('RobotSettings Advanced tab', () => {
vi.mocked(UseOlderAspirateBehavior).mockReturnValue(
<div>Mock UseOlderAspirateBehavior Section</div>
)
vi.mocked(UseOlderProtocol).mockReturnValue(
<div>Mock UseOlderProtocol Section</div>
)
when(useIsFlex).calledWith('otie').thenReturn(false)
vi.mocked(EnableStatusLight).mockReturnValue(
<div>mock EnableStatusLight</div>
Expand Down Expand Up @@ -210,17 +205,6 @@ describe('RobotSettings Advanced tab', () => {
).toBeNull()
})

it('should render UseOlderProtocol section for OT-2', () => {
render()
screen.getByText('Mock UseOlderProtocol Section')
})

it('should not render UseOlderProtocol section for Flex', () => {
when(useIsFlex).calledWith('otie').thenReturn(true)
render()
expect(screen.queryByText('Mock UseOlderProtocol Section')).toBeNull()
})

it('should not render EnableStatusLight section for OT-2', () => {
render()
expect(screen.queryByText('mock EnableStatusLight')).not.toBeInTheDocument()
Expand Down
15 changes: 0 additions & 15 deletions robot-server/tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,6 @@ def pytest_tavern_beta_after_every_response(
print(json.dumps(response.json(), indent=4))


@pytest.fixture
def ot2_server_set_disable_fast_analysis(
ot2_server_base_url: str,
) -> Generator[None, None, None]:
"""For integration tests that need to set then clear the
disableFastProtocolUpload feature flag"""
url = f"{ot2_server_base_url}/settings"
data = {"id": "disableFastProtocolUpload", "value": True}
with _requests_session() as requests_session:
requests_session.post(url, json=data)
yield None
data["value"] = None
requests.post(url, json=data)


@pytest.fixture
def ot2_server_base_url(_ot2_session_server: str) -> Generator[str, None, None]:
"""Return the URL for a running dev server.
Expand Down
Loading

0 comments on commit 114c9ba

Please sign in to comment.