[PATCH 0/9] HFP AT parser skeleton

[PATCH 0/9] HFP AT parser skeleton

[Marcin Kraglak]

Hi,

This set contains skeleton of hfp_at parser and some basic unit tests.
Unit tests check proper commands handling for now. If API will be acceptable
more test cases for fetching values will be added.

Comments are welcome 

Marcin Kraglak (9):
  shared: Add skeleton of hfp_at parser
  unit: Add hfp_at tester
  shared/hfp_at: Add skeleton of hfp_at_process_data
  unit/test_hfp_at: Add test cases for processing basic commands
  shared/hfp_at: Add function for getting number
  shared/hfp_at: Add function to open container
  shared/hfp_at: Add function to close container
  shared/hfp_at: Add function to get string
  shared/hfp_at: Add function to get unquoted string

 .gitignore          |   1 +
 Makefile.am         |   8 +-
 src/shared/hfp_at.c | 350 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/hfp_at.h |  54 ++++++++
 unit/test-hfp-at.c  | 347 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 759 insertions(+), 1 deletion(-)
 create mode 100644 src/shared/hfp_at.c
 create mode 100644 src/shared/hfp_at.h
 create mode 100644 unit/test-hfp-at.c

-- 
1.8.5.3

[PATCH 1/9] shared: Add skeleton of hfp_at parser

[Marcin Kraglak]

It will parse AT frames in HFP profile implementation. Use can register
handlers for specified prefixes. Callbacks will be called with one of
hfp_cmd_type. Default handler will have prefix NULL, and type passed
to callback will be always HFP_AT_UNKNOWN.
---
 src/shared/hfp_at.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/hfp_at.h | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100644 src/shared/hfp_at.c
 create mode 100644 src/shared/hfp_at.h

diff --git a/src/shared/hfp_at.c b/src/shared/hfp_at.c
new file mode 100644
index 0000000..bd1899b
--- /dev/null
+++ b/src/shared/hfp_at.c
@@ -0,0 +1,50 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Intel Corporation. All rights reserved.
+ *
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+#include <stdbool.h>
+#include <string.h>
+#include <ctype.h>
+
+#include "src/shared/util.h"
+#include "src/shared/hfp_at.h"
+
+struct hfp_at {
+	const struct hfp_at_handler *cmd_handlers;
+};
+
+struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers)
+{
+	struct hfp_at *hfp_at;
+
+	hfp_at = new0(struct hfp_at, 1);
+	if (!hfp_at)
+		return NULL;
+
+	hfp_at->cmd_handlers = handlers;
+
+	return hfp_at;
+}
+
+void hfp_at_free(struct hfp_at *hfp_at)
+{
+	free(hfp_at);
+}
diff --git a/src/shared/hfp_at.h b/src/shared/hfp_at.h
new file mode 100644
index 0000000..fe074f2
--- /dev/null
+++ b/src/shared/hfp_at.h
@@ -0,0 +1,43 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Intel Corporation. All rights reserved.
+ *
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+struct hfp_at;
+
+enum hfp_cmd_type {
+	HFP_AT_READ,
+	HFP_AT_SET,
+	HFP_AT_TEST,
+	HFP_AT_COMMAND,
+	HFP_AT_UNKNOWN
+};
+
+typedef void (*hfp_at_cmd_cb)(struct hfp_at *hfp_at, enum hfp_cmd_type type,
+					const char *at, void *user_data);
+
+struct hfp_at_handler {
+	const char *prefix;
+	hfp_at_cmd_cb cb;
+};
+
+struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers);
+void hfp_at_free(struct hfp_at *hfp_at);
-- 
1.8.5.3

[PATCH 2/9] unit: Add hfp_at tester

[Marcin Kraglak]

It will test hfp_at parser. Initial versione performs three test cases
with hfp_at_new() calls.
---
 .gitignore         |  1 +
 Makefile.am        |  8 ++++-
 unit/test-hfp-at.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 103 insertions(+), 1 deletion(-)
 create mode 100644 unit/test-hfp-at.c

diff --git a/.gitignore b/.gitignore
index 94c0c78..fa34348 100644
--- a/.gitignore
+++ b/.gitignore
@@ -84,6 +84,7 @@ unit/test-gdbus-client
 unit/test-sdp
 unit/test-lib
 unit/test-mgmt
+unit/test-hfp-at
 tools/mgmt-tester
 tools/smp-tester
 tools/gap-tester
diff --git a/Makefile.am b/Makefile.am
index 0c79e58..c886ba8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -225,7 +225,8 @@ AM_CFLAGS += @DBUS_CFLAGS@ @GLIB_CFLAGS@
 AM_CPPFLAGS = -I$(builddir)/lib -I$(srcdir)/gdbus
 
 
-unit_tests += unit/test-eir unit/test-uuid unit/test-textfile unit/test-crc
+unit_tests += unit/test-eir unit/test-uuid unit/test-textfile unit/test-crc \
+		unit/test-hfp-at
 
 unit_test_eir_SOURCES = unit/test-eir.c src/eir.c src/uuid-helper.c
 unit_test_eir_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
@@ -239,6 +240,11 @@ unit_test_textfile_LDADD = @GLIB_LIBS@
 unit_test_crc_SOURCES = unit/test-crc.c monitor/crc.h monitor/crc.c
 unit_test_crc_LDADD = @GLIB_LIBS@
 
+unit_test_hfp_at_SOURCES = unit/test-hfp-at.c \
+				src/shared/util.h src/shared/util.c \
+				src/shared/hfp_at.h src/shared/hfp_at.c
+unit_test_hfp_at_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
+
 unit_tests += unit/test-ringbuf unit/test-queue
 
 unit_test_ringbuf_SOURCES = unit/test-ringbuf.c \
diff --git a/unit/test-hfp-at.c b/unit/test-hfp-at.c
new file mode 100644
index 0000000..4374a70
--- /dev/null
+++ b/unit/test-hfp-at.c
@@ -0,0 +1,95 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Intel Corporation
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <glib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include "src/shared/hfp_at.h"
+
+struct test_data {
+	const struct hfp_at_handler *handlers;
+};
+
+static void test_init(gconstpointer data)
+{
+	struct hfp_at *hfp_at;
+	const struct test_data *test_data = data;
+
+	hfp_at = hfp_at_new(test_data->handlers);
+	g_assert(hfp_at != NULL);
+
+	hfp_at_free(hfp_at);
+}
+
+static void process_generic(struct hfp_at *hfp_at, enum hfp_cmd_type type,
+						const char *at, void *user_data)
+{
+}
+
+const struct hfp_at_handler handlers_1[] = {};
+
+const struct hfp_at_handler handlers_2[] = {
+	{
+		.prefix = NULL,
+		.cb = process_generic
+	}
+};
+
+const struct hfp_at_handler handlers_3[] = {
+	{
+		.prefix = "+BRSF",
+		.cb = process_generic
+	},
+	{
+		.prefix = NULL,
+		.cb = process_generic
+	}
+};
+
+const struct test_data data_1 = {
+	.handlers = handlers_1,
+};
+
+const struct test_data data_2 = {
+	.handlers = handlers_2,
+};
+
+const struct test_data data_3 = {
+	.handlers = handlers_3,
+};
+
+int main(int argc, char *argv[])
+{
+	g_test_init(&argc, &argv, NULL);
+	/* Tests for hfp_at_new() */
+	g_test_add_data_func("/hfp_at/init_1", &data_1, test_init);
+	g_test_add_data_func("/hfp_at/init_2", &data_2, test_init);
+	g_test_add_data_func("/hfp_at/init_3", &data_3, test_init);
+
+	return g_test_run();
+}
-- 
1.8.5.3

[PATCH 3/9] shared/hfp_at: Add skeleton of hfp_at_process_data

[Marcin Kraglak]

It will handle AT frames and recognize their commands. It will also
call proper callback. If no callback was found, default handler
will be called.
---
 src/shared/hfp_at.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/hfp_at.h |   3 +
 2 files changed, 178 insertions(+)

diff --git a/src/shared/hfp_at.c b/src/shared/hfp_at.c
index bd1899b..eff0a38 100644
--- a/src/shared/hfp_at.c
+++ b/src/shared/hfp_at.c
@@ -29,8 +29,183 @@
 
 struct hfp_at {
 	const struct hfp_at_handler *cmd_handlers;
+	int offset;
 };
 
+/* Last cmd handler should have prefix NULL and will be returned if no other
+ * will be found
+ */
+static const struct hfp_at_handler *hfp_at_find_cb(struct hfp_at *hfp_at,
+						char *pref, uint8_t len)
+{
+	const struct hfp_at_handler *handler = hfp_at->cmd_handlers;
+
+	while (handler->prefix) {
+		if (strlen(handler->prefix) == len)
+			if (!memcmp(handler->prefix, pref, len))
+				break;
+
+		handler++;
+	}
+
+	return handler;
+}
+
+static void hfp_at_call_handler(struct hfp_at *hfp_at, char *prefix, int len,
+					const char *data, void *user_data,
+					enum hfp_cmd_type type)
+{
+	const struct hfp_at_handler *handler;
+
+	handler = hfp_at_find_cb(hfp_at, prefix, len);
+
+	if (handler->prefix) {
+		handler->cb(hfp_at, type, data, user_data);
+		return;
+	}
+
+	/* We set offset to 0 because it is unknown command */
+	hfp_at->offset = 0;
+	handler->cb(hfp_at, HFP_AT_UNKNOWN, data, user_data);
+}
+
+static bool hfp_at_process_basic(struct hfp_at *hfp_at, const char *data,
+								void *user_data)
+{
+	const char *prefix = data + hfp_at->offset;
+	enum hfp_cmd_type type;
+	char lookup_prefix[4];
+	uint8_t pref_len = 0;
+	bool equal = false;
+	int i;
+
+	/* Check if first sign is character */
+	if (isalpha(prefix[pref_len])) {
+		/* Handle S-parameter prefix */
+		if (toupper(prefix[pref_len]) == 'S') {
+			do {
+				pref_len++;
+			} while (isdigit(prefix[pref_len]));
+			/*S-parameter must be followed with number*/
+			if (pref_len == 1)
+				pref_len--;
+		} else {
+			/* It's just one-character prefix */
+			pref_len++;
+		}
+	} else if (prefix[pref_len] == '&' && isalpha(prefix[pref_len + 1])) {
+		/* Two-signed prefix starting with & */
+		pref_len = 2;
+	}
+
+	if (pref_len == 0 || pref_len > sizeof(lookup_prefix))
+		return false;
+
+	for (i = 0; i < pref_len; i++)
+		lookup_prefix[i] = toupper(prefix[i]);
+
+	hfp_at->offset += pref_len;
+
+	if (lookup_prefix[0] == 'D') {
+		type = HFP_AT_SET;
+
+		goto done;
+	}
+
+	if (data[hfp_at->offset] == '=') {
+		hfp_at->offset++;
+		equal = true;
+	}
+
+	if (data[hfp_at->offset] == '?') {
+		hfp_at->offset++;
+
+		if (equal)
+			type = HFP_AT_TEST;
+		else
+			type = HFP_AT_READ;
+	} else if (equal) {
+		type = HFP_AT_SET;
+	} else {
+		type = HFP_AT_COMMAND;
+	}
+
+done:
+	hfp_at_call_handler(hfp_at, lookup_prefix, pref_len, data, user_data,
+									type);
+
+	return true;
+}
+
+static bool hfp_at_process_extended(struct hfp_at *hfp_at, const char *data,
+								void *user_data)
+{
+	const char *prefix = data + hfp_at->offset;
+	const char *separators = ";?=";
+	enum hfp_cmd_type type;
+	char lookup_prefix[18];
+	bool equal = false;
+	uint8_t pref_len;
+	int i;
+
+	/* Lookup for first separator */
+	pref_len = strcspn(prefix, separators);
+
+	if (pref_len > 17 || pref_len < 2)
+		return false;
+
+	for (i = 0; i < pref_len; i++)
+		lookup_prefix[i] = toupper(prefix[i]);
+
+	hfp_at->offset += pref_len;
+
+	if (data[hfp_at->offset] == '=') {
+		hfp_at->offset++;
+		equal = true;
+	}
+
+	if (data[hfp_at->offset] == '?') {
+		hfp_at->offset++;
+
+		if (equal)
+			type = HFP_AT_TEST;
+		else
+			type = HFP_AT_READ;
+	} else if (equal) {
+		type = HFP_AT_SET;
+	} else {
+		type = HFP_AT_COMMAND;
+	}
+
+	hfp_at_call_handler(hfp_at, lookup_prefix, pref_len, data, user_data,
+									type);
+
+	return true;
+}
+
+static bool hfp_at_is_extended_cmd(struct hfp_at *hfp_at, const char *data)
+{
+	/* Extended commands names always begin with character "+" V250 5.4.1*/
+	return data[hfp_at->offset] == '+';
+}
+
+bool hfp_at_process_data(struct hfp_at *hfp_at, const char *data,
+								void *user_data)
+{
+	if (strlen(data) < 3)
+		return false;
+
+	if (strncmp(data, "AT", 2))
+		return false;
+
+	hfp_at->offset = 2;
+
+	if (hfp_at_is_extended_cmd(hfp_at, data))
+		return hfp_at_process_extended(hfp_at, data, user_data);
+	else
+		return hfp_at_process_basic(hfp_at, data, user_data);
+}
+
 struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers)
 {
 	struct hfp_at *hfp_at;
diff --git a/src/shared/hfp_at.h b/src/shared/hfp_at.h
index fe074f2..0b628b5 100644
--- a/src/shared/hfp_at.h
+++ b/src/shared/hfp_at.h
@@ -39,5 +39,8 @@ struct hfp_at_handler {
 	hfp_at_cmd_cb cb;
 };
 
+bool hfp_at_process_data(struct hfp_at *hfp_at, const char *data,
+							void *user_data);
+
 struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers);
 void hfp_at_free(struct hfp_at *hfp_at);
