Skip to content

Sol 2 fix strobe frequency quantization#5469

Closed
kieranskelly wants to merge 21 commits intowled:mainfrom
GridLights:SOL-2-fix-strobe-frequency-quantization
Closed

Sol 2 fix strobe frequency quantization#5469
kieranskelly wants to merge 21 commits intowled:mainfrom
GridLights:SOL-2-fix-strobe-frequency-quantization

Conversation

@kieranskelly
Copy link
Copy Markdown

@kieranskelly kieranskelly commented Apr 2, 2026

Summary by CodeRabbit

  • New Features

    • Added numerous LED animation effects: custom shapes, novas, black holes, breathing patterns, spirals, and hz-controlled strobing effects.
    • GridLights version information now available via the API.
  • Chores

    • Updated animation rendering frame rate to 100 FPS for smoother effects.
    • Restructured build workflows for improved ESP32 device targeting.
    • Configuration files now tracked in version control.
    • Removed captive portal DNS functionality from access point mode.

Michael Griffin and others added 21 commits November 26, 2024 02:44
- Add gridlights-build.yml GitHub Actions workflow that builds esp32dev
  on every push and pull request, with pip/platformio/.pio caching
- Add platformio_override.ini with GridLights hardware defaults:
  LED pin 4, 37x SK6812 RGBW, relay pin 5 (active low)
- Remove platformio_override.ini from .gitignore so board config is shared

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaced by gridlights-build.yml which uses non-deprecated action versions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Allows manual runs from the GitHub Actions tab.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The .bin lives at .pio/build/<env>/firmware.bin, not build_output/.
Also set if-no-files-found to error so a missing binary fails the build.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…bah, aHourglass, AlmostNuke, aNuke)

- Added 35 new effects in 7 families, each at 3/6/9/12/20 Hz variants
- Mode IDs 214-248, MODE_COUNT updated to 249
- Fixed Frame struct initializers from {data,60,N} to {data,37,1,60,N,255}
- Added missing aSidewaysMerkabah3 implementation
- Fixed duplicate aSidewaysMerkabah6 addEffect registration

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds GRIDLIGHTS_VERSION build flag (2.2.1) to platformio_override.ini and
exposes it as gl_ver in the serializeInfo() response, distinct from the base
WLED ver field.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fails any pull request targeting main where GRIDLIGHTS_VERSION in
platformio_override.ini has not been incremented relative to main.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace edge-triggered strobe toggle with wall-time computation:
  strobeState = (currentTime % cycleTime) < (cycleTime / 2)

The previous toggle approach fired at most once per FRAMETIME, capping
effective flicker frequency at ~1000/(2*FRAMETIME) Hz (~16Hz at 30fps).
The new approach derives strobe state directly from millis(), removing
the dependency on render-call frequency.

Also removes the now-unused lastStrobeTime static variable from both
mode_custom_diamond_spin and mode_black_hole_custom.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 2, 2026

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

This PR introduces GridLights-specific build infrastructure, adds numerous LED animation effects with a frame-based rendering system, increases target FPS from 42 to 100, disables captive portal DNS functionality, and replaces the legacy CI workflow with GridLights-targeted build and version-check workflows.

Changes

Cohort / File(s) Summary
GitHub Actions Workflows
.github/workflows/gridlights-build.yml, .github/workflows/version-check.yml
Added two new workflows: one to build firmware for ESP32 with artifact uploads, and another to enforce semantic version bumps in platformio_override.ini on PRs targeting main.
Removed CI Workflow
.github/workflows/wled-ci.yml
Deleted legacy multi-environment PlatformIO CI workflow that previously built across multiple ESP8266/ESP32 targets and created releases on tags.
Build Configuration
platformio.ini, platformio_override.ini, .gitignore
Commented out multi-environment CI defaults in platformio.ini, set esp32dev as single-binary default, added new platformio_override.ini with GridLights version 2.2.1 and ESP32-specific build flags, and removed platformio_override.ini from .gitignore to version-control it.
LED Effects System
wled00/FX.cpp, wled00/FX.h
Added Frame struct and generic mode_custom_shapes() renderer supporting strobed frame-based animations; implemented 20+ new effect modes (squares, diamonds, novas, black holes, Hz-controlled patterns); increased WLED_FPS from 42 to 100; expanded MODE_COUNT from 187 to 249 with explicit FX_MODE_* macro IDs.
Effect Timing & Infrastructure
wled00/FX_fcn.cpp
Updated Segment::setPixelColor() IRAM attribute from IRAM_ATTR_YN to IRAM_ATTR; modified WS2812FX::service() to update _lastShow immediately after the delay guard, enabling frame timing to advance based on the target schedule.
API & Network
wled00/json.cpp, wled00/wled.cpp
Added gl_ver field to /json/info response with GRIDLIGHTS_VERSION value; disabled captive-portal DNS server logic by commenting out DNS request processing and teardown calls in AP mode.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~70 minutes

Possibly related PRs

Suggested reviewers

  • DedeHai
  • willmmiles
🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The pull request title focuses on 'strobe frequency quantization,' but the changeset contains extensive additions to LED effect modes, CI/CD workflows, version management, and configuration changes far beyond strobe frequency fixes. Revise the title to reflect the full scope of changes, such as 'Add custom LED effects, update CI workflows, and increase frame rate' or provide a more accurate summary of the primary changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 21.62% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch SOL-2-fix-strobe-frequency-quantization

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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