Hi Luiz,
> On Wed, Nov 5, 2014 at 6:00 AM, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote:
> 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.
>
Sounds good to me. I'll probably end up doing this refactor anyway, so
don't worry about it.
>>> +}
>>> +
>>> 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
Cheers,
Arman
--
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
