Emergency Calls

From: Andras Domokos <[email protected]>

From: Andras Domokos <[email protected]>

Signed-off-by Andras Domokos <[email protected]>

---
 include/voicecall.h |   12 +++
 src/voicecall.c     |  221 ++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 215 insertions(+), 18 deletions(-)

include/voicecall.h +3 -0

index 2356fcf..d530148 100644
@@ -38,6 +38,9 @@ typedef void (*ofono_call_list_cb_t)(const struct ofono_error *error,
 					const struct ofono_call *call_list,
 					void *data);
 
+typedef void (*ofono_voicecall_emergency_notify_cb_t)(ofono_bool_t state,
+							void *data);
+
 /* Voice call related functionality, including ATD, ATA, +CHLD, CTFR, CLCC
  * and VTS.
  *

@@ -116,6 +119,15 @@ void ofono_voicecall_set_data(struct ofono_voicecall *vc, void *data);
 void *ofono_voicecall_get_data(struct ofono_voicecall *vc);
 int ofono_voicecall_get_next_callid(struct ofono_voicecall *vc);

include/voicecall.h +0 -0

 #ifdef __cplusplus
 }
 #endif

src/voicecall.c +4 -0

index 7b5fe3b..a619b30 100644
@@ -25,6 +25,7 @@
 
 #include <string.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <time.h>
 #include <errno.h>
 #include <stdint.h>

@@ -56,6 +57,9 @@ struct ofono_voicecall {
 	void *driver_data;
 	struct ofono_atom *atom;
 	struct dial_request *dial_req;
+	struct ofono_watchlist *emergency_watches;
+	unsigned int emergency_state;
+	unsigned int modem_state_watch;
 };
 
 struct voicecall {

src/voicecall.c +6 -0

 static gint call_compare_by_id(gconstpointer a, gconstpointer b)
 {

@@ -121,6 +127,145 @@ static void add_to_en_list(GSList **l, const char **list)
 		*l = g_slist_prepend(*l, g_strdup(list[i++]));
 }
 
+static gint number_compare(gconstpointer a, gconstpointer b)
+{
+	const char *s1 = a, *s2 = b;
+	return strcmp(s1, s2);
+}
+

src/voicecall.c +43 -0

+static void notify_emergency_watches(struct ofono_voicecall *vc,
+					ofono_bool_t state)
+{
+	struct ofono_watchlist_item *item;
+	GSList *l;
+	ofono_voicecall_emergency_notify_cb_t notify;
+
+	if (vc->emergency_watches == NULL)
+		return;
+
+	for (l = vc->emergency_watches->items; l; l = l->next) {
+		item = l->data;
+		notify = item->notify;
+		notify(state, item->notify_data);
+	}
+}
+
+unsigned int __ofono_voicecall_add_emergency_watch(struct ofono_voicecall *vc,
+				ofono_voicecall_emergency_notify_cb_t notify,
+				void *data, ofono_destroy_func destroy)
+{
+	struct ofono_watchlist_item *item;
+
+	if (vc == NULL || notify == NULL)
+		return 0;
+
+	item = g_new0(struct ofono_watchlist_item, 1);
+
+	item->notify = notify;
+	item->destroy = destroy;
+	item->notify_data = data;
+
+	return __ofono_watchlist_add_item(vc->emergency_watches, item);
+}
+
+void __ofono_voicecall_remove_emergency_watch(struct ofono_voicecall *vc,
+						unsigned int id)
+{
+	__ofono_watchlist_remove_item(vc->emergency_watches, id);
+}
+
+ofono_bool_t ofono_voicecall_get_emergency_state(struct ofono_voicecall *vc)
+{

src/voicecall.c +18 -0

+		ofono_dbus_signal_property_changed(conn, path,
+						OFONO_VOICECALL_INTERFACE,
+						"EmergencyMode",
+						DBUS_TYPE_BOOLEAN,
+						&state);
+		notify_emergency_watches(vc, state);
+	}
+}
+
+void __ofono_voicecall_dec_emergency_state(struct ofono_voicecall *vc)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(vc->atom);
+	gboolean state = FALSE;
+
+	if (!--vc->emergency_state) {
+		ofono_dbus_signal_property_changed(conn, path,
+						OFONO_VOICECALL_INTERFACE,

src/voicecall.c +42 -0

+static void modem_state_watch(enum ofono_modem_state state, void *data)
+{
+	struct ofono_voicecall *vc = data;
+	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
+	DBusMessage *reply;
+	const char *number;
+	struct ofono_phone_number ph;
+	const char *clirstr;
+	enum ofono_clir_option clir;
+
+	if (!vc->pending)
+		return;
+
+	if (strcmp(dbus_message_get_member(vc->pending), "Dial"))
+		return;
+
+	if (dbus_message_get_args(vc->pending, NULL, DBUS_TYPE_STRING, &number,
+					DBUS_TYPE_STRING, &clirstr,
+					DBUS_TYPE_INVALID) == FALSE) {
+		reply = __ofono_error_invalid_args(vc->pending);
+		__ofono_dbus_pending_reply(&vc->pending, reply);
+		return;
+	}
+
+	/* don't care here about non-emergency calls */
+	if (!emergency_number(vc, number))
+		return;
+
+	if (!ofono_modem_get_online(modem)) {
+		reply = __ofono_error_failed(vc->pending);
+		__ofono_dbus_pending_reply(&vc->pending, reply);
+		__ofono_voicecall_dec_emergency_state(vc);
+		return;
+	}
+
+	clir_string_to_clir(clirstr, &clir);
+	string_to_phone_number(number, &ph);
+
+	vc->driver->dial(vc, &ph, clir, OFONO_CUG_OPTION_DEFAULT,
+				manager_dial_callback, vc);
+}
+
 static const char *disconnect_reason_to_string(enum ofono_disconnect_reason r)
 {
 	switch (r) {

src/voicecall.c +1 -0

 {
 	GSList *l;
 	struct voicecall *v;

@@ -918,6 +1064,7 @@ static DBusMessage *manager_get_properties(DBusConnection *conn,
 	int i;
 	GSList *l;
 	char **list;
+	ofono_bool_t emergency_state;
 
 	reply = dbus_message_new_method_return(msg);
 

src/voicecall.c +12 -1

 	/* property EmergencyNumbers */
 	list = g_new0(char *, g_slist_length(vc->en_list) + 1);
 

@@ -1066,8 +1217,11 @@ static void manager_dial_callback(const struct ofono_error *error, void *data)
 
 		dbus_message_append_args(reply, DBUS_TYPE_OBJECT_PATH, &path,
 						DBUS_TYPE_INVALID);
-	} else
+	} else {
+		if (emergency_number(vc, number))
+			__ofono_voicecall_dec_emergency_state(vc);
 		reply = __ofono_error_failed(vc->pending);
+	}
 
 	__ofono_dbus_pending_reply(&vc->pending, reply);
 

@@ -1118,6 +1272,14 @@ static DBusMessage *manager_dial(DBusConnection *conn,
 
 	string_to_phone_number(number, &ph);
 
+	if (emergency_number(vc, number)) {
+		struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
+		ofono_bool_t online = ofono_modem_get_online(modem);
+		__ofono_voicecall_inc_emergency_state(vc);
+		if (!online)
+			return NULL;
+	}
+
 	vc->driver->dial(vc, &ph, clir, OFONO_CUG_OPTION_DEFAULT,
 				manager_dial_callback, vc);
 

src/voicecall.c +35 -15

 	time_t ts;
 	enum call_status prev_status;
 

@@ -1684,48 +1846,52 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
 		return;
 	}
 
-	call = l->data;
+	v = l->data;
 
 	ts = time(NULL);
-	prev_status = call->call->status;
+	prev_status = v->call->status;
 
 	l = g_slist_find_custom(vc->multiparty_list, GUINT_TO_POINTER(id),
 				call_compare_by_id);
 
 	if (l) {
 		vc->multiparty_list =
-			g_slist_remove(vc->multiparty_list, call);
+			g_slist_remove(vc->multiparty_list, v);
 
 		if (vc->multiparty_list->next == NULL) { /* Size == 1 */
-			struct voicecall *v = vc->multiparty_list->data;
+			struct voicecall *v2 = vc->multiparty_list->data;
 
-			voicecall_emit_multiparty(v, FALSE);
+			voicecall_emit_multiparty(v2, FALSE);
 			g_slist_free(vc->multiparty_list);
 			vc->multiparty_list = 0;
 		}
 	}
 
-	vc->release_list = g_slist_remove(vc->release_list, call);
+	vc->release_list = g_slist_remove(vc->release_list, v);
 
 	if (reason != OFONO_DISCONNECT_REASON_UNKNOWN)
-		voicecall_emit_disconnect_reason(call, reason);
+		voicecall_emit_disconnect_reason(v, reason);
 
-	voicecall_set_call_status(call, CALL_STATUS_DISCONNECTED);
+	voicecall_set_call_status(v, CALL_STATUS_DISCONNECTED);
 
-	if (!call->untracked) {
+	if (!v->untracked) {
 		if (prev_status == CALL_STATUS_INCOMING ||
 				prev_status == CALL_STATUS_WAITING)
-			__ofono_history_call_missed(modem, call->call, ts);
+			__ofono_history_call_missed(modem, v->call, ts);
 		else
-			__ofono_history_call_ended(modem, call->call,
-							call->detect_time, ts);
+			__ofono_history_call_ended(modem, v->call,
+							v->detect_time, ts);
 	}
 
-	voicecalls_emit_call_removed(vc, call);
+	voicecalls_emit_call_removed(vc, v);
+
+	if (emergency_number(vc,
+			phone_number_to_string(&v->call->phone_number)))
+		__ofono_voicecall_dec_emergency_state(vc);
 
-	voicecall_dbus_unregister(vc, call);
+	voicecall_dbus_unregister(vc, v);
 
-	vc->call_list = g_slist_remove(vc->call_list, call);
+	vc->call_list = g_slist_remove(vc->call_list, v);
 }
 
 void ofono_voicecall_notify(struct ofono_voicecall *vc,

@@ -1969,6 +2135,9 @@ static void voicecall_unregister(struct ofono_atom *atom)
 		vc->sim_watch = 0;
 	}
 
+	__ofono_watchlist_free(vc->emergency_watches);
+	vc->emergency_watches = NULL;
+
 	if (vc->dial_req)
 		dial_request_finish(vc);
 

@@ -1985,6 +2154,7 @@ static void voicecall_unregister(struct ofono_atom *atom)
 static void voicecall_remove(struct ofono_atom *atom)
 {
 	struct ofono_voicecall *vc = __ofono_atom_get_data(atom);
+	struct ofono_modem *modem = __ofono_atom_get_modem(atom);
 
 	DBG("atom: %p", atom);
 

@@ -2012,6 +2182,11 @@ static void voicecall_remove(struct ofono_atom *atom)
 		vc->sim = NULL;
 	}
 
+	if (vc->modem_state_watch) {
+		__ofono_modem_remove_state_watch(modem, vc->modem_state_watch);
+		vc->modem_state_watch = 0;
+	}
+
 	g_free(vc);
 }
 

@@ -2122,6 +2297,10 @@ void ofono_voicecall_register(struct ofono_voicecall *vc)
 	}
 
 	ofono_modem_add_interface(modem, OFONO_VOICECALL_MANAGER_INTERFACE);
+	vc->emergency_watches = __ofono_watchlist_new(g_free);
+	vc->modem_state_watch = __ofono_modem_add_state_watch(modem,
+							modem_state_watch,
+							vc, NULL);
 
 	/*
 	 * Start out with the 22.101 mandated numbers, if we have a SIM and

@@ -2208,6 +2387,9 @@ static void dial_request_cb(const struct ofono_error *error, void *data)
 
 	if (v == NULL) {
 		dial_request_finish(vc);
+		if (emergency_number(vc,
+				phone_number_to_string(&vc->dial_req->ph)))
+			__ofono_voicecall_inc_emergency_state(vc);
 		return;
 	}
 

Hi Andras,

Quoted 27 lines · Andras Domokos

From: Andras Domokos<[email protected]>

Signed-off-by Andras Domokos<[email protected]>

---
  include/voicecall.h |   12 +++
  src/voicecall.c     |  221 ++++++++++++++++++++++++++++++++++++++++++++++----
  2 files changed, 215 insertions(+), 18 deletions(-)

include/voicecall.h +3 -0

index 2356fcf..d530148 100644
@@ -38,6 +38,9 @@ typedef void (*ofono_call_list_cb_t)(const struct ofono_error *error,
                                       const struct ofono_call *call_list,
                                       void *data);
 
+typedef void (*ofono_voicecall_emergency_notify_cb_t)(ofono_bool_t state,
+                                                     void *data);
+
  /* Voice call related functionality, including ATD, ATA, +CHLD, CTFR, CLCC
   * and VTS.
   *

@@ -116,6 +119,15 @@ void ofono_voicecall_set_data(struct ofono_voicecall *vc, void *data);
  void *ofono_voicecall_get_data(struct ofono_voicecall *vc);
  int ofono_voicecall_get_next_callid(struct ofono_voicecall *vc);
 

include/voicecall.h void ofono_voicecall_set_data(struct ofono_voicecall *vc, void *data);

Quoted 10 lines

include/voicecall.h +9 -0

+unsigned int __ofono_voicecall_add_emergency_watch(struct ofono_voicecall *vc,
+                             ofono_voicecall_emergency_notify_cb_t notify,
+                             void *data, ofono_destroy_func destroy);
+void __ofono_voicecall_remove_emergency_watch(struct ofono_voicecall *vc,
+                                             unsigned int id);
+ofono_bool_t ofono_voicecall_get_emergency_state(struct ofono_voicecall *vc);
+void __ofono_voicecall_set_emergency_state(struct ofono_voicecall *vc,
+                                             int state);
+
 

Just some general comments, but this patch seems to be backwards from the earlier proposal. Namely EmergencyMode is a property on the Modem interface, not on the VoiceCallManager. See doc/modem-api.txt, Emergency property.

In general I think that the emergency_watch is unnecessary. Having a reference counted emergency tracking inside the modem object and a modem online state watch should be sufficient.

Quoted 27 lines

include/voicecall.h +0 -0

  #ifdef __cplusplus
  }

  #endif

src/voicecall.c +4 -0

index 7b5fe3b..a619b30 100644
@@ -25,6 +25,7 @@
 
  #include<string.h>
  #include<stdio.h>
+#include<stdlib.h>
  #include<time.h>
  #include<errno.h>
  #include<stdint.h>

@@ -56,6 +57,9 @@ struct ofono_voicecall {
       void *driver_data;
       struct ofono_atom *atom;
       struct dial_request *dial_req;
+     struct ofono_watchlist *emergency_watches;
+     unsigned int emergency_state;
+     unsigned int modem_state_watch;
  };
 
  struct voicecall {

@@ -85,6 +89,8 @@ static const char *default_en_list_no_sim[] = { "119", "118", "999", "110",
 

src/voicecall.c static const char *default_en_list_no_sim[] = { "119", "118", "999", "110",

Quoted 6 lines

src/voicecall.c +2 -0

  static void generic_callback(const struct ofono_error *error, void *data);
  static void multirelease_callback(const struct ofono_error *err, void *data);
+static const char *voicecall_build_path(struct ofono_voicecall *vc,
+                                     const struct ofono_call *call);
 
 

It is generally against the coding style to forward-declare static functions. If this function is needed, please simply move it higher.

Quoted 14 lines

src/voicecall.c +0 -0

  static gint call_compare_by_id(gconstpointer a, gconstpointer b)

  {

src/voicecall.c +7 -0

@@ -121,6 +127,145 @@ static void add_to_en_list(GSList **l, const char **list)
               *l = g_slist_prepend(*l, g_strdup(list[i++]));
  }
 
+static gint number_compare(gconstpointer a, gconstpointer b)
+{
+     const char *s1 = a, *s2 = b;
+     return strcmp(s1, s2);
+}
+
+static ofono_bool_t emergency_number(struct ofono_voicecall *vc,

src/voicecall.c static void add_to_en_list(GSList **l, const char **list) · line 146

Quoted 10 lines

src/voicecall.c +9 -0

+                                     const char *number)
+{
+     if (!number)
+             return FALSE;
+
+     return g_slist_find_custom(vc->en_list,
+                             number, number_compare) ? TRUE : FALSE;
+}
+
 

Please add this as a separate patch handling the 'Extend the voicecall interface with a property indicating whether this call is an emergency call' task

Quoted 44 lines

src/voicecall.c +44 -0

+static void notify_emergency_watches(struct ofono_voicecall *vc,
+                                     ofono_bool_t state)
+{
+     struct ofono_watchlist_item *item;
+     GSList *l;
+     ofono_voicecall_emergency_notify_cb_t notify;
+
+     if (vc->emergency_watches == NULL)
+             return;
+
+     for (l = vc->emergency_watches->items; l; l = l->next) {
+             item = l->data;
+             notify = item->notify;
+             notify(state, item->notify_data);
+     }
+}
+
+unsigned int __ofono_voicecall_add_emergency_watch(struct ofono_voicecall *vc,
+                             ofono_voicecall_emergency_notify_cb_t notify,
+                             void *data, ofono_destroy_func destroy)
+{
+     struct ofono_watchlist_item *item;
+
+     if (vc == NULL || notify == NULL)
+             return 0;
+
+     item = g_new0(struct ofono_watchlist_item, 1);
+
+     item->notify = notify;
+     item->destroy = destroy;
+     item->notify_data = data;
+
+     return __ofono_watchlist_add_item(vc->emergency_watches, item);
+}
+
+void __ofono_voicecall_remove_emergency_watch(struct ofono_voicecall *vc,
+                                             unsigned int id)
+{
+     __ofono_watchlist_remove_item(vc->emergency_watches, id);
+}
+
+ofono_bool_t ofono_voicecall_get_emergency_state(struct ofono_voicecall *vc)
+{
+     return vc->emergency_state ? 1 : 0;

src/voicecall.c static void add_to_en_list(GSList **l, const char **list)

Quoted 10 lines

src/voicecall.c +9 -0

+}
+
+void __ofono_voicecall_inc_emergency_state(struct ofono_voicecall *vc)
+{
+     DBusConnection *conn = ofono_dbus_get_connection();
+     const char *path = __ofono_atom_get_path(vc->atom);
+     gboolean state = TRUE;
+
+     if (!vc->emergency_state++) {
 

Please avoid such coding style. Parsing such a statement takes considerable effort.

vc->emergency_state++;

if (vc->emergency_state> 1)

         return;

ofono_dbus_signal...

would be much more readable.

Quoted 19 lines

src/voicecall.c +19 -0

+             ofono_dbus_signal_property_changed(conn, path,
+                                             OFONO_VOICECALL_INTERFACE,
+                                             "EmergencyMode",
+                                             DBUS_TYPE_BOOLEAN,
+&state);
+             notify_emergency_watches(vc, state);
+     }
+}
+
+void __ofono_voicecall_dec_emergency_state(struct ofono_voicecall *vc)
+{
+     DBusConnection *conn = ofono_dbus_get_connection();
+     const char *path = __ofono_atom_get_path(vc->atom);
+     gboolean state = FALSE;
+
+     if (!--vc->emergency_state) {
+             ofono_dbus_signal_property_changed(conn, path,
+                                             OFONO_VOICECALL_INTERFACE,
+                                             "EmergencyMode",

src/voicecall.c static void add_to_en_list(GSList **l, const char **list)

Quoted 10 lines

src/voicecall.c +9 -0

+                                             DBUS_TYPE_BOOLEAN,
+&state);
+             notify_emergency_watches(vc, state);
+     }
+}
+
+static ofono_bool_t clir_string_to_clir(const char *clirstr,
+                                     enum ofono_clir_option *clir);
+static void manager_dial_callback(const struct ofono_error *error, void *data);
 

Again, please avoid forward declarations

Quoted 46 lines

src/voicecall.c +42 -0

+static void modem_state_watch(enum ofono_modem_state state, void *data)
+{
+     struct ofono_voicecall *vc = data;
+     struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
+     DBusMessage *reply;
+     const char *number;
+     struct ofono_phone_number ph;
+     const char *clirstr;
+     enum ofono_clir_option clir;
+
+     if (!vc->pending)
+             return;
+
+     if (strcmp(dbus_message_get_member(vc->pending), "Dial"))
+             return;
+
+     if (dbus_message_get_args(vc->pending, NULL, DBUS_TYPE_STRING,&number,
+                                     DBUS_TYPE_STRING,&clirstr,
+                                     DBUS_TYPE_INVALID) == FALSE) {
+             reply = __ofono_error_invalid_args(vc->pending);
+             __ofono_dbus_pending_reply(&vc->pending, reply);
+             return;
+     }
+
+     /* don't care here about non-emergency calls */
+     if (!emergency_number(vc, number))
+             return;
+
+     if (!ofono_modem_get_online(modem)) {
+             reply = __ofono_error_failed(vc->pending);
+             __ofono_dbus_pending_reply(&vc->pending, reply);
+             __ofono_voicecall_dec_emergency_state(vc);
+             return;
+     }
+
+     clir_string_to_clir(clirstr,&clir);
+     string_to_phone_number(number,&ph);
+
+     vc->driver->dial(vc,&ph, clir, OFONO_CUG_OPTION_DEFAULT,
+                             manager_dial_callback, vc);
+}
+

  static const char *disconnect_reason_to_string(enum ofono_disconnect_reason r)
  {
       switch (r) {

src/voicecall.c static gboolean voicecalls_can_dtmf(struct ofono_voicecall *vc) · line 868

Quoted 7 lines

src/voicecall.c +2 -1

@@ -718,7 +863,8 @@ static gboolean voicecalls_can_dtmf(struct ofono_voicecall *vc)
       return FALSE;
  }
 
-static gboolean voicecalls_have_with_status(struct ofono_voicecall *vc, int status)
+static gboolean voicecalls_have_with_status(struct ofono_voicecall *vc,
+                                             int status)
 

Why is this here? This chunk does not seem related to this patch

Quoted 12 lines

src/voicecall.c +0 -0

  {
       GSList *l;

       struct voicecall *v;

src/voicecall.c +1 -0

@@ -918,6 +1064,7 @@ static DBusMessage *manager_get_properties(DBusConnection *conn,
       int i;
       GSList *l;
       char **list;
+     ofono_bool_t emergency_state;
 
       reply = dbus_message_new_method_return(msg);
 

src/voicecall.c static DBusMessage *manager_get_properties(DBusConnection *conn, · line 1084

Quoted 8 lines

src/voicecall.c +4 -0

@@ -930,6 +1077,10 @@ static DBusMessage *manager_get_properties(DBusConnection *conn,
                                       OFONO_PROPERTIES_ARRAY_SIGNATURE,
                                       &dict);
 
+     emergency_state = ofono_voicecall_get_emergency_state(vc);
+     ofono_dbus_dict_append(&dict, "EmergencyMode",
+                             DBUS_TYPE_BOOLEAN,&emergency_state);
+
 

As mentioned before, Emergency was agreed to be on the Modem interface

Quoted 32 lines

src/voicecall.c +12 -1

       /* property EmergencyNumbers */
       list = g_new0(char *, g_slist_length(vc->en_list) + 1);

@@ -1066,8 +1217,11 @@ static void manager_dial_callback(const struct ofono_error *error, void *data)
 
               dbus_message_append_args(reply, DBUS_TYPE_OBJECT_PATH,&path,
                                               DBUS_TYPE_INVALID);
-     } else
+     } else {
+             if (emergency_number(vc, number))
+                     __ofono_voicecall_dec_emergency_state(vc);
               reply = __ofono_error_failed(vc->pending);
+     }
 
       __ofono_dbus_pending_reply(&vc->pending, reply);
 

@@ -1118,6 +1272,14 @@ static DBusMessage *manager_dial(DBusConnection *conn,
 
       string_to_phone_number(number,&ph);
 
+     if (emergency_number(vc, number)) {
+             struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
+             ofono_bool_t online = ofono_modem_get_online(modem);
+             __ofono_voicecall_inc_emergency_state(vc);
+             if (!online)
+                     return NULL;
+     }
+
       vc->driver->dial(vc,&ph, clir, OFONO_CUG_OPTION_DEFAULT,
                               manager_dial_callback, vc);
 

src/voicecall.c void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id, · line 1833

Quoted 6 lines

src/voicecall.c +1 -1

@@ -1667,7 +1829,7 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
  {
       struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
       GSList *l;
-     struct voicecall *call;
+     struct voicecall *v;
 

Why? Please refrain from doing this. If you feel the variable naming could be improved, then send a separate patch. As it is, it has no bearing on emergency call handling...

Quoted 124 lines

src/voicecall.c +35 -15

       time_t ts;
       enum call_status prev_status;

@@ -1684,48 +1846,52 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
               return;
       }
 
-     call = l->data;
+     v = l->data;
 
       ts = time(NULL);
-     prev_status = call->call->status;
+     prev_status = v->call->status;
 
       l = g_slist_find_custom(vc->multiparty_list, GUINT_TO_POINTER(id),
                               call_compare_by_id);
 
       if (l) {
               vc->multiparty_list =
-                     g_slist_remove(vc->multiparty_list, call);
+                     g_slist_remove(vc->multiparty_list, v);
 
               if (vc->multiparty_list->next == NULL) { /* Size == 1 */
-                     struct voicecall *v = vc->multiparty_list->data;
+                     struct voicecall *v2 = vc->multiparty_list->data;
 
-                     voicecall_emit_multiparty(v, FALSE);
+                     voicecall_emit_multiparty(v2, FALSE);
                       g_slist_free(vc->multiparty_list);
                       vc->multiparty_list = 0;
               }
       }
 
-     vc->release_list = g_slist_remove(vc->release_list, call);
+     vc->release_list = g_slist_remove(vc->release_list, v);
 
       if (reason != OFONO_DISCONNECT_REASON_UNKNOWN)
-             voicecall_emit_disconnect_reason(call, reason);
+             voicecall_emit_disconnect_reason(v, reason);
 
-     voicecall_set_call_status(call, CALL_STATUS_DISCONNECTED);
+     voicecall_set_call_status(v, CALL_STATUS_DISCONNECTED);
 
-     if (!call->untracked) {
+     if (!v->untracked) {
               if (prev_status == CALL_STATUS_INCOMING ||
                               prev_status == CALL_STATUS_WAITING)
-                     __ofono_history_call_missed(modem, call->call, ts);
+                     __ofono_history_call_missed(modem, v->call, ts);
               else
-                     __ofono_history_call_ended(modem, call->call,
-                                                     call->detect_time, ts);
+                     __ofono_history_call_ended(modem, v->call,
+                                                     v->detect_time, ts);
       }
 
-     voicecalls_emit_call_removed(vc, call);
+     voicecalls_emit_call_removed(vc, v);
+
+     if (emergency_number(vc,
+                     phone_number_to_string(&v->call->phone_number)))
+             __ofono_voicecall_dec_emergency_state(vc);
 
-     voicecall_dbus_unregister(vc, call);
+     voicecall_dbus_unregister(vc, v);
 
-     vc->call_list = g_slist_remove(vc->call_list, call);
+     vc->call_list = g_slist_remove(vc->call_list, v);
  }
 
  void ofono_voicecall_notify(struct ofono_voicecall *vc,

@@ -1969,6 +2135,9 @@ static void voicecall_unregister(struct ofono_atom *atom)
               vc->sim_watch = 0;
       }
 
+     __ofono_watchlist_free(vc->emergency_watches);
+     vc->emergency_watches = NULL;
+
       if (vc->dial_req)
               dial_request_finish(vc);
 

@@ -1985,6 +2154,7 @@ static void voicecall_unregister(struct ofono_atom *atom)
  static void voicecall_remove(struct ofono_atom *atom)
  {
       struct ofono_voicecall *vc = __ofono_atom_get_data(atom);
+     struct ofono_modem *modem = __ofono_atom_get_modem(atom);
 
       DBG("atom: %p", atom);
 

@@ -2012,6 +2182,11 @@ static void voicecall_remove(struct ofono_atom *atom)
               vc->sim = NULL;
       }
 
+     if (vc->modem_state_watch) {
+             __ofono_modem_remove_state_watch(modem, vc->modem_state_watch);
+             vc->modem_state_watch = 0;
+     }
+
       g_free(vc);
  }
 

@@ -2122,6 +2297,10 @@ void ofono_voicecall_register(struct ofono_voicecall *vc)
       }
 
       ofono_modem_add_interface(modem, OFONO_VOICECALL_MANAGER_INTERFACE);
+     vc->emergency_watches = __ofono_watchlist_new(g_free);
+     vc->modem_state_watch = __ofono_modem_add_state_watch(modem,
+                                                     modem_state_watch,
+                                                     vc, NULL);
 
       /*
        * Start out with the 22.101 mandated numbers, if we have a SIM and

@@ -2208,6 +2387,9 @@ static void dial_request_cb(const struct ofono_error *error, void *data)
 
       if (v == NULL) {
               dial_request_finish(vc);
+             if (emergency_number(vc,
+                             phone_number_to_string(&vc->dial_req->ph)))
+                     __ofono_voicecall_inc_emergency_state(vc);
               return;
       }
 

@@ -2233,6 +2415,9 @@ static void dial_request_cb(const struct ofono_error *error, void *data)
 

src/voicecall.c static void dial_request_cb(const struct ofono_error *error, void *data) · line 2423

Quoted 9 lines

src/voicecall.c +3 -0

  static void dial_request(struct ofono_voicecall *vc)
  {
+     if (emergency_number(vc, phone_number_to_string(&vc->dial_req->ph)))
+             __ofono_voicecall_inc_emergency_state(vc);
+
       vc->driver->dial(vc,&vc->dial_req->ph, OFONO_CLIR_OPTION_DEFAULT,
                               OFONO_CUG_OPTION_DEFAULT, dial_request_cb, vc);
  }

Regards,
-Denis

Quoted 5 lines

In general I think that the emergency_watch is unnecessary. Having a reference counted emergency tracking inside the modem object and a modem online state watch should be sufficient.

The idea with the emergency watch is that any subsystem can get the notification when the emergency mode is entered and react on it. To give you a more complex example, it might well be that the gprs connection needs to be torn down when making an emergency call in 2G mode, there are such networks out there that prevents you from making an emergency call if your device is attached to a PDP context. In this given situation it comes to the question how to bring down the gprs connection. It can be done such that the gprs atom will tear down the connection after receiving the EmergencyMode notification, or another option is to have gprs connection handling functions made available by gprs and to deal with the gprs connection within voicecall (or somewhere else). The online/offline mode change handling in fact is bringing up the some issue, how the mode change handling should be implemented when making the emergency call. My idea was let every subsystem deal with the specifics of its own subsystem.

Quoted 5 lines

Just some general comments, but this patch seems to be backwards from the earlier proposal. Namely EmergencyMode is a property on the Modem interface, not on the VoiceCallManager. See doc/modem-api.txt, Emergency property.

I thought it was more logical to have the EmergencyMode property linked to voicecall since it is about a special call case, but I am fine with moving that property up to modem level.

Quoted 5 lines

In general I think that the emergency_watch is unnecessary. Having a reference counted emergency tracking inside the modem object and a modem online state watch should be sufficient.

The idea with the emergency watch is that any subsystem can get the notification when the emergency mode is entered and react on it.

To give you a more complex example, it might well be that the gprs connection needs to be torn down when making an emergency call in 2G mode, there are such networks out there that prevents you from making an emergency call if your device is attached to a PDP context. In this given situation it comes to the question how to bring down the gprs connection. It can be done such that the gprs atom will tear down the connection after receiving the EmergencyMode notification, or another option is to have gprs connection handling functions made

available by gprs and to deal with the gprs connection within voicecall (or somewhere else). The online/offline mode change handling in fact is bringing up the some issue, how the mode change handling should be implemented when making the emergency call. My idea was let every subsystem deal with the specifics of its own subsystem.

Quoted 4 lines

To give you a more complex example, it might well be that the gprs connection needs to be torn down when making an emergency call in 2G mode, there are such networks out there that prevents you from making an emergency call if your device is attached to a

PDP context.

Quoted 1 line

In this given situation it comes to the question how to

bring down the

Quoted 1 line

gprs connection. It can be done such that the gprs atom

will tear down

Quoted 2 lines

the connection after receiving the EmergencyMode notification, or another option is to have gprs connection handling functions made

Have we established that this is actually still needed? I thought you guys said all the networks that have this problem have been fixed by now?

Quoted 9 lines

Quoted 4 lines

To give you a more complex example, it might well be that the gprs connection needs to be torn down when making an emergency call in 2G mode, there are such networks out there that prevents you from making an emergency call if your device is attached to a

PDP context.

Quoted 1 line

In this given situation it comes to the question how to

bring down the

Quoted 1 line

gprs connection. It can be done such that the gprs atom

will tear down

Quoted 6 lines

Quoted 2 lines

the connection after receiving the EmergencyMode notification, or another option is to have gprs connection handling functions made

Have we established that this is actually still needed? I thought you guys said all the networks that have this problem have been fixed by now?

AFAIK, this is still a product requirement for us. I can recheck, but I just asked last week and the answer was a very definite "Yes, this is still required".

Quoted 1 line

Quoted 1 line

Have we established that this is actually still needed?

I thought you

Quoted 4 lines

Quoted 2 lines

guys said all the networks that have this problem have been fixed by now?

AFAIK, this is still a product requirement for us. I can

recheck, but I just asked last week and the answer was a very definite "Yes, this is still required".

as Denis and I mentioned, we have been told that this is not needed anymore. So where is this still needed?

Hi Andras,

Quoted 11 lines

Quoted 6 lines

Just some general comments, but this patch seems to be backwards from the earlier proposal. Namely EmergencyMode is a property on the Modem interface, not on the VoiceCallManager. See doc/modem-api.txt, Emergency property.

I thought it was more logical to have the EmergencyMode property linked to voicecall since it is about a special call case, but I am fine with moving that property up to modem level.

Oh I can see that as well, but I think earlier we agreed that it should be on the Modem interface. This has several advantages: offline / online toggling has to happen in the modem and it is easier for the power management framework to monitor it there. So unless you feel really strongly against that I suggest we stick with the earlier proposal.

Quoted 9 lines

Quoted 6 lines

In general I think that the emergency_watch is unnecessary. Having a reference counted emergency tracking inside the modem object and a modem online state watch should be sufficient.

The idea with the emergency watch is that any subsystem can get the notification when the emergency mode is entered and react on it.

Don't get me wrong, it might be useful in the future. But for the context of supporting emergency calls in the voicecall driver the emergency watch is not really needed. In general I prefer to review code which has an immediate or foreseeable need.

In this case if we detect an emergency call dial and we're offline, we:

- Save the pending call
- establish an online watch
- ofono_modem_inc_emergency_mode()

Once we are online:
- Dial the call

Once the call ends
- ofono_modem_dec_emergency_mode()

Nowhere do we actually need to use an emergency watch itself.

Quoted 9 lines

To give you a more complex example, it might well be that the gprs connection needs to be torn down when making an emergency call in 2G mode, there are such networks out there that prevents you from making an emergency call if your device is attached to a PDP context. In this given situation it comes to the question how to bring down the gprs connection. It can be done such that the gprs atom will tear down the connection after receiving the EmergencyMode notification, or another option is to have gprs connection handling functions made

Have we established that this is actually still needed? I thought you guys said all the networks that have this problem have been fixed by now?

If this is still required, I suggest you group the emergency watch functions with patches implementing the above functionality.

Quoted 6 lines

available by gprs and to deal with the gprs connection within voicecall (or somewhere else). The online/offline mode change handling in fact is bringing up the some issue, how the mode change handling should be implemented when making the emergency call. My idea was let every subsystem deal with the specifics of its own subsystem.

Let the modem figure out the specifics. Basically as long as the count for emergency is greater than 1, Offline mode should not be entered. Once it reaches 0, the online mode should go back to the previous value.

Regards,
-Denis