Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

drv/bluetooth/stm32_cc2640: send correct max size #229

Merged
merged 2 commits into from
Jan 22, 2024
Merged

drv/bluetooth/stm32_cc2640: send correct max size #229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d83815
drv/bluetooth/stm32_cc2640: send correct max size
dlech Jan 21, 2024
d501df1
drv/bluetooth_stm32_cc2640: respond to prepared write
dlech Jan 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@

### Fixes
- Fix observing stopping on City and Technic hubs after some time ([support#1096]).
- Fix Bluetooth locking up when connecting Bluetooth adapter with small MTU to Technic and City hubs ([support#947]).

[support#947]: https://github.com/pybricks/support/issues/947
[support#1096]: https://github.com/pybricks/support/issues/1096

## [3.3.0] - 2023-11-24
Expand Down
9 changes: 9 additions & 0 deletions lib/ble5stack/central/att.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,15 @@ bStatus_t ATT_WriteRsp(uint16_t connHandle) {
return HCI_sendHCICommand(ATT_CMD_WRITE_RSP, buf, 2);
}

bStatus_t ATT_ExecuteWriteRsp(uint16_t connHandle) {
uint8_t buf[2];

buf[0] = connHandle & 0xFF;
buf[1] = (connHandle >> 8) & 0xFF;

return HCI_sendHCICommand(ATT_CMD_EXECUTE_WRITE_RSP, buf, 2);
}

bStatus_t ATT_HandleValueNoti(uint16_t connHandle, attHandleValueNoti_t *pNoti) {
uint8_t buf[32];

Expand Down
6 changes: 3 additions & 3 deletions lib/ble5stack/central/att.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ extern "C"
#define ATT_CMD_PREPAREWRITEREQ 0xFD16
#define ATT_CMD_PREPAREWRITERSP 0xFD17
#define ATT_CMD_EXECUTEWRITEREQ 0xFD18
#define ATT_CMD_EXECUTEWRITERSP 0xFD19
#define ATT_CMD_EXECUTE_WRITE_RSP 0xFD19
#define ATT_CMD_HANDLE_VALUE_NOTI 0xFD1B
#define ATT_CMD_HANDLEVALUEIND 0xFD1D
#define ATT_CMD_HANDLEVALUECFM 0xFD1E
Expand Down Expand Up @@ -125,9 +125,9 @@ extern "C"
#define ATT_EVENT_READ_BY_GRP_TYPE_RSP 0x0511
#define ATT_EVENT_WRITE_REQ 0x0512
#define ATT_EVENT_WRITE_RSP 0x0513
#define ATT_EVENT_PREPAREWRITEREQ 0x0516
#define ATT_EVENT_PREPARE_WRITE_REQ 0x0516
#define ATT_EVENT_PREPAREWRITERSP 0x0517
#define ATT_EVENT_EXECUTEWRITEREQ 0x0518
#define ATT_EVENT_EXECUTE_WRITE_REQ 0x0518
#define ATT_EVENT_EXECUTEWRITERSP 0x0519
#define ATT_EVENT_HANDLE_VALUE_NOTI 0x051B
#define ATT_EVENT_HANDLEVALUEIND 0x051D
Expand Down
41 changes: 35 additions & 6 deletions lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ static bool device_discovery_done;
static bool advertising_data_received;
// handle to connected Bluetooth device
static uint16_t conn_handle = NO_CONNECTION;
static uint16_t conn_mtu;
// handle to connected remote control
static uint16_t remote_handle = NO_CONNECTION;
// handle to LWP3 characteristic on remote
Expand Down Expand Up @@ -1094,12 +1095,17 @@ static void handle_event(uint8_t *packet) {

switch (event_code) {
case ATT_EVENT_EXCHANGE_MTU_REQ: {
// uint16_t client_mtu = (data[7] << 8) | data[6];
attExchangeMTURsp_t rsp;
uint16_t client_mtu = (data[7] << 8) | data[6];

// REVISIT: Just saving the main connection MTU for now.
// If we allow multiple connections, this will need to be
// changed.
if (connection_handle == conn_handle) {
conn_mtu = MIN(client_mtu, PBDRV_BLUETOOTH_MAX_MTU_SIZE);
}

attExchangeMTURsp_t rsp;
rsp.serverRxMTU = PBDRV_BLUETOOTH_MAX_MTU_SIZE;
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, why does this response not use the conn_mtu value?

Copy link
Member Author

Choose a reason for hiding this comment

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

Per the Bluetooth spec, both the central and peripheral send their own value and take the lesser of the two. This way, it doesn't matter who asks first.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! If you have time at some point, I have a few more Bluetooth questions 😄

// REVISIT: may need to keep a table of min(client_mtu, MAX_ATT_MTU_SIZE)
// for each connection if any known clients have smaller MTU
ATT_ExchangeMTURsp(connection_handle, &rsp);
}
break;
Expand Down Expand Up @@ -1277,8 +1283,8 @@ static void handle_event(uint8_t *packet) {
attReadRsp_t rsp;
uint8_t buf[PBIO_PYBRICKS_HUB_CAPABILITIES_VALUE_SIZE];

// REVISIT: client MTU may be smaller, in which case we can't used fixed value for MTU
pbio_pybricks_hub_capabilities(buf, PBDRV_BLUETOOTH_MAX_MTU_SIZE - 3, PBSYS_APP_HUB_FEATURE_FLAGS, PBSYS_PROGRAM_LOAD_MAX_PROGRAM_SIZE);
// REVISIT: this assumes connection_handle == conn_handle
pbio_pybricks_hub_capabilities(buf, conn_mtu - 3, PBSYS_APP_HUB_FEATURE_FLAGS, PBSYS_PROGRAM_LOAD_MAX_PROGRAM_SIZE);
rsp.len = sizeof(buf);
rsp.pValue = buf;
ATT_ReadRsp(connection_handle, &rsp);
Expand Down Expand Up @@ -1427,6 +1433,27 @@ static void handle_event(uint8_t *packet) {
}
break;

case ATT_EVENT_PREPARE_WRITE_REQ: {
uint16_t char_handle = (data[7] << 8) | data[6];

// long writes are not currently supported for any characteristic
attErrorRsp_t rsp;
rsp.reqOpcode = ATT_PREPARE_WRITE_REQ;
rsp.handle = char_handle;
rsp.errCode = ATT_ERR_ATTR_NOT_LONG;
ATT_ErrorRsp(connection_handle, &rsp);
}
break;

case ATT_EVENT_EXECUTE_WRITE_REQ: {
// uint8_t flags = data[6];

// Send this to keep host happy. Should only be receiving
// cancel since we don't support long writes.
ATT_ExecuteWriteRsp(connection_handle);
}
break;

case ATT_EVENT_HANDLE_VALUE_NOTI: {
// TODO: match callback to handle
// uint8_t attr_handle = (data[7] << 8) | data[6];
Expand All @@ -1445,6 +1472,8 @@ static void handle_event(uint8_t *packet) {
// we currently only allow connection from one central
conn_handle = (data[11] << 8) | data[10];
DBG("link: %04x", conn_handle);
// assume minimum MTU until we get an exchange MTU request
conn_mtu = ATT_MTU_SIZE;

// On 2019 and newer MacBooks, the default interval was
// measured to be 15 ms. This caused advertisement to
Expand Down