Emergency Calls (4th round)

From: Andras Domokos <[email protected]>

---
 src/modem.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 src/ofono.h |    8 ++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)

From: Andras Domokos <[email protected]>

---
 src/modem.c |  134 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/ofono.h |    4 ++
 2 files changed, 138 insertions(+), 0 deletions(-)

src/modem.c +1 -0

index f73cc1d..4307914 100644
@@ -61,6 +61,7 @@ enum modem_state {
 struct ofono_modem {
 	char			*path;
 	enum modem_state	modem_state;
+	enum modem_state	modem_state_pre_emergency;
 	GSList			*atoms;
 	struct ofono_watchlist	*atom_watches;
 	GSList			*interface_list;

src/modem.c +49 -0

 	struct ofono_watchlist	*online_watches;
 	GHashTable		*properties;
 	struct ofono_sim	*sim;

@@ -514,6 +516,127 @@ static void offline_cb(const struct ofono_error *error, void *data)
 		modem_change_state(modem, MODEM_STATE_OFFLINE);
 }
 
+ofono_bool_t ofono_modem_get_emergency_mode(struct ofono_modem *modem)
+{
+	if (modem == NULL)
+		return FALSE;
+
+	return modem->emergency_mode != 0;
+}
+
+static void modem_change_online(struct ofono_modem *modem,
+				enum modem_state new_state)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	enum modem_state old_state = modem->modem_state;
+	ofono_bool_t new_online = new_state == MODEM_STATE_ONLINE;
+
+	DBG("old state: %d, new state: %d", old_state, new_state);
+
+	if (new_online == modem->online)
+		return;
+
+	modem->modem_state = new_state;
+	modem->online = new_online;
+
+	ofono_dbus_signal_property_changed(conn, modem->path,
+					OFONO_MODEM_INTERFACE, "Online",
+					DBUS_TYPE_BOOLEAN, &modem->online);
+
+	notify_online_watches(modem);
+}
+
+static void emergency_online_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_modem *modem = data;
+
+	DBG("modem: %p", modem);
+
+	if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
+			modem->modem_state != MODEM_STATE_ONLINE)
+		modem_change_online(modem, MODEM_STATE_ONLINE);
+}
+
+static void emergency_offline_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_modem *modem = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = ofono_modem_get_path(modem);
+	gboolean state = FALSE;
+
+	DBG("modem: %p", modem);

src/modem.c +9 -0

+						DBUS_TYPE_BOOLEAN,
+						&state);
+}
+
+void ofono_modem_inc_emergency_mode(struct ofono_modem *modem)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = ofono_modem_get_path(modem);
+	gboolean state = TRUE;

src/modem.c +11 -0

+						DBUS_TYPE_BOOLEAN,
+						&state);
+
+	modem->modem_state_pre_emergency = modem->modem_state;
+
+	if (modem->modem_state == MODEM_STATE_ONLINE)
+		return;
+
+	modem->driver->set_online(modem, TRUE, emergency_online_cb, modem);
+}
+

src/modem.c +4 -0

+						DBUS_TYPE_BOOLEAN,
+						&state);
+}
+
 static DBusMessage *set_property_online(struct ofono_modem *modem,
 					DBusMessage *msg,
 					DBusMessageIter *var)

src/modem.c +0 -0

 	/* Remove the atoms even if the driver is no longer available */
 	if (powered == FALSE)
 		modem_change_state(modem, MODEM_STATE_POWER_OFF);

src/ofono.h +0 -0

index 01cd6c0..ac56a85 100644

Hi Andras,

Quoted 20 lines · Andras Domokos

From: Andras Domokos<[email protected]>

---
  src/modem.c |  134 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  src/ofono.h |    4 ++
  2 files changed, 138 insertions(+), 0 deletions(-)

src/modem.c +1 -0

index f73cc1d..4307914 100644
@@ -61,6 +61,7 @@ enum modem_state {
  struct ofono_modem {
       char                    *path;
       enum modem_state        modem_state;
+     enum modem_state        modem_state_pre_emergency;
       GSList                  *atoms;
       struct ofono_watchlist  *atom_watches;
       GSList                  *interface_list;

src/modem.c struct ofono_modem { · line 77

Quoted 5 lines

src/modem.c +1 -0

@@ -72,6 +73,7 @@ struct ofono_modem {
       ofono_bool_t            powered_pending;
       guint                   timeout;
       ofono_bool_t            online;
+     unsigned int            emergency_mode;
 

I really prefer this to be called 'emergency'

Quoted 57 lines

src/modem.c +0 -0

       struct ofono_watchlist  *online_watches;
       GHashTable              *properties;

       struct ofono_sim        *sim;

src/modem.c +50 -0

@@ -514,6 +516,127 @@ static void offline_cb(const struct ofono_error *error, void *data)
               modem_change_state(modem, MODEM_STATE_OFFLINE);
  }
 
+ofono_bool_t ofono_modem_get_emergency_mode(struct ofono_modem *modem)
+{
+     if (modem == NULL)
+             return FALSE;
+
+     return modem->emergency_mode != 0;
+}
+
+static void modem_change_online(struct ofono_modem *modem,
+                             enum modem_state new_state)
+{
+     DBusConnection *conn = ofono_dbus_get_connection();
+     enum modem_state old_state = modem->modem_state;
+     ofono_bool_t new_online = new_state == MODEM_STATE_ONLINE;
+
+     DBG("old state: %d, new state: %d", old_state, new_state);
+
+     if (new_online == modem->online)
+             return;
+
+     modem->modem_state = new_state;
+     modem->online = new_online;
+
+     ofono_dbus_signal_property_changed(conn, modem->path,
+                                     OFONO_MODEM_INTERFACE, "Online",
+                                     DBUS_TYPE_BOOLEAN,&modem->online);
+
+     notify_online_watches(modem);
+}
+
+static void emergency_online_cb(const struct ofono_error *error, void *data)
+{
+     struct ofono_modem *modem = data;
+
+     DBG("modem: %p", modem);
+
+     if (error->type == OFONO_ERROR_TYPE_NO_ERROR&&
+                     modem->modem_state != MODEM_STATE_ONLINE)
+             modem_change_online(modem, MODEM_STATE_ONLINE);
+}
+
+static void emergency_offline_cb(const struct ofono_error *error, void *data)
+{
+     struct ofono_modem *modem = data;
+     DBusConnection *conn = ofono_dbus_get_connection();
+     const char *path = ofono_modem_get_path(modem);
+     gboolean state = FALSE;
+
+     DBG("modem: %p", modem);
+

src/modem.c static void offline_cb(const struct ofono_error *error, void *data) · line 578

Quoted 10 lines

src/modem.c +9 -0

+     if (error->type == OFONO_ERROR_TYPE_NO_ERROR&&
+                     modem->modem_state == MODEM_STATE_ONLINE)
+             modem_change_online(modem, modem->modem_state_pre_emergency);
+
+     modem->emergency_mode--;
+
+     ofono_dbus_signal_property_changed(conn, path,
+                                             OFONO_MODEM_INTERFACE,
+                                             "EmergencyMode",
 

The property should really be called 'Emergency' to be in line with the current API proposal (doc/modem-api.txt& TODO)

Quoted 10 lines

src/modem.c +10 -0

+                                             DBUS_TYPE_BOOLEAN,
+&state);
+}
+
+void ofono_modem_inc_emergency_mode(struct ofono_modem *modem)
+{
+     DBusConnection *conn = ofono_dbus_get_connection();
+     const char *path = ofono_modem_get_path(modem);
+     gboolean state = TRUE;
+

src/modem.c static void offline_cb(const struct ofono_error *error, void *data)

Quoted 10 lines

src/modem.c +9 -0

+     DBG("modem: %p", modem);
+
+     modem->emergency_mode++;
+     if (modem->emergency_mode>  1)
+             return;
+
+     ofono_dbus_signal_property_changed(conn, path,
+                                             OFONO_MODEM_INTERFACE,
+                                             "EmergencyMode",
 

Again, 'Emergency' here

Quoted 12 lines

src/modem.c +12 -0

+                                             DBUS_TYPE_BOOLEAN,
+&state);
+
+     modem->modem_state_pre_emergency = modem->modem_state;
+
+     if (modem->modem_state == MODEM_STATE_ONLINE)
+             return;
+
+     modem->driver->set_online(modem, TRUE, emergency_online_cb, modem);
+}
+
+void ofono_modem_dec_emergency_mode(struct ofono_modem *modem)

src/modem.c static void offline_cb(const struct ofono_error *error, void *data)

Quoted 10 lines

src/modem.c +9 -0

+{
+     DBusConnection *conn = ofono_dbus_get_connection();
+     const char *path = ofono_modem_get_path(modem);
+     gboolean state = FALSE;
+
+     DBG("modem: %p", modem);
+
+     if (modem->emergency_mode == 0)
+             return;
 

Can you be a bit more pedantic and send an ofono_error for this case? We probably want to track whether some plugin abuses the reference counting.

src/modem.c static void offline_cb(const struct ofono_error *error, void *data) · line 625

Quoted 9 lines

src/modem.c +8 -0

+
+     if (modem->emergency_mode>  1) {
+             modem->emergency_mode--;
+             return;
+     }
+
+     if (modem->modem_state == MODEM_STATE_ONLINE&&
+             modem->modem_state_pre_emergency != MODEM_STATE_ONLINE) {

Please indent one more, item M4 in coding-style.txt

src/modem.c static void offline_cb(const struct ofono_error *error, void *data) · line 635

Quoted 11 lines

src/modem.c +10 -0

+             modem->driver->set_online(modem, FALSE,
+                                             emergency_offline_cb, modem);
+             return;
+     }
+
+     modem->emergency_mode--;
+
+     ofono_dbus_signal_property_changed(conn, path,
+                                             OFONO_MODEM_INTERFACE,
+                                             "EmergencyMode",

And again, 'Emergency' here.

Quoted 8 lines

src/modem.c +4 -0

+                                             DBUS_TYPE_BOOLEAN,
+&state);
+}
+

static DBusMessage *set_property_online(struct ofono_modem *modem,
DBusMessage *msg, DBusMessageIter *var)

src/modem.c static DBusMessage *set_property_online(struct ofono_modem *modem, · line 664

Quoted 7 lines

src/modem.c +3 -0

@@ -535,6 +658,9 @@ static DBusMessage *set_property_online(struct ofono_modem *modem,
       if (modem->modem_state<  MODEM_STATE_OFFLINE)
               return __ofono_error_not_available(msg);
 
+     if (modem->emergency_mode)
+             return __ofono_error_failed(msg);
+
 

Is it worth to create a new dbus error for this case? Perhaps ofono_error_emergency, or emergency_active?

src/modem.c void __ofono_modem_append_properties(struct ofono_modem *modem, · line 692

Quoted 9 lines

src/modem.c +1 -0

       if (modem->online == online)
               return dbus_message_new_method_return(msg);

@@ -562,6 +688,7 @@ void __ofono_modem_append_properties(struct ofono_modem *modem,
       int i;
       GSList *l;
       struct ofono_atom *devinfo_atom;
+     ofono_bool_t state;
 

Please rename this variable to 'val' or 'value'

src/modem.c void __ofono_modem_append_properties(struct ofono_modem *modem, · line 701

Quoted 9 lines

src/modem.c +2 -0

       ofono_dbus_dict_append(dict, "Online", DBUS_TYPE_BOOLEAN,
                               &modem->online);

@@ -569,6 +696,10 @@ void __ofono_modem_append_properties(struct ofono_modem *modem,
       ofono_dbus_dict_append(dict, "Powered", DBUS_TYPE_BOOLEAN,
                               &modem->powered);
 
+     state = ofono_modem_get_emergency_mode(modem);
+     ofono_dbus_dict_append(dict, "EmergencyMode",
 

And 'Emergency' here

src/modem.c static int set_powered(struct ofono_modem *modem, ofono_bool_t powered) · line 782

Quoted 11 lines

src/modem.c +2 -0

+                             DBUS_TYPE_BOOLEAN,&state);
+
       devinfo_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_DEVINFO);
 

       /* We cheat a little here and don't check the registered status */

src/modem.c +1 -0

@@ -647,6 +778,9 @@ static int set_powered(struct ofono_modem *modem, ofono_bool_t powered)
       if (modem->powered_pending == powered)
               return -EALREADY;
 
+     if (modem->emergency_mode)
 

I really would prefer modem->emergency> 0 here

src/modem.c static int set_powered(struct ofono_modem *modem, ofono_bool_t powered)

Quoted 3 lines

src/modem.c +2 -0

+             return -EINVAL;
+
 

If we introduce a new ofono error, then returning something like EPERM or EACCESS for the case where emergency mode is active might be a good idea.

Quoted 17 lines

src/modem.c +0 -0

       /* Remove the atoms even if the driver is no longer available */

       if (powered == FALSE)
               modem_change_state(modem, MODEM_STATE_POWER_OFF);

src/ofono.h +4 -0

index 01cd6c0..ac56a85 100644
@@ -185,6 +185,10 @@ unsigned int __ofono_modem_add_online_watch(struct ofono_modem *modem,
  void __ofono_modem_remove_online_watch(struct ofono_modem *modem,
               unsigned int id);
 
+ofono_bool_t ofono_modem_get_emergency_mode(struct ofono_modem *modem);
+void ofono_modem_inc_emergency_mode(struct ofono_modem *modem);
+void ofono_modem_dec_emergency_mode(struct ofono_modem *modem);
+
  #include<ofono/call-barring.h>
 

src/ofono.h unsigned int __ofono_modem_add_online_watch(struct ofono_modem *modem, · line 194

Quoted 2 lines

src/ofono.h +0 -0

  gboolean __ofono_call_barring_is_busy(struct ofono_call_barring *cb);

Otherwise this one looks good to me.

Regards,
-Denis

src/modem.c +9 -0

+
+	modem->modem_state = new_state;
+	modem->online = new_online;
+
+	ofono_dbus_signal_property_changed(conn, modem->path,
+					OFONO_MODEM_INTERFACE, "Online",
+					DBUS_TYPE_BOOLEAN, &modem->online);
+
+	notify_online_watches(modem);

src/modem.c +5 -0

+		modem_change_online(modem, MODEM_STATE_ONLINE);
+}
+
+static void emergency_offline_cb(const struct ofono_error *error, void *data)
+{

src/modem.c +22 -0

+
+	modem->emergency--;
+
+	ofono_dbus_signal_property_changed(conn, path,
+					OFONO_MODEM_INTERFACE,
+					"Emergency",
+					DBUS_TYPE_BOOLEAN,
+					&state);
+}
+
+void ofono_modem_inc_emergency(struct ofono_modem *modem)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = ofono_modem_get_path(modem);
+	gboolean state = TRUE;
+
+	DBG("modem: %p", modem);
+
+	modem->emergency++;
+	if (modem->emergency > 1)
+		return;
+

src/modem.c +4 -0

+
+	modem->driver->set_online(modem, TRUE, emergency_online_cb, modem);
+}
+

src/ofono.h +0 -0

 void __ofono_dbus_pending_reply(DBusMessage **msg, DBusMessage *reply);
 

<snip>

src/modem.c static void offline_cb(const struct ofono_error *error, void *data) · line 535

Quoted 6 lines

src/modem.c +6 -0

+static void modem_change_online(struct ofono_modem *modem,
+                             enum modem_state new_state)
+{
+     DBusConnection *conn = ofono_dbus_get_connection();
+     enum modem_state old_state = modem->modem_state;
+     ofono_bool_t new_online = new_state == MODEM_STATE_ONLINE;

Quoted 4 lines

+     DBG("old state: %d, new state: %d", old_state, new_state); +
+     if (new_online == modem->online)
+             return;

You check for online not being equal here

Quoted 9 lines

src/modem.c +9 -0

+
+     modem->modem_state = new_state;
+     modem->online = new_online;
+
+     ofono_dbus_signal_property_changed(conn, modem->path,
+                                     OFONO_MODEM_INTERFACE, "Online",
+                                     DBUS_TYPE_BOOLEAN, &modem->online);
+
+     notify_online_watches(modem);

src/modem.c static void offline_cb(const struct ofono_error *error, void *data) · line 559

Quoted 10 lines

src/modem.c +10 -0

+}
+
+static void emergency_online_cb(const struct ofono_error *error, void *data)
+{
+     struct ofono_modem *modem = data;
+
+     DBG("modem: %p", modem);
+
+     if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
+                     modem->modem_state != MODEM_STATE_ONLINE)

And perform essentially the same check here...  Seems wasteful

Quoted 5 lines

+             modem_change_online(modem, MODEM_STATE_ONLINE);

src/modem.c +4 -0

+}
+
+static void emergency_offline_cb(const struct ofono_error *error, void *data)
+{

src/modem.c static void offline_cb(const struct ofono_error *error, void *data) · line 574

Quoted 10 lines

src/modem.c +10 -0

+     struct ofono_modem *modem = data;
+     DBusConnection *conn = ofono_dbus_get_connection();
+     const char *path = ofono_modem_get_path(modem);
+     gboolean state = FALSE;
+
+     DBG("modem: %p", modem);
+
+     if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
+                     modem->modem_state == MODEM_STATE_ONLINE)
+             modem_change_online(modem, modem->modem_state_pre_emergency);

Same comment as above

Quoted 31 lines

src/modem.c +31 -0

+
+     modem->emergency--;
+
+     ofono_dbus_signal_property_changed(conn, path,
+                                     OFONO_MODEM_INTERFACE,
+                                     "Emergency",
+                                     DBUS_TYPE_BOOLEAN,
+                                     &state);
+}
+
+void ofono_modem_inc_emergency(struct ofono_modem *modem)
+{
+     DBusConnection *conn = ofono_dbus_get_connection();
+     const char *path = ofono_modem_get_path(modem);
+     gboolean state = TRUE;
+
+     DBG("modem: %p", modem);
+
+     modem->emergency++;
+     if (modem->emergency > 1)
+             return;
+
+     ofono_dbus_signal_property_changed(conn, path,
+                                             OFONO_MODEM_INTERFACE,
+                                             "Emergency",
+                                             DBUS_TYPE_BOOLEAN,
+                                             &state);
+
+     modem->modem_state_pre_emergency = modem->modem_state;
+
+     if (modem->modem_state == MODEM_STATE_ONLINE)

Quoted 1 line

+             return;

So my only major concern left is here actually.  If we activate an emergency call and a set_property(Online, blah) is active, we get into some funny race conditions.  I mentioned these to Pekka on IRC.  But basically the worst one is if we have an Online=False operation pending. In this case your call proceeds, but then gets terminated shortly thereafter by the offline procedure ;)

Quoted 6 lines

So my only major concern left is here actually. If we activate an emergency call and a set_property(Online, blah) is active, we get into some funny race conditions. I mentioned these to Pekka on IRC. But basically the worst one is if we have an Online=False operation pending.
In this case your call proceeds, but then gets terminated shortly
thereafter by the offline procedure ;)

That is a tricky problem. I'd say the most straightforward way is to return an error if there is a pending operation and push these problems up the stack. In any case dialer has to retry ecall if user tries to make the emergency call just after or before moving device to offline / removing SIM card / etc. However, I'm not sure if we can propagate the error in all cases where ofono_modem_inc_emergency() gets called.

In order to manage the worst case in best possible manner, I'd check for set_online in progress here. Return FALSE from ofono_modem_get_online() immediately after the set_online(FALSE) call is made. Also, check the emergency state again in after set_online callback response. The online/offline watches should be called before the set_online() call. (I have no idea what we will do should set_online(FALSE) fail)?

---
 src/voicecall.c |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 110 insertions(+), 1 deletions(-)

src/voicecall.c +1 -0

index 3af614b..066cdb9 100644
@@ -52,6 +52,7 @@ struct ofono_voicecall {
 	struct ofono_sim *sim;
 	unsigned int sim_watch;
 	unsigned int sim_state_watch;
+	unsigned int modem_online_watch;
 	const struct ofono_voicecall_driver *driver;
 	void *driver_data;
 	struct ofono_atom *atom;

@@ -133,6 +134,22 @@ static void add_to_en_list(GSList **l, const char **list)
 		*l = g_slist_prepend(*l, g_strdup(list[i++]));
 }
 

src/voicecall.c +13 -1

+		return FALSE;
+
+	return g_slist_find_custom(vc->en_list,
+				number, number_compare) ? TRUE : FALSE;
+}
+
 static const char *disconnect_reason_to_string(enum ofono_disconnect_reason r)
 {
 	switch (r) {

@@ -1125,6 +1142,7 @@ static struct voicecall *dial_handle_result(struct ofono_voicecall *vc,
 static void manager_dial_callback(const struct ofono_error *error, void *data)
 {
 	struct ofono_voicecall *vc = data;
+	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
 	DBusMessage *reply;
 	const char *number;
 	gboolean need_to_emit;

@@ -1143,8 +1161,12 @@ 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_modem_dec_emergency_mode(modem);
+
 		reply = __ofono_error_failed(vc->pending);
+	}
 
 	__ofono_dbus_pending_reply(&vc->pending, reply);
 

@@ -1156,6 +1178,7 @@ static DBusMessage *manager_dial(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
 	struct ofono_voicecall *vc = data;
+	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
 	const char *number;
 	struct ofono_phone_number ph;
 	const char *clirstr;

src/voicecall.c +16 -0

+			return NULL;
+	}
+
 	vc->driver->dial(vc, &ph, clir, OFONO_CUG_OPTION_DEFAULT,
 				manager_dial_callback, vc);
 

@@ -1748,6 +1780,7 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
 				const struct ofono_error *error)
 {
 	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
+	const char *number;
 	GSList *l;
 	struct voicecall *call;
 	time_t ts;

@@ -1767,6 +1800,7 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
 	}
 
 	call = l->data;
+	number = phone_number_to_string(&call->call->phone_number);
 
 	ts = time(NULL);
 	prev_status = call->call->status;

@@ -1805,6 +1839,9 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
 
 	voicecalls_emit_call_removed(vc, call);
 
+	if (emergency_number(vc, number))
+		ofono_modem_dec_emergency_mode(modem);
+
 	voicecall_dbus_unregister(vc, call);
 
 	vc->call_list = g_slist_remove(vc->call_list, call);

@@ -2067,6 +2104,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);
 

@@ -2108,6 +2146,12 @@ static void voicecall_remove(struct ofono_atom *atom)
 		g_queue_free(vc->toneq);
 	}
 
+	if (vc->modem_online_watch) {
+		__ofono_modem_remove_online_watch(modem,
+						vc->modem_online_watch);
+		vc->modem_online_watch = 0;
+	}
+
 	g_free(vc);
 }
 

@@ -2205,6 +2249,7 @@ static void sim_watch(struct ofono_atom *atom,
 static void dial_request_cb(const struct ofono_error *error, void *data)
 {
 	struct ofono_voicecall *vc = data;
+	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
 	gboolean need_to_emit;
 	struct voicecall *v;
 

src/voicecall.c +13 -0

+		if (emergency_number(vc,
+				phone_number_to_string(&vc->dial_req->ph)))
+			ofono_modem_dec_emergency_mode(modem);
 		return;
 	}
 

@@ -2237,6 +2285,53 @@ static void dial_request_cb(const struct ofono_error *error, void *data)
 		voicecalls_emit_call_added(vc, v);
 }
 
+static void modem_online_watch(ofono_bool_t online, 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;
+

src/voicecall.c +16 -0

+		reply = __ofono_error_failed(vc->pending);
+		__ofono_dbus_pending_reply(&vc->pending, reply);
+		ofono_modem_dec_emergency_mode(modem);
+		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);
+}
+
 void ofono_voicecall_register(struct ofono_voicecall *vc)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();

@@ -2255,6 +2350,9 @@ void ofono_voicecall_register(struct ofono_voicecall *vc)
 	}
 
 	ofono_modem_add_interface(modem, OFONO_VOICECALL_MANAGER_INTERFACE);
+	vc->modem_online_watch = __ofono_modem_add_online_watch(modem,
+							modem_online_watch,
+							vc, NULL);
 
 	/*
 	 * Start out with the 22.101 mandated numbers, if we have a SIM and

@@ -2331,6 +2429,17 @@ ofono_bool_t __ofono_voicecall_is_busy(struct ofono_voicecall *vc,
 

Hi Andras,

Quoted 21 lines · Andras Domokos

---
  src/voicecall.c |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
  1 files changed, 110 insertions(+), 1 deletions(-)

src/voicecall.c +2 -0

index 3af614b..066cdb9 100644
@@ -52,6 +52,7 @@ struct ofono_voicecall {
       struct ofono_sim *sim;
       unsigned int sim_watch;
       unsigned int sim_state_watch;
+     unsigned int modem_online_watch;
       const struct ofono_voicecall_driver *driver;
       void *driver_data;
       struct ofono_atom *atom;

@@ -133,6 +134,22 @@ 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)

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

Quoted 10 lines

src/voicecall.c +9 -0

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

Just nit picking here, but in general we really prefer this to be written like this:

if (number == NULL)

This is much easier to read when you don't know if number is a string or an integer. Yes I know we're not always consistent about doing this, particularly in voicecall.c.

Quoted 40 lines

src/voicecall.c +6 -0

+             return FALSE;
+
+     return g_slist_find_custom(vc->en_list,
+                             number, number_compare) ? TRUE : FALSE;
+}
+
  static const char *disconnect_reason_to_string(enum ofono_disconnect_reason r)
  {

       switch (r) {

src/voicecall.c +7 -1

@@ -1125,6 +1142,7 @@ static struct voicecall *dial_handle_result(struct ofono_voicecall *vc,
  static void manager_dial_callback(const struct ofono_error *error, void *data)
  {
       struct ofono_voicecall *vc = data;
+     struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
       DBusMessage *reply;
       const char *number;
       gboolean need_to_emit;

@@ -1143,8 +1161,12 @@ 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_modem_dec_emergency_mode(modem);
+
               reply = __ofono_error_failed(vc->pending);
+     }
 
       __ofono_dbus_pending_reply(&vc->pending, reply);
 

@@ -1156,6 +1178,7 @@ static DBusMessage *manager_dial(DBusConnection *conn,
                                       DBusMessage *msg, void *data)
  {
       struct ofono_voicecall *vc = data;
+     struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
       const char *number;
       struct ofono_phone_number ph;
       const char *clirstr;

src/voicecall.c static DBusMessage *manager_dial(DBusConnection *conn, · line 1227

Quoted 10 lines

src/voicecall.c +6 -0

@@ -1195,6 +1218,15 @@ static DBusMessage *manager_dial(DBusConnection *conn,
 
       string_to_phone_number(number,&ph);
 
+     if (emergency_number(vc, number)) {
+             ofono_bool_t online = ofono_modem_get_online(modem);
+
+             ofono_modem_inc_emergency_mode(modem);
+
+             if (!online)
 

Do me a favor and change this to:
if (online == FALSE)

Quoted 62 lines

src/voicecall.c +16 -0

+                     return NULL;
+     }
+
       vc->driver->dial(vc,&ph, clir, OFONO_CUG_OPTION_DEFAULT,
                               manager_dial_callback, vc);

@@ -1748,6 +1780,7 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
                               const struct ofono_error *error)
  {
       struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
+     const char *number;
       GSList *l;
       struct voicecall *call;
       time_t ts;

@@ -1767,6 +1800,7 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
       }
 
       call = l->data;
+     number = phone_number_to_string(&call->call->phone_number);
 
       ts = time(NULL);
       prev_status = call->call->status;

@@ -1805,6 +1839,9 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
 
       voicecalls_emit_call_removed(vc, call);
 
+     if (emergency_number(vc, number))
+             ofono_modem_dec_emergency_mode(modem);
+
       voicecall_dbus_unregister(vc, call);
 
       vc->call_list = g_slist_remove(vc->call_list, call);

@@ -2067,6 +2104,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);
 

@@ -2108,6 +2146,12 @@ static void voicecall_remove(struct ofono_atom *atom)
               g_queue_free(vc->toneq);
       }
 
+     if (vc->modem_online_watch) {
+             __ofono_modem_remove_online_watch(modem,
+                                             vc->modem_online_watch);
+             vc->modem_online_watch = 0;
+     }
+
       g_free(vc);
  }
 

@@ -2205,6 +2249,7 @@ static void sim_watch(struct ofono_atom *atom,
  static void dial_request_cb(const struct ofono_error *error, void *data)
  {
       struct ofono_voicecall *vc = data;
+     struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
       gboolean need_to_emit;
       struct voicecall *v;
 

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

Quoted 4 lines

src/voicecall.c +0 -0

@@ -2214,6 +2259,9 @@ static void dial_request_cb(const struct ofono_error *error, void *data)
 
       if (v == NULL) {
               dial_request_finish(vc);
 

Please add an empty line here based on item M1.

Quoted 21 lines

src/voicecall.c +14 -0

+             if (emergency_number(vc,
+                             phone_number_to_string(&vc->dial_req->ph)))
+                     ofono_modem_dec_emergency_mode(modem);
               return;
       }

@@ -2237,6 +2285,53 @@ static void dial_request_cb(const struct ofono_error *error, void *data)
               voicecalls_emit_call_added(vc, v);
  }
 
+static void modem_online_watch(ofono_bool_t online, 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 (ofono_modem_get_emergency_mode(modem) != TRUE)

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

Quoted 10 lines

src/voicecall.c +9 -0

+             return;
+
+     if (vc->dial_req)
+             vc->driver->dial(vc,&vc->dial_req->ph,
+                                     OFONO_CLIR_OPTION_DEFAULT,
+                                     OFONO_CUG_OPTION_DEFAULT,
+                                     dial_request_cb, vc);
+
+     if (!vc->pending)
 

if (vc->pending == NULL) here please

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

Quoted 12 lines

src/voicecall.c +11 -0

+             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)
+             return;
+
+     if (!emergency_number(vc, number))
 

Please do emergency_number() == FALSE here

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

Quoted 4 lines

src/voicecall.c +3 -0

+             return;
+
+     if (!online) {
 

And online == FALSE here

Quoted 29 lines

src/voicecall.c +13 -0

+             reply = __ofono_error_failed(vc->pending);
+             __ofono_dbus_pending_reply(&vc->pending, reply);
+             ofono_modem_dec_emergency_mode(modem);
+             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);
+}
+

  void ofono_voicecall_register(struct ofono_voicecall *vc)
  {
       DBusConnection *conn = ofono_dbus_get_connection();

src/voicecall.c +3 -0

@@ -2255,6 +2350,9 @@ void ofono_voicecall_register(struct ofono_voicecall *vc)
       }
 
       ofono_modem_add_interface(modem, OFONO_VOICECALL_MANAGER_INTERFACE);
+     vc->modem_online_watch = __ofono_modem_add_online_watch(modem,
+                                                     modem_online_watch,
+                                                     vc, NULL);
 
       /*
        * Start out with the 22.101 mandated numbers, if we have a SIM and

@@ -2331,6 +2429,17 @@ ofono_bool_t __ofono_voicecall_is_busy(struct ofono_voicecall *vc,
 
  static void dial_request(struct ofono_voicecall *vc)

src/voicecall.c ofono_bool_t __ofono_voicecall_is_busy(struct ofono_voicecall *vc, · line 2440

Quoted 10 lines

src/voicecall.c +8 -0

  {
+     struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
+
+     if (emergency_number(vc, phone_number_to_string(&vc->dial_req->ph))) {
+             ofono_bool_t online = ofono_modem_get_online(modem);
+
+             ofono_modem_inc_emergency_mode(modem);
+
+             if (!online)
 

And online == FALSE here

Quoted 7 lines

src/voicecall.c +3 -0

+                     return;
+     }
+
       vc->driver->dial(vc,&vc->dial_req->ph, OFONO_CLIR_OPTION_DEFAULT,
                               OFONO_CUG_OPTION_DEFAULT, dial_request_cb, vc);

  }

Otherwise, looks good to me.

Have you figured out how to support the E911 call-back requirements?

Regards,
-Denis

Thanks for the comments, I am going to resend the patch with the changes I made based on your comments.