Skip to content

EATT improvements #2019

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

szymon-czapracki
Copy link
Contributor

@szymon-czapracki szymon-czapracki commented Apr 4, 2025

Add some now functionalities to EATT.
Multiple channel connections.
Collision mitigation.
New auto-connect procedure.

@github-actions github-actions bot added host size/M Medium PR labels Apr 4, 2025
@szymon-czapracki szymon-czapracki force-pushed the eatt_work branch 3 times, most recently from 8997eaa to f87e7f9 Compare April 12, 2025 02:21
@szymon-czapracki szymon-czapracki marked this pull request as ready for review April 12, 2025 02:21
@szymon-czapracki szymon-czapracki changed the title [WIP] EATT improvements EATT improvements Apr 14, 2025
description: >
Enable auto connect for EATT
value: 1
restrictions:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

BLE_EATT_CHAN_PER_CONN:
description: >
Maximum number of supported EATT channels per connection
value: 5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be per con, and should be compiled in only when peripheral is supported

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

uint16_t conn_handle;
};

static struct conn_cb_arg conn_cb_arg;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, per conn

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so eatt seems to be also passed as arg to this function (line 245) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and no - eatt is passed when this funciton is called in setup_cb. But when server is running, and events are happening, eatt structure might be incomplete, as some of the fields are assigned as a result of event-handling within this function. Thus I thought to look if EATT exsists for specific connection - do you see a better solution here?

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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. "

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@szymon-czapracki szymon-czapracki force-pushed the eatt_work branch 6 times, most recently from 99ba184 to 2a609ce Compare April 17, 2025 14:30
Introduce BLE_EATT_AUTO_CONNECT syscfg option
to allow manual EATT connection setup

Introduce a configurable limit for maximum number
of L2CAP channels per connection
@szymon-czapracki szymon-czapracki force-pushed the eatt_work branch 2 times, most recently from 67f74bb to 01097ec Compare April 24, 2025 10:35
@szymon-czapracki szymon-czapracki requested a review from sjanc May 5, 2025 08:57
@szymon-czapracki szymon-czapracki force-pushed the eatt_work branch 4 times, most recently from ea9b56a to b7b61e4 Compare May 7, 2025 15:31
Provide flexibility in managing EATT.
Introduce function that manually establishes EATT.

Add function that iterates over specific connection
EATT and returns the maximum number of channels
left for use.
Previously we established only one channel for EATT.
Now application can define this number.
Function is removed, as ble_eatt_connect() is
designed to perform this role for
both manual & auto-connect scenarios.
Implement new way for auto-connect.
Add initial delay to avoid collision.
Add support for multiple-channels in event function.
Refactor logging within event handler function.
Introduce accepted channels field in eatt structure.
Introduce operations for collision mitigation handling.
Add specification-defined delay for retrying connection.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants