feat(librepods): backport PR #519 for AirPods Max 2 support

[Bad3r]

Summary

• Backport add basic airpods max 2 support (linux only) kavishdevar/librepods#519 onto the v0.2.5 source tree pinned in modules/base/custom-packages-overlay.nix so AirPods Max 2nd Gen (BLE id 0x2D20, model A3454) is recognised by the Linux Qt client.
• Without this patch the Max 2 falls through to the Unknown enum branch: it gets the generic pod icon, the dual-stem battery layout (which doesn't match a single-headset device), and BLE scans report it as an unknown model id.
• Patch lives at packages/librepods/airpods-max-2.patch and is wired into the existing prev.librepods.overrideAttrs block via patches = (old.patches or [ ]) ++ [ ../../packages/librepods/airpods-max-2.patch ];.
• Paths inside the patch are rooted at linux/ because the nixpkgs librepods derivation sets sourceRoot = "source/linux". The upstream PR's Proximity Pairing Message.md docs change is intentionally omitted because it lives outside the build root and has no runtime effect.
• .typos.toml gains an ANC = "ANC" entry (Active Noise Cancellation) so the patch's AirPods4ANC enum value does not trip the spell-check hook.
• Drop this patch — and the typos allowlist entry if no other consumer relies on it — once upstream merges PR #519 and a librepods release shipping it is pinned. Tracking note left in the overlay comment.

Test plan

• nix flake check --accept-flake-config --no-build --offline → all checks passed
• nix build .#nixosConfigurations.system76.pkgs.librepods → builds cleanly with the patch applied
• strings $out/bin/.librepods-wrapped | rg 'AirPodsMax2|A3454' → both symbols present in the resulting binary, confirming the patch's enum and model-number additions made it into the build output
• Hardware verification on the System76 host once the closure is deployed: connect AirPods Max 2, confirm the tray/UI shows the Max icon and a single battery row (instead of the default pod icon and dual-stem layout)

[claude]

Claude finished @Bad3r's task in 2m 45s —— View job

---

Claude PR Review

• Read changed files
• Cross-check the patch against upstream PR #519
• Verify patch paths match sourceRoot = "source/linux" (confirmed against nixpkgs/pkgs/by-name/li/librepods/package.nix)
• Validate overlay wiring
• Post review

---

Verdict: LGTM. Small, faithful, well-documented backport. The four code hunks are byte-identical to upstream PR #519 (verified via gh api repos/kavishdevar/librepods/pulls/519/files); only the Proximity Pairing Message.md docs hunk is dropped, which is the right call given sourceRoot = "source/linux" and the docs file's lack of runtime effect. The path-stripping (upstream linux/ble/blemanager.cpp → ble/blemanager.cpp) is correct for the nixpkgs derivation's source root (confirmed sourceRoot = "source/linux" upstream).

Code quality

• packages/librepods/airpods-max-2.patch — clear Subject: and explanatory header that calls out the upstream tracker and the path-rooting reasoning. Drop-when-merged tracking note is present in both the patch header and the overlay comment, which is the right place for it.
• modules/base/custom-packages-overlay.nix:49-63 — overlay change is minimal and idiomatic; (old.patches or [ ]) ++ [ ... ] correctly preserves any pre-existing patches from the nixpkgs derivation. The expanded comment block clearly explains the why.
• .typos.toml:29 — ANC = "ANC" is appropriately scoped and commented. Worth noting: the AirPods4ANC token already exists in upstream librepods source, so this allowlist entry was technically already needed — the new patch just makes it visible to the spell-check hook (because typos scans the patch file content). Adding it now is the right call.

Correctness concerns (none blocking)

1. Enum insertion ordering: the patch inserts AirPodsMax2 between AirPodsMaxUSBC and AirPods4 in enums.h:32, which shifts the integer values of AirPods4 and AirPods4ANC. This would only matter if the enum value were ever serialized to disk / IPC / network as an integer. A quick search of upstream librepods at v0.2.5 doesn't show such serialization (the enum is used in switch statements and as map values), and upstream PR #519 does the same insertion, so this is faithful and safe.
2. Map ordering in getModelName: ble/blemanager.cpp:16 is a value-initialized std::unordered_map-style container keyed by unique BLE IDs — insertion order doesn't affect lookup correctness. Fine.
3. Patch format: no index abc..def lines or From: header, but git apply / patch -p1 only need diff --git + ---/+++ + hunks, and you've already verified nix build applies it cleanly.

Minor nits (optional)

• The PR body says "Paths inside the patch are rooted at linux/". Slightly ambiguous — the patch paths don't include a linux/ prefix; they're relative to the linux/ build root (which is what sourceRoot = "source/linux" selects). Wording like "Paths are relative to the linux/ build root (sourceRoot)" would be clearer for the next person diffing this against upstream. Not worth a force-push unless you're touching the description anyway.
• The overlay comment block (modules/base/custom-packages-overlay.nix:45-52) now spans 8 lines covering both the version bump and the patch. Readable as-is. If this grows further (a third concern), consider splitting into separate comment blocks above each overrideAttrs argument so the rationale stays attached to the line it explains.

Testing

• nix flake check ✓, nix build .#…librepods ✓, and strings | rg 'AirPodsMax2|A3454' ✓ in the PR body — that's a strong signal the patch lands in the binary.
• Hardware verification correctly left unchecked; that's an on-host post-deploy step, not something achievable in CI.
• One additional check worth running once before merging, just to be paranoid about future refloats: nix build .#nixosConfigurations.tpnix.pkgs.librepods — the overlay is shared across both hosts per CLAUDE.md, and you've only confirmed the system76 closure.

Maintenance hook

Once upstream PR #519 merges and a librepods release pinning it ships in nixpkgs (or in your overlay's version bump), drop:

1. packages/librepods/airpods-max-2.patch
2. The patches = (old.patches or [ ]) ++ [ ... ] line in modules/base/custom-packages-overlay.nix
3. The ANC = "ANC" entry in .typos.toml if no other consumer references it (grep first)

The overlay comment already records this. ✓

--- · Branch

[gemini-code-assist]

Code Review

This pull request introduces support for AirPods Max 2 by adding a backported patch to the librepods package and updating the Nix overlay. The review identifies that the backport is incomplete as it lacks updates to the toString function, which may cause display issues in the UI. Additionally, it is recommended to explicitly define the patches list in the Nix overlay rather than appending to the existing list to prevent the inheritance of incompatible patches from previous versions.

[claude]

Claude finished @Bad3r's task in 2m 11s —— View job

---

Re-review after gemini-code-assist comments

Both gemini comments are addressed below. Short version: the toString concern is incorrect (no such hunk in upstream PR #519 and no toString exists in librepods); the explicit-patches concern is theoretical (nixpkgs librepods has no patches to inherit). One real new concern in the second commit is worth flagging: the SBC-XQ disable is daemon-wide.

• Read changed files on tip of branch (63546fb)
• Cross-check patch against upstream PR #519 via gh api
• Verify prev.librepods patch attribute in nixpkgs
• Check for toString symbol in upstream enums.h
• Post verdict

---

gemini comment 1 — "missing toString hunk in enums.h": INCORRECT, no change needed

I pulled upstream PR #519's file list via gh api repos/kavishdevar/librepods/pulls/519/files — there are exactly three changed files: Proximity Pairing Message.md, linux/ble/blemanager.cpp, and linux/enums.h. No toString hunk exists in upstream PR #519. The four code hunks in packages/librepods/airpods-max-2.patch are byte-identical to the two source-file patches there.

I also fetched linux/enums.h at v0.2.5 directly: there is no toString(AirPodsModel) function in librepods. The enum is wrapped with Qt's Q_ENUM_NS(AirPodsModel), which gives the framework reflection via QMetaEnum::valueToKey() — so the display string is derived from the C++ identifier name ("AirPodsMax2") automatically. The patch's enum addition is sufficient; gemini hallucinated a hunk that does not exist upstream and a function that does not exist in the codebase.

gemini comment 2 — "explicitly define patches list": theoretical, current form is fine

I pulled the upstream nixpkgs derivation (pkgs/by-name/li/librepods/package.nix) — it has no patches attribute at all. (old.patches or [ ]) evaluates to [ ], so it's functionally identical to the suggested patches = [ … ]. There is no stale-patch inheritance risk to defend against in the v0.2.0 → v0.2.5 bump.

The or [ ] form is also idiomatic across this overlay — in the same file marktext does (old.nativeBuildInputs or [ ]) ++ [ … ]. Switching one and not the other for symmetric reasons would be inconsistent. Leave as-is.

If you wanted to address gemini's spirit (consistency with version/src/buildInputs already being set absolutely, not relatively), that's a defensible style call — but it's a style preference, not a correctness fix, and the or [ ] pattern is standard NixOS overlay style. I wouldn't change it.

---

New since my prior review: commit 63546fb

Two genuine additions on top of the original PR #519 backport — both worth a closer look since gemini didn't comment on them:

1. packages/librepods/airpods-max-2.patch:63-65 — getModelIcon now returns {"podmax.png", "podmax.png"} for all three Max variants instead of the upstream {"podmax.png", "max_case.png"}. The patch header explains why (the max_case.png asset doesn't exist on main, linux/rust, or in Android res-apple/; case PodColumn is hidden for Max anyway since caseAvailable stays false; QQuickImage still resolves the source URL on creation). This is a deviation from upstream PR #519 — fine on the merits, but it means the patch is now both a backport and a local fix, and it touches the upstream AirPodsMaxLightning/AirPodsMaxUSBC rows, not just the new Max 2 case. The commit subject says stub max_case.png which captures the intent. Worth submitting upstream as a separate fix so this part of the patch can be dropped someday too.

2. modules/apps/librepods.nix:54-58 — services.pipewire.wireplumber.extraConfig."51-disable-sbc-xq" sets bluez5.enable-sbc-xq = false daemon-wide. The comment is honest about this: "bluez5.enable-sbc-xq is a daemon-wide property in WirePlumber's BlueZ monitor, so this disables SBC-XQ for every bluetooth sink rather than just the Max 2". That tradeoff is reasonable for the AirPods Max 2 left-channel-only audio bug, but it has system-wide blast radius:

   • Any other AAC- or SBC-XQ-capable headset (recent Sony/Bose/etc.) on either host will now fall back to plain SBC at lower bitrate when SBC-XQ would have negotiated. AAC-capable sinks (most current Bluetooth headphones) negotiate AAC and are unaffected; pure-SBC sinks are unaffected. The user-visible regression band is SBC-XQ-only sinks, which is narrow but real.
   • Always-on regardless of programs.librepods.extended.enable is appropriate (the WirePlumber config is inside mkIf cfg.enable, so it follows the gate — good).
   • Tracking note in the comment is good; consider also linking the upstream WirePlumber issue (or PR for per-device opt-out) if one exists, so the next person knows what milestone unblocks the workaround.

   No change requested — just flagging the scope so the user is aware. If the daemon-wide SBC-XQ disable ever becomes painful, WirePlumber rules can match on device.product.id or BLE address for per-device configuration; that would be a tighter fix but considerably more code.

Other items unchanged from prior review

• (old.patches or [ ]) ++ [ … ] — idiomatic, leave it.
• Patch path-rooting (linux/ble/blemanager.cpp → ble/blemanager.cpp) — correct for sourceRoot = "source/linux".
• .typos.toml:29 ANC = "ANC" — appropriately scoped.
• Enum insertion ordering in enums.h:32 — faithful to upstream and not serialized in librepods, so safe.

Verdict

LGTM. Gemini's two suggested changes are both non-actionable:

1. The toString hunk doesn't exist upstream and no toString function exists in the code to update.
2. The patches list is empty upstream so or [ ] is equivalent and idiomatic.

The new commit's max_case.png fix and SBC-XQ workaround are reasonable and well-documented. Single suggestion (optional, non-blocking): a one-line link in modules/apps/librepods.nix pointing to the upstream librepods comment / WirePlumber issue that motivated enable-sbc-xq = false so the next maintainer knows where the workaround came from. Not a force-push reason on its own.

Branch ·

[Bad3r]

Review pass: addressed and rejected items

Worked through every reviewer comment, validated each against the upstream sources, and split the accepted changes into two atomic commits (pushed in a moment so this reply lands first per request).

Accepted (with implementation)

1. commit 50e8c777 -- docs(librepods): point SBC-XQ note to specific upstream comment

Source: Claude review, "consider also linking the upstream WirePlumber issue (or PR for per-device opt-out)" plus a follow-up audit of the existing reference.

Found that the comment in modules/apps/librepods.nix already pointed at upstream PR #519 but the URL fragment was truncated to bare #issuecomment (missing the comment id). Clicking it scrolled to the top of the PR rather than the workaround report. Pinned to #issuecomment-4230312279 -- the comment from the PR author (sachin3796) documenting the left-channel-only audio bug and the bluez5.enable-sbc-xq = false fix.

Searched for an upstream WirePlumber issue tracking per-device SBC-XQ opt-out (GitHub via gh search issues, PipeWire/wireplumber repo); none found. WirePlumber is hosted on gitlab.freedesktop.org and does not appear to track this. Did not invent a placeholder link. The existing tracking note already records the conditions for dropping the workaround.

2. commit 5379aa27 -- docs(librepods): clarify path-rooting note in patch header

Source: Claude first-review nit, "Slightly ambiguous -- the patch paths don't include a linux/ prefix; they're relative to the linux/ build root."

Reworded the patch header from "Paths are rooted at linux/" to make the upstream-to-patch path mapping explicit (linux/ble/blemanager.cpp becomes ble/blemanager.cpp). Future maintainers diffing against PR #519 now have a one-line explanation of why the prefix is stripped.

Rejected (with rationale, threads resolved)

Gemini #1 (high) -- "missing toString hunk in enums.h" -- non-actionable.

gh api repos/kavishdevar/librepods/pulls/519/files shows PR #519 changes exactly three files; there is no toString hunk anywhere upstream. gh api repos/kavishdevar/librepods/contents/linux/enums.h?ref=v0.2.5 confirms there is no toString(AirPodsModel) function in the codebase at all (only parseModelNumber, getModelIcon, isModelHeadset). Display strings are derived from the C++ identifier names through Qt's Q_ENUM_NS(AirPodsModel) and QMetaEnum::valueToKey(), so the enum name addition is sufficient. Full reasoning posted in-thread; thread resolved.

Gemini #2 (medium) -- "explicitly define patches list instead of appending to (old.patches or [ ])" -- theoretically defensive, no practical effect.

Verified against /data/git/NixOS-nixpkgs/pkgs/by-name/li/librepods/package.nix: upstream nixpkgs librepods declares no patches attribute. (old.patches or [ ]) evaluates to [ ], so the two forms are functionally identical. The or [ ] form is the idiomatic NixOS-overlay default and matches the same-file marktext override (line 77). Switching this one alone would create within-file inconsistency. Full reasoning posted in-thread; thread resolved.

Out of scope, not acted on

• Submit max_case.png -> podmax.png fix upstream (Claude). Belongs in kavishdevar/librepods, not this NixOS configuration. The patch header already records the conditions for dropping the workaround.
• Hardware verification on System76 (own test plan). Post-deploy step; not achievable in this branch's CI.

Validation

nix build .#nixosConfigurations.system76.pkgs.librepods   # OK
nix build .#nixosConfigurations.tpnix.pkgs.librepods      # OK (Claude's prior-review request)
nix build .#nixosConfigurations.system76.config.system.build.toplevel   # OK
nix build .#nixosConfigurations.tpnix.config.system.build.toplevel      # OK
strings .../bin/.librepods-wrapped | rg 'AirPodsMax2|A3454'             # both symbols present
pre-commit run --files modules/apps/librepods.nix packages/librepods/airpods-max-2.patch   # all hooks pass

[claude]

Claude finished @Bad3r's task in 1m 34s —— View job

---

Re-review after Bad3r's reply + clarification commits

• Pull latest branch state and diff vs prior tip (63546fb → 5379aa2)
• Verify 50e8c777 SBC-XQ comment now points to a real, accessible upstream comment
• Verify 5379aa277 patch-header rewording is accurate vs sourceRoot = "source/linux"
• Check for unrelated regressions / ensure rejected gemini items were not silently changed
• Post verdict

---

Verdict: LGTM, ready to merge. Both clarification commits land exactly what the prior reviews flagged, and the rejection rationale on the gemini items checks out against upstream sources.

50e8c777 — SBC-XQ comment URL fix

Pulled the comment id directly: gh api repos/kavishdevar/librepods/issues/comments/4230312279 resolves to a real comment by sachin3796 whose body opens with "the default selected codec 'SBC-XQ' — if this codec is used, audio from the computer only comes out of the left speaker channel … I've worked around this by adding the following to my wireplumber configuration" — i.e. the exact workaround report this comment block cites. Truncated fragment is now a working anchor. ✓

The follow-up audit on whether a separate WirePlumber issue exists (for per-device opt-out) was a sensible thing to check before fabricating a link; correctly declined to invent one. The drop conditions in the existing comment are sufficient signal for the next maintainer.

5379aa277 — patch-header path-rooting clarification

The new wording reads:

Patch paths are relative to the linux/ build root because the nixpkgs librepods derivation sets sourceRoot = "source/linux". Upstream paths like linux/ble/blemanager.cpp therefore become ble/blemanager.cpp here.

Matches reality (the patch entries at packages/librepods/airpods-max-2.patch:31 and :42 are a/ble/blemanager.cpp and a/enums.h — no linux/ prefix). The explicit upstream→patch mapping is exactly what I'd suggested; future maintainers diffing this file against PR #519 will see why the prefix is stripped without having to reason it out. ✓

Rejected items confirmed not silently changed

• modules/base/custom-packages-overlay.nix:61 — still patches = (old.patches or [ ]) ++ [ … ]. Consistent with marktext's (old.nativeBuildInputs or [ ]) ++ [ … ] two overrideAttrs blocks below at line 78. Bad3r's rationale (functional equivalence + within-file consistency + defensive against future upstream patches) holds. ✓
• packages/librepods/airpods-max-2.patch — still 4 hunks, no fabricated toString hunk. Confirmed via prior-review evidence that no toString(AirPodsModel) function exists in linux/enums.h@v0.2.5 and Qt's Q_ENUM_NS(AirPodsModel) provides display strings via QMetaEnum::valueToKey(). The rejection is well-grounded. ✓

Ship checklist

• Test plan in PR body shows nix flake check, nix build .#…system76.pkgs.librepods, nix build .#…tpnix.pkgs.librepods, both toplevel closures, strings | rg 'AirPodsMax2|A3454', and pre-commit run all green. Hardware verification correctly remains the post-deploy step.
• No regressions introduced by the docs commits (single-file diffs, comment-only).
• Resolved review threads on this branch are all either accepted-and-implemented or rejected-with-validated-rationale; nothing left in flight.

Nothing more needed from this end. ✅
· Branch