RE: [PATCH v2 6/6] monitor/rfcomm: Add mcc type handlers code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

> -----Original Message-----
> From: linux-bluetooth-owner@xxxxxxxxxxxxxxx [mailto:linux-bluetooth-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Gowtham Anandha Babu
> Sent: Wednesday, November 05, 2014 5:55 PM
> To: 'Luiz Augusto von Dentz'
> Cc: linux-bluetooth@xxxxxxxxxxxxxxx; 'Dmitry Kasatkin'; 'Bharat Panda';
> cpgs@xxxxxxxxxxx
> Subject: RE: [PATCH v2 6/6] monitor/rfcomm: Add mcc type handlers code
> 
> Hi,
> 
> > -----Original Message-----
> > From: Luiz Augusto von Dentz [mailto:luiz.dentz@xxxxxxxxx]
> > Sent: Tuesday, November 04, 2014 5:46 PM
> > To: Gowtham Anandha Babu
> > Cc: linux-bluetooth@xxxxxxxxxxxxxxx; Dmitry Kasatkin; Bharat Panda;
> > cpgs@xxxxxxxxxxx
> > Subject: Re: [PATCH v2 6/6] monitor/rfcomm: Add mcc type handlers code
> >
> > Hi,
> >
> > On Mon, Nov 3, 2014 at 5:25 PM, Gowtham Anandha Babu
> > <gowtham.ab@xxxxxxxxxxx> wrote:
> > > From: Bharat Panda <bharat.panda@xxxxxxxxxxx>
> > >
> > > Adds different mcc type handlers code to print in btmon.
> > > ---
> > > Below provided is the btmon rfcomm logs.
> > >> ACL Data RX: Handle 75 flags 0x02 dlen 8                     [hci0] 22.280283
> > >       Channel: 65 len 4 [PSM 3 mode 0] {chan 1}
> > >        RFCOMM(s): SABM
> > >         Address : (0x83)
> > >         CR Bit: 1
> > >         DLCI : (0x20)
> > >         Poll/FInal Bit : 1
> > >         Length : 0
> > >         FCS : (0xca)
> > > < ACL Data TX: Handle 75 flags 0x00 dlen 8                     [hci0] 22.317803
> > >       Channel: 65 len 4 [PSM 3 mode 0] {chan 1}
> > >        RFCOMM(s): UA
> > >         Address : (0x83)
> > >         CR Bit: 1
> > >         DLCI : (0x20)
> > >         Poll/FInal Bit : 1
> > >         Length : 0
> > >         FCS : (0x01)
> > > < ACL Data TX: Handle 75 flags 0x00 dlen 12                    [hci0] 22.317822
> > >       Channel: 65 len 8 [PSM 3 mode 0] {chan 1}
> > >        RFCOMM(s): MSC CMD
> > >         Address : (0x01)
> > >         CR Bit: 0
> > >         DLCI : (0x00)
> > >         Poll/FInal Bit : 0
> > >         Length : 4
> > >         FCS : (0xaa)
> > >         MCC Length 2
> > >         83 8d aa                                         ...
> > > < ACL Data TX: Handle 75 flags 0x00 dlen 9                     [hci0] 22.346681
> > >       Channel: 65 len 5 [PSM 3 mode 0] {chan 1}
> > >        RFCOMM(d): UIH
> > >         Address : (0x81)
> > >         CR Bit: 0
> > >         DLCI : (0x20)
> > >         Poll/FInal Bit : 1
> > >         Length : 0
> > >         FCS : (0x1e)
> > >         Credits 33
> > > ---
> >
> > This is actually what I wanted to avoid, now you have to include all
> > the frames you changing instead of doing one by one. Btw, did you make
> > sure this is consistent with the spec and what we have done for L2CAP?
> >
> > >  monitor/rfcomm.c | 21 +++++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > >
> > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> > > b85e8fd..10dc4e8 100644
> > > --- a/monitor/rfcomm.c
> > > +++ b/monitor/rfcomm.c
> > > @@ -90,48 +90,69 @@ static void print_rfcomm_hdr(const struct
> > l2cap_frame *frame,
> > >         print_field("FCS : (0x%2.2x)", fcs);  }
> > >
> > > +static void print_mcc(struct rfcomm_lmcc mcc) {
> > > +       print_field("MCC Length %d", mcc.length); }
> > > +
> > >  static inline void mcc_test(const struct l2cap_frame *frame,
> > >                                 struct rfcomm_lhdr hdr, struct
> > > rfcomm_lmcc mcc)  {
> > > +       print_rfcomm_hdr(frame, hdr);
> > > +       print_mcc(mcc);
> > >  }
> > >
> > >  static inline void mcc_fcon(const struct l2cap_frame *frame,
> > >                                 struct rfcomm_lhdr hdr, struct
> > > rfcomm_lmcc mcc)  {
> > > +       print_rfcomm_hdr(frame, hdr);
> > > +       print_mcc(mcc);
> > >  }
> > >
> > >  static inline void mcc_fcoff(const struct l2cap_frame *frame,
> > >                                 struct rfcomm_lhdr hdr, struct
> > > rfcomm_lmcc mcc)  {
> > > +       print_rfcomm_hdr(frame, hdr);
> > > +       print_mcc(mcc);
> > >  }
> > >
> > >  static inline void mcc_msc(const struct l2cap_frame *frame,
> > >                                 struct rfcomm_lhdr hdr, struct
> > > rfcomm_lmcc mcc)  {
> > > +       print_rfcomm_hdr(frame, hdr);
> > > +       print_mcc(mcc);
> > >         packet_hexdump(frame->data, frame->size);  }
> > >
> > >  static inline void mcc_rpn(const struct l2cap_frame *frame,
> > >                                 struct rfcomm_lhdr hdr, struct
> > > rfcomm_lmcc mcc)  {
> > > +       print_rfcomm_hdr(frame, hdr);
> > > +       print_mcc(mcc);
> > >         packet_hexdump(frame->data, frame->size);  }
> > >
> > >  static inline void mcc_rls(const struct l2cap_frame *frame,
> > >                                 struct rfcomm_lhdr hdr, struct
> > > rfcomm_lmcc mcc)  {
> > > +       print_rfcomm_hdr(frame, hdr);
> > > +       print_mcc(mcc);
> > >         packet_hexdump(frame->data, frame->size);  }
> > >
> > >  static inline void mcc_pn(const struct l2cap_frame *frame,
> > >                                 struct rfcomm_lhdr hdr, struct
> > > rfcomm_lmcc mcc)  {
> > > +       print_rfcomm_hdr(frame, hdr);
> > > +       print_mcc(mcc);
> > >         packet_hexdump(frame->data, frame->size);  }
> > >
> > >  static inline void mcc_nsc(const struct l2cap_frame *frame,
> > >                                 struct rfcomm_lhdr hdr, struct
> > > rfcomm_lmcc mcc)  {
> > > +       print_rfcomm_hdr(frame, hdr);
> > > +       print_mcc(mcc);
> > >         packet_hexdump(frame->data, frame->size);  }
> > >
> > > --
> > > 1.9.1
> > >
> > > --
> > > 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
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
> 
> I have revisited the specs and came up with a modified below
> implementation.
> 
> #ifdef HAVE_CONFIG_H
> #include <config.h>
> #endif
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <ctype.h>
> #include <inttypes.h>
> 
> #include <bluetooth/bluetooth.h>
> 
> #include "src/shared/util.h"
> #include "bt.h"
> #include "packet.h"
> #include "display.h"
> #include "l2cap.h"
> #include "uuid.h"
> #include "keys.h"
> #include "sdp.h"
> #include "rfcomm.h"
> 
> #define GET_LEN8(length) ((length & 0xfe) >> 1) #define GET_LEN16(length)
> ((length & 0xfffe) >> 1)
> 
> struct rfcomm_lhdr {
> 	uint8_t address;
> 	uint8_t control;
> 	uint16_t length;
> } __attribute__((packed));
> 
> struct rfcomm_lmcc {
> 	uint8_t type;
> 	uint16_t length;
> } __attribute__((packed));
> 
> struct rfcomm_data {
> 	uint8_t frame;
> 	const char *str;
> };
> 
> static const struct rfcomm_data rfcomm_table[] = {
> 	{ 0x2f, "Set Async Balance Mode (SABM) "},
> 	{ 0x63, "Unnumbered Ack (UA)"},
> 	{ 0x0f, "Disconnect Mode (DM)"},
> 	{ 0x43, "Disconnect (DISC)"},
> 	{ 0xef, "Header Check (UIH)"},
> 	{ }
> };
> void rfcomm_packet(const struct l2cap_frame *frame) {
> 	uint8_t ctype, length, ex_length;
> 	const char *frame_str, *frame_color;
> 	int i;
> 	struct l2cap_frame rfcomm_frame;
> 	struct rfcomm_lhdr hdr;
> 	const struct rfcomm_data *rfcomm_data = NULL;
> 
> 	l2cap_frame_pull(&rfcomm_frame, frame, 0);
> 
> 	if (!l2cap_frame_get_u8(&rfcomm_frame, &hdr.address) ||
> 			!l2cap_frame_get_u8(&rfcomm_frame,
> &hdr.control) ||
> 			!l2cap_frame_get_u8(&rfcomm_frame, &length)){
> 		print_text(COLOR_ERROR, "Frame too short");
> 		packet_hexdump(frame->data, frame->size);
> 		return;
> 	}
> 
> 	/* length maybe 1 or 2 octets */
> 	if (RFCOMM_TEST_EA(length))
> 		hdr.length = (uint16_t) GET_LEN8(length);
> 	else {
> 		if (!l2cap_frame_get_u8(&rfcomm_frame, &ex_length)) {
> 			print_text(COLOR_ERROR, "Frame too short");
> 			packet_hexdump(frame->data, frame->size);
> 			return;
> 		}
> 		hdr.length = ((uint16_t)length << 8) | ex_length;
> 		hdr.length = GET_LEN16(hdr.length);
> 	}
> 
> 	ctype = RFCOMM_GET_TYPE(hdr.control);
> 
> 	for (i = 0; rfcomm_table[i].str; i++) {
> 		if (rfcomm_table[i].frame == ctype) {
> 			rfcomm_data = &rfcomm_table[i];
> 			break;
> 		}
> 	}
> 
> 	if (rfcomm_data) {
> 		if (frame->in)
> 			frame_color = COLOR_MAGENTA;
> 		else
> 			frame_color = COLOR_BLUE;
> 		frame_str = rfcomm_data->str;
> 	} else {
> 		frame_color = COLOR_WHITE_BG;
> 		frame_str = "Unknown";
> 	}
> 
> 	if (!rfcomm_data) {
> 		packet_hexdump(frame->data, frame->size);
> 		return;
> 	}
> 
> 	print_indent(6, frame_color, "RFCOMM: ", frame_str, COLOR_OFF,
> 				"(0x%2.2x)", ctype);
> 
> 	/* Common print header function call */
> 
> 	/* UIH frame handling function call */
> 
> 	packet_hexdump(frame->data, frame->size); }
> 
> Is it fine, for at least initial RFCOMM skeleton ?
> If its ok, then I will keep this as my initial patches and work on the further
> implementation.
> Here I am printing the logs as soon as pulling the data from l2cap_frame.
> 
> Regards,
> Gowtham
> 
> --
> 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

I have incorporated the review comments and submitted the new patch set.  

Regards,
Gowtham

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




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux