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/30 01:50:07 UTC
[GitHub] [tvm] mehrdadh opened a new pull request, #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
mehrdadh opened a new pull request, #13518:
URL: https://github.com/apache/tvm/pull/13518
This PR adds serial_number as a project option to Arduino project API. In addition, it is added to micro pytest_plugin to enable testing with assigned 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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037390304
##########
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:
done. We should fix this issue. It's only because we are running microtvm tutorials in GPU where the dependencies are not installed
##########
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:
done
##########
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:
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] driazati commented on a diff in pull request #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
driazati commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1038427602
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -594,21 +622,24 @@ def flash(self, options):
)
def open_transport(self, options):
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
import serial
import serial.tools.list_ports
Review Comment:
No we never implemented anything like that, though here it might be easiest to just install `pyserial` in the GPU docker image and call it a day
--
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 merged pull request #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh merged PR #13518:
URL: https://github.com/apache/tvm/pull/13518
--
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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037390808
##########
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:
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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on PR #13518:
URL: https://github.com/apache/tvm/pull/13518#issuecomment-1331542884
cc @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] driazati commented on a diff in pull request #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
driazati commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037705898
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -594,21 +622,24 @@ def flash(self, options):
)
def open_transport(self, options):
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
import serial
import serial.tools.list_ports
Review Comment:
if this file is imported but this function isn't run during the tutorial docs build then it would fail, this fixes it with a lazy import. it would be nice to link to an issue about this though for more context
--
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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1038565162
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -524,26 +534,42 @@ 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")
+ def _auto_detect_port(self, arduino_cli_cmd: str, board: str, serial_number: str) -> str:
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
+ import serial.tools.list_ports
- desired_fqbn = self._get_fqbn(board)
- for device in self._parse_connected_boards(list_cmd_output):
- if device["fqbn"] == desired_fqbn:
- return device["port"]
+ 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()
Review Comment:
yeah. Are you suggesting to just return it in the for loop?
##########
tests/micro/arduino/README.md:
##########
@@ -33,3 +33,9 @@ To see the list of supported values for `--board`, run:
```
$ pytest --help
```
+
+If you like to test with a real hardware, you have the option to pass the serial number
Review Comment:
added 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 a diff in pull request #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037691317
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -524,26 +534,42 @@ 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")
+ def _auto_detect_port(self, arduino_cli_cmd: str, board: str, serial_number: str) -> str:
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
+ import serial.tools.list_ports
- desired_fqbn = self._get_fqbn(board)
- for device in self._parse_connected_boards(list_cmd_output):
- if device["fqbn"] == desired_fqbn:
- return device["port"]
+ 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()
Review Comment:
this was an issue. I changed `_get_arduino_port` function to address this issue. Now in this function we have this priority:
1. port
2. detect port using serial_number
3. auto detect port with arduino-cli
In 2 and 3 we show proper exception if it was not able to find the port
--
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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037688607
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -524,26 +534,42 @@ 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")
+ def _auto_detect_port(self, arduino_cli_cmd: str, board: str, serial_number: str) -> str:
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
+ import serial.tools.list_ports
- desired_fqbn = self._get_fqbn(board)
- for device in self._parse_connected_boards(list_cmd_output):
- if device["fqbn"] == desired_fqbn:
- return device["port"]
+ 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()
Review Comment:
investigating 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] guberti commented on a diff in pull request #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
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: 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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037691317
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -524,26 +534,42 @@ 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")
+ def _auto_detect_port(self, arduino_cli_cmd: str, board: str, serial_number: str) -> str:
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
+ import serial.tools.list_ports
- desired_fqbn = self._get_fqbn(board)
- for device in self._parse_connected_boards(list_cmd_output):
- if device["fqbn"] == desired_fqbn:
- return device["port"]
+ 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()
Review Comment:
this was an issue. I changed `_get_arduino_port` function to address this issue. Now in this function we have this priority:
1. port
2. detect port using serial_number
3. auto detect port with arduino-cli
In 2 and 3 we show proper exception if it was not able to find the port. Also I changed project option help messages to include this change
--
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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1038355181
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -594,21 +622,24 @@ def flash(self, options):
)
def open_transport(self, options):
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
import serial
import serial.tools.list_ports
Review Comment:
https://github.com/apache/tvm/issues/13540
--
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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037691317
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -524,26 +534,42 @@ 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")
+ def _auto_detect_port(self, arduino_cli_cmd: str, board: str, serial_number: str) -> str:
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
+ import serial.tools.list_ports
- desired_fqbn = self._get_fqbn(board)
- for device in self._parse_connected_boards(list_cmd_output):
- if device["fqbn"] == desired_fqbn:
- return device["port"]
+ 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()
Review Comment:
this was an issue. I changed `_get_arduino_port` function to address this issue. Now in this function we have this priority:
1. port
2. detect port using serial_number
3. auto detect port with arduino-cli
In 2 and 3 we show proper exception if it was not able to find the port. Also I changed project option help messages to include this change.
Also if both port and serial_number are set it will show exception
--
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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037389478
##########
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:
good point, I added more info to both help messages. It should be clear now
##########
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:
done
##########
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:
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 a diff in pull request #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037390567
##########
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:
done
##########
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:
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 a diff in pull request #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1038350222
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -594,21 +622,24 @@ def flash(self, options):
)
def open_transport(self, options):
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
import serial
import serial.tools.list_ports
Review Comment:
@driazati I remember there was some discussion about running tutorials in the related docker and only render the results in GPU/Doc stage. Did that happen?
--
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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
guberti commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037672769
##########
tests/micro/arduino/test_arduino_rpc_server.py:
##########
@@ -318,31 +354,48 @@ def test_byoc_microtvm(board, arduino_cli_cmd, microtvm_debug, workspace_dir):
arduino_board=board,
arduino_cli_cmd=arduino_cli_cmd,
workspace_dir=workspace_dir,
+ serial_number=serial_number,
)
def _make_add_sess_with_shape(
- model, arduino_board, arduino_cli_cmd, workspace_dir, shape, build_config
+ model,
+ arduino_board,
+ arduino_cli_cmd,
+ workspace_dir,
+ shape,
+ build_config,
+ serial_number: str = None,
):
A = tvm.te.placeholder(shape, dtype="int8")
C = tvm.te.compute(A.shape, lambda i: A[i] + A[i], name="C")
sched = tvm.te.create_schedule(C.op)
return _make_sess_from_op(
- model, arduino_board, arduino_cli_cmd, workspace_dir, "add", sched, [A, C], build_config
+ model,
+ arduino_board,
+ arduino_cli_cmd,
+ workspace_dir,
+ "add",
+ sched,
+ [A, C],
+ build_config,
+ serial_number,
)
@pytest.mark.parametrize(
"shape,",
[
pytest.param((1 * 1024,), id="(1*1024)"),
- pytest.param((4 * 1024,), id="(4*1024)"),
- pytest.param((16 * 1024,), id="(16*1024)"),
+ # pytest.param((4 * 1024,), id="(4*1024)"),
+ # pytest.param((16 * 1024,), id="(16*1024)"),
Review Comment:
Why did we comment these out?
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -594,21 +622,24 @@ def flash(self, options):
)
def open_transport(self, options):
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
import serial
import serial.tools.list_ports
Review Comment:
Can you clarify this comment a bit? I don't understand how this fixes the GPU docker on tutorials.
##########
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:
Rather than having one override the other, I'd prefer to raise an error if there is a conflict. Your call, though.
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -524,26 +534,42 @@ 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")
+ def _auto_detect_port(self, arduino_cli_cmd: str, board: str, serial_number: str) -> str:
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
+ import serial.tools.list_ports
- desired_fqbn = self._get_fqbn(board)
- for device in self._parse_connected_boards(list_cmd_output):
- if device["fqbn"] == desired_fqbn:
- return device["port"]
+ 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()
Review Comment:
I think this logic is bugged. If I pass an invalid `serial_number`, I get:
```
E File "/home/guberti/tvm/tests/micro/arduino/workspace_due/2022-12-01T16-09-00/project/microtvm_api_server.py", line 560, in _auto_detect_port
E if device_port:
E UnboundLocalError: local variable 'device_port' referenced before assignment
```
This makes sense, as `device_port` is never defined if `device["fqbn"] == desired_fqbn`.
--
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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037687384
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -594,21 +622,24 @@ def flash(self, options):
)
def open_transport(self, options):
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
import serial
import serial.tools.list_ports
Review Comment:
When I moved these imports to the top of the file, there was an error with doc stage in GPU. I think the reason is that we don't have dependencies for this in GPU docker. cc @driazati
--
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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1037687542
##########
tests/micro/arduino/test_arduino_rpc_server.py:
##########
@@ -318,31 +354,48 @@ def test_byoc_microtvm(board, arduino_cli_cmd, microtvm_debug, workspace_dir):
arduino_board=board,
arduino_cli_cmd=arduino_cli_cmd,
workspace_dir=workspace_dir,
+ serial_number=serial_number,
)
def _make_add_sess_with_shape(
- model, arduino_board, arduino_cli_cmd, workspace_dir, shape, build_config
+ model,
+ arduino_board,
+ arduino_cli_cmd,
+ workspace_dir,
+ shape,
+ build_config,
+ serial_number: str = None,
):
A = tvm.te.placeholder(shape, dtype="int8")
C = tvm.te.compute(A.shape, lambda i: A[i] + A[i], name="C")
sched = tvm.te.create_schedule(C.op)
return _make_sess_from_op(
- model, arduino_board, arduino_cli_cmd, workspace_dir, "add", sched, [A, C], build_config
+ model,
+ arduino_board,
+ arduino_cli_cmd,
+ workspace_dir,
+ "add",
+ sched,
+ [A, C],
+ build_config,
+ serial_number,
)
@pytest.mark.parametrize(
"shape,",
[
pytest.param((1 * 1024,), id="(1*1024)"),
- pytest.param((4 * 1024,), id="(4*1024)"),
- pytest.param((16 * 1024,), id="(16*1024)"),
+ # pytest.param((4 * 1024,), id="(4*1024)"),
+ # pytest.param((16 * 1024,), id="(16*1024)"),
Review Comment:
thanks for catching this. I will uncomment them
--
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 #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13518:
URL: https://github.com/apache/tvm/pull/13518#issuecomment-1331542564
<!---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, @leandron <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] areusch commented on a diff in pull request #13518: [microTVM][Arduino]Add `serial_number` to project options and tests
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #13518:
URL: https://github.com/apache/tvm/pull/13518#discussion_r1038560004
##########
tests/micro/arduino/README.md:
##########
@@ -33,3 +33,9 @@ To see the list of supported values for `--board`, run:
```
$ pytest --help
```
+
+If you like to test with a real hardware, you have the option to pass the serial number
Review Comment:
```suggestion
If you would like to test with a real hardware and need to target one of many identical devices, you have the option to pass the serial number
```
##########
apps/microtvm/arduino/template_project/microtvm_api_server.py:
##########
@@ -524,26 +534,42 @@ 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")
+ def _auto_detect_port(self, arduino_cli_cmd: str, board: str, serial_number: str) -> str:
+ # TODO: This is to avoid breaking GPU docker on running the tutorials.
+ import serial.tools.list_ports
- desired_fqbn = self._get_fqbn(board)
- for device in self._parse_connected_boards(list_cmd_output):
- if device["fqbn"] == desired_fqbn:
- return device["port"]
+ 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()
Review Comment:
hm but isn't device_port only set if we find a valid port?
--
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