[PATCH 0/3] Emergency Calls (2nd round)

[PATCH 0/3] Emergency Calls (2nd round)

[Andras Domokos]

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

From: Andras Domokos <Andras.Domokos@nokia.com>

Here is a new proposal for emergency calls handling.

Steps in handling emergency calls:
- subscribe to modem online notifications (add modem online watcher)
- an emergency call detected (phone number is emergency number)
- increment emergency mode
  - switch modem online if not in online mode
  - advertise "EmergencyMode" property change on D-Bus (for first call)
- if modem is not online postpone making the call, otherwise make
  de emergency call
- when modem online notification comes and there is postponed call request
  make the emergency call
- when an emergency call ends decrement emergency mode
  - advertise "EmergencyMode" property change on D-Bus (for last call)

Note: modem remains online even if it was offline before starting the 
emergency call

Andras Domokos (3):
  modem: add modem online-offline watch
  modem: add EmergencyMode property
  voicecall: add emergency call handling

 src/modem.c     |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/ofono.h     |   12 ++++++
 src/voicecall.c |  101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 211 insertions(+), 1 deletions(-)

[RFC PATCH 1/3] modem: add modem online-offline watch

[Andras Domokos]

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


Signed-off-by: Andras Domokos <Andras.Domokos@nokia.com>
---
 src/modem.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 src/ofono.h |    8 ++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/src/modem.c b/src/modem.c
index 3776461..f73cc1d 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -72,6 +72,7 @@ struct ofono_modem {
 	ofono_bool_t		powered_pending;
 	guint			timeout;
 	ofono_bool_t		online;
+	struct ofono_watchlist	*online_watches;
 	GHashTable		*properties;
 	struct ofono_sim	*sim;
 	unsigned int		sim_watch;
@@ -362,6 +363,22 @@ static void flush_atoms(struct ofono_modem *modem, enum modem_state new_state)
 	}
 }
 
+static void notify_online_watches(struct ofono_modem *modem)
+{
+	struct ofono_watchlist_item *item;
+	GSList *l;
+	ofono_modem_online_notify_func notify;
+
+	if (modem->online_watches == NULL)
+		return;
+
+	for (l = modem->online_watches->items; l; l = l->next) {
+		item = l->data;
+		notify = item->notify;
+		notify(modem->online, item->notify_data);
+	}
+}
+
 static void modem_change_state(struct ofono_modem *modem,
 				enum modem_state new_state)
 {
@@ -404,11 +421,13 @@ static void modem_change_state(struct ofono_modem *modem,
 			__ofono_history_probe_drivers(modem);
 			__ofono_nettime_probe_drivers(modem);
 		}
+		notify_online_watches(modem);
 		break;
 
 	case MODEM_STATE_ONLINE:
 		if (driver->post_online)
 			driver->post_online(modem);
+		notify_online_watches(modem);
 		break;
 	}
 }
@@ -437,6 +456,29 @@ static void sim_state_watch(enum ofono_sim_state new_state, void *user)
 	}
 }
 
+unsigned __ofono_modem_add_online_watch(struct ofono_modem *modem,
+		ofono_modem_online_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->online_watches, item);
+}
+
+void __ofono_modem_remove_online_watch(struct ofono_modem *modem, unsigned id)
+{
+	__ofono_watchlist_remove_item(modem->online_watches, id);
+}
+
 static void online_cb(const struct ofono_error *error, void *data)
 {
 	struct ofono_modem *modem = data;
@@ -1472,6 +1514,7 @@ int ofono_modem_register(struct ofono_modem *modem)
 	modem->driver_type = NULL;
 
 	modem->atom_watches = __ofono_watchlist_new(g_free);
+	modem->online_watches = __ofono_watchlist_new(g_free);
 
 	emit_modem_added(modem);
 	call_modemwatches(modem, TRUE);
@@ -1503,6 +1546,9 @@ static void modem_unregister(struct ofono_modem *modem)
 	__ofono_watchlist_free(modem->atom_watches);
 	modem->atom_watches = NULL;
 
+	__ofono_watchlist_free(modem->online_watches);
+	modem->online_watches = NULL;
+
 	modem->sim_watch = 0;
 	modem->sim_ready_watch = 0;
 
diff --git a/src/ofono.h b/src/ofono.h
index ab6ecd2..35335b6 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -177,6 +177,14 @@ unsigned int __ofono_modemwatch_add(ofono_modemwatch_cb_t cb, void *user,
 					ofono_destroy_func destroy);
 gboolean __ofono_modemwatch_remove(unsigned int id);
 
+typedef void (*ofono_modem_online_notify_func)(ofono_bool_t state,
+		void *data);
+unsigned int __ofono_modem_add_online_watch(struct ofono_modem *modem,
+		ofono_modem_online_notify_func notify,
+		void *data, ofono_destroy_func destroy);
+void __ofono_modem_remove_online_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: add modem online-offline watch

[Marcel Holtmann]

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

Hi Andras,

> Signed-off-by: Andras Domokos <Andras.Domokos@nokia.com>

please redo the patches without the Signed-off-by line. It is mentioned
in doc/coding-style.txt that we do not use this.

Regards

Marcel

Re: [RFC PATCH 1/3] modem: add modem online-offline watch

[Denis Kenzior]

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

Hi Andras,

On 11/09/2010 02:59 AM, Andras Domokos wrote:
> 
> Signed-off-by: Andras Domokos <Andras.Domokos@nokia.com>

As Marcel already mentioned, we do not use Signed-off-by.

Otherwise this patch looks good to me.

Regards,
-Denis

[RFC PATCH 2/3] modem: add EmergencyMode property

[Andras Domokos]

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


Signed-off-by: Andras Domokos <Andras.Domokos@nokia.com>
---
 src/modem.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/ofono.h |    4 ++++
 2 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/src/modem.c b/src/modem.c
index f73cc1d..d7ad90e 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -72,6 +72,7 @@ struct ofono_modem {
 	ofono_bool_t		powered_pending;
 	guint			timeout;
 	ofono_bool_t		online;
+	unsigned int		emergency_mode;
 	struct ofono_watchlist	*online_watches;
 	GHashTable		*properties;
 	struct ofono_sim	*sim;
@@ -479,6 +480,50 @@ void __ofono_modem_remove_online_watch(struct ofono_modem *modem, unsigned id)
 	__ofono_watchlist_remove_item(modem->online_watches, id);
 }
 
+ofono_bool_t ofono_modem_get_emergency_mode(struct ofono_modem *modem)
+{
+	if (modem == NULL)
+		return FALSE;
+
+	return modem->emergency_mode != 0;
+}
+
+void ofono_modem_inc_emergency_mode(struct ofono_modem *modem)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = ofono_modem_get_path(modem);
+	ofono_bool_t emergency_mode = modem->emergency_mode;
+	gboolean state = TRUE;
+
+	modem->emergency_mode++;
+	if (emergency_mode)
+		return;
+
+	ofono_dbus_signal_property_changed(conn, path,
+						OFONO_MODEM_INTERFACE,
+						"EmergencyMode",
+						DBUS_TYPE_BOOLEAN,
+						&state);
+	modem_change_state(modem, MODEM_STATE_ONLINE);
+}
+
+void ofono_modem_dec_emergency_mode(struct ofono_modem *modem)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = ofono_modem_get_path(modem);
+	gboolean state = FALSE;
+
+	modem->emergency_mode--;
+	if (modem->emergency_mode)
+		return;
+
+	ofono_dbus_signal_property_changed(conn, path,
+						OFONO_MODEM_INTERFACE,
+						"EmergencyMode",
+						DBUS_TYPE_BOOLEAN,
+						&state);
+}
+
 static void online_cb(const struct ofono_error *error, void *data)
 {
 	struct ofono_modem *modem = data;
@@ -535,6 +580,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 && online == FALSE)
+		return __ofono_error_failed(msg);
+
 	if (modem->online == online)
 		return dbus_message_new_method_return(msg);
 
