fix(rust-sdk): recognize PCM16 audio codec - #13050
Conversation
Map protocol codec ID 0 to Pcm16 through the existing public device conversion. Add integration coverage for known codecs, unknown values, and empty reads, and document the supported codec map. Validation: 11 crate tests, rustfmt, Clippy with warnings denied, and 11 selected shared repository checks passed. The history guard also passed with full history. Fixes BasedHardware#13036.
|
The three-file fix and regression coverage are ready for review. All 11 crate tests, formatting, Clippy, both required repository preflight commands, and the installed pre-push gate passed locally. The Rust SDK workflow is awaiting approval from a maintainer; no hosted CI jobs have run yet. Could a maintainer approve the workflow run when reviewing? The public-enum compatibility note and the unapproved USD 25 proposal remain documented in the PR and issue #13036. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Thanks @DITlieD — verified end to end; this is a correct, well-scoped fix. What I checked:
Notes for the merge decision:
Nice first contribution — a clear issue report (#13036), a minimal diff, and tests that exercise the public API rather than internals. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
|
Thanks @Git-on-my-level for checking the fix and regression. Could you confirm whether issue #13036 qualifies for the proposed USD 25 bounty, and who can approve that amount? The proposal is still unapproved; I would like to clarify eligibility before treating it as paid work. |
kodjima33
left a comment
There was a problem hiding this comment.
fix(rust-sdk): adds missing PCM16(0) codec ID to AudioCodec::from_id, matches protocol doc, small scoped diff with tests, confirmed not already fixed on main. Confidence 5/5.
The Rust SDK returned
AudioCodec::Unknown(0)when the audio-codec characteristic contained the protocol-defined PCM16 value. AddAudioCodec::Pcm16and route ID 0 through the existing conversion used byDevice::audio_codec().Fixes #13036.
The regression exercises the public device method with a synthetic BLE read and verifies the requested service/characteristic. The expected codec IDs come from
sdks/device/PROTOCOL.md. Existing codecs, unknown values and truncated reads retain their behavior. The existing Rust SDK workflow runs the new integration tests.Compatibility: a new public enum variant requires downstream exhaustive matches to account for
Pcm16. This change does not claim physical-device verification or an observed recording failure.Validation:
Unknown(0).Product invariants affected
none
Failure class
Failure-Class: FC-mirrored-model-omits-new-member
The Rust codec enum omits a member already defined by the shared in-tree protocol. The existing
AudioCodec::from_id()conversion owns classification for every Rust device caller; this patch repairs that mapping and adds behavioral coverage at its public caller, without introducing another classifier or a new guard service.The optional USD 25 bounty proposal in issue 13036 has not been approved. This description makes no payment claim.