-- 
1.8.5.3

[PATCH 4/9] unit/test_hfp_at: Add test cases for processing basic commands

[Marcin Kraglak]

It will test processing basic commands including S-parameter commands,
D-commands.
---
 unit/test-hfp-at.c | 252 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 252 insertions(+)

diff --git a/unit/test-hfp-at.c b/unit/test-hfp-at.c
index 4374a70..9cb716d 100644
--- a/unit/test-hfp-at.c
+++ b/unit/test-hfp-at.c
@@ -33,6 +33,9 @@
 
 struct test_data {
 	const struct hfp_at_handler *handlers;
+	const char *process_data;
+	bool process_return_val;
+	enum hfp_cmd_type returned_type;
 };
 
 static void test_init(gconstpointer data)
@@ -46,9 +49,27 @@ static void test_init(gconstpointer data)
 	hfp_at_free(hfp_at);
 }
 
+static void test_process_data(gconstpointer data)
+{
+	struct hfp_at *hfp_at;
+	const struct test_data *test_data = data;
+	bool ret;
+
+	hfp_at = hfp_at_new(test_data->handlers);
+	g_assert(hfp_at != NULL);
+
+	ret = hfp_at_process_data(hfp_at, test_data->process_data,
+							(void *)test_data);
+	g_assert(ret == test_data->process_return_val);
+
+	hfp_at_free(hfp_at);
+}
+
 static void process_generic(struct hfp_at *hfp_at, enum hfp_cmd_type type,
 						const char *at, void *user_data)
 {
+	struct test_data *test_data = user_data;
+	g_assert(type == test_data->returned_type);
 }
 
 const struct hfp_at_handler handlers_1[] = {};
@@ -62,8 +83,63 @@ const struct hfp_at_handler handlers_2[] = {
 
 const struct hfp_at_handler handlers_3[] = {
 	{
+		.prefix = NULL,
+		.cb = process_generic
+	},
+	{
 		.prefix = "+BRSF",
 		.cb = process_generic
+	}
+};
+
+const struct hfp_at_handler handlers_4[] = {
+	{
+		.prefix = "S12",
+		.cb = process_generic
+	},
+	{
+		.prefix = NULL,
+		.cb = process_generic
+	}
+};
+
+const struct hfp_at_handler handlers_5[] = {
+	{
+		.prefix = "S12",
+		.cb = process_generic
+	},
+	{
+		.prefix = NULL,
+		.cb = process_generic
+	}
+};
+
+const struct hfp_at_handler handlers_6[] = {
+	{
+		.prefix = "D",
+		.cb = process_generic
+	},
+	{
+		.prefix = NULL,
+		.cb = process_generic
+	}
+};
+
+const struct hfp_at_handler handlers_7[] = {
+	{
+		.prefix = "&A",
+		.cb = process_generic
+	},
+	{
+		.prefix = NULL,
+		.cb = process_generic
+	}
+};
+
+const struct hfp_at_handler handlers_8[] = {
+	{
+		.prefix = "A",
+		.cb = process_generic
 	},
 	{
 		.prefix = NULL,
@@ -83,6 +159,136 @@ const struct test_data data_3 = {
 	.handlers = handlers_3,
 };
 
+const struct test_data prefix_S_1_data = {
+	.handlers = handlers_4,
+	.process_data = "ATS12=12,6",
+	.process_return_val = true,
+	.returned_type = HFP_AT_SET
+};
+
+const struct test_data prefix_S_2_data = {
+	.handlers = handlers_4,
+	.process_data = "ATS12=?12,6",
+	.process_return_val = true,
+	.returned_type = HFP_AT_TEST
+};
+
+const struct test_data prefix_S_3_data = {
+	.handlers = handlers_4,
+	.process_data = "ATS12?",
+	.process_return_val = true,
+	.returned_type = HFP_AT_READ
+};
+
+const struct test_data prefix_S_4_data = {
+	.handlers = handlers_4,
+	.process_data = "ATS12",
+	.process_return_val = true,
+	.returned_type = HFP_AT_COMMAND
+};
+
+const struct test_data prefix_S_5_data = {
+	.handlers = handlers_5,
+	.process_data = "ATS1",
+	.process_return_val = true,
+	.returned_type = HFP_AT_UNKNOWN
+};
+
+const struct test_data prefix_D_1_data = {
+	.handlers = handlers_5,
+	.process_data = "ATD0046131415",
+	.process_return_val = true,
+	.returned_type = HFP_AT_UNKNOWN
+};
+
+const struct test_data prefix_D_2_data = {
+	.handlers = handlers_6,
+	.process_data = "ATD0046131415",
+	.process_return_val = true,
+	.returned_type = HFP_AT_SET
+};
+
+const struct test_data prefix_D_3_data = {
+	.handlers = handlers_2,
+	.process_data = "ATD0046131415",
+	.process_return_val = true,
+	.returned_type = HFP_AT_UNKNOWN
+};
+
+const struct test_data prefix_anda_1_data = {
+	.handlers = handlers_7,
+	.process_data = "AT&A=12",
+	.process_return_val = true,
+	.returned_type = HFP_AT_SET
+};
+
+const struct test_data prefix_anda_2_data = {
+	.handlers = handlers_7,
+	.process_data = "AT&A=?12",
+	.process_return_val = true,
+	.returned_type = HFP_AT_TEST
+};
+
+const struct test_data prefix_anda_3_data = {
+	.handlers = handlers_7,
+	.process_data = "AT&A?12",
+	.process_return_val = true,
+	.returned_type = HFP_AT_READ
+};
+
+const struct test_data prefix_anda_4_data = {
+	.handlers = handlers_7,
+	.process_data = "AT&A",
+	.process_return_val = true,
+	.returned_type = HFP_AT_COMMAND
+};
+
+const struct test_data prefix_a_1_data = {
+	.handlers = handlers_8,
+	.process_data = "ATA",
+	.process_return_val = true,
+	.returned_type = HFP_AT_COMMAND
+};
+
+const struct test_data prefix_a_2_data = {
+	.handlers = handlers_8,
+	.process_data = "ATA=12",
+	.process_return_val = true,
+	.returned_type = HFP_AT_SET
+};
+
+const struct test_data prefix_a_3_data = {
+	.handlers = handlers_8,
+	.process_data = "ATA=?12",
+	.process_return_val = true,
+	.returned_type = HFP_AT_TEST
+};
+
+const struct test_data prefix_a_4_data = {
+	.handlers = handlers_8,
+	.process_data = "ATA?",
+	.process_return_val = true,
+	.returned_type = HFP_AT_READ
+};
+
+const struct test_data invalid_command_1_data = {
+	.handlers = handlers_2,
+	.process_data = "AYA?",
+	.process_return_val = false,
+};
+
+const struct test_data invalid_command_2_data = {
+	.handlers = handlers_2,
+	.process_data = "AT?",
+	.process_return_val = false,
+};
+
+const struct test_data invalid_command_3_data = {
+	.handlers = handlers_2,
+	.process_data = "AT",
+	.process_return_val = false,
+};
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -90,6 +296,52 @@ int main(int argc, char *argv[])
 	g_test_add_data_func("/hfp_at/init_1", &data_1, test_init);
 	g_test_add_data_func("/hfp_at/init_2", &data_2, test_init);
 	g_test_add_data_func("/hfp_at/init_3", &data_3, test_init);
+	/* Basic commands tests */
+	g_test_add_data_func("/hfp_at/prefix_S_1", &prefix_S_1_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_S_2", &prefix_S_2_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_S_3", &prefix_S_3_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_S_4", &prefix_S_4_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_S_5", &prefix_S_5_data,
+							test_process_data);
+
+	g_test_add_data_func("/hfp_at/prefix_D_1", &prefix_D_1_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_D_2", &prefix_D_2_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_D_3", &prefix_D_3_data,
+							test_process_data);
+
+	g_test_add_data_func("/hfp_at/prefix_&A_1", &prefix_anda_1_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_&A_2", &prefix_anda_2_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_&A_3", &prefix_anda_3_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_&A_4", &prefix_anda_4_data,
+							test_process_data);
+
+	g_test_add_data_func("/hfp_at/prefix_A_1", &prefix_a_1_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_A_1", &prefix_a_2_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_A_1", &prefix_a_3_data,
+							test_process_data);
+	g_test_add_data_func("/hfp_at/prefix_A_1", &prefix_a_4_data,
+							test_process_data);
+
+	g_test_add_data_func("/hfp_at/invalid_command_1",
+					&invalid_command_1_data,
+					test_process_data);
+	g_test_add_data_func("/hfp_at/invalid_command_2",
+					&invalid_command_2_data,
+					test_process_data);
+	g_test_add_data_func("/hfp_at/invalid_command_3",
+					&invalid_command_3_data,
+					test_process_data);
 
 	return g_test_run();
 }
-- 
1.8.5.3

[PATCH 5/9] shared/hfp_at: Add function for getting number

[Marcin Kraglak]

It will fetch decimal value from AT command.
---
 src/shared/hfp_at.c | 40 ++++++++++++++++++++++++++++++++++++++++
 src/shared/hfp_at.h |  2 ++
 2 files changed, 42 insertions(+)

diff --git a/src/shared/hfp_at.c b/src/shared/hfp_at.c
index eff0a38..623dab3 100644
--- a/src/shared/hfp_at.c
+++ b/src/shared/hfp_at.c
@@ -206,6 +206,46 @@ bool hfp_at_process_data(struct hfp_at *hfp_at, const char *data,
 		return hfp_at_process_basic(hfp_at, data, user_data);
 }
 
+static void hfp_at_next_field(struct hfp_at *hfp_at, const char *data)
+{
+	if (data[hfp_at->offset] == ',')
+		hfp_at->offset++;
+}
+
+static void hfp_at_skip_whitespace(struct hfp_at *hfp_at, const char *data)
+{
+	while (data[hfp_at->offset] == ' ')
+		hfp_at->offset++;
+}
+
+bool hfp_at_get_number(struct hfp_at *hfp_at, const char *data, int *val)
+{
+	int temp = 0;
+	int i;
+
+	hfp_at_skip_whitespace(hfp_at, data);
+
+	i = hfp_at->offset;
+
+	while (data[i] >= '0' && data[i] <= '9') {
+		temp *= 10;
+		temp += data[i++] - '0';
+	}
+
+	if (i == hfp_at->offset)
+		return false;
+
+	if (val)
+		*val = temp;
+
+	hfp_at->offset = i;
+
+	hfp_at_skip_whitespace(hfp_at, data);
+	hfp_at_next_field(hfp_at, data);
+
+	return true;
+}
+
 struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers)
 {
 	struct hfp_at *hfp_at;
diff --git a/src/shared/hfp_at.h b/src/shared/hfp_at.h
index 0b628b5..2e2a757 100644
--- a/src/shared/hfp_at.h
+++ b/src/shared/hfp_at.h
@@ -42,5 +42,7 @@ struct hfp_at_handler {
 bool hfp_at_process_data(struct hfp_at *hfp_at, const char *data,
 							void *user_data);
 
+bool hfp_at_get_number(struct hfp_at *hfp_at, const char *data, int *val);
+
 struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers);
 void hfp_at_free(struct hfp_at *hfp_at);
-- 
1.8.5.3

[PATCH 6/9] shared/hfp_at: Add function to open container

[Marcin Kraglak]

It will look for "(" parenthesis and skip it if found.
---
 src/shared/hfp_at.c | 13 +++++++++++++
 src/shared/hfp_at.h |  1 +
 2 files changed, 14 insertions(+)

diff --git a/src/shared/hfp_at.c b/src/shared/hfp_at.c
index 623dab3..3bf1fd6 100644
--- a/src/shared/hfp_at.c
+++ b/src/shared/hfp_at.c
@@ -246,6 +246,19 @@ bool hfp_at_get_number(struct hfp_at *hfp_at, const char *data, int *val)
 	return true;
 }
 
+bool hfp_at_open_container(struct hfp_at *hfp_at, const char *data)
+{
+	hfp_at_skip_whitespace(hfp_at, data);
+
+	/* The list shall be preceded by a left parenthesis "(") */
+	if (data[hfp_at->offset] != '(')
+		return false;
+
+	hfp_at->offset++;
+
+	return true;
+}
+
 struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers)
 {
 	struct hfp_at *hfp_at;
diff --git a/src/shared/hfp_at.h b/src/shared/hfp_at.h
index 2e2a757..d5dcf9d 100644
--- a/src/shared/hfp_at.h
+++ b/src/shared/hfp_at.h
@@ -43,6 +43,7 @@ bool hfp_at_process_data(struct hfp_at *hfp_at, const char *data,
 							void *user_data);
 
 bool hfp_at_get_number(struct hfp_at *hfp_at, const char *data, int *val);
+bool hfp_at_open_container(struct hfp_at *hfp_at, const char *data);
 
 struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers);
 void hfp_at_free(struct hfp_at *hfp_at);
