Re: [PATCH] voicecall: send CallRemoved for calls when voicecall atom is unregistered

[Mike <puffy.taco@gmail.com>]

[-- 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;
>