[PATCH v2 1/5] android: Define class of device as four bytes in IPC doc

[PATCH v2 1/5] android: Define class of device as four bytes in IPC doc

[Szymon Janc]

For PIN and SSP requests callback define CoD as 4 bytes. This will
allow HAL library to pass CoD direclty to callback. Will also match
how CoD is passed as property.
---
 android/hal-ipc-api.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
index dc0d067..e7af8a3 100644
--- a/android/hal-ipc-api.txt
+++ b/android/hal-ipc-api.txt
@@ -354,13 +354,13 @@ Notifications:
 
 		Notification parameters: Remote address (6 octets)
 		                         Remote name (249 octets)
-		                         Class of device (3 octets)
+		                         Class of device (4 octets)
 
 	Opcode 0x87 - SSP Request notification
 
 		Notification parameters: Remote address (6 octets)
 		                         Remote name (249 octets)
-		                         Class of device (3 octets)
+		                         Class of device (4 octets)
 		                         Pairing variant (1 octet)
 		                         Passkey (4 octets)
 
-- 
1.8.4.1

[PATCH v2 2/5] android: Update IPC headers to match SSP and PIN requests events

[Szymon Janc]

Name should be 249 bytes so it is always NULL terminated string.
Class of device is send as uint32. This will allow to make simple
passing of data in HAL library without need of copying data.
---
 android/hal-msg.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index a4eb2a8..80b47d6 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -356,15 +356,15 @@ struct hal_ev_discovery_state_changed {
 #define HAL_EV_PIN_REQUEST		0x86
 struct hal_ev_pin_request {
 	uint8_t bdaddr[6];
-	uint8_t name[249 - 1];
-	uint8_t class_of_dev[3];
+	uint8_t name[249];
+	uint32_t class_of_dev;
 } __attribute__((packed));
 
 #define HAL_EV_SSP_REQUEST		0x87
 struct hal_ev_ssp_request {
 	uint8_t  bdaddr[6];
-	uint8_t  name[249 - 1];
-	uint8_t  class_of_dev[3];
+	uint8_t  name[249];
+	uint32_t  class_of_dev;
 	uint8_t  pairing_variant;
 	uint32_t passkey;
 } __attribute__((packed));
-- 
1.8.4.1

[PATCH v2 3/5] android/hal: Add support for handling bond state change event

[Szymon Janc]

---
 android/hal-bluetooth.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 5f6dcbe..067f420 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -68,6 +68,17 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
 	bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
 }
 
+static void handle_bond_state_change(void *buf)
+{
+	struct hal_ev_bond_state_changed *ev = buf;
+	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
+
+	if (!bt_hal_cbacks->bond_state_changed_cb)
+		return;
+
+	bt_hal_cbacks->bond_state_changed_cb(ev->status, addr, ev->state);
+}
+
 void bt_thread_associate(void)
 {
 	if (bt_hal_cbacks->thread_evt_cb)
@@ -98,6 +109,9 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
 	case HAL_EV_ADAPTER_PROPS_CHANGED:
 		handle_adapter_props_changed(buf, len);
 		break;
+	case HAL_EV_BOND_STATE_CHANGED:
+		handle_bond_state_change(buf);
+		break;
 	default:
 		DBG("Unhandled callback opcode=0x%x", opcode);
 		break;
-- 
1.8.4.1

[PATCH v2 4/5] android/hal: Add support for handling pin request event

[Szymon Janc]

---
 android/hal-bluetooth.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 067f420..43db8cb 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -79,6 +79,16 @@ static void handle_bond_state_change(void *buf)
 	bt_hal_cbacks->bond_state_changed_cb(ev->status, addr, ev->state);
 }
 
+static void handle_pin_request(void *buf)
+{
+	struct hal_ev_pin_request *ev = buf;
+	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
+	bt_bdname_t *name = (bt_bdname_t *) ev->name;
+
+	if (bt_hal_cbacks->pin_request_cb)
+		bt_hal_cbacks->pin_request_cb(addr, name, ev->class_of_dev);
+}
+
 void bt_thread_associate(void)
 {
 	if (bt_hal_cbacks->thread_evt_cb)
@@ -112,6 +122,9 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
 	case HAL_EV_BOND_STATE_CHANGED:
 		handle_bond_state_change(buf);
 		break;
+	case HAL_EV_PIN_REQUEST:
+		handle_pin_request(buf);
+		break;
 	default:
 		DBG("Unhandled callback opcode=0x%x", opcode);
 		break;
-- 
1.8.4.1

[PATCH v2 5/5] android/hal: Add support for handling SSP request event

[Szymon Janc]

---
 android/hal-bluetooth.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 43db8cb..0c9e8c8 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -89,6 +89,18 @@ static void handle_pin_request(void *buf)
 		bt_hal_cbacks->pin_request_cb(addr, name, ev->class_of_dev);
 }
 
+static void handle_ssp_request(void *buf)
+{
+	struct hal_ev_ssp_request *ev = buf;
+	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
+	bt_bdname_t *name = (bt_bdname_t *) ev->name;
+
+	if (bt_hal_cbacks->ssp_request_cb)
+		bt_hal_cbacks->ssp_request_cb(addr, name, ev->class_of_dev,
+							ev->pairing_variant,
+							ev->passkey);
+}
+
 void bt_thread_associate(void)
 {
 	if (bt_hal_cbacks->thread_evt_cb)
@@ -125,6 +137,9 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
 	case HAL_EV_PIN_REQUEST:
 		handle_pin_request(buf);
 		break;
+	case HAL_EV_SSP_REQUEST:
+		handle_ssp_request(buf);
+		break;
 	default:
 		DBG("Unhandled callback opcode=0x%x", opcode);
 		break;
-- 
1.8.4.1

Re: [PATCH v2 3/5] android/hal: Add support for handling bond state change event

[Andrei Emeltchenko]

Hi Szymon,

On Tue, Oct 29, 2013 at 11:16:27AM +0100, Szymon Janc wrote:
> ---
>  android/hal-bluetooth.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> index 5f6dcbe..067f420 100644
> --- a/android/hal-bluetooth.c
> +++ b/android/hal-bluetooth.c
> @@ -68,6 +68,17 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
>  	bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
>  }
>  
> +static void handle_bond_state_change(void *buf)
> +{
> +	struct hal_ev_bond_state_changed *ev = buf;
> +	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
> +
> +	if (!bt_hal_cbacks->bond_state_changed_cb)
> +		return;
> +
> +	bt_hal_cbacks->bond_state_changed_cb(ev->status, addr, ev->state);

We shall use the same style like for other callbacks.

Best regards 
Andrei Emeltchenko 

> +}
> +
>  void bt_thread_associate(void)
>  {
>  	if (bt_hal_cbacks->thread_evt_cb)
> @@ -98,6 +109,9 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
>  	case HAL_EV_ADAPTER_PROPS_CHANGED:
>  		handle_adapter_props_changed(buf, len);
>  		break;
> +	case HAL_EV_BOND_STATE_CHANGED:
> +		handle_bond_state_change(buf);
> +		break;
>  	default:
>  		DBG("Unhandled callback opcode=0x%x", opcode);
>  		break;
> -- 
> 1.8.4.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/5] android/hal: Add support for handling bond state change event

[Szymon Janc]

Hi Andrei,

On Tuesday 29 of October 2013 12:27:24 Andrei Emeltchenko wrote:
> Hi Szymon,
> 
> On Tue, Oct 29, 2013 at 11:16:27AM +0100, Szymon Janc wrote:
> > ---
> >  android/hal-bluetooth.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> > index 5f6dcbe..067f420 100644
> > --- a/android/hal-bluetooth.c
> > +++ b/android/hal-bluetooth.c
> > @@ -68,6 +68,17 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
> >  	bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
> >  }
> >  
> > +static void handle_bond_state_change(void *buf)
> > +{
> > +	struct hal_ev_bond_state_changed *ev = buf;
> > +	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
> > +
> > +	if (!bt_hal_cbacks->bond_state_changed_cb)
> > +		return;
> > +
> > +	bt_hal_cbacks->bond_state_changed_cb(ev->status, addr, ev->state);
> 
> We shall use the same style like for other callbacks.

In that case reverting check allow to not break function call into 2 lines.
I find it more readable (and there will be reverted check for less trivial
callbacks e.g. with properties)

-- 
BR
Szymon Janc

Re: [PATCH v2 3/5] android/hal: Add support for handling bond state change event

[Andrei Emeltchenko]

On Tue, Oct 29, 2013 at 11:32:29AM +0100, Szymon Janc wrote:
> Hi Andrei,
> 
> On Tuesday 29 of October 2013 12:27:24 Andrei Emeltchenko wrote:
> > Hi Szymon,
> > 
> > On Tue, Oct 29, 2013 at 11:16:27AM +0100, Szymon Janc wrote:
> > > ---
> > >  android/hal-bluetooth.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> > > index 5f6dcbe..067f420 100644
> > > --- a/android/hal-bluetooth.c
> > > +++ b/android/hal-bluetooth.c
> > > @@ -68,6 +68,17 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
> > >  	bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
> > >  }
> > >  
> > > +static void handle_bond_state_change(void *buf)
> > > +{
> > > +	struct hal_ev_bond_state_changed *ev = buf;
> > > +	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
> > > +
> > > +	if (!bt_hal_cbacks->bond_state_changed_cb)
> > > +		return;
> > > +
> > > +	bt_hal_cbacks->bond_state_changed_cb(ev->status, addr, ev->state);
> > 
> > We shall use the same style like for other callbacks.
> 
> In that case reverting check allow to not break function call into 2 lines.
> I find it more readable (and there will be reverted check for less trivial
> callbacks e.g. with properties)

Those checks at least need to be consistent. Your next patch use other
way. We shall agree about best way for this check, maybe some #define like
I have in my patches sent some time ago? We could also add debug traces
then.

Best regards 
Andrei Emeltchenko 

Re: [PATCH v2 3/5] android/hal: Add support for handling bond state change event

[Szymon Janc]

Hi Andrei,

> On Tue, Oct 29, 2013 at 11:32:29AM +0100, Szymon Janc wrote:
> > Hi Andrei,
> > 
> > On Tuesday 29 of October 2013 12:27:24 Andrei Emeltchenko wrote:
> > > Hi Szymon,
> > > 
> > > On Tue, Oct 29, 2013 at 11:16:27AM +0100, Szymon Janc wrote:
> > > > ---
> > > >  android/hal-bluetooth.c | 14 ++++++++++++++
> > > >  1 file changed, 14 insertions(+)
> > > > 
> > > > diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> > > > index 5f6dcbe..067f420 100644
> > > > --- a/android/hal-bluetooth.c
> > > > +++ b/android/hal-bluetooth.c
> > > > @@ -68,6 +68,17 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
> > > >  	bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
> > > >  }
> > > >  
> > > > +static void handle_bond_state_change(void *buf)
> > > > +{
> > > > +	struct hal_ev_bond_state_changed *ev = buf;
> > > > +	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
> > > > +
> > > > +	if (!bt_hal_cbacks->bond_state_changed_cb)
> > > > +		return;
> > > > +
> > > > +	bt_hal_cbacks->bond_state_changed_cb(ev->status, addr, ev->state);
> > > 
> > > We shall use the same style like for other callbacks.
> > 
> > In that case reverting check allow to not break function call into 2 lines.
> > I find it more readable (and there will be reverted check for less trivial
> > callbacks e.g. with properties)
> 
> Those checks at least need to be consistent. Your next patch use other
> way. We shall agree about best way for this check, maybe some #define like
> I have in my patches sent some time ago? We could also add debug traces
> then.

In next patch reverting check wouldn't give any benefit as function call
doesn't fit 1 line. Not that this matter a lot since this is trivial callback
handler. I'll send v3 with this check changed if this really bothers you :)

As for macro, I would prefer to not have any as this makes code harder to
follow especially for non-trivial handlers  (where you can return early in case
callback is null).

(I was even thinking about open-coding interface_ready() functions... )

-- 
BR
Szymon Janc