csullivan commented on a change in pull request #10581:
URL: https://github.com/apache/tvm/pull/10581#discussion_r833921897



##########
File path: python/tvm/contrib/hexagon/session.py
##########
@@ -37,10 +40,12 @@ class Session:
 

Review comment:
       Can we update the docstring to reflect the API change?

##########
File path: tests/python/contrib/test_hexagon/conftest.py
##########
@@ -59,27 +64,134 @@ def requires_hexagon_toolchain(*args):
 
 
 @tvm.testing.fixture
-def android_serial_number() -> str:
-    return os.getenv(ANDROID_SERIAL_NUMBER, default=None)
+def android_serial_number() -> Optional[str]:
+    serial = os.getenv(ANDROID_SERIAL_NUMBER, default="")
+    # Setting ANDROID_SERIAL_NUMBER to an empty string should be
+    # equivalent to having it unset.
+    if not serial.strip():
+        serial = None
+    return serial
 
 
[email protected]
-def tvm_tracker_host() -> str:
-    return os.getenv(TVM_TRACKER_HOST, default=None)
+# NOTE on server ports:
+# These tests use different port numbers for the RPC server (7070 + ...).
+# The reason is that an RPC session cannot be gracefully closed without
+# triggering TIME_WAIT state on the server socket. This prevents another
+# server to bind to the same port until the wait time elapses.
 
+listen_port_min = 2000  # Well above the privileged ports (1024 or lower)
+listen_port_max = 9000  # Below the search range end (port_end=9199) of RPC 
server
+previous_port = [None]

Review comment:
       Were you intending to track more than just the previous port given the 
list type? Only `previous_port[0]` is currently used  in the implementation. 

##########
File path: python/tvm/contrib/hexagon/build.py
##########
@@ -195,28 +195,37 @@ def start_session(self) -> Session:
             "timeout": 0,
             "key": self.HEXAGON_REMOTE_DEVICE_KEY,
         }
-        return Session(hexagon_remote_kw)
+        return Session(self, hexagon_remote_kw)
 
-    def load_module(self, module_name: Union[str, pathlib.Path], session: 
Session):
+    def load_module(self, module: Union[str, pathlib.Path], session: Session):

Review comment:
       `module` type hints need to be updated to include `tvm.runtime.Module` 
as indicated in the docstring 

##########
File path: python/tvm/contrib/hexagon/session.py
##########
@@ -37,10 +40,12 @@ class Session:
 
     def __init__(
         self,
+        launcher: "HexagonLauncherRPC",

Review comment:
       I believe you want the actual HexagonLauncherRPC type here instead of 
the string literal? 




-- 
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]


Reply via email to