Skip to content

Conversation

@netmindz
Copy link
Member

@netmindz netmindz commented Nov 17, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Removed a compile-time error that was blocking compilation when DMX output mode was enabled, allowing successful builds with DMX support.

@netmindz netmindz added this to the 0.15.2 milestone Nov 17, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 17, 2025

Walkthrough

A compile-time guard that prevented compilation when WLED_ENABLE_DMX was defined has been removed from the audio reactive module. This eliminates the previous build-time incompatibility between audio reactive and DMX Out functionality without altering runtime logic.

Changes

Cohort / File(s) Change Summary
Audio Reactive Module
usermods/audioreactive/audio_reactive.cpp
Removed #ifdef WLED_ENABLE_DMX preprocessor guard that emitted a compile-time error, allowing the audio reactive module to compile when DMX Out is enabled

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify the removed preprocessor guard and error message to understand the original incompatibility
  • Confirm whether removing this guard introduces any hidden runtime conflicts between audio reactive and DMX functionality
  • Assess whether other modules contain similar guards that may need equivalent updates

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: removing a compile-time guard that prevented audio reactive functionality from working with DMX output, directly addressing issue #4298.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 271e9ac and d1ed584.

📒 Files selected for processing (1)
  • usermods/audioreactive/audio_reactive.cpp (0 hunks)
💤 Files with no reviewable changes (1)
  • usermods/audioreactive/audio_reactive.cpp

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.

@netmindz netmindz removed this from the 0.15.2 milestone Nov 17, 2025
@netmindz netmindz requested a review from softhack007 November 17, 2025 17:46
@DedeHai
Copy link
Collaborator

DedeHai commented Nov 17, 2025

do you know why this was added in the first place?

@softhack007
Copy link
Member

do you know why this was added in the first place?

DMX ? no idea, DMX never made any sense to me 😝

AR vs. DMX serial out? You can find a lengthy discussion in #4298. It looks like some users had crashes when running both DMX-serial out and audioreactive on the same board, so this was added as a precaution, until we understand better what's going on.

@softhack007
Copy link
Member

softhack007 commented Nov 17, 2025

@netmindz It might be good to also try my last comment in #4298 (comment)

void handleDMXOutput()
{
// don't act, when in DMX Proxy mode
if (e131ProxyUniverse != 0) return;

handleDMXOutput() currently doesn't have any rate limiting, Its called in every loop() iteration, so might run far too often while there is actually no change to any LEDs. I think adding a rate limit right here should help to avoid Serial buffer congestion.

Something like the below snippet could help to align dmx output rates with LEDs rendering times

   static unsigned last_dmx_time = millis();
   if (millis() - last_dmx_time < strip.getFrameTime()) return;  // you can also try "10" or whatever the minumum frametime for DMX is
   last_dmx_time = millis();

Important: needs testing with real DMX hardware, to verify that DMX timing is still correct.

@netmindz netmindz self-assigned this Nov 18, 2025
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.

3 participants