* Re: [PATCH] Bluetooth: Defer connection-parameter removal when unpairing without disconnecting
From: Alfonso Acosta @ 2014-10-09 13:26 UTC (permalink / raw)
To: BlueZ development
In-Reply-To: <1412859828-6224-1-git-send-email-fons@spotify.com>
Hi,
As an alternative to this patch, I am thinking that it may be worth
considering two other options:
1. Adding an additional repairing operation: MGMT_OP_REPAIR_DEVICE.
Making the repairing semantics explicit would allow us to keep the connection
parameters even if we choose to disconnect when unpairing. On the
other hand, one could argue that it clutters the API with an extra
operation which is the composite of two already existing operations.
2. Make MGMT_OP_PAIR_DEVICE behave like the repairing operation in (1)
if the device was already paired. If I recall properly, Johan suggested
this on IRC.
Comments?
Thanks,
Fons
On Thu, Oct 9, 2014 at 3:03 PM, Alfonso Acosta <fons@spotify.com> wrote:
> Systematically removing the LE connection parameters and autoconnect
> action is inconvenient for rebonding without disconnecting from
> userland (i.e. unpairing followed by repairing without
> disconnecting). The parameters will be lost after unparing and
> userland needs to take care of book-keeping them and re-adding them.
>
> This patch allows userland to forget about parameter management when
> rebonding without disconnecting. It defers clearing the connection
> parameters when unparing without disconnecting, giving a chance of
> keeping the parameters if a repairing happens before the connection is
> closed.
>
> Signed-off-by: Alfonso Acosta <fons@spotify.com>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_conn.c | 6 ++++++
> net/bluetooth/mgmt.c | 16 +++++++++++++---
> 3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 07ddeed62..b8685a7 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -555,6 +555,7 @@ enum {
> HCI_CONN_STK_ENCRYPT,
> HCI_CONN_AUTH_INITIATOR,
> HCI_CONN_DROP,
> + HCI_CONN_PARAM_REMOVAL_PEND,
> };
>
> static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index b9517bd..d106f29 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -356,6 +356,9 @@ static void hci_conn_timeout(struct work_struct *work)
> conn->state = BT_CLOSED;
> break;
> }
> +
> + if (test_and_clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->flags))
> + hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type);
> }
>
> /* Enter sniff mode */
> @@ -544,6 +547,9 @@ int hci_conn_del(struct hci_conn *conn)
>
> hci_conn_del_sysfs(conn);
>
> + if (test_and_clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->flags))
> + hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type);
> +
> hci_dev_put(hdev);
>
> hci_conn_put(conn);
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 3fd88b0..0f2c0e9 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -2700,6 +2700,7 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
> struct hci_cp_disconnect dc;
> struct pending_cmd *cmd;
> struct hci_conn *conn;
> + struct hci_conn *le_conn = NULL;
> int err;
>
> memset(&rp, 0, sizeof(rp));
> @@ -2736,7 +2737,17 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
>
> hci_remove_irk(hdev, &cp->addr.bdaddr, addr_type);
>
> - hci_conn_params_del(hdev, &cp->addr.bdaddr, addr_type);
> + le_conn = hci_conn_hash_lookup_ba(hdev, LE_LINK,
> + &cp->addr.bdaddr);
> +
> + /* If the BLE connection is being used, defer clearing up
> + * the connection parameters until closing to give a
> + * chance of keeping them if a repairing happens.
> + */
> + if (le_conn && !cp->disconnect)
> + set_bit(HCI_CONN_PARAM_REMOVAL_PEND, &le_conn->flags);
> + else
> + hci_conn_params_del(hdev, &cp->addr.bdaddr, addr_type);
>
> err = hci_remove_ltk(hdev, &cp->addr.bdaddr, addr_type);
> }
> @@ -2752,8 +2763,7 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
> conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
> &cp->addr.bdaddr);
> else
> - conn = hci_conn_hash_lookup_ba(hdev, LE_LINK,
> - &cp->addr.bdaddr);
> + conn = le_conn;
> } else {
> conn = NULL;
> }
> --
> 1.9.1
>
--
Alfonso Acosta
Embedded Systems Engineer at Spotify
Birger Jarlsgatan 61, Stockholm, Sweden
http://www.spotify.com
^ permalink raw reply
* [PATCH] Bluetooth: Defer connection-parameter removal when unpairing without disconnecting
From: Alfonso Acosta @ 2014-10-09 13:03 UTC (permalink / raw)
To: linux-bluetooth
Systematically removing the LE connection parameters and autoconnect
action is inconvenient for rebonding without disconnecting from
userland (i.e. unpairing followed by repairing without
disconnecting). The parameters will be lost after unparing and
userland needs to take care of book-keeping them and re-adding them.
This patch allows userland to forget about parameter management when
rebonding without disconnecting. It defers clearing the connection
parameters when unparing without disconnecting, giving a chance of
keeping the parameters if a repairing happens before the connection is
closed.
Signed-off-by: Alfonso Acosta <fons@spotify.com>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_conn.c | 6 ++++++
net/bluetooth/mgmt.c | 16 +++++++++++++---
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 07ddeed62..b8685a7 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -555,6 +555,7 @@ enum {
HCI_CONN_STK_ENCRYPT,
HCI_CONN_AUTH_INITIATOR,
HCI_CONN_DROP,
+ HCI_CONN_PARAM_REMOVAL_PEND,
};
static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b9517bd..d106f29 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -356,6 +356,9 @@ static void hci_conn_timeout(struct work_struct *work)
conn->state = BT_CLOSED;
break;
}
+
+ if (test_and_clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->flags))
+ hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type);
}
/* Enter sniff mode */
@@ -544,6 +547,9 @@ int hci_conn_del(struct hci_conn *conn)
hci_conn_del_sysfs(conn);
+ if (test_and_clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->flags))
+ hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type);
+
hci_dev_put(hdev);
hci_conn_put(conn);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3fd88b0..0f2c0e9 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2700,6 +2700,7 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
struct hci_cp_disconnect dc;
struct pending_cmd *cmd;
struct hci_conn *conn;
+ struct hci_conn *le_conn = NULL;
int err;
memset(&rp, 0, sizeof(rp));
@@ -2736,7 +2737,17 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
hci_remove_irk(hdev, &cp->addr.bdaddr, addr_type);
- hci_conn_params_del(hdev, &cp->addr.bdaddr, addr_type);
+ le_conn = hci_conn_hash_lookup_ba(hdev, LE_LINK,
+ &cp->addr.bdaddr);
+
+ /* If the BLE connection is being used, defer clearing up
+ * the connection parameters until closing to give a
+ * chance of keeping them if a repairing happens.
+ */
+ if (le_conn && !cp->disconnect)
+ set_bit(HCI_CONN_PARAM_REMOVAL_PEND, &le_conn->flags);
+ else
+ hci_conn_params_del(hdev, &cp->addr.bdaddr, addr_type);
err = hci_remove_ltk(hdev, &cp->addr.bdaddr, addr_type);
}
@@ -2752,8 +2763,7 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
&cp->addr.bdaddr);
else
- conn = hci_conn_hash_lookup_ba(hdev, LE_LINK,
- &cp->addr.bdaddr);
+ conn = le_conn;
} else {
conn = NULL;
}
--
1.9.1
^ permalink raw reply related
* RE: [PATCH] Monitor: Fix indentation for AVRCP PASS THROUGH commands
From: Vikrampal @ 2014-10-09 12:59 UTC (permalink / raw)
To: 'Luiz Augusto von Dentz'
Cc: linux-bluetooth, 'Dmitry Kasatkin', cpgs
In-Reply-To: <CABBYNZJ-SdhX2S6_BPoPYe2qyavnpqhWAYgKmPi4qAABsrMw2g@mail.gmail.com>
Hi Luiz,
> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> Sent: Thursday, October 09, 2014 5:29 PM
> To: Vikrampal Yadav
> Cc: linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; cpgs@samsung.com
> Subject: Re: [PATCH] Monitor: Fix indentation for AVRCP PASS THROUGH
> commands
>
> Hi Vikram,
>
> On Thu, Oct 9, 2014 at 2:57 PM, Vikrampal Yadav
> <vikram.pal@samsung.com> wrote:
> > Intendation for AVRCP PASS THROUGH commands' decoding fixed.
>
> Please use lower case at the beginning e.g. monitor:
>
> > ---
> > monitor/avctp.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/monitor/avctp.c b/monitor/avctp.c index a4e34c5..4abd18f
> > 100644
> > --- a/monitor/avctp.c
> > +++ b/monitor/avctp.c
> > @@ -686,13 +686,13 @@ static bool avrcp_passthrough_packet(struct
> avctp_frame *avctp_frame,
> > if (!l2cap_frame_get_u8(frame, &op))
> > return false;
> >
> > - print_field("%*cOperation: 0x%02x (%s %s)", (indent - 2), ' ', op,
> > + print_field("%*cOperation: 0x%02x (%s %s)", (indent - 8), ' ',
> > + op,
> > op2str(op), op & 0x80 ? "Released" :
> > "Pressed");
> >
> > if (!l2cap_frame_get_u8(frame, &len))
> > return false;
> >
> > - print_field("%*cLength: 0x%02x", (indent - 2), ' ', len);
> > + print_field("%*cLength: 0x%02x", (indent - 8), ' ', len);
> >
> > packet_hexdump(frame->data, frame->size);
> > return true;
> > --
> > 1.9.1
>
>
> Could you please start adding the output of the btmon to the description
> once you add new parsers like this that way we can spot more easily
> formatting bugs such as this.
>
>
>
> --
> Luiz Augusto von Dentz
Sure!
Regards,
Vikram
^ permalink raw reply
* Re: [PATCH] bnep: Add extra debug information for bnep_up
From: Szymon Janc @ 2014-10-09 12:48 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <3300021.NoEnUzGqr8@uw000953>
On Thursday 09 of October 2014 13:30:44 Szymon Janc wrote:
> Hi Andrei,
>
> On Thursday 09 of October 2014 13:17:43 Andrei Emeltchenko wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > Adding extra debug information helps to investigate failing cases
> > ---
> > profiles/network/bnep.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> > index 136709d..a6b8728 100644
> > --- a/profiles/network/bnep.c
> > +++ b/profiles/network/bnep.c
> > @@ -210,7 +210,8 @@ static int bnep_if_up(const char *devname)
> > close(sk);
> >
> > if (err < 0) {
> > - error("Could not bring up %s", devname);
> > + error("Could not bring up %s: %s(%d)", devname, strerror(errno),
> > + errno);
> > return err;
> > }
>
> You should use -err no errno here. Errno might be already overwritten by call
> to close().
Well, not exactly, errno should be also stored to err before calling close().
--
Best regards,
Szymon Janc
^ permalink raw reply
* [PATCH 10/10] android/client: Add completion for mce method
From: Grzegorz Kolodziejczyk @ 2014-10-09 12:45 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Aleksander Drewnicki
In-Reply-To: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
From: Aleksander Drewnicki <ext.aleksander.drewnicki@tieto.com>
This patch adds completion functions to mce method.
---
android/client/if-mce.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/android/client/if-mce.c b/android/client/if-mce.c
index 76c9f73..5f101e0 100644
--- a/android/client/if-mce.c
+++ b/android/client/if-mce.c
@@ -54,6 +54,15 @@ static void init_p(int argc, const char **argv)
EXEC(if_mce->init, &mce_cbacks);
}
+static void get_remote_mas_instances_c(int argc, const char **argv,
+ enum_func *enum_func, void **user)
+{
+ if (argc == 3) {
+ *user = NULL;
+ *enum_func = enum_devices;
+ }
+}
+
/* search for MAS instances on remote device */
static void get_remote_mas_instances_p(int argc, const char **argv)
@@ -68,7 +77,7 @@ static void get_remote_mas_instances_p(int argc, const char **argv)
static struct method methods[] = {
STD_METHOD(init),
- STD_METHODH(get_remote_mas_instances, "<addr>"),
+ STD_METHODCH(get_remote_mas_instances, "<addr>"),
END_METHOD
};
--
1.9.3
^ permalink raw reply related
* [PATCH 09/10] android/client: Add code for mce method
From: Grzegorz Kolodziejczyk @ 2014-10-09 12:45 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Aleksander Drewnicki
In-Reply-To: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
From: Aleksander Drewnicki <ext.aleksander.drewnicki@tieto.com>
This patch adds implementation of mce method to haltest.
---
android/client/if-mce.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/android/client/if-mce.c b/android/client/if-mce.c
index c671fb2..76c9f73 100644
--- a/android/client/if-mce.c
+++ b/android/client/if-mce.c
@@ -58,6 +58,12 @@ static void init_p(int argc, const char **argv)
static void get_remote_mas_instances_p(int argc, const char **argv)
{
+ bt_bdaddr_t addr;
+
+ RETURN_IF_NULL(if_mce);
+ VERIFY_ADDR_ARG(2, &addr);
+
+ EXEC(if_mce->get_remote_mas_instances, &addr);
}
static struct method methods[] = {
--
1.9.3
^ permalink raw reply related
* [PATCH 08/10] android/client: Add code for mce callback
From: Grzegorz Kolodziejczyk @ 2014-10-09 12:45 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Aleksander Drewnicki
In-Reply-To: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
From: Aleksander Drewnicki <ext.aleksander.drewnicki@tieto.com>
This adds implementation for mce callback.
---
android/client/if-mce.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/android/client/if-mce.c b/android/client/if-mce.c
index f9ff5f1..c671fb2 100644
--- a/android/client/if-mce.c
+++ b/android/client/if-mce.c
@@ -28,6 +28,16 @@ static void btmce_remote_mas_instances_cb(bt_status_t status,
int num_instances,
btmce_mas_instance_t *instances)
{
+ int i;
+
+ haltest_info("%s: status=%s bd_addr=%s num_instance=%d\n", __func__,
+ bt_status_t2str(status), bdaddr2str(bd_addr),
+ num_instances);
+
+ for (i = 0; i < num_instances; i++)
+ haltest_info("id=%d scn=%d msg_types=%d name=%s\n",
+ instances[i].id, instances[i].scn,
+ instances[i].msg_types, instances[i].p_name);
}
static btmce_callbacks_t mce_cbacks = {
--
1.9.3
^ permalink raw reply related
* [PATCH 07/10] android/client: Add skeleton for mce calls
From: Grzegorz Kolodziejczyk @ 2014-10-09 12:45 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Aleksander Drewnicki
In-Reply-To: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
From: Aleksander Drewnicki <ext.aleksander.drewnicki@tieto.com>
This patch adds skeleton for all methods of mce along with
all callbacks.
---
android/Android.mk | 4 +++-
android/Makefile.am | 1 +
android/client/haltest.c | 2 ++
android/client/if-bt.c | 3 +++
android/client/if-main.h | 3 +++
android/client/if-mce.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 74 insertions(+), 1 deletion(-)
create mode 100644 android/client/if-mce.c
diff --git a/android/Android.mk b/android/Android.mk
index a9e5d4b..aefe41c 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -184,7 +184,9 @@ LOCAL_SRC_FILES := \
bluez/android/hal-utils.c \
ifeq ($(BLUEZ_EXTENSIONS), true)
-LOCAL_SRC_FILES += bluez/android/client/if-hf-client.c
+LOCAL_SRC_FILES += \
+ bluez/android/client/if-hf-client.c \
+ bluez/android/client/if-mce.c
endif
LOCAL_C_INCLUDES += \
diff --git a/android/Makefile.am b/android/Makefile.am
index 9279bbd..b3eb1c5 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -125,6 +125,7 @@ android_haltest_SOURCES = android/client/haltest.c \
android/client/if-sock.c \
android/client/if-audio.c \
android/client/if-sco.c \
+ android/client/if-mce.c \
android/hardware/hardware.c \
android/hal-utils.h android/hal-utils.c
android_haltest_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/android \
diff --git a/android/client/haltest.c b/android/client/haltest.c
index 5e1633d..781e45c 100644
--- a/android/client/haltest.c
+++ b/android/client/haltest.c
@@ -50,6 +50,7 @@ const struct interface *interfaces[] = {
&sock_if,
#ifdef BLUEZ_EXTENSIONS
&hf_client_if,
+ &mce_if,
#endif
NULL
};
@@ -397,6 +398,7 @@ static void init(void)
BT_PROFILE_SOCKETS_ID,
#ifdef BLUEZ_EXTENSIONS
BT_PROFILE_HANDSFREE_CLIENT_ID,
+ BT_PROFILE_MAP_CLIENT_ID,
#endif
};
const struct method *m;
diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index 8f98ef3..2d7ac79 100644
--- a/android/client/if-bt.c
+++ b/android/client/if-bt.c
@@ -736,6 +736,7 @@ static void get_profile_interface_c(int argc, const char **argv,
BT_PROFILE_AV_RC_ID,
#ifdef BLUEZ_EXTENSIONS
BT_PROFILE_HANDSFREE_CLIENT_ID,
+ BT_PROFILE_MAP_CLIENT_ID,
#endif
NULL
};
@@ -778,6 +779,8 @@ static void get_profile_interface_p(int argc, const char **argv)
#ifdef BLUEZ_EXTENSIONS
else if (strcmp(BT_PROFILE_HANDSFREE_CLIENT_ID, id) == 0)
pif = (const void **) &if_hf_client;
+ else if (strcmp(BT_PROFILE_MAP_CLIENT_ID, id) == 0)
+ pif = (const void **) &if_mce;
#endif
else
haltest_error("%s is not correct for get_profile_interface\n",
diff --git a/android/client/if-main.h b/android/client/if-main.h
index 8aac3e3..cb5f558 100644
--- a/android/client/if-main.h
+++ b/android/client/if-main.h
@@ -39,6 +39,7 @@
#ifdef BLUEZ_EXTENSIONS
#include <hardware/bt_hf_client.h>
+#include <hardware/bt_mce.h>
#endif
#include <hardware/bt_rc.h>
@@ -63,6 +64,7 @@ extern const btgatt_server_interface_t *if_gatt_server;
extern const btgatt_client_interface_t *if_gatt_client;
#ifdef BLUEZ_EXTENSIONS
extern const bthf_client_interface_t *if_hf_client;
+extern const btmce_interface_t *if_mce;
#endif
/*
@@ -89,6 +91,7 @@ extern const struct interface hh_if;
extern const struct interface hl_if;
#ifdef BLUEZ_EXTENSIONS
extern const struct interface hf_client_if;
+extern const struct interface mce_if;
#endif
/* Interfaces that will show up in tool (first part of command line) */
diff --git a/android/client/if-mce.c b/android/client/if-mce.c
new file mode 100644
index 0000000..f9ff5f1
--- /dev/null
+++ b/android/client/if-mce.c
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+#include "if-main.h"
+#include "../hal-utils.h"
+
+const btmce_interface_t *if_mce = NULL;
+
+/*
+ * Callback for get_remote_mas_instances
+ */
+static void btmce_remote_mas_instances_cb(bt_status_t status,
+ bt_bdaddr_t *bd_addr,
+ int num_instances,
+ btmce_mas_instance_t *instances)
+{
+}
+
+static btmce_callbacks_t mce_cbacks = {
+ .size = sizeof(mce_cbacks),
+ .remote_mas_instances_cb = btmce_remote_mas_instances_cb,
+};
+
+/* init */
+
+static void init_p(int argc, const char **argv)
+{
+ RETURN_IF_NULL(if_mce);
+
+ EXEC(if_mce->init, &mce_cbacks);
+}
+
+/* search for MAS instances on remote device */
+
+static void get_remote_mas_instances_p(int argc, const char **argv)
+{
+}
+
+static struct method methods[] = {
+ STD_METHOD(init),
+ STD_METHODH(get_remote_mas_instances, "<addr>"),
+ END_METHOD
+};
+
+const struct interface mce_if = {
+ .name = "mce",
+ .methods = methods
+};
--
1.9.3
^ permalink raw reply related
* [PATCH 06/10] android/ipc-tester: Add cases for MAP client msg size
From: Grzegorz Kolodziejczyk @ 2014-10-09 12:45 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
Add cases testing message size verification for MAP client opcodes.
---
android/ipc-tester.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/android/ipc-tester.c b/android/ipc-tester.c
index 7dd25e8..357dda9 100644
--- a/android/ipc-tester.c
+++ b/android/ipc-tester.c
@@ -1486,5 +1486,19 @@ int main(int argc, char *argv[])
HAL_SERVICE_ID_BLUETOOTH,
HAL_SERVICE_ID_HANDSFREE_CLIENT);
+ /* check for valid data size for MAP CLIENT */
+ test_datasize_valid("MAP CLIENT Get instances+",
+ HAL_SERVICE_ID_MAP_CLIENT,
+ HAL_OP_MAP_CLIENT_GET_INSTANCES,
+ sizeof(struct hal_cmd_map_client_get_instances),
+ 1, HAL_SERVICE_ID_BLUETOOTH,
+ HAL_SERVICE_ID_MAP_CLIENT);
+ test_datasize_valid("MAP CLIENT Get instances-",
+ HAL_SERVICE_ID_MAP_CLIENT,
+ HAL_OP_MAP_CLIENT_GET_INSTANCES,
+ sizeof(struct hal_cmd_map_client_get_instances),
+ -1, HAL_SERVICE_ID_BLUETOOTH,
+ HAL_SERVICE_ID_MAP_CLIENT);
+
return tester_run();
}
--
1.9.3
^ permalink raw reply related
* [PATCH 05/10] android/ipc-tester: Add case for MAP client service opcode boundries
From: Grzegorz Kolodziejczyk @ 2014-10-09 12:45 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This patch adds test sending out of range opcode for service.
---
android/ipc-tester.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/android/ipc-tester.c b/android/ipc-tester.c
index 161777d..7dd25e8 100644
--- a/android/ipc-tester.c
+++ b/android/ipc-tester.c
@@ -940,6 +940,10 @@ int main(int argc, char *argv[])
HAL_SERVICE_ID_BLUETOOTH,
HAL_SERVICE_ID_HANDSFREE_CLIENT);
+ test_opcode_valid("MAP_CLIENT", HAL_SERVICE_ID_MAP_CLIENT, 0x01, 0,
+ HAL_SERVICE_ID_BLUETOOTH,
+ HAL_SERVICE_ID_MAP_CLIENT);
+
/* check for valid data size */
test_datasize_valid("CORE Register+", HAL_SERVICE_ID_CORE,
HAL_OP_REGISTER_MODULE,
--
1.9.3
^ permalink raw reply related
* [PATCH 04/10] android/ipc-tester: Add missing service opcode boundries test cases
From: Grzegorz Kolodziejczyk @ 2014-10-09 12:45 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This patch adds tests sending out of range opcode for each service.
---
android/ipc-tester.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/android/ipc-tester.c b/android/ipc-tester.c
index ea71c8d..161777d 100644
--- a/android/ipc-tester.c
+++ b/android/ipc-tester.c
@@ -919,9 +919,23 @@ int main(int argc, char *argv[])
test_opcode_valid("PAN", HAL_SERVICE_ID_PAN, 0x05, 0,
HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_PAN);
+ test_opcode_valid("HANDSFREE", HAL_SERVICE_ID_HANDSFREE, 0x0f, 0,
+ HAL_SERVICE_ID_BLUETOOTH,
+ HAL_SERVICE_ID_HANDSFREE);
+
test_opcode_valid("A2DP", HAL_SERVICE_ID_A2DP, 0x03, 0,
HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_A2DP);
+ test_opcode_valid("HEALTH", HAL_SERVICE_ID_HEALTH, 0x06, 0,
+ HAL_SERVICE_ID_BLUETOOTH,
+ HAL_SERVICE_ID_HEALTH);
+
+ test_opcode_valid("AVRCP", HAL_SERVICE_ID_AVRCP, 0x0b, 0,
+ HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_AVRCP);
+
+ test_opcode_valid("GATT", HAL_SERVICE_ID_GATT, 0x24, 0,
+ HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_GATT);
+
test_opcode_valid("HF_CLIENT", HAL_SERVICE_ID_HANDSFREE_CLIENT, 0x10, 0,
HAL_SERVICE_ID_BLUETOOTH,
HAL_SERVICE_ID_HANDSFREE_CLIENT);
--
1.9.3
^ permalink raw reply related
* [PATCH 03/10] android/map-client: Add support for get remote MAS instances
From: Grzegorz Kolodziejczyk @ 2014-10-09 12:45 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This allows to get remote mas instances. In case of wrong sdp record
fail status will be returned in notification.
---
android/map-client.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 123 insertions(+), 1 deletion(-)
diff --git a/android/map-client.c b/android/map-client.c
index 1001b36..adeae4b 100644
--- a/android/map-client.c
+++ b/android/map-client.c
@@ -30,21 +30,143 @@
#include <stdint.h>
#include <glib.h>
+#include "lib/sdp.h"
+#include "lib/sdp_lib.h"
+#include "src/sdp-client.h"
+
#include "ipc.h"
#include "lib/bluetooth.h"
#include "map-client.h"
#include "src/log.h"
#include "hal-msg.h"
+#include "ipc-common.h"
+#include "utils.h"
+#include "src/shared/util.h"
static struct ipc *hal_ipc = NULL;
static bdaddr_t adapter_addr;
+static int fill_mce_inst(void *buf, int32_t id, int32_t scn, int32_t msg_type,
+ const void *name, uint8_t name_len)
+{
+ struct hal_map_client_mas_instance *inst = buf;
+
+ inst->id = id;
+ inst->scn = scn;
+ inst->msg_types = msg_type;
+ inst->name_len = name_len;
+
+ if (name_len)
+ memcpy(inst->name, name, name_len);
+
+ return sizeof(*inst) + name_len;
+}
+
+static void map_client_sdp_search_cb(sdp_list_t *recs, int err, gpointer data)
+{
+ uint8_t buf[IPC_MTU];
+ struct hal_ev_map_client_remote_mas_instances *ev = (void *) buf;
+ bdaddr_t *dst = data;
+ sdp_list_t *list, *protos;
+ uint8_t status;
+ int32_t id, scn, msg_type, name_len, num_instances = 0;
+ char *name;
+ size_t size;
+
+ size = sizeof(*ev);
+ bdaddr2android(dst, &ev->bdaddr);
+
+ if (err < 0) {
+ error("mce: Unable to get SDP record: %s", strerror(-err));
+ status = HAL_STATUS_FAILED;
+ goto fail;
+ }
+
+ for (list = recs; list != NULL; list = list->next) {
+ sdp_record_t *rec = list->data;
+ sdp_data_t *data;
+
+ data = sdp_data_get(rec, SDP_ATTR_MAS_INSTANCE_ID);
+ if (data) {
+ id = data->val.uint8;
+ } else {
+ error("mce: cannot get mas instance id");
+ status = HAL_STATUS_FAILED;
+ goto fail;
+ }
+
+ data = sdp_data_get(rec, SDP_ATTR_SVCNAME_PRIMARY);
+ if (data) {
+ name = data->val.str;
+ name_len = data->unitSize;
+ } else {
+ error("mce: cannot get mas instance name");
+ status = HAL_STATUS_FAILED;
+ goto fail;
+ }
+
+ data = sdp_data_get(rec, SDP_ATTR_SUPPORTED_MESSAGE_TYPES);
+ if (data) {
+ msg_type = data->val.uint8;
+ } else {
+ error("mce: cannot get mas instance msg type");
+ status = HAL_STATUS_FAILED;
+ goto fail;
+ }
+
+ if (!sdp_get_access_protos(rec, &protos)) {
+ scn = sdp_get_proto_port(protos, RFCOMM_UUID);
+
+ sdp_list_foreach(protos,
+ (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(protos, NULL);
+ } else {
+ error("mce: cannot get mas instance rfcomm channel");
+ status = HAL_STATUS_FAILED;
+ goto fail;
+ }
+
+ size += fill_mce_inst(buf + size, id, scn, msg_type, name,
+ name_len);
+ num_instances++;
+ }
+
+ status = HAL_STATUS_SUCCESS;
+
+fail:
+ ev->num_instances = num_instances;
+ ev->status = status;
+
+ ipc_send_notif(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT,
+ HAL_EV_MAP_CLIENT_REMOTE_MAS_INSTANCES, size, buf);
+
+ free(dst);
+}
+
static void handle_get_instances(const void *buf, uint16_t len)
{
+ const struct hal_cmd_map_client_get_instances *cmd = buf;
+ uint8_t status;
+ bdaddr_t *dst;
+ uuid_t uuid;
+
DBG("");
+ dst = new0(bdaddr_t, 1);
+ android2bdaddr(&cmd->bdaddr, dst);
+
+ sdp_uuid16_create(&uuid, MAP_MSE_SVCLASS_ID);
+
+ if (bt_search_service(&adapter_addr, dst, &uuid,
+ map_client_sdp_search_cb, dst, NULL, 0)) {
+ error("mce: Failed to search SDP details");
+ status = HAL_STATUS_FAILED;
+ free(dst);
+ }
+
+ status = HAL_STATUS_SUCCESS;
ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT,
- HAL_OP_MAP_CLIENT_GET_INSTANCES, HAL_STATUS_FAILED);
+ HAL_OP_MAP_CLIENT_GET_INSTANCES, status);
}
static const struct ipc_handler cmd_handlers[] = {
--
1.9.3
^ permalink raw reply related
* [PATCH 02/10] android/map-client: Add stubs for MAP client commands handlers
From: Grzegorz Kolodziejczyk @ 2014-10-09 12:45 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
Add empty handlers for MAP client IPC commands.
---
android/map-client.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/android/map-client.c b/android/map-client.c
index 4556461..1001b36 100644
--- a/android/map-client.c
+++ b/android/map-client.c
@@ -28,17 +28,48 @@
#include <stdbool.h>
#include <stdlib.h>
#include <stdint.h>
+#include <glib.h>
#include "ipc.h"
#include "lib/bluetooth.h"
#include "map-client.h"
+#include "src/log.h"
+#include "hal-msg.h"
+
+static struct ipc *hal_ipc = NULL;
+static bdaddr_t adapter_addr;
+
+static void handle_get_instances(const void *buf, uint16_t len)
+{
+ DBG("");
+
+ ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT,
+ HAL_OP_MAP_CLIENT_GET_INSTANCES, HAL_STATUS_FAILED);
+}
+
+static const struct ipc_handler cmd_handlers[] = {
+ {handle_get_instances, false,
+ sizeof(struct hal_cmd_map_client_get_instances)},
+};
bool bt_map_client_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode)
{
- return false;
+ DBG("");
+
+ bacpy(&adapter_addr, addr);
+
+ hal_ipc = ipc;
+
+ ipc_register(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT, cmd_handlers,
+ G_N_ELEMENTS(cmd_handlers));
+
+ return true;
}
void bt_map_client_unregister(void)
{
+ DBG("");
+ ipc_unregister(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT);
+ hal_ipc = NULL;
}
--
1.9.3
^ permalink raw reply related
* [PATCH 01/10] android/map-client: Add initial files
From: Grzegorz Kolodziejczyk @ 2014-10-09 12:45 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This adds initial daemon code for MAP client profile.
---
android/Android.mk | 1 +
android/Makefile.am | 1 +
android/main.c | 12 ++++++++++++
android/map-client.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
android/map-client.h | 26 ++++++++++++++++++++++++++
5 files changed, 84 insertions(+)
create mode 100644 android/map-client.c
create mode 100644 android/map-client.h
diff --git a/android/Android.mk b/android/Android.mk
index f25d60e..a9e5d4b 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -57,6 +57,7 @@ LOCAL_SRC_FILES := \
bluez/android/gatt.c \
bluez/android/health.c \
bluez/profiles/health/mcap.c \
+ bluez/android/map-client.c \
bluez/src/log.c \
bluez/src/shared/mgmt.c \
bluez/src/shared/util.c \
diff --git a/android/Makefile.am b/android/Makefile.am
index f11cff6..9279bbd 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -46,6 +46,7 @@ android_bluetoothd_SOURCES = android/main.c \
android/gatt.h android/gatt.c \
android/health.h android/health.c \
profiles/health/mcap.h profiles/health/mcap.c \
+ android/map-client.h android/map-client.c \
attrib/att.c attrib/att.h \
attrib/gatt.c attrib/gatt.h \
attrib/gattrib.c attrib/gattrib.h \
diff --git a/android/main.c b/android/main.c
index 703b3b6..b5f6937 100644
--- a/android/main.c
+++ b/android/main.c
@@ -62,6 +62,7 @@
#include "gatt.h"
#include "health.h"
#include "handsfree-client.h"
+#include "map-client.h"
#include "utils.h"
#define DEFAULT_VENDOR "BlueZ"
@@ -235,6 +236,14 @@ static void service_register(const void *buf, uint16_t len)
}
break;
+ case HAL_SERVICE_ID_MAP_CLIENT:
+ if (!bt_map_client_register(hal_ipc, &adapter_bdaddr,
+ m->mode)) {
+ status = HAL_STATUS_FAILED;
+ goto failed;
+ }
+
+ break;
default:
DBG("service %u not supported", m->service_id);
status = HAL_STATUS_FAILED;
@@ -288,6 +297,9 @@ static bool unregister_service(uint8_t id)
case HAL_SERVICE_ID_HANDSFREE_CLIENT:
bt_hf_client_unregister();
break;
+ case HAL_SERVICE_ID_MAP_CLIENT:
+ bt_map_client_unregister();
+ break;
default:
DBG("service %u not supported", id);
return false;
diff --git a/android/map-client.c b/android/map-client.c
new file mode 100644
index 0000000..4556461
--- /dev/null
+++ b/android/map-client.c
@@ -0,0 +1,44 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdint.h>
+
+#include "ipc.h"
+#include "lib/bluetooth.h"
+#include "map-client.h"
+
+bool bt_map_client_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode)
+{
+ return false;
+}
+
+void bt_map_client_unregister(void)
+{
+
+}
diff --git a/android/map-client.h b/android/map-client.h
new file mode 100644
index 0000000..0e63072
--- /dev/null
+++ b/android/map-client.h
@@ -0,0 +1,26 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+bool bt_map_client_register(struct ipc *ipc, const bdaddr_t *addr,
+ uint8_t mode);
+void bt_map_client_unregister(void);
--
1.9.3
^ permalink raw reply related
* [PATCH 00/10] android/map-client: Add MAP client daemon implementation
From: Grzegorz Kolodziejczyk @ 2014-10-09 12:45 UTC (permalink / raw)
To: linux-bluetooth
v1.
*Add MAP client daemon implementation (skeleton, body)
*Add haltest MAP client support
Aleksander Drewnicki (4):
android/client: Add skeleton for mce calls
android/client: Add code for mce callback
android/client: Add code for mce method
android/client: Add completion for mce method
Grzegorz Kolodziejczyk (6):
android/map-client: Add initial files
android/map-client: Add stubs for MAP client commands handlers
android/map-client: Add support for get remote MAS instances
android/ipc-tester: Add missing service opcode boundries test cases
android/ipc-tester: Add case for MAP client service opcode boundries
android/ipc-tester: Add cases for MAP client msg size
android/Android.mk | 5 +-
android/Makefile.am | 2 +
android/client/haltest.c | 2 +
android/client/if-bt.c | 3 +
android/client/if-main.h | 3 +
android/client/if-mce.c | 87 +++++++++++++++++++++
android/ipc-tester.c | 32 ++++++++
android/main.c | 12 +++
android/map-client.c | 197 +++++++++++++++++++++++++++++++++++++++++++++++
android/map-client.h | 26 +++++++
10 files changed, 368 insertions(+), 1 deletion(-)
create mode 100644 android/client/if-mce.c
create mode 100644 android/map-client.c
create mode 100644 android/map-client.h
--
1.9.3
^ permalink raw reply
* [PATCH] monitor: Fix indentation for AVRCP PASS THROUGH commands
From: Vikrampal Yadav @ 2014-10-09 12:13 UTC (permalink / raw)
To: linux-bluetooth; +Cc: luiz.dentz, d.kasatkin, vikram.pal, cpgs
Intendation for AVRCP PASS THROUGH commands' decoding fixed.
---
monitor/avctp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/monitor/avctp.c b/monitor/avctp.c
index a4e34c5..4abd18f 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -686,13 +686,13 @@ static bool avrcp_passthrough_packet(struct avctp_frame *avctp_frame,
if (!l2cap_frame_get_u8(frame, &op))
return false;
- print_field("%*cOperation: 0x%02x (%s %s)", (indent - 2), ' ', op,
+ print_field("%*cOperation: 0x%02x (%s %s)", (indent - 8), ' ', op,
op2str(op), op & 0x80 ? "Released" : "Pressed");
if (!l2cap_frame_get_u8(frame, &len))
return false;
- print_field("%*cLength: 0x%02x", (indent - 2), ' ', len);
+ print_field("%*cLength: 0x%02x", (indent - 8), ' ', len);
packet_hexdump(frame->data, frame->size);
return true;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] Monitor: Fix indentation for AVRCP PASS THROUGH commands
From: Luiz Augusto von Dentz @ 2014-10-09 11:58 UTC (permalink / raw)
To: Vikrampal Yadav; +Cc: linux-bluetooth@vger.kernel.org, Dmitry Kasatkin, cpgs
In-Reply-To: <1412855826-3665-1-git-send-email-vikram.pal@samsung.com>
Hi Vikram,
On Thu, Oct 9, 2014 at 2:57 PM, Vikrampal Yadav <vikram.pal@samsung.com> wrote:
> Intendation for AVRCP PASS THROUGH commands' decoding fixed.
Please use lower case at the beginning e.g. monitor:
> ---
> monitor/avctp.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/monitor/avctp.c b/monitor/avctp.c
> index a4e34c5..4abd18f 100644
> --- a/monitor/avctp.c
> +++ b/monitor/avctp.c
> @@ -686,13 +686,13 @@ static bool avrcp_passthrough_packet(struct avctp_frame *avctp_frame,
> if (!l2cap_frame_get_u8(frame, &op))
> return false;
>
> - print_field("%*cOperation: 0x%02x (%s %s)", (indent - 2), ' ', op,
> + print_field("%*cOperation: 0x%02x (%s %s)", (indent - 8), ' ', op,
> op2str(op), op & 0x80 ? "Released" : "Pressed");
>
> if (!l2cap_frame_get_u8(frame, &len))
> return false;
>
> - print_field("%*cLength: 0x%02x", (indent - 2), ' ', len);
> + print_field("%*cLength: 0x%02x", (indent - 8), ' ', len);
>
> packet_hexdump(frame->data, frame->size);
> return true;
> --
> 1.9.1
Could you please start adding the output of the btmon to the
description once you add new parsers like this that way we can spot
more easily formatting bugs such as this.
--
Luiz Augusto von Dentz
^ permalink raw reply
* [PATCH] Monitor: Fix indentation for AVRCP PASS THROUGH commands
From: Vikrampal Yadav @ 2014-10-09 11:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: luiz.dentz, d.kasatkin, vikram.pal, cpgs
Intendation for AVRCP PASS THROUGH commands' decoding fixed.
---
monitor/avctp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/monitor/avctp.c b/monitor/avctp.c
index a4e34c5..4abd18f 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -686,13 +686,13 @@ static bool avrcp_passthrough_packet(struct avctp_frame *avctp_frame,
if (!l2cap_frame_get_u8(frame, &op))
return false;
- print_field("%*cOperation: 0x%02x (%s %s)", (indent - 2), ' ', op,
+ print_field("%*cOperation: 0x%02x (%s %s)", (indent - 8), ' ', op,
op2str(op), op & 0x80 ? "Released" : "Pressed");
if (!l2cap_frame_get_u8(frame, &len))
return false;
- print_field("%*cLength: 0x%02x", (indent - 2), ' ', len);
+ print_field("%*cLength: 0x%02x", (indent - 8), ' ', len);
packet_hexdump(frame->data, frame->size);
return true;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] bnep: Add extra debug information for bnep_up
From: Szymon Janc @ 2014-10-09 11:30 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1412849863-6691-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
On Thursday 09 of October 2014 13:17:43 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Adding extra debug information helps to investigate failing cases
> ---
> profiles/network/bnep.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index 136709d..a6b8728 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -210,7 +210,8 @@ static int bnep_if_up(const char *devname)
> close(sk);
>
> if (err < 0) {
> - error("Could not bring up %s", devname);
> + error("Could not bring up %s: %s(%d)", devname, strerror(errno),
> + errno);
> return err;
> }
You should use -err no errno here. Errno might be already overwritten by call
to close().
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [PATCH] android/tester: Fix possible NULL pointer passing to function
From: Grzegorz Kolodziejczyk @ 2014-10-09 11:26 UTC (permalink / raw)
To: Andrei Emeltchenko, Grzegorz Kolodziejczyk, linux-bluetooth
In-Reply-To: <20141009112024.GB27705@aemeltch-MOBL1>
Hi Andrei,
On 9 October 2014 13:20, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> Hi Grzegorz,
>
> On Thu, Oct 09, 2014 at 12:56:19PM +0200, Grzegorz Kolodziejczyk wrote:
>> Hi Andrei,
>>
>> On 9 October 2014 12:16, Andrei Emeltchenko
>> <Andrei.Emeltchenko.news@gmail.com> wrote:
>> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>> >
>> > Silence code analyzers and follow strict C standard where passing NULL
>> > pointer results in undefined behaviour.
>> > ---
>> > android/tester-main.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/android/tester-main.c b/android/tester-main.c
>> > index 30e1c59..ee3444f 100644
>> > --- a/android/tester-main.c
>> > +++ b/android/tester-main.c
>> > @@ -801,12 +801,12 @@ static bool match_data(struct step *step)
>> > return false;
>> > }
>> >
>> > - if (exp->store_srvc_handle)
>> > + if (exp->store_srvc_handle && step->callback_result.srvc_handle)
>> > memcpy(exp->store_srvc_handle,
>> > step->callback_result.srvc_handle,
>> > sizeof(*exp->store_srvc_handle));
>> >
>> > - if (exp->store_char_handle)
>> > + if (exp->store_char_handle && step->callback_result.char_handle)
>> > memcpy(exp->store_char_handle,
>> > step->callback_result.char_handle,
>> > sizeof(*exp->store_char_handle));
>> > --
>> > 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
>>
>>
>> I think it can cause test cause bad behavior by not setting variable
>> if test step is obliged to do it.
>
> Do you mean that memcpy(src, 0, size) stores something useful?
>
I mean that memcpy(src, 0, size) shows us that something with test
step is wrong, during test case development it should be analyzed and
fixed.
It's only possible to expect storing srvc_handle while we get
service_added callback - this callback always returns service handle -
similar for characteristics.
I prefer to handle it in this way as I mentioned in previous comment.
> Best regards
> Andrei Emeltchenko
>
>> Test cases are defined in this way that one step e.g. 'A' stores
>> service handle and step 'B' uses this handle.
>> If value is not set it cause wrong test case behavior in next steps.
>> This check of exp (if store of srvc handle is mandatory for this step)
>> and step srvc handle (received in callback) only mask problem
>> with storing attribute handle value - in result it shows only that
>> memory was not duplicated well while copying data from incoming
>> callback handler.
>> Summarizing - we consciously force writing this srvc, char handle
>> value, if something is wrong - that means something with test
>> case is wrong and it should be fixed.
>>
>> Best way to handle it - static analyzer and this logic safe is to do
>> it in this way:
>> "if (exp->store_srvc_handle) {
>> if (!step->callback_result.srvc_handle) {
>> tester_debug("wrong srvc handle received in callback");
>> return false;
>> } else {
>> memcpy(exp->store_char_handle,
>> step->callback_result.char_handle,
>>
>> sizeof(*exp->store_char_handle));
>> }
>> }
>>
>> Best Regards,
>> Grzegorz
Best regards,
Grzegorz
^ permalink raw reply
* Re: [PATCH] android/tester: Fix possible NULL pointer passing to function
From: Andrei Emeltchenko @ 2014-10-09 11:20 UTC (permalink / raw)
To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
In-Reply-To: <CAMv5Gbd8kj0BbgSKL7BkComzpB4qLEXrNX1dCSb5oviHGoH1dA@mail.gmail.com>
Hi Grzegorz,
On Thu, Oct 09, 2014 at 12:56:19PM +0200, Grzegorz Kolodziejczyk wrote:
> Hi Andrei,
>
> On 9 October 2014 12:16, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > Silence code analyzers and follow strict C standard where passing NULL
> > pointer results in undefined behaviour.
> > ---
> > android/tester-main.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/android/tester-main.c b/android/tester-main.c
> > index 30e1c59..ee3444f 100644
> > --- a/android/tester-main.c
> > +++ b/android/tester-main.c
> > @@ -801,12 +801,12 @@ static bool match_data(struct step *step)
> > return false;
> > }
> >
> > - if (exp->store_srvc_handle)
> > + if (exp->store_srvc_handle && step->callback_result.srvc_handle)
> > memcpy(exp->store_srvc_handle,
> > step->callback_result.srvc_handle,
> > sizeof(*exp->store_srvc_handle));
> >
> > - if (exp->store_char_handle)
> > + if (exp->store_char_handle && step->callback_result.char_handle)
> > memcpy(exp->store_char_handle,
> > step->callback_result.char_handle,
> > sizeof(*exp->store_char_handle));
> > --
> > 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
>
>
> I think it can cause test cause bad behavior by not setting variable
> if test step is obliged to do it.
Do you mean that memcpy(src, 0, size) stores something useful?
Best regards
Andrei Emeltchenko
> Test cases are defined in this way that one step e.g. 'A' stores
> service handle and step 'B' uses this handle.
> If value is not set it cause wrong test case behavior in next steps.
> This check of exp (if store of srvc handle is mandatory for this step)
> and step srvc handle (received in callback) only mask problem
> with storing attribute handle value - in result it shows only that
> memory was not duplicated well while copying data from incoming
> callback handler.
> Summarizing - we consciously force writing this srvc, char handle
> value, if something is wrong - that means something with test
> case is wrong and it should be fixed.
>
> Best way to handle it - static analyzer and this logic safe is to do
> it in this way:
> "if (exp->store_srvc_handle) {
> if (!step->callback_result.srvc_handle) {
> tester_debug("wrong srvc handle received in callback");
> return false;
> } else {
> memcpy(exp->store_char_handle,
> step->callback_result.char_handle,
>
> sizeof(*exp->store_char_handle));
> }
> }
>
> Best Regards,
> Grzegorz
^ permalink raw reply
* Re: [PATCH] android/tester: Fix possible NULL pointer passing to function
From: Grzegorz Kolodziejczyk @ 2014-10-09 10:56 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1412849812-6498-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
On 9 October 2014 12:16, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Silence code analyzers and follow strict C standard where passing NULL
> pointer results in undefined behaviour.
> ---
> android/tester-main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/android/tester-main.c b/android/tester-main.c
> index 30e1c59..ee3444f 100644
> --- a/android/tester-main.c
> +++ b/android/tester-main.c
> @@ -801,12 +801,12 @@ static bool match_data(struct step *step)
> return false;
> }
>
> - if (exp->store_srvc_handle)
> + if (exp->store_srvc_handle && step->callback_result.srvc_handle)
> memcpy(exp->store_srvc_handle,
> step->callback_result.srvc_handle,
> sizeof(*exp->store_srvc_handle));
>
> - if (exp->store_char_handle)
> + if (exp->store_char_handle && step->callback_result.char_handle)
> memcpy(exp->store_char_handle,
> step->callback_result.char_handle,
> sizeof(*exp->store_char_handle));
> --
> 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
I think it can cause test cause bad behavior by not setting variable
if test step is obliged to do it.
Test cases are defined in this way that one step e.g. 'A' stores
service handle and step 'B' uses this handle.
If value is not set it cause wrong test case behavior in next steps.
This check of exp (if store of srvc handle is mandatory for this step)
and step srvc handle (received in callback) only mask problem
with storing attribute handle value - in result it shows only that
memory was not duplicated well while copying data from incoming
callback handler.
Summarizing - we consciously force writing this srvc, char handle
value, if something is wrong - that means something with test
case is wrong and it should be fixed.
Best way to handle it - static analyzer and this logic safe is to do
it in this way:
"if (exp->store_srvc_handle) {
if (!step->callback_result.srvc_handle) {
tester_debug("wrong srvc handle received in callback");
return false;
} else {
memcpy(exp->store_char_handle,
step->callback_result.char_handle,
sizeof(*exp->store_char_handle));
}
}
Best Regards,
Grzegorz
^ permalink raw reply
* [PATCH] bnep: Add extra debug information for bnep_up
From: Andrei Emeltchenko @ 2014-10-09 10:17 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Adding extra debug information helps to investigate failing cases
---
profiles/network/bnep.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 136709d..a6b8728 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -210,7 +210,8 @@ static int bnep_if_up(const char *devname)
close(sk);
if (err < 0) {
- error("Could not bring up %s", devname);
+ error("Could not bring up %s: %s(%d)", devname, strerror(errno),
+ errno);
return err;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v4 1/3] shared/att.c: Add signed command outgoing and CSRK function
From: Luiz Augusto von Dentz @ 2014-10-09 10:17 UTC (permalink / raw)
To: Gu, Chao Jie; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <3D02B219753AD44CBDDDE484323B17741130DDF2@SHSMSX104.ccr.corp.intel.com>
Hi,
On Thu, Oct 9, 2014 at 12:38 PM, Gu, Chao Jie <chao.jie.gu@intel.com> wrote:
> Hi Luiz,
>> >
>> >> > @@ -77,6 +78,16 @@ struct bt_att {
>> >> > bt_att_debug_func_t debug_callback;
>> >> > bt_att_destroy_func_t debug_destroy;
>> >> > void *debug_data;
>> >> > +
>> >> > + struct bt_crypto *crypto;
>> >> > +
>> >> > + bool valid_local_csrk;
>> >> > + uint8_t local_csrk[16];
>> >> > + uint32_t local_sign_cnt;
>> >> > +
>> >> > + bool valid_remote_csrk;
>> >> > + uint8_t remote_csrk[16];
>> >> > + uint32_t remote_sign_cnt;
>> >>
>> >> Maybe it is better to have pointers to a structs so you can free the
>> >> data once it becomes invalid and it also removes the necessity of
>> >> extra flags just to tell if the data is still valid since you can check if the pointer is
>> not NULL then it must be valid.
>> >
>> > This is good idea to remove the extra flags. However, we define the
>> > struct to do this which would result in some problem. Now we
>> > initialize the local_sign_cnt in the bt_att_new function and only add
>> > local_sign_cnt value when signed write command outgoing. Meanwile, we
>> > set local CSRK in the bt_gatt_client_set_local_csrk function when there is valid
>> CSRK provided. This two Parameter set at different time now.
>>
>> Well I guess this was your design, it doesn't have to be this way, in fact I believe
>> there is no point of initializing the counter if there is no valid CSRK because one
>> depend on the other, also perhaps you have a bug in bt_gatt_client_set_local_csrk
>> since I believe every time you set CSRK you should reset the counter otherwise you
>> may be carrying the counters from the past CSRK which probably won't work.
>
> You said condition resuilt in bug which I want to avoid, because we use this command line
> By btgatt-client tool
> Write-value -w -c <CSRK> <value_handle> <value>
> If user write signed command twice, it would call bt_gatt_client_set_local_csrk twice.
> So it will restore sign_cnt, that's why I initialize the sign_cnt in bt_att_new to avoid this bug.
Considering what the spec says about SignCounter and that we do in
fact store the counter persistently I believe the counter is valid as
long as CSRK is valid, which means it is not per connection and
initializing it on bt_att_new is probably wrong since the counter will
probably be out of sync. We should either create a dedicate command to
set CSRK and the SignCounter or make write-value to take the counter
as well otherwise it wont work except with a fresh CSRK and
SignCounter which perhaps is the case of btgatt-client but not if we
want to reuse in the daemons.
> In fact, the only way is that we should tell user set CSRK once if CSRK did not change to
> resolve this problem.
>
>> Also don't to have to pass the counter when setting the key, the counter probably
>> need to be restored on every connection so bt_gatt_client_set_local_csrk and
>> bt_att_set_remote_csrk probably needs to take the counter and instead of having a
>> valid flag you can probably reset by setting NULL as key or an invalid counter if one
>> exists.
>
> If we define the new struct naming signed_info to store csrk and sign_cnt, what's your point?
>
>>
>> > If we have pointers to a struct including local_csrk and
>> > local_signd_cnt, we initialize the local_sign_cnt meanwhile the struct should not
>> be NULL because of incuding it. In fact, local_csrk is not valid at this time.
>> > If we initialize the local_signed_cnt when CSRK is valid , it would be
>> > risk to change local_signed_cnt by user calling bt_gatt_client_set_local_csrk.
>> >> > };
>> >> >
>> >> > enum att_op_type {
>> >> > @@ -176,6 +187,8 @@ struct att_send_op {
>> >> > bt_att_response_func_t callback;
>> >> > bt_att_destroy_func_t destroy;
>> >> > void *user_data;
>> >> > +
>> >> > + struct bt_att *att;
>> >> > };
>> >> >
>> >> > static void destroy_att_send_op(void *data) @@ -277,6 +290,10 @@
>> >> > static bool encode_pdu(struct att_send_op *op, const void *pdu,
>> >> > uint16_t length,
>> >> > uint16_t mtu) {
>> >> > uint16_t pdu_len = 1;
>> >> > + struct bt_att *att = op->att;
>> >> > +
>> >> > + if (op->opcode & ATT_OP_SIGNED_MASK)
>> >> > + pdu_len += BT_ATT_SIGNATURE_LEN;
>> >> >
>> >> > if (length && pdu)
>> >> > pdu_len += length;
>> >> > @@ -293,17 +310,32 @@ static bool encode_pdu(struct att_send_op
>> >> > *op, const
>> >> void *pdu,
>> >> > if (pdu_len > 1)
>> >> > memcpy(op->pdu + 1, pdu, length);
>> >> >
>> >> > + if (op->opcode & ATT_OP_SIGNED_MASK) {
>> >> > + if (bt_crypto_sign_att(att->crypto,
>> >> > + att->local_csrk,
>> >> > + op->pdu,
>> >> > + 1 + length,
>> >> > + att->local_sign_cnt,
>> >> > + &((uint8_t *) op->pdu)[1 +
>> >> length]))
>> >> > + att->local_sign_cnt++;
>> >> > + else
>> >> > + return false;
>> >> > + }
>> >> > +
>> >>
>> >> You can probably bail out early if you check !(op->opcode &
>> >> ATT_OP_SIGNED_MASK), also perhaps it is a good idea to start defining
>> >> structs for the PDUs to simplify the access.
>> >>
>> > If we bail out this checking early , maybe we have to need two
>> > different encode_pdu function to implement it such as naming new
>> encode_signed_pdu function to do this.
>>
>> But you return true anyway after this code, no idea why you need a new function for
>> that, all I was suggesting is the following:
>>
>> if (!(op->opcode & ATT_OP_SIGNED_MASK))
>> return true;
>>
>> if (!bt_crypto_sign_att...)
>> return false;
>>
>> att->local_sign_cnt++;
>>
>> return true;
>>
>
> I got your meaning now. I have returned false when bt_crypto_sign_att. Failed
>
> + else
> + return false;
>
> Your proposal is good so as to make code more clearer. I would accept that.
>
> Thanks
> Chaojie Gu
--
Luiz Augusto von Dentz
^ permalink raw reply
* [PATCH] android/tester: Fix possible NULL pointer passing to function
From: Andrei Emeltchenko @ 2014-10-09 10:16 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Silence code analyzers and follow strict C standard where passing NULL
pointer results in undefined behaviour.
---
android/tester-main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/android/tester-main.c b/android/tester-main.c
index 30e1c59..ee3444f 100644
--- a/android/tester-main.c
+++ b/android/tester-main.c
@@ -801,12 +801,12 @@ static bool match_data(struct step *step)
return false;
}
- if (exp->store_srvc_handle)
+ if (exp->store_srvc_handle && step->callback_result.srvc_handle)
memcpy(exp->store_srvc_handle,
step->callback_result.srvc_handle,
sizeof(*exp->store_srvc_handle));
- if (exp->store_char_handle)
+ if (exp->store_char_handle && step->callback_result.char_handle)
memcpy(exp->store_char_handle,
step->callback_result.char_handle,
sizeof(*exp->store_char_handle));
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox