Skip to content

Feature/web based onboarding#9

Open
DonSidro wants to merge 3 commits intoPython-roborock:mainfrom
DonSidro:feature/web_based_onboarding
Open

Feature/web based onboarding#9
DonSidro wants to merge 3 commits intoPython-roborock:mainfrom
DonSidro:feature/web_based_onboarding

Conversation

@DonSidro
Copy link
Copy Markdown
Contributor

Added a web based onboarding to help people that doesn't/dont know how to use CLI.

Also updated onboarding.md to reflect the web based onboarding.

Configure Page
Skærmbillede 2026-04-17 002829
Select vacuum Page
Skærmbillede 2026-04-17 002940
Send onboarding Page
Skærmbillede 2026-04-17 003140
Reconnect and poll Page
Skærmbillede 2026-04-17 004109
Done Page
Skærmbillede 2026-04-17 004006

Has been tested on one device onboarding, might need more testing if possible.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a browser-based onboarding flow (served from localhost) to guide users through vacuum pairing without requiring CLI interaction, and updates the onboarding docs accordingly.

Changes:

  • Introduces a standalone FastAPI + Uvicorn local web server (start_onboarding_gui.py) implementing the onboarding state machine and HTTP API.
  • Adds a single-page web UI (ui.html) that polls state, renders step-by-step guidance, and shows a live log.
  • Updates documentation (docs/onboarding.md) to describe how to run and use the new web UI onboarding flow.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File Description
ui.html New browser UI for configuring, selecting a device, sending onboarding, polling, and viewing logs.
start_onboarding_gui.py New local FastAPI server + worker thread implementing the onboarding flow and serving UI/state.
docs/onboarding.md Documents the GUI onboarding workflow, phases, and troubleshooting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread start_onboarding_gui.py
api = RemoteOnboardingApi(
base_url=config.api_base_url,
admin_password=config.admin_password,
ssl_context=ssl._create_unverified_context(),
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

The GUI onboarding client disables TLS certificate verification by using ssl._create_unverified_context() when talking to the main server. This exposes the admin password/session cookies to MITM and differs from the CLI onboarding script which uses default verification. Please remove the unverified context (or make it an explicit opt-in for self-signed deployments, e.g. a toggle/flag + clear warning).

Suggested change
ssl_context=ssl._create_unverified_context(),

Copilot uses AI. Check for mistakes.
Comment thread start_onboarding_gui.py


@app.get("/", response_class=HTMLResponse)
async def index(request: Request) -> HTMLResponse:
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

The / (index) route returns the UI HTML without calling _check_token, but the docs state the token is required on every request. Consider requiring the token for the index page too (or adjust the docs) so that access control is consistent across all endpoints.

Suggested change
async def index(request: Request) -> HTMLResponse:
async def index(request: Request) -> HTMLResponse:
_check_token(request)

Copilot uses AI. Check for mistakes.
Comment thread docs/onboarding.md
3. Starts a local server bound to localhost only.
4. Opens your default browser to `http://127.0.0.1:<port>/?token=<token>`.

The server is not reachable from your LAN. The token is required on every request, so other processes or browser tabs on the same machine cannot poke at it either. If the browser does not open on its own, copy the URL printed in the terminal (including the `?token=...` part) and open it manually.
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

This section says the token is required on every request, but the GUI server currently serves / without token validation (only the /api/* endpoints enforce _check_token). Either protect the index route as well or soften this statement so the documentation matches the implementation.

Suggested change
The server is not reachable from your LAN. The token is required on every request, so other processes or browser tabs on the same machine cannot poke at it either. If the browser does not open on its own, copy the URL printed in the terminal (including the `?token=...` part) and open it manually.
The server is not reachable from your LAN. The token is included in the launch URL and is required for the protected UI/API requests, so other processes or browser tabs on the same machine cannot drive the onboarding flow unless they have that URL. If the browser does not open on its own, copy the URL printed in the terminal (including the `?token=...` part) and open it manually.

Copilot uses AI. Check for mistakes.
Comment thread ui.html
Comment on lines +719 to +723
async function quit() {
if (!confirm("Shut down the local server?")) return;
try { await api("POST", "/api/quit"); } catch {}
document.body.innerHTML = '<div class="wrap"><div class="panel"><div class="panel-body"><h2>Server stopped</h2><div class="note">You can close this tab now.</div></div></div></div>';
}
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

quit() replaces document.body.innerHTML but the 1s polling interval keeps running afterwards, leading to repeated errors when poll() tries to touch DOM nodes that no longer exist. Consider storing the interval id and clearing it (and/or using a stopped flag) when quitting so polling stops cleanly.

Copilot uses AI. Check for mistakes.
Comment thread start_onboarding_gui.py
Comment on lines +392 to +396
sock.sendto(wifi_pkt, target)
output.write(f"TOKEN_S={token_s}\n")
output.write(f"TOKEN_T={token_t}\n")
output.write(f"WIFI_BODY_SENT={json.dumps(body, separators=(',', ':'))}\n")

Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

The UI log currently includes WIFI_BODY_SENT=... which serializes and displays the full onboarding payload, including the home Wi‑Fi password (passwd). Since the log is shown in the browser and returned via /api/state, please redact sensitive fields (Wi‑Fi password, tokens) before logging to reduce accidental disclosure (screenshots/copy-paste, etc.).

Copilot uses AI. Check for mistakes.
Comment thread start_onboarding_gui.py
Comment on lines +462 to +464
def snapshot(self) -> list[dict[str, Any]]:
return [{"ts": e.ts, "level": e.level, "msg": e.msg} for e in self._entries]

Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

_UILogBuffer.snapshot() reads _entries while the worker thread appends to it, but there’s no lock around these operations. Consider adding synchronization (e.g., a threading.Lock inside _UILogBuffer or reusing _state_lock) to make log snapshotting thread-safe.

Copilot uses AI. Check for mistakes.
Comment thread start_onboarding_gui.py
Comment on lines +860 to +863
_INDEX_HTML = (Path(__file__).parent / "ui.html").read_text(encoding="utf-8")

app = FastAPI(title="Vacuum Onboarding", docs_url=None, redoc_url=None, openapi_url=None)

Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

This adds a new FastAPI-driven GUI onboarding flow but there are no tests covering its API/state-machine behavior. Since the repo already tests the CLI onboarding flow, consider adding a focused test module for start_onboarding_gui.py (e.g., _build_config_from_payload validation and basic /api/* token enforcement + a couple of phase transitions via FastAPI TestClient).

Copilot uses AI. Check for mistakes.
@Lash-L
Copy link
Copy Markdown
Contributor

Lash-L commented Apr 17, 2026

Will test this out in a few days! I have a few things i want to get working that I will need to reauth my vacuum for, so that will be a perfect time for me to test it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants