[PATCH 1/7] Remove old server_parse_line

[PATCH 1/7] Remove old server_parse_line

[Zhenhua Zhang]

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

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

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index c9256e2..4175112 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -173,55 +173,10 @@ static void g_at_server_send_result(GAtServer *server, GAtServerResult result)
 	send_common(server, buf, MIN(len, sizeof(buf)-1));
 }
 
-static inline gboolean is_at_command_prefix(const char c)
-{
-	switch (c) {
-	case '+':
-	case '*':
-	case '!':
-	case '%':
-		return TRUE;
-	default:
-		return FALSE;
-	}
-}
-
-static GAtServerResult parse_at_command(GAtServer *server, char *buf)
-{
-	return G_AT_SERVER_RESULT_ERROR;
-}
-
-static GAtServerResult parse_v250_settings(GAtServer *server, char *buf)
-{
-	return G_AT_SERVER_RESULT_ERROR;
-}
-
 static GAtServerResult server_parse_line(GAtServer *server, char *line)
 {
-	gsize i = 0;
-	char c;
 	GAtServerResult res = G_AT_SERVER_RESULT_ERROR;
 
-	if (line == NULL)
-		goto done;
-
-	if (line[0] == '\0') {
-		res = G_AT_SERVER_RESULT_OK;
-		goto done;
-	}
-
-	c = line[i];
-	/* skip semicolon */
-	if (c == ';')
-		c = line[++i];
-
-	if (is_at_command_prefix(c))
-		res = parse_at_command(server, line + i);
-	else if (g_ascii_isalpha(c) || c == '&')
-		res = parse_v250_settings(server, line + i);
-
-done:
-	g_free(line);
 	return res;
 }
 
-- 
1.6.6.1

[PATCH 2/7] Add framework of server parser

[Zhenhua Zhang]

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

1. The parser fetches one basic or extended command per iteration.
The prefix is the command prefix without parameter. For example, in
command "AT+CLIP=1", prefix is "+CLIP".
2. Search registered notification node in command_list. If found,
get command type and invoke callback, and then return result.
3. Termiate the execution if the result in an error. Otherwise,
fetch next command.
---
 gatchat/gatserver.c |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 94 insertions(+), 1 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index 4175112..c54170e 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -173,9 +173,99 @@ static void g_at_server_send_result(GAtServer *server, GAtServerResult result)
 	send_common(server, buf, MIN(len, sizeof(buf)-1));
 }
 
+static inline gboolean is_extended_command_prefix(const char c)
+{
+	switch (c) {
+	case '+':
+	case '*':
+	case '!':
+	case '%':
+		return TRUE;
+	default:
+		return FALSE;
+	}
+}
+
+static gboolean is_basic_command_prefix(char *buf)
+{
+	if (g_ascii_isalpha(buf[0]))
+		return TRUE;
+
+	if (buf[0] == '&' && g_ascii_isalpha(buf[1]))
+		return TRUE;
+
+	return FALSE;
+}
+
+static GAtServerResult at_notify_callback(GAtServer *server, char *command,
+					char *prefix)
+{
+	int res = G_AT_SERVER_RESULT_ERROR;
+
+	return res;
+}
+
+static char *parse_extended_command(GAtServer *server, char *buf,
+					char *prefix)
+{
+	return NULL;
+}
+
+static char *parse_basic_command(GAtServer *server, char *buf, char *prefix)
+{
+	return NULL;
+}
+
+static char *server_parse_next_command(GAtServer *server, char *buf,
+					char *prefix)
+{
+	char *command = NULL;
+
+	if (is_extended_command_prefix(*buf))
+		command = parse_extended_command(server, buf, prefix);
+	else if (is_basic_command_prefix(buf))
+		command = parse_basic_command(server, buf, prefix);
+
+	return command;
+}
+
 static GAtServerResult server_parse_line(GAtServer *server, char *line)
 {
 	GAtServerResult res = G_AT_SERVER_RESULT_ERROR;
+	char *command = NULL;
+	char prefix[20];
+
+	while (line) {
+		if (*line == '\0') {
+			res = G_AT_SERVER_RESULT_OK;
+			break;
+		}
+
+		/* skip semicolon */
+		if (*line == ';')
+			line++;
+
+		memset(prefix, 0, sizeof(prefix));
+
+		command = server_parse_next_command(server, line, prefix);
+		if (!command) {
+			res = G_AT_SERVER_RESULT_ERROR;
+			break;
+		}
+
+		res = at_notify_callback(server, command, prefix);
+		/* Termiate the execution if command result in an error */
+		if (res != G_AT_SERVER_RESULT_OK)
+			break;
+
+		line += strlen(command);
+
+		g_free(command);
+		command = NULL;
+	}
+
+	if (command)
+		g_free(command);
 
 	return res;
 }
@@ -331,6 +421,7 @@ static void new_bytes(GAtServer *p)
 	unsigned char *buf = ring_buffer_read_ptr(p->read_buf, p->read_so_far);
 	enum ParserState state;
 	GAtServerResult result;
+	char *line;
 
 	while (p->channel && (p->read_so_far < len)) {
 		gsize rbytes = MIN(len - p->read_so_far, wrap - p->read_so_far);
@@ -361,8 +452,10 @@ static void new_bytes(GAtServer *p)
 			break;
 
 		case PARSER_RESULT_COMMAND:
-			result = server_parse_line(p, extract_line(p));
+			line = extract_line(p);
+			result = server_parse_line(p, line);
 			g_at_server_send_result(p, result);
+			g_free(line);
 			break;
 
 		case PARSER_RESULT_REPEAT_LAST:
-- 
1.6.6.1

[PATCH 3/7] Add basic command parsing

[Zhenhua Zhang]

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

According to V.250 5.3.1, the basic command is either a single
character or the '&' followed by a single character except for 'D'
and 'S'.

The subparameter should be the numbers.
---
 gatchat/gatserver.c |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 66 insertions(+), 1 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index c54170e..b3c3f5f 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -211,9 +211,68 @@ static char *parse_extended_command(GAtServer *server, char *buf,
 	return NULL;
 }
 
+static gboolean get_basic_prefix(char *buf, char *prefix)
+{
+	char c = buf[0];
+
+	if (g_ascii_isalpha(c)) {
+		c = g_ascii_toupper(c);
+
+		if (c == 'S') {
+			int i = 0;
+
+			prefix[0] = 'S';
+
+			/* V.250 5.3.2 'S' command follows with
+			 * a parameter number.
+			 */
+			while (g_ascii_isdigit(buf[++i]))
+				prefix[i] = buf[i];
+
+			prefix[i] = '\0';
+		} else {
+			prefix[0] = c;
+			prefix[1] = '\0';
+		}
+	} else if (c == '&') {
+		prefix[0] = '&';
+		prefix[1] = g_ascii_toupper(buf[1]);
+		prefix[2] = '\0';
+	} else
+		return FALSE;
+
+	return TRUE;
+}
+
 static char *parse_basic_command(GAtServer *server, char *buf, char *prefix)
 {
-	return NULL;
+	int i = 0;
+	char t = server->v250.s3;
+	char c = g_ascii_toupper(buf[0]);
+
+	if (!get_basic_prefix(buf, prefix))
+		return NULL;
+
+	i = strlen(prefix);
+
+	if (c == 'D') {
+		/* All following characters are the part of the call */
+		while (buf[i] && buf[i] != t && buf[i] != ';')
+			i++;
+	} else {
+		/* V.250 5.3.2 Skip '=' for S-parameters if have */
+		if (c == 'S' && buf[i] == '=')
+			i++;
+
+		if (buf[i] == '?')
+			i++;
+
+		/* V.250 5.3.1 The subparameter are all digits if have */
+		while (g_ascii_isdigit(buf[i]))
+			i++;
+	}
+
+	return g_strndup(buf, i);
 }
 
 static char *server_parse_next_command(GAtServer *server, char *buf,
@@ -258,6 +317,12 @@ static GAtServerResult server_parse_line(GAtServer *server, char *line)
 		if (res != G_AT_SERVER_RESULT_OK)
 			break;
 
+		/* Commands like ATA, ATD cause the remainder line
+		 * to be ignored.
+		 */
+		if (!strcmp(prefix, "A") || !strcmp(prefix, "D"))
+			break;
+
 		line += strlen(command);
 
 		g_free(command);
-- 
1.6.6.1

[PATCH 4/7] Add extended command parsing

[Zhenhua Zhang]

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

See V.250 5.4 Extended syntax commands for details.
---
 gatchat/gatserver.c |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 55 insertions(+), 1 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index b3c3f5f..4a19b6b 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -197,6 +197,28 @@ static gboolean is_basic_command_prefix(char *buf)
 	return FALSE;
 }
 
+static gboolean is_extended_character(const char c)
+{
+	if (g_ascii_isalpha(c))
+		return TRUE;
+
+	if (g_ascii_isdigit(c))
+		return TRUE;
+
+	switch (c) {
+	case '!':
+	case '%':
+	case '-':
+	case '.':
+	case '/':
+	case ':':
+	case '_':
+		return TRUE;
+	default:
+		return FALSE;
+	}
+}
+
 static GAtServerResult at_notify_callback(GAtServer *server, char *command,
 					char *prefix)
 {
@@ -205,10 +227,42 @@ static GAtServerResult at_notify_callback(GAtServer *server, char *command,
 	return res;
 }
 
+static gboolean get_extended_prefix(char *buf, char *prefix, char t)
+{
+	char c;
+	int i = 0;
+
+	/* Skip '+' */
+	prefix[0] = buf[0];
+
+	while ((c = buf[++i])) {
+		/* V.250 5.4.1 Extended command naming rules */
+		if (is_extended_character(c))
+			prefix[i] = g_ascii_toupper(c);
+		else
+			break;
+	}
+
+	prefix[i] = '\0';
+	return TRUE;
+}
+
 static char *parse_extended_command(GAtServer *server, char *buf,
 					char *prefix)
 {
-	return NULL;
+	char *command = NULL;
+	char t = server->v250.s3;
+	char c = buf[0];
+	int i = 0;
+
+	while (c && c != t && c != ';')
+		c = buf[++i];
+
+	command = g_strndup(buf, i);
+
+	get_extended_prefix(command, prefix, t);
+
+	return command;
 }
 
 static gboolean get_basic_prefix(char *buf, char *prefix)
-- 
1.6.6.1

[PATCH 5/7] Add server at command data structure

[Zhenhua Zhang]

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

Implement hashtable to store server supported command list.

AT command could be action command or parameter command. Defined
in V.250 5.2.5.
---
 gatchat/gatserver.c |   29 +++++++++++++++++++++++++++++
 gatchat/gatserver.h |   21 +++++++++++++++++++++
 2 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index 4a19b6b..829dc1d 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -75,6 +75,16 @@ static const char *server_result_to_string(GAtServerResult result)
 	}
 }
 
+/* AT command set that server supported */
+struct at_command {
+	int id;
+	char *prefix;
+	GAtServerCommandType type;
+	GAtServerNotifyFunc notify;
+	gpointer user_data;
+	GDestroyNotify destroy_notify;
+};
+
 /* Basic command setting for V.250 */
 struct v250_settings {
 	char s3;			/* set by S3=<val> */
@@ -99,6 +109,8 @@ struct _GAtServer {
 	gpointer user_disconnect_data;		/* User disconnect data */
 	GAtDebugFunc debugf;			/* Debugging output function */
 	gpointer debug_data;			/* Data to pass to debug func */
+	GHashTable *command_list;		/* List of supported at command */
+	guint next_command_id;			/* Next command id */
 	struct ring_buffer *read_buf;		/* Current read buffer */
 	GQueue *write_queue;			/* Write buffer queue */
 	guint max_read_attempts;		/* Max reads per select */
@@ -714,6 +726,14 @@ static gboolean can_write_data(GIOChannel *channel, GIOCondition cond,
 	return FALSE;
 }
 
+static void at_command_free(gpointer key, gpointer value, gpointer data)
+{
+	struct at_command *node = value;
+
+	g_free(node->prefix);
+	g_free(node);
+}
+
 static void write_queue_free(GQueue *write_queue)
 {
 	struct ring_buffer *write_buf;
@@ -733,6 +753,11 @@ static void g_at_server_cleanup(GAtServer *server)
 	/* Cleanup pending data to write */
 	write_queue_free(server->write_queue);
 
+	/* Cleanup registered notifications */
+	g_hash_table_foreach(server->command_list, at_command_free, NULL);
+	g_hash_table_destroy(server->command_list);
+	server->command_list = NULL;
+
 	server->channel = NULL;
 }
 
@@ -792,6 +817,7 @@ GAtServer *g_at_server_new(GIOChannel *io)
 	server->ref_count = 1;
 	v250_settings_create(&server->v250);
 	server->channel = io;
+	server->command_list = g_hash_table_new(g_str_hash, g_str_equal);
 	server->read_buf = ring_buffer_new(BUF_SIZE);
 	if (!server->read_buf)
 		goto error;
@@ -816,6 +842,9 @@ GAtServer *g_at_server_new(GIOChannel *io)
 	return server;
 
 error:
+	if (server->command_list)
+		g_hash_table_destroy(server->command_list);
+
 	if (server->read_buf)
 		ring_buffer_free(server->read_buf);
 
diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
index 698f7e0..5db9321 100644
--- a/gatchat/gatserver.h
+++ b/gatchat/gatserver.h
@@ -26,6 +26,7 @@
 extern "C" {
 #endif
 
+#include "gatresult.h"
 #include "gatutil.h"
 
 struct _GAtServer;
@@ -46,6 +47,26 @@ enum _GAtServerResult {
 
 typedef enum _GAtServerResult GAtServerResult;
 
+enum _GAtServerRequestType {
+	G_AT_SERVER_REQUEST_TYPE_NONE,
+	G_AT_SERVER_REQUEST_TYPE_ACTION,
+	G_AT_SERVER_REQUEST_TYPE_QUERY,
+	G_AT_SERVER_REQUEST_TYPE_SET,
+	G_AT_SERVER_REQUEST_TYPE_SUPPORT,
+};
+
+typedef enum _GAtServerRequestType GAtServerRequestType;
+
+enum _GAtServerCommandType {
+	G_AT_SERVER_COMMAND_TYPE_ACTION,
+	G_AT_SERVER_COMMAND_TYPE_PARAMETER,
+};
+
+typedef enum _GAtServerCommandType GAtServerCommandType;
+
+typedef int (*GAtServerNotifyFunc)(GAtServerRequestType type, GAtResult *result,
+					gpointer user_data);
+
 GAtServer *g_at_server_new(GIOChannel *io);
 
 GAtServer *g_at_server_ref(GAtServer *server);
-- 
1.6.6.1

[PATCH 6/7] Add notify at command callback

[Zhenhua Zhang]

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

Search command_list to get notification node.

If the command is an action command, the type may be "action" or
"support".

If the command is a parameter command, the type may be either
"support", "query" or "set".
---
 gatchat/gatserver.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index 829dc1d..bea51f5 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -231,10 +231,63 @@ static gboolean is_extended_character(const char c)
 	}
 }
 
+static GAtServerRequestType get_command_type(char *buf, struct at_command *node)
+{
+	GAtServerRequestType type = G_AT_SERVER_REQUEST_TYPE_NONE;
+	char *prefix = node->prefix;
+	GAtServerCommandType cmd_type = node->type;
+
+	buf += strlen(prefix);
+
+	if (cmd_type == G_AT_SERVER_COMMAND_TYPE_ACTION) {
+		if (buf[0] == '=' && buf[1] == '?')
+			type = G_AT_SERVER_REQUEST_TYPE_SUPPORT;
+		else if (buf[0] == '?')
+			type = G_AT_SERVER_REQUEST_TYPE_NONE;
+		else
+			type = G_AT_SERVER_REQUEST_TYPE_ACTION;
+	} else {
+		if (buf[0] == '=' && buf[1] == '?')
+			type = G_AT_SERVER_REQUEST_TYPE_SUPPORT;
+		else if (buf[0] == '?')
+			type = G_AT_SERVER_REQUEST_TYPE_QUERY;
+		else {
+			/* Two cases:
+			 * A valid basic command followed by digits.
+			 * 	If no digit follows, zero is assumed.
+			 * A valid extended command followed by '='.
+			 */
+			if (is_basic_command_prefix(prefix))
+				type = G_AT_SERVER_REQUEST_TYPE_SET;
+			else if (is_extended_command_prefix(*prefix) &&
+					buf[0] == '=')
+				type = G_AT_SERVER_REQUEST_TYPE_SET;
+		}
+	}
+
+	return type;
+}
+
 static GAtServerResult at_notify_callback(GAtServer *server, char *command,
 					char *prefix)
 {
 	int res = G_AT_SERVER_RESULT_ERROR;
+	struct at_command *node;
+
+	node = g_hash_table_lookup(server->command_list, prefix);
+	if (node && node->notify) {
+		GAtServerRequestType type;
+		GAtResult result;
+
+		type = get_command_type(command, node);
+		if (type == G_AT_SERVER_REQUEST_TYPE_NONE)
+			return res;
+
+		result.lines = g_slist_prepend(NULL, command);
+		result.final_or_pdu = 0;
+
+		res = node->notify(type, &result, node->user_data);
+	}
 
 	return res;
 }
-- 
1.6.6.1

[PATCH 7/7] Add g_at_server_register to register callback

[Zhenhua Zhang]

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

---
 gatchat/gatserver.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 gatchat/gatserver.h |    6 ++++++
 2 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index bea51f5..c49e5f8 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -987,3 +987,50 @@ gboolean g_at_server_set_debug(GAtServer *server, GAtDebugFunc func,
 
 	return TRUE;
 }
+
+guint g_at_server_register(GAtServer *server, const char *prefix,
+					GAtServerCommandType type,
+					GAtServerNotifyFunc notify,
+					gpointer user_data,
+					GDestroyNotify destroy_notify)
+{
+	struct at_command *node;
+
+	if (server == NULL || server->command_list == NULL)
+		return 0;
+
+	if (notify == NULL)
+		return 0;
+
+	if (prefix == NULL || strlen(prefix) == 0)
+		return 0;
+
+	node = g_hash_table_lookup(server->command_list, prefix);
+
+	if (node) {
+		g_hash_table_remove(server->command_list, prefix);
+
+		if (node->destroy_notify)
+			node->destroy_notify(node->user_data);
+
+		g_free(node->prefix);
+
+		g_free(node);
+	}
+
+	node = g_try_new0(struct at_command, 1);
+
+	if (!node)
+		return 0;
+
+	node->id = server->next_command_id++;
+	node->prefix = g_strdup(prefix);
+	node->type = type;
+	node->notify = notify;
+	node->user_data = user_data;
+	node->destroy_notify = destroy_notify;
+
+	g_hash_table_insert(server->command_list, node->prefix, node);
+
+	return node->id;
+}
diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
index 5db9321..59cdf25 100644
--- a/gatchat/gatserver.h
+++ b/gatchat/gatserver.h
@@ -80,6 +80,12 @@ gboolean g_at_server_set_debug(GAtServer *server,
 					GAtDebugFunc func,
 					gpointer user);
 
+guint g_at_server_register(GAtServer *server, const char *prefix,
+					GAtServerCommandType type,
+					GAtServerNotifyFunc notify,
+					gpointer user_data,
+					GDestroyNotify destroy_notify);
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.6.6.1

Re: [PATCH 5/7] Add server at command data structure

[Denis Kenzior]

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

Hi Zhenhua,

> Implement hashtable to store server supported command list.
> 
> AT command could be action command or parameter command. Defined
> in V.250 5.2.5.
> ---
>  gatchat/gatserver.c |   29 +++++++++++++++++++++++++++++
>  gatchat/gatserver.h |   21 +++++++++++++++++++++
>  2 files changed, 50 insertions(+), 0 deletions(-)
> 
> diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
> index 4a19b6b..829dc1d 100644
> --- a/gatchat/gatserver.c
> +++ b/gatchat/gatserver.c
> @@ -75,6 +75,16 @@ static const char
>  *server_result_to_string(GAtServerResult result) }
>  }
> 
> +/* AT command set that server supported */
> +struct at_command {
> +	int id;
> +	char *prefix;

I don't see the point for id or prefix here.  The prefix is already stored as 
the key in the hash table and frankly we should always be using that as it is 
unique for our purposes.

> +	GAtServerCommandType type;
> +	GAtServerNotifyFunc notify;
> +	gpointer user_data;
> +	GDestroyNotify destroy_notify;
> +};
> +
>  /* Basic command setting for V.250 */
>  struct v250_settings {
>  	char s3;			/* set by S3=<val> */
> @@ -99,6 +109,8 @@ struct _GAtServer {
>  	gpointer user_disconnect_data;		/* User disconnect data */
>  	GAtDebugFunc debugf;			/* Debugging output function */
>  	gpointer debug_data;			/* Data to pass to debug func */
> +	GHashTable *command_list;		/* List of supported at command */
> +	guint next_command_id;			/* Next command id */

Get rid of this part

>  	struct ring_buffer *read_buf;		/* Current read buffer */
>  	GQueue *write_queue;			/* Write buffer queue */
>  	guint max_read_attempts;		/* Max reads per select */
> @@ -714,6 +726,14 @@ static gboolean can_write_data(GIOChannel *channel,
>  GIOCondition cond, return FALSE;
>  }
> 
> +static void at_command_free(gpointer key, gpointer value, gpointer data)
> +{
> +	struct at_command *node = value;
> +
> +	g_free(node->prefix);
> +	g_free(node);
> +}
> +
>  static void write_queue_free(GQueue *write_queue)
>  {
>  	struct ring_buffer *write_buf;
> @@ -733,6 +753,11 @@ static void g_at_server_cleanup(GAtServer *server)
>  	/* Cleanup pending data to write */
>  	write_queue_free(server->write_queue);
> 
> +	/* Cleanup registered notifications */
> +	g_hash_table_foreach(server->command_list, at_command_free, NULL);

This is unnecessary if you properly initialize the hash table, see below.

> +	g_hash_table_destroy(server->command_list);
> +	server->command_list = NULL;
> +
>  	server->channel = NULL;
>  }
> 
> @@ -792,6 +817,7 @@ GAtServer *g_at_server_new(GIOChannel *io)
>  	server->ref_count = 1;
>  	v250_settings_create(&server->v250);
>  	server->channel = io;
> +	server->command_list = g_hash_table_new(g_str_hash, g_str_equal);

Use the _new_full version here and specify the destructor.

>  	server->read_buf = ring_buffer_new(BUF_SIZE);
>  	if (!server->read_buf)
>  		goto error;
> @@ -816,6 +842,9 @@ GAtServer *g_at_server_new(GIOChannel *io)
>  	return server;
> 
>  error:
> +	if (server->command_list)
> +		g_hash_table_destroy(server->command_list);
> +
>  	if (server->read_buf)
>  		ring_buffer_free(server->read_buf);
> 
> diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
> index 698f7e0..5db9321 100644
> --- a/gatchat/gatserver.h
> +++ b/gatchat/gatserver.h
> @@ -26,6 +26,7 @@
>  extern "C" {
>  #endif
> 
> +#include "gatresult.h"
>  #include "gatutil.h"
> 
>  struct _GAtServer;
> @@ -46,6 +47,26 @@ enum _GAtServerResult {
> 
>  typedef enum _GAtServerResult GAtServerResult;
> 
> +enum _GAtServerRequestType {
> +	G_AT_SERVER_REQUEST_TYPE_NONE,

> +	G_AT_SERVER_REQUEST_TYPE_ACTION,

What exactly does this one mean, how is it different from SET or NONE?

> +	G_AT_SERVER_REQUEST_TYPE_QUERY,
> +	G_AT_SERVER_REQUEST_TYPE_SET,
> +	G_AT_SERVER_REQUEST_TYPE_SUPPORT,
> +};
> +
> +typedef enum _GAtServerRequestType GAtServerRequestType;
> +
> +enum _GAtServerCommandType {
> +	G_AT_SERVER_COMMAND_TYPE_ACTION,
> +	G_AT_SERVER_COMMAND_TYPE_PARAMETER,

Again, what do these mean?  Might want to include a simple comment here.

> +};
> +
> +typedef enum _GAtServerCommandType GAtServerCommandType;
> +
> +typedef int (*GAtServerNotifyFunc)(GAtServerRequestType type, GAtResult
>  *result, +					gpointer user_data);
> +
>  GAtServer *g_at_server_new(GIOChannel *io);
> 
>  GAtServer *g_at_server_ref(GAtServer *server);
> 

Re: [PATCH 7/7] Add g_at_server_register to register callback

[Denis Kenzior]

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

Hi Zhenhua,

> ---
>  gatchat/gatserver.c |   47 
+++++++++++++++++++++++++++++++++++++++++++++++
>  gatchat/gatserver.h |    6 ++++++
>  2 files changed, 53 insertions(+), 0 deletions(-)
> 
> diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
> index bea51f5..c49e5f8 100644
> --- a/gatchat/gatserver.c
> +++ b/gatchat/gatserver.c
> @@ -987,3 +987,50 @@ gboolean g_at_server_set_debug(GAtServer *server,
>  GAtDebugFunc func,
> 
>  	return TRUE;
>  }
> +
> +guint g_at_server_register(GAtServer *server, const char *prefix,
> +					GAtServerCommandType type,
> +					GAtServerNotifyFunc notify,
> +					gpointer user_data,
> +					GDestroyNotify destroy_notify)
> +{
> +	struct at_command *node;
> +
> +	if (server == NULL || server->command_list == NULL)
> +		return 0;
> +
> +	if (notify == NULL)
> +		return 0;
> +
> +	if (prefix == NULL || strlen(prefix) == 0)
> +		return 0;
> +

> +	node = g_hash_table_lookup(server->command_list, prefix);
> +
> +	if (node) {
> +		g_hash_table_remove(server->command_list, prefix);
> +
> +		if (node->destroy_notify)
> +			node->destroy_notify(node->user_data);
> +
> +		g_free(node->prefix);
> +
> +		g_free(node);
> +	}

If you properly setup your hash table then you can simply get rid of this 
part.

> +
> +	node = g_try_new0(struct at_command, 1);
> +
> +	if (!node)
> +		return 0;
> +
> +	node->id = server->next_command_id++;
> +	node->prefix = g_strdup(prefix);
> +	node->type = type;
> +	node->notify = notify;
> +	node->user_data = user_data;
> +	node->destroy_notify = destroy_notify;
> +
> +	g_hash_table_insert(server->command_list, node->prefix, node);

Use g_hash_table_replace here.

> +
> +	return node->id;
> +}
> diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
> index 5db9321..59cdf25 100644
> --- a/gatchat/gatserver.h
> +++ b/gatchat/gatserver.h
> @@ -80,6 +80,12 @@ gboolean g_at_server_set_debug(GAtServer *server,
>  					GAtDebugFunc func,
>  					gpointer user);
> 
> +guint g_at_server_register(GAtServer *server, const char *prefix,
> +					GAtServerCommandType type,
> +					GAtServerNotifyFunc notify,
> +					gpointer user_data,
> +					GDestroyNotify destroy_notify);

Return a gboolean here.  And where's my unregister function?

> +
>  #ifdef __cplusplus
>  }
>  #endif
> 

Regards,
-Denis

Re: [PATCH 5/7] Add server at command data structure

[Zhenhua Zhang]

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

Hi Denis,

On 03/04/2010 03:23 AM, Denis Kenzior wrote:
> Hi Zhenhua,
>
>> ---
>>   struct _GAtServer;
>> @@ -46,6 +47,26 @@ enum _GAtServerResult {
>>
>>   typedef enum _GAtServerResult GAtServerResult;
>>
>> +enum _GAtServerRequestType {
>> +	G_AT_SERVER_REQUEST_TYPE_NONE,
>
>> +	G_AT_SERVER_REQUEST_TYPE_ACTION,
>
> What exactly does this one mean, how is it different from SET or NONE?

As we discussed, TYPE_ACTION will be removed.

>> +	G_AT_SERVER_REQUEST_TYPE_QUERY,
>> +	G_AT_SERVER_REQUEST_TYPE_SET,
>> +	G_AT_SERVER_REQUEST_TYPE_SUPPORT,
>> +};
>> +
>> +typedef enum _GAtServerRequestType GAtServerRequestType;
>> +
>> +enum _GAtServerCommandType {
>> +	G_AT_SERVER_COMMAND_TYPE_ACTION,
>> +	G_AT_SERVER_COMMAND_TYPE_PARAMETER,
>
> Again, what do these mean?  Might want to include a simple comment here.

This enum will be removed as well.

>> +};
>> +
>> +typedef enum _GAtServerCommandType GAtServerCommandType;
>> +
>> +typedef int (*GAtServerNotifyFunc)(GAtServerRequestType type, GAtResult
>>   *result, +					gpointer user_data);
>> +
>>   GAtServer *g_at_server_new(GIOChannel *io);
>>
>>   GAtServer *g_at_server_ref(GAtServer *server);
>>
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
>

Re: [PATCH 7/7] Add g_at_server_register to register callback

[Zhenhua Zhang]

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

Hi Denis,

On 03/04/2010 03:28 AM, Denis Kenzior wrote:
> Hi Zhenhua,
>
>> ---
>>   gatchat/gatserver.c |   47
> +++++++++++++++++++++++++++++++++++++++++++++++
>>   gatchat/gatserver.h |    6 ++++++
>>   2 files changed, 53 insertions(+), 0 deletions(-)
>>
>> +guint g_at_server_register(GAtServer *server, const char *prefix,
>> +					GAtServerCommandType type,
>> +					GAtServerNotifyFunc notify,
>> +					gpointer user_data,
>> +					GDestroyNotify destroy_notify);
>
> Return a gboolean here.  And where's my unregister function?

OK. g_at_server_unregister will be added next time. :-)

>> +
>>   #ifdef __cplusplus
>>   }
>>   #endif
>>
>
> Regards,
> -Denis
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
>