fix(mmce): session-death resilience -- distinct errno, visible bails, VCD retry, MX4SIO settle (HW batch S3/S5/S6/S7) - #248
Conversation
…aunch bails, VCD-scan retry, MX4SIO settle gate (HW batch S3/S5/S6/S7) Four verified mechanisms from the MC-boot HW batch triage (adversarially CONFIRMED/PLAUSIBLE), all pre-existing holes EXPOSED by one degraded-MMCE session -- none of the 07-20 merges caused them: S3 -- ART DEATH IS A MISCLASSIFICATION, NOT A WEDGE. mmceman's mmce_fs_open returned ONE bare -1 for six different failures (fd-pool exhausted, three packet timeouts, garbled reply, AND the card's genuine not-found). OPL's staging arm mapped every failed open to ERR_BAD_FILE = memoized permanently absent on the fail-epoch -- so a rapid-nav contention storm branded every browsed game's art "nonexistent" for the whole session. FIX, two halves that land together: a new mmceman patch (mmceman-fs-open-enoent.patch, applied by BOTH build scripts; same content applies at both pins -- validated) makes ONLY the card's explicit bad-fd reply return -ENOENT; and the staging arm now memoizes ONLY on errno==ENOENT, everything else takes the existing transient lane (ERR_FILE_IO, lazy once-per-generation re-probe) so art SELF-HEALS when the bus quiets. Generic loose-file branch (BDM/HDD) untouched. If the newlib errno glue is ever unfaithful, worst case is a bounded re-probe. S5 -- "DOES GAMEID AND THEN NOTHING" WAS A SILENT BAIL. GameID uses fd-less devctls (they work on a degraded channel); the iso open after it failed and returned to the menu with only a LOG. Both launch bails (slot-lost + iso-open) now guiWarning before returning, the iso open gets a bounded poll-first retry (10x200ms -- ~0ms on a healthy card), and the LOG carries path+code (the #246 doctrine). S6 -- ONE CONTENDED VCD SCAN LATCHED AN EMPTY PAGE FOREVER. vcdFillGameList cannot tell ABSENT from CONTENDED (-1 both) and NOUPDATE then never rescanned. Bounded retry in the proven first-scan-sentinel shape: ~15 passes at the ~2s cadence, re-armed by a fresh L3 toggle, self-quiescing on success/expiry/card-removal. Also revives the manual refresh button in the failed state. S7 -- LIME GREEN = ee_core's marker before LoadElf from cdrom0:, i.e. cdvdman waiting (deliberately untimed, upstream design -- NOT touched) for the MX4SIO SD to enumerate. MX4SIO shares SIO2 with the MMCE, and the cross-device GameID switch still in flight through the IOP reboot can starve that enumeration. mmceSendGameID is now tri-state (-1 = switch sent, settle NOT confirmed; still truthy so the mmce-launch callers keep their own settle), remembers the exact slot it targeted, and the SDC launch leg alone gates on mmceGameIdSettle(5000) -- settled = presence answers AND busy clear-or- UNAVAILABLE (requiring a readable busy bit would stall every firmware lacking that devctl). On expiry: warn (new label, APPENDED at _base.yml end per the position rule) and LAUNCH ANYWAY -- mitigation, never a gate. Other BDM transports don't share SIO2 and pay nothing. Builds clean (make opl.elf + lang regen, exit 0). HW-pending: Nathan's rig, same session recipe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughMMCE handling now distinguishes missing files from transient errors, confirms cross-device GameID switches, retries failed VCD scans, reports launch failures, and applies the new mmceman error patch during installation. ChangesMMCE reliability updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant bdmLaunchGame
participant mmceSendGameID
participant mmceGameIdSettle
participant MMCEDevice
participant GUI
bdmLaunchGame->>mmceSendGameID: send startup GameID
mmceSendGameID->>MMCEDevice: switch target device
mmceSendGameID-->>bdmLaunchGame: failure
bdmLaunchGame->>mmceGameIdSettle: probe for up to 5000 ms
mmceGameIdSettle->>MMCEDevice: poll presence and busy state
mmceGameIdSettle-->>bdmLaunchGame: settled or expired
bdmLaunchGame->>GUI: show unsettled warning on expiry
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several robustness improvements for MMCE support, addressing issues with contended bus states, transient failures, and cross-device launches sharing SIO2 (such as MX4SIO). Key changes include returning distinct -ENOENT errors on open failures, implementing a bounded retry mechanism for VCD scans, adding a post-GameID settle gate for MX4SIO, and ensuring that launch failures are visibly reported to the user. Additionally, texture loading now distinguishes between permanent and transient failures based on errno. The review feedback suggests explicitly including <errno.h> in src/textures.c to avoid relying on transitive headers, and resetting mmceGameIdTarget at the start of mmceSendGameID to prevent stale state polling.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| readFunction = &texReadMemBoundedFunction; | ||
| } else if (filePath) { | ||
| if (texShouldUseMemoryReader(filePath)) { | ||
| errno = 0; |
There was a problem hiding this comment.
To ensure codebase robustness and prevent potential compilation failures due to future SDK header refactorings, please explicitly include <errno.h> in src/textures.c since errno and ENOENT are now used in this file. While it might currently be transitively included by other headers, relying on transitive includes is discouraged.
| int mmceSendGameID(const char *startup, const char *protectMcPath, int vmcSlotMask) | ||
| { |
There was a problem hiding this comment.
To prevent stale state from a previous successful GameID switch being polled if a subsequent switch fails or is skipped, it is safer to reset mmceGameIdTarget[0] = '\0'; at the very beginning of mmceSendGameID.
| int mmceSendGameID(const char *startup, const char *protectMcPath, int vmcSlotMask) | |
| { | |
| int mmceSendGameID(const char *startup, const char *protectMcPath, int vmcSlotMask) | |
| { | |
| mmceGameIdTarget[0] = '\0'; |
References
- When handling removable media or card swaps, ensure that static loading or caching flags are reset when the media is removed or absent. This ensures that newly inserted media can properly re-register and load its assets.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bdmsupport.c`:
- Around line 1231-1241: The MX4SIO settle gate currently runs after the
Neutrino early-return path, so Neutrino launches bypass it. Update
bdmTryNeutrinoLaunch() to perform the same mmceSendGameID failure check and
mmceGameIdSettle(5000) warning flow before teardown and launch, restricted to
bdmDriverIsMx4sio(bdmCurrentDriver), while preserving the existing gate for
other launch paths.
In `@src/mmcesupport.c`:
- Around line 569-570: Update the mmceNeedsUpdate() branch handling
mmcePrefix[0] == '\0' to reset both mmceVcdScanFailed and mmceVcdScanRetries,
matching the existing reset in the list-update path so removed and reinserted
cards receive a fresh retry budget.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 98dda537-5a46-41e9-a311-3949bded6ee7
📒 Files selected for processing (8)
.github/patches/mmceman-fs-open-enoent.patch.github/scripts/install_coherent_mmce.sh.github/scripts/patch_stock_mmceman.shinclude/mmcesupport.hlng_tmpl/_base.ymlsrc/bdmsupport.csrc/mmcesupport.csrc/textures.c
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: build-ps2dev-latest
- GitHub Check: build-debug (eesio_debug, :v20250725-2)
- GitHub Check: build-variants-ps2dev-latest (EXTRA_FEATURES=1, PADEMU=1)
- GitHub Check: build-debug-ps2dev-latest (ingame_debug)
- GitHub Check: build-debug (DTL_T10000=1, :v20250725-2)
- GitHub Check: build-debug-ps2dev-latest (iopcore_ppctty_debug)
- GitHub Check: build-debug-ps2dev-latest (iopcore_debug)
- GitHub Check: build-variants-ps2dev-latest (EXTRA_FEATURES=0, PADEMU=0)
- GitHub Check: build-debug (iopcore_ppctty_debug, :v20250725-2)
- GitHub Check: build-debug (ingame_ppctty_debug, :v20250725-2)
- GitHub Check: build
- GitHub Check: build-debug (ingame_debug, :v20250725-2)
- GitHub Check: build-variants (EXTRA_FEATURES=0, PADEMU=1)
- GitHub Check: build-variants (EXTRA_FEATURES=1, PADEMU=0)
- GitHub Check: build-debug (iopcore_debug, :v20250725-2)
- GitHub Check: build-variants (EXTRA_FEATURES=0, PADEMU=0)
- GitHub Check: build-variants (EXTRA_FEATURES=1, PADEMU=1)
🧰 Additional context used
🪛 Cppcheck (2.21.0)
src/mmcesupport.c
[style] 196-196: The function 'mmceGameIdSettle' is never used.
(unusedFunction)
🔇 Additional comments (7)
.github/patches/mmceman-fs-open-enoent.patch (1)
1-17: LGTM!.github/scripts/install_coherent_mmce.sh (1)
60-73: LGTM!.github/scripts/patch_stock_mmceman.sh (1)
56-65: LGTM!src/textures.c (1)
625-641: LGTM!include/mmcesupport.h (1)
30-32: LGTM!src/mmcesupport.c (1)
29-35: LGTM!Also applies to: 81-81, 157-209, 454-471, 574-584, 888-891, 986-1003
lng_tmpl/_base.yml (1)
1008-1009: LGTM!
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mmcesupport.c (1)
888-891: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse an MMCE-specific warning for slot/port detection failures.
This path handles MMCE slot/port detection failure but displays
_STR_ERR_FILE_INVALID, which can mislead users into troubleshooting the game file instead of the device. Use a dedicated localized MMCE/port error label here; keep the file-invalid label for the ISO-open failure path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mmcesupport.c` around lines 888 - 891, Update the warning call in the MMCE slot/port detection failure path to use the dedicated localized MMCE/port error label instead of _STR_ERR_FILE_INVALID. Leave _STR_ERR_FILE_INVALID unchanged in the ISO-open failure path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/mmcesupport.c`:
- Around line 888-891: Update the warning call in the MMCE slot/port detection
failure path to use the dedicated localized MMCE/port error label instead of
_STR_ERR_FILE_INVALID. Leave _STR_ERR_FILE_INVALID unchanged in the ISO-open
failure path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d5b6a321-3ccc-4bd2-8973-772194f9d8ce
📒 Files selected for processing (1)
src/mmcesupport.c
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: build-debug (iopcore_ppctty_debug, :v20250725-2)
- GitHub Check: build-variants-ps2dev-latest (EXTRA_FEATURES=0, PADEMU=1)
- GitHub Check: build-ps2dev-latest
- GitHub Check: build
🔇 Additional comments (1)
src/mmcesupport.c (1)
29-35: LGTM!Also applies to: 80-81, 157-160, 175-209, 454-471, 569-584, 986-1003
- Neutrino BDM leg gets the SAME MX4SIO settle gate as the native leg (CodeRabbit major -- real: it also sends the cross-device GameID and shares SIO2 through the IOP reboot). - Card-gone with an empty failed VCD list now resets the retry budget in mmceNeedsUpdate's early return -- a reinserted card no longer inherits a dead VCD page (CodeRabbit major -- real edge). - mmceGameIdTarget cleared at the top of every send: only a switch made by THIS call can be settled against (Gemini -- unreachable via the current sole consumer, taken as free hardening). - Explicit <errno.h> in textures.c (Gemini -- transitively included, taken for robustness). Builds clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/mmcesupport.c (2)
198-211: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDo not sleep past the requested settle timeout.
The
waited <= timeoutMscondition still executesDelayThread(200 * 1000)after the final poll. With the currentmmceGameIdSettle(5000)callers, this blocks for at least 5200 ms instead of the promised 5000 ms.Proposed fix
- for (int waited = 0; waited <= timeoutMs; waited += 200) { + for (int waited = 0; waited <= timeoutMs;) { if (fileXioDevctl(mmceGameIdTarget, 0x1, NULL, 0, NULL, 0) != -1) { int busy = fileXioDevctl(mmceGameIdTarget, 0x2, NULL, 0, NULL, 0); if (busy < 0 || (busy & 1) == 0) return 0; } - DelayThread(200 * 1000); + if (waited == timeoutMs) + break; + int delayMs = timeoutMs - waited; + if (delayMs > 200) + delayMs = 200; + DelayThread(delayMs * 1000); + waited += delayMs; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mmcesupport.c` around lines 198 - 211, Update the wait loop in mmceGameIdSettle so it never calls DelayThread after the requested timeout has been reached; retain polling at 200 ms intervals while waited remains within timeoutMs, but skip the final sleep when no time remains.
33-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReset the VCD retry state during
mmceInit().These static fields survive
mmceInit(), butmmceInit()only resets the game lists. Re-enabling MMCE or applying settings while the same card remains present can retain an exhausted retry budget and leave the VCD page atNOUPDATEuntil the user manually toggles it.Proposed fix
mmceVcdGameCount = 0; mmceVcdGames = NULL; + mmceVcdScanFailed = 0; + mmceVcdScanRetries = 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mmcesupport.c` around lines 33 - 35, Reset mmceVcdScanFailed and mmceVcdScanRetries at the start of mmceInit(), alongside the existing game-list state reset. Ensure reinitialization restores the full VCD scan retry budget so a previously exhausted scan does not remain in NOUPDATE.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/mmcesupport.c`:
- Around line 198-211: Update the wait loop in mmceGameIdSettle so it never
calls DelayThread after the requested timeout has been reached; retain polling
at 200 ms intervals while waited remains within timeoutMs, but skip the final
sleep when no time remains.
- Around line 33-35: Reset mmceVcdScanFailed and mmceVcdScanRetries at the start
of mmceInit(), alongside the existing game-list state reset. Ensure
reinitialization restores the full VCD scan retry budget so a previously
exhausted scan does not remain in NOUPDATE.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 475343a4-42f9-489e-b701-a60d8988a109
📒 Files selected for processing (3)
src/bdmsupport.csrc/mmcesupport.csrc/textures.c
📜 Review details
⏰ Context from checks skipped due to timeout. (19)
- GitHub Check: check-format
- GitHub Check: build-debug-ps2dev-latest (ingame_debug)
- GitHub Check: build-debug-ps2dev-latest (iopcore_debug)
- GitHub Check: build-debug-ps2dev-latest (iopcore_ppctty_debug)
- GitHub Check: build-variants-ps2dev-latest (EXTRA_FEATURES=1, PADEMU=1)
- GitHub Check: build-variants-ps2dev-latest (EXTRA_FEATURES=0, PADEMU=1)
- GitHub Check: build-variants-ps2dev-latest (EXTRA_FEATURES=0, PADEMU=0)
- GitHub Check: build-debug (ingame_debug, :v20250725-2)
- GitHub Check: build-debug (DTL_T10000=1, :v20250725-2)
- GitHub Check: build-variants-ps2dev-latest (EXTRA_FEATURES=1, PADEMU=0)
- GitHub Check: build-debug-ps2dev-latest (DTL_T10000=1)
- GitHub Check: build-debug (iopcore_debug, :v20250725-2)
- GitHub Check: build-debug (eesio_debug, :v20250725-2)
- GitHub Check: build-variants (EXTRA_FEATURES=0, PADEMU=1)
- GitHub Check: build-variants (EXTRA_FEATURES=1, PADEMU=0)
- GitHub Check: build
- GitHub Check: build-debug-ps2dev-latest (eesio_debug)
- GitHub Check: build-variants (EXTRA_FEATURES=1, PADEMU=1)
- GitHub Check: build-ps2dev-latest
🔇 Additional comments (3)
src/textures.c (1)
6-6: LGTM!Also applies to: 626-642
src/mmcesupport.c (1)
87-87: LGTM!Also applies to: 159-189, 426-430, 461-478, 576-591, 895-898, 993-1010
src/bdmsupport.c (1)
911-918: LGTM!Also applies to: 1238-1247
1 participant
Four verified mechanisms from the MC-boot HW batch (10-agent triage, adversarially verified) — all pre-existing holes exposed by one degraded-MMCE session; none of yesterday's merges caused them:
-1for six different open failures; OPL memoized every one as "file doesn't exist" for the session. New driver patch (applies at both pins, validated) makes only the card's explicit not-found return-ENOENT; the staging arm memoizes only on ENOENT — everything else takes the existing transient lane and art self-heals when the bus quiets. BDM/HDD branch untouched.LoadElfmarker; cdvdman's (deliberately untimed, untouched) wait for the MX4SIO SD, starved by an MMCE GameID switch still in flight through the IOP reboot on the shared SIO2.mmceSendGameIDis tri-state, remembers its target slot, and the SDC leg only runs a 5 s settle gate — warn-and-launch-anyway on expiry, never a block. Busy-bit-less firmware exits immediately on presence (no stall).Companion PRs from the same batch: #247 (S4 interlaced-PNG art),
claude/hdd-coexist(S1/S2 APA+ATA).Builds clean (incl. lang regen for the one appended label). HW-pending: the same session recipe that produced the batch.
🤖 Generated with Claude Code
Summary by CodeRabbit