[PATCH] voicecall: Add EmergencyCall property.

[PATCH] voicecall: Add EmergencyCall property.

[John Mathew]

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

---
 doc/voicecall-api.txt |    4 ++++
 src/voicecall.c       |   34 +++++++++++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/doc/voicecall-api.txt b/doc/voicecall-api.txt
index f0ba316..60c692c 100644
--- a/doc/voicecall-api.txt
+++ b/doc/voicecall-api.txt
@@ -125,3 +125,7 @@ Properties	string LineIdentification [readonly]
 
 			Icon identifier to be used instead of or together
 			with the text information.
+
+		boolean EmergencyCall [readonly]
+
+			Contains the indication if the voice call is emergency call or not.
diff --git a/src/voicecall.c b/src/voicecall.c
index bd64432..e39f4b7 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -315,6 +315,20 @@ static void tone_request_finish(struct ofono_voicecall *vc,
 	g_free(entry);
 }
 
+static gboolean voicecall_isemergency(struct voicecall *v)
+{
+	struct ofono_call *call = v->call;
+	const char *lineid_str;
+
+	lineid_str = phone_number_to_string(&call->phone_number);
+
+	if (g_slist_find_custom(v->vc->en_list, lineid_str,
+						(GCompareFunc) strcmp))
+		return TRUE;
+	else
+		return FALSE;
+}
+
 static void append_voicecall_properties(struct voicecall *v,
 					DBusMessageIter *dict)
 {
@@ -322,7 +336,8 @@ static void append_voicecall_properties(struct voicecall *v,
 	const char *status;
 	const char *callerid;
 	const char *timestr;
-	ofono_bool_t mpty;
+	ofono_bool_t mpt;
+	dbus_bool_t emergency_call;
 
 	status = call_status_to_string(call->status);
 	callerid = phone_number_to_string(&call->phone_number);
@@ -358,6 +373,14 @@ static void append_voicecall_properties(struct voicecall *v,
 	if (v->icon_id)
 		ofono_dbus_dict_append(dict, "Icon", DBUS_TYPE_BYTE,
 					&v->icon_id);
+
+	if (voicecall_isemergency(v))
+			emergency_call = TRUE;
+	else
+			emergency_call = FALSE;
+
+	ofono_dbus_dict_append(dict, "EmergencyCall", DBUS_TYPE_BOOLEAN,
+					&emergency_call);
 }
 
 static DBusMessage *voicecall_get_properties(DBusConnection *conn,
@@ -722,6 +745,15 @@ static void voicecall_set_call_lineid(struct voicecall *v,
 						OFONO_VOICECALL_INTERFACE,
 						"LineIdentification",
 						DBUS_TYPE_STRING, &lineid_str);
+
+	if (voicecall_isemergency(v)) {
+		dbus_bool_t emergency_call = TRUE;
+		ofono_dbus_signal_property_changed(conn, path,
+						OFONO_VOICECALL_INTERFACE,
+						"EmergencyCall",
+						DBUS_TYPE_BOOLEAN,
+						&emergency_call);
+	}
 }
 
 static gboolean voicecall_dbus_register(struct voicecall *v)
-- 
1.7.0.4

Re: [PATCH] voicecall: Add EmergencyCall property.

[Marcel Holtmann]

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

Hi John,

>  doc/voicecall-api.txt |    4 ++++
>  src/voicecall.c       |   34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 37 insertions(+), 1 deletions(-)
> 
> diff --git a/doc/voicecall-api.txt b/doc/voicecall-api.txt
> index f0ba316..60c692c 100644
> --- a/doc/voicecall-api.txt
> +++ b/doc/voicecall-api.txt
> @@ -125,3 +125,7 @@ Properties	string LineIdentification [readonly]
>  
>  			Icon identifier to be used instead of or together
>  			with the text information.
> +
> +		boolean EmergencyCall [readonly]
> +
> +			Contains the indication if the voice call is emergency call or not.

do we really wanna call this EmergencyCall instead of just Emergency? In
theory the "call" is already in the interface name.

Regards

Marcel

RE: [PATCH] voicecall: Add EmergencyCall property.

[John.Mathew]

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

Hi Marcel,

Will change the property name to Emergency and resend the patch.

Regards,
John 

-----Original Message-----
From: ofono-bounces(a)ofono.org [mailto:ofono-bounces(a)ofono.org] On Behalf
Of Marcel Holtmann
Sent: 11 November 2010 17:15
To: ofono(a)ofono.org
Subject: Re: [PATCH] voicecall: Add EmergencyCall property.

Hi John,

>  doc/voicecall-api.txt |    4 ++++
>  src/voicecall.c       |   34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 37 insertions(+), 1 deletions(-)
> 
> diff --git a/doc/voicecall-api.txt b/doc/voicecall-api.txt index 
> f0ba316..60c692c 100644
> --- a/doc/voicecall-api.txt
> +++ b/doc/voicecall-api.txt
> @@ -125,3 +125,7 @@ Properties	string LineIdentification
[readonly]
>  
>  			Icon identifier to be used instead of or
together
>  			with the text information.
> +
> +		boolean EmergencyCall [readonly]
> +
> +			Contains the indication if the voice call is
emergency call or not.

do we really wanna call this EmergencyCall instead of just Emergency? In
theory the "call" is already in the interface name.

Regards

Marcel


_______________________________________________
ofono mailing list
ofono(a)ofono.org
http://lists.ofono.org/listinfo/ofono

Re: [PATCH] voicecall: Add EmergencyCall property.

[Denis Kenzior]

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

Hi John,

> +static gboolean voicecall_isemergency(struct voicecall *v)

Please use voicecall_is_emergency

> +{
> +	struct ofono_call *call = v->call;
> +	const char *lineid_str;
> +
> +	lineid_str = phone_number_to_string(&call->phone_number);
> +
> +	if (g_slist_find_custom(v->vc->en_list, lineid_str,
> +						(GCompareFunc) strcmp))

As a general rule, the only function casting we allow is for GFunc use
of g_free.  I really prefer that you write a separate function here.

> +		return TRUE;
> +	else

Please drop the else, it isn't actually required and makes the code
slightly cleaner.

> +		return FALSE;
> +}
> +
>  static void append_voicecall_properties(struct voicecall *v,
>  					DBusMessageIter *dict)
>  {
> @@ -322,7 +336,8 @@ static void append_voicecall_properties(struct voicecall *v,
>  	const char *status;
>  	const char *callerid;
>  	const char *timestr;
> -	ofono_bool_t mpty;
> +	ofono_bool_t mpt;

Is there a reason you're changing this variable name?

> +	dbus_bool_t emergency_call;
>  
>  	status = call_status_to_string(call->status);
>  	callerid = phone_number_to_string(&call->phone_number);
> @@ -358,6 +373,14 @@ static void append_voicecall_properties(struct voicecall *v,
>  	if (v->icon_id)
>  		ofono_dbus_dict_append(dict, "Icon", DBUS_TYPE_BYTE,
>  					&v->icon_id);
> +
> +	if (voicecall_isemergency(v))
> +			emergency_call = TRUE;
> +	else
> +			emergency_call = FALSE;
> +
> +	ofono_dbus_dict_append(dict, "EmergencyCall", DBUS_TYPE_BOOLEAN,
> +					&emergency_call);
>  }
>  
>  static DBusMessage *voicecall_get_properties(DBusConnection *conn,
> @@ -722,6 +745,15 @@ static void voicecall_set_call_lineid(struct voicecall *v,
>  						OFONO_VOICECALL_INTERFACE,
>  						"LineIdentification",
>  						DBUS_TYPE_STRING, &lineid_str);
> +
> +	if (voicecall_isemergency(v)) {
> +		dbus_bool_t emergency_call = TRUE;

In general we prefer an empty line after every variable declaration block.

> +		ofono_dbus_signal_property_changed(conn, path,
> +						OFONO_VOICECALL_INTERFACE,
> +						"EmergencyCall",
> +						DBUS_TYPE_BOOLEAN,
> +						&emergency_call);
> +	}
>  }
>  
>  static gboolean voicecall_dbus_register(struct voicecall *v)

Regards,
-Denis

RE: [PATCH] voicecall: Add EmergencyCall property.

[Marcel Holtmann]

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

Hi John,

please do not top post on this mailing list. I am really serious with
the fact that I am going to ignore people from now on that do that.

> Will change the property name to Emergency and resend the patch.

This was a question open for discussion. What would be the appropriate
property name?

Regards

Marcel

RE: [PATCH] voicecall: Add EmergencyCall property.

[John.Mathew]

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


Hi Denis,

> +static gboolean voicecall_isemergency(struct voicecall *v)

> Please use voicecall_is_emergency

Will change

> +{
> +     struct ofono_call *call = v->call;
> +     const char *lineid_str;
> +
> +     lineid_str = phone_number_to_string(&call->phone_number);
> +
> +     if (g_slist_find_custom(v->vc->en_list, lineid_str,
> +                                             (GCompareFunc) strcmp))

> As a general rule, the only function casting we allow is for GFunc use of g_free.  I really prefer that you write a separate function here.

I had two options, to write a function comparing the string, or to use strcmp. A search in ofono source showed me 2 instances where strcmp is used and I followed it here too.

I can write a function to compare the phone number, and will make a patch.

> +             return TRUE;
> +     else

> Please drop the else, it isn't actually required and makes the code slightly cleaner.

Agreed, will make the change.

> +             return FALSE;
> +}
> +
>  static void append_voicecall_properties(struct voicecall *v,
>                                       DBusMessageIter *dict)
>  {
> @@ -322,7 +336,8 @@ static void append_voicecall_properties(struct voicecall *v,
>       const char *status;
>       const char *callerid;
>       const char *timestr;
> -     ofono_bool_t mpty;
> +     ofono_bool_t mpt;

> Is there a reason you're changing this variable name?

Will recity this, came in as typo mistake.

> +     dbus_bool_t emergency_call;
> 
>       status = call_status_to_string(call->status);
>       callerid = phone_number_to_string(&call->phone_number);
> @@ -358,6 +373,14 @@ static void append_voicecall_properties(struct voicecall *v,
>       if (v->icon_id)
>               ofono_dbus_dict_append(dict, "Icon", DBUS_TYPE_BYTE,
>                                       &v->icon_id);
> +
> +     if (voicecall_isemergency(v))
> +                     emergency_call = TRUE;
> +     else
> +                     emergency_call = FALSE;
> +
> +     ofono_dbus_dict_append(dict, "EmergencyCall", DBUS_TYPE_BOOLEAN,
> +                                     &emergency_call);
>  }
> 
>  static DBusMessage *voicecall_get_properties(DBusConnection *conn, @@
> -722,6 +745,15 @@ static void voicecall_set_call_lineid(struct voicecall *v,
>                                               OFONO_VOICECALL_INTERFACE,
>                                               "LineIdentification",
>                                               DBUS_TYPE_STRING, &lineid_str);
> +
> +     if (voicecall_isemergency(v)) {
> +             dbus_bool_t emergency_call = TRUE;

> In general we prefer an empty line after every variable declaration block.

Sure will change

> +             ofono_dbus_signal_property_changed(conn, path,
> +                                             OFONO_VOICECALL_INTERFACE,
> +                                             "EmergencyCall",
> +                                             DBUS_TYPE_BOOLEAN,
> +                                             &emergency_call);
> +     }
>  }
> 
>  static gboolean voicecall_dbus_register(struct voicecall *v)

Regards,
John 


[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 7506 bytes --]

RE: [PATCH] voicecall: Add EmergencyCall property.

[John.Mathew]

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

 
Hi Marcel,

> This was a question open for discussion. What would be the appropriate
property name?

As per my understanding, this property is only to provide the Emergency
call indication to the application side, so that applications can
display a different UI compared to the normal UI. In this case, I think
the property name "Emergency" is enough. 

Since this is a property for the call, the following fits as well. We
can remove the existing multiparty property and add it to the new
property.

string Type [readonly]
       Contains the Type of the current call.

       The Values can be:
               "Normal"
               "Emergency"
		   "Data"
	         "Multiparty"	

Let me know your opinion on this.
                          
regards
John

RE: [PATCH] voicecall: Add EmergencyCall property.

[John.Mathew]

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

Hi Marcel, 

>
>> This was a question open for discussion. What would be the 
>appropriate property name?
>
>As per my understanding, this property is only to provide the 
>Emergency call indication to the application side, so that 
>applications can display a different UI compared to the normal 
>UI. In this case, I think the property name "Emergency" is enough. 
>
>Since this is a property for the call, the following fits as 
>well. We can remove the existing multiparty property and add 
>it to the new property.
>
>string Type [readonly]
>       Contains the Type of the current call.
>
>       The Values can be:
>               "Normal"
>               "Emergency"
>		   "Data"
>	         "Multiparty"	
>
>Let me know your opinion on this.
> 
                         
Any comments?

Re: [PATCH] voicecall: Add EmergencyCall property.

[Denis Kenzior]

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

Hi John,

On 12/03/2010 02:00 AM, John.Mathew(a)elektrobit.com wrote:
> Hi Marcel, 
> 
>>
>>> This was a question open for discussion. What would be the 
>> appropriate property name?
>>
>> As per my understanding, this property is only to provide the 
>> Emergency call indication to the application side, so that 
>> applications can display a different UI compared to the normal 
>> UI. In this case, I think the property name "Emergency" is enough. 
>>
>> Since this is a property for the call, the following fits as 
>> well. We can remove the existing multiparty property and add 
>> it to the new property.
>>
>> string Type [readonly]
>>       Contains the Type of the current call.
>>
>>       The Values can be:
>>               "Normal"
>>               "Emergency"
>> 		   "Data"
>> 	         "Multiparty"	
>>
>> Let me know your opinion on this.
>>
>                          
> Any comments?
> 

We're not going to support data calls, not by re-using the Voicecall
interface anyway.  So I still prefer the original proposal of using
boolean Emergency property here.

Regards,
-Denis

[PATCH] voicecall: Add emergency property

[John Mathew]

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

---
 doc/voicecall-api.txt |    5 +++++
 src/voicecall.c       |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/doc/voicecall-api.txt b/doc/voicecall-api.txt
index f0ba316..95a43a6 100644
--- a/doc/voicecall-api.txt
+++ b/doc/voicecall-api.txt
@@ -125,3 +125,8 @@ Properties	string LineIdentification [readonly]
 
 			Icon identifier to be used instead of or together
 			with the text information.
+
+		boolean Emergency [readonly]
+
+			Contains the indication if the voice call is an emergency
+			call or not.
diff --git a/src/voicecall.c b/src/voicecall.c
index dbf3e9a..3f9fe54 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -316,6 +316,23 @@ static void tone_request_finish(struct ofono_voicecall *vc,
 	g_free(entry);
 }
 
+static gint number_compare(gconstpointer a, gconstpointer b)
+{
+	const char *s1 = a, *s2 = b;
+	return strcmp(s1, s2);
+}
+
+static gboolean voicecall_is_emergency(struct voicecall *v)
+{
+	struct ofono_call *call = v->call;
+	const char *lineid_str;
+
+	lineid_str = phone_number_to_string(&call->phone_number);
+
+	return (g_slist_find_custom(v->vc->en_list, lineid_str,
+						number_compare)) ? TRUE : FALSE;
+}
+
 static void append_voicecall_properties(struct voicecall *v,
 					DBusMessageIter *dict)
 {
@@ -324,6 +341,7 @@ static void append_voicecall_properties(struct voicecall *v,
 	const char *callerid;
 	const char *timestr;
 	ofono_bool_t mpty;
+	dbus_bool_t emergency_call;
 
 	status = call_status_to_string(call->status);
 	callerid = phone_number_to_string(&call->phone_number);
@@ -359,6 +377,15 @@ static void append_voicecall_properties(struct voicecall *v,
 	if (v->icon_id)
 		ofono_dbus_dict_append(dict, "Icon", DBUS_TYPE_BYTE,
 					&v->icon_id);
+
+	if (voicecall_is_emergency(v) == TRUE)
+		emergency_call = TRUE;
+	else
+		emergency_call = FALSE;
+
+	ofono_dbus_dict_append(dict, "Emergency", DBUS_TYPE_BOOLEAN,
+					&emergency_call);
+
 }
 
 static DBusMessage *voicecall_get_properties(DBusConnection *conn,
@@ -721,6 +748,16 @@ static void voicecall_set_call_lineid(struct voicecall *v,
 						OFONO_VOICECALL_INTERFACE,
 						"LineIdentification",
 						DBUS_TYPE_STRING, &lineid_str);
+
+	if (voicecall_is_emergency(v)) {
+		dbus_bool_t emergency_call = TRUE;
+
+		ofono_dbus_signal_property_changed(conn, path,
+						OFONO_VOICECALL_INTERFACE,
+						"Emergency",
+						DBUS_TYPE_BOOLEAN,
+						&emergency_call);
+	}
 }
 
 static gboolean voicecall_dbus_register(struct voicecall *v)
-- 
1.7.0.4

Re: [PATCH] voicecall: Add emergency property

[Denis Kenzior]

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

Hi John,

On 12/07/2010 06:20 AM, John Mathew wrote:
> ---
>  doc/voicecall-api.txt |    5 +++++
>  src/voicecall.c       |   37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 0 deletions(-)
> 

I applied this patch but broke it up into two.  One for the doc changes
and one for voicecall atom changes.

I've also updated the features document and the TODO list for you ;)

Regards,
-Denis