search

bluekitchen / btstack

Dual-mode Bluetooth stack, with small memory footprint.
http://bluekitchen-gmbh.com
Other
1.74k stars 618 forks source link

btstack_run_loop_set_timer stopped from running after disconnect and connect device. #570

Closed emanuellopes closed 8 months ago

emanuellopes commented 9 months ago

Describe the bug I used the hid_keyboard_demo and reimplemented the code in raspberry pico W. After some debug testing I notice the loop handler doesn't fire after bluetooth reconnection.

To Reproduce

Steps to reproduce the behavior:

#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <inttypes.h>
#include "pico/cyw43_arch.h"

#include "btstack.h"
#include "header/btstack_config.h"

#include "header/hid_descriptor.h"
#include "header/btstack_hid.h"

#include "../header/pico_queue_lib.h"
#include "../config/keyboard_config.h"

// When not set to 0xffff, sniff and sniff subrating are enabled
static uint16_t hid_ssr_host_max_latency = 1600;
static uint16_t hid_ssr_host_min_timeout = 3200;
static uint16_t hid_supervision_timeout = 3200;

// STATE

static uint8_t hid_service_buffer[300];
static uint8_t device_id_sdp_service_buffer[100];
static const char hid_device_name[] = BLE_NAME;
static btstack_packet_callback_registration_t hci_event_callback_registration;
static uint16_t hid_cid;
static uint8_t hid_boot_device = 0;

// HID Report sending
static uint8_t send_modifier;
static uint8_t send_keycode;

// HID
static btstack_timer_source_t text_timer;

static enum {
    APP_BOOTING,
    APP_NOT_CONNECTED,
    APP_CONNECTING,
    APP_CONNECTED
} app_state = APP_BOOTING;

//=======Code Modified   Begin=================
static void text_timer_handler(btstack_timer_source_t *ts)
{
    struct
    {
        uint8_t modifier;   /**< Keyboard modifier (KEYBOARD_MODIFIER_* masks). */
        uint8_t reserved;   /**< Reserved for OEM use, always set to 0. */
        uint8_t keycode[6]; /**< Key codes of the currently pressed keys. */
    } report_q;             // same structure as hid_keyboard_report_t from tusb.h

    uint8_t pressed_key = report_q.keycode[0];

    if (queue_try_remove(&hid_keyboard_report_queue, &report_q))
    {
        uint8_t report[] = {
            0xa1, // keyboard descriptor
            REPORT_ID_KEYBOARD,
            report_q.modifier,
            0,
            report_q.keycode[0],
            report_q.keycode[1],
            report_q.keycode[2],
            report_q.keycode[3],
            report_q.keycode[4],
            report_q.keycode[5]};

        if (pressed_key != 0)
        {
            printf("------------------Btstack ------------\n");
            printf("process Report modifier: %d, reserved: %d pressed_key %d\n", report_q.modifier, report_q.reserved, pressed_key);
            printf("------------------ Btstack ------------\\n\nn");
        }

        hid_device_send_interrupt_message(hid_cid, &report[0], sizeof(report));
    }
    hid_device_request_can_send_now_event(hid_cid);
}

static void packet_handler(uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t packet_size)
{
    UNUSED(channel);
    UNUSED(packet_size);
    uint8_t status;
    switch (packet_type)
    {
    case HCI_EVENT_PACKET:
        switch (hci_event_packet_get_type(packet))
        {
        case BTSTACK_EVENT_STATE:
            if (btstack_event_state_get_state(packet) != HCI_STATE_WORKING)
                return;
            app_state = APP_NOT_CONNECTED;
            break;

        case HCI_EVENT_USER_CONFIRMATION_REQUEST:
            // ssp: inform about user confirmation request
            printf("SSP User Confirmation Request with numeric value '%06" PRIu32 "'\n", hci_event_user_confirmation_request_get_numeric_value(packet));
            printf("SSP User Confirmation Auto accept\n");
            break;

        case HCI_EVENT_HID_META:
            switch (hci_event_hid_meta_get_subevent_code(packet))
            {
            case HID_SUBEVENT_CONNECTION_OPENED:
                status = hid_subevent_connection_opened_get_status(packet);
                if (status != ERROR_CODE_SUCCESS)
                {
                    // outgoing connection failed
                    printf("Connection failed, status 0x%x\n", status);
                    app_state = APP_NOT_CONNECTED;
                    hid_cid = 0;
                    return;
                }
                app_state = APP_CONNECTED;
                hid_cid = hid_subevent_connection_opened_get_hid_cid(packet);
                btstack_run_loop_set_timer_handler(&text_timer, text_timer_handler);
                btstack_run_loop_set_timer(&text_timer, TEXT_PERIOD_MS);
                btstack_run_loop_add_timer(&text_timer);
                cyw43_arch_gpio_put(CYW43_WL_GPIO_LED_PIN, 0);
                break;
            case HID_SUBEVENT_CONNECTION_CLOSED:
                printf("HID Disconnected\n");
               btstack_run_loop_remove_timer(&text_timer);
                app_state = APP_NOT_CONNECTED;
                hid_cid = 0;
                cyw43_arch_gpio_put(CYW43_WL_GPIO_LED_PIN, 1);
                break;
            case HID_SUBEVENT_CAN_SEND_NOW:
                btstack_run_loop_set_timer_handler(&text_timer, text_timer_handler);
                btstack_run_loop_set_timer(&text_timer, TEXT_PERIOD_MS);
                btstack_run_loop_add_timer(&text_timer);

                break;
            default:
                break;
            }
            break;
        default:
            break;
        }
        break;
    default:
        break;
    }
}

void btstack_main()
{
    cyw43_arch_gpio_put(CYW43_WL_GPIO_LED_PIN, 1);
    sleep_ms(200);
    cyw43_arch_gpio_put(CYW43_WL_GPIO_LED_PIN, 0);
    sleep_ms(200);
    cyw43_arch_gpio_put(CYW43_WL_GPIO_LED_PIN, 1);

    // allow to get found by inquiry
    gap_discoverable_control(1);
    // use Limited Discoverable Mode; Peripheral; Keyboard as CoD
    gap_set_class_of_device(HID_DEVICE_SUBCLASS);
    // Name of device
    gap_set_local_name(hid_device_name);
    // allow for role switch in general and sniff mode
    gap_set_default_link_policy_settings(LM_LINK_POLICY_ENABLE_ROLE_SWITCH | LM_LINK_POLICY_ENABLE_SNIFF_MODE);
    // allow for role switch on outgoing connections - this allow HID Host to become master when we re-connect to it
    gap_set_allow_role_switch(true);

    // L2CAP
    l2cap_init();

    // SDP Server
    sdp_init();
    memset(hid_service_buffer, 0, sizeof(hid_service_buffer));

    uint8_t hid_virtual_cable = 0;
    uint8_t hid_remote_wake = 1;
    uint8_t hid_reconnect_initiate = 1;
    uint8_t hid_normally_connectable = 1;

    hid_sdp_record_t hid_params = {
        // hid sevice subclass 2540 Keyboard, hid counntry code 33 US
        HID_DEVICE_SUBCLASS, // hid_device_subclass
        KEYBOARD_COUNTRY,    // hid_country_code
        hid_virtual_cable,
        hid_remote_wake,
        hid_reconnect_initiate,
        hid_normally_connectable,
        hid_boot_device,
        hid_ssr_host_max_latency,
        hid_ssr_host_min_timeout,
        hid_supervision_timeout,
        hid_descriptor_keyboard,
        sizeof(hid_descriptor_keyboard),
        hid_device_name};

    hid_create_sdp_record(hid_service_buffer, 0x10001, &hid_params);

    printf("HID service record size: %u\n", de_get_len(hid_service_buffer));
    sdp_register_service(hid_service_buffer);

    // See https://www.bluetooth.com/specifications/assigned-numbers/company-identifiers if you don't have a USB Vendor ID and need a Bluetooth Vendor ID
    // device info: BlueKitchen GmbH, product 1, version 1
    device_id_create_sdp_record(device_id_sdp_service_buffer, 0x10003, DEVICE_ID_VENDOR_ID_SOURCE_BLUETOOTH, BLUETOOTH_COMPANY_ID_BLUEKITCHEN_GMBH, 1, 1);
    printf("Device ID SDP service record size: %u\n", de_get_len((uint8_t *)device_id_sdp_service_buffer));
    sdp_register_service(device_id_sdp_service_buffer);

    // HID Device
    hid_device_init(hid_boot_device, sizeof(hid_descriptor_keyboard), hid_descriptor_keyboard);

    // register for HCI events
    hci_event_callback_registration.callback = &packet_handler;
    hci_add_event_handler(&hci_event_callback_registration);

    // register for HID events
    hid_device_register_packet_handler(&packet_handler);

    // turn on!
    hci_power_control(HCI_POWER_ON);
}

Expected behavior The loop timer handler should run in a loop until device is connected.

#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <inttypes.h>
#include "pico/cyw43_arch.h"

#include "btstack.h"
#include "header/btstack_config.h"

#include "header/hid_descriptor.h"
#include "header/btstack_hid.h"

#include "../queue/keyboard_queue.h"
#include "../config/keyboard_config.h"
#include "../utils_hid/utils_hid.h"

// When not set to 0xffff, sniff and sniff subrating are enabled
static uint16_t hid_ssr_host_max_latency = 1600;
static uint16_t hid_ssr_host_min_timeout = 3200;
static uint16_t hid_supervision_timeout = 3200;

