Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH BlueZ v6 03/18] gatt: Register Manager D-Bus Interface
From: Claudio Takahasi @ 2014-02-04 17:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: claudio.takahasi, Alvaro Silva
In-Reply-To: <1391536429-8345-1-git-send-email-claudio.takahasi@openbossa.org>

From: Alvaro Silva <alvaro.silva@openbossa.org>

This patch registers GATT Service Manager D-Bus Interface. This
interface implements the methods to allow external application register
and unregister GATT Services.
---
 Makefile.am     |  1 +
 src/gatt-dbus.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/gatt-dbus.h | 25 +++++++++++++++++++
 src/gatt.c      |  9 +++++++
 4 files changed, 110 insertions(+)
 create mode 100644 src/gatt-dbus.c
 create mode 100644 src/gatt-dbus.h

diff --git a/Makefile.am b/Makefile.am
index 5f61d50..e630fcb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -146,6 +146,7 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
 			src/adapter.h src/adapter.c \
 			src/profile.h src/profile.c \
 			src/service.h src/service.c \
+			src/gatt-dbus.h src/gatt-dbus.c \
 			src/gatt.h src/gatt.c \
 			src/device.h src/device.c src/attio.h \
 			src/dbus-common.c src/dbus-common.h \
diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
new file mode 100644
index 0000000..183c611
--- /dev/null
+++ b/src/gatt-dbus.c
@@ -0,0 +1,75 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Instituto Nokia de Tecnologia - INdT
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program 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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; 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 <stdint.h>
+
+#include <glib.h>
+#include <dbus/dbus.h>
+#include <gdbus/gdbus.h>
+
+#include "dbus-common.h"
+#include "log.h"
+
+#include "gatt-dbus.h"
+
+#define GATT_MGR_IFACE			"org.bluez.GattManager1"
+
+static DBusMessage *register_service(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	return dbus_message_new_method_return(msg);
+}
+
+static DBusMessage *unregister_service(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	return dbus_message_new_method_return(msg);
+}
+
+static const GDBusMethodTable methods[] = {
+	{ GDBUS_EXPERIMENTAL_METHOD("RegisterService",
+				GDBUS_ARGS({ "service", "o"},
+						{ "options", "a{sv}"}),
+				NULL, register_service) },
+	{ GDBUS_EXPERIMENTAL_METHOD("UnregisterService",
+				GDBUS_ARGS({"service", "o"}),
+				NULL, unregister_service) },
+	{ }
+};
+
+gboolean gatt_dbus_manager_register(void)
+{
+	return g_dbus_register_interface(btd_get_dbus_connection(),
+					"/org/bluez", GATT_MGR_IFACE,
+					methods, NULL, NULL, NULL, NULL);
+}
+
+void gatt_dbus_manager_unregister(void)
+{
+	g_dbus_unregister_interface(btd_get_dbus_connection(), "/org/bluez",
+							GATT_MGR_IFACE);
+}
diff --git a/src/gatt-dbus.h b/src/gatt-dbus.h
new file mode 100644
index 0000000..310cfa9
--- /dev/null
+++ b/src/gatt-dbus.h
@@ -0,0 +1,25 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Instituto Nokia de Tecnologia - INdT
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program 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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+gboolean gatt_dbus_manager_register(void);
+void gatt_dbus_manager_unregister(void);
diff --git a/src/gatt.c b/src/gatt.c
index 06619f0..e8b691a 100644
--- a/src/gatt.c
+++ b/src/gatt.c
@@ -25,14 +25,23 @@
 #include <config.h>
 #endif
 
+#include <glib.h>
+
+#include "log.h"
+
+#include "gatt-dbus.h"
 #include "gatt.h"
 
 void gatt_init(void)
 {
+	DBG("Starting GATT server");
 
+	gatt_dbus_manager_register();
 }
 
 void gatt_cleanup(void)
 {
+	DBG("Stopping GATT server");
 
+	gatt_dbus_manager_unregister();
 }
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ v6 02/18] gatt: Add stub for gatt.{c, h} files
From: Claudio Takahasi @ 2014-02-04 17:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: claudio.takahasi, Alvaro Silva
In-Reply-To: <1391536429-8345-1-git-send-email-claudio.takahasi@openbossa.org>

From: Alvaro Silva <alvaro.silva@openbossa.org>

These files implement functions to handle ATT transactions, and expose
functions to allow other entities to manage GATT based services. It
is a replacement for src/attrib-server.c.
---
 Makefile.am |  1 +
 src/gatt.c  | 38 ++++++++++++++++++++++++++++++++++++++
 src/gatt.h  | 26 ++++++++++++++++++++++++++
 src/main.c  |  4 ++++
 4 files changed, 69 insertions(+)
 create mode 100644 src/gatt.c
 create mode 100644 src/gatt.h

diff --git a/Makefile.am b/Makefile.am
index 1a44a9f..5f61d50 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -146,6 +146,7 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
 			src/adapter.h src/adapter.c \
 			src/profile.h src/profile.c \
 			src/service.h src/service.c \
+			src/gatt.h src/gatt.c \
 			src/device.h src/device.c src/attio.h \
 			src/dbus-common.c src/dbus-common.h \
 			src/eir.h src/eir.c \
diff --git a/src/gatt.c b/src/gatt.c
new file mode 100644
index 0000000..06619f0
--- /dev/null
+++ b/src/gatt.c
@@ -0,0 +1,38 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Instituto Nokia de Tecnologia - INdT
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program 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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; 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 "gatt.h"
+
+void gatt_init(void)
+{
+
+}
+
+void gatt_cleanup(void)
+{
+
+}
diff --git a/src/gatt.h b/src/gatt.h
new file mode 100644
index 0000000..3a320b4
--- /dev/null
+++ b/src/gatt.h
@@ -0,0 +1,26 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Instituto Nokia de Tecnologia - INdT
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program 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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+void gatt_init(void);
+
+void gatt_cleanup(void);
diff --git a/src/main.c b/src/main.c
index 91d90b4..fccc838 100644
--- a/src/main.c
+++ b/src/main.c
@@ -55,6 +55,7 @@
 #include "dbus-common.h"
 #include "agent.h"
 #include "profile.h"
+#include "gatt.h"
 #include "systemd.h"
 
 #define BLUEZ_NAME "org.bluez"
@@ -545,6 +546,8 @@ int main(int argc, char *argv[])
 
 	g_dbus_set_flags(gdbus_flags);
 
+	gatt_init();
+
 	if (option_compat == TRUE)
 		sdp_flags |= SDP_SERVER_COMPAT;
 
@@ -595,6 +598,7 @@ int main(int argc, char *argv[])
 	btd_profile_cleanup();
 	btd_agent_cleanup();
 	btd_device_cleanup();
+	gatt_cleanup();
 
 	adapter_cleanup();
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ v6 01/18] doc: Add experimental GATT API
From: Claudio Takahasi @ 2014-02-04 17:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: claudio.takahasi
In-Reply-To: <1391536429-8345-1-git-send-email-claudio.takahasi@openbossa.org>

This patch proposes an unified GATT API for local and remote services.
---
 doc/gatt-api.txt | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)
 create mode 100644 doc/gatt-api.txt

diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
new file mode 100644
index 0000000..0f6b084
--- /dev/null
+++ b/doc/gatt-api.txt
@@ -0,0 +1,142 @@
+BlueZ D-Bus GATT API description
+********************************
+
+GATT local and remote services share the same high-level D-Bus API. Local
+refers to GATT based service exported by a BlueZ plugin or an external
+application. Remote refers to GATT services exported by the peer.
+
+BlueZ acts as a proxy, translating ATT operations to D-Bus method calls and
+Properties (or the opposite). Support for D-Bus Object Manager is mandatory for
+external services to allow seamless GATT declarations (Service, Characteristic
+and Descriptors) discovery.
+
+Releasing a registered GATT service is not defined yet. Any API extension
+should avoid breaking the defined API, and if possible keep an unified GATT
+remote and local services representation.
+
+Service hierarchy
+=================
+
+GATT remote and local service representation. Object path for local services
+is freely definable.
+
+External applications implementing local services must register the services
+using GattManager1 registration method and must implement the methods and
+properties defined in GattService1 interface.
+
+Service		org.bluez
+Interface	org.bluez.GattService1 [Experimental]
+Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/serviceXX
+
+Properties	string UUID [read-only]
+
+			128-bit service UUID.
+
+		array{object} Includes [read-only]: Not implemented
+
+			Array of object paths representing the included
+			services of this service.
+
+
+Characteristic hierarchy
+========================
+
+For local GATT defined services, the object paths need to follow the service
+path hierarchy and are freely definable.
+
+Service		org.bluez
+Interface	org.bluez.GattCharacteristic1 [Experimental]
+Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/serviceXX/charYYYY
+
+Properties	string UUID [read-only]
+
+			128-bit characteristic UUID.
+
+		object Service [read-only]
+
+			Object path of the GATT service the characteristc
+			belongs to.
+
+		array{byte} Value [read-write]
+
+			Value read from the remote Bluetooth device or from
+			the external application implementing GATT services.
+
+		array{string} Flags [read-only, optional]
+
+			Defines how the characteristic value can be used. See
+			Core spec page 1898, "Table 3.5: Characteristic
+			Properties bit field" and page 1900, "Table 3.8:
+			Characteristic Extended Properties bit field". Allowed
+			values: "broadcast", "read", "write-without-response",
+			"write", "notify", "indicate",
+			"authenticated-signed-writes", "reliable-write", and
+			"writable-auxiliaries".
+
+
+Characteristic Descriptors hierarchy
+====================================
+
+Local or remote GATT characteristic descriptors hierarchy.
+
+Service		org.bluez
+Interface	org.bluez.GattDescriptor1 [Experimental]
+Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/serviceXX/charYYYY/descriptorZZZ
+
+Properties	string UUID [read-only]
+
+			128-bit descriptor UUID.
+
+		object Characteristic [read-only]
+
+			Object path of the GATT characteristc the descriptor
+			belongs to.
+
+		array{byte} Value [read-write]
+
+			Raw characteristic descriptor value read from the
+			remote Bluetooth device or from the external
+			application implementing GATT services.
+
+		string Permissions [read-only]: To be defined
+
+			Defines read/write authentication and authorization
+			requirements.
+
+Service Manager hierarchy
+=============================
+
+Service Manager allows external applications to register GATT based
+services. Services must follow the API for Service and Characteristic
+described above.
+
+Local GATT services, characteristics and characteristic descriptors are
+discovered automatically using the D-Bus Object Manager interface.
+
+Service		org.bluez
+Interface	org.bluez.GattManager1 [Experimental]
+Object path	/org/bluez
+
+Methods		RegisterService(object service, dict options)
+
+			Registers remote application service exported under
+			interface GattService1. Characteristic objects must
+			be hierarchical to their service and must use the
+			interface GattCharacteristic1. D-Bus Object Manager
+			is used to fetch the exported objects.
+
+			"service" object path together with the D-Bus system
+			bus connection ID define the identification of the
+			application registering a GATT based service.
+
+			Possible errors: org.bluez.Error.InvalidArguments
+					 org.bluez.Error.AlreadyExists
+
+		UnregisterService(object service)
+
+			This unregisters the service that has been
+			previously registered. The object path parameter
+			must match the same value that has been used
+			on registration.
+
+			Possible errors: org.bluez.Error.DoesNotExist
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ v6 00/18] GATT API: External Services
From: Claudio Takahasi @ 2014-02-04 17:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: claudio.takahasi
In-Reply-To: <1390854924-31904-1-git-send-email-claudio.takahasi@openbossa.org>

This patchset implements the minimal support for adding local services
declarations.

Limitation: Remove services and multiple services exported by the same
remote will be implemented the next series.

Changes from PATCH v6 to PATCH v5:
* Rebase
* Fixed warning (gcc 4.6.3/32-bits): "ignoring return value of ‘write’"

Changes from PATCH v4 to PATCH v5:
* Removed Release() method of GattService1 interface

Changes from PATCH v3 to PATCH v4:
* Rebase
* src/gatt.c: Replaced GIOChannel/GAttrib by "io".

Changes from PATCH v2 to PATCH v3:
* Rebase
* Interfaces renamed: s/GattServiceManager1/GattManager1,
  s/Characteristic1/GattCharacteristic1, s/Descriptor/GattDescriptor1
* test/gatt-service.c: s/fprintf/printf

Changes from PATCH v1 to PATCH v2:
* Rebase
* Included patch "doc: Add GATT API"
* Interfaces renamed: s/Service1/GattService1, and
  s/ServiceManager1/GattServiceManager1
* Removed patch "gatt: Implement UnregisterService" from this patchset

Changes from PATCH v0 to PATCH v1:
* Rebase

Changes from RFC v0 to PATCH v0:
* Changed copyright year : s/2013/2014
* Fixed coding style
* Added gatt-service binary to gitignore
* Added extra comment in the source code

Features:
* API for internal and external services declaration
* Unix socket for testing purpose: services are exported through unix
  sockets to avoid breaking the current attribute server.

How to test:
  Run bluetoothd with EXPERIMENTAL flag (-E)
  Replace /etc/dbus-1/system.d/bluetooth.conf and reload DBus settings
  $gatttool -L --primary (or interactive mode)

Roughly upstreaming plan (steps):
 * GATT Server: External Services -> pathset GATT API: External Services
 * GATT Server: External Characteristics (Server)
 * GATT Server: External Descriptors (Server)
 * Replacement for GAttrib: use "io"
 * Replace attribute server
 * Remove ATTIO and automatic connection mechanism from userspace
 * Fix all GATT internal plugins
 * GATT Client: Remote Services

Alvaro Silva (6):
  gatt: Add stub for gatt.{c, h} files
  gatt: Register Manager D-Bus Interface
  gatt: Add registering external service
  gatt: Add external services tracking
  gatt: Register ATT command/event handler
  gatt: Add Discover All Primary Services

Andre Guedes (1):
  gatt: Add helper for creating GATT services

Claudio Takahasi (11):
  doc: Add experimental GATT API
  lib: Move GATT UUID to uuid.h
  gatt: Add server unix socket
  gattrib: Use default ATT LE MTU for non-standard sockets
  test: Add external service GATT skeleton
  gitignore: Add test/gatt-service
  test: Add signal handling for gatt-service
  test: Add registering external service
  gatttool: Add unix socket connect
  gatttool: Add unix socket support for interactive mode
  bluetooth.conf: Add ObjectManager interface

 .gitignore           |   1 +
 Makefile.am          |   2 +
 Makefile.tools       |   5 +
 attrib/gatt.h        |  25 ----
 attrib/gattrib.c     |  16 +--
 attrib/gatttool.c    |  27 +++-
 attrib/gatttool.h    |   1 +
 attrib/interactive.c |  19 +--
 attrib/utils.c       |  54 ++++++++
 doc/gatt-api.txt     | 142 +++++++++++++++++++
 lib/uuid.h           |  30 ++++
 src/bluetooth.conf   |   1 +
 src/gatt-dbus.c      | 271 ++++++++++++++++++++++++++++++++++++
 src/gatt-dbus.h      |  25 ++++
 src/gatt.c           | 379 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/gatt.h           |  36 +++++
 src/main.c           |   4 +
 test/gatt-service.c  | 267 ++++++++++++++++++++++++++++++++++++
 18 files changed, 1257 insertions(+), 48 deletions(-)
 create mode 100644 doc/gatt-api.txt
 create mode 100644 src/gatt-dbus.c
 create mode 100644 src/gatt-dbus.h
 create mode 100644 src/gatt.c
 create mode 100644 src/gatt.h
 create mode 100644 test/gatt-service.c

-- 
1.8.3.1


^ permalink raw reply

* Re: [PATCH v2 1/3] android/bluetooth: Add threshold to RSSI change
From: Marcel Holtmann @ 2014-02-04 16:43 UTC (permalink / raw)
  To: Szymon Janc; +Cc: BlueZ development
In-Reply-To: <1391531663-13143-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

> There is no need to report very small RSSI changes.
> ---
> v2: Fix name and use abs()
> 
> android/bluetooth.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/android/bluetooth.c b/android/bluetooth.c
> index d74d792..5aed4af 100644
> --- a/android/bluetooth.c
> +++ b/android/bluetooth.c
> @@ -1045,6 +1045,12 @@ static uint8_t bdaddr_type2android(uint8_t type)
> 	return HAL_TYPE_LE;
> }
> 
> +static bool rssi_above_threshold(int old, int new)
> +{
> +	/* only 8 dBm or more */
> +	return abs(old - new) >= 8;
> +}
> +
> static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
> 					int8_t rssi, bool confirm,
> 					const uint8_t *data, uint8_t data_len)
> @@ -1113,7 +1119,7 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
> 		(*num_prop)++;
> 	}
> 
> -	if (rssi) {
> +	if (rssi && rssi_above_threshold(dev->rssi, rssi)) {

this is a good first step, but I think we should additionally work on handling pathloss via the TX power EIR element in combination with RSSI. Since that really gives a good idea of the distance of a device and if it has moved or not.

The 3D glasses for example rely on this for proximity association.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH] Bluetooth: hidp: make sure input buffers are big enough
From: Marcel Holtmann @ 2014-02-04 16:39 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: David Herrmann, open list:HID CORE LAYER,
	linux-bluetooth@vger.kernel.org development, Gustavo F. Padovan
In-Reply-To: <alpine.LNX.2.00.1402041446130.14066@pobox.suse.cz>

Hi Jiri,

>>>>>> Due to various reasons I will not have access to any testing HW for the
>>>>>> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd
>>>>>> be appreciated.
>>>>>> 
>>>>>> Otherwise I'll be working on it by the end of this week.
>>>>> 
>>>>> David,
>>>>> 
>>>>> just got back to this, finally ... did you have time to work on this at
>>>>> all, or should I just start from scratch?
>>>> 
>>>> Sorry, no. Fosdem is this weekend and I needed to get my code ready
>>>> for that. But I'll finally have time again next week.
>>> 
>>> Okay, thanks. I then guess we should proceed with this bandaid (double
>>> allocation) for 3.14
>> 
>> It would be really nice if we could get the HIDP patch into 3.14-rc2
>> and backported to stable. There have been quite a bunch of reports now
>> and I dislike adding a GFP_ATOMIC allocation in HID core. 
> 
> I agree with David; Gustavo, what is your take on this, please?

I leave this up to Gustavo to get this into wireless tree for 3.14-rc2.

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel


^ permalink raw reply

* [PATCH v2 3/3] android/bluetooth: Send prop change event only if prop was changed
From: Szymon Janc @ 2014-02-04 16:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1391531663-13143-1-git-send-email-szymon.janc@tieto.com>

---
 android/bluetooth.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 3becc45..9f8e7b4 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1118,7 +1118,7 @@ static void update_device(struct device *dev, int8_t rssi,
 	ev->status = HAL_STATUS_SUCCESS;
 	bdaddr2android(&dev->bdaddr, ev->bdaddr);
 
-	if (eir->class) {
+	if (eir->class && dev->class != eir->class) {
 		dev->class = eir->class;
 		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_CLASS,
 					sizeof(dev->class), &dev->class);
@@ -1132,7 +1132,7 @@ static void update_device(struct device *dev, int8_t rssi,
 		ev->num_props++;
 	}
 
-	if (eir->name) {
+	if (eir->name && strcmp(dev->name, eir->name)) {
 		g_free(dev->name);
 		dev->name = g_strdup(eir->name);
 		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_NAME,
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v2 2/3] android/bluetooth: Refactor update_found_device function
From: Szymon Janc @ 2014-02-04 16:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1391531663-13143-1-git-send-email-szymon.janc@tieto.com>

This function grown too big and was hard to follow. Split it to helpers
for clarity.
---
 android/bluetooth.c | 153 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 93 insertions(+), 60 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 5aed4af..3becc45 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1051,97 +1051,132 @@ static bool rssi_above_threshold(int old, int new)
 	return abs(old - new) >= 8;
 }
 
-static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
-					int8_t rssi, bool confirm,
-					const uint8_t *data, uint8_t data_len)
+static void update_new_device(struct device *dev, int8_t rssi,
+						const struct eir_data *eir)
 {
 	uint8_t buf[BLUEZ_HAL_MTU];
-	struct eir_data eir;
-	struct device *dev;
-	uint8_t *num_prop;
-	uint8_t opcode;
-	int size = 0;
+	struct hal_ev_device_found *ev = (void*) buf;
+	bdaddr_t android_bdaddr;
+	uint8_t android_type;
+	int size;
 
 	memset(buf, 0, sizeof(buf));
-	memset(&eir, 0, sizeof(eir));
-
-	eir_parse(&eir, data, data_len);
-
-	dev = find_device(bdaddr);
 
-	/*
-	 * Device found event needs to be send also for known device if this is
-	 * new discovery session. Otherwise framework will ignore it.
-	 */
-	if (!dev || !dev->found) {
-		struct hal_ev_device_found *ev = (void *) buf;
-		bdaddr_t android_bdaddr;
-		uint8_t android_type;
+	if (adapter.discovering)
+		dev->found = true;
 
-		if (!dev)
-			dev = create_device(bdaddr, bdaddr_type);
+	size = sizeof(*ev);
 
-		dev->found = true;
+	bdaddr2android(&dev->bdaddr, &android_bdaddr);
+	size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_ADDR,
+						sizeof(android_bdaddr),
+						&android_bdaddr);
+	ev->num_props++;
 
-		size += sizeof(*ev);
+	android_type = bdaddr_type2android(dev->bdaddr_type);
+	size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_TYPE,
+				sizeof(android_type), &android_type);
+	ev->num_props++;
 
-		num_prop = &ev->num_props;
-		opcode = HAL_EV_DEVICE_FOUND;
+	if (eir->class) {
+		dev->class = eir->class;
+		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_CLASS,
+					sizeof(dev->class), &dev->class);
+		ev->num_props++;
+	}
 
-		bdaddr2android(bdaddr, &android_bdaddr);
+	if (rssi && rssi_above_threshold(dev->rssi, rssi)) {
+		dev->rssi = rssi;
+		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_RSSI,
+						sizeof(dev->rssi), &dev->rssi);
+		ev->num_props++;
+	}
 
-		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_ADDR,
-							sizeof(android_bdaddr),
-							&android_bdaddr);
-		(*num_prop)++;
+	if (eir->name) {
+		g_free(dev->name);
+		dev->name = g_strdup(eir->name);
+		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_NAME,
+						strlen(dev->name), dev->name);
+		ev->num_props++;
+	}
 
-		android_type = bdaddr_type2android(dev->bdaddr_type);
-		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_TYPE,
-					sizeof(android_type), &android_type);
-		(*num_prop)++;
-	} else {
-		struct hal_ev_remote_device_props *ev = (void *) buf;
+	ipc_send_notif(HAL_SERVICE_ID_BLUETOOTH, HAL_EV_DEVICE_FOUND, size,
+									buf);
+}
 
-		size += sizeof(*ev);
+static void update_device(struct device *dev, int8_t rssi,
+						const struct eir_data *eir)
+{
+	uint8_t buf[BLUEZ_HAL_MTU];
+	struct hal_ev_remote_device_props *ev = (void *) buf;
+	int size;
 
-		num_prop = &ev->num_props;
-		opcode = HAL_EV_REMOTE_DEVICE_PROPS;
+	memset(buf, 0, sizeof(buf));
 
-		ev->status = HAL_STATUS_SUCCESS;
-		bdaddr2android(bdaddr, ev->bdaddr);
-	}
+	size = sizeof(*ev);
 
-	if (eir.class) {
-		dev->class = eir.class;
+	ev->status = HAL_STATUS_SUCCESS;
+	bdaddr2android(&dev->bdaddr, ev->bdaddr);
 
+	if (eir->class) {
+		dev->class = eir->class;
 		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_CLASS,
-						sizeof(eir.class), &eir.class);
-		(*num_prop)++;
+					sizeof(dev->class), &dev->class);
+		ev->num_props++;
 	}
 
 	if (rssi && rssi_above_threshold(dev->rssi, rssi)) {
 		dev->rssi = rssi;
-
 		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_RSSI,
 						sizeof(dev->rssi), &dev->rssi);
-		(*num_prop)++;
+		ev->num_props++;
 	}
 
-	if (eir.name) {
+	if (eir->name) {
 		g_free(dev->name);
-		dev->name = g_strdup(eir.name);
-
+		dev->name = g_strdup(eir->name);
 		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_NAME,
-						strlen(eir.name), eir.name);
-		(*num_prop)++;
+						strlen(dev->name), dev->name);
+		ev->num_props++;
 	}
 
+	if (ev->num_props)
+		ipc_send_notif(HAL_SERVICE_ID_BLUETOOTH,
+					HAL_EV_REMOTE_DEVICE_PROPS, size, buf);
+}
+
+static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
+					int8_t rssi, bool confirm,
+					const uint8_t *data, uint8_t data_len)
+{
+	uint8_t buf[BLUEZ_HAL_MTU];
+	struct eir_data eir;
+	struct device *dev;
+
+	memset(buf, 0, sizeof(buf));
+	memset(&eir, 0, sizeof(eir));
+
+	eir_parse(&eir, data, data_len);
+
+	dev = find_device(bdaddr);
+
+	/* Device found event needs to be send also for known device if this is
+	 * new discovery session. Otherwise framework will ignore it.
+	 */
+	if (!dev || !dev->found) {
+		if (!dev)
+			dev = create_device(bdaddr, bdaddr_type);
+
+		update_new_device(dev, rssi, &eir);
+	} else {
+		update_device(dev, rssi, &eir);
+	}
+
+	eir_data_free(&eir);
+
 	if (dev->bond_state != HAL_BOND_STATE_BONDED)
 		cache_device(dev);
 
-	if (*num_prop)
-		ipc_send_notif(HAL_SERVICE_ID_BLUETOOTH, opcode, size, buf);
-
 	if (confirm) {
 		char addr[18];
 
@@ -1149,8 +1184,6 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
 		info("Device %s needs name confirmation.", addr);
 		confirm_device_name(bdaddr, bdaddr_type);
 	}
-
-	eir_data_free(&eir);
 }
 
 static void mgmt_device_found_event(uint16_t index, uint16_t length,
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v2 1/3] android/bluetooth: Add threshold to RSSI change
From: Szymon Janc @ 2014-02-04 16:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

There is no need to report very small RSSI changes.
---
v2: Fix name and use abs()

 android/bluetooth.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index d74d792..5aed4af 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1045,6 +1045,12 @@ static uint8_t bdaddr_type2android(uint8_t type)
 	return HAL_TYPE_LE;
 }
 
+static bool rssi_above_threshold(int old, int new)
+{
+	/* only 8 dBm or more */
+	return abs(old - new) >= 8;
+}
+
 static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
 					int8_t rssi, bool confirm,
 					const uint8_t *data, uint8_t data_len)
@@ -1113,7 +1119,7 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
 		(*num_prop)++;
 	}
 
-	if (rssi) {
+	if (rssi && rssi_above_threshold(dev->rssi, rssi)) {
 		dev->rssi = rssi;
 
 		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_RSSI,
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 11/11] android/unit: Add cases for msg size verification
From: Jakub Tyszkowski @ 2014-02-04 14:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391524749-2518-1-git-send-email-jakub.tyszkowski@tieto.com>

This patch adds checking for proper msg size verification in case it is
not declared in handlers that this is variable sized message. In
such case malformed data should not be accepted.
---
 android/test-ipc.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/android/test-ipc.c b/android/test-ipc.c
index d05544d..a276063 100644
--- a/android/test-ipc.c
+++ b/android/test-ipc.c
@@ -532,6 +532,38 @@ static const struct test_data test_cmd_service_offrange = {
 	.expected_signal = SIGTERM
 };
 
+static const struct vardata test_cmd_invalid_data_1 = {
+	.hdr.service_id = 0,
+	.hdr.opcode = 1,
+	.hdr.len = sizeof(VARDATA_EX1),
+	.data = VARDATA_EX1,
+};
+
+static const struct test_data test_cmd_msg_invalid_1 = {
+	.cmd = &test_cmd_invalid_data_1,
+	.cmd_size = sizeof(struct hal_hdr) + sizeof(VARDATA_EX1) - 1,
+	.service = 0,
+	.handlers = cmd_handlers,
+	.handlers_size = 1,
+	.expected_signal = SIGTERM
+};
+
+static const struct vardata test_cmd_invalid_data_2 = {
+	.hdr.service_id = 0,
+	.hdr.opcode = 1,
+	.hdr.len = sizeof(VARDATA_EX1) - 1,
+	.data = VARDATA_EX1,
+};
+
+static const struct test_data test_cmd_msg_invalid_2 = {
+	.cmd = &test_cmd_invalid_data_2,
+	.cmd_size = sizeof(struct hal_hdr) + sizeof(VARDATA_EX1),
+	.service = 0,
+	.handlers = cmd_handlers,
+	.handlers_size = 1,
+	.expected_signal = SIGTERM
+};
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -568,6 +600,12 @@ int main(int argc, char *argv[])
 	g_test_add_data_func("/android_ipc/test_cmd_hdr_invalid",
 					&test_cmd_hdr_invalid,
 					test_cmd_reg);
+	g_test_add_data_func("/android_ipc/test_cmd_msg_invalid_1",
+					&test_cmd_msg_invalid_1,
+					test_cmd_reg);
+	g_test_add_data_func("/android_ipc/test_cmd_msg_invalid_2",
+					&test_cmd_msg_invalid_2,
+					test_cmd_reg);
 
 	return g_test_run();
 }
-- 
1.8.5.2


^ permalink raw reply related

* [PATCH 10/11] android/unit: Add case for sending incomplete header
From: Jakub Tyszkowski @ 2014-02-04 14:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391524749-2518-1-git-send-email-jakub.tyszkowski@tieto.com>

Header size is the bare minimum that should always be sent.
---
 android/test-ipc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/android/test-ipc.c b/android/test-ipc.c
index e4463eb..d05544d 100644
--- a/android/test-ipc.c
+++ b/android/test-ipc.c
@@ -461,6 +461,15 @@ static const struct test_data test_cmd_opcode_invalid_1 = {
 	.expected_signal = SIGTERM
 };
 
+static const struct test_data test_cmd_hdr_invalid = {
+	.cmd = &test_cmd_1_hdr,
+	.cmd_size = sizeof(test_cmd_1_hdr) - 1,
+	.service = 0,
+	.handlers = cmd_handlers,
+	.handlers_size = 1,
+	.expected_signal = SIGTERM
+};
+
 #define VARDATA_EX1 "some data example"
 
 struct vardata {
@@ -556,6 +565,9 @@ int main(int argc, char *argv[])
 	g_test_add_data_func("/android_ipc/test_cmd_service_offrange",
 					&test_cmd_service_offrange,
 					test_cmd_reg);
+	g_test_add_data_func("/android_ipc/test_cmd_hdr_invalid",
+					&test_cmd_hdr_invalid,
+					test_cmd_reg);
 
 	return g_test_run();
 }
-- 
1.8.5.2


^ permalink raw reply related

* [PATCH 09/11] android/unit: Add negative case for msg size verification
From: Jakub Tyszkowski @ 2014-02-04 14:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391524749-2518-1-git-send-email-jakub.tyszkowski@tieto.com>

Case for checking message size declared inside the header against the
amount of data sent for variable sized message.
---
 android/test-ipc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/android/test-ipc.c b/android/test-ipc.c
index 6bb6cd6..e4463eb 100644
--- a/android/test-ipc.c
+++ b/android/test-ipc.c
@@ -499,6 +499,15 @@ static const struct test_data test_cmd_vardata_valid_2 = {
 	.handlers_size = 1,
 };
 
+static const struct test_data test_cmd_vardata_invalid_1 = {
+	.cmd = &test_cmd_vardata,
+	.cmd_size = sizeof(struct hal_hdr) + sizeof(VARDATA_EX1) - 1,
+	.service = 0,
+	.handlers = cmd_vardata_handlers,
+	.handlers_size = 1,
+	.expected_signal = SIGTERM
+};
+
 static const struct hal_hdr test_cmd_service_offrange_hdr = {
 	.service_id = HAL_SERVICE_ID_MAX + 1,
 	.opcode = 1,
@@ -541,6 +550,9 @@ int main(int argc, char *argv[])
 	g_test_add_data_func("/android_ipc/test_cmd_vardata_valid_2",
 					&test_cmd_vardata_valid_2,
 					test_cmd_reg);
+	g_test_add_data_func("/android_ipc/test_cmd_vardata_invalid_1",
+					&test_cmd_vardata_invalid_1,
+					test_cmd_reg);
 	g_test_add_data_func("/android_ipc/test_cmd_service_offrange",
 					&test_cmd_service_offrange,
 					test_cmd_reg);
-- 
1.8.5.2


^ permalink raw reply related

* [PATCH 08/11] android/unit: Add case for out of range service
From: Jakub Tyszkowski @ 2014-02-04 14:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391524749-2518-1-git-send-email-jakub.tyszkowski@tieto.com>

Check reaction for sending message to services not listed inside
hal-msg.h
---
 android/test-ipc.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/android/test-ipc.c b/android/test-ipc.c
index 2120d15..6bb6cd6 100644
--- a/android/test-ipc.c
+++ b/android/test-ipc.c
@@ -499,6 +499,21 @@ static const struct test_data test_cmd_vardata_valid_2 = {
 	.handlers_size = 1,
 };
 
+static const struct hal_hdr test_cmd_service_offrange_hdr = {
+	.service_id = HAL_SERVICE_ID_MAX + 1,
+	.opcode = 1,
+	.len = 0
+};
+
+static const struct test_data test_cmd_service_offrange = {
+	.cmd = &test_cmd_service_offrange_hdr,
+	.cmd_size = sizeof(struct hal_hdr),
+	.service = HAL_SERVICE_ID_MAX + 1,
+	.handlers = cmd_handlers,
+	.handlers_size = 1,
+	.expected_signal = SIGTERM
+};
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -526,6 +541,9 @@ int main(int argc, char *argv[])
 	g_test_add_data_func("/android_ipc/test_cmd_vardata_valid_2",
 					&test_cmd_vardata_valid_2,
 					test_cmd_reg);
+	g_test_add_data_func("/android_ipc/test_cmd_service_offrange",
+					&test_cmd_service_offrange,
+					test_cmd_reg);
 
 	return g_test_run();
 }
-- 
1.8.5.2


^ permalink raw reply related

* [PATCH 07/11] android/unit: Add another case for variable sized data
From: Jakub Tyszkowski @ 2014-02-04 14:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391524749-2518-1-git-send-email-jakub.tyszkowski@tieto.com>

This patch adds test for variable length data handling. Handlers struct
have static values representing minimum payload. It cannot be predicted
how large data will be sent so they should accept data larger than
declared inside ipc_handler array, which holds the minimum size of such
message.
---
 android/test-ipc.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/android/test-ipc.c b/android/test-ipc.c
index 523011e..2120d15 100644
--- a/android/test-ipc.c
+++ b/android/test-ipc.c
@@ -487,6 +487,18 @@ static const struct test_data test_cmd_vardata_valid = {
 	.handlers_size = 1,
 };
 
+static const struct ipc_handler cmd_vardata_handlers_valid2[] = {
+	{ test_cmd_handler_1, true, sizeof(VARDATA_EX1) - 1 }
+};
+
+static const struct test_data test_cmd_vardata_valid_2 = {
+	.cmd = &test_cmd_vardata,
+	.cmd_size = sizeof(struct hal_hdr) + sizeof(VARDATA_EX1),
+	.service = 0,
+	.handlers = cmd_vardata_handlers_valid2,
+	.handlers_size = 1,
+};
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -511,6 +523,9 @@ int main(int argc, char *argv[])
 	g_test_add_data_func("/android_ipc/test_cmd_vardata_valid",
 					&test_cmd_vardata_valid,
 					test_cmd_reg);
+	g_test_add_data_func("/android_ipc/test_cmd_vardata_valid_2",
+					&test_cmd_vardata_valid_2,
+					test_cmd_reg);
 
 	return g_test_run();
 }
-- 
1.8.5.2


^ permalink raw reply related

* [PATCH 06/11] android/unit: Add test case for variable sized data
From: Jakub Tyszkowski @ 2014-02-04 14:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391524749-2518-1-git-send-email-jakub.tyszkowski@tieto.com>

Check if sending variable length data with proper msg size declared inside the
header succeeds.
---
 android/test-ipc.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/android/test-ipc.c b/android/test-ipc.c
index a2d3085..523011e 100644
--- a/android/test-ipc.c
+++ b/android/test-ipc.c
@@ -461,6 +461,32 @@ static const struct test_data test_cmd_opcode_invalid_1 = {
 	.expected_signal = SIGTERM
 };
 
+#define VARDATA_EX1 "some data example"
+
+struct vardata {
+	struct hal_hdr hdr;
+	uint8_t data[BLUEZ_HAL_MTU - sizeof(struct hal_hdr)];
+} __attribute__((packed));
+
+static const struct vardata test_cmd_vardata = {
+	.hdr.service_id = 0,
+	.hdr.opcode = 1,
+	.hdr.len = sizeof(VARDATA_EX1),
+	.data = VARDATA_EX1,
+};
+
+static const struct ipc_handler cmd_vardata_handlers[] = {
+	{ test_cmd_handler_1, true, sizeof(VARDATA_EX1) }
+};
+
+static const struct test_data test_cmd_vardata_valid = {
+	.cmd = &test_cmd_vardata,
+	.cmd_size = sizeof(struct hal_hdr) + sizeof(VARDATA_EX1),
+	.service = 0,
+	.handlers = cmd_vardata_handlers,
+	.handlers_size = 1,
+};
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -482,6 +508,9 @@ int main(int argc, char *argv[])
 	g_test_add_data_func("/android_ipc/test_cmd_opcode_invalid_1",
 					&test_cmd_opcode_invalid_1,
 					test_cmd_reg);
+	g_test_add_data_func("/android_ipc/test_cmd_vardata_valid",
+					&test_cmd_vardata_valid,
+					test_cmd_reg);
 
 	return g_test_run();
 }
-- 
1.8.5.2


^ permalink raw reply related

* [PATCH 05/11] android/unit: Add support for variable length data
From: Jakub Tyszkowski @ 2014-02-04 14:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391524749-2518-1-git-send-email-jakub.tyszkowski@tieto.com>

This patch adds sending messages larger than just hal_hdr, and fixes
response verification which worked only for empty messages but was
failing when sending something more than just header.
---
 android/test-ipc.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/android/test-ipc.c b/android/test-ipc.c
index cb6f518..a2d3085 100644
--- a/android/test-ipc.c
+++ b/android/test-ipc.c
@@ -44,7 +44,7 @@
 
 struct test_data {
 	uint32_t expected_signal;
-	const struct hal_hdr *cmd;
+	const void *cmd;
 	uint16_t cmd_size;
 	uint8_t service;
 	const struct ipc_handler *handlers;
@@ -79,9 +79,16 @@ static gboolean cmd_watch(GIOChannel *io, GIOCondition cond,
 {
 	struct context *context = user_data;
 	const struct test_data *test_data = context->data;
+	const struct hal_hdr *sent_msg = test_data->cmd;
 	uint8_t buf[128];
 	int sk;
 
+	struct hal_hdr success_resp = {
+		.service_id = sent_msg->service_id,
+		.opcode = sent_msg->opcode,
+		.len = 0,
+	};
+
 	g_assert(test_data->expected_signal == 0);
 
 	if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
@@ -91,8 +98,8 @@ static gboolean cmd_watch(GIOChannel *io, GIOCondition cond,
 
 	sk = g_io_channel_unix_get_fd(io);
 
-	g_assert(read(sk, buf, sizeof(buf)) == test_data->cmd_size);
-	g_assert(!memcmp(test_data->cmd, buf, test_data->cmd_size));
+	g_assert(read(sk, buf, sizeof(buf)) == sizeof(struct hal_hdr));
+	g_assert(!memcmp(&success_resp, buf, sizeof(struct hal_hdr)));
 
 	context_quit(context);
 
-- 
1.8.5.2


^ permalink raw reply related

* [PATCH 04/11] android/unit: Add case for opcode without handler
From: Jakub Tyszkowski @ 2014-02-04 14:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391524749-2518-1-git-send-email-jakub.tyszkowski@tieto.com>

This test case checks if IPC shuts down on unhandled opcode.
---
 android/test-ipc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/android/test-ipc.c b/android/test-ipc.c
index 1e027fa..cb6f518 100644
--- a/android/test-ipc.c
+++ b/android/test-ipc.c
@@ -445,6 +445,15 @@ static const struct test_data test_cmd_opcode_valid_2 = {
 	.handlers_size = 2,
 };
 
+static const struct test_data test_cmd_opcode_invalid_1 = {
+	.cmd = &test_cmd_2_hdr,
+	.cmd_size = sizeof(test_cmd_2_hdr),
+	.service = 0,
+	.handlers = cmd_handlers,
+	.handlers_size = 1,
+	.expected_signal = SIGTERM
+};
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -463,6 +472,9 @@ int main(int argc, char *argv[])
 					&test_cmd_opcode_valid_1, test_cmd_reg);
 	g_test_add_data_func("/android_ipc/test_cmd_opcode_valid_2",
 					&test_cmd_opcode_valid_2, test_cmd_reg);
+	g_test_add_data_func("/android_ipc/test_cmd_opcode_invalid_1",
+					&test_cmd_opcode_invalid_1,
+					test_cmd_reg);
 
 	return g_test_run();
 }
-- 
1.8.5.2


^ permalink raw reply related

* [PATCH 03/11] android/unit: Add test cases for proper handler calls
From: Jakub Tyszkowski @ 2014-02-04 14:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391524749-2518-1-git-send-email-jakub.tyszkowski@tieto.com>

This patch adds tests for calling proper opcode handler. Two handlers
are registered, but one always results in failure. No failure means that
proper opcode <-> handler maching is done by the ipc mechanism.
---
 android/test-ipc.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/android/test-ipc.c b/android/test-ipc.c
index 8e31507..1e027fa 100644
--- a/android/test-ipc.c
+++ b/android/test-ipc.c
@@ -368,6 +368,16 @@ static void test_cmd_handler_1(const void *buf, uint16_t len)
 	ipc_send_rsp(0, 1, 0);
 }
 
+static void test_cmd_handler_2(const void *buf, uint16_t len)
+{
+	ipc_send_rsp(0, 2, 0);
+}
+
+static void test_cmd_handler_invalid(const void *buf, uint16_t len)
+{
+	raise(SIGTERM);
+}
+
 static const struct test_data test_init_1 = {};
 
 static const struct hal_hdr test_cmd_1_hdr = {
@@ -376,6 +386,12 @@ static const struct hal_hdr test_cmd_1_hdr = {
 	.len = 0
 };
 
+static const struct hal_hdr test_cmd_2_hdr = {
+	.service_id = 0,
+	.opcode = 2,
+	.len = 0
+};
+
 static const struct test_data test_cmd_service_invalid_1 = {
 	.cmd = &test_cmd_1_hdr,
 	.cmd_size = sizeof(test_cmd_1_hdr),
@@ -403,6 +419,32 @@ static const struct test_data test_cmd_service_invalid_2 = {
 	.expected_signal = SIGTERM
 };
 
+static const struct ipc_handler cmd_handlers_invalid_2[] = {
+	{ test_cmd_handler_1, false, 0 },
+	{ test_cmd_handler_invalid, false, 0 }
+};
+
+static const struct ipc_handler cmd_handlers_invalid_1[] = {
+	{ test_cmd_handler_invalid, false, 0 },
+	{ test_cmd_handler_2, false, 0 },
+};
+
+static const struct test_data test_cmd_opcode_valid_1 = {
+	.cmd = &test_cmd_1_hdr,
+	.cmd_size = sizeof(test_cmd_1_hdr),
+	.service = 0,
+	.handlers = cmd_handlers_invalid_2,
+	.handlers_size = 2,
+};
+
+static const struct test_data test_cmd_opcode_valid_2 = {
+	.cmd = &test_cmd_2_hdr,
+	.cmd_size = sizeof(test_cmd_2_hdr),
+	.service = 0,
+	.handlers = cmd_handlers_invalid_1,
+	.handlers_size = 2,
+};
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -417,6 +459,10 @@ int main(int argc, char *argv[])
 					&test_cmd_service_valid_1, test_cmd_reg);
 	g_test_add_data_func("/android_ipc/test_cmd_service_invalid_2",
 					&test_cmd_service_invalid_2, test_cmd_reg_1);
+	g_test_add_data_func("/android_ipc/test_cmd_opcode_valid_1",
+					&test_cmd_opcode_valid_1, test_cmd_reg);
+	g_test_add_data_func("/android_ipc/test_cmd_opcode_valid_2",
+					&test_cmd_opcode_valid_2, test_cmd_reg);
 
 	return g_test_run();
 }
-- 
1.8.5.2


^ permalink raw reply related

* [PATCH 02/11] android/unit: Rename cmd handler
From: Jakub Tyszkowski @ 2014-02-04 14:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391524749-2518-1-git-send-email-jakub.tyszkowski@tieto.com>

This handler responses for opcode == 1, thus should use proper naming to
avoid confision when more functions sending different responses will be
added.
---
 android/test-ipc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/android/test-ipc.c b/android/test-ipc.c
index d0f3f6b..8e31507 100644
--- a/android/test-ipc.c
+++ b/android/test-ipc.c
@@ -363,7 +363,7 @@ static void test_cmd_reg_1(gconstpointer data)
 	ipc_cleanup();
 }
 
-static void test_cmd_handler(const void *buf, uint16_t len)
+static void test_cmd_handler_1(const void *buf, uint16_t len)
 {
 	ipc_send_rsp(0, 1, 0);
 }
@@ -383,7 +383,7 @@ static const struct test_data test_cmd_service_invalid_1 = {
 };
 
 static const struct ipc_handler cmd_handlers[] = {
-	{ test_cmd_handler, false, 0 }
+	{ test_cmd_handler_1, false, 0 }
 };
 
 static const struct test_data test_cmd_service_valid_1 = {
-- 
1.8.5.2


^ permalink raw reply related

* [PATCH 01/11] android/unit: Fix checking for expected termination
From: Jakub Tyszkowski @ 2014-02-04 14:38 UTC (permalink / raw)
  To: linux-bluetooth

This fix makes sure that when signalled termination is expected,
it actually happens. If IPC termination is expected no response will be
sent, so cmd_watch will never be executed. But if it is executed when
expecting termination, its a failure.
---
 android/test-ipc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/android/test-ipc.c b/android/test-ipc.c
index 8af5739..d0f3f6b 100644
--- a/android/test-ipc.c
+++ b/android/test-ipc.c
@@ -82,6 +82,8 @@ static gboolean cmd_watch(GIOChannel *io, GIOCondition cond,
 	uint8_t buf[128];
 	int sk;
 
+	g_assert(test_data->expected_signal == 0);
+
 	if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
 		g_assert(FALSE);
 		return FALSE;
-- 
1.8.5.2


^ permalink raw reply related

* Re: [PATCH 3/3] avrcp: Fix possible buffer overflow and correct length
From: Andrei Emeltchenko @ 2014-02-04 14:08 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391098376-26834-3-git-send-email-Andrei.Emeltchenko.news@gmail.com>

On Thu, Jan 30, 2014 at 06:12:56PM +0200, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Wrong length was given and it was also possible to crash.

ping

> ---
>  profiles/audio/avrcp.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 128f7d3..f9fce5c 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -1899,8 +1899,12 @@ static void avrcp_get_current_player_value(struct avrcp *session,
>  {
>  	uint8_t buf[AVRCP_HEADER_LENGTH + 5];
>  	struct avrcp_header *pdu = (void *) buf;
> +	uint16_t length = AVRCP_HEADER_LENGTH + count + 1;
>  	int i;
>  
> +	if (count + 1 > 5)
> +		return;
> +
>  	memset(buf, 0, sizeof(buf));
>  
>  	set_company_id(pdu->company_id, IEEEID_BTSIG);
> @@ -1913,7 +1917,7 @@ static void avrcp_get_current_player_value(struct avrcp *session,
>  		pdu->params[i + 1] = attrs[i];
>  
>  	avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS,
> -					AVC_SUBUNIT_PANEL, buf, sizeof(buf),
> +					AVC_SUBUNIT_PANEL, buf, length,
>  					avrcp_player_value_rsp, session);
>  }
>  
> -- 
> 1.8.3.2
> 
> --
> 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

^ permalink raw reply

* Re: [PATCH 2/3] avrcp: Fix using incorrect buffer for SetVolume
From: Andrei Emeltchenko @ 2014-02-04 14:08 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391098376-26834-2-git-send-email-Andrei.Emeltchenko.news@gmail.com>

On Thu, Jan 30, 2014 at 06:12:55PM +0200, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> The command requires one parameter.

ping

> ---
>  profiles/audio/avrcp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 2e1a940..128f7d3 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -3706,7 +3706,7 @@ int avrcp_set_volume(struct btd_device *dev, uint8_t volume)
>  {
>  	struct avrcp_server *server;
>  	struct avrcp *session;
> -	uint8_t buf[AVRCP_HEADER_LENGTH + 2];
> +	uint8_t buf[AVRCP_HEADER_LENGTH + 1];
>  	struct avrcp_header *pdu = (void *) buf;
>  
>  	server = find_server(servers, device_get_adapter(dev));
> -- 
> 1.8.3.2
> 
> --
> 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

^ permalink raw reply

* [PATCH] unit/avctp: Add TP/NFR/BV-04-C test
From: Andrei Emeltchenko @ 2014-02-04 13:47 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Add test TP/NFR/BV-04-C for AVCTP.
---
 unit/test-avctp.c | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/unit/test-avctp.c b/unit/test-avctp.c
index 6d7e34a..581f88c 100644
--- a/unit/test-avctp.c
+++ b/unit/test-avctp.c
@@ -240,15 +240,6 @@ static void execute_context(struct context *context)
 	destroy_context(context);
 }
 
-static void test_client(gconstpointer data)
-{
-	struct context *context = create_context(0x0100, data);
-
-	avctp_send_vendordep_req(context->session, 0, 0, NULL, 0, NULL, NULL);
-
-	execute_context(context);
-}
-
 static size_t handler(struct avctp *session,
 					uint8_t transaction, uint8_t *code,
 					uint8_t *subunit, uint8_t *operands,
@@ -265,6 +256,33 @@ static size_t handler(struct avctp *session,
 	return operand_count;
 }
 
+static gboolean handler_response(struct avctp *session,
+					uint8_t code, uint8_t subunit,
+					uint8_t *operands, size_t operand_count,
+					void *user_data)
+{
+	struct context *context = user_data;
+
+	DBG("code 0x%02x subunit %d operand_count %zu", code, subunit,
+								operand_count);
+
+	g_assert_cmpint(code, ==, 0x0a);
+	g_assert_cmpint(subunit, ==, 0);
+	g_assert_cmpint(operand_count, ==, 0);
+
+	return context_quit(context);
+}
+
+static void test_client(gconstpointer data)
+{
+	struct context *context = create_context(0x0100, data);
+
+	avctp_send_vendordep_req(context->session, 0, 0, NULL, 0,
+						handler_response, context);
+
+	execute_context(context);
+}
+
 static void test_server(gconstpointer data)
 {
 	struct context *context = create_context(0x0100, data);
@@ -322,6 +340,10 @@ int main(int argc, char *argv[])
 				raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x00, 0x00),
 				raw_pdu(0x02, 0x11, 0x0e, 0x00, 0x00, 0x00));
 
+	define_test("/TP/NFR/BV-04-C", test_client,
+				raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x00, 0x00),
+				raw_pdu(0x02, 0x11, 0x0e, 0x0a, 0x00, 0x00));
+
 	define_test("/TP/NFR/BI-01-C", test_server,
 				raw_pdu(0x00, 0xff, 0xff, 0x00, 0x00, 0x00),
 				raw_pdu(0x03, 0xff, 0xff));
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH] android/avctp: Fix memory leak: unregister pdu handler
From: Luiz Augusto von Dentz @ 2014-02-04 13:47 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1391506089-23985-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Tue, Feb 4, 2014 at 11:28 AM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Unregister pdu handlers on shutdown.
> ---
>  android/avctp.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/android/avctp.c b/android/avctp.c
> index 8f35403..8681cb0 100644
> --- a/android/avctp.c
> +++ b/android/avctp.c
> @@ -1462,6 +1462,15 @@ void avctp_shutdown(struct avctp *session)
>         if (session->browsing)
>                 avctp_channel_destroy(session->browsing);
>
> +       if (session->passthrough_id > 0)
> +               avctp_unregister_pdu_handler(session, session->passthrough_id);
> +
> +       if (session->unit_id > 0)
> +               avctp_unregister_pdu_handler(session, session->unit_id);
> +
> +       if (session->subunit_id > 0)
> +               avctp_unregister_pdu_handler(session, session->subunit_id);
> +
>         if (session->control)
>                 avctp_channel_destroy(session->control);
>
> --
> 1.8.3.2

There is no leak, avctp_channel_destroy cleanup all the registered handlers.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH] Bluetooth: hidp: make sure input buffers are big enough
From: Jiri Kosina @ 2014-02-04 13:46 UTC (permalink / raw)
  To: David Herrmann
  Cc: Marcel Holtmann, open list:HID CORE LAYER,
	linux-bluetooth@vger.kernel.org development, Gustavo F. Padovan
In-Reply-To: <CANq1E4S++uqMzQue_0jC3N=Lm8Os5V-48Hw5M4vWvM+2Vx91yA@mail.gmail.com>

On Mon, 3 Feb 2014, David Herrmann wrote:

> >> >> Due to various reasons I will not have access to any testing HW for the
> >> >> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd
> >> >> be appreciated.
> >> >>
> >> >> Otherwise I'll be working on it by the end of this week.
> >> >
> >> > David,
> >> >
> >> > just got back to this, finally ... did you have time to work on this at
> >> > all, or should I just start from scratch?
> >>
> >> Sorry, no. Fosdem is this weekend and I needed to get my code ready
> >> for that. But I'll finally have time again next week.
> >
> > Okay, thanks. I then guess we should proceed with this bandaid (double
> > allocation) for 3.14
> 
> It would be really nice if we could get the HIDP patch into 3.14-rc2
> and backported to stable. There have been quite a bunch of reports now
> and I dislike adding a GFP_ATOMIC allocation in HID core. 

I agree with David; Gustavo, what is your take on this, please?

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox