Skip to content

Commit 84cee1a

Browse files
dhalbertdhalbert
committed
rename and improve PacketBuffer packet length property
1 parent 2d7cf4b commit 84cee1a

11 files changed

Lines changed: 120 additions & 53 deletions

File tree

‎ports/nrf/common-hal/_bleio/Characteristic.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ size_t common_hal_bleio_characteristic_get_value(bleio_characteristic_obj_t *sel
127127
// self->value is set by evt handler.
128128
return common_hal_bleio_gattc_read(self->handle, conn_handle, buf, len);
129129
} else {
130+
// conn_handle is ignored for non-system attributes.
130131
return common_hal_bleio_gatts_read(self->handle, conn_handle, buf, len);
131132
}
132133
}
@@ -152,6 +153,7 @@ void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self,
152153
(self->props & CHAR_PROP_WRITE_NO_RESPONSE));
153154
} else {
154155
// Always write the value locally even if no connections are active.
156+
// conn_handle is ignored for non-system attributes, so we use BLE_CONN_HANDLE_INVALID.
155157
common_hal_bleio_gatts_write(self->handle, BLE_CONN_HANDLE_INVALID, bufinfo);
156158
// Check to see if we need to notify or indicate any active connections.
157159
for (size_t i = 0; i < BLEIO_TOTAL_CONNECTION_COUNT; i++) {

‎ports/nrf/common-hal/_bleio/Connection.c‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,11 @@ mp_float_t common_hal_bleio_connection_get_connection_interval(bleio_connection_
371371
return 1.25f * self->conn_params.min_conn_interval;
372372
}
373373

374+
// Return the current negotiated MTU length, minus overhead.
375+
mp_int_t common_hal_bleio_connection_get_max_packet_length(bleio_connection_internal_t *self) {
376+
return (self->mtu == 0 ? BLE_GATT_ATT_MTU_DEFAULT : self->mtu) - 3;
377+
}
378+
374379
void common_hal_bleio_connection_set_connection_interval(bleio_connection_internal_t *self, mp_float_t new_interval) {
375380
self->conn_params_updating = true;
376381
uint16_t interval = new_interval / 1.25f;
@@ -752,3 +757,16 @@ mp_obj_t bleio_connection_new_from_internal(bleio_connection_internal_t* interna
752757

753758
return MP_OBJ_FROM_PTR(connection);
754759
}
760+
761+
// Find the connection that uses the given conn_handle. Return NULL if not found.
762+
bleio_connection_internal_t *bleio_conn_handle_to_connection(uint16_t conn_handle) {
763+
bleio_connection_internal_t *connection;
764+
for (size_t i = 0; i < BLEIO_TOTAL_CONNECTION_COUNT; i++) {
765+
connection = &bleio_connections[i];
766+
if (connection->conn_handle == conn_handle) {
767+
return connection;
768+
}
769+
}
770+
771+
return NULL;
772+
}

‎ports/nrf/common-hal/_bleio/Connection.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,5 +87,6 @@ bool connection_on_ble_evt(ble_evt_t *ble_evt, void *self_in);
8787

8888
uint16_t bleio_connection_get_conn_handle(bleio_connection_obj_t *self);
8989
mp_obj_t bleio_connection_new_from_internal(bleio_connection_internal_t* connection);
90+
bleio_connection_internal_t *bleio_conn_handle_to_connection(uint16_t conn_handle);
9091

9192
#endif // MICROPY_INCLUDED_NRF_COMMON_HAL_BLEIO_CONNECTION_H

‎ports/nrf/common-hal/_bleio/PacketBuffer.c‎

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,18 @@ STATIC uint32_t queue_next_write(bleio_packet_buffer_obj_t *self) {
107107
}
108108

109109
STATIC bool packet_buffer_on_ble_client_evt(ble_evt_t *ble_evt, void *param) {
110-
bleio_packet_buffer_obj_t *self = (bleio_packet_buffer_obj_t *) param;
110+
const uint16_t evt_id = ble_evt->header.evt_id;
111+
// Check if this is a GATTC event so we can make sure the conn_handle is valid.
112+
if (evt_id < BLE_GATTC_EVT_BASE || evt_id > BLE_GATTC_EVT_LAST) {
113+
return false;
114+
}
115+
111116
uint16_t conn_handle = ble_evt->evt.gattc_evt.conn_handle;
117+
bleio_packet_buffer_obj_t *self = (bleio_packet_buffer_obj_t *) param;
112118
if (conn_handle != self->conn_handle) {
113119
return false;
114120
}
115-
switch (ble_evt->header.evt_id) {
121+
switch (evt_id) {
116122
case BLE_GATTC_EVT_HVX: {
117123
// A remote service wrote to this characteristic.
118124
ble_gattc_evt_hvx_t* evt_hvx = &ble_evt->evt.gattc_evt.params.hvx;
@@ -142,9 +148,9 @@ STATIC bool packet_buffer_on_ble_client_evt(ble_evt_t *ble_evt, void *param) {
142148

143149
STATIC bool packet_buffer_on_ble_server_evt(ble_evt_t *ble_evt, void *param) {
144150
bleio_packet_buffer_obj_t *self = (bleio_packet_buffer_obj_t *) param;
145-
uint16_t conn_handle = ble_evt->evt.gatts_evt.conn_handle;
146151
switch (ble_evt->header.evt_id) {
147152
case BLE_GATTS_EVT_WRITE: {
153+
uint16_t conn_handle = ble_evt->evt.gatts_evt.conn_handle;
148154
// A client wrote to this server characteristic.
149155

150156
ble_gatts_evt_write_t *evt_write = &ble_evt->evt.gatts_evt.params.write;
@@ -168,7 +174,7 @@ STATIC bool packet_buffer_on_ble_server_evt(ble_evt_t *ble_evt, void *param) {
168174
break;
169175
}
170176
case BLE_GAP_EVT_DISCONNECTED: {
171-
if (self->conn_handle == conn_handle) {
177+
if (self->conn_handle == ble_evt->evt.gap_evt.conn_handle) {
172178
self->conn_handle = BLE_CONN_HANDLE_INVALID;
173179
}
174180
}
@@ -202,7 +208,6 @@ void common_hal_bleio_packet_buffer_construct(
202208
}
203209

204210
if (incoming) {
205-
// This is a macro.
206211
ringbuf_alloc(&self->ringbuf, buffer_size * (sizeof(uint16_t) + characteristic->max_length), false);
207212

208213
if (self->ringbuf.buf == NULL) {
@@ -249,7 +254,7 @@ void common_hal_bleio_packet_buffer_construct(
249254
}
250255
}
251256

252-
int common_hal_bleio_packet_buffer_readinto(bleio_packet_buffer_obj_t *self, uint8_t *data, size_t len) {
257+
mp_int_t common_hal_bleio_packet_buffer_readinto(bleio_packet_buffer_obj_t *self, uint8_t *data, size_t len) {
253258
if (ringbuf_count(&self->ringbuf) < 2) {
254259
return 0;
255260
}
@@ -280,7 +285,7 @@ void common_hal_bleio_packet_buffer_write(bleio_packet_buffer_obj_t *self, uint8
280285
if (self->conn_handle == BLE_CONN_HANDLE_INVALID) {
281286
return;
282287
}
283-
uint16_t packet_size = common_hal_bleio_packet_buffer_get_packet_size(self);
288+
uint16_t packet_size = common_hal_bleio_packet_buffer_get_incoming_packet_length(self);
284289
uint16_t max_size = packet_size - len;
285290
while (max_size < self->pending_size && self->conn_handle != BLE_CONN_HANDLE_INVALID) {
286291
RUN_BACKGROUND_TASKS;
@@ -308,26 +313,30 @@ void common_hal_bleio_packet_buffer_write(bleio_packet_buffer_obj_t *self, uint8
308313
}
309314
}
310315

311-
uint16_t common_hal_bleio_packet_buffer_get_packet_size(bleio_packet_buffer_obj_t *self) {
312-
uint16_t mtu;
313-
if (self->conn_handle == BLE_CONN_HANDLE_INVALID) {
314-
return 0;
315-
}
316-
bleio_connection_internal_t *connection;
317-
for (size_t i = 0; i < BLEIO_TOTAL_CONNECTION_COUNT; i++) {
318-
connection = &bleio_connections[i];
319-
if (connection->conn_handle == self->conn_handle) {
320-
break;
316+
mp_int_t common_hal_bleio_packet_buffer_get_incoming_packet_length(bleio_packet_buffer_obj_t *self) {
317+
// If this PacketBuffer is being used for NOTIFY or INDICATE from
318+
// a remote service, the maximum size is what can be sent in one
319+
// BLE packet. But we must be connected to know that value.
320+
//
321+
// Otherwise it can be a long as the characteristic
322+
// will permit, whether or not we're connected.
323+
324+
if (self->characteristic != NULL &&
325+
self->characteristic->service != NULL &&
326+
self->characteristic->service->is_remote &&
327+
(common_hal_bleio_characteristic_get_properties(self->characteristic) &
328+
(CHAR_PROP_INDICATE | CHAR_PROP_NOTIFY)) &&
329+
self->conn_handle != BLE_CONN_HANDLE_INVALID) {
330+
bleio_connection_internal_t *connection = bleio_conn_handle_to_connection(self->conn_handle);
331+
if (connection) {
332+
return MIN(common_hal_bleio_connection_get_max_packet_length(connection),
333+
self->characteristic->max_length);
321334
}
335+
// There's no current connection, so we don't know the MTU, and
336+
// we can't tell what the largest incoming packet length would be.
337+
return -1;
322338
}
323-
if (connection->mtu == 0) {
324-
mtu = BLE_GATT_ATT_MTU_DEFAULT;
325-
}
326-
if (self->characteristic->max_length > mtu) {
327-
mtu = self->characteristic->max_length;
328-
}
329-
uint16_t att_overhead = 3;
330-
return mtu - att_overhead;
339+
return self->characteristic->max_length;
331340
}
332341

333342
bool common_hal_bleio_packet_buffer_deinited(bleio_packet_buffer_obj_t *self) {

‎ports/nrf/common-hal/_bleio/PacketBuffer.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ typedef struct {
4141
// the other is waiting to be queued and can be extended.
4242
uint8_t* outgoing[2];
4343
uint16_t pending_size;
44+
// We remember the conn_handle so we can do a NOTIFY/INDICATE to a client.
45+
// We can find out the conn_handle on a Characteristic write or a CCCD write (but not a read).
4446
uint16_t conn_handle;
4547
uint8_t pending_index;
4648
uint8_t write_type;

‎ports/nrf/common-hal/_bleio/Service.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ typedef struct bleio_service_obj {
3939
bool is_remote;
4040
bool is_secondary;
4141
bleio_uuid_obj_t *uuid;
42+
// The connection object is set only when this is a remote service.
43+
// A local service doesn't know the connection.
4244
mp_obj_t connection;
4345
mp_obj_list_t *characteristic_list;
4446
// Range of attribute handles of this remote service.

‎ports/nrf/common-hal/_bleio/__init__.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ void common_hal_bleio_check_connected(uint16_t conn_handle) {
118118

119119
// GATTS read of a Characteristic or Descriptor.
120120
size_t common_hal_bleio_gatts_read(uint16_t handle, uint16_t conn_handle, uint8_t* buf, size_t len) {
121-
// conn_handle might be BLE_CONN_HANDLE_INVALID if we're not connected, but that's OK, because
122-
// we can still read and write the local value.
121+
// conn_handle is ignored unless this is a system attribute.
122+
// If we're not connected, but that's OK, because we can still read and write the local value.
123123

124124
ble_gatts_value_t gatts_value = {
125125
.p_value = buf,
@@ -132,8 +132,8 @@ size_t common_hal_bleio_gatts_read(uint16_t handle, uint16_t conn_handle, uint8_
132132
}
133133

134134
void common_hal_bleio_gatts_write(uint16_t handle, uint16_t conn_handle, mp_buffer_info_t *bufinfo) {
135-
// conn_handle might be BLE_CONN_HANDLE_INVALID if we're not connected, but that's OK, because
136-
// we can still read and write the local value.
135+
// conn_handle is ignored unless this is a system attribute.
136+
// If we're not connected, but that's OK, because we can still read and write the local value.
137137

138138
ble_gatts_value_t gatts_value = {
139139
.p_value = bufinfo->buf,

‎shared-bindings/_bleio/Connection.c‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,25 @@ STATIC mp_obj_t bleio_connection_get_connection_interval(mp_obj_t self_in) {
214214
}
215215
STATIC MP_DEFINE_CONST_FUN_OBJ_1(bleio_connection_get_connection_interval_obj, bleio_connection_get_connection_interval);
216216

217+
//| .. attribute:: max_packet_length
218+
//|
219+
//| The maximum number of data bytes that can be sent in a single transmission,
220+
//| not including overhead bytes.
221+
//|
222+
//| This is the maximum number of bytes that can be sent in a notification,
223+
//| which must be sent in a single packet.
224+
//| But for a regular characteristic read or write, may be sent in multiple packets,
225+
//| so this limit does not apply.
226+
//|
227+
STATIC mp_obj_t bleio_connection_get_max_packet_length(mp_obj_t self_in) {
228+
bleio_connection_obj_t *self = MP_OBJ_TO_PTR(self_in);
229+
230+
bleio_connection_ensure_connected(self);
231+
return mp_obj_new_int(common_hal_bleio_connection_get_max_packet_length(self->connection));
232+
}
233+
STATIC MP_DEFINE_CONST_FUN_OBJ_1(bleio_connection_get_max_packet_length_obj, bleio_connection_get_max_packet_length);
234+
235+
217236
STATIC mp_obj_t bleio_connection_set_connection_interval(mp_obj_t self_in, mp_obj_t interval_in) {
218237
bleio_connection_obj_t *self = MP_OBJ_TO_PTR(self_in);
219238

@@ -233,6 +252,13 @@ const mp_obj_property_t bleio_connection_connection_interval_obj = {
233252
(mp_obj_t)&mp_const_none_obj },
234253
};
235254

255+
const mp_obj_property_t bleio_connection_max_packet_length_obj = {
256+
.base.type = &mp_type_property,
257+
.proxy = { (mp_obj_t)&bleio_connection_get_max_packet_length_obj,
258+
(mp_obj_t)&mp_const_none_obj,
259+
(mp_obj_t)&mp_const_none_obj },
260+
};
261+
236262
STATIC const mp_rom_map_elem_t bleio_connection_locals_dict_table[] = {
237263
// Methods
238264
{ MP_ROM_QSTR(MP_QSTR_pair), MP_ROM_PTR(&bleio_connection_pair_obj) },
@@ -243,7 +269,7 @@ STATIC const mp_rom_map_elem_t bleio_connection_locals_dict_table[] = {
243269
{ MP_ROM_QSTR(MP_QSTR_connected), MP_ROM_PTR(&bleio_connection_connected_obj) },
244270
{ MP_ROM_QSTR(MP_QSTR_paired), MP_ROM_PTR(&bleio_connection_paired_obj) },
245271
{ MP_ROM_QSTR(MP_QSTR_connection_interval), MP_ROM_PTR(&bleio_connection_connection_interval_obj) },
246-
272+
{ MP_ROM_QSTR(MP_QSTR_max_packet_length), MP_ROM_PTR(&bleio_connection_max_packet_length_obj) },
247273
};
248274

249275
STATIC MP_DEFINE_CONST_DICT(bleio_connection_locals_dict, bleio_connection_locals_dict_table);

‎shared-bindings/_bleio/Connection.h‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,12 @@
3434

3535
extern const mp_obj_type_t bleio_connection_type;
3636

37-
extern void common_hal_bleio_connection_pair(bleio_connection_internal_t *self, bool bond);
38-
extern void common_hal_bleio_connection_disconnect(bleio_connection_internal_t *self);
39-
extern bool common_hal_bleio_connection_get_connected(bleio_connection_obj_t *self);
40-
extern bool common_hal_bleio_connection_get_paired(bleio_connection_obj_t *self);
41-
extern mp_obj_tuple_t *common_hal_bleio_connection_discover_remote_services(bleio_connection_obj_t *self, mp_obj_t service_uuids_whitelist);
37+
void common_hal_bleio_connection_pair(bleio_connection_internal_t *self, bool bond);
38+
void common_hal_bleio_connection_disconnect(bleio_connection_internal_t *self);
39+
bool common_hal_bleio_connection_get_connected(bleio_connection_obj_t *self);
40+
mp_int_t common_hal_bleio_connection_get_max_packet_length(bleio_connection_internal_t *self);
41+
bool common_hal_bleio_connection_get_paired(bleio_connection_obj_t *self);
42+
mp_obj_tuple_t *common_hal_bleio_connection_discover_remote_services(bleio_connection_obj_t *self, mp_obj_t service_uuids_whitelist);
4243

4344
mp_float_t common_hal_bleio_connection_get_connection_interval(bleio_connection_internal_t *self);
4445
void common_hal_bleio_connection_set_connection_interval(bleio_connection_internal_t *self, mp_float_t new_interval);

‎shared-bindings/_bleio/PacketBuffer.c‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
//|
4343
//| Accumulates a Characteristic's incoming packets in a FIFO buffer and facilitates packet aware
4444
//| outgoing writes. A packet's size is either the characteristic length or the maximum transmission
45-
//| unit (MTU), whichever is smaller. The MTU can change so check `packet_size` before creating a
45+
//| unit (MTU), whichever is smaller. The MTU can change so check `incoming_packet_length` before creating a
4646
//| buffer to store data.
4747
//|
4848
//| When we're the server, we ignore all connections besides the first to subscribe to
@@ -71,7 +71,7 @@ STATIC mp_obj_t bleio_packet_buffer_make_new(const mp_obj_type_t *type, size_t n
7171

7272
const mp_obj_t characteristic = args[ARG_characteristic].u_obj;
7373

74-
const int buffer_size = args[ARG_buffer_size].u_int;
74+
const mp_int_t buffer_size = args[ARG_buffer_size].u_int;
7575
if (buffer_size < 1) {
7676
mp_raise_ValueError_varg(translate("%q must be >= 1"), MP_QSTR_buffer_size);
7777
}
@@ -97,7 +97,7 @@ STATIC void check_for_deinit(bleio_packet_buffer_obj_t *self) {
9797
//| .. method:: readinto(buf)
9898
//|
9999
//| Reads a single BLE packet into the ``buf``. Raises an exception if the next packet is longer
100-
//| than the given buffer. Use `packet_size` to read the maximum length of a single packet.
100+
//| than the given buffer. Use `incoming_packet_length` to read the maximum length of a single packet.
101101
//|
102102
//| :return: number of bytes read and stored into ``buf``
103103
//| :rtype: int
@@ -109,7 +109,7 @@ STATIC mp_obj_t bleio_packet_buffer_readinto(mp_obj_t self_in, mp_obj_t buffer_o
109109
mp_buffer_info_t bufinfo;
110110
mp_get_buffer_raise(buffer_obj, &bufinfo, MP_BUFFER_WRITE);
111111

112-
int size = common_hal_bleio_packet_buffer_readinto(self, bufinfo.buf, bufinfo.len);
112+
mp_int_t size = common_hal_bleio_packet_buffer_readinto(self, bufinfo.buf, bufinfo.len);
113113
if (size < 0) {
114114
mp_raise_ValueError_varg(translate("Buffer too short by %d bytes"), size * -1);
115115
}
@@ -166,33 +166,39 @@ STATIC mp_obj_t bleio_packet_buffer_deinit(mp_obj_t self_in) {
166166
}
167167
STATIC MP_DEFINE_CONST_FUN_OBJ_1(bleio_packet_buffer_deinit_obj, bleio_packet_buffer_deinit);
168168

169-
//| .. attribute:: packet_size
169+
//| .. attribute:: incoming_packet_length
170170
//|
171-
//| Maximum size of each packet in bytes. This is the minimum of the Characteristic length and
172-
//| the negotiated Maximum Transfer Unit (MTU).
171+
//| Maximum size of a packet.
172+
//| If the packet is arriving from a remote service via notify or indicate,
173+
//| the maximum size is `Connection.max_packet_length`.
174+
//| Otherwise it is the ``max_length`` of the `Characteristic`.
173175
//|
174-
STATIC mp_obj_t bleio_packet_buffer_get_packet_size(mp_obj_t self_in) {
176+
STATIC mp_obj_t bleio_packet_buffer_get_incoming_packet_length(mp_obj_t self_in) {
175177
bleio_packet_buffer_obj_t *self = MP_OBJ_TO_PTR(self_in);
176178

177-
return MP_OBJ_NEW_SMALL_INT(common_hal_bleio_packet_buffer_get_packet_size(self));
179+
mp_int_t size = common_hal_bleio_packet_buffer_get_incoming_packet_length(self);
180+
if (size < 0) {
181+
mp_raise_ValueError(translate("No connection: size cannot be determined"));
182+
}
183+
return MP_OBJ_NEW_SMALL_INT(size);
178184
}
179-
STATIC MP_DEFINE_CONST_FUN_OBJ_1(bleio_packet_buffer_get_packet_size_obj, bleio_packet_buffer_get_packet_size);
185+
STATIC MP_DEFINE_CONST_FUN_OBJ_1(bleio_packet_buffer_get_incoming_packet_length_obj, bleio_packet_buffer_get_incoming_packet_length);
180186

181-
const mp_obj_property_t bleio_packet_buffer_packet_size_obj = {
187+
const mp_obj_property_t bleio_packet_buffer_incoming_packet_length_obj = {
182188
.base.type = &mp_type_property,
183-
.proxy = { (mp_obj_t)&bleio_packet_buffer_get_packet_size_obj,
189+
.proxy = { (mp_obj_t)&bleio_packet_buffer_get_incoming_packet_length_obj,
184190
(mp_obj_t)&mp_const_none_obj,
185191
(mp_obj_t)&mp_const_none_obj },
186192
};
187193

188194
STATIC const mp_rom_map_elem_t bleio_packet_buffer_locals_dict_table[] = {
189-
{ MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&bleio_packet_buffer_deinit_obj) },
195+
{ MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&bleio_packet_buffer_deinit_obj) },
190196

191197
// Standard stream methods.
192-
{ MP_OBJ_NEW_QSTR(MP_QSTR_readinto), MP_ROM_PTR(&bleio_packet_buffer_readinto_obj) },
193-
{ MP_OBJ_NEW_QSTR(MP_QSTR_write), MP_ROM_PTR(&bleio_packet_buffer_write_obj) },
198+
{ MP_OBJ_NEW_QSTR(MP_QSTR_readinto), MP_ROM_PTR(&bleio_packet_buffer_readinto_obj) },
199+
{ MP_OBJ_NEW_QSTR(MP_QSTR_write), MP_ROM_PTR(&bleio_packet_buffer_write_obj) },
194200

195-
{ MP_OBJ_NEW_QSTR(MP_QSTR_packet_size), MP_ROM_PTR(&bleio_packet_buffer_packet_size_obj) },
201+
{ MP_OBJ_NEW_QSTR(MP_QSTR_incoming_packet_length), MP_ROM_PTR(&bleio_packet_buffer_incoming_packet_length_obj) },
196202
};
197203

198204
STATIC MP_DEFINE_CONST_DICT(bleio_packet_buffer_locals_dict, bleio_packet_buffer_locals_dict_table);

0 commit comments

Comments
 (0)