[PATCH 1/5] common: create GList helper ofono_call_compare

[PATCH 1/5] common: create GList helper ofono_call_compare

[Alexander Couzens]

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

replaces at_util_call_compare (atmodem) and
call_compare (rild).
---
 drivers/atmodem/atutil.c        | 17 ++---------------
 drivers/atmodem/atutil.h        |  1 -
 drivers/atmodem/voicecall.c     |  2 +-
 drivers/hfpmodem/voicecall.c    |  2 +-
 drivers/huaweimodem/voicecall.c |  2 +-
 drivers/ifxmodem/voicecall.c    |  2 +-
 drivers/rilmodem/voicecall.c    | 16 +---------------
 drivers/stemodem/voicecall.c    |  2 +-
 src/common.c                    | 14 ++++++++++++++
 src/common.h                    |  2 ++
 10 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/drivers/atmodem/atutil.c b/drivers/atmodem/atutil.c
index 14873483..1a48b606 100644
--- a/drivers/atmodem/atutil.c
+++ b/drivers/atmodem/atutil.c
@@ -31,6 +31,7 @@
 #define OFONO_API_SUBJECT_TO_CHANGE
 #include <ofono/log.h>
 #include <ofono/types.h>
+#include "../src/common.h"
 
 #include "atutil.h"
 #include "vendor.h"
@@ -101,20 +102,6 @@ gint at_util_call_compare_by_id(gconstpointer a, gconstpointer b)
 	return 0;
 }
 
-gint at_util_call_compare(gconstpointer a, gconstpointer b)
-{
-	const struct ofono_call *ca = a;
-	const struct ofono_call *cb = b;
-
-	if (ca->id < cb->id)
-		return -1;
-
-	if (ca->id > cb->id)
-		return 1;
-
-	return 0;
-}
-
 GSList *at_util_parse_clcc(GAtResult *result, unsigned int *ret_mpty_ids)
 {
 	GAtResultIter iter;
@@ -173,7 +160,7 @@ GSList *at_util_parse_clcc(GAtResult *result, unsigned int *ret_mpty_ids)
 		else
 			call->clip_validity = 2;
 
-		l = g_slist_insert_sorted(l, call, at_util_call_compare);
+		l = g_slist_insert_sorted(l, call, ofono_call_compare);
 
 		if (mpty)
 			mpty_ids |= 1 << id;
diff --git a/drivers/atmodem/atutil.h b/drivers/atmodem/atutil.h
index 5cb88b78..95e09900 100644
--- a/drivers/atmodem/atutil.h
+++ b/drivers/atmodem/atutil.h
@@ -54,7 +54,6 @@ void decode_at_error(struct ofono_error *error, const char *final);
 gint at_util_call_compare_by_status(gconstpointer a, gconstpointer b);
 gint at_util_call_compare_by_phone_number(gconstpointer a, gconstpointer b);
 gint at_util_call_compare_by_id(gconstpointer a, gconstpointer b);
-gint at_util_call_compare(gconstpointer a, gconstpointer b);
 GSList *at_util_parse_clcc(GAtResult *result, unsigned int *mpty_ids);
 gboolean at_util_parse_reg(GAtResult *result, const char *prefix,
 				int *mode, int *status,
diff --git a/drivers/atmodem/voicecall.c b/drivers/atmodem/voicecall.c
index e4c59c26..6fee39bc 100644
--- a/drivers/atmodem/voicecall.c
+++ b/drivers/atmodem/voicecall.c
@@ -132,7 +132,7 @@ static struct ofono_call *create_call(struct ofono_voicecall *vc, int type,
 	call->clip_validity = clip;
 	call->cnap_validity = CNAP_VALIDITY_NOT_AVAILABLE;
 
-	d->calls = g_slist_insert_sorted(d->calls, call, at_util_call_compare);
+	d->calls = g_slist_insert_sorted(d->calls, call, ofono_call_compare);
 
 	return call;
 }
diff --git a/drivers/hfpmodem/voicecall.c b/drivers/hfpmodem/voicecall.c
index ffdf4b7b..1a282903 100644
--- a/drivers/hfpmodem/voicecall.c
+++ b/drivers/hfpmodem/voicecall.c
@@ -128,7 +128,7 @@ static struct ofono_call *create_call(struct ofono_voicecall *vc, int type,
 		call->phone_number.type = num_type;
 	}
 
-	d->calls = g_slist_insert_sorted(d->calls, call, at_util_call_compare);
+	d->calls = g_slist_insert_sorted(d->calls, call, ofono_call_compare);
 
 	call->clip_validity = clip;
 
diff --git a/drivers/huaweimodem/voicecall.c b/drivers/huaweimodem/voicecall.c
index f55568d3..0887f652 100644
--- a/drivers/huaweimodem/voicecall.c
+++ b/drivers/huaweimodem/voicecall.c
@@ -76,7 +76,7 @@ static struct ofono_call *create_call(struct ofono_voicecall *vc, int type,
 
 	call->clip_validity = clip;
 
-	d->calls = g_slist_insert_sorted(d->calls, call, at_util_call_compare);
+	d->calls = g_slist_insert_sorted(d->calls, call, ofono_call_compare);
 
 	return call;
 }
diff --git a/drivers/ifxmodem/voicecall.c b/drivers/ifxmodem/voicecall.c
index 45b5ca42..581a7906 100644
--- a/drivers/ifxmodem/voicecall.c
+++ b/drivers/ifxmodem/voicecall.c
@@ -107,7 +107,7 @@ static struct ofono_call *create_call(struct ofono_voicecall *vc, int type,
 
 	call->clip_validity = clip;
 
-	d->calls = g_slist_insert_sorted(d->calls, call, at_util_call_compare);
+	d->calls = g_slist_insert_sorted(d->calls, call, ofono_call_compare);
 
 	return call;
 }
diff --git a/drivers/rilmodem/voicecall.c b/drivers/rilmodem/voicecall.c
index b7180b90..f248d871 100644
--- a/drivers/rilmodem/voicecall.c
+++ b/drivers/rilmodem/voicecall.c
@@ -117,20 +117,6 @@ done:
 	ofono_voicecall_disconnected(vc, reqdata->id, reason, NULL);
 }
 
-static int call_compare(gconstpointer a, gconstpointer b)
-{
-	const struct ofono_call *ca = a;
-	const struct ofono_call *cb = b;
-
-	if (ca->id < cb->id)
-		return -1;
-
-	if (ca->id > cb->id)
-		return 1;
-
-	return 0;
-}
-
 static void clcc_poll_cb(struct ril_msg *message, gpointer user_data)
 {
 	struct ofono_voicecall *vc = user_data;
@@ -209,7 +195,7 @@ static void clcc_poll_cb(struct ril_msg *message, gpointer user_data)
 			call->id, call->status, call->type,
 			call->phone_number.number, call->name);
 
-		calls = g_slist_insert_sorted(calls, call, call_compare);
+		calls = g_slist_insert_sorted(calls, call, ofono_call_compare);
 	}
 
 no_calls:
diff --git a/drivers/stemodem/voicecall.c b/drivers/stemodem/voicecall.c
index 356ab7c6..19276111 100644
--- a/drivers/stemodem/voicecall.c
+++ b/drivers/stemodem/voicecall.c
@@ -128,7 +128,7 @@ static struct ofono_call *create_call(struct ofono_voicecall *vc, int type,
 
 	call->clip_validity = clip;
 
-	d->calls = g_slist_insert_sorted(d->calls, call, at_util_call_compare);
+	d->calls = g_slist_insert_sorted(d->calls, call, ofono_call_compare);
 
 	return call;
 }
diff --git a/src/common.c b/src/common.c
index 17d1d58a..75bb94dc 100644
--- a/src/common.c
+++ b/src/common.c
@@ -736,3 +736,17 @@ void ofono_call_init(struct ofono_call *call)
 	call->cnap_validity = CNAP_VALIDITY_NOT_AVAILABLE;
 	call->clip_validity = CLIP_VALIDITY_NOT_AVAILABLE;
 }
+
+gint ofono_call_compare(gconstpointer a, gconstpointer b)
+{
+	const struct ofono_call *ca = a;
+	const struct ofono_call *cb = b;
+
+	if (ca->id < cb->id)
+		return -1;
+
+	if (ca->id > cb->id)
+		return 1;
+
+	return 0;
+}
diff --git a/src/common.h b/src/common.h
index 05f2a851..46bae577 100644
--- a/src/common.h
+++ b/src/common.h
@@ -184,3 +184,5 @@ const char *registration_tech_to_string(int tech);
 const char *packet_bearer_to_string(int bearer);
 
 gboolean is_valid_apn(const char *apn);
+
+gint ofono_call_compare(gconstpointer a, gconstpointer b);
-- 
2.13.3

[PATCH 2/5] voicecall,common: promote call_status_to_string() public

[Alexander Couzens]

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

call_status_to_string() is useful for debug output.
Change signature to contain enum call_status
Replace default case to get compiler warning when new enums added
---
 src/common.c    | 21 +++++++++++++++++++++
 src/common.h    |  1 +
 src/voicecall.c | 24 ++----------------------
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/src/common.c b/src/common.c
index 75bb94dc..6cfd7c92 100644
--- a/src/common.c
+++ b/src/common.c
@@ -750,3 +750,24 @@ gint ofono_call_compare(gconstpointer a, gconstpointer b)
 
 	return 0;
 }
+
+const char *ofono_call_status_to_string(enum call_status status)
+{
+	switch (status) {
+	case CALL_STATUS_ACTIVE:
+		return "active";
+	case CALL_STATUS_HELD:
+		return "held";
+	case CALL_STATUS_DIALING:
+		return "dialing";
+	case CALL_STATUS_ALERTING:
+		return "alerting";
+	case CALL_STATUS_INCOMING:
+		return "incoming";
+	case CALL_STATUS_WAITING:
+		return "waiting";
+	case CALL_STATUS_DISCONNECTED:
+		return "disconnected";
+	}
+	return "unknown";
+}
diff --git a/src/common.h b/src/common.h
index 46bae577..6cb2b521 100644
--- a/src/common.h
+++ b/src/common.h
@@ -186,3 +186,4 @@ const char *packet_bearer_to_string(int bearer);
 gboolean is_valid_apn(const char *apn);
 
 gint ofono_call_compare(gconstpointer a, gconstpointer b);
+const char *ofono_call_status_to_string(enum call_status status);
diff --git a/src/voicecall.c b/src/voicecall.c
index 6907b502..c99f11fa 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -174,26 +174,6 @@ static const char *disconnect_reason_to_string(enum ofono_disconnect_reason r)
 	}
 }
 
-static const char *call_status_to_string(int status)
-{
-	switch (status) {
-	case CALL_STATUS_ACTIVE:
-		return "active";
-	case CALL_STATUS_HELD:
-		return "held";
-	case CALL_STATUS_DIALING:
-		return "dialing";
-	case CALL_STATUS_ALERTING:
-		return "alerting";
-	case CALL_STATUS_INCOMING:
-		return "incoming";
-	case CALL_STATUS_WAITING:
-		return "waiting";
-	default:
-		return "disconnected";
-	}
-}
-
 static const char *phone_and_clip_to_string(const struct ofono_phone_number *n,
 						int clip_validity)
 {
@@ -421,7 +401,7 @@ static void append_voicecall_properties(struct voicecall *v,
 	ofono_bool_t mpty;
 	dbus_bool_t emergency_call;
 
-	status = call_status_to_string(call->status);
+	status = ofono_call_status_to_string(call->status);
 
 	ofono_dbus_dict_append(dict, "State", DBUS_TYPE_STRING, &status);
 
@@ -920,7 +900,7 @@ static void voicecall_set_call_status(struct voicecall *call, int status)
 
 	call->call->status = status;
 
-	status_str = call_status_to_string(status);
+	status_str = ofono_call_status_to_string(status);
 	path = voicecall_build_path(call->vc, call->call);
 
 	ofono_dbus_signal_property_changed(conn, path,
-- 
2.13.3

[PATCH 3/5] voicecall: use ofono_call_status_name in DBG messages

[Alexander Couzens]

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

---
 src/voicecall.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index c99f11fa..e3ae80cf 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -2250,9 +2250,10 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
 	struct voicecall *v = NULL;
 	struct ofono_call *newcall;
 
-	DBG("Got a voicecall event, status: %d, id: %u, number: %s"
-			" called_number: %s, called_name %s", call->status,
-			call->id, call->phone_number.number,
+	DBG("Got a voicecall event, status: %s (%d), id: %u, number: %s"
+			" called_number: %s, called_name %s",
+			ofono_call_status_to_string(call->status),
+			call->status, call->id, call->phone_number.number,
 			call->called_number.number, call->name);
 
 	l = g_slist_find_custom(vc->call_list, GUINT_TO_POINTER(call->id),
-- 
2.13.3

[PATCH 4/5] voicecall.h: declare struct ofono_modem

[Alexander Couzens]

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

Fix a warning when only voicecall.h is included:
'struct ofono_modem declared' inside parameter list will not be visible
outside of this definition or declaration struct ofono_voicecall
*ofono_voicecall_create(struct ofono_modem *modem,...
---
 include/voicecall.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/voicecall.h b/include/voicecall.h
index 4d0b3886..eb897e5e 100644
--- a/include/voicecall.h
+++ b/include/voicecall.h
@@ -28,6 +28,7 @@ extern "C" {
 
 #include <ofono/types.h>
 
+struct ofono_modem;
 struct ofono_voicecall;
 
 typedef void (*ofono_voicecall_cb_t)(const struct ofono_error *error,
-- 
2.13.3

[PATCH 5/5] add call-list helper to manage voice call lists

[Alexander Couzens]

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

Many drivers asks the modem for a complete call list of current calls.
These list of calls can be feeded into call-list which parse the
list and notify ofono for new calls.
---
 Makefile.am           |  13 ++-
 include/call-list.h   |  38 +++++++++
 src/call-list.c       | 100 ++++++++++++++++++++++
 unit/test-call-list.c | 225 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 373 insertions(+), 3 deletions(-)
 create mode 100644 include/call-list.h
 create mode 100644 src/call-list.c
 create mode 100644 unit/test-call-list.c

diff --git a/Makefile.am b/Makefile.am
index 078e3f13..727410aa 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -22,7 +22,7 @@ pkginclude_HEADERS = include/log.h include/plugin.h include/history.h \
 			include/private-network.h include/cdma-netreg.h \
 			include/cdma-provision.h include/handsfree.h \
 			include/handsfree-audio.h include/siri.h \
-			include/netmon.h include/lte.h
+			include/netmon.h include/lte.h include/call-list.h
 
 nodist_pkginclude_HEADERS = include/version.h
 
@@ -620,7 +620,7 @@ src_ofonod_SOURCES = $(builtin_sources) $(gatchat_sources) src/ofono.ver \
 			src/cdma-provision.c src/handsfree.c \
 			src/handsfree-audio.c src/bluetooth.h \
 			src/hfp.h src/siri.c \
-			src/netmon.c src/lte.c
+			src/netmon.c src/lte.c src/call-list.c
 
 src_ofonod_LDADD = gdbus/libgdbus-internal.la $(builtin_libadd) \
 			@GLIB_LIBS@ @DBUS_LIBS@ -ldl
@@ -794,7 +794,8 @@ unit_tests = unit/test-common unit/test-util unit/test-idmap \
 				unit/test-rilmodem-cs \
 				unit/test-rilmodem-sms \
 				unit/test-rilmodem-cb \
-				unit/test-rilmodem-gprs
+				unit/test-rilmodem-gprs \
+				unit/test-call-list
 
 noinst_PROGRAMS = $(unit_tests) \
 			unit/test-sms-root unit/test-mux unit/test-caif
@@ -836,6 +837,12 @@ unit_test_sms_root_SOURCES = unit/test-sms-root.c \
 unit_test_sms_root_LDADD = @GLIB_LIBS@
 unit_objects += $(unit_test_sms_root_OBJECTS)
 
+unit_test_call_list_SOURCES = \
+				     src/common.c src/util.c \
+				     src/call-list.c unit/test-call-list.c
+unit_test_call_list_LDADD = @GLIB_LIBS@ -ldl
+unit_objects += $(unit_test_call_list_OBJECTS)
+
 unit_test_mux_SOURCES = unit/test-mux.c $(gatchat_sources)
 unit_test_mux_LDADD = @GLIB_LIBS@
 unit_objects += $(unit_test_mux_OBJECTS)
diff --git a/include/call-list.h b/include/call-list.h
new file mode 100644
index 00000000..dbd6ddd6
--- /dev/null
+++ b/include/call-list.h
@@ -0,0 +1,38 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017 Alexander Couzens <lynxis@fe80.eu>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  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.
+ *
+ */
+
+#include <glib.h>
+
+struct ofono_voicecall;
+struct ofono_phone_number;
+
+/*
+ * Can be called by the driver in the dialing callback,
+ * when the new call id already known
+ */
+void ofono_call_list_dial_callback(struct ofono_voicecall *vc,
+				   GSList **call_list,
+				   const struct ofono_phone_number *ph,
+				   int call_id);
+
+/*
+ * Called with a list of known calls e.g. clcc.
+ * Call list will take ownership of all ofono call within the calls.
+ */
+void ofono_call_list_notify(struct ofono_voicecall *vc,
+			    GSList **call_list,
+			    GSList *calls);
diff --git a/src/call-list.c b/src/call-list.c
new file mode 100644
index 00000000..fd39719a
--- /dev/null
+++ b/src/call-list.c
@@ -0,0 +1,100 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017 Alexander Couzens <lynxis@fe80.eu>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  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.
+ *
+ */
+
+#include <glib.h>
+
+#include "common.h"
+
+#include <ofono/types.h>
+#include <ofono/log.h>
+#include <ofono/voicecall.h>
+#include <ofono/call-list.h>
+
+#include <string.h>
+
+void ofono_call_list_dial_callback(struct ofono_voicecall *vc,
+				   GSList **call_list,
+				   const struct ofono_phone_number *ph,
+				   int call_id)
+{
+	struct ofono_call *call;
+
+	call = g_new0(struct ofono_call, 1);
+	call->id = call_id;
+
+	memcpy(&call->called_number, ph, sizeof(*ph));
+	call->direction = CALL_DIRECTION_MOBILE_ORIGINATED;
+	call->status = CALL_STATUS_DIALING;
+	call->type = 0; /* voice */
+
+	*call_list = g_slist_insert_sorted(*call_list,
+					    call,
+					    ofono_call_compare);
+	ofono_voicecall_notify(vc, call);
+}
+
+void ofono_call_list_notify(struct ofono_voicecall *vc,
+			    GSList **call_list,
+			    GSList *calls)
+{
+	GSList *old_calls = *call_list;
+	GSList *new_calls = calls;
+	struct ofono_call *new_call, *old_call;
+
+	while (old_calls || new_calls) {
+		old_call = old_calls ? old_calls->data : NULL;
+		new_call = new_calls ? new_calls->data : NULL;
+
+		/* we drop disconnected calls and treat them as not existent */
+		if (new_call && new_call->status == CALL_STATUS_DISCONNECTED) {
+			new_calls = new_calls->next;
+			calls = g_slist_remove(calls, new_call);
+			g_free(new_call);
+			continue;
+		}
+
+		if (old_call &&
+				(new_call == NULL ||
+				(new_call->id > old_call->id))) {
+			ofono_voicecall_disconnected(
+						vc,
+						old_call->id,
+						OFONO_DISCONNECT_REASON_UNKNOWN,
+						NULL);
+			old_calls = old_calls->next;
+		} else if (new_call &&
+			   (old_call == NULL ||
+			   (new_call->id < old_call->id))) {
+
+			/* new call, signal it */
+			if (new_call->type == 0)
+				ofono_voicecall_notify(vc, new_call);
+
+			new_calls = new_calls->next;
+		} else {
+			if (memcmp(new_call, old_call, sizeof(*new_call))
+					&& new_call->type == 0)
+				ofono_voicecall_notify(vc, new_call);
+
+			new_calls = new_calls->next;
+			old_calls = old_calls->next;
+		}
+	}
+
+	g_slist_free_full(*call_list, g_free);
+	*call_list = calls;
+}
diff --git a/unit/test-call-list.c b/unit/test-call-list.c
new file mode 100644
index 00000000..980038ab
--- /dev/null
+++ b/unit/test-call-list.c
@@ -0,0 +1,225 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017 Alexander Couzens <lynxis@fe80.eu>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  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.
+ *
+ */
+
+
+#include <glib.h>
+#include <string.h>
+
+
+#include "../src/common.h"
+#include <ofono/types.h>
+#include <ofono/call-list.h>
+
+struct voicecall {
+};
+
+struct notified {
+	unsigned int id;
+	enum call_status status;
+};
+
+static struct notified notified_list[32];
+static int notified_idx;
+static int notified_check;
+
+void reset_notified(void)
+{
+	notified_idx = 0;
+	notified_check = 0;
+	memset(&notified_list, 0, sizeof(notified_list));
+}
+
+void ofono_voicecall_notify(struct ofono_voicecall *vc,
+				struct ofono_call *call)
+{
+	notified_list[notified_idx].id = call->id;
+	notified_list[notified_idx].status = call->status;
+	notified_idx++;
+}
+
+void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
+				enum ofono_disconnect_reason reason,
+				const struct ofono_error *error)
+{
+	notified_list[notified_idx].id = id;
+	notified_list[notified_idx].status = CALL_STATUS_DISCONNECTED;
+	notified_idx++;
+}
+
+static GSList *create_call(
+		GSList *calls,
+		unsigned int id,
+		enum call_status status,
+		enum call_direction direction)
+{
+	struct ofono_call *call = g_new0(struct ofono_call, 1);
+
+	call->id = id;
+	call->status = status;
+	call->direction = direction;
+
+	calls = g_slist_insert_sorted(calls, call, ofono_call_compare);
+
+	return calls;
+}
+
+static void assert_notified(unsigned int call_id, int call_status)
+{
+	g_assert(notified_idx >= notified_check);
+	g_assert(notified_list[notified_check].id == call_id);
+	g_assert(notified_list[notified_check].status == call_status);
+
+	notified_check++;
+}
+
+static void test_notify_disconnected(void)
+{
+	struct ofono_voicecall *vc = NULL;
+	struct ofono_phone_number ph;
+	GSList *call_list;
+	GSList *calls;
+
+	strcpy(ph.number, "004888123456");
+	ph.type = 0;
+
+	/* reset test */
+	reset_notified();
+	call_list = NULL;
+
+	/* fill disconnected call*/
+	calls = create_call(NULL, 1, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* incoming call */
+	calls = create_call(NULL, 1, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 1, CALL_STATUS_ALERTING,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* answer call */
+	calls = create_call(NULL, 1, CALL_STATUS_ACTIVE,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 1, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* another call waiting */
+	calls = create_call(NULL, 1, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 1, CALL_STATUS_ACTIVE,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 2, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 2, CALL_STATUS_WAITING,
+				   CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 2, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* end all calls */
+	ofono_call_list_notify(vc, &call_list, NULL);
+
+	/* verify call history */
+	assert_notified(1, CALL_STATUS_ALERTING);
+	assert_notified(1, CALL_STATUS_ACTIVE);
+	assert_notified(2, CALL_STATUS_WAITING);
+	assert_notified(1, CALL_STATUS_DISCONNECTED);
+	assert_notified(2, CALL_STATUS_DISCONNECTED);
+
+	g_assert(notified_check == notified_idx);
+	g_slist_free_full(call_list, g_free);
+}
+
+static void test_notify(void)
+{
+	struct ofono_voicecall *vc = NULL;
+	struct ofono_phone_number ph;
+	GSList *call_list;
+	GSList *calls;
+
+	strcpy(ph.number, "004888123456");
+	ph.type = 0;
+
+	/* reset test */
+	reset_notified();
+	call_list = NULL;
+
+	/* incoming call */
+	calls = create_call(NULL, 1, CALL_STATUS_ALERTING,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* answer call */
+	calls = create_call(NULL, 1, CALL_STATUS_ACTIVE,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* another call waiting */
+	calls = create_call(NULL, 1, CALL_STATUS_ACTIVE,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 2, CALL_STATUS_WAITING,
+				   CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* end all calls */
+	ofono_call_list_notify(vc, &call_list, NULL);
+
+	/* verify call history */
+	assert_notified(1, CALL_STATUS_ALERTING);
+	assert_notified(1, CALL_STATUS_ACTIVE);
+	assert_notified(2, CALL_STATUS_WAITING);
+	assert_notified(1, CALL_STATUS_DISCONNECTED);
+	assert_notified(2, CALL_STATUS_DISCONNECTED);
+
+	g_assert(notified_check == notified_idx);
+	g_slist_free_full(call_list, g_free);
+}
+
+static void test_dial_callback(void)
+{
+	struct ofono_voicecall *vc = NULL;
+	struct ofono_phone_number ph;
+	struct ofono_call *call;
+	GSList *call_list;
+
+	/* reset test */
+	reset_notified();
+	call_list = NULL;
+
+	strcpy(ph.number, "0099301234567890");
+	ph.type = 0;
+
+	ofono_call_list_dial_callback(vc, &call_list, &ph, 33);
+
+	call = call_list->data;
+
+	g_assert(strcmp(call->called_number.number, ph.number) == 0);
+	g_slist_free_full(call_list, g_free);
+}
+
+int main(int argc, char **argv)
+{
+	g_test_init(&argc, &argv, NULL);
+
+	g_test_add_func("/test-call-list/dial_callback", test_dial_callback);
+	g_test_add_func("/test-call-list/test_notify", test_notify);
+	g_test_add_func("/test-call-list/test_notify_disconnected",
+			test_notify_disconnected);
+	return g_test_run();
+}
-- 
2.13.3

[PATCH v2] add call-list helper to manage voice call lists

[Alexander Couzens]

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

Many drivers asks the modem for a complete call list of current calls.
These list of calls can be feeded into call-list which parse the
list and notify ofono for new calls.
---

v2: fix a bug when notify_calls is faster than the dial_callback.
Improved dial_callback testcase to cover this.

 Makefile.am           |  13 ++-
 include/call-list.h   |  38 ++++++++
 src/call-list.c       | 114 ++++++++++++++++++++++++
 unit/test-call-list.c | 237 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 399 insertions(+), 3 deletions(-)
 create mode 100644 include/call-list.h
 create mode 100644 src/call-list.c
 create mode 100644 unit/test-call-list.c

diff --git a/Makefile.am b/Makefile.am
index 078e3f13..4336297b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -22,7 +22,7 @@ pkginclude_HEADERS = include/log.h include/plugin.h include/history.h \
 			include/private-network.h include/cdma-netreg.h \
 			include/cdma-provision.h include/handsfree.h \
 			include/handsfree-audio.h include/siri.h \
-			include/netmon.h include/lte.h
+			include/netmon.h include/lte.h include/call-list.h
 
 nodist_pkginclude_HEADERS = include/version.h
 
@@ -620,7 +620,7 @@ src_ofonod_SOURCES = $(builtin_sources) $(gatchat_sources) src/ofono.ver \
 			src/cdma-provision.c src/handsfree.c \
 			src/handsfree-audio.c src/bluetooth.h \
 			src/hfp.h src/siri.c \
-			src/netmon.c src/lte.c
+			src/netmon.c src/lte.c src/call-list.c
 
 src_ofonod_LDADD = gdbus/libgdbus-internal.la $(builtin_libadd) \
 			@GLIB_LIBS@ @DBUS_LIBS@ -ldl
@@ -794,7 +794,8 @@ unit_tests = unit/test-common unit/test-util unit/test-idmap \
 				unit/test-rilmodem-cs \
 				unit/test-rilmodem-sms \
 				unit/test-rilmodem-cb \
-				unit/test-rilmodem-gprs
+				unit/test-rilmodem-gprs \
+				unit/test-call-list
 
 noinst_PROGRAMS = $(unit_tests) \
 			unit/test-sms-root unit/test-mux unit/test-caif
@@ -836,6 +837,12 @@ unit_test_sms_root_SOURCES = unit/test-sms-root.c \
 unit_test_sms_root_LDADD = @GLIB_LIBS@
 unit_objects += $(unit_test_sms_root_OBJECTS)
 
+unit_test_call_list_SOURCES = \
+				     src/common.c src/util.c src/log.c \
+				     src/call-list.c unit/test-call-list.c
+unit_test_call_list_LDADD = @GLIB_LIBS@ -ldl
+unit_objects += $(unit_test_call_list_OBJECTS)
+
 unit_test_mux_SOURCES = unit/test-mux.c $(gatchat_sources)
 unit_test_mux_LDADD = @GLIB_LIBS@
 unit_objects += $(unit_test_mux_OBJECTS)
diff --git a/include/call-list.h b/include/call-list.h
new file mode 100644
index 00000000..dbd6ddd6
--- /dev/null
+++ b/include/call-list.h
@@ -0,0 +1,38 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017 Alexander Couzens <lynxis@fe80.eu>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  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.
+ *
+ */
+
+#include <glib.h>
+
+struct ofono_voicecall;
+struct ofono_phone_number;
+
+/*
+ * Can be called by the driver in the dialing callback,
+ * when the new call id already known
+ */
+void ofono_call_list_dial_callback(struct ofono_voicecall *vc,
+				   GSList **call_list,
+				   const struct ofono_phone_number *ph,
+				   int call_id);
+
+/*
+ * Called with a list of known calls e.g. clcc.
+ * Call list will take ownership of all ofono call within the calls.
+ */
+void ofono_call_list_notify(struct ofono_voicecall *vc,
+			    GSList **call_list,
+			    GSList *calls);
diff --git a/src/call-list.c b/src/call-list.c
new file mode 100644
index 00000000..0eec4913
--- /dev/null
+++ b/src/call-list.c
@@ -0,0 +1,114 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017 Alexander Couzens <lynxis@fe80.eu>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  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.
+ *
+ */
+
+#include <glib.h>
+
+#include "common.h"
+
+#include <ofono/types.h>
+#include <ofono/log.h>
+#include <ofono/voicecall.h>
+#include <ofono/call-list.h>
+
+#include <string.h>
+
+void ofono_call_list_dial_callback(struct ofono_voicecall *vc,
+				   GSList **call_list,
+				   const struct ofono_phone_number *ph,
+				   int call_id)
+{
+	GSList *list;
+	struct ofono_call *call;
+
+	/* list_notify could be triggered before this call back is handled */
+	list = g_slist_find_custom(*call_list,
+				   GINT_TO_POINTER(call_id),
+				   ofono_call_compare_by_id);
+
+	if (list && list->data) {
+		call = list->data;
+		DBG("Call id %d already known. In state %s(%d)",
+		    call_id, ofono_call_status_to_string(call->status),
+		    call->status);
+		return;
+	}
+
+	call = g_new0(struct ofono_call, 1);
+	call->id = call_id;
+
+	memcpy(&call->called_number, ph, sizeof(*ph));
+	call->direction = CALL_DIRECTION_MOBILE_ORIGINATED;
+	call->status = CALL_STATUS_DIALING;
+	call->type = 0; /* voice */
+
+	*call_list = g_slist_insert_sorted(*call_list,
+					    call,
+					    ofono_call_compare);
+	ofono_voicecall_notify(vc, call);
+}
+
+void ofono_call_list_notify(struct ofono_voicecall *vc,
+			    GSList **call_list,
+			    GSList *calls)
+{
+	GSList *old_calls = *call_list;
+	GSList *new_calls = calls;
+	struct ofono_call *new_call, *old_call;
+
+	while (old_calls || new_calls) {
+		old_call = old_calls ? old_calls->data : NULL;
+		new_call = new_calls ? new_calls->data : NULL;
+
+		/* we drop disconnected calls and treat them as not existent */
+		if (new_call && new_call->status == CALL_STATUS_DISCONNECTED) {
+			new_calls = new_calls->next;
+			calls = g_slist_remove(calls, new_call);
+			g_free(new_call);
+			continue;
+		}
+
+		if (old_call &&
+				(new_call == NULL ||
+				(new_call->id > old_call->id))) {
+			ofono_voicecall_disconnected(
+						vc,
+						old_call->id,
+						OFONO_DISCONNECT_REASON_UNKNOWN,
+						NULL);
+			old_calls = old_calls->next;
+		} else if (new_call &&
+			   (old_call == NULL ||
+			   (new_call->id < old_call->id))) {
+
+			/* new call, signal it */
+			if (new_call->type == 0)
+				ofono_voicecall_notify(vc, new_call);
+
+			new_calls = new_calls->next;
+		} else {
+			if (memcmp(new_call, old_call, sizeof(*new_call))
+					&& new_call->type == 0)
+				ofono_voicecall_notify(vc, new_call);
+
+			new_calls = new_calls->next;
+			old_calls = old_calls->next;
+		}
+	}
+
+	g_slist_free_full(*call_list, g_free);
+	*call_list = calls;
+}
diff --git a/unit/test-call-list.c b/unit/test-call-list.c
new file mode 100644
index 00000000..896d3c93
--- /dev/null
+++ b/unit/test-call-list.c
@@ -0,0 +1,237 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017 Alexander Couzens <lynxis@fe80.eu>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  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.
+ *
+ */
+
+
+#include <glib.h>
+#include <string.h>
+
+
+#include "../src/common.h"
+#include <ofono/types.h>
+#include <ofono/call-list.h>
+
+struct voicecall {
+};
+
+struct notified {
+	unsigned int id;
+	enum call_status status;
+};
+
+static struct notified notified_list[32];
+static int notified_idx;
+static int notified_check;
+
+void reset_notified(void)
+{
+	notified_idx = 0;
+	notified_check = 0;
+	memset(&notified_list, 0, sizeof(notified_list));
+}
+
+void ofono_voicecall_notify(struct ofono_voicecall *vc,
+				struct ofono_call *call)
+{
+	notified_list[notified_idx].id = call->id;
+	notified_list[notified_idx].status = call->status;
+	notified_idx++;
+}
+
+void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
+				enum ofono_disconnect_reason reason,
+				const struct ofono_error *error)
+{
+	notified_list[notified_idx].id = id;
+	notified_list[notified_idx].status = CALL_STATUS_DISCONNECTED;
+	notified_idx++;
+}
+
+static GSList *create_call(
+		GSList *calls,
+		unsigned int id,
+		enum call_status status,
+		enum call_direction direction)
+{
+	struct ofono_call *call = g_new0(struct ofono_call, 1);
+
+	call->id = id;
+	call->status = status;
+	call->direction = direction;
+
+	calls = g_slist_insert_sorted(calls, call, ofono_call_compare);
+
+	return calls;
+}
+
+static void assert_notified(unsigned int call_id, int call_status)
+{
+	g_assert(notified_idx >= notified_check);
+	g_assert(notified_list[notified_check].id == call_id);
+	g_assert(notified_list[notified_check].status == call_status);
+
+	notified_check++;
+}
+
+static void test_notify_disconnected(void)
+{
+	struct ofono_voicecall *vc = NULL;
+	struct ofono_phone_number ph;
+	GSList *call_list;
+	GSList *calls;
+
+	strcpy(ph.number, "004888123456");
+	ph.type = 0;
+
+	/* reset test */
+	reset_notified();
+	call_list = NULL;
+
+	/* fill disconnected call*/
+	calls = create_call(NULL, 1, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* incoming call */
+	calls = create_call(NULL, 1, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 1, CALL_STATUS_ALERTING,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* answer call */
+	calls = create_call(NULL, 1, CALL_STATUS_ACTIVE,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 1, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* another call waiting */
+	calls = create_call(NULL, 1, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 1, CALL_STATUS_ACTIVE,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 2, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 2, CALL_STATUS_WAITING,
+				   CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 2, CALL_STATUS_DISCONNECTED,
+			    CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* end all calls */
+	ofono_call_list_notify(vc, &call_list, NULL);
+
+	/* verify call history */
+	assert_notified(1, CALL_STATUS_ALERTING);
+	assert_notified(1, CALL_STATUS_ACTIVE);
+	assert_notified(2, CALL_STATUS_WAITING);
+	assert_notified(1, CALL_STATUS_DISCONNECTED);
+	assert_notified(2, CALL_STATUS_DISCONNECTED);
+
+	g_assert(notified_check == notified_idx);
+	g_slist_free_full(call_list, g_free);
+}
+
+static void test_notify(void)
+{
+	struct ofono_voicecall *vc = NULL;
+	struct ofono_phone_number ph;
+	GSList *call_list;
+	GSList *calls;
+
+	strcpy(ph.number, "004888123456");
+	ph.type = 0;
+
+	/* reset test */
+	reset_notified();
+	call_list = NULL;
+
+	/* incoming call */
+	calls = create_call(NULL, 1, CALL_STATUS_ALERTING,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* answer call */
+	calls = create_call(NULL, 1, CALL_STATUS_ACTIVE,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* another call waiting */
+	calls = create_call(NULL, 1, CALL_STATUS_ACTIVE,
+			   CALL_DIRECTION_MOBILE_TERMINATED);
+	calls = create_call(calls, 2, CALL_STATUS_WAITING,
+				   CALL_DIRECTION_MOBILE_TERMINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+
+	/* end all calls */
+	ofono_call_list_notify(vc, &call_list, NULL);
+
+	/* verify call history */
+	assert_notified(1, CALL_STATUS_ALERTING);
+	assert_notified(1, CALL_STATUS_ACTIVE);
+	assert_notified(2, CALL_STATUS_WAITING);
+	assert_notified(1, CALL_STATUS_DISCONNECTED);
+	assert_notified(2, CALL_STATUS_DISCONNECTED);
+
+	g_assert(notified_check == notified_idx);
+	g_slist_free_full(call_list, g_free);
+}
+
+static void test_dial_callback(void)
+{
+	struct ofono_voicecall *vc = NULL;
+	struct ofono_phone_number ph;
+	struct ofono_call *call;
+	GSList *call_list, *calls;
+
+	/* reset test */
+	reset_notified();
+	call_list = NULL;
+
+	strcpy(ph.number, "0099301234567890");
+	ph.type = 0;
+
+	/* check if a call gets added to the call_list */
+	ofono_call_list_dial_callback(vc, &call_list, &ph, 33);
+
+	call = call_list->data;
+	g_assert(strcmp(call->called_number.number, ph.number) == 0);
+	g_slist_free_full(call_list, g_free);
+
+	/* check when notify is faster than dial_callback */
+	call_list = NULL;
+	calls = create_call(NULL, 1, CALL_STATUS_DIALING,
+			   CALL_DIRECTION_MOBILE_ORIGINATED);
+	ofono_call_list_notify(vc, &call_list, calls);
+	ofono_call_list_dial_callback(vc, &call_list, &ph, 1);
+	call = call_list->data;
+	g_assert(call_list->next == NULL);
+	g_slist_free_full(call_list, g_free);
+
+	call_list = NULL;
+}
+
+int main(int argc, char **argv)
+{
+	g_test_init(&argc, &argv, NULL);
+
+	g_test_add_func("/test-call-list/test_notify", test_notify);
+	g_test_add_func("/test-call-list/test_notify_disconnected",
+			test_notify_disconnected);
+	g_test_add_func("/test-call-list/dial_callback", test_dial_callback);
+	return g_test_run();
+}
-- 
2.13.3

Re: [PATCH 4/5] voicecall.h: declare struct ofono_modem

[Denis Kenzior]

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

Hi Alexander,

On 07/25/2017 08:26 AM, Alexander Couzens wrote:
> Fix a warning when only voicecall.h is included:
> 'struct ofono_modem declared' inside parameter list will not be visible
> outside of this definition or declaration struct ofono_voicecall
> *ofono_voicecall_create(struct ofono_modem *modem,...
> ---
>  include/voicecall.h | 1 +
>  1 file changed, 1 insertion(+)
>

Applied, thanks.

Regards,
-Denis

Re: [PATCH v2] add call-list helper to manage voice call lists

[Denis Kenzior]

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

Hi Alexander,

On 07/26/2017 06:23 PM, Alexander Couzens wrote:
> Many drivers asks the modem for a complete call list of current calls.
> These list of calls can be feeded into call-list which parse the
> list and notify ofono for new calls.
> ---
>
> v2: fix a bug when notify_calls is faster than the dial_callback.
> Improved dial_callback testcase to cover this.
>
>  Makefile.am           |  13 ++-
>  include/call-list.h   |  38 ++++++++
>  src/call-list.c       | 114 ++++++++++++++++++++++++
>  unit/test-call-list.c | 237 ++++++++++++++++++++++++++++++++++++++++++++++++++

So this is not in line with our patch submission guidelines.  See HACKING...

>  4 files changed, 399 insertions(+), 3 deletions(-)
>  create mode 100644 include/call-list.h
>  create mode 100644 src/call-list.c
>  create mode 100644 unit/test-call-list.c
>

<snip>

> diff --git a/include/call-list.h b/include/call-list.h
> new file mode 100644
> index 00000000..dbd6ddd6
> --- /dev/null
> +++ b/include/call-list.h
> @@ -0,0 +1,38 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2017 Alexander Couzens <lynxis@fe80.eu>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  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.
> + *
> + */
> +
> +#include <glib.h>
> +

We are not exposing glib into the oFono plugin API.  So the use of 
GSList, etc data structures across driver <-> core boundary is not 
possible.  The reasons are somewhat historic, but essentially non-GLib 
based plugins must be possible.  Also, oFono's use of GLib will be 
replaced by ell [1] in the future.

> +struct ofono_voicecall;
> +struct ofono_phone_number;
> +
> +/*
> + * Can be called by the driver in the dialing callback,
> + * when the new call id already known
> + */
> +void ofono_call_list_dial_callback(struct ofono_voicecall *vc,
> +				   GSList **call_list,
> +				   const struct ofono_phone_number *ph,
> +				   int call_id);
> +
> +/*
> + * Called with a list of known calls e.g. clcc.
> + * Call list will take ownership of all ofono call within the calls.
> + */
> +void ofono_call_list_notify(struct ofono_voicecall *vc,
> +			    GSList **call_list,
> +			    GSList *calls);

I'm still not quite sure what you're trying to solve with these changes? 
  A bit more info on the motivation of the patches in the commit 
description would be helpful...

Regards,
-Denis

[1] https://git.kernel.org/pub/scm/libs/ell/ell.git/

Re: [PATCH v2] add call-list helper to manage voice call lists

[Alexander Couzens]

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

On Mon, 7 Aug 2017 12:42:55 -0500
Denis Kenzior <denkenz@gmail.com> wrote:

Hi Dennis,

> So this is not in line with our patch submission guidelines.  See
> HACKING...
I'm sorry. I missed that one. Hopefully I got the coding style right so
far.

> We are not exposing glib into the oFono plugin API.  So the use of 
> GSList, etc data structures across driver <-> core boundary is not 
> possible.  The reasons are somewhat historic, but essentially
> non-GLib based plugins must be possible.  Also, oFono's use of GLib
> will be replaced by ell [1] in the future.
Interesting. Do you have any plans when and how the migratation will
be done?

> I'm still not quite sure what you're trying to solve with these
> changes? A bit more info on the motivation of the patches in the
> commit description would be helpful...

My motivation is to get rid of the generic part in the
modem drivers. E.g. look at the rild and atmodem, they have a lot
similiar code when parsing the call list.

So it would be nice to replace those. On the other side I've written a
voicecall driver for qmi which is using the call list, but I'm still
testing it.

Best,
Alex

PS. I better had tagged this patch RFC...
-- 
Alexander Couzens

mail: lynxis(a)fe80.eu
jabber: lynxis(a)fe80.eu
mobile: +4915123277221
gpg: 390D CF78 8BF9 AA50 4F8F  F1E2 C29E 9DA6 A0DF 8604

[-- Attachment #2: attachment.sig --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] add call-list helper to manage voice call lists

[Denis Kenzior]

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

Hi Alexander,

>> We are not exposing glib into the oFono plugin API.  So the use of
>> GSList, etc data structures across driver <-> core boundary is not
>> possible.  The reasons are somewhat historic, but essentially
>> non-GLib based plugins must be possible.  Also, oFono's use of GLib
>> will be replaced by ell [1] in the future.
> Interesting. Do you have any plans when and how the migratation will
> be done?
> 

No concrete timelines, but most likely we will simply introduce a 
dependency on ell at some point and start converting code over until all 
traces of GLib are eradicated.

>> I'm still not quite sure what you're trying to solve with these
>> changes? A bit more info on the motivation of the patches in the
>> commit description would be helpful...
> 
> My motivation is to get rid of the generic part in the
> modem drivers. E.g. look at the rild and atmodem, they have a lot
> similiar code when parsing the call list.
> 
> So it would be nice to replace those. On the other side I've written a
> voicecall driver for qmi which is using the call list, but I'm still
> testing it.
>

Okay, I see now.  In the ideal world the call state notification 
mechanism provides all the information needed so that a driver internal 
call list is not needed.  See calypsomodem/voicecall.c for example.

For drivers that do use a call list, maybe we can put the common code 
into drivers/common/*.[ch] or so.

Regards,
-Denis