Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 5/5] android/tester: Add TYPE_OF_DEVICE get prop success test case
From: Grzegorz Kolodziejczyk @ 2013-12-30 14:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388413509-31265-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>

This adds TYPE_OF_DEVICE get property success test case.
---
 android/android-tester.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index c548e35..c89b97a 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -844,6 +844,17 @@ static const struct generic_data bluetooth_getprop_cod_success_test = {
 	.expected_property.len = sizeof(getprop_cod),
 };
 
+static bt_device_type_t getprop_tod = BT_DEVICE_DEVTYPE_BREDR;
+
+static const struct generic_data bluetooth_getprop_tod_success_test = {
+	.expected_hal_cb.adapter_properties_cb = getprop_success_cb,
+	.expected_cb_count = 1,
+	.expected_adapter_status = BT_STATUS_SUCCESS,
+	.expected_property.type = BT_PROPERTY_TYPE_OF_DEVICE,
+	.expected_property.val = &getprop_tod,
+	.expected_property.len = sizeof(getprop_tod),
+};
+
 static const struct generic_data bluetooth_discovery_start_success_test = {
 	.expected_hal_cb.discovery_state_changed_cb =
 						discovery_start_success_cb,
@@ -1232,6 +1243,19 @@ static void test_getprop_cod_success(const void *test_data)
 	check_expected_status(adapter_status);
 }
 
+static void test_getprop_tod_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_discovery_start_success(const void *test_data)
 {
 	struct test_data *data = tester_get_data();
@@ -1811,6 +1835,11 @@ int main(int argc, char *argv[])
 					setup_enabled_adapter,
 					test_getprop_cod_success, teardown);
 
+	test_bredrle("Bluetooth Get TYPE_OF_DEVICE - Success",
+					&bluetooth_getprop_tod_success_test,
+					setup_enabled_adapter,
+					test_getprop_tod_success, teardown);
+
 	test_bredrle("Bluetooth BREDR Discovery Start - Success",
 				&bluetooth_discovery_start_success_test,
 				setup_enabled_adapter,
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 4/5] android/tester: Add CLASS_OF_DEVICE get prop success test case
From: Grzegorz Kolodziejczyk @ 2013-12-30 14:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388413509-31265-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>

This adds CLASS_OF_DEVICE get property success test case.
---
 android/android-tester.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index 15f427c..c548e35 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -833,6 +833,17 @@ static const struct generic_data
 	.set_property.len = sizeof(setprop_bonded_devices),
 };
 
+static uint32_t getprop_cod = 0;
+
+static const struct generic_data bluetooth_getprop_cod_success_test = {
+	.expected_hal_cb.adapter_properties_cb = getprop_success_cb,
+	.expected_cb_count = 1,
+	.expected_adapter_status = BT_STATUS_SUCCESS,
+	.expected_property.type = BT_PROPERTY_CLASS_OF_DEVICE,
+	.expected_property.val = &getprop_cod,
+	.expected_property.len = sizeof(getprop_cod),
+};
+
 static const struct generic_data bluetooth_discovery_start_success_test = {
 	.expected_hal_cb.discovery_state_changed_cb =
 						discovery_start_success_cb,
@@ -1208,6 +1219,19 @@ static void test_setprop_bonded_devices_invalid(const void *test_data)
 	check_expected_status(adapter_status);
 }
 
+static void test_getprop_cod_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_discovery_start_success(const void *test_data)
 {
 	struct test_data *data = tester_get_data();
@@ -1782,6 +1806,11 @@ int main(int argc, char *argv[])
 				setup_enabled_adapter,
 				test_setprop_bonded_devices_invalid, teardown);
 
+	test_bredrle("Bluetooth Get CLASS_OF_DEVICE - Success",
+					&bluetooth_getprop_cod_success_test,
+					setup_enabled_adapter,
+					test_getprop_cod_success, teardown);
+
 	test_bredrle("Bluetooth BREDR Discovery Start - Success",
 				&bluetooth_discovery_start_success_test,
 				setup_enabled_adapter,
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 3/5] android/tester: Add BONDED_DEVICES set prop fail test case
From: Grzegorz Kolodziejczyk @ 2013-12-30 14:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388413509-31265-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>

This adds BONDED_DEVICES 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 358c58b..15f427c 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -821,6 +821,18 @@ static const struct generic_data
 	.expected_property.len = sizeof(setprop_scanmode_connectable),
 };
 
+static bt_bdaddr_t setprop_bonded_devices = {
+	.address = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 },
+};
+
+static const struct generic_data
+			bluetooth_setprop_bonded_devices_invalid_test = {
+	.expected_adapter_status = BT_STATUS_FAIL,
+	.set_property.type = BT_PROPERTY_ADAPTER_BONDED_DEVICES,
+	.set_property.val = &setprop_bonded_devices,
+	.set_property.len = sizeof(setprop_bonded_devices),
+};
+
 static const struct generic_data bluetooth_discovery_start_success_test = {
 	.expected_hal_cb.discovery_state_changed_cb =
 						discovery_start_success_cb,
@@ -1183,6 +1195,19 @@ static void test_setprop_scanmode_connectable_success(const void *test_data)
 	check_expected_status(adapter_status);
 }
 
+static void test_setprop_bonded_devices_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_discovery_start_success(const void *test_data)
 {
 	struct test_data *data = tester_get_data();
@@ -1752,6 +1777,11 @@ int main(int argc, char *argv[])
 			setup_enabled_adapter,
 			test_setprop_scanmode_connectable_success, teardown);
 
+	test_bredrle("Bluetooth Set BONDED_DEVICES - Invalid",
+				&bluetooth_setprop_bonded_devices_invalid_test,
+				setup_enabled_adapter,
+				test_setprop_bonded_devices_invalid, teardown);
+
 	test_bredrle("Bluetooth BREDR Discovery Start - Success",
 				&bluetooth_discovery_start_success_test,
 				setup_enabled_adapter,
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 2/5] android/tester: Add SCAN_MODE=CONNECTABLE set prop success test case
From: Grzegorz Kolodziejczyk @ 2013-12-30 14:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388413509-31265-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>

This adds SCAN_MODE property set to CONNECTABLE - success test case.
---
 android/android-tester.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index d7ebc02..358c58b 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -809,6 +809,18 @@ static const struct generic_data bluetooth_setprop_bdaddr_invalid_test = {
 	.set_property.len = sizeof(setprop_bdaddr),
 };
 
+static bt_scan_mode_t setprop_scanmode_connectable = BT_SCAN_MODE_CONNECTABLE;
+
+static const struct generic_data
+			bluetooth_setprop_scanmode_connectable_success_test = {
+	.expected_hal_cb.adapter_properties_cb = getprop_success_cb,
+	.expected_cb_count = 1,
+	.expected_adapter_status = BT_STATUS_SUCCESS,
+	.expected_property.type = BT_PROPERTY_ADAPTER_SCAN_MODE,
+	.expected_property.val = &setprop_scanmode_connectable,
+	.expected_property.len = sizeof(setprop_scanmode_connectable),
+};
+
 static const struct generic_data bluetooth_discovery_start_success_test = {
 	.expected_hal_cb.discovery_state_changed_cb =
 						discovery_start_success_cb,
@@ -1158,6 +1170,19 @@ static void test_setprop_bdaddr_invalid(const void *test_data)
 	check_expected_status(adapter_status);
 }
 
+static void test_setprop_scanmode_connectable_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_discovery_start_success(const void *test_data)
 {
 	struct test_data *data = tester_get_data();
@@ -1722,6 +1747,11 @@ int main(int argc, char *argv[])
 					setup_enabled_adapter,
 					test_setprop_bdaddr_invalid, teardown);
 
+	test_bredrle("Bluetooth Set SCAN_MODE CONNECTABLE - Success",
+			&bluetooth_setprop_scanmode_connectable_success_test,
+			setup_enabled_adapter,
+			test_setprop_scanmode_connectable_success, teardown);
+
 	test_bredrle("Bluetooth BREDR Discovery Start - Success",
 				&bluetooth_discovery_start_success_test,
 				setup_enabled_adapter,
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 1/5] android/tester: Add BDADDR set prop fail test case
From: Grzegorz Kolodziejczyk @ 2013-12-30 14:25 UTC (permalink / raw)
  To: linux-bluetooth

This adds BDADDR set property fail test case due to only get
possibility.
---
 android/android-tester.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index ee23fff..d7ebc02 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -798,6 +798,17 @@ static const struct generic_data
 	.set_property.len = sizeof(setprop_remote_service),
 };
 
+static bt_bdaddr_t setprop_bdaddr = {
+	.address = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 },
+};
+
+static const struct generic_data bluetooth_setprop_bdaddr_invalid_test = {
+	.expected_adapter_status = BT_STATUS_FAIL,
+	.set_property.type = BT_PROPERTY_BDADDR,
+	.set_property.val = &setprop_bdaddr,
+	.set_property.len = sizeof(setprop_bdaddr),
+};
+
 static const struct generic_data bluetooth_discovery_start_success_test = {
 	.expected_hal_cb.discovery_state_changed_cb =
 						discovery_start_success_cb,
@@ -1134,6 +1145,19 @@ static void test_setprop_service_record_invalid(const void *test_data)
 	check_expected_status(adapter_status);
 }
 
+static void test_setprop_bdaddr_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_discovery_start_success(const void *test_data)
 {
 	struct test_data *data = tester_get_data();
@@ -1693,6 +1717,11 @@ int main(int argc, char *argv[])
 				setup_enabled_adapter,
 				test_setprop_service_record_invalid, teardown);
 
+	test_bredrle("Bluetooth Set BDADDR - Invalid",
+					&bluetooth_setprop_bdaddr_invalid_test,
+					setup_enabled_adapter,
+					test_setprop_bdaddr_invalid, teardown);
+
 	test_bredrle("Bluetooth BREDR Discovery Start - Success",
 				&bluetooth_discovery_start_success_test,
 				setup_enabled_adapter,
-- 
1.8.4.2


^ permalink raw reply related

* Re: [PATCH v2] Bluetooth: Add hci_h4p driver
From: Pali Rohár @ 2013-12-30 14:04 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Pavel Machek, Marcel Holtmann,
	Ивайло Димитров,
	Gustavo F. Padovan, Johan Hedberg, linux-kernel,
	linux-bluetooth@vger.kernel.org development, Ville Tervo
In-Reply-To: <20131230131942.GA13816@earth.universe>

[-- Attachment #1: Type: Text/Plain, Size: 1029 bytes --]

On Monday 30 December 2013 14:19:44 Sebastian Reichel wrote:
> Hi,
> 
> On Mon, Dec 30, 2013 at 01:13:50PM +0100, Pavel Machek wrote:
> > [...]
> > 
> > Well, I can rename config option, but renaming the module
> > would break existing userland, no?
> 
> Why is the userland depending on the module name?
> 

grep told me that hci_h4p is used only in these (text) files:

* /etc/modprobe.d/maemo.conf
   alias platform:hci_h4p hci_h4p

So this is not needed if kernel driver contains correct 
MODULE_ALIAS macro.

* /etc/event.d/bluetooth-sysinfo
   echo ... > /sys/devices/platform/hci_h4p/bdaddr

This upstart script contains code for reading mac address from 
special sysinfod daemon and setting it to kernel driver. Once 
there will be uniform way for setting mac address that script can 
be changed.

So I think that renaming module will not break anything if kernel 
module will get (somehow) correct mac address (or if random one 
is acceptable).

-- 
Pali Rohár
pali.rohar@gmail.com


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH v2] Bluetooth: Add hci_h4p driver
From: Pali Rohár @ 2013-12-30 13:54 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Marcel Holtmann,
	Ивайло Димитров,
	Gustavo F. Padovan, Johan Hedberg, linux-kernel,
	linux-bluetooth@vger.kernel.org development, Ville Tervo,
	Sebastian Reichel
In-Reply-To: <20131230121350.GB31236@amd.pavel.ucw.cz>

[-- Attachment #1: Type: Text/Plain, Size: 1015 bytes --]

On Monday 30 December 2013 13:13:50 Pavel Machek wrote:
> > > +
> > > +	if (not_valid) {
> > > +		dev_info(info->dev, "Valid bluetooth address not 
found,
> > > setting some random\n"); +		/* When address is not valid,
> > > use some random but Nokia MAC */ +		memcpy(info-
>bd_addr,
> > > nokia_oui, 3);
> > > +		get_random_bytes(info->bd_addr + 3, 3);
> > > +	}
> > 
> > This behavior is extremely dangerous. I would rather have
> > the device init or powering on the device fail instead of
> > making up a number that might clash with a real Nokia
> > device.
> 
> Perhaps people can donate bt addresses from their
> no-longer-functional bluetooth devices and we can select from
> such pool here? ;-).
> 
> Is there some experimental range we can allocate from?
> 

Wifi driver wl1251 (it is already in mainline) is doing same: 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/ti/wl1251/main.c#n1431

-- 
Pali Rohár
pali.rohar@gmail.com


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [RFC BlueZ 3/9] android: Add audio open command/response struct
From: Luiz Augusto von Dentz @ 2013-12-30 13:36 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1532451.hxPrzbXmr3@athlon>

Hi Szymon,

On Mon, Dec 30, 2013 at 3:27 PM, Szymon Janc <szymon.janc@gmail.com> wrote:
> Hi Luiz,
>
> On Monday 30 December 2013 14:34:09 Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> This adds the definitions to audio open command and response.
>> ---
>>  android/a2dp.c            |  9 +++++++++
>>  android/audio-ipc-api.txt |  2 +-
>>  android/hal-msg.h         | 18 ++++++++++++++++++
>>  android/ipc.c             |  5 ++++-
>>  4 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/android/a2dp.c b/android/a2dp.c
>> index 63f1f58..5cb01f7 100644
>> --- a/android/a2dp.c
>> +++ b/android/a2dp.c
>> @@ -352,7 +352,16 @@ static sdp_record_t *a2dp_record(void)
>>       return record;
>>  }
>>
>> +static void bt_audio_open(const void *buf, uint16_t len)
>> +{
>> +     DBG("Not Implemented");
>> +
>> +     ipc_send_rsp(HAL_SERVICE_ID_AUDIO, AUDIO_OP_OPEN, HAL_STATUS_FAILED);
>> +}
>> +
>>  static const struct ipc_handler audio_handlers[] = {
>> +     /* AUDIO_OP_OPEN */
>> +     { bt_audio_open, true, sizeof(struct audio_cmd_open) },
>>  };
>>
>>  bool bt_a2dp_register(const bdaddr_t *addr)
>> diff --git a/android/audio-ipc-api.txt b/android/audio-ipc-api.txt
>> index 1c42800..37a1569 100644
>> --- a/android/audio-ipc-api.txt
>> +++ b/android/audio-ipc-api.txt
>> @@ -49,9 +49,9 @@ Identifier: "audio" (BT_AUDIO_ID)
>>
>>               Command parameters: Service UUID (16 octets)
>>                                   Codec ID (1 octet)
>> +                                 Number of codec presets (1 octet)
>>                                   Codec capabilities length (1 octet)
>>                                   Codec capabilities (variable)
>> -                                 Number of codec presets (1 octet)
>>                                   Codec preset # length (1 octet)
>>                                   Codec preset # configuration (variable)
>>                                   ...
>> diff --git a/android/hal-msg.h b/android/hal-msg.h
>> index 1afb1bc..4b52e5e 100644
>> --- a/android/hal-msg.h
>> +++ b/android/hal-msg.h
>> @@ -567,3 +567,21 @@ struct hal_ev_a2dp_audio_state {
>>       uint8_t state;
>>       uint8_t bdaddr[6];
>>  } __attribute__((packed));
>> +
>> +#define AUDIO_OP_OPEN                                0x01
>> +struct audio_cmd_open {
>> +     uint16_t uuid;
>> +     uint8_t codec;
>> +     uint8_t presets;
>> +     uint8_t len;
>> +     uint8_t data[0];
>
> Maybe this could be
> struct audio_preset[0];
> ? (if that would make code cleaner)

Indeed, will fix it.

Luiz Augusto von Dentz

^ permalink raw reply

* Re: [RFC BlueZ 1/9] android: Add initial code for audio IPC
From: Luiz Augusto von Dentz @ 2013-12-30 13:34 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1919373.99Va0DTt5V@athlon>

Hi Szymon,

On Mon, Dec 30, 2013 at 3:26 PM, Szymon Janc <szymon.janc@gmail.com> wrote:
> Hi Luiz,
>
> On Monday 30 December 2013 14:34:07 Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> This add initial code for listen and accept connections on the abstract
>> socket defined for the audio IPC.
>> ---
>>  android/hal-msg.h |  1 +
>>  android/ipc.c     | 53
>> +++++++++++++++++++++++++++++++++++++++++++++++------ android/ipc.h     |
>> 3 +++
>>  3 files changed, 51 insertions(+), 6 deletions(-)
>>
>> diff --git a/android/hal-msg.h b/android/hal-msg.h
>> index c351501..b14eced 100644
>> --- a/android/hal-msg.h
>> +++ b/android/hal-msg.h
>> @@ -24,6 +24,7 @@
>>  #define BLUEZ_HAL_MTU 1024
>>
>>  static const char BLUEZ_HAL_SK_PATH[] = "\0bluez_hal_socket";
>> +static const char BLUEZ_AUDIO_SK_PATH[] = "\0bluez_audio_socket";
>>
>>  struct hal_hdr {
>>       uint8_t  service_id;
>> diff --git a/android/ipc.c b/android/ipc.c
>> index 9e8ccc3..6cdbf60 100644
>> --- a/android/ipc.c
>> +++ b/android/ipc.c
>> @@ -49,6 +49,7 @@ static struct service_handler services[HAL_SERVICE_ID_MAX
>> + 1];
>>
>>  static GIOChannel *cmd_io = NULL;
>>  static GIOChannel *notif_io = NULL;
>> +static GIOChannel *audio_io = NULL;
>>
>>  static void ipc_handle_msg(const void *buf, ssize_t len)
>>  {
>> @@ -145,7 +146,8 @@ static gboolean notif_watch_cb(GIOChannel *io,
>> GIOCondition cond, return FALSE;
>>  }
>>
>> -static GIOChannel *connect_hal(GIOFunc connect_cb)
>> +static GIOChannel *connect_hal(const char *path, size_t size,
>> +                                                     GIOFunc connect_cb)
>>  {
>>       struct sockaddr_un addr;
>>       GIOCondition cond;
>> @@ -167,11 +169,11 @@ static GIOChannel *connect_hal(GIOFunc connect_cb)
>>       memset(&addr, 0, sizeof(addr));
>>       addr.sun_family = AF_UNIX;
>>
>> -     memcpy(addr.sun_path, BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH));
>> +     memcpy(addr.sun_path, path, size);
>>
>>       if (connect(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
>> -             error("IPC: failed to connect HAL socket: %d (%s)", errno,
>> -                                                     strerror(errno));
>> +             error("IPC: failed to connect HAL socket %s: %d (%s)", &path[1],
>> +                                                     errno, strerror(errno));
>>               g_io_channel_unref(io);
>>               return NULL;
>>       }
>> @@ -218,7 +220,8 @@ static gboolean cmd_connect_cb(GIOChannel *io,
>> GIOCondition cond, return FALSE;
>>       }
>>
>> -     notif_io = connect_hal(notif_connect_cb);
>> +     notif_io = connect_hal(BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH),
>> +                                                     notif_connect_cb);
>>       if (!notif_io)
>>               raise(SIGTERM);
>>
>> @@ -227,7 +230,8 @@ static gboolean cmd_connect_cb(GIOChannel *io,
>> GIOCondition cond,
>>
>>  void ipc_init(void)
>>  {
>> -     cmd_io = connect_hal(cmd_connect_cb);
>> +     cmd_io = connect_hal(BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH),
>> +                                                     cmd_connect_cb);
>>       if (!cmd_io)
>>               raise(SIGTERM);
>>  }
>> @@ -338,3 +342,40 @@ void ipc_unregister(uint8_t service)
>>       services[service].handler = NULL;
>>       services[service].size = 0;
>>  }
>> +
>> +static gboolean audio_connect_cb(GIOChannel *io, GIOCondition cond,
>> +                                                     gpointer user_data)
>> +{
>> +     DBG("");
>> +
>> +     if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
>> +             error("Audio IPC: socket connect failed, terminating");
>
> This message is misleading since we should raise(SIGTERM) to terminate.
>
> But I wonder if we should really terminate on audio IPC failure...
> That is failing on IPC error is OK (since that is a bug in our code anyway),
> but on not being able to connect we could simply fail to register a2dp HAL.
> Same goes with socket being closed - in such case we could fail any request to
> a2dp HAL until connection is recovered.

Yep, the terminating part of the message is misleading it should not
really exit just fail to initialize a2dp.

>> +             audio_ipc_cleanup();
>> +             return FALSE;
>> +     }
>> +
>> +     cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
>> +
>> +     g_io_add_watch(audio_io, cond, cmd_watch_cb, NULL);
>> +
>> +     info("Audio IPC: successfully connected");
>> +
>> +     return FALSE;
>> +}
>> +
>> +void audio_ipc_init(void)
>> +{
>> +     audio_io = connect_hal(BLUEZ_AUDIO_SK_PATH, sizeof(BLUEZ_AUDIO_SK_PATH),
>> +                                                     audio_connect_cb);
>> +     if (!cmd_io)
>
> audio_io here?

Yep, that is a bug.

-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [RFC BlueZ 3/9] android: Add audio open command/response struct
From: Szymon Janc @ 2013-12-30 13:27 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1388406855-8809-3-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Monday 30 December 2013 14:34:09 Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This adds the definitions to audio open command and response.
> ---
>  android/a2dp.c            |  9 +++++++++
>  android/audio-ipc-api.txt |  2 +-
>  android/hal-msg.h         | 18 ++++++++++++++++++
>  android/ipc.c             |  5 ++++-
>  4 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/android/a2dp.c b/android/a2dp.c
> index 63f1f58..5cb01f7 100644
> --- a/android/a2dp.c
> +++ b/android/a2dp.c
> @@ -352,7 +352,16 @@ static sdp_record_t *a2dp_record(void)
>  	return record;
>  }
> 
> +static void bt_audio_open(const void *buf, uint16_t len)
> +{
> +	DBG("Not Implemented");
> +
> +	ipc_send_rsp(HAL_SERVICE_ID_AUDIO, AUDIO_OP_OPEN, HAL_STATUS_FAILED);
> +}
> +
>  static const struct ipc_handler audio_handlers[] = {
> +	/* AUDIO_OP_OPEN */
> +	{ bt_audio_open, true, sizeof(struct audio_cmd_open) },
>  };
> 
>  bool bt_a2dp_register(const bdaddr_t *addr)
> diff --git a/android/audio-ipc-api.txt b/android/audio-ipc-api.txt
> index 1c42800..37a1569 100644
> --- a/android/audio-ipc-api.txt
> +++ b/android/audio-ipc-api.txt
> @@ -49,9 +49,9 @@ Identifier: "audio" (BT_AUDIO_ID)
> 
>  		Command parameters: Service UUID (16 octets)
>  				    Codec ID (1 octet)
> +				    Number of codec presets (1 octet)
>  				    Codec capabilities length (1 octet)
>  				    Codec capabilities (variable)
> -				    Number of codec presets (1 octet)
>  				    Codec preset # length (1 octet)
>  				    Codec preset # configuration (variable)
>  				    ...
> diff --git a/android/hal-msg.h b/android/hal-msg.h
> index 1afb1bc..4b52e5e 100644
> --- a/android/hal-msg.h
> +++ b/android/hal-msg.h
> @@ -567,3 +567,21 @@ struct hal_ev_a2dp_audio_state {
>  	uint8_t state;
>  	uint8_t bdaddr[6];
>  } __attribute__((packed));
> +
> +#define AUDIO_OP_OPEN				0x01
> +struct audio_cmd_open {
> +	uint16_t uuid;
> +	uint8_t codec;
> +	uint8_t presets;
> +	uint8_t len;
> +	uint8_t data[0];

Maybe this could be
struct audio_preset[0];
? (if that would make code cleaner)

> +} __attribute__((packed));
> +
> +struct audio_preset {
> +	uint8_t len;
> +	uint8_t data[0];
> +} __attribute__((packed));
> +
> +struct audio_rsp_open {
> +	uint8_t id;
> +} __attribute__((packed));
> diff --git a/android/ipc.c b/android/ipc.c
> index 6cdbf60..bb16553 100644
> --- a/android/ipc.c
> +++ b/android/ipc.c
> @@ -301,7 +301,10 @@ void ipc_send_rsp(uint8_t service_id, uint8_t opcode,
> uint8_t status) struct hal_status s;
>  	int sk;
> 
> -	sk = g_io_channel_unix_get_fd(cmd_io);
> +	if (service_id == HAL_SERVICE_ID_AUDIO)
> +		sk = g_io_channel_unix_get_fd(audio_io);
> +	else
> +		sk = g_io_channel_unix_get_fd(cmd_io);
> 
>  	if (status == HAL_STATUS_SUCCESS) {
>  		ipc_send(sk, service_id, opcode, 0, NULL, -1);

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [RFC BlueZ 1/9] android: Add initial code for audio IPC
From: Szymon Janc @ 2013-12-30 13:26 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1388406855-8809-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Monday 30 December 2013 14:34:07 Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This add initial code for listen and accept connections on the abstract
> socket defined for the audio IPC.
> ---
>  android/hal-msg.h |  1 +
>  android/ipc.c     | 53
> +++++++++++++++++++++++++++++++++++++++++++++++------ android/ipc.h     | 
> 3 +++
>  3 files changed, 51 insertions(+), 6 deletions(-)
> 
> diff --git a/android/hal-msg.h b/android/hal-msg.h
> index c351501..b14eced 100644
> --- a/android/hal-msg.h
> +++ b/android/hal-msg.h
> @@ -24,6 +24,7 @@
>  #define BLUEZ_HAL_MTU 1024
> 
>  static const char BLUEZ_HAL_SK_PATH[] = "\0bluez_hal_socket";
> +static const char BLUEZ_AUDIO_SK_PATH[] = "\0bluez_audio_socket";
> 
>  struct hal_hdr {
>  	uint8_t  service_id;
> diff --git a/android/ipc.c b/android/ipc.c
> index 9e8ccc3..6cdbf60 100644
> --- a/android/ipc.c
> +++ b/android/ipc.c
> @@ -49,6 +49,7 @@ static struct service_handler services[HAL_SERVICE_ID_MAX
> + 1];
> 
>  static GIOChannel *cmd_io = NULL;
>  static GIOChannel *notif_io = NULL;
> +static GIOChannel *audio_io = NULL;
> 
>  static void ipc_handle_msg(const void *buf, ssize_t len)
>  {
> @@ -145,7 +146,8 @@ static gboolean notif_watch_cb(GIOChannel *io,
> GIOCondition cond, return FALSE;
>  }
> 
> -static GIOChannel *connect_hal(GIOFunc connect_cb)
> +static GIOChannel *connect_hal(const char *path, size_t size,
> +							GIOFunc connect_cb)
>  {
>  	struct sockaddr_un addr;
>  	GIOCondition cond;
> @@ -167,11 +169,11 @@ static GIOChannel *connect_hal(GIOFunc connect_cb)
>  	memset(&addr, 0, sizeof(addr));
>  	addr.sun_family = AF_UNIX;
> 
> -	memcpy(addr.sun_path, BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH));
> +	memcpy(addr.sun_path, path, size);
> 
>  	if (connect(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
> -		error("IPC: failed to connect HAL socket: %d (%s)", errno,
> -							strerror(errno));
> +		error("IPC: failed to connect HAL socket %s: %d (%s)", &path[1],
> +							errno, strerror(errno));
>  		g_io_channel_unref(io);
>  		return NULL;
>  	}
> @@ -218,7 +220,8 @@ static gboolean cmd_connect_cb(GIOChannel *io,
> GIOCondition cond, return FALSE;
>  	}
> 
> -	notif_io = connect_hal(notif_connect_cb);
> +	notif_io = connect_hal(BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH),
> +							notif_connect_cb);
>  	if (!notif_io)
>  		raise(SIGTERM);
> 
> @@ -227,7 +230,8 @@ static gboolean cmd_connect_cb(GIOChannel *io,
> GIOCondition cond,
> 
>  void ipc_init(void)
>  {
> -	cmd_io = connect_hal(cmd_connect_cb);
> +	cmd_io = connect_hal(BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH),
> +							cmd_connect_cb);
>  	if (!cmd_io)
>  		raise(SIGTERM);
>  }
> @@ -338,3 +342,40 @@ void ipc_unregister(uint8_t service)
>  	services[service].handler = NULL;
>  	services[service].size = 0;
>  }
> +
> +static gboolean audio_connect_cb(GIOChannel *io, GIOCondition cond,
> +							gpointer user_data)
> +{
> +	DBG("");
> +
> +	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> +		error("Audio IPC: socket connect failed, terminating");

This message is misleading since we should raise(SIGTERM) to terminate.

But I wonder if we should really terminate on audio IPC failure...
That is failing on IPC error is OK (since that is a bug in our code anyway), 
but on not being able to connect we could simply fail to register a2dp HAL.
Same goes with socket being closed - in such case we could fail any request to
a2dp HAL until connection is recovered.

> +		audio_ipc_cleanup();
> +		return FALSE;
> +	}
> +
> +	cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> +
> +	g_io_add_watch(audio_io, cond, cmd_watch_cb, NULL);
> +
> +	info("Audio IPC: successfully connected");
> +
> +	return FALSE;
> +}
> +
> +void audio_ipc_init(void)
> +{
> +	audio_io = connect_hal(BLUEZ_AUDIO_SK_PATH, sizeof(BLUEZ_AUDIO_SK_PATH),
> +							audio_connect_cb);
> +	if (!cmd_io)

audio_io here?

> +		raise(SIGTERM);
> +}
> +
> +void audio_ipc_cleanup(void)
> +{
> +	if (audio_io) {
> +		g_io_channel_shutdown(audio_io, TRUE, NULL);
> +		g_io_channel_unref(audio_io);
> +		audio_io = NULL;
> +	}
> +}
> diff --git a/android/ipc.h b/android/ipc.h
> index 6cd102b..8e92811 100644
> --- a/android/ipc.h
> +++ b/android/ipc.h
> @@ -37,3 +37,6 @@ void ipc_send_notif(uint8_t service_id, uint8_t opcode, 
> uint16_t len, void ipc_register(uint8_t service, const struct ipc_handler
> *handlers, uint8_t size);
>  void ipc_unregister(uint8_t service);
> +
> +void audio_ipc_init(void);
> +void audio_ipc_cleanup(void);

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCH v2] Bluetooth: Add hci_h4p driver
From: Sebastian Reichel @ 2013-12-30 13:19 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Marcel Holtmann, Pali Rohár,
	Ивайло Димитров,
	Gustavo F. Padovan, Johan Hedberg, linux-kernel,
	linux-bluetooth@vger.kernel.org development, Ville Tervo
In-Reply-To: <20131230121350.GB31236@amd.pavel.ucw.cz>

[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]

Hi,

On Mon, Dec 30, 2013 at 01:13:50PM +0100, Pavel Machek wrote:
> [...]
>
> Well, I can rename config option, but renaming the module would break
> existing userland, no?

Why is the userland depending on the module name?

> >  Can we also make this just depend on some device tree information
> > and not on a specific architecture. I know that this driver is
> > pretty much OMAP specific, but if we want this upstream, we should
> > at least try to make it more generic.
> 
> Nokia N900 is certainly moving towards device tree, but we are not
> ready, yet...

Tony plans to remove OMAP3 boardcode (incl. omap3-rx51) in 3.14.

> > > [...]
> > >
> > Please do not introduce public includes for a driver. This
> > should be all confined to the driver itself or if it platform
> > data, it should go into the place for platform data.
> 
> (Could you insert newlines after 80 or so characters?)
> 
> Where would you like platform_data definition to go? That indeed is
> for platform data, and quick grep shows drivers normally do public
> header files for that.

Probably it can simply be removed, because it's not useful in 3.14?

> [...]

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [RFC 6/6] android/audio: Add listener thread on the Audio HAL socket
From: Luiz Augusto von Dentz @ 2013-12-30 13:07 UTC (permalink / raw)
  To: Lukasz Rymanowski; +Cc: linux-bluetooth@vger.kernel.org, Johan Hedberg
In-Reply-To: <CAMudyAv386wWW8YQKY9u_6SV6ZaSijwxm=qud0rk6sTtKsgRog@mail.gmail.com>

Hi Lukasz,

On Mon, Dec 30, 2013 at 1:40 PM, Lukasz Rymanowski
<lukasz.rymanowski@tieto.com> wrote:
> Hi Luiz,
>
> On 30 December 2013 12:31, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> Hi Lukasz,
>>
>> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
>> <lukasz.rymanowski@tieto.com> wrote:
>>> This patch add thread which is reponsible for listen on audio HAL
>>> socket, register a2dp endpoint(s) and maintain socket.
>>> When bluetooth daemon goes down, HAL audio plugin starts to listen on Audio HAL
>>> socket again.
>>>
>>> ---
>>>  android/Makefile.am |   2 +
>>>  android/hal-audio.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  android/hal-audio.h |  18 +++++++
>>>  3 files changed, 165 insertions(+)
>>>  create mode 100644 android/hal-audio.h
>>>
>>> diff --git a/android/Makefile.am b/android/Makefile.am
>>> index eaf39bd..bd90c13 100644
>>> --- a/android/Makefile.am
>>> +++ b/android/Makefile.am
>>> @@ -112,6 +112,8 @@ android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>>>
>>>  android_libaudio_internal_la_CFLAGS = -I$(srcdir)/android
>>>
>>> +android_libaudio_internal_la_LDFLAGS = -pthread
>>> +
>>>  endif
>>>
>>>  EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>> index 011a699..0e3bc70 100644
>>> --- a/android/hal-audio.c
>>> +++ b/android/hal-audio.c
>>> @@ -16,18 +16,30 @@
>>>   */
>>>
>>>  #include <errno.h>
>>> +#include <pthread.h>
>>> +#include <poll.h>
>>>  #include <stdio.h>
>>>  #include <stdlib.h>
>>>  #include <string.h>
>>> +#include <sys/socket.h>
>>> +#include <sys/un.h>
>>> +#include <unistd.h>
>>>
>>>  #include <hardware/audio.h>
>>>  #include <hardware/hardware.h>
>>>
>>> +#include "hal-audio.h"
>>>  #include "hal-log.h"
>>>
>>>  struct a2dp_audio_dev {
>>>         struct audio_hw_device dev;
>>>         struct a2dp_stream_out *stream_out;
>>> +
>>> +       pthread_t bt_watcher;
>>> +       pthread_mutex_t hal_sk_mutex;
>>> +       pthread_cond_t bt_watcher_cond;
>>> +
>>> +       int hal_sk;
>>>  };
>>>
>>>  struct a2dp_stream_out {
>>> @@ -384,15 +396,135 @@ static int audio_dump(const audio_hw_device_t *device, int fd)
>>>
>>>  static int audio_close(hw_device_t *device)
>>>  {
>>> +       struct audio_hw_device *dev = (struct audio_hw_device *)device;
>>> +       struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>>> +

Hmm, Im afraid these are not the same pointers as you do  *device =
&a2dp_dev->dev.common; so this will probably cause invalid accesses.

>>>         DBG("");
>>> +
>>> +       if (a2dp_dev) {
>>> +               /* TODO: We could try to unregister Endpoint here */
>>> +               shutdown(a2dp_dev->hal_sk, 2);
>>> +
>>> +               pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
>>> +               pthread_cond_signal(&a2dp_dev->bt_watcher_cond);
>>> +               pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
>>> +
>>> +               (void) pthread_join(a2dp_dev->bt_watcher, NULL);
>>> +               free(a2dp_dev);
>>> +       }
>>> +
>>>         free(device);
>>>         return 0;
>>>  }
>>>
>>> +static int wait_for_client(void)
>>> +{
>>> +       struct sockaddr_un addr;
>>> +       int err;
>>> +       int sk;
>>> +       int new_sk;
>>> +
>>> +       DBG("");
>>> +
>>> +       sk = socket(PF_LOCAL, SOCK_SEQPACKET, 0);
>>> +       if (sk < 0) {
>>> +               err = errno;
>>> +               error("Failed to create socket: %d (%s)", err, strerror(err));
>>> +               return -1;
>>> +       }
>>> +
>>> +       memset(&addr, 0, sizeof(addr));
>>> +       addr.sun_family = AF_UNIX;
>>> +
>>> +       memcpy(addr.sun_path, BLUEZ_AUDIO_HAL_SK_PATH,
>>> +                                       sizeof(BLUEZ_AUDIO_HAL_SK_PATH));
>>> +
>>> +       if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
>>> +               err = errno;
>>> +               error("Failed to bind socket: %d (%s)", err, strerror(err));
>>> +               goto error;
>>> +       }
>>> +
>>> +       if (listen(sk, 1) < 0) {
>>> +               err = errno;
>>> +               error("Failed to bind socket: %d (%s)", err, strerror(err));
>>> +               goto error;
>>> +       }
>>> +
>>> +       new_sk = accept(sk, NULL, NULL);
>>> +       if (new_sk < 0) {
>>> +               err = errno;
>>> +               error("Failed to accept socket: %d (%s)", err, strerror(err));
>>> +               goto error;
>>> +       }
>>> +
>>> +       close(sk);
>>> +       return new_sk;
>>> +
>>> +error:
>>> +       close(sk);
>>> +       return -1;
>>> +}
>>> +
>>> +static void *bluetoothd_watcher(void *data)
>>> +{
>>> +       int err;
>>> +       struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)data;
>>> +       struct pollfd pfd;
>>> +       struct timeval now;
>>> +       struct timespec timeout;
>>> +
>>> +       DBG("");
>>> +
>>> +       while (1) {
>>> +               a2dp_dev->hal_sk = wait_for_client();
>>> +               if (a2dp_dev->hal_sk < 0) {
>>> +                       error("Failed to create listening socket");
>>> +                       continue;
>>> +               }
>>> +
>>> +               DBG("Audio IPC: Connected");
>>> +
>>> +               /* TODO: Register ENDPOINT here */
>>> +
>>> +               memset(&pfd, 0, sizeof(pfd));
>>> +               pfd.fd = a2dp_dev->hal_sk;
>>> +               pfd.events = POLLHUP | POLLERR | POLLNVAL;
>>> +
>>> +               pthread_mutex_lock(&a2dp_dev->hal_sk_mutex);
>>> +
>>> +               /* Check if socket is still alive */
>>> +               err = poll(&pfd, 1, -1);
>>> +               if (pfd.revents & (POLLHUP | POLLERR | POLLNVAL)) {
>>> +                       info("Audio HAL: Socket closed");
>>> +                       a2dp_dev->hal_sk = -1;
>>> +               }
>>> +
>>> +               /* Maybe audio system is closing and audio_close() has been called? */
>>> +               gettimeofday(&now, NULL);
>>> +               timeout.tv_sec = now.tv_sec;
>>> +               timeout.tv_nsec = now.tv_usec * 1000;
>>> +               timeout.tv_sec += 1;
>>> +
>>> +               err = pthread_cond_timedwait(&a2dp_dev->bt_watcher_cond,
>>> +                                       &a2dp_dev->hal_sk_mutex, &timeout);
>>> +
>>> +               pthread_mutex_unlock(&a2dp_dev->hal_sk_mutex);
>>> +               if (err != ETIMEDOUT)
>>> +                       /* Seems that device has been closed */
>>> +                       break;
>>> +       }
>>> +
>>> +       info("Closing bluetooth_watcher thread");
>>> +       pthread_exit(NULL);
>>> +       return NULL;
>>> +}
>>> +
>>>  static int audio_open(const hw_module_t *module, const char *name,
>>>                                                         hw_device_t **device)
>>>  {
>>>         struct a2dp_audio_dev *a2dp_dev;
>>> +       int err;
>>>
>>>         DBG("");
>>>
>>> @@ -427,6 +559,19 @@ static int audio_open(const hw_module_t *module, const char *name,
>>>
>>>         *device = &a2dp_dev->dev.common;
>>>
>>> +       a2dp_dev->hal_sk = -1;
>>> +
>>> +       pthread_mutex_init(&a2dp_dev->hal_sk_mutex, NULL);
>>> +       pthread_cond_init(&a2dp_dev->bt_watcher_cond, NULL);
>>> +
>>> +       err = pthread_create(&a2dp_dev->bt_watcher, NULL, bluetoothd_watcher, a2dp_dev);
>>> +       if (err < 0) {
>>> +               a2dp_dev->bt_watcher = 0;
>>> +               error("Failed to start bluetoothd watcher thread: %d (%s)", -err,
>>> +                                                               strerror(-err));
>>> +               return (-err);
>>> +       }
>>> +
>>>         return 0;
>>>  }
>>>
>>> diff --git a/android/hal-audio.h b/android/hal-audio.h
>>> new file mode 100644
>>> index 0000000..93e49f6
>>> --- /dev/null
>>> +++ b/android/hal-audio.h
>>> @@ -0,0 +1,18 @@
>>> +/*
>>> + *
>>> + * Copyright (C) 2013 Intel Corporation
>>> + *
>>> + * Licensed under the Apache License, Version 2.0 (the "License");
>>> + * you may not use this file except in compliance with the License.
>>> + * You may obtain a copy of the License at
>>> + *
>>> + *      http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + * Unless required by applicable law or agreed to in writing, software
>>> + * distributed under the License is distributed on an "AS IS" BASIS,
>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>> + * See the License for the specific language governing permissions and
>>> + * limitations under the License.
>>> + *
>>> + */
>>> +static const char BLUEZ_AUDIO_HAL_SK_PATH[] = "\0bluez_audio_socket";
>>> --
>>> 1.8.4
>>
>> Ive started working on the daemon side and should be able to post some
>> patches today, Im not really sure we are going to need a thread for
>> command handling since all the commands will be sent by the plugin
>> side so they can be synchronous, the only problem would be if the
>> plugin cannot really block even for a short period.
>>
>
> This Thread is only going to be used for accept, endpoint register and
> then listening for socket close (so we can start listen for connection
> from new bluetoothd).
>
> Commands and response will be handled in main thread (ex.
> audio_open(), out_write()) and we can block there for a while - no
> problem with that

Well it seems the init/listen stage needs to be non-blocking so we
don't interrupt the audio init procedure which may block bluetooth HAL
to initialized, the endpoint then should be registered whenever
bluetoothd connects.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [RFC 5/6] android/audio: Add wrapper struct for audio structures
From: Luiz Augusto von Dentz @ 2013-12-30 12:43 UTC (permalink / raw)
  To: Lukasz Rymanowski; +Cc: linux-bluetooth@vger.kernel.org, Johan Hedberg
In-Reply-To: <CAMudyAsGXQ3cPjv_5R5cFrM=asYr8ZZH2YmLdOh95mEiz0oc8w@mail.gmail.com>

Hi Lukasz,

On Mon, Dec 30, 2013 at 2:37 PM, Lukasz Rymanowski
<lukasz.rymanowski@tieto.com> wrote:
> Hi Luiz,
>
> On 30 December 2013 13:04, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> Hi Lukasz,
>>
>> On Mon, Dec 30, 2013 at 1:33 PM, Lukasz Rymanowski
>> <lukasz.rymanowski@tieto.com> wrote:
>>> Hi Luiz,
>>>
>>> On 30 December 2013 12:27, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>>> Hi Lukasz,
>>>>
>>>> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
>>>> <lukasz.rymanowski@tieto.com> wrote:
>>>>> This patch add wrapping struct for audio_hw_dev and audio_stream_out.
>>>>> We will need this to keep some more addition info related to a2dp stream
>>>>> and also hal IPC later on
>>>>>
>>>>> ---
>>>>>  android/Makefile.am |  1 +
>>>>>  android/hal-audio.c | 97 +++++++++++++++++++++++++++++------------------------
>>>>>  2 files changed, 55 insertions(+), 43 deletions(-)
>>>>>
>>>>> diff --git a/android/Makefile.am b/android/Makefile.am
>>>>> index dec81ce..eaf39bd 100644
>>>>> --- a/android/Makefile.am
>>>>> +++ b/android/Makefile.am
>>>>> @@ -104,6 +104,7 @@ android_android_tester_LDFLAGS = -pthread
>>>>>  noinst_LTLIBRARIES += android/libaudio-internal.la
>>>>>
>>>>>  android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>>>>> +                                       android/hal-audio.h \
>>>>>                                         android/hardware/audio.h \
>>>>>                                         android/hardware/audio_effect.h \
>>>>>                                         android/hardware/hardware.h \
>>>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>>>> index 7f4a3f2..011a699 100644
>>>>> --- a/android/hal-audio.c
>>>>> +++ b/android/hal-audio.c
>>>>> @@ -25,6 +25,15 @@
>>>>>
>>>>>  #include "hal-log.h"
>>>>>
>>>>> +struct a2dp_audio_dev {
>>>>> +       struct audio_hw_device dev;
>>>>> +       struct a2dp_stream_out *stream_out;
>>>>> +};
>>>>> +
>>>>> +struct a2dp_stream_out {
>>>>> +       struct audio_stream_out stream;
>>>>> +};
>>>>> +
>>>>>  static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
>>>>>                                                                 size_t bytes)
>>>>>  {
>>>>> @@ -230,32 +239,34 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>>>>                                         struct audio_stream_out **stream_out)
>>>>>
>>>>>  {
>>>>> -       struct audio_stream_out *out;
>>>>> +       struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>>>>> +       struct a2dp_stream_out *a2dp_out;
>>>>>
>>>>> -       out = calloc(1, sizeof(struct audio_stream_out));
>>>>> -       if (!out)
>>>>> +       a2dp_out = calloc(1, sizeof(struct a2dp_stream_out));
>>>>> +       if (!a2dp_out)
>>>>>                 return -ENOMEM;
>>>>>
>>>>>         DBG("");
>>>>>
>>>>> -       out->common.get_sample_rate = out_get_sample_rate;
>>>>> -       out->common.set_sample_rate = out_set_sample_rate;
>>>>> -       out->common.get_buffer_size = out_get_buffer_size;
>>>>> -       out->common.get_channels = out_get_channels;
>>>>> -       out->common.get_format = out_get_format;
>>>>> -       out->common.set_format = out_set_format;
>>>>> -       out->common.standby = out_standby;
>>>>> -       out->common.dump = out_dump;
>>>>> -       out->common.set_parameters = out_set_parameters;
>>>>> -       out->common.get_parameters = out_get_parameters;
>>>>> -       out->common.add_audio_effect = out_add_audio_effect;
>>>>> -       out->common.remove_audio_effect = out_remove_audio_effect;
>>>>> -       out->get_latency = out_get_latency;
>>>>> -       out->set_volume = out_set_volume;
>>>>> -       out->write = out_write;
>>>>> -       out->get_render_position = out_get_render_position;
>>>>> +       a2dp_out->stream.common.get_sample_rate = out_get_sample_rate;
>>>>> +       a2dp_out->stream.common.set_sample_rate = out_set_sample_rate;
>>>>> +       a2dp_out->stream.common.get_buffer_size = out_get_buffer_size;
>>>>> +       a2dp_out->stream.common.get_channels = out_get_channels;
>>>>> +       a2dp_out->stream.common.get_format = out_get_format;
>>>>> +       a2dp_out->stream.common.set_format = out_set_format;
>>>>> +       a2dp_out->stream.common.standby = out_standby;
>>>>> +       a2dp_out->stream.common.dump = out_dump;
>>>>> +       a2dp_out->stream.common.set_parameters = out_set_parameters;
>>>>> +       a2dp_out->stream.common.get_parameters = out_get_parameters;
>>>>> +       a2dp_out->stream.common.add_audio_effect = out_add_audio_effect;
>>>>> +       a2dp_out->stream.common.remove_audio_effect = out_remove_audio_effect;
>>>>> +       a2dp_out->stream.get_latency = out_get_latency;
>>>>> +       a2dp_out->stream.set_volume = out_set_volume;
>>>>> +       a2dp_out->stream.write = out_write;
>>>>> +       a2dp_out->stream.get_render_position = out_get_render_position;
>>>>>
>>>>> -       *stream_out = out;
>>>>> +       *stream_out = &a2dp_out->stream;
>>>>> +       a2dp_dev->stream_out = a2dp_out;
>>>>>
>>>>>         return 0;
>>>>>  }
>>>>> @@ -381,7 +392,7 @@ static int audio_close(hw_device_t *device)
>>>>>  static int audio_open(const hw_module_t *module, const char *name,
>>>>>                                                         hw_device_t **device)
>>>>>  {
>>>>> -       struct audio_hw_device *audio;
>>>>> +       struct a2dp_audio_dev *a2dp_dev;
>>>>>
>>>>>         DBG("");
>>>>>
>>>>> @@ -391,30 +402,30 @@ static int audio_open(const hw_module_t *module, const char *name,
>>>>>                 return -EINVAL;
>>>>>         }
>>>>>
>>>>> -       audio = calloc(1, sizeof(struct audio_hw_device));
>>>>> -       if (!audio)
>>>>> +       a2dp_dev = calloc(1, sizeof(struct a2dp_audio_dev));
>>>>> +       if (!a2dp_dev)
>>>>>                 return -ENOMEM;
>>>>>
>>>>> -       audio->common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>>>> -       audio->common.module = (struct hw_module_t *) module;
>>>>> -       audio->common.close = audio_close;
>>>>> -
>>>>> -       audio->init_check = audio_init_check;
>>>>> -       audio->set_voice_volume = audio_set_voice_volume;
>>>>> -       audio->set_master_volume = audio_set_master_volume
>>>>> -       audio->set_mode = audio_set_mode;
>>>>> -       audio->set_mic_mute = audio_set_mic_mute;
>>>>> -       audio->get_mic_mute = audio_get_mic_mute;
>>>>> -       audio->set_parameters = audio_set_parameters;
>>>>> -       audio->get_parameters = audio_get_parameters;
>>>>> -       audio->get_input_buffer_size = audio_get_input_buffer_size;
>>>>> -       audio->open_output_stream = audio_open_output_stream;
>>>>> -       audio->close_output_stream = audio_close_output_stream;
>>>>> -       audio->open_input_stream = audio_open_input_stream;
>>>>> -       audio->close_input_stream = audio_close_input_stream;
>>>>> -       audio->dump = audio_dump;
>>>>> -
>>>>> -       *device = &audio->common;
>>>>> +       a2dp_dev->dev.common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>>>> +       a2dp_dev->dev.common.module = (struct hw_module_t *) module;
>>>>> +       a2dp_dev->dev.common.close = audio_close;
>>>>> +
>>>>> +       a2dp_dev->dev.init_check = audio_init_check;
>>>>> +       a2dp_dev->dev.set_voice_volume = audio_set_voice_volume;
>>>>> +       a2dp_dev->dev.set_master_volume = audio_set_master_volume;
>>>>> +       a2dp_dev->dev.set_mode = audio_set_mode;
>>>>> +       a2dp_dev->dev.set_mic_mute = audio_set_mic_mute;
>>>>> +       a2dp_dev->dev.get_mic_mute = audio_get_mic_mute;
>>>>> +       a2dp_dev->dev.set_parameters = audio_set_parameters;
>>>>> +       a2dp_dev->dev.get_parameters = audio_get_parameters;
>>>>> +       a2dp_dev->dev.get_input_buffer_size = audio_get_input_buffer_size;
>>>>> +       a2dp_dev->dev.open_output_stream = audio_open_output_stream;
>>>>> +       a2dp_dev->dev.close_output_stream = audio_close_output_stream;
>>>>> +       a2dp_dev->dev.open_input_stream = audio_open_input_stream;
>>>>> +       a2dp_dev->dev.close_input_stream = audio_close_input_stream;
>>>>> +       a2dp_dev->dev.dump = audio_dump;
>>>>> +
>>>>> +       *device = &a2dp_dev->dev.common;
>>>>>
>>>>>         return 0;
>>>>>  }
>>>>> --
>>>>> 1.8.4
>>>>
>>>> How is this supposed to be accessed, are you planning on adding a
>>>> global variable?
>>>>
>>>>
>>> Have a look on audio_open_output_stream(). This is actually how they did so
>>> far and it is fine for me.
>>> I want to avoid globals here.
>>
>> I can see that you allocate a different struct for a2dp_dev and
>> a2dp_out, but then you return a member of the struct back to HAL but
>> after that you wont access these structs anymore and the HAL only uses
>> the members that you return so you probably gonna need to lookup for
>> these structs to access any auxiliary data that we might want to add,
>> also these structs never seems to be freed which I suppose can
>> generate memory leaks.
>>
>>
> You right I missed free of a2dp_out
> It will be done in  audio_close_output_stream(). Access to that will
> be similar as to a2dp_dev() in audio_close()

Well it doesn't seems you are changing audio_close, in fact the only
thing that audio_close frees is hw_device_t *device not struct
a2dp_audio_dev so you still have to deal with mapping access between
those struct and it doesn't seems the HAL have any concept of
user_data to attach so this normally indicates we need to somehow
store pointer to access latter.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [RFC 5/6] android/audio: Add wrapper struct for audio structures
From: Lukasz Rymanowski @ 2013-12-30 12:37 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org, Johan Hedberg
In-Reply-To: <CABBYNZK5rRnRnrvDCx7w9zsrG4wZyf8GpGBhLq57fi_o_kNVxA@mail.gmail.com>

Hi Luiz,

On 30 December 2013 13:04, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Lukasz,
>
> On Mon, Dec 30, 2013 at 1:33 PM, Lukasz Rymanowski
> <lukasz.rymanowski@tieto.com> wrote:
>> Hi Luiz,
>>
>> On 30 December 2013 12:27, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>> Hi Lukasz,
>>>
>>> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
>>> <lukasz.rymanowski@tieto.com> wrote:
>>>> This patch add wrapping struct for audio_hw_dev and audio_stream_out.
>>>> We will need this to keep some more addition info related to a2dp stream
>>>> and also hal IPC later on
>>>>
>>>> ---
>>>>  android/Makefile.am |  1 +
>>>>  android/hal-audio.c | 97 +++++++++++++++++++++++++++++------------------------
>>>>  2 files changed, 55 insertions(+), 43 deletions(-)
>>>>
>>>> diff --git a/android/Makefile.am b/android/Makefile.am
>>>> index dec81ce..eaf39bd 100644
>>>> --- a/android/Makefile.am
>>>> +++ b/android/Makefile.am
>>>> @@ -104,6 +104,7 @@ android_android_tester_LDFLAGS = -pthread
>>>>  noinst_LTLIBRARIES += android/libaudio-internal.la
>>>>
>>>>  android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>>>> +                                       android/hal-audio.h \
>>>>                                         android/hardware/audio.h \
>>>>                                         android/hardware/audio_effect.h \
>>>>                                         android/hardware/hardware.h \
>>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>>> index 7f4a3f2..011a699 100644
>>>> --- a/android/hal-audio.c
>>>> +++ b/android/hal-audio.c
>>>> @@ -25,6 +25,15 @@
>>>>
>>>>  #include "hal-log.h"
>>>>
>>>> +struct a2dp_audio_dev {
>>>> +       struct audio_hw_device dev;
>>>> +       struct a2dp_stream_out *stream_out;
>>>> +};
>>>> +
>>>> +struct a2dp_stream_out {
>>>> +       struct audio_stream_out stream;
>>>> +};
>>>> +
>>>>  static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
>>>>                                                                 size_t bytes)
>>>>  {
>>>> @@ -230,32 +239,34 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>>>                                         struct audio_stream_out **stream_out)
>>>>
>>>>  {
>>>> -       struct audio_stream_out *out;
>>>> +       struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>>>> +       struct a2dp_stream_out *a2dp_out;
>>>>
>>>> -       out = calloc(1, sizeof(struct audio_stream_out));
>>>> -       if (!out)
>>>> +       a2dp_out = calloc(1, sizeof(struct a2dp_stream_out));
>>>> +       if (!a2dp_out)
>>>>                 return -ENOMEM;
>>>>
>>>>         DBG("");
>>>>
>>>> -       out->common.get_sample_rate = out_get_sample_rate;
>>>> -       out->common.set_sample_rate = out_set_sample_rate;
>>>> -       out->common.get_buffer_size = out_get_buffer_size;
>>>> -       out->common.get_channels = out_get_channels;
>>>> -       out->common.get_format = out_get_format;
>>>> -       out->common.set_format = out_set_format;
>>>> -       out->common.standby = out_standby;
>>>> -       out->common.dump = out_dump;
>>>> -       out->common.set_parameters = out_set_parameters;
>>>> -       out->common.get_parameters = out_get_parameters;
>>>> -       out->common.add_audio_effect = out_add_audio_effect;
>>>> -       out->common.remove_audio_effect = out_remove_audio_effect;
>>>> -       out->get_latency = out_get_latency;
>>>> -       out->set_volume = out_set_volume;
>>>> -       out->write = out_write;
>>>> -       out->get_render_position = out_get_render_position;
>>>> +       a2dp_out->stream.common.get_sample_rate = out_get_sample_rate;
>>>> +       a2dp_out->stream.common.set_sample_rate = out_set_sample_rate;
>>>> +       a2dp_out->stream.common.get_buffer_size = out_get_buffer_size;
>>>> +       a2dp_out->stream.common.get_channels = out_get_channels;
>>>> +       a2dp_out->stream.common.get_format = out_get_format;
>>>> +       a2dp_out->stream.common.set_format = out_set_format;
>>>> +       a2dp_out->stream.common.standby = out_standby;
>>>> +       a2dp_out->stream.common.dump = out_dump;
>>>> +       a2dp_out->stream.common.set_parameters = out_set_parameters;
>>>> +       a2dp_out->stream.common.get_parameters = out_get_parameters;
>>>> +       a2dp_out->stream.common.add_audio_effect = out_add_audio_effect;
>>>> +       a2dp_out->stream.common.remove_audio_effect = out_remove_audio_effect;
>>>> +       a2dp_out->stream.get_latency = out_get_latency;
>>>> +       a2dp_out->stream.set_volume = out_set_volume;
>>>> +       a2dp_out->stream.write = out_write;
>>>> +       a2dp_out->stream.get_render_position = out_get_render_position;
>>>>
>>>> -       *stream_out = out;
>>>> +       *stream_out = &a2dp_out->stream;
>>>> +       a2dp_dev->stream_out = a2dp_out;
>>>>
>>>>         return 0;
>>>>  }
>>>> @@ -381,7 +392,7 @@ static int audio_close(hw_device_t *device)
>>>>  static int audio_open(const hw_module_t *module, const char *name,
>>>>                                                         hw_device_t **device)
>>>>  {
>>>> -       struct audio_hw_device *audio;
>>>> +       struct a2dp_audio_dev *a2dp_dev;
>>>>
>>>>         DBG("");
>>>>
>>>> @@ -391,30 +402,30 @@ static int audio_open(const hw_module_t *module, const char *name,
>>>>                 return -EINVAL;
>>>>         }
>>>>
>>>> -       audio = calloc(1, sizeof(struct audio_hw_device));
>>>> -       if (!audio)
>>>> +       a2dp_dev = calloc(1, sizeof(struct a2dp_audio_dev));
>>>> +       if (!a2dp_dev)
>>>>                 return -ENOMEM;
>>>>
>>>> -       audio->common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>>> -       audio->common.module = (struct hw_module_t *) module;
>>>> -       audio->common.close = audio_close;
>>>> -
>>>> -       audio->init_check = audio_init_check;
>>>> -       audio->set_voice_volume = audio_set_voice_volume;
>>>> -       audio->set_master_volume = audio_set_master_volume;
>>>> -       audio->set_mode = audio_set_mode;
>>>> -       audio->set_mic_mute = audio_set_mic_mute;
>>>> -       audio->get_mic_mute = audio_get_mic_mute;
>>>> -       audio->set_parameters = audio_set_parameters;
>>>> -       audio->get_parameters = audio_get_parameters;
>>>> -       audio->get_input_buffer_size = audio_get_input_buffer_size;
>>>> -       audio->open_output_stream = audio_open_output_stream;
>>>> -       audio->close_output_stream = audio_close_output_stream;
>>>> -       audio->open_input_stream = audio_open_input_stream;
>>>> -       audio->close_input_stream = audio_close_input_stream;
>>>> -       audio->dump = audio_dump;
>>>> -
>>>> -       *device = &audio->common;
>>>> +       a2dp_dev->dev.common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>>> +       a2dp_dev->dev.common.module = (struct hw_module_t *) module;
>>>> +       a2dp_dev->dev.common.close = audio_close;
>>>> +
>>>> +       a2dp_dev->dev.init_check = audio_init_check;
>>>> +       a2dp_dev->dev.set_voice_volume = audio_set_voice_volume;
>>>> +       a2dp_dev->dev.set_master_volume = audio_set_master_volume;
>>>> +       a2dp_dev->dev.set_mode = audio_set_mode;
>>>> +       a2dp_dev->dev.set_mic_mute = audio_set_mic_mute;
>>>> +       a2dp_dev->dev.get_mic_mute = audio_get_mic_mute;
>>>> +       a2dp_dev->dev.set_parameters = audio_set_parameters;
>>>> +       a2dp_dev->dev.get_parameters = audio_get_parameters;
>>>> +       a2dp_dev->dev.get_input_buffer_size = audio_get_input_buffer_size;
>>>> +       a2dp_dev->dev.open_output_stream = audio_open_output_stream;
>>>> +       a2dp_dev->dev.close_output_stream = audio_close_output_stream;
>>>> +       a2dp_dev->dev.open_input_stream = audio_open_input_stream;
>>>> +       a2dp_dev->dev.close_input_stream = audio_close_input_stream;
>>>> +       a2dp_dev->dev.dump = audio_dump;
>>>> +
>>>> +       *device = &a2dp_dev->dev.common;
>>>>
>>>>         return 0;
>>>>  }
>>>> --
>>>> 1.8.4
>>>
>>> How is this supposed to be accessed, are you planning on adding a
>>> global variable?
>>>
>>>
>> Have a look on audio_open_output_stream(). This is actually how they did so
>> far and it is fine for me.
>> I want to avoid globals here.
>
> I can see that you allocate a different struct for a2dp_dev and
> a2dp_out, but then you return a member of the struct back to HAL but
> after that you wont access these structs anymore and the HAL only uses
> the members that you return so you probably gonna need to lookup for
> these structs to access any auxiliary data that we might want to add,
> also these structs never seems to be freed which I suppose can
> generate memory leaks.
>
>
You right I missed free of a2dp_out
It will be done in  audio_close_output_stream(). Access to that will
be similar as to a2dp_dev() in audio_close()

> --
> Luiz Augusto von Dentz

^ permalink raw reply

* [RFC BlueZ 9/9] android: Add hal_audio_ipc_init and hal_audio_ipc_cleanup
From: Luiz Augusto von Dentz @ 2013-12-30 12:34 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388406855-8809-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

These function initialize the audio IPC in the HAL side.
---
 android/hal-audio.c |  9 +++++++++
 android/hal-ipc.c   | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 android/hal-ipc.h   |  3 +++
 3 files changed, 68 insertions(+)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 7f4a3f2..2e7dcca 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -24,6 +24,7 @@
 #include <hardware/hardware.h>
 
 #include "hal-log.h"
+#include "hal-ipc.h"
 
 static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 								size_t bytes)
@@ -374,6 +375,9 @@ static int audio_dump(const audio_hw_device_t *device, int fd)
 static int audio_close(hw_device_t *device)
 {
 	DBG("");
+
+	hal_audio_ipc_cleanup();
+
 	free(device);
 	return 0;
 }
@@ -391,6 +395,11 @@ static int audio_open(const hw_module_t *module, const char *name,
 		return -EINVAL;
 	}
 
+	if (!hal_audio_ipc_init()) {
+		error("Unable to initialize audio IPC");
+		return -ENOTCONN;
+	}
+
 	audio = calloc(1, sizeof(struct audio_hw_device));
 	if (!audio)
 		return -ENOMEM;
diff --git a/android/hal-ipc.c b/android/hal-ipc.c
index b19704a..3a1fcc7 100644
--- a/android/hal-ipc.c
+++ b/android/hal-ipc.c
@@ -38,6 +38,7 @@
 
 static int cmd_sk = -1;
 static int notif_sk = -1;
+static int audio_sk = -1;
 
 static pthread_mutex_t cmd_sk_mutex = PTHREAD_MUTEX_INITIALIZER;
 
@@ -449,3 +450,58 @@ int hal_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len, void *param,
 
 	return BT_STATUS_SUCCESS;
 }
+
+bool hal_audio_ipc_init(void)
+{
+	struct sockaddr_un addr;
+	int sk;
+	int err;
+
+	sk = socket(AF_LOCAL, SOCK_SEQPACKET, 0);
+	if (sk < 0) {
+		err = errno;
+		error("Failed to create socket: %d (%s)", err,
+							strerror(err));
+		return false;
+	}
+
+	memset(&addr, 0, sizeof(addr));
+	addr.sun_family = AF_UNIX;
+
+	memcpy(addr.sun_path, BLUEZ_AUDIO_SK_PATH, sizeof(BLUEZ_AUDIO_SK_PATH));
+
+	if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
+		err = errno;
+		error("Failed to bind socket: %d (%s)", err, strerror(err));
+		close(sk);
+		return false;
+	}
+
+	if (listen(sk, 2) < 0) {
+		err = errno;
+		error("Failed to listen on socket: %d (%s)", err,
+								strerror(err));
+		close(sk);
+		return false;
+	}
+
+	audio_sk = accept_connection(sk);
+	if (audio_sk < 0) {
+		close(sk);
+		return false;
+	}
+
+	info("audio connected");
+
+	close(sk);
+
+	return true;
+}
+
+void hal_audio_ipc_cleanup(void)
+{
+	close(audio_sk);
+	audio_sk = -1;
+
+	shutdown(audio_sk, SHUT_RD);
+}
diff --git a/android/hal-ipc.h b/android/hal-ipc.h
index 2fbf30f..bd1682d 100644
--- a/android/hal-ipc.h
+++ b/android/hal-ipc.h
@@ -30,3 +30,6 @@ int hal_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len, void *param,
 void hal_ipc_register(uint8_t service, const struct hal_ipc_handler *handlers,
 								uint8_t size);
 void hal_ipc_unregister(uint8_t service);
+
+bool hal_audio_ipc_init(void);
+void hal_audio_ipc_cleanup(void);
-- 
1.8.4.2


^ permalink raw reply related

* [RFC BlueZ 8/9] android: Add stream suspend command/response struct
From: Luiz Augusto von Dentz @ 2013-12-30 12:34 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388406855-8809-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds the definitions to stream suspend command and response.
---
 android/a2dp.c    | 10 ++++++++++
 android/hal-msg.h |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/android/a2dp.c b/android/a2dp.c
index 284d44d..9dbb470 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -390,6 +390,14 @@ static void bt_stream_resume(const void *buf, uint16_t len)
 							HAL_STATUS_FAILED);
 }
 
+static void bt_stream_suspend(const void *buf, uint16_t len)
+{
+	DBG("Not Implemented");
+
+	ipc_send_rsp(HAL_SERVICE_ID_AUDIO, AUDIO_OP_SUSPEND_STREAM,
+							HAL_STATUS_FAILED);
+}
+
 static const struct ipc_handler audio_handlers[] = {
 	/* AUDIO_OP_OPEN */
 	{ bt_audio_open, true, sizeof(struct audio_cmd_open) },
@@ -401,6 +409,8 @@ static const struct ipc_handler audio_handlers[] = {
 	{ bt_stream_close, false, sizeof(struct audio_cmd_close_stream) },
 	/* AUDIO_OP_RESUME_STREAM */
 	{ bt_stream_resume, false, sizeof(struct audio_cmd_resume_stream) },
+	/* AUDIO_OP_SUSPEND_STREAM */
+	{ bt_stream_suspend, false, sizeof(struct audio_cmd_suspend_stream) },
 };
 
 bool bt_a2dp_register(const bdaddr_t *addr)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index fe433d1..ef40dac 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -610,3 +610,8 @@ struct audio_cmd_close_stream {
 struct audio_cmd_resume_stream {
 	uint8_t id;
 } __attribute__((packed));
+
+#define AUDIO_OP_SUSPEND_STREAM			0x06
+struct audio_cmd_suspend_stream {
+	uint8_t id;
+} __attribute__((packed));
-- 
1.8.4.2


^ permalink raw reply related

* [RFC BlueZ 7/9] android: Add stream resume command/response struct
From: Luiz Augusto von Dentz @ 2013-12-30 12:34 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388406855-8809-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds the definitions to stream resume command and response.
---
 android/a2dp.c    | 10 ++++++++++
 android/hal-msg.h |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/android/a2dp.c b/android/a2dp.c
index 2dba482..284d44d 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -382,6 +382,14 @@ static void bt_stream_close(const void *buf, uint16_t len)
 							HAL_STATUS_FAILED);
 }
 
+static void bt_stream_resume(const void *buf, uint16_t len)
+{
+	DBG("Not Implemented");
+
+	ipc_send_rsp(HAL_SERVICE_ID_AUDIO, AUDIO_OP_RESUME_STREAM,
+							HAL_STATUS_FAILED);
+}
+
 static const struct ipc_handler audio_handlers[] = {
 	/* AUDIO_OP_OPEN */
 	{ bt_audio_open, true, sizeof(struct audio_cmd_open) },
@@ -391,6 +399,8 @@ static const struct ipc_handler audio_handlers[] = {
 	{ bt_stream_open, false, sizeof(struct audio_cmd_open_stream) },
 	/* AUDIO_OP_CLOSE_STREAM */
 	{ bt_stream_close, false, sizeof(struct audio_cmd_close_stream) },
+	/* AUDIO_OP_RESUME_STREAM */
+	{ bt_stream_resume, false, sizeof(struct audio_cmd_resume_stream) },
 };
 
 bool bt_a2dp_register(const bdaddr_t *addr)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 2816f6f..fe433d1 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -605,3 +605,8 @@ struct audio_rsp_open_stream {
 struct audio_cmd_close_stream {
 	uint8_t id;
 } __attribute__((packed));
+
+#define AUDIO_OP_RESUME_STREAM			0x05
+struct audio_cmd_resume_stream {
+	uint8_t id;
+} __attribute__((packed));
-- 
1.8.4.2


^ permalink raw reply related

* [RFC BlueZ 6/9] android: Add stream close command/response struct
From: Luiz Augusto von Dentz @ 2013-12-30 12:34 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388406855-8809-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds the definitions to stream close command and response.
---
 android/a2dp.c    | 10 ++++++++++
 android/hal-msg.h |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/android/a2dp.c b/android/a2dp.c
index 104b950..2dba482 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -374,6 +374,14 @@ static void bt_stream_open(const void *buf, uint16_t len)
 							HAL_STATUS_FAILED);
 }
 
+static void bt_stream_close(const void *buf, uint16_t len)
+{
+	DBG("Not Implemented");
+
+	ipc_send_rsp(HAL_SERVICE_ID_AUDIO, AUDIO_OP_CLOSE_STREAM,
+							HAL_STATUS_FAILED);
+}
+
 static const struct ipc_handler audio_handlers[] = {
 	/* AUDIO_OP_OPEN */
 	{ bt_audio_open, true, sizeof(struct audio_cmd_open) },
@@ -381,6 +389,8 @@ static const struct ipc_handler audio_handlers[] = {
 	{ bt_audio_close, false, sizeof(struct audio_cmd_close) },
 	/* AUDIO_OP_OPEN_STREAM */
 	{ bt_stream_open, false, sizeof(struct audio_cmd_open_stream) },
+	/* AUDIO_OP_CLOSE_STREAM */
+	{ bt_stream_close, false, sizeof(struct audio_cmd_close_stream) },
 };
 
 bool bt_a2dp_register(const bdaddr_t *addr)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 62674fa..2816f6f 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -600,3 +600,8 @@ struct audio_rsp_open_stream {
 	uint8_t len;
 	uint8_t data[0];
 } __attribute__((packed));
+
+#define AUDIO_OP_CLOSE_STREAM			0x04
+struct audio_cmd_close_stream {
+	uint8_t id;
+} __attribute__((packed));
-- 
1.8.4.2


^ permalink raw reply related

* [RFC BlueZ 5/9] android: Add stream open command/response struct
From: Luiz Augusto von Dentz @ 2013-12-30 12:34 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388406855-8809-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds the definitions to stream open command and response.
---
 android/a2dp.c    | 10 ++++++++++
 android/hal-msg.h | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/android/a2dp.c b/android/a2dp.c
index 43590f0..104b950 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -366,11 +366,21 @@ static void bt_audio_close(const void *buf, uint16_t len)
 	ipc_send_rsp(HAL_SERVICE_ID_AUDIO, AUDIO_OP_CLOSE, HAL_STATUS_FAILED);
 }
 
+static void bt_stream_open(const void *buf, uint16_t len)
+{
+	DBG("Not Implemented");
+
+	ipc_send_rsp(HAL_SERVICE_ID_AUDIO, AUDIO_OP_OPEN_STREAM,
+							HAL_STATUS_FAILED);
+}
+
 static const struct ipc_handler audio_handlers[] = {
 	/* AUDIO_OP_OPEN */
 	{ bt_audio_open, true, sizeof(struct audio_cmd_open) },
 	/* AUDIO_OP_CLOSE */
 	{ bt_audio_close, false, sizeof(struct audio_cmd_close) },
+	/* AUDIO_OP_OPEN_STREAM */
+	{ bt_stream_open, false, sizeof(struct audio_cmd_open_stream) },
 };
 
 bool bt_a2dp_register(const bdaddr_t *addr)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index f359952..62674fa 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -590,3 +590,13 @@ struct audio_rsp_open {
 struct audio_cmd_close {
 	uint8_t id;
 } __attribute__((packed));
+
+#define AUDIO_OP_OPEN_STREAM			0x03
+struct audio_cmd_open_stream {
+	uint8_t id;
+} __attribute__((packed));
+
+struct audio_rsp_open_stream {
+	uint8_t len;
+	uint8_t data[0];
+} __attribute__((packed));
-- 
1.8.4.2


^ permalink raw reply related

* [RFC BlueZ 4/9] android: Add audio close command/response struct
From: Luiz Augusto von Dentz @ 2013-12-30 12:34 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388406855-8809-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds the definitions to audio close command and response.
---
 android/a2dp.c    | 9 +++++++++
 android/hal-msg.h | 5 +++++
 2 files changed, 14 insertions(+)

diff --git a/android/a2dp.c b/android/a2dp.c
index 5cb01f7..43590f0 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -359,9 +359,18 @@ static void bt_audio_open(const void *buf, uint16_t len)
 	ipc_send_rsp(HAL_SERVICE_ID_AUDIO, AUDIO_OP_OPEN, HAL_STATUS_FAILED);
 }
 
+static void bt_audio_close(const void *buf, uint16_t len)
+{
+	DBG("Not Implemented");
+
+	ipc_send_rsp(HAL_SERVICE_ID_AUDIO, AUDIO_OP_CLOSE, HAL_STATUS_FAILED);
+}
+
 static const struct ipc_handler audio_handlers[] = {
 	/* AUDIO_OP_OPEN */
 	{ bt_audio_open, true, sizeof(struct audio_cmd_open) },
+	/* AUDIO_OP_CLOSE */
+	{ bt_audio_close, false, sizeof(struct audio_cmd_close) },
 };
 
 bool bt_a2dp_register(const bdaddr_t *addr)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 4b52e5e..f359952 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -585,3 +585,8 @@ struct audio_preset {
 struct audio_rsp_open {
 	uint8_t id;
 } __attribute__((packed));
+
+#define AUDIO_OP_CLOSE				0x02
+struct audio_cmd_close {
+	uint8_t id;
+} __attribute__((packed));
-- 
1.8.4.2


^ permalink raw reply related

* [RFC BlueZ 3/9] android: Add audio open command/response struct
From: Luiz Augusto von Dentz @ 2013-12-30 12:34 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388406855-8809-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds the definitions to audio open command and response.
---
 android/a2dp.c            |  9 +++++++++
 android/audio-ipc-api.txt |  2 +-
 android/hal-msg.h         | 18 ++++++++++++++++++
 android/ipc.c             |  5 ++++-
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/android/a2dp.c b/android/a2dp.c
index 63f1f58..5cb01f7 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -352,7 +352,16 @@ static sdp_record_t *a2dp_record(void)
 	return record;
 }
 
+static void bt_audio_open(const void *buf, uint16_t len)
+{
+	DBG("Not Implemented");
+
+	ipc_send_rsp(HAL_SERVICE_ID_AUDIO, AUDIO_OP_OPEN, HAL_STATUS_FAILED);
+}
+
 static const struct ipc_handler audio_handlers[] = {
+	/* AUDIO_OP_OPEN */
+	{ bt_audio_open, true, sizeof(struct audio_cmd_open) },
 };
 
 bool bt_a2dp_register(const bdaddr_t *addr)
diff --git a/android/audio-ipc-api.txt b/android/audio-ipc-api.txt
index 1c42800..37a1569 100644
--- a/android/audio-ipc-api.txt
+++ b/android/audio-ipc-api.txt
@@ -49,9 +49,9 @@ Identifier: "audio" (BT_AUDIO_ID)
 
 		Command parameters: Service UUID (16 octets)
 				    Codec ID (1 octet)
+				    Number of codec presets (1 octet)
 				    Codec capabilities length (1 octet)
 				    Codec capabilities (variable)
-				    Number of codec presets (1 octet)
 				    Codec preset # length (1 octet)
 				    Codec preset # configuration (variable)
 				    ...
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 1afb1bc..4b52e5e 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -567,3 +567,21 @@ struct hal_ev_a2dp_audio_state {
 	uint8_t state;
 	uint8_t bdaddr[6];
 } __attribute__((packed));
+
+#define AUDIO_OP_OPEN				0x01
+struct audio_cmd_open {
+	uint16_t uuid;
+	uint8_t codec;
+	uint8_t presets;
+	uint8_t len;
+	uint8_t data[0];
+} __attribute__((packed));
+
+struct audio_preset {
+	uint8_t len;
+	uint8_t data[0];
+} __attribute__((packed));
+
+struct audio_rsp_open {
+	uint8_t id;
+} __attribute__((packed));
diff --git a/android/ipc.c b/android/ipc.c
index 6cdbf60..bb16553 100644
--- a/android/ipc.c
+++ b/android/ipc.c
@@ -301,7 +301,10 @@ void ipc_send_rsp(uint8_t service_id, uint8_t opcode, uint8_t status)
 	struct hal_status s;
 	int sk;
 
-	sk = g_io_channel_unix_get_fd(cmd_io);
+	if (service_id == HAL_SERVICE_ID_AUDIO)
+		sk = g_io_channel_unix_get_fd(audio_io);
+	else
+		sk = g_io_channel_unix_get_fd(cmd_io);
 
 	if (status == HAL_STATUS_SUCCESS) {
 		ipc_send(sk, service_id, opcode, 0, NULL, -1);
-- 
1.8.4.2


^ permalink raw reply related

* [RFC BlueZ 2/9] android/A2DP: Add initial code to handle audio IPC commands
From: Luiz Augusto von Dentz @ 2013-12-30 12:34 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1388406855-8809-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds initial code to handle audio IPC commands and defines a new
service id (10).
---
 android/a2dp.c    | 13 ++++++++++++-
 android/hal-msg.h |  3 ++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/android/a2dp.c b/android/a2dp.c
index 581d094..63f1f58 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -352,6 +352,9 @@ static sdp_record_t *a2dp_record(void)
 	return record;
 }
 
+static const struct ipc_handler audio_handlers[] = {
+};
+
 bool bt_a2dp_register(const bdaddr_t *addr)
 {
 	GError *err = NULL;
@@ -359,6 +362,8 @@ bool bt_a2dp_register(const bdaddr_t *addr)
 
 	DBG("");
 
+	audio_ipc_init();
+
 	bacpy(&adapter_addr, addr);
 
 	server = bt_io_listen(connect_cb, NULL, NULL, NULL, &err,
@@ -388,6 +393,9 @@ bool bt_a2dp_register(const bdaddr_t *addr)
 	ipc_register(HAL_SERVICE_ID_A2DP, cmd_handlers,
 						G_N_ELEMENTS(cmd_handlers));
 
+	ipc_register(HAL_SERVICE_ID_AUDIO, audio_handlers,
+						G_N_ELEMENTS(audio_handlers));
+
 	return true;
 
 fail:
@@ -411,8 +419,9 @@ void bt_a2dp_unregister(void)
 	g_slist_foreach(devices, a2dp_device_disconnected, NULL);
 	devices = NULL;
 
-
 	ipc_unregister(HAL_SERVICE_ID_A2DP);
+	ipc_unregister(HAL_SERVICE_ID_AUDIO);
+
 	bt_adapter_remove_record(record_id);
 	record_id = 0;
 
@@ -421,4 +430,6 @@ void bt_a2dp_unregister(void)
 		g_io_channel_unref(server);
 		server = NULL;
 	}
+
+	audio_ipc_cleanup();
 }
diff --git a/android/hal-msg.h b/android/hal-msg.h
index b14eced..1afb1bc 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -45,8 +45,9 @@ struct hal_hdr {
 #define HAL_SERVICE_ID_HEALTH		7
 #define HAL_SERVICE_ID_AVRCP		8
 #define HAL_SERVICE_ID_GATT		9
+#define HAL_SERVICE_ID_AUDIO		10
 
-#define HAL_SERVICE_ID_MAX HAL_SERVICE_ID_GATT
+#define HAL_SERVICE_ID_MAX HAL_SERVICE_ID_AUDIO
 
 /* Core Service */
 
-- 
1.8.4.2


^ permalink raw reply related

* [RFC BlueZ 1/9] android: Add initial code for audio IPC
From: Luiz Augusto von Dentz @ 2013-12-30 12:34 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This add initial code for listen and accept connections on the abstract
socket defined for the audio IPC.
---
 android/hal-msg.h |  1 +
 android/ipc.c     | 53 +++++++++++++++++++++++++++++++++++++++++++++++------
 android/ipc.h     |  3 +++
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index c351501..b14eced 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -24,6 +24,7 @@
 #define BLUEZ_HAL_MTU 1024
 
 static const char BLUEZ_HAL_SK_PATH[] = "\0bluez_hal_socket";
+static const char BLUEZ_AUDIO_SK_PATH[] = "\0bluez_audio_socket";
 
 struct hal_hdr {
 	uint8_t  service_id;
diff --git a/android/ipc.c b/android/ipc.c
index 9e8ccc3..6cdbf60 100644
--- a/android/ipc.c
+++ b/android/ipc.c
@@ -49,6 +49,7 @@ static struct service_handler services[HAL_SERVICE_ID_MAX + 1];
 
 static GIOChannel *cmd_io = NULL;
 static GIOChannel *notif_io = NULL;
+static GIOChannel *audio_io = NULL;
 
 static void ipc_handle_msg(const void *buf, ssize_t len)
 {
@@ -145,7 +146,8 @@ static gboolean notif_watch_cb(GIOChannel *io, GIOCondition cond,
 	return FALSE;
 }
 
-static GIOChannel *connect_hal(GIOFunc connect_cb)
+static GIOChannel *connect_hal(const char *path, size_t size,
+							GIOFunc connect_cb)
 {
 	struct sockaddr_un addr;
 	GIOCondition cond;
@@ -167,11 +169,11 @@ static GIOChannel *connect_hal(GIOFunc connect_cb)
 	memset(&addr, 0, sizeof(addr));
 	addr.sun_family = AF_UNIX;
 
-	memcpy(addr.sun_path, BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH));
+	memcpy(addr.sun_path, path, size);
 
 	if (connect(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
-		error("IPC: failed to connect HAL socket: %d (%s)", errno,
-							strerror(errno));
+		error("IPC: failed to connect HAL socket %s: %d (%s)", &path[1],
+							errno, strerror(errno));
 		g_io_channel_unref(io);
 		return NULL;
 	}
@@ -218,7 +220,8 @@ static gboolean cmd_connect_cb(GIOChannel *io, GIOCondition cond,
 		return FALSE;
 	}
 
-	notif_io = connect_hal(notif_connect_cb);
+	notif_io = connect_hal(BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH),
+							notif_connect_cb);
 	if (!notif_io)
 		raise(SIGTERM);
 
@@ -227,7 +230,8 @@ static gboolean cmd_connect_cb(GIOChannel *io, GIOCondition cond,
 
 void ipc_init(void)
 {
-	cmd_io = connect_hal(cmd_connect_cb);
+	cmd_io = connect_hal(BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH),
+							cmd_connect_cb);
 	if (!cmd_io)
 		raise(SIGTERM);
 }
@@ -338,3 +342,40 @@ void ipc_unregister(uint8_t service)
 	services[service].handler = NULL;
 	services[service].size = 0;
 }
+
+static gboolean audio_connect_cb(GIOChannel *io, GIOCondition cond,
+							gpointer user_data)
+{
+	DBG("");
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+		error("Audio IPC: socket connect failed, terminating");
+		audio_ipc_cleanup();
+		return FALSE;
+	}
+
+	cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
+
+	g_io_add_watch(audio_io, cond, cmd_watch_cb, NULL);
+
+	info("Audio IPC: successfully connected");
+
+	return FALSE;
+}
+
+void audio_ipc_init(void)
+{
+	audio_io = connect_hal(BLUEZ_AUDIO_SK_PATH, sizeof(BLUEZ_AUDIO_SK_PATH),
+							audio_connect_cb);
+	if (!cmd_io)
+		raise(SIGTERM);
+}
+
+void audio_ipc_cleanup(void)
+{
+	if (audio_io) {
+		g_io_channel_shutdown(audio_io, TRUE, NULL);
+		g_io_channel_unref(audio_io);
+		audio_io = NULL;
+	}
+}
diff --git a/android/ipc.h b/android/ipc.h
index 6cd102b..8e92811 100644
--- a/android/ipc.h
+++ b/android/ipc.h
@@ -37,3 +37,6 @@ void ipc_send_notif(uint8_t service_id, uint8_t opcode,  uint16_t len,
 void ipc_register(uint8_t service, const struct ipc_handler *handlers,
 								uint8_t size);
 void ipc_unregister(uint8_t service);
+
+void audio_ipc_init(void);
+void audio_ipc_cleanup(void);
-- 
1.8.4.2


^ permalink raw reply related

* Re: [RFC 0/6] Audio HAL plugin concept
From: Luiz Augusto von Dentz @ 2013-12-30 12:29 UTC (permalink / raw)
  To: Lukasz Rymanowski; +Cc: linux-bluetooth@vger.kernel.org, Johan Hedberg
In-Reply-To: <1388398647-25420-1-git-send-email-lukasz.rymanowski@tieto.com>

Hi Lukasz,

On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
<lukasz.rymanowski@tieto.com> wrote:
> Here is the set od some startup patches for audio hal plugin plus some trivial
> fixes which are not RFC actually.
>
> As we want audio plugin to register endpoint we need some special way to
> handle it having in mind that audio device is open on the system startup
> and closed on system down. We need to handle bluetooth daemon start/stop
> in between.
>
> Need your comment on that. For me it does not look bad but we still have
> time to consider having some fixed or configurable way (eg. some config file)
> to register endpoints in bluetooth daemon instead.
>
> Lukasz Rymanowski (6):
>   android/ipc: Remove not needed include
>   android/audio: Fix Makefile.am for libaudio
>   android: Minor fix to Android Bluetooth Audio protocol API doc
>   android: Keep stream_out example in the API doc
>   android/audio: Add wrapper stuct for audio structures
>   android/audio: Add listener thread on the Audio HAL socket
>
>  android/Makefile.am       |   5 +-
>  android/audio-ipc-api.txt |  18 ++--
>  android/hal-audio.c       | 242 ++++++++++++++++++++++++++++++++++++++--------
>  android/hal-audio.h       |  18 ++++
>  android/ipc.c             |   1 -
>  5 files changed, 231 insertions(+), 53 deletions(-)
>  create mode 100644 android/hal-audio.h
>
> --
> 1.8.4

Patches 1--4 are now upstream, thanks. For patches 5 and 6 lets
continue to discuss what is the best way to do the socket handling.


-- 
Luiz Augusto von Dentz

^ 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