Skip to content

Update crude parallel I2S detection #4713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
willmmiles merged 1 commit into from
Jun 14, 2025
Merged

Update crude parallel I2S detection #4713

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion wled00/data/settings_leds.htm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@
});
const S2 = (oMaxB == 14) && (maxV == 4);
const S3 = (oMaxB == 14) && (maxV == 6);
if (oMaxB == 19 || S2 || S3) { // TODO: crude ESP32 & S2/S3 detection
if (oMaxB == 32 || S2 || S3) { // TODO: crude ESP32 & S2/S3 detection
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

LGTM! Change correctly updates ESP32 detection for parallel I2S.

This change appropriately addresses the issue where ESP32 devices reporting 32 buses were missing the parallel I2S checkbox. The S2/S3 detection logic remains unchanged.

However, please verify backward compatibility and update the related comment inconsistency:

  1. Backward compatibility concern: If older ESP32 firmware/hardware still reports oMaxB == 19, those devices would lose the parallel I2S option. Please confirm that all target ESP32 devices now report 32 buses.

  2. Comment inconsistency: The comment in the bLimits function (line 46) states "20 - ESP32" for maxB, but the actual detection values have been 19 (old) and now 32 (new). Please update this comment to reflect the correct value.

🏁 Script executed:

#!/bin/bash
# Description: Check for other references to ESP32 bus counts to ensure consistency
# Expected: Find any hardcoded values or comments that might need updating

# Search for potential ESP32 bus count references
rg -i "esp32.*bus" -A 2 -B 2
rg "\b(19|20|32)\b.*esp32" -i -A 2 -B 2
rg "maxB.*esp32" -i -A 2 -B 2

Length of output: 6635

Ensure ESP32 detection comment is updated and handle legacy bus counts if needed

Please address the following:

  • Update the comment in wled00/data/settings_leds.htm to reflect the new “32” bus count for ESP32:

    - oMaxB = maxB = b; // maxB - max buses (can be changed if using ESP32 parallel I2S): 20 - ESP32, 14 - S3/S2, 6 - C3, 4 - 8266
+ oMaxB = maxB = b; // maxB - max buses (can be changed if using ESP32 parallel I2S): 32 - ESP32, 14 - S3/S2, 6 - C3, 4 - 8266

  • If you need to support any older ESP32 variants still reporting 19 buses, consider restoring a fallback in the UI logic. For example:

    if (oMaxB == 32 || oMaxB == 19 || S2 || S3) { … }

  • Confirm that all current ESP32 targets now report 32 buses and that no other hardcoded “19” or “20” bus‐count references remain.

🤖 Prompt for AI Agents 
In wled00/data/settings_leds.htm at line 356, update the comment related to
ESP32 bus count to reflect the new value of 32 instead of 19 or 20.
Additionally, modify the condition to include a fallback for older ESP32 devices
that report 19 buses by changing the if statement to check for oMaxB == 32 or
oMaxB == 19 or S2 or S3. Finally, verify that no other parts of the codebase
contain hardcoded values of 19 or 20 for ESP32 bus counts and update or remove
them as necessary to ensure consistency and backward compatibility.

if (maxLC > 300 || dC <= 2) {
d.Sf["PR"].checked = false;
gId("prl").classList.add("hide");
Expand Down
Loading