guberti commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037092340
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -524,26 +531,39 @@ def _parse_connected_boards(self, tabular_str):
device[col_name] = str_row[column.start() :
column.end()].strip()
yield device
- def _auto_detect_port(self, arduino_cli_cmd: str, board: str) -> str:
- list_cmd = [self._get_arduino_cli_cmd(arduino_cli_cmd), "board",
"list"]
- list_cmd_output = subprocess.run(
- list_cmd, check=True, stdout=subprocess.PIPE
- ).stdout.decode("utf-8")
-
- desired_fqbn = self._get_fqbn(board)
- for device in self._parse_connected_boards(list_cmd_output):
- if device["fqbn"] == desired_fqbn:
- return device["port"]
+ def _auto_detect_port(self, arduino_cli_cmd: str, board: str,
serial_number) -> str:
Review Comment:
Type annotation for serial_number?
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -524,26 +531,39 @@ def _parse_connected_boards(self, tabular_str):
device[col_name] = str_row[column.start() :
column.end()].strip()
yield device
- def _auto_detect_port(self, arduino_cli_cmd: str, board: str) -> str:
- list_cmd = [self._get_arduino_cli_cmd(arduino_cli_cmd), "board",
"list"]
- list_cmd_output = subprocess.run(
- list_cmd, check=True, stdout=subprocess.PIPE
- ).stdout.decode("utf-8")
-
- desired_fqbn = self._get_fqbn(board)
- for device in self._parse_connected_boards(list_cmd_output):
- if device["fqbn"] == desired_fqbn:
- return device["port"]
+ def _auto_detect_port(self, arduino_cli_cmd: str, board: str,
serial_number) -> str:
+ if not serial_number:
+ # If serial_number is not set, it is assumed only one board
+ # with this type is connected to this host machine.
+ list_cmd = [self._get_arduino_cli_cmd(arduino_cli_cmd), "board",
"list"]
+ list_cmd_output = subprocess.run(
+ list_cmd, check=True, stdout=subprocess.PIPE
+ ).stdout.decode("utf-8")
+
+ desired_fqbn = self._get_fqbn(board)
+ for device in self._parse_connected_boards(list_cmd_output):
+ if device["fqbn"] == desired_fqbn:
+ device_port = device["port"]
+ break
+ else:
+ com_ports = serial.tools.list_ports.comports()
+ for port in com_ports:
+ if port.serial_number == serial_number:
+ device_port = port.device
+ if device_port:
+ return device_port
# If no compatible boards, raise an error
raise BoardAutodetectFailed()
- def _get_arduino_port(self, arduino_cli_cmd: str, board: str, port: int):
+ def _get_arduino_port(self, arduino_cli_cmd: str, board: str, port: int,
serial_number: str):
Review Comment:
Update type annotations to reflect that port and serial_number are optional.
##########
python/tvm/micro/testing/pytest_plugin.py:
##########
@@ -60,6 +60,11 @@ def pytest_addoption(parser):
"Also, it will enable debug level logging in project generation."
),
)
+ parser.addoption(
+ "--serial",
Review Comment:
I'd prefer calling this `--serial_number` - we call it `serial_number`
everywhere else in the code, and "serial" has other meanings that could be
confusing.
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -524,26 +531,39 @@ def _parse_connected_boards(self, tabular_str):
device[col_name] = str_row[column.start() :
column.end()].strip()
yield device
- def _auto_detect_port(self, arduino_cli_cmd: str, board: str) -> str:
- list_cmd = [self._get_arduino_cli_cmd(arduino_cli_cmd), "board",
"list"]
- list_cmd_output = subprocess.run(
- list_cmd, check=True, stdout=subprocess.PIPE
- ).stdout.decode("utf-8")
-
- desired_fqbn = self._get_fqbn(board)
- for device in self._parse_connected_boards(list_cmd_output):
- if device["fqbn"] == desired_fqbn:
- return device["port"]
+ def _auto_detect_port(self, arduino_cli_cmd: str, board: str,
serial_number) -> str:
+ if not serial_number:
+ # If serial_number is not set, it is assumed only one board
+ # with this type is connected to this host machine.
+ list_cmd = [self._get_arduino_cli_cmd(arduino_cli_cmd), "board",
"list"]
+ list_cmd_output = subprocess.run(
+ list_cmd, check=True, stdout=subprocess.PIPE
+ ).stdout.decode("utf-8")
+
+ desired_fqbn = self._get_fqbn(board)
+ for device in self._parse_connected_boards(list_cmd_output):
+ if device["fqbn"] == desired_fqbn:
+ device_port = device["port"]
+ break
+ else:
+ com_ports = serial.tools.list_ports.comports()
+ for port in com_ports:
+ if port.serial_number == serial_number:
+ device_port = port.device
+ if device_port:
+ return device_port
# If no compatible boards, raise an error
raise BoardAutodetectFailed()
- def _get_arduino_port(self, arduino_cli_cmd: str, board: str, port: int):
+ def _get_arduino_port(self, arduino_cli_cmd: str, board: str, port: int,
serial_number: str):
if not self._port:
if port:
self._port = port
else:
- self._port = self._auto_detect_port(arduino_cli_cmd, board)
+ self._port = self._auto_detect_port(
+ arduino_cli_cmd, board, serial_number=serial_number
Review Comment:
```suggestion
arduino_cli_cmd, board, serial_number
```
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -93,6 +93,13 @@ class BoardAutodetectFailed(Exception):
default=None,
help="Port to use for connecting to hardware.",
),
+ server.ProjectOption(
+ "serial_number",
+ optional=["open_transport", "flash"],
+ type="str",
+ default=None,
+ help=("Board serial number."),
+ ),
Review Comment:
Can we clarify in the `help` strings how the `port` and `serial_number`
options interact? What happens if we pass both options or neither?
##########
tests/micro/arduino/test_arduino_rpc_server.py:
##########
@@ -56,30 +59,50 @@ def _make_session(model, arduino_board, arduino_cli_cmd,
workspace_dir, mod, bui
def _make_sess_from_op(
- model, arduino_board, arduino_cli_cmd, workspace_dir, op_name, sched,
arg_bufs, build_config
+ model,
+ arduino_board,
+ arduino_cli_cmd,
+ workspace_dir,
+ op_name,
+ sched,
+ arg_bufs,
+ build_config,
+ serial_number: str,
Review Comment:
Fix type annotation to reflect that `serial_number` is optional. This
happens in a few other tests as well.
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -560,17 +580,21 @@ def _get_board_from_makefile(self, makefile_path:
pathlib.Path) -> str:
FLASH_MAX_RETRIES = 5
def flash(self, options):
+ import serial.tools.list_ports
Review Comment:
Add a comment explaining why we can't have this at the top with the rest of
the imports?
##########
python/tvm/micro/testing/pytest_plugin.py:
##########
@@ -60,6 +60,11 @@ def pytest_addoption(parser):
"Also, it will enable debug level logging in project generation."
),
)
+ parser.addoption(
+ "--serial",
+ default=None,
+ help="If set true, use the FVP emulator to run the test",
Review Comment:
Fix `help` string?
##########
python/tvm/micro/testing/pytest_plugin.py:
##########
@@ -130,3 +135,8 @@ def pytest_configure(config):
"markers",
"skip_boards(board): skip test for the given board",
)
+
+
Review Comment:
We should also update `tests/micro/arduino/README.md` to mention that you
can pass in a 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]