Skip to content

Commit 1570ef2

Browse files
dhalbertdhalbert
committed
specifying attribute length; fix up value setting
1 parent d047b73 commit 1570ef2

File tree

11 files changed

+193
-82
lines changed

11 files changed

+193
-82
lines changed

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

Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ STATIC uint16_t characteristic_get_cccd(uint16_t cccd_handle, uint16_t conn_hand
4343

4444
const uint32_t err_code = sd_ble_gatts_value_get(conn_handle, cccd_handle, &value);
4545

46-
4746
if (err_code == BLE_ERROR_GATTS_SYS_ATTR_MISSING) {
4847
// CCCD is not set, so say that neither Notify nor Indicate is enabled.
4948
cccd = 0;
@@ -125,16 +124,24 @@ STATIC void characteristic_gattc_read(bleio_characteristic_obj_t *characteristic
125124
ble_drv_remove_event_handler(characteristic_on_gattc_read_rsp_evt, characteristic);
126125
}
127126

128-
void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self, bleio_uuid_obj_t *uuid, bleio_characteristic_properties_t props, bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm, mp_obj_list_t *descriptor_list) {
127+
void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self, bleio_uuid_obj_t *uuid, bleio_characteristic_properties_t props, bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm, mp_int_t max_length, bool fixed_length, mp_obj_list_t *descriptor_list) {
129128
self->service = MP_OBJ_NULL;
130129
self->uuid = uuid;
131-
self->value = mp_const_none;
130+
self->value = mp_const_empty_bytes;
132131
self->handle = BLE_GATT_HANDLE_INVALID;
133132
self->props = props;
134133
self->read_perm = read_perm;
135134
self->write_perm = write_perm;
136135
self->descriptor_list = descriptor_list;
137136

137+
const mp_int_t max_length_max = fixed_length ? BLE_GATTS_FIX_ATTR_LEN_MAX : BLE_GATTS_VAR_ATTR_LEN_MAX;
138+
if (max_length < 0 || max_length > max_length_max) {
139+
mp_raise_ValueError_varg(translate("max_length must be 0-%d when fixed_length is %s"),
140+
max_length_max, fixed_length ? "True" : "False");
141+
}
142+
self->max_length = max_length;
143+
self->fixed_length = fixed_length;
144+
138145
for (size_t descriptor_idx = 0; descriptor_idx < descriptor_list->len; ++descriptor_idx) {
139146
bleio_descriptor_obj_t *descriptor =
140147
MP_OBJ_TO_PTR(descriptor_list->items[descriptor_idx]);
@@ -151,48 +158,63 @@ bleio_service_obj_t *common_hal_bleio_characteristic_get_service(bleio_character
151158
}
152159

153160
mp_obj_t common_hal_bleio_characteristic_get_value(bleio_characteristic_obj_t *self) {
154-
uint16_t conn_handle = common_hal_bleio_device_get_conn_handle(self->service->device);
155-
if (common_hal_bleio_service_get_is_remote(self->service)) {
156-
// self->value is set by evt handler.
157-
characteristic_gattc_read(self);
158-
} else {
159-
self->value = common_hal_bleio_gatts_read(self->handle, conn_handle);
161+
// Do GATT operations only if this characteristic has been added to a registered service.
162+
if (self->handle != BLE_GATT_HANDLE_INVALID) {
163+
uint16_t conn_handle = common_hal_bleio_device_get_conn_handle(self->service->device);
164+
if (common_hal_bleio_service_get_is_remote(self->service)) {
165+
// self->value is set by evt handler.
166+
characteristic_gattc_read(self);
167+
} else {
168+
self->value = common_hal_bleio_gatts_read(self->handle, conn_handle);
169+
}
160170
}
161171

162172
return self->value;
163173
}
164174

165175
void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self, mp_buffer_info_t *bufinfo) {
166-
uint16_t conn_handle = common_hal_bleio_device_get_conn_handle(self->service->device);
167-
168-
if (common_hal_bleio_service_get_is_remote(self->service)) {
169-
// Last argument is true if write-no-reponse desired.
170-
common_hal_bleio_gattc_write(self->handle, conn_handle, bufinfo,
171-
(self->props & CHAR_PROP_WRITE_NO_RESPONSE));
172-
} else {
173-
bool sent = false;
174-
uint16_t cccd = 0;
175-
176-
const bool notify = self->props & CHAR_PROP_NOTIFY;
177-
const bool indicate = self->props & CHAR_PROP_INDICATE;
178-
if (notify | indicate) {
179-
cccd = characteristic_get_cccd(self->cccd_handle, conn_handle);
180-
}
176+
// Do GATT operations only if this characteristic has been added to a registered service.
177+
if (self->handle != BLE_GATT_HANDLE_INVALID) {
178+
uint16_t conn_handle = common_hal_bleio_device_get_conn_handle(self->service->device);
179+
180+
if (common_hal_bleio_service_get_is_remote(self->service)) {
181+
// Last argument is true if write-no-reponse desired.
182+
common_hal_bleio_gattc_write(self->handle, conn_handle, bufinfo,
183+
(self->props & CHAR_PROP_WRITE_NO_RESPONSE));
184+
} else {
185+
if (self->fixed_length && bufinfo->len != self->max_length) {
186+
mp_raise_ValueError(translate("Value length required fixed length"));
187+
}
188+
if (bufinfo->len > self->max_length) {
189+
mp_raise_ValueError(translate("Value length > max_length"));
190+
}
181191

182-
// It's possible that both notify and indicate are set.
183-
if (notify && (cccd & BLE_GATT_HVX_NOTIFICATION)) {
184-
characteristic_gatts_notify_indicate(self->handle, conn_handle, bufinfo, BLE_GATT_HVX_NOTIFICATION);
185-
sent = true;
186-
}
187-
if (indicate && (cccd & BLE_GATT_HVX_INDICATION)) {
188-
characteristic_gatts_notify_indicate(self->handle, conn_handle, bufinfo, BLE_GATT_HVX_INDICATION);
189-
sent = true;
190-
}
192+
bool sent = false;
193+
uint16_t cccd = 0;
194+
195+
const bool notify = self->props & CHAR_PROP_NOTIFY;
196+
const bool indicate = self->props & CHAR_PROP_INDICATE;
197+
if (notify | indicate) {
198+
cccd = characteristic_get_cccd(self->cccd_handle, conn_handle);
199+
}
200+
201+
// It's possible that both notify and indicate are set.
202+
if (notify && (cccd & BLE_GATT_HVX_NOTIFICATION)) {
203+
characteristic_gatts_notify_indicate(self->handle, conn_handle, bufinfo, BLE_GATT_HVX_NOTIFICATION);
204+
sent = true;
205+
}
206+
if (indicate && (cccd & BLE_GATT_HVX_INDICATION)) {
207+
characteristic_gatts_notify_indicate(self->handle, conn_handle, bufinfo, BLE_GATT_HVX_INDICATION);
208+
sent = true;
209+
}
191210

192-
if (!sent) {
193-
common_hal_bleio_gatts_write(self->handle, conn_handle, bufinfo);
211+
if (!sent) {
212+
common_hal_bleio_gatts_write(self->handle, conn_handle, bufinfo);
213+
}
194214
}
195215
}
216+
217+
self->value = mp_obj_new_bytes(bufinfo->buf, bufinfo->len);
196218
}
197219

198220
bleio_uuid_obj_t *common_hal_bleio_characteristic_get_uuid(bleio_characteristic_obj_t *self) {

‎ports/nrf/common-hal/bleio/Characteristic.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ typedef struct {
3939
bleio_service_obj_t *service;
4040
bleio_uuid_obj_t *uuid;
4141
mp_obj_t value;
42+
uint16_t max_length;
43+
bool fixed_length;
4244
uint16_t handle;
4345
bleio_characteristic_properties_t props;
4446
bleio_attribute_security_mode_t read_perm;

‎ports/nrf/common-hal/bleio/Descriptor.c

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,20 +35,28 @@
3535

3636
static volatile bleio_descriptor_obj_t *m_read_descriptor;
3737

38-
void common_hal_bleio_descriptor_construct(bleio_descriptor_obj_t *self, bleio_uuid_obj_t *uuid, bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm) {
38+
void common_hal_bleio_descriptor_construct(bleio_descriptor_obj_t *self, bleio_uuid_obj_t *uuid, bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm, mp_int_t max_length, bool fixed_length) {
3939
self->characteristic = MP_OBJ_NULL;
4040
self->uuid = uuid;
41-
self->value = mp_const_none;
42-
self->handle = BLE_CONN_HANDLE_INVALID;
41+
self->value = mp_const_empty_bytes;
42+
self->handle = BLE_GATT_HANDLE_INVALID;
4343
self->read_perm = read_perm;
4444
self->write_perm = write_perm;
45+
46+
const mp_int_t max_length_max = fixed_length ? BLE_GATTS_FIX_ATTR_LEN_MAX : BLE_GATTS_VAR_ATTR_LEN_MAX;
47+
if (max_length < 0 || max_length > max_length_max) {
48+
mp_raise_ValueError_varg(translate("max_length must be 0-%d when fixed_length is %s"),
49+
max_length_max, fixed_length ? "True" : "False");
50+
}
51+
self->max_length = max_length;
52+
self->fixed_length = fixed_length;
4553
}
4654

4755
bleio_uuid_obj_t *common_hal_bleio_descriptor_get_uuid(bleio_descriptor_obj_t *self) {
4856
return self->uuid;
4957
}
5058

51-
mp_obj_t common_hal_bleio_descriptor_get_characteristic(bleio_descriptor_obj_t *self) {
59+
bleio_characteristic_obj_t *common_hal_bleio_descriptor_get_characteristic(bleio_descriptor_obj_t *self) {
5260
return self->characteristic;
5361
}
5462

@@ -97,22 +105,37 @@ STATIC void descriptor_gattc_read(bleio_descriptor_obj_t *descriptor) {
97105
}
98106

99107
mp_obj_t common_hal_bleio_descriptor_get_value(bleio_descriptor_obj_t *self) {
100-
if (common_hal_bleio_service_get_is_remote(self->characteristic->service)) {
101-
descriptor_gattc_read(self);
102-
} else {
103-
self->value = common_hal_bleio_gatts_read(
104-
self->handle, common_hal_bleio_device_get_conn_handle(self->characteristic->service->device));
108+
// Do GATT operations only if this descriptor has been registered
109+
if (self->handle != BLE_GATT_HANDLE_INVALID) {
110+
if (common_hal_bleio_service_get_is_remote(self->characteristic->service)) {
111+
descriptor_gattc_read(self);
112+
} else {
113+
self->value = common_hal_bleio_gatts_read(
114+
self->handle, common_hal_bleio_device_get_conn_handle(self->characteristic->service->device));
115+
}
105116
}
106117

107118
return self->value;
108119
}
109120

110121
void common_hal_bleio_descriptor_set_value(bleio_descriptor_obj_t *self, mp_buffer_info_t *bufinfo) {
111-
uint16_t conn_handle = common_hal_bleio_device_get_conn_handle(self->characteristic->service->device);
112-
if (common_hal_bleio_service_get_is_remote(self->characteristic->service)) {
113-
// false means WRITE_REQ, not write-no-response
114-
common_hal_bleio_gattc_write(self->handle, conn_handle, bufinfo, false);
115-
} else {
116-
common_hal_bleio_gatts_write(self->handle, conn_handle, bufinfo);
122+
// Do GATT operations only if this descriptor has been registered.
123+
if (self->handle != BLE_GATT_HANDLE_INVALID) {
124+
uint16_t conn_handle = common_hal_bleio_device_get_conn_handle(self->characteristic->service->device);
125+
if (common_hal_bleio_service_get_is_remote(self->characteristic->service)) {
126+
// false means WRITE_REQ, not write-no-response
127+
common_hal_bleio_gattc_write(self->handle, conn_handle, bufinfo, false);
128+
} else {
129+
if (self->fixed_length && bufinfo->len != self->max_length) {
130+
mp_raise_ValueError(translate("Value length != required fixed length"));
131+
}
132+
if (bufinfo->len > self->max_length) {
133+
mp_raise_ValueError(translate("Value length > max_length"));
134+
}
135+
136+
common_hal_bleio_gatts_write(self->handle, conn_handle, bufinfo);
137+
}
117138
}
139+
140+
self->value = mp_obj_new_bytes(bufinfo->buf, bufinfo->len);
118141
}

‎ports/nrf/common-hal/bleio/Descriptor.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ typedef struct {
4040
bleio_characteristic_obj_t *characteristic;
4141
bleio_uuid_obj_t *uuid;
4242
mp_obj_t value;
43+
uint16_t max_length;
44+
bool fixed_length;
4345
uint16_t handle;
4446
bleio_attribute_security_mode_t read_perm;
4547
bleio_attribute_security_mode_t write_perm;

‎ports/nrf/common-hal/bleio/Peripheral.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,21 +149,18 @@ STATIC void peripheral_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
149149

150150
case BLE_GAP_EVT_CONN_SEC_UPDATE: {
151151
ble_gap_conn_sec_t* conn_sec = &ble_evt->evt.gap_evt.params.conn_sec_update.conn_sec;
152-
mp_printf(&mp_plat_print, "sm: %d, lv: %d\n", conn_sec->sec_mode.sm, conn_sec->sec_mode.lv);
153152
if (conn_sec->sec_mode.sm <= 1 && conn_sec->sec_mode.lv <= 1) {
154153
// Security setup did not succeed:
155154
// mode 0, level 0 means no access
156155
// mode 1, level 1 means open link
157156
// mode >=1 and/or level >=1 means encryption is set up
158157
self->pair_status = PAIR_NOT_PAIRED;
159-
mp_printf(&mp_plat_print, "PAIR_NOT_PAIRED\n");
160158
} else {
161159
// TODO: see Bluefruit lib
162160
// if ( !bond_load_cccd(_role, _conn_hdl, _ediv) ) {
163161
// sd_ble_gatts_sys_attr_set(_conn_hdl, NULL, 0, 0);
164162
// }
165163
self->pair_status = PAIR_PAIRED;
166-
mp_printf(&mp_plat_print, "PAIR_PAIRED\n");
167164
}
168165
break;
169166
}

‎ports/nrf/common-hal/bleio/Service.c

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ void common_hal_bleio_service_add_all_characteristics(bleio_service_obj_t *self)
7272
bleio_characteristic_obj_t *characteristic =
7373
MP_OBJ_TO_PTR(self->characteristic_list->items[characteristic_idx]);
7474

75+
if (characteristic->handle != BLE_GATT_HANDLE_INVALID) {
76+
mp_raise_ValueError(translate("Characteristic already in use by another Service."));
77+
}
78+
7579
ble_gatts_char_md_t char_md = {
7680
.char_props.broadcast = (characteristic->props & CHAR_PROP_BROADCAST) ? 1 : 0,
7781
.char_props.read = (characteristic->props & CHAR_PROP_READ) ? 1 : 0,
@@ -97,17 +101,22 @@ void common_hal_bleio_service_add_all_characteristics(bleio_service_obj_t *self)
97101

98102
ble_gatts_attr_md_t char_attr_md = {
99103
.vloc = BLE_GATTS_VLOC_STACK,
100-
.vlen = 1,
104+
.vlen = !characteristic->fixed_length,
101105
};
102106

103107
BLE_GAP_CONN_SEC_MODE_SET_OPEN(&char_attr_md.read_perm);
104108
BLE_GAP_CONN_SEC_MODE_SET_OPEN(&char_attr_md.write_perm);
105109

110+
mp_buffer_info_t char_value_bufinfo;
111+
mp_get_buffer_raise(characteristic->value, &char_value_bufinfo, MP_BUFFER_READ);
112+
106113
ble_gatts_attr_t char_attr = {
107114
.p_uuid = &char_uuid,
108115
.p_attr_md = &char_attr_md,
109-
.init_len = sizeof(uint8_t),
110-
.max_len = GATT_MAX_DATA_LENGTH,
116+
.init_len = char_value_bufinfo.len,
117+
.p_value = char_value_bufinfo.buf,
118+
.init_offs = 0,
119+
.max_len = characteristic->max_length,
111120
};
112121

113122
ble_gatts_char_handles_t char_handles;
@@ -118,10 +127,6 @@ void common_hal_bleio_service_add_all_characteristics(bleio_service_obj_t *self)
118127
mp_raise_OSError_msg_varg(translate("Failed to add characteristic, err 0x%04x"), err_code);
119128
}
120129

121-
if (characteristic->handle != BLE_GATT_HANDLE_INVALID) {
122-
mp_raise_ValueError(translate("Characteristic already in use by another Service."));
123-
}
124-
125130
characteristic->user_desc_handle = char_handles.user_desc_handle;
126131
characteristic->cccd_handle = char_handles.cccd_handle;
127132
characteristic->sccd_handle = char_handles.sccd_handle;
@@ -138,25 +143,27 @@ void common_hal_bleio_service_add_all_characteristics(bleio_service_obj_t *self)
138143
ble_gatts_attr_md_t desc_attr_md = {
139144
// Data passed is not in a permanent location and should be copied.
140145
.vloc = BLE_GATTS_VLOC_STACK,
141-
.vlen = 1,
146+
.vlen = !descriptor->fixed_length,
142147
};
143148

144149
BLE_GAP_CONN_SEC_MODE_SET_OPEN(&desc_attr_md.read_perm);
145150
BLE_GAP_CONN_SEC_MODE_SET_OPEN(&desc_attr_md.write_perm);
146151

147-
mp_buffer_info_t bufinfo;
148-
mp_get_buffer_raise(descriptor->value, &bufinfo, MP_BUFFER_READ);
152+
mp_buffer_info_t desc_value_bufinfo;
153+
mp_get_buffer_raise(descriptor->value, &desc_value_bufinfo, MP_BUFFER_READ);
149154

150155
ble_gatts_attr_t desc_attr = {
151156
.p_uuid = &desc_uuid,
152157
.p_attr_md = &desc_attr_md,
153-
.init_len = bufinfo.len,
154-
.p_value = bufinfo.buf,
158+
.init_len = desc_value_bufinfo.len,
159+
.p_value = desc_value_bufinfo.buf,
155160
.init_offs = 0,
156-
.max_len = GATT_MAX_DATA_LENGTH,
161+
.max_len = descriptor->max_length,
157162
};
158163

159164
err_code = sd_ble_gatts_descriptor_add(characteristic->handle, &desc_attr, &descriptor->handle);
160-
}
161-
}
165+
166+
} // loop over descriptors
167+
168+
} // loop over characteristics
162169
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,9 @@ STATIC void on_char_discovery_rsp(ble_gattc_evt_char_disc_rsp_t *response, mp_ob
218218

219219
// Call common_hal_bleio_characteristic_construct() to initalize some fields and set up evt handler.
220220
common_hal_bleio_characteristic_construct(
221-
characteristic, uuid, props, SEC_MODE_OPEN, SEC_MODE_OPEN, mp_obj_new_list(0, NULL));
221+
characteristic, uuid, props, SEC_MODE_OPEN, SEC_MODE_OPEN,
222+
GATT_MAX_DATA_LENGTH, false, // max_length, fixed_length: values may not matter for gattc
223+
mp_obj_new_list(0, NULL));
222224
characteristic->handle = gattc_char->handle_value;
223225
characteristic->service = m_char_discovery_service;
224226

@@ -272,7 +274,8 @@ STATIC void on_desc_discovery_rsp(ble_gattc_evt_desc_disc_rsp_t *response, mp_ob
272274
// For now, just leave the UUID as NULL.
273275
}
274276

275-
common_hal_bleio_descriptor_construct(descriptor, uuid, SEC_MODE_OPEN, SEC_MODE_OPEN);
277+
common_hal_bleio_descriptor_construct(descriptor, uuid, SEC_MODE_OPEN, SEC_MODE_OPEN,
278+
GATT_MAX_DATA_LENGTH, false);
276279
descriptor->handle = gattc_desc->handle;
277280
descriptor->characteristic = m_desc_discovery_characteristic;
278281

0 commit comments

Comments
 (0)