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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to