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 2022/11/14 17:59:13 UTC
[GitHub] [tvm] mehrdadh opened a new pull request, #13377: [microTVM][Zephyr]Add serial_number test args
mehrdadh opened a new pull request, #13377:
URL: https://github.com/apache/tvm/pull/13377
This PR does these changes:
- add serial number to specify device serial number in Zephyr testing
- change Zephyr server API to run tests with specific serial number
- remove west_cmd in test arguments as it is not used
- refactor Zephyr server API to specify required/optional project options at the beginning of each API call.
--
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
[GitHub] [tvm] gromero commented on pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
gromero commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1320066788
@mehrdadh I changed the title to add a space after the tags and tweak it a bit the wording to emphasize the new option 'serial_number' since the tweak on the tests would be a merely a consequence of it, although I do guess you're adding it for the internal test fleet.
--
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
[GitHub] [tvm] gromero commented on a diff in pull request #13377: [microTVM][Zephyr]Add serial_number test args
Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1026481126
##########
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,
+ board: str,
+ use_fvp: bool,
+ west_cmd: str,
+ zephyr_base: str,
+ verbose: bool,
+ cmsis_path: pathlib.Path,
+ ) -> str:
cmake_args = "\n# cmake args\n"
- if options.get("verbose"):
+ if verbose:
cmake_args += "set(CMAKE_VERBOSE_MAKEFILE TRUE)\n"
- if options.get("zephyr_base"):
- cmake_args += f"set(ZEPHYR_BASE {options['zephyr_base']})\n"
+ if zephyr_base:
+ cmake_args += f"set(ZEPHYR_BASE {zephyr_base})\n"
- if options.get("west_cmd"):
- cmake_args += f"set(WEST {options['west_cmd']})\n"
+ if west_cmd:
+ cmake_args += f"set(WEST {west_cmd})\n"
- if self._is_qemu(options["board"], options.get("use_fvp")):
+ if self._is_qemu(board, use_fvp):
# Some boards support more than one emulator, so ensure QEMU is set.
cmake_args += f"set(EMU_PLATFORM qemu)\n"
- if self._is_fvp(options["board"], options.get("use_fvp")):
+ if self._is_fvp(board, use_fvp):
cmake_args += "set(EMU_PLATFORM armfvp)\n"
cmake_args += "set(ARMFVP_FLAGS -I)\n"
- cmake_args += f"set(BOARD {options['board']})\n"
+ cmake_args += f"set(BOARD {board})\n"
- enable_cmsis = self._cmsis_required(mlf_extracted_path)
- if enable_cmsis:
- assert os.environ.get("CMSIS_PATH"), "CMSIS_PATH is not defined."
- cmake_args += f"set(ENABLE_CMSIS {str(enable_cmsis).upper()})\n"
+ if self._cmsis_required(mlf_extracted_path):
+ assert cmsis_path, CMSIS_PATH_ERROR
+ cmake_args += f"set(CMSIS_PATH {str(cmsis_path)})\n"
return cmake_args
def generate_project(self, model_library_format_path, standalone_crt_dir, project_dir, options):
zephyr_board = options["board"]
+ project_type = options["project_type"]
+
+ zephyr_base = get_zephyr_base(options)
+ warning_as_error = options.get("warning_as_error")
+ use_fvp = options.get("use_fvp")
+ west_cmd = get_west_cmd(options)
+ verbose = options.get("verbose")
+
+ recommended_heap_size = _get_recommended_heap_size_bytes(options)
+ heap_size_bytes = options.get("heap_size_bytes")
+ board_mem_size = _get_board_mem_size_bytes(options)
+
+ compile_definitions = options.get("compile_definitions")
+ config_main_stack_size = options.get("config_main_stack_size")
+
+ extra_files_tar = options.get("extra_files_tar")
+
+ cmsis_path = get_cmsis_path(options)
Review Comment:
Now getting this exception since `pathlib.Path()` is not expecting a `None` type:
```
$ tvmc micro create-project --force /tmp/micro_speech ~/scripts/micro_speech.tar zephyr --project-option board=nrf5340dk_nrf5340_cpuapp project_type=host_driven
WARNING:__main__:Board memory information is not available.
The following error occurred on the Project API server side:
calling method generate_project: JSON-RPC error # -32000: calling method generate_project
Traceback (most recent call last):
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 486, in serve_one_request # <--- Outermost server-side stack frame
self._dispatch_request(request)
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 598, in _dispatch_request
return_value = dispatch_method(**params)
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 630, in _dispatch_generate_project
options,
File "/home/gromero/git/tvm/build/microtvm_template_projects/zephyr/microtvm_api_server.py", line 564, in generate_project
cmsis_path = get_cmsis_path(options)
File "/home/gromero/git/tvm/build/microtvm_template_projects/zephyr/microtvm_api_server.py", line 383, in get_cmsis_path
return pathlib.Path(cmsis_path)
File "/usr/lib/python3.6/pathlib.py", line 1001, in __new__
self = cls._from_parts(args, init=False)
File "/usr/lib/python3.6/pathlib.py", line 656, in _from_parts
drv, root, parts = self._parse_args(args)
File "/usr/lib/python3.6/pathlib.py", line 640, in _parse_args
a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not NoneType
```
--
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
[GitHub] [tvm] mehrdadh commented on pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1325549261
@gromero thanks for catching those! PTAL.
--
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
[GitHub] [tvm] gromero commented on a diff in pull request #13377: [microTVM][Zephyr]Add serial_number test args
Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1024510858
##########
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,
+ board: str,
+ use_fvp: bool,
+ west_cmd: str,
+ zephyr_base: str,
+ verbose: bool,
+ cmsis_path: pathlib.Path,
+ ) -> str:
cmake_args = "\n# cmake args\n"
- if options.get("verbose"):
+ if verbose:
cmake_args += "set(CMAKE_VERBOSE_MAKEFILE TRUE)\n"
- if options.get("zephyr_base"):
- cmake_args += f"set(ZEPHYR_BASE {options['zephyr_base']})\n"
+ if zephyr_base:
+ cmake_args += f"set(ZEPHYR_BASE {zephyr_base})\n"
- if options.get("west_cmd"):
- cmake_args += f"set(WEST {options['west_cmd']})\n"
+ if west_cmd:
+ cmake_args += f"set(WEST {west_cmd})\n"
- if self._is_qemu(options["board"], options.get("use_fvp")):
+ if self._is_qemu(board, use_fvp):
# Some boards support more than one emulator, so ensure QEMU is set.
cmake_args += f"set(EMU_PLATFORM qemu)\n"
- if self._is_fvp(options["board"], options.get("use_fvp")):
+ if self._is_fvp(board, use_fvp):
cmake_args += "set(EMU_PLATFORM armfvp)\n"
cmake_args += "set(ARMFVP_FLAGS -I)\n"
- cmake_args += f"set(BOARD {options['board']})\n"
+ cmake_args += f"set(BOARD {board})\n"
- enable_cmsis = self._cmsis_required(mlf_extracted_path)
- if enable_cmsis:
- assert os.environ.get("CMSIS_PATH"), "CMSIS_PATH is not defined."
- cmake_args += f"set(ENABLE_CMSIS {str(enable_cmsis).upper()})\n"
+ if self._cmsis_required(mlf_extracted_path):
+ assert cmsis_path, CMSIS_PATH_ERROR
+ cmake_args += f"set(CMSIS_PATH {str(cmsis_path)})\n"
return cmake_args
def generate_project(self, model_library_format_path, standalone_crt_dir, project_dir, options):
zephyr_board = options["board"]
+ project_type = options["project_type"]
+
+ zephyr_base = get_zephyr_base(options)
+ warning_as_error = options.get("warning_as_error")
+ use_fvp = options.get("use_fvp")
+ west_cmd = get_west_cmd(options)
+ verbose = options.get("verbose")
+
+ recommended_heap_size = _get_recommended_heap_size_bytes(options)
+ heap_size_bytes = options.get("heap_size_bytes")
+ board_mem_size = _get_board_mem_size_bytes(options)
+
+ compile_definitions = options.get("compile_definitions")
+ config_main_stack_size = options.get("config_main_stack_size")
+
+ extra_files_tar = options.get("extra_files_tar")
+
+ cmsis_path = get_cmsis_path(options)
Review Comment:
Here, although `cmsis_path` is declared as optional, it's actually required, so on an env, without CMSIS installed I get:
```
$ tvmc micro create-project --force /tmp/micro_speech ~/scripts/micro_speech.tar zephyr --project-option board=nrf5340dk_nrf5340_cpuapp project_type=host_driven
WARNING:__main__:Board memory information is not available.
The following error occurred on the Project API server side:
calling method generate_project: JSON-RPC error # -32000: calling method generate_project
Traceback (most recent call last):
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 486, in serve_one_request # <--- Outermost server-side stack frame
self._dispatch_request(request)
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 598, in _dispatch_request
return_value = dispatch_method(**params)
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 630, in _dispatch_generate_project
options,
File "/home/gromero/git/tvm/build/microtvm_template_projects/zephyr/microtvm_api_server.py", line 564, in generate_project
cmsis_path = get_cmsis_path(options)
File "/home/gromero/git/tvm/build/microtvm_template_projects/zephyr/microtvm_api_server.py", line 382, in get_cmsis_path
cmsis_path = options.get("cmsis_path", os.environ["CMSIS_PATH"])
File "/usr/lib/python3.6/os.py", line 669, in __getitem__
raise KeyError(key) from None
KeyError: 'CMSIS_PATH
```
--
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
[GitHub] [tvm] gromero commented on a diff in pull request #13377: [microTVM][Zephyr]Add serial_number test args
Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1024510858
##########
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,
+ board: str,
+ use_fvp: bool,
+ west_cmd: str,
+ zephyr_base: str,
+ verbose: bool,
+ cmsis_path: pathlib.Path,
+ ) -> str:
cmake_args = "\n# cmake args\n"
- if options.get("verbose"):
+ if verbose:
cmake_args += "set(CMAKE_VERBOSE_MAKEFILE TRUE)\n"
- if options.get("zephyr_base"):
- cmake_args += f"set(ZEPHYR_BASE {options['zephyr_base']})\n"
+ if zephyr_base:
+ cmake_args += f"set(ZEPHYR_BASE {zephyr_base})\n"
- if options.get("west_cmd"):
- cmake_args += f"set(WEST {options['west_cmd']})\n"
+ if west_cmd:
+ cmake_args += f"set(WEST {west_cmd})\n"
- if self._is_qemu(options["board"], options.get("use_fvp")):
+ if self._is_qemu(board, use_fvp):
# Some boards support more than one emulator, so ensure QEMU is set.
cmake_args += f"set(EMU_PLATFORM qemu)\n"
- if self._is_fvp(options["board"], options.get("use_fvp")):
+ if self._is_fvp(board, use_fvp):
cmake_args += "set(EMU_PLATFORM armfvp)\n"
cmake_args += "set(ARMFVP_FLAGS -I)\n"
- cmake_args += f"set(BOARD {options['board']})\n"
+ cmake_args += f"set(BOARD {board})\n"
- enable_cmsis = self._cmsis_required(mlf_extracted_path)
- if enable_cmsis:
- assert os.environ.get("CMSIS_PATH"), "CMSIS_PATH is not defined."
- cmake_args += f"set(ENABLE_CMSIS {str(enable_cmsis).upper()})\n"
+ if self._cmsis_required(mlf_extracted_path):
+ assert cmsis_path, CMSIS_PATH_ERROR
+ cmake_args += f"set(CMSIS_PATH {str(cmsis_path)})\n"
return cmake_args
def generate_project(self, model_library_format_path, standalone_crt_dir, project_dir, options):
zephyr_board = options["board"]
+ project_type = options["project_type"]
+
+ zephyr_base = get_zephyr_base(options)
+ warning_as_error = options.get("warning_as_error")
+ use_fvp = options.get("use_fvp")
+ west_cmd = get_west_cmd(options)
+ verbose = options.get("verbose")
+
+ recommended_heap_size = _get_recommended_heap_size_bytes(options)
+ heap_size_bytes = options.get("heap_size_bytes")
+ board_mem_size = _get_board_mem_size_bytes(options)
+
+ compile_definitions = options.get("compile_definitions")
+ config_main_stack_size = options.get("config_main_stack_size")
+
+ extra_files_tar = options.get("extra_files_tar")
+
+ cmsis_path = get_cmsis_path(options)
Review Comment:
Here, although `cmsis_path` is declared as optiona, here it's actually required, so on an env, without CMSIS installed I get:
```
$ tvmc micro create-project --force /tmp/micro_speech ~/scripts/micro_speech.tar zephyr --project-option board=nrf5340dk_nrf5340_cpuapp project_type=host_driven
WARNING:__main__:Board memory information is not available.
The following error occurred on the Project API server side:
calling method generate_project: JSON-RPC error # -32000: calling method generate_project
Traceback (most recent call last):
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 486, in serve_one_request # <--- Outermost server-side stack frame
self._dispatch_request(request)
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 598, in _dispatch_request
return_value = dispatch_method(**params)
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 630, in _dispatch_generate_project
options,
File "/home/gromero/git/tvm/build/microtvm_template_projects/zephyr/microtvm_api_server.py", line 564, in generate_project
cmsis_path = get_cmsis_path(options)
File "/home/gromero/git/tvm/build/microtvm_template_projects/zephyr/microtvm_api_server.py", line 382, in get_cmsis_path
cmsis_path = options.get("cmsis_path", os.environ["CMSIS_PATH"])
File "/usr/lib/python3.6/os.py", line 669, in __getitem__
raise KeyError(key) from None
KeyError: 'CMSIS_PATH
```
--
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
[GitHub] [tvm] mehrdadh commented on pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1322797030
@guberti in cortex_m docker image, the `west_cmd` is alos '/venv/apache-tvm-py3.7/bin/python3 -m west'
and it works without issue. Not sure what happened in that case
--
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
[GitHub] [tvm] mehrdadh commented on pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1322805764
@guberti I added details about using serial_number in testing.
--
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
[GitHub] [tvm] mehrdadh commented on a diff in pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1028605997
##########
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,
+ board: str,
+ use_fvp: bool,
+ west_cmd: str,
+ zephyr_base: str,
+ verbose: bool,
+ cmsis_path: pathlib.Path,
+ ) -> str:
cmake_args = "\n# cmake args\n"
- if options.get("verbose"):
+ if verbose:
cmake_args += "set(CMAKE_VERBOSE_MAKEFILE TRUE)\n"
- if options.get("zephyr_base"):
- cmake_args += f"set(ZEPHYR_BASE {options['zephyr_base']})\n"
+ if zephyr_base:
+ cmake_args += f"set(ZEPHYR_BASE {zephyr_base})\n"
- if options.get("west_cmd"):
- cmake_args += f"set(WEST {options['west_cmd']})\n"
+ if west_cmd:
+ cmake_args += f"set(WEST {west_cmd})\n"
- if self._is_qemu(options["board"], options.get("use_fvp")):
+ if self._is_qemu(board, use_fvp):
# Some boards support more than one emulator, so ensure QEMU is set.
cmake_args += f"set(EMU_PLATFORM qemu)\n"
- if self._is_fvp(options["board"], options.get("use_fvp")):
+ if self._is_fvp(board, use_fvp):
cmake_args += "set(EMU_PLATFORM armfvp)\n"
cmake_args += "set(ARMFVP_FLAGS -I)\n"
- cmake_args += f"set(BOARD {options['board']})\n"
+ cmake_args += f"set(BOARD {board})\n"
- enable_cmsis = self._cmsis_required(mlf_extracted_path)
- if enable_cmsis:
- assert os.environ.get("CMSIS_PATH"), "CMSIS_PATH is not defined."
- cmake_args += f"set(ENABLE_CMSIS {str(enable_cmsis).upper()})\n"
+ if self._cmsis_required(mlf_extracted_path):
+ assert cmsis_path, CMSIS_PATH_ERROR
+ cmake_args += f"set(CMSIS_PATH {str(cmsis_path)})\n"
return cmake_args
def generate_project(self, model_library_format_path, standalone_crt_dir, project_dir, options):
zephyr_board = options["board"]
+ project_type = options["project_type"]
+
+ zephyr_base = get_zephyr_base(options)
+ warning_as_error = options.get("warning_as_error")
+ use_fvp = options.get("use_fvp")
+ west_cmd = get_west_cmd(options)
+ verbose = options.get("verbose")
+
+ recommended_heap_size = _get_recommended_heap_size_bytes(options)
+ heap_size_bytes = options.get("heap_size_bytes")
+ board_mem_size = _get_board_mem_size_bytes(options)
+
+ compile_definitions = options.get("compile_definitions")
+ config_main_stack_size = options.get("config_main_stack_size")
+
+ extra_files_tar = options.get("extra_files_tar")
+
+ cmsis_path = get_cmsis_path(options)
Review Comment:
Fixed this, I added a test to build without this options/environment variable to protect this.
--
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
[GitHub] [tvm] mehrdadh commented on a diff in pull request #13377: [microTVM][Zephyr]Add serial_number test args
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1025798464
##########
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,
+ board: str,
+ use_fvp: bool,
+ west_cmd: str,
+ zephyr_base: str,
+ verbose: bool,
+ cmsis_path: pathlib.Path,
+ ) -> str:
cmake_args = "\n# cmake args\n"
- if options.get("verbose"):
+ if verbose:
cmake_args += "set(CMAKE_VERBOSE_MAKEFILE TRUE)\n"
- if options.get("zephyr_base"):
- cmake_args += f"set(ZEPHYR_BASE {options['zephyr_base']})\n"
+ if zephyr_base:
+ cmake_args += f"set(ZEPHYR_BASE {zephyr_base})\n"
- if options.get("west_cmd"):
- cmake_args += f"set(WEST {options['west_cmd']})\n"
+ if west_cmd:
+ cmake_args += f"set(WEST {west_cmd})\n"
- if self._is_qemu(options["board"], options.get("use_fvp")):
+ if self._is_qemu(board, use_fvp):
# Some boards support more than one emulator, so ensure QEMU is set.
cmake_args += f"set(EMU_PLATFORM qemu)\n"
- if self._is_fvp(options["board"], options.get("use_fvp")):
+ if self._is_fvp(board, use_fvp):
cmake_args += "set(EMU_PLATFORM armfvp)\n"
cmake_args += "set(ARMFVP_FLAGS -I)\n"
- cmake_args += f"set(BOARD {options['board']})\n"
+ cmake_args += f"set(BOARD {board})\n"
- enable_cmsis = self._cmsis_required(mlf_extracted_path)
- if enable_cmsis:
- assert os.environ.get("CMSIS_PATH"), "CMSIS_PATH is not defined."
- cmake_args += f"set(ENABLE_CMSIS {str(enable_cmsis).upper()})\n"
+ if self._cmsis_required(mlf_extracted_path):
+ assert cmsis_path, CMSIS_PATH_ERROR
+ cmake_args += f"set(CMSIS_PATH {str(cmsis_path)})\n"
return cmake_args
def generate_project(self, model_library_format_path, standalone_crt_dir, project_dir, options):
zephyr_board = options["board"]
+ project_type = options["project_type"]
+
+ zephyr_base = get_zephyr_base(options)
+ warning_as_error = options.get("warning_as_error")
+ use_fvp = options.get("use_fvp")
+ west_cmd = get_west_cmd(options)
+ verbose = options.get("verbose")
+
+ recommended_heap_size = _get_recommended_heap_size_bytes(options)
+ heap_size_bytes = options.get("heap_size_bytes")
+ board_mem_size = _get_board_mem_size_bytes(options)
+
+ compile_definitions = options.get("compile_definitions")
+ config_main_stack_size = options.get("config_main_stack_size")
+
+ extra_files_tar = options.get("extra_files_tar")
+
+ cmsis_path = get_cmsis_path(options)
Review Comment:
This is a bug, I fixed it.
--
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
[GitHub] [tvm] guberti commented on a diff in pull request #13377: [microTVM][Zephyr]Add serial_number test args
Posted by 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
[GitHub] [tvm] mehrdadh commented on a diff in pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1030936759
##########
tests/micro/zephyr/README.md:
##########
@@ -40,3 +40,9 @@ To see the list of supported values for `--board`, run:
```
$ pytest test_zephyr.py --help
```
+
+If you like to test with a real hardware, you have the option to pass the serial number
Review Comment:
I'll apply it to the next PR if the CI passed.
--
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
[GitHub] [tvm] mehrdadh commented on a diff in pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1028609219
##########
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:
added
--
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
[GitHub] [tvm] mehrdadh commented on a diff in pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1028611077
##########
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:
changed that.
--
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
[GitHub] [tvm] mehrdadh commented on pull request #13377: [microTVM][Zephyr]Add serial_number test args
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1319289177
@gromero the change from `zephyr_board` to `board` happened in previous PRs to make it standard across the API servers. It is not part of this PR.
--
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
[GitHub] [tvm] guberti commented on pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
guberti commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1320110246
> @mehrdadh I changed the title to add a space after the tags and tweak it a bit the wording to emphasize the new option 'serial_number' since the tweak on the tests would be a merely a consequence of it, although I do guess you're adding it for the internal test fleet.
We should also update the README.md file to explain correct usage:
- https://github.com/apache/tvm/blob/main/tests/micro/zephyr/README.md
--
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
[GitHub] [tvm] alanmacd commented on a diff in pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
alanmacd commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1030926148
##########
tests/micro/zephyr/README.md:
##########
@@ -40,3 +40,9 @@ To see the list of supported values for `--board`, run:
```
$ pytest test_zephyr.py --help
```
+
+If you like to test with a real hardware, you have the option to pass the serial number
Review Comment:
nit:
```suggestion
If you like to test with real hardware, you have the option to pass the serial number
```
--
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
[GitHub] [tvm] mehrdadh commented on a diff in pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1028608311
##########
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:
done
--
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
[GitHub] [tvm] mehrdadh commented on pull request #13377: [microTVM][Zephyr]Add serial_number test args
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1317388951
cc @areusch @guberti for review
--
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
[GitHub] [tvm] tvm-bot commented on pull request #13377: [microTVM][Zephyr]Add serial_number test args
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1314158876
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @alanmacd, @gromero <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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
[GitHub] [tvm] gromero commented on pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
gromero commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1325595186
To whom / Committer / Maintainer that will squash & merge this PR, please consider copying the text from this PR body/description and pasting it as the commit message, avoiding the commit message prepared by GH automatically that brings the review comments, it just takes 1 second and produces a clearer git log. Also in consonance with the [Commit Message Guideline](https://github.com/apache/tvm/blob/main/docs/contribute/pull_request.rst#commit-message-guideline). Thanks!
--
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
[GitHub] [tvm] gromero merged pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
gromero merged PR #13377:
URL: https://github.com/apache/tvm/pull/13377
--
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
[GitHub] [tvm] tvm-bot commented on pull request #13377: [microTVM][Zephyr]Add serial_number test args
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1314158891
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @alanmacd, @gromero <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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
[GitHub] [tvm] gromero commented on pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
gromero commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1325813519
Thanks @mehrdadh @guberti @alanmacd! It's merged.
--
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
[GitHub] [tvm] gromero commented on a diff in pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #13377:
URL: https://github.com/apache/tvm/pull/13377#discussion_r1030745182
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -310,18 +314,11 @@ def _get_nrf_device_args(options):
help=("If given, port number to use when running the local gdbserver."),
),
server.ProjectOption(
- "nrfjprog_snr",
- optional=["open_transport"],
- type="int",
- default=None,
- help=("When used with nRF targets, serial # of the attached board to use, from nrfjprog."),
- ),
- server.ProjectOption(
- "openocd_serial",
+ "serial_number",
Review Comment:
`serial_number` is also used by the `flash` method and must be advertised accordingly, so please add the method `flash` to the `optional` list below.
##########
tests/micro/zephyr/README.md:
##########
@@ -40,3 +40,9 @@ To see the list of supported values for `--board`, run:
```
$ pytest test_zephyr.py --help
```
+
+If you like to test with a real hardware, you have the option to pass the serial number
+for your development board.
+```
+$ pytes test_zephyr.py --board=nrf5340dk_nrf5340_cpuapp --serial="0672FF5"
Review Comment:
Typo: `pytest`
##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -97,6 +97,10 @@ 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 set the `CMSIS_PATH` env variable."
Review Comment:
Please, concat the two strings into a single one or wrap the line adding a space at the end of the first string, otherwise in the current state it will print this way:
```
E assert cmsis_path, CMSIS_PATH_ERROR
E AssertionError: cmsis_path is not defined! Please pass it asan option or set the `CMSIS_PATH` env variable.
```
--
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
[GitHub] [tvm] mehrdadh commented on pull request #13377: [microTVM][Zephyr] Add 'serial_number' option
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on PR #13377:
URL: https://github.com/apache/tvm/pull/13377#issuecomment-1325710946
I rebased because of this commit: https://github.com/apache/tvm/commit/8cccc253dae1474e17eca47ce67b7a34e0c41eba
--
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