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 2021/10/21 17:12:02 UTC

[GitHub] [tvm] areusch commented on a change in pull request #9274: [microTVM] Add platform version check to template project

areusch commented on a change in pull request #9274:
URL: https://github.com/apache/tvm/pull/9274#discussion_r733883895



##########
File path: apps/microtvm/arduino/template_project/microtvm_api_server.py
##########
@@ -36,13 +36,17 @@
 import serial.tools.list_ports
 from tvm.micro.project_api import server
 
+_LOG = logging.getLogger("MicroTVM API Server")

Review comment:
       can you use the convention of `logging.getLogger(__name__)`? ooc how come you changed the other one? open to departing from convention if it's not working

##########
File path: apps/microtvm/arduino/template_project/microtvm_api_server.py
##########
@@ -36,13 +36,17 @@
 import serial.tools.list_ports
 from tvm.micro.project_api import server
 
+_LOG = logging.getLogger("MicroTVM API Server")
+
 MODEL_LIBRARY_FORMAT_RELPATH = pathlib.Path("src") / "model" / "model.tar"
 API_SERVER_DIR = pathlib.Path(os.path.dirname(__file__) or os.path.getcwd())
 BUILD_DIR = API_SERVER_DIR / "build"
 MODEL_LIBRARY_FORMAT_PATH = API_SERVER_DIR / MODEL_LIBRARY_FORMAT_RELPATH
 
 IS_TEMPLATE = not (API_SERVER_DIR / MODEL_LIBRARY_FORMAT_RELPATH).exists()
 
+ARDUINO_CLI_VERSION = 0.18

Review comment:
       comment why this is here--what is the semantic meaning of this version?

##########
File path: apps/microtvm/reference-vm/arduino/base-box/base_box_provision.sh
##########
@@ -31,8 +31,9 @@ cd ~
 sudo apt-get install -y ca-certificates
 
 # Install Arduino-CLI (specific version)
+ARDUINO_CLI_VERSION="0.18.3"

Review comment:
       add a note to keep in sync with the version defined in apps/microtvm/arduino/template_project/microtvm_api_server.py

##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -61,6 +61,10 @@
 
 BOARDS = API_SERVER_DIR / "boards.json"
 
+
+ZEPHYR_VERSION = 2.5

Review comment:
       comment similarly

##########
File path: docker/install/ubuntu_install_zephyr.sh
##########
@@ -44,9 +44,10 @@ sudo apt-get install -y cmake
 pip3 install west
 
 # Init ZephyrProject
+ZEPHYR_VERSION="v2.5-branch"

Review comment:
       add similar note but explain why we checkout `-branch` here.

##########
File path: apps/microtvm/arduino/template_project/microtvm_api_server.py
##########
@@ -138,6 +142,11 @@ class BoardAutodetectFailed(Exception):
     server.ProjectOption(
         "verbose", help="True to pass --verbose flag to arduino-cli compile and upload"
     ),
+    server.ProjectOption(
+        "warning_as_error",
+        choices=(True, False),
+        help="Treat warnings as errors.",

Review comment:
       maybe state "and raise an Exception"

##########
File path: apps/microtvm/arduino/template_project/microtvm_api_server.py
##########
@@ -335,7 +344,25 @@ 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:
+        version_output = subprocess.check_output([arduino_cli_path, "version"], encoding="utf-8")
+        version_output = (
+            version_output.replace("\n", "").replace("\r", "").replace(":", "").lower().split(" ")
+        )
+        full_version = version_output[version_output.index("version") + 1].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(options["arduino_cli_cmd"])
+        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 ValueError(message)

Review comment:
       can you raise a ServerError subclass?




-- 
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