From 662cddc29ed3a05cbca695da2cc5e792ead69785 Mon Sep 17 00:00:00 2001 From: Milanka Ringwald Date: Tue, 9 Oct 2018 17:26:19 +0200 Subject: [PATCH] btstack_hid_parser, hid_device: use one hid_report_type struct, move report_size_valid from test to library --- src/btstack_hid_parser.c | 16 +++++++------- src/btstack_hid_parser.h | 20 ++++++++--------- src/classic/hid_device.c | 34 +++++++++++++++++++++++++---- src/classic/hid_device.h | 14 ++++++------ test/hid_parser/hid_parser_test.c | 4 ++-- test/pts/Makefile | 2 +- test/pts/hid_device_test.c | 36 +------------------------------ test/pts/hid_host_test.c | 2 +- 8 files changed, 59 insertions(+), 69 deletions(-) diff --git a/src/btstack_hid_parser.c b/src/btstack_hid_parser.c index c9a7a76f6..5cb4194a0 100644 --- a/src/btstack_hid_parser.c +++ b/src/btstack_hid_parser.c @@ -258,13 +258,13 @@ static void hid_process_item(btstack_hid_parser_t * parser, hid_descriptor_item_ case Main: switch (item->item_tag){ case Input: - valid_field = parser->report_type == BTSTACK_HID_REPORT_TYPE_INPUT; + valid_field = parser->report_type == HID_REPORT_TYPE_INPUT; break; case Output: - valid_field = parser->report_type == BTSTACK_HID_REPORT_TYPE_OUTPUT; + valid_field = parser->report_type == HID_REPORT_TYPE_OUTPUT; break; case Feature: - valid_field = parser->report_type == BTSTACK_HID_REPORT_TYPE_FEATURE; + valid_field = parser->report_type == HID_REPORT_TYPE_FEATURE; break; default: break; @@ -335,7 +335,7 @@ static void btstack_hid_parser_find_next_usage(btstack_hid_parser_t * parser){ // PUBLIC API -void btstack_hid_parser_init(btstack_hid_parser_t * parser, const uint8_t * hid_descriptor, uint16_t hid_descriptor_len, btstack_hid_report_type_t hid_report_type, const uint8_t * hid_report, uint16_t hid_report_len){ +void btstack_hid_parser_init(btstack_hid_parser_t * parser, const uint8_t * hid_descriptor, uint16_t hid_descriptor_len, hid_report_type_t hid_report_type, const uint8_t * hid_report, uint16_t hid_report_len){ memset(parser, 0, sizeof(btstack_hid_parser_t)); @@ -408,7 +408,7 @@ void btstack_hid_parser_get_field(btstack_hid_parser_t * parser, uint16_t * usag } } -int btstack_hid_get_report_size_for_id(int report_id, btstack_hid_report_type_t report_type, uint16_t hid_descriptor_len, const uint8_t * hid_descriptor){ +int btstack_hid_get_report_size_for_id(int report_id, hid_report_type_t report_type, uint16_t hid_descriptor_len, const uint8_t * hid_descriptor){ int total_report_size = 0; int report_size = 0; int report_count = 0; @@ -442,15 +442,15 @@ int btstack_hid_get_report_size_for_id(int report_id, btstack_hid_report_type_t // printf("tag %d, report_type %d\n", item.item_tag, report_type); switch ((MainItemTag)item.item_tag){ case Input: - if (report_type != BTSTACK_HID_REPORT_TYPE_INPUT) break; + if (report_type != HID_REPORT_TYPE_INPUT) break; valid_report_type = 1; break; case Output: - if (report_type != BTSTACK_HID_REPORT_TYPE_OUTPUT) break; + if (report_type != HID_REPORT_TYPE_OUTPUT) break; valid_report_type = 1; break; case Feature: - if (report_type != BTSTACK_HID_REPORT_TYPE_FEATURE) break; + if (report_type != HID_REPORT_TYPE_FEATURE) break; valid_report_type = 1; break; default: diff --git a/src/btstack_hid_parser.h b/src/btstack_hid_parser.h index d8082df1f..ec31b5966 100644 --- a/src/btstack_hid_parser.h +++ b/src/btstack_hid_parser.h @@ -101,19 +101,19 @@ typedef struct { uint8_t data_size; } hid_descriptor_item_t; -typedef enum { - BTSTACK_HID_REPORT_TYPE_OTHER = 0x0, - BTSTACK_HID_REPORT_TYPE_INPUT, - BTSTACK_HID_REPORT_TYPE_OUTPUT, - BTSTACK_HID_REPORT_TYPE_FEATURE, -} btstack_hid_report_type_t; - typedef enum { BTSTACK_HID_PARSER_SCAN_FOR_REPORT_ITEM, BTSTACK_HID_PARSER_USAGES_AVAILABLE, BTSTACK_HID_PARSER_COMPLETE, } btstack_hid_parser_state_t; +typedef enum { + HID_REPORT_TYPE_RESERVED = 0, + HID_REPORT_TYPE_INPUT, + HID_REPORT_TYPE_OUTPUT, + HID_REPORT_TYPE_FEATURE +} hid_report_type_t; + typedef struct { // Descriptor @@ -121,7 +121,7 @@ typedef struct { uint16_t descriptor_len; // Report - btstack_hid_report_type_t report_type; + hid_report_type_t report_type; const uint8_t * report; uint16_t report_len; @@ -166,7 +166,7 @@ typedef struct { * @param hid_report * @param hid_report_len */ -void btstack_hid_parser_init(btstack_hid_parser_t * parser, const uint8_t * hid_descriptor, uint16_t hid_descriptor_len, btstack_hid_report_type_t hid_report_type, const uint8_t * hid_report, uint16_t hid_report_len); +void btstack_hid_parser_init(btstack_hid_parser_t * parser, const uint8_t * hid_descriptor, uint16_t hid_descriptor_len, hid_report_type_t hid_report_type, const uint8_t * hid_report, uint16_t hid_report_len); /** * @brief Checks if more fields are available @@ -198,7 +198,7 @@ void btstack_hid_parse_descriptor_item(hid_descriptor_item_t * item, const uint8 * @param hid_descriptor_len * @param hid_descriptor */ -int btstack_hid_get_report_size_for_id(int report_id, btstack_hid_report_type_t report_type, uint16_t hid_descriptor_len, const uint8_t * hid_descriptor); +int btstack_hid_get_report_size_for_id(int report_id, hid_report_type_t report_type, uint16_t hid_descriptor_len, const uint8_t * hid_descriptor); /* API_END */ #if defined __cplusplus diff --git a/src/classic/hid_device.c b/src/classic/hid_device.c index 46b9ea045..6a1eba185 100644 --- a/src/classic/hid_device.c +++ b/src/classic/hid_device.c @@ -46,6 +46,7 @@ #include "l2cap.h" #include "btstack_event.h" #include "btstack_debug.h" +#include "btstack_hid_parser.h" typedef enum { HID_DEVICE_IDLE, @@ -79,6 +80,8 @@ typedef struct hid_device { static hid_device_t _hid_device; static uint8_t hid_boot_protocol_mode_supported; static uint8_t hid_report_ids_declared; +static const uint8_t * hid_descriptor; +static uint16_t hid_descriptor_len; static hid_handshake_param_type_t dummy_write_report(uint16_t hid_cid, hid_report_type_t report_type, uint16_t report_id, uint8_t report_max_size, int * out_report_size, uint8_t * out_report){ UNUSED(hid_cid); @@ -156,7 +159,7 @@ void hid_create_sdp_record( uint8_t hid_virtual_cable, uint8_t hid_reconnect_initiate, uint8_t hid_boot_device, - const uint8_t * hid_descriptor, uint16_t hid_descriptor_size, + const uint8_t * descriptor, uint16_t descriptor_size, const char *device_name){ uint8_t * attribute; @@ -263,7 +266,7 @@ void hid_create_sdp_record( uint8_t* hidDescriptor = de_push_sequence(attribute); { de_add_number(hidDescriptor, DE_UINT, DE_SIZE_8, 0x22); // Report Descriptor - de_add_data(hidDescriptor, DE_STRING, hid_descriptor_size, (uint8_t *) hid_descriptor); + de_add_data(hidDescriptor, DE_STRING, descriptor_size, (uint8_t *) descriptor); } de_pop_sequence(attribute, hidDescriptor); } @@ -683,9 +686,10 @@ static void packet_handler(uint8_t packet_type, uint16_t channel, uint8_t * pack /** * @brief Set up HID Device */ -void hid_device_init(uint8_t boot_protocol_mode_supported, uint8_t report_ids_declared){ +void hid_device_init(uint8_t boot_protocol_mode_supported, uint16_t descriptor_len, const uint8_t * descriptor){ hid_boot_protocol_mode_supported = boot_protocol_mode_supported; - hid_report_ids_declared = report_ids_declared; + hid_descriptor = descriptor; + hid_descriptor_len = descriptor_len; l2cap_register_service(packet_handler, PSM_HID_INTERRUPT, 100, LEVEL_2); l2cap_register_service(packet_handler, PSM_HID_CONTROL, 100, LEVEL_2); } @@ -836,3 +840,25 @@ int hid_device_in_boot_protocol_mode(uint16_t hid_cid){ } return hid_device->protocol_mode == HID_PROTOCOL_MODE_BOOT; } + + +int hid_report_size_valid(uint16_t cid, int report_id, hid_report_type_t report_type, int report_size){ + // printf("report size %d, report type %d, report id %d\n", report_size, report_type, report_id); + if (!report_size) return 0; + if (hid_device_in_boot_protocol_mode(cid)){ + switch (report_id){ + case HID_BOOT_MODE_KEYBOARD_ID: + if (report_size < 8) return 0; + break; + case HID_BOOT_MODE_MOUSE_ID: + if (report_size < 1) return 0; + break; + default: + return 0; + } + } else { + int size = btstack_hid_get_report_size_for_id(report_id, report_type, hid_descriptor_len, hid_descriptor); + if (size == 0 || size != report_size) return 0; + } + return 1; +} diff --git a/src/classic/hid_device.h b/src/classic/hid_device.h index 059cf8fa3..6f67a075f 100644 --- a/src/classic/hid_device.h +++ b/src/classic/hid_device.h @@ -41,6 +41,7 @@ #include #include "btstack_defines.h" #include "bluetooth.h" +#include "btstack_hid_parser.h" #if defined __cplusplus extern "C" { @@ -84,13 +85,6 @@ typedef enum { HID_CONTROL_PARAM_VIRTUAL_CABLE_UNPLUG } hid_control_param_t; -typedef enum { - HID_REPORT_TYPE_RESERVED = 0, - HID_REPORT_TYPE_INPUT, - HID_REPORT_TYPE_OUTPUT, - HID_REPORT_TYPE_FEATURE -} hid_report_type_t; - typedef enum { HID_PROTOCOL_MODE_BOOT = 0, HID_PROTOCOL_MODE_REPORT @@ -133,8 +127,10 @@ void hid_create_sdp_record( /** * @brief Set up HID Device * @param boot_protocol_mode_supported + * @param hid_descriptor_len + * @param hid_descriptor */ -void hid_device_init(uint8_t boot_protocol_mode_supported, uint8_t report_ids_declared); +void hid_device_init(uint8_t boot_protocol_mode_supported, uint16_t hid_descriptor_len, const uint8_t * hid_descriptor); /** * @brief Register callback for the HID Device client. @@ -199,6 +195,8 @@ void hid_device_send_control_message(uint16_t hid_cid, const uint8_t * message, */ int hid_device_in_boot_protocol_mode(uint16_t hid_cid); + +int hid_report_size_valid(uint16_t cid, int report_id, hid_report_type_t report_type, int report_size); /* API_END */ /* Only needed for PTS Testing */ diff --git a/test/hid_parser/hid_parser_test.c b/test/hid_parser/hid_parser_test.c index ea0fe76a6..0b6ebfdb2 100644 --- a/test/hid_parser/hid_parser_test.c +++ b/test/hid_parser/hid_parser_test.c @@ -332,8 +332,8 @@ TEST(HID, GetReportSize){ int report_size = 0; const uint8_t * hid_descriptor = combo_descriptor_with_report_ids; uint16_t hid_descriptor_len = sizeof(combo_descriptor_with_report_ids); - // report_size = btstack_hid_get_report_size_for_id(1, BTSTACK_HID_REPORT_TYPE_INPUT, hid_descriptor_len, hid_descriptor); - // CHECK_EQUAL(3, report_size); + report_size = btstack_hid_get_report_size_for_id(1, BTSTACK_HID_REPORT_TYPE_INPUT, hid_descriptor_len, hid_descriptor); + CHECK_EQUAL(3, report_size); hid_descriptor = hid_descriptor_keyboard_boot_mode; hid_descriptor_len = sizeof(hid_descriptor_keyboard_boot_mode); diff --git a/test/pts/Makefile b/test/pts/Makefile index a61a905a0..c808f49ab 100644 --- a/test/pts/Makefile +++ b/test/pts/Makefile @@ -135,7 +135,7 @@ all: ${EXAMPLES} hid_host_test: ${CORE_OBJ} ${COMMON_OBJ} ${CLASSIC_OBJ} ${SDP_CLIENT} btstack_hid_parser.o hid_host_test.o ${CC} $^ ${CFLAGS} ${LDFLAGS} -o $@ -hid_device_test: ${CORE_OBJ} ${COMMON_OBJ} ${CLASSIC_OBJ} ${SDP_CLIENT} btstack_ring_buffer.o hid_device.o hid_device_test.o +hid_device_test: ${CORE_OBJ} ${COMMON_OBJ} ${CLASSIC_OBJ} ${SDP_CLIENT} btstack_hid_parser.o btstack_ring_buffer.o hid_device.o hid_device_test.o ${CC} $^ ${CFLAGS} ${LDFLAGS} -o $@ hog_demo_test: hog_demo_test.h ${CORE_OBJ} ${COMMON_OBJ} ${ATT_OBJ} ${GATT_SERVER_OBJ} ${SM_OBJ} battery_service_server.o device_information_service_server.o hids_device.o btstack_ring_buffer.o hog_demo_test.c diff --git a/test/pts/hid_device_test.c b/test/pts/hid_device_test.c index dd8e64667..094358a13 100644 --- a/test/pts/hid_device_test.c +++ b/test/pts/hid_device_test.c @@ -281,35 +281,6 @@ static int prepare_keyboard_report(hid_report_type_t report_type, int modifier, return pos; } -static int hid_report_size_valid(uint16_t cid, int report_id, hid_report_type_t report_type, int report_size){ - printf("report size %d, report type %d, report id %d\n", report_size, report_type, report_id); - if (!report_size) return 0; - if (hid_device_in_boot_protocol_mode(cid)){ - switch (report_id){ - case HID_BOOT_MODE_KEYBOARD_ID: - if (report_size < 8) return 0; - break; - case HID_BOOT_MODE_MOUSE_ID: - if (report_size < 1) return 0; - break; - default: - return 0; - } - } else { - switch (report_type){ - case HID_REPORT_TYPE_INPUT: - if (report_size < 8) return 0; - break; - case HID_REPORT_TYPE_OUTPUT: - if (report_size < 1) return 0; - break; - default: - return 0; - } - } - return 1; -} - static hid_report_id_status_t hid_report_id_status(uint16_t cid, uint16_t report_id){ if (hid_device_in_boot_protocol_mode(cid)){ switch (report_id){ @@ -671,12 +642,7 @@ int btstack_main(int argc, const char * argv[]){ 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); -#ifdef REPORT_ID_DECLARED - // HID Device - hid_device_init(hid_boot_device, 1); -#else - hid_device_init(hid_boot_device, 0); -#endif + hid_device_init(hid_boot_device, sizeof(hid_descriptor_keyboard_boot_mode), hid_descriptor_keyboard_boot_mode); // register for HCI events hci_event_callback_registration.callback = &packet_handler; hci_add_event_handler(&hci_event_callback_registration); diff --git a/test/pts/hid_host_test.c b/test/pts/hid_host_test.c index 0f1864858..0819a1025 100644 --- a/test/pts/hid_host_test.c +++ b/test/pts/hid_host_test.c @@ -290,7 +290,7 @@ static void hid_host_handle_interrupt_report(const uint8_t * report, uint16_t re report++; report_len--; btstack_hid_parser_t parser; - btstack_hid_parser_init(&parser, hid_descriptor, hid_descriptor_len, BTSTACK_HID_REPORT_TYPE_INPUT, report, report_len); + btstack_hid_parser_init(&parser, hid_descriptor, hid_descriptor_len, HID_REPORT_TYPE_INPUT, report, report_len); int shift = 0; uint8_t new_keys[NUM_KEYS]; memset(new_keys, 0, sizeof(new_keys));