@@ -562,6 +610,7 @@ void __ofono_modem_append_properties(struct ofono_modem *modem,
 	int i;
 	GSList *l;
 	struct ofono_atom *devinfo_atom;
+	ofono_bool_t state;
 
 	ofono_dbus_dict_append(dict, "Online", DBUS_TYPE_BOOLEAN,
 				&modem->online);
@@ -569,6 +618,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",
+				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 */
diff --git a/src/ofono.h b/src/ofono.h
index 35335b6..b5c32b4 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -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>
 
 gboolean __ofono_call_barring_is_busy(struct ofono_call_barring *cb);
-- 
1.7.0.4

Re: [RFC PATCH 2/3] modem: add EmergencyMode property

[Denis Kenzior]

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

Hi Andras,

On 11/09/2010 02:59 AM, Andras Domokos wrote:
> 
> Signed-off-by: Andras Domokos <Andras.Domokos@nokia.com>
> ---
>  src/modem.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/ofono.h |    4 ++++
>  2 files changed, 57 insertions(+), 0 deletions(-)
> 
> diff --git a/src/modem.c b/src/modem.c
> index f73cc1d..d7ad90e 100644
> --- a/src/modem.c
> +++ b/src/modem.c
> @@ -72,6 +72,7 @@ struct ofono_modem {
>  	ofono_bool_t		powered_pending;
>  	guint			timeout;
>  	ofono_bool_t		online;
> +	unsigned int		emergency_mode;
>  	struct ofono_watchlist	*online_watches;
>  	GHashTable		*properties;
>  	struct ofono_sim	*sim;
> @@ -479,6 +480,50 @@ void __ofono_modem_remove_online_watch(struct ofono_modem *modem, unsigned id)
>  	__ofono_watchlist_remove_item(modem->online_watches, id);
>  }
>  
> +ofono_bool_t ofono_modem_get_emergency_mode(struct ofono_modem *modem)
> +{
> +	if (modem == NULL)
> +		return FALSE;
> +
> +	return modem->emergency_mode != 0;
> +}
> +
> +void ofono_modem_inc_emergency_mode(struct ofono_modem *modem)
> +{
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	const char *path = ofono_modem_get_path(modem);
> +	ofono_bool_t emergency_mode = modem->emergency_mode;

I suggest getting rid of this variable

> +	gboolean state = TRUE;
> +
> +	modem->emergency_mode++;
> +	if (emergency_mode)
> +		return;
> +

And checking if modem->emergency_mode > 1 here...

> +	ofono_dbus_signal_property_changed(conn, path,
> +						OFONO_MODEM_INTERFACE,
> +						"EmergencyMode",
> +						DBUS_TYPE_BOOLEAN,
> +						&state);
> +	modem_change_state(modem, MODEM_STATE_ONLINE);

So we have to be a bit careful here.  You can't simply call
modem_change_state here.  This results in calling the post_online
method, which populates the atoms that function when the radio is on.
Instead you need to call the set_online driver method directly.

Also, you probably do not want to populate the online atoms in an
emergency call situation.  Each atom driver / atom will perform
initialization procedures when they're newly added.  This can
potentially interfere with the voice call setup time, and not desirable.
 Not to mention that if we're actually in an emergency situation with
the SIM removed or PIN locked, we do not want to populate the atoms in
the first place.

So the modem 'ONLINE_STATE' has to be slightly de-coupled from whether
the radio is 'online'.

> +}
> +
> +void ofono_modem_dec_emergency_mode(struct ofono_modem *modem)
> +{
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	const char *path = ofono_modem_get_path(modem);
> +	gboolean state = FALSE;
> +
> +	modem->emergency_mode--;
> +	if (modem->emergency_mode)
> +		return;
> +
> +	ofono_dbus_signal_property_changed(conn, path,
> +						OFONO_MODEM_INTERFACE,
> +						"EmergencyMode",
> +						DBUS_TYPE_BOOLEAN,
> +						&state);

Here you should turn the radio off if it was off before starting the
emergency call.

> +}
> +
>  static void online_cb(const struct ofono_error *error, void *data)
>  {
>  	struct ofono_modem *modem = data;
> @@ -535,6 +580,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 && online == FALSE)
> +		return __ofono_error_failed(msg);
> +
>  	if (modem->online == online)
>  		return dbus_message_new_method_return(msg);
>  
> @@ -562,6 +610,7 @@ void __ofono_modem_append_properties(struct ofono_modem *modem,
>  	int i;
>  	GSList *l;
>  	struct ofono_atom *devinfo_atom;
> +	ofono_bool_t state;
>  
>  	ofono_dbus_dict_append(dict, "Online", DBUS_TYPE_BOOLEAN,
>  				&modem->online);
> @@ -569,6 +618,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",
> +				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 */
> diff --git a/src/ofono.h b/src/ofono.h
> index 35335b6..b5c32b4 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -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>
>  
>  gboolean __ofono_call_barring_is_busy(struct ofono_call_barring *cb);

Regards,
-Denis

Re: [RFC PATCH 2/3] modem: add EmergencyMode property

[Andras Domokos]

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

Hi Denis,


On 11/09/2010 04:37 PM, ext Denis Kenzior wrote:
> Hi Andras,
>
> On 11/09/2010 02:59 AM, Andras Domokos wrote:
>    
>> Signed-off-by: Andras Domokos<Andras.Domokos@nokia.com>
>> ---
>>   src/modem.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   src/ofono.h |    4 ++++
>>   2 files changed, 57 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/modem.c b/src/modem.c
>> index f73cc1d..d7ad90e 100644
>> --- a/src/modem.c
>> +++ b/src/modem.c
>> @@ -72,6 +72,7 @@ struct ofono_modem {
>>   	ofono_bool_t		powered_pending;
>>   	guint			timeout;
>>   	ofono_bool_t		online;
>> +	unsigned int		emergency_mode;
>>   	struct ofono_watchlist	*online_watches;
>>   	GHashTable		*properties;
>>   	struct ofono_sim	*sim;
>> @@ -479,6 +480,50 @@ void __ofono_modem_remove_online_watch(struct ofono_modem *modem, unsigned id)
>>   	__ofono_watchlist_remove_item(modem->online_watches, id);
>>   }
>>
>> +ofono_bool_t ofono_modem_get_emergency_mode(struct ofono_modem *modem)
>> +{
>> +	if (modem == NULL)
>> +		return FALSE;
>> +
>> +	return modem->emergency_mode != 0;
>> +}
>> +
>> +void ofono_modem_inc_emergency_mode(struct ofono_modem *modem)
>> +{
>> +	DBusConnection *conn = ofono_dbus_get_connection();
>> +	const char *path = ofono_modem_get_path(modem);
>> +	ofono_bool_t emergency_mode = modem->emergency_mode;
>>      
> I suggest getting rid of this variable
>
>    
>> +	gboolean state = TRUE;
>> +
>> +	modem->emergency_mode++;
>> +	if (emergency_mode)
>> +		return;
>> +
>>      
> And checking if modem->emergency_mode>  1 here...
>
>    
You are right, makes the code shorter with less variables.
>> +	ofono_dbus_signal_property_changed(conn, path,
>> +						OFONO_MODEM_INTERFACE,
>> +						"EmergencyMode",
>> +						DBUS_TYPE_BOOLEAN,
>> +						&state);
>> +	modem_change_state(modem, MODEM_STATE_ONLINE);
>>      
> So we have to be a bit careful here.  You can't simply call
> modem_change_state here.  This results in calling the post_online
> method, which populates the atoms that function when the radio is on.
> Instead you need to call the set_online driver method directly.
>
> Also, you probably do not want to populate the online atoms in an
> emergency call situation.  Each atom driver / atom will perform
> initialization procedures when they're newly added.  This can
> potentially interfere with the voice call setup time, and not desirable.
>   Not to mention that if we're actually in an emergency situation with
> the SIM removed or PIN locked, we do not want to populate the atoms in
> the first place.
>
> So the modem 'ONLINE_STATE' has to be slightly de-coupled from whether
> the radio is 'online'.
>
>    
Good point, thanks. I am going to call directly the modem driver
set_online function with my own callback.

Now the question is whether modem->online should be toggled
according to the radio state changes? I think yes, and have the
change signaled on D-Bus (modem "Online" poperty).

There is another question then, when to notify the modem online
watchers, supposedly whenever modem->online changes, including
the changes triggered by the emergency call case.

>> +}
>> +
>> +void ofono_modem_dec_emergency_mode(struct ofono_modem *modem)
>> +{
>> +	DBusConnection *conn = ofono_dbus_get_connection();
>> +	const char *path = ofono_modem_get_path(modem);
>> +	gboolean state = FALSE;
>> +
>> +	modem->emergency_mode--;
>> +	if (modem->emergency_mode)
>> +		return;
>> +
>> +	ofono_dbus_signal_property_changed(conn, path,
>> +						OFONO_MODEM_INTERFACE,
>> +						"EmergencyMode",
>> +						DBUS_TYPE_BOOLEAN,
>> +						&state);
>>      
> Here you should turn the radio off if it was off before starting the
> emergency call.
>
>    

The rationale behind leaving the radio on after emergency calls
was to have the theoretical possibility the emergency call center
to call you back (when SIM card is present...). If this is an unrealistic
scenario, I'll disable the radio after emergency calls in case it was off
before the call.

>    
>> +}
>> +
>>   static void online_cb(const struct ofono_error *error, void *data)
>>   {
>>   	struct ofono_modem *modem = data;
>> @@ -535,6 +580,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&&  online == FALSE)
>> +		return __ofono_error_failed(msg);
>> +
>>   	if (modem->online == online)
>>   		return dbus_message_new_method_return(msg);
>>
>> @@ -562,6 +610,7 @@ void __ofono_modem_append_properties(struct ofono_modem *modem,
>>   	int i;
>>   	GSList *l;
>>   	struct ofono_atom *devinfo_atom;
>> +	ofono_bool_t state;
>>
>>   	ofono_dbus_dict_append(dict, "Online", DBUS_TYPE_BOOLEAN,
>>   				&modem->online);
>> @@ -569,6 +618,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",
>> +				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 */
>> diff --git a/src/ofono.h b/src/ofono.h
>> index 35335b6..b5c32b4 100644
>> --- a/src/ofono.h
>> +++ b/src/ofono.h
>> @@ -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>
>>
>>   gboolean __ofono_call_barring_is_busy(struct ofono_call_barring *cb);
>>      
> Regards,
> -Denis
>    
Regards,
Andras

Re: [RFC PATCH 2/3] modem: add EmergencyMode property

[Denis Kenzior]

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

Hi Andras,

>> So we have to be a bit careful here.  You can't simply call
>> modem_change_state here.  This results in calling the post_online
>> method, which populates the atoms that function when the radio is on.
>> Instead you need to call the set_online driver method directly.
>>
>> Also, you probably do not want to populate the online atoms in an
>> emergency call situation.  Each atom driver / atom will perform
>> initialization procedures when they're newly added.  This can
>> potentially interfere with the voice call setup time, and not desirable.
>>   Not to mention that if we're actually in an emergency situation with
>> the SIM removed or PIN locked, we do not want to populate the atoms in
>> the first place.
>>
>> So the modem 'ONLINE_STATE' has to be slightly de-coupled from whether
>> the radio is 'online'.
>>
>>    
> Good point, thanks. I am going to call directly the modem driver
> set_online function with my own callback.
> 
> Now the question is whether modem->online should be toggled
> according to the radio state changes? I think yes, and have the
> change signaled on D-Bus (modem "Online" poperty).

That makes sense to me, yes.

> 
> There is another question then, when to notify the modem online
> watchers, supposedly whenever modem->online changes, including
> the changes triggered by the emergency call case.

In the current structure this is just fine.  So let us run with it for
now and see what happens.  In the future we might find something a bit
more complicated is required, but we cross that bridge when we come to it.

> 
>>> +}
>>> +
>>> +void ofono_modem_dec_emergency_mode(struct ofono_modem *modem)
>>> +{
>>> +    DBusConnection *conn = ofono_dbus_get_connection();
>>> +    const char *path = ofono_modem_get_path(modem);
>>> +    gboolean state = FALSE;
>>> +
>>> +    modem->emergency_mode--;
>>> +    if (modem->emergency_mode)
>>> +        return;
>>> +
>>> +    ofono_dbus_signal_property_changed(conn, path,
>>> +                        OFONO_MODEM_INTERFACE,
>>> +                        "EmergencyMode",
>>> +                        DBUS_TYPE_BOOLEAN,
>>> +                        &state);
>>>      
>> Here you should turn the radio off if it was off before starting the
>> emergency call.
>>
>>    
> 
> The rationale behind leaving the radio on after emergency calls
> was to have the theoretical possibility the emergency call center
> to call you back (when SIM card is present...). If this is an unrealistic
> scenario, I'll disable the radio after emergency calls in case it was off
> before the call.
> 

That is a fair point, however, just silently leaving us Online might not
be a good idea.

I suggest finding out how exactly this is supposed to work.  E.g. can we
get away with starting an X minute timeout every time we leave emergency
mode and go offline automatically?  This timeout would obviously have to
be canceled if the user sets Online manually.

Regards,
-Denis

Re: [RFC PATCH 2/3] modem: add EmergencyMode property

[Marcel Holtmann]

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

Hi Denis,

> >> Here you should turn the radio off if it was off before starting the
> >> emergency call.
> >>
> >>    
> > 
> > The rationale behind leaving the radio on after emergency calls
> > was to have the theoretical possibility the emergency call center
> > to call you back (when SIM card is present...). If this is an unrealistic
> > scenario, I'll disable the radio after emergency calls in case it was off
> > before the call.
> > 
> 
> That is a fair point, however, just silently leaving us Online might not
> be a good idea.
> 
> I suggest finding out how exactly this is supposed to work.  E.g. can we
> get away with starting an X minute timeout every time we leave emergency
> mode and go offline automatically?  This timeout would obviously have to
> be canceled if the user sets Online manually.

on one hand I see this making perfect sense. For example just leave the
modem in online state for lets say 15 minutes. But then on the other
hand I am worried about regulatory requirements where the modem is
suppose to be offline.

So what I would propose here to actually leave the modem online for a
really short period of time like 30 seconds or so. And then bring it
back down. Then we can tweak that value to our needs. And in addition
the user could switched it back online by himself and we avoid going
into offline mode and back to online mode.

Of course when switching to online mode by user we need to ensure that
all atoms are brought up properly.

Regards

Marcel

[RFC PATCH 3/3] voicecall: add emergency call handling

[Andras Domokos]

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


Signed-off-by: Andras Domokos <Andras.Domokos@nokia.com>
---
 src/voicecall.c |  101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 100 insertions(+), 1 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index bd64432..0268ce1 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -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)
+{
+	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 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,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_modem_dec_emergency_mode(modem);
 		reply = __ofono_error_failed(vc->pending);
+	}
 
 	__ofono_dbus_pending_reply(&vc->pending, reply);
 
@@ -1156,6 +1177,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;
@@ -1195,6 +1217,13 @@ 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)
+			return NULL;
+	}
+
 	vc->driver->dial(vc, &ph, clir, OFONO_CUG_OPTION_DEFAULT,
 				manager_dial_callback, vc);
 
@@ -1748,6 +1777,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 +1797,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 +1836,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);
@@ -1814,6 +1848,7 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
 				const struct ofono_call *call)
 {
 	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
+	const char *number;
 	GSList *l;
 	struct voicecall *v = NULL;
 	struct ofono_call *newcall;
@@ -1860,6 +1895,10 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
 
 	vc->call_list = g_slist_insert_sorted(vc->call_list, v, call_compare);
 
+	number = phone_number_to_string(&v->call->phone_number);
+	if (emergency_number(vc, number))
+		ofono_modem_inc_emergency_mode(modem);
+
 	voicecalls_emit_call_added(vc, v);
 
 	return;
@@ -2067,6 +2106,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 +2148,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);
 }
 
@@ -2202,6 +2248,47 @@ static void sim_watch(struct ofono_atom *atom,
 	sim_state_watch(ofono_sim_get_state(sim), vc);
 }
 
+static void modem_online_watch(ofono_bool_t 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;
+	}
+
+	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_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();
@@ -2220,6 +2307,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
@@ -2297,6 +2387,7 @@ ofono_bool_t __ofono_voicecall_is_busy(struct ofono_voicecall *vc,
 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;
 
@@ -2306,6 +2397,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_modem_inc_emergency_mode(modem);
 		return;
 	}
 
@@ -2331,6 +2425,11 @@ static void dial_request_cb(const struct ofono_error *error, void *data)
 
 static void dial_request(struct ofono_voicecall *vc)
 {
+	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
+
+	if (emergency_number(vc, phone_number_to_string(&vc->dial_req->ph)))
+		ofono_modem_inc_emergency_mode(modem);
+
 	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 3/3] voicecall: add emergency call handling

[Denis Kenzior]

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

Hi Andras,

On 11/09/2010 03:00 AM, Andras Domokos wrote:
> 
> Signed-off-by: Andras Domokos <Andras.Domokos@nokia.com>
> ---
>  src/voicecall.c |  101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 100 insertions(+), 1 deletions(-)
> 
> diff --git a/src/voicecall.c b/src/voicecall.c
> index bd64432..0268ce1 100644
> --- a/src/voicecall.c
> +++ b/src/voicecall.c
> @@ -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)
> +{
> +	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 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,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_modem_dec_emergency_mode(modem);

Empty line here, please see coding-style.txt, Section M1

>  		reply = __ofono_error_failed(vc->pending);
> +	}
>  
>  	__ofono_dbus_pending_reply(&vc->pending, reply);
>  
> @@ -1156,6 +1177,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;
> @@ -1195,6 +1217,13 @@ 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);

Empty line here

> +		ofono_modem_inc_emergency_mode(modem);

And one here

> +		if (!online)
> +			return NULL;
> +	}
> +
>  	vc->driver->dial(vc, &ph, clir, OFONO_CUG_OPTION_DEFAULT,
>  				manager_dial_callback, vc);
>  
> @@ -1748,6 +1777,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 +1797,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 +1836,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);
> +

I'm not convinced that you have the reference tracking completely
correct.  You increment the ref when a call is dialed, as well as when
it is notified + created.  However, you only decrement it when it is
disconnected...

>  	voicecall_dbus_unregister(vc, call);
>  
>  	vc->call_list = g_slist_remove(vc->call_list, call);
> @@ -1814,6 +1848,7 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
>  				const struct ofono_call *call)
>  {
>  	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
> +	const char *number;
>  	GSList *l;
>  	struct voicecall *v = NULL;
>  	struct ofono_call *newcall;
> @@ -1860,6 +1895,10 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
>  
>  	vc->call_list = g_slist_insert_sorted(vc->call_list, v, call_compare);
>  
> +	number = phone_number_to_string(&v->call->phone_number);
> +	if (emergency_number(vc, number))
> +		ofono_modem_inc_emergency_mode(modem);
> +
>  	voicecalls_emit_call_added(vc, v);
>  
>  	return;
> @@ -2067,6 +2106,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 +2148,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);
>  }
>  
> @@ -2202,6 +2248,47 @@ static void sim_watch(struct ofono_atom *atom,
>  	sim_state_watch(ofono_sim_get_state(sim), vc);
>  }
>  
> +static void modem_online_watch(ofono_bool_t 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;
> +	}
> +
> +	if (!emergency_number(vc, number))
> +		return;
> +
> +	if (!ofono_modem_get_online(modem)) {

Why do you need to use ofono_modem_get_online when the state parameter
is already being passed in?

> +		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();
> @@ -2220,6 +2307,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
> @@ -2297,6 +2387,7 @@ ofono_bool_t __ofono_voicecall_is_busy(struct ofono_voicecall *vc,
>  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;
>  
> @@ -2306,6 +2397,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_modem_inc_emergency_mode(modem);

Do you mean ofono_modem_dec_emergency_mode here?

>  		return;
>  	}
>  
> @@ -2331,6 +2425,11 @@ static void dial_request_cb(const struct ofono_error *error, void *data)
>  
>  static void dial_request(struct ofono_voicecall *vc)
>  {
> +	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
> +
> +	if (emergency_number(vc, phone_number_to_string(&vc->dial_req->ph)))
> +		ofono_modem_inc_emergency_mode(modem);
> +

This part looks a bit fishy, you need to delay the dial request until we
entered online mode.

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

Regards,
-Denis