ricardoquesada / bluepad32

Bluetooth gamepad, mouse and keyboard support for ESP32 and PicoW
https://bluepad32.readthedocs.io/
Other
503 stars 53 forks source link

[Bug]: Regression - Rapidly calling Controller->setColorLED() creates errors #103

Closed abstractBoyd closed 1 month ago

abstractBoyd commented 1 month ago

What happened?

I ran into an issue recently with the Arduino ESP32 libraries, a program that used to work fine suddenly stopped working after I updated the board driver to esp32_bluepad 4.0.4. The output I was seeing was the attached pictures with a lot of that repeating error message in between bluepadError1 bluepadError2

Bluepad32 Version

Other

Bluepad32 version custom

Arduino esp32_bluepad32 version 4.0.4

Bluepad32 Platform

Arduino IDE

Platform version

Arduino IDE 2.2.1

Controller

DualSense 4 (offbrand)

Microcontroller

ESP32

Microcontroller board

ESP32-WROOM DevKit

OS

Windows

Relevant log output

Firmware: Bluepad32 for Arduino v4.0.4
BD Addr: D4:8A:FC:A0:2F:46
Number of stored bluetooth MAC addresses: 4
DS4: Failed to create virtual device
CALLBACK: Gamepad is connected, index=0
Gamepad model: DualShock 4, VID=0x054c, PID=0x05c4
on_l2cap_data_packet: invalid packet size, ignoring packet
ERROR: circular buffer full. Cannot queue report
ERROR: circular buffer full. Cannot queue report
ERROR: circular buffer full. Cannot queue report
ERROR: circular buffer full. Cannot queue report
ERROR: circular buffer full. Cannot queue report
ERROR: circular buffer full. Cannot queue report
ERROR: circular buffer full. Cannot queue report
...
ERROR: circular buffer full. Cannot queue report
ERROR: circular buffer full. Cannot queue report
EASSERT_PARAM(2097152 0), in rwbt.c at line 393
Guru Meditation Error: Core  0 panic'ed (IllegalInstruction). Exception was unhandled.
Memory dump at 0x4009069c: 00f01d00 00004136 f01d0000
Core  0 register dump:
PC      : 0x400906a3  PS      : 0x00060134  A0      : 0x80085972  A1      : 0x3ffc21cc  
A2      : 0x00000000  A3      : 0x00200000  A4      : 0x00000000  A5      : 0x3f41746c  
A6      : 0x00000189  A7      : 0xfffffffc  A8      : 0x8000814b  A9      : 0x3ffc213c  
A10     : 0x00000000  A11     : 0x3ffc215f  A12     : 0x3ffc210b  A13     : 0x00000033  
A14     : 0x00000000  A15     : 0x3ffc2110  SAR     : 0x00000004  EXCCAUSE: 0x00000000  
EXCVADDR: 0x00000000  LBEG    : 0x400efd40  LEND    : 0x400efd4e  LCOUNT  : 0x00000000  

Backtrace: 0x400906a0:0x3ffc21cc |<-CORRUPTED

ELF file SHA256: 7efc3caacc04d5ac

Rebooting...
ets Jul 29 2019 12:21:46

rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:1
load:0x3fff0030,len:1344
load:0x40078000,len:13964
load:0x40080400,len:3600
entry 0x400805f0
Bluepad32 (C) 2016-2024 Ricardo Quesada and contributors.
Version: v4.0.4
BTstack: Copyright (C) 2017 BlueKitchen GmbH.
BTstack up and running at D4:8A:FC:A0:2F:46
Firmware: Bluepad32 for Arduino v4.0.4
BD Addr: D4:8A:FC:A0:2F:46
Number of stored bluetooth MAC addresses: 4
Firmware: Bluepad32 for Arduino v4.0.4

Relevant sketch

void setup() {
  Serial.begin(115200);
  Serial.printf("Firmware: %s\n", BP32.firmwareVersion());
  const uint8_t *addr = BP32.localBdAddress();
  Serial.printf("BD Addr: %2X:%2X:%2X:%2X:%2X:%2X\n", addr[0], addr[1], addr[2],
                  addr[3], addr[4], addr[5]);

  const bd_addr_t* addresses = nullptr;
  int total = 0;
  // Call the function with the address of addresses and the address of total
  uni_bt_allowlist_get_all(&addresses, &total);
  // we know the btAddress if we have at least one in the list
  knowsBtAddress = total > 0;
  uni_bt_allowlist_set_enabled(knowsBtAddress);
  Serial.printf("Number of stored bluetooth MAC addresses: %d\n", total);

  btResetButtonClicked = isClicked(BLUETOOTH_UNPAIR_BUTTON);

  // Setup the Bluepad32 callbacks
  BP32.setup(&onConnectedGamepad, &onDisconnectedGamepad);
}

// Arduino loop function. Runs in CPU 1
void loop() 
{
    // This call fetches all the gamepad info from the NINA (ESP32) module.
    BP32.update();

    for (int i = 0; i < MAX_GAMEPADS; i++)
    {
        GamepadPtr myGamepad = myGamepads[i];
        if (myGamepad && myGamepad->isConnected())
        {
            processGamepad(myGamepad);
        }
    }
    vTaskDelay (1);
}   // loop()

void processGamepad(ControllerPtr g_gpp) {
    // Process only if controller is connected (BLE is paired and connected).
    if ((g_gpp != NULL) && (g_gpp->isConnected()))
    {
      g_gpp->setColorLED(0, 0, 0);
    }
}
ricardoquesada commented 1 month ago

which is the last known version that works ?

v4.0.4 is using ESP32 Arduino Core 2.0.15, and has some bugs... I'll try to update it to 2.0.16 soon

ricardoquesada commented 1 month ago

mmmm.. has nothing to do with arduino core.

so, what happens is that setColorLED() is trying to send a packet, but the buffer is full. I should refactor that code.

...but you should avoid calling setColorLED() that frequently.

ricardoquesada commented 1 month ago

you can add something like:

  // pseudo code
  if (ledcolor != (0,0,0))
      setColorLED(0,0,0);

or stuff like that

abstractBoyd commented 1 month ago

which is the last known version that works ?

v4.0.4 is using ESP32 Arduino Core 2.0.15, and has some bugs... I'll try to update it to 2.0.16 soon

3.7.0 is the last one I checked that seemed to work with that code.

abstractBoyd commented 1 month ago

mmmm.. has nothing to do with arduino core.

so, what happens is that setColorLED() is trying to send a packet, but the buffer is full. I should refactor that code.

...but you should avoid calling setColorLED() that frequently.

Absolutely, I immediately updated our code as soon as I noticed what it was doing - in my defense, it's inherited code :)

I am thankful that the bug revealed the inefficiency in our code, and I simply hope to return the favor and help make your code better as well :)

ricardoquesada commented 1 month ago

great!

(I'm closing the bug... only because this is the "expected" behavior... using a vTaskDelay(1) and calling setColorLED() will flood the circular buffer).

Feel free to re-open if you think otherwise (if so, explain why).

Thanks