Emergency Calls

Emergency Calls

[Andras Domokos]

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

Hi,

This is a proposal for handling emergency calls in ofono:

Detect when an emergency call is requested/ended and notify the interested,
subscribed parties, so that steps can be taken to ensure that the emergency
call can be established and maintained.

The "EmergencyMode" boolean property added to the VoicecallManager D-Bus
interface will reflect the emergency call situation.

There is an emergency watchlist in ofono_voicecall, where the watchers of
the interested parties are stored. When there is a change with regards to 
emergency calls (call starts/ends), the watchers in the list are notified.
One interested watcher is the modem, it has to change the modem state from
offline to online for the duration of an emergency call.

The modem has a modem state watchlist, so that interested parties can 
learn about the modem state changes. For emergency calls, voicecall manager
needs to know when the modem reached the online state, if a modem state change
to online was necessary.

The important details are in the patches.

Cheers,
Andras

Andras Domokos (3):
  modem: modem state watch added
  voicecall: emergency call handling added
  modem: emergency state handling added

 include/voicecall.h |   12 +++
 src/modem.c         |  101 +++++++++++++++++++++---
 src/ofono.h         |   15 ++++
 src/voicecall.c     |  221 ++++++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 319 insertions(+), 30 deletions(-)

[RFC PATCH 1/3] modem: modem state watch added

[Andras Domokos]

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


Signed-off-by: Andras Domokos <andras.domokos@nokia.com>
---
 src/modem.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++-----------
 src/ofono.h |   15 +++++++++++++
 2 files changed, 67 insertions(+), 12 deletions(-)

diff --git a/src/modem.c b/src/modem.c
index 7a29edf..9aeb49c 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -51,16 +51,9 @@ enum property_type {
 	PROPERTY_TYPE_BOOLEAN,
 };
 
-enum modem_state {
-	MODEM_STATE_POWER_OFF,
-	MODEM_STATE_PRE_SIM,
-	MODEM_STATE_OFFLINE,
-	MODEM_STATE_ONLINE,
-};
-
 struct ofono_modem {
 	char			*path;
-	enum modem_state	modem_state;
+	enum ofono_modem_state	modem_state;
 	GSList			*atoms;
 	struct ofono_watchlist	*atom_watches;
 	GSList			*interface_list;
@@ -72,6 +65,7 @@ struct ofono_modem {
 	ofono_bool_t		powered_pending;
 	guint			timeout;
 	ofono_bool_t		online;
+	struct ofono_watchlist	*state_watches;
 	GHashTable		*properties;
 	struct ofono_sim	*sim;
 	unsigned int		sim_watch;
@@ -94,7 +88,7 @@ struct ofono_devinfo {
 
 struct ofono_atom {
 	enum ofono_atom_type type;
-	enum modem_state modem_state;
+	enum ofono_modem_state modem_state;
 	void (*destruct)(struct ofono_atom *atom);
 	void (*unregister)(struct ofono_atom *atom);
 	void *data;
@@ -324,7 +318,8 @@ void __ofono_atom_free(struct ofono_atom *atom)
 	g_free(atom);
 }
 
-static void flush_atoms(struct ofono_modem *modem, enum modem_state new_state)
+static void flush_atoms(struct ofono_modem *modem,
+			enum ofono_modem_state new_state)
 {
 	GSList *cur;
 	GSList *prev;
@@ -360,11 +355,50 @@ static void flush_atoms(struct ofono_modem *modem, enum modem_state new_state)
 	}
 }
 
+static void notify_state_watches(struct ofono_modem *modem)
+{
+	struct ofono_watchlist_item *item;
+	GSList *l;
+	ofono_modem_state_notify_func notify;
+
+	if (modem->state_watches == NULL)
+		return;
+
+	for (l = modem->state_watches->items; l; l = l->next) {
+		item = l->data;
+		notify = item->notify;
+		notify(modem->modem_state, item->notify_data);
+	}
+}
+
+unsigned __ofono_modem_add_state_watch(struct ofono_modem *modem,
+		ofono_modem_state_notify_func notify,
+		void *data, ofono_destroy_func destroy)
+{
+	struct ofono_watchlist_item *item;
+
+	if (modem == 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(modem->state_watches, item);
+}
+
+void __ofono_modem_remove_state_watch(struct ofono_modem *modem, unsigned id)
+{
+	__ofono_watchlist_remove_item(modem->state_watches, id);
+}
+
 static void modem_change_state(struct ofono_modem *modem,
-				enum modem_state new_state)
+				enum ofono_modem_state new_state)
 {
 	struct ofono_modem_driver const *driver = modem->driver;
-	enum modem_state old_state = modem->modem_state;
+	enum ofono_modem_state old_state = modem->modem_state;
 	ofono_bool_t new_online = new_state == MODEM_STATE_ONLINE;
 
 	if (old_state == new_state)
@@ -407,6 +441,8 @@ static void modem_change_state(struct ofono_modem *modem,
 			driver->post_online(modem);
 		break;
 	}
+
+	notify_state_watches(modem);
 }
 
 static void sim_state_watch(enum ofono_sim_state new_state, void *user)
@@ -1457,6 +1493,7 @@ int ofono_modem_register(struct ofono_modem *modem)
 	modem->driver_type = NULL;
 
 	modem->atom_watches = __ofono_watchlist_new(g_free);
+	modem->state_watches = __ofono_watchlist_new(g_free);
 
 	emit_modem_added(modem);
 	call_modemwatches(modem, TRUE);
@@ -1488,6 +1525,9 @@ static void modem_unregister(struct ofono_modem *modem)
 	__ofono_watchlist_free(modem->atom_watches);
 	modem->atom_watches = NULL;
 
+	__ofono_watchlist_free(modem->state_watches);
+	modem->state_watches = NULL;
+
 	modem->sim_watch = 0;
 	modem->sim_ready_watch = 0;
 
diff --git a/src/ofono.h b/src/ofono.h
index 6c7f649..b132727 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -177,6 +177,21 @@ unsigned int __ofono_modemwatch_add(ofono_modemwatch_cb_t cb, void *user,
 					ofono_destroy_func destroy);
 gboolean __ofono_modemwatch_remove(unsigned int id);
 
+enum ofono_modem_state {
+	MODEM_STATE_POWER_OFF,
+	MODEM_STATE_PRE_SIM,
+	MODEM_STATE_OFFLINE,
+	MODEM_STATE_ONLINE,
+};
+
+typedef void (*ofono_modem_state_notify_func)(enum ofono_modem_state state,
+		void *data);
+unsigned int __ofono_modem_add_state_watch(struct ofono_modem *modem,
+		ofono_modem_state_notify_func notify,
+		void *data, ofono_destroy_func destroy);
+void __ofono_modem_remove_state_watch(struct ofono_modem *modem,
+		unsigned int id);
+
 #include <ofono/call-barring.h>
 
 gboolean __ofono_call_barring_is_busy(struct ofono_call_barring *cb);
-- 
1.7.0.4

Re: [RFC PATCH 1/3] modem: modem state watch added

[Denis Kenzior]

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

Hi Andras,

On 10/25/2010 03:03 AM, Andras Domokos wrote:
> From: Andras Domokos <andras.domokos@nokia.com>
> 

<snip>

> diff --git a/src/ofono.h b/src/ofono.h
> index 6c7f649..b132727 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -177,6 +177,21 @@ unsigned int __ofono_modemwatch_add(ofono_modemwatch_cb_t cb, void *user,
>  					ofono_destroy_func destroy);
>  gboolean __ofono_modemwatch_remove(unsigned int id);
>  
> +enum ofono_modem_state {
> +	MODEM_STATE_POWER_OFF,
> +	MODEM_STATE_PRE_SIM,
> +	MODEM_STATE_OFFLINE,
> +	MODEM_STATE_ONLINE,

Please see doc/coding-style.txt item M11.

> +};
> +
> +typedef void (*ofono_modem_state_notify_func)(enum ofono_modem_state state,
> +		void *data);
> +unsigned int __ofono_modem_add_state_watch(struct ofono_modem *modem,
> +		ofono_modem_state_notify_func notify,
> +		void *data, ofono_destroy_func destroy);
> +void __ofono_modem_remove_state_watch(struct ofono_modem *modem,
> +		unsigned int id);
> +

I believe that me and Pekka already agreed not to use MODEM_STATE, but
instead only watch for the online state of the modem.

>  #include <ofono/call-barring.h>
>  
>  gboolean __ofono_call_barring_is_busy(struct ofono_call_barring *cb);

Regards,
-Denis

[RFC PATCH 2/3] voicecall: emergency call handling added

[Andras Domokos]

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


Signed-off-by: Andras Domokos <andras.domokos@nokia.com>
---
 include/voicecall.h |   12 +++
 src/voicecall.c     |  221 ++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 215 insertions(+), 18 deletions(-)

diff --git a/include/voicecall.h b/include/voicecall.h
index 2356fcf..d530148 100644
--- a/include/voicecall.h
+++ b/include/voicecall.h
@@ -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);
 
+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);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/voicecall.c b/src/voicecall.c
index 7b5fe3b..a619b30 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -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",
 
 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);
 
 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);
+}
+
+static ofono_bool_t emergency_number(struct ofono_voicecall *vc,
+					const char *number)
+{
+	if (!number)
+		return FALSE;
+
+	return g_slist_find_custom(vc->en_list,
+				number, number_compare) ? TRUE : FALSE;
+}
+
+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;
+}
+
+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++) {
+		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",
+						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);
+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) {
@@ -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)
 {
 	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);
 
@@ -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);
+
 	/* 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);
 
@@ -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;
 	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)
 
 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);
 }
-- 
1.7.0.4

Re: [RFC PATCH 2/3] voicecall: emergency call handling added

[Denis Kenzior]

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

Hi Andras,

On 10/25/2010 03:03 AM, Andras Domokos wrote:
> From: Andras Domokos <andras.domokos@nokia.com>
> 
> 
> Signed-off-by: Andras Domokos <andras.domokos@nokia.com>
> ---
>  include/voicecall.h |   12 +++
>  src/voicecall.c     |  221 ++++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 215 insertions(+), 18 deletions(-)
> 
> diff --git a/include/voicecall.h b/include/voicecall.h
> index 2356fcf..d530148 100644
> --- a/include/voicecall.h
> +++ b/include/voicecall.h
> @@ -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);
>  
> +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.

>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/src/voicecall.c b/src/voicecall.c
> index 7b5fe3b..a619b30 100644
> --- a/src/voicecall.c
> +++ b/src/voicecall.c
> @@ -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",
>  
>  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.

>  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);
> +}
> +
> +static ofono_bool_t emergency_number(struct ofono_voicecall *vc,
> +					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.

> +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;
> +}
> +
> +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.

> +		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",
> +						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

> +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) {
> @@ -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

>  {
>  	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);
>  
> @@ -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

>  	/* 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);
>  
> @@ -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...

>  	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)
>  
>  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

Re: [RFC PATCH 2/3] voicecall: emergency call handling added

[Andras Domokos]

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

Hi Denis,

Thank you for your comments, here is my response.

On 10/28/2010 06:48 AM, ext Denis Kenzior wrote:
> Hi Andras,
>
> On 10/25/2010 03:03 AM, Andras Domokos wrote:
>    
>> From: Andras Domokos<andras.domokos@nokia.com>
>>
>>
>> Signed-off-by: Andras Domokos<andras.domokos@nokia.com>
>> ---
>>   include/voicecall.h |   12 +++
>>   src/voicecall.c     |  221 ++++++++++++++++++++++++++++++++++++++++++++++----
>>   2 files changed, 215 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/voicecall.h b/include/voicecall.h
>> index 2356fcf..d530148 100644
>> --- a/include/voicecall.h
>> +++ b/include/voicecall.h
>> @@ -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);
>>
>> +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.
>    
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.

> 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.

>>   #ifdef __cplusplus
>>   }
>>   #endif
>> diff --git a/src/voicecall.c b/src/voicecall.c
>> index 7b5fe3b..a619b30 100644
>> --- a/src/voicecall.c
>> +++ b/src/voicecall.c
>> @@ -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",
>>
>>   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.
>    
OK, I'll conform to the agreed coding policy.
>    
>>   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);
>> +}
>> +
>> +static ofono_bool_t emergency_number(struct ofono_voicecall *vc,
>> +                                     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
>    
>    
>> +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;
>> +}
>> +
>> +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.
>
>    
>> +             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",
>> +                                             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
>
>    
>> +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) {
>> @@ -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
>
>    
>>   {
>>        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);
>>
>> @@ -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
>
>    
>>        /* 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);
>>
>> @@ -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...
>
>    
>>        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)
>>
>>   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
>    
Regards,
Andras

Re: [RFC PATCH 2/3] voicecall: emergency call handling added

[Marcel Holtmann]

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

Hi Andras,

> > 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.

we had this specific discussion before and my understanding is that all
these networks have been updated by now anyway. So this is not an issue
anymore.

However in the end this is not something oFono should be doing in the
first place. The modem should do this. oFono does not know if something
is or becomes an emergency call. Only the modem itself knows if it
starts the procedure to make an emergency call.

Regards

Marcel

Re: [RFC PATCH 2/3] voicecall: emergency call handling added

[Denis Kenzior]

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

Hi Andras,

>> 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.

>> 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.

> 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.

> 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

RE: [RFC PATCH 2/3] voicecall: emergency call handling added

[Mika.Liljeberg]

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

Hi,

> > 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?

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".

	MikaL

RE: [RFC PATCH 2/3] voicecall: emergency call handling added

[Marcel Holtmann]

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

Hi Mika,

> > > 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?
> 
> 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?

And again, this is something that the modem firmware should be doing
anyway (if it is needed). The modem firmware knows best. And we do have
the GPRS suspend support already to signal this back to the user.

Regards

Marcel

RE: [RFC PATCH 2/3] voicecall: emergency call handling added

[Mika.Liljeberg]

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

Hi Marcel, 

> > > 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".
> 
> as Denis and I mentioned, we have been told that this is not needed
> anymore. So where is this still needed?

Ok, I double checked the product requirements and it turns out that you're right. The requirement is no longer there for oFono based stuff. Seems we've had some wires crossed at our end, so the situation was not entirely clear.

Sorry for the confusion,

	MikaL

Re: [RFC PATCH 2/3] voicecall: emergency call handling added

[Andras Domokos]

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

Hi Denis,

On 10/29/2010 08:32 PM, ext Denis Kenzior wrote:
> Hi Andras,
>
>    
>>> 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.
>
>    
I see the advantage of using the Modem interface and I am fine
with basing the implementation on it.
>>> 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.
>
>    
>> 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.
>
>    
>> 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.
>    

Based on the comments and after some clarifications made in our
team, I consider the emergency watch unnecessary. We can forget
about the very corner case and go with the simpler approach.

OK, I am going to come up with a new set of patches.

> Regards,
> -Denis
>    

Regards,
Andras

[RFC PATCH 3/3] modem: emergency state handling added

[Andras Domokos]

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


Signed-off-by: Andras Domokos <andras.domokos@nokia.com>
---
 src/modem.c |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/src/modem.c b/src/modem.c
index 9aeb49c..9e96cce 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -70,6 +70,8 @@ struct ofono_modem {
 	struct ofono_sim	*sim;
 	unsigned int		sim_watch;
 	unsigned int		sim_ready_watch;
+	unsigned int		voicecall_watch;
+	unsigned int		emergency_watch;
 	const struct ofono_modem_driver *driver;
 	void			*driver_data;
 	char			*driver_type;
@@ -544,6 +546,34 @@ ofono_bool_t ofono_modem_get_online(struct ofono_modem *modem)
 	return modem->online;
 }
 
+static void emergency_state_watch(ofono_bool_t state, void *data);
+static void voicecall_watch(struct ofono_atom *atom,
+			enum ofono_atom_watch_condition cond, void *data)
+{
+	struct ofono_modem *modem = data;
+	struct ofono_voicecall *vc;
+
+	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
+		modem->emergency_watch = 0;
+		return;
+	}
+
+	vc = __ofono_atom_get_data(atom);
+	modem->emergency_watch = __ofono_voicecall_add_emergency_watch(vc,
+							emergency_state_watch,
+							modem, NULL);
+}
+
+static void emergency_state_watch(ofono_bool_t state, void *data)
+{
+	struct ofono_modem *modem = data;
+
+	DBG("Emergency mode is %s", state ? "On" : "Off");
+
+	if (state)
+			modem_change_state(modem, MODEM_STATE_ONLINE);
+}
+
 void __ofono_modem_append_properties(struct ofono_modem *modem,
 						DBusMessageIter *dict)
 {
@@ -1502,6 +1532,10 @@ int ofono_modem_register(struct ofono_modem *modem)
 					OFONO_ATOM_TYPE_SIM,
 					sim_watch, modem, NULL);
 
+	modem->voicecall_watch = __ofono_modem_add_atom_watch(modem,
+					OFONO_ATOM_TYPE_VOICECALL,
+					voicecall_watch, modem, NULL);
+
 	return 0;
 }
 
@@ -1531,6 +1565,9 @@ static void modem_unregister(struct ofono_modem *modem)
 	modem->sim_watch = 0;
 	modem->sim_ready_watch = 0;
 
+	modem->voicecall_watch = 0;
+	modem->emergency_watch = 0;
+
 	g_slist_foreach(modem->interface_list, (GFunc) g_free, NULL);
 	g_slist_free(modem->interface_list);
 	modem->interface_list = NULL;
-- 
1.7.0.4