Merge branch 'edge' into RQA-2591-engage-axis-before-move

.github/workflows/components-test-build-deploy.yaml

@@ -174,10 +174,13 @@ jobs:
         with:
           node-version: '18.19.0'
           registry-url: 'https://registry.npmjs.org'
+      - name: 'install udev for usb-detection'
+        run: sudo apt-get update && sudo apt-get install libudev-dev
       - name: 'setup-js'
         run: |
           npm config set cache ./.npm-cache
           yarn config set cache-folder ./.yarn-cache
+          make setup-js
       - name: 'build typescript'
         run: make build-ts
       - name: 'build library'

.github/workflows/opentrons-ai-client-test-build-deploy.yaml

@@ -0,0 +1,78 @@
+# Run tests, build the app, and deploy it cross platform
+
+name: 'OpentronsAI client test, build, and deploy'
+
+# ToDo (kk:04/16/2024) Add build and deploy task
+
+on:
+  push:
+    paths:
+      - 'Makefile'
+      - 'opentrons-ai-client/**/*'
+      - 'components/**/*'
+      - '*.js'
+      - '*.json'
+      - 'yarn.lock'
+      - '.github/workflows/app-test-build-deploy.yaml'
+      - '.github/workflows/utils.js'
+    branches:
+      - '**'
+    tags:
+      - 'v*'
+      - 'ot3@*'
+  pull_request:
+    paths:
+      - 'Makefile'
+      - 'opentrons-ai-client/**/*'
+      - 'components/**/*'
+      - '*.js'
+      - '*.json'
+      - 'yarn.lock'
+  workflow_dispatch:
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}-${{ github.ref_name != 'edge' || github.run_id}}-${{ github.ref_type != 'tag' || github.run_id }}
+  cancel-in-progress: true
+
+env:
+  CI: true
+
+jobs:
+  js-unit-test:
+    runs-on: 'ubuntu-22.04'
+    name: 'opentrons ai frontend unit tests'
+    timeout-minutes: 60
+    steps:
+      - uses: 'actions/checkout@v3'
+      - uses: 'actions/setup-node@v3'
+        with:
+          node-version: '18.19.0'
+      - name: 'install udev'
+        run: sudo apt-get update && sudo apt-get install libudev-dev
+      - name: 'set complex environment variables'
+        id: 'set-vars'
+        uses: actions/github-script@v6
+        with:
+          script: |
+            const { buildComplexEnvVars } = require(`${process.env.GITHUB_WORKSPACE}/.github/workflows/utils.js`)
+            buildComplexEnvVars(core, context)
+      - name: 'cache yarn cache'
+        uses: actions/cache@v3
+        with:
+          path: |
+            ${{ github.workspace }}/.npm-cache/_prebuild
+            ${{ github.workspace }}/.yarn-cache
+          key: js-${{ secrets.GH_CACHE_VERSION }}-${{ runner.os }}-yarn-${{ hashFiles('yarn.lock') }}
+      - name: 'setup-js'
+        run: |
+          npm config set cache ${{ github.workspace }}/.npm-cache
+          yarn config set cache-folder ${{ github.workspace }}/.yarn-cache
+          make setup-js
+      - name: 'test frontend packages'
+        run: |
+          make -C opentrons-ai-client test-cov
+      - name: 'Upload coverage report'
+        uses: codecov/codecov-action@v3
+        with:
+          files: ./coverage/lcov.info
+          flags: opentrons-ai-client

api/Pipfile

@@ -48,3 +48,4 @@ pytest-profiling = "~=1.7.0"
 # TODO(mc, 2022-03-31): upgrade sphinx, remove this subdep pin
 jinja2 = ">=2.3,<3.1"
 hypothesis = "==6.96.1"
+performance-metrics = {file = "../performance-metrics", editable = true}

api/src/opentrons/cli/analyze.py

@@ -28,6 +28,7 @@
 )
 
 from opentrons_shared_data.robot.dev_types import RobotType
+from opentrons.util.performance_helpers import track_analysis
 
 
 @click.command()
@@ -63,6 +64,7 @@ def _get_input_files(files_and_dirs: Sequence[Path]) -> List[Path]:
     return results
 
 
+@track_analysis
 async def _analyze(
     files_and_dirs: Sequence[Path],
     json_output: Optional[AsyncPath],

api/src/opentrons/config/__init__.py

@@ -284,6 +284,13 @@ class ConfigElement(NamedTuple):
         ConfigElementType.DIR,
         "The dir where module calibration is stored",
     ),
+    ConfigElement(
+        "performance_metrics_dir",
+        "Performance Metrics Directory",
+        Path("performance_metrics_data"),
+        ConfigElementType.DIR,
+        "The dir where performance metrics are stored",
+    ),
 )
 #: The available configuration file elements to modify. All of these can be
 #: changed by editing opentrons.json, where the keys are the name elements,
@@ -602,3 +609,7 @@ def get_tip_length_cal_path() -> Path:
 
 def get_custom_tiprack_def_path() -> Path:
     return get_opentrons_path("custom_tiprack_dir")
+
+
+def get_performance_metrics_data_dir() -> Path:
+    return get_opentrons_path("performance_metrics_dir")

api/src/opentrons/hardware_control/instruments/ot2/pipette.py

@@ -26,18 +26,19 @@
     InvalidLiquidClassName,
     CommandPreconditionViolated,
 )
+from opentrons_shared_data.pipette.ul_per_mm import (
+    piecewise_volume_conversion,
+    PIPETTING_FUNCTION_FALLBACK_VERSION,
+    PIPETTING_FUNCTION_LATEST_VERSION,
+)
 
 
 from opentrons.types import Point, Mount
 from opentrons.config import robot_configs
 from opentrons.config.types import RobotConfig
 from opentrons.drivers.types import MoveSplit
 from ..instrument_abc import AbstractInstrument
-from ..instrument_helpers import (
-    piecewise_volume_conversion,
-    PIPETTING_FUNCTION_FALLBACK_VERSION,
-    PIPETTING_FUNCTION_LATEST_VERSION,
-)
+
 from .instrument_calibration import (
     PipetteOffsetByPipetteMount,
     load_pipette_offset,

api/src/opentrons/hardware_control/instruments/ot3/pipette.py

@@ -25,12 +25,12 @@
     CommandPreconditionViolated,
     PythonException,
 )
