areusch commented on code in PR #12818: URL: https://github.com/apache/tvm/pull/12818#discussion_r979013935
########## apps/microtvm/arduino/template_project/microtvm_api_server.py: ########## @@ -313,7 +297,90 @@ def _find_modified_include_path(self, project_dir, file_path, include_path): # It's probably a standard C/C++ header return include_path + CMSIS_INCLUDE_HEADERS = [ + "arm_nn_math_types.h", + "arm_nn_tables.h", + "arm_nn_types.h", + "arm_nnfunctions.h", + "arm_nnsupportfunctions.h", + ] + + def _cmsis_required(self, project_path: 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(): + # Encoding is for reading C generated code which also includes hex numbers + 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 self.CMSIS_INCLUDE_HEADERS): + return True + return False + + def _copy_cmsis(self, project_path: 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 self.CMSIS_INCLUDE_HEADERS: + shutil.copy2( + cmsis_path / "CMSIS" / "NN" / "Include" / item, + project_path / "include" / "cmsis" / item, + ) + + # These tokens are used in the Makefile.template file. + # They are replaced with proper value in generate_project step. + FQBN_TOKEN = "<FQBN>" + VERBOSE_FLAG_TOKEN = "<VERBOSE_FLAG>" + ARUINO_CLI_CMD_TOKEN = "<ARUINO_CLI_CMD>" + BOARD_TOKEN = "<BOARD>" + BUILD_EXTRA_FLAGS_TOKEN = "<BUILD_EXTRA_FLAGS>" + + def _populate_makefile( + self, + makefile_template_path: pathlib.Path, + makefile_path: pathlib.Path, + board: str, Review Comment: yeah i agree there is a bit of a problem that everything depends on all options. typically this problem manifests itself in more complex codebases where refactoring becomes difficult because the member functions organically grow to the point they need interfaces, but interfaces are hard to define because the dependency net is too wide. the drawback to factoring stuff out here is that it's possible to modify it, and then the meaning of a var could change depending on if it used the modified value or the original one present in `options`. for example, you access some vars as mandatory here and use `.get()` which means their value could be the var or None. overall it's a tradeoff between these two. do you feel the code is sufficiently complex to need interfaces? -- 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-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org