fix(api): make opentrons_hardware not required (#9605)

The OT2 doesn't have opentrons_hardware installed - that's only an OT3
thing. We have a responsibility to make that actually function. This
setup wasn't actually being tested.

This commit adds
- New makefile targets to "setup-ot2" in relevant python subprojects by
running a pipenv sync and then removing opentrons_hardware from the
virtualenv. This is not good, but absent being able to have separate
pipfiles it seems like the best fix available.
- Github workflow entries to lint and test under these conditions. This
is also not good because it will make these tests take longer, but same
thing about the conditions.

To be truly optional, it must be possible to import opentrons without
the ot3 feature flag set and not ever need opentrons_hardware. That was
mostly true, but there were some exceptions (default re-exports) that
needed to be updated.

.github/workflows/api-test-lint-deploy.yaml

@@ -49,7 +49,7 @@ jobs:
       - uses: './.github/actions/python/setup'
         with:
           project: 'api'
-      - name: Lint
+      - name: Lint with opentrons_hardware
         run: make -C api lint
   test:
     name: 'opentrons package tests on ${{ matrix.os }}, python ${{ matrix.python }}'
@@ -61,6 +61,14 @@ jobs:
         # TODO(mc, 2022-02-24): expand this matrix to 3.8 and 3.9,
         # preferably in a nightly cronjob on edge or something
         python: ['3.7', '3.10']
+        with-ot-hardware: ['true', 'false']
+        exclude:
+          - os: 'windows-2019'
+            with-ot-hardware: 'true'
+          - os: 'macos-latest'
+            with-ot-hardware: 'true'
+          - python: '3.10'
+            with-ot-hardware: 'true'
     runs-on: '${{ matrix.os }}'
     steps:
       - uses: 'actions/checkout@v2'
@@ -79,9 +87,16 @@ jobs:
       - uses: './.github/actions/python/setup'
         with:
           project: 'api'
-      - name: Test
+      - if: ${{ matrix.with-ot-hardware == 'false' }}
+        name: Test without opentrons_hardware
+        run: |
+          make -C api setup-ot2
+          make -C api test-ot2
+      - if: ${{ matrix.with-ot-hardware == 'true' }}
+        name: Test with opentrons_hardware
         run: make -C api test
-      - uses: 'codecov/codecov-action@v2'
+      - if: ${{ matrix.with-ot-hardware == 'true' }}
+        uses: 'codecov/codecov-action@v2'
         with:
           files: ./api/coverage.xml
           flags: api

.github/workflows/robot-server-lint-test.yaml

@@ -41,6 +41,9 @@ jobs:
     name: 'robot server package linting and tests'
     timeout-minutes: 20
     runs-on: 'ubuntu-18.04'
+    strategy:
+      matrix:
+        with-ot-hardware: ['true', 'false']
     steps:
       - uses: 'actions/checkout@v2'
       - uses: 'actions/setup-node@v1'
@@ -53,11 +56,25 @@ jobs:
       - uses: './.github/actions/python/setup'
         with:
           project: 'robot-server'
-      - name: Lint
-        run: make -C robot-server lint
-      - name: Test
+      - if: ${{ matrix.with-ot-hardware == 'false' }}
+        name: Lint without opentrons_hardware
+        run: |
+          make -C robot-server setup-ot2
+          make -C robot-server lint
+      - if: ${{ matrix.with-ot-hardware == 'true' }}
+        name: Lint with opentrons_hardware
+        run: |
+          make -C robot-server lint
+      - if: ${{ matrix.with-ot-hardware == 'false' }}
+        name: Test without opentrons_hardware
+        run: |
+          make -C robot-server setup-ot2
+          make -C robot-server test
+      - if: ${{ matrix.with-ot-hardware == 'true' }}
+        name: Test with opentrons_hardware
         run: make -C robot-server test
-      - uses: 'codecov/codecov-action@v2'
+      - if: ${{ matrix.with-ot-hardware == 'true' }}
+        uses: 'codecov/codecov-action@v2'
         with:
           files: ./robot-server/coverage.xml
           flags: robot-server

api/Makefile

@@ -73,6 +73,12 @@ setup:
 	$(pipenv) sync $(pipenv_opts)
 	$(pipenv) run pip freeze
 
+.PHONY: setup-ot2
+setup-ot2:
+	$(pipenv) sync $(pipenv_opts)
+	$(pipenv) run pip uninstall -y opentrons_hardware
+	$(pipenv) run pip freeze
+
 .PHONY: clean
 clean: docs-clean
 	$(clean_cmd)
@@ -101,6 +107,10 @@ sdist: $(sdist_file)
 test:
 	$(pytest) $(tests) $(test_opts)
 
+.PHONY: test-ot2
+test-ot2:
+	$(pytest) $(tests) $(test_opts) --ot2-only --ignore-glob="**/*ot3*"
+
 .PHONY: lint
 lint:
 	$(python) -m mypy src tests

api/README.rst

@@ -30,7 +30,7 @@ The Opentrons library has two purposes:
 Setting Up For Development
 --------------------------
 
-First, read the `top-level contributing guide section on setup <https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#environment-and-repository>`_. As that document states, once you have installed the prerequisites you can simply run ``make setup`` in this subdirectory.
+First, read the `top-level contributing guide section on setup <https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#environment-and-repository>`_. As that document states, once you have installed the prerequisites you can simply run ``make setup`` in this subdirectory. If you want to make the environment as it will be on an OT-2 specifically, run ``make setup-ot2``
 
 The only additional prerequisite concerns building documentation. If you want to build the PDF version of the documentation, you will need an installation of `LaTeX <https://www.latex-project.org/get/>`_ that includes the ``pdflatex`` tool. Note that if you don’t install this, everything will still work - you just won’t get the PDF documentation.
 
@@ -81,3 +81,8 @@ Configuration
 -------------
 
 The module has a lot of configuration, some of which is only relevant when running on an actual robot, but some of which could be useful during simulation. When the module is first imported, it will try and write configuration files in ``~/.opentrons/config.json`` (``C:\Users\%USERNAME%\.opentrons\config.json`` on Windows). This contains mostly paths to other configuration files and directories, most of which will be in that folder. The location can be changed by setting the environment variable ``OT_API_CONFIG_DIR`` to another path. Inividual settings in the config file can be overridden by setting environment variables named like ``OT_API_${UPPERCASED_VAR_NAME}`` (for instance, to override the ``serial_log_file`` config element you could set the environment variable ``OT_API_SERIAL_LOG_FILE`` to a different path).
+
+Dealing With Robot Versions
+---------------------------
+
+The OT2 does not use the ``hardware`` subdirectory or the ``opentrons_hardware`` package. This can be a problem to work around. Please keep imports of ``opentrons_hardware`` to limited places inside the hardware_control submodule and tests of that submodule, and ensure that anything outside these safe areas conditionally imports ``opentrons_hardware`` or imports it inside a non-file scope in a place used only outside an OT2. In tests, any test that uses the OT3 hardware controller will be skipped in the ``test-ot2`` Makefile recipe.

api/conftest.py

@@ -0,0 +1,12 @@
+"""rootfile conftest - settings that must be in root."""
+import pytest
+
+# Options must be added at the root level for pytest to properly
+# pick them up. Technically, the main conftest that we use in
+# tests/opentrons is not the root level.
+def pytest_addoption(parser):
+    parser.addoption(
+        "--ot2-only",
+        action="store_true",
+        help="only run OT2 based tests",
+    )

api/src/opentrons/hardware_control/backends/__init__.py

@@ -1,6 +1,7 @@
 from .controller import Controller
 from .simulator import Simulator
-from .ot3controller import OT3Controller
-from .ot3simulator import OT3Simulator
 
-__all__ = ["Controller", "Simulator", "OT3Controller", "OT3Simulator", "ot3utils"]
+# only expose the ot2 interfaces in __init__ so everything works if opentrons_hardware
+# is not present
+
+__all__ = ["Controller", "Simulator"]

api/src/opentrons/hardware_control/ot3api.py

@@ -35,7 +35,8 @@
     generate_hardware_configs_ot3,
     load_from_config_and_check_skip,
 )
