-
Notifications
You must be signed in to change notification settings - Fork 0
Implement DroidCast screenshot backend #46
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,71 @@ | ||||||||||
| import asyncio | ||||||||||
| import base64 | ||||||||||
| import requests | ||||||||||
| from typing import Callable, Coroutine | ||||||||||
|
|
||||||||||
| # Define AdbRunFn type for type hinting | ||||||||||
| # async def _adb_run(*args, timeout) -> bytes | ||||||||||
| AdbRunFn = Callable[..., Coroutine[None, None, bytes]] | ||||||||||
|
|
||||||||||
| async def _capture_droidcast(*, adb_run: AdbRunFn, serial: str, adb_exe: str) -> str: | ||||||||||
|
||||||||||
| async def _capture_droidcast(*, adb_run: AdbRunFn, serial: str, adb_exe: str) -> str: | |
| async def _capture_droidcast(*, adb_run: AdbRunFn, _serial: str, _adb_exe: str) -> str: |
Copilot
AI
Mar 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HTTP URL uses /screenshot?format=png, but the bundled DroidCast implementation in alas_wrapped/module/device/method/droidcast.py documents PNG screenshots coming from the /preview endpoint (and /screenshot returning a raw bitmap for DroidCast_raw). Using /screenshot?format=png is likely to 404 with the APK that exists in this repo; align the endpoint with the APK/version you ship.
Copilot
AI
Mar 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function uses requests.get(...) directly, which will honor HTTP_PROXY/HTTPS_PROXY env vars by default; that can break localhost calls in proxied environments. Consider using a requests.Session() with trust_env = False (as done in alas_wrapped/module/device/method/droidcast.py) or otherwise explicitly disabling proxy use for this local request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 The Roast: The APK file you're looking for is like that friend who says they'll be there at 8 but shows up at 9 with a different name. The repo has DroidCast_raw-release-1.0.apk, but you're hunting for DroidCast-debug-1.2.3.apk. This isn't a scavenger hunt — this is going to blow up at runtime with a very confused error message.
🩹 The Fix:
| apk_path = "alas_wrapped/bin/DroidCast/DroidCast-debug-1.2.3.apk" | |
| apk_path = "alas_wrapped/bin/DroidCast/DroidCast_raw-release-1.0.apk" |
📏 Severity: critical
Copilot
AI
Mar 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apk_path points to alas_wrapped/bin/DroidCast/DroidCast-debug-1.2.3.apk, but the repo only contains alas_wrapped/bin/DroidCast/DroidCast_raw-release-1.0.apk. As written, the install step will fail at runtime; update the path (and any related logic) to reference an APK that actually exists in-repo (or resolve it dynamically).
| apk_path = "alas_wrapped/bin/DroidCast/DroidCast-debug-1.2.3.apk" | |
| apk_path = "alas_wrapped/bin/DroidCast/DroidCast_raw-release-1.0.apk" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 The Roast: except Exception: is the Python equivalent of "whatever, man." You're catching KeyboardInterrupt, SystemExit, and that one weird exception your coworker's library throws when Mercury is in retrograde. The fallback might hide real problems like a rug hides... well, things you sweep under rugs.
🩹 The Fix: Be specific — catch RuntimeError or whatever adb_run actually raises. Your future debugging self will send you a thank-you card.
📏 Severity: warning
Copilot
AI
Mar 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback app_process launch references CLASSPATH=/data/local/tmp/DroidCast.dex, but this code never pushes that file to the device, and there is no DroidCast.dex artifact in the repo. The existing ALAS droidcast launcher uses the remote APK path as the CLASSPATH; this fallback path/class should be updated so it can actually work on a fresh device.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 The Roast: You're wrapping an exception in RuntimeError like a bad Christmas gift, but you kept the receipt... just not the original wrapping paper. The original traceback gets lost in translation, so when this fails in production, you'll be playing detective with half the clues.
🩹 The Fix: Use raise RuntimeError(f"DroidCast HTTP failure: {e}") from e to preserve the chain. Or just let it propagate if the message is clear enough.
📏 Severity: suggestion
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,124 @@ | ||||||
| import pytest | ||||||
| import unittest.mock as mock | ||||||
| import base64 | ||||||
| import io | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔥 The Roast: 🩹 The Fix: ```suggestion
|
||||||
| import io |
Copilot
AI
Mar 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Root pytest.ini restricts collection to testpaths = agent_orchestrator, so this new adb_vision/test_server.py won’t run under the default pytest invocation. To ensure the new DroidCast backend stays covered in CI, either move these tests under agent_orchestrator/ (or tests/) or update the test configuration accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 The Roast: This test is asserting that a file exists at a path that... doesn't exist. It's like testing that your house keys work on a door that's not your house. The test will pass because you're mocking adb_run, but it's validating a hardcoded path that's wrong — the classic "the tests pass but the code doesn't work" scenario.
🩹 The Fix: Update to match the actual APK filename: alas_wrapped/bin/DroidCast/DroidCast_raw-release-1.0.apk
📏 Severity: warning
Copilot
AI
Mar 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test asserts an install call using alas_wrapped/bin/DroidCast/DroidCast-debug-1.2.3.apk, but that APK isn’t present in the repo (only DroidCast_raw-release-1.0.apk exists). Once the implementation is corrected to use a real APK path, update this assertion to match.
| adb_run.assert_any_call("install", "-r", "alas_wrapped/bin/DroidCast/DroidCast-debug-1.2.3.apk", timeout=30.0) | |
| adb_run.assert_any_call("install", "-r", "alas_wrapped/bin/DroidCast/DroidCast_raw-release-1.0.apk", timeout=30.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 The Roast: This function signature is like bringing a passport to a domestic flight — technically you're prepared for anything, but you're not actually going anywhere.
serialandadb_exeare just sitting there, unused, taking up space in the parameter list like that one camping chair nobody sits in.🩹 The Fix: Either use them (pass to
adb_runif needed) or remove them. Ifadb_runis already bound to a serial, document that.📏 Severity: suggestion