Replace bundled ROM extraction with runtime import/download flow
Commodore ROMs are copyrighted and can no longer ship inside the APK. MainActivity now detects missing ROMs on first launch and blocks startup with a dialog offering two paths: pick files via the system file picker, or download the official VICE 3.8 tarball and extract the four ROMs from it client-side (minimal USTAR reader, no extra deps). - Drop the extractViceRoms Gradle task and asset bundling; add res/extract_roms.sh for local sideloading during development instead - gitignore keystores/keystore.properties ahead of a signed release - Move architecture.md into docs/, refresh it for the screen-mirroring and watch-input additions, and add accompanying Mermaid diagrams - Add docs/debugging.md and docs/publish.md (Play Store release notes, ROM-import compliance rationale)
This commit is contained in:
ml
@@ -0,0 +1,222 @@
|
||||
# Debugging guide
|
||||
|
||||
How to get diagnostic output from both apps, plus a catalog of real bugs hit
|
||||
during development — each written as symptom → diagnosis → fix, since that's
|
||||
the order you'll actually encounter them in. See `CLAUDE.md` for build/install
|
||||
commands and `docs/architecture.md` for what each component does.
|
||||
|
||||
## 1. Getting logs
|
||||
|
||||
### Watch app
|
||||
|
||||
```bash
|
||||
pebble logs --phone <phone-ip>
|
||||
```
|
||||
|
||||
Streams both the C app's `APP_LOG`/`text_layer` state changes and the
|
||||
PebbleKit JS `console.log` output from inside Core for Pebble, interleaved.
|
||||
This is the only place JS errors (failed `fetch`, JSON parse errors,
|
||||
`sendAppMessage` failures) show up — they are otherwise silent on the watch.
|
||||
|
||||
### Companion app
|
||||
|
||||
```bash
|
||||
adb logcat | grep -E 'ViceJNI|SuM'
|
||||
```
|
||||
|
||||
- Tag `ViceJNI` (`vice_jni.c`) carries two kinds of lines:
|
||||
- Direct `LOGI`/`LOGE` calls from our own JNI code, e.g.
|
||||
`machine_read_snapshot → 0`.
|
||||
- Everything VICE itself prints to stdout/stderr, redirected through a pipe
|
||||
and re-emitted with a `VICE: ` prefix by `stderr_reader_thread`.
|
||||
**These two categories run on different threads** — see §3.3, it's a useful
|
||||
fact when you need to tell whether code is actually running on the VICE
|
||||
thread.
|
||||
- Tag `SuM` (`MainActivity.kt`, `TAG` constant) carries UI-level events: watch
|
||||
key presses, disk load/attach results, HTTP server start/stop.
|
||||
|
||||
## 2. Watch app pitfalls
|
||||
|
||||
### 2.1 App shows splash, then exits back to the watch's launcher
|
||||
|
||||
**Symptom:** the splash window appears, the timer fires, and instead of
|
||||
transitioning to the main screen the app quits entirely (back to the watch
|
||||
face or app list) — not a crash log, just gone.
|
||||
|
||||
**Diagnosis:** the Pebble window stack was briefly empty. The original splash
|
||||
implementation did:
|
||||
|
||||
```c
|
||||
static void splash_timeout_handler(void *data) {
|
||||
window_stack_push(s_main_window, true); // queued, not necessarily applied yet
|
||||
window_stack_remove(s_splash_window, false);
|
||||
}
|
||||
```
|
||||
|
||||
Pebble's own docs warn: *"If there are no windows for the app left on the
|
||||
stack, the app will be killed by the system, shortly."* Push and remove are
|
||||
not guaranteed to be atomic with respect to each other from the app's
|
||||
perspective, so this push-then-remove ordering can race.
|
||||
|
||||
**Fix:** never let the stack reach zero windows, even momentarily. Push the
|
||||
*replacement* window first and keep it on the stack permanently underneath
|
||||
the transient one, so the timeout handler only ever pops — it never has to
|
||||
push-and-remove in the same breath:
|
||||
|
||||
```c
|
||||
// init(): main_window pushed first, splash pushed on top of it.
|
||||
window_stack_push(s_main_window, true);
|
||||
...
|
||||
window_stack_push(s_splash_window, true);
|
||||
|
||||
// timeout handler: main_window is still underneath, so the stack never empties.
|
||||
static void splash_timeout_handler(void *data) {
|
||||
window_stack_remove(s_splash_window, true);
|
||||
}
|
||||
```
|
||||
|
||||
### 2.2 AppMessage silently never arrives / `sendAppMessage failed`
|
||||
|
||||
**Symptom:** PebbleKit JS logs a `sendAppMessage failed` error, or nothing at
|
||||
all and the watch's `TextLayer` never updates.
|
||||
|
||||
**Diagnosis checklist:**
|
||||
- **Inbox too small.** The watch's `app_message_open(inboxSize, outboxSize)`
|
||||
inbox must fit the *largest* payload plus dictionary overhead. The screen
|
||||
mirror payload is up to ~2080 bytes (40×25 cells, up to 2 UTF-8 bytes each
|
||||
for umlaut overrides, + 25 newlines) — the inbox is opened at 2200 bytes to
|
||||
leave headroom. If you add a new larger payload type, bump this.
|
||||
- **Symbolic keys.** `{ 'SCREEN': value }` does not reliably resolve via
|
||||
`package.json`'s `messageKeys` under Core for Pebble. Always use the
|
||||
hardcoded numeric key, identically defined in both the C `enum` and
|
||||
`index.js` (`KEY_SCREEN = 2` in both places).
|
||||
- **`localhost` vs `127.0.0.1`.** Core for Pebble's JS runtime does not
|
||||
resolve `localhost` — `pkjs`'s `fetch`/`XMLHttpRequest` calls must target
|
||||
`127.0.0.1:8888`.
|
||||
|
||||
### 2.3 Pebble build fails with a Python traceback mentioning `relpath`
|
||||
|
||||
**Symptom:**
|
||||
|
||||
```
|
||||
AttributeError: 'NoneType' object has no attribute 'relpath'
|
||||
File ".../waflib/extras/process_sdk_resources.py", line 22, in find_most_specific_filename
|
||||
```
|
||||
|
||||
**Diagnosis:** the Pebble SDK's build system hardcodes the resource folder
|
||||
name to `resources/` (`bld.path.find_node("resources")` in
|
||||
`pebble_sdk.py`) — *not* `res/`. If `package.json`'s `resources.media[].file`
|
||||
points at a file that doesn't exist under a real top-level `resources/`
|
||||
folder, `resources_node` resolves to `None` and the path-join call inside the
|
||||
SDK throws this traceback instead of a normal "file not found" error.
|
||||
|
||||
**Fix:** keep all `media` resource files (PNGs, etc.) under a top-level
|
||||
`resources/` directory, never `res/`.
|
||||
|
||||
## 3. Companion app / VICE pitfalls
|
||||
|
||||
### 3.1 Snapshot restore: screen looks right, but buttons do nothing
|
||||
|
||||
**Symptom:** after `loadState()`, the saved screen renders correctly (memory,
|
||||
VIC-II, SID, CIA state are all genuinely restored), but the game never
|
||||
responds to keyboard/joystick input again.
|
||||
|
||||
**Diagnosis:** `maincpu_mainloop()` keeps the 6510's registers as
|
||||
**stack-local C variables** (`reg_pc`, `reg_a`, ...), not in the global
|
||||
`maincpu_regs` struct. They're synced only inside `DO_INTERRUPT()`'s
|
||||
`EXPORT_REGISTERS()` / `IMPORT_REGISTERS()` macros. Calling
|
||||
`machine_write_snapshot()` / `machine_read_snapshot()` directly from
|
||||
`video_canvas_refresh()` (i.e. outside an interrupt) means:
|
||||
- **Save** records whatever `maincpu_regs.pc` happened to hold from the *last*
|
||||
interrupt — stale, not the actual current PC.
|
||||
- **Load** writes the saved PC into `maincpu_regs.pc`, but the CPU loop keeps
|
||||
running from its own unrelated `reg_pc` — the restored PC never takes
|
||||
effect.
|
||||
|
||||
The CPU ends up executing from a PC that has nothing to do with the saved
|
||||
game state. Screen/CIA/SID content looks fine because *those* are restored
|
||||
correctly; only the actual instruction pointer is wrong, which silently
|
||||
breaks "the game keeps running but never processes input" without crashing.
|
||||
|
||||
**Fix:** route both save and load through `interrupt_maincpu_trigger_trap()`,
|
||||
so the snapshot call happens inside `DO_INTERRUPT(IK_TRAP)` and gets the
|
||||
`EXPORT_REGISTERS()`/`IMPORT_REGISTERS()` sync for free. See
|
||||
`save_state_trap()` / `load_state_trap()` in `vice_jni.c`, and
|
||||
`docs/architecture.md` §4.4 for the full sequence diagram.
|
||||
|
||||
**How this was actually diagnosed:** by comparing thread IDs in `adb logcat`.
|
||||
`LOGI("machine_read_snapshot → %d", r)` (called directly from inside the trap
|
||||
function) appeared on the same TID as `android_sound_close()`'s Android
|
||||
audio-stack log lines — i.e. the VICE/CPU thread. The `VICE: ...`-prefixed
|
||||
lines (from `stderr_reader_thread`) appeared on a *different* TID. That
|
||||
confirmed the trap really was running on the CPU thread, which ruled out a
|
||||
threading mistake and pointed at the register-sync logic instead.
|
||||
|
||||
### 3.2 `VICE: Sync reset` in the log is not a C64 reset
|
||||
|
||||
**Symptom:** `VICE: Sync reset` appears in logcat right after a snapshot
|
||||
load, looking like the machine just got reset (which would explain broken
|
||||
input — but doesn't, in fact).
|
||||
|
||||
**Diagnosis:** this line comes from `vsync_suspend_speed_eval()` in
|
||||
`vsync.c`, triggered when the sound device closes/reopens during the
|
||||
snapshot's `sound_snapshot_finish()` call. It resets internal frame-timing
|
||||
statistics only — it is unrelated to `machine_reset()`/`maincpu_reset()` and
|
||||
does not touch CPU or memory state. Don't chase it as the cause of a
|
||||
post-restore bug; it's noise.
|
||||
|
||||
### 3.3 `VICE: Error - T64 snapshot support is not implemented`
|
||||
|
||||
**Symptom:** this warning appears on every snapshot load, but
|
||||
`machine_read_snapshot()` still returns `0` (success).
|
||||
|
||||
**Diagnosis:** snapshots are saved with `save_disks=0` (disk management is
|
||||
handled by our own UI, not VICE's), so the drive module writes a minimal
|
||||
entry on save. On restore, `drive_snapshot_read_module()` logs this warning
|
||||
while parsing that minimal entry but still returns success — harmless.
|
||||
|
||||
### 3.4 Disk/reset/snapshot APIs must only be called from the VICE thread
|
||||
|
||||
**Symptom:** intermittent corruption or crashes after calling `loadDisk()`,
|
||||
`attachDisk()`, `resetMachine()`, `saveState()`, or `loadState()` directly
|
||||
from the calling (UI or HTTP) thread.
|
||||
|
||||
**Diagnosis:** all five funnel into mutex-guarded pending-path buffers
|
||||
(`g_pending_disk`, `g_pending_attach`, `g_pending_reset`,
|
||||
`g_pending_save_state`, `g_pending_load_state`) that `video_canvas_refresh()`
|
||||
drains once per rendered frame, on the VICE thread. This is intentional —
|
||||
`autostart_disk()`, `file_system_attach_disk()`, `machine_trigger_reset()`,
|
||||
and the snapshot calls are not safe to call cross-thread.
|
||||
|
||||
**Fix:** never call these JNI entry points' underlying VICE functions
|
||||
directly from Kotlin; always go through the existing pending-queue pattern
|
||||
in `vice_jni.c` if adding a new one.
|
||||
|
||||
### 3.5 `injectKey()` / `getScreenText()` have no locking — is that a bug?
|
||||
|
||||
No — this is deliberate, not a bug to "fix" if you notice it while reading
|
||||
the code. Both are called directly from non-VICE threads (the UI thread for
|
||||
on-screen keyboard taps, an HTTP worker thread for watch input and the screen
|
||||
mirror poll) without synchronizing with the VICE thread. Worst case, a race
|
||||
produces one stale keyboard-matrix bit or one stale screen byte for a single
|
||||
frame, which self-corrects on the next call. This is a different category
|
||||
from §3.1/§3.4: those are correctness-critical (a wrong PC or a cross-thread
|
||||
VICE API call corrupts state permanently), this isn't.
|
||||
|
||||
## 4. Isolating the watch UI from the companion app
|
||||
|
||||
To iterate on watch-side rendering/layout without a live companion app or
|
||||
phone, simulate the AppMessage payload directly against an emulator instance:
|
||||
|
||||
```bash
|
||||
pebble install --emulator emery --vnc
|
||||
pebble send-app-message --emulator emery --vnc --string 2="<40x25 test text>"
|
||||
pebble screenshot --vnc --no-open /tmp/screen.png
|
||||
```
|
||||
|
||||
(Key `2` is `KEY_SCREEN` — see the protocol table in `docs/architecture.md`.)
|
||||
This was how word-wrap and clipping behavior were verified across screen
|
||||
sizes (emery vs. basalt) before wiring up the real HTTP/VICE pipeline. Prefer
|
||||
testing on a real watch once the companion app is in the loop — the emulator
|
||||
is best for fast, isolated layout iteration, not for verifying end-to-end
|
||||
behavior.
|
||||
Reference in New Issue
Block a user