Hi Marcel,
> I am not really convinced that doing all the encoding and decoding in such a way is a good idea. The low-level bt_att_send should not really care about the PDU payload or anything. It should just now what are the expected result PDUs and nothing else. I think we should stick to the ATT low-level protocol.
>
I thought the whole point of having opcodes + param structures was so
that they could be passed down to bt_att_send? Somebody needs to
encode them into the PDU and if bt_att_send will accept structures as
they are currently defined in src/shared/att-types.h, then this needs
to be done here. I'm fine with not doing it that way, so then
bt_att_send will accept the raw PDU as is and we do away with the
parameters structures? Let's decide this now.
> If you want to add extra helpers for using the ATT procedures like Find Information, then we better add actually proper helpers on top of bt_att_send (compared to internally hacked into it). I am thinking along the lines of bt_add_find_info() and then have parameters that make sense.
>
> However I am also fine in just making this part (the ATT procedures) part of our new GATT implementation.
>
I wanted to at least create an abstraction so that these low-level ATT
protocol operations are not hidden inside an upper layer and can be
used elsewhere (e.g. by unit tests or command-line tools) and to make
debugging easier. So then this is what I propose:
1. Change the bt_att_send signature: bt_att_send(struct bt_att *att,
void *pdu, uint16_t len, ...); No opcode is passed as it's contained
in the first byte of the PDU.
2. Change the misnamed bt_att_request_func_t to: typedef
(*bt_att_pdu_func_t)(const void *pdu, uint16_t len, void *user_data),
to be invoked upon a response to a request and for all incoming PDUs.
3. Get rid of all the parameters structures in
src/shared/att-types.h. Replace them with PDU encode/decode helpers a
la attrib/att.h.
So then one would do something like:
uint8_t pdu[5];
uint16_t start = ...;
uint16_t end = ...;
bt_att_encode_find_info_req(start, end, pdu);
bt_att_send(att, pdu, sizeof(pdu), find_info_cb, NULL, NULL);
And then for incoming PDUs you would have a bt_att_decode_... helper
for each operation.
-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
