All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] voicecall: send CallRemoved for calls when voicecall atom is unregistered
  2012-04-18 22:02 [PATCH] voicecall: send CallRemoved for calls when voicecall atom is unregistered Mike Brudevold
@ 2012-04-18 15:52 ` Denis Kenzior
  2012-04-19 20:50   ` Mike
  0 siblings, 1 reply; 5+ messages in thread
From: Denis Kenzior @ 2012-04-18 15:52 UTC (permalink / raw)
  To: ofono

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

Hi Mike,

On 04/18/2012 05:02 PM, Mike Brudevold wrote:
> From: Michael Brudevold <michael.brudevold@logicpd.com>
> 
> The CallRemoved signal was not being sent for existing calls when the voicecall
> atom was unregistered. An example of this is the lack of a CallRemoved signal
> when an HFP AG is disconnected due to SLC timeout while a call is active.
> ---
>  src/voicecall.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/src/voicecall.c b/src/voicecall.c
> index 104b600..263f1ee 100644
> --- a/src/voicecall.c
> +++ b/src/voicecall.c
> @@ -2699,8 +2699,11 @@ static void voicecall_unregister(struct ofono_atom *atom)
>  	if (vc->dial_req)
>  		dial_request_finish(vc);
>  
> -	for (l = vc->call_list; l; l = l->next)
> +	for (l = vc->call_list; l; l = l->next) {
> +		voicecalls_emit_call_removed(vc, l->data);
> +
>  		voicecall_dbus_unregister(vc, l->data);
> +	}
>  

In some respect this is on purpose, the application should listen to the
PropertyChanged on the main Modem object and keep track of Interfaces
property.  e.g. if the VoiceCallManager goes away the application can
pretty much assume that all calls are gone.

I could use some more convincing that this is really needed...

Regards,
-Denis

>  	g_slist_free(vc->call_list);
>  	vc->call_list = NULL;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] voicecall: send CallRemoved for calls when voicecall atom is unregistered
@ 2012-04-18 22:02 Mike Brudevold
  2012-04-18 15:52 ` Denis Kenzior
  0 siblings, 1 reply; 5+ messages in thread
From: Mike Brudevold @ 2012-04-18 22:02 UTC (permalink / raw)
  To: ofono

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

From: Michael Brudevold <michael.brudevold@logicpd.com>

The CallRemoved signal was not being sent for existing calls when the voicecall
atom was unregistered. An example of this is the lack of a CallRemoved signal
when an HFP AG is disconnected due to SLC timeout while a call is active.
---
 src/voicecall.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index 104b600..263f1ee 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -2699,8 +2699,11 @@ static void voicecall_unregister(struct ofono_atom *atom)
 	if (vc->dial_req)
 		dial_request_finish(vc);
 
-	for (l = vc->call_list; l; l = l->next)
+	for (l = vc->call_list; l; l = l->next) {
+		voicecalls_emit_call_removed(vc, l->data);
+
 		voicecall_dbus_unregister(vc, l->data);
+	}
 
 	g_slist_free(vc->call_list);
 	vc->call_list = NULL;
-- 
1.7.5.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] voicecall: send CallRemoved for calls when voicecall atom is unregistered
  2012-04-19 20:50   ` Mike
@ 2012-04-19 14:02     ` Denis Kenzior
  2012-04-20 13:36       ` Mike
  0 siblings, 1 reply; 5+ messages in thread
From: Denis Kenzior @ 2012-04-19 14:02 UTC (permalink / raw)
  To: ofono

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

Hi Mike,

> 
> The only reason I added it is 4 hours of debugging my GUI and ofono to
> figure out why the GUI didn't notice the D-Bus path for the voice call
> was no longer around.  The documentation for CallRemoved says "Signal
> that is sent when a voice call has been released."  This implies it
> should be sent every time it is released.  If you update the
> documentation to make it clear that this is not always the case, then
> it's fine by me.
> 

Please check that commit 7f51df4d satisfies your requirements.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] voicecall: send CallRemoved for calls when voicecall atom is unregistered
  2012-04-18 15:52 ` Denis Kenzior
@ 2012-04-19 20:50   ` Mike
  2012-04-19 14:02     ` Denis Kenzior
  0 siblings, 1 reply; 5+ messages in thread
From: Mike @ 2012-04-19 20:50 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

On Wed, Apr 18, 2012 at 10:52 AM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Mike,
>
> On 04/18/2012 05:02 PM, Mike Brudevold wrote:
>> From: Michael Brudevold <michael.brudevold@logicpd.com>
>>
>> The CallRemoved signal was not being sent for existing calls when the voicecall
>> atom was unregistered. An example of this is the lack of a CallRemoved signal
>> when an HFP AG is disconnected due to SLC timeout while a call is active.
>> ---
>>  src/voicecall.c |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/voicecall.c b/src/voicecall.c
>> index 104b600..263f1ee 100644
>> --- a/src/voicecall.c
>> +++ b/src/voicecall.c
>> @@ -2699,8 +2699,11 @@ static void voicecall_unregister(struct ofono_atom *atom)
>>       if (vc->dial_req)
>>               dial_request_finish(vc);
>>
>> -     for (l = vc->call_list; l; l = l->next)
>> +     for (l = vc->call_list; l; l = l->next) {
>> +             voicecalls_emit_call_removed(vc, l->data);
>> +
>>               voicecall_dbus_unregister(vc, l->data);
>> +     }
>>
>
> In some respect this is on purpose, the application should listen to the
> PropertyChanged on the main Modem object and keep track of Interfaces
> property.  e.g. if the VoiceCallManager goes away the application can
> pretty much assume that all calls are gone.
>
> I could use some more convincing that this is really needed...

The only reason I added it is 4 hours of debugging my GUI and ofono to
figure out why the GUI didn't notice the D-Bus path for the voice call
was no longer around.  The documentation for CallRemoved says "Signal
that is sent when a voice call has been released."  This implies it
should be sent every time it is released.  If you update the
documentation to make it clear that this is not always the case, then
it's fine by me.

Mike

>
> Regards,
> -Denis
>
>>       g_slist_free(vc->call_list);
>>       vc->call_list = NULL;
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] voicecall: send CallRemoved for calls when voicecall atom is unregistered
  2012-04-19 14:02     ` Denis Kenzior
@ 2012-04-20 13:36       ` Mike
  0 siblings, 0 replies; 5+ messages in thread
From: Mike @ 2012-04-20 13:36 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

On Thu, Apr 19, 2012 at 9:02 AM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Mike,
>
>>
>> The only reason I added it is 4 hours of debugging my GUI and ofono to
>> figure out why the GUI didn't notice the D-Bus path for the voice call
>> was no longer around.  The documentation for CallRemoved says "Signal
>> that is sent when a voice call has been released."  This implies it
>> should be sent every time it is released.  If you update the
>> documentation to make it clear that this is not always the case, then
>> it's fine by me.
>>
>
> Please check that commit 7f51df4d satisfies your requirements.

Looks good, thanks!

Mike

>
> Regards,
> -Denis

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-04-20 13:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-18 22:02 [PATCH] voicecall: send CallRemoved for calls when voicecall atom is unregistered Mike Brudevold
2012-04-18 15:52 ` Denis Kenzior
2012-04-19 20:50   ` Mike
2012-04-19 14:02     ` Denis Kenzior
2012-04-20 13:36       ` Mike

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.