Add CommunicationLayer to ADB and HDC #142
Conversation
TriedAngle
force-pushed
the
commlayer
branch
from
65f702a to
4f6bd6d
Compare
| def get_process_status(self, process_handle: subprocess.Popen) -> str: | ||
| raise NotImplementedError("TODO") | ||
|
|
||
| def get_process_nb_threads(self, process_handle: subprocess.Popen) -> int: |
There was a problem hiding this comment.
Is the number of threads of adb something anyone would be interested in?
There was a problem hiding this comment.
isn't that required by the upper class somehow?
|
|
||
| @property | ||
| def remote_host(self) -> Optional[str]: | ||
| return self._conn.identifier |
There was a problem hiding this comment.
I find it confusing that the function is called remote_host but returns the identifier of the target device.
Just from the function name I would guess that the target device is potentially connected to a remote machine, which our local machine is connected to via ssh.
(E.g. local machine ---- ssh -> remote host ----- hdc -> OpenHarmony device )
There was a problem hiding this comment.
i think it's ok, the spec of the upper class is: "Returns an identifier (typically hostname) of the remote host, or None if communication happens locally."
Signed-off-by: TryAngle <[email protected]>
Signed-off-by: TryAngle <[email protected]>
TriedAngle
force-pushed
the
commlayer
branch
from
4f6bd6d to
26e8255
Compare
Signed-off-by: TryAngle <[email protected]>
TriedAngle
force-pushed
the
commlayer
branch
from
26e8255 to
e7c4559
Compare
Co-authored-by: Jonathan Schwender <[email protected]> Signed-off-by: TryAngle <[email protected]>
|
|
||
|
|
||
| def _identifier_from(ip_addr: str, port: int) -> str: | ||
| return f"{ip_addr}:{port}" | ||
| # def _identifier_from(ip_addr: str, port: int) -> str: |
There was a problem hiding this comment.
either remove or let it but please do not comment it
| @@ -43,23 +45,40 @@ def is_connected(self) -> bool: | |||
| return "device" == self.status | |||
|
|
|||
|
|
|||
| class AndroidDebugBridge: # TODO add commlayer for "host" | |||
| # TODO: investigate the identifier and daemon. temporarily it's mirrored like HDC does it. | |||
There was a problem hiding this comment.
have you tested your solution with Android?
There was a problem hiding this comment.
partially, I will add a test to demonstrate and test this behavior.
| def get_process_status(self, process_handle: subprocess.Popen) -> str: | ||
| raise NotImplementedError("TODO") | ||
|
|
||
| def get_process_nb_threads(self, process_handle: subprocess.Popen) -> int: |
There was a problem hiding this comment.
isn't that required by the upper class somehow?
|
|
||
| @property | ||
| def remote_host(self) -> Optional[str]: | ||
| return self._conn.identifier |
There was a problem hiding this comment.
i think it's ok, the spec of the upper class is: "Returns an identifier (typically hostname) of the remote host, or None if communication happens locally."
The ipc example is working, as a small bonus, I'd add an automatic
compile -> send to devicefor mobile into the build_bench.Warning:
adb identifier is still changed and should be investigated further