-from .backends import OT3Controller, OT3Simulator
+from .backends.ot3controller import OT3Controller
+from .backends.ot3simulator import OT3Simulator
 from .execution_manager import ExecutionManagerProvider
 from .pause_manager import PauseManager
 from .module_control import AttachedModulesControl

api/tests/opentrons/conftest.py

@@ -26,7 +26,6 @@
 from opentrons import config
 from opentrons import hardware_control as hc
 from opentrons.hardware_control import HardwareControlAPI, API, ThreadManager
-from opentrons.hardware_control.ot3api import OT3API
 from opentrons.protocol_api import ProtocolContext
 from opentrons.types import Location, Point
 
@@ -112,7 +111,11 @@ async def enable_door_safety_switch():
 
 
 @pytest.fixture
-async def enable_ot3_hardware_controller():
+async def enable_ot3_hardware_controller(request):
+    # this is from the command line parameters added in root conftest
+    if request.config.getoption("--ot2-only"):
+        pytest.skip("testing only ot2")
+
     await config.advanced_settings.set_adv_setting("enableOT3HardwareController", True)
     yield
     await config.advanced_settings.set_adv_setting("enableOT3HardwareController", False)
@@ -151,9 +154,9 @@ def virtual_smoothie_env(monkeypatch):
     params=["ot2", "ot3"],
 )
 async def machine_variant_ffs(request, loop):
