AR: add compile-time flag for "Automatic Gain Control" option #4690
Conversation
## Walkthrough
A new preprocessor conditional for the ESP32 architecture sets a default value for `SR_AGC` if not defined, and initializes the `soundAgc` variable with this value in the audio_reactive.cpp file. The readme documentation is updated to describe the new `SR_AGC` build flag and corrects a minor typo.
## Changes
| Files/Group | Change Summary |
|-------------------------------------------------|---------------------------------------------------------------------------------------------------------|
| usermods/audioreactive/audio_reactive.cpp | Added a preprocessor block to define `SR_AGC` as 0 if undefined (ESP32 only). `soundAgc` now initializes with `SR_AGC` instead of a hardcoded value. No logic changes. |
| usermods/audioreactive/readme.md | Updated documentation to describe the new `-D SR_AGC=x` build flag and corrected a typo regarding RAM resources. No code changes. |Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.31.1)usermods/audioreactive/audio_reactive.cppTip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Automatic Gain Control is a very important aspect of the audioreactive plugin, and is vitally important when the external music volume constantly changes. It makes sense to allow users to choose their preferred AGC behavior at compile-time, since they can already set the Gain and Squelch via flags. Adds `SR_AGC` as a flag, which defaults to 0 (off).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
usermods/audioreactive/readme.md (1)
63-63: Ensure flag description consistency
The new-D SR_AGC=xentry is clear, but to mirror the style ofSR_SQUELCHandSR_GAIN, consider placing the default value(0)immediately after the setting name and moving the(Only ESP32)qualifier to the end.Proposed diff:
-* `-D SR_AGC=x` : (Only ESP32) Default "AGC (Automatic Gain Control)" setting (0): 0=off, 1=normal, 2=vivid, 3=lazy +* `-D SR_AGC=x` : Default "AGC (Automatic Gain Control)" setting (0): 0=off, 1=normal, 2=vivid, 3=lazy (only ESP32)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
usermods/audioreactive/audio_reactive.cpp(1 hunks)usermods/audioreactive/readme.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- usermods/audioreactive/audio_reactive.cpp
🧰 Additional context used
🪛 LanguageTool
usermods/audioreactive/readme.md
[uncategorized] ~65-~65: Loose punctuation mark.
Context: ...need this). * -D I2S_USE_16BIT_SAMPLES: Use 16bit instead of 32bit for internal...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (1)
usermods/audioreactive/readme.md (1)
65-65: Approve typo correction
The spelling of "resources" has been fixed.🧰 Tools
🪛 LanguageTool
[uncategorized] ~65-~65: Loose punctuation mark.
Context: ...need this). *-D I2S_USE_16BIT_SAMPLES: Use 16bit instead of 32bit for internal...(UNLIKELY_OPENING_PUNCTUATION)
|
Oh, I pushed one small tweak: Since the AGC feature only exists for the PS: I disagree with CodeRabbit about putting the note at the end of the line, since the example then looks like "Setting 3 = Lazy (Only ESP32)" which sounds like it only applies to setting 3, and would totally confuse users. And since the AR readme doesn't use any punctuation within bullet points, it gets very awkward to add anything to the end of the sentence. I am open to suggestions if anyone has a better sentence structure, but it's such a minor detail that I don't think it's worth arguing over. 😄 |
Automatic Gain Control is a very important aspect of the audioreactive plugin, and is vitally important when the external music volume constantly changes.
It makes sense to allow users to choose their preferred AGC behavior at compile-time, since they can already set the Gain and Squelch via flags.
Adds
SR_AGCas a flag, which defaults to 0 (off).Notes:
#ifndefsand adding#defineswithin them when flags are only relevant for certain platforms (ESP32), and indented the nested ifdefs the exact same way it's done elsewhere in the AudioReactive usermod. I even copied the end-of-line comment style. 🤷♂️SR_GAIN, SquelchSR_SQUELCHand now AGCSR_AGC.0_15_xbranch, where the repo structure is different. I'll await your feedback first, but I can open a pull request for that branch too if necessary, to save you time. 👍Summary by CodeRabbit
Summary by CodeRabbit
Documentation
Style