Skip to content

espressif: fix connect() timeout and Ctrl-C handling - #11559

Open
dhalbert wants to merge 2 commits into
adafruit:mainfrom
dhalbert:espressif-connect-timeout
Open

dhalbert wants to merge 2 commits into
adafruit:mainfrom
dhalbert:espressif-connect-timeout

Conversation

@dhalbert

@dhalbert dhalbert commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Diagnosis and code by Claude, reviewed by @dhalbert.

Problem

  • When connect() timed out, it usually returned a useless Connection, with connected False, instead of raising an exception.
    • The timeout argument of _bleio.Adapter.connect() was used both by NimBLE and by connect()'s own wait loop. NimBLE reports its timeout only after the controller cancels the connection attempt, so the wait loop usually stopped first.
    • The connection status was then still invalid, which matched an unused connection slot, so connect() returned that.
  • NimBLE also kept trying to connect, so the next connect() raised "Already in progress".
  • When NimBLE did report first, its timeout came out as "Unknown BLE error: 13".
  • Ctrl-C during connect() also left NimBLE connecting.
  • With a peer that starts the MTU exchange itself, such as Zephyr, every connect() waited out its full timeout and returned a useless Connection, though the real connection was up.
    • NimBLE answers the peer's MTU request before CircuitPython sees the connection, and then refuses CircuitPython's own MTU exchange with BLE_HS_EALREADY. So _mtu_reply(), which ends the wait, was never called.
    • The negotiated MTU was also never recorded, so max_packet_length was 20.

Fix

  • connect() now waits for NimBLE's BLE_GAP_EVENT_CONNECT, which NimBLE always sends, and raises BluetoothError: Failed to connect: timeout on BLE_HS_ETIMEOUT. This matches nordic.
  • On Ctrl-C, it cancels the attempt with ble_gap_conn_cancel() and waits for the cancellation.
  • The status is reset before ble_gap_connect(), so an early event can't be lost.
  • If the MTU exchange can't be started, which is the usual case with a Zephyr peer, the wait now ends anyway, and the MTU is read with ble_att_mtu().

Testing

Metro ESP32-S3 with this PR as central, nRF54LM20 DK on zephyr-cp as an advertising peripheral:

Test Result
connect() to an absent address, 3 s timeout (twice) BluetoothError: Failed to connect: timeout after 3.0 s
connect() to the DK right after each timeout connected
connect() to the DK connected in 1.0 s, max_packet_length 248; before: useless Connection after the full timeout, max_packet_length 20
Ctrl-C during a 30 s connect() to an absent address KeyboardInterrupt; next connect() to the DK and next absent timeout both normal

dhalbert and others added 2 commits October 11, 2026 18:51
When connect() timed out, it usually returned a useless Connection,
with connected False, instead of raising an exception. The timeout was
used both by NimBLE and by connect()'s own wait loop. NimBLE reports
its timeout only after the controller cancels the connection attempt,
so the wait loop usually stopped first. The connection status was then
still invalid, which matched an unused connection slot, so connect()
returned that. NimBLE also kept trying to connect, so the next
connect() raised "Already in progress". When NimBLE did report first,
its timeout came out as "Unknown BLE error: 13".

Wait for NimBLE's BLE_GAP_EVENT_CONNECT instead, which it always sends,
and raise "Failed to connect: timeout" on BLE_HS_ETIMEOUT. On Ctrl-C,
cancel the attempt and wait for the cancellation. Reset the status
before ble_gap_connect(), and end the wait if the MTU exchange can't
be started.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A peer such as Zephyr starts the MTU exchange itself right after
connecting. NimBLE answers it before _new_connection() runs, and then
ble_gattc_exchange_mtu() returns BLE_HS_EALREADY, so _mtu_reply() is
never called. The BLE_GAP_EVENT_MTU went to _connect_event(), which
ignores it, so connection->mtu stayed 0 and max_packet_length reported
20 instead of the negotiated MTU minus 3.

Get the MTU with ble_att_mtu() in that case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dhalbert
dhalbert requested a review from mikeysklar October 12, 2026 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant