Skip to content
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.

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

Missing calls to onDisconnect() callback #886

Open
benjie-git opened this issue Feb 6, 2025 · 24 comments
Open

Missing calls to onDisconnect() callback #886

benjie-git opened this issue Feb 6, 2025 · 24 comments

Comments

@benjie-git
Copy link

benjie-git commented Feb 6, 2025

I'm using NimBLE 2.2.1 on an ESP32-S3.

I'm running a BLE server that allows up to 4 devices to connect. I have CONFIG_BT_NIMBLE_MAX_CONNECTIONS set to 4. I keep advertising unless I have 4 active connections.

I've noticed that when I have 2 connected clients, I can disconnect one (initiated from the client side), and I see an onDisconnect() call, and the client will quickly reconnect. I can then disconnect from the other client, and I'll see an onDisconnect() call, and the client will reconnect. I can continue alternating, and this keeps working.

But if I disconnect and reconnect from the same client 2 times in a row, the onDisconnect() call is not called the 2nd time, and the server->getConnectedCount() fails to decrement. After disconnecting a 3rd time in a row from the same client, there is still no onDisconnect() call, the connected count reaches 4, advertising seems to stop, and my clients can no longer connect, although any remaining connected clients remain connected and continue working. At that point I need to reboot the ESP32 to get advertising to work again.

I'm seeing this problem when I disconnect/reconnect multiple times from my Mac, although I do not see the problem when I disconnect/reconnect repeatedly from an iPhone.

@h2zero
Copy link
Owner

h2zero commented Feb 6, 2025

How quickly are you repeating the disconnect? Example code?

@benjie-git
Copy link
Author

Here is a test case. It acts as an XBox game controller. When I disconnect quickly after connection a few times in a row, NimBLE seems to keep a connection that the mac thinks it has already closed. After doing this a couple of times, it runs out of connection slots (4), and stops advertising/allowing new connections.

But I added workaround code into onConnect() (currently commented out), to disconnect any lingering connections from the same address as the current incoming connection. If you can first reproduce the problem, then you can uncomment and try the workaround.

#include <Arduino.h>
#include <NimBLEDevice.h>
#include <NimBLEHIDDevice.h>

const char *XB_NAME = "Xbox Wireless Controller";
const char *XB_MANUFACTURER = "Microsoft";

#define SERVICE_UUID_DEVICE_INFORMATION        "180A"      // Service - Device information
#define CHARACTERISTIC_UUID_SYSTEM_ID          "2A23"      // Characteristic - System ID 0x2A23
#define CHARACTERISTIC_UUID_MODEL_NUMBER       "2A24"      // Characteristic - Model Number String - 0x2A24
#define CHARACTERISTIC_UUID_SERIAL_NUMBER      "2A25"      // Characteristic - Serial Number String - 0x2A25
#define CHARACTERISTIC_UUID_FIRMWARE_REVISION  "2A26"      // Characteristic - Firmware Revision String - 0x2A26

// Product: latest Xbox series X wireless controller
#define VENDOR_USB_SOURCE 0x02
#define XBOX_VENDOR_ID 0x045E

#define XBOX_INPUT_REPORT_ID 0x01
#define XBOX_EXTRA_INPUT_REPORT_ID 0x02
#define XBOX_OUTPUT_REPORT_ID 0x03
#define XBOX_EXTRA_OUTPUT_REPORT_ID 0x04

#define XBOX_PRODUCT_ID 0x0B13
#define XBOX_BCD_DEVICE_ID 0x0509
#define XBOX_SERIAL "09711282953030"
#define XBOX_MODEL "1914"
#define XBOX_FW_VER "5.9.2709.0"

static const uint8_t Xbox_HIDDescriptor[] {
    0x05, 0x01,                 //(GLOBAL) USAGE_PAGE         0x0001 Generic Desktop Page 
    0x09, 0x05,                 //(LOCAL)  USAGE              0x00010005 Game Pad (Application Collection)  
    0xA1, 0x01,                 //(MAIN)   COLLECTION         0x01 Application (Usage=0x00010005: Page=Generic Desktop Page, Usage=Game Pad, Type=Application Collection)
    0x85, XBOX_INPUT_REPORT_ID, //  (GLOBAL) REPORT_ID          0x01 (1)  
    0x09, 0x01,                 //  (LOCAL)  USAGE              0x00010001 Pointer (Physical Collection)  
    0xA1, 0x00,                 //  (MAIN)   COLLECTION         0x00 Physical (Usage=0x00010001: Page=Generic Desktop Page, Usage=Pointer, Type=Physical Collection)
    0x09, 0x30,                 //    (LOCAL)  USAGE              0x00010030 X (Dynamic Value)  
    0x09, 0x31,                 //    (LOCAL)  USAGE              0x00010031 Y (Dynamic Value)  
    0x15, 0x00,                 //    (GLOBAL) LOGICAL_MINIMUM    0x00 (0)  <-- Info: Consider replacing 15 00 with 14
    0x27, 0xFF, 0xFF, 0x00, 0x00,//    (GLOBAL) LOGICAL_MAXIMUM    0x0000FFFF (65535)  
    0x95, 0x02,                 //    (GLOBAL) REPORT_COUNT       0x02 (2) Number of fields  
    0x75, 0x10,                 //    (GLOBAL) REPORT_SIZE        0x10 (16) Number of bits per field  
    0x81, 0x02,                 //    (MAIN)   INPUT              0x00000002 (2 fields x 16 bits) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0xC0,                       //  (MAIN)   END_COLLECTION     Physical 
    0x09, 0x01,                 //  (LOCAL)  USAGE              0x00010001 Pointer (Physical Collection)  
    0xA1, 0x00,                 //  (MAIN)   COLLECTION         0x00 Physical (Usage=0x00010001: Page=Generic Desktop Page, Usage=Pointer, Type=Physical Collection)
    0x09, 0x32,                 //    (LOCAL)  USAGE              0x00010032 Z (Dynamic Value)  
    0x09, 0x35,                 //    (LOCAL)  USAGE              0x00010035 Rz (Dynamic Value)  
    0x15, 0x00,                 //    (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x27, 0xFF, 0xFF, 0x00, 0x00,//    (GLOBAL) LOGICAL_MAXIMUM    0x0000FFFF (65535) <-- Redundant: LOGICAL_MAXIMUM is already 65535 
    0x95, 0x02,                 //    (GLOBAL) REPORT_COUNT       0x02 (2) Number of fields <-- Redundant: REPORT_COUNT is already 2 
    0x75, 0x10,                 //    (GLOBAL) REPORT_SIZE        0x10 (16) Number of bits per field <-- Redundant: REPORT_SIZE is already 16 
    0x81, 0x02,                 //    (MAIN)   INPUT              0x00000002 (2 fields x 16 bits) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0xC0,                       //  (MAIN)   END_COLLECTION     Physical 
    0x05, 0x02,                 //  (GLOBAL) USAGE_PAGE         0x0002 Simulation Controls Page 
    0x09, 0xC5,                 //  (LOCAL)  USAGE              0x000200C5 Brake (Dynamic Value)  
    0x15, 0x00,                 //  (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x26, 0xFF, 0x03,           //  (GLOBAL) LOGICAL_MAXIMUM    0x03FF (1023)  
    0x95, 0x01,                 //  (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields  
    0x75, 0x0A,                 //  (GLOBAL) REPORT_SIZE        0x0A (10) Number of bits per field  
    0x81, 0x02,                 //  (MAIN)   INPUT              0x00000002 (1 field x 10 bits) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x15, 0x00,                 //  (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x25, 0x00,                 //  (GLOBAL) LOGICAL_MAXIMUM    0x00 (0)  <-- Info: Consider replacing 25 00 with 24
    0x75, 0x06,                 //  (GLOBAL) REPORT_SIZE        0x06 (6) Number of bits per field  
    0x95, 0x01,                 //  (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields <-- Redundant: REPORT_COUNT is already 1 
    0x81, 0x03,                 //  (MAIN)   INPUT              0x00000003 (1 field x 6 bits) 1=Constant 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x05, 0x02,                 //  (GLOBAL) USAGE_PAGE         0x0002 Simulation Controls Page <-- Redundant: USAGE_PAGE is already 0x0002
    0x09, 0xC4,                 //  (LOCAL)  USAGE              0x000200C4 Accelerator (Dynamic Value)  
    0x15, 0x00,                 //  (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x26, 0xFF, 0x03,           //  (GLOBAL) LOGICAL_MAXIMUM    0x03FF (1023)  
    0x95, 0x01,                 //  (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields <-- Redundant: REPORT_COUNT is already 1 
    0x75, 0x0A,                 //  (GLOBAL) REPORT_SIZE        0x0A (10) Number of bits per field  
    0x81, 0x02,                 //  (MAIN)   INPUT              0x00000002 (1 field x 10 bits) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x15, 0x00,                 //  (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x25, 0x00,                 //  (GLOBAL) LOGICAL_MAXIMUM    0x00 (0)  <-- Info: Consider replacing 25 00 with 24
    0x75, 0x06,                 //  (GLOBAL) REPORT_SIZE        0x06 (6) Number of bits per field  
    0x95, 0x01,                 //  (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields <-- Redundant: REPORT_COUNT is already 1 
    0x81, 0x03,                 //  (MAIN)   INPUT              0x00000003 (1 field x 6 bits) 1=Constant 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x05, 0x01,                 //  (GLOBAL) USAGE_PAGE         0x0001 Generic Desktop Page 
    0x09, 0x39,                 //  (LOCAL)  USAGE              0x00010039 Hat switch (Dynamic Value)  
    0x15, 0x01,                 //  (GLOBAL) LOGICAL_MINIMUM    0x01 (1)  
    0x25, 0x08,                 //  (GLOBAL) LOGICAL_MAXIMUM    0x08 (8)  
    0x35, 0x00,                 //  (GLOBAL) PHYSICAL_MINIMUM   0x00 (0)  <-- Info: Consider replacing 35 00 with 34
    0x46, 0x3B, 0x01,           //  (GLOBAL) PHYSICAL_MAXIMUM   0x013B (315)  
    0x66, 0x14, 0x00,           //  (GLOBAL) UNIT               0x0014 Rotation in degrees [1° units] (4=System=English Rotation, 1=Rotation=Degrees)  <-- Info: Consider replacing 66 1400 with 65 14
    0x75, 0x04,                 //  (GLOBAL) REPORT_SIZE        0x04 (4) Number of bits per field  
    0x95, 0x01,                 //  (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields <-- Redundant: REPORT_COUNT is already 1 
    0x81, 0x42,                 //  (MAIN)   INPUT              0x00000042 (1 field x 4 bits) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 1=Null 0=NonVolatile 0=Bitmap 
    0x75, 0x04,                 //  (GLOBAL) REPORT_SIZE        0x04 (4) Number of bits per field <-- Redundant: REPORT_SIZE is already 4 
    0x95, 0x01,                 //  (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields <-- Redundant: REPORT_COUNT is already 1 
    0x15, 0x00,                 //  (GLOBAL) LOGICAL_MINIMUM    0x00 (0)  <-- Info: Consider replacing 15 00 with 14
    0x25, 0x00,                 //  (GLOBAL) LOGICAL_MAXIMUM    0x00 (0)  <-- Info: Consider replacing 25 00 with 24
    0x35, 0x00,                 //  (GLOBAL) PHYSICAL_MINIMUM   0x00 (0) <-- Redundant: PHYSICAL_MINIMUM is already 0 <-- Info: Consider replacing 35 00 with 34
    0x45, 0x00,                 //  (GLOBAL) PHYSICAL_MAXIMUM   0x00 (0)  <-- Info: Consider replacing 45 00 with 44
    0x65, 0x00,                 //  (GLOBAL) UNIT               0x00 No unit (0=None)  <-- Info: Consider replacing 65 00 with 64
    0x81, 0x03,                 //  (MAIN)   INPUT              0x00000003 (1 field x 4 bits) 1=Constant 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x05, 0x09,                 //  (GLOBAL) USAGE_PAGE         0x0009 Button Page 
    0x19, 0x01,                 //  (LOCAL)  USAGE_MINIMUM      0x00090001 Button 1 Primary/trigger (Selector, On/Off Control, Momentary Control, or One Shot Control)  
    0x29, 0x0F,                 //  (LOCAL)  USAGE_MAXIMUM      0x0009000F Button 15 (Selector, On/Off Control, Momentary Control, or One Shot Control)  
    0x15, 0x00,                 //  (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x25, 0x01,                 //  (GLOBAL) LOGICAL_MAXIMUM    0x01 (1)  
    0x75, 0x01,                 //  (GLOBAL) REPORT_SIZE        0x01 (1) Number of bits per field  
    0x95, 0x0F,                 //  (GLOBAL) REPORT_COUNT       0x0F (15) Number of fields  
    0x81, 0x02,                 //  (MAIN)   INPUT              0x00000002 (15 fields x 1 bit) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x15, 0x00,                 //  (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x25, 0x00,                 //  (GLOBAL) LOGICAL_MAXIMUM    0x00 (0)  <-- Info: Consider replacing 25 00 with 24
    0x75, 0x01,                 //  (GLOBAL) REPORT_SIZE        0x01 (1) Number of bits per field <-- Redundant: REPORT_SIZE is already 1 
    0x95, 0x01,                 //  (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields  
    0x81, 0x03,                 //  (MAIN)   INPUT              0x00000003 (1 field x 1 bit) 1=Constant 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x05, 0x0C,                 //  (GLOBAL) USAGE_PAGE         0x000C Consumer Device Page 
    0x0A, 0xB2, 0x00,           //  (LOCAL)  USAGE              0x000C00B2 Record (On/Off Control)  <-- Info: Consider replacing 0A B200 with 09 B2
    0x15, 0x00,                 //  (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x25, 0x01,                 //  (GLOBAL) LOGICAL_MAXIMUM    0x01 (1)  
    0x95, 0x01,                 //  (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields <-- Redundant: REPORT_COUNT is already 1 
    0x75, 0x01,                 //  (GLOBAL) REPORT_SIZE        0x01 (1) Number of bits per field <-- Redundant: REPORT_SIZE is already 1 
    0x81, 0x02,                 //  (MAIN)   INPUT              0x00000002 (1 field x 1 bit) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x15, 0x00,                 //  (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x25, 0x00,                 //  (GLOBAL) LOGICAL_MAXIMUM    0x00 (0)  <-- Info: Consider replacing 25 00 with 24
    0x75, 0x07,                 //  (GLOBAL) REPORT_SIZE        0x07 (7) Number of bits per field  
    0x95, 0x01,                 //  (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields <-- Redundant: REPORT_COUNT is already 1 
    0x81, 0x03,                 //  (MAIN)   INPUT              0x00000003 (1 field x 7 bits) 1=Constant 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x05, 0x0F,                 //  (GLOBAL) USAGE_PAGE         0x000F Physical Interface Device Page 
    0x09, 0x21,                 //  (LOCAL)  USAGE              0x000F0021 Set Effect Report (Logical Collection)  
    0x85, XBOX_OUTPUT_REPORT_ID,//  (GLOBAL) REPORT_ID          0x03 (3)  
    0xA1, 0x02,                 //  (MAIN)   COLLECTION         0x02 Logical (Usage=0x000F0021: Page=Physical Interface Device Page, Usage=Set Effect Report, Type=Logical Collection)
    0x09, 0x97,                 //    (LOCAL)  USAGE              0x000F0097 DC Enable Actuators (Selector)  
    0x15, 0x00,                 //    (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x25, 0x01,                 //    (GLOBAL) LOGICAL_MAXIMUM    0x01 (1)  
    0x75, 0x04,                 //    (GLOBAL) REPORT_SIZE        0x04 (4) Number of bits per field  
    0x95, 0x01,                 //    (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields <-- Redundant: REPORT_COUNT is already 1 
    0x91, 0x02,                 //    (MAIN)   OUTPUT             0x00000002 (1 field x 4 bits) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x15, 0x00,                 //    (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x25, 0x00,                 //    (GLOBAL) LOGICAL_MAXIMUM    0x00 (0)  <-- Info: Consider replacing 25 00 with 24
    0x75, 0x04,                 //    (GLOBAL) REPORT_SIZE        0x04 (4) Number of bits per field <-- Redundant: REPORT_SIZE is already 4 
    0x95, 0x01,                 //    (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields <-- Redundant: REPORT_COUNT is already 1 
    0x91, 0x03,                 //    (MAIN)   OUTPUT             0x00000003 (1 field x 4 bits) 1=Constant 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x09, 0x70,                 //    (LOCAL)  USAGE              0x000F0070 Magnitude (Dynamic Value)  
    0x15, 0x00,                 //    (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x25, 0x64,                 //    (GLOBAL) LOGICAL_MAXIMUM    0x64 (100)  
    0x75, 0x08,                 //    (GLOBAL) REPORT_SIZE        0x08 (8) Number of bits per field  
    0x95, 0x04,                 //    (GLOBAL) REPORT_COUNT       0x04 (4) Number of fields  
    0x91, 0x02,                 //    (MAIN)   OUTPUT             0x00000002 (4 fields x 8 bits) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x09, 0x50,                 //    (LOCAL)  USAGE              0x000F0050 Duration (Dynamic Value)  
    0x66, 0x01, 0x10,           //    (GLOBAL) UNIT               0x1001 Time in seconds [1 s units] (1=System=SI Linear, 1=Time=Seconds)  
    0x55, 0x0E,                 //    (GLOBAL) UNIT_EXPONENT      0x0E (Unit Value x 10⁻²)  
    0x15, 0x00,                 //    (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x26, 0xFF, 0x00,           //    (GLOBAL) LOGICAL_MAXIMUM    0x00FF (255)  
    0x75, 0x08,                 //    (GLOBAL) REPORT_SIZE        0x08 (8) Number of bits per field <-- Redundant: REPORT_SIZE is already 8 
    0x95, 0x01,                 //    (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields  
    0x91, 0x02,                 //    (MAIN)   OUTPUT             0x00000002 (1 field x 8 bits) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x09, 0xA7,                 //    (LOCAL)  USAGE              0x000F00A7 Start Delay (Dynamic Value)  
    0x15, 0x00,                 //    (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x26, 0xFF, 0x00,           //    (GLOBAL) LOGICAL_MAXIMUM    0x00FF (255) <-- Redundant: LOGICAL_MAXIMUM is already 255 
    0x75, 0x08,                 //    (GLOBAL) REPORT_SIZE        0x08 (8) Number of bits per field <-- Redundant: REPORT_SIZE is already 8 
    0x95, 0x01,                 //    (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields <-- Redundant: REPORT_COUNT is already 1 
    0x91, 0x02,                 //    (MAIN)   OUTPUT             0x00000002 (1 field x 8 bits) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0x65, 0x00,                 //    (GLOBAL) UNIT               0x00 No unit (0=None)  <-- Info: Consider replacing 65 00 with 64
    0x55, 0x00,                 //    (GLOBAL) UNIT_EXPONENT      0x00 (Unit Value x 10⁰)  <-- Info: Consider replacing 55 00 with 54
    0x09, 0x7C,                 //    (LOCAL)  USAGE              0x000F007C Loop Count (Dynamic Value)  
    0x15, 0x00,                 //    (GLOBAL) LOGICAL_MINIMUM    0x00 (0) <-- Redundant: LOGICAL_MINIMUM is already 0 <-- Info: Consider replacing 15 00 with 14
    0x26, 0xFF, 0x00,           //    (GLOBAL) LOGICAL_MAXIMUM    0x00FF (255) <-- Redundant: LOGICAL_MAXIMUM is already 255 
    0x75, 0x08,                 //    (GLOBAL) REPORT_SIZE        0x08 (8) Number of bits per field <-- Redundant: REPORT_SIZE is already 8 
    0x95, 0x01,                 //    (GLOBAL) REPORT_COUNT       0x01 (1) Number of fields <-- Redundant: REPORT_COUNT is already 1 
    0x91, 0x02,                 //    (MAIN)   OUTPUT             0x00000002 (1 field x 8 bits) 0=Data 1=Variable 0=Absolute 0=NoWrap 0=Linear 0=PrefState 0=NoNull 0=NonVolatile 0=Bitmap 
    0xC0,                       //(MAIN)   END_COLLECTION     Logical 
    0xC0                    //(MAIN)   END_COLLECTION     Application 
}; 

#pragma pack(push, 1)
struct XboxGamepadInputReportData {
    uint16_t x = 0;             // Left joystick X
    uint16_t y = 0;             // Left joystick Y
    uint16_t z = 0;             // Right jostick X
    uint16_t rz = 0;            // Right joystick Y
    uint16_t brake = 0;         // 10 bits for brake (left trigger) + 6 bit padding (2 bytes)
    uint16_t accelerator = 0;   // 10 bits for accelerator (right trigger) + 6bit padding
    uint8_t  hat = 0x00;        // 4bits for hat switch (Dpad) + 4 bit padding (1 byte) 
    uint16_t buttons = 0x00;    // 15 * 1bit for buttons + 1 bit padding (2 bytes)
    uint8_t  share = 0x00;      // 1 bits for share/menu button + 7 bit padding (1 byte)
};
#pragma pack(pop)

static NimBLEServer *pServer;
static NimBLEHIDDevice *hid;
static NimBLECharacteristic *inputChar;
static XboxGamepadInputReportData inputReport;


class ServerCallbacks : public NimBLEServerCallbacks {
    void onConnect(NimBLEServer* pServer, NimBLEConnInfo& connInfo) override {

        // // Detect old connections from the same peer address, and disconnect them
        // // This section works around issue #886  https://github.com/h2zero/NimBLE-Arduino/issues/886
        // std::vector<uint16_t> peers = pServer->getPeerDevices();
        // for(const uint16_t& peerHandle : peers) {
        //     NimBLEConnInfo peer = pServer->getPeerInfoByHandle(peerHandle);
        //     if (peer.getConnHandle() != connInfo.getConnHandle() && peer.getAddress() == connInfo.getAddress()) {
        //         printf("Found duplicate peer\n");
        //         NimBLEDevice::getServer()->disconnect(peerHandle);
        //     }
        // }
        // // End workaround

        printf("Client connected %d    (%s)\n", pServer->getConnectedCount(), connInfo.getAddress().toString().c_str());
        pServer->updateConnParams(connInfo.getConnHandle(), 6, 12, 0, 180);
        NimBLEDevice::startAdvertising();
    }

    void onAuthenticationComplete(NimBLEConnInfo& connInfo) override {
        /** Check that encryption was successful, if not we disconnect the client */
        if (!connInfo.isEncrypted()) {
            NimBLEDevice::getServer()->disconnect(connInfo.getConnHandle());
            printf("Encrypt connection failed - disconnecting client (%s)\n", connInfo.getAddress().toString().c_str());
            return;
        }

        printf("Secured connection    (%s)\n", connInfo.getAddress().toString().c_str());
    }

    void onConnParamsUpdate(NimBLEConnInfo& connInfo) override {
        printf("Conn Params %d, %d   (%s)\n", connInfo.getConnInterval(),
            connInfo.getConnTimeout(), connInfo.getAddress().toString().c_str());
    }

    void onDisconnect(NimBLEServer* pServer, NimBLEConnInfo& connInfo, int reason) override {
        printf("Client disconnected %d (%s)\n", pServer->getConnectedCount(), connInfo.getAddress().toString().c_str());
        NimBLEDevice::startAdvertising();
    }
} serverCallbacks;


void setup(void) {
    vTaskDelay(1500);
    printf("\n\nStarting NimBLE Server\n");

    NimBLEDevice::init(XB_NAME);
    NimBLEDevice::setSecurityAuth(true, false, false);

    pServer = NimBLEDevice::createServer();
    pServer->setCallbacks(&serverCallbacks);

    hid = new NimBLEHIDDevice(pServer);
    hid->setManufacturer(XB_MANUFACTURER);
    hid->setPnp(VENDOR_USB_SOURCE, XBOX_VENDOR_ID, XBOX_PRODUCT_ID, XBOX_BCD_DEVICE_ID);
    hid->setHidInfo(0x00, 0x01);
    hid->setReportMap((uint8_t*)Xbox_HIDDescriptor, sizeof(Xbox_HIDDescriptor));

    inputChar = hid->getInputReport(XBOX_INPUT_REPORT_ID);
    inputChar->setValue((uint8_t*)&inputReport, sizeof(inputReport));

    // Create device UUID
    NimBLEService *service = pServer->getServiceByUUID(SERVICE_UUID_DEVICE_INFORMATION);
  
    // Create characteristics
    BLECharacteristic* characteristic_Model_Number = service->createCharacteristic(
        CHARACTERISTIC_UUID_MODEL_NUMBER, NIMBLE_PROPERTY::READ);
    characteristic_Model_Number->setValue((const uint8_t*)XBOX_MODEL, strlen(XBOX_MODEL));
  
    BLECharacteristic* characteristic_Serial_Number = service->createCharacteristic(
        CHARACTERISTIC_UUID_SERIAL_NUMBER, NIMBLE_PROPERTY::READ);
    characteristic_Serial_Number->setValue((const uint8_t*)XBOX_SERIAL, strlen(XBOX_SERIAL));
  
    BLECharacteristic* characteristic_Firmware_Revision = service->createCharacteristic(
        CHARACTERISTIC_UUID_FIRMWARE_REVISION, NIMBLE_PROPERTY::READ);
    characteristic_Firmware_Revision->setValue((const uint8_t*)XBOX_FW_VER, strlen(XBOX_FW_VER));
  
    hid->setBatteryLevel(100);

    hid->startServices();

    // Start BLE advertisement
    pServer->getAdvertising()->setAppearance(HID_GAMEPAD);
    pServer->getAdvertising()->addServiceUUID(hid->getHidService()->getUUID());
    pServer->getAdvertising()->enableScanResponse(true);
    pServer->getAdvertising()->start();
    printf("Advertising Started\n");
}


void loop() {
    inputReport.x += 200;
    inputReport.rz += 200;
    inputChar->setValue((uint8_t*)&inputReport, sizeof(inputReport));
    inputChar->notify();

    vTaskDelay(20);
}

@h2zero
Copy link
Owner

h2zero commented Feb 7, 2025

Try changing this pServer->updateConnParams(connInfo.getConnHandle(), 6, 12, 0, 180); to have a lower supervision timeout, say 30.

@benjie-git
Copy link
Author

Thanks! I just tested with pServer->updateConnParams(connInfo.getConnHandle(), 6, 12, 0, 30); Unfortunately, this change doesn't completely solve my issue. This issue is intermittent, and takes a few tries to trigger, but I think it is taking more disconnect/reconnect attempts to cause the issue now, with this change. But it feels like it's masking some other problem.

@h2zero
Copy link
Owner

h2zero commented Feb 7, 2025

It takes time for the disconnect to happen and if the peer is reconnecting faster than you disconnect from them then you will have ghost connections. reducing the timeout reduced the amount of time for the disconnect to register, it was testing, not fixing.

In your code you are continuously advertising so the client is able to reconnect faster than the disconnect event can complete. The fact you saw a change indicates that, so, as an HID device you should only advertise when you do not have a peer connected, removing NimBLEDevice::startAdvertising(); from the onConnect callback should fix this.

@benjie-git
Copy link
Author

Thanks. My use case is using this as a controller for multiple clients at once, which I know is not standard HID operation, so I believe that to do this I need to continue advertising after each connection, so that the remaining clients can connect.

@h2zero
Copy link
Owner

h2zero commented Feb 7, 2025

In that case I would suggest you stop advertising when disconnecting a client and then start it when the onDisconnect callback is called.

@benjie-git
Copy link
Author

Thanks. The disconnects are being initiated by the clients. I tried disabling advertising in onDisconnect() and enabling it again after a couple of seconds, but the same client will try to auto-reconnect immediately after disconnecting, and succeeds, maybe because advertising was still going, up until milliseconds earlier. I additionally tried rejecting connections in onConnect() within this couple second time window after onDisconnect() is called, but I still ended up with ghost connections.

That said, my workaround included in the above code is working well enough for me for now.

@doudar
Copy link
Contributor

doudar commented Feb 18, 2025

I'm getting the same issue, where pClient.isConnected() == false after a disconnect, but onDisconnect() is never called.

@h2zero
Copy link
Owner

h2zero commented Feb 18, 2025

@doudar Are you referring to the client onConnect or server? Please check your callback signatures, add the override specifier to be sure.

@doudar
Copy link
Contributor

doudar commented Feb 18, 2025

@doudar Are you referring to the client onConnect or server? Please check your callback signatures, add the override specifier to be sure.

My issue is with the client - When a remote server disconnects, the onDisconnect() callback isn't being invoked. onConnect() is called correctly.

 public:
  void onConnect(NimBLEClient* pClient) override;
  void onDisconnect(NimBLEClient* pClient, int reason) override;
  void onPassKeyEntry(NimBLEConnInfo& connInfo) override;
  void onConfirmPasskey(NimBLEConnInfo& connInfo, uint32_t pass_key) override;
  void onAuthenticationComplete(NimBLEConnInfo& connInfo) override;
};

@h2zero
Copy link
Owner

h2zero commented Feb 18, 2025

Weird, this should not be possible. The isConnected function explicitly checks for a valid connection handle and the only time it is set to invalid is in the disconnect event, which calls the callback.

@benjie-git
Copy link
Author

Weird, this should not be possible. The isConnected function explicitly checks for a valid connection handle and the only time it is set to invalid is in the disconnect event, which calls the callback.

I'm unclear on how exactly the callbacks are called. Are they running inside an interrupt? Any chance they could pop up midway through some other code, with the callbacks running while the connection is in an inconsistent state?

@h2zero
Copy link
Owner

h2zero commented Feb 18, 2025

Please enable debug logs and look for this to be printed:

NIMBLE_LOGD(LOG_TAG, "disconnect; reason=%d, %s", rc, NimBLEUtils::returnCodeToString(rc));

@h2zero
Copy link
Owner

h2zero commented Feb 26, 2025

@doudar could you please try removing the if block around this:

if (rc != (BLE_HS_ERR_HCI_BASE + BLE_ERR_CONN_ESTABLISHMENT) || pClient->m_config.asyncConnect) {

@h2zero
Copy link
Owner

h2zero commented Feb 26, 2025

Wondering if this yet again a unreliable disconnect event from the controller, need to find a way to reproduce this.

@benjie-git
Copy link
Author

@h2zero I confirmed that for my use case (ESP32 running as a server, allowing multiple clients) removing this if guard around the onDisconnect() call does not fix the issue. I still intermittently see skipped onDisconnect() calls. It only seems to happen sometimes, when I'm connecting and disconnecting very quickly - under 1 sec. I manually disconnect from the client side, as soon as I see the client show that it has connected. When this error happens, I can check in my own onConnect() code to see if I'm already connected to the incoming address, and if so, disconnect the old connection, and this seems to consistently work around the problem.

So maybe the client is optimistically showing that it has connected before the connection is complete, and I'm initiating a disconnect before the connection is fully established?

@doudar could you please try removing the if block around this:

NimBLE-Arduino/src/NimBLEClient.cpp

Line 957 in 83030f6

if (rc != (BLE_HS_ERR_HCI_BASE + BLE_ERR_CONN_ESTABLISHMENT) || pClient->m_config.asyncConnect) {

@h2zero
Copy link
Owner

h2zero commented Feb 26, 2025

Thank you @benjie-git, that suggest was mainly for @doudar as his application is seeing the issue on the client whereas yours is on the server side. There is no such if block in the server event handler so this was more seeking confirmation that the issue is the same on the client and therefore an upstream bug.

@h2zero
Copy link
Owner

h2zero commented Feb 26, 2025

One thing that both of you may want to try is the NimBLE 1.6 core PR #891 as the issue may have been resolved there.

@benjie-git
Copy link
Author

Hah, of course. Well, I tried using the nimble-core-1.6 branch, but my clients are not able to connect at all. The server sees the clients connection attempt, but the client never sees the connection succeed. I erased my flash, and cleared the client-side bonds as well, but no dice.

@h2zero
Copy link
Owner

h2zero commented Feb 26, 2025

Thanks for trying, that's very strange, any debug logs? It's still a WIP so I haven't done any testing yet.

@benjie-git
Copy link
Author

I get the following debug output:

D NimBLEServer: >> handleGapEvent: 
Client connected 1                        <=== From my code's onConnect()
E NimBLEServer: Update params error: 6, 
D NimBLEAdvertising: >> Advertising start: duration=0, dirAddr=NULL
D NimBLEAdvertising: << Advertising start
D NimBLEServer: << handleGapEvent
D NimBLEServer: >> handleGapEvent: 
D NimBLEServer: << handleGapEvent
D NimBLEServer: >> handleGapEvent: 
D NimBLEServer: << handleGapEvent
D NimBLEServer: >> handleGapEvent: 
D NimBLEServer: << handleGapEvent
D NimBLEServer: >> handleGapEvent: 
I NimBLEServer: mtu update event; conn_handle=1 mtu=255
D NimBLEServerCallbacks: onMTUChange(): Default
D NimBLEServer: << handleGapEvent

My code's onConnect() is called, but not my onConnParamsUpdate() nor my onAuthenticationComplete().

@h2zero
Copy link
Owner

h2zero commented Mar 21, 2025

There has been quite a few reports of this issue and I am wondering if it is chip/arduino core version specific. It seems to revolve around the supervision timeout not generating the disconnect event. I've tested with a regular esp32, with arduino core versions 3.0.1 and 3.1.3 and am not able to repro this error, tested by walking out of range.

@benjie-git
Copy link
Author

I've only been able to get the error to show itself by connecting my computer as a client (using a 2023 mac) to the ESP32 acting as a server, and then from the mac side, disconnecting ESP32 device, wait for it to automatically reconnect, and immediately disconnecting it again, and repeating this a few times. Usually I can get a skipped disconnect event in 3-5 tries.

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

No branches or pull requests

3 participants