From f933913a5717737e4239df3203ad962436a7cbe6 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sun, 11 Oct 2026 17:45:19 -0400 Subject: [PATCH 1/2] zephyr-cp: report GATT ATT errors as specific exceptions GATT client reads and writes passed ATT error codes from the peer to `raise_zephyr_error()`, which expects a negative errno, so they came out as `OSError` with a negative number. They now go through a new `check_gattc_status()`, which maps the same ATT errors as espressif and nordic: - 0x02, 0x03: `BluetoothError("read not permitted")` / `"write not permitted"` - 0x05: `SecurityError("Insufficient authentication")` - 0x08: `SecurityError("Insufficient authorization")` - 0x0C, 0x0F: `SecurityError("Insufficient encryption")` - 0x0D: `ValueError("Invalid length")` - other ATT errors: "Unknown gatt error: 0x00xx" Zephyr reports a disconnect during the request, and other local failures, as `BT_ATT_ERR_UNLIKELY`. So the connection state is checked first, and a dropped connection raises `ConnectionError`. Each request holds a reference on the `bt_conn` so the check is safe after a disconnect. Co-Authored-By: Claude Opus 5.5 --- locale/circuitpython.pot | 12 ++++ ports/zephyr-cp/common-hal/_bleio/__init__.c | 65 ++++++++++++++++++-- 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/locale/circuitpython.pot b/locale/circuitpython.pot index 4969538a2ac..bd9d91f6210 100644 --- a/locale/circuitpython.pot +++ b/locale/circuitpython.pot @@ -1366,6 +1366,7 @@ msgstr "" #: ports/zephyr-cp/common-hal/_bleio/Characteristic.c #: ports/zephyr-cp/common-hal/_bleio/Connection.c #: ports/zephyr-cp/common-hal/_bleio/Descriptor.c +#: ports/zephyr-cp/common-hal/_bleio/__init__.c #: shared-bindings/_bleio/CharacteristicBuffer.c msgid "Not connected" msgstr "" @@ -1424,11 +1425,13 @@ msgstr "" #: ports/espressif/common-hal/_bleio/__init__.c #: ports/nordic/common-hal/_bleio/__init__.c +#: ports/zephyr-cp/common-hal/_bleio/__init__.c msgid "Insufficient authentication" msgstr "" #: ports/espressif/common-hal/_bleio/__init__.c #: ports/nordic/common-hal/_bleio/__init__.c +#: ports/zephyr-cp/common-hal/_bleio/__init__.c msgid "Insufficient encryption" msgstr "" @@ -1892,6 +1895,7 @@ msgid "Unknown system firmware error: %04x" msgstr "" #: ports/nordic/common-hal/_bleio/__init__.c +#: ports/zephyr-cp/common-hal/_bleio/__init__.c #, c-format msgid "Unknown gatt error: 0x%04x" msgstr "" @@ -2412,6 +2416,14 @@ msgstr "" msgid "Invalid advertising data" msgstr "" +#: ports/zephyr-cp/common-hal/_bleio/__init__.c +msgid "%q not permitted" +msgstr "" + +#: ports/zephyr-cp/common-hal/_bleio/__init__.c +msgid "Insufficient authorization" +msgstr "" + #: ports/zephyr-cp/common-hal/audiobusio/I2SOut.c #: ports/zephyr-cp/common-hal/busio/I2C.c #: ports/zephyr-cp/common-hal/busio/SPI.c diff --git a/ports/zephyr-cp/common-hal/_bleio/__init__.c b/ports/zephyr-cp/common-hal/_bleio/__init__.c index b51db00835a..f3ae847e31a 100644 --- a/ports/zephyr-cp/common-hal/_bleio/__init__.c +++ b/ports/zephyr-cp/common-hal/_bleio/__init__.c @@ -16,6 +16,8 @@ #include "supervisor/shared/bluetooth/bluetooth.h" #include "supervisor/shared/tick.h" +#include + // The singleton _bleio.Adapter object bleio_adapter_obj_t common_hal_bleio_adapter_obj; @@ -142,6 +144,47 @@ static void on_gattc_write(struct bt_conn *conn, uint8_t err, ctx->done = true; } +// Raise an exception for a nonzero ATT error code from a GATT client read or write. +// connected is whether the connection was still up when the request finished. +static void check_gattc_status(bool connected, uint8_t err) { + if (err == 0) { + return; + } + // Zephyr reports local failures, including a disconnect while the request was + // outstanding, as BT_ATT_ERR_UNLIKELY. Check the connection before blaming the peer. + if (!connected) { + mp_raise_ConnectionError(MP_ERROR_TEXT("Not connected")); + } + switch (err) { + case BT_ATT_ERR_READ_NOT_PERMITTED: + mp_raise_bleio_BluetoothError(MP_ERROR_TEXT("%q not permitted"), MP_QSTR_read); + return; + case BT_ATT_ERR_WRITE_NOT_PERMITTED: + mp_raise_bleio_BluetoothError(MP_ERROR_TEXT("%q not permitted"), MP_QSTR_write); + return; + case BT_ATT_ERR_AUTHENTICATION: + mp_raise_bleio_SecurityError(MP_ERROR_TEXT("Insufficient authentication")); + return; + case BT_ATT_ERR_AUTHORIZATION: + mp_raise_bleio_SecurityError(MP_ERROR_TEXT("Insufficient authorization")); + return; + case BT_ATT_ERR_INSUFFICIENT_ENCRYPTION: + case BT_ATT_ERR_ENCRYPTION_KEY_SIZE: + mp_raise_bleio_SecurityError(MP_ERROR_TEXT("Insufficient encryption")); + return; + case BT_ATT_ERR_INVALID_ATTRIBUTE_LEN: + mp_arg_error_invalid(MP_QSTR_length); + return; + default: + mp_raise_bleio_BluetoothError(MP_ERROR_TEXT("Unknown gatt error: 0x%04x"), err); + } +} + +static bool conn_is_connected(struct bt_conn *conn) { + struct bt_conn_info info; + return bt_conn_get_info(conn, &info) == 0 && info.state == BT_CONN_STATE_CONNECTED; +} + size_t bleio_gattc_read_sync(struct bt_conn *conn, uint16_t handle, uint8_t *buf, size_t len) { gattc_read_ctx_t ctx = { @@ -159,9 +202,14 @@ size_t bleio_gattc_read_sync(struct bt_conn *conn, uint16_t handle, read_params.single.handle = handle; read_params.single.offset = 0; + // `bt_conn_ref()` holds a reference to the connection, so conn stays valid until + // its state is checked below, even if a disconnect drops the connection's own + // reference meanwhile. + bt_conn_ref(conn); int err = bt_gatt_read(conn, &read_params); if (err != 0) { active_read_ctx = NULL; + bt_conn_unref(conn); raise_zephyr_error(err); } @@ -170,9 +218,9 @@ size_t bleio_gattc_read_sync(struct bt_conn *conn, uint16_t handle, } active_read_ctx = NULL; - if (ctx.err != 0) { - raise_zephyr_error(ctx.err); - } + bool connected = conn_is_connected(conn); + bt_conn_unref(conn); + check_gattc_status(connected, ctx.err); return ctx.read_len; } @@ -192,9 +240,14 @@ void bleio_gattc_write_sync(struct bt_conn *conn, uint16_t handle, write_params.data = data; write_params.length = len; + // `bt_conn_ref()` holds a reference to the connection, so conn stays valid until + // its state is checked below, even if a disconnect drops the connection's own + // reference meanwhile. + bt_conn_ref(conn); int err = bt_gatt_write(conn, &write_params); if (err != 0) { active_write_ctx = NULL; + bt_conn_unref(conn); raise_zephyr_error(err); } @@ -203,7 +256,7 @@ void bleio_gattc_write_sync(struct bt_conn *conn, uint16_t handle, } active_write_ctx = NULL; - if (ctx.err != 0) { - raise_zephyr_error(ctx.err); - } + bool connected = conn_is_connected(conn); + bt_conn_unref(conn); + check_gattc_status(connected, ctx.err); } From eb84689b8ff8e2b546e7a1da41a475e973ea3ff3 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Mon, 12 Oct 2026 20:00:32 -0400 Subject: [PATCH 2/2] zephyr-cp: raise if bt_conn_ref() fails in GATT client read/write bt_conn_ref() returns NULL if the peer disconnects just before the call. The following bt_conn_unref() would then underflow the reference count. Co-Authored-By: Claude Opus 5.5 --- ports/zephyr-cp/common-hal/_bleio/__init__.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/ports/zephyr-cp/common-hal/_bleio/__init__.c b/ports/zephyr-cp/common-hal/_bleio/__init__.c index f3ae847e31a..c29e7aae085 100644 --- a/ports/zephyr-cp/common-hal/_bleio/__init__.c +++ b/ports/zephyr-cp/common-hal/_bleio/__init__.c @@ -204,8 +204,11 @@ size_t bleio_gattc_read_sync(struct bt_conn *conn, uint16_t handle, // `bt_conn_ref()` holds a reference to the connection, so conn stays valid until // its state is checked below, even if a disconnect drops the connection's own - // reference meanwhile. - bt_conn_ref(conn); + // reference meanwhile. It returns NULL if the peer has already disconnected. + if (bt_conn_ref(conn) == NULL) { + active_read_ctx = NULL; + mp_raise_ConnectionError(MP_ERROR_TEXT("Not connected")); + } int err = bt_gatt_read(conn, &read_params); if (err != 0) { active_read_ctx = NULL; @@ -242,8 +245,11 @@ void bleio_gattc_write_sync(struct bt_conn *conn, uint16_t handle, // `bt_conn_ref()` holds a reference to the connection, so conn stays valid until // its state is checked below, even if a disconnect drops the connection's own - // reference meanwhile. - bt_conn_ref(conn); + // reference meanwhile. It returns NULL if the peer has already disconnected. + if (bt_conn_ref(conn) == NULL) { + active_write_ctx = NULL; + mp_raise_ConnectionError(MP_ERROR_TEXT("Not connected")); + } int err = bt_gatt_write(conn, &write_params); if (err != 0) { active_write_ctx = NULL;