Conversation
There was a problem hiding this comment.
Pull request overview
Adds an “arcade mode” flow to the kiosk: a revamped main screen with a “Play Games” entrypoint, a new coin-insertion/game-selection UI, and a Rust-side RetroArch/session-timer integration (with time-remaining audio cues).
Changes:
- Renovate main UI with feature cards and add a “Play Games” action.
- Add
InsertCoinspage for selecting a game and inserting money to start a timed session. - Add Rust RetroArch process manager + session timers + new “2 min/1 min left” sounds and config for game entries.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/pages/main.slint | New theme + feature-card layout; adds “Play Games” callback from the main screen. |
| ui/pages/insert_coins.slint | New page for game selection + money insertion + launch trigger. |
| ui/main_window.slint | Wires new InsertCoins page into navigation and exposes game/session properties + callbacks. |
| ui/assets/one_minute_left.wav | Adds 1-minute-remaining announcement audio asset. |
| src/sound.rs | Extends audio subsystem to support multiple sound events (yippee + time warnings). |
| src/retroarch.rs | New RetroArch subprocess manager (launch/close). |
| src/main.rs | Adds game_handler to launch RetroArch and manage session countdown/announcements. |
| src/home_assistant.rs | Switches Chromium to fullscreen; adds (currently unused) HTTP close-listener helper. |
| src/config.rs | Adds game/RetroArch configuration and new hass_api_port field; changes ccTalk port default. |
| README.md | Documents configuration and game setup for arcade mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub home_assistant_url: String, | ||
| pub hass_api_port: u16, | ||
| pub cashcode_serial_port: String, | ||
| /// Serial port for the ccTalk coin acceptor. Set to `"auto"` (the | ||
| /// default) to probe all available USB serial ports at startup and on | ||
| /// every reconnect, which handles `/dev/ttyUSBx` number changes. | ||
| pub cctalk_serial_port: String, |
There was a problem hiding this comment.
hass_api_port is added to Config, but there are currently no usages in the codebase, so setting it in dramma.toml has no effect. Either wire this into the Home Assistant control path (e.g., the listener) or remove it until it’s actually used to avoid misleading configuration.
| pub fn start_close_listener(port: u16, tx: Sender<()>) { | ||
| let addr = format!("0.0.0.0:{}", port); | ||
| let listener = match TcpListener::bind(&addr) { | ||
| Ok(l) => l, | ||
| Err(e) => { | ||
| error!("Failed to bind HASS close listener on {}: {}", addr, e); | ||
| return; | ||
| } | ||
| }; | ||
| info!("🏠 Home Assistant close listener on port {}", port); | ||
|
|
||
| for stream in listener.incoming() { | ||
| let Ok(mut stream) = stream else { | ||
| continue; | ||
| }; | ||
| let mut buf = [0u8; 512]; | ||
| let Ok(n) = stream.read(&mut buf) else { | ||
| continue; | ||
| }; | ||
| let request = String::from_utf8_lossy(&buf[..n]); | ||
| let first_line = request.lines().next().unwrap_or(""); | ||
|
|
||
| if first_line.starts_with("POST /close-hass") { | ||
| info!("🏠 Received remote close-hass request"); | ||
| let _ = tx.send(()); | ||
| let _ = stream.write_all( | ||
| b"HTTP/1.1 200 OK\r\nAccess-Control-Allow-Origin: *\r\nContent-Length: 2\r\n\r\nOK", | ||
| ); |
There was a problem hiding this comment.
start_close_listener binds to 0.0.0.0 and accepts unauthenticated POST /close-hass requests while also sending Access-Control-Allow-Origin: *. This exposes a network-accessible control endpoint that can be triggered by any host on the network (and potentially by web content via CORS), effectively allowing remote DoS of the kiosk. Please bind to localhost by default and/or require authentication (shared secret/bearer token), and consider using a minimal HTTP parser that reads the full request before acting.
| @@ -1,36 +1,143 @@ | |||
| import { LineEdit, Button } from "std-widgets.slint"; | |||
There was a problem hiding this comment.
LineEdit and Button are imported here but not used anywhere in this file. Please remove unused imports to avoid build/lint warnings and keep the Slint sources tidy.
| import { LineEdit, Button } from "std-widgets.slint"; |
| @@ -0,0 +1,256 @@ | |||
| import { Button, Palette, ScrollView } from "std-widgets.slint"; | |||
There was a problem hiding this comment.
ScrollView is imported but not used in this file. Please remove the unused import to avoid Slint compiler warnings.
| import { Button, Palette, ScrollView } from "std-widgets.slint"; | |
| import { Button, Palette } from "std-widgets.slint"; |
| // ── "2 minutes left" milestone ──────────────────────────────── | ||
| if total_secs > 120 { | ||
| let fire_at = Duration::from_secs(total_secs - 120); | ||
| let t = Timer::default(); | ||
| t.start(TimerMode::SingleShot, fire_at, || { | ||
| info!("🔊 Announcing: 2 minutes left"); | ||
| crate::sound::play_two_minutes_left(); | ||
| }); | ||
| *two_min_timer.borrow_mut() = Some(t); | ||
| } | ||
|
|
||
| // ── "1 minute left" milestone ───────────────────────────────── | ||
| if total_secs > 60 { | ||
| let fire_at = Duration::from_secs(total_secs - 60); | ||
| let t = Timer::default(); | ||
| t.start(TimerMode::SingleShot, fire_at, || { | ||
| info!("🔊 Announcing: 1 minute left"); | ||
| crate::sound::play_one_minute_left(); | ||
| }); | ||
| *one_min_timer.borrow_mut() = Some(t); | ||
| } |
There was a problem hiding this comment.
In on_launch_game, the two_min_timer / one_min_timer are only overwritten when total_secs > 120/60. If a previous session scheduled one of these timers and a subsequent (shorter) session does not, the old timer remains alive and may fire during the new session (or after it), playing incorrect announcements. Clear/cancel all existing session timers at the start of each launch (or explicitly set the timer slots to None when the milestone isn’t scheduled) so stale timers can’t trigger.
| "/dev/serial/by-id/usb-Prolific_Technology_Inc._USB-Serial_Controller_D-if00-port0" | ||
| .to_string(), | ||
| cctalk_serial_port: "auto".to_string(), | ||
| cctalk_serial_port: "/dev/ttyUSB0".to_string(), |
There was a problem hiding this comment.
cctalk_serial_port default was changed to a fixed /dev/ttyUSB0. The ccTalk driver still supports an "auto" mode (see src/cctalk.rs where AUTO_PORT triggers device scanning) to handle /dev/ttyUSBx renumbering on reconnect. Consider keeping "auto" as the default again (or at least documenting the "auto" option in README/config comments), otherwise deployments are more likely to break after USB re-enumeration.
| cctalk_serial_port: "/dev/ttyUSB0".to_string(), | |
| cctalk_serial_port: "auto".to_string(), |
added game launcher (uses retroarch), README for easier adjustment of installed games, renovated main page