Hi Arman,
> This patch adds handling for incoming ATT protocol response PDUs. Basic
> incoming PDU handling logic added to the io read handler.
> ---
> src/shared/att.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 117 insertions(+), 1 deletion(-)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index f3ece02..e967273 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -370,6 +370,109 @@ set_handler:
> att->writer_active = true;
> }
>
> +static bool request_complete(struct bt_att *att, uint8_t req_opcode,
> + uint8_t rsp_opcode, const void *param,
> + uint16_t len)
> +{
> + struct att_send_op *op = att->pending_req;
> +
> + if (!op) {
> + /* There is no pending request so the response is unexpected.
> + * TODO: In such a case, the connection should be dropped
> + * entirely. For now, simply ignore the PDU.
> + */
is this what the specification say?
> + wakeup_writer(att);
> + return false;
> + }
> +
> + if (op->opcode != req_opcode)
> + /* The request opcode corresponding to the received response
> + * opcode does not match the currently pending request.
> + * TODO: The connection should be dropped in this case, since
> + * an invalid response PDU has been received. For now, keep
> + * waiting for the correct response.
> + */
> + return false;
In these case { } are a bit of sugar to make it easier on the eyes. The compiler knows what it is, but my brain gets confused too easy ;)
> +
> + if (op->callback)
> + op->callback(rsp_opcode, param, len, op->user_data);
> +
> + destroy_att_send_op(op);
> + att->pending_req = NULL;
> +
> + if (!att->destroyed)
> + wakeup_writer(att);
I would have done this:
if (att->destroyed)
return true;
wakeup_writer(att);
I prefer it this way since it is easier to read when you go through the function. Especially with the att->destroyed cases for the reentrant support. We want to make it clear that we expect that object to be gone.
> +
> + return true;
> +}
> +
> +static bool handle_error_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
> + ssize_t pdu_len)
> +{
> + struct bt_att_error_rsp_param p;
> + bool result;
> +
> + if (pdu_len != 5)
> + return false;
> +
> + memset(&p, 0, sizeof(p));
> + p.request_opcode = pdu[1];
> + p.handle = get_le16(pdu + 2);
> + p.error_code = pdu[4];
> +
> + /* Note: avoid a tail call */
I do not get this comment. Explain please.
> + result = request_complete(att, p.request_opcode, opcode, &p, sizeof(p));
> + return result;
return request_complete(..
> +}
> +
> +static bool handle_mtu_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
> + ssize_t pdu_len)
> +{
> + struct bt_att_mtu_rsp_param p;
> + bool result;
> +
> + if (pdu_len != 3)
> + return false;
> +
> + memset(&p, 0, sizeof(p));
> + p.server_rx_mtu = get_le16(pdu + 1);
> +
> + /* Note: avoid a tail call */
> + result = request_complete(att, BT_ATT_OP_MTU_REQ, opcode,
> + &p, sizeof(p));
> + return result;
See above.
> +}
> +
> +static void handle_rsp(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
> + ssize_t pdu_len)
> +{
> + bool success;
> +
> + switch (opcode) {
> + case BT_ATT_OP_ERROR_RSP:
> + success = handle_error_rsp(att, opcode, pdu, pdu_len);
> + break;
> + case BT_ATT_OP_MTU_RSP:
> + success = handle_mtu_rsp(att, opcode, pdu, pdu_len);
> + break;
> + default:
> + success = false;
> + util_debug(att->debug_callback, att->debug_data,
> + "Unknown response opcode: 0x%02x", opcode);
> + break;
> + }
> +
> + if (success)
> + return;
> +
> + util_debug(att->debug_callback, att->debug_data,
> + "Failed to handle respone PDU; opcode: 0x%02x", opcode);
> +
> + if (att->pending_req)
> + request_complete(att, att->pending_req->opcode,
> + BT_ATT_OP_ERROR_RSP, NULL, 0);
> +}
> +
> static void read_watch_destroy(void *user_data)
> {
> struct bt_att *att = user_data;
> @@ -389,6 +492,7 @@ static void read_watch_destroy(void *user_data)
> static bool can_read_data(struct io *io, void *user_data)
> {
> struct bt_att *att = user_data;
> + uint8_t opcode;
> uint8_t *pdu;
> ssize_t bytes_read;
>
> @@ -402,7 +506,19 @@ static bool can_read_data(struct io *io, void *user_data)
> if (bytes_read < ATT_MIN_PDU_LEN)
> goto done;
>
> - /* TODO: Handle different types of PDUs here */
> + pdu = att->buf;
> + opcode = pdu[0];
> +
> + /* Act on the received PDU based on the opcode type */
> + switch (get_op_type(opcode)) {
> + case ATT_OP_TYPE_RSP:
> + handle_rsp(att, opcode, pdu, bytes_read);
> + break;
> + default:
> + util_debug(att->debug_callback, att->debug_data,
> + "ATT opcode cannot be handled: 0x%02x", opcode);
> + break;
> + }
>
> done:
> if (att->destroyed)
Regards
Marcel
--
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
