[PATCH 0/4] Implement missing PAN interface and notifications

[PATCH 0/4] Implement missing PAN interface and notifications

[Ravi kumar Veeramally]

Patch set implements missing cleanup interface and adds event structs
to hal-msg.h and implements connection and control state notifications.

Ravi kumar Veeramally (4):
  android/pan: Add PAN related defines and event struct to hsl-msg
    header
  android/pan: Add PAN cleanup interface implementation
  android/pan: Add notify method to PAN notifications
  android/pan: Handle connection and control state notifications

 android/hal-msg.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
 android/hal-pan.c | 47 +++++++++++++++++++++++++++++++++++++++++++----
 android/hal.h     |  1 +
 3 files changed, 88 insertions(+), 4 deletions(-)

-- 
1.8.3.2

[PATCH 1/4] android/pan: Add PAN related defines and event struct to hsl-msg header

[Ravi kumar Veeramally]

---
 android/hal-msg.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index 569c8ea..00d6e39 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -317,6 +317,33 @@ struct hal_cmd_a2dp_disconnect {
 
 /* PAN HAL API */
 
+/* PAN Roles */
+#define HAL_PAN_ROLE_NONE	0x00
+#define HAL_PAN_ROLE_NAP	0x01
+#define HAL_PAN_ROLE_PANU	0x02
+
+/* PAN Control states */
+#define HAL_PAN_CTRL_ENABLED	0x00
+#define HAL_PAN_CTRL_DISABLED	0x01
+
+/* PAN Connection states */
+#define HAL_PAN_STATE_CONNECTED		0x00
+#define HAL_PAN_STATE_CONNECTING	0x01
+#define HAL_PAN_STATE_DISCONNECTED	0x02
+#define HAL_PAN_STATE_DISCONNECTING	0x03
+
+/* PAN status values */
+#define HAL_PAN_STATUS_FAIL		0x01
+#define HAL_PAN_STATUS_NOT_READY	0x02
+#define HAL_PAN_STATUS_NO_MEMORY	0x03
+#define HAL_PAN_STATUS_BUSY		0x04
+#define HAL_PAN_STATUS_DONE		0x05
+#define HAL_PAN_STATUS_UNSUPORTED	0x06
+#define HAL_PAN_STATUS_INVAL		0x07
+#define HAL_PAN_STATUS_UNHANDLED	0x08
+#define HAL_PAN_STATUS_AUTH_FAILED	0x09
+#define HAL_PAN_STATUS_DEVICE_DOWN	0x0A
+
 #define HAL_OP_PAN_ENABLE	0x01
 struct hal_cmd_pan_enable {
 	uint8_t local_role;
@@ -486,6 +513,23 @@ struct hal_ev_hidhost_virtual_unplug {
 	uint8_t  status;
 } __attribute__((packed));
 
+#define HAL_EV_PAN_CONN_STATE			0x81
+struct hal_ev_pan_conn_state {
+	uint8_t  state;
+	uint8_t  status;
+	uint8_t  bdaddr[6];
+	uint8_t  local_role;
+	uint8_t  remote_role;
+} __attribute__((packed));
+
+#define HAL_EV_PAN_CTRL_STATE			0x82
+struct hal_ev_pan_ctrl_state {
+	uint8_t  state;
+	uint8_t  status;
+	uint8_t  local_role;
+	uint8_t  name[17];
+} __attribute__((packed));
+
 #define HAL_EV_A2DP_CONNECTION_STATE	0x81
 struct hal_ev_a2dp_connection_state {
 	uint8_t state;
-- 
1.8.3.2

Re: [PATCH 1/4] android/pan: Add PAN related defines and event struct to hsl-msg header

[Johan Hedberg]

Hi Ravi,

On Mon, Nov 11, 2013, Ravi kumar Veeramally wrote:
> +#define HAL_EV_PAN_CONN_STATE			0x81
> +struct hal_ev_pan_conn_state {
> +	uint8_t  state;
> +	uint8_t  status;
> +	uint8_t  bdaddr[6];
> +	uint8_t  local_role;
> +	uint8_t  remote_role;
> +} __attribute__((packed));
> +
> +#define HAL_EV_PAN_CTRL_STATE			0x82
> +struct hal_ev_pan_ctrl_state {
> +	uint8_t  state;
> +	uint8_t  status;
> +	uint8_t  local_role;
> +	uint8_t  name[17];
> +} __attribute__((packed));

These are in different in hal-ipc-api.txt:

        Opcode 0x81 - Control State notification
	Opcode 0x82 - Connection State notification

You really need to start paying more attention to details like this. As
long as you don't do it it means I need to spend more time verifying
every detail in your patches, which in turn means that it takes longer
before your patches get reviewed and eventually go upstream.

Johan

Re: [PATCH 1/4] android/pan: Add PAN related defines and event struct to hsl-msg header

[Ravi Kumar Veeramally]

Hi Johan,

On 11/12/2013 03:33 PM, Johan Hedberg wrote:
> Hi Ravi,
>
> On Mon, Nov 11, 2013, Ravi kumar Veeramally wrote:
>> +#define HAL_EV_PAN_CONN_STATE			0x81
>> +struct hal_ev_pan_conn_state {
>> +	uint8_t  state;
>> +	uint8_t  status;
>> +	uint8_t  bdaddr[6];
>> +	uint8_t  local_role;
>> +	uint8_t  remote_role;
>> +} __attribute__((packed));
>> +
>> +#define HAL_EV_PAN_CTRL_STATE			0x82
>> +struct hal_ev_pan_ctrl_state {
>> +	uint8_t  state;
>> +	uint8_t  status;
>> +	uint8_t  local_role;
>> +	uint8_t  name[17];
>> +} __attribute__((packed));
> These are in different in hal-ipc-api.txt:
>
>          Opcode 0x81 - Control State notification
> 	Opcode 0x82 - Connection State notification
>
> You really need to start paying more attention to details like this. As
> long as you don't do it it means I need to spend more time verifying
> every detail in your patches, which in turn means that it takes longer
> before your patches get reviewed and eventually go upstream.
>
> Johan
>
  Really sorry about that and wasting your time. I will keep that in mind.
  Sorry once again.

  Regards,
  Ravi.

[PATCH 2/4] android/pan: Add PAN cleanup interface implementation

[Ravi kumar Veeramally]

---
 android/hal-pan.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/android/hal-pan.c b/android/hal-pan.c
index cacc6c0..a560055 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -114,16 +114,18 @@ static bt_status_t pan_init(const btpan_callbacks_t *callbacks)
 
 static void pan_cleanup()
 {
+	struct hal_cmd_register_module cmd;
+
 	DBG("");
 
 	if (!interface_ready())
 		return;
 
-	/* TODO: disable service */
-
-	/* TODO: stop PAN thread */
-
 	cbs = NULL;
+	cmd.service_id = HAL_SERVICE_ID_PAN;
+
+	hal_ipc_cmd(HAL_SERVICE_ID_PAN, HAL_OP_UNREGISTER_MODULE,
+					sizeof(cmd), &cmd, 0, NULL, NULL);
 }
 
 static btpan_interface_t pan_if = {
-- 
1.8.3.2

[PATCH 3/4] android/pan: Add notify method to PAN notifications

[Ravi kumar Veeramally]

---
 android/hal-pan.c | 6 ++++++
 android/hal.h     | 1 +
 2 files changed, 7 insertions(+)

diff --git a/android/hal-pan.c b/android/hal-pan.c
index a560055..76f5159 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -31,6 +31,12 @@ static bool interface_ready(void)
 	return cbs != NULL;
 }
 
+void bt_notify_pan(uint16_t opcode, void *buf, uint16_t len)
+{
+	if (!interface_ready())
+		return;
+}
+
 static bt_status_t pan_enable(int local_role)
 {
 	struct hal_cmd_pan_enable cmd;
diff --git a/android/hal.h b/android/hal.h
index 2ce7932..be5d491 100644
--- a/android/hal.h
+++ b/android/hal.h
@@ -31,3 +31,4 @@ void bt_thread_associate(void);
 void bt_thread_disassociate(void);
 void bt_notify_hidhost(uint16_t opcode, void *buf, uint16_t len);
 void bt_notify_a2dp(uint16_t opcode, void *buf, uint16_t len);
+void bt_notify_pan(uint16_t opcode, void *buf, uint16_t len);
-- 
1.8.3.2

[PATCH 4/4] android/pan: Handle connection and control state notifications

[Ravi kumar Veeramally]

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

diff --git a/android/hal-pan.c b/android/hal-pan.c
index 76f5159..4173925 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -31,10 +31,41 @@ static bool interface_ready(void)
 	return cbs != NULL;
 }
 
+static void handle_conn_state(void *buf)
+{
+	struct hal_ev_pan_conn_state *ev = buf;
+
+	if (cbs->connection_state_cb)
+		cbs->connection_state_cb(ev->state, ev->status,
+					(bt_bdaddr_t *) ev->bdaddr,
+					ev->local_role, ev->remote_role);
+}
+
+static void handle_ctrl_state(void *buf)
+{
+	struct hal_ev_pan_ctrl_state *ev = buf;
+
+	if (cbs->control_state_cb)
+		cbs->control_state_cb(ev->state, ev->status,
+					ev->local_role, (char *)ev->name);
+}
+
 void bt_notify_pan(uint16_t opcode, void *buf, uint16_t len)
 {
 	if (!interface_ready())
 		return;
+
+	switch (opcode) {
+	case HAL_EV_PAN_CONN_STATE:
+		handle_conn_state(buf);
+		break;
+	case HAL_EV_PAN_CTRL_STATE:
+		handle_ctrl_state(buf);
+		break;
+	default:
+		DBG("Unhandled callback opcode=0x%x", opcode);
+		break;
+	}
 }
 
 static bt_status_t pan_enable(int local_role)
-- 
1.8.3.2