Reduce LED and LCM plumbing and extract WS2812 support - #23
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors the Float Out Boy example’s LED + LCM plumbing by characterizing key wire contracts in tests, moving WS2812 DMA/PWM mechanics into a dedicated (package-specific) STM32 driver module, and simplifying the higher-level LED/LCM state code while preserving feature behavior.
Changes:
- Move float-to-integer wire conversion helpers into
vesc-protocoland re-export them for Float Out Boy wire encoding. - Simplify LCM request parsing and response framing, and reduce duplicated packet prefix handling.
- Extract the Float Out Boy WS2812 timer/DMA driver into
vescpkg-rs::stm32::float_out_boy_ws2812and update the internal LED driver to use it.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| examples/float-out-boy/src/wire.rs | Reuses shared protocol-buffer numeric conversion helpers and simplifies timestamp truncation. |
| examples/float-out-boy/src/package/state/lcm.rs | Streamlines LCM name handling, packet construction, payload copying, and command dispatch. |
| examples/float-out-boy/src/package/state/internal_leds/hardware.rs | Delegates ARM WS2812 setup/quiesce/restart/teardown to the extracted STM32 module. |
| examples/float-out-boy/src/package/state/internal_leds/driver.rs | Switches pulse allocation type to the extracted WS2812 pulse buffer and tightens encoding/test characterization. |
| examples/float-out-boy/src/main.rs | Introduces helper macros (const_field_getters!, wire_enum!) used to reduce repeated boilerplate. |
| examples/float-out-boy/src/leds.rs | Replaces repeated enum/id boilerplate via macros and simplifies renderer/internal helper visibility. |
| examples/float-out-boy/src/lcm/mode.rs | Converts LED mode to an explicit wire enum with stable IDs. |
| examples/float-out-boy/src/lcm/hardware.rs | Refactors internal layout bookkeeping (role indexing + offsets array) and reduces repeated getters. |
| examples/float-out-boy/src/lcm.rs | Updates exports to match removed/relocated hardware config types. |
| examples/float-out-boy/src/config.rs | Uses TryFrom<u8>-based enum decoding/validation and adds mutation-based acceptance testing. |
| crates/vescpkg-rs/src/stm32/float_out_boy_ws2812.rs | Adds the package-specific extracted WS2812 TIM/DMA driver plus pulse-buffer support and mapping tests. |
| crates/vescpkg-rs/src/stm32.rs | Exposes the new provisional Float Out Boy WS2812 STM32 module. |
| crates/vesc-protocol/src/buffer.rs | Adds no-helper float→integer saturating truncation helpers with tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
515b8d1 to
4176013
Compare
4176013 to
325d30d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
examples/float-out-boy/src/package/state/lcm.rs:111
poll_requestnow returns early on an empty payload, which leavesself.nameunchanged. Previouslyself.namewas cleared for every poll request, even when no name bytes were present, so this is a behavioral change/regression (stale device name can persist). Removing thepayload.is_empty()guard preserves the prior “clear then copy if present” semantics (copying a 0-length slice is fine).
if !self.enabled() || payload.is_empty() {
return;
}
What this changes
What the added tests prove
Proof boundary
The six signed commits preserve the actual source timeline in both author and committer dates. Their combined patch is identical to the original three area commits after accounting for the SDK check-matrix repair already present in the base PR. The full repository check suite passes. This reconstruction does not claim fresh physical LED, LCM, or controller validation.