Skip to content

fixed ADA light Serial conditions for USB CDC (some ESP32 C3 / S2 / S3 ) #4792

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

Open
rommo911 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fixed ADA light Serial conditions for USB CDC (some ESP32 C3 / S2 / S3 ) #4792

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
82d099f
fixed ADA light Serial conditions for USB CDC (aka ESP32 C3 / S2 / S3 )
rommo911 Jul 28, 2025
48d698b
restored missing Serial condition
rommo911 Jul 28, 2025
8a26210
Refactor serial condition for USB CDC support / check can RX
rommo911 Nov 9, 2025
8a09156
restored original condition for serialCanRX in handleSerial()
rommo911 Nov 12, 2025
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
7 changes: 7 additions & 0 deletions wled00/wled.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,15 @@ void WLED::setup()
findWiFi(true); // start scanning for available WiFi-s

// all GPIOs are allocated at this point
#ifdef ARDUINO_USB_CDC_ON_BOOT
// USB CDC means USB D+ D- are used .. pin allocator will always return. force it as enabled instead
serialCanRX = true;
serialCanTX = true;
#else
// use RX/TX as set by the framework - these boards do _not_ have RX=3 and TX=1
serialCanRX = !PinManager::isPinAllocated(hardwareRX); // Serial RX pin (GPIO 3 on ESP32 and ESP8266)
serialCanTX = !PinManager::isPinAllocated(hardwareTX) || PinManager::getPinOwner(hardwareTX) == PinOwner::DebugOut; // Serial TX pin (GPIO 1 on ESP32 and ESP8266)
#endif // ARDUINO_USB_CDC_ON_BOOT

#ifdef WLED_ENABLE_ADALIGHT
//Serial RX (Adalight, Improv, Serial JSON) only possible if GPIO3 unused
Expand Down
4 changes: 3 additions & 1 deletion wled00/wled_serial.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ void sendBytes(){

void handleSerial()
{
if (!(serialCanRX && Serial)) return; // arduino docs: `if (Serial)` indicates whether or not the USB CDC serial connection is open. For all non-USB CDC ports, this will always return true
if (serialCanRX == false && Serial) {

@softhack007 softhack007 Nov 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @rommo911, could it be that your proposed change does not lead to the same behaviour when not having USB-CDC ?

old condition truth table:

canRX Serial result
false - true = return
true false true = return
true true false = continue

new condition truth table:

canRX Serial result
false true true = return
false false false = continue
true - false = continue

As the new condition you propose seems to be broken - how about just removing the "early exit" for the case of USB_CDC ?

#if ARDUINO_USB_CDC_ON_BOOT
   if (!Serial) return;
#else
   if (!(serialCanRX && Serial)) return;
#endif

@blazoncek not sure if serialCanRX is used in other places in the code, so we still need the change in wled.cpp?

@blazoncek blazoncek Nov 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

serialCanRX is only used to determine if ADA/Improv can be used. Nothing else AFAIK.

return;
}; // arduino docs: `if (Serial)` indicates whether or not the USB CDC serial connection is open. For all non-USB CDC ports, this will always return true

static auto state = AdaState::Header_A;
static uint16_t count = 0;
Expand Down