Correct the radio matrix in the README - #140
Merged
Merged
Conversation
Two errors, both of which the CLAUDE.md PR inherited by paraphrasing them. The RN42/RN4678 split was written as a single "expansion-board revision 5 vs 6+" cutoff. It isn't: the transition tracks sensor generation, the gates differ per board ID, and assembly variants of one PCB are encoded as higher minor revisions — so a bare revision number means nothing without its board ID. Now points at SHIMMER3_BOARD_REVISIONS.md instead of restating a matrix this file cannot keep current. The device table also listed Shimmer3R as "nRF52 (BLE) + RN4678". It is neither — it carries a CYW20820 (Vela IF820). This repo's own source already says so: WiredShimmerClient parses "the CYW20820's version record", and the firmware and board-revisions doc agree. The "native nRF52 BLE" comparison further down is corrected to match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the review comment on #139, which flagged that the RN42/RN4678 description
over-simplifies which revisions carry which radio. The same text is here in the README — in fact
this is where the CLAUDE.md wording came from — so it needs fixing at source.
Two corrections:
1. The revision cutoff. The README said boards up to expansion-board revision 5 carry an RN42
and revision 6+ an RN4678. That isn't how it works: the transition tracks sensor generation, the
revision gates differ per board ID (SR31 / SR38 / SR47 / SR48 / SR49 each run their own
major/minor scheme), and assembly variants of one PCB are encoded as a higher minor revision — so
a bare revision number means nothing without its board ID. It now points at
SHIMMER3_BOARD_REVISIONS.mdrather than restating a matrix this file can't keep current. The runtime detail worth keeping (the
two EEPROM bits in
ShimBt_startCommon, and no-EEPROM meaning the RN42 fleet) stays.2. The Shimmer3R radio. The device table listed Shimmer3R as
nRF52 (BLE) + RN4678. It isneither — it carries a CYW20820 (Vela IF820). This repo's own source already says so
(
WiredShimmerClient.tsparses "the CYW20820's version record", andbrandRecord.tsdistinguishesthe RN4678's 31-byte advertisement from Shimmer3R's full payload);
shimmer3r-firmwareand theboard-revisions doc agree. The "native nRF52 BLE" comparison further down is corrected to match.
Worth a check from someone with the PCB index workbook: the
SR31-6-0 onwardgate in the table isleft as-is, since I had no way to verify it and it is at least board-ID-specific.
🤖 Generated with Claude Code