[GitHub] [tvm] areusch commented on a change in pull request #9584: [microTVM][TVMC] Add TVMC test for Arduino and Zephyr

[GitBox <gi...@apache.org>]

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



##########
File path: apps/microtvm/arduino/template_project/microtvm_api_server.py
##########
@@ -78,7 +80,13 @@ class BoardAutodetectFailed(Exception):
     ),
     server.ProjectOption(
         "arduino_cli_cmd",
-        required=["build", "flash", "open_transport"],
+        required=["generate_project", "build", "flash", "open_transport"]
+        if not ARDUINO_CLI_CMD

Review comment:
       can you bring this ternary (foo if bar else baz) into a separate line or parenthesize it so it indents?

##########
File path: apps/microtvm/arduino/template_project/microtvm_api_server.py
##########
@@ -248,7 +256,12 @@ def _convert_includes(self, project_dir, source_dir):
         for ext in ("c", "h", "cpp"):
             for filename in source_dir.rglob(f"*.{ext}"):
                 with filename.open() as file:
-                    lines = file.readlines()
+                    try:
+                        lines = file.readlines()
+                    # TODO: This exception only happens using `tvmc micro` and is not catched on Arduino tests.
+                    # Needs more investigation.
+                    except UnicodeDecodeError:

Review comment:
       do we know more about this or have a stacktrace? I think best fix if we can't work around the error would be to open the file in binary mode (`filename.open("rb")`) and then below:
   ```
   for i, line in in enumerate(lines):
     try:
       line_str = str(line, "utf-8")
     except UnicodeDecodeError:
       continue
     result = re.search(r"#include...", line_str)
     # ...
   ```

##########
File path: python/tvm/micro/project.py
##########
@@ -56,6 +56,17 @@ def read(self, n, timeout_sec):
         return self._api_client.read_transport(n, timeout_sec)["data"]
 
 
+def prepare_options(received_options: dict, all_options: dict) -> dict:

Review comment:
       i agree this should be addressed server-side. we could also create a dict subclass for options and provide another method (e.g. get_or_default) which provides the convenience wrapper but still allows the user to distinguish between explicitly-specified vs default options). we can do this in a follow-on if we want to.




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