[PATCH_v2 1/4] android/hal-pan: Return error in case of unsupported PAN roles

[PATCH_v2 1/4] android/hal-pan: Return error in case of unsupported PAN roles

[Ravi kumar Veeramally]

---
 android/hal-pan.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/android/hal-pan.c b/android/hal-pan.c
index 2bc560e..a2e6060 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -77,6 +77,9 @@ static bt_status_t pan_enable(int local_role)
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
 
+	if (!(local_role == BTPAN_ROLE_PANU || local_role == BTPAN_ROLE_PANNAP))
+		return BT_STATUS_UNSUPPORTED;
+
 	cmd.local_role = local_role;
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_PAN, HAL_OP_PAN_ENABLE,
@@ -112,6 +115,20 @@ static bt_status_t pan_connect(const bt_bdaddr_t *bd_addr, int local_role,
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
 
+	switch (local_role) {
+	case BTPAN_ROLE_PANNAP:
+		if (remote_role != BTPAN_ROLE_PANU)
+			return BT_STATUS_UNSUPPORTED;
+		break;
+	case BTPAN_ROLE_PANU:
+		if (remote_role != BTPAN_ROLE_PANNAP &&
+						remote_role != BTPAN_ROLE_PANU)
+			return BT_STATUS_UNSUPPORTED;
+		break;
+	default:
+		return BT_STATUS_UNSUPPORTED;
+	}
+
 	memcpy(cmd.bdaddr, bd_addr, sizeof(cmd.bdaddr));
 	cmd.local_role = local_role;
 	cmd.remote_role = remote_role;
-- 
1.8.3.2

[PATCH_v2 2/4] android: Handle multiple init(register) and cleanup(unregister) calls properly

[Ravi kumar Veeramally]

This can be tested with haltest.
---
 android/a2dp.c      | 6 ++++++
 android/bluetooth.c | 6 ++++++
 android/hidhost.c   | 6 ++++++
 android/pan.c       | 6 ++++++
 4 files changed, 24 insertions(+)

diff --git a/android/a2dp.c b/android/a2dp.c
index 74d0082..a9e7c65 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -332,6 +332,9 @@ bool bt_a2dp_register(int sk, const bdaddr_t *addr)
 
 	DBG("");
 
+	if (notification_sk >= 0)
+		return false;
+
 	bacpy(&adapter_addr, addr);
 
 	server = bt_io_listen(connect_cb, NULL, NULL, NULL, &err,
@@ -365,6 +368,9 @@ void bt_a2dp_unregister(void)
 {
 	DBG("");
 
+	if (notification_sk < 0)
+		return;
+
 	notification_sk = -1;
 
 	bt_adapter_remove_record(record_id);
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 7dc2ec3..11b9d76 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -2275,6 +2275,9 @@ bool bt_bluetooth_register(int sk)
 {
 	DBG("");
 
+	if (notification_sk >= 0)
+		return false;
+
 	notification_sk = sk;
 
 	return true;
@@ -2284,5 +2287,8 @@ void bt_bluetooth_unregister(void)
 {
 	DBG("");
 
+	if (notification_sk < 0)
+		return;
+
 	notification_sk = -1;
 }
diff --git a/android/hidhost.c b/android/hidhost.c
index 842b8ad..df21f81 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -1190,6 +1190,9 @@ bool bt_hid_register(int sk, const bdaddr_t *addr)
 
 	DBG("");
 
+	if (notification_sk >= 0)
+		return false;
+
 	bacpy(&adapter_addr, addr);
 
 	ctrl_io = bt_io_listen(connect_cb, NULL, NULL, NULL, &err,
@@ -1224,6 +1227,9 @@ void bt_hid_unregister(void)
 {
 	DBG("");
 
+	if (notification_sk < 0)
+		return;
+
 	notification_sk = -1;
 
 	if (ctrl_io) {
diff --git a/android/pan.c b/android/pan.c
index fba86b8..ada458a 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -95,6 +95,9 @@ bool bt_pan_register(int sk, const bdaddr_t *addr)
 {
 	DBG("");
 
+	if (notification_sk >= 0)
+		return false;
+
 	notification_sk = sk;
 
 	return true;
@@ -104,5 +107,8 @@ void bt_pan_unregister(void)
 {
 	DBG("");
 
+	if (notification_sk < 0)
+		return;
+
 	notification_sk = -1;
 }
-- 
1.8.3.2

[PATCH_v2 3/4] android/hidhost: Handle error case properly in interrupt_connect_cb

[Ravi kumar Veeramally]

In case of conn_err in interrupt_connect_cb, device is freed but
connection status is not notified. Declared a local variable and
handled error case properly in case of conn_err and uhid failures.
Now connection status notified before freeing device.
---
 android/hidhost.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/android/hidhost.c b/android/hidhost.c
index df21f81..049dd6d 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -522,7 +522,6 @@ static int uhid_create(struct hid_device *dev)
 	dev->uhid_fd = open(UHID_DEVICE_FILE, O_RDWR | O_CLOEXEC);
 	if (dev->uhid_fd < 0) {
 		error("Failed to open uHID device: %s", strerror(errno));
-		bt_hid_notify_state(dev, HAL_HIDHOST_STATE_NO_HID);
 		return -errno;
 	}
 
@@ -541,7 +540,6 @@ static int uhid_create(struct hid_device *dev)
 		error("Failed to create uHID device: %s", strerror(errno));
 		close(dev->uhid_fd);
 		dev->uhid_fd = -1;
-		bt_hid_notify_state(dev, HAL_HIDHOST_STATE_NO_HID);
 		return -errno;
 	}
 
@@ -559,16 +557,20 @@ static void interrupt_connect_cb(GIOChannel *chan, GError *conn_err,
 							gpointer user_data)
 {
 	struct hid_device *dev = user_data;
+	uint8_t state;
 
 	DBG("");
 
 	if (conn_err) {
 		error("%s", conn_err->message);
+		state = HAL_HIDHOST_STATE_FAILED;
 		goto failed;
 	}
 
-	if (uhid_create(dev) < 0)
+	if (uhid_create(dev) < 0) {
+		state = HAL_HIDHOST_STATE_NO_HID;
 		goto failed;
+	}
 
 	dev->intr_watch = g_io_add_watch(dev->intr_io,
 				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
@@ -579,6 +581,7 @@ static void interrupt_connect_cb(GIOChannel *chan, GError *conn_err,
 	return;
 
 failed:
+	bt_hid_notify_state(dev, state);
 	hid_device_free(dev);
 }
 
-- 
1.8.3.2

[PATCH_v2 4/4] android/hidhost: Free all connected devices in profile cleanup call

[Ravi kumar Veeramally]

This can be easily verified with haltest tool.
---
 android/hidhost.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/android/hidhost.c b/android/hidhost.c
index 049dd6d..502b10b 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -1226,6 +1226,14 @@ bool bt_hid_register(int sk, const bdaddr_t *addr)
 	return true;
 }
 
+static void free_hid_devices(gpointer data, gpointer user_data)
+{
+	struct hid_device *dev = data;
+
+	bt_hid_notify_state(dev, HAL_HIDHOST_STATE_DISCONNECTED);
+	hid_device_free(dev);
+}
+
 void bt_hid_unregister(void)
 {
 	DBG("");
@@ -1233,6 +1241,8 @@ void bt_hid_unregister(void)
 	if (notification_sk < 0)
 		return;
 
+	g_slist_foreach(devices, free_hid_devices, NULL);
+	devices = NULL;
 	notification_sk = -1;
 
 	if (ctrl_io) {
-- 
1.8.3.2

Re: [PATCH_v2 1/4] android/hal-pan: Return error in case of unsupported PAN roles

[Johan Hedberg]

Hi Ravi,

On Tue, Nov 19, 2013, Ravi kumar Veeramally wrote:
> ---
>  android/hal-pan.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

All four patches have been applied. Thanks.

Johan

Re: [PATCH_v2 2/4] android: Handle multiple init(register) and cleanup(unregister) calls properly

[Szymon Janc]

Hi Ravi,

On Tue, Nov 19, 2013 at 3:56 PM, Ravi kumar Veeramally
<ravikumar.veeramally@linux.intel.com> wrote:
> This can be tested with haltest.

I think this should be already handled by Core service. Were you able
to get success response from daemon on multiple sequent init or
cleanup calls?

btw: I think we should fix this on hal side as well, currently only
bluetooth hal is handling error from daemon correctly. There is also
question if we should return success on second init call...

-- 
BR
Szymon Janc

Re: [PATCH_v2 2/4] android: Handle multiple init(register) and cleanup(unregister) calls properly

[Luiz Augusto von Dentz]

Hi Szymon,

On Wed, Nov 20, 2013 at 9:02 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi Ravi,
>
> On Tue, Nov 19, 2013 at 3:56 PM, Ravi kumar Veeramally
> <ravikumar.veeramally@linux.intel.com> wrote:
>> This can be tested with haltest.
>
> I think this should be already handled by Core service. Were you able
> to get success response from daemon on multiple sequent init or
> cleanup calls?
>
> btw: I think we should fix this on hal side as well, currently only
> bluetooth hal is handling error from daemon correctly. There is also
> question if we should return success on second init call...

I would just return success if init was already called and should
probably be done in a central place, perhaps in the HAL side.


-- 
Luiz Augusto von Dentz