* Re: [RFC 0/4] android: Permanent storage support
From: Szymon Janc @ 2013-12-18 12:23 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <3D8D935B-BE1E-4158-B2F5-8F49851A718C@holtmann.org>
Hi Marcel,
> Hi Szymon,
>
> > This is first RFC for storage format for Android daemon.
> >
> > Some highlights:
> > - storage is much simple in format than Linux daemon (no addresses in paths
> > or filenames, only 1 adapter support etc)
> > - only info that requires persistency over daemon lifetime from Framework
> > perspective is stored
> > - settings file for adapter config
> > - devices file for remote device info (groups are remote devices addresses)
> > - Android storage path is /data/misc/bluez (I've decided to not use
> > /data/misc/bluetooth to avoid any clashes as there still seems to be
> > leftovers available in Android tree eg. there are still references to that
> > directory in CTS testcases and init files of Android 4.4)
>
> I would use /data/misc/bluetooth actually. Especially since the daemon is also called bluetoothd and not bluezd.
>
> What is the impact on just using /data/misc/bluetooth and see if anything really breaks.
OK, this will require proper chown in device init.rc (as it is set to system/system
in system/core/rootdir/init.rc), but that would be needed for /data/misc/bluez path
anyway...
>
> > I'm not sure how to handle storage on Linux host so for now it is set to
> > STORAGEDIR/android/ directory (if directory is missing it is not created
> > by daemon).
>
> That is actually fine. Works for me.
>
> > Storage format is as following sample:
> >
> > settings file:
> > [General]
> > Address=20:68:9D:3A:52:5E
> > Name=BlueZ Android
> > DiscoverableTimeout=0
> >
> > devices file:
> > [00:0D:3C:B1:C4:AC]
> > Type=0
> > Name=Nokia BH-503
> > Class=2360324
> > LinkKey=0x306A400930B0AE36D7AC45D8DC50F1A0
>
> I would leave the 0x out of it. Just make it a hex encoded string.
OK.
>
> > LinkKeyType=0
> > LinkKeyPINLength=4
> > Services=0x0000110B00001000800000805F9B34FB;0x0000110C00001000800000805F9B34FB;0x0000110E00001000800000805F9B34FB;0x0000111E00001000800000805F9B34FB;0x0000110800001000800000805F9B34FB;
>
> Lets store these as UUID strings. We convert between them anyway.
We actually don't use strings in Android daemon and keep UUIDs as 16bytes
arrays (same format as HAL expects).
So we would first need to convert to bt_uuid_t to be able to use lib/uuid.h
uuid<->string helpers. We could also just add '-' where needed while converting
array to hexstring if that really improves readability.
>
> > [00:1F:20:17:87:D7]
> > Type=0
> > Name=Bluetooth Laser Travel Mouse
> > Class=9600
> > LinkKey=0x3725CC552EAB8AB82D843BFEB14D2E47
> > LinkKeyType=0
> > LinkKeyPINLength=4
> > Services=0x0000112400001000800000805F9B34FB;0x0000120000001000800000805F9B34FB;
> >
> > [40:B0:FA:39:FF:B8]
> > Type=0
> > Name=Nexus 4
> > Class=5898764
> > LinkKey=0x1E201EE8E82E1E50682622077D372F20
> > LinkKeyType=5
> > LinkKeyPINLength=0
> > Services=0x0000180100001000800000805F9B34FB;0x0000180000001000800000805F9B34FB;0x0000111200001000800000805F9B34FB;0x0000111F00001000800000805F9B34FB;0x0000110C00001000800000805F9B34FB;0x0000110A00001000800000805F9B34FB;0x0000110E00001000800000805F9B34FB;0x0000111600001000800000805F9B34FB;0x0000111500001000800000805F9B34FB;0x0000113200001000800000805F9B34FB;0x0000112F00001000800000805F9B34FB;0x0000110500001000800000805F9B34FB;
> >
> >
> > Comments are welcome.
>
> I think the only discussion we could have is if we want to store them as 1 file per remote device or all devices in one file. I am personally fine with this approach since it is a limited scope with Android anyway. Also upgrading to a more complex storage later is easily possible.
FWIW, we could also always keep g_key_file in memory and only write on update
(to reduce reads). And that would be more complicated if we use multiple files.
--
BR
Szymon Janc
^ permalink raw reply
* Re: [PATCH] avctp: Move avctp.c|h from profiles/audio to protocol/
From: Luiz Augusto von Dentz @ 2013-12-18 11:57 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Ravi kumar Veeramally,
linux-bluetooth@vger.kernel.org development
In-Reply-To: <13E32B04-DDEA-4BE1-BCAB-40C0395D638F@holtmann.org>
Hi Marcel, Ravi,
On Wed, Dec 18, 2013 at 1:39 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Ravi,
>
>> This is an initial patch for decoupling avctp.c|h from profiles/audio.
>> Goal is to remove external dependency (glib is enough) and reusable
>> to android/* sources.
>> ---
>> Makefile.plugins | 2 +-
>> {profiles/audio => protocol}/avctp.c | 7 +------
>> {profiles/audio => protocol}/avctp.h | 0
>> 3 files changed, 2 insertions(+), 7 deletions(-)
>> rename {profiles/audio => protocol}/avctp.c (99%)
>> rename {profiles/audio => protocol}/avctp.h (100%)
>
> actually, no.
>
> The breaks my nicely tab-tab-completion handling when going through the source code. Pro{file,tocol} have a way to close prefix.
Lets handle these similar to AVDTP by copying it to android, remove
dependencies to core the add to android build and create proper unit
tests for it, eventually it should replace all the existing
implementation of AVCTP but first we need to have the unit tests done.
Later we can decide where it should be, perhaps it can even be inside
lib/ for example (though internally it might still depend on glib.)
--
Luiz Augusto von Dentz
^ permalink raw reply
* [PATCH 2/2] android/tester: Add bluetooth HAL test case - name prefix
From: Grzegorz Kolodziejczyk @ 2013-12-18 11:54 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1387367677-8099-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This add prefix for bluetooth HAL test case names. Bluetooth HAL test
cases now can be easily distinguished. Now Bluetooth HAL test cases
can be run separately.
---
android/android-tester.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/android/android-tester.c b/android/android-tester.c
index e013e02..55fa702 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -1167,63 +1167,63 @@ int main(int argc, char *argv[])
tester_init(&argc, &argv);
- test_bredrle("Init", NULL, setup_base, test_dummy, teardown);
+ test_bredrle("Bluetooth Init", NULL, setup_base, test_dummy, teardown);
- test_bredrle("Enable - Success", &bluetooth_enable_success_test,
+ test_bredrle("Bluetooth Enable - Success", &bluetooth_enable_success_test,
setup_base, test_enable, teardown);
- test_bredrle("Enable - Done", &bluetooth_enable_done_test,
+ test_bredrle("Bluetooth Enable - Done", &bluetooth_enable_done_test,
setup_enabled_adapter, test_enable_done, teardown);
- test_bredrle("Disable - Success", &bluetooth_disable_success_test,
+ test_bredrle("Bluetooth Disable - Success", &bluetooth_disable_success_test,
setup_enabled_adapter, test_disable, teardown);
- test_bredrle("Set BDNAME - Success",
+ test_bredrle("Bluetooth Set BDNAME - Success",
&bluetooth_setprop_bdname_success_test,
setup_enabled_adapter,
test_setprop_bdname_success, teardown);
- test_bredrle("Set SCAN_MODE - Success",
+ test_bredrle("Bluetooth Set SCAN_MODE - Success",
&bluetooth_setprop_scanmode_success_test,
setup_enabled_adapter,
test_setprop_scanmode_succes, teardown);
- test_bredrle("Set DISCOVERY_TIMEOUT - Success",
+ test_bredrle("Bluetooth Set DISCOVERY_TIMEOUT - Success",
&bluetooth_setprop_disctimeout_success_test,
setup_enabled_adapter,
test_setprop_disctimeout_succes, teardown);
- test_bredrle("Get BDADDR - Success",
+ test_bredrle("Bluetooth Get BDADDR - Success",
&bluetooth_getprop_bdaddr_success_test,
setup_enabled_adapter,
test_getprop_bdaddr_success, teardown);
- test_bredrle("Get BDNAME - Success",
+ test_bredrle("Bluetooth Get BDNAME - Success",
&bluetooth_getprop_bdname_success_test,
setup_enabled_adapter,
test_getprop_bdname_success, teardown);
- test_bredrle("Set UUID - Invalid",
+ test_bredrle("Bluetooth Set UUID - Invalid",
&bluetooth_setprop_uuid_invalid_test,
setup_enabled_adapter,
test_setprop_uuid_invalid, teardown);
- test_bredrle("Set CLASS_OF_DEVICE - Invalid",
+ test_bredrle("Bluetooth Set CLASS_OF_DEVICE - Invalid",
&bluetooth_setprop_cod_invalid_test,
setup_enabled_adapter,
test_setprop_cod_invalid, teardown);
- test_bredrle("Set TYPE_OF_DEVICE - Invalid",
+ test_bredrle("Bluetooth Set TYPE_OF_DEVICE - Invalid",
&bluetooth_setprop_tod_invalid_test,
setup_enabled_adapter,
test_setprop_tod_invalid, teardown);
- test_bredrle("Set REMOTE_RSSI - Invalid",
+ test_bredrle("Bluetooth Set REMOTE_RSSI - Invalid",
&bluetooth_setprop_remote_rssi_invalid_test,
setup_enabled_adapter,
test_setprop_rssi_invalid, teardown);
- test_bredrle("Set SERVICE_RECORD - Invalid",
+ test_bredrle("Bluetooth Set SERVICE_RECORD - Invalid",
&bluetooth_setprop_service_record_invalid_test,
setup_enabled_adapter,
test_setprop_service_record_invalid, teardown);
--
1.8.4.2
^ permalink raw reply related
* [PATCH 1/2] android/tester: Move Bluetooth HAL functions before socket HAL
From: Grzegorz Kolodziejczyk @ 2013-12-18 11:54 UTC (permalink / raw)
To: linux-bluetooth
This patch move bluetooth HAL test functions before socket
HAL functions. This make it more readable for future test cases growth.
---
android/android-tester.c | 272 +++++++++++++++++++++++------------------------
1 file changed, 136 insertions(+), 136 deletions(-)
diff --git a/android/android-tester.c b/android/android-tester.c
index 0f0432d..e013e02 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -826,6 +826,11 @@ static void teardown(const void *test_data)
tester_teardown_complete();
}
+static void test_dummy(const void *test_data)
+{
+ tester_test_passed();
+}
+
static void test_enable(const void *test_data)
{
struct test_data *data = tester_get_data();
@@ -857,9 +862,138 @@ static void test_disable(const void *test_data)
data->if_bluetooth->disable();
}
-static void test_dummy(const void *test_data)
+static void test_setprop_bdname_success(const void *test_data)
{
- tester_test_passed();
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+
+ check_expected_status(adapter_status);
+}
+
+static void test_setprop_scanmode_succes(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+}
+
+static void test_setprop_disctimeout_succes(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+}
+
+static void test_getprop_bdaddr_success(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t prop = test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->get_adapter_property(prop.type);
+ check_expected_status(adapter_status);
+}
+
+static void test_getprop_bdname_success(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+
+ adapter_status = data->if_bluetooth->get_adapter_property((*prop).type);
+ check_expected_status(adapter_status);
+}
+
+static void test_setprop_uuid_invalid(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+}
+
+static void test_setprop_cod_invalid(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+}
+
+static void test_setprop_tod_invalid(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+}
+
+static void test_setprop_rssi_invalid(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+}
+
+static void test_setprop_service_record_invalid(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
}
/* Test Socket HAL */
@@ -1014,140 +1148,6 @@ clean:
close(sock_fd);
}
-static void test_setprop_bdname_success(const void *test_data)
-{
- struct test_data *data = tester_get_data();
- const struct generic_data *test = data->test_data;
- const bt_property_t *prop = &test->expected_property;
- bt_status_t adapter_status;
-
- init_test_conditions(data);
-
- adapter_status = data->if_bluetooth->set_adapter_property(prop);
-
- check_expected_status(adapter_status);
-}
-
-static void test_setprop_scanmode_succes(const void *test_data)
-{
- struct test_data *data = tester_get_data();
- const struct generic_data *test = data->test_data;
- const bt_property_t *prop = &test->expected_property;
- bt_status_t adapter_status;
-
- init_test_conditions(data);
-
- adapter_status = data->if_bluetooth->set_adapter_property(prop);
- check_expected_status(adapter_status);
-}
-
-static void test_setprop_disctimeout_succes(const void *test_data)
-{
- struct test_data *data = tester_get_data();
- const struct generic_data *test = data->test_data;
- const bt_property_t *prop = &test->expected_property;
- bt_status_t adapter_status;
-
- init_test_conditions(data);
-
- adapter_status = data->if_bluetooth->set_adapter_property(prop);
- check_expected_status(adapter_status);
-}
-
-static void test_getprop_bdaddr_success(const void *test_data)
-{
- struct test_data *data = tester_get_data();
- const struct generic_data *test = data->test_data;
- const bt_property_t prop = test->expected_property;
- bt_status_t adapter_status;
-
- init_test_conditions(data);
-
- adapter_status = data->if_bluetooth->get_adapter_property(prop.type);
- check_expected_status(adapter_status);
-}
-
-static void test_getprop_bdname_success(const void *test_data)
-{
- struct test_data *data = tester_get_data();
- const struct generic_data *test = data->test_data;
- const bt_property_t *prop = &test->expected_property;
- bt_status_t adapter_status;
-
- init_test_conditions(data);
-
- adapter_status = data->if_bluetooth->set_adapter_property(prop);
- check_expected_status(adapter_status);
-
- adapter_status = data->if_bluetooth->get_adapter_property((*prop).type);
- check_expected_status(adapter_status);
-}
-
-static void test_setprop_uuid_invalid(const void *test_data)
-{
- struct test_data *data = tester_get_data();
- const struct generic_data *test = data->test_data;
- const bt_property_t *prop = &test->expected_property;
- bt_status_t adapter_status;
-
- init_test_conditions(data);
-
- adapter_status = data->if_bluetooth->set_adapter_property(prop);
- check_expected_status(adapter_status);
-}
-
-static void test_setprop_cod_invalid(const void *test_data)
-{
- struct test_data *data = tester_get_data();
- const struct generic_data *test = data->test_data;
- const bt_property_t *prop = &test->expected_property;
- bt_status_t adapter_status;
-
- init_test_conditions(data);
-
- adapter_status = data->if_bluetooth->set_adapter_property(prop);
- check_expected_status(adapter_status);
-}
-
-static void test_setprop_tod_invalid(const void *test_data)
-{
- struct test_data *data = tester_get_data();
- const struct generic_data *test = data->test_data;
- const bt_property_t *prop = &test->expected_property;
- bt_status_t adapter_status;
-
- init_test_conditions(data);
-
- adapter_status = data->if_bluetooth->set_adapter_property(prop);
- check_expected_status(adapter_status);
-}
-
-static void test_setprop_rssi_invalid(const void *test_data)
-{
- struct test_data *data = tester_get_data();
- const struct generic_data *test = data->test_data;
- const bt_property_t *prop = &test->expected_property;
- bt_status_t adapter_status;
-
- init_test_conditions(data);
-
- adapter_status = data->if_bluetooth->set_adapter_property(prop);
- check_expected_status(adapter_status);
-}
-
-static void test_setprop_service_record_invalid(const void *test_data)
-{
- struct test_data *data = tester_get_data();
- const struct generic_data *test = data->test_data;
- const bt_property_t *prop = &test->expected_property;
- bt_status_t adapter_status;
-
- init_test_conditions(data);
-
- adapter_status = data->if_bluetooth->set_adapter_property(prop);
- check_expected_status(adapter_status);
-}
-
#define test_bredrle(name, data, test_setup, test, test_teardown) \
do { \
struct test_data *user; \
--
1.8.4.2
^ permalink raw reply related
* Re: [PATCH] avctp: Move avctp.c|h from profiles/audio to protocol/
From: Ravi kumar Veeramally @ 2013-12-18 11:49 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <13E32B04-DDEA-4BE1-BCAB-40C0395D638F@holtmann.org>
Hi Marcel,
Ok, I just had a chat with Luiz now. Actually task description is outdated.
Sorry for inconvenience. Please ignore this patch.
Regards,
Ravi.
On 18.12.2013 13:39, Marcel Holtmann wrote:
> Hi Ravi,
>
>> This is an initial patch for decoupling avctp.c|h from profiles/audio.
>> Goal is to remove external dependency (glib is enough) and reusable
>> to android/* sources.
>> ---
>> Makefile.plugins | 2 +-
>> {profiles/audio => protocol}/avctp.c | 7 +------
>> {profiles/audio => protocol}/avctp.h | 0
>> 3 files changed, 2 insertions(+), 7 deletions(-)
>> rename {profiles/audio => protocol}/avctp.c (99%)
>> rename {profiles/audio => protocol}/avctp.h (100%)
> actually, no.
>
> The breaks my nicely tab-tab-completion handling when going through the source code. Pro{file,tocol} have a way to close prefix.
>
> Regards
>
> Marcel
>
> --
> 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] avctp: Move avctp.c|h from profiles/audio to protocol/
From: Marcel Holtmann @ 2013-12-18 11:39 UTC (permalink / raw)
To: Ravi kumar Veeramally; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1387365861-23356-1-git-send-email-ravikumar.veeramally@linux.intel.com>
Hi Ravi,
> This is an initial patch for decoupling avctp.c|h from profiles/audio.
> Goal is to remove external dependency (glib is enough) and reusable
> to android/* sources.
> ---
> Makefile.plugins | 2 +-
> {profiles/audio => protocol}/avctp.c | 7 +------
> {profiles/audio => protocol}/avctp.h | 0
> 3 files changed, 2 insertions(+), 7 deletions(-)
> rename {profiles/audio => protocol}/avctp.c (99%)
> rename {profiles/audio => protocol}/avctp.h (100%)
actually, no.
The breaks my nicely tab-tab-completion handling when going through the source code. Pro{file,tocol} have a way to close prefix.
Regards
Marcel
^ permalink raw reply
* Re: [RFC 0/4] android: Permanent storage support
From: Marcel Holtmann @ 2013-12-18 11:37 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1387365660-25281-1-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
> This is first RFC for storage format for Android daemon.
>
> Some highlights:
> - storage is much simple in format than Linux daemon (no addresses in paths
> or filenames, only 1 adapter support etc)
> - only info that requires persistency over daemon lifetime from Framework
> perspective is stored
> - settings file for adapter config
> - devices file for remote device info (groups are remote devices addresses)
> - Android storage path is /data/misc/bluez (I've decided to not use
> /data/misc/bluetooth to avoid any clashes as there still seems to be
> leftovers available in Android tree eg. there are still references to that
> directory in CTS testcases and init files of Android 4.4)
I would use /data/misc/bluetooth actually. Especially since the daemon is also called bluetoothd and not bluezd.
What is the impact on just using /data/misc/bluetooth and see if anything really breaks.
> I'm not sure how to handle storage on Linux host so for now it is set to
> STORAGEDIR/android/ directory (if directory is missing it is not created
> by daemon).
That is actually fine. Works for me.
> Storage format is as following sample:
>
> settings file:
> [General]
> Address=20:68:9D:3A:52:5E
> Name=BlueZ Android
> DiscoverableTimeout=0
>
> devices file:
> [00:0D:3C:B1:C4:AC]
> Type=0
> Name=Nokia BH-503
> Class=2360324
> LinkKey=0x306A400930B0AE36D7AC45D8DC50F1A0
I would leave the 0x out of it. Just make it a hex encoded string.
> LinkKeyType=0
> LinkKeyPINLength=4
> Services=0x0000110B00001000800000805F9B34FB;0x0000110C00001000800000805F9B34FB;0x0000110E00001000800000805F9B34FB;0x0000111E00001000800000805F9B34FB;0x0000110800001000800000805F9B34FB;
Lets store these as UUID strings. We convert between them anyway.
> [00:1F:20:17:87:D7]
> Type=0
> Name=Bluetooth Laser Travel Mouse
> Class=9600
> LinkKey=0x3725CC552EAB8AB82D843BFEB14D2E47
> LinkKeyType=0
> LinkKeyPINLength=4
> Services=0x0000112400001000800000805F9B34FB;0x0000120000001000800000805F9B34FB;
>
> [40:B0:FA:39:FF:B8]
> Type=0
> Name=Nexus 4
> Class=5898764
> LinkKey=0x1E201EE8E82E1E50682622077D372F20
> LinkKeyType=5
> LinkKeyPINLength=0
> Services=0x0000180100001000800000805F9B34FB;0x0000180000001000800000805F9B34FB;0x0000111200001000800000805F9B34FB;0x0000111F00001000800000805F9B34FB;0x0000110C00001000800000805F9B34FB;0x0000110A00001000800000805F9B34FB;0x0000110E00001000800000805F9B34FB;0x0000111600001000800000805F9B34FB;0x0000111500001000800000805F9B34FB;0x0000113200001000800000805F9B34FB;0x0000112F00001000800000805F9B34FB;0x0000110500001000800000805F9B34FB;
>
>
> Comments are welcome.
I think the only discussion we could have is if we want to store them as 1 file per remote device or all devices in one file. I am personally fine with this approach since it is a limited scope with Android anyway. Also upgrading to a more complex storage later is easily possible.
Regards
Marcel
^ permalink raw reply
* [PATCH] avctp: Move avctp.c|h from profiles/audio to protocol/
From: Ravi kumar Veeramally @ 2013-12-18 11:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ravi kumar Veeramally
This is an initial patch for decoupling avctp.c|h from profiles/audio.
Goal is to remove external dependency (glib is enough) and reusable
to android/* sources.
---
Makefile.plugins | 2 +-
{profiles/audio => protocol}/avctp.c | 7 +------
{profiles/audio => protocol}/avctp.h | 0
3 files changed, 2 insertions(+), 7 deletions(-)
rename {profiles/audio => protocol}/avctp.c (99%)
rename {profiles/audio => protocol}/avctp.h (100%)
diff --git a/Makefile.plugins b/Makefile.plugins
index 6a1ddbf..a64d4f5 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -41,7 +41,7 @@ builtin_sources += profiles/audio/source.h profiles/audio/source.c \
builtin_modules += avrcp
builtin_sources += profiles/audio/control.h profiles/audio/control.c \
- profiles/audio/avctp.h profiles/audio/avctp.c \
+ protocol/avctp.h protocol/avctp.c \
profiles/audio/avrcp.h profiles/audio/avrcp.c \
profiles/audio/player.h profiles/audio/player.c
diff --git a/profiles/audio/avctp.c b/protocol/avctp.c
similarity index 99%
rename from profiles/audio/avctp.c
rename to protocol/avctp.c
index 6669ddc..a09cd93 100644
--- a/profiles/audio/avctp.c
+++ b/protocol/avctp.c
@@ -32,12 +32,7 @@
#include <stdbool.h>
#include <errno.h>
#include <unistd.h>
-#include <assert.h>
-#include <signal.h>
-#include <sys/types.h>
-#include <sys/stat.h>
#include <fcntl.h>
-#include <netinet/in.h>
#include <bluetooth/bluetooth.h>
#include <bluetooth/sdp.h>
@@ -54,7 +49,7 @@
#include "error.h"
#include "uinput.h"
#include "avctp.h"
-#include "avrcp.h"
+#include <profiles/audio/avrcp.h>
/* AV/C Panel 1.23, page 76:
* command with the pressed value is valid for two seconds
diff --git a/profiles/audio/avctp.h b/protocol/avctp.h
similarity index 100%
rename from profiles/audio/avctp.h
rename to protocol/avctp.h
--
1.8.3.2
^ permalink raw reply related
* [RFC 4/4] android/bluetooth: Add support for restoring devices from storage
From: Szymon Janc @ 2013-12-18 11:21 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1387365660-25281-1-git-send-email-szymon.janc@tieto.com>
This adds support to restore bonded devices from storage (including
linkkeys).
---
android/bluetooth.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 112 insertions(+), 1 deletion(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 0ef6191..d1fc9e4 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1581,6 +1581,117 @@ static void clear_uuids(void)
sizeof(cp), &cp, NULL, NULL, NULL);
}
+static void create_device_from_info(GKeyFile *key_file, const char *peer)
+{
+ struct device *dev;
+ uint8_t type;
+ bdaddr_t bdaddr;
+ char **uuids;
+ char *str;
+
+ DBG("%s", peer);
+
+ type = g_key_file_get_integer(key_file, peer, "Type", NULL);
+
+ str2ba(peer, &bdaddr);
+ dev = create_device(&bdaddr, type);
+
+ dev->bond_state = HAL_BOND_STATE_BONDED;
+
+ str = g_key_file_get_string(key_file, peer, "Name", NULL);
+ if (str) {
+ g_free(dev->name);
+ dev->name = str;
+ }
+
+ str = g_key_file_get_string(key_file, peer, "FriendlyName", NULL);
+ if (str) {
+ g_free(dev->friendly_name);
+ dev->friendly_name = str;
+ }
+
+ dev->class = g_key_file_get_integer(key_file, peer, "Class", NULL);
+
+ uuids = g_key_file_get_string_list(key_file, peer, "Services", NULL,
+ NULL);
+ if (uuids) {
+ char **uuid;
+
+ for (uuid = uuids; *uuid; uuid++) {
+ uint8_t *u = g_malloc0(16);
+ int i;
+
+ for (i = 0; i < 16; i++)
+ sscanf((*uuid) + 2 + (i * 2), "%02hhX", &u[i]);
+
+ dev->uuids = g_slist_append(dev->uuids, u);
+ }
+
+ g_strfreev(uuids);
+ }
+}
+
+static struct mgmt_link_key_info *get_key_info(GKeyFile *key_file, const char *peer)
+{
+ struct mgmt_link_key_info *info = NULL;
+ char *str;
+ unsigned int i;
+
+ str = g_key_file_get_string(key_file, peer, "LinkKey", NULL);
+ if (!str || strlen(str) != 34)
+ goto failed;
+
+ info = g_new0(struct mgmt_link_key_info, 1);
+
+ str2ba(peer, &info->addr.bdaddr);
+
+ info->addr.type = g_key_file_get_integer(key_file, peer, "Type", NULL);
+
+ for (i = 0; i < sizeof(info->val); i++)
+ sscanf(str + 2 + (i * 2), "%02hhX", &info->val[i]);
+
+ info->type = g_key_file_get_integer(key_file, peer, "LinkKeyType",
+ NULL);
+ info->pin_len = g_key_file_get_integer(key_file, peer,
+ "LinkKeyPINLength", NULL);
+
+failed:
+ g_free(str);
+
+ return info;
+}
+
+static void load_devices_info(bt_bluetooth_ready cb)
+{
+ GKeyFile *key_file;
+ gchar **devs;
+ gsize len = 0;
+ unsigned int i;
+ GSList *keys = NULL;
+
+ key_file = g_key_file_new();
+
+ g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/devices", 0,
+ NULL);
+
+ devs = g_key_file_get_groups(key_file, &len);
+
+ for (i = 0; i < len; i++) {
+ struct mgmt_link_key_info *key_info;
+
+ create_device_from_info(key_file, devs[i]);
+
+ key_info = get_key_info(key_file, devs[i]);
+ if (key_info)
+ keys = g_slist_prepend(keys, key_info);
+
+ /* TODO ltk */
+ }
+
+ load_link_keys(keys, cb);
+ g_slist_free_full(keys, g_free);
+}
+
static void read_info_complete(uint8_t status, uint16_t length,
const void *param, void *user_data)
{
@@ -1635,7 +1746,7 @@ static void read_info_complete(uint8_t status, uint16_t length,
clear_uuids();
- load_link_keys(NULL, cb);
+ load_devices_info(cb);
set_io_capability();
set_device_id();
--
1.8.3.2
^ permalink raw reply related
* [RFC 3/4] android/bluetooth: Add support for storing link keys
From: Szymon Janc @ 2013-12-18 11:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1387365660-25281-1-git-send-email-szymon.janc@tieto.com>
When new linkkey event is received store linkkey in devices info file.
Stored info includes linkkey, linkkey type and pin length.
---
android/bluetooth.c | 38 ++++++++++++++++++++++++++++++++++----
1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 69412dd..0ef6191 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -496,7 +496,37 @@ static void mgmt_dev_class_changed_event(uint16_t index, uint16_t length,
static void store_link_key(const bdaddr_t *dst, const uint8_t *key,
uint8_t type, uint8_t pin_length)
{
- /* TODO store link key */
+ GKeyFile *key_file;
+ char key_str[35];
+ gsize length = 0;
+ char addr[18];
+ char *data;
+ int i;
+
+ key_file = g_key_file_new();
+
+ if (!g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/devices",
+ 0, NULL))
+ return;
+
+ ba2str(dst, addr);
+
+ DBG("%s type %u pin_len %u", addr, type, pin_length);
+
+ key_str[0] = '0';
+ key_str[1] = 'x';
+ for (i = 0; i < 16; i++)
+ sprintf(key_str + 2 + (i * 2), "%2.2X", key[i]);
+
+ g_key_file_set_string(key_file, addr, "LinkKey", key_str);
+ g_key_file_set_integer(key_file, addr, "LinkKeyType", type);
+ g_key_file_set_integer(key_file, addr, "LinkKeyPINLength", pin_length);
+
+ data = g_key_file_to_data(key_file, &length, NULL);
+ g_file_set_contents(ANDROID_STORAGEDIR"/devices", data, length, NULL);
+ g_free(data);
+
+ g_key_file_free(key_file);
}
static void send_bond_state_change(const bdaddr_t *addr, uint8_t status,
@@ -723,6 +753,9 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
return;
}
+ set_device_bond_state(&addr->bdaddr, HAL_STATUS_SUCCESS,
+ HAL_BOND_STATE_BONDED);
+
if (ev->store_hint) {
const struct mgmt_link_key_info *key = &ev->key;
@@ -730,9 +763,6 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
key->pin_len);
}
- set_device_bond_state(&addr->bdaddr, HAL_STATUS_SUCCESS,
- HAL_BOND_STATE_BONDED);
-
browse_remote_sdp(&addr->bdaddr);
}
--
1.8.3.2
^ permalink raw reply related
* [RFC 2/4] android/bluetooth: Add initial support for storing device info
From: Szymon Janc @ 2013-12-18 11:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1387365660-25281-1-git-send-email-szymon.janc@tieto.com>
This allows to store information about remote device. For now this is
stored only for bonded devices. Currently stored data includes devices
ddress, type, name, friendly name, class and uuids.
---
android/bluetooth.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 71 insertions(+), 1 deletion(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 114a466..69412dd 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -196,6 +196,72 @@ static void load_adapter_config(void)
g_key_file_free(key_file);
}
+static void store_device_info(struct device *dev)
+{
+ GKeyFile *key_file;
+ char addr[18];
+ gsize length = 0;
+ char **uuids = NULL;
+ char *str;
+
+ if (dev->bond_state != HAL_BOND_STATE_BONDED)
+ return;
+
+ ba2str(&dev->bdaddr, addr);
+
+ key_file = g_key_file_new();
+ g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/devices", 0,
+ NULL);
+
+ g_key_file_set_integer(key_file, addr, "Type", dev->bdaddr_type);
+
+ g_key_file_set_string(key_file, addr, "Name", dev->name);
+
+ if (dev->friendly_name)
+ g_key_file_set_string(key_file, addr, "FriendlyName",
+ dev->friendly_name);
+ else
+ g_key_file_remove_key(key_file, addr, "FriendlyName", NULL);
+
+ if (dev->class)
+ g_key_file_set_integer(key_file, addr, "Class", dev->class);
+ else
+ g_key_file_remove_key(key_file, addr, "Class", NULL);
+
+ if (dev->uuids) {
+ GSList *l;
+ int i;
+
+ uuids = g_new0(char *, g_slist_length(dev->uuids) + 1);
+
+ for (i = 0, l = dev->uuids; l; l = g_slist_next(l), i++) {
+ int j;
+ uint8_t *u = l->data;
+ char *uuid_str = g_malloc0(35);
+
+ uuid_str[0] = '0';
+ uuid_str[1] = 'x';
+
+ for (j = 0; j < 16; j++)
+ sprintf(uuid_str + 2 + (j * 2), "%2.2X", u[j]);
+
+ uuids[i] = uuid_str;
+ }
+
+ g_key_file_set_string_list(key_file, addr, "Services",
+ (const char **)uuids, i);
+ } else {
+ g_key_file_remove_key(key_file, addr, "Services", NULL);
+ }
+
+ str = g_key_file_to_data(key_file, &length, NULL);
+ g_file_set_contents(ANDROID_STORAGEDIR"/devices", str, length, NULL);
+ g_free(str);
+
+ g_key_file_free(key_file);
+ g_strfreev(uuids);
+}
+
static int bdaddr_cmp(gconstpointer a, gconstpointer b)
{
const bdaddr_t *bda = a;
@@ -458,6 +524,8 @@ static void set_device_bond_state(const bdaddr_t *addr, uint8_t status,
if (dev->bond_state != state) {
dev->bond_state = state;
send_bond_state_change(&dev->bdaddr, status, state);
+
+ store_device_info(dev);
}
}
@@ -498,6 +566,8 @@ static void set_device_uuids(struct device *dev, GSList *uuids)
g_slist_free_full(dev->uuids, g_free);
dev->uuids = uuids;
+ store_device_info(dev);
+
send_device_uuids_notif(dev);
}
@@ -2539,7 +2609,7 @@ static uint8_t set_device_friendly_name(struct device *dev, const uint8_t *val,
g_free(dev->friendly_name);
dev->friendly_name = g_strndup((const char *) val, len);
- /* TODO store friendly name */
+ store_device_info(dev);
send_device_property(&dev->bdaddr, HAL_PROP_DEVICE_FRIENDLY_NAME,
strlen(dev->friendly_name), dev->friendly_name);
--
1.8.3.2
^ permalink raw reply related
* [RFC 1/4] android/bluetooth: Add initial support for permanent storage
From: Szymon Janc @ 2013-12-18 11:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1387365660-25281-1-git-send-email-szymon.janc@tieto.com>
This patch adds initial support for storing adapter configuration.
Currently stored data is address, name and discoverable timeout.
Since Android daemon storage format is to be simpler than Linux check
if correct adapter is used before going operational. This is
a precaution to avoid e.g. using linkkeys generated for different
controller.
---
android/Android.mk | 3 +-
android/bluetooth.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++---
configure.ac | 3 ++
3 files changed, 95 insertions(+), 6 deletions(-)
diff --git a/android/Android.mk b/android/Android.mk
index ebc3219..1411092 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -8,7 +8,8 @@ pathmap_INCL += glib:external/bluetooth/glib
# Specify common compiler flags
BLUEZ_COMMON_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\" \
- -DPLATFORM_SDK_VERSION=$(PLATFORM_SDK_VERSION)
+ -DPLATFORM_SDK_VERSION=$(PLATFORM_SDK_VERSION) \
+ -DANDROID_STORAGEDIR=\"/data/misc/bluez\" \
# Disable warnings enabled by Android but not enabled in autotools build
BLUEZ_COMMON_CFLAGS += -Wno-pointer-arith -Wno-missing-field-initializers
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 97d4aae..114a466 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -27,6 +27,10 @@
#include <errno.h>
#include <stdlib.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
#include <glib.h>
@@ -123,6 +127,75 @@ static GSList *devices = NULL;
/* This list contains addresses which are asked for records */
static GSList *browse_reqs;
+static void store_adapter_config(void)
+{
+ GKeyFile *key_file;
+ gsize length = 0;
+ char addr[18];
+ char *data;
+
+ key_file = g_key_file_new();
+
+ if (!g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/settings",
+ 0, NULL)) {
+ int fd = open(ANDROID_STORAGEDIR"/settings", O_CREAT, 0600);
+ if (fd < 0) {
+ error("Failed to create adapter config file: %d (%s)",
+ errno, strerror(errno));
+ return;
+ }
+
+ close(fd);
+ }
+
+ ba2str(&adapter.bdaddr, addr);
+
+ g_key_file_set_string(key_file, "General", "Address", addr);
+ g_key_file_set_string(key_file, "General", "Name", adapter.name);
+ g_key_file_set_integer(key_file, "General", "DiscoverableTimeout",
+ adapter.discoverable_timeout);
+
+ data = g_key_file_to_data(key_file, &length, NULL);
+
+ g_file_set_contents(ANDROID_STORAGEDIR"/settings", data, length, NULL);
+
+ g_free(data);
+ g_key_file_free(key_file);
+}
+
+static void load_adapter_config(void)
+{
+ GError *gerr = NULL;
+ GKeyFile *key_file;
+ char *str;
+
+ key_file = g_key_file_new();
+
+ g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/settings", 0,
+ NULL);
+
+ str = g_key_file_get_string(key_file, "General", "Address", NULL);
+ if (!str) {
+ g_key_file_free(key_file);
+ return;
+ }
+
+ str2ba(str, &adapter.bdaddr);
+ g_free(str);
+
+ adapter.name = g_key_file_get_string(key_file, "General", "Name", NULL);
+
+ adapter.discoverable_timeout = g_key_file_get_integer(key_file,
+ "General", "DiscoverableTimeout", &gerr);
+ if (gerr) {
+ adapter.discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT;
+ g_error_free(gerr);
+ gerr = NULL;
+ }
+
+ g_key_file_free(key_file);
+}
+
static int bdaddr_cmp(gconstpointer a, gconstpointer b)
{
const bdaddr_t *bda = a;
@@ -215,6 +288,8 @@ static void adapter_set_name(const uint8_t *name)
g_free(adapter.name);
adapter.name = g_strdup((const char *) name);
+ store_adapter_config();
+
adapter_name_changed(name);
}
@@ -1385,9 +1460,10 @@ static uint8_t set_adapter_discoverable_timeout(const void *buf, uint16_t len)
* There is no need to use kernel feature for that.
* Just need to store this value here */
- /* TODO: This should be in some storage */
memcpy(&adapter.discoverable_timeout, timeout, sizeof(uint32_t));
+ store_adapter_config();
+
send_adapter_property(HAL_PROP_ADAPTER_DISC_TIMEOUT,
sizeof(adapter.discoverable_timeout),
&adapter.discoverable_timeout);
@@ -1434,17 +1510,26 @@ static void read_info_complete(uint8_t status, uint16_t length,
goto failed;
}
+ load_adapter_config();
+
+ if (!bacmp(&adapter.bdaddr, BDADDR_ANY)) {
+ bacpy(&adapter.bdaddr, &rp->bdaddr);
+ adapter.name = g_strdup((const char *) rp->name);
+ store_adapter_config();
+ set_adapter_name(rp->name, strlen((char *)rp->name));
+ } else if (bacmp(&adapter.bdaddr, &rp->bdaddr)) {
+ error("Bluetooth address mismatch");
+ err = -ENODEV;
+ goto failed;
+ }
+
/* Store adapter information */
- bacpy(&adapter.bdaddr, &rp->bdaddr);
adapter.dev_class = rp->dev_class[0] | (rp->dev_class[1] << 8) |
(rp->dev_class[2] << 16);
- adapter.name = g_strdup((const char *) rp->name);
supported_settings = btohs(rp->supported_settings);
adapter.current_settings = btohs(rp->current_settings);
- /* TODO: Read discoverable timeout from storage here */
-
/* TODO: Register all event notification handlers */
register_mgmt_handlers();
diff --git a/configure.ac b/configure.ac
index 18d0b55..5171c38 100644
--- a/configure.ac
+++ b/configure.ac
@@ -252,4 +252,7 @@ AC_ARG_ENABLE(android, AC_HELP_STRING([--enable-android],
[enable_android=${enableval}])
AM_CONDITIONAL(ANDROID, test "${enable_android}" = "yes")
+AC_DEFINE_UNQUOTED(ANDROID_STORAGEDIR, "${storagedir}/android",
+ [Directory for the Android daemon storage files])
+
AC_OUTPUT(Makefile src/bluetoothd.8 lib/bluez.pc)
--
1.8.3.2
^ permalink raw reply related
* [RFC 0/4] android: Permanent storage support
From: Szymon Janc @ 2013-12-18 11:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
Hi All,
This is first RFC for storage format for Android daemon.
Some highlights:
- storage is much simple in format than Linux daemon (no addresses in paths
or filenames, only 1 adapter support etc)
- only info that requires persistency over daemon lifetime from Framework
perspective is stored
- settings file for adapter config
- devices file for remote device info (groups are remote devices addresses)
- Android storage path is /data/misc/bluez (I've decided to not use
/data/misc/bluetooth to avoid any clashes as there still seems to be
leftovers available in Android tree eg. there are still references to that
directory in CTS testcases and init files of Android 4.4)
I'm not sure how to handle storage on Linux host so for now it is set to
STORAGEDIR/android/ directory (if directory is missing it is not created
by daemon).
Storage format is as following sample:
settings file:
[General]
Address=20:68:9D:3A:52:5E
Name=BlueZ Android
DiscoverableTimeout=0
devices file:
[00:0D:3C:B1:C4:AC]
Type=0
Name=Nokia BH-503
Class=2360324
LinkKey=0x306A400930B0AE36D7AC45D8DC50F1A0
LinkKeyType=0
LinkKeyPINLength=4
Services=0x0000110B00001000800000805F9B34FB;0x0000110C00001000800000805F9B34FB;0x0000110E00001000800000805F9B34FB;0x0000111E00001000800000805F9B34FB;0x0000110800001000800000805F9B34FB;
[00:1F:20:17:87:D7]
Type=0
Name=Bluetooth Laser Travel Mouse
Class=9600
LinkKey=0x3725CC552EAB8AB82D843BFEB14D2E47
LinkKeyType=0
LinkKeyPINLength=4
Services=0x0000112400001000800000805F9B34FB;0x0000120000001000800000805F9B34FB;
[40:B0:FA:39:FF:B8]
Type=0
Name=Nexus 4
Class=5898764
LinkKey=0x1E201EE8E82E1E50682622077D372F20
LinkKeyType=5
LinkKeyPINLength=0
Services=0x0000180100001000800000805F9B34FB;0x0000180000001000800000805F9B34FB;0x0000111200001000800000805F9B34FB;0x0000111F00001000800000805F9B34FB;0x0000110C00001000800000805F9B34FB;0x0000110A00001000800000805F9B34FB;0x0000110E00001000800000805F9B34FB;0x0000111600001000800000805F9B34FB;0x0000111500001000800000805F9B34FB;0x0000113200001000800000805F9B34FB;0x0000112F00001000800000805F9B34FB;0x0000110500001000800000805F9B34FB;
Comments are welcome.
Szymon Janc (4):
android/bluetooth: Add initial support for permanent storage
android/bluetooth: Add initial support for storing device info
android/bluetooth: Add support for storing link keys
android/bluetooth: Add support for restoring devices from storage
android/Android.mk | 3 +-
android/bluetooth.c | 317 ++++++++++++++++++++++++++++++++++++++++++++++++++--
configure.ac | 3 +
3 files changed, 311 insertions(+), 12 deletions(-)
--
1.8.3.2
^ permalink raw reply
* Re: [RFC v4 05/12] Bluetooth: Stop scanning on LE connection
From: Johan Hedberg @ 2013-12-18 11:11 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1387289112.1231.21.camel@bali>
Hi Andre,
On Tue, Dec 17, 2013, Andre Guedes wrote:
> > > @@ -518,8 +518,17 @@ static void create_le_conn_complete(struct hci_dev *hdev, u8 status)
> > > {
> > > struct hci_conn *conn;
> > >
> > > - if (status == 0)
> > > + if (status == 0) {
> > > + /* If the discovery procedure was running, we prematurely
> > > + * stopped it. So we have to change the discovery state.
> > > + */
> > > + if (hdev->discovery.state == DISCOVERY_FINDING) {
> > > + cancel_delayed_work(&hdev->le_scan_disable);
> > > + hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> > > + }
> > > +
> > > return;
> > > + }
> > >
> > > BT_ERR("HCI request failed to create LE connection: status 0x%2.2x",
> > > status);
> > > @@ -552,6 +561,18 @@ static int hci_create_le_conn(struct hci_conn *conn)
> > >
> > > hci_req_init(&req, hdev);
> > >
> > > + /* If controller is scanning, we stop it since some controllers are
> > > + * not able to scan and connect at the same time.
> > > + */
> > > + if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) {
> > > + struct hci_cp_le_set_scan_enable enable_cp;
> > > +
> > > + memset(&enable_cp, 0, sizeof(enable_cp));
> > > + enable_cp.enable = LE_SCAN_DISABLE;
> > > + hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(enable_cp),
> > > + &enable_cp);
> > > + }
> > > +
> > > memset(&cp, 0, sizeof(cp));
> > > cp.scan_interval = cpu_to_le16(hdev->le_scan_interval);
> > > cp.scan_window = cpu_to_le16(hdev->le_scan_window);
> >
> > The way this all hangs together with a discovery process started through
> > mgmt_start_discovery feels a bit flimsy to me. Particularly, have you
> > ensured that everything is fine if you've got inquiry ongoing when you
> > do hci_discovery_set_state(hdev, DISCOVERY_STOPPED). Also, are there any
> > risks of race conditions here, e.g. is it fine to let the
> > cancel_delayed_work() call be in create_le_conn_complete() instead of
> > doing it in hci_create_le_conn()?
>
> Yes, I did lots of testing, including the inquiry test you mentioned,
> and I didn't find any issues.
>
> I failed to see any race conditons since cancel_delayed_work() does not
> block. So it is fine to call cancel_delayed_work() in
> create_le_conn_complete() as well as in hci_create_le_conn().
I presume you'd want to prevent the delayed work from getting called as
soon as you've entered the if (test_bit(HCI_LE_SCAN)) section in
hci_create_le_conn? In theory it might get called before
create_le_conn_complete. In such a scenario
le_scan_disable_work_complete could e.g. trigger inquiry before
create_le_conn_complete is called, couldn't it?
> > What also makes this hard to track is that the condition you're testing
> > for first is the HCI_LE_SCAN bit, but then later you look at
> > discovery.state == DISCOVERY_FINDING. For the casual reader there's no
> > direct indication of how these two are releated for the various types of
> > discovery that are possible (LE-only, BR/EDR-only and interleaved).
>
> Yes, I see your point. However, we can't check HCI_LE_SCAN in
> create_le_conn_complete() because the flag was already cleared in
> hci_cc_le_set_scan_enable().
Understood.
> > I don't mean to say that this is a nack for the patch, but I'd like to
> > know that you've considered and tested this kind of cases. I had to
> > spend quite some time looking through the existing code and this patch
> > and still couldn't arrive at absolute confidence of its correctness,
> > meaning there should hopefully be some room for simplification.
>
> So I think we can do some simplification by moving this discovery
> handling from create_le_conn_complete() to hci_create_le_conn(), at
> least I think this code will become a bit easier to follow. What do you
> think?
I think it would at least close the potential race I described above,
i.e. the delayed work getting triggered before create_le_conn_complete.
Johan
^ permalink raw reply
* Re: [PATCH 1/3] android/tester: Check status returned from HAL calls
From: Johan Hedberg @ 2013-12-18 10:26 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1387356913-32666-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
On Wed, Dec 18, 2013, Andrei Emeltchenko wrote:
> In test setup phase check status returned from enable() call.
> ---
> android/android-tester.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
All three patches have been applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH v2 1/5] android/tester: Add UUIDS set prop fail test case
From: Andrei Emeltchenko @ 2013-12-18 10:25 UTC (permalink / raw)
To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
In-Reply-To: <1387360393-853-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
Hi Grzegorz,
On Wed, Dec 18, 2013 at 10:53:09AM +0100, Grzegorz Kolodziejczyk wrote:
> This adds UUIDS set property fail test case due to only get
> possibility.
> ---
> android/android-tester.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/android/android-tester.c b/android/android-tester.c
> index 92e6080..0025f20 100644
> --- a/android/android-tester.c
> +++ b/android/android-tester.c
> @@ -596,6 +596,18 @@ static const struct generic_data bluetooth_getprop_bdname_success_test = {
> .expected_property.len = 17
> };
>
> +static unsigned char setprop_uuids[] = { 0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00,
> + 0x00, 0x80, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00 };
> +
> +static const struct generic_data bluetooth_setprop_uuid_invalid_test = {
> + .expected_hal_callbacks = {ADAPTER_TEST_END},
Do we agree to use spaces before and after braces?
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCH v2 1/5] android/tester: Add UUIDS set prop fail test case
From: Johan Hedberg @ 2013-12-18 10:23 UTC (permalink / raw)
To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
In-Reply-To: <1387360393-853-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
Hi Grzegorz,
On Wed, Dec 18, 2013, Grzegorz Kolodziejczyk wrote:
> This adds UUIDS set property fail test case due to only get
> possibility.
> ---
> android/android-tester.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
All five patches have been applied. Thanks.
Johan
^ permalink raw reply
* [PATCH v2 5/5] android/tester: Add SERVICE_RECORD set prop fail test case
From: Grzegorz Kolodziejczyk @ 2013-12-18 9:53 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1387360393-853-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This adds SERVICE_RECORD set property fail test case due to only
get possibility.
---
android/android-tester.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/android/android-tester.c b/android/android-tester.c
index 9bae94d..3d439de 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -638,6 +638,21 @@ static const struct generic_data bluetooth_setprop_remote_rssi_invalid_test = {
.expected_property.len = sizeof(setprop_remote_rssi)
};
+static bt_service_record_t setprop_remote_service = {
+ .uuid = { {0x00} },
+ .channel = 12,
+ .name = "bt_name"
+};
+
+static const struct generic_data
+ bluetooth_setprop_service_record_invalid_test = {
+ .expected_hal_callbacks = {ADAPTER_TEST_END},
+ .expected_adapter_status = BT_STATUS_FAIL,
+ .expected_property.type = BT_PROPERTY_SERVICE_RECORD,
+ .expected_property.val = &setprop_remote_service,
+ .expected_property.len = sizeof(setprop_remote_service)
+};
+
static bt_callbacks_t bt_callbacks = {
.size = sizeof(bt_callbacks),
.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -1067,6 +1082,19 @@ static void test_setprop_rssi_invalid(const void *test_data)
check_expected_status(adapter_status);
}
+static void test_setprop_service_record_invalid(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+}
+
#define test_bredrle(name, data, test_setup, test, test_teardown) \
do { \
struct test_data *user; \
@@ -1142,6 +1170,11 @@ int main(int argc, char *argv[])
setup_enabled_adapter,
test_setprop_rssi_invalid, teardown);
+ test_bredrle("Set SERVICE_RECORD - Invalid",
+ &bluetooth_setprop_service_record_invalid_test,
+ setup_enabled_adapter,
+ test_setprop_service_record_invalid, teardown);
+
test_bredrle("Socket Init", NULL, setup_socket_interface,
test_dummy, teardown);
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 4/5] android/tester: Add RSSI set prop fail test case
From: Grzegorz Kolodziejczyk @ 2013-12-18 9:53 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1387360393-853-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This adds RSSI set property fail test case due to be only remote device
property.
---
android/android-tester.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/android/android-tester.c b/android/android-tester.c
index 2f2bbff..9bae94d 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -628,6 +628,16 @@ static const struct generic_data bluetooth_setprop_tod_invalid_test = {
.expected_property.len = sizeof(setprop_type_of_device)
};
+static int32_t setprop_remote_rssi = 0;
+
+static const struct generic_data bluetooth_setprop_remote_rssi_invalid_test = {
+ .expected_hal_callbacks = {ADAPTER_TEST_END},
+ .expected_adapter_status = BT_STATUS_FAIL,
+ .expected_property.type = BT_PROPERTY_REMOTE_RSSI,
+ .expected_property.val = &setprop_remote_rssi,
+ .expected_property.len = sizeof(setprop_remote_rssi)
+};
+
static bt_callbacks_t bt_callbacks = {
.size = sizeof(bt_callbacks),
.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -1044,6 +1054,19 @@ static void test_setprop_tod_invalid(const void *test_data)
check_expected_status(adapter_status);
}
+static void test_setprop_rssi_invalid(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+}
+
#define test_bredrle(name, data, test_setup, test, test_teardown) \
do { \
struct test_data *user; \
@@ -1114,6 +1137,11 @@ int main(int argc, char *argv[])
setup_enabled_adapter,
test_setprop_tod_invalid, teardown);
+ test_bredrle("Set REMOTE_RSSI - Invalid",
+ &bluetooth_setprop_remote_rssi_invalid_test,
+ setup_enabled_adapter,
+ test_setprop_rssi_invalid, teardown);
+
test_bredrle("Socket Init", NULL, setup_socket_interface,
test_dummy, teardown);
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 3/5] android/tester: Add TOD set prop fail test case
From: Grzegorz Kolodziejczyk @ 2013-12-18 9:53 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1387360393-853-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This adds TYPE_OF_DEVICE set property fail test case due to only
get possibility.
---
android/android-tester.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/android/android-tester.c b/android/android-tester.c
index 345d5c6..2f2bbff 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -618,6 +618,16 @@ static const struct generic_data bluetooth_setprop_cod_invalid_test = {
.expected_property.len = sizeof(setprop_class_of_device)
};
+static bt_device_type_t setprop_type_of_device = BT_DEVICE_DEVTYPE_BREDR;
+
+static const struct generic_data bluetooth_setprop_tod_invalid_test = {
+ .expected_hal_callbacks = {ADAPTER_TEST_END},
+ .expected_adapter_status = BT_STATUS_FAIL,
+ .expected_property.type = BT_PROPERTY_TYPE_OF_DEVICE,
+ .expected_property.val = &setprop_type_of_device,
+ .expected_property.len = sizeof(setprop_type_of_device)
+};
+
static bt_callbacks_t bt_callbacks = {
.size = sizeof(bt_callbacks),
.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -1021,6 +1031,19 @@ static void test_setprop_cod_invalid(const void *test_data)
check_expected_status(adapter_status);
}
+static void test_setprop_tod_invalid(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+}
+
#define test_bredrle(name, data, test_setup, test, test_teardown) \
do { \
struct test_data *user; \
@@ -1086,6 +1109,11 @@ int main(int argc, char *argv[])
setup_enabled_adapter,
test_setprop_cod_invalid, teardown);
+ test_bredrle("Set TYPE_OF_DEVICE - Invalid",
+ &bluetooth_setprop_tod_invalid_test,
+ setup_enabled_adapter,
+ test_setprop_tod_invalid, teardown);
+
test_bredrle("Socket Init", NULL, setup_socket_interface,
test_dummy, teardown);
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 2/5] android/tester: Add COD set prop fail test case
From: Grzegorz Kolodziejczyk @ 2013-12-18 9:53 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1387360393-853-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>
This adds CLASS_OF_DEVICE set property fail test case due to only
get possibility.
---
android/android-tester.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/android/android-tester.c b/android/android-tester.c
index 0025f20..345d5c6 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -608,6 +608,16 @@ static const struct generic_data bluetooth_setprop_uuid_invalid_test = {
.expected_property.len = sizeof(setprop_uuids)
};
+static uint32_t setprop_class_of_device = 0;
+
+static const struct generic_data bluetooth_setprop_cod_invalid_test = {
+ .expected_hal_callbacks = {ADAPTER_TEST_END},
+ .expected_adapter_status = BT_STATUS_FAIL,
+ .expected_property.type = BT_PROPERTY_CLASS_OF_DEVICE,
+ .expected_property.val = &setprop_class_of_device,
+ .expected_property.len = sizeof(setprop_class_of_device)
+};
+
static bt_callbacks_t bt_callbacks = {
.size = sizeof(bt_callbacks),
.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -998,6 +1008,19 @@ static void test_setprop_uuid_invalid(const void *test_data)
check_expected_status(adapter_status);
}
+static void test_setprop_cod_invalid(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+}
+
#define test_bredrle(name, data, test_setup, test, test_teardown) \
do { \
struct test_data *user; \
@@ -1058,6 +1081,11 @@ int main(int argc, char *argv[])
setup_enabled_adapter,
test_setprop_uuid_invalid, teardown);
+ test_bredrle("Set CLASS_OF_DEVICE - Invalid",
+ &bluetooth_setprop_cod_invalid_test,
+ setup_enabled_adapter,
+ test_setprop_cod_invalid, teardown);
+
test_bredrle("Socket Init", NULL, setup_socket_interface,
test_dummy, teardown);
--
1.8.4.2
^ permalink raw reply related
* [PATCH v2 1/5] android/tester: Add UUIDS set prop fail test case
From: Grzegorz Kolodziejczyk @ 2013-12-18 9:53 UTC (permalink / raw)
To: linux-bluetooth
This adds UUIDS set property fail test case due to only get
possibility.
---
android/android-tester.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/android/android-tester.c b/android/android-tester.c
index 92e6080..0025f20 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -596,6 +596,18 @@ static const struct generic_data bluetooth_getprop_bdname_success_test = {
.expected_property.len = 17
};
+static unsigned char setprop_uuids[] = { 0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00,
+ 0x00, 0x80, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00 };
+
+static const struct generic_data bluetooth_setprop_uuid_invalid_test = {
+ .expected_hal_callbacks = {ADAPTER_TEST_END},
+ .expected_adapter_status = BT_STATUS_FAIL,
+ .expected_property.type = BT_PROPERTY_UUIDS,
+ .expected_property.val = &setprop_uuids,
+ .expected_property.len = sizeof(setprop_uuids)
+};
+
static bt_callbacks_t bt_callbacks = {
.size = sizeof(bt_callbacks),
.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -973,6 +985,19 @@ static void test_getprop_bdname_success(const void *test_data)
check_expected_status(adapter_status);
}
+static void test_setprop_uuid_invalid(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ const bt_property_t *prop = &test->expected_property;
+ bt_status_t adapter_status;
+
+ init_test_conditions(data);
+
+ adapter_status = data->if_bluetooth->set_adapter_property(prop);
+ check_expected_status(adapter_status);
+}
+
#define test_bredrle(name, data, test_setup, test, test_teardown) \
do { \
struct test_data *user; \
@@ -1028,6 +1053,11 @@ int main(int argc, char *argv[])
setup_enabled_adapter,
test_getprop_bdname_success, teardown);
+ test_bredrle("Set UUID - Invalid",
+ &bluetooth_setprop_uuid_invalid_test,
+ setup_enabled_adapter,
+ test_setprop_uuid_invalid, teardown);
+
test_bredrle("Socket Init", NULL, setup_socket_interface,
test_dummy, teardown);
--
1.8.4.2
^ permalink raw reply related
* Re: [PATCH 2/4] emulator: Add data handler for l2cap connections
From: Johan Hedberg @ 2013-12-18 9:40 UTC (permalink / raw)
To: Marcin Kraglak; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <CABD6X-+XUgQDoJgetbEAjcCazQ396UGUG9qnKbfVHLQu0u+UfA@mail.gmail.com>
Hi Marcin,
On Wed, Dec 18, 2013, Marcin Kraglak wrote:
> On 18 December 2013 09:40, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> > Hi Marcin,
> >
> > On Tue, Dec 17, 2013, Marcin wrote:
> >> +void bthost_l2cap_set_data_handler(struct bthost *bthost,
> >> + bthost_l2cap_data_cb handler,
> >> + uint16_t psm)
> >> +{
> >> + bthost->read_data_psm = psm;
> >> + bthost->l2cap_data_cb = handler;
> >> +}
> >
> > The function is called just "data handler" but it applies to a more
> > specific read_data_psm. Should this perhaps be simply called data_psm?
> >
> >
> >> +typedef void (*bthost_l2cap_data_cb) (const void *data, uint16_t len);
> >> +
> >> +void bthost_l2cap_set_data_handler(struct bthost *bthost,
> >> + bthost_l2cap_data_cb handler,
> >> + uint16_t psm);
> >> +
> >
> > The problem with your approach is that it will not be usable for
> > protocols that have multiple channels to the same PSM such as AVDTP.
> > To cover this I'd suggest you to consider having something like the
> > following:
> >
> > typedef void (*bthost_l2cap_connect_cb) (uint16_t handle, uint16_t cid);
> >
> > void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
> > bthost_l2cap_connect_cb func);
> > /* the above could replace the existing bthost_set_server_psm */
> >
> > void bthost_connect_l2cap(struct bthost *bthost, uint16_t handle, uint16_t psm,
> > bthost_l2cap_connect_cb func);
> >
> > For sending and receiving data you wouldn't then need any new functions
> > but you could use the already existing bthost_add_cid_hook (for
> > receiving data) and bthost_send_cid (for sending data).
> >
> > Thoughts?
> >
> > Johan
>
> So first is to extend existing method bthost_add_l2cap_server and call
> back after connect, and create method bthost_connect_l2cap (instead of
> calling bthost_l2cap_req())?
Maybe you can skip adding this higher level bthost_connect_l2cap for now
and just use the existing bthost_l2cap_req. What I was thinking is that
this new feature is kind of above the existing lower-level access
functions but it's fine to try to keep using them as long as things
don't get too messy.
Johan
^ permalink raw reply
* Re: [PATCH 6/7] android/tester: Add RSSI set prop fail test case
From: Johan Hedberg @ 2013-12-18 8:57 UTC (permalink / raw)
To: Andrei Emeltchenko, Grzegorz Kolodziejczyk, linux-bluetooth
In-Reply-To: <20131218084600.GA26311@aemeltch-MOBL1>
Hi Andrei,
On Wed, Dec 18, 2013, Andrei Emeltchenko wrote:
> On Wed, Dec 18, 2013 at 10:00:34AM +0200, Johan Hedberg wrote:
> > Hi Grzegorz,
> >
> > On Tue, Dec 17, 2013, Grzegorz Kolodziejczyk wrote:
> > > +static int32_t setprop_remote_rssi = 0;
> > > +
> > > +static const struct generic_data bluetooth_setprop_remote_rssi_invalid_test = {
> > > + .expected_hal_callbacks = {ADAPTER_TEST_END},
>
> btw: do we need to have space after "{" ?
Yes, and before } too.
Johan
^ permalink raw reply
* Re: [PATCH 2/4] emulator: Add data handler for l2cap connections
From: Marcin Kraglak @ 2013-12-18 8:56 UTC (permalink / raw)
To: Marcin, linux-bluetooth@vger.kernel.org development
In-Reply-To: <20131218084056.GH12649@x220.p-661hnu-f1>
Hi Johan,
On 18 December 2013 09:40, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Marcin,
>
> On Tue, Dec 17, 2013, Marcin wrote:
>> +void bthost_l2cap_set_data_handler(struct bthost *bthost,
>> + bthost_l2cap_data_cb handler,
>> + uint16_t psm)
>> +{
>> + bthost->read_data_psm = psm;
>> + bthost->l2cap_data_cb = handler;
>> +}
>
> The function is called just "data handler" but it applies to a more
> specific read_data_psm. Should this perhaps be simply called data_psm?
>
>
>> +typedef void (*bthost_l2cap_data_cb) (const void *data, uint16_t len);
>> +
>> +void bthost_l2cap_set_data_handler(struct bthost *bthost,
>> + bthost_l2cap_data_cb handler,
>> + uint16_t psm);
>> +
>
> The problem with your approach is that it will not be usable for
> protocols that have multiple channels to the same PSM such as AVDTP.
> To cover this I'd suggest you to consider having something like the
> following:
>
> typedef void (*bthost_l2cap_connect_cb) (uint16_t handle, uint16_t cid);
>
> void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
> bthost_l2cap_connect_cb func);
> /* the above could replace the existing bthost_set_server_psm */
>
> void bthost_connect_l2cap(struct bthost *bthost, uint16_t handle, uint16_t psm,
> bthost_l2cap_connect_cb func);
>
> For sending and receiving data you wouldn't then need any new functions
> but you could use the already existing bthost_add_cid_hook (for
> receiving data) and bthost_send_cid (for sending data).
>
> Thoughts?
>
> Johan
So first is to extend existing method bthost_add_l2cap_server and call
back after connect, and create method bthost_connect_l2cap (instead of
calling bthost_l2cap_req())?
BR
Marcin
^ permalink raw reply
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