[PATCH 0/7] L2CAP control field + AVRCP browsing PDU parsing

[PATCH 0/7] L2CAP control field + AVRCP browsing PDU parsing

[Vikrampal Yadav]

Support for ERTM control bytes adjustment, parsing L2CAP extended control
field and AVRCP SetBrowsedPlayer added in Bluetooth monitor.

Vikrampal Yadav (7):
  monitor: Adjust for ERTM control bytes
  monitor: Make the parameter name generic
  monitor: Add functionality to store extented control in DB
  monitor: Extract extended L2CAP extended control field
  monitor: Add support for parsing L2CAP extended control field
  monitor: Add support for parsing L2CAP control field
  monitor: Add AVRCP SetBrowsedPlayer support

 monitor/avctp.c |  86 +++++++++++++++++++-
 monitor/l2cap.c | 242 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 322 insertions(+), 6 deletions(-)

-- 
1.9.1

[PATCH 1/7] monitor: Adjust for ERTM control bytes

[Vikrampal Yadav]

Adjustment for ERTM control bytes fixed.
---
 monitor/l2cap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/monitor/l2cap.c b/monitor/l2cap.c
index ebdd20f..635b967 100644
--- a/monitor/l2cap.c
+++ b/monitor/l2cap.c
@@ -2621,6 +2621,9 @@ static void l2cap_frame(uint16_t index, bool in, uint16_t handle,
 		connless_packet(index, in, handle, cid, data, size);
 		break;
 	case 0x0003:
+		/* Adjust for ERTM control bytes */
+		data += 2;
+		size -= 2;
 		amp_packet(index, in, handle, cid, data, size);
 		break;
 	case 0x0004:
-- 
1.9.1

[PATCH 2/7] monitor: Make the parameter name generic

[Vikrampal Yadav]

Changed dcid to cid to make it sound generic as this function is called
with both dcid and scid.
---
 monitor/l2cap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/monitor/l2cap.c b/monitor/l2cap.c
index 635b967..48fab8c 100644
--- a/monitor/l2cap.c
+++ b/monitor/l2cap.c
@@ -421,7 +421,7 @@ static struct {
 };
 
 static void print_config_options(const struct l2cap_frame *frame,
-				uint8_t offset, uint16_t dcid, bool response)
+				uint8_t offset, uint16_t cid, bool response)
 {
 	const uint8_t *data = frame->data + offset;
 	uint16_t size = frame->size - offset;
@@ -496,7 +496,7 @@ static void print_config_options(const struct l2cap_frame *frame,
                         break;
 		case 0x04:
 			if (response)
-				assign_mode(frame, data[consumed + 2], dcid);
+				assign_mode(frame, data[consumed + 2], cid);
 
 			switch (data[consumed + 2]) {
 			case 0x00:
-- 
1.9.1

[PATCH 3/7] monitor: Add functionality to store extented control in DB

[Vikrampal Yadav]

A function added to store extented control in DB.
---
 monitor/l2cap.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/monitor/l2cap.c b/monitor/l2cap.c
index 48fab8c..ef84925 100644
--- a/monitor/l2cap.c
+++ b/monitor/l2cap.c
@@ -54,6 +54,7 @@ struct chan_data {
 	uint16_t psm;
 	uint8_t  ctrlid;
 	uint8_t  mode;
+	uint8_t  ext_ctrl;
 };
 
 static struct chan_data chan_list[MAX_CHAN];
@@ -255,6 +256,32 @@ static uint16_t get_chan(const struct l2cap_frame *frame)
 	return 0;
 }
 
+static void assign_ext_ctrl(const struct l2cap_frame *frame,
+					uint8_t ext_ctrl, uint16_t dcid)
+{
+	int i;
+
+	for (i = 0; i < MAX_CHAN; i++) {
+		if (chan_list[i].index != frame->index)
+			continue;
+
+		if (chan_list[i].handle != frame->handle)
+			continue;
+
+		if (frame->in) {
+			if (chan_list[i].scid == dcid) {
+				chan_list[i].ext_ctrl = ext_ctrl;
+				break;
+			}
+		} else {
+			if (chan_list[i].dcid == dcid) {
+				chan_list[i].ext_ctrl = ext_ctrl;
+				break;
+			}
+		}
+	}
+}
+
 #define MAX_INDEX 16
 
 struct index_data {
@@ -573,8 +600,9 @@ static void print_config_options(const struct l2cap_frame *frame,
 					get_le32(data + consumed + 14));
 			break;
 		case 0x07:
-			print_field("  Max window size: %d",
+			print_field("  Extended window size: %d",
 					get_le16(data + consumed + 2));
+			assign_ext_ctrl(frame, 1, cid);
 			break;
 		default:
 			packet_hexdump(data + consumed + 2, len);
-- 
1.9.1

[PATCH 4/7] monitor: Extract extended L2CAP extended control field

[Vikrampal Yadav]

Support for extracting extended L2CAP extended control field.
---
 monitor/l2cap.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 58 insertions(+), 2 deletions(-)

diff --git a/monitor/l2cap.c b/monitor/l2cap.c
index ef84925..427c4a2 100644
--- a/monitor/l2cap.c
+++ b/monitor/l2cap.c
@@ -282,6 +282,31 @@ static void assign_ext_ctrl(const struct l2cap_frame *frame,
 	}
 }
 
+static uint8_t get_ext_ctrl(const struct l2cap_frame *frame)
+{
+	int i;
+
+	for (i = 0; i < MAX_CHAN; i++) {
+		if (chan_list[i].index != frame->index &&
+						chan_list[i].ctrlid == 0)
+			continue;
+
+		if (chan_list[i].handle != frame->handle &&
+					chan_list[i].ctrlid != frame->index)
+			continue;
+
+		if (frame->in) {
+			if (chan_list[i].scid == frame->cid)
+				return chan_list[i].ext_ctrl;
+		} else {
+			if (chan_list[i].dcid == frame->cid)
+				return chan_list[i].ext_ctrl;
+		}
+	}
+
+	return 0;
+}
+
 #define MAX_INDEX 16
 
 struct index_data {
@@ -2640,6 +2665,9 @@ static void l2cap_frame(uint16_t index, bool in, uint16_t handle,
 			uint16_t cid, const void *data, uint16_t size)
 {
 	struct l2cap_frame frame;
+	uint32_t ctrl32 = 0;
+	uint16_t ctrl16 = 0;
+	uint8_t ext_ctrl;
 
 	switch (cid) {
 	case 0x0001:
@@ -2666,10 +2694,38 @@ static void l2cap_frame(uint16_t index, bool in, uint16_t handle,
 	default:
 		l2cap_frame_init(&frame, index, in, handle, cid, data, size);
 
-		print_indent(6, COLOR_CYAN, "Channel:", "", COLOR_OFF,
-				" %d len %d [PSM %d mode %d] {chan %d}",
+		if (frame.mode > 0) {
+			ext_ctrl = get_ext_ctrl(&frame);
+
+			if (ext_ctrl) {
+				if (!l2cap_frame_get_le32(&frame, &ctrl32))
+					return;
+
+				print_indent(6, COLOR_CYAN, "Channel:", "",
+						COLOR_OFF, " %d len %d"
+						" ext_ctrl 0x%8.8x"
+						" [PSM %d mode %d] {chan %d}",
+						cid, size, ctrl32, frame.psm,
+						frame.mode, frame.chan);
+			} else {
+				if (!l2cap_frame_get_le16(&frame, &ctrl16))
+					return;
+
+				print_indent(6, COLOR_CYAN, "Channel:", "",
+						COLOR_OFF, " %d len %d"
+						" ctrl 0x%4.4x"
+						" [PSM %d mode %d] {chan %d}",
+						cid, size, ctrl16, frame.psm,
+						frame.mode, frame.chan);
+			}
+
+			printf("\n");
+		} else {
+			print_indent(6, COLOR_CYAN, "Channel:", "", COLOR_OFF,
+					" %d len %d [PSM %d mode %d] {chan %d}",
 						cid, size, frame.psm,
 						frame.mode, frame.chan);
+		}
 
 		switch (frame.psm) {
 		case 0x0001:
-- 
1.9.1

[PATCH 5/7] monitor: Add support for parsing L2CAP extended control field

[Vikrampal Yadav]

Support for parsing L2CAP extended control field added.
---
 monitor/l2cap.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/monitor/l2cap.c b/monitor/l2cap.c
index 427c4a2..f5ee0cd 100644
--- a/monitor/l2cap.c
+++ b/monitor/l2cap.c
@@ -44,6 +44,48 @@
 #include "avctp.h"
 #include "rfcomm.h"
 
+/* L2CAP Control Field bit masks */
+#define L2CAP_CTRL_SAR_MASK		0xC000
+#define L2CAP_CTRL_REQSEQ_MASK		0x3F00
+#define L2CAP_CTRL_TXSEQ_MASK		0x007E
+#define L2CAP_CTRL_SUPERVISE_MASK	0x000C
+
+#define L2CAP_CTRL_RETRANS		0x0080
+#define L2CAP_CTRL_FINAL		0x0080
+#define L2CAP_CTRL_POLL			0x0010
+#define L2CAP_CTRL_FRAME_TYPE		0x0001 /* I- or S-Frame */
+
+#define L2CAP_CTRL_TXSEQ_SHIFT		1
+#define L2CAP_CTRL_SUPER_SHIFT		2
+#define L2CAP_CTRL_REQSEQ_SHIFT		8
+#define L2CAP_CTRL_SAR_SHIFT		14
+
+#define L2CAP_EXT_CTRL_TXSEQ_MASK	0xFFFC0000
+#define L2CAP_EXT_CTRL_SAR_MASK		0x00030000
+#define L2CAP_EXT_CTRL_SUPERVISE_MASK	0x00030000
+#define L2CAP_EXT_CTRL_REQSEQ_MASK	0x0000FFFC
+
+#define L2CAP_EXT_CTRL_POLL		0x00040000
+#define L2CAP_EXT_CTRL_FINAL		0x00000002
+#define L2CAP_EXT_CTRL_FRAME_TYPE	0x00000001 /* I- or S-Frame */
+
+#define L2CAP_EXT_CTRL_REQSEQ_SHIFT	2
+#define L2CAP_EXT_CTRL_SAR_SHIFT	16
+#define L2CAP_EXT_CTRL_SUPER_SHIFT	16
+#define L2CAP_EXT_CTRL_TXSEQ_SHIFT	18
+
+/* L2CAP Supervisory Function */
+#define L2CAP_SUPER_RR		0x00
+#define L2CAP_SUPER_REJ		0x01
+#define L2CAP_SUPER_RNR		0x02
+#define L2CAP_SUPER_SREJ	0x03
+
+/* L2CAP Segmentation and Reassembly */
+#define L2CAP_SAR_UNSEGMENTED 	0x00
+#define L2CAP_SAR_START		0x01
+#define L2CAP_SAR_END		0x02
+#define L2CAP_SAR_CONTINUE	0x03
+
 #define MAX_CHAN 64
 
 struct chan_data {
@@ -307,6 +349,73 @@ static uint8_t get_ext_ctrl(const struct l2cap_frame *frame)
 	return 0;
 }
 
+static char *sar2str(uint8_t sar)
+{
+	switch (sar) {
+	case L2CAP_SAR_UNSEGMENTED:
+		return "Unsegmented";
+	case L2CAP_SAR_START:
+		return "Start";
+	case L2CAP_SAR_END:
+		return "End";
+	case L2CAP_SAR_CONTINUE:
+		return "Continuation";
+	default:
+		return "Bad SAR";
+	}
+}
+
+static char *supervisory2str(uint8_t supervisory)
+{
+	switch (supervisory) {
+	case L2CAP_SUPER_RR:
+		return "Receiver Ready (RR)";
+	case L2CAP_SUPER_REJ:
+		return "Reject (REJ)";
+	case L2CAP_SUPER_RNR:
+		return "Receiver Not Ready (RNR)";
+	case L2CAP_SUPER_SREJ:
+		return "Select Reject (SREJ)";
+	default:
+		return "Bad Supervisory";
+	}
+}
+
+static void l2cap_ctrl_ext_parse(struct l2cap_frame *frame, uint32_t ctrl)
+{
+	printf("      %s:",
+		ctrl & L2CAP_EXT_CTRL_FRAME_TYPE ? "S-frame" : "I-frame");
+
+	if (ctrl & L2CAP_EXT_CTRL_FRAME_TYPE) {
+		printf(" %s",
+		supervisory2str((ctrl & L2CAP_EXT_CTRL_SUPERVISE_MASK) >>
+						L2CAP_EXT_CTRL_SUPER_SHIFT));
+
+		if (ctrl & L2CAP_EXT_CTRL_POLL)
+			printf(" P-bit");
+	} else {
+		uint8_t sar = (ctrl & L2CAP_EXT_CTRL_SAR_MASK) >>
+						L2CAP_EXT_CTRL_SAR_SHIFT;
+		printf(" %s", sar2str(sar));
+		if (sar == L2CAP_SAR_START) {
+			uint16_t len;
+
+			if (!l2cap_frame_get_le16(frame, &len))
+				return;
+
+			printf(" (len %d)", len);
+		}
+		printf(" TxSeq %d", (ctrl & L2CAP_EXT_CTRL_TXSEQ_MASK) >>
+						L2CAP_EXT_CTRL_TXSEQ_SHIFT);
+	}
+
+	printf(" ReqSeq %d", (ctrl & L2CAP_EXT_CTRL_REQSEQ_MASK) >>
+						L2CAP_EXT_CTRL_REQSEQ_SHIFT);
+
+	if (ctrl & L2CAP_EXT_CTRL_FINAL)
+		printf(" F-bit");
+}
+
 #define MAX_INDEX 16
 
 struct index_data {
@@ -2707,6 +2816,8 @@ static void l2cap_frame(uint16_t index, bool in, uint16_t handle,
 						" [PSM %d mode %d] {chan %d}",
 						cid, size, ctrl32, frame.psm,
 						frame.mode, frame.chan);
+
+				l2cap_ctrl_ext_parse(&frame, ctrl32);
 			} else {
 				if (!l2cap_frame_get_le16(&frame, &ctrl16))
 					return;
-- 
1.9.1

[PATCH 6/7] monitor: Add support for parsing L2CAP control field

[Vikrampal Yadav]

Support for parsing L2CAP control field added.
---
 monitor/l2cap.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/monitor/l2cap.c b/monitor/l2cap.c
index f5ee0cd..16959df 100644
--- a/monitor/l2cap.c
+++ b/monitor/l2cap.c
@@ -416,6 +416,38 @@ static void l2cap_ctrl_ext_parse(struct l2cap_frame *frame, uint32_t ctrl)
 		printf(" F-bit");
 }
 
+static void l2cap_ctrl_parse(struct l2cap_frame *frame, uint32_t ctrl)
+{
+	printf("      %s:",
+			ctrl & L2CAP_CTRL_FRAME_TYPE ? "S-frame" : "I-frame");
+
+	if (ctrl & 0x01) {
+		printf(" %s",
+			supervisory2str((ctrl & L2CAP_CTRL_SUPERVISE_MASK) >>
+						L2CAP_CTRL_SUPER_SHIFT));
+
+		if (ctrl & L2CAP_CTRL_POLL)
+			printf(" P-bit");
+	} else {
+		uint8_t sar = (ctrl & L2CAP_CTRL_SAR_MASK) >> L2CAP_CTRL_SAR_SHIFT;
+		printf(" %s", sar2str(sar));
+		if (sar == L2CAP_SAR_START) {
+			uint16_t len;
+
+			if (!l2cap_frame_get_le16(frame, &len))
+				return;
+
+			printf(" (len %d)", len);
+		}
+		printf(" TxSeq %d", (ctrl & L2CAP_CTRL_TXSEQ_MASK) >> L2CAP_CTRL_TXSEQ_SHIFT);
+	}
+
+	printf(" ReqSeq %d", (ctrl & L2CAP_CTRL_REQSEQ_MASK) >> L2CAP_CTRL_REQSEQ_SHIFT);
+
+	if (ctrl & L2CAP_CTRL_FINAL)
+		printf(" F-bit");
+}
+
 #define MAX_INDEX 16
 
 struct index_data {
@@ -2828,6 +2860,8 @@ static void l2cap_frame(uint16_t index, bool in, uint16_t handle,
 						" [PSM %d mode %d] {chan %d}",
 						cid, size, ctrl16, frame.psm,
 						frame.mode, frame.chan);
+
+				l2cap_ctrl_parse(&frame, ctrl16);
 			}
 
 			printf("\n");
-- 
1.9.1

[PATCH 7/7] monitor: Add AVRCP SetBrowsedPlayer support

[Vikrampal Yadav]

Support for decoding AVRCP SetBrowsedPlayer added in Bluetooth monitor.
---
 monitor/avctp.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 85 insertions(+), 1 deletion(-)

diff --git a/monitor/avctp.c b/monitor/avctp.c
index af91ecc..0a1e92d 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -1637,11 +1637,95 @@ static bool avrcp_control_packet(struct avctp_frame *avctp_frame)
 	}
 }
 
+static bool avrcp_set_browsed_player(struct avctp_frame *avctp_frame)
+{
+	struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
+	uint32_t items;
+	uint16_t id, uids, charset;
+	uint8_t status, folders, indent = 2;
+
+	if (avctp_frame->hdr & 0x02)
+		goto response;
+
+	if (!l2cap_frame_get_be16(frame, &id))
+		return false;
+
+	print_field("%*cPlayerID: 0x%04x (%u)", indent, ' ', id, id);
+	return true;
+
+response:
+	if (!l2cap_frame_get_u8(frame, &status))
+		return false;
+
+	print_field("%*cStatus: 0x%02x (%s)", indent, ' ', status,
+							error2str(status));
+
+	if (!l2cap_frame_get_be16(frame, &uids))
+		return false;
+
+	print_field("%*cUIDCounter: 0x%04x (%u)", indent, ' ', uids, uids);
+
+	if (!l2cap_frame_get_be32(frame, &items))
+		return false;
+
+	print_field("%*cNumber of Items: 0x%08x (%u)", indent, ' ',
+								items, items);
+
+	if (!l2cap_frame_get_be16(frame, &charset))
+		return false;
+
+	print_field("%*cCharsetID: 0x%04x (%s)", indent, ' ', charset,
+							charset2str(charset));
+
+	if (!l2cap_frame_get_u8(frame, &folders))
+		return false;
+
+	print_field("%*cFolder Depth: 0x%02x (%u)", indent, ' ', folders,
+								folders);
+
+	for (; folders > 0; folders--) {
+		uint8_t len;
+
+		if (!l2cap_frame_get_u8(frame, &len))
+			return false;
+
+		printf("Folder: ");
+		for (; len > 0; len--) {
+			uint8_t c;
+
+			if (!l2cap_frame_get_u8(frame, &c))
+				return false;
+
+			printf("%1c", isprint(c) ? c : '.');
+		}
+		printf("\n");
+	}
+
+	return true;
+}
+
 static bool avrcp_browsing_packet(struct avctp_frame *avctp_frame)
 {
 	struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
+	uint16_t len;
+	uint8_t pduid;
+
+	if (!l2cap_frame_get_u8(frame, &pduid))
+		return false;
+
+	if (!l2cap_frame_get_be16(frame, &len))
+		return false;
+
+	print_field("AVRCP: %s: len 0x%04x", pdu2str(pduid), len);
+
+	switch (pduid) {
+	case AVRCP_SET_BROWSED_PLAYER:
+		avrcp_set_browsed_player(avctp_frame);
+		break;
+	default:
+		packet_hexdump(frame->data, frame->size);
+	}
 
-	packet_hexdump(frame->data, frame->size);
 	return true;
 }
 
-- 
1.9.1

Re: [PATCH 0/7] L2CAP control field + AVRCP browsing PDU parsing

[Luiz Augusto von Dentz]

Hi Vikram,

On Fri, Nov 21, 2014 at 3:47 PM, Vikrampal Yadav <vikram.pal@samsung.com> wrote:
> Support for ERTM control bytes adjustment, parsing L2CAP extended control
> field and AVRCP SetBrowsedPlayer added in Bluetooth monitor.
>
> Vikrampal Yadav (7):
>   monitor: Adjust for ERTM control bytes
>   monitor: Make the parameter name generic
>   monitor: Add functionality to store extented control in DB
>   monitor: Extract extended L2CAP extended control field
>   monitor: Add support for parsing L2CAP extended control field
>   monitor: Add support for parsing L2CAP control field
>   monitor: Add AVRCP SetBrowsedPlayer support
>
>  monitor/avctp.c |  86 +++++++++++++++++++-
>  monitor/l2cap.c | 242 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 322 insertions(+), 6 deletions(-)
>
> --
> 1.9.1

Sorry for the delay, this one got lost in my inbox, most patches seems
to be missing the output they generate, could you please add it.


-- 
Luiz Augusto von Dentz

Re: [PATCH 1/7] monitor: Adjust for ERTM control bytes

[Luiz Augusto von Dentz]

Hi Vikram,

On Fri, Nov 21, 2014 at 3:47 PM, Vikrampal Yadav <vikram.pal@samsung.com> wrote:
> Adjustment for ERTM control bytes fixed.
> ---
>  monitor/l2cap.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/monitor/l2cap.c b/monitor/l2cap.c
> index ebdd20f..635b967 100644
> --- a/monitor/l2cap.c
> +++ b/monitor/l2cap.c
> @@ -2621,6 +2621,9 @@ static void l2cap_frame(uint16_t index, bool in, uint16_t handle,
>                 connless_packet(index, in, handle, cid, data, size);
>                 break;
>         case 0x0003:
> +               /* Adjust for ERTM control bytes */
> +               data += 2;
> +               size -= 2;
>                 amp_packet(index, in, handle, cid, data, size);
>                 break;
>         case 0x0004:
> --
> 1.9.1

What are those bytes, shouldn't they be printed?


-- 
Luiz Augusto von Dentz

RE: [PATCH 1/7] monitor: Adjust for ERTM control bytes

[Vikrampal]

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> Sent: Friday, November 28, 2014 2:08 PM
> To: Vikrampal Yadav
> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; cpgs@samsung.com
> Subject: Re: [PATCH 1/7] monitor: Adjust for ERTM control bytes
> 
> Hi Vikram,
> 
> On Fri, Nov 21, 2014 at 3:47 PM, Vikrampal Yadav
> <vikram.pal@samsung.com> wrote:
> > Adjustment for ERTM control bytes fixed.
> > ---
> >  monitor/l2cap.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/monitor/l2cap.c b/monitor/l2cap.c index ebdd20f..635b967
> > 100644
> > --- a/monitor/l2cap.c
> > +++ b/monitor/l2cap.c
> > @@ -2621,6 +2621,9 @@ static void l2cap_frame(uint16_t index, bool in,
> uint16_t handle,
> >                 connless_packet(index, in, handle, cid, data, size);
> >                 break;
> >         case 0x0003:
> > +               /* Adjust for ERTM control bytes */
> > +               data += 2;
> > +               size -= 2;
> >                 amp_packet(index, in, handle, cid, data, size);
> >                 break;
> >         case 0x0004:
> > --
> > 1.9.1
> 
> What are those bytes, shouldn't they be printed?

These bytes are not being printed even in hcidump.
> 
> 
> --
> Luiz Augusto von Dentz

Regards,
Vikram

RE: [PATCH 0/7] L2CAP control field + AVRCP browsing PDU parsing

[Vikrampal]

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> Sent: Friday, November 28, 2014 2:07 PM
> To: Vikrampal Yadav
> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; cpgs@samsung.com
> Subject: Re: [PATCH 0/7] L2CAP control field + AVRCP browsing PDU parsing
> 
> Hi Vikram,
> 
> On Fri, Nov 21, 2014 at 3:47 PM, Vikrampal Yadav
> <vikram.pal@samsung.com> wrote:
> > Support for ERTM control bytes adjustment, parsing L2CAP extended
> > control field and AVRCP SetBrowsedPlayer added in Bluetooth monitor.
> >
> > Vikrampal Yadav (7):
> >   monitor: Adjust for ERTM control bytes
> >   monitor: Make the parameter name generic
> >   monitor: Add functionality to store extented control in DB
> >   monitor: Extract extended L2CAP extended control field
> >   monitor: Add support for parsing L2CAP extended control field
> >   monitor: Add support for parsing L2CAP control field
> >   monitor: Add AVRCP SetBrowsedPlayer support
> >
> >  monitor/avctp.c |  86 +++++++++++++++++++-  monitor/l2cap.c | 242
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 322 insertions(+), 6 deletions(-)
> >
> > --
> > 1.9.1
> 
> Sorry for the delay, this one got lost in my inbox, most patches seems to be
> missing the output they generate, could you please add it.

Most patches in this series are helper patches for the last patch in this series.
The final patch actually prints the output which shows the work done by
Other patches.
> 
> 
> --
> Luiz Augusto von Dentz

Regards,
Vikram

Re: [PATCH 0/7] L2CAP control field + AVRCP browsing PDU parsing

[Luiz Augusto von Dentz]

Hi Vikram,

On Wed, Dec 3, 2014 at 9:08 AM, Vikrampal <vikram.pal@samsung.com> wrote:
> Hi Luiz,
>
>> -----Original Message-----
>> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
>> Sent: Friday, November 28, 2014 2:07 PM
>> To: Vikrampal Yadav
>> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; cpgs@samsung.com
>> Subject: Re: [PATCH 0/7] L2CAP control field + AVRCP browsing PDU parsing
>>
>> Hi Vikram,
>>
>> On Fri, Nov 21, 2014 at 3:47 PM, Vikrampal Yadav
>> <vikram.pal@samsung.com> wrote:
>> > Support for ERTM control bytes adjustment, parsing L2CAP extended
>> > control field and AVRCP SetBrowsedPlayer added in Bluetooth monitor.
>> >
>> > Vikrampal Yadav (7):
>> >   monitor: Adjust for ERTM control bytes
>> >   monitor: Make the parameter name generic
>> >   monitor: Add functionality to store extented control in DB
>> >   monitor: Extract extended L2CAP extended control field
>> >   monitor: Add support for parsing L2CAP extended control field
>> >   monitor: Add support for parsing L2CAP control field
>> >   monitor: Add AVRCP SetBrowsedPlayer support
>> >
>> >  monitor/avctp.c |  86 +++++++++++++++++++-  monitor/l2cap.c | 242
>> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>> >  2 files changed, 322 insertions(+), 6 deletions(-)
>> >
>> > --
>> > 1.9.1
>>
>> Sorry for the delay, this one got lost in my inbox, most patches seems to be
>> missing the output they generate, could you please add it.
>
> Most patches in this series are helper patches for the last patch in this series.
> The final patch actually prints the output which shows the work done by
> Other patches.

Actually from patch 4/7 onward they all print different output and I
remember commenting before that I want to see then in the patches so
we can avoid format errors, typos etc.

-- 
Luiz Augusto von Dentz

Re: [PATCH 1/7] monitor: Adjust for ERTM control bytes

[Luiz Augusto von Dentz]

Hi Vikram,

On Wed, Dec 3, 2014 at 9:05 AM, Vikrampal <vikram.pal@samsung.com> wrote:
> Hi Luiz,
>
>> -----Original Message-----
>> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
>> Sent: Friday, November 28, 2014 2:08 PM
>> To: Vikrampal Yadav
>> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; cpgs@samsung.com
>> Subject: Re: [PATCH 1/7] monitor: Adjust for ERTM control bytes
>>
>> Hi Vikram,
>>
>> On Fri, Nov 21, 2014 at 3:47 PM, Vikrampal Yadav
>> <vikram.pal@samsung.com> wrote:
>> > Adjustment for ERTM control bytes fixed.
>> > ---
>> >  monitor/l2cap.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>> >
>> > diff --git a/monitor/l2cap.c b/monitor/l2cap.c index ebdd20f..635b967
>> > 100644
>> > --- a/monitor/l2cap.c
>> > +++ b/monitor/l2cap.c
>> > @@ -2621,6 +2621,9 @@ static void l2cap_frame(uint16_t index, bool in,
>> uint16_t handle,
>> >                 connless_packet(index, in, handle, cid, data, size);
>> >                 break;
>> >         case 0x0003:
>> > +               /* Adjust for ERTM control bytes */
>> > +               data += 2;
>> > +               size -= 2;
>> >                 amp_packet(index, in, handle, cid, data, size);
>> >                 break;
>> >         case 0x0004:
>> > --
>> > 1.9.1
>>
>> What are those bytes, shouldn't they be printed?
>
> These bytes are not being printed even in hcidump.

Which is probably wrong, this is the ERTM control bytes that should be
parsed the same way regardless of the CID, hcidump apparently only
parse ERTM control in case of Connection oriented channel and ignores
the fact AMP Manager Channel also uses ERTM.

>>
>>
>> --
>> Luiz Augusto von Dentz
>
> Regards,
> Vikram
>



-- 
Luiz Augusto von Dentz

RE: [PATCH 0/7] L2CAP control field + AVRCP browsing PDU parsing

[Vikrampal]

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> Sent: Wednesday, December 03, 2014 1:32 PM
> To: Vikrampal
> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; cpgs@samsung.com
> Subject: Re: [PATCH 0/7] L2CAP control field + AVRCP browsing PDU parsing
> 
> Hi Vikram,
> 
> On Wed, Dec 3, 2014 at 9:08 AM, Vikrampal <vikram.pal@samsung.com>
> wrote:
> > Hi Luiz,
> >
> >> -----Original Message-----
> >> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> >> Sent: Friday, November 28, 2014 2:07 PM
> >> To: Vikrampal Yadav
> >> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin;
> >> cpgs@samsung.com
> >> Subject: Re: [PATCH 0/7] L2CAP control field + AVRCP browsing PDU
> >> parsing
> >>
> >> Hi Vikram,
> >>
> >> On Fri, Nov 21, 2014 at 3:47 PM, Vikrampal Yadav
> >> <vikram.pal@samsung.com> wrote:
> >> > Support for ERTM control bytes adjustment, parsing L2CAP extended
> >> > control field and AVRCP SetBrowsedPlayer added in Bluetooth monitor.
> >> >
> >> > Vikrampal Yadav (7):
> >> >   monitor: Adjust for ERTM control bytes
> >> >   monitor: Make the parameter name generic
> >> >   monitor: Add functionality to store extented control in DB
> >> >   monitor: Extract extended L2CAP extended control field
> >> >   monitor: Add support for parsing L2CAP extended control field
> >> >   monitor: Add support for parsing L2CAP control field
> >> >   monitor: Add AVRCP SetBrowsedPlayer support
> >> >
> >> >  monitor/avctp.c |  86 +++++++++++++++++++-  monitor/l2cap.c | 242
> >> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >> >  2 files changed, 322 insertions(+), 6 deletions(-)
> >> >
> >> > --
> >> > 1.9.1
> >>
> >> Sorry for the delay, this one got lost in my inbox, most patches
> >> seems to be missing the output they generate, could you please add it.
> >
> > Most patches in this series are helper patches for the last patch in this
> series.
> > The final patch actually prints the output which shows the work done
> > by Other patches.
> 
> Actually from patch 4/7 onward they all print different output and I
> remember commenting before that I want to see then in the patches so we
> can avoid format errors, typos etc.

Oh! I have not pasted the output even in the final patch definition. I'm thinking
that I've done that. My mistake! I'll paste the final output which will comprise the
outputs from all the patches combined.

> 
> --
> Luiz Augusto von Dentz

Regards,
Vikram