-from ..instrument_abc import AbstractInstrument
-from ..instrument_helpers import (
+from opentrons_shared_data.pipette.ul_per_mm import (
     piecewise_volume_conversion,
     PIPETTING_FUNCTION_FALLBACK_VERSION,
     PIPETTING_FUNCTION_LATEST_VERSION,
 )
+from ..instrument_abc import AbstractInstrument
 from .instrument_calibration import (
     save_pipette_offset_calibration,
     load_pipette_offset,

api/src/opentrons/hardware_control/ot3api.py

@@ -1677,8 +1677,8 @@ async def retract_axis(self, axis: Axis) -> None:
                 # home the axis
                 await self._home_axis(axis)
 
-        await self._cache_current_position()
-        await self._cache_encoder_position()
+            await self._cache_current_position()
+            await self._cache_encoder_position()
 
     # Gantry/frame (i.e. not pipette) config API
     @property

api/src/opentrons/protocols/parameters/validation.py

@@ -20,7 +20,7 @@ def validate_variable_name_unique(
     variable_name: str, other_variable_names: Set[str]
 ) -> None:
     """Validate that the given variable name is unique."""
-    if variable_name in other_variable_names:
+    if isinstance(variable_name, str) and variable_name in other_variable_names:
         raise ParameterNameError(
             f'"{variable_name}" is already defined as a variable name for another parameter.'
             f" All variable names must be unique."
@@ -222,7 +222,7 @@ def _validate_min_and_max(
             # These asserts are for the type checker and should never actually be asserted false
             assert isinstance(minimum, (int, float))
             assert isinstance(maximum, (int, float))
-            if maximum <= minimum:
+            if maximum < minimum:
                 raise ParameterDefinitionError(
                     "Maximum must be greater than the minimum"
                 )

api/src/opentrons/util/performance_helpers.py

@@ -0,0 +1,76 @@
+"""Performance helpers for tracking robot context."""
+
+from pathlib import Path
+from opentrons_shared_data.performance.dev_types import (
+    SupportsTracking,
+    F,
+    RobotContextState,
+)
+from opentrons_shared_data.robot.dev_types import RobotTypeEnum
+from typing import Callable, Type
+from opentrons.config import (
+    feature_flags as ff,
+    get_performance_metrics_data_dir,
+    robot_configs,
+)
+
+
+_should_track = ff.enable_performance_metrics(
+    RobotTypeEnum.robot_literal_to_enum(robot_configs.load().model)
+)
+
+
+def _handle_package_import() -> Type[SupportsTracking]:
+    """Handle the import of the performance_metrics package.
+
+    If the package is not available, return a stubbed tracker.
+    """
+    try:
+        from performance_metrics import RobotContextTracker
+
+        return RobotContextTracker
+    except ImportError:
+        return StubbedTracker
+
+
+package_to_use = _handle_package_import()
+_robot_context_tracker: SupportsTracking | None = None
+
+
+class StubbedTracker(SupportsTracking):
+    """A stubbed tracker that does nothing."""
+
+    def __init__(self, storage_location: Path, should_track: bool) -> None:
+        """Initialize the stubbed tracker."""
+        pass
+
+    def track(self, state: RobotContextState) -> Callable[[F], F]:
+        """Return the function unchanged."""
+
+        def inner_decorator(func: F) -> F:
+            """Return the function unchanged."""
+            return func
+
+        return inner_decorator
+
+    def store(self) -> None:
+        """Do nothing."""
+        pass
+
+
+def _get_robot_context_tracker() -> SupportsTracking:
+    """Singleton for the robot context tracker."""
+    global _robot_context_tracker
+    if _robot_context_tracker is None:
+        # TODO: replace with path lookup and should_store lookup
+        _robot_context_tracker = package_to_use(
+            get_performance_metrics_data_dir(), _should_track
+        )
+    return _robot_context_tracker
+
+
+def track_analysis(func: F) -> F:
+    """Track the analysis of a protocol."""
+    return _get_robot_context_tracker().track(RobotContextState.ANALYZING_PROTOCOL)(
+        func
+    )

api/tests/opentrons/cli/test_cli.py

@@ -1,4 +1,6 @@
 """Test cli execution."""
+
+
 import json
 import tempfile
 import textwrap
@@ -9,8 +11,17 @@
 
 import pytest
 from click.testing import CliRunner
+from opentrons.util.performance_helpers import _get_robot_context_tracker
+
 
-from opentrons.cli.analyze import analyze
+# Enable tracking for the RobotContextTracker
+# This must come before the import of the analyze CLI
+context_tracker = _get_robot_context_tracker()
+
+# Ignore the type error for the next line, as we're setting a private attribute for testing purposes
+context_tracker._should_track = True  # type: ignore[attr-defined]
+
+from opentrons.cli.analyze import analyze  # noqa: E402
 
 
 def _list_fixtures(version: int) -> Iterator[Path]:
@@ -242,3 +253,24 @@ def test_python_error_line_numbers(
     assert result.json_output is not None
     [error] = result.json_output["errors"]
     assert error["detail"] == expected_detail
+
+
+def test_track_analysis(tmp_path: Path) -> None:
+    """Test that the RobotContextTracker tracks analysis."""
+    protocol_source = textwrap.dedent(
+        """
+        requirements = {"apiLevel": "2.15"}
+
+        def run(protocol):
+            pass
+        """
+    )
+
+    protocol_source_file = tmp_path / "protocol.py"
+    protocol_source_file.write_text(protocol_source, encoding="utf-8")
+
+    before_analysis = len(context_tracker._storage)  # type: ignore[attr-defined]
+
+    _get_analysis_result([protocol_source_file])
+
+    assert len(context_tracker._storage) == before_analysis + 1  # type: ignore[attr-defined]

api/tests/opentrons/protocols/parameters/test_validation.py

@@ -13,8 +13,9 @@
 
 
 def test_validate_variable_name_unique() -> None:
-    """It should no-op if the name is unique and raise if it is not."""
+    """It should no-op if the name is unique or if it's not a string, and raise if it is not."""
     subject.validate_variable_name_unique("one of a kind", {"fee", "foo", "fum"})
+    subject.validate_variable_name_unique({}, {"fee", "foo", "fum"})  # type: ignore[arg-type]
     with pytest.raises(ParameterNameError):
         subject.validate_variable_name_unique("copy", {"paste", "copy", "cut"})
 
@@ -103,10 +104,12 @@ def test_ensure_variable_name_raises_keyword(variable_name: str) -> None:
 def test_validate_options() -> None:
     """It should not raise when given valid constraints"""
     subject.validate_options(123, 1, 100, None, int)
+    subject.validate_options(123, 100, 100, None, int)
     subject.validate_options(
         123, None, None, [{"display_name": "abc", "value": 456}], int
     )
     subject.validate_options(12.3, 1.1, 100.9, None, float)
+    subject.validate_options(12.3, 1.1, 1.1, None, float)
     subject.validate_options(
         12.3, None, None, [{"display_name": "abc", "value": 45.6}], float
     )

api/tests/opentrons/util/test_performance_helpers.py

@@ -0,0 +1,28 @@
+"""Tests for performance_helpers."""
+
+from pathlib import Path
+from opentrons_shared_data.performance.dev_types import RobotContextState
+from opentrons.util.performance_helpers import (
+    StubbedTracker,
+    _get_robot_context_tracker,
+)
+
+
+def test_return_function_unchanged() -> None:
+    """Test that the function is returned unchanged when using StubbedTracker."""
+    tracker = StubbedTracker(Path("/path/to/storage"), True)
+
+    def func_to_track() -> None:
+        pass
+
+    assert (
+        tracker.track(RobotContextState.ANALYZING_PROTOCOL)(func_to_track)
+        is func_to_track
+    )
+
+
+def test_singleton_tracker() -> None:
+    """Test that the tracker is a singleton."""
+    tracker = _get_robot_context_tracker()
+    tracker2 = _get_robot_context_tracker()
+    assert tracker is tracker2