2013-11-11 14:53:37

by Ravi kumar Veeramally

[permalink] [raw]
Subject: [PATCH 0/4] Implement missing PAN interface and notifications

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



2013-11-12 13:43:13

by Ravi kumar Veeramally

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

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.

2013-11-12 13:33:28

by Johan Hedberg

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

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

2013-11-11 14:53:40

by Ravi kumar Veeramally

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

---
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


2013-11-11 14:53:41

by Ravi kumar Veeramally

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

---
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


2013-11-11 14:53:39

by Ravi kumar Veeramally

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

---
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


2013-11-11 14:53:38

by Ravi kumar Veeramally

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

---
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