Bluetooth: testlib: `bt_testlib_connect`: better error logs
Changes to logging:
- Don't log "Connected" if there was an error.
- Identify the relevant bt_conn object in log messages by its index.
- Special case non-fatal errors:
- Failure due to out of free conn object is INF.
- Improve transparancy of error messages:
- For errno, include the name of the API.
- For HCI errors, include the common prefix of the symbols
'BT_HCI_ERR_' so they are easier to look up.
This change includes some light refactoring to make the code more
understandable, but does not change any behavior.
Signed-off-by: Aleksander Wasaznik <aleksander.wasaznik@nordicsemi.no>
diff --git a/tests/bluetooth/common/testlib/src/connect.c b/tests/bluetooth/common/testlib/src/connect.c
index e8f5a90..9e181a5 100644
--- a/tests/bluetooth/common/testlib/src/connect.c
+++ b/tests/bluetooth/common/testlib/src/connect.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2023 Nordic Semiconductor ASA
+/* Copyright (c) 2023-2024 Nordic Semiconductor ASA
* SPDX-License-Identifier: Apache-2.0
*/
@@ -12,10 +12,10 @@
LOG_MODULE_REGISTER(bt_testlib_connect, LOG_LEVEL_INF);
struct bt_testlib_connect_closure {
- uint8_t conn_err;
- struct bt_conn **conn;
+ uint8_t conn_cb_connected_err;
+ struct bt_conn **connp;
struct k_mutex lock;
- struct k_condvar done;
+ struct k_condvar conn_cb_connected_match;
};
/* Context pool (with capacity of one). */
@@ -23,63 +23,89 @@
static K_MUTEX_DEFINE(g_ctx_lock);
static struct bt_testlib_connect_closure *g_ctx;
-static void connected_cb(struct bt_conn *conn, uint8_t conn_err)
+static void on_conn_cb_connected(struct bt_conn *conn, uint8_t conn_err)
{
/* Loop over each (allocated) item in pool. */
k_mutex_lock(&g_ctx_lock, K_FOREVER);
- if (g_ctx && conn == *g_ctx->conn) {
- g_ctx->conn_err = conn_err;
- k_condvar_signal(&g_ctx->done);
+ if (g_ctx && conn == *g_ctx->connp) {
+ g_ctx->conn_cb_connected_err = conn_err;
+ k_condvar_signal(&g_ctx->conn_cb_connected_match);
}
k_mutex_unlock(&g_ctx_lock);
}
-BT_CONN_CB_DEFINE(conn_callbacks) = {
- .connected = connected_cb,
+BT_CONN_CB_DEFINE(conn_cb) = {
+ .connected = on_conn_cb_connected,
};
-int bt_testlib_connect(const bt_addr_le_t *peer, struct bt_conn **conn)
+int bt_testlib_connect(const bt_addr_le_t *peer, struct bt_conn **connp)
{
- int api_err;
+ int err;
struct bt_testlib_connect_closure ctx = {
- .conn = conn,
+ .connp = connp,
};
+ uint8_t conn_index;
- __ASSERT_NO_MSG(conn);
- __ASSERT_NO_MSG(*conn == NULL);
+ __ASSERT_NO_MSG(connp);
+ __ASSERT_NO_MSG(*connp == NULL);
- k_condvar_init(&ctx.done);
+ k_condvar_init(&ctx.conn_cb_connected_match);
+ /* If multiple threads call into this funciton, they will wait
+ * for their turn here. The Zephyr host does not support
+ * concurrent connection creation.
+ */
k_sem_take(&g_ctx_free, K_FOREVER);
k_mutex_lock(&g_ctx_lock, K_FOREVER);
g_ctx = &ctx;
- api_err = bt_conn_le_create(peer, BT_CONN_LE_CREATE_CONN, BT_LE_CONN_PARAM_DEFAULT, conn);
+ err = bt_conn_le_create(peer, BT_CONN_LE_CREATE_CONN, BT_LE_CONN_PARAM_DEFAULT, connp);
- if (!api_err) {
- LOG_INF("Connecting.. conn %u", bt_conn_index(*conn));
- k_condvar_wait(&ctx.done, &g_ctx_lock, K_FOREVER);
- LOG_INF("Connect complete");
+ if (!err) {
+ conn_index = bt_conn_index(*connp);
+ LOG_INF("bt_conn_le_create ok conn %u", conn_index);
+
+ k_condvar_wait(&ctx.conn_cb_connected_match, &g_ctx_lock, K_FOREVER);
}
g_ctx = NULL;
k_mutex_unlock(&g_ctx_lock);
k_sem_give(&g_ctx_free);
- if (api_err) {
- LOG_ERR("bt_conn_le_create err %d", api_err);
- __ASSERT_NO_MSG(api_err < 0);
- return api_err;
+ /* Merge the error codes. The errors from `bt_conn_le_create`
+ * are negative, leaving the positive space for the HCI errors
+ * from `conn_cb_connected`.
+ */
+ __ASSERT_NO_MSG(err <= 0);
+ __ASSERT_NO_MSG(0 <= ctx.conn_cb_connected_err);
+ __ASSERT_NO_MSG(!err || !ctx.conn_cb_connected_err);
+ err = err + ctx.conn_cb_connected_err;
+
+ /* This is just logging. */
+ switch (err) {
+ case -ENOMEM:
+ LOG_INF("bt_conn_le_create -ENOMEM: No free connection objects available.");
+ break;
+ case 0:
+ LOG_INF("conn %u: connected", conn_index);
+ break;
+ case BT_HCI_ERR_UNKNOWN_CONN_ID:
+ LOG_INF("conn %u: timed out", conn_index);
+ break;
+ default:
+ if (err < 0) {
+ LOG_ERR("bt_conn_le_create err %d", err);
+ } else {
+ LOG_ERR("conn %u: BT_HCI_ERR_ 0x%02x", conn_index, err);
+ }
}
- if (ctx.conn_err) {
- LOG_ERR("Connect HCI err %d", ctx.conn_err);
- __ASSERT_NO_MSG(ctx.conn_err >= 0);
- return ctx.conn_err;
- }
+ /* Note: `connp` is never unrefed in this funciton, even in case
+ * of errors. This is as documented.
+ */
- return 0;
+ return err;
}
