Better version handling for Arduino

apps/microtvm/arduino/template_project/microtvm_api_server.py

@@ -34,7 +34,6 @@
 import re
 
 import serial
-import serial.tools.list_ports
 from tvm.micro.project_api import server
 
 _LOG = logging.getLogger(__name__)
@@ -46,10 +45,7 @@
 
 IS_TEMPLATE = not (API_SERVER_DIR / MODEL_LIBRARY_FORMAT_RELPATH).exists()
 
-# Used to check Arduino CLI version installed on the host.
-# We only check two levels of the version.
-ARDUINO_CLI_VERSION = 0.18
-
+MIN_ARDUINO_CLI_VERSION = 0.18
 
 BOARDS = API_SERVER_DIR / "boards.json"
 
@@ -126,6 +122,7 @@ def __init__(self):
         self._proc = None
         self._port = None
         self._serial = None
+        self._version = None
 
     def server_info_query(self, tvm_version):
         return server.ServerInfo(
@@ -314,25 +311,7 @@ def _find_modified_include_path(self, project_dir, file_path, include_path):
         # It's probably a standard C/C++ header
         return include_path
 
-    def _get_platform_version(self, arduino_cli_path: str) -> float:
-        # sample output of this command:
-        # 'arduino-cli alpha Version: 0.18.3 Commit: d710b642 Date: 2021-05-14T12:36:58Z\n'
-        version_output = subprocess.check_output([arduino_cli_path, "version"], encoding="utf-8")
-        full_version = re.findall(r"version: ([\.0-9]*)", version_output.lower())
-        full_version = full_version[0].split(".")
-        version = float(f"{full_version[0]}.{full_version[1]}")
-
-        return version
-
     def generate_project(self, model_library_format_path, standalone_crt_dir, project_dir, options):
-        # Check Arduino version
-        version = self._get_platform_version(self._get_arduino_cli_cmd(options))
-        if version != ARDUINO_CLI_VERSION:
-            message = f"Arduino CLI version found is not supported: found {version}, expected {ARDUINO_CLI_VERSION}."
-            if options.get("warning_as_error") is not None and options["warning_as_error"]:
-                raise server.ServerError(message=message)
-            _LOG.warning(message)
-
         # Reference key directories with pathlib
         project_dir = pathlib.Path(project_dir)
         project_dir.mkdir()
@@ -368,11 +347,44 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
         # Recursively change includes
         self._convert_includes(project_dir, source_dir)
 
+    def _get_arduino_cli_cmd(self, options: dict):
+        arduino_cli_cmd = options.get("arduino_cli_cmd", ARDUINO_CLI_CMD)
+        assert arduino_cli_cmd, "'arduino_cli_cmd' command not passed and not found by default!"
+        return arduino_cli_cmd
+
+    def _get_platform_version(self, arduino_cli_path: str) -> float:
+        # sample output of this command:
+        # 'arduino-cli alpha Version: 0.18.3 Commit: d710b642 Date: 2021-05-14T12:36:58Z\n'
+        version_output = subprocess.run(
+            [arduino_cli_path, "version"], check=True, stdout=subprocess.PIPE
+        ).stdout.decode("utf-8")
+
+        full_version = re.findall(r"version: ([\.0-9]*)", version_output.lower())
+        full_version = full_version[0].split(".")
+        version = float(f"{full_version[0]}.{full_version[1]}")
+        return version
+
+    # This will only be run for build and upload
+    def _check_platform_version(self, options):
+        if not self._version:
+            cli_command = self._get_arduino_cli_cmd(options)
+            self._version = self._get_platform_version(cli_command)
+
+        if self._version < MIN_ARDUINO_CLI_VERSION:
+            message = (
+                f"Arduino CLI version too old: found {self._version}, "
+                f"need at least {ARDUINO_CLI_VERSION}."
+            )
+            if options.get("warning_as_error") is not None and options["warning_as_error"]:
+                raise server.ServerError(message=message)
+            _LOG.warning(message)
+
     def _get_fqbn(self, options):
         o = BOARD_PROPERTIES[options["arduino_board"]]
         return f"{o['package']}:{o['architecture']}:{o['board']}"
 
     def build(self, options):
+        self._check_platform_version(options)
         BUILD_DIR.mkdir()
 
         compile_cmd = [
@@ -391,11 +403,6 @@ def build(self, options):
         # Specify project to compile
         subprocess.run(compile_cmd, check=True)
 
-    def _get_arduino_cli_cmd(self, options: dict):
-        arduino_cli_cmd = options.get("arduino_cli_cmd", ARDUINO_CLI_CMD)
-        assert arduino_cli_cmd, "'arduino_cli_cmd' command not passed and not found by default!"
-        return arduino_cli_cmd
-
     POSSIBLE_BOARD_LIST_HEADERS = ("Port", "Protocol", "Type", "Board Name", "FQBN", "Core")
 
     def _parse_connected_boards(self, tabular_str):
@@ -438,6 +445,7 @@ def _auto_detect_port(self, options):
 
         desired_fqbn = self._get_fqbn(options)
         for device in self._parse_connected_boards(list_cmd_output):
+            print(device)
             if device["fqbn"] == desired_fqbn:
                 return device["port"]
 
@@ -454,6 +462,7 @@ def _get_arduino_port(self, options):
         return self._port
 
     def flash(self, options):
+        self._check_platform_version(options)
         port = self._get_arduino_port(options)
 
         upload_cmd = [

apps/microtvm/arduino/template_project/tests/test_arduino_microtvm_api_server.py

@@ -63,20 +63,20 @@ def test_find_modified_include_path(self, mock_pathlib_path):
         )
         assert valid_output == valid_arduino_import
 
+    # Format for arduino-cli v0.18.2
     BOARD_CONNECTED_V18 = (
         "Port         Type              Board Name          FQBN                        Core             \n"
         "/dev/ttyACM0 Serial Port (USB) Arduino Nano 33 BLE arduino:mbed_nano:nano33ble arduino:mbed_nano\n"
         "/dev/ttyACM1 Serial Port (USB) Arduino Nano 33     arduino:mbed_nano:nano33    arduino:mbed_nano\n"
         "/dev/ttyS4   Serial Port       Unknown                                                          \n"
         "\n"
     )
-    # Format for arduino-cli v0.18.2
-    BOARD_DISCONNECTED_V18 = (
-        "Port       Type        Board Name FQBN Core\n"
-        "/dev/ttyS4 Serial Port Unknown             \n"
+    # Format for arduino-cli v0.21.1 and above
+    BOARD_CONNECTED_V21 = (
+        "Port         Protocol Type Board Name FQBN                        Core             \n"
+        "/dev/ttyACM0 serial                   arduino:mbed_nano:nano33ble arduino:mbed_nano\n"
         "\n"
     )
-    # Format for arduino-cli v0.21.1 and above
     BOARD_DISCONNECTED_V21 = (
         "Port       Protocol Type        Board Name FQBN Core\n"
         "/dev/ttyS4 serial   Serial Port Unknown\n"
@@ -85,13 +85,14 @@ def test_find_modified_include_path(self, mock_pathlib_path):
 
     def test_parse_connected_boards(self):
         h = microtvm_api_server.Handler()
-        boards = h._parse_connected_boards(self.BOARD_DISCONNECTED_V18)
+        boards = h._parse_connected_boards(self.BOARD_CONNECTED_V21)
         assert list(boards) == [{
-            "port": "/dev/ttyS4",
-            "type": "Serial Port",
-            "board name": "Unknown",
-            "fqbn": "",
-            "core": "",
+            "port": "/dev/ttyACM0",
+            "protocol": "serial",
+            "type": "",
+            "board name": "",
+            "fqbn": "arduino:mbed_nano:nano33ble",
+            "core": "arduino:mbed_nano",
         }]
 
 
@@ -105,9 +106,8 @@ def test_auto_detect_port(self, sub_mock):
         assert handler._auto_detect_port(self.DEFAULT_OPTIONS) == "/dev/ttyACM0"
 
         # Test it raises an exception when no board is connected
-        sub_mock.return_value.stdout = bytes(self.BOARD_DISCONNECTED_V18, "utf-8")
-        with pytest.raises(microtvm_api_server.BoardAutodetectFailed):
-            handler._auto_detect_port(self.DEFAULT_OPTIONS)
+        sub_mock.return_value.stdout = bytes(self.BOARD_CONNECTED_V21, "utf-8")
+        assert handler._auto_detect_port(self.DEFAULT_OPTIONS) == "/dev/ttyACM0"
 
         # Should work with old or new arduino-cli version
         sub_mock.return_value.stdout = bytes(self.BOARD_DISCONNECTED_V21, "utf-8")
@@ -122,16 +122,29 @@ def test_auto_detect_port(self, sub_mock):
             == "/dev/ttyACM1"
         )
 
+    CLI_VERSION = "arduino-cli  Version: 0.21.1 Commit: 9fcbb392 Date: 2022-02-24T15:41:45Z\n"
+
     @mock.patch("subprocess.run")
-    def test_flash(self, mock_subprocess_run):
+    def test_flash(self, mock_run):
+        mock_run.return_value.stdout = bytes(self.CLI_VERSION, "utf-8")
+
         handler = microtvm_api_server.Handler()
         handler._port = "/dev/ttyACM0"
 
         # Test no exception thrown when command works
         handler.flash(self.DEFAULT_OPTIONS)
-        mock_subprocess_run.assert_called_once()
+
+        # Test we checked version then called upload
+        assert mock_run.call_count == 2
+        assert mock_run.call_args_list[0][0] == (['arduino-cli', 'version'],)
+        assert mock_run.call_args_list[1][0][0][0:2] == ['arduino-cli', 'upload']
+        mock_run.reset_mock()
 
         # Test exception raised when `arduino-cli upload` returns error code
-        mock_subprocess_run.side_effect = subprocess.CalledProcessError(2, [])
+        mock_run.side_effect = subprocess.CalledProcessError(2, [])
         with pytest.raises(subprocess.CalledProcessError):
             handler.flash(self.DEFAULT_OPTIONS)
+
+        # Version information should be cached and not checked again
+        mock_run.assert_called_once()
+        assert mock_run.call_args[0][0][0:2] == ['arduino-cli', 'upload']