Hi Arman,
On Tue, Nov 4, 2014 at 9:25 PM, Arman Uguray <armansito@xxxxxxxxxxxx> wrote:
>> +static void attribute_read_cb(struct gatt_db_attribute *attrib, int err,
>> + const uint8_t *value, size_t length,
>> + void *user_data)
>> +{
>> + struct iovec *iov = user_data;
>> +
>> + iov->iov_base = (void *) value;
>> + iov->iov_len = length;
>
> I guess now we're depending on the fact that this code will execute
> before gatt_db_attribute_read returns below. I would have this
> function drive the state machine instead, since we wouldn't want this
> code to break in case we ever decide to execute this via an idle
> callback in gatt-db.
This one we will probably have to figure out later otherwise I will
have to change quite a lot in read_by_grp_type_cb since it will
probably not be possible to respond directly, instead we need to wait
the reads to complete, I really want to avoid unnecessary changes
because this patch will be quite big.
>> +}
>> +
>> static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
>> uint16_t mtu,
>> uint8_t *pdu, uint16_t *len)
>> {
>> int iter = 0;
>> uint16_t start_handle, end_handle;
>> - uint8_t *value;
>> - int value_len;
>> + struct iovec value;
>> uint8_t data_val_len;
>>
>> *len = 0;
>>
>> while (queue_peek_head(q)) {
>> - start_handle = PTR_TO_UINT(queue_pop_head(q));
>> - value = NULL;
>> - value_len = 0;
>> + struct gatt_db_attribute *attrib = queue_pop_head(q);
>> +
>> + value.iov_base = NULL;
>> + value.iov_len = 0;
>>
>> /*
>> * This should never be deferred to the read callback for
>> * primary/secondary service declarations.
>> */
>> - if (!gatt_db_read(db, start_handle, 0,
>> + if (!gatt_db_attribute_read(attrib, 0,
>> BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
>> - NULL, &value,
>> - &value_len) || value_len < 0)
>> + NULL, attribute_read_cb,
>> + &value) || !value.iov_len)
>> return false;
>>
>> /*
>> @@ -124,21 +136,22 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
>> * value is seen.
>> */
>> if (iter == 0) {
>> - data_val_len = value_len;
>> + data_val_len = value.iov_len;
>> pdu[0] = data_val_len + 4;
>> iter++;
>> - } else if (value_len != data_val_len)
>> + } else if (value.iov_len != data_val_len)
>> break;
>>
>> /* Stop if this unit would surpass the MTU */
>> if (iter + data_val_len + 4 > mtu)
>> break;
>>
>> - end_handle = gatt_db_get_end_handle(db, start_handle);
>> + gatt_db_attribute_get_service_handles(attrib, &start_handle,
>> + &end_handle);
>>
>> put_le16(start_handle, pdu + iter);
>> put_le16(end_handle, pdu + iter + 2);
>> - memcpy(pdu + iter + 4, value, value_len);
>> + memcpy(pdu + iter + 4, value.iov_base, value.iov_len);
>>
>> iter += data_val_len + 4;
>> }
>> --
>> 1.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Apart from the few comments that I left above, I'm fine with this. Let
> me know when you've decided to go ahead with it and I'll start
> rebasing my work on top of your patch right away since I don't depend
> on the android/gatt changes that you'll have to make.
>
> Cheers,
> Arman
--
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
