[GitHub] [tvm] guberti commented on a diff in pull request #13377: [microTVM][Zephyr]Add serial_number test args

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

guberti commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1026202767


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -97,6 +97,11 @@ def check_call(cmd_args, *args, **kwargs):
     + [(k, False) for k in ("0", "OFF", "NO", "FALSE", "N", "IGNORE", "NOTFOUND", "")]
 )
 
+CMSIS_PATH_ERROR = (
+    "cmsis_path is not defined! Please pass it as"
+    "an option or provide it with setting `CMSIS_PATH` env variable."
+)

Review Comment:
   ```suggestion
   CMSIS_PATH_ERROR = (
       "cmsis_path is not defined! Please pass it as"
       "an option or set the `CMSIS_PATH` env variable."
   )
   ```



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -504,42 +506,68 @@ def _cmsis_required(self, project_path: Union[str, pathlib.Path]) -> bool:
                     return True
         return False
 
-    def _generate_cmake_args(self, mlf_extracted_path, options) -> str:
+    def _generate_cmake_args(
+        self,
+        mlf_extracted_path,

Review Comment:
   What's the type here?



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -591,25 +619,29 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
             with open(API_SERVER_DIR / f"{CMAKELIST_FILENAME}.template", "r") as cmake_template_f:
                 for line in cmake_template_f:
                     if self.API_SERVER_CRT_LIBS_TOKEN in line:
-                        crt_libs = self.CRT_LIBS_BY_PROJECT_TYPE[options["project_type"]]
+                        crt_libs = self.CRT_LIBS_BY_PROJECT_TYPE[project_type]
                         line = line.replace("<API_SERVER_CRT_LIBS>", crt_libs)
 
                     if self.CMAKE_ARGS_TOKEN in line:
-                        line = self._generate_cmake_args(extract_path, options)
+                        line = self._generate_cmake_args(
+                            extract_path,
+                            zephyr_board,
+                            use_fvp,
+                            west_cmd,
+                            zephyr_base,
+                            verbose,
+                            cmsis_path,
+                        )
 
                     if self.QEMU_PIPE_TOKEN in line:
                         self.qemu_pipe_dir = pathlib.Path(tempfile.mkdtemp())
                         line = line.replace(self.QEMU_PIPE_TOKEN, str(self.qemu_pipe_dir / "fifo"))
 
-                    if self.CMSIS_PATH_TOKEN in line and self._cmsis_required(extract_path):
-                        line = line.replace(self.CMSIS_PATH_TOKEN, str(os.environ["CMSIS_PATH"]))
-
                     cmake_f.write(line)
 
-                heap_size = _get_recommended_heap_size_bytes(options)
-                if options.get("heap_size_bytes"):
-                    board_mem_size = _get_board_mem_size_bytes(options)
-                    heap_size = options["heap_size_bytes"]
+                heap_size = recommended_heap_size
+                if heap_size_bytes:
+                    heap_size = heap_size_bytes

Review Comment:
   Having two different variables `heap_size_bytes` and `heap_size` might be confusing. Could we instead do:
   ```suggestion
                   heap_size = options.get("heap_size_bytes") or recommended_heap_size
   ```



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