* [PATCH BlueZ 1/3] monitor: Add frame number support
@ 2017-04-20 12:32 Luiz Augusto von Dentz
2017-04-20 12:32 ` [PATCH BlueZ 2/3] monitor: Fix not decoding control frames Luiz Augusto von Dentz
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2017-04-20 12:32 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This adds -f/--frames options which can be used to show the frame
number:
> HCI Event: Command Complete (0x0e) plen 4 #86 [hci1] 13:00:02.639412
LE Set Scan Parameters (0x08|0x000b) ncmd 1
Status: Success (0x00)
< HCI Command: LE Set Scan Enable (0x08|0x000c) plen 2 #87 [hci1] 13:00:02.639663
Scanning: Enabled (0x01)
Filter duplicates: Enabled (0x01)
> HCI Event: Command Complete (0x0e) plen 4 #88 [hci1] 13:00:02.640420
LE Set Scan Enable (0x08|0x000c) ncmd 2
Status: Success (0x00)
---
monitor/main.c | 7 ++++++-
monitor/packet.c | 37 +++++++++++++++++++++++++++----------
monitor/packet.h | 1 +
3 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/monitor/main.c b/monitor/main.c
index f9bca22..e1a2375 100644
--- a/monitor/main.c
+++ b/monitor/main.c
@@ -70,6 +70,7 @@ static void usage(void)
"\t-T, --date Show time and date information\n"
"\t-S, --sco Dump SCO traffic\n"
"\t-E, --ellisys [ip] Send Ellisys HCI Injection\n"
+ "\t-f, --frames Show frame counter\n"
"\t-h, --help Show help options\n");
}
@@ -86,6 +87,7 @@ static const struct option main_options[] = {
{ "date", no_argument, NULL, 'T' },
{ "sco", no_argument, NULL, 'S' },
{ "ellisys", required_argument, NULL, 'E' },
+ { "frames", no_argument, NULL, 'f' },
{ "todo", no_argument, NULL, '#' },
{ "version", no_argument, NULL, 'v' },
{ "help", no_argument, NULL, 'h' },
@@ -113,7 +115,7 @@ int main(int argc, char *argv[])
for (;;) {
int opt;
- opt = getopt_long(argc, argv, "d:r:w:a:s:p:i:tTSE:vh",
+ opt = getopt_long(argc, argv, "d:r:w:a:s:p:i:tfTSE:vh",
main_options, NULL);
if (opt < 0)
break;
@@ -171,6 +173,9 @@ int main(int argc, char *argv[])
ellisys_server = optarg;
ellisys_port = 24352;
break;
+ case 'f':
+ filter_mask |= PACKET_FILTER_SHOW_FRAME;
+ break;
case '#':
packet_todo();
lmp_todo();
diff --git a/monitor/packet.c b/monitor/packet.c
index 3c43356..e5f2a32 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -58,6 +58,7 @@
#include "packet.h"
#define COLOR_CHANNEL_LABEL COLOR_WHITE
+#define COLOR_FRAME_LABEL COLOR_WHITE
#define COLOR_INDEX_LABEL COLOR_WHITE
#define COLOR_TIMESTAMP COLOR_YELLOW
@@ -259,6 +260,18 @@ void packet_select_index(uint16_t index)
#define print_space(x) printf("%*c", (x), ' ');
+#define MAX_INDEX 16
+
+struct index_data {
+ uint8_t type;
+ uint8_t bdaddr[6];
+ uint16_t manufacturer;
+ size_t frame;
+};
+
+static struct index_data index_list[MAX_INDEX];
+
+
static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
uint16_t index, const char *channel,
const char *color, const char *label,
@@ -280,6 +293,20 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
ts_pos += n;
ts_len += n;
}
+ } else if (filter_mask & PACKET_FILTER_SHOW_FRAME &&
+ (ident == '<' || ident == '>')) {
+ if (use_color()) {
+ n = sprintf(ts_str + ts_pos, "%s", COLOR_FRAME_LABEL);
+ if (n > 0)
+ ts_pos += n;
+ }
+
+ n = sprintf(ts_str + ts_pos, " #%zu",
+ ++index_list[index].frame);
+ if (n > 0) {
+ ts_pos += n;
+ ts_len += n;
+ }
}
if ((filter_mask & PACKET_FILTER_SHOW_INDEX) &&
@@ -3828,16 +3855,6 @@ static int addr2str(const uint8_t *addr, char *str)
addr[5], addr[4], addr[3], addr[2], addr[1], addr[0]);
}
-#define MAX_INDEX 16
-
-struct index_data {
- uint8_t type;
- uint8_t bdaddr[6];
- uint16_t manufacturer;
-};
-
-static struct index_data index_list[MAX_INDEX];
-
void packet_monitor(struct timeval *tv, struct ucred *cred,
uint16_t index, uint16_t opcode,
const void *data, uint16_t size)
diff --git a/monitor/packet.h b/monitor/packet.h
index 354f4fe..2516ec1 100644
--- a/monitor/packet.h
+++ b/monitor/packet.h
@@ -33,6 +33,7 @@
#define PACKET_FILTER_SHOW_TIME_OFFSET (1 << 3)
#define PACKET_FILTER_SHOW_ACL_DATA (1 << 4)
#define PACKET_FILTER_SHOW_SCO_DATA (1 << 5)
+#define PACKET_FILTER_SHOW_FRAME (1 << 6)
void packet_set_filter(unsigned long filter);
void packet_add_filter(unsigned long filter);
--
2.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH BlueZ 2/3] monitor: Fix not decoding control frames
2017-04-20 12:32 [PATCH BlueZ 1/3] monitor: Add frame number support Luiz Augusto von Dentz
@ 2017-04-20 12:32 ` Luiz Augusto von Dentz
2017-04-20 12:32 ` [PATCH BlueZ 3/3] client: Always start an agent Luiz Augusto von Dentz
2017-04-20 12:57 ` [PATCH BlueZ 1/3] monitor: Add frame number support Marcel Holtmann
2 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2017-04-20 12:32 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
In order to enable decoding control frames packet_monitor needs to check
if the index is set to HCI_DEV_NONE since that will call packet_ctrl_open
which setups the ctrl and assign it a cookie.
---
monitor/packet.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/monitor/packet.c b/monitor/packet.c
index e5f2a32..b14a82d 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -3866,10 +3866,11 @@ void packet_monitor(struct timeval *tv, struct ucred *cred,
uint16_t manufacturer;
const char *ident;
- if (index_filter && index_number != index)
- return;
-
- index_current = index;
+ if (index != HCI_DEV_NONE) {
+ if (index_filter && index_number != index)
+ return;
+ index_current = index;
+ }
if (tv && time_offset == ((time_t) -1))
time_offset = tv->tv_sec;
--
2.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH BlueZ 3/3] client: Always start an agent
2017-04-20 12:32 [PATCH BlueZ 1/3] monitor: Add frame number support Luiz Augusto von Dentz
2017-04-20 12:32 ` [PATCH BlueZ 2/3] monitor: Fix not decoding control frames Luiz Augusto von Dentz
@ 2017-04-20 12:32 ` Luiz Augusto von Dentz
2017-04-20 12:52 ` Marcel Holtmann
2017-04-20 12:57 ` [PATCH BlueZ 1/3] monitor: Add frame number support Marcel Holtmann
2 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2017-04-20 12:32 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Always register agent with 'KeyboardDisplay' capability.
---
client/main.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/client/main.c b/client/main.c
index 8c9d5c3..7043081 100644
--- a/client/main.c
+++ b/client/main.c
@@ -54,6 +54,8 @@
#define PROMPT_ON COLOR_BLUE "[bluetooth]" COLOR_OFF "# "
#define PROMPT_OFF "Waiting to connect to bluetoothd..."
+#define AGENT_CAP "KeyboardDisplay"
+
static GMainLoop *main_loop;
static DBusConnection *dbus_conn;
@@ -2352,23 +2354,9 @@ static guint setup_signalfd(void)
static gboolean option_version = FALSE;
-static gboolean parse_agent(const char *key, const char *value,
- gpointer user_data, GError **error)
-{
- if (value)
- auto_register_agent = g_strdup(value);
- else
- auto_register_agent = g_strdup("");
-
- return TRUE;
-}
-
static GOptionEntry options[] = {
{ "version", 'v', 0, G_OPTION_ARG_NONE, &option_version,
"Show version information and exit" },
- { "agent", 'a', G_OPTION_FLAG_OPTIONAL_ARG,
- G_OPTION_ARG_CALLBACK, parse_agent,
- "Register agent handler", "CAPABILITY" },
{ NULL },
};
@@ -2399,6 +2387,8 @@ int main(int argc, char *argv[])
g_option_context_free(context);
+ auto_register_agent = g_strdup(AGENT_CAP);
+
if (option_version == TRUE) {
printf("%s\n", VERSION);
exit(0);
--
2.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH BlueZ 3/3] client: Always start an agent
2017-04-20 12:32 ` [PATCH BlueZ 3/3] client: Always start an agent Luiz Augusto von Dentz
@ 2017-04-20 12:52 ` Marcel Holtmann
2017-04-20 13:13 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 8+ messages in thread
From: Marcel Holtmann @ 2017-04-20 12:52 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
> Always register agent with 'KeyboardDisplay' capability.
> ---
> client/main.c | 18 ++++--------------
> 1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/client/main.c b/client/main.c
> index 8c9d5c3..7043081 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -54,6 +54,8 @@
> #define PROMPT_ON COLOR_BLUE "[bluetooth]" COLOR_OFF "# "
> #define PROMPT_OFF "Waiting to connect to bluetoothd..."
>
> +#define AGENT_CAP "KeyboardDisplay"
> +
> static GMainLoop *main_loop;
> static DBusConnection *dbus_conn;
>
> @@ -2352,23 +2354,9 @@ static guint setup_signalfd(void)
>
> static gboolean option_version = FALSE;
>
> -static gboolean parse_agent(const char *key, const char *value,
> - gpointer user_data, GError **error)
> -{
> - if (value)
> - auto_register_agent = g_strdup(value);
> - else
> - auto_register_agent = g_strdup("");
> -
> - return TRUE;
> -}
> -
> static GOptionEntry options[] = {
> { "version", 'v', 0, G_OPTION_ARG_NONE, &option_version,
> "Show version information and exit" },
> - { "agent", 'a', G_OPTION_FLAG_OPTIONAL_ARG,
> - G_OPTION_ARG_CALLBACK, parse_agent,
> - "Register agent handler", "CAPABILITY" },
> { NULL },
> };
actually leave the agent option and make the argument mandatory. We should be registering with “” as agent string by default. That ensures that we use the defaults from bluetoothd and mgmt. However the command line option can overwrite the IO capability.
Regards
Marcel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH BlueZ 3/3] client: Always start an agent
2017-04-20 12:52 ` Marcel Holtmann
@ 2017-04-20 13:13 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2017-04-20 13:13 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
Hi Marcel,
On Thu, Apr 20, 2017 at 3:52 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Luiz,
>
>> Always register agent with 'KeyboardDisplay' capability.
>> ---
>> client/main.c | 18 ++++--------------
>> 1 file changed, 4 insertions(+), 14 deletions(-)
>>
>> diff --git a/client/main.c b/client/main.c
>> index 8c9d5c3..7043081 100644
>> --- a/client/main.c
>> +++ b/client/main.c
>> @@ -54,6 +54,8 @@
>> #define PROMPT_ON COLOR_BLUE "[bluetooth]" COLOR_OFF "# "
>> #define PROMPT_OFF "Waiting to connect to bluetoothd..."
>>
>> +#define AGENT_CAP "KeyboardDisplay"
>> +
>> static GMainLoop *main_loop;
>> static DBusConnection *dbus_conn;
>>
>> @@ -2352,23 +2354,9 @@ static guint setup_signalfd(void)
>>
>> static gboolean option_version =3D FALSE;
>>
>> -static gboolean parse_agent(const char *key, const char *value,
>> - gpointer user_data, GError **error=
)
>> -{
>> - if (value)
>> - auto_register_agent =3D g_strdup(value);
>> - else
>> - auto_register_agent =3D g_strdup("");
>> -
>> - return TRUE;
>> -}
>> -
>> static GOptionEntry options[] =3D {
>> { "version", 'v', 0, G_OPTION_ARG_NONE, &option_version,
>> "Show version information and exit" },
>> - { "agent", 'a', G_OPTION_FLAG_OPTIONAL_ARG,
>> - G_OPTION_ARG_CALLBACK, parse_agent,
>> - "Register agent handler", "CAPABILITY" },
>> { NULL },
>> };
>
> actually leave the agent option and make the argument mandatory. We shoul=
d be registering with =E2=80=9C=E2=80=9D as agent string by default. That e=
nsures that we use the defaults from bluetoothd and mgmt. However the comma=
nd line option can overwrite the IO capability.
Alright, will send a v2 in a moment.
> Regards
>
> Marcel
>
--=20
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH BlueZ 1/3] monitor: Add frame number support
2017-04-20 12:32 [PATCH BlueZ 1/3] monitor: Add frame number support Luiz Augusto von Dentz
2017-04-20 12:32 ` [PATCH BlueZ 2/3] monitor: Fix not decoding control frames Luiz Augusto von Dentz
2017-04-20 12:32 ` [PATCH BlueZ 3/3] client: Always start an agent Luiz Augusto von Dentz
@ 2017-04-20 12:57 ` Marcel Holtmann
2017-04-20 13:20 ` Luiz Augusto von Dentz
2 siblings, 1 reply; 8+ messages in thread
From: Marcel Holtmann @ 2017-04-20 12:57 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
> This adds -f/--frames options which can be used to show the frame
> number:
>
>> HCI Event: Command Complete (0x0e) plen 4 #86 [hci1] 13:00:02.639412
> LE Set Scan Parameters (0x08|0x000b) ncmd 1
> Status: Success (0x00)
> < HCI Command: LE Set Scan Enable (0x08|0x000c) plen 2 #87 [hci1] 13:00:02.639663
> Scanning: Enabled (0x01)
> Filter duplicates: Enabled (0x01)
>> HCI Event: Command Complete (0x0e) plen 4 #88 [hci1] 13:00:02.640420
> LE Set Scan Enable (0x08|0x000c) ncmd 2
> Status: Success (0x00)
> ---
> monitor/main.c | 7 ++++++-
> monitor/packet.c | 37 +++++++++++++++++++++++++++----------
> monitor/packet.h | 1 +
> 3 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/monitor/main.c b/monitor/main.c
> index f9bca22..e1a2375 100644
> --- a/monitor/main.c
> +++ b/monitor/main.c
> @@ -70,6 +70,7 @@ static void usage(void)
> "\t-T, --date Show time and date information\n"
> "\t-S, --sco Dump SCO traffic\n"
> "\t-E, --ellisys [ip] Send Ellisys HCI Injection\n"
> + "\t-f, --frames Show frame counter\n"
> "\t-h, --help Show help options\n");
> }
I would rather use -C / --counter as frame counter.
> @@ -86,6 +87,7 @@ static const struct option main_options[] = {
> { "date", no_argument, NULL, 'T' },
> { "sco", no_argument, NULL, 'S' },
> { "ellisys", required_argument, NULL, 'E' },
> + { "frames", no_argument, NULL, 'f' },
> { "todo", no_argument, NULL, '#' },
> { "version", no_argument, NULL, 'v' },
> { "help", no_argument, NULL, 'h' },
> @@ -113,7 +115,7 @@ int main(int argc, char *argv[])
> for (;;) {
> int opt;
>
> - opt = getopt_long(argc, argv, "d:r:w:a:s:p:i:tTSE:vh",
> + opt = getopt_long(argc, argv, "d:r:w:a:s:p:i:tfTSE:vh",
> main_options, NULL);
> if (opt < 0)
> break;
> @@ -171,6 +173,9 @@ int main(int argc, char *argv[])
> ellisys_server = optarg;
> ellisys_port = 24352;
> break;
> + case 'f':
> + filter_mask |= PACKET_FILTER_SHOW_FRAME;
> + break;
> case '#':
> packet_todo();
> lmp_todo();
> diff --git a/monitor/packet.c b/monitor/packet.c
> index 3c43356..e5f2a32 100644
> --- a/monitor/packet.c
> +++ b/monitor/packet.c
> @@ -58,6 +58,7 @@
> #include "packet.h"
>
> #define COLOR_CHANNEL_LABEL COLOR_WHITE
> +#define COLOR_FRAME_LABEL COLOR_WHITE
> #define COLOR_INDEX_LABEL COLOR_WHITE
> #define COLOR_TIMESTAMP COLOR_YELLOW
>
> @@ -259,6 +260,18 @@ void packet_select_index(uint16_t index)
>
> #define print_space(x) printf("%*c", (x), ' ');
>
> +#define MAX_INDEX 16
> +
> +struct index_data {
> + uint8_t type;
> + uint8_t bdaddr[6];
> + uint16_t manufacturer;
> + size_t frame;
> +};
> +
> +static struct index_data index_list[MAX_INDEX];
> +
> +
Too many empty lines here.
> static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
> uint16_t index, const char *channel,
> const char *color, const char *label,
> @@ -280,6 +293,20 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
> ts_pos += n;
> ts_len += n;
> }
> + } else if (filter_mask & PACKET_FILTER_SHOW_FRAME &&
> + (ident == '<' || ident == '>')) {
Lets not use the ident to choose here.
> + if (use_color()) {
> + n = sprintf(ts_str + ts_pos, "%s", COLOR_FRAME_LABEL);
> + if (n > 0)
> + ts_pos += n;
> + }
> +
> + n = sprintf(ts_str + ts_pos, " #%zu",
> + ++index_list[index].frame);
I would prefer if we add the frame number during input and not during display of the packet. That should be kept separate. Otherwise we create a mess like in hcidump.
> + if (n > 0) {
> + ts_pos += n;
> + ts_len += n;
> + }
> }
>
> if ((filter_mask & PACKET_FILTER_SHOW_INDEX) &&
> @@ -3828,16 +3855,6 @@ static int addr2str(const uint8_t *addr, char *str)
> addr[5], addr[4], addr[3], addr[2], addr[1], addr[0]);
> }
>
> -#define MAX_INDEX 16
> -
> -struct index_data {
> - uint8_t type;
> - uint8_t bdaddr[6];
> - uint16_t manufacturer;
> -};
> -
> -static struct index_data index_list[MAX_INDEX];
> -
> void packet_monitor(struct timeval *tv, struct ucred *cred,
> uint16_t index, uint16_t opcode,
> const void *data, uint16_t size)
> diff --git a/monitor/packet.h b/monitor/packet.h
> index 354f4fe..2516ec1 100644
> --- a/monitor/packet.h
> +++ b/monitor/packet.h
> @@ -33,6 +33,7 @@
> #define PACKET_FILTER_SHOW_TIME_OFFSET (1 << 3)
> #define PACKET_FILTER_SHOW_ACL_DATA (1 << 4)
> #define PACKET_FILTER_SHOW_SCO_DATA (1 << 5)
> +#define PACKET_FILTER_SHOW_FRAME (1 << 6)
Call it FRAME_COUNTER or something similar. Just FRAME is misleading. Or we might just always show the frame counter anyway. It is not really in the way.
Regards
Marcel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH BlueZ 1/3] monitor: Add frame number support
2017-04-20 12:57 ` [PATCH BlueZ 1/3] monitor: Add frame number support Marcel Holtmann
@ 2017-04-20 13:20 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2017-04-20 13:20 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
Hi Marcel,
On Thu, Apr 20, 2017 at 3:57 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> This adds -f/--frames options which can be used to show the frame
>> number:
>>
>>> HCI Event: Command Complete (0x0e) plen 4 #86 [hci1] 13:00:02.639412
>> LE Set Scan Parameters (0x08|0x000b) ncmd 1
>> Status: Success (0x00)
>> < HCI Command: LE Set Scan Enable (0x08|0x000c) plen 2 #87 [hci1] 13:00:02.639663
>> Scanning: Enabled (0x01)
>> Filter duplicates: Enabled (0x01)
>>> HCI Event: Command Complete (0x0e) plen 4 #88 [hci1] 13:00:02.640420
>> LE Set Scan Enable (0x08|0x000c) ncmd 2
>> Status: Success (0x00)
>> ---
>> monitor/main.c | 7 ++++++-
>> monitor/packet.c | 37 +++++++++++++++++++++++++++----------
>> monitor/packet.h | 1 +
>> 3 files changed, 34 insertions(+), 11 deletions(-)
>>
>> diff --git a/monitor/main.c b/monitor/main.c
>> index f9bca22..e1a2375 100644
>> --- a/monitor/main.c
>> +++ b/monitor/main.c
>> @@ -70,6 +70,7 @@ static void usage(void)
>> "\t-T, --date Show time and date information\n"
>> "\t-S, --sco Dump SCO traffic\n"
>> "\t-E, --ellisys [ip] Send Ellisys HCI Injection\n"
>> + "\t-f, --frames Show frame counter\n"
>> "\t-h, --help Show help options\n");
>> }
>
> I would rather use -C / --counter as frame counter.
>
>> @@ -86,6 +87,7 @@ static const struct option main_options[] = {
>> { "date", no_argument, NULL, 'T' },
>> { "sco", no_argument, NULL, 'S' },
>> { "ellisys", required_argument, NULL, 'E' },
>> + { "frames", no_argument, NULL, 'f' },
>> { "todo", no_argument, NULL, '#' },
>> { "version", no_argument, NULL, 'v' },
>> { "help", no_argument, NULL, 'h' },
>> @@ -113,7 +115,7 @@ int main(int argc, char *argv[])
>> for (;;) {
>> int opt;
>>
>> - opt = getopt_long(argc, argv, "d:r:w:a:s:p:i:tTSE:vh",
>> + opt = getopt_long(argc, argv, "d:r:w:a:s:p:i:tfTSE:vh",
>> main_options, NULL);
>> if (opt < 0)
>> break;
>> @@ -171,6 +173,9 @@ int main(int argc, char *argv[])
>> ellisys_server = optarg;
>> ellisys_port = 24352;
>> break;
>> + case 'f':
>> + filter_mask |= PACKET_FILTER_SHOW_FRAME;
>> + break;
>> case '#':
>> packet_todo();
>> lmp_todo();
>> diff --git a/monitor/packet.c b/monitor/packet.c
>> index 3c43356..e5f2a32 100644
>> --- a/monitor/packet.c
>> +++ b/monitor/packet.c
>> @@ -58,6 +58,7 @@
>> #include "packet.h"
>>
>> #define COLOR_CHANNEL_LABEL COLOR_WHITE
>> +#define COLOR_FRAME_LABEL COLOR_WHITE
>> #define COLOR_INDEX_LABEL COLOR_WHITE
>> #define COLOR_TIMESTAMP COLOR_YELLOW
>>
>> @@ -259,6 +260,18 @@ void packet_select_index(uint16_t index)
>>
>> #define print_space(x) printf("%*c", (x), ' ');
>>
>> +#define MAX_INDEX 16
>> +
>> +struct index_data {
>> + uint8_t type;
>> + uint8_t bdaddr[6];
>> + uint16_t manufacturer;
>> + size_t frame;
>> +};
>> +
>> +static struct index_data index_list[MAX_INDEX];
>> +
>> +
>
> Too many empty lines here.
Will fix it.
>> static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
>> uint16_t index, const char *channel,
>> const char *color, const char *label,
>> @@ -280,6 +293,20 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
>> ts_pos += n;
>> ts_len += n;
>> }
>> + } else if (filter_mask & PACKET_FILTER_SHOW_FRAME &&
>> + (ident == '<' || ident == '>')) {
>
> Lets not use the ident to choose here.
>
>> + if (use_color()) {
>> + n = sprintf(ts_str + ts_pos, "%s", COLOR_FRAME_LABEL);
>> + if (n > 0)
>> + ts_pos += n;
>> + }
>> +
>> + n = sprintf(ts_str + ts_pos, " #%zu",
>> + ++index_list[index].frame);
>
> I would prefer if we add the frame number during input and not during display of the packet. That should be kept separate. Otherwise we create a mess like in hcidump.
I don't quite follow the during input vs display, or do you mean we
should increment directly in the io callback?
>> + if (n > 0) {
>> + ts_pos += n;
>> + ts_len += n;
>> + }
>> }
>>
>> if ((filter_mask & PACKET_FILTER_SHOW_INDEX) &&
>> @@ -3828,16 +3855,6 @@ static int addr2str(const uint8_t *addr, char *str)
>> addr[5], addr[4], addr[3], addr[2], addr[1], addr[0]);
>> }
>>
>> -#define MAX_INDEX 16
>> -
>> -struct index_data {
>> - uint8_t type;
>> - uint8_t bdaddr[6];
>> - uint16_t manufacturer;
>> -};
>> -
>> -static struct index_data index_list[MAX_INDEX];
>> -
>> void packet_monitor(struct timeval *tv, struct ucred *cred,
>> uint16_t index, uint16_t opcode,
>> const void *data, uint16_t size)
>> diff --git a/monitor/packet.h b/monitor/packet.h
>> index 354f4fe..2516ec1 100644
>> --- a/monitor/packet.h
>> +++ b/monitor/packet.h
>> @@ -33,6 +33,7 @@
>> #define PACKET_FILTER_SHOW_TIME_OFFSET (1 << 3)
>> #define PACKET_FILTER_SHOW_ACL_DATA (1 << 4)
>> #define PACKET_FILTER_SHOW_SCO_DATA (1 << 5)
>> +#define PACKET_FILTER_SHOW_FRAME (1 << 6)
>
> Call it FRAME_COUNTER or something similar. Just FRAME is misleading. Or we might just always show the frame counter anyway. It is not really in the way.
I wondering if we should enable it by default and don't bother with
the option adding yet another option.
> Regards
>
> Marcel
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH BlueZ 1/3] monitor: Add frame counter support
@ 2017-04-23 20:14 Luiz Augusto von Dentz
2017-04-23 20:14 ` [PATCH BlueZ 2/3] monitor: Fix not decoding control frames Luiz Augusto von Dentz
0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2017-04-23 20:14 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This adds frame counter support for io frames:
> HCI Event: Command Complete (0x0e) plen 4 #86 [hci1] 13:00:02.639412
LE Set Scan Parameters (0x08|0x000b) ncmd 1
Status: Success (0x00)
< HCI Command: LE Set Scan Enable (0x08|0x000c) plen 2 #87 [hci1] 13:00:02.639663
Scanning: Enabled (0x01)
Filter duplicates: Enabled (0x01)
> HCI Event: Command Complete (0x0e) plen 4 #88 [hci1] 13:00:02.640420
LE Set Scan Enable (0x08|0x000c) ncmd 2
Status: Success (0x00)
---
monitor/packet.c | 45 +++++++++++++++++++++++++++++++++++----------
1 file changed, 35 insertions(+), 10 deletions(-)
diff --git a/monitor/packet.c b/monitor/packet.c
index 3c43356..89423d1 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -58,6 +58,7 @@
#include "packet.h"
#define COLOR_CHANNEL_LABEL COLOR_WHITE
+#define COLOR_FRAME_LABEL COLOR_WHITE
#define COLOR_INDEX_LABEL COLOR_WHITE
#define COLOR_TIMESTAMP COLOR_YELLOW
@@ -259,6 +260,17 @@ void packet_select_index(uint16_t index)
#define print_space(x) printf("%*c", (x), ' ');
+#define MAX_INDEX 16
+
+struct index_data {
+ uint8_t type;
+ uint8_t bdaddr[6];
+ uint16_t manufacturer;
+ size_t frame;
+};
+
+static struct index_data index_list[MAX_INDEX];
+
static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
uint16_t index, const char *channel,
const char *color, const char *label,
@@ -267,6 +279,7 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
int col = num_columns();
char line[256], ts_str[96];
int n, ts_len = 0, ts_pos = 0, len = 0, pos = 0;
+ static size_t last_frame;
if (channel) {
if (use_color()) {
@@ -280,6 +293,20 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
ts_pos += n;
ts_len += n;
}
+ } else if (index != HCI_DEV_NONE &&
+ index_list[index].frame != last_frame) {
+ if (use_color()) {
+ n = sprintf(ts_str + ts_pos, "%s", COLOR_FRAME_LABEL);
+ if (n > 0)
+ ts_pos += n;
+ }
+
+ n = sprintf(ts_str + ts_pos, " #%zu", index_list[index].frame);
+ if (n > 0) {
+ ts_pos += n;
+ ts_len += n;
+ }
+ last_frame = index_list[index].frame;
}
if ((filter_mask & PACKET_FILTER_SHOW_INDEX) &&
@@ -3828,16 +3855,6 @@ static int addr2str(const uint8_t *addr, char *str)
addr[5], addr[4], addr[3], addr[2], addr[1], addr[0]);
}
-#define MAX_INDEX 16
-
-struct index_data {
- uint8_t type;
- uint8_t bdaddr[6];
- uint16_t manufacturer;
-};
-
-static struct index_data index_list[MAX_INDEX];
-
void packet_monitor(struct timeval *tv, struct ucred *cred,
uint16_t index, uint16_t opcode,
const void *data, uint16_t size)
@@ -9156,6 +9173,8 @@ void packet_hci_command(struct timeval *tv, struct ucred *cred, uint16_t index,
char extra_str[25], vendor_str[150];
int i;
+ index_list[index].frame++;
+
if (size < HCI_COMMAND_HDR_SIZE) {
sprintf(extra_str, "(len %d)", size);
print_packet(tv, cred, '*', index, NULL, COLOR_ERROR,
@@ -9257,6 +9276,8 @@ void packet_hci_event(struct timeval *tv, struct ucred *cred, uint16_t index,
char extra_str[25];
int i;
+ index_list[index].frame++;
+
if (size < HCI_EVENT_HDR_SIZE) {
sprintf(extra_str, "(len %d)", size);
print_packet(tv, cred, '*', index, NULL, COLOR_ERROR,
@@ -9329,6 +9350,8 @@ void packet_hci_acldata(struct timeval *tv, struct ucred *cred, uint16_t index,
uint8_t flags = acl_flags(handle);
char handle_str[16], extra_str[32];
+ index_list[index].frame++;
+
if (size < HCI_ACL_HDR_SIZE) {
if (in)
print_packet(tv, cred, '*', index, NULL, COLOR_ERROR,
@@ -9371,6 +9394,8 @@ void packet_hci_scodata(struct timeval *tv, struct ucred *cred, uint16_t index,
uint8_t flags = acl_flags(handle);
char handle_str[16], extra_str[32];
+ index_list[index].frame++;
+
if (size < HCI_SCO_HDR_SIZE) {
if (in)
print_packet(tv, cred, '*', index, NULL, COLOR_ERROR,
--
2.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH BlueZ 2/3] monitor: Fix not decoding control frames
2017-04-23 20:14 [PATCH BlueZ 1/3] monitor: Add frame counter support Luiz Augusto von Dentz
@ 2017-04-23 20:14 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2017-04-23 20:14 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
In order to enable decoding control frames packet_monitor needs to check
if the index is set to HCI_DEV_NONE since that will call packet_ctrl_open
which setups the ctrl and assign it a cookie.
---
monitor/packet.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/monitor/packet.c b/monitor/packet.c
index 89423d1..2e09dda 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -3866,10 +3866,11 @@ void packet_monitor(struct timeval *tv, struct ucred *cred,
uint16_t manufacturer;
const char *ident;
- if (index_filter && index_number != index)
- return;
-
- index_current = index;
+ if (index != HCI_DEV_NONE) {
+ if (index_filter && index_number != index)
+ return;
+ index_current = index;
+ }
if (tv && time_offset == ((time_t) -1))
time_offset = tv->tv_sec;
--
2.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-04-23 20:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-20 12:32 [PATCH BlueZ 1/3] monitor: Add frame number support Luiz Augusto von Dentz
2017-04-20 12:32 ` [PATCH BlueZ 2/3] monitor: Fix not decoding control frames Luiz Augusto von Dentz
2017-04-20 12:32 ` [PATCH BlueZ 3/3] client: Always start an agent Luiz Augusto von Dentz
2017-04-20 12:52 ` Marcel Holtmann
2017-04-20 13:13 ` Luiz Augusto von Dentz
2017-04-20 12:57 ` [PATCH BlueZ 1/3] monitor: Add frame number support Marcel Holtmann
2017-04-20 13:20 ` Luiz Augusto von Dentz
-- strict thread matches above, loose matches on Subject: below --
2017-04-23 20:14 [PATCH BlueZ 1/3] monitor: Add frame counter support Luiz Augusto von Dentz
2017-04-23 20:14 ` [PATCH BlueZ 2/3] monitor: Fix not decoding control frames Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox