sjanc commented on code in PR #2019:
URL: https://github.com/apache/mynewt-nimble/pull/2019#discussion_r2043841382
##########
nimble/host/src/ble_eatt.c:
##########
@@ -75,7 +76,14 @@ static struct ble_gap_event_listener ble_eatt_listener;
static struct ble_npl_event g_read_sup_cl_feat_ev;
static void ble_eatt_setup_cb(struct ble_npl_event *ev);
-static void ble_eatt_start(uint16_t conn_handle);
+
+struct os_callout eatt_conn_timer;
Review Comment:
this needs to be per con, and should be compiled in only when peripheral is
supported
##########
nimble/host/src/ble_eatt.c:
##########
@@ -75,7 +76,14 @@ static struct ble_gap_event_listener ble_eatt_listener;
static struct ble_npl_event g_read_sup_cl_feat_ev;
static void ble_eatt_setup_cb(struct ble_npl_event *ev);
-static void ble_eatt_start(uint16_t conn_handle);
+
+struct os_callout eatt_conn_timer;
+
+struct conn_cb_arg {
+ uint16_t conn_handle;
+};
+
+static struct conn_cb_arg conn_cb_arg;
Review Comment:
same here, per conn
##########
nimble/host/src/ble_eatt.c:
##########
@@ -219,39 +244,77 @@ ble_eatt_l2cap_event_fn(struct ble_l2cap_event *event,
void *arg)
{
struct ble_eatt *eatt = arg;
struct ble_gap_conn_desc desc;
+ uint8_t free_channels;
uint8_t opcode;
int rc;
switch (event->type) {
case BLE_L2CAP_EVENT_COC_CONNECTED:
- BLE_EATT_LOG_DEBUG("eatt: Connected \n");
+ eatt = ble_eatt_find_by_conn_handle(event->connect.conn_handle);
Review Comment:
so eatt seems to be also passed as arg to this function (line 245) ?
##########
nimble/host/syscfg.yml:
##########
@@ -331,6 +331,19 @@ syscfg.defs:
description: >
MTU used for EATT channels.
value: 128
+ BLE_EATT_AUTO_CONNECT:
+ description: >
+ Enable auto connect for EATT
+ value: 1
+ restrictions:
Review Comment:
in newt values are always evaluated before restrictions so this is not doing
what you want
I assume this should be enabled by default when EATT is enabled, which means
that value and restrictions should match eg what we have for
BLE_GATT_NOTIFY_MULTIPLE is:
BLE_GATT_NOTIFY_MULTIPLE:
description: >
Enables sending and receiving of GATT multiple handle
notifications. (0/1)
value: (MYNEWT_VAL_BLE_VERSION >= 52)
restrictions:
- '(BLE_VERSION >= 52) if 1'
Also, I'd suggest to make it depends on BLE_EATT_CHAN_NUM > 0 instead of
enhanced CoC.
##########
nimble/host/syscfg.yml:
##########
@@ -331,6 +331,19 @@ syscfg.defs:
description: >
MTU used for EATT channels.
value: 128
+ BLE_EATT_AUTO_CONNECT:
+ description: >
+ Enable auto connect for EATT
+ value: 1
+ restrictions:
+ - BLE_L2CAP_ENHANCED_COC
+ BLE_EATT_CHAN_PER_CONN:
+ description: >
+ Maximum number of supported EATT channels per connection
+ value: 5
Review Comment:
same here,
I'm not sure if there is support arithmetic in here, if not I'd suggest to
keep it simple for now and default to 0 when eatt is disabled and 1 when
enabled, default can be tuned later, while target can set what value is
needed
##########
nimble/host/src/ble_eatt.c:
##########
@@ -355,7 +441,22 @@ ble_gatt_eatt_write_cl_cb(uint16_t conn_handle,
return 0;
}
- ble_eatt_start(conn_handle);
+#if (MYNEWT_VAL(BLE_EATT_AUTO_CONNECT))
+ struct ble_gap_conn_desc desc;
+ int rc;
+
+ rc = ble_gap_conn_find(conn_handle, &desc);
+ assert(rc == 0);
+
+ if (desc.role == BLE_GAP_ROLE_SLAVE) {
+ /* Add initial delay as peripheral to avoid collision */
+ conn_cb_arg.conn_handle = conn_handle;
+ os_callout_reset(&eatt_conn_timer, 500 * OS_TICKS_PER_SEC / 1000);
Review Comment:
I'd suggest to use same value as spec recommends for GATT collisions
mitigation resolving
"Peripheral shall wait a minimum of 100 ms before retrying; on LE
connections, the Peripheral shall wait
at least 2 × (connPeripheralLatency + 1) × connInterval if that is longer. "
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
