You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/09/21 17:49:45 UTC

[GitHub] [tvm] mehrdadh commented on a diff in pull request #12818: [microTVM] Use default Project Options in template projects and add Makefile for Arduino template project

mehrdadh commented on code in PR #12818:
URL: https://github.com/apache/tvm/pull/12818#discussion_r976769358


##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -55,27 +57,29 @@
     raise FileNotFoundError(f"Board file {{{BOARDS}}} does not exist.")
 
 
+def get_cmsis_path(cmsis_path: pathlib.Path) -> pathlib.Path:
+    """Returns CMSIS dependency path"""
+    if cmsis_path:
+        return pathlib.Path(cmsis_path)
+    if os.environ.get("CMSIS_PATH"):
+        return pathlib.Path(os.environ.get("CMSIS_PATH"))
+    assert False, "'cmsis_path' option not passed!"
+
+
 class BoardAutodetectFailed(Exception):
     """Raised when no attached hardware is found matching the requested board"""
 
 
 PROJECT_TYPES = ["example_project", "host_driven"]
 
-PROJECT_OPTIONS = [
-    server.ProjectOption(
-        "arduino_board",
-        required=["build", "flash", "open_transport"],
-        choices=list(BOARD_PROPERTIES),
-        type="str",
-        help="Name of the Arduino board to build for.",
-    ),
+PROJECT_OPTIONS = server.default_project_options(
+    project_type={"choices": tuple(PROJECT_TYPES)},
+    board={"choices": list(BOARD_PROPERTIES), "optional": ["flash", "open_transport"]},
+    warning_as_error={"optional": ["build", "flash"]},
+) + [
     server.ProjectOption(
         "arduino_cli_cmd",
-        required=(
-            ["generate_project", "build", "flash", "open_transport"]
-            if not ARDUINO_CLI_CMD
-            else None
-        ),
+        required=(["generate_project", "flash", "open_transport"] if not ARDUINO_CLI_CMD else None),

Review Comment:
   arduino_cli_cmd is set as a variable in the Makefile in generate_project step. Later in the build we don't need that, you just call `make build` to build the project.



##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -313,7 +304,51 @@ def _find_modified_include_path(self, project_dir, file_path, include_path):
         # It's probably a standard C/C++ header
         return include_path
 
+    def _cmsis_required(self, project_path: Union[str, pathlib.Path]) -> bool:
+        """Check if CMSIS dependency is required."""
+        project_path = pathlib.Path(project_path)
+        for path in (project_path / "src" / "model").iterdir():
+            if path.is_file():
+                with open(path, "r", encoding="ISO-8859-1") as lib_f:

Review Comment:
   added comment



##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -368,43 +436,33 @@ def _get_platform_version(self, arduino_cli_path: str) -> float:
         return version.parse(str_version)
 
     # This will only be run for build and upload
-    def _check_platform_version(self, options):
+    def _check_platform_version(self, arduino_cli_cmd: str, warning_as_error: bool):
         if not self._version:
-            cli_command = self._get_arduino_cli_cmd(options)
+            cli_command = self._get_arduino_cli_cmd(arduino_cli_cmd)
             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 {str(MIN_ARDUINO_CLI_VERSION)}."
             )
-            if options.get("warning_as_error") is not None and options["warning_as_error"]:
+            if warning_as_error is not None and warning_as_error:
                 raise server.ServerError(message=message)
             _LOG.warning(message)
 
-    def _get_fqbn(self, options):
-        o = BOARD_PROPERTIES[options["arduino_board"]]
+    def _get_fqbn(self, board: str):
+        o = BOARD_PROPERTIES[board]
         return f"{o['package']}:{o['architecture']}:{o['board']}"
 
     def build(self, options):
-        self._check_platform_version(options)
-        BUILD_DIR.mkdir()
-
-        compile_cmd = [
-            self._get_arduino_cli_cmd(options),
-            "compile",
-            "./project/",
-            "--fqbn",
-            self._get_fqbn(options),
-            "--build-path",
-            BUILD_DIR.resolve(),
-        ]
-
-        if options.get("verbose"):
-            compile_cmd.append("--verbose")
+        # List all used project options
+        arduino_cli_cmd = options.get("arduino_cli_cmd")
+        warning_as_error = options.get("warning_as_error")
 
+        self._check_platform_version(arduino_cli_cmd, warning_as_error)
+        compile_cmd = ["make", "build"]
         # Specify project to compile
-        subprocess.run(compile_cmd, check=True)
+        subprocess.run(compile_cmd, check=True, cwd=API_SERVER_DIR)

Review Comment:
   because compile_cmd is `make build` and that needs to be in the same directory as Makefile



##########
python/tvm/micro/project_api/server.py:
##########
@@ -759,6 +779,68 @@ def write_with_timeout(fd, data, timeout_sec):  # pylint: disable=invalid-name
     return num_written
 
 
+def default_project_options(**kw) -> typing.List[ProjectOption]:
+    """Get default Project Options
+
+    Attributes of any default option can be updated. Here is an example
+    when attribute `optional` from `verbose` option needs to be updates:
+
+        default_project_options(verbose={"optional": ["build"]})
+
+    This will update the `optional` attribute of `verbose` ProjectOption
+    to be `["build"]`.
+
+    Returns
+    -------
+    options: List[ProjectOption]
+        A list of default ProjectOption with modifications.
+    """
+    options = [
+        ProjectOption(
+            "verbose",
+            optional=["generate_project"],
+            type="bool",
+            help="Run build with verbose output.",

Review Comment:
   it shows more debugging messages from build and flash steps



##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -440,54 +498,59 @@ def _parse_connected_boards(self, tabular_str):
                 device[col_name] = str_row[column.start() : column.end()].strip()
             yield device
 
-    def _auto_detect_port(self, options):
-        list_cmd = [self._get_arduino_cli_cmd(options), "board", "list"]
+    def _auto_detect_port(self, arduino_cli_cmd: str, board: str) -> str:
+        list_cmd = [self._get_arduino_cli_cmd(arduino_cli_cmd), "board", "list"]
         list_cmd_output = subprocess.run(
             list_cmd, check=True, stdout=subprocess.PIPE
         ).stdout.decode("utf-8")
 
-        desired_fqbn = self._get_fqbn(options)
+        desired_fqbn = self._get_fqbn(board)
         for device in self._parse_connected_boards(list_cmd_output):
             if device["fqbn"] == desired_fqbn:
                 return device["port"]
 
         # If no compatible boards, raise an error
         raise BoardAutodetectFailed()
 
-    def _get_arduino_port(self, options):
+    def _get_arduino_port(self, arduino_cli_cmd: str, board: str, port: int):
         if not self._port:
-            if "port" in options and options["port"]:
-                self._port = options["port"]
+            if port:
+                self._port = port
             else:
-                self._port = self._auto_detect_port(options)
+                self._port = self._auto_detect_port(arduino_cli_cmd, board)
 
         return self._port
 
+    def _get_board_from_makefile(self, makefile_path: pathlib.Path) -> str:
+        """Get Board from generated Makefile."""
+        with open(makefile_path) as makefile_f:
+            line = makefile_f.readline()
+            if "BOARD" in line:
+                board = line.replace(" ", "").replace("\n", "").replace("\r", "").split(":=")[1]

Review Comment:
   done, thanks!



##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -55,27 +57,29 @@
     raise FileNotFoundError(f"Board file {{{BOARDS}}} does not exist.")
 
 
+def get_cmsis_path(cmsis_path: pathlib.Path) -> pathlib.Path:
+    """Returns CMSIS dependency path"""
+    if cmsis_path:
+        return pathlib.Path(cmsis_path)
+    if os.environ.get("CMSIS_PATH"):
+        return pathlib.Path(os.environ.get("CMSIS_PATH"))
+    assert False, "'cmsis_path' option not passed!"
+
+
 class BoardAutodetectFailed(Exception):
     """Raised when no attached hardware is found matching the requested board"""
 
 
 PROJECT_TYPES = ["example_project", "host_driven"]
 
-PROJECT_OPTIONS = [
-    server.ProjectOption(
-        "arduino_board",
-        required=["build", "flash", "open_transport"],
-        choices=list(BOARD_PROPERTIES),
-        type="str",
-        help="Name of the Arduino board to build for.",
-    ),
+PROJECT_OPTIONS = server.default_project_options(
+    project_type={"choices": tuple(PROJECT_TYPES)},
+    board={"choices": list(BOARD_PROPERTIES), "optional": ["flash", "open_transport"]},

Review Comment:
   board is required for generate_project, which is the default. But you can optionally pass it for flash or open_transport because by default it would get it from the generated Makefile 



##########
apps/microtvm/arduino/template_project/Makefile.template:
##########
@@ -0,0 +1,57 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+

Review Comment:
   I added an `info` command in the makefile which explains how to use this file



##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -313,7 +304,51 @@ def _find_modified_include_path(self, project_dir, file_path, include_path):
         # It's probably a standard C/C++ header
         return include_path
 
+    def _cmsis_required(self, project_path: Union[str, pathlib.Path]) -> bool:
+        """Check if CMSIS dependency is required."""
+        project_path = pathlib.Path(project_path)
+        for path in (project_path / "src" / "model").iterdir():
+            if path.is_file():
+                with open(path, "r", encoding="ISO-8859-1") as lib_f:
+                    lib_content = lib_f.read()
+                if any(
+                    header in lib_content
+                    for header in [
+                        "<arm_nnsupportfunctions.h>",
+                        "arm_nn_types.h",
+                        "arm_nnfunctions.h",
+                    ]
+                ):

Review Comment:
   done.



##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -176,6 +161,12 @@ def _copy_standalone_crt(self, source_dir, standalone_crt_dir):
         "src/runtime/crt/tab",
     ]
 
+    FQBN_TOKEN = "<FQBN>"

Review Comment:
   done



##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -313,7 +304,51 @@ def _find_modified_include_path(self, project_dir, file_path, include_path):
         # It's probably a standard C/C++ header
         return include_path
 
+    def _cmsis_required(self, project_path: Union[str, pathlib.Path]) -> bool:

Review Comment:
   done



##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -341,16 +376,49 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
         shutil.copy2(model_library_format_path, project_dir / MODEL_LIBRARY_FORMAT_RELPATH)
 
         # For AOT, template model.h with metadata to minimize space usage
-        if options["project_type"] == "example_project":
+        if project_type == "example_project":
             self._template_model_header(source_dir, metadata)
 
         self._change_cpp_file_extensions(source_dir)
 
         # 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)
+        # create include directory
+        (project_dir / "include").mkdir()
+        build_extra_flags = "-I./include"
+
+        if self._cmsis_required(project_dir):
+            build_extra_flags += f" -I./include/cmsis"
+            self._copy_cmsis(project_dir, cmsis_path)
+
+        # Populate Makefile
+        with open(project_dir / MAKEFILE_FILENAME, "w") as makefile_f:

Review Comment:
   done



##########
python/tvm/micro/project_api/server.py:
##########
@@ -759,6 +779,68 @@ def write_with_timeout(fd, data, timeout_sec):  # pylint: disable=invalid-name
     return num_written
 
 
+def default_project_options(**kw) -> typing.List[ProjectOption]:
+    """Get default Project Options
+
+    Attributes of any default option can be updated. Here is an example
+    when attribute `optional` from `verbose` option needs to be updates:
+
+        default_project_options(verbose={"optional": ["build"]})
+
+    This will update the `optional` attribute of `verbose` ProjectOption
+    to be `["build"]`.
+
+    Returns
+    -------
+    options: List[ProjectOption]
+        A list of default ProjectOption with modifications.
+    """
+    options = [
+        ProjectOption(
+            "verbose",
+            optional=["generate_project"],
+            type="bool",
+            help="Run build with verbose output.",
+        ),
+        ProjectOption(
+            "project_type",
+            required=["generate_project"],
+            type="str",
+            help="Type of project to generate.",
+        ),
+        ProjectOption(
+            "board",
+            required=["generate_project"],
+            type="str",
+            help="Name of the board to build for.",

Review Comment:
   this is not specific to arduino, so I would like to keep it generic



##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -313,7 +304,51 @@ def _find_modified_include_path(self, project_dir, file_path, include_path):
         # It's probably a standard C/C++ header
         return include_path
 
+    def _cmsis_required(self, project_path: Union[str, pathlib.Path]) -> bool:
+        """Check if CMSIS dependency is required."""
+        project_path = pathlib.Path(project_path)
+        for path in (project_path / "src" / "model").iterdir():
+            if path.is_file():
+                with open(path, "r", encoding="ISO-8859-1") as lib_f:
+                    lib_content = lib_f.read()
+                if any(
+                    header in lib_content
+                    for header in [
+                        "<arm_nnsupportfunctions.h>",
+                        "arm_nn_types.h",
+                        "arm_nnfunctions.h",
+                    ]
+                ):
+                    return True
+        return False
+
+    def _copy_cmsis(self, project_path: Union[str, pathlib.Path], cmsis_path: str):
+        """Copy CMSIS header files to project.
+        Note: We use this CMSIS package:https://www.arduino.cc/reference/en/libraries/arduino_cmsis-dsp/
+        However, the latest release does not include header files that are copied in this function.
+        """
+        (project_path / "include" / "cmsis").mkdir()
+        cmsis_path = get_cmsis_path(cmsis_path)
+        for item in [
+            "arm_nn_types.h",

Review Comment:
   done.



##########
python/tvm/micro/project_api/server.py:
##########
@@ -64,6 +64,26 @@ def __new__(cls, name, **kw):
 
         return super().__new__(cls, **kw)
 
+    def replace(self, **kw):
+        """Update attributes associated to the project option."""
+        updated_option = self
+        for key, val in kw.items():
+            if key == "choices":
+                updated_option = updated_option._replace(choices=val)
+            elif key == "default":
+                updated_option = updated_option._replace(default=val)
+            elif key == "type":
+                updated_option = updated_option._replace(type=val)
+            elif key == "required":
+                updated_option = updated_option._replace(required=val)
+            elif key == "optional":
+                updated_option = updated_option._replace(optional=val)
+            elif key == "help":
+                updated_option = updated_option._replace(help=val)
+            else:
+                raise ValueError("Attribute {} is not supported.".format(key))

Review Comment:
   done



##########
python/tvm/micro/project_api/server.py:
##########
@@ -759,6 +779,68 @@ def write_with_timeout(fd, data, timeout_sec):  # pylint: disable=invalid-name
     return num_written
 
 
+def default_project_options(**kw) -> typing.List[ProjectOption]:
+    """Get default Project Options
+
+    Attributes of any default option can be updated. Here is an example
+    when attribute `optional` from `verbose` option needs to be updates:
+
+        default_project_options(verbose={"optional": ["build"]})
+
+    This will update the `optional` attribute of `verbose` ProjectOption
+    to be `["build"]`.
+
+    Returns
+    -------
+    options: List[ProjectOption]
+        A list of default ProjectOption with modifications.
+    """
+    options = [
+        ProjectOption(
+            "verbose",
+            optional=["generate_project"],
+            type="bool",
+            help="Run build with verbose output.",
+        ),
+        ProjectOption(
+            "project_type",
+            required=["generate_project"],
+            type="str",
+            help="Type of project to generate.",
+        ),
+        ProjectOption(
+            "board",
+            required=["generate_project"],
+            type="str",
+            help="Name of the board to build for.",
+        ),
+        ProjectOption(
+            "cmsis_path",
+            optional=["generate_project"],
+            type="str",
+            help="Path to the CMSIS directory.",
+        ),
+        ProjectOption(
+            "warning_as_error",
+            optional=["generate_project"],
+            type="bool",
+            default=False,
+            help="Treat warnings as errors and raise an Exception.",
+        ),
+    ]
+    for name, config in kw.items():
+        option_found = False
+        for ind, option in enumerate(options):
+            if option.name == name:
+                options[ind] = option.replace(**config)
+                option_found = True
+                break
+        if not option_found:
+            raise ValueError("Option {} was not found in default ProjectOptions.".format(name))

Review Comment:
   these are updating the default options and I think the function string explains it



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org