[PATCH 0/5] Add support for decoding RFCOMM MCC Message Type

[PATCH 0/5] Add support for decoding RFCOMM MCC Message Type

[Gowtham Anandha Babu]

This patch set implements the handlers for decoding
MSC, RPN, RLS, PN, NSC frames for RFCOMM in bluetooth
monitor.


Gowtham Anandha Babu (5):
  monitor/rfcomm.c: Add support for MSC frame decoding
  monitor/rfcomm: Add support for RPN frame decoding
  monitor/rfcomm: Add support for RLS frame decoding
  monitor/rfcomm: Add support for PN frame decoding
  monitor/rfcomm: Add support for NSC frame decoding

 monitor/rfcomm.c | 243 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 236 insertions(+), 7 deletions(-)

-- 
1.9.1

[PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame decoding

[Gowtham Anandha Babu]

Changes made to decode MSC frame in RFCOMM for btmon.

      RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
         Address: 0x01 cr 0 dlci 0x00
         Control: 0xef poll/final 0
         Length: 4
         FCS: 0xaa
         MCC Message type: Modem Status Command CMD(0x38)
           Length: 2
           dlci 32
           fc 0 rtc 1 rtr 1 ic 0 dv 1
---
 monitor/rfcomm.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 69 insertions(+), 7 deletions(-)

diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
index b3db98b..969b29f 100644
--- a/monitor/rfcomm.c
+++ b/monitor/rfcomm.c
@@ -49,11 +49,24 @@ static char *cr_str[] = {
 	"CMD"
 };
 
-#define CR_STR(type) cr_str[GET_CR(type)]
-#define GET_LEN8(length) ((length & 0xfe) >> 1)
-#define GET_LEN16(length) ((length & 0xfffe) >> 1)
-#define GET_CR(type)	((type & 0x02) >> 1)
-#define GET_PF(ctr) (((ctr) >> 4) & 0x1)
+/* RFCOMM frame parsing macros */
+#define CR_STR(type)		cr_str[GET_CR(type)]
+#define GET_LEN8(length)	((length & 0xfe) >> 1)
+#define GET_LEN16(length)	((length & 0xfffe) >> 1)
+#define GET_CR(type)		((type & 0x02) >> 1)
+#define GET_PF(ctr)		(((ctr) >> 4) & 0x1)
+
+/* MSC macros */
+#define GET_V24_FC(sigs)	((sigs & 0x02) >> 1)
+#define GET_V24_RTC(sigs)	((sigs & 0x04) >> 2)
+#define GET_V24_RTR(sigs)	((sigs & 0x08) >> 3)
+#define GET_V24_IC(sigs)	((sigs & 0x40) >> 6)
+#define GET_V24_DV(sigs)	((sigs & 0x80) >> 7)
+
+#define GET_BRK_SIG1(sigs)	((sigs & 0x02) >> 1)
+#define GET_BRK_SIG2(sigs)	((sigs & 0x04) >> 2)
+#define GET_BRK_SIG3(sigs)	((sigs & 0x08) >> 3)
+#define GET_BRK_SIG_LEN(sigs)	((sigs & 0xf0) >> 4)
 
 struct rfcomm_lhdr {
 	uint8_t address;
@@ -63,6 +76,12 @@ struct rfcomm_lhdr {
 	uint8_t credits; /* only for UIH frame */
 } __attribute__((packed));
 
+struct rfcomm_lmsc {
+	uint8_t dlci;
+	uint8_t v24_sig;
+	uint8_t break_sig;
+} __attribute__((packed));
+
 struct rfcomm_lmcc {
 	uint8_t type;
 	uint16_t length;
@@ -92,6 +111,43 @@ static void print_rfcomm_hdr(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
 	print_field("%*cFCS: 0x%2.2x", indent, ' ', hdr.fcs);
 }
 
+static inline bool mcc_msc(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
+{
+	struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
+	struct rfcomm_lmsc msc;
+
+	if (!l2cap_frame_get_u8(frame, &msc.dlci))
+		return false;
+
+	print_field("%*cdlci %d ", indent, ' ', RFCOMM_GET_DLCI(msc.dlci));
+
+	if (!l2cap_frame_get_u8(frame, &msc.v24_sig))
+		return false;
+
+	/* v24 control signals */
+	print_field("%*cfc %d rtc %d rtr %d ic %d dv %d", indent, ' ',
+		GET_V24_FC(msc.v24_sig), GET_V24_RTC(msc.v24_sig),
+		GET_V24_RTR(msc.v24_sig), GET_V24_IC(msc.v24_sig),
+					GET_V24_DV(msc.v24_sig));
+
+	if (frame->size < 2)
+		goto done;
+
+	/* break signals (optional) */
+
+	if (!l2cap_frame_get_u8(frame, &msc.break_sig))
+		return false;
+
+	printf("%2.2x", msc.break_sig);
+
+	print_field("%*cb1 %d b2 %d b3 %d len %d", indent, ' ',
+		GET_BRK_SIG1(msc.break_sig), GET_BRK_SIG2(msc.break_sig),
+		GET_BRK_SIG3(msc.break_sig), GET_BRK_SIG_LEN(msc.break_sig));
+
+done:
+	return true;
+}
+
 struct mcc_data {
 	uint8_t type;
 	const char *str;
@@ -151,7 +207,13 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
 	print_field("%*cLength: %d", indent+2, ' ', mcc.length);
 
 	rfcomm_frame->mcc = mcc;
-	packet_hexdump(frame->data, frame->size);
+
+	switch (type) {
+	case RFCOMM_MSC:
+		return mcc_msc(rfcomm_frame, indent+2);
+	default:
+		packet_hexdump(frame->data, frame->size);
+	}
 
 	return true;
 }
@@ -225,7 +287,7 @@ void rfcomm_packet(const struct l2cap_frame *frame)
 
 	l2cap_frame_pull(&tmp_frame, l2cap_frame, l2cap_frame->size-1);
 
-	if(!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
+	if (!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
 		goto fail;
 
 	/* Decoding frame type */
-- 
1.9.1

[PATCH 2/5] monitor/rfcomm: Add support for RPN frame decoding

[Gowtham Anandha Babu]

Changes made to decode RPN frame in RFCOMM for btmon.

      RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
         Address: 0x03 cr 1 dlci 0x00
         Control: 0xef poll/final 0
         Length: 10
         FCS: 0x70
         MCC Message type: Remote Port Negotiation Command CMD(0x24)
           Length: 8
           dlci 32
           br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
           rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
           pm 0x3f7f
---
 monitor/rfcomm.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
index 969b29f..5e5545c 100644
--- a/monitor/rfcomm.c
+++ b/monitor/rfcomm.c
@@ -68,6 +68,18 @@ static char *cr_str[] = {
 #define GET_BRK_SIG3(sigs)	((sigs & 0x08) >> 3)
 #define GET_BRK_SIG_LEN(sigs)	((sigs & 0xf0) >> 4)
 
+/* RPN macros */
+#define GET_RPN_DB(parity)	(parity & 0x03)
+#define GET_RPN_SB(parity)	((parity & 0x04) >> 2)
+#define GET_RPN_PARITY(parity)	((parity & 0x08) >> 3)
+#define GET_RPN_PTYPE(parity)	((parity & 0x03) >> 3)
+#define GET_RPN_XIN(io)		(io & 0x01)
+#define GET_RPN_XOUT(io)	((io & 0x02) >> 1)
+#define GET_RPN_RTRI(io)	((io & 0x04) >> 2)
+#define GET_RPN_RTRO(io)	((io & 0x08) >> 3)
+#define GET_RPN_RTCI(io)	((io & 0x10) >> 4)
+#define GET_RPN_RTCO(io)	((io & 0x20) >> 5)
+
 struct rfcomm_lhdr {
 	uint8_t address;
 	uint8_t control;
@@ -82,6 +94,16 @@ struct rfcomm_lmsc {
 	uint8_t break_sig;
 } __attribute__((packed));
 
+struct rfcomm_rpn {
+	uint8_t dlci;
+	uint8_t bit_rate;
+	uint8_t parity;
+	uint8_t io;
+	uint8_t xon;
+	uint8_t xoff;
+	uint16_t pm;
+} __attribute__ ((packed));
+
 struct rfcomm_lmcc {
 	uint8_t type;
 	uint16_t length;
@@ -148,6 +170,55 @@ done:
 	return true;
 }
 
+static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
+{
+	struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
+	struct rfcomm_rpn rpn;
+
+	if (!l2cap_frame_get_u8(frame, &rpn.dlci))
+		return false;
+
+	print_field("%*cdlci %d", indent, ' ', RFCOMM_GET_DLCI(rpn.dlci));
+
+	if (frame->size < 7)
+		goto done;
+
+	/* port value octets (optional) */
+
+	if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
+		return false;
+
+	if (!l2cap_frame_get_u8(frame, &rpn.parity))
+		return false;
+
+	if (!l2cap_frame_get_u8(frame, &rpn.io))
+		return false;
+
+	print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo %d", indent, ' ',
+		rpn.bit_rate, GET_RPN_DB(rpn.parity), GET_RPN_SB(rpn.parity),
+		GET_RPN_PARITY(rpn.parity), GET_RPN_PTYPE(rpn.parity),
+		GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
+
+	if (!l2cap_frame_get_u8(frame, &rpn.xon))
+		return false;
+
+	if (!l2cap_frame_get_u8(frame, &rpn.xoff))
+		return false;
+
+	print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff %d",
+		indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
+		GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
+		rpn.xoff);
+
+	if (!l2cap_frame_get_be16(frame, &rpn.pm))
+		return false;
+
+	print_field("%*cpm 0x%04x", indent, ' ', __bswap_16(rpn.pm));
+
+done:
+	return true;
+}
+
 struct mcc_data {
 	uint8_t type;
 	const char *str;
@@ -211,6 +282,8 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
 	switch (type) {
 	case RFCOMM_MSC:
 		return mcc_msc(rfcomm_frame, indent+2);
+	case RFCOMM_RPN:
+		return mcc_rpn(rfcomm_frame, indent+2);
 	default:
 		packet_hexdump(frame->data, frame->size);
 	}
-- 
1.9.1

[PATCH 3/5] monitor/rfcomm: Add support for RLS frame decoding

[Gowtham Anandha Babu]

Changes made to decode RLS frame in RFCOMM for btmon.

      RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
         Address: 0x03 cr 1 dlci 0x00
         Control: 0xef poll/final 0
         Length: 4
         FCS: 0x70
         MCC Message type: Remote Line Status CMD(0x14)
           Length: 2
           dlci 32 error: 5
---
 monitor/rfcomm.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
index 5e5545c..c934b3c 100644
--- a/monitor/rfcomm.c
+++ b/monitor/rfcomm.c
@@ -80,6 +80,9 @@ static char *cr_str[] = {
 #define GET_RPN_RTCI(io)	((io & 0x10) >> 4)
 #define GET_RPN_RTCO(io)	((io & 0x20) >> 5)
 
+/* RLS macro */
+#define GET_ERROR(err)		(err & 0x0f)
+
 struct rfcomm_lhdr {
 	uint8_t address;
 	uint8_t control;
@@ -104,6 +107,12 @@ struct rfcomm_rpn {
 	uint16_t pm;
 } __attribute__ ((packed));
 
+struct rfcomm_rls {
+	uint8_t dlci;
+	uint8_t error;
+} __attribute__((packed));
+
+
 struct rfcomm_lmcc {
 	uint8_t type;
 	uint16_t length;
@@ -219,6 +228,24 @@ done:
 	return true;
 }
 
+static inline bool mcc_rls(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
+{
+	struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
+	struct rfcomm_rls rls;
+
+	if (!l2cap_frame_get_u8(frame, &rls.dlci))
+		return false;
+
+	if (!l2cap_frame_get_u8(frame, &rls.error))
+		return false;
+
+	print_field("%*cdlci %d error: %d", indent, ' ',
+			RFCOMM_GET_DLCI(rls.dlci), GET_ERROR(rls.error));
+
+	return true;
+}
+
+
 struct mcc_data {
 	uint8_t type;
 	const char *str;
@@ -284,6 +311,8 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
 		return mcc_msc(rfcomm_frame, indent+2);
 	case RFCOMM_RPN:
 		return mcc_rpn(rfcomm_frame, indent+2);
+	case RFCOMM_RLS:
+		return mcc_rls(rfcomm_frame, indent+2);
 	default:
 		packet_hexdump(frame->data, frame->size);
 	}
-- 
1.9.1

[PATCH 4/5] monitor/rfcomm: Add support for PN frame decoding

[Gowtham Anandha Babu]

Changes made to decode PN frame in RFCOMM for btmon.

      RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
         Address: 0x01 cr 0 dlci 0x00
         Control: 0xef poll/final 0
         Length: 10
         FCS: 0xaa
         MCC Message type: DLC Parameter Negotiation RSP(0x20)
           Length: 8
           dlci 32 frame_type 0 credit_flow 14 pri 39
           ack_timer 0 frame_size 666 max_retrans 0 credits 7
---
 monitor/rfcomm.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
index c934b3c..b448527 100644
--- a/monitor/rfcomm.c
+++ b/monitor/rfcomm.c
@@ -83,6 +83,12 @@ static char *cr_str[] = {
 /* RLS macro */
 #define GET_ERROR(err)		(err & 0x0f)
 
+/* PN macros */
+#define GET_FRM_TYPE(ctrl)	((ctrl & 0x0f))
+#define GET_CRT_FLOW(ctrl)	((ctrl & 0xf0) >> 4)
+#define GET_PRIORITY(prio)	((prio & 0x3f))
+#define GET_PN_DLCI(dlci)	((dlci & 0x3f))
+
 struct rfcomm_lhdr {
 	uint8_t address;
 	uint8_t control;
@@ -245,6 +251,45 @@ static inline bool mcc_rls(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
 	return true;
 }
 
+static inline bool mcc_pn(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
+{
+	struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
+	struct rfcomm_pn pn;
+
+	/* rfcomm_pn struct is defined in rfcomm.h */
+
+	if (!l2cap_frame_get_u8(frame, &pn.dlci))
+		return false;
+
+	if (!l2cap_frame_get_u8(frame, &pn.flow_ctrl))
+		return false;
+
+	if (!l2cap_frame_get_u8(frame, &pn.priority))
+		return false;
+
+	print_field("%*cdlci %d frame_type %d credit_flow %d pri %d", indent,
+			' ', GET_PN_DLCI(pn.dlci), GET_FRM_TYPE(pn.flow_ctrl),
+			GET_CRT_FLOW(pn.flow_ctrl), GET_PRIORITY(pn.priority));
+
+	if (!l2cap_frame_get_u8(frame, &pn.ack_timer))
+		return false;
+
+	if (!l2cap_frame_get_be16(frame, &pn.mtu))
+		return false;
+
+	if (!l2cap_frame_get_u8(frame, &pn.max_retrans))
+		return false;
+
+	if (!l2cap_frame_get_u8(frame, &pn.credits))
+		return false;
+
+	print_field("%*cack_timer %d frame_size %d max_retrans %d credits %d",
+			indent, ' ', pn.ack_timer, __bswap_16(pn.mtu),
+			pn.max_retrans, pn.credits);
+
+	return true;
+}
+
 
 struct mcc_data {
 	uint8_t type;
@@ -313,6 +358,8 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
 		return mcc_rpn(rfcomm_frame, indent+2);
 	case RFCOMM_RLS:
 		return mcc_rls(rfcomm_frame, indent+2);
+	case RFCOMM_PN:
+		return mcc_pn(rfcomm_frame, indent+2);
 	default:
 		packet_hexdump(frame->data, frame->size);
 	}
-- 
1.9.1

[PATCH 5/5] monitor/rfcomm: Add support for NSC frame decoding

[Gowtham Anandha Babu]

Changes made to decode NSC frame in RFCOMM for btmon.
Not able capture the output.
---
 monitor/rfcomm.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
index b448527..69ca195 100644
--- a/monitor/rfcomm.c
+++ b/monitor/rfcomm.c
@@ -118,6 +118,9 @@ struct rfcomm_rls {
 	uint8_t error;
 } __attribute__((packed));
 
+struct rfcomm_nsc {
+	uint8_t cmd_type;
+} __attribute__((packed));
 
 struct rfcomm_lmcc {
 	uint8_t type;
@@ -290,6 +293,19 @@ static inline bool mcc_pn(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
 	return true;
 }
 
+static inline bool mcc_nsc(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
+{
+	struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
+	struct rfcomm_nsc nsc;
+
+	if (!l2cap_frame_get_u8(frame, &nsc.cmd_type))
+		return false;
+
+	print_field("%*ccr %d, mcc_cmd_type %x", indent, ' ',
+		GET_CR(nsc.cmd_type), RFCOMM_GET_MCC_TYPE(nsc.cmd_type));
+
+	return true;
+}
 
 struct mcc_data {
 	uint8_t type;
@@ -360,6 +376,8 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
 		return mcc_rls(rfcomm_frame, indent+2);
 	case RFCOMM_PN:
 		return mcc_pn(rfcomm_frame, indent+2);
+	case RFCOMM_NSC:
+		return mcc_nsc(rfcomm_frame, indent+2);
 	default:
 		packet_hexdump(frame->data, frame->size);
 	}
-- 
1.9.1

Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame decoding

[Luiz Augusto von Dentz]

Hi Gowtham,

On Tue, Dec 2, 2014 at 11:37 AM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Changes made to decode MSC frame in RFCOMM for btmon.
>
>       RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
>          Address: 0x01 cr 0 dlci 0x00
>          Control: 0xef poll/final 0
>          Length: 4
>          FCS: 0xaa
>          MCC Message type: Modem Status Command CMD(0x38)
>            Length: 2
>            dlci 32
>            fc 0 rtc 1 rtr 1 ic 0 dv 1
> ---
>  monitor/rfcomm.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 69 insertions(+), 7 deletions(-)
>
> diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
> index b3db98b..969b29f 100644
> --- a/monitor/rfcomm.c
> +++ b/monitor/rfcomm.c
> @@ -49,11 +49,24 @@ static char *cr_str[] = {
>         "CMD"
>  };
>
> -#define CR_STR(type) cr_str[GET_CR(type)]
> -#define GET_LEN8(length) ((length & 0xfe) >> 1)
> -#define GET_LEN16(length) ((length & 0xfffe) >> 1)
> -#define GET_CR(type)   ((type & 0x02) >> 1)
> -#define GET_PF(ctr) (((ctr) >> 4) & 0x1)
> +/* RFCOMM frame parsing macros */
> +#define CR_STR(type)           cr_str[GET_CR(type)]
> +#define GET_LEN8(length)       ((length & 0xfe) >> 1)
> +#define GET_LEN16(length)      ((length & 0xfffe) >> 1)
> +#define GET_CR(type)           ((type & 0x02) >> 1)
> +#define GET_PF(ctr)            (((ctr) >> 4) & 0x1)
> +
> +/* MSC macros */
> +#define GET_V24_FC(sigs)       ((sigs & 0x02) >> 1)
> +#define GET_V24_RTC(sigs)      ((sigs & 0x04) >> 2)
> +#define GET_V24_RTR(sigs)      ((sigs & 0x08) >> 3)
> +#define GET_V24_IC(sigs)       ((sigs & 0x40) >> 6)
> +#define GET_V24_DV(sigs)       ((sigs & 0x80) >> 7)
> +
> +#define GET_BRK_SIG1(sigs)     ((sigs & 0x02) >> 1)
> +#define GET_BRK_SIG2(sigs)     ((sigs & 0x04) >> 2)
> +#define GET_BRK_SIG3(sigs)     ((sigs & 0x08) >> 3)
> +#define GET_BRK_SIG_LEN(sigs)  ((sigs & 0xf0) >> 4)
>
>  struct rfcomm_lhdr {
>         uint8_t address;
> @@ -63,6 +76,12 @@ struct rfcomm_lhdr {
>         uint8_t credits; /* only for UIH frame */
>  } __attribute__((packed));
>
> +struct rfcomm_lmsc {
> +       uint8_t dlci;
> +       uint8_t v24_sig;
> +       uint8_t break_sig;
> +} __attribute__((packed));
> +
>  struct rfcomm_lmcc {
>         uint8_t type;
>         uint16_t length;
> @@ -92,6 +111,43 @@ static void print_rfcomm_hdr(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
>         print_field("%*cFCS: 0x%2.2x", indent, ' ', hdr.fcs);
>  }
>
> +static inline bool mcc_msc(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
> +{
> +       struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> +       struct rfcomm_lmsc msc;
> +
> +       if (!l2cap_frame_get_u8(frame, &msc.dlci))
> +               return false;
> +
> +       print_field("%*cdlci %d ", indent, ' ', RFCOMM_GET_DLCI(msc.dlci));
> +
> +       if (!l2cap_frame_get_u8(frame, &msc.v24_sig))
> +               return false;
> +
> +       /* v24 control signals */
> +       print_field("%*cfc %d rtc %d rtr %d ic %d dv %d", indent, ' ',
> +               GET_V24_FC(msc.v24_sig), GET_V24_RTC(msc.v24_sig),
> +               GET_V24_RTR(msc.v24_sig), GET_V24_IC(msc.v24_sig),
> +                                       GET_V24_DV(msc.v24_sig));
> +
> +       if (frame->size < 2)
> +               goto done;
> +
> +       /* break signals (optional) */
> +
> +       if (!l2cap_frame_get_u8(frame, &msc.break_sig))
> +               return false;
> +
> +       printf("%2.2x", msc.break_sig);

Im very suspicious the printf above will break indentation, you better
add a frame that does exercise this code to make sure it is not
happening.

> +       print_field("%*cb1 %d b2 %d b3 %d len %d", indent, ' ',
> +               GET_BRK_SIG1(msc.break_sig), GET_BRK_SIG2(msc.break_sig),
> +               GET_BRK_SIG3(msc.break_sig), GET_BRK_SIG_LEN(msc.break_sig));
> +
> +done:
> +       return true;
> +}
> +
>  struct mcc_data {
>         uint8_t type;
>         const char *str;
> @@ -151,7 +207,13 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
>         print_field("%*cLength: %d", indent+2, ' ', mcc.length);
>
>         rfcomm_frame->mcc = mcc;
> -       packet_hexdump(frame->data, frame->size);
> +
> +       switch (type) {
> +       case RFCOMM_MSC:
> +               return mcc_msc(rfcomm_frame, indent+2);
> +       default:
> +               packet_hexdump(frame->data, frame->size);
> +       }
>
>         return true;
>  }
> @@ -225,7 +287,7 @@ void rfcomm_packet(const struct l2cap_frame *frame)
>
>         l2cap_frame_pull(&tmp_frame, l2cap_frame, l2cap_frame->size-1);
>
> -       if(!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
> +       if (!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
>                 goto fail;
>
>         /* Decoding frame type */
> --
> 1.9.1
>
> --
> 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



-- 
Luiz Augusto von Dentz

RE: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame decoding

[Gowtham Anandha Babu]

Hi Luiz,

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
> Sent: Tuesday, December 02, 2014 4:22 PM
> To: Gowtham Anandha Babu
> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat Panda;
> cpgs@samsung.com
> Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
> decoding
> 
> Hi Gowtham,
> 
> On Tue, Dec 2, 2014 at 11:37 AM, Gowtham Anandha Babu
> <gowtham.ab@samsung.com> wrote:
> > Changes made to decode MSC frame in RFCOMM for btmon.
> >
> >       RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> >          Address: 0x01 cr 0 dlci 0x00
> >          Control: 0xef poll/final 0
> >          Length: 4
> >          FCS: 0xaa
> >          MCC Message type: Modem Status Command CMD(0x38)
> >            Length: 2
> >            dlci 32
> >            fc 0 rtc 1 rtr 1 ic 0 dv 1
> > ---
> >  monitor/rfcomm.c | 76
> > ++++++++++++++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 69 insertions(+), 7 deletions(-)
> >
> > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> > b3db98b..969b29f 100644
> > --- a/monitor/rfcomm.c
> > +++ b/monitor/rfcomm.c
> > @@ -49,11 +49,24 @@ static char *cr_str[] = {
> >         "CMD"
> >  };
> >
> > -#define CR_STR(type) cr_str[GET_CR(type)] -#define GET_LEN8(length)
> > ((length & 0xfe) >> 1) -#define GET_LEN16(length) ((length & 0xfffe)
> > >> 1)
> > -#define GET_CR(type)   ((type & 0x02) >> 1)
> > -#define GET_PF(ctr) (((ctr) >> 4) & 0x1)
> > +/* RFCOMM frame parsing macros */
> > +#define CR_STR(type)           cr_str[GET_CR(type)]
> > +#define GET_LEN8(length)       ((length & 0xfe) >> 1)
> > +#define GET_LEN16(length)      ((length & 0xfffe) >> 1)
> > +#define GET_CR(type)           ((type & 0x02) >> 1)
> > +#define GET_PF(ctr)            (((ctr) >> 4) & 0x1)
> > +
> > +/* MSC macros */
> > +#define GET_V24_FC(sigs)       ((sigs & 0x02) >> 1)
> > +#define GET_V24_RTC(sigs)      ((sigs & 0x04) >> 2)
> > +#define GET_V24_RTR(sigs)      ((sigs & 0x08) >> 3)
> > +#define GET_V24_IC(sigs)       ((sigs & 0x40) >> 6)
> > +#define GET_V24_DV(sigs)       ((sigs & 0x80) >> 7)
> > +
> > +#define GET_BRK_SIG1(sigs)     ((sigs & 0x02) >> 1)
> > +#define GET_BRK_SIG2(sigs)     ((sigs & 0x04) >> 2)
> > +#define GET_BRK_SIG3(sigs)     ((sigs & 0x08) >> 3)
> > +#define GET_BRK_SIG_LEN(sigs)  ((sigs & 0xf0) >> 4)
> >
> >  struct rfcomm_lhdr {
> >         uint8_t address;
> > @@ -63,6 +76,12 @@ struct rfcomm_lhdr {
> >         uint8_t credits; /* only for UIH frame */  }
> > __attribute__((packed));
> >
> > +struct rfcomm_lmsc {
> > +       uint8_t dlci;
> > +       uint8_t v24_sig;
> > +       uint8_t break_sig;
> > +} __attribute__((packed));
> > +
> >  struct rfcomm_lmcc {
> >         uint8_t type;
> >         uint16_t length;
> > @@ -92,6 +111,43 @@ static void print_rfcomm_hdr(struct rfcomm_frame
> *rfcomm_frame, uint8_t indent)
> >         print_field("%*cFCS: 0x%2.2x", indent, ' ', hdr.fcs);  }
> >
> > +static inline bool mcc_msc(struct rfcomm_frame *rfcomm_frame, uint8_t
> > +indent) {
> > +       struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> > +       struct rfcomm_lmsc msc;
> > +
> > +       if (!l2cap_frame_get_u8(frame, &msc.dlci))
> > +               return false;
> > +
> > +       print_field("%*cdlci %d ", indent, ' ',
> > + RFCOMM_GET_DLCI(msc.dlci));
> > +
> > +       if (!l2cap_frame_get_u8(frame, &msc.v24_sig))
> > +               return false;
> > +
> > +       /* v24 control signals */
> > +       print_field("%*cfc %d rtc %d rtr %d ic %d dv %d", indent, ' ',
> > +               GET_V24_FC(msc.v24_sig), GET_V24_RTC(msc.v24_sig),
> > +               GET_V24_RTR(msc.v24_sig), GET_V24_IC(msc.v24_sig),
> > +                                       GET_V24_DV(msc.v24_sig));
> > +
> > +       if (frame->size < 2)
> > +               goto done;
> > +
> > +       /* break signals (optional) */
> > +
> > +       if (!l2cap_frame_get_u8(frame, &msc.break_sig))
> > +               return false;
> > +
> > +       printf("%2.2x", msc.break_sig);
> 
> Im very suspicious the printf above will break indentation, you better add a
> frame that does exercise this code to make sure it is not happening.
> 

That printf was added to cross check the break signals.
I Forgot to remove that line. Will update in v2.

> > +       print_field("%*cb1 %d b2 %d b3 %d len %d", indent, ' ',
> > +               GET_BRK_SIG1(msc.break_sig), GET_BRK_SIG2(msc.break_sig),
> > +               GET_BRK_SIG3(msc.break_sig),
> > + GET_BRK_SIG_LEN(msc.break_sig));
> > +
> > +done:
> > +       return true;
> > +}
> > +
> >  struct mcc_data {
> >         uint8_t type;
> >         const char *str;
> > @@ -151,7 +207,13 @@ static inline bool mcc_frame(struct rfcomm_frame
> *rfcomm_frame, uint8_t indent)
> >         print_field("%*cLength: %d", indent+2, ' ', mcc.length);
> >
> >         rfcomm_frame->mcc = mcc;
> > -       packet_hexdump(frame->data, frame->size);
> > +
> > +       switch (type) {
> > +       case RFCOMM_MSC:
> > +               return mcc_msc(rfcomm_frame, indent+2);
> > +       default:
> > +               packet_hexdump(frame->data, frame->size);
> > +       }
> >
> >         return true;
> >  }
> > @@ -225,7 +287,7 @@ void rfcomm_packet(const struct l2cap_frame
> > *frame)
> >
> >         l2cap_frame_pull(&tmp_frame, l2cap_frame,
> > l2cap_frame->size-1);
> >
> > -       if(!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
> > +       if (!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
> >                 goto fail;
> >
> >         /* Decoding frame type */
> > --
> > 1.9.1
> >
> > --
> > 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
> 
> 
> 
> --
> Luiz Augusto von Dentz
> --
> 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

Regards,
Gowtham Anandha Babu

RE: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame decoding

[Gowtham Anandha Babu]

Hi Luiz,

> -----Original Message-----
> From: Gowtham Anandha Babu [mailto:gowtham.ab@samsung.com]
> Sent: Tuesday, December 02, 2014 4:30 PM
> To: 'Luiz Augusto von Dentz'
> Cc: 'linux-bluetooth@vger.kernel.org'; 'Dmitry Kasatkin'; 'Bharat Panda';
> 'cpgs@samsung.com'
> Subject: RE: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
> decoding
> 
> Hi Luiz,
> 
> > -----Original Message-----
> > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> > owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
> > Sent: Tuesday, December 02, 2014 4:22 PM
> > To: Gowtham Anandha Babu
> > Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat Panda;
> > cpgs@samsung.com
> > Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
> > decoding
> >
> > Hi Gowtham,
> >
> > On Tue, Dec 2, 2014 at 11:37 AM, Gowtham Anandha Babu
> > <gowtham.ab@samsung.com> wrote:
> > > Changes made to decode MSC frame in RFCOMM for btmon.
> > >
> > >       RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > >          Address: 0x01 cr 0 dlci 0x00
> > >          Control: 0xef poll/final 0
> > >          Length: 4
> > >          FCS: 0xaa
> > >          MCC Message type: Modem Status Command CMD(0x38)
> > >            Length: 2
> > >            dlci 32
> > >            fc 0 rtc 1 rtr 1 ic 0 dv 1
> > > ---
> > >  monitor/rfcomm.c | 76
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++------
> > >  1 file changed, 69 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> > > b3db98b..969b29f 100644
> > > --- a/monitor/rfcomm.c
> > > +++ b/monitor/rfcomm.c
> > > @@ -49,11 +49,24 @@ static char *cr_str[] = {
> > >         "CMD"
> > >  };
> > >
> > > -#define CR_STR(type) cr_str[GET_CR(type)] -#define GET_LEN8(length)
> > > ((length & 0xfe) >> 1) -#define GET_LEN16(length) ((length & 0xfffe)
> > > >> 1)
> > > -#define GET_CR(type)   ((type & 0x02) >> 1)
> > > -#define GET_PF(ctr) (((ctr) >> 4) & 0x1)
> > > +/* RFCOMM frame parsing macros */
> > > +#define CR_STR(type)           cr_str[GET_CR(type)]
> > > +#define GET_LEN8(length)       ((length & 0xfe) >> 1)
> > > +#define GET_LEN16(length)      ((length & 0xfffe) >> 1)
> > > +#define GET_CR(type)           ((type & 0x02) >> 1)
> > > +#define GET_PF(ctr)            (((ctr) >> 4) & 0x1)
> > > +
> > > +/* MSC macros */
> > > +#define GET_V24_FC(sigs)       ((sigs & 0x02) >> 1)
> > > +#define GET_V24_RTC(sigs)      ((sigs & 0x04) >> 2)
> > > +#define GET_V24_RTR(sigs)      ((sigs & 0x08) >> 3)
> > > +#define GET_V24_IC(sigs)       ((sigs & 0x40) >> 6)
> > > +#define GET_V24_DV(sigs)       ((sigs & 0x80) >> 7)
> > > +
> > > +#define GET_BRK_SIG1(sigs)     ((sigs & 0x02) >> 1)
> > > +#define GET_BRK_SIG2(sigs)     ((sigs & 0x04) >> 2)
> > > +#define GET_BRK_SIG3(sigs)     ((sigs & 0x08) >> 3)
> > > +#define GET_BRK_SIG_LEN(sigs)  ((sigs & 0xf0) >> 4)
> > >
> > >  struct rfcomm_lhdr {
> > >         uint8_t address;
> > > @@ -63,6 +76,12 @@ struct rfcomm_lhdr {
> > >         uint8_t credits; /* only for UIH frame */  }
> > > __attribute__((packed));
> > >
> > > +struct rfcomm_lmsc {
> > > +       uint8_t dlci;
> > > +       uint8_t v24_sig;
> > > +       uint8_t break_sig;
> > > +} __attribute__((packed));
> > > +
> > >  struct rfcomm_lmcc {
> > >         uint8_t type;
> > >         uint16_t length;
> > > @@ -92,6 +111,43 @@ static void print_rfcomm_hdr(struct
> rfcomm_frame
> > *rfcomm_frame, uint8_t indent)
> > >         print_field("%*cFCS: 0x%2.2x", indent, ' ', hdr.fcs);  }
> > >
> > > +static inline bool mcc_msc(struct rfcomm_frame *rfcomm_frame,
> > > +uint8_t
> > > +indent) {
> > > +       struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> > > +       struct rfcomm_lmsc msc;
> > > +
> > > +       if (!l2cap_frame_get_u8(frame, &msc.dlci))
> > > +               return false;
> > > +
> > > +       print_field("%*cdlci %d ", indent, ' ',
> > > + RFCOMM_GET_DLCI(msc.dlci));
> > > +
> > > +       if (!l2cap_frame_get_u8(frame, &msc.v24_sig))
> > > +               return false;
> > > +
> > > +       /* v24 control signals */
> > > +       print_field("%*cfc %d rtc %d rtr %d ic %d dv %d", indent, ' ',
> > > +               GET_V24_FC(msc.v24_sig), GET_V24_RTC(msc.v24_sig),
> > > +               GET_V24_RTR(msc.v24_sig), GET_V24_IC(msc.v24_sig),
> > > +                                       GET_V24_DV(msc.v24_sig));
> > > +
> > > +       if (frame->size < 2)
> > > +               goto done;
> > > +
> > > +       /* break signals (optional) */
> > > +
> > > +       if (!l2cap_frame_get_u8(frame, &msc.break_sig))
> > > +               return false;
> > > +
> > > +       printf("%2.2x", msc.break_sig);
> >
> > Im very suspicious the printf above will break indentation, you better
> > add a frame that does exercise this code to make sure it is not happening.
> >
> 
> That printf was added to cross check the break signals.
> I Forgot to remove that line. Will update in v2.
> 

I tried testing with many test cases, not able to capture the break signals.
Any suggestions to capture the break signals?
Btw the implementation here is exactly same as in hcidump.

> > > +       print_field("%*cb1 %d b2 %d b3 %d len %d", indent, ' ',
> > > +               GET_BRK_SIG1(msc.break_sig), GET_BRK_SIG2(msc.break_sig),
> > > +               GET_BRK_SIG3(msc.break_sig),
> > > + GET_BRK_SIG_LEN(msc.break_sig));
> > > +
> > > +done:
> > > +       return true;
> > > +}
> > > +
> > >  struct mcc_data {
> > >         uint8_t type;
> > >         const char *str;
> > > @@ -151,7 +207,13 @@ static inline bool mcc_frame(struct
> > > rfcomm_frame
> > *rfcomm_frame, uint8_t indent)
> > >         print_field("%*cLength: %d", indent+2, ' ', mcc.length);
> > >
> > >         rfcomm_frame->mcc = mcc;
> > > -       packet_hexdump(frame->data, frame->size);
> > > +
> > > +       switch (type) {
> > > +       case RFCOMM_MSC:
> > > +               return mcc_msc(rfcomm_frame, indent+2);
> > > +       default:
> > > +               packet_hexdump(frame->data, frame->size);
> > > +       }
> > >
> > >         return true;
> > >  }
> > > @@ -225,7 +287,7 @@ void rfcomm_packet(const struct l2cap_frame
> > > *frame)
> > >
> > >         l2cap_frame_pull(&tmp_frame, l2cap_frame,
> > > l2cap_frame->size-1);
> > >
> > > -       if(!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
> > > +       if (!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
> > >                 goto fail;
> > >
> > >         /* Decoding frame type */
> > > --
> > > 1.9.1
> > >
> > > --
> > > 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
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
> > --
> > 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
> 
> Regards,
> Gowtham Anandha Babu

Regards,
Gowtham Anandha Babu

Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame decoding

[Luiz Augusto von Dentz]

Hi Gowtham,

On Tue, Dec 2, 2014 at 2:37 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Hi Luiz,
>
>> -----Original Message-----
>> From: Gowtham Anandha Babu [mailto:gowtham.ab@samsung.com]
>> Sent: Tuesday, December 02, 2014 4:30 PM
>> To: 'Luiz Augusto von Dentz'
>> Cc: 'linux-bluetooth@vger.kernel.org'; 'Dmitry Kasatkin'; 'Bharat Panda';
>> 'cpgs@samsung.com'
>> Subject: RE: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
>> decoding
>>
>> Hi Luiz,
>>
>> > -----Original Message-----
>> > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> > owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
>> > Sent: Tuesday, December 02, 2014 4:22 PM
>> > To: Gowtham Anandha Babu
>> > Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat Panda;
>> > cpgs@samsung.com
>> > Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
>> > decoding
>> >
>> > Hi Gowtham,
>> >
>> > On Tue, Dec 2, 2014 at 11:37 AM, Gowtham Anandha Babu
>> > <gowtham.ab@samsung.com> wrote:
>> > > Changes made to decode MSC frame in RFCOMM for btmon.
>> > >
>> > >       RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
>> > >          Address: 0x01 cr 0 dlci 0x00
>> > >          Control: 0xef poll/final 0
>> > >          Length: 4
>> > >          FCS: 0xaa
>> > >          MCC Message type: Modem Status Command CMD(0x38)
>> > >            Length: 2
>> > >            dlci 32
>> > >            fc 0 rtc 1 rtr 1 ic 0 dv 1
>> > > ---
>> > >  monitor/rfcomm.c | 76
>> > > ++++++++++++++++++++++++++++++++++++++++++++++++++------
>> > >  1 file changed, 69 insertions(+), 7 deletions(-)
>> > >
>> > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
>> > > b3db98b..969b29f 100644
>> > > --- a/monitor/rfcomm.c
>> > > +++ b/monitor/rfcomm.c
>> > > @@ -49,11 +49,24 @@ static char *cr_str[] = {
>> > >         "CMD"
>> > >  };
>> > >
>> > > -#define CR_STR(type) cr_str[GET_CR(type)] -#define GET_LEN8(length)
>> > > ((length & 0xfe) >> 1) -#define GET_LEN16(length) ((length & 0xfffe)
>> > > >> 1)
>> > > -#define GET_CR(type)   ((type & 0x02) >> 1)
>> > > -#define GET_PF(ctr) (((ctr) >> 4) & 0x1)
>> > > +/* RFCOMM frame parsing macros */
>> > > +#define CR_STR(type)           cr_str[GET_CR(type)]
>> > > +#define GET_LEN8(length)       ((length & 0xfe) >> 1)
>> > > +#define GET_LEN16(length)      ((length & 0xfffe) >> 1)
>> > > +#define GET_CR(type)           ((type & 0x02) >> 1)
>> > > +#define GET_PF(ctr)            (((ctr) >> 4) & 0x1)
>> > > +
>> > > +/* MSC macros */
>> > > +#define GET_V24_FC(sigs)       ((sigs & 0x02) >> 1)
>> > > +#define GET_V24_RTC(sigs)      ((sigs & 0x04) >> 2)
>> > > +#define GET_V24_RTR(sigs)      ((sigs & 0x08) >> 3)
>> > > +#define GET_V24_IC(sigs)       ((sigs & 0x40) >> 6)
>> > > +#define GET_V24_DV(sigs)       ((sigs & 0x80) >> 7)
>> > > +
>> > > +#define GET_BRK_SIG1(sigs)     ((sigs & 0x02) >> 1)
>> > > +#define GET_BRK_SIG2(sigs)     ((sigs & 0x04) >> 2)
>> > > +#define GET_BRK_SIG3(sigs)     ((sigs & 0x08) >> 3)
>> > > +#define GET_BRK_SIG_LEN(sigs)  ((sigs & 0xf0) >> 4)
>> > >
>> > >  struct rfcomm_lhdr {
>> > >         uint8_t address;
>> > > @@ -63,6 +76,12 @@ struct rfcomm_lhdr {
>> > >         uint8_t credits; /* only for UIH frame */  }
>> > > __attribute__((packed));
>> > >
>> > > +struct rfcomm_lmsc {
>> > > +       uint8_t dlci;
>> > > +       uint8_t v24_sig;
>> > > +       uint8_t break_sig;
>> > > +} __attribute__((packed));
>> > > +
>> > >  struct rfcomm_lmcc {
>> > >         uint8_t type;
>> > >         uint16_t length;
>> > > @@ -92,6 +111,43 @@ static void print_rfcomm_hdr(struct
>> rfcomm_frame
>> > *rfcomm_frame, uint8_t indent)
>> > >         print_field("%*cFCS: 0x%2.2x", indent, ' ', hdr.fcs);  }
>> > >
>> > > +static inline bool mcc_msc(struct rfcomm_frame *rfcomm_frame,
>> > > +uint8_t
>> > > +indent) {
>> > > +       struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
>> > > +       struct rfcomm_lmsc msc;
>> > > +
>> > > +       if (!l2cap_frame_get_u8(frame, &msc.dlci))
>> > > +               return false;
>> > > +
>> > > +       print_field("%*cdlci %d ", indent, ' ',
>> > > + RFCOMM_GET_DLCI(msc.dlci));
>> > > +
>> > > +       if (!l2cap_frame_get_u8(frame, &msc.v24_sig))
>> > > +               return false;
>> > > +
>> > > +       /* v24 control signals */
>> > > +       print_field("%*cfc %d rtc %d rtr %d ic %d dv %d", indent, ' ',
>> > > +               GET_V24_FC(msc.v24_sig), GET_V24_RTC(msc.v24_sig),
>> > > +               GET_V24_RTR(msc.v24_sig), GET_V24_IC(msc.v24_sig),
>> > > +                                       GET_V24_DV(msc.v24_sig));
>> > > +
>> > > +       if (frame->size < 2)
>> > > +               goto done;
>> > > +
>> > > +       /* break signals (optional) */
>> > > +
>> > > +       if (!l2cap_frame_get_u8(frame, &msc.break_sig))
>> > > +               return false;
>> > > +
>> > > +       printf("%2.2x", msc.break_sig);
>> >
>> > Im very suspicious the printf above will break indentation, you better
>> > add a frame that does exercise this code to make sure it is not happening.
>> >
>>
>> That printf was added to cross check the break signals.
>> I Forgot to remove that line. Will update in v2.
>>
>
> I tried testing with many test cases, not able to capture the break signals.
> Any suggestions to capture the break signals?
> Btw the implementation here is exactly same as in hcidump.

Perhaps you can try some test case with PTS, please check the TS if
there is a TC that tests break signals.

>> > > +       print_field("%*cb1 %d b2 %d b3 %d len %d", indent, ' ',
>> > > +               GET_BRK_SIG1(msc.break_sig), GET_BRK_SIG2(msc.break_sig),
>> > > +               GET_BRK_SIG3(msc.break_sig),
>> > > + GET_BRK_SIG_LEN(msc.break_sig));
>> > > +
>> > > +done:
>> > > +       return true;
>> > > +}
>> > > +
>> > >  struct mcc_data {
>> > >         uint8_t type;
>> > >         const char *str;
>> > > @@ -151,7 +207,13 @@ static inline bool mcc_frame(struct
>> > > rfcomm_frame
>> > *rfcomm_frame, uint8_t indent)
>> > >         print_field("%*cLength: %d", indent+2, ' ', mcc.length);
>> > >
>> > >         rfcomm_frame->mcc = mcc;
>> > > -       packet_hexdump(frame->data, frame->size);
>> > > +
>> > > +       switch (type) {
>> > > +       case RFCOMM_MSC:
>> > > +               return mcc_msc(rfcomm_frame, indent+2);
>> > > +       default:
>> > > +               packet_hexdump(frame->data, frame->size);
>> > > +       }
>> > >
>> > >         return true;
>> > >  }
>> > > @@ -225,7 +287,7 @@ void rfcomm_packet(const struct l2cap_frame
>> > > *frame)
>> > >
>> > >         l2cap_frame_pull(&tmp_frame, l2cap_frame,
>> > > l2cap_frame->size-1);
>> > >
>> > > -       if(!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
>> > > +       if (!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
>> > >                 goto fail;
>> > >
>> > >         /* Decoding frame type */
>> > > --
>> > > 1.9.1
>> > >
>> > > --
>> > > 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
>> >
>> >
>> >
>> > --
>> > Luiz Augusto von Dentz
>> > --
>> > 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
>>
>> Regards,
>> Gowtham Anandha Babu
>
> Regards,
> Gowtham Anandha Babu
>



-- 
Luiz Augusto von Dentz

RE: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame decoding

[Gowtham Anandha Babu]

Hi Luiz,

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
> Sent: Tuesday, December 02, 2014 6:11 PM
> To: Gowtham Anandha Babu
> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat Panda;
> cpgs@samsung.com
> Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
> decoding
> 
> Hi Gowtham,
> 
> On Tue, Dec 2, 2014 at 2:37 PM, Gowtham Anandha Babu
> <gowtham.ab@samsung.com> wrote:
> > Hi Luiz,
> >
> >> -----Original Message-----
> >> From: Gowtham Anandha Babu [mailto:gowtham.ab@samsung.com]
> >> Sent: Tuesday, December 02, 2014 4:30 PM
> >> To: 'Luiz Augusto von Dentz'
> >> Cc: 'linux-bluetooth@vger.kernel.org'; 'Dmitry Kasatkin'; 'Bharat
> >> Panda'; 'cpgs@samsung.com'
> >> Subject: RE: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
> >> decoding
> >>
> >> Hi Luiz,
> >>
> >> > -----Original Message-----
> >> > From: linux-bluetooth-owner@vger.kernel.org
> >> > [mailto:linux-bluetooth- owner@vger.kernel.org] On Behalf Of Luiz
> >> > Augusto von Dentz
> >> > Sent: Tuesday, December 02, 2014 4:22 PM
> >> > To: Gowtham Anandha Babu
> >> > Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat Panda;
> >> > cpgs@samsung.com
> >> > Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC
> >> > frame decoding
> >> >
> >> > Hi Gowtham,
> >> >
> >> > On Tue, Dec 2, 2014 at 11:37 AM, Gowtham Anandha Babu
> >> > <gowtham.ab@samsung.com> wrote:
> >> > > Changes made to decode MSC frame in RFCOMM for btmon.
> >> > >
> >> > >       RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> >> > >          Address: 0x01 cr 0 dlci 0x00
> >> > >          Control: 0xef poll/final 0
> >> > >          Length: 4
> >> > >          FCS: 0xaa
> >> > >          MCC Message type: Modem Status Command CMD(0x38)
> >> > >            Length: 2
> >> > >            dlci 32
> >> > >            fc 0 rtc 1 rtr 1 ic 0 dv 1
> >> > > ---
> >> > >  monitor/rfcomm.c | 76
> >> > > ++++++++++++++++++++++++++++++++++++++++++++++++++-----
> -
> >> > >  1 file changed, 69 insertions(+), 7 deletions(-)
> >> > >
> >> > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> >> > > b3db98b..969b29f 100644
> >> > > --- a/monitor/rfcomm.c
> >> > > +++ b/monitor/rfcomm.c
> >> > > @@ -49,11 +49,24 @@ static char *cr_str[] = {
> >> > >         "CMD"
> >> > >  };
> >> > >
> >> > > -#define CR_STR(type) cr_str[GET_CR(type)] -#define
> >> > > GET_LEN8(length) ((length & 0xfe) >> 1) -#define
> >> > > GET_LEN16(length) ((length & 0xfffe)
> >> > > >> 1)
> >> > > -#define GET_CR(type)   ((type & 0x02) >> 1)
> >> > > -#define GET_PF(ctr) (((ctr) >> 4) & 0x1)
> >> > > +/* RFCOMM frame parsing macros */
> >> > > +#define CR_STR(type)           cr_str[GET_CR(type)]
> >> > > +#define GET_LEN8(length)       ((length & 0xfe) >> 1)
> >> > > +#define GET_LEN16(length)      ((length & 0xfffe) >> 1)
> >> > > +#define GET_CR(type)           ((type & 0x02) >> 1)
> >> > > +#define GET_PF(ctr)            (((ctr) >> 4) & 0x1)
> >> > > +
> >> > > +/* MSC macros */
> >> > > +#define GET_V24_FC(sigs)       ((sigs & 0x02) >> 1)
> >> > > +#define GET_V24_RTC(sigs)      ((sigs & 0x04) >> 2)
> >> > > +#define GET_V24_RTR(sigs)      ((sigs & 0x08) >> 3)
> >> > > +#define GET_V24_IC(sigs)       ((sigs & 0x40) >> 6)
> >> > > +#define GET_V24_DV(sigs)       ((sigs & 0x80) >> 7)
> >> > > +
> >> > > +#define GET_BRK_SIG1(sigs)     ((sigs & 0x02) >> 1)
> >> > > +#define GET_BRK_SIG2(sigs)     ((sigs & 0x04) >> 2)
> >> > > +#define GET_BRK_SIG3(sigs)     ((sigs & 0x08) >> 3)
> >> > > +#define GET_BRK_SIG_LEN(sigs)  ((sigs & 0xf0) >> 4)
> >> > >
> >> > >  struct rfcomm_lhdr {
> >> > >         uint8_t address;
> >> > > @@ -63,6 +76,12 @@ struct rfcomm_lhdr {
> >> > >         uint8_t credits; /* only for UIH frame */  }
> >> > > __attribute__((packed));
> >> > >
> >> > > +struct rfcomm_lmsc {
> >> > > +       uint8_t dlci;
> >> > > +       uint8_t v24_sig;
> >> > > +       uint8_t break_sig;
> >> > > +} __attribute__((packed));
> >> > > +
> >> > >  struct rfcomm_lmcc {
> >> > >         uint8_t type;
> >> > >         uint16_t length;
> >> > > @@ -92,6 +111,43 @@ static void print_rfcomm_hdr(struct
> >> rfcomm_frame
> >> > *rfcomm_frame, uint8_t indent)
> >> > >         print_field("%*cFCS: 0x%2.2x", indent, ' ', hdr.fcs);  }
> >> > >
> >> > > +static inline bool mcc_msc(struct rfcomm_frame *rfcomm_frame,
> >> > > +uint8_t
> >> > > +indent) {
> >> > > +       struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> >> > > +       struct rfcomm_lmsc msc;
> >> > > +
> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.dlci))
> >> > > +               return false;
> >> > > +
> >> > > +       print_field("%*cdlci %d ", indent, ' ',
> >> > > + RFCOMM_GET_DLCI(msc.dlci));
> >> > > +
> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.v24_sig))
> >> > > +               return false;
> >> > > +
> >> > > +       /* v24 control signals */
> >> > > +       print_field("%*cfc %d rtc %d rtr %d ic %d dv %d", indent, ' ',
> >> > > +               GET_V24_FC(msc.v24_sig), GET_V24_RTC(msc.v24_sig),
> >> > > +               GET_V24_RTR(msc.v24_sig), GET_V24_IC(msc.v24_sig),
> >> > > +                                       GET_V24_DV(msc.v24_sig));
> >> > > +
> >> > > +       if (frame->size < 2)
> >> > > +               goto done;
> >> > > +
> >> > > +       /* break signals (optional) */
> >> > > +
> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.break_sig))
> >> > > +               return false;
> >> > > +
> >> > > +       printf("%2.2x", msc.break_sig);
> >> >
> >> > Im very suspicious the printf above will break indentation, you
> >> > better add a frame that does exercise this code to make sure it is not
> happening.
> >> >
> >>
> >> That printf was added to cross check the break signals.
> >> I Forgot to remove that line. Will update in v2.
> >>
> >
> > I tried testing with many test cases, not able to capture the break signals.
> > Any suggestions to capture the break signals?
> > Btw the implementation here is exactly same as in hcidump.
> 
> Perhaps you can try some test case with PTS, please check the TS if there is a
> TC that tests break signals.
> 

I referred the RFCOMM TS and tried all test cases for RFCOMM in PTS, still not able to capture the break signals.
In the RFCOMM specs also, break signal fields are not explained much.
Meanwhile I tried executing the MAP, PBAP, OPP, FTP connection establishment test cases, nothing worked out.

Since this is optional, can we skip this (break signal decoding) for now? Or what do you think?

> >> > > +       print_field("%*cb1 %d b2 %d b3 %d len %d", indent, ' ',
> >> > > +               GET_BRK_SIG1(msc.break_sig),
> GET_BRK_SIG2(msc.break_sig),
> >> > > +               GET_BRK_SIG3(msc.break_sig),
> >> > > + GET_BRK_SIG_LEN(msc.break_sig));
> >> > > +
> >> > > +done:
> >> > > +       return true;
> >> > > +}
> >> > > +
> >> > >  struct mcc_data {
> >> > >         uint8_t type;
> >> > >         const char *str;
> >> > > @@ -151,7 +207,13 @@ static inline bool mcc_frame(struct
> >> > > rfcomm_frame
> >> > *rfcomm_frame, uint8_t indent)
> >> > >         print_field("%*cLength: %d", indent+2, ' ', mcc.length);
> >> > >
> >> > >         rfcomm_frame->mcc = mcc;
> >> > > -       packet_hexdump(frame->data, frame->size);
> >> > > +
> >> > > +       switch (type) {
> >> > > +       case RFCOMM_MSC:
> >> > > +               return mcc_msc(rfcomm_frame, indent+2);
> >> > > +       default:
> >> > > +               packet_hexdump(frame->data, frame->size);
> >> > > +       }
> >> > >
> >> > >         return true;
> >> > >  }
> >> > > @@ -225,7 +287,7 @@ void rfcomm_packet(const struct l2cap_frame
> >> > > *frame)
> >> > >
> >> > >         l2cap_frame_pull(&tmp_frame, l2cap_frame,
> >> > > l2cap_frame->size-1);
> >> > >
> >> > > -       if(!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
> >> > > +       if (!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
> >> > >                 goto fail;
> >> > >
> >> > >         /* Decoding frame type */
> >> > > --
> >> > > 1.9.1
> >> > >
> >> > > --
> >> > > 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
> >> >
> >> >
> >> >
> >> > --
> >> > Luiz Augusto von Dentz
> >> > --
> >> > 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
> >>
> >> Regards,
> >> Gowtham Anandha Babu
> >
> > Regards,
> > Gowtham Anandha Babu
> >
> 
> 
> 
> --
> Luiz Augusto von Dentz
> --
> 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


Regards,
Gowtham Anandha Babu

Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame decoding

[Luiz Augusto von Dentz]

Hi Gowtham,

On Tue, Dec 2, 2014 at 4:39 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Hi Luiz,
>
>> -----Original Message-----
>> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
>> Sent: Tuesday, December 02, 2014 6:11 PM
>> To: Gowtham Anandha Babu
>> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat Panda;
>> cpgs@samsung.com
>> Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
>> decoding
>>
>> Hi Gowtham,
>>
>> On Tue, Dec 2, 2014 at 2:37 PM, Gowtham Anandha Babu
>> <gowtham.ab@samsung.com> wrote:
>> > Hi Luiz,
>> >
>> >> -----Original Message-----
>> >> From: Gowtham Anandha Babu [mailto:gowtham.ab@samsung.com]
>> >> Sent: Tuesday, December 02, 2014 4:30 PM
>> >> To: 'Luiz Augusto von Dentz'
>> >> Cc: 'linux-bluetooth@vger.kernel.org'; 'Dmitry Kasatkin'; 'Bharat
>> >> Panda'; 'cpgs@samsung.com'
>> >> Subject: RE: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
>> >> decoding
>> >>
>> >> Hi Luiz,
>> >>
>> >> > -----Original Message-----
>> >> > From: linux-bluetooth-owner@vger.kernel.org
>> >> > [mailto:linux-bluetooth- owner@vger.kernel.org] On Behalf Of Luiz
>> >> > Augusto von Dentz
>> >> > Sent: Tuesday, December 02, 2014 4:22 PM
>> >> > To: Gowtham Anandha Babu
>> >> > Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat Panda;
>> >> > cpgs@samsung.com
>> >> > Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC
>> >> > frame decoding
>> >> >
>> >> > Hi Gowtham,
>> >> >
>> >> > On Tue, Dec 2, 2014 at 11:37 AM, Gowtham Anandha Babu
>> >> > <gowtham.ab@samsung.com> wrote:
>> >> > > Changes made to decode MSC frame in RFCOMM for btmon.
>> >> > >
>> >> > >       RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
>> >> > >          Address: 0x01 cr 0 dlci 0x00
>> >> > >          Control: 0xef poll/final 0
>> >> > >          Length: 4
>> >> > >          FCS: 0xaa
>> >> > >          MCC Message type: Modem Status Command CMD(0x38)
>> >> > >            Length: 2
>> >> > >            dlci 32
>> >> > >            fc 0 rtc 1 rtr 1 ic 0 dv 1
>> >> > > ---
>> >> > >  monitor/rfcomm.c | 76
>> >> > > ++++++++++++++++++++++++++++++++++++++++++++++++++-----
>> -
>> >> > >  1 file changed, 69 insertions(+), 7 deletions(-)
>> >> > >
>> >> > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
>> >> > > b3db98b..969b29f 100644
>> >> > > --- a/monitor/rfcomm.c
>> >> > > +++ b/monitor/rfcomm.c
>> >> > > @@ -49,11 +49,24 @@ static char *cr_str[] = {
>> >> > >         "CMD"
>> >> > >  };
>> >> > >
>> >> > > -#define CR_STR(type) cr_str[GET_CR(type)] -#define
>> >> > > GET_LEN8(length) ((length & 0xfe) >> 1) -#define
>> >> > > GET_LEN16(length) ((length & 0xfffe)
>> >> > > >> 1)
>> >> > > -#define GET_CR(type)   ((type & 0x02) >> 1)
>> >> > > -#define GET_PF(ctr) (((ctr) >> 4) & 0x1)
>> >> > > +/* RFCOMM frame parsing macros */
>> >> > > +#define CR_STR(type)           cr_str[GET_CR(type)]
>> >> > > +#define GET_LEN8(length)       ((length & 0xfe) >> 1)
>> >> > > +#define GET_LEN16(length)      ((length & 0xfffe) >> 1)
>> >> > > +#define GET_CR(type)           ((type & 0x02) >> 1)
>> >> > > +#define GET_PF(ctr)            (((ctr) >> 4) & 0x1)
>> >> > > +
>> >> > > +/* MSC macros */
>> >> > > +#define GET_V24_FC(sigs)       ((sigs & 0x02) >> 1)
>> >> > > +#define GET_V24_RTC(sigs)      ((sigs & 0x04) >> 2)
>> >> > > +#define GET_V24_RTR(sigs)      ((sigs & 0x08) >> 3)
>> >> > > +#define GET_V24_IC(sigs)       ((sigs & 0x40) >> 6)
>> >> > > +#define GET_V24_DV(sigs)       ((sigs & 0x80) >> 7)
>> >> > > +
>> >> > > +#define GET_BRK_SIG1(sigs)     ((sigs & 0x02) >> 1)
>> >> > > +#define GET_BRK_SIG2(sigs)     ((sigs & 0x04) >> 2)
>> >> > > +#define GET_BRK_SIG3(sigs)     ((sigs & 0x08) >> 3)
>> >> > > +#define GET_BRK_SIG_LEN(sigs)  ((sigs & 0xf0) >> 4)
>> >> > >
>> >> > >  struct rfcomm_lhdr {
>> >> > >         uint8_t address;
>> >> > > @@ -63,6 +76,12 @@ struct rfcomm_lhdr {
>> >> > >         uint8_t credits; /* only for UIH frame */  }
>> >> > > __attribute__((packed));
>> >> > >
>> >> > > +struct rfcomm_lmsc {
>> >> > > +       uint8_t dlci;
>> >> > > +       uint8_t v24_sig;
>> >> > > +       uint8_t break_sig;
>> >> > > +} __attribute__((packed));
>> >> > > +
>> >> > >  struct rfcomm_lmcc {
>> >> > >         uint8_t type;
>> >> > >         uint16_t length;
>> >> > > @@ -92,6 +111,43 @@ static void print_rfcomm_hdr(struct
>> >> rfcomm_frame
>> >> > *rfcomm_frame, uint8_t indent)
>> >> > >         print_field("%*cFCS: 0x%2.2x", indent, ' ', hdr.fcs);  }
>> >> > >
>> >> > > +static inline bool mcc_msc(struct rfcomm_frame *rfcomm_frame,
>> >> > > +uint8_t
>> >> > > +indent) {
>> >> > > +       struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
>> >> > > +       struct rfcomm_lmsc msc;
>> >> > > +
>> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.dlci))
>> >> > > +               return false;
>> >> > > +
>> >> > > +       print_field("%*cdlci %d ", indent, ' ',
>> >> > > + RFCOMM_GET_DLCI(msc.dlci));
>> >> > > +
>> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.v24_sig))
>> >> > > +               return false;
>> >> > > +
>> >> > > +       /* v24 control signals */
>> >> > > +       print_field("%*cfc %d rtc %d rtr %d ic %d dv %d", indent, ' ',
>> >> > > +               GET_V24_FC(msc.v24_sig), GET_V24_RTC(msc.v24_sig),
>> >> > > +               GET_V24_RTR(msc.v24_sig), GET_V24_IC(msc.v24_sig),
>> >> > > +                                       GET_V24_DV(msc.v24_sig));
>> >> > > +
>> >> > > +       if (frame->size < 2)
>> >> > > +               goto done;
>> >> > > +
>> >> > > +       /* break signals (optional) */
>> >> > > +
>> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.break_sig))
>> >> > > +               return false;
>> >> > > +
>> >> > > +       printf("%2.2x", msc.break_sig);
>> >> >
>> >> > Im very suspicious the printf above will break indentation, you
>> >> > better add a frame that does exercise this code to make sure it is not
>> happening.
>> >> >
>> >>
>> >> That printf was added to cross check the break signals.
>> >> I Forgot to remove that line. Will update in v2.
>> >>
>> >
>> > I tried testing with many test cases, not able to capture the break signals.
>> > Any suggestions to capture the break signals?
>> > Btw the implementation here is exactly same as in hcidump.
>>
>> Perhaps you can try some test case with PTS, please check the TS if there is a
>> TC that tests break signals.
>>
>
> I referred the RFCOMM TS and tried all test cases for RFCOMM in PTS, still not able to capture the break signals.
> In the RFCOMM specs also, break signal fields are not explained much.
> Meanwhile I tried executing the MAP, PBAP, OPP, FTP connection establishment test cases, nothing worked out.
>
> Since this is optional, can we skip this (break signal decoding) for now? Or what do you think?

Then skip it and add a TODO to the code.


-- 
Luiz Augusto von Dentz

RE: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame decoding

[Gowtham Anandha Babu]

Hi Luiz,

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
> Sent: Tuesday, December 02, 2014 8:41 PM
> To: Gowtham Anandha Babu
> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat Panda;
> cpgs@samsung.com
> Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
> decoding
> 
> Hi Gowtham,
> 
> On Tue, Dec 2, 2014 at 4:39 PM, Gowtham Anandha Babu
> <gowtham.ab@samsung.com> wrote:
> > Hi Luiz,
> >
> >> -----Original Message-----
> >> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> >> owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
> >> Sent: Tuesday, December 02, 2014 6:11 PM
> >> To: Gowtham Anandha Babu
> >> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat Panda;
> >> cpgs@samsung.com
> >> Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
> >> decoding
> >>
> >> Hi Gowtham,
> >>
> >> On Tue, Dec 2, 2014 at 2:37 PM, Gowtham Anandha Babu
> >> <gowtham.ab@samsung.com> wrote:
> >> > Hi Luiz,
> >> >
> >> >> -----Original Message-----
> >> >> From: Gowtham Anandha Babu [mailto:gowtham.ab@samsung.com]
> >> >> Sent: Tuesday, December 02, 2014 4:30 PM
> >> >> To: 'Luiz Augusto von Dentz'
> >> >> Cc: 'linux-bluetooth@vger.kernel.org'; 'Dmitry Kasatkin'; 'Bharat
> >> >> Panda'; 'cpgs@samsung.com'
> >> >> Subject: RE: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC
> >> >> frame decoding
> >> >>
> >> >> Hi Luiz,
> >> >>
> >> >> > -----Original Message-----
> >> >> > From: linux-bluetooth-owner@vger.kernel.org
> >> >> > [mailto:linux-bluetooth- owner@vger.kernel.org] On Behalf Of
> >> >> > Luiz Augusto von Dentz
> >> >> > Sent: Tuesday, December 02, 2014 4:22 PM
> >> >> > To: Gowtham Anandha Babu
> >> >> > Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat
> >> >> > Panda; cpgs@samsung.com
> >> >> > Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC
> >> >> > frame decoding
> >> >> >
> >> >> > Hi Gowtham,
> >> >> >
> >> >> > On Tue, Dec 2, 2014 at 11:37 AM, Gowtham Anandha Babu
> >> >> > <gowtham.ab@samsung.com> wrote:
> >> >> > > Changes made to decode MSC frame in RFCOMM for btmon.
> >> >> > >
> >> >> > >       RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> >> >> > >          Address: 0x01 cr 0 dlci 0x00
> >> >> > >          Control: 0xef poll/final 0
> >> >> > >          Length: 4
> >> >> > >          FCS: 0xaa
> >> >> > >          MCC Message type: Modem Status Command CMD(0x38)
> >> >> > >            Length: 2
> >> >> > >            dlci 32
> >> >> > >            fc 0 rtc 1 rtr 1 ic 0 dv 1
> >> >> > > ---
> >> >> > >  monitor/rfcomm.c | 76
> >> >> > > ++++++++++++++++++++++++++++++++++++++++++++++++++--
> ---
> >> -
> >> >> > >  1 file changed, 69 insertions(+), 7 deletions(-)
> >> >> > >
> >> >> > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> >> >> > > b3db98b..969b29f 100644
> >> >> > > --- a/monitor/rfcomm.c
> >> >> > > +++ b/monitor/rfcomm.c
> >> >> > > @@ -49,11 +49,24 @@ static char *cr_str[] = {
> >> >> > >         "CMD"
> >> >> > >  };
> >> >> > >
> >> >> > > -#define CR_STR(type) cr_str[GET_CR(type)] -#define
> >> >> > > GET_LEN8(length) ((length & 0xfe) >> 1) -#define
> >> >> > > GET_LEN16(length) ((length & 0xfffe)
> >> >> > > >> 1)
> >> >> > > -#define GET_CR(type)   ((type & 0x02) >> 1)
> >> >> > > -#define GET_PF(ctr) (((ctr) >> 4) & 0x1)
> >> >> > > +/* RFCOMM frame parsing macros */
> >> >> > > +#define CR_STR(type)           cr_str[GET_CR(type)]
> >> >> > > +#define GET_LEN8(length)       ((length & 0xfe) >> 1)
> >> >> > > +#define GET_LEN16(length)      ((length & 0xfffe) >> 1)
> >> >> > > +#define GET_CR(type)           ((type & 0x02) >> 1)
> >> >> > > +#define GET_PF(ctr)            (((ctr) >> 4) & 0x1)
> >> >> > > +
> >> >> > > +/* MSC macros */
> >> >> > > +#define GET_V24_FC(sigs)       ((sigs & 0x02) >> 1)
> >> >> > > +#define GET_V24_RTC(sigs)      ((sigs & 0x04) >> 2)
> >> >> > > +#define GET_V24_RTR(sigs)      ((sigs & 0x08) >> 3)
> >> >> > > +#define GET_V24_IC(sigs)       ((sigs & 0x40) >> 6)
> >> >> > > +#define GET_V24_DV(sigs)       ((sigs & 0x80) >> 7)
> >> >> > > +
> >> >> > > +#define GET_BRK_SIG1(sigs)     ((sigs & 0x02) >> 1)
> >> >> > > +#define GET_BRK_SIG2(sigs)     ((sigs & 0x04) >> 2)
> >> >> > > +#define GET_BRK_SIG3(sigs)     ((sigs & 0x08) >> 3)
> >> >> > > +#define GET_BRK_SIG_LEN(sigs)  ((sigs & 0xf0) >> 4)
> >> >> > >
> >> >> > >  struct rfcomm_lhdr {
> >> >> > >         uint8_t address;
> >> >> > > @@ -63,6 +76,12 @@ struct rfcomm_lhdr {
> >> >> > >         uint8_t credits; /* only for UIH frame */  }
> >> >> > > __attribute__((packed));
> >> >> > >
> >> >> > > +struct rfcomm_lmsc {
> >> >> > > +       uint8_t dlci;
> >> >> > > +       uint8_t v24_sig;
> >> >> > > +       uint8_t break_sig;
> >> >> > > +} __attribute__((packed));
> >> >> > > +
> >> >> > >  struct rfcomm_lmcc {
> >> >> > >         uint8_t type;
> >> >> > >         uint16_t length;
> >> >> > > @@ -92,6 +111,43 @@ static void print_rfcomm_hdr(struct
> >> >> rfcomm_frame
> >> >> > *rfcomm_frame, uint8_t indent)
> >> >> > >         print_field("%*cFCS: 0x%2.2x", indent, ' ', hdr.fcs);
> >> >> > > }
> >> >> > >
> >> >> > > +static inline bool mcc_msc(struct rfcomm_frame *rfcomm_frame,
> >> >> > > +uint8_t
> >> >> > > +indent) {
> >> >> > > +       struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> >> >> > > +       struct rfcomm_lmsc msc;
> >> >> > > +
> >> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.dlci))
> >> >> > > +               return false;
> >> >> > > +
> >> >> > > +       print_field("%*cdlci %d ", indent, ' ',
> >> >> > > + RFCOMM_GET_DLCI(msc.dlci));
> >> >> > > +
> >> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.v24_sig))
> >> >> > > +               return false;
> >> >> > > +
> >> >> > > +       /* v24 control signals */
> >> >> > > +       print_field("%*cfc %d rtc %d rtr %d ic %d dv %d", indent, ' ',
> >> >> > > +               GET_V24_FC(msc.v24_sig), GET_V24_RTC(msc.v24_sig),
> >> >> > > +               GET_V24_RTR(msc.v24_sig), GET_V24_IC(msc.v24_sig),
> >> >> > > +
> >> >> > > + GET_V24_DV(msc.v24_sig));
> >> >> > > +
> >> >> > > +       if (frame->size < 2)
> >> >> > > +               goto done;
> >> >> > > +
> >> >> > > +       /* break signals (optional) */
> >> >> > > +
> >> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.break_sig))
> >> >> > > +               return false;
> >> >> > > +
> >> >> > > +       printf("%2.2x", msc.break_sig);
> >> >> >
> >> >> > Im very suspicious the printf above will break indentation, you
> >> >> > better add a frame that does exercise this code to make sure it
> >> >> > is not
> >> happening.
> >> >> >
> >> >>
> >> >> That printf was added to cross check the break signals.
> >> >> I Forgot to remove that line. Will update in v2.
> >> >>
> >> >
> >> > I tried testing with many test cases, not able to capture the break
> signals.
> >> > Any suggestions to capture the break signals?
> >> > Btw the implementation here is exactly same as in hcidump.
> >>
> >> Perhaps you can try some test case with PTS, please check the TS if
> >> there is a TC that tests break signals.
> >>
> >
> > I referred the RFCOMM TS and tried all test cases for RFCOMM in PTS, still
> not able to capture the break signals.
> > In the RFCOMM specs also, break signal fields are not explained much.
> > Meanwhile I tried executing the MAP, PBAP, OPP, FTP connection
> establishment test cases, nothing worked out.
> >
> > Since this is optional, can we skip this (break signal decoding) for now? Or
> what do you think?
> 
> Then skip it and add a TODO to the code.
> 

I skipped this for now and updated the TODO in v1 and submitted the same.
Please have a look at it.

> 
> --
> Luiz Augusto von Dentz
> --
> 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

Regards,
Gowtham Anandha Babu