-    if request.node.get_closest_marker("ot2_only") and request.param == "ot2":
+    if request.node.get_closest_marker("ot3_only") and request.param == "ot2":
         pytest.skip()
-    if request.node.get_closest_marker("ot3_only") and request.param == "ot3":
+    if request.node.get_closest_marker("ot2_only") and request.param == "ot3":
         pytest.skip()
 
     old = config.advanced_settings.get_adv_setting("enableOT3HardwareController")
@@ -187,6 +190,8 @@ async def ot2_hardware(request, loop, virtual_smoothie_env):
 
 
 async def _build_ot3_hw() -> AsyncIterator[ThreadManager[HardwareControlAPI]]:
+    from opentrons.hardware_control.ot3api import OT3API
+
     hw_sim = ThreadManager(OT3API.build_hardware_simulator)
     old_config = config.robot_configs.load()
     try:
@@ -199,6 +204,9 @@ async def _build_ot3_hw() -> AsyncIterator[ThreadManager[HardwareControlAPI]]:
 
 @pytest.fixture
 async def ot3_hardware(request, loop, enable_ot3_hardware_controller):
+    # this is from the command line parameters added in root conftest
+    if request.config.getoption("--ot2-only"):
+        pytest.skip("testing only ot2")
     async for hw in _build_ot3_hw():
         yield hw
 
@@ -214,6 +222,8 @@ async def hardware(request, loop, virtual_smoothie_env):
         pytest.skip()
     if request.node.get_closest_marker("ot3_only") and request.param() == _build_ot2_hw:
         pytest.skip()
+    if request.param() == _build_ot3_hw and request.config.getoption("--ot2-only"):
+        pytest.skip("testing only ot2")
 
     # param() return a function we have to call
     async for hw in request.param()():

api/tests/opentrons/hardware_control/backends/test_ot3_controller.py

@@ -1,6 +1,6 @@
 import pytest
 from mock import AsyncMock, patch
-from opentrons.hardware_control.backends import OT3Controller
+from opentrons.hardware_control.backends.ot3controller import OT3Controller
 from opentrons_hardware.drivers.can_bus import CanMessenger
 from opentrons.config.types import OT3Config
 from opentrons.config.robot_configs import build_config_ot3

api/tests/opentrons/hardware_control/test_instruments.py

@@ -8,69 +8,90 @@
 import typeguard
 from opentrons import types
 from opentrons.hardware_control import API
-from opentrons.hardware_control.ot3api import OT3API
-from opentrons.hardware_control.types import Axis, OT3Mount
+from opentrons.hardware_control.types import Axis
 from opentrons.hardware_control.dev_types import PipetteDict
 
 
 LEFT_PIPETTE_PREFIX = "p10_single"
 LEFT_PIPETTE_MODEL = "{}_v1".format(LEFT_PIPETTE_PREFIX)
 LEFT_PIPETTE_ID = "testy"
 
-dummy_instruments_attached = {
-    types.Mount.LEFT: {
-        "model": LEFT_PIPETTE_MODEL,
-        "id": LEFT_PIPETTE_ID,
-        "name": LEFT_PIPETTE_PREFIX,
-    },
-    types.Mount.RIGHT: {
-        "model": None,
-        "id": None,
-        "name": None,
-    },
-}
+
+def dummy_instruments_attached():
+    return {
+        types.Mount.LEFT: {
+            "model": LEFT_PIPETTE_MODEL,
+            "id": LEFT_PIPETTE_ID,
+            "name": LEFT_PIPETTE_PREFIX,
+        },
+        types.Mount.RIGHT: {
+            "model": None,
+            "id": None,
+            "name": None,
+        },
+    }
 
 
 @pytest.fixture
 def dummy_instruments():
-    return dummy_instruments_attached
+    return dummy_instruments_attached()
 
 
-dummy_instruments_attached_ot3 = {
-    OT3Mount.LEFT: {
-        "model": "p1000_single_v3.0",
-        "id": "testy",
-        "name": "p1000_single_gen3",
-    },
-    OT3Mount.RIGHT: {"model": None, "id": None, "name": None},
-}
+def dummy_instruments_attached_ot3():
+    return {
+        types.Mount.LEFT: {
+            "model": "p1000_single_v3.0",
+            "id": "testy",
+            "name": "p1000_single_gen3",
+        },
+        types.Mount.RIGHT: {"model": None, "id": None, "name": None},
+    }
 
 
 @pytest.fixture
 def dummy_instruments_ot3():
-    return dummy_instruments_attached_ot3
+    return dummy_instruments_attached_ot3()
+
+
+def wrap_build_ot3_sim():
+    from opentrons.hardware_control.ot3api import OT3API
+
+    return OT3API.build_hardware_simulator
+
+
+@pytest.fixture
+def ot3_api_obj(request):
+    if request.config.getoption("--ot2-only"):
+        pytest.skip("testing ot2 only")
+    from opentrons.hardware_control.ot3api import OT3API
+
+    return OT3API.build_hardware_simulator
 
 
 @pytest.fixture(
     params=[
-        (API.build_hardware_simulator, dummy_instruments_attached),
-        (OT3API.build_hardware_simulator, dummy_instruments_attached_ot3),
+        (lambda: API.build_hardware_simulator, dummy_instruments_attached),
+        (wrap_build_ot3_sim, dummy_instruments_attached_ot3),
     ],
     ids=["ot2", "ot3"],
 )
 def sim_and_instr(request):
     if (
         request.node.get_closest_marker("ot2_only")
-        and request.param[0] == OT3API.build_hardware_simulator
+        and request.param[0] == wrap_build_ot3_sim
     ):
         pytest.skip()
     if (
         request.node.get_closest_marker("ot3_only")
         and request.param[0] == API.build_hardware_simulator
     ):
         pytest.skip()
+    if request.param[0] == wrap_build_ot3_sim and request.config.getoption(
+        "--ot2-only"
+    ):
+        pytest.skip("testing ot2 only")
 
-    yield request.param
+    yield (request.param[0](), request.param[1]())
 
 
 @pytest.fixture
@@ -326,10 +347,8 @@ async def test_aspirate_old(dummy_instruments, loop, old_aspiration):
     assert pos[Axis.B] == new_plunger_pos
 
 
-async def test_aspirate_ot3(dummy_instruments_ot3, loop):
-    hw_api = await OT3API.build_hardware_simulator(
-        attached_instruments=dummy_instruments_ot3, loop=loop
-    )
+async def test_aspirate_ot3(dummy_instruments_ot3, ot3_api_obj, loop):
+    hw_api = await ot3_api_obj(attached_instruments=dummy_instruments_ot3, loop=loop)
     await hw_api.home()
     await hw_api.cache_instruments()
 
@@ -371,10 +390,8 @@ async def test_dispense_ot2(dummy_instruments, loop):
     assert (await hw_api.current_position(mount))[Axis.B] == plunger_pos_2
 
 
-async def test_dispense_ot3(dummy_instruments_ot3, loop):
-    hw_api = await OT3API.build_hardware_simulator(
-        attached_instruments=dummy_instruments_ot3, loop=loop
-    )
+async def test_dispense_ot3(dummy_instruments_ot3, ot3_api_obj, loop):
+    hw_api = await ot3_api_obj(attached_instruments=dummy_instruments_ot3, loop=loop)
     await hw_api.home()
 
     await hw_api.cache_instruments()

notify-server/Makefile

@@ -57,6 +57,12 @@ setup:
 	$(pipenv) sync $(pipenv_opts)
 	$(pipenv) run pip freeze
 
+.PHONY: setup-ot2
+setup-ot2:
+	$(pipenv) sync $(pipenv_opts)
+	$(pipenv) run pip uninstall -y opentrons_hardware
+	$(pipenv) run pip freeze
+
 .PHONY: clean
 clean:
 	$(clean_cmd)