[PATCH 1/7] monitor: Add AVRCP ListPlayerApplicationSettingValues support

[PATCH 1/7] monitor: Add AVRCP ListPlayerApplicationSettingValues support

[Vikrampal Yadav]

Support for decoding AVRCP ListPlayerApplicationSettingValues
added in Bluetooth monitor.
---
 monitor/avctp.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/monitor/avctp.c b/monitor/avctp.c
index 9df8e75..ff1de1e 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -429,6 +429,60 @@ static const char *attr2str(uint8_t attr)
 	}
 }
 
+static const char *value2str(uint8_t attr, uint8_t value)
+{
+	switch (attr) {
+	case AVRCP_ATTRIBUTE_ILEGAL:
+		return "Illegal";
+	case AVRCP_ATTRIBUTE_EQUALIZER:
+		switch (value) {
+		case 0x01:
+			return "OFF";
+		case 0x02:
+			return "ON";
+		default:
+			return "Reserved";
+		}
+	case AVRCP_ATTRIBUTE_REPEAT_MODE:
+		switch (value) {
+		case 0x01:
+			return "OFF";
+		case 0x02:
+			return "Single Track Repeat";
+		case 0x03:
+			return "All Track Repeat";
+		case 0x04:
+			return "Group Repeat";
+		default:
+			return "Reserved";
+		}
+	case AVRCP_ATTRIBUTE_SHUFFLE:
+		switch (value) {
+		case 0x01:
+			return "OFF";
+		case 0x02:
+			return "All Track Suffle";
+		case 0x03:
+			return "Group Suffle";
+		default:
+			return "Reserved";
+		}
+	case AVRCP_ATTRIBUTE_SCAN:
+		switch (value) {
+		case 0x01:
+			return "OFF";
+		case 0x02:
+			return "All Track Scan";
+		case 0x03:
+			return "Group Scan";
+		default:
+			return "Reserved";
+		}
+	default:
+		return "Unknown";
+	}
+}
+
 static void avrcp_passthrough_packet(const struct l2cap_frame *frame)
 {
 }
@@ -504,6 +558,31 @@ static void avrcp_list_player_values(const struct l2cap_frame *frame,
 					uint8_t ctype, uint8_t len,
 					uint8_t indent)
 {
+	static uint8_t attr = 0;
+	uint8_t num, i;
+
+	if (len < 1) {
+		print_text(COLOR_ERROR, "PDU malformed");
+		packet_hexdump(frame->data, frame->size);
+		return;
+	}
+
+	if (ctype > AVC_CTYPE_GENERAL_INQUIRY) {
+		num = *((uint8_t *) frame->data);
+		print_field("%*cValueCount: 0x%02x", (indent - 8), ' ', num);
+
+		for (i = 0; num > 0; num--, i++) {
+			uint8_t value;
+
+			value = *((uint8_t *) (frame->data + 1 + i));
+			print_field("%*cValueID: 0x%02x (%s)", (indent - 8),
+					' ', value, value2str(attr, value));
+		}
+	} else {
+		attr = *((uint8_t *) frame->data);
+		print_field("%*cAttributeID: 0x%02x (%s)", (indent - 8), ' ',
+							attr, attr2str(attr));
+	}
 }
 
 static void avrcp_get_current_player_value(const struct l2cap_frame *frame,
-- 
1.9.1

[PATCH 2/7] monitor: Add AVRCP GetCurrentPlayerApplicationSettingValue support

[Vikrampal Yadav]

Support for decoding AVRCP GetCurrentPlayerApplicationSettingValue
added in Bluetooth monitor.
---
 monitor/avctp.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/monitor/avctp.c b/monitor/avctp.c
index ff1de1e..1aff224 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -589,6 +589,42 @@ static void avrcp_get_current_player_value(const struct l2cap_frame *frame,
 						uint8_t ctype, uint8_t len,
 						uint8_t indent)
 {
+	uint8_t num, i;
+
+	if (len < 1) {
+		print_text(COLOR_ERROR, "PDU malformed");
+		packet_hexdump(frame->data, frame->size);
+		return;
+	}
+
+	num = *((uint8_t *) frame->data);
+
+	if (ctype > AVC_CTYPE_GENERAL_INQUIRY) {
+		print_field("%*cValueCount: 0x%02x", (indent - 8), ' ', num);
+
+		for (i = 0; num > 0; num--, i++) {
+			uint8_t attr, value;
+
+			attr = *((uint8_t *) (frame->data + 1 + (i * 2)));
+			print_field("%*cAttributeID: 0x%02x (%s)",
+				(indent - 8), ' ', attr, attr2str(attr));
+
+			value = *((uint8_t *) (frame->data + 2 + (i * 2)));
+			print_field("%*cValueID: 0x%02x (%s)", (indent - 8),
+					' ', value, value2str(attr, value));
+		}
+	} else {
+		print_field("%*cAttributeCount: 0x%02x",
+				(indent - 8), ' ', num);
+
+		for (i = 0; num > 0; num--, i++) {
+			uint8_t attr;
+
+			attr = *((uint8_t *) (frame->data + 1 + i));
+			print_field("%*cAttributeID: 0x%02x (%s)",
+				(indent - 8), ' ', attr, attr2str(attr));
+		}
+	}
 }
 
 static void avrcp_set_player_value(const struct l2cap_frame *frame,
-- 
1.9.1

[PATCH 3/7] monitor: Add AVRCP SetPlayerApplicationSettingValue support

[Vikrampal Yadav]

Support for decoding AVRCP SetPlayerApplicationSettingValue
added in Bluetooth monitor.
---
 monitor/avctp.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/monitor/avctp.c b/monitor/avctp.c
index 1aff224..a0a3a4c 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -631,6 +631,31 @@ static void avrcp_set_player_value(const struct l2cap_frame *frame,
 					uint8_t ctype, uint8_t len,
 					uint8_t indent)
 {
+	uint8_t num, i;
+
+	if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
+		return;
+
+	if (len < 1) {
+		print_text(COLOR_ERROR, "PDU malformed");
+		packet_hexdump(frame->data, frame->size);
+		return;
+	}
+
+	num = *((uint8_t *) frame->data);
+	print_field("%*cAttributeCount: 0x%02x", (indent - 8), ' ', num);
+
+	for (i = 0; num > 0; num--, i++) {
+		uint8_t attr, value;
+
+		attr = *((uint8_t *) (frame->data + 1 + (i * 2)));
+		print_field("%*cAttributeID: 0x%02x (%s)", (indent - 8), ' ',
+							attr, attr2str(attr));
+
+		value = *((uint8_t *) (frame->data + 2 + (i * 2)));
+		print_field("%*cValueID: 0x%02x (%s)", (indent - 8), ' ',
+						value, value2str(attr, value));
+	}
 }
 
 static void avrcp_get_player_attribute_text(const struct l2cap_frame *frame,
-- 
1.9.1

[PATCH 4/7] monitor: Add AVRCP GetPlayerApplicationSettingAttributeText support

[Vikrampal Yadav]

Support for decoding AVRCP GetPlayerApplicationSettingAttributeText
added in Bluetooth monitor.
---
 monitor/avctp.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/monitor/avctp.c b/monitor/avctp.c
index a0a3a4c..a3a2d84 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -28,6 +28,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <ctype.h>
 #include <string.h>
 #include <inttypes.h>
 
@@ -483,6 +484,39 @@ static const char *value2str(uint8_t attr, uint8_t value)
 	}
 }
 
+static const char *charset2str(uint16_t charset)
+{
+	switch (charset) {
+	case 1:
+	case 2:
+		return "Reserved";
+	case 3:
+		return "ASCII";
+	case 4:
+		return "ISO_8859-1";
+	case 5:
+		return "ISO_8859-2";
+	case 6:
+		return "ISO_8859-3";
+	case 7:
+		return "ISO_8859-4";
+	case 8:
+		return "ISO_8859-5";
+	case 9:
+		return "ISO_8859-6";
+	case 10:
+		return "ISO_8859-7";
+	case 11:
+		return "ISO_8859-8";
+	case 12:
+		return "ISO_8859-9";
+	case 106:
+		return "UTF-8";
+	default:
+		return "Unknown";
+	}
+}
+
 static void avrcp_passthrough_packet(const struct l2cap_frame *frame)
 {
 }
@@ -662,6 +696,54 @@ static void avrcp_get_player_attribute_text(const struct l2cap_frame *frame,
 						uint8_t ctype, uint8_t len,
 						uint8_t indent)
 {
+	uint8_t num, i, j;
+
+	if (len < 1) {
+		print_text(COLOR_ERROR, "PDU malformed");
+		packet_hexdump(frame->data, frame->size);
+		return;
+	}
+
+	num = *((uint8_t *) frame->data);
+	print_field("%*cAttributeCount: 0x%02x", (indent - 8), ' ', num);
+
+	if (ctype > AVC_CTYPE_GENERAL_INQUIRY) {
+		for (i = 0; num > 0; num--, i++) {
+			uint8_t attr, len;
+			uint8_t totallen = 0;
+			uint16_t charset;
+
+			attr = *((uint8_t *) (frame->data + 1 + 4 * i + totallen));
+			print_field("%*cAttributeID: 0x%02x (%s)",
+				(indent - 8), ' ', attr, attr2str(attr));
+
+			charset = get_be16(frame->data + 2 + 4 * i + totallen);
+			print_field("%*cCharsetID: 0x%04x (%s)",
+					(indent - 8), ' ', charset,
+					charset2str(charset));
+
+			len = *((uint8_t *) (frame->data + 4 + 4 * i + totallen));
+			print_field("%*cStringLength: 0x%02x",
+					(indent - 8), ' ', len);
+
+			totallen =+ len;
+
+			printf("String: ");
+			for (j = 0; len > 0; len--, j++) {
+				uint8_t c = *((uint8_t *) (frame->data + 5 + 4 * i + totallen + j));
+				printf("%1c", isprint(c) ? c : '.');
+			}
+			printf("\n");
+		}
+	} else {
+		for (i = 0; num > 0; num--, i++) {
+			uint8_t attr;
+
+			attr = *((uint8_t *) (frame->data + 1 + i));
+			print_field("%*cAttributeID: 0x%02x (%s)",
+				(indent - 8), ' ', attr, attr2str(attr));
+		}
+	}
 }
 
 static void avrcp_get_player_value_text(const struct l2cap_frame *frame,
-- 
1.9.1

[PATCH 5/7] monitor: Add AVRCP GetPlayerApplicationSettingValueText support

[Vikrampal Yadav]

Support for decoding AVRCP GetPlayerApplicationSettingValueText
added in Bluetooth monitor.
---
 monitor/avctp.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/monitor/avctp.c b/monitor/avctp.c
index a3a2d84..6444c28 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -750,6 +750,62 @@ static void avrcp_get_player_value_text(const struct l2cap_frame *frame,
 					uint8_t ctype, uint8_t len,
 					uint8_t indent)
 {
+	static uint8_t attr = 0;
+	uint8_t num, i, j;
+
+	if (len < 1) {
+		print_text(COLOR_ERROR, "PDU malformed");
+		packet_hexdump(frame->data, frame->size);
+		return;
+	}
+
+	if (ctype > AVC_CTYPE_GENERAL_INQUIRY) {
+		num = *((uint8_t *) frame->data);
+		print_field("%*cValueCount: 0x%02x", (indent - 8), ' ', num);
+
+		for (i = 0; num > 0; num--, i++) {
+			uint8_t value, len;
+			uint8_t totallen = 0;
+			uint16_t charset;
+
+			value = *((uint8_t *) (frame->data + 1 + 4 * i + totallen));
+			print_field("%*cValueID: 0x%02x (%s)", (indent - 8), ' ',
+						value, value2str(attr, value));
+
+			charset = get_be16(frame->data + 2 + 4 * i + totallen);
+			print_field("%*cCharsetIDID: 0x%02x (%s)",
+					(indent - 8), ' ',
+					charset, charset2str(charset));
+
+			len = *((uint8_t *) (frame->data + 4 + 4 * i + totallen));
+			print_field("%*cStringLength: 0x%02x", (indent - 8), ' ', len);
+
+			totallen =+ len;
+
+			printf("String: ");
+			for (j = 0; len > 0; len--, j++) {
+				uint8_t c =
+					*((uint8_t *) (frame->data + 5 + 4 * i + totallen + j));
+				printf("%1c", isprint(c) ? c : '.');
+			}
+			printf("\n");
+		}
+	} else {
+		attr = *((uint8_t *) frame->data);
+		print_field("%*cAttributeID: 0x%02x (%s)", (indent - 8), ' ',
+							attr, attr2str(attr));
+
+		num = *((uint8_t *) (frame->data + 1));
+		print_field("%*cValueCount: 0x%02x", (indent - 8), ' ', num);
+
+		for (i = 0; num > 0; num--, i++) {
+			uint8_t value;
+
+			value = *((uint8_t *) (frame->data + 2 + i));
+			print_field("%*cValueID: 0x%02x (%s)", (indent - 8),
+					' ', value, value2str(attr, value));
+		}
+	}
 }
 
 static void avrcp_displayable_charset(const struct l2cap_frame *frame,
-- 
1.9.1

[PATCH 6/7] monitor: Add AVRCP InformDisplayableCharacterSet support

[Vikrampal Yadav]

Support for decoding AVRCP InformDisplayableCharacterSet added
in Bluetooth monitor.
---
 monitor/avctp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/monitor/avctp.c b/monitor/avctp.c
index 6444c28..c75b3ff 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -812,6 +812,28 @@ static void avrcp_displayable_charset(const struct l2cap_frame *frame,
 					uint8_t ctype, uint8_t len,
 					uint8_t indent)
 {
+	uint8_t num, i;
+
+	if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
+		return;
+
+	if (len < 2) {
+		print_text(COLOR_ERROR, "PDU malformed");
+		packet_hexdump(frame->data, frame->size);
+		return;
+	}
+
+	num = *((uint8_t *) frame->data);
+	print_field("%*cCharsetCount: 0x%02x", (indent - 8), ' ', num);
+
+	for (i = 0; num > 0; num--, i++) {
+		uint16_t charset;
+
+		charset = get_be16(frame->data + 1 + (2 * i));
+		print_field("%*cCharsetID: 0x%04x (%s)",
+					(indent - 8), ' ', charset,
+					charset2str(charset));
+	}
 }
 
 static void avrcp_ct_battery_status(const struct l2cap_frame *frame,
-- 
1.9.1

[PATCH 7/7] monitor: Fix AVRCP rejected packet handling error

[Vikrampal Yadav]

Rectified the handling of AVRCP rejected packets in btmon.
---
 monitor/avctp.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/monitor/avctp.c b/monitor/avctp.c
index c75b3ff..32dfd15 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -910,7 +910,8 @@ static const struct avrcp_ctrl_pdu_data avrcp_ctrl_pdu_table[] = {
 	{ }
 };
 
-static void avrcp_rejected_packet(const struct l2cap_frame *frame)
+static void avrcp_rejected_packet(const struct l2cap_frame *frame,
+					uint8_t indent)
 {
 	uint8_t status;
 
@@ -921,7 +922,8 @@ static void avrcp_rejected_packet(const struct l2cap_frame *frame)
 	}
 
 	status = *((uint8_t *) frame->data);
-	print_field("Error: 0x%02x (%s)", status, error2str(status));
+	print_field("%*cError: 0x%02x (%s)", (indent - 8), ' ',
+					status, error2str(status));
 }
 
 static void avrcp_pdu_packet(const struct l2cap_frame *frame, uint8_t ctype,
@@ -947,7 +949,8 @@ static void avrcp_pdu_packet(const struct l2cap_frame *frame, uint8_t ctype,
 	}
 
 	if (ctype == 0xA) {
-		avrcp_rejected_packet(frame);
+		l2cap_frame_pull(&avrcp_frame, frame, 4);
+		avrcp_rejected_packet(&avrcp_frame, indent + 2);
 		return;
 	}
 
-- 
1.9.1

Re: [PATCH 4/7] monitor: Add AVRCP GetPlayerApplicationSettingAttributeText support

[Luiz Augusto von Dentz]

Hi Vikrampal,

On Wed, Aug 20, 2014 at 12:24 PM, Vikrampal Yadav
<vikram.pal@samsung.com> wrote:
> Support for decoding AVRCP GetPlayerApplicationSettingAttributeText
> added in Bluetooth monitor.
> ---
>  monitor/avctp.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>
> diff --git a/monitor/avctp.c b/monitor/avctp.c
> index a0a3a4c..a3a2d84 100644
> --- a/monitor/avctp.c
> +++ b/monitor/avctp.c
> @@ -28,6 +28,7 @@
>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <ctype.h>
>  #include <string.h>
>  #include <inttypes.h>
>
> @@ -483,6 +484,39 @@ static const char *value2str(uint8_t attr, uint8_t value)
>         }
>  }
>
> +static const char *charset2str(uint16_t charset)
> +{
> +       switch (charset) {
> +       case 1:
> +       case 2:
> +               return "Reserved";
> +       case 3:
> +               return "ASCII";
> +       case 4:
> +               return "ISO_8859-1";
> +       case 5:
> +               return "ISO_8859-2";
> +       case 6:
> +               return "ISO_8859-3";
> +       case 7:
> +               return "ISO_8859-4";
> +       case 8:
> +               return "ISO_8859-5";
> +       case 9:
> +               return "ISO_8859-6";
> +       case 10:
> +               return "ISO_8859-7";
> +       case 11:
> +               return "ISO_8859-8";
> +       case 12:
> +               return "ISO_8859-9";
> +       case 106:
> +               return "UTF-8";
> +       default:
> +               return "Unknown";
> +       }
> +}
> +
>  static void avrcp_passthrough_packet(const struct l2cap_frame *frame)
>  {
>  }
> @@ -662,6 +696,54 @@ static void avrcp_get_player_attribute_text(const struct l2cap_frame *frame,
>                                                 uint8_t ctype, uint8_t len,
>                                                 uint8_t indent)
>  {
> +       uint8_t num, i, j;
> +
> +       if (len < 1) {
> +               print_text(COLOR_ERROR, "PDU malformed");
> +               packet_hexdump(frame->data, frame->size);
> +               return;
> +       }
> +
> +       num = *((uint8_t *) frame->data);
> +       print_field("%*cAttributeCount: 0x%02x", (indent - 8), ' ', num);
> +
> +       if (ctype > AVC_CTYPE_GENERAL_INQUIRY) {
> +               for (i = 0; num > 0; num--, i++) {
> +                       uint8_t attr, len;
> +                       uint8_t totallen = 0;
> +                       uint16_t charset;
> +
> +                       attr = *((uint8_t *) (frame->data + 1 + 4 * i + totallen));
> +                       print_field("%*cAttributeID: 0x%02x (%s)",
> +                               (indent - 8), ' ', attr, attr2str(attr));
> +
> +                       charset = get_be16(frame->data + 2 + 4 * i + totallen);
> +                       print_field("%*cCharsetID: 0x%04x (%s)",
> +                                       (indent - 8), ' ', charset,
> +                                       charset2str(charset));
> +
> +                       len = *((uint8_t *) (frame->data + 4 + 4 * i + totallen));
> +                       print_field("%*cStringLength: 0x%02x",
> +                                       (indent - 8), ' ', len);
> +
> +                       totallen =+ len;
> +
> +                       printf("String: ");
> +                       for (j = 0; len > 0; len--, j++) {
> +                               uint8_t c = *((uint8_t *) (frame->data + 5 + 4 * i + totallen + j));
> +                               printf("%1c", isprint(c) ? c : '.');
> +                       }
> +                       printf("\n");
> +               }
> +       } else {
> +               for (i = 0; num > 0; num--, i++) {
> +                       uint8_t attr;
> +
> +                       attr = *((uint8_t *) (frame->data + 1 + i));
> +                       print_field("%*cAttributeID: 0x%02x (%s)",
> +                               (indent - 8), ' ', attr, attr2str(attr));
> +               }
> +       }

Please try to minimize the use of nested indentation, you can use goto
as we did in tools/parser/avrcp.c

-- 
Luiz Augusto von Dentz

Re: [PATCH 7/7] monitor: Fix AVRCP rejected packet handling error

[Luiz Augusto von Dentz]

Hi Vikrampal,

On Wed, Aug 20, 2014 at 12:24 PM, Vikrampal Yadav
<vikram.pal@samsung.com> wrote:
> Rectified the handling of AVRCP rejected packets in btmon.
> ---
>  monitor/avctp.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/monitor/avctp.c b/monitor/avctp.c
> index c75b3ff..32dfd15 100644
> --- a/monitor/avctp.c
> +++ b/monitor/avctp.c
> @@ -910,7 +910,8 @@ static const struct avrcp_ctrl_pdu_data avrcp_ctrl_pdu_table[] = {
>         { }
>  };
>
> -static void avrcp_rejected_packet(const struct l2cap_frame *frame)
> +static void avrcp_rejected_packet(const struct l2cap_frame *frame,
> +                                       uint8_t indent)
>  {
>         uint8_t status;
>
> @@ -921,7 +922,8 @@ static void avrcp_rejected_packet(const struct l2cap_frame *frame)
>         }
>
>         status = *((uint8_t *) frame->data);
> -       print_field("Error: 0x%02x (%s)", status, error2str(status));
> +       print_field("%*cError: 0x%02x (%s)", (indent - 8), ' ',
> +                                       status, error2str(status));
>  }
>
>  static void avrcp_pdu_packet(const struct l2cap_frame *frame, uint8_t ctype,
> @@ -947,7 +949,8 @@ static void avrcp_pdu_packet(const struct l2cap_frame *frame, uint8_t ctype,
>         }
>
>         if (ctype == 0xA) {
> -               avrcp_rejected_packet(frame);
> +               l2cap_frame_pull(&avrcp_frame, frame, 4);
> +               avrcp_rejected_packet(&avrcp_frame, indent + 2);
>                 return;
>         }
>
> --
> 1.9.1

Applied this one, please next time send the fixes first.


-- 
Luiz Augusto von Dentz

RE: [PATCH 4/7] monitor: Add AVRCP GetPlayerApplicationSettingAttributeText support

[Vikrampal]

Hi Luiz,

> -----Original Message-----
> From: Vikrampal Yadav [mailto:vikram.pal@samsung.com]
> Sent: Wednesday, August 20, 2014 2:54 PM
> To: linux-bluetooth@vger.kernel.org
> Cc: luiz.dentz@gmail.com; d.kasatkin@samsung.com;
> vikram.pal@samsung.com; cpgs@samsung.com
> Subject: [PATCH 4/7] monitor: Add AVRCP
> GetPlayerApplicationSettingAttributeText support
> 
> Support for decoding AVRCP GetPlayerApplicationSettingAttributeText
> added in Bluetooth monitor.
> ---
>  monitor/avctp.c | 82
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/monitor/avctp.c b/monitor/avctp.c index a0a3a4c..a3a2d84
> 100644
> --- a/monitor/avctp.c
> +++ b/monitor/avctp.c
> @@ -28,6 +28,7 @@
> 
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <ctype.h>
>  #include <string.h>
>  #include <inttypes.h>
> 
> @@ -483,6 +484,39 @@ static const char *value2str(uint8_t attr, uint8_t
> value)
>  	}
>  }
> 
> +static const char *charset2str(uint16_t charset) {
> +	switch (charset) {
> +	case 1:
> +	case 2:
> +		return "Reserved";
> +	case 3:
> +		return "ASCII";
> +	case 4:
> +		return "ISO_8859-1";
> +	case 5:
> +		return "ISO_8859-2";
> +	case 6:
> +		return "ISO_8859-3";
> +	case 7:
> +		return "ISO_8859-4";
> +	case 8:
> +		return "ISO_8859-5";
> +	case 9:
> +		return "ISO_8859-6";
> +	case 10:
> +		return "ISO_8859-7";
> +	case 11:
> +		return "ISO_8859-8";
> +	case 12:
> +		return "ISO_8859-9";
> +	case 106:
> +		return "UTF-8";
> +	default:
> +		return "Unknown";
> +	}
> +}
> +
>  static void avrcp_passthrough_packet(const struct l2cap_frame *frame)  {
}
> @@ -662,6 +696,54 @@ static void avrcp_get_player_attribute_text(const
> struct l2cap_frame *frame,
>  						uint8_t ctype, uint8_t len,
>  						uint8_t indent)
>  {
> +	uint8_t num, i, j;
> +
> +	if (len < 1) {
> +		print_text(COLOR_ERROR, "PDU malformed");
> +		packet_hexdump(frame->data, frame->size);
> +		return;
> +	}
> +
> +	num = *((uint8_t *) frame->data);
> +	print_field("%*cAttributeCount: 0x%02x", (indent - 8), ' ', num);
> +
> +	if (ctype > AVC_CTYPE_GENERAL_INQUIRY) {
> +		for (i = 0; num > 0; num--, i++) {
> +			uint8_t attr, len;
> +			uint8_t totallen = 0;
> +			uint16_t charset;
> +
> +			attr = *((uint8_t *) (frame->data + 1 + 4 * i +
> totallen));
> +			print_field("%*cAttributeID: 0x%02x (%s)",
> +				(indent - 8), ' ', attr, attr2str(attr));
> +
> +			charset = get_be16(frame->data + 2 + 4 * i +
totallen);
> +			print_field("%*cCharsetID: 0x%04x (%s)",
> +					(indent - 8), ' ', charset,
> +					charset2str(charset));
> +
> +			len = *((uint8_t *) (frame->data + 4 + 4 * i +
totallen));
> +			print_field("%*cStringLength: 0x%02x",
> +					(indent - 8), ' ', len);
> +
> +			totallen =+ len;
> +
> +			printf("String: ");
> +			for (j = 0; len > 0; len--, j++) {
> +				uint8_t c = *((uint8_t *) (frame->data + 5 +
4
> * i + totallen + j));
> +				printf("%1c", isprint(c) ? c : '.');
> +			}
> +			printf("\n");
> +		}
> +	} else {
> +		for (i = 0; num > 0; num--, i++) {
> +			uint8_t attr;
> +
> +			attr = *((uint8_t *) (frame->data + 1 + i));
> +			print_field("%*cAttributeID: 0x%02x (%s)",
> +				(indent - 8), ' ', attr, attr2str(attr));
> +		}
> +	}
>  }
> 
>  static void avrcp_get_player_value_text(const struct l2cap_frame *frame,
> --
> 1.9.1

Even after using goto, one particular line uint8_t c = *((uint8_t *)
(frame->data + 5 + 4 * i + totallen + j));
Is going upto 85 characters. Is it acceptable?
If not how to break this particular line meaningfully?

Regards,
Vikram

Re: [PATCH 4/7] monitor: Add AVRCP GetPlayerApplicationSettingAttributeText support

[Luiz Augusto von Dentz]

Hi Vikrampal,

On Wed, Aug 20, 2014 at 4:10 PM, Vikrampal <vikram.pal@samsung.com> wrote:
> Hi Luiz,
>
>> -----Original Message-----
>> From: Vikrampal Yadav [mailto:vikram.pal@samsung.com]
>> Sent: Wednesday, August 20, 2014 2:54 PM
>> To: linux-bluetooth@vger.kernel.org
>> Cc: luiz.dentz@gmail.com; d.kasatkin@samsung.com;
>> vikram.pal@samsung.com; cpgs@samsung.com
>> Subject: [PATCH 4/7] monitor: Add AVRCP
>> GetPlayerApplicationSettingAttributeText support
>>
>> Support for decoding AVRCP GetPlayerApplicationSettingAttributeText
>> added in Bluetooth monitor.
>> ---
>>  monitor/avctp.c | 82
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 82 insertions(+)
>>
>> diff --git a/monitor/avctp.c b/monitor/avctp.c index a0a3a4c..a3a2d84
>> 100644
>> --- a/monitor/avctp.c
>> +++ b/monitor/avctp.c
>> @@ -28,6 +28,7 @@
>>
>>  #include <stdio.h>
>>  #include <stdlib.h>
>> +#include <ctype.h>
>>  #include <string.h>
>>  #include <inttypes.h>
>>
>> @@ -483,6 +484,39 @@ static const char *value2str(uint8_t attr, uint8_t
>> value)
>>       }
>>  }
>>
>> +static const char *charset2str(uint16_t charset) {
>> +     switch (charset) {
>> +     case 1:
>> +     case 2:
>> +             return "Reserved";
>> +     case 3:
>> +             return "ASCII";
>> +     case 4:
>> +             return "ISO_8859-1";
>> +     case 5:
>> +             return "ISO_8859-2";
>> +     case 6:
>> +             return "ISO_8859-3";
>> +     case 7:
>> +             return "ISO_8859-4";
>> +     case 8:
>> +             return "ISO_8859-5";
>> +     case 9:
>> +             return "ISO_8859-6";
>> +     case 10:
>> +             return "ISO_8859-7";
>> +     case 11:
>> +             return "ISO_8859-8";
>> +     case 12:
>> +             return "ISO_8859-9";
>> +     case 106:
>> +             return "UTF-8";
>> +     default:
>> +             return "Unknown";
>> +     }
>> +}
>> +
>>  static void avrcp_passthrough_packet(const struct l2cap_frame *frame)  {
> }
>> @@ -662,6 +696,54 @@ static void avrcp_get_player_attribute_text(const
>> struct l2cap_frame *frame,
>>                                               uint8_t ctype, uint8_t len,
>>                                               uint8_t indent)
>>  {
>> +     uint8_t num, i, j;
>> +
>> +     if (len < 1) {
>> +             print_text(COLOR_ERROR, "PDU malformed");
>> +             packet_hexdump(frame->data, frame->size);
>> +             return;
>> +     }
>> +
>> +     num = *((uint8_t *) frame->data);
>> +     print_field("%*cAttributeCount: 0x%02x", (indent - 8), ' ', num);
>> +
>> +     if (ctype > AVC_CTYPE_GENERAL_INQUIRY) {
>> +             for (i = 0; num > 0; num--, i++) {
>> +                     uint8_t attr, len;
>> +                     uint8_t totallen = 0;
>> +                     uint16_t charset;
>> +
>> +                     attr = *((uint8_t *) (frame->data + 1 + 4 * i +
>> totallen));
>> +                     print_field("%*cAttributeID: 0x%02x (%s)",
>> +                             (indent - 8), ' ', attr, attr2str(attr));
>> +
>> +                     charset = get_be16(frame->data + 2 + 4 * i +
> totallen);
>> +                     print_field("%*cCharsetID: 0x%04x (%s)",
>> +                                     (indent - 8), ' ', charset,
>> +                                     charset2str(charset));
>> +
>> +                     len = *((uint8_t *) (frame->data + 4 + 4 * i +
> totallen));
>> +                     print_field("%*cStringLength: 0x%02x",
>> +                                     (indent - 8), ' ', len);
>> +
>> +                     totallen =+ len;
>> +
>> +                     printf("String: ");
>> +                     for (j = 0; len > 0; len--, j++) {
>> +                             uint8_t c = *((uint8_t *) (frame->data + 5 +
> 4
>> * i + totallen + j));
>> +                             printf("%1c", isprint(c) ? c : '.');
>> +                     }
>> +                     printf("\n");
>> +             }
>> +     } else {
>> +             for (i = 0; num > 0; num--, i++) {
>> +                     uint8_t attr;
>> +
>> +                     attr = *((uint8_t *) (frame->data + 1 + i));
>> +                     print_field("%*cAttributeID: 0x%02x (%s)",
>> +                             (indent - 8), ' ', attr, attr2str(attr));
>> +             }
>> +     }
>>  }
>>
>>  static void avrcp_get_player_value_text(const struct l2cap_frame *frame,
>> --
>> 1.9.1
>
> Even after using goto, one particular line uint8_t c = *((uint8_t *)
> (frame->data + 5 + 4 * i + totallen + j));
> Is going upto 85 characters. Is it acceptable?
> If not how to break this particular line meaningfully?

You don't have to assign in the declaration, actually all those offset
calculation looks very hard to understand so perhaps you should be
pulling ahead on each iteration.


-- 
Luiz Augusto von Dentz