From d660eb4752dc3001f6246d8cb39584fb31f596ed Mon Sep 17 00:00:00 2001 From: Astra Date: Thu, 7 Nov 2024 20:57:37 +0900 Subject: [PATCH 1/8] WIP: fix lost BadBLE keystrokes --- lib/ble_profile/extra_services/hid_service.c | 37 +++++++++++++++----- targets/f7/ble_glue/furi_ble/gatt.c | 13 ++++++- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/lib/ble_profile/extra_services/hid_service.c b/lib/ble_profile/extra_services/hid_service.c index e46d2010c52..62720db9cb5 100644 --- a/lib/ble_profile/extra_services/hid_service.c +++ b/lib/ble_profile/extra_services/hid_service.c @@ -10,13 +10,13 @@ #define TAG "BleHid" #define BLE_SVC_HID_REPORT_MAP_MAX_LEN (255) -#define BLE_SVC_HID_REPORT_MAX_LEN (255) -#define BLE_SVC_HID_REPORT_REF_LEN (2) -#define BLE_SVC_HID_INFO_LEN (4) -#define BLE_SVC_HID_CONTROL_POINT_LEN (1) +#define BLE_SVC_HID_REPORT_MAX_LEN (255) +#define BLE_SVC_HID_REPORT_REF_LEN (2) +#define BLE_SVC_HID_INFO_LEN (4) +#define BLE_SVC_HID_CONTROL_POINT_LEN (1) -#define BLE_SVC_HID_INPUT_REPORT_COUNT (3) -#define BLE_SVC_HID_OUTPUT_REPORT_COUNT (0) +#define BLE_SVC_HID_INPUT_REPORT_COUNT (3) +#define BLE_SVC_HID_OUTPUT_REPORT_COUNT (0) #define BLE_SVC_HID_FEATURE_REPORT_COUNT (0) #define BLE_SVC_HID_REPORT_COUNT \ (BLE_SVC_HID_INPUT_REPORT_COUNT + BLE_SVC_HID_OUTPUT_REPORT_COUNT + \ @@ -148,15 +148,18 @@ struct BleServiceHid { BleGattCharacteristicInstance output_report_chars[BLE_SVC_HID_OUTPUT_REPORT_COUNT]; BleGattCharacteristicInstance feature_report_chars[BLE_SVC_HID_FEATURE_REPORT_COUNT]; GapSvcEventHandler* event_handler; + + bool busy; }; static BleEventAckStatus ble_svc_hid_event_handler(void* event, void* context) { - UNUSED(context); + BleServiceHid* hid_svc = context; BleEventAckStatus ret = BleEventNotAck; hci_event_pckt* event_pckt = (hci_event_pckt*)(((hci_uart_pckt*)event)->data); evt_blecore_aci* blecore_evt = (evt_blecore_aci*)event_pckt->data; // aci_gatt_attribute_modified_event_rp0* attribute_modified; + if(event_pckt->evt == HCI_VENDOR_SPECIFIC_DEBUG_EVT_CODE) { if(blecore_evt->ecode == ACI_GATT_ATTRIBUTE_MODIFIED_VSEVT_CODE) { // Process modification events @@ -164,6 +167,9 @@ static BleEventAckStatus ble_svc_hid_event_handler(void* event, void* context) { } else if(blecore_evt->ecode == ACI_GATT_SERVER_CONFIRMATION_VSEVT_CODE) { // Process notification confirmation ret = BleEventAckFlowEnable; + } else if(blecore_evt->ecode == ACI_GATT_TX_POOL_AVAILABLE_VSEVT_CODE) { + // Ready for TX + hid_svc->busy = false; } } return ret; @@ -171,6 +177,7 @@ static BleEventAckStatus ble_svc_hid_event_handler(void* event, void* context) { BleServiceHid* ble_svc_hid_start(void) { BleServiceHid* hid_svc = malloc(sizeof(BleServiceHid)); + hid_svc->busy = false; // Register event handler hid_svc->event_handler = @@ -269,13 +276,27 @@ bool ble_svc_hid_update_input_report( furi_assert(data); furi_assert(hid_svc); furi_assert(input_report_num < BLE_SVC_HID_INPUT_REPORT_COUNT); + bool ret = false; HidSvcDataWrapper report_data = { .data_ptr = data, .data_len = len, }; - return ble_gatt_characteristic_update( + + ret = ble_gatt_characteristic_update( hid_svc->svc_handle, &hid_svc->input_report_chars[input_report_num], &report_data); + if(ret) hid_svc->busy = true; + + while(hid_svc->busy) { + furi_delay_tick(1); + + if(!hid_svc->busy) { + ret = ble_gatt_characteristic_update( + hid_svc->svc_handle, &hid_svc->input_report_chars[input_report_num], &report_data); + } + } + + return ret; } bool ble_svc_hid_update_info(BleServiceHid* hid_svc, uint8_t* data) { diff --git a/targets/f7/ble_glue/furi_ble/gatt.c b/targets/f7/ble_glue/furi_ble/gatt.c index e407865833c..5482eb5db90 100644 --- a/targets/f7/ble_glue/furi_ble/gatt.c +++ b/targets/f7/ble_glue/furi_ble/gatt.c @@ -42,6 +42,7 @@ void ble_gatt_characteristic_init( &char_instance->handle); if(status) { FURI_LOG_E(TAG, "Failed to add %s char: %d", char_descriptor->name, status); + furi_assert(false, "Failed to add characteristic"); } char_instance->descriptor_handle = 0; @@ -68,6 +69,7 @@ void ble_gatt_characteristic_init( &char_instance->descriptor_handle); if(status) { FURI_LOG_E(TAG, "Failed to add %s char descriptor: %d", char_descriptor->name, status); + furi_assert(false, "Failed to add characteristic descriptor"); } if(release_data) { free((void*)char_data); @@ -82,6 +84,7 @@ void ble_gatt_characteristic_delete( if(status) { FURI_LOG_E( TAG, "Failed to delete %s char: %d", char_instance->characteristic->name, status); + furi_assert(false, "Failed to delete characteristic"); } free((void*)char_instance->characteristic); } @@ -114,7 +117,13 @@ bool ble_gatt_characteristic_update( tBleStatus result = aci_gatt_update_char_value( svc_handle, char_instance->handle, 0, char_data_size, char_data); if(result) { - FURI_LOG_E(TAG, "Failed updating %s characteristic: %d", char_descriptor->name, result); + if(result == BLE_STATUS_INSUFFICIENT_RESOURCES) { + FURI_LOG_E(TAG, "Insufficient resources for %s characteristic", char_descriptor->name); + } else { + FURI_LOG_E( + TAG, "Failed updating %s characteristic: %d", char_descriptor->name, result); + furi_assert(false, "Failed to update characteristic"); + } } if(release_data) { free((void*)char_data); @@ -132,6 +141,7 @@ bool ble_gatt_service_add( Service_UUID_Type, Service_UUID, Service_Type, Max_Attribute_Records, Service_Handle); if(result) { FURI_LOG_E(TAG, "Failed to add service: %x", result); + furi_assert(false, "Failed to add service"); } return result == BLE_STATUS_SUCCESS; @@ -141,6 +151,7 @@ bool ble_gatt_service_delete(uint16_t svc_handle) { tBleStatus result = aci_gatt_del_service(svc_handle); if(result) { FURI_LOG_E(TAG, "Failed to delete service: %x", result); + furi_assert(false, "Failed to delete service"); } return result == BLE_STATUS_SUCCESS; From ffac21b5b1bbbca935e1192137490d0fc1416d30 Mon Sep 17 00:00:00 2001 From: Astra Date: Fri, 8 Nov 2024 19:31:43 +0900 Subject: [PATCH 2/8] Switch to semaphores for synchronization --- lib/ble_profile/extra_services/hid_service.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/lib/ble_profile/extra_services/hid_service.c b/lib/ble_profile/extra_services/hid_service.c index 62720db9cb5..d058ff76393 100644 --- a/lib/ble_profile/extra_services/hid_service.c +++ b/lib/ble_profile/extra_services/hid_service.c @@ -149,7 +149,7 @@ struct BleServiceHid { BleGattCharacteristicInstance feature_report_chars[BLE_SVC_HID_FEATURE_REPORT_COUNT]; GapSvcEventHandler* event_handler; - bool busy; + FuriSemaphore* busy; }; static BleEventAckStatus ble_svc_hid_event_handler(void* event, void* context) { @@ -169,7 +169,7 @@ static BleEventAckStatus ble_svc_hid_event_handler(void* event, void* context) { ret = BleEventAckFlowEnable; } else if(blecore_evt->ecode == ACI_GATT_TX_POOL_AVAILABLE_VSEVT_CODE) { // Ready for TX - hid_svc->busy = false; + furi_semaphore_release(hid_svc->busy); } } return ret; @@ -177,7 +177,7 @@ static BleEventAckStatus ble_svc_hid_event_handler(void* event, void* context) { BleServiceHid* ble_svc_hid_start(void) { BleServiceHid* hid_svc = malloc(sizeof(BleServiceHid)); - hid_svc->busy = false; + hid_svc->busy = furi_semaphore_alloc(1, 0); // Register event handler hid_svc->event_handler = @@ -285,15 +285,10 @@ bool ble_svc_hid_update_input_report( ret = ble_gatt_characteristic_update( hid_svc->svc_handle, &hid_svc->input_report_chars[input_report_num], &report_data); - if(ret) hid_svc->busy = true; - - while(hid_svc->busy) { - furi_delay_tick(1); - - if(!hid_svc->busy) { - ret = ble_gatt_characteristic_update( - hid_svc->svc_handle, &hid_svc->input_report_chars[input_report_num], &report_data); - } + if(ret) { + furi_semaphore_acquire(hid_svc->busy, FuriWaitForever); + ret = ble_gatt_characteristic_update( + hid_svc->svc_handle, &hid_svc->input_report_chars[input_report_num], &report_data); } return ret; From 512be5fb38c29993d69746123e2fd548525185f1 Mon Sep 17 00:00:00 2001 From: Astra Date: Mon, 11 Nov 2024 19:57:00 +0900 Subject: [PATCH 3/8] Move checking to the gap level --- targets/f7/api_symbols.csv | 3 ++- targets/f7/ble_glue/furi_ble/gatt.c | 15 +++++++++++---- targets/f7/ble_glue/gap.c | 12 ++++++++++++ targets/f7/ble_glue/gap.h | 3 +++ 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/targets/f7/api_symbols.csv b/targets/f7/api_symbols.csv index ee81f76a97a..a558c9e37b7 100644 --- a/targets/f7/api_symbols.csv +++ b/targets/f7/api_symbols.csv @@ -1,5 +1,5 @@ entry,status,name,type,params -Version,+,78.1,, +Version,+,78.2,, Header,+,applications/drivers/subghz/cc1101_ext/cc1101_ext_interconnect.h,, Header,+,applications/services/bt/bt_service/bt.h,, Header,+,applications/services/bt/bt_service/bt_keys_storage.h,, @@ -1930,6 +1930,7 @@ Function,-,gap_init,_Bool,"GapConfig*, GapEventCallback, void*" Function,-,gap_start_advertising,void, Function,-,gap_stop_advertising,void, Function,-,gap_thread_stop,void, +Function,-,gap_wait_for_tx_pool_acailable,void,FuriWait Function,-,getc,int,FILE* Function,-,getc_unlocked,int,FILE* Function,-,getchar,int, diff --git a/targets/f7/ble_glue/furi_ble/gatt.c b/targets/f7/ble_glue/furi_ble/gatt.c index 5482eb5db90..18b635b885d 100644 --- a/targets/f7/ble_glue/furi_ble/gatt.c +++ b/targets/f7/ble_glue/furi_ble/gatt.c @@ -1,4 +1,5 @@ #include "gatt.h" +#include "gap.h" #include #include @@ -119,15 +120,21 @@ bool ble_gatt_characteristic_update( if(result) { if(result == BLE_STATUS_INSUFFICIENT_RESOURCES) { FURI_LOG_E(TAG, "Insufficient resources for %s characteristic", char_descriptor->name); - } else { - FURI_LOG_E( - TAG, "Failed updating %s characteristic: %d", char_descriptor->name, result); - furi_assert(false, "Failed to update characteristic"); + gap_wait_for_tx_pool_acailable(FuriWaitForever); + result = aci_gatt_update_char_value( + svc_handle, char_instance->handle, 0, char_data_size, char_data); } } + if(release_data) { free((void*)char_data); } + + if(result != BLE_STATUS_SUCCESS) { + FURI_LOG_E(TAG, "Failed updating %s characteristic: %d", char_descriptor->name, result); + furi_assert(false, "Failed to update characteristic"); + } + return result != BLE_STATUS_SUCCESS; } diff --git a/targets/f7/ble_glue/gap.c b/targets/f7/ble_glue/gap.c index 732440ccf90..e88ea47597f 100644 --- a/targets/f7/ble_glue/gap.c +++ b/targets/f7/ble_glue/gap.c @@ -40,6 +40,7 @@ typedef struct { bool enable_adv; bool is_secure; uint8_t negotiation_round; + FuriSemaphore* tx_pool_busy; } Gap; typedef enum { @@ -299,6 +300,11 @@ BleEventFlowStatus ble_event_app_notification(void* pckt) { } break; } + + case ACI_GATT_TX_POOL_AVAILABLE_VSEVT_CODE: + FURI_LOG_D(TAG, "TX pool available event"); + furi_semaphore_release(gap->tx_pool_busy); + break; } default: break; @@ -309,6 +315,11 @@ BleEventFlowStatus ble_event_app_notification(void* pckt) { return BleEventFlowEnable; } +void gap_wait_for_tx_pool_acailable(FuriWait wait) { + furi_check(gap); + furi_check(furi_semaphore_acquire(gap->tx_pool_busy, wait) == FuriStatusOk); +} + static void set_advertisment_service_uid(uint8_t* uid, uint8_t uid_len) { if(uid_len == 2) { gap->service.adv_svc_uuid[0] = AD_TYPE_16_BIT_SERV_UUID; @@ -527,6 +538,7 @@ bool gap_init(GapConfig* config, GapEventCallback on_event_cb, void* context) { gap = malloc(sizeof(Gap)); gap->config = config; + gap->tx_pool_busy = furi_semaphore_alloc(1, 0); // Create advertising timer gap->advertise_timer = furi_timer_alloc(gap_advetise_timer_callback, FuriTimerTypeOnce, NULL); // Initialization of GATT & GAP layer diff --git a/targets/f7/ble_glue/gap.h b/targets/f7/ble_glue/gap.h index a90d0730471..f28f0433380 100644 --- a/targets/f7/ble_glue/gap.h +++ b/targets/f7/ble_glue/gap.h @@ -2,6 +2,7 @@ #include #include +#include #include @@ -89,6 +90,8 @@ void gap_thread_stop(void); void gap_emit_ble_beacon_status_event(bool active); +void gap_wait_for_tx_pool_acailable(FuriWait wait); + #ifdef __cplusplus } #endif From 8caef1867d541f02ef4997a827e3eeddcf359353 Mon Sep 17 00:00:00 2001 From: Astra Date: Mon, 11 Nov 2024 20:01:46 +0900 Subject: [PATCH 4/8] Remove leftovers from hid_service --- lib/ble_profile/extra_services/hid_service.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/lib/ble_profile/extra_services/hid_service.c b/lib/ble_profile/extra_services/hid_service.c index d058ff76393..2fee4175dc6 100644 --- a/lib/ble_profile/extra_services/hid_service.c +++ b/lib/ble_profile/extra_services/hid_service.c @@ -148,12 +148,10 @@ struct BleServiceHid { BleGattCharacteristicInstance output_report_chars[BLE_SVC_HID_OUTPUT_REPORT_COUNT]; BleGattCharacteristicInstance feature_report_chars[BLE_SVC_HID_FEATURE_REPORT_COUNT]; GapSvcEventHandler* event_handler; - - FuriSemaphore* busy; }; static BleEventAckStatus ble_svc_hid_event_handler(void* event, void* context) { - BleServiceHid* hid_svc = context; + UNUSED(context); BleEventAckStatus ret = BleEventNotAck; hci_event_pckt* event_pckt = (hci_event_pckt*)(((hci_uart_pckt*)event)->data); @@ -167,9 +165,6 @@ static BleEventAckStatus ble_svc_hid_event_handler(void* event, void* context) { } else if(blecore_evt->ecode == ACI_GATT_SERVER_CONFIRMATION_VSEVT_CODE) { // Process notification confirmation ret = BleEventAckFlowEnable; - } else if(blecore_evt->ecode == ACI_GATT_TX_POOL_AVAILABLE_VSEVT_CODE) { - // Ready for TX - furi_semaphore_release(hid_svc->busy); } } return ret; @@ -177,7 +172,6 @@ static BleEventAckStatus ble_svc_hid_event_handler(void* event, void* context) { BleServiceHid* ble_svc_hid_start(void) { BleServiceHid* hid_svc = malloc(sizeof(BleServiceHid)); - hid_svc->busy = furi_semaphore_alloc(1, 0); // Register event handler hid_svc->event_handler = @@ -285,11 +279,6 @@ bool ble_svc_hid_update_input_report( ret = ble_gatt_characteristic_update( hid_svc->svc_handle, &hid_svc->input_report_chars[input_report_num], &report_data); - if(ret) { - furi_semaphore_acquire(hid_svc->busy, FuriWaitForever); - ret = ble_gatt_characteristic_update( - hid_svc->svc_handle, &hid_svc->input_report_chars[input_report_num], &report_data); - } return ret; } From c368d2b11509a93fa6425b9a1db8d06b65b07fa8 Mon Sep 17 00:00:00 2001 From: Astra Date: Mon, 11 Nov 2024 20:04:35 +0900 Subject: [PATCH 5/8] Remove more leftovers from hid_service --- lib/ble_profile/extra_services/hid_service.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/ble_profile/extra_services/hid_service.c b/lib/ble_profile/extra_services/hid_service.c index 2fee4175dc6..9f9a0f7520e 100644 --- a/lib/ble_profile/extra_services/hid_service.c +++ b/lib/ble_profile/extra_services/hid_service.c @@ -270,17 +270,14 @@ bool ble_svc_hid_update_input_report( furi_assert(data); furi_assert(hid_svc); furi_assert(input_report_num < BLE_SVC_HID_INPUT_REPORT_COUNT); - bool ret = false; HidSvcDataWrapper report_data = { .data_ptr = data, .data_len = len, }; - ret = ble_gatt_characteristic_update( + return ble_gatt_characteristic_update( hid_svc->svc_handle, &hid_svc->input_report_chars[input_report_num], &report_data); - - return ret; } bool ble_svc_hid_update_info(BleServiceHid* hid_svc, uint8_t* data) { From 1b9b3e944f160615a1a153648f6e6fde143b2ac8 Mon Sep 17 00:00:00 2001 From: Astra Date: Mon, 11 Nov 2024 20:07:29 +0900 Subject: [PATCH 6/8] De-allocate the semaphore after use --- targets/f7/ble_glue/gap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/targets/f7/ble_glue/gap.c b/targets/f7/ble_glue/gap.c index e88ea47597f..8101baed0ed 100644 --- a/targets/f7/ble_glue/gap.c +++ b/targets/f7/ble_glue/gap.c @@ -604,6 +604,8 @@ void gap_thread_stop(void) { gap->command_queue = NULL; furi_timer_free(gap->advertise_timer); gap->advertise_timer = NULL; + furi_semaphore_free(gap->tx_pool_busy); + gap->tx_pool_busy = NULL; ble_event_dispatcher_reset(); free(gap); From 73a9fb10362102360b5cb45055bcee361b16ffb7 Mon Sep 17 00:00:00 2001 From: Astra Date: Mon, 11 Nov 2024 20:13:56 +0900 Subject: [PATCH 7/8] Change the timeout to account for unforeseen situation --- targets/f7/ble_glue/furi_ble/gatt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/targets/f7/ble_glue/furi_ble/gatt.c b/targets/f7/ble_glue/furi_ble/gatt.c index 18b635b885d..36a398138ea 100644 --- a/targets/f7/ble_glue/furi_ble/gatt.c +++ b/targets/f7/ble_glue/furi_ble/gatt.c @@ -120,7 +120,7 @@ bool ble_gatt_characteristic_update( if(result) { if(result == BLE_STATUS_INSUFFICIENT_RESOURCES) { FURI_LOG_E(TAG, "Insufficient resources for %s characteristic", char_descriptor->name); - gap_wait_for_tx_pool_acailable(FuriWaitForever); + gap_wait_for_tx_pool_acailable(1000); // 1 second timeout result = aci_gatt_update_char_value( svc_handle, char_instance->handle, 0, char_data_size, char_data); } From 2be7d3af509bd95cbd8d6f7b58802f9ae7f9264a Mon Sep 17 00:00:00 2001 From: Astra Date: Mon, 11 Nov 2024 20:17:27 +0900 Subject: [PATCH 8/8] Update F18 API --- targets/f18/api_symbols.csv | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/targets/f18/api_symbols.csv b/targets/f18/api_symbols.csv index b5d51a0dd5c..6101d9f729c 100644 --- a/targets/f18/api_symbols.csv +++ b/targets/f18/api_symbols.csv @@ -1,5 +1,5 @@ entry,status,name,type,params -Version,+,78.1,, +Version,+,78.2,, Header,+,applications/services/bt/bt_service/bt.h,, Header,+,applications/services/bt/bt_service/bt_keys_storage.h,, Header,+,applications/services/cli/cli.h,, @@ -1711,6 +1711,7 @@ Function,-,gap_init,_Bool,"GapConfig*, GapEventCallback, void*" Function,-,gap_start_advertising,void, Function,-,gap_stop_advertising,void, Function,-,gap_thread_stop,void, +Function,-,gap_wait_for_tx_pool_acailable,void,FuriWait Function,-,getc,int,FILE* Function,-,getc_unlocked,int,FILE* Function,-,getchar,int,