[PATCH v3 0/7] Clear the ground for AG card implementation

[PATCH v3 0/7] Clear the ground for AG card implementation

[Paulo Borges]

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

v3 changelog:

* add patches that move hfp version to hfp.h.

***

* patches 1 to 3 stores and closes remote connections.

* patches 4 to 7 moves HFP version definitions to hfp.h.

Paulo Borges (7):
  hfp_ag_bluez5: Create a hash to store connections
  hfp_ag_bluez5: Store fd in hash when connect
  hfp_ag_bluez5: Remove fd from hash when disconnect
  phonesim: Include hfp.h
  hfp_ag_bluez5: Include hfp.h
  hfp: Move HFP versions definitions to hfp.h
  hfp_ag_bluez5: Remove unused includes

 drivers/hfpmodem/slc.h  |    6 -----
 plugins/hfp_ag_bluez5.c |   65 ++++++++++++++++++++++++++++++++++++++++++-----
 plugins/phonesim.c      |    1 +
 src/hfp.h               |    6 +++++
 4 files changed, 65 insertions(+), 13 deletions(-)

-- 
1.7.9.5

[PATCH v3 1/7] hfp_ag_bluez5: Create a hash to store connections

[Paulo Borges]

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

We need to store active connections so we can disconnect them at
RequestDisconnect().
---
 plugins/hfp_ag_bluez5.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/plugins/hfp_ag_bluez5.c b/plugins/hfp_ag_bluez5.c
index 64ea8ca..ba84d90 100644
--- a/plugins/hfp_ag_bluez5.c
+++ b/plugins/hfp_ag_bluez5.c
@@ -51,6 +51,16 @@
 static guint modemwatch_id;
 static GList *modems;
 static GHashTable *sim_hash = NULL;
+static GHashTable *connection_hash;
+
+static void connection_removed_notify(gpointer data)
+{
+	int fd = GPOINTER_TO_INT(data);
+
+	DBG("%d", fd);
+
+	close(fd);
+}
 
 static DBusMessage *profile_new_connection(DBusConnection *conn,
 						DBusMessage *msg, void *data)
@@ -254,6 +264,9 @@ static int hfp_ag_init(void)
 	modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
 	__ofono_modem_foreach(call_modemwatch, NULL);
 
+	connection_hash = g_hash_table_new_full(g_str_hash, g_str_equal,
+					g_free, connection_removed_notify);
+
 	return 0;
 }
 
@@ -265,6 +278,8 @@ static void hfp_ag_exit(void)
 	g_dbus_unregister_interface(conn, HFP_AG_EXT_PROFILE_PATH,
 						BLUEZ_PROFILE_INTERFACE);
 
+	g_hash_table_destroy(connection_hash);
+
 	g_list_free(modems);
 	g_hash_table_foreach_remove(sim_hash, sim_watch_remove, NULL);
 	g_hash_table_destroy(sim_hash);
-- 
1.7.9.5

[PATCH v3 2/7] hfp_ag_bluez5: Store fd in hash when connect

[Paulo Borges]

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

The file descriptor is stored after the emulator is registered.
---
 plugins/hfp_ag_bluez5.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/plugins/hfp_ag_bluez5.c b/plugins/hfp_ag_bluez5.c
index ba84d90..e53709d 100644
--- a/plugins/hfp_ag_bluez5.c
+++ b/plugins/hfp_ag_bluez5.c
@@ -113,6 +113,8 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
 	}
 
 	ofono_emulator_register(em, fd);
+	g_hash_table_insert(connection_hash, g_strdup(device),
+							GINT_TO_POINTER(fd));
 
 	return dbus_message_new_method_return(msg);
 
-- 
1.7.9.5

[PATCH v3 3/7] hfp_ag_bluez5: Remove fd from hash when disconnect

[Paulo Borges]

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

When a RequestDisconnect() is received, the file descriptor must be
closed. This way, the related emulator will be freed.

But the emulator can also be disconnected automatically if its
GAtServer closes. In this case, the hash will hold an invalid file
descriptor.

So, after the emulator is registered, we watch for G_IO_HUP in
the corresponding file descriptor. If this condition happens, we
can safely remove the file descriptor from the hash.
---
 plugins/hfp_ag_bluez5.c |   43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/plugins/hfp_ag_bluez5.c b/plugins/hfp_ag_bluez5.c
index e53709d..cc59cf7 100644
--- a/plugins/hfp_ag_bluez5.c
+++ b/plugins/hfp_ag_bluez5.c
@@ -62,11 +62,24 @@ static void connection_removed_notify(gpointer data)
 	close(fd);
 }
 
+static gboolean emulator_disconnected_cb(GIOChannel *io, GIOCondition cond,
+								gpointer data)
+{
+	char *device = data;
+
+	DBG("Remove %s", device);
+
+	g_hash_table_remove(connection_hash, device);
+
+	return FALSE;
+}
+
 static DBusMessage *profile_new_connection(DBusConnection *conn,
 						DBusMessage *msg, void *data)
 {
 	DBusMessageIter entry;
 	const char *device;
+	GIOChannel *io;
 	int fd;
 	struct ofono_emulator *em;
 	struct ofono_modem *modem;
@@ -113,6 +126,12 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
 	}
 
 	ofono_emulator_register(em, fd);
+
+	io = g_io_channel_unix_new(fd);
+	g_io_add_watch_full(io, G_PRIORITY_DEFAULT, G_IO_HUP,
+			emulator_disconnected_cb, g_strdup(device), g_free);
+	g_io_channel_unref(io);
+
 	g_hash_table_insert(connection_hash, g_strdup(device),
 							GINT_TO_POINTER(fd));
 
@@ -146,11 +165,29 @@ static DBusMessage *profile_cancel(DBusConnection *conn,
 static DBusMessage *profile_disconnection(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
+	DBusMessageIter iter;
+	const char *device;
+
 	DBG("Profile handler RequestDisconnection");
 
-	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
-					".NotImplemented",
-					"Implementation not provided");
+	if (!dbus_message_iter_init(msg, &iter))
+		goto invalid;
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_OBJECT_PATH)
+		goto invalid;
+
+	dbus_message_iter_get_basic(&iter, &device);
+
+	DBG("%s", device);
+
+	if (!g_hash_table_remove(connection_hash, device))
+		goto invalid;
+
+	return dbus_message_new_method_return(msg);
+
+invalid:
+	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE ".Rejected",
+					"Invalid arguments in method call");
 }
 
 static const GDBusMethodTable profile_methods[] = {
-- 
1.7.9.5

Re: [PATCH v3 3/7] hfp_ag_bluez5: Remove fd from hash when disconnect

[Denis Kenzior]

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

Hi Paulo,

On 04/15/2013 04:41 PM, Paulo Borges wrote:
> When a RequestDisconnect() is received, the file descriptor must be
> closed. This way, the related emulator will be freed.

Actually you need to call shutdown() on it.  When multiple references to 
the file descriptor exist, close() simply deletes a reference.  It does 
not close the actual socket.

>
> But the emulator can also be disconnected automatically if its
> GAtServer closes. In this case, the hash will hold an invalid file
> descriptor.
>
> So, after the emulator is registered, we watch for G_IO_HUP in
> the corresponding file descriptor. If this condition happens, we
> can safely remove the file descriptor from the hash.

Overall I like this approach much better than the previous one.  I'd 
still re-arrange the patch set as follows:

- init/free hash table (same as patch 1)
- G_IO_HUP changes
- RequestDisconnect implementation

> ---
>   plugins/hfp_ag_bluez5.c |   43 ++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 40 insertions(+), 3 deletions(-)
>

Regards,
-Denis

Re: [PATCH v3 3/7] hfp_ag_bluez5: Remove fd from hash when disconnect

[Paulo Borges]

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

Hi Denis,

On Thu, Apr 18, 2013 at 10:53 AM, Denis Kenzior <denkenz@gmail.com> wrote:

> Hi Paulo,
>
>
> On 04/15/2013 04:41 PM, Paulo Borges wrote:
>
>> [...]
>
>
> Actually you need to call shutdown() on it.  When multiple references to
> the file descriptor exist, close() simply deletes a reference.  It does not
> close the actual socket.
>
> [...]
>
>
> Overall I like this approach much better than the previous one.  I'd still
> re-arrange the patch set as follows:
>
> - init/free hash table (same as patch 1)
> - G_IO_HUP changes
> - RequestDisconnect implementation
>
>
Ok, I'll re-send this patches soon.

-- 
Cheers,
Paulo.

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1309 bytes --]

[PATCH v3 4/7] phonesim: Include hfp.h

[Paulo Borges]

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

---
 plugins/phonesim.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 26f96d0..b56b3ca 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -65,6 +65,7 @@
 #include <drivers/atmodem/atutil.h>
 #include <drivers/hfpmodem/slc.h>
 
+#include "hfp.h"
 #include "ofono.h"
 
 static const char *none_prefix[] = { NULL };
-- 
1.7.9.5

Re: [PATCH v3 4/7] phonesim: Include hfp.h

[Denis Kenzior]

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

Hi Paulo,

On 04/15/2013 04:41 PM, Paulo Borges wrote:
> ---
>   plugins/phonesim.c |    1 +
>   1 file changed, 1 insertion(+)
>

Patch has been applied, thanks.

Regards,
-Denis

[PATCH v3 5/7] hfp_ag_bluez5: Include hfp.h

[Paulo Borges]

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

---
 plugins/hfp_ag_bluez5.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/plugins/hfp_ag_bluez5.c b/plugins/hfp_ag_bluez5.c
index cc59cf7..3b76179 100644
--- a/plugins/hfp_ag_bluez5.c
+++ b/plugins/hfp_ag_bluez5.c
@@ -40,6 +40,7 @@
 
 #include <drivers/hfpmodem/slc.h>
 
+#include "hfp.h"
 #include "bluez5.h"
 
 #ifndef DBUS_TYPE_UNIX_FD
-- 
1.7.9.5

Re: [PATCH v3 5/7] hfp_ag_bluez5: Include hfp.h

[Denis Kenzior]

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

Hi Paulo,

On 04/15/2013 04:41 PM, Paulo Borges wrote:
> ---
>   plugins/hfp_ag_bluez5.c |    1 +
>   1 file changed, 1 insertion(+)
>

Patch has been applied, thanks.

Regards,
-Denis

[PATCH v3 6/7] hfp: Move HFP versions definitions to hfp.h

[Paulo Borges]

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

hfp.h should store all HFP related definitions.
---
 drivers/hfpmodem/slc.h |    6 ------
 src/hfp.h              |    6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hfpmodem/slc.h b/drivers/hfpmodem/slc.h
index b71ffe9..dd6f926 100644
--- a/drivers/hfpmodem/slc.h
+++ b/drivers/hfpmodem/slc.h
@@ -27,12 +27,6 @@
 #define AG_CHLD_3	0x20
 #define AG_CHLD_4	0x40
 
-enum hfp_version {
-	HFP_VERSION_1_5 =	0x0105,
-	HFP_VERSION_1_6 =	0x0106,
-	HFP_VERSION_LATEST =	HFP_VERSION_1_6,
-};
-
 enum hfp_indicator {
 	HFP_INDICATOR_SERVICE = 0,
 	HFP_INDICATOR_CALL,
diff --git a/src/hfp.h b/src/hfp.h
index 64a7055..b54de56 100644
--- a/src/hfp.h
+++ b/src/hfp.h
@@ -50,3 +50,9 @@ enum hfp_codec {
 	HFP_CODEC_CVSD = 0x01,
 	HFP_CODEC_MSBC = 0x02,
 };
+
+enum hfp_version {
+	HFP_VERSION_1_5 =	0x0105,
+	HFP_VERSION_1_6 =	0x0106,
+	HFP_VERSION_LATEST =	HFP_VERSION_1_6,
+};
-- 
1.7.9.5

Re: [PATCH v3 6/7] hfp: Move HFP versions definitions to hfp.h

[Denis Kenzior]

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

Hi Paulo,

On 04/15/2013 04:41 PM, Paulo Borges wrote:
> hfp.h should store all HFP related definitions.
> ---
>   drivers/hfpmodem/slc.h |    6 ------
>   src/hfp.h              |    6 ++++++
>   2 files changed, 6 insertions(+), 6 deletions(-)
>

Patch has been applied, thanks.

Regards,
-Denis

[PATCH v3 7/7] hfp_ag_bluez5: Remove unused includes

[Paulo Borges]

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

---
 plugins/hfp_ag_bluez5.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/plugins/hfp_ag_bluez5.c b/plugins/hfp_ag_bluez5.c
index 3b76179..f9e7dff 100644
--- a/plugins/hfp_ag_bluez5.c
+++ b/plugins/hfp_ag_bluez5.c
@@ -29,17 +29,13 @@
 #include <sys/socket.h>
 #include <glib.h>
 #include <ofono.h>
-
 #include <gdbus.h>
-#include <gatchat.h>
 
 #define OFONO_API_SUBJECT_TO_CHANGE
 #include <ofono/plugin.h>
 #include <ofono/log.h>
 #include <ofono/modem.h>
 
-#include <drivers/hfpmodem/slc.h>
-
 #include "hfp.h"
 #include "bluez5.h"
 
-- 
1.7.9.5

Re: [PATCH v3 7/7] hfp_ag_bluez5: Remove unused includes

[Denis Kenzior]

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

Hi Paulo,

On 04/15/2013 04:41 PM, Paulo Borges wrote:
> ---
>   plugins/hfp_ag_bluez5.c |    4 ----
>   1 file changed, 4 deletions(-)

Patch has been applied, thanks.

Regards,
-Denis