-- 
1.8.5.3

[PATCH 7/9] shared/hfp_at: Add function to close container

[Marcin Kraglak]

It will look for ")" parenthesis and skip it if found.
---
 src/shared/hfp_at.c | 13 +++++++++++++
 src/shared/hfp_at.h |  1 +
 2 files changed, 14 insertions(+)

diff --git a/src/shared/hfp_at.c b/src/shared/hfp_at.c
index 3bf1fd6..2db40b2 100644
--- a/src/shared/hfp_at.c
+++ b/src/shared/hfp_at.c
@@ -259,6 +259,19 @@ bool hfp_at_open_container(struct hfp_at *hfp_at, const char *data)
 	return true;
 }
 
+bool hfp_at_close_container(struct hfp_at *hfp_at, const char *data)
+{
+	hfp_at_skip_whitespace(hfp_at, data);
+
+	/* The list shall be followed by a right parenthesis (")" V250 5.7.3.1*/
+	if (data[hfp_at->offset] != ')')
+		return false;
+
+	hfp_at->offset++;
+
+	return true;
+}
+
 struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers)
 {
 	struct hfp_at *hfp_at;
diff --git a/src/shared/hfp_at.h b/src/shared/hfp_at.h
index d5dcf9d..e709acb 100644
--- a/src/shared/hfp_at.h
+++ b/src/shared/hfp_at.h
@@ -44,6 +44,7 @@ bool hfp_at_process_data(struct hfp_at *hfp_at, const char *data,
 
 bool hfp_at_get_number(struct hfp_at *hfp_at, const char *data, int *val);
 bool hfp_at_open_container(struct hfp_at *hfp_at, const char *data);
+bool hfp_at_close_container(struct hfp_at *hfp_at, const char *data);
 
 struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers);
 void hfp_at_free(struct hfp_at *hfp_at);
-- 
1.8.5.3

[PATCH 8/9] shared/hfp_at: Add function to get string

[Marcin Kraglak]

It will look for quoted sting and write it to buffer.
---
 src/shared/hfp_at.c | 32 ++++++++++++++++++++++++++++++++
 src/shared/hfp_at.h |  2 ++
 2 files changed, 34 insertions(+)

diff --git a/src/shared/hfp_at.c b/src/shared/hfp_at.c
index 2db40b2..750133f 100644
--- a/src/shared/hfp_at.c
+++ b/src/shared/hfp_at.c
@@ -246,6 +246,38 @@ bool hfp_at_get_number(struct hfp_at *hfp_at, const char *data, int *val)
 	return true;
 }
 
+bool hfp_at_get_string(struct hfp_at *hfp_at, const char *data, char *buf,
+								uint8_t len)
+{
+	int i = 0;
+
+	hfp_at_skip_whitespace(hfp_at, data);
+
+	if (data[hfp_at->offset] != '"')
+		return false;
+
+	hfp_at->offset++;
+
+	while (data[hfp_at->offset] != '\0' && data[hfp_at->offset] != '"') {
+		if (i < len)
+			buf[i++] = data[hfp_at->offset];
+		hfp_at->offset++;
+	}
+
+	if (i < len)
+		buf[i++] = '\0';
+
+	if (data[hfp_at->offset] == '"')
+		hfp_at->offset++;
+	else
+		return false;
+
+	hfp_at_skip_whitespace(hfp_at, data);
+	hfp_at_next_field(hfp_at, data);
+
+	return true;
+}
+
 bool hfp_at_open_container(struct hfp_at *hfp_at, const char *data)
 {
 	hfp_at_skip_whitespace(hfp_at, data);
diff --git a/src/shared/hfp_at.h b/src/shared/hfp_at.h
index e709acb..4da381b 100644
--- a/src/shared/hfp_at.h
+++ b/src/shared/hfp_at.h
@@ -45,6 +45,8 @@ bool hfp_at_process_data(struct hfp_at *hfp_at, const char *data,
 bool hfp_at_get_number(struct hfp_at *hfp_at, const char *data, int *val);
 bool hfp_at_open_container(struct hfp_at *hfp_at, const char *data);
 bool hfp_at_close_container(struct hfp_at *hfp_at, const char *data);
+bool hfp_at_get_string(struct hfp_at *hfp_at, const char *data, char *buf,
+								uint8_t len);
 
 struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers);
 void hfp_at_free(struct hfp_at *hfp_at);
-- 
1.8.5.3

[PATCH 9/9] shared/hfp_at: Add function to get unquoted string

[Marcin Kraglak]

---
 src/shared/hfp_at.c | 27 +++++++++++++++++++++++++++
 src/shared/hfp_at.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/src/shared/hfp_at.c b/src/shared/hfp_at.c
index 750133f..50e8614 100644
--- a/src/shared/hfp_at.c
+++ b/src/shared/hfp_at.c
@@ -278,6 +278,33 @@ bool hfp_at_get_string(struct hfp_at *hfp_at, const char *data, char *buf,
 	return true;
 }
 
+bool hfp_at_get_unquoted_string(struct hfp_at *hfp_at, const char *data,
+							char *buf, uint8_t len)
+{
+	int i = 0;
+	char c;
+
+	hfp_at_skip_whitespace(hfp_at, data);
+
+	c = data[hfp_at->offset];
+	if (c == '"' || c == ')' || c == '(')
+		return false;
+
+	while (data[hfp_at->offset] != '\0' && data[hfp_at->offset] != ','
+					&& data[hfp_at->offset] != ')') {
+		if (i < len)
+			buf[i++] = data[hfp_at->offset];
+		hfp_at->offset++;
+	}
+
+	if (i < len)
+		buf[i++] = '\0';
+
+	hfp_at_next_field(hfp_at, data);
+
+	return true;
+}
+
 bool hfp_at_open_container(struct hfp_at *hfp_at, const char *data)
 {
 	hfp_at_skip_whitespace(hfp_at, data);
diff --git a/src/shared/hfp_at.h b/src/shared/hfp_at.h
index 4da381b..76bdb03 100644
--- a/src/shared/hfp_at.h
+++ b/src/shared/hfp_at.h
@@ -47,6 +47,8 @@ bool hfp_at_open_container(struct hfp_at *hfp_at, const char *data);
 bool hfp_at_close_container(struct hfp_at *hfp_at, const char *data);
 bool hfp_at_get_string(struct hfp_at *hfp_at, const char *data, char *buf,
 								uint8_t len);
+bool hfp_at_get_unquoted_string(struct hfp_at *hfp_at, const char *data,
+							char *buf, uint8_t len);
 
 struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers);
 void hfp_at_free(struct hfp_at *hfp_at);
-- 
1.8.5.3

Re: [PATCH 1/9] shared: Add skeleton of hfp_at parser

[Marcel Holtmann]

Hi Marcin,

> It will parse AT frames in HFP profile implementation. Use can register
> handlers for specified prefixes. Callbacks will be called with one of
> hfp_cmd_type. Default handler will have prefix NULL, and type passed
> to callback will be always HFP_AT_UNKNOWN.
> ---
> src/shared/hfp_at.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/hfp_at.h | 43 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 93 insertions(+)
> create mode 100644 src/shared/hfp_at.c
> create mode 100644 src/shared/hfp_at.h
> 
> diff --git a/src/shared/hfp_at.c b/src/shared/hfp_at.c
> new file mode 100644
> index 0000000..bd1899b
> --- /dev/null
> +++ b/src/shared/hfp_at.c
> @@ -0,0 +1,50 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2014  Intel Corporation. All rights reserved.
> + *
> + *
> + *  This library is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU Lesser General Public
> + *  License as published by the Free Software Foundation; either
> + *  version 2.1 of the License, or (at your option) any later version.
> + *
> + *  This library is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  Lesser General Public License for more details.
> + *
> + *  You should have received a copy of the GNU Lesser General Public
> + *  License along with this library; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +#include <stdbool.h>
> +#include <string.h>
> +#include <ctype.h>
> +
> +#include "src/shared/util.h"
> +#include "src/shared/hfp_at.h"
> +
> +struct hfp_at {
> +	const struct hfp_at_handler *cmd_handlers;
> +};
> +
> +struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers)
> +{
> +	struct hfp_at *hfp_at;
> +
> +	hfp_at = new0(struct hfp_at, 1);
> +	if (!hfp_at)
> +		return NULL;
> +
> +	hfp_at->cmd_handlers = handlers;
> +
> +	return hfp_at;
> +}
> +
> +void hfp_at_free(struct hfp_at *hfp_at)
> +{
> +	free(hfp_at);
> +}
> diff --git a/src/shared/hfp_at.h b/src/shared/hfp_at.h
> new file mode 100644
> index 0000000..fe074f2
> --- /dev/null
> +++ b/src/shared/hfp_at.h
> @@ -0,0 +1,43 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2014  Intel Corporation. All rights reserved.
> + *
> + *
> + *  This library is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU Lesser General Public
> + *  License as published by the Free Software Foundation; either
> + *  version 2.1 of the License, or (at your option) any later version.
> + *
> + *  This library is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  Lesser General Public License for more details.
> + *
> + *  You should have received a copy of the GNU Lesser General Public
> + *  License along with this library; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +struct hfp_at;
> +
> +enum hfp_cmd_type {
> +	HFP_AT_READ,
> +	HFP_AT_SET,
> +	HFP_AT_TEST,
> +	HFP_AT_COMMAND,
> +	HFP_AT_UNKNOWN
> +};
> +
> +typedef void (*hfp_at_cmd_cb)(struct hfp_at *hfp_at, enum hfp_cmd_type type,
> +					const char *at, void *user_data);
> +
> +struct hfp_at_handler {
> +	const char *prefix;
> +	hfp_at_cmd_cb cb;
> +};
> +
> +struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers);
> +void hfp_at_free(struct hfp_at *hfp_at);

so I rather see this integrated into hfp_gw natively. No need for new files or a new structure with all new resource allocation.

Lets just use hfp_gw_set_command_handler for the default case where we have no specific handler to parse the data since that will be needed for unknown AT command callback anyway.

And then add hfp_gw_register_command(hfp, prefix, callback, user_data, destroy) with a matching unregister function. Use a large table does not really makes sense to me. Individual commands similar to GAtServer works seems more reasonable and lot more flexible.

Regards

Marcel

Re: [PATCH 3/9] shared/hfp_at: Add skeleton of hfp_at_process_data

[Marcel Holtmann]

Hi Marcin,

> It will handle AT frames and recognize their commands. It will also
> call proper callback. If no callback was found, default handler
> will be called.
> ---
> src/shared/hfp_at.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/hfp_at.h |   3 +
> 2 files changed, 178 insertions(+)
> 
> diff --git a/src/shared/hfp_at.c b/src/shared/hfp_at.c
> index bd1899b..eff0a38 100644
> --- a/src/shared/hfp_at.c
> +++ b/src/shared/hfp_at.c
> @@ -29,8 +29,183 @@
> 
> struct hfp_at {
> 	const struct hfp_at_handler *cmd_handlers;
> +	int offset;
> };
> 
> +/* Last cmd handler should have prefix NULL and will be returned if no other
> + * will be found
> + */
> +static const struct hfp_at_handler *hfp_at_find_cb(struct hfp_at *hfp_at,
> +						char *pref, uint8_t len)
> +{
> +	const struct hfp_at_handler *handler = hfp_at->cmd_handlers;
> +
> +	while (handler->prefix) {
> +		if (strlen(handler->prefix) == len)
> +			if (!memcmp(handler->prefix, pref, len))
> +				break;
> +
> +		handler++;
> +	}
> +
> +	return handler;
> +}
> +
> +static void hfp_at_call_handler(struct hfp_at *hfp_at, char *prefix, int len,
> +					const char *data, void *user_data,
> +					enum hfp_cmd_type type)
> +{
> +	const struct hfp_at_handler *handler;
> +
> +	handler = hfp_at_find_cb(hfp_at, prefix, len);
> +
> +	if (handler->prefix) {
> +		handler->cb(hfp_at, type, data, user_data);
> +		return;
> +	}
> +
> +	/* We set offset to 0 because it is unknown command */
> +	hfp_at->offset = 0;
> +	handler->cb(hfp_at, HFP_AT_UNKNOWN, data, user_data);
> +}
> +
> +static bool hfp_at_process_basic(struct hfp_at *hfp_at, const char *data,
> +								void *user_data)
> +{
> +	const char *prefix = data + hfp_at->offset;
> +	enum hfp_cmd_type type;
> +	char lookup_prefix[4];
> +	uint8_t pref_len = 0;
> +	bool equal = false;
> +	int i;
> +
> +	/* Check if first sign is character */
> +	if (isalpha(prefix[pref_len])) {
> +		/* Handle S-parameter prefix */
> +		if (toupper(prefix[pref_len]) == 'S') {
> +			do {
> +				pref_len++;
> +			} while (isdigit(prefix[pref_len]));
> +			/*S-parameter must be followed with number*/
> +			if (pref_len == 1)
> +				pref_len--;
> +		} else {
> +			/* It's just one-character prefix */
> +			pref_len++;
> +		}
> +	} else if (prefix[pref_len] == '&' && isalpha(prefix[pref_len + 1])) {
> +		/* Two-signed prefix starting with & */
> +		pref_len = 2;
> +	}

I am not sure we actually need this. HFP is pretty much only a subset. I am fine with not bothering with any of the details of V250 if we do not have to. Let Java deal with it if they want to.

> +
> +	if (pref_len == 0 || pref_len > sizeof(lookup_prefix))
> +		return false;
> +
> +	for (i = 0; i < pref_len; i++)
> +		lookup_prefix[i] = toupper(prefix[i]);
> +
> +	hfp_at->offset += pref_len;
> +
> +	if (lookup_prefix[0] == 'D') {
> +		type = HFP_AT_SET;
> +
> +		goto done;
> +	}
> +
> +	if (data[hfp_at->offset] == '=') {
> +		hfp_at->offset++;
> +		equal = true;
> +	}
> +
> +	if (data[hfp_at->offset] == '?') {
> +		hfp_at->offset++;
> +
> +		if (equal)
> +			type = HFP_AT_TEST;
> +		else
> +			type = HFP_AT_READ;
> +	} else if (equal) {
> +		type = HFP_AT_SET;
> +	} else {
> +		type = HFP_AT_COMMAND;
> +	}
> +
> +done:
> +	hfp_at_call_handler(hfp_at, lookup_prefix, pref_len, data, user_data,
> +									type);
> +
> +	return true;
> +}
> +
> +static bool hfp_at_process_extended(struct hfp_at *hfp_at, const char *data,
> +								void *user_data)
> +{
> +	const char *prefix = data + hfp_at->offset;
> +	const char *separators = ";?=";
> +	enum hfp_cmd_type type;
> +	char lookup_prefix[18];
> +	bool equal = false;
> +	uint8_t pref_len;
> +	int i;
> +
> +	/* Lookup for first separator */
> +	pref_len = strcspn(prefix, separators);
> +
> +	if (pref_len > 17 || pref_len < 2)
> +		return false;
> +
> +	for (i = 0; i < pref_len; i++)
> +		lookup_prefix[i] = toupper(prefix[i]);
> +
> +	hfp_at->offset += pref_len;
> +
> +	if (data[hfp_at->offset] == '=') {
> +		hfp_at->offset++;
> +		equal = true;
> +	}
> +
> +	if (data[hfp_at->offset] == '?') {
> +		hfp_at->offset++;
> +
> +		if (equal)
> +			type = HFP_AT_TEST;
> +		else
> +			type = HFP_AT_READ;
> +	} else if (equal) {
> +		type = HFP_AT_SET;
> +	} else {
> +		type = HFP_AT_COMMAND;
> +	}

Still trying to figure out why we need a variable equal here. Might want to restructure the code a little bit. 

Also do not forget to check the length of your string. Remote devices can send you garbage.

I also fail to see what difference between basic and extended is. Seems like a lot of the basic code is shared.

> +
> +	hfp_at_call_handler(hfp_at, lookup_prefix, pref_len, data, user_data,
> +									type);
> +
> +	return true;
> +}
> +
> +static bool hfp_at_is_extended_cmd(struct hfp_at *hfp_at, const char *data)
> +{
> +	/* Extended commands names always begin with character "+" V250 5.4.1*/
> +	return data[hfp_at->offset] == '+';
> +}

Lets not go overboard with functions that serve only a single purpose. Do that inline in the code.

> +
> +bool hfp_at_process_data(struct hfp_at *hfp_at, const char *data,
> +								void *user_data)
> +{
> +	if (strlen(data) < 3)
> +		return false;
> +
> +	if (strncmp(data, "AT", 2))
> +		return false;

actually they way I read is that AT and at are valid. However not At and not aT. We would also need to strip any leading whitespaces. This should be done on level down if I have not already implemented it.

> +
> +	hfp_at->offset = 2;
> +
> +	if (hfp_at_is_extended_cmd(hfp_at, data))
> +		return hfp_at_process_extended(hfp_at, data, user_data);
> +	else
> +		return hfp_at_process_basic(hfp_at, data, user_data);
> +}
> +
> struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers)
> {
> 	struct hfp_at *hfp_at;
> diff --git a/src/shared/hfp_at.h b/src/shared/hfp_at.h
> index fe074f2..0b628b5 100644
> --- a/src/shared/hfp_at.h
> +++ b/src/shared/hfp_at.h
> @@ -39,5 +39,8 @@ struct hfp_at_handler {
> 	hfp_at_cmd_cb cb;
> };
> 
> +bool hfp_at_process_data(struct hfp_at *hfp_at, const char *data,
> +							void *user_data);
> +
> struct hfp_at *hfp_at_new(const struct hfp_at_handler *handlers);
> void hfp_at_free(struct hfp_at *hfp_at);
> -- 
> 1.8.5.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>