Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[microTVM][Arduino]Add serial_number to project options and tests #13518

Merged
merged 11 commits into from
Dec 5, 2022
+635 −109
Merged

[microTVM][Arduino]Add serial_number to project options and tests #13518

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
00a074c
add serial_number to arduino
mehrdadh Nov 30, 2022
c1bc9c4
fix error
mehrdadh Nov 30, 2022
acc7791
fix import
mehrdadh Nov 30, 2022
50cfec0
address comments
mehrdadh Dec 1, 2022
3986d2b
fix bug
mehrdadh Dec 1, 2022
5446612
lint
mehrdadh Dec 1, 2022
f617a48
fix string bug
mehrdadh Dec 1, 2022
305adbc
add pyusb to dependecies
mehrdadh Dec 1, 2022
a163072
address last comments
mehrdadh Dec 2, 2022
b760ca6
address Andrew comment
mehrdadh Dec 2, 2022
a5b0210
lint
mehrdadh Dec 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 45 additions & 14 deletions apps/microtvm/arduino/template_project/microtvm_api_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,17 @@ class BoardAutodetectFailed(Exception):
optional=["flash", "open_transport"],
type="int",
default=None,
help="Port to use for connecting to hardware.",
help=(
"Port to use for connecting to hardware. "
"If port and serial_number options are not set it will try to autodetect the port."
),
),
server.ProjectOption(
"serial_number",
optional=["open_transport", "flash"],
type="str",
default=None,
help=("Board serial number. If serial_number option is set, port option is ignored."),
),
mehrdadh marked this conversation as resolved.
Show resolved Hide resolved
]

Expand Down Expand Up @@ -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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

investigating this

Copy link
Member Author

@mehrdadh mehrdadh Dec 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm but isn't device_port only set if we find a valid port?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. Are you suggesting to just return it in the for loop?


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 = None, serial_number: str = None
):
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)

return self._port

Expand All @@ -565,12 +591,14 @@ def flash(self, options):
warning_as_error = options.get("warning_as_error")
port = options.get("port")
board = options.get("board")
serial_number = options.get("serial_number")

if not board:
board = self._get_board_from_makefile(API_SERVER_DIR / MAKEFILE_FILENAME)

cli_command = self._get_arduino_cli_cmd(arduino_cli_cmd)
self._check_platform_version(cli_command, warning_as_error)
port = self._get_arduino_port(cli_command, board, port)
port = self._get_arduino_port(cli_command, board, port, serial_number)

upload_cmd = ["make", "flash", f"PORT={port}"]
for _ in range(self.FLASH_MAX_RETRIES):
Expand All @@ -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
Comment on lines +637 to 639
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify this comment a bit? I don't understand how this fixes the GPU docker on tutorials.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#13540

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


# List all used project options
arduino_cli_cmd = options.get("arduino_cli_cmd")
port = options.get("port")
board = options.get("board")
serial_number = options.get("serial_number")

if not board:
board = self._get_board_from_makefile(API_SERVER_DIR / MAKEFILE_FILENAME)

# Zephyr example doesn't throw an error in this case
if self._serial is not None:
return

port = self._get_arduino_port(arduino_cli_cmd, board, port)
port = self._get_arduino_port(arduino_cli_cmd, board, port, serial_number)

# It takes a moment for the Arduino code to finish initializing
# and start communicating over serial
Expand Down
Loading