[PATCH v2 1/2] gatserver: add command finished functions

[PATCH v2 1/2] gatserver: add command finished functions

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

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

---
 gatchat/gatserver.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 gatchat/gatserver.h |    7 +++++++
 2 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index ab785f7..8e92071 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -115,6 +115,9 @@ struct _GAtServer {
 	gpointer user_disconnect_data;		/* User disconnect data */
 	GAtDebugFunc debugf;			/* Debugging output function */
 	gpointer debug_data;			/* Data to pass to debug func */
+	GAtServerFinishFunc finishf;		/* Command finish function */
+	gpointer finish_data;			/* Finish func data */
+	guint finish_source;
 	GHashTable *command_list;		/* List of AT commands */
 	GQueue *write_queue;			/* Write buffer queue */
 	guint max_read_attempts;		/* Max reads per select */
@@ -1177,12 +1180,27 @@ static void server_wakeup_writer(GAtServer *server)
 	g_at_io_set_write_handler(server->io, can_write_data, server);
 }
 
+static gboolean notify_finish(void *user_data)
+{
+	GAtServer *server = user_data;
+
+	server->finishf(server, server->finish_data);
+	server->finish_source = 0;
+
+	return FALSE;
+}
+
 static void server_resume(GAtServer *server)
 {
 	if (server->suspended == FALSE)
 		return;
 
 	server->suspended = FALSE;
+
+	if (server->finishf)
+		server->finish_source = g_timeout_add_seconds(0, notify_finish,
+								server);
+
 	g_at_io_set_read_handler(server->io, new_bytes, server);
 }
 
@@ -1336,6 +1354,11 @@ void g_at_server_unref(GAtServer *server)
 	if (server == NULL)
 		return;
 
+	if (server->finish_source) {
+		g_source_remove(server->finish_source);
+		server->finish_source = 0;
+	}
+
 	is_zero = g_atomic_int_dec_and_test(&server->ref_count);
 
 	if (is_zero == FALSE)
@@ -1455,3 +1478,24 @@ gboolean g_at_server_unregister(GAtServer *server, const char *prefix)
 
 	return TRUE;
 }
+
+gboolean g_at_server_set_finish_callback(GAtServer *server,
+					GAtServerFinishFunc finish,
+					gpointer user_data)
+{
+	if (server == NULL)
+		return FALSE;
+
+	server->finishf = finish;
+	server->finish_data = user_data;
+
+	return TRUE;
+}
+
+gboolean g_at_server_command_pending(GAtServer *server)
+{
+	if (server == NULL)
+		return FALSE;
+
+	return server->suspended;
+}
diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
index bb0ae84..599c992 100644
--- a/gatchat/gatserver.h
+++ b/gatchat/gatserver.h
@@ -69,6 +69,8 @@ typedef void (*GAtServerNotifyFunc)(GAtServer *server,
 					GAtServerRequestType type,
 					GAtResult *result, gpointer user_data);
 
+typedef void (*GAtServerFinishFunc)(GAtServer *server, gpointer user_data);
+
 GAtServer *g_at_server_new(GIOChannel *io);
 GIOChannel *g_at_server_get_channel(GAtServer *server);
 GAtIO *g_at_server_get_io(GAtServer *server);
@@ -94,6 +96,11 @@ gboolean g_at_server_register(GAtServer *server, const char *prefix,
 					GDestroyNotify destroy_notify);
 gboolean g_at_server_unregister(GAtServer *server, const char *prefix);
 
+gboolean g_at_server_set_finish_callback(GAtServer *server,
+					GAtServerFinishFunc finish,
+					gpointer user_data);
+gboolean g_at_server_command_pending(GAtServer *server);
+
 /* Send a final result code. E.g. G_AT_SERVER_RESULT_NO_DIALTONE */
 void g_at_server_send_final(GAtServer *server, GAtServerResult result);
 
-- 
1.7.1

[PATCH v2 2/2] emulator: fix indicator notification

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

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

---
 src/emulator.c  |   33 +++++++++++++++++++++++++++++++--
 src/voicecall.c |   11 -----------
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/src/emulator.c b/src/emulator.c
index deb4780..06ec06c 100644
--- a/src/emulator.c
+++ b/src/emulator.c
@@ -59,6 +59,7 @@ struct indicator {
 	int value;
 	int min;
 	int max;
+	gboolean deferred;
 };
 
 static void emulator_debug(const char *str, void *data)
@@ -345,6 +346,29 @@ static struct ofono_call *find_call_with_status(struct ofono_emulator *em,
 	return __ofono_voicecall_find_call_with_status(vc, status);
 }
 
+static void notify_deferred_indicators(GAtServer *server, void *user_data)
+{
+	struct ofono_emulator *em = user_data;
+	int i;
+	char buf[20];
+	GSList *l;
+	struct indicator *ind;
+
+	for (i = 1, l = em->indicators; l; l = l->next, i++) {
+		ind = l->data;
+
+		if (!ind->deferred)
+			continue;
+
+		if (em->events_mode == 3 && em->events_ind && em->slc) {
+			sprintf(buf, "+CIEV: %d,%d", i, ind->value);
+			g_at_server_send_unsolicited(em->server, buf);
+		}
+
+		ind->deferred = FALSE;
+	}
+}
+
 static gboolean notify_ccwa(void *user_data)
 {
 	struct ofono_emulator *em = user_data;
@@ -832,6 +856,8 @@ void ofono_emulator_register(struct ofono_emulator *em, int fd)
 	g_at_server_set_debug(em->server, emulator_debug, "Server");
 	g_at_server_set_disconnect_function(em->server,
 						emulator_disconnect, em);
+	g_at_server_set_finish_callback(em->server, notify_deferred_indicators,
+						em);
 
 	if (em->type == OFONO_EMULATOR_TYPE_HFP) {
 		emulator_add_indicator(em, OFONO_EMULATOR_IND_SERVICE, 0, 1, 0);
@@ -1124,8 +1150,11 @@ void ofono_emulator_set_indicator(struct ofono_emulator *em,
 		notify_ccwa(em);
 
 	if (em->events_mode == 3 && em->events_ind && em->slc) {
-		sprintf(buf, "+CIEV: %d,%d", i, ind->value);
-		g_at_server_send_unsolicited(em->server, buf);
+		if (!g_at_server_command_pending(em->server)) {
+			sprintf(buf, "+CIEV: %d,%d", i, ind->value);
+			g_at_server_send_unsolicited(em->server, buf);
+		} else
+			ind->deferred = TRUE;
 	}
 
 	/*
diff --git a/src/voicecall.c b/src/voicecall.c
index 23976af..168ce96 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -700,9 +700,6 @@ static void emulator_call_status_cb(struct ofono_atom *atom, void *data)
 	struct ofono_emulator *em = __ofono_atom_get_data(atom);
 	struct emulator_status *s = data;
 
-	if (em == s->vc->pending_em)
-		return;
-
 	ofono_emulator_set_indicator(em, OFONO_EMULATOR_IND_CALL, s->status);
 }
 
@@ -711,9 +708,6 @@ static void emulator_callsetup_status_cb(struct ofono_atom *atom, void *data)
 	struct ofono_emulator *em = __ofono_atom_get_data(atom);
 	struct emulator_status *s = data;
 
-	if (em == s->vc->pending_em)
-		return;
-
 	ofono_emulator_set_indicator(em, OFONO_EMULATOR_IND_CALLSETUP,
 					s->status);
 }
@@ -723,9 +717,6 @@ static void emulator_callheld_status_cb(struct ofono_atom *atom, void *data)
 	struct ofono_emulator *em = __ofono_atom_get_data(atom);
 	struct emulator_status *s = data;
 
-	if (em == s->vc->pending_em)
-		return;
-
 	ofono_emulator_set_indicator(em, OFONO_EMULATOR_IND_CALLHELD,
 					s->status);
 }
@@ -3210,8 +3201,6 @@ static void emulator_dial_callback(const struct ofono_error *error, void *data)
 
 	vc->pending_em = NULL;
 
-	notify_emulator_call_status(vc);
-
 	if (need_to_emit)
 		voicecalls_emit_call_added(vc, v);
 }
-- 
1.7.1

Re: [PATCH v2 1/2] gatserver: add command finished functions

[Denis Kenzior]

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

Hi Frédéric,

On 08/04/2011 09:29 AM, Frédéric Danis wrote:
> ---
>  gatchat/gatserver.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  gatchat/gatserver.h |    7 +++++++
>  2 files changed, 51 insertions(+), 0 deletions(-)
> 

I haven't forgotten about this patch, but I found a nasty corner case in
GAtServer that breaks the current implementation.  I'm still looking for
a nice solution.

> diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
> index ab785f7..8e92071 100644
> --- a/gatchat/gatserver.c
> +++ b/gatchat/gatserver.c
> @@ -115,6 +115,9 @@ struct _GAtServer {
>  	gpointer user_disconnect_data;		/* User disconnect data */
>  	GAtDebugFunc debugf;			/* Debugging output function */
>  	gpointer debug_data;			/* Data to pass to debug func */
> +	GAtServerFinishFunc finishf;		/* Command finish function */
> +	gpointer finish_data;			/* Finish func data */
> +	guint finish_source;
>  	GHashTable *command_list;		/* List of AT commands */
>  	GQueue *write_queue;			/* Write buffer queue */
>  	guint max_read_attempts;		/* Max reads per select */
> @@ -1177,12 +1180,27 @@ static void server_wakeup_writer(GAtServer *server)
>  	g_at_io_set_write_handler(server->io, can_write_data, server);
>  }
>  
> +static gboolean notify_finish(void *user_data)
> +{
> +	GAtServer *server = user_data;
> +
> +	server->finishf(server, server->finish_data);
> +	server->finish_source = 0;
> +
> +	return FALSE;
> +}
> +
>  static void server_resume(GAtServer *server)
>  {
>  	if (server->suspended == FALSE)
>  		return;
>  
>  	server->suspended = FALSE;
> +
> +	if (server->finishf)
> +		server->finish_source = g_timeout_add_seconds(0, notify_finish,
> +								server);
> +
>  	g_at_io_set_read_handler(server->io, new_bytes, server);
>  }
>  

<snip>

Can you brainstorm some ideas of a way not to use a g_timeout_add and do
the finish notify synchronously?  I suspect there is a nice(r) solution.

Regards,
-Denis

Re: [PATCH v2 2/2] emulator: fix indicator notification

[Denis Kenzior]

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

Hi Frédéric,

On 08/04/2011 09:29 AM, Frédéric Danis wrote:
> ---
>  src/emulator.c  |   33 +++++++++++++++++++++++++++++++--
>  src/voicecall.c |   11 -----------
>  2 files changed, 31 insertions(+), 13 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH v2 1/2] gatserver: add command finished functions

[Denis Kenzior]

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

Hi Frédéric,

On 08/04/2011 09:29 AM, Frédéric Danis wrote:
> ---
>  gatchat/gatserver.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  gatchat/gatserver.h |    7 +++++++
>  2 files changed, 51 insertions(+), 0 deletions(-)
> 

I pushed my own version of this patch.  I have performed basic testing
here on my end and it seems to be working out quite nicely.  However, I
have not tested the HFP emulator proper, so could you please double
check my logic and make sure that I haven't done anything silly?

Thanks,
-Denis