// STATE

static uint8_t hid_service_buffer[300];
static uint8_t device_id_sdp_service_buffer[100];
static const char hid_device_name[] = BLE_NAME;
static btstack_packet_callback_registration_t hci_event_callback_registration;
static uint16_t hid_cid;
static uint8_t hid_boot_device = 0;

// HID Report sending
static uint8_t send_modifier;
static uint8_t send_keycode;

// HID
static btstack_timer_source_t text_timer;

static enum {
    APP_BOOTING,
    APP_NOT_CONNECTED,
    APP_CONNECTING,
    APP_CONNECTED
} app_state = APP_BOOTING;

static uint8_t const keycode2ascii[128][2] = {HID_KEYCODE_TO_ASCII_BTSTACK};
static void send_report(uint8_t *report, int len);
static void restart_timer_loop();

//=======Code Modified   Begin=================
static void text_timer_handler(btstack_timer_source_t *ts)
{
    bool have_hid = false;
    uint8_t report[10];
    printf("...........................\n");
    if (queue_try_remove(&hid_keyboard_report_queue, &report))
    {
        have_hid = true;
        printf("received report\n");
        uint8_t report_id = report[0];
        if (report_id != 3)
        {
            report[0] = 5;
            uint8_t report_hidp[10];

            convert_hid_report_to_hidp_keyboard(report, report_hidp);
            send_report(report_hidp, 10);
        }
        else
        {
            uint8_t report_hidp[4];
            convert_hid_report_to_hidp_consumer(report, report_hidp);
            send_report(report_hidp, 4);
        }
    }

    printf("hid fire send event now\n");
    printf("hid fire send event now------end\n");
    if (have_hid)
    {
        printf("have hid");
        hid_device_request_can_send_now_event(hid_cid);
    }
    else
    {
        printf("add loop");
        restart_timer_loop();
    }
}

static void send_report(uint8_t *report, int len)
{
    printf("------------------Btstack recompile report ------------\n");
    printf("Report ID: %d\n", report[1]);
    printf("Modifier or second position: %d\n", report[2]);
    printf("OEM or 3rd position: %d\n", report[3]);

    bool const is_shift = report[report[2]] & (2 | 32);
    if (len == 10)
    {
        for (int j = 4; j < len; j++)
        {
            uint8_t pressed_key = report[j];
            uint8_t ch = keycode2ascii[pressed_key][is_shift ? 1 : 0];
            printf(" keycode: %d %d %c\n", j, pressed_key, ch);
        }
    }
    printf("\n------------------ Btstack recompile report ------------\n\n");

    hid_device_send_interrupt_message(hid_cid, report, sizeof(report));
}

static void restart_timer_loop()
{
    printf("adding looopp");
    btstack_run_loop_set_timer(&text_timer, TEXT_PERIOD_MS);
    btstack_run_loop_add_timer(&text_timer);
}

static void packet_handler(uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t packet_size)
{
    UNUSED(channel);
    UNUSED(packet_size);
    uint8_t status;
    switch (packet_type)
    {
    case HCI_EVENT_PACKET:
        switch (hci_event_packet_get_type(packet))
        {
        case BTSTACK_EVENT_STATE:
            if (btstack_event_state_get_state(packet) != HCI_STATE_WORKING)
                return;
            app_state = APP_NOT_CONNECTED;
            printf("HID Disconnected BTSTACK_EVENT_STATE\n");
            clean_keyboard_queue(&hid_keyboard_report_queue); // clean queue to avoid outputing a lot of keys
            break;

        case HCI_EVENT_USER_CONFIRMATION_REQUEST:
            // ssp: inform about user confirmation request
            printf("SSP User Confirmation Request with numeric value '%06" PRIu32 "'\n", hci_event_user_confirmation_request_get_numeric_value(packet));
            printf("SSP User Confirmation Auto accept\n");
            break;

        case HCI_EVENT_HID_META:
            switch (hci_event_hid_meta_get_subevent_code(packet))
            {
            case HID_SUBEVENT_CONNECTION_OPENED:
                printf("HID_SUBEVENT_CONNECTION_OPENED\n");
                clean_keyboard_queue(&hid_keyboard_report_queue); // clean queue to avoid outputing a lot of keys
                status = hid_subevent_connection_opened_get_status(packet);
                if (status != ERROR_CODE_SUCCESS)
                {
                    // outgoing connection failed
                    printf("Connection failed, status 0x%x\n", status);
                    app_state = APP_NOT_CONNECTED;
                    hid_cid = 0;
                    return;
                }
                printf("HID Connected\n");
                app_state = APP_CONNECTED;
                hid_cid = hid_subevent_connection_opened_get_hid_cid(packet);
                text_timer.process = &text_timer_handler;
                restart_timer_loop();
                cyw43_arch_gpio_put(CYW43_WL_GPIO_LED_PIN, 0);
                break;
            case HID_SUBEVENT_CONNECTION_CLOSED:
                btstack_run_loop_remove_timer(&text_timer);
                cyw43_arch_gpio_put(CYW43_WL_GPIO_LED_PIN, 1);
                clean_keyboard_queue(&hid_keyboard_report_queue); // clean queue to avoid outputing a lot of keys
                printf("HID Disconnected\n");
                app_state = APP_NOT_CONNECTED;
                hid_cid = 0;
                break;
            case HID_SUBEVENT_CAN_SEND_NOW:
                printf("HID_SUBEVENT_CAN_SEND_NOW\n");
                restart_timer_loop();
                break;
            default:
                break;
            }
            break;
        default:
            break;
        }
        break;
    default:
        break;
    }
}

void btstack_main()
{
    cyw43_arch_gpio_put(CYW43_WL_GPIO_LED_PIN, 1);

    // allow to get found by inquiry
    gap_discoverable_control(1);
    // use Limited Discoverable Mode; Peripheral; Keyboard as CoD
    gap_set_class_of_device(HID_DEVICE_SUBCLASS);
    // Name of device
    gap_set_local_name(hid_device_name);
    // allow for role switch in general and sniff mode
    gap_set_default_link_policy_settings(LM_LINK_POLICY_ENABLE_ROLE_SWITCH | LM_LINK_POLICY_ENABLE_SNIFF_MODE);
    // allow for role switch on outgoing connections - this allow HID Host to become master when we re-connect to it
    gap_set_allow_role_switch(true);

    // L2CAP
    l2cap_init();

    // SDP Server
    sdp_init();
    memset(hid_service_buffer, 0, sizeof(hid_service_buffer));

    uint8_t hid_virtual_cable = 0;
    uint8_t hid_remote_wake = 1;
    uint8_t hid_reconnect_initiate = 1;
    uint8_t hid_normally_connectable = 1;

    hid_sdp_record_t hid_params = {
        // hid sevice subclass 2540 Keyboard, hid counntry code 33 US
        HID_DEVICE_SUBCLASS, // hid_device_subclass
        KEYBOARD_COUNTRY,    // hid_country_code
        hid_virtual_cable,
        hid_remote_wake,
        hid_reconnect_initiate,
        hid_normally_connectable,
        hid_boot_device,
        hid_ssr_host_max_latency,
        hid_ssr_host_min_timeout,
        hid_supervision_timeout,
        hid_descriptor_keyboard,
        sizeof(hid_descriptor_keyboard),
        hid_device_name};

    hid_create_sdp_record(hid_service_buffer, 0x10001, &hid_params);

    printf("HID service record size: %u\n", de_get_len(hid_service_buffer));
    sdp_register_service(hid_service_buffer);

    // See https://www.bluetooth.com/specifications/assigned-numbers/company-identifiers if you don't have a USB Vendor ID and need a Bluetooth Vendor ID
    // device info: BlueKitchen GmbH, product 1, version 1
    device_id_create_sdp_record(device_id_sdp_service_buffer, 0x10003, DEVICE_ID_VENDOR_ID_SOURCE_BLUETOOTH, BLUETOOTH_COMPANY_ID_BLUEKITCHEN_GMBH, 1, 1);
    printf("Device ID SDP service record size: %u\n", de_get_len((uint8_t *)device_id_sdp_service_buffer));
    sdp_register_service(device_id_sdp_service_buffer);

    // HID Device
    hid_device_init(hid_boot_device, sizeof(hid_descriptor_keyboard), hid_descriptor_keyboard);

    // register for HCI events
    hci_event_callback_registration.callback = &packet_handler;
    hci_add_event_handler(&hci_event_callback_registration);

    // register for HID events
    hid_device_register_packet_handler(&packet_handler);

    // turn on!
    hci_power_control(HCI_POWER_ON);
}

On my side I've fixed the issue using the example led_bliking.c Instead of using function btstack_run_loop_set_timer_handler(&text_timer, text_timer_handler);

I have changed to text_timer.process = &text_timer_handler; on device connection.

I'm not sure if this is the correct implementation but solved my issue.

Do you have any recommendation to improve this?

Thank you

mringwal commented 8 months ago

Hi Emmanuel

BTstack expects that hid_device_request_can_send_now_event is called if the user wants to send and that the user calls hid_device_send_interrupt_message upon

In your code, you're starting a timer upon HID_SUBEVENT_CAN_SEND_NOW.

Pease retest this issue with the examples (and minimal changes) The timer should to type should be restarted on connect. For this, remove the timer and add it again.

text_timer.process = &text_timer_handler;

is identical to

btstack_run_loop_set_timer_handler(&text_timer, text_timer_handler);