Add BLE remote control example (AEGHB-286) #283

CheahHaoYi · 2023-07-10T06:48:32Z

Add an example that demonstrate how to use various ESP APIs to create a remote control using an ESP.

CLAassistant · 2023-07-10T06:48:37Z

All committers have signed the CLA.

leeebo · 2023-07-10T07:48:57Z

@CheahHaoYi Thanks for your contribution, we'll help review and then merge it!

bubblesnake · 2023-07-11T02:12:39Z

examples/bluetooth/ble_remote_control/main/controller.c

+    set_button_bit(PIN_BUTTON_D, gpio_get_level(PIN_BUTTON_D));
+#endif
+
+    *button_in = button_input;


There is no operation on button_input in this function, yet it is being assigned to *button_in. This will look strange. You may have the current value of button_input returned by set_button_bit. In fact, the button_input can be encapsulated by get/set/clear functions. There is no need for the caller to manage it.

bubblesnake · 2023-07-11T02:14:42Z

examples/bluetooth/ble_remote_control/main/controller.c

+ * @param[out]  cali_handle Pointer to calibration handle
+*/
+esp_err_t adc_calibration_init(adc_channel_t channel, adc_cali_handle_t*  cali_handle)
+{


Put local functions together.

bubblesnake · 2023-07-11T02:15:38Z

examples/bluetooth/ble_remote_control/main/controller.c

+#if ADC_CALI_SCHEME_CURVE_FITTING_SUPPORTED
+    ESP_LOGI(TAG, "deregister %s calibration scheme", "Curve Fitting");
+    ESP_ERROR_CHECK(adc_cali_delete_scheme_curve_fitting(handle));
+


Remove line to have consistent spacing in #if block.

bubblesnake · 2023-07-11T02:27:45Z

examples/bluetooth/ble_remote_control/main/hidd.c

+void hid_close_event(void); 
+
+void hid_event_callback(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if,
+									esp_ble_gatts_cb_param_t *param)


Indentation.

bubblesnake · 2023-07-11T02:28:53Z

examples/bluetooth/ble_remote_control/main/hidd.c

+        break;
+    case ESP_GATTS_CREAT_ATTR_TAB_EVT:
+        if (param->add_attr_tab.status == ESP_GATT_OK) {
+            ESP_LOGI(HID_DEMO_TAG, "Create attribute table successfully, table size = %d",param->add_attr_tab.num_handle);


Space after comma.

bubblesnake · 2023-07-11T02:29:13Z

examples/bluetooth/ble_remote_control/main/hidd.c

+        break;
+    case ESP_GATTS_START_EVT:
+        ESP_LOGI(HID_DEMO_TAG, "Start Service %s, service_handle %d",
+            (param->start.status == ESP_GATT_OK ? "Sucessful" : "Failed") , param->start.service_handle);


Extra space before comma.

bubblesnake · 2023-07-11T02:35:16Z

examples/bluetooth/ble_remote_control/README.md

+# ESP-IDF BLE Remote Control Example
+
+This example shows how to create a BLE Joystick with an ESP32 implemented in BLUEDROID. 
+


"ESP32 series SoC"

esp-cjh · 2023-07-11T09:58:15Z

examples/bluetooth/ble_remote_control/main/hidd.c

+            sizeof(val_batt_level),
+            (uint8_t *) &val_batt_level,
+        },
+    },


It's better to add the Client Characteristic Configuration descriptor for Battery Level Characteristic. So that we can send notification to client when the battery level changes.

esp-cjh · 2023-07-11T11:52:43Z

examples/bluetooth/ble_remote_control/main/gap_gatts.c

+    .p_service_data = NULL,
+    .service_uuid_len = sizeof(hidd_service_uuid128),
+    .p_service_uuid = hidd_service_uuid128,
+    .flag = 0x6,


It's better to replace with .flag = (ESP_BLE_ADV_FLAG_GEN_DISC | ESP_BLE_ADV_FLAG_BREDR_NOT_SPT), to improve code readability.

esp-cjh · 2023-07-11T12:11:08Z

examples/bluetooth/ble_remote_control/main/hidd.c

+        ESP_LOGI(HID_DEMO_TAG, "Profile Reciving a new connection, conn_id %d", param->connect.conn_id);
+        // Save the connection ID, need this for sending notification in send_user_input()
+        client.connection_id = param->connect.conn_id; 
+        is_connected = true;


Don't set is_connected here. Because the connection is required to be encrypted for HID profile. We should set it to true in ESP_GAP_BLE_AUTH_CMPL_EVT.

esp-cjh · 2023-07-11T12:20:52Z

examples/bluetooth/ble_remote_control/main/gap_gatts.c

+        gap_security_request_event(param);
+        break;
+    case ESP_GAP_BLE_AUTH_CMPL_EVT:
+        ESP_LOGI(HID_DEMO_TAG, "ESP_GAP_BLE_AUTH_CMPL_EVT");


We should check whether the authentication is successful by param->ble_security.auth_cmpl.success. If authentication is successful then we set is_connected to true.

bubblesnake · 2023-07-31T01:55:48Z

examples/bluetooth/ble_remote_control/main/hidd.c

+{   
+    // Same service instance ID as the one passed into esp_ble_gatts_create_attr_tab
+    ESP_LOGI(HID_DEMO_TAG, "Service Instance ID: %d, UUID: 0x%x", param->add_attr_tab.svc_inst_id, param->add_attr_tab.svc_uuid.uuid.uuid16);
+    // ESP_LOGI(HID_DEMO_TAG, "UUID of Service: 0x%x", param->add_attr_tab.svc_uuid.uuid.uuid16);


bubblesnake · 2023-07-31T01:56:21Z

examples/bluetooth/ble_remote_control/main/hidd.c

+    }
+
+    ESP_LOGI(HID_DEMO_TAG, "Create attribute table successfully, the number handle = %d\n",param->add_attr_tab.num_handle);
+    // memcpy(handler_table, param->add_attr_tab.handles, param->add_attr_tab.num_handle * sizeof(uint16_t));


bubblesnake · 2023-07-31T02:02:22Z

examples/bluetooth/ble_remote_control/main/main.c

+
+        // Alternatively, to simply poll user input can do:
+        // joystick_adc_read(&x_axis, &y_axis);
+        // button_read(&button_in);


Are these comments still useful?

The comments here are to provide a simplified implementation of the remote control (where user just poll for the inputs), if let's say the event queue implementation does not fit the user's use case.

bubblesnake

Clean up.

bubblesnake

Looks good to me.

leeebo · 2023-08-03T02:16:04Z

Hi @CheahHaoYi Thanks again for the contributions, we're preparing to merge it. Before that, we suggest the following:

Add a license declaration in each C headers and source files, please follow the license format
Squash the 5 commits to one for a better look

Refactor project, Fix based on PR comments Fix indentation Add encapsulation of button_input Remove unused comments Add license

leeebo · 2023-08-08T05:05:52Z

Hi @CheahHaoYi sdkconfig.defaults also needs to be provided to ensure the example can be compiled successfully by default.

For IDF>=5.0, please run idf.py save-defconfig under your project to save the default values to file.

leeebo · 2023-08-08T08:15:11Z

Like this example bluedroid/ble/gatt_server, to support different targets, multiple sdkconfig.defaults.{target} needs to provide

CheahHaoYi · 2023-08-11T07:53:44Z

Added the sdkconfig.defaults file, specialized support for different targets would not be necessary for this example I believe

CheahHaoYi marked this pull request as draft July 10, 2023 06:52

github-actions bot changed the title ~~Add BLE remote control example~~ Add BLE remote control example (AEGHB-286) Jul 10, 2023

CheahHaoYi marked this pull request as ready for review July 10, 2023 06:57

wujiangang requested review from bubblesnake and esp-cjh July 10, 2023 08:49

bubblesnake suggested changes Jul 11, 2023

View reviewed changes

esp-cjh reviewed Jul 11, 2023

View reviewed changes

leeebo added the bluetooth label Jul 12, 2023

bubblesnake reviewed Jul 31, 2023

View reviewed changes

bubblesnake approved these changes Aug 1, 2023

View reviewed changes

Add BLE remote control example

2f50af7

Refactor project, Fix based on PR comments Fix indentation Add encapsulation of button_input Remove unused comments Add license

CheahHaoYi force-pushed the master branch from c1cd57d to 2f50af7 Compare August 7, 2023 07:58

Add sdkconfig.defaults file

4cb0a41

Remove ESP32-S2 (does not support BT/BLE)

1fb8b5f

zhanzhaocheng closed this in 05e5139 Aug 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BLE remote control example (AEGHB-286) #283

Add BLE remote control example (AEGHB-286) #283

CheahHaoYi commented Jul 10, 2023

CLAassistant commented Jul 10, 2023 •

edited

Loading

leeebo commented Jul 10, 2023

bubblesnake Jul 11, 2023

bubblesnake Jul 11, 2023

bubblesnake Jul 11, 2023

bubblesnake Jul 11, 2023

bubblesnake Jul 11, 2023

bubblesnake Jul 11, 2023

bubblesnake Jul 11, 2023

esp-cjh Jul 11, 2023

esp-cjh Jul 11, 2023

esp-cjh Jul 11, 2023

esp-cjh Jul 11, 2023

bubblesnake Jul 31, 2023

bubblesnake Jul 31, 2023

bubblesnake Jul 31, 2023

CheahHaoYi Aug 1, 2023

bubblesnake left a comment

bubblesnake left a comment

leeebo commented Aug 3, 2023

leeebo commented Aug 8, 2023

leeebo commented Aug 8, 2023

CheahHaoYi commented Aug 11, 2023

		# ESP-IDF BLE Remote Control Example

		This example shows how to create a BLE Joystick with an ESP32 implemented in BLUEDROID.

Add BLE remote control example (AEGHB-286) #283

Add BLE remote control example (AEGHB-286) #283

Conversation

CheahHaoYi commented Jul 10, 2023

CLAassistant commented Jul 10, 2023 • edited Loading

leeebo commented Jul 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bubblesnake left a comment

Choose a reason for hiding this comment

bubblesnake left a comment

Choose a reason for hiding this comment

leeebo commented Aug 3, 2023

leeebo commented Aug 8, 2023

leeebo commented Aug 8, 2023

CheahHaoYi commented Aug 11, 2023

CLAassistant commented Jul 10, 2023 •

edited

Loading