fix(foundation): stabilize async discovery and bus reset recovery by gly11 · Pull Request #18 · mrmidi/ASFireWire

gly11 · 2026-05-27T04:27:39Z

Summary

This PR extracts the async, discovery, and bus-reset foundation fixes from the larger bring-up branch so they can be reviewed independently before higher-level protocol work.

Async TX/RX Foundations

Fix packet helper behavior used by async request/response routing
Harden packet routing for request handlers and response sender paths
Preserve response matching correctness across full generation values
Retain full transaction response payloads instead of truncating larger results

Bus Reset & Controller Bring-Up

Add bounded recovery for manual resets that do not produce a bus-reset IRQ
Delay discovery after accepted topology using the Apple-style scan delay
Track reset diagnostics needed by recovery decisions
Improve gap-count policy for unknown previous gap state and two-node local-root topologies
Apply local contender eligibility while keeping root delegation policy-controlled

Config ROM & Discovery

Improve ROM scan fallback and completion behavior across speed retries
Keep existing devices during inconclusive zero-ROM scans when topology still shows a remote link-active node
Add discovery convergence checks to prevent premature device loss
Extend device/unit discovery metadata and Swift parsing for the updated wire format

Tests

Async packet serialization compatibility tests
Response sender header format tests
Bus manager gap optimization tests
Bus reset coordinator recovery tests
Discovery convergence tests
ROM scanner completion and node state machine tests
Transaction storage tests

Why this PR

These changes are shared foundations for subsequent protocol work, but they are useful and reviewable on their own. Keeping them separate reduces the follow-up PR size and lets reviewers focus first on async response handling, bus reset recovery, and discovery convergence.

This PR intentionally does not include higher-level protocol session/command plumbing, debug UI, local diagnostic scripts, install helpers, or documentation-only planning notes.

Verification

cmake -S . -B build-pr1 -DCMAKE_BUILD_TYPE=Debug
cmake --build build-pr1 --target BusManagerGapOptimizationTests BusResetCoordinatorTests DiscoveryConvergenceTests ROMScanNodeStateMachineTests ROMScannerCompletionTests ResponseSenderHeaderFormatTests TransactionStorageTests ASFWPacketTests --parallel 8

Directly ran:

build-pr1/tests/BusManagerGapOptimizationTests
build-pr1/tests/BusResetCoordinatorTests
build-pr1/tests/DiscoveryConvergenceTests
build-pr1/tests/ROMScanNodeStateMachineTests
build-pr1/tests/ROMScannerCompletionTests
build-pr1/tests/ResponseSenderHeaderFormatTests
build-pr1/tests/TransactionStorageTests
build-pr1/tests/ASFWPacketTests

Also verified with:

xcodebuild -project ASFW.xcodeproj -scheme ASFW -configuration Debug -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO build

The Xcode build succeeds. Existing Swift warnings and a CoreSimulator version warning are still present, but they do not block the build.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 31c784ea56

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-27T04:52:55Z

+                    "ConfigROMStore::FindLatestForNode: node=%u using gen=%u with unit profile "
+                    "instead of partial gen=%u",
+                    nodeId, latestWithUnitProfile->gen.value, latest ? latest->gen.value : 0);
+        return latestWithUnitProfile;


Do not return older ROMs for reused node IDs

When the latest ROM cached for this node is partial, this fallback returns any older ROM with unit directories for the same node number. Node IDs are reassigned across bus resets and device unplug/replug, so if device A previously had a rich ROM at node 0 and device B later appears at node 0 with only a minimal/partial ROM, ExportConfigROM with a stale/zero generation will expose A's raw ROM for B. Prefer the richer cache only when it is for the same GUID as the latest node entry, otherwise return the latest generation for the node.

Useful? React with 👍 / 👎.

Fixed in the latest update. The fallback now only reuses an older richer ROM when it matches the latest node entry's GUID, and the stack includes a regression test covering node ID reuse with a different GUID.

mrmidi · 2026-05-27T14:20:45Z

Could you please update stale comments in PacketRouter.hpp / PacketHelpers.hpp that still imply “big-endian packet header” or “zero-copy” since your code is correct.

Fixed in f693d99. I updated the PacketRouter/PacketHelpers comments to describe OHCI AR DMA memory order and the callback-scoped dispatch view, and removed the stale zero-copy wording. This is comment-only; git diff --check passes for the updated PR #18 range.

mrmidi · 2026-05-27T14:28:04Z

    const char* StateString() const;
    static const char* StateString(State state);

+    enum class RecoveryReasonCode : uint8_t {


Kindly add source in the comment: where does it comes from?

Added in b66b488. These are ASFW-defined BusResetCoordinator diagnostics labels, not OHCI/IEEE 1394 wire or register values. The comment now states that they label the coordinator FSM branch that most recently recorded a recovery trigger.

mrmidi · 2026-05-27T14:33:54Z

+constexpr uint32_t kNikonOui = 0x0090B5;
+constexpr uint8_t kNikonSlowPublishRetryBudget = 4;
+constexpr uint64_t kNikonSlowPublishDelayNs = 500ULL * 1'000'000ULL;


Not the best place for such variables, but let's leave it here for now. We will think of better place later

Noted. I’ll keep the placement unchanged in this PR and leave centralizing these constants for a follow-up cleanup.

mrmidi · 2026-05-27T14:35:13Z


 namespace {

+constexpr uint32_t kUnitSpecIdSBP2 = 0x00609E;


mrmidi · 2026-05-27T14:44:09Z

Thanks. This is amazing piece of work.

gly11 · 2026-05-27T14:48:54Z

Thanks. This is amazing piece of work.

I’m happy to contribute. This is an amazing project, and I’m glad my PR helps : )

fix(foundation): stabilize async discovery and bus reset recovery

DICE and main had diverged into two parallel lines. This merge records DICE (40a796c) as a second parent so main's prior history — including gly11's merged PRs #18/#19/#20 and the sbp2/ci/foundation fixes — is preserved as ancestry, while the resulting tree is taken wholesale from DICE. main-only code is superseded by DICE's implementation but remains recoverable from history (refs/backup/main-pre-dice-merge = 757456d). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

fix(foundation): stabilize async discovery and bus reset recovery

DICE and main had diverged into two parallel lines. This merge records DICE (40a796c) as a second parent so main's prior history — including gly11's merged PRs mrmidi#18/mrmidi#19/mrmidi#20 and the sbp2/ci/foundation fixes — is preserved as ancestry, while the resulting tree is taken wholesale from DICE. main-only code is superseded by DICE's implementation but remains recoverable from history (refs/backup/main-pre-dice-merge = 757456d). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

fix(foundation): extract async discovery and bus reset fixes

31c784e

gly11 mentioned this pull request May 27, 2026

feat(sbp2): add session registry and command flow #19

Merged

gly11 marked this pull request as ready for review May 27, 2026 04:47

gly11 mentioned this pull request May 27, 2026

feat(sbp2): complete SBP-2 login, command transport, and INQUIRY flow #14

Closed

chatgpt-codex-connector Bot reviewed May 27, 2026

View reviewed changes

fix(configrom): guard node ROM fallback by GUID

2b39cc4

mrmidi reviewed May 27, 2026

View reviewed changes

docs(async): clarify AR packet view comments

f693d99

mrmidi reviewed May 27, 2026

View reviewed changes

docs(bus): identify recovery reason codes

b66b488

mrmidi merged commit 4c2c155 into mrmidi:main May 27, 2026
2 checks passed

mrmidi added a commit that referenced this pull request Jun 12, 2026

Merge pull request #18 from gly11/pr/foundation-async-discovery

ce71358

fix(foundation): stabilize async discovery and bus reset recovery

forkt69 pushed a commit to forkt69/ASFireWire that referenced this pull request Aug 28, 2026

Merge pull request mrmidi#18 from gly11/pr/foundation-async-discovery

44acc12

fix(foundation): stabilize async discovery and bus reset recovery

Navigation Menu

Uh oh!

fix(foundation): stabilize async discovery and bus reset recovery - #18

fix(foundation): stabilize async discovery and bus reset recovery#18
mrmidi merged 4 commits into
mrmidi:mainfrom
gly11:pr/foundation-async-discovery

gly11 commented May 27, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 27, 2026

Uh oh!

gly11 May 27, 2026

Uh oh!

mrmidi May 27, 2026

Uh oh!

gly11 May 27, 2026

Uh oh!

mrmidi May 27, 2026

Uh oh!

gly11 May 27, 2026

Uh oh!

mrmidi May 27, 2026

Uh oh!

gly11 May 27, 2026

Uh oh!

mrmidi May 27, 2026

Uh oh!

mrmidi commented May 27, 2026

Uh oh!

gly11 commented May 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

gly11 commented May 27, 2026

Summary

Async TX/RX Foundations

Bus Reset & Controller Bring-Up

Config ROM & Discovery

Tests

Why this PR

Verification

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrmidi commented May 27, 2026

Uh oh!

gly11 commented May 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants