[PATCH 0/5] Prototype to handle asynchronized callback

[PATCH 0/5] Prototype to handle asynchronized callback

[Zhenhua Zhang]

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

Hi,

These patches are the prototype to handle the asynchronized callback
for AtServer.

During the callback function execution period, GAtServer waits for its
result and then process next command. At the same time, the server is
capable to accept new data comes from the client side and cache them in
the read buffer.

Please review them. Any comments are welcome!

In-Reply-To: 

[PATCH 1/5] Refactor the command parsing framework

[Zhenhua Zhang]

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

---
 gatchat/gatserver.c |  182 +++++++++++++++++++++++++++++++++++---------------
 gatchat/gatserver.h |    7 ++-
 2 files changed, 133 insertions(+), 56 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index c75fbf5..74c765d 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -112,9 +112,51 @@ struct _GAtServer {
 	guint max_read_attempts;		/* Max reads per select */
 	enum ParserState parser_state;
 	gboolean destroyed;			/* Re-entrancy guard */
+	char *read_line;			/* Current read line */
+	unsigned int read_pos;			/* Current read offset */
+	int notify_source;			/* Source of notify callback */
+	GAtServerResult last_result;		/* Result of last command */
+};
+
+struct at_notify_node {
+	GAtServer *server;
+	char *command;
+	char *prefix;
+	GAtServerRequestType type;
 };
 
 static void g_at_server_wakeup_writer(GAtServer *server);
+static void server_parse_line(gpointer user_data);
+
+static struct at_notify_node *at_node_new(GAtServer *server,
+						char *buf, char *prefix,
+						GAtServerRequestType type)
+{
+	struct at_notify_node *ret;
+
+	ret = g_try_new0(struct at_notify_node, 1);
+
+	if (!ret)
+		return ret;
+
+	ret->server = server;
+	ret->command = g_strdup(buf);
+	ret->prefix = g_strdup(prefix);
+	ret->type = type;
+
+	return ret;
+}
+
+static void at_notify_destroy(gpointer user_data)
+{
+	struct at_notify_node *notify = user_data;
+
+	notify->server->notify_source = 0;
+
+	g_free(notify->command);
+	g_free(notify->prefix);
+	g_free(notify);
+}
 
 static struct ring_buffer *allocate_next(GAtServer *server)
 {
@@ -194,28 +236,32 @@ static inline gboolean is_extended_command_prefix(const char c)
 	}
 }
 
-static void at_command_notify(GAtServer *server, char *command,
-				char *prefix, GAtServerRequestType type)
+static gboolean at_command_notify(gpointer user_data)
 {
+	struct at_notify_node *notify = user_data;
+	GAtServer *server = notify->server;
 	struct at_command *node;
 	GAtResult result;
 
-	node = g_hash_table_lookup(server->command_list, prefix);
+	node = g_hash_table_lookup(server->command_list, notify->prefix);
 
 	if (node == NULL) {
 		g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
-		return;
+		return FALSE;
 	}
 
-	result.lines = g_slist_prepend(NULL, command);
+	result.lines = g_slist_prepend(NULL, notify->command);
 	result.final_or_pdu = 0;
 
-	node->notify(type, &result, node->user_data);
+	node->notify(notify->type, &result, node->user_data,
+			server_parse_line, server);
 
 	g_slist_free(result.lines);
+
+	return FALSE;
 }
 
-static unsigned int parse_extended_command(GAtServer *server, char *buf)
+static void parse_extended_command(GAtServer *server, char *buf)
 {
 	const char *valid_extended_chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
 						"0123456789!%-./:_";
@@ -226,11 +272,12 @@ static unsigned int parse_extended_command(GAtServer *server, char *buf)
 	gboolean seen_equals = FALSE;
 	char prefix[18]; /* According to V250, 5.4.1 */
 	GAtServerRequestType type;
+	struct at_notify_node *notify = NULL;
 
 	prefix_len = strcspn(buf, separators);
 
 	if (prefix_len > 17 || prefix_len < 2)
-		return 0;
+		goto error;
 
 	/* Convert to upper case, we will always use upper case naming */
 	for (i = 0; i < prefix_len; i++)
@@ -239,17 +286,17 @@ static unsigned int parse_extended_command(GAtServer *server, char *buf)
 	prefix[prefix_len] = '\0';
 
 	if (strspn(prefix + 1, valid_extended_chars) != (prefix_len - 1))
-		return 0;
+		goto error;
 
 	/*
 	 * V.250 Section 5.4.1: "The first character following "+" shall be
 	 * an alphabetic character in the range "A" through "Z".
 	 */
 	if (prefix[1] <= 'A' || prefix[1] >= 'Z')
-		return 0;
+		goto error;
 
 	if (buf[i] != '\0' && buf[i] != ';' && buf[i] != '?' && buf[i] != '=')
-		return 0;
+		goto error;
 
 	type = G_AT_SERVER_REQUEST_TYPE_COMMAND_ONLY;
 
@@ -265,16 +312,16 @@ static unsigned int parse_extended_command(GAtServer *server, char *buf)
 
 		if (buf[i] == '?') {
 			if (seen_question || seen_equals)
-				return 0;
+				goto error;
 
 			if (buf[i + 1] != '\0' && buf[i + 1] != ';')
-				return 0;
+				goto error;
 
 			seen_question = TRUE;
 			type = G_AT_SERVER_REQUEST_TYPE_QUERY;
 		} else if (buf[i] == '=') {
 			if (seen_equals || seen_question)
-				return 0;
+				goto error;
 
 			seen_equals = TRUE;
 
@@ -291,10 +338,22 @@ next:
 	/* We can scratch in this buffer, so mark ';' as null */
 	buf[i] = '\0';
 
-	at_command_notify(server, buf, prefix, type);
+	notify = at_node_new(server, buf, prefix, type);
+	if (!notify)
+		goto error;
 
 	/* Also consume the terminating null */
-	return i + 1;
+	server->read_pos += i + 1;
+	server->notify_source = g_idle_add_full(G_PRIORITY_DEFAULT,
+						at_command_notify,
+						notify, g_free);
+
+	return;
+
+error:
+	g_free(notify);
+
+	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
 }
 
 static int get_basic_prefix_size(const char *buf)
@@ -336,16 +395,17 @@ static int get_basic_prefix_size(const char *buf)
 	return 0;
 }
 
-static unsigned int parse_basic_command(GAtServer *server, char *buf)
+static void parse_basic_command(GAtServer *server, char *buf)
 {
 	gboolean seen_equals = FALSE;
 	char prefix[4], tmp;
 	unsigned int i, prefix_size;
 	GAtServerRequestType type;
+	struct at_notify_node *notify = NULL;
 
 	prefix_size = get_basic_prefix_size(buf);
 	if (prefix_size == 0)
-		return 0;
+		goto error;
 
 	i = prefix_size;
 	prefix[0] = g_ascii_toupper(buf[0]);
@@ -390,56 +450,59 @@ static unsigned int parse_basic_command(GAtServer *server, char *buf)
 	}
 
 done:
-	if (prefix_size <= 3) {
-		memcpy(prefix + 1, buf + 1, prefix_size - 1);
-		prefix[prefix_size] = '\0';
-
-		tmp = buf[i];
-		buf[i] = '\0';
-		at_command_notify(server, buf, prefix, type);
-		buf[i] = tmp;
-	} else /* Handle S-parameter with 100+ */
-		g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
+	if (prefix_size > 3)
+		goto error;
 
 	/* Commands like ATA, ATZ cause the remainder line
 	 * to be ignored.
 	 */
 	if (prefix[0] == 'A' || prefix[0] == 'Z')
-		return strlen(buf);
+		i = strlen(buf);
 
 	/* Consume the seperator ';' */
 	if (buf[i] == ';')
 		i += 1;
 
-	return i;
+	memcpy(prefix + 1, buf + 1, prefix_size - 1);
+	prefix[prefix_size] = '\0';
+
+	tmp = buf[i];
+	buf[i] = '\0';
+
+	notify = at_node_new(server, buf, prefix, type);
+	if (!notify)
+		goto error;
+
+	buf[i] = tmp;
+
+	server->read_pos += i;
+	server->notify_source = g_idle_add_full(G_PRIORITY_DEFAULT,
+						at_command_notify,
+						notify, at_notify_destroy);
+
+	return;
+
+error:
+	g_free(notify);
+
+	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
 }
 
-static void server_parse_line(GAtServer *server, char *line)
+static void server_parse_line(gpointer user_data)
 {
-	unsigned int pos = 0;
+	GAtServer *server = user_data;
+	char *line = server->read_line;
+	unsigned int pos = server->read_pos;
 	unsigned int len = strlen(line);
 
-	if (len == 0) {
-		g_at_server_send_final(server, G_AT_SERVER_RESULT_OK);
+	/* If we have a non-OK result or no further commands, then stop now */
+	if (server->last_result != G_AT_SERVER_RESULT_OK || pos >= len)
 		return;
-	}
-
-	while (pos < len) {
-		unsigned int consumed;
 
-		if (is_extended_command_prefix(line[pos]))
-			consumed = parse_extended_command(server, line + pos);
-		else
-			consumed = parse_basic_command(server, line + pos);
-
-		if (consumed == 0) {
-			g_at_server_send_final(server,
-						G_AT_SERVER_RESULT_ERROR);
-			break;
-		}
-
-		pos += consumed;
-	}
+	if (is_extended_command_prefix(line[pos]))
+		parse_extended_command(server, line + pos);
+	else
+		parse_basic_command(server, line + pos);
 }
 
 static enum ParserResult server_feed(GAtServer *server,
@@ -621,11 +684,15 @@ static void new_bytes(GAtServer *p)
 
 		case PARSER_RESULT_COMMAND:
 		{
-			char *line = extract_line(p);
+			g_free(p->read_line);
 
-			if (line) {
-				server_parse_line(p, line);
-				g_free(line);
+			/* Save current line as last line */
+			p->read_line = extract_line(p);
+			if (p->read_line) {
+				p->read_pos = 0;
+				p->last_result = G_AT_SERVER_RESULT_OK;
+
+				server_parse_line(p);
 			} else
 				g_at_server_send_final(p,
 						G_AT_SERVER_RESULT_ERROR);
@@ -795,6 +862,11 @@ static void g_at_server_cleanup(GAtServer *server)
 	g_hash_table_destroy(server->command_list);
 	server->command_list = NULL;
 
+	g_free(server->read_line);
+
+	if (server->notify_source)
+		g_source_remove(server->notify_source);
+
 	server->channel = NULL;
 }
 
diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
index 2ae19ca..4383a58 100644
--- a/gatchat/gatserver.h
+++ b/gatchat/gatserver.h
@@ -64,8 +64,13 @@ enum _GAtServerRequestType {
 
 typedef enum _GAtServerRequestType GAtServerRequestType;
 
+typedef void (*GAtServerNotifyCallback)(gpointer user_data);
+
 typedef void (*GAtServerNotifyFunc)(GAtServerRequestType type,
-					GAtResult *result, gpointer user_data);
+					GAtResult *result,
+					gpointer user_data,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data);
 
 GAtServer *g_at_server_new(GIOChannel *io);
 
-- 
1.6.6.1

[PATCH 2/5] Add server send final result code

[Zhenhua Zhang]

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

---
 gatchat/gatserver.c |   93 ++++++++++++++++++++++++++++++++++++++++++++++----
 gatchat/gatserver.h |   20 +++++++++++
 2 files changed, 105 insertions(+), 8 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index 74c765d..fdc4180 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -33,6 +33,8 @@
 #include "gatserver.h"
 
 #define BUF_SIZE 4096
+/* <cr><lf> + the max length of information text + <cr><lf> */
+#define MAX_TEXT_SIZE 2052
 /* #define WRITE_SCHEDULER_DEBUG 1 */
 
 enum ParserState {
@@ -198,11 +200,10 @@ static void send_common(GAtServer *server, const char *buf, unsigned int len)
 	g_at_server_wakeup_writer(server);
 }
 
-static void g_at_server_send_final(GAtServer *server, GAtServerResult result)
+static void send_result_common(GAtServer *server, const char *result)
 {
 	struct v250_settings v250 = server->v250;
-	const char *result_str = server_result_to_string(result);
-	char buf[1024];
+	char buf[MAX_TEXT_SIZE];
 	char t = v250.s3;
 	char r = v250.s4;
 	unsigned int len;
@@ -210,19 +211,86 @@ static void g_at_server_send_final(GAtServer *server, GAtServerResult result)
 	if (v250.quiet)
 		return;
 
-	if (result_str == NULL)
+	if (result == NULL)
 		return;
 
 	if (v250.is_v1)
-		len = snprintf(buf, sizeof(buf), "%c%c%s%c%c", t, r, result_str,
+		len = snprintf(buf, sizeof(buf), "%c%c%s%c%c", t, r, result,
 				t, r);
 	else
-		len = snprintf(buf, sizeof(buf), "%u%c", (unsigned int) result,
+		len = snprintf(buf, sizeof(buf), "%s%c", result,
 				t);
 
 	send_common(server, buf, MIN(len, sizeof(buf)-1));
 }
 
+static void g_at_server_send_flush(GAtServer *server)
+{
+	GAtServerResult result = server->last_result;
+	char buf[1024];
+
+	/* Do not emit error if extended error has already been emitted */
+	if (result == G_AT_SERVER_RESULT_EXT_ERROR)
+		return;
+
+	if (server->v250.is_v1)
+		sprintf(buf, "%s", server_result_to_string(result));
+	else
+		sprintf(buf, "%u", (unsigned int)result);
+
+	send_result_common(server, buf);
+}
+
+inline void g_at_server_send_final(GAtServer *server, GAtServerResult result)
+{
+	/* Cache the result until the whole command line is parsed */
+	server->last_result = result;
+}
+
+void g_at_server_send_ext_final(GAtServer *server, const char *result)
+{
+	send_result_common(server, result);
+
+	server->last_result = G_AT_SERVER_RESULT_EXT_ERROR;
+}
+
+void g_at_server_send_intermediate(GAtServer *server, const char *result)
+{
+	send_result_common(server, result);
+}
+
+void g_at_server_send_unsolicited(GAtServer *server, const char *result)
+{
+	send_result_common(server, result);
+}
+
+void g_at_server_send_info_text(GAtServer *server, GSList *text)
+{
+	char buf[MAX_TEXT_SIZE];
+	char t = server->v250.s3;
+	char r = server->v250.s4;
+	unsigned int len;
+	GSList *l = text;
+	char *line;
+
+	if (!text)
+		return;
+
+	while (l) {
+		line = l->data;
+		if (!line)
+			return;
+
+		len = snprintf(buf, sizeof(buf), "%c%c%s", t, r, line);
+		send_common(server, buf, MIN(len, sizeof(buf)-1));
+
+		l = l->next;
+	}
+
+	len = snprintf(buf, sizeof(buf), "%c%c", t, r);
+	send_common(server, buf, len);
+}
+
 static inline gboolean is_extended_command_prefix(const char c)
 {
 	switch (c) {
@@ -247,6 +315,7 @@ static gboolean at_command_notify(gpointer user_data)
 
 	if (node == NULL) {
 		g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
+		g_at_server_send_flush(server);
 		return FALSE;
 	}
 
@@ -496,8 +565,10 @@ static void server_parse_line(gpointer user_data)
 	unsigned int len = strlen(line);
 
 	/* If we have a non-OK result or no further commands, then stop now */
-	if (server->last_result != G_AT_SERVER_RESULT_OK || pos >= len)
+	if (server->last_result != G_AT_SERVER_RESULT_OK || pos >= len) {
+		g_at_server_send_flush(server);
 		return;
+	}
 
 	if (is_extended_command_prefix(line[pos]))
 		parse_extended_command(server, line + pos);
@@ -679,6 +750,7 @@ static void new_bytes(GAtServer *p)
 			 * Empty commands must be OK by the DCE
 			 */
 			g_at_server_send_final(p, G_AT_SERVER_RESULT_OK);
+			g_at_server_send_flush(p);
 			ring_buffer_drain(p->read_buf, p->read_so_far);
 			break;
 
@@ -693,15 +765,20 @@ static void new_bytes(GAtServer *p)
 				p->last_result = G_AT_SERVER_RESULT_OK;
 
 				server_parse_line(p);
-			} else
+			} else {
 				g_at_server_send_final(p,
 						G_AT_SERVER_RESULT_ERROR);
+
+				g_at_server_send_flush(p);
+			}
+
 			break;
 		}
 
 		case PARSER_RESULT_REPEAT_LAST:
 			/* TODO */
 			g_at_server_send_final(p, G_AT_SERVER_RESULT_OK);
+			g_at_server_send_flush(p);
 			ring_buffer_drain(p->read_buf, p->read_so_far);
 			break;
 
diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
index 4383a58..7112e90 100644
--- a/gatchat/gatserver.h
+++ b/gatchat/gatserver.h
@@ -92,6 +92,26 @@ gboolean g_at_server_register(GAtServer *server, char *prefix,
 					GDestroyNotify destroy_notify);
 gboolean g_at_server_unregister(GAtServer *server, const char *prefix);
 
+/* Send a final result code. E.g. NO DIALTONE */
+void g_at_server_send_final(GAtServer *server, GAtServerResult result);
+
+/* Send an extended final result code. E.g. +CME ERROR: SIM failure.
+ * In this case, callback should return G_AT_SERVER_RESULT_EXT_ERROR as
+ * result so that server will not send final result code again.
+ */
+void g_at_server_send_ext_final(GAtServer *server, const char *result);
+
+/* Send an intermediate result code to report the progress. E.g. CONNECT */
+void g_at_server_send_intermediate(GAtServer *server, const char *result);
+
+/* Send an unsolicited result code. E.g. RING */
+void g_at_server_send_unsolicited(GAtServer *server, const char *result);
+
+/* Send an information text. The text could contain multiple lines. Each
+ * line, including line terminators, should not exceed 2048 characters.
+ */
+void g_at_server_send_info_text(GAtServer *server, GSList *text);
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.6.6.1

[PATCH 3/5] Add set server to offline mode during parse line

[Zhenhua Zhang]

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

---
 gatchat/gatserver.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index fdc4180..5d2a036 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -118,6 +118,7 @@ struct _GAtServer {
 	unsigned int read_pos;			/* Current read offset */
 	int notify_source;			/* Source of notify callback */
 	GAtServerResult last_result;		/* Result of last command */
+	gboolean is_online;			/* Online or offline mode */
 };
 
 struct at_notify_node {
@@ -239,6 +240,8 @@ static void g_at_server_send_flush(GAtServer *server)
 		sprintf(buf, "%u", (unsigned int)result);
 
 	send_result_common(server, buf);
+
+	server->is_online = TRUE;
 }
 
 inline void g_at_server_send_final(GAtServer *server, GAtServerResult result)
@@ -728,7 +731,7 @@ static void new_bytes(GAtServer *p)
 	unsigned char *buf = ring_buffer_read_ptr(p->read_buf, p->read_so_far);
 	enum ParserResult result;
 
-	while (p->channel && (p->read_so_far < len)) {
+	while (p->channel && p->is_online && (p->read_so_far < len)) {
 		gsize rbytes = MIN(len - p->read_so_far, wrap - p->read_so_far);
 		result = server_feed(p, (char *)buf, &rbytes);
 
@@ -764,6 +767,12 @@ static void new_bytes(GAtServer *p)
 				p->read_pos = 0;
 				p->last_result = G_AT_SERVER_RESULT_OK;
 
+				/* Set offline mode so that server won't feed
+				 * the next command line until current line is
+				 * processed and final result code is flushed.
+				 */
+				p->is_online = FALSE;
+
 				server_parse_line(p);
 			} else {
 				g_at_server_send_final(p,
@@ -1013,6 +1022,7 @@ GAtServer *g_at_server_new(GIOChannel *io)
 	server->ref_count = 1;
 	v250_settings_create(&server->v250);
 	server->channel = io;
+	server->is_online = TRUE;
 	server->command_list = g_hash_table_new_full(g_str_hash, g_str_equal,
 							g_free,
 							at_notify_node_destroy);
-- 
1.6.6.1

[PATCH 4/5] Add register S3-S5 basic command callbacks

[Zhenhua Zhang]

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

---
 gatchat/gatserver.c |  162 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 162 insertions(+), 0 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index 5d2a036..9d4d221 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -294,6 +294,159 @@ void g_at_server_send_info_text(GAtServer *server, GSList *text)
 	send_common(server, buf, len);
 }
 
+static gboolean get_result_value(GAtServer *server, GAtResult *result,
+						const char *command,
+						int min, int max, int *value)
+{
+	GAtResultIter iter;
+	int val;
+	char prefix[10];
+
+	if (command[0] != 'S')
+		return FALSE;
+
+	sprintf(prefix, "%s=", command);
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, prefix))
+		return FALSE;
+
+	if (!g_at_result_iter_next_number(&iter, &val))
+		return FALSE;
+
+	if (val < min || val > max)
+		return FALSE;
+
+	*value = val;
+
+	return TRUE;
+}
+
+static void send_info_text(GAtServer *server, char *buf)
+{
+	GSList *text = NULL;
+
+	text = g_slist_append(NULL, buf);
+	g_at_server_send_info_text(server, text);
+	g_slist_free(text);
+}
+
+static void set_s_value(GAtServer *server, const char *prefix, int val)
+{
+	char c = prefix[1];
+
+	switch (c) {
+	case '3':
+		server->v250.s3 = val;
+		break;
+
+	case '4':
+		server->v250.s4 = val;
+		break;
+
+	case '5':
+		server->v250.s5 = val;
+		break;
+
+	default:
+		break;
+	}
+}
+
+static int get_s_value(GAtServer *server, const char *prefix)
+{
+	char c = prefix[1];
+	int val = 0;
+
+	switch (c) {
+	case '3':
+		val = server->v250.s3;
+		break;
+
+	case '4':
+		val = server->v250.s4;
+		break;
+
+	case '5':
+		val = server->v250.s5;
+		break;
+
+	default:
+		break;
+	}
+
+	return val;
+}
+
+static void s_template_cb(GAtServerRequestType type,
+					GAtResult *result, gpointer user_data,
+					const char *prefix,
+					int min, int max,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data)
+{
+	GAtServerResult res = G_AT_SERVER_RESULT_ERROR;
+	GAtServer *server = user_data;
+	char buf[20];
+	int val;
+
+	switch (type) {
+	case G_AT_SERVER_REQUEST_TYPE_SET:
+		if (!get_result_value(server, result, prefix, min, max, &val))
+			goto done;
+
+		set_s_value(server, prefix, val);
+		res = G_AT_SERVER_RESULT_OK;
+		break;
+
+	case G_AT_SERVER_REQUEST_TYPE_QUERY:
+		val = get_s_value(server, prefix);
+		sprintf(buf, "%03d", val);
+		send_info_text(server, buf);
+		res = G_AT_SERVER_RESULT_OK;
+		break;
+
+	case G_AT_SERVER_REQUEST_TYPE_SUPPORT:
+		sprintf(buf, "%s: (%d-%d)", prefix, min, max);
+		send_info_text(server, buf);
+		res = G_AT_SERVER_RESULT_OK;
+		break;
+
+	default:
+		break;
+	}
+
+done:
+	g_at_server_send_final(server, res);
+
+	cb(cb_data);
+}
+
+static void at_s3_cb(GAtServerRequestType type, GAtResult *result,
+					gpointer user_data,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data)
+{
+	s_template_cb(type, result, user_data, "S3", 0, 127, cb, cb_data);
+}
+
+static void at_s4_cb(GAtServerRequestType type, GAtResult *result,
+					gpointer user_data,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data)
+{
+	s_template_cb(type, result, user_data, "S4", 0, 127, cb, cb_data);
+}
+
+static void at_s5_cb(GAtServerRequestType type, GAtResult *result,
+					gpointer user_data,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data)
+{
+	s_template_cb(type, result, user_data, "S5", 0, 127, cb, cb_data);
+}
+
 static inline gboolean is_extended_command_prefix(const char c)
 {
 	switch (c) {
@@ -1008,6 +1161,13 @@ static void at_notify_node_destroy(gpointer data)
 	g_free(node);
 }
 
+static void basic_command_register(GAtServer *server)
+{
+	g_at_server_register(server, "S3", at_s3_cb, server, NULL);
+	g_at_server_register(server, "S4", at_s4_cb, server, NULL);
+	g_at_server_register(server, "S5", at_s5_cb, server, NULL);
+}
+
 GAtServer *g_at_server_new(GIOChannel *io)
 {
 	GAtServer *server;
@@ -1047,6 +1207,8 @@ GAtServer *g_at_server_new(GIOChannel *io)
 				received_data, server,
 				(GDestroyNotify)read_watcher_destroy_notify);
 
+	basic_command_register(server);
+
 	return server;
 
 error:
-- 
1.6.6.1

[PATCH 5/5] Add register ATE and other basic command callbacks

[Zhenhua Zhang]

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

---
 gatchat/gatserver.c |  195 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 191 insertions(+), 4 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index 9d4d221..06b5751 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -302,10 +302,10 @@ static gboolean get_result_value(GAtServer *server, GAtResult *result,
 	int val;
 	char prefix[10];
 
-	if (command[0] != 'S')
-		return FALSE;
-
-	sprintf(prefix, "%s=", command);
+	if (command[0] == 'S')
+		sprintf(prefix, "%s=", command);
+	else
+		strcpy(prefix, command);
 
 	g_at_result_iter_init(&iter, result);
 
@@ -447,6 +447,187 @@ static void at_s5_cb(GAtServerRequestType type, GAtResult *result,
 	s_template_cb(type, result, user_data, "S5", 0, 127, cb, cb_data);
 }
 
+static void set_v250_value(GAtServer *server, const char *prefix, int val)
+{
+	char c = prefix[0];
+
+	if (c == '&') {
+		c = prefix[1];
+		switch (c) {
+		case 'C':
+			server->v250.c109 = val;
+			break;
+
+		case 'D':
+			server->v250.c108 = val;
+			break;
+		default:
+			break;
+		}
+	} else {
+		switch (c) {
+		case 'E':
+			server->v250.echo = val;
+			break;
+
+		case 'Q':
+			server->v250.quiet = val;
+			break;
+
+		case 'V':
+			server->v250.is_v1 = val;
+			break;
+
+		case 'X':
+			server->v250.res_format = val;
+			break;
+		}
+	}
+}
+
+static int get_v250_value(GAtServer *server, const char *prefix)
+{
+	char c = prefix[0];
+	int val = 0;
+
+	if (c == '&') {
+		c = prefix[1];
+
+		switch (c) {
+		case 'C':
+			val = server->v250.c109;
+			break;
+
+		case 'D':
+			val = server->v250.c108;
+			break;
+
+		default:
+			break;
+		}
+	} else {
+		switch (c) {
+		case 'E':
+			val = server->v250.echo;
+			break;
+
+		case 'Q':
+			val = server->v250.quiet;
+			break;
+
+		case 'V':
+			val = server->v250.is_v1;
+			break;
+
+		case 'X':
+			val = server->v250.res_format;
+			break;
+
+		default:
+			break;
+		}
+	}
+
+	return val;
+}
+
+static void at_template_cb(GAtServerRequestType type,
+					GAtResult *result, gpointer user_data,
+					const char *prefix,
+					int min, int max, int deftval,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data)
+{
+	GAtServerResult res = G_AT_SERVER_RESULT_ERROR;
+	GAtServer *server = user_data;
+	char buf[20];
+	int val;
+
+	switch (type) {
+	case G_AT_SERVER_REQUEST_TYPE_SET:
+		if (!get_result_value(server, result, prefix, min, max, &val))
+			goto done;
+
+		set_v250_value(server, prefix, val);
+		res = G_AT_SERVER_RESULT_OK;
+		break;
+
+	case G_AT_SERVER_REQUEST_TYPE_QUERY:
+		val = get_v250_value(server, prefix);
+		sprintf(buf, "%s: %d", prefix, val);
+		send_info_text(server, buf);
+		res = G_AT_SERVER_RESULT_OK;
+		break;
+
+	case G_AT_SERVER_REQUEST_TYPE_SUPPORT:
+		sprintf(buf, "%s: (%d-%d)", prefix, min, max);
+		send_info_text(server, buf);
+		res = G_AT_SERVER_RESULT_OK;
+		break;
+
+	case G_AT_SERVER_REQUEST_TYPE_COMMAND_ONLY:
+		set_v250_value(server, prefix, deftval);
+		res = G_AT_SERVER_RESULT_OK;
+		break;
+
+	default:
+		break;
+	}
+
+done:
+	g_at_server_send_final(server, res);
+
+	cb(cb_data);
+}
+
+static void at_e_cb(GAtServerRequestType type, GAtResult *result,
+					gpointer user_data,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data)
+{
+	at_template_cb(type, result, user_data, "E", 0, 1, 1, cb, cb_data);
+}
+
+static void at_q_cb(GAtServerRequestType type, GAtResult *result,
+					gpointer user_data,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data)
+{
+	at_template_cb(type, result, user_data, "Q", 0, 1, 0, cb, cb_data);
+}
+
+static void at_v_cb(GAtServerRequestType type, GAtResult *result,
+					gpointer user_data,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data)
+{
+	at_template_cb(type, result, user_data, "V", 0, 1, 1, cb, cb_data);
+}
+
+static void at_x_cb(GAtServerRequestType type, GAtResult *result,
+					gpointer user_data,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data)
+{
+	at_template_cb(type, result, user_data, "X", 0, 4, 4, cb, cb_data);
+}
+
+static void at_c109_cb(GAtServerRequestType type, GAtResult *result,
+					gpointer user_data,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data)
+{
+	at_template_cb(type, result, user_data, "&C", 0, 1, 1, cb, cb_data);
+}
+
+static void at_c108_cb(GAtServerRequestType type, GAtResult *result,
+					gpointer user_data,
+					GAtServerNotifyCallback cb,
+					gpointer cb_data)
+{
+	at_template_cb(type, result, user_data, "&D", 0, 2, 2, cb, cb_data);
+}
+
 static inline gboolean is_extended_command_prefix(const char c)
 {
 	switch (c) {
@@ -1166,6 +1347,12 @@ static void basic_command_register(GAtServer *server)
 	g_at_server_register(server, "S3", at_s3_cb, server, NULL);
 	g_at_server_register(server, "S4", at_s4_cb, server, NULL);
 	g_at_server_register(server, "S5", at_s5_cb, server, NULL);
+	g_at_server_register(server, "E", at_e_cb, server, NULL);
+	g_at_server_register(server, "Q", at_q_cb, server, NULL);
+	g_at_server_register(server, "V", at_v_cb, server, NULL);
+	g_at_server_register(server, "X", at_x_cb, server, NULL);
+	g_at_server_register(server, "&C", at_c109_cb, server, NULL);
+	g_at_server_register(server, "&D", at_c108_cb, server, NULL);
 }
 
 GAtServer *g_at_server_new(GIOChannel *io)
-- 
1.6.6.1

Re: [PATCH 1/5] Refactor the command parsing framework

[Denis Kenzior]

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

Hi Zhenhua,

> ---
>  gatchat/gatserver.c |  182
>  +++++++++++++++++++++++++++++++++++--------------- gatchat/gatserver.h |  
>   7 ++-
>  2 files changed, 133 insertions(+), 56 deletions(-)
> 
> -	at_command_notify(server, buf, prefix, type);
> +	notify = at_node_new(server, buf, prefix, type);
> +	if (!notify)
> +		goto error;
> 
>  	/* Also consume the terminating null */
> -	return i + 1;
> +	server->read_pos += i + 1;
> +	server->notify_source = g_idle_add_full(G_PRIORITY_DEFAULT,
> +						at_command_notify,
> +						notify, g_free);
> +
> +	return;
> +
> +error:
> +	g_free(notify);
> +
> +	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
>  }

Err, OK stop right there.  This is really way too complicated.  How about we 
simply set a flag before calling at_command_notify.  If after executing it 
send_final or send_ext_final response has been sent, then we continue 
processing, otherwise we restart when send_ext_final or send_final will be 
called again.  You really don't need to touch this function at all.

> +typedef void (*GAtServerNotifyCallback)(gpointer user_data);
> +

Get rid of this, not necessary.

>  typedef void (*GAtServerNotifyFunc)(GAtServerRequestType type,
> -					GAtResult *result, gpointer user_data);
> +					GAtResult *result,
> +					gpointer user_data,
> +					GAtServerNotifyCallback cb,
> +					gpointer cb_data);
> 

Keep the NotifyFunc the way it is, your changes are really not required.

Regards,
-Denis

RE: [PATCH 1/5] Refactor the command parsing framework

[Zhang, Zhenhua]

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

Hi Denis,

Denis Kenzior wrote:
> Hi Zhenhua,
> 
>> ---
>>  gatchat/gatserver.c |  182
>>  +++++++++++++++++++++++++++++++++++---------------
>>  gatchat/gatserver.h |   7 ++- 2 files changed, 133 insertions(+),
>> 56 deletions(-) 
>> 
>> -	at_command_notify(server, buf, prefix, type);
>> +	notify = at_node_new(server, buf, prefix, type);
>> +	if (!notify)
>> +		goto error;
>> 
>>  	/* Also consume the terminating null */
>> -	return i + 1;
>> +	server->read_pos += i + 1;
>> +	server->notify_source = g_idle_add_full(G_PRIORITY_DEFAULT,
>> +						at_command_notify, +						notify, g_free);
>> +
>> +	return;
>> +
>> +error:
>> +	g_free(notify);
>> +
>> +	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
>>  }
> 
> Err, OK stop right there.  This is really way too complicated.
> How about we simply set a flag before calling
> at_command_notify.  If after executing it send_final or
> send_ext_final response has been sent, then we continue
> processing, otherwise we restart when send_ext_final or
> send_final will be called again.  You really don't need to touch this
> function at all. 

OK. So the problem is if the at_command_notify is blocked by user callback, the mainloop won't have chance to read data in from non-blocking I/O I guess. That's the reason I added this. If we don't need to handle such case, then the logic could be much simplier. We could discuss it on IRC.

>> +typedef void (*GAtServerNotifyCallback)(gpointer user_data);
>> +
> 
> Get rid of this, not necessary.
> 
>>  typedef void (*GAtServerNotifyFunc)(GAtServerRequestType type,
>> -					GAtResult *result,
> gpointer user_data);
>> +					GAtResult *result,
>> +					gpointer user_data,
>> +					GAtServerNotifyCallback cb,
>> +					gpointer cb_data);
>> 
> 
> Keep the NotifyFunc the way it is, your changes are really not
> required. 
> 
> Regards,
> -Denis
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono



Regards,
Zhenhua

Re: [PATCH 1/5] Refactor the command parsing framework

[Denis Kenzior]

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

Hi Zhenhua

> > Err, OK stop right there.  This is really way too complicated.
> > How about we simply set a flag before calling
> > at_command_notify.  If after executing it send_final or
> > send_ext_final response has been sent, then we continue
> > processing, otherwise we restart when send_ext_final or
> > send_final will be called again.  You really don't need to touch this
> > function at all.
> 
> OK. So the problem is if the at_command_notify is blocked by user callback,
>  the mainloop won't have chance to read data in from non-blocking I/O I
>  guess. That's the reason I added this. If we don't need to handle such
>  case, then the logic could be much simplier. We could discuss it on IRC.

So what you want to assume here is that for long running operations the 
callback with schedule its own g_sources and will eventually call send_final.  
This works because the server can only have one outstanding command at a time. 
You don't need to worry about blocking semantics here (they just work) and we 
won't consider re-entrant event loops for now.

Regards,
-Denis