Skip to content

Add R2 Uppity Spinner ALT (highfalutintodd fork) board - #1

Merged
travisccook merged 2 commits into
travisccook:mainfrom
highfalutintodd:add-r2uppityspinner-alt-board
Jul 6, 2026
Merged

Add R2 Uppity Spinner ALT (highfalutintodd fork) board#1
travisccook merged 2 commits into
travisccook:mainfrom
highfalutintodd:add-r2uppityspinner-alt-board

Conversation

@highfalutintodd

@highfalutintodd highfalutintodd commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a device-native board file for the R2UppitySpinnerV3 ALT firmware (my fork of Mimir Reynisson's original) at fw v3.5.3, covering 41 commands.

  • Board id: r2uppityspinner-alt
  • Confidence: community — added by the fork's maintainer, verified against the current firmware source but not yet round-tripped through the hosted composer on hardware.
  • Groups: Playback, Lifter, Rotary, Random Mode, Lights, Timing, Configuration.
  • Notable safety choices: :PX marked power (ESTOP), all rotary/lifter motion movement, light-kit + wait cosmetic, all #P* config commands config or power (reboot).
  • :PD (relative rotation) range constrained to ±180° with a commentLabel note explaining the chain-multiple workaround — the firmware currently clamps multi-revolution single calls to shortest-path (fix on the 3.5.4 backlog). When 3.5.4 ships, the range can widen without a schema change.
  • Excluded from this PR (deliberate): #PS<n>:{sequence} store-sequence (its argument is the very sub-language the composer builds), and the SMQ pairing flows (#PRPAIR, #PRUNPAIR, #PRNAME, #PRSECRET, #PRMASTERKEY*) which are interactive rather than one-shot.

Version bumps

  • libraries/manifest.json: libraryVersion 2.1.0 → 2.2.0 (minor: new board), added entry.
  • releases.json: matching bump and release notes.

Fixture updates

Two existing tests hard-coded the pre-bump version / component count:

  • test/load-node.test.js — three '2.1.0''2.2.0', components.length 8 → 9.
  • test/engine.test.js — one '2.1.0''2.2.0'.

No behavioral test changes.

Local checks

  • `npm run validate` — clean (all 10 board files + cross-file merge pass).
  • `npm test` — 136/136 pass. Every one of the 41 new commands round-trips through its `examples` via the auto-generated `test/library.test.js` sweep.

Test plan

  • Schema validator passes (`npm run validate`).
  • Full test suite green (`npm test`).
  • Try the board in the hosted composer once merged and confirm all 41 commands appear + insert correctly.
  • Spot-check a few commands (`:PP100,60`, `:PL7`, `:PA180,40,80`) by pasting the composer output into the R2Uppity serial port and observing the periscope respond.

Happy to iterate on safety classifications, group naming, or the ID prefix scheme — this is intended as a starting point.

🤖 Generated with Claude Code

Adds a device-native board for the R2UppitySpinnerV3 ALT firmware
(highfalutintodd fork of Mimir Reynisson's original) at fw v3.5.3.

Covers 41 commands across 7 groups:
- Playback: :PS<seq>
- Lifter: :PP, :PP,speed, :PPR, :PH, :PH<speed>, :PX (ESTOP)
- Rotary: :PR, :PRR, :PA, :PA,speed,max, :PAR, :PD, :PDR
- Random Mode: :PM, :PM<G/M/A>, :PM,lift,rot,min,max
- Lights: :PL<mode>, :PLR (light-kit modes 0-7)
- Timing: :PW<s>, :PWR<max>
- Configuration: 20 #P* commands (calibrate, ID, motor type, expert
  mode, WiFi, aggressiveness default, baud, limit polarities, RC
  mode, factory reset, reboot, etc.)

Confidence: community. :PD relative-rotation range constrained to
+/-180 to match the firmware 3.5.3 clamp (multi-rev fix backlogged
for 3.5.4); commentLabel notes the chain-multiple workaround.

Bumps libraryVersion 2.1.0 -> 2.2.0 (minor: new board) and updates
releases.json and two fixture assertions in engine.test.js /
load-node.test.js that hard-coded the previous version and board
count.

npm run validate: clean
npm test: 136/136 passing
@travisccook

Copy link
Copy Markdown
Owner

@claude please review this

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Claude finished @travisccook's task in 5m 3s —— View job


Review: R2 Uppity Spinner ALT board

  • Read trigger comment and gather PR context
  • Read CLAUDE.md / repo conventions
  • Review new board file libraries/boards/r2uppityspinner-alt.json
  • Review manifest/releases version bump
  • Review test fixture updates
  • Check CI status (green: run 28819446813)
  • Post review feedback

Solid contribution overall — clean template usage, sensible grouping, and the manifest/releases/test-fixture bumps are all consistent and correct. One correctness issue worth fixing before merge, plus two minor nits.

🐛 Negative-range int params won't parse back (engine gap, not just data)

uppity.rotary.spin (libraries/boards/r2uppityspinner-alt.json:172-176, :PR{speed}, min: -100) and uppity.rotary.rel (:239-242, :PD{degrees}, min: -180) are the first commands in the whole library to use a non-enum int param with a negative range. The template encoder's matcher builds its regex for plain int params unconditionally as '(\\d+)' (src/droidnet-command-library.js:233, in _buildTemplateMatcher), which cannot match a leading -.

Concrete effect: match(':PR-80') and match(':PD-90') — both of which are the board's own listed examples — return null. parseWCBValue then falls back to treating them as an unrecognized raw step (per the wire-format contract in CLAUDE.md) instead of a structured, editable "Spin Rotary at Speed" / "Rotate Relative Degrees" step. encode() (the forward direction) is unaffected since it just does string interpolation — this only breaks re-parsing/editing values that were pasted in or reloaded.

This isn't caught by CI because test/web.test.js's round-trip check (examples for fully-bounded commands round-trip to a recognized step) only exercises cmd.examples[0] for each command, and the first example for both commands happens to be non-negative (:PR0, :PD180).

Since this is a shared-engine limitation (not something a board JSON file alone can work around), the fix belongs in _buildTemplateMatcher: allow an optional leading - for int-param groups (e.g. '(-?\\d+)'), plus a regression test asserting match(':PR-80') resolves to commandId: 'uppity.rotary.spin' with params.speed === '-80'. Worth doing as part of this PR (or immediately after) since it directly affects two of the board's documented examples.

Fix this →

Minor: duplicate literal template

uppity.cfg.lifterLimit (line 571) and uppity.cfg.rotaryLimit (line 584) both use the exact literal template "#P{polarity}". This works today only because the two enums' codes never overlap (NCL/NOL vs NCR/NOR), so match() stays unambiguous — but it reads oddly for a future maintainer to see two different command ids with an identical template string. Consider a comment or distinct-looking template to make clear they're intentionally two halves of the same wire prefix.

Nit: safety-classification consistency

The PR description says "#P* config commands config or power (reboot)," but several commands the board itself documents as rebooting are marked config rather than power: #PWIFIRESET, #PWIFI{state}, #PR (rotary toggle), #PBAUD{rate}, and #PFACTORY (explicitly called "destructive" in its own name) — only #PRESTART got power. Functionally this doesn't change UI behavior (both classes trigger the same non-cosmetic confirm), but for internal consistency with the stated rule, these look like good candidates for power. Since the author explicitly invited feedback on safety classification, flagging rather than blocking.

Not reviewed further (verified fine)

  • Command ID uniqueness across all boards: no collisions.
  • Schema validation + full test suite: green in CI.
  • Version bump discipline (libraryVersion 2.1.0 → 2.2.0, releases.json, fixture updates in test/engine.test.js/test/load-node.test.js): all correct and consistent with the minor-bump (new board) rule in CLAUDE.md.
  • Anchoring of the template matcher (^...$) correctly prevents false-positive prefix collisions between similarly-prefixed literal commands (e.g. #PR vs #PRC{state} vs #PRESTART).

@travisccook
travisccook merged commit 6b0a188 into travisccook:main Jul 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants