[PATCH 00/10] shared/att: Implement sending ATT protocol requests.

[PATCH 00/10] shared/att: Implement sending ATT protocol requests.

[Arman Uguray]

This patch set adds support for sending all remaining ATT protocol requests
via bt_att_send and handling of the corresponding response PDUs.

Implemented are:
  - All outgoing ATT protocol requests.
  - All incoming ATT protocol responses.

NOT implemented (and on the TODO list) are:
  - Outgoing ATT protocol commands.
  - Outgoing signed PDUs.
  - Outgoing notifications/indications.
  - Handling of incoming notifications/indications and outgoing confirmation.
  - Handling of incoming ATT protocol requests and outgoing responses.

Arman Uguray (10):
  shared/att: Implement outgoing "Find Information" request/response.
  shared/att: Implement outgoing "Find By Type Value" request/response.
  shared/att: Implement outgoing "Read By Type" request/response.
  shared/att: Implement outgoing "Read" request/response.
  shared/att: Implement outgoing "Read Blob" request/response.
  shared/att: Implement outgoing "Read Multiple" request/response.
  shared/att: Implement outgoing "Read By Group Type" request/response.
  shared/att: Implement outgoing "Write" request/response.
  shared/att: Implement outgoing "Prepare Write" request/response.
  shared/att: Implement outgoing "Execute Write" request/response.

 src/shared/att-types.h |  16 +-
 src/shared/att.c       | 551 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 559 insertions(+), 8 deletions(-)

-- 
2.0.0.526.g5318336

[PATCH 01/10] shared/att: Implement outgoing "Find Information" request/response.

[Arman Uguray]

This patch add support for "Find Information" requests sent via
bt_att_send and the corresponding response.
---
 src/shared/att.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index 57f887e..22dd471 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -165,6 +165,30 @@ static bool encode_mtu_req(struct att_send_op *op, const void *param,
 	return true;
 }
 
+static bool encode_find_info_req(struct att_send_op *op, const void *param,
+						uint16_t length, uint16_t mtu)
+{
+	const struct bt_att_find_info_req_param *p = param;
+	const uint16_t len = 5;
+
+	if (length != sizeof(*p))
+		return false;
+
+	if (len > mtu)
+		return false;
+
+	op->pdu = malloc(len);
+	if (!op->pdu)
+		return false;
+
+	((uint8_t *) op->pdu)[0] = op->opcode;
+	put_le16(p->start_handle, ((uint8_t *) op->pdu) + 1);
+	put_le16(p->end_handle, ((uint8_t *) op->pdu) + 3);
+	op->len = len;
+
+	return true;
+}
+
 static bool encode_pdu(struct att_send_op *op, const void *param,
 						uint16_t length, uint16_t mtu)
 {
@@ -191,6 +215,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
 	switch (op->opcode) {
 	case BT_ATT_OP_MTU_REQ:
 		return encode_mtu_req(op, param, length, mtu);
+	case BT_ATT_OP_FIND_INFO_REQ:
+		return encode_find_info_req(op, param, length, mtu);
 	default:
 		break;
 	}
@@ -471,6 +497,29 @@ static bool handle_mtu_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 							&param, sizeof(param));
 }
 
+static bool handle_find_info_rsp(struct bt_att *att, uint8_t opcode,
+						uint8_t *pdu, ssize_t pdu_len)
+{
+	struct bt_att_find_info_rsp_param param;
+
+	/* PDU must have the following data:
+	 * - 1 octet: ATT opcode
+	 * - 1 octet: Format
+	 * - 4 to MTU-2 octetsS; Information data
+	 */
+	if (pdu_len < 6)
+		return false;
+
+	memset(&param, 0, sizeof(param));
+
+	param.format = pdu[1];
+	param.length = pdu_len - 2;
+	param.info_data = pdu + 2;
+
+	return request_complete(att, BT_ATT_OP_FIND_INFO_REQ, opcode,
+							&param, sizeof(param));
+}
+
 static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								ssize_t pdu_len)
 {
@@ -483,6 +532,9 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	case BT_ATT_OP_MTU_RSP:
 		success = handle_mtu_rsp(att, opcode, pdu, pdu_len);
 		break;
+	case BT_ATT_OP_FIND_INFO_RSP:
+		success = handle_find_info_rsp(att, opcode, pdu, pdu_len);
+		break;
 	default:
 		success = false;
 		util_debug(att->debug_callback, att->debug_data,
-- 
2.0.0.526.g5318336

Re: [PATCH 01/10] shared/att: Implement outgoing "Find Information" request/response.

[Marcel Holtmann]

Hi Arman,

> This patch add support for "Find Information" requests sent via
> bt_att_send and the corresponding response.
> ---
> src/shared/att.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
> 
> diff --git a/src/shared/att.c b/src/shared/att.c
> index 57f887e..22dd471 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -165,6 +165,30 @@ static bool encode_mtu_req(struct att_send_op *op, const void *param,
> 	return true;
> }
> 
> +static bool encode_find_info_req(struct att_send_op *op, const void *param,
> +						uint16_t length, uint16_t mtu)
> +{
> +	const struct bt_att_find_info_req_param *p = param;
> +	const uint16_t len = 5;
> +
> +	if (length != sizeof(*p))
> +		return false;
> +
> +	if (len > mtu)
> +		return false;
> +
> +	op->pdu = malloc(len);
> +	if (!op->pdu)
> +		return false;
> +
> +	((uint8_t *) op->pdu)[0] = op->opcode;
> +	put_le16(p->start_handle, ((uint8_t *) op->pdu) + 1);
> +	put_le16(p->end_handle, ((uint8_t *) op->pdu) + 3);

this ptr + x operation is actually defined on void pointers. At least with gcc. The casting should not be needed.

> +	op->len = len;
> +
> +	return true;
> +}

> +
> static bool encode_pdu(struct att_send_op *op, const void *param,
> 						uint16_t length, uint16_t mtu)
> {
> @@ -191,6 +215,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
> 	switch (op->opcode) {
> 	case BT_ATT_OP_MTU_REQ:
> 		return encode_mtu_req(op, param, length, mtu);
> +	case BT_ATT_OP_FIND_INFO_REQ:
> +		return encode_find_info_req(op, param, length, mtu);
> 	default:
> 		break;
> 	}
> @@ -471,6 +497,29 @@ static bool handle_mtu_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
> 							&param, sizeof(param));
> }
> 
> +static bool handle_find_info_rsp(struct bt_att *att, uint8_t opcode,
> +						uint8_t *pdu, ssize_t pdu_len)
> +{
> +	struct bt_att_find_info_rsp_param param;
> +
> +	/* PDU must have the following data:
> +	 * - 1 octet: ATT opcode
> +	 * - 1 octet: Format
> +	 * - 4 to MTU-2 octetsS; Information data
> +	 */
> +	if (pdu_len < 6)
> +		return false;
> +
> +	memset(&param, 0, sizeof(param));
> +
> +	param.format = pdu[1];
> +	param.length = pdu_len - 2;
> +	param.info_data = pdu + 2;
> +
> +	return request_complete(att, BT_ATT_OP_FIND_INFO_REQ, opcode,
> +							&param, sizeof(param));
> +}
> +
> static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
> 								ssize_t pdu_len)
> {
> @@ -483,6 +532,9 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
> 	case BT_ATT_OP_MTU_RSP:
> 		success = handle_mtu_rsp(att, opcode, pdu, pdu_len);
> 		break;
> +	case BT_ATT_OP_FIND_INFO_RSP:
> +		success = handle_find_info_rsp(att, opcode, pdu, pdu_len);
> +		break;

Help me out here on why are we doing this in this layer.

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.

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. That is how I would have done it. When working on mgmt there was no need to try to add more logic than needed to it. Same as when we added QMI support to oFono. The biggest help was to have a simple asynchronous non-blocking implementation of the low-level transport. Everything else was easy to do in the higher layers.

Have you looked at the actually GATT defined procedure and just try to implement them using bt_att_send and PDU payload building and parsing in the higher layer.

Regards

Marcel

Re: [PATCH 01/10] shared/att: Implement outgoing "Find Information" request/response.

[Szymon Janc]

Hi,

On Monday 07 of July 2014 09:20:59 Marcel Holtmann wrote:
> Hi Arman,
> 
> > This patch add support for "Find Information" requests sent via
> > bt_att_send and the corresponding response.
> > ---
> > src/shared/att.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 52 insertions(+)
> > 
> > diff --git a/src/shared/att.c b/src/shared/att.c
> > index 57f887e..22dd471 100644
> > --- a/src/shared/att.c
> > +++ b/src/shared/att.c
> > @@ -165,6 +165,30 @@ static bool encode_mtu_req(struct att_send_op *op, const void *param,
> > 	return true;
> > }
> > 
> > +static bool encode_find_info_req(struct att_send_op *op, const void *param,
> > +						uint16_t length, uint16_t mtu)
> > +{
> > +	const struct bt_att_find_info_req_param *p = param;
> > +	const uint16_t len = 5;
> > +
> > +	if (length != sizeof(*p))
> > +		return false;
> > +
> > +	if (len > mtu)
> > +		return false;
> > +
> > +	op->pdu = malloc(len);
> > +	if (!op->pdu)
> > +		return false;
> > +
> > +	((uint8_t *) op->pdu)[0] = op->opcode;
> > +	put_le16(p->start_handle, ((uint8_t *) op->pdu) + 1);
> > +	put_le16(p->end_handle, ((uint8_t *) op->pdu) + 3);
> 
> this ptr + x operation is actually defined on void pointers. At least with gcc. The casting should not be needed.

And why not just have pdu defined as uint8_t * in struct att_send_op?
>From what I see in code (and this patchset) it is always used as array of
bytes, not a data pointer.

> 
> > +	op->len = len;
> > +
> > +	return true;
> > +}
> 
> > +
> > static bool encode_pdu(struct att_send_op *op, const void *param,
> > 						uint16_t length, uint16_t mtu)
> > {
> > @@ -191,6 +215,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
> > 	switch (op->opcode) {
> > 	case BT_ATT_OP_MTU_REQ:
> > 		return encode_mtu_req(op, param, length, mtu);
> > +	case BT_ATT_OP_FIND_INFO_REQ:
> > +		return encode_find_info_req(op, param, length, mtu);
> > 	default:
> > 		break;
> > 	}
> > @@ -471,6 +497,29 @@ static bool handle_mtu_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
> > 							&param, sizeof(param));
> > }
> > 
> > +static bool handle_find_info_rsp(struct bt_att *att, uint8_t opcode,
> > +						uint8_t *pdu, ssize_t pdu_len)
> > +{
> > +	struct bt_att_find_info_rsp_param param;
> > +
> > +	/* PDU must have the following data:
> > +	 * - 1 octet: ATT opcode
> > +	 * - 1 octet: Format
> > +	 * - 4 to MTU-2 octetsS; Information data
> > +	 */
> > +	if (pdu_len < 6)
> > +		return false;
> > +
> > +	memset(&param, 0, sizeof(param));
> > +
> > +	param.format = pdu[1];
> > +	param.length = pdu_len - 2;
> > +	param.info_data = pdu + 2;
> > +
> > +	return request_complete(att, BT_ATT_OP_FIND_INFO_REQ, opcode,
> > +							&param, sizeof(param));
> > +}
> > +
> > static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
> > 								ssize_t pdu_len)
> > {
> > @@ -483,6 +532,9 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
> > 	case BT_ATT_OP_MTU_RSP:
> > 		success = handle_mtu_rsp(att, opcode, pdu, pdu_len);
> > 		break;
> > +	case BT_ATT_OP_FIND_INFO_RSP:
> > +		success = handle_find_info_rsp(att, opcode, pdu, pdu_len);
> > +		break;
> 
> Help me out here on why are we doing this in this layer.
> 
> 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.
> 
> 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. That is how I would have done it. When working on mgmt there was no need to try to add more logic than needed to it. Same as when we added QMI support to oFono. The biggest help was to have a simple asynchronous non-blocking implementation of the low-level transport. Everything else was easy to do in the higher layers.
> 
> Have you looked at the actually GATT defined procedure and just try to implement them using bt_att_send and PDU payload building and parsing in the higher layer.
> 
> Regards
> 
> Marcel
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Best regards, 
Szymon Janc

Re: [PATCH 01/10] shared/att: Implement outgoing "Find Information" request/response.

[Marcel Holtmann]

Hi Szymon,

>>> This patch add support for "Find Information" requests sent via
>>> bt_att_send and the corresponding response.
>>> ---
>>> src/shared/att.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 52 insertions(+)
>>> 
>>> diff --git a/src/shared/att.c b/src/shared/att.c
>>> index 57f887e..22dd471 100644
>>> --- a/src/shared/att.c
>>> +++ b/src/shared/att.c
>>> @@ -165,6 +165,30 @@ static bool encode_mtu_req(struct att_send_op *op, const void *param,
>>> 	return true;
>>> }
>>> 
>>> +static bool encode_find_info_req(struct att_send_op *op, const void *param,
>>> +						uint16_t length, uint16_t mtu)
>>> +{
>>> +	const struct bt_att_find_info_req_param *p = param;
>>> +	const uint16_t len = 5;
>>> +
>>> +	if (length != sizeof(*p))
>>> +		return false;
>>> +
>>> +	if (len > mtu)
>>> +		return false;
>>> +
>>> +	op->pdu = malloc(len);
>>> +	if (!op->pdu)
>>> +		return false;
>>> +
>>> +	((uint8_t *) op->pdu)[0] = op->opcode;
>>> +	put_le16(p->start_handle, ((uint8_t *) op->pdu) + 1);
>>> +	put_le16(p->end_handle, ((uint8_t *) op->pdu) + 3);
>> 
>> this ptr + x operation is actually defined on void pointers. At least with gcc. The casting should not be needed.
> 
> And why not just have pdu defined as uint8_t * in struct att_send_op?

I want void pointers there. If you want an unint8_t * then cast it into a local pointer. Or cast it into a packed struct matching your payload.

> From what I see in code (and this patchset) it is always used as array of
> bytes, not a data pointer.

And over time it will change to using packed structs and the direct uin8_t access will go away step by step. So no need to overthink this.

Regards

Marcel

Re: [PATCH 01/10] shared/att: Implement outgoing "Find Information" request/response.

[Arman Uguray]

Hi Marcel,

> I am not really convinced that doing all the encoding and decoding in suc=
h a way is a good idea. The low-level bt_att_send should not really care ab=
out the PDU payload or anything. It should just now what are the expected r=
esult PDUs and nothing else. I think we should stick to the ATT low-level p=
rotocol.
>

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 I=
nformation, then we better add actually proper helpers on top of bt_att_sen=
d (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 =3D ...;
    uint16_t end =3D ...;
    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

Re: [PATCH 01/10] shared/att: Implement outgoing "Find Information" request/response.

[Marcel Holtmann]

Hi Arman,

>> 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.

lets use raw PDU for bt_att_send. The only thing it really needs to handle are the responses and errors that match to the opcode. And for bt_att_register handle the mapping confirmations correctly.

>> 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.

Keep it as opcode + payload. I really want the opcode in the signature.

>  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.

Not sure why? I want the difference between bt_att_send and bt_att_register.

>  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.

Such helpers seems to be at first glance a good idea, but the devil is in the detail. Some helpers might be a really good idea. Others might end up just being too complex and you end up in spaghetti code.

What I think might be useful that for simple and rather common ATT commands, we have a wrapper around bt_att_send. As mentioned earlier, bt_att_find_info might such a wrapper. It takes start and end parameters and calls a proper callback with all information available.

That said, it might be also better to have this all in GATT procedures. So that we get bt_gatt_discover_primary_services as such function and internally it uses many bt_att_send.

I am not a big fan of too many abstractions. They all look reasonable in the beginning, but later on they just make code more complicated for no apparent reason. I can name numerous examples where actually not having wrappers or extra abstraction created cleaner code for us.

So my advice would be to take one step back now. Look at what we really need in terms of GATT procedures and start with adding bt_gatt (src/share/gatt.c). Do all the encoding and decoding of ATT payloads in bt_gatt and then figure out where the pain points are. If you find common code that justifies a more generic bt_att helper, then move it to bt_att.

Regards

Marcel

Re: [PATCH 01/10] shared/att: Implement outgoing "Find Information" request/response.

[Arman Uguray]

Hi Marcel,

>>  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.
>
> Keep it as opcode + payload. I really want the opcode in the signature.
>

Sounds good to me. But we still expect the passed in pdu to contain
the opcode as the first byte right? Or are we saying to pass in the
opcode + the rest of the PDU separately?

>>  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.
>
> Not sure why? I want the difference between bt_att_send and bt_att_regist=
er.
>

No difference in the callback signatures if that's what you're asking.
My original intention was to have the (same type of) callback passed
to bt_att_send and bt_att_register take in a void pointer to a param
structure. Now the callback will just accept the raw PDU (or opcode +
the rest of the PDU in separate args). bt_att_request_func_t is a bit
misnamed (since it doesn't only apply to ATT protocol requests), so
changing that to bt_att_pdu_func_t. Also removing
bt_att_notify_func_t.

> Such helpers seems to be at first glance a good idea, but the devil is in=
 the detail. Some helpers might be a really good idea. Others might end up =
just being too complex and you end up in spaghetti code.
>
> What I think might be useful that for simple and rather common ATT comman=
ds, we have a wrapper around bt_att_send. As mentioned earlier, bt_att_find=
_info might such a wrapper. It takes start and end parameters and calls a p=
roper callback with all information available.
>
> That said, it might be also better to have this all in GATT procedures. S=
o that we get bt_gatt_discover_primary_services as such function and intern=
ally it uses many bt_att_send.
>
> I am not a big fan of too many abstractions. They all look reasonable in =
the beginning, but later on they just make code more complicated for no app=
arent reason. I can name numerous examples where actually not having wrappe=
rs or extra abstraction created cleaner code for us.
>
> So my advice would be to take one step back now. Look at what we really n=
eed in terms of GATT procedures and start with adding bt_gatt (src/share/ga=
tt.c). Do all the encoding and decoding of ATT payloads in bt_gatt and then=
 figure out where the pain points are. If you find common code that justifi=
es a more generic bt_att helper, then move it to bt_att.
>

Sounds good to me.

Re: [PATCH 01/10] shared/att: Implement outgoing "Find Information" request/response.

[Marcel Holtmann]

Hi Arman,

>>> 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.
>> 
>> Keep it as opcode + payload. I really want the opcode in the signature.
>> 
> 
> Sounds good to me. But we still expect the passed in pdu to contain
> the opcode as the first byte right? Or are we saying to pass in the
> opcode + the rest of the PDU separately?

I want opcode + payload. So the opcode is not part of the PDU we pass in.

>>> 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.
>> 
>> Not sure why? I want the difference between bt_att_send and bt_att_register.
>> 
> 
> No difference in the callback signatures if that's what you're asking.
> My original intention was to have the (same type of) callback passed
> to bt_att_send and bt_att_register take in a void pointer to a param
> structure. Now the callback will just accept the raw PDU (or opcode +
> the rest of the PDU in separate args). bt_att_request_func_t is a bit
> misnamed (since it doesn't only apply to ATT protocol requests), so
> changing that to bt_att_pdu_func_t. Also removing
> bt_att_notify_func_t.

Keep them separate for now. Even if they have the same signature. We can fix that later on if we see that it makes more sense.

Regards

Marcel

[PATCH 02/10] shared/att: Implement outgoing "Find By Type Value" request/response.

[Arman Uguray]

This patch adds support for "Find By Type Value" requests sent via
bt_att_send and the corresponding response.
---
 src/shared/att-types.h |  4 +--
 src/shared/att.c       | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index 636a5e3..b6f32dd 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -58,7 +58,7 @@ struct bt_att_find_info_rsp_param {
 
 /* Find By Type Value */
 #define BT_ATT_OP_FIND_BY_TYPE_VAL_REQ		0x06
-struct bt_att_find_by_type_value_req_param {
+struct bt_att_find_by_type_val_req_param {
 	uint16_t start_handle;
 	uint16_t end_handle;
 	uint16_t type;  /* 2 octet UUID */
@@ -67,7 +67,7 @@ struct bt_att_find_by_type_value_req_param {
 };
 
 #define BT_ATT_OP_FIND_BY_TYPE_VAL_RSP		0x07
-struct bt_att_find_by_type_value_rsp_param {
+struct bt_att_find_by_type_val_rsp_param {
 	const uint8_t *handles_info_list;
 	uint16_t length;
 };
diff --git a/src/shared/att.c b/src/shared/att.c
index 22dd471..9e16a6f 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -189,6 +189,40 @@ static bool encode_find_info_req(struct att_send_op *op, const void *param,
 	return true;
 }
 
+static bool encode_find_by_type_val_req(struct att_send_op *op,
+					const void *param, uint16_t length,
+					uint16_t mtu)
+{
+	const struct bt_att_find_by_type_val_req_param *p = param;
+	const uint16_t len = 7 + p->length;
+
+	if (length != sizeof(*p))
+		return false;
+
+	if (len > mtu)
+		return false;
+
+	op->pdu = malloc(len);
+	if (!op->pdu)
+		return false;
+
+	((uint8_t *) op->pdu)[0] = op->opcode;
+	put_le16(p->start_handle, ((uint8_t *) op->pdu) + 1);
+	put_le16(p->end_handle, ((uint8_t *) op->pdu) + 3);
+	put_le16(p->type, ((uint8_t *) op->pdu) + 5);
+	op->len = len;
+
+	if (p->length == 0)
+		return true;
+
+	if (!p->value)
+		return false;
+
+	memcpy(((uint8_t *) op->pdu) + 7, p->value, p->length);
+
+	return true;
+}
+
 static bool encode_pdu(struct att_send_op *op, const void *param,
 						uint16_t length, uint16_t mtu)
 {
@@ -217,6 +251,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
 		return encode_mtu_req(op, param, length, mtu);
 	case BT_ATT_OP_FIND_INFO_REQ:
 		return encode_find_info_req(op, param, length, mtu);
+	case BT_ATT_OP_FIND_BY_TYPE_VAL_REQ:
+		return encode_find_by_type_val_req(op, param, length, mtu);
 	default:
 		break;
 	}
@@ -520,6 +556,33 @@ static bool handle_find_info_rsp(struct bt_att *att, uint8_t opcode,
 							&param, sizeof(param));
 }
 
+static bool handle_find_by_type_val_rsp(struct bt_att *att, uint8_t opcode,
+						uint8_t *pdu, ssize_t pdu_len)
+{
+	struct bt_att_find_by_type_val_rsp_param param;
+
+	/* PDU must contain at least the following:
+	 * - 1 octet: ATT opcode
+	 * - 4 to MTU-1 octets: Handles Information List
+	 */
+	if (pdu_len < 5)
+		return false;
+
+	/* Each Handle Information field is composed of 4 octets. Return error,
+	 * if the length of the field is not a multiple of 4.
+	 */
+	if ((pdu_len - 1) % 4)
+		return false;
+
+	memset(&param, 0, sizeof(param));
+
+	param.length = pdu_len -1;
+	param.handles_info_list = pdu + 1;
+
+	return request_complete(att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ, opcode,
+							&param, sizeof(param));
+}
+
 static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								ssize_t pdu_len)
 {
@@ -535,6 +598,10 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	case BT_ATT_OP_FIND_INFO_RSP:
 		success = handle_find_info_rsp(att, opcode, pdu, pdu_len);
 		break;
+	case BT_ATT_OP_FIND_BY_TYPE_VAL_RSP:
+		success = handle_find_by_type_val_rsp(att, opcode,
+								pdu, pdu_len);
+		break;
 	default:
 		success = false;
 		util_debug(att->debug_callback, att->debug_data,
-- 
2.0.0.526.g5318336

[PATCH 03/10] shared/att: Implement outgoing "Read By Type" request/response.

[Arman Uguray]

This patch adds support for "Read By Type" requests sent via bt_att_send
and the corresponding response.
---
 src/shared/att.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index 9e16a6f..abf1440 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -223,6 +223,44 @@ static bool encode_find_by_type_val_req(struct att_send_op *op,
 	return true;
 }
 
+static bool encode_read_by_type_req(struct att_send_op *op,
+					const void *param, uint16_t length,
+					uint16_t mtu)
+{
+	const struct bt_att_read_by_type_req_param *p = param;
+	uint8_t uuid[16];
+	uint8_t uuid_len;
+	uint16_t len;
+
+	if (length != sizeof(*p))
+		return false;
+
+	if (p->type.type == BT_UUID16) {
+		uuid_len = 2;
+		put_le16(p->type.value.u16, uuid);
+	} else if (p->type.type == BT_UUID128) {
+		uuid_len = 16;
+		bswap_128(&p->type.value.u128, uuid);
+	} else
+		return false;
+
+	len = 5 + uuid_len;
+	if (len > mtu)
+		return false;
+
+	op->pdu = malloc(len);
+	if (!op->pdu)
+		return false;
+
+	((uint8_t *) op->pdu)[0] = op->opcode;
+	put_le16(p->start_handle, ((uint8_t *) op->pdu) + 1);
+	put_le16(p->end_handle, ((uint8_t *) op->pdu) + 3);
+	memcpy(((uint8_t *) op->pdu) + 5, uuid, uuid_len);
+	op->len = len;
+
+	return true;
+}
+
 static bool encode_pdu(struct att_send_op *op, const void *param,
 						uint16_t length, uint16_t mtu)
 {
@@ -253,6 +291,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
 		return encode_find_info_req(op, param, length, mtu);
 	case BT_ATT_OP_FIND_BY_TYPE_VAL_REQ:
 		return encode_find_by_type_val_req(op, param, length, mtu);
+	case BT_ATT_OP_READ_BY_TYPE_REQ:
+		return encode_read_by_type_req(op, param, length, mtu);
 	default:
 		break;
 	}
@@ -583,6 +623,32 @@ static bool handle_find_by_type_val_rsp(struct bt_att *att, uint8_t opcode,
 							&param, sizeof(param));
 }
 
+static bool handle_read_by_type_rsp(struct bt_att *att, uint8_t opcode,
+						uint8_t *pdu, ssize_t pdu_len)
+{
+	struct bt_att_read_by_type_rsp_param param;
+
+	/* PDU must contain at least the following:
+	 * - 1 octet: ATT opcode
+	 * - 1 octet: The size of each Attribute handle-value pair
+	 * - 2 to MTU-2 octets: Attribute Data List
+	 */
+	if (pdu_len < 4)
+		return false;
+
+	memset(&param, 0, sizeof(param));
+
+	param.length = pdu[1];
+	param.attr_data_list = pdu + 2;
+	param.list_length = pdu_len - 2;
+
+	if (param.list_length % param.length)
+		return false;
+
+	return request_complete(att, BT_ATT_OP_READ_BY_TYPE_REQ, opcode,
+							&param, sizeof(param));
+}
+
 static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								ssize_t pdu_len)
 {
@@ -602,6 +668,9 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 		success = handle_find_by_type_val_rsp(att, opcode,
 								pdu, pdu_len);
 		break;
+	case BT_ATT_OP_READ_BY_TYPE_RSP:
+		success = handle_read_by_type_rsp(att, opcode, pdu, pdu_len);
+		break;
 	default:
 		success = false;
 		util_debug(att->debug_callback, att->debug_data,
-- 
2.0.0.526.g5318336

[PATCH 04/10] shared/att: Implement outgoing "Read" request/response.

[Arman Uguray]

This patch adds support for "Read" requests sent via bt_att_send and the
corresponding response.
---
 src/shared/att.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index abf1440..288442f 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -261,6 +261,30 @@ static bool encode_read_by_type_req(struct att_send_op *op,
 	return true;
 }
 
+static bool encode_read_req(struct att_send_op *op,
+					const void *param, uint16_t length,
+					uint16_t mtu)
+{
+	const struct bt_att_read_req_param *p = param;
+	uint16_t len = 3;
+
+	if (length != sizeof(*p))
+		return false;
+
+	if (len > mtu)
+		return false;
+
+	op->pdu = malloc(len);
+	if (!op->pdu)
+		return false;
+
+	((uint8_t *) op->pdu)[0] = op->opcode;
+	put_le16(p->handle, ((uint8_t *) op->pdu) + 1);
+	op->len = len;
+
+	return true;
+}
+
 static bool encode_pdu(struct att_send_op *op, const void *param,
 						uint16_t length, uint16_t mtu)
 {
@@ -293,6 +317,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
 		return encode_find_by_type_val_req(op, param, length, mtu);
 	case BT_ATT_OP_READ_BY_TYPE_REQ:
 		return encode_read_by_type_req(op, param, length, mtu);
+	case BT_ATT_OP_READ_REQ:
+		return encode_read_req(op, param, length, mtu);
 	default:
 		break;
 	}
@@ -649,6 +675,22 @@ static bool handle_read_by_type_rsp(struct bt_att *att, uint8_t opcode,
 							&param, sizeof(param));
 }
 
+static bool handle_read_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
+								ssize_t pdu_len)
+{
+	struct bt_att_read_rsp_param param;
+
+	memset(&param, 0, sizeof(param));
+
+	if (pdu_len > 1) {
+		param.length = pdu_len - 1;
+		param.value = pdu + 1;
+	}
+
+	return request_complete(att, BT_ATT_OP_READ_REQ, opcode, &param,
+								sizeof(param));
+}
+
 static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								ssize_t pdu_len)
 {
@@ -671,6 +713,9 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	case BT_ATT_OP_READ_BY_TYPE_RSP:
 		success = handle_read_by_type_rsp(att, opcode, pdu, pdu_len);
 		break;
+	case BT_ATT_OP_READ_RSP:
+		success = handle_read_rsp(att, opcode, pdu, pdu_len);
+		break;
 	default:
 		success = false;
 		util_debug(att->debug_callback, att->debug_data,
-- 
2.0.0.526.g5318336

[PATCH 05/10] shared/att: Implement outgoing "Read Blob" request/response.

[Arman Uguray]

This patch adds support for "Read Blob" requests sent via bt_att_send
and the corresponding response.
---
 src/shared/att.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index 288442f..1b6d568 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -285,6 +285,31 @@ static bool encode_read_req(struct att_send_op *op,
 	return true;
 }
 
+static bool encode_read_blob_req(struct att_send_op *op,
+					const void *param, uint16_t length,
+					uint16_t mtu)
+{
+	const struct bt_att_read_blob_req_param *p = param;
+	uint16_t len = 5;
+
+	if (length != sizeof(*p))
+		return false;
+
+	if (len > mtu)
+		return false;
+
+	op->pdu = malloc(len);
+	if (!op->pdu)
+		return false;
+
+	((uint8_t *) op->pdu)[0] = op->opcode;
+	put_le16(p->handle, ((uint8_t *) op->pdu) + 1);
+	put_le16(p->offset, ((uint8_t *) op->pdu) + 3);
+	op->len = len;
+
+	return true;
+}
+
 static bool encode_pdu(struct att_send_op *op, const void *param,
 						uint16_t length, uint16_t mtu)
 {
@@ -319,6 +344,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
 		return encode_read_by_type_req(op, param, length, mtu);
 	case BT_ATT_OP_READ_REQ:
 		return encode_read_req(op, param, length, mtu);
+	case BT_ATT_OP_READ_BLOB_REQ:
+		return encode_read_blob_req(op, param, length, mtu);
 	default:
 		break;
 	}
@@ -691,6 +718,21 @@ static bool handle_read_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								sizeof(param));
 }
 
+static bool handle_read_blob_rsp(struct bt_att *att, uint8_t opcode,
+						uint8_t *pdu, ssize_t pdu_len)
+{
+	struct bt_att_read_blob_rsp_param param;
+	memset(&param, 0, sizeof(param));
+
+	if (pdu_len > 1) {
+		param.length = pdu_len - 1;
+		param.part_value = pdu + 1;
+	}
+
+	return request_complete(att, BT_ATT_OP_READ_BLOB_REQ, opcode, &param,
+								sizeof(param));
+}
+
 static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								ssize_t pdu_len)
 {
@@ -716,6 +758,9 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	case BT_ATT_OP_READ_RSP:
 		success = handle_read_rsp(att, opcode, pdu, pdu_len);
 		break;
+	case BT_ATT_OP_READ_BLOB_RSP:
+		success = handle_read_blob_rsp(att, opcode, pdu, pdu_len);
+		break;
 	default:
 		success = false;
 		util_debug(att->debug_callback, att->debug_data,
-- 
2.0.0.526.g5318336

[PATCH 06/10] shared/att: Implement outgoing "Read Multiple" request/response.

[Arman Uguray]

This patch adds support for "Read Multiple" requests sent via
bt_att_send and the corresponding response.
---
 src/shared/att-types.h |  4 ++--
 src/shared/att.c       | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index b6f32dd..ae2dfb6 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -114,13 +114,13 @@ struct bt_att_read_blob_rsp_param {
 
 /* Read Multiple */
 #define BT_ATT_OP_READ_MULT_REQ			0x0e
-struct bt_att_read_multiple_req_param {
+struct bt_att_read_mult_req_param {
 	const uint16_t *handles;
 	uint16_t num_handles;
 };
 
 #define BT_ATT_OP_READ_MULT_RSP			0x0f
-struct bt_att_read_multiple_rsp_param {
+struct bt_att_read_mult_rsp_param {
 	const uint8_t *values;
 	uint16_t length;
 };
diff --git a/src/shared/att.c b/src/shared/att.c
index 1b6d568..2658969 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -310,6 +310,37 @@ static bool encode_read_blob_req(struct att_send_op *op,
 	return true;
 }
 
+static bool encode_read_mult_req(struct att_send_op *op,
+					const void *param, uint16_t length,
+					uint16_t mtu)
+{
+	const struct bt_att_read_mult_req_param *p = param;
+	uint16_t len;
+	int i;
+
+	if (length != sizeof(*p))
+		return false;
+
+	if (p->num_handles < 1)
+		return false;
+
+	len = 1 + 2 * p->num_handles;
+	if (len > mtu)
+		return false;
+
+	op->pdu = malloc(len);
+	if (!op->pdu)
+		return false;
+
+	((uint8_t *) op->pdu)[0] = op->opcode;
+	op->len = len;
+
+	for (i = 0; i < p->num_handles; i++)
+		put_le16(p->handles[i], ((uint8_t *) op->pdu) + 1 + 2 * i);
+
+	return true;
+}
+
 static bool encode_pdu(struct att_send_op *op, const void *param,
 						uint16_t length, uint16_t mtu)
 {
@@ -346,6 +377,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
 		return encode_read_req(op, param, length, mtu);
 	case BT_ATT_OP_READ_BLOB_REQ:
 		return encode_read_blob_req(op, param, length, mtu);
+	case BT_ATT_OP_READ_MULT_REQ:
+		return encode_read_mult_req(op, param, length, mtu);
 	default:
 		break;
 	}
@@ -733,6 +766,22 @@ static bool handle_read_blob_rsp(struct bt_att *att, uint8_t opcode,
 								sizeof(param));
 }
 
+static bool handle_read_mult_rsp(struct bt_att *att, uint8_t opcode,
+						uint8_t *pdu, ssize_t pdu_len)
+{
+	struct bt_att_read_mult_rsp_param param;
+
+	memset(&param, 0, sizeof(param));
+
+	if (pdu_len > 1) {
+		param.length = pdu_len - 1;
+		param.values = pdu + 1;
+	}
+
+	return request_complete(att, BT_ATT_OP_READ_MULT_REQ, opcode, &param,
+								sizeof(param));
+}
+
 static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								ssize_t pdu_len)
 {
@@ -761,6 +810,9 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	case BT_ATT_OP_READ_BLOB_RSP:
 		success = handle_read_blob_rsp(att, opcode, pdu, pdu_len);
 		break;
+	case BT_ATT_OP_READ_MULT_RSP:
+		success = handle_read_mult_rsp(att, opcode, pdu, pdu_len);
+		break;
 	default:
 		success = false;
 		util_debug(att->debug_callback, att->debug_data,
-- 
2.0.0.526.g5318336

[PATCH 07/10] shared/att: Implement outgoing "Read By Group Type" request/response.

[Arman Uguray]

This patch adds support for "Read By Group Type" requests sent via
bt_att_send and the corresponding response.
---
 src/shared/att-types.h |  4 +--
 src/shared/att.c       | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index ae2dfb6..4809716 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -127,14 +127,14 @@ struct bt_att_read_mult_rsp_param {
 
 /* Read By Group Type */
 #define BT_ATT_OP_READ_BY_GRP_TYPE_REQ		0x10
-struct bt_att_read_by_group_type_req_param {
+struct bt_att_read_by_grp_type_req_param {
 	uint16_t start_handle;
 	uint16_t end_handle;
 	bt_uuid_t type;
 };
 
 #define BT_ATT_OP_READ_BY_GRP_TYPE_RSP		0x11
-struct bt_att_read_by_group_type_rsp_param {
+struct bt_att_read_by_grp_type_rsp_param {
 	uint8_t length;
 	const uint8_t *attr_data_list;
 	uint16_t list_length;  /* Length of "attr_data_list" */
diff --git a/src/shared/att.c b/src/shared/att.c
index 2658969..3406b24 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -341,6 +341,44 @@ static bool encode_read_mult_req(struct att_send_op *op,
 	return true;
 }
 
+static bool encode_read_by_grp_type_req(struct att_send_op *op,
+					const void *param, uint16_t length,
+					uint16_t mtu)
+{
+	const struct bt_att_read_by_grp_type_req_param *p = param;
+	uint8_t uuid[16];
+	uint8_t uuid_len;
+	uint16_t len;
+
+	if (length != sizeof(*p))
+		return false;
+
+	if (p->type.type == BT_UUID16) {
+		uuid_len = 2;
+		put_le16(p->type.value.u16, uuid);
+	} else if (p->type.type == BT_UUID128) {
+		uuid_len = 16;
+		bswap_128(&p->type.value.u128, uuid);
+	} else
+		return false;
+
+	len = 5 + uuid_len;
+	if (len > mtu)
+		return false;
+
+	op->pdu = malloc(len);
+	if (!op->pdu)
+		return false;
+
+	((uint8_t *) op->pdu)[0] = op->opcode;
+	put_le16(p->start_handle, ((uint8_t *) op->pdu) + 1);
+	put_le16(p->end_handle, ((uint8_t *) op->pdu) + 3);
+	memcpy(((uint8_t *) op->pdu) + 5, uuid, uuid_len);
+	op->len = len;
+
+	return true;
+}
+
 static bool encode_pdu(struct att_send_op *op, const void *param,
 						uint16_t length, uint16_t mtu)
 {
@@ -379,6 +417,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
 		return encode_read_blob_req(op, param, length, mtu);
 	case BT_ATT_OP_READ_MULT_REQ:
 		return encode_read_mult_req(op, param, length, mtu);
+	case BT_ATT_OP_READ_BY_GRP_TYPE_REQ:
+		return encode_read_by_grp_type_req(op, param, length, mtu);
 	default:
 		break;
 	}
@@ -782,6 +822,32 @@ static bool handle_read_mult_rsp(struct bt_att *att, uint8_t opcode,
 								sizeof(param));
 }
 
+static bool handle_read_by_grp_type_rsp(struct bt_att *att, uint8_t opcode,
+						uint8_t *pdu, ssize_t pdu_len)
+{
+	struct bt_att_read_by_grp_type_rsp_param param;
+
+	/* PDU must contain at least the following:
+	 * - 1 octet: ATT opcode.
+	 * - 1 octet: Length
+	 * - 2 to MTU-2 octets: Attribute Data List
+	 */
+	if (pdu_len < 4)
+		return false;
+
+	memset(&param, 0, sizeof(param));
+
+	param.length = pdu[1];
+	param.attr_data_list = pdu + 2;
+	param.list_length = pdu_len - 2;
+
+	if (param.list_length % param.length)
+		return false;
+
+	return request_complete(att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ, opcode,
+							&param, sizeof(param));
+}
+
 static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								ssize_t pdu_len)
 {
@@ -813,6 +879,10 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	case BT_ATT_OP_READ_MULT_RSP:
 		success = handle_read_mult_rsp(att, opcode, pdu, pdu_len);
 		break;
+	case BT_ATT_OP_READ_BY_GRP_TYPE_RSP:
+		success = handle_read_by_grp_type_rsp(att, opcode,
+								pdu, pdu_len);
+		break;
 	default:
 		success = false;
 		util_debug(att->debug_callback, att->debug_data,
-- 
2.0.0.526.g5318336

[PATCH 08/10] shared/att: Implement outgoing "Write" request/response.

[Arman Uguray]

This patch adds support for "Write" requests sent via bt_att_send and
the corresponding response. The same encode helper will also be used for the
"Write" and "Signed Write" commands in the future.
---
 src/shared/att.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index 3406b24..caf7d35 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -379,6 +379,39 @@ static bool encode_read_by_grp_type_req(struct att_send_op *op,
 	return true;
 }
 
+static bool encode_write_pdu(struct att_send_op *op, const void *param,
+						uint16_t length, uint16_t mtu)
+{
+	const struct bt_att_write_param *p = param;
+	uint16_t len;
+
+	if (length != sizeof(*p))
+		return false;
+
+	len = 3 + p->length;
+	if (len > mtu)
+		return false;
+
+	op->pdu = malloc(len);
+	if (!op->pdu)
+		return false;
+
+	((uint8_t *) op->pdu)[0] = op->opcode;
+	put_le16(p->handle, ((uint8_t *) op->pdu) + 1);
+	op->len = len;
+
+	if (p->length) {
+		if (!p->value) {
+			free(op->pdu);
+			return false;
+		}
+
+		memcpy(((uint8_t *) op->pdu) + 3, p->value, p->length);
+	}
+
+	return true;
+}
+
 static bool encode_pdu(struct att_send_op *op, const void *param,
 						uint16_t length, uint16_t mtu)
 {
@@ -419,6 +452,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
 		return encode_read_mult_req(op, param, length, mtu);
 	case BT_ATT_OP_READ_BY_GRP_TYPE_REQ:
 		return encode_read_by_grp_type_req(op, param, length, mtu);
+	case BT_ATT_OP_WRITE_REQ:
+		return encode_write_pdu(op, param, length, mtu);
 	default:
 		break;
 	}
@@ -848,6 +883,15 @@ static bool handle_read_by_grp_type_rsp(struct bt_att *att, uint8_t opcode,
 							&param, sizeof(param));
 }
 
+static bool handle_write_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
+								ssize_t pdu_len)
+{
+	if (pdu_len > 1)
+		return false;
+
+	return request_complete(att, BT_ATT_OP_WRITE_REQ, opcode, NULL, 0);
+}
+
 static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								ssize_t pdu_len)
 {
@@ -883,6 +927,9 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 		success = handle_read_by_grp_type_rsp(att, opcode,
 								pdu, pdu_len);
 		break;
+	case BT_ATT_OP_WRITE_RSP:
+		success = handle_write_rsp(att, opcode, pdu, pdu_len);
+		break;
 	default:
 		success = false;
 		util_debug(att->debug_callback, att->debug_data,
-- 
2.0.0.526.g5318336

[PATCH 09/10] shared/att: Implement outgoing "Prepare Write" request/response.

[Arman Uguray]

This patch adds support for "Prepare Write" requests sent via
bt_att_send and the corresponding response.
---
 src/shared/att-types.h |  4 +--
 src/shared/att.c       | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index 4809716..7dc22ad 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -162,7 +162,7 @@ struct bt_att_write_param {
 
 /* Prepare Write */
 #define BT_ATT_OP_PREP_WRITE_REQ		0x16
-struct bt_att_prepare_write_req_param {
+struct bt_att_prep_write_req_param {
 	uint16_t handle;
 	uint16_t offset;
 	const uint8_t *part_value;
@@ -170,7 +170,7 @@ struct bt_att_prepare_write_req_param {
 };
 
 #define BT_ATT_OP_PREP_WRITE_RSP		0x17
-struct bt_att_prepare_write_rsp_param {
+struct bt_att_prep_write_rsp_param {
 	uint16_t handle;
 	uint16_t offset;
 	const uint8_t *part_value;
diff --git a/src/shared/att.c b/src/shared/att.c
index caf7d35..d6c2a63 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -412,6 +412,41 @@ static bool encode_write_pdu(struct att_send_op *op, const void *param,
 	return true;
 }
 
+static bool encode_prep_write_req(struct att_send_op *op, const void *param,
+						uint16_t length, uint16_t mtu)
+{
+	const struct bt_att_prep_write_req_param *p = param;
+	uint16_t len;
+
+	if (length != sizeof(*p))
+		return false;
+
+	len = 5 + p->length;
+	if (len > mtu)
+		return false;
+
+	op->pdu = malloc(len);
+	if (!op->pdu)
+		return false;
+
+	((uint8_t *) op->pdu)[0] = op->opcode;
+	put_le16(p->handle, ((uint8_t *) op->pdu) + 1);
+	put_le16(p->offset, ((uint8_t *) op->pdu) + 3);
+	op->len = len;
+	op->len = len;
+
+	if (p->length) {
+		if (!p->part_value) {
+			free(op->pdu);
+			return false;
+		}
+
+		memcpy(((uint8_t *) op->pdu) + 5, p->part_value, p->length);
+	}
+
+	return true;
+}
+
 static bool encode_pdu(struct att_send_op *op, const void *param,
 						uint16_t length, uint16_t mtu)
 {
@@ -454,6 +489,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
 		return encode_read_by_grp_type_req(op, param, length, mtu);
 	case BT_ATT_OP_WRITE_REQ:
 		return encode_write_pdu(op, param, length, mtu);
+	case BT_ATT_OP_PREP_WRITE_REQ:
+		return encode_prep_write_req(op, param, length, mtu);
 	default:
 		break;
 	}
@@ -892,6 +929,33 @@ static bool handle_write_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	return request_complete(att, BT_ATT_OP_WRITE_REQ, opcode, NULL, 0);
 }
 
+static bool handle_prep_write_rsp(struct bt_att *att, uint8_t opcode,
+						uint8_t *pdu, ssize_t pdu_len)
+{
+	struct bt_att_prep_write_rsp_param param;
+
+	/* PDU must contain at least the following:
+	 * - 1 octet: ATT opcode
+	 * - 2 octets: Attribute handle
+	 * - 2 octets: Offset
+	 */
+	if (pdu_len < 5)
+		return false;
+
+	memset(&param, 0, sizeof(param));
+
+	param.handle = get_le16(pdu + 1);
+	param.offset = get_le16(pdu + 3);
+
+	if (pdu_len > 5) {
+		param.length = pdu_len - 5;
+		param.part_value = pdu + 5;
+	}
+
+	return request_complete(att, BT_ATT_OP_PREP_WRITE_REQ, opcode, &param,
+								sizeof(param));
+}
+
 static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								ssize_t pdu_len)
 {
@@ -930,6 +994,9 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	case BT_ATT_OP_WRITE_RSP:
 		success = handle_write_rsp(att, opcode, pdu, pdu_len);
 		break;
+	case BT_ATT_OP_PREP_WRITE_RSP:
+		success = handle_prep_write_rsp(att, opcode, pdu, pdu_len);
+		break;
 	default:
 		success = false;
 		util_debug(att->debug_callback, att->debug_data,
-- 
2.0.0.526.g5318336

[PATCH 10/10] shared/att: Implement outgoing "Execute Write" request/response.

[Arman Uguray]

This patch adds support for "Execute Write" requests sent via
bt_att_send and the corresponding response.
---
 src/shared/att.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index d6c2a63..0e67786 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -447,6 +447,29 @@ static bool encode_prep_write_req(struct att_send_op *op, const void *param,
 	return true;
 }
 
+static bool encode_exec_write_req(struct att_send_op *op, const void *param,
+						uint16_t length, uint16_t mtu)
+{
+	const struct bt_att_exec_write_req_param *p = param;
+	uint16_t len = 2;
+
+	if (length != sizeof(*p))
+		return false;
+
+	if (len > mtu)
+		return false;
+
+	op->pdu = malloc(len);
+	if (!op->pdu)
+		return false;
+
+	((uint8_t *) op->pdu)[0] = op->opcode;
+	((uint8_t *) op->pdu)[1] = p->flags;
+	op->len = len;
+
+	return true;
+}
+
 static bool encode_pdu(struct att_send_op *op, const void *param,
 						uint16_t length, uint16_t mtu)
 {
@@ -491,6 +514,8 @@ static bool encode_pdu(struct att_send_op *op, const void *param,
 		return encode_write_pdu(op, param, length, mtu);
 	case BT_ATT_OP_PREP_WRITE_REQ:
 		return encode_prep_write_req(op, param, length, mtu);
+	case BT_ATT_OP_EXEC_WRITE_REQ:
+		return encode_exec_write_req(op, param, length, mtu);
 	default:
 		break;
 	}
@@ -956,6 +981,15 @@ static bool handle_prep_write_rsp(struct bt_att *att, uint8_t opcode,
 								sizeof(param));
 }
 
+static bool handle_exec_write_rsp(struct bt_att *att, uint8_t opcode,
+						uint8_t *pdu, ssize_t pdu_len)
+{
+	if (pdu_len > 1)
+		return false;
+
+	return request_complete(att, BT_ATT_OP_EXEC_WRITE_REQ, opcode, NULL, 0);
+}
+
 static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								ssize_t pdu_len)
 {
@@ -997,6 +1031,9 @@ static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	case BT_ATT_OP_PREP_WRITE_RSP:
 		success = handle_prep_write_rsp(att, opcode, pdu, pdu_len);
 		break;
+	case BT_ATT_OP_EXEC_WRITE_RSP:
+		success = handle_exec_write_rsp(att, opcode, pdu, pdu_len);
+		break;
 	default:
 		success = false;
 		util_debug(att->debug_callback, att->debug_data,
-- 
2.0.0.526.g5318336