Hi Denis,
Denis Kenzior wrote:
>> I'm picking up a really old thread here...
>Yes a really old thread ;)
Better late than never, right? :-)
...
>> However, I think we can solve this even simpler if we just
>> call hangup for any ALERTING/DIALING call..
>This is a real gray area. On some devices this will work, on others it
>might have un-intentional consequences. At least on the calypso,
>sending CHUP/ATH will terminate all calls not in the WAITING state.
Ok, so we should go forward with this proposal then.
I'll try to send a new RFC within the next couple of days.
My initial intention here was not to submit patches
on src/voicecall.c, but to make sure I understood your proposal properly.
Let me know how you think we should go forward with this, as this
impacts all drivers/*/voicecall.c
>> - if (call->status == CALL_STATUS_INCOMING) {
>> + if (vc->driver->hangup_active && call->status == CALL_STATUS_INCOMING)
>> {
>
>You're breaking the logic somewhat here. If the call is INCOMING, we
>shouldn't skip the rest of the block if hangup_active is not implemented.
>
>> if (vc->driver->hangup == NULL)
>> return __ofono_error_not_implemented(msg);
>>
>> vc->pending = dbus_message_ref(msg);
>> - vc->driver->hangup(vc, generic_callback, vc);
>> + vc->driver->hangup_active(vc, generic_callback, vc);
>>
>> return NULL;
>> }
>
>Here we need to check whether hangup_active or hangup_all are
>implemented. If both are, then prefer hangup_all. This would make it
>easier to keep compatibility with current drivers.
Did you have something like this in mind then:
if (call->status == CALL_STATUS_INCOMING) {
vc->pending = dbus_message_ref(msg);
if (vc->driver->hangup_all)
vc->driver->hangup_all(vc, generic_callback, vc);
else if (vc->driver->hangup_active)
vc->driver->hangup_active(vc, generic_callback, vc);
else
return __ofono_error_not_implemented(msg);
return NULL;
}
Should we do not_implemented here or did you intend the drivers to be allowed
to not implement hangup_active nor hangup_all, and fall through to
release_specific?
>
>> @@ -286,12 +286,12 @@ static DBusMessage *voicecall_hangup(DBusConnection
>> *conn,
>>
>> num_calls = g_slist_length(vc->call_list);
>>
>> - if (num_calls == 1 && vc->driver->hangup &&
>> + if (vc->driver->hangup_active && num_calls == 1 && vc->driver->hangup
>> &&
>
>This should probably be a condition something like:
>
>if (num_calls == 1 && (vc->driver->hangup || vc->driver->hangup_active)
>...
>
>> (call->status == CALL_STATUS_ACTIVE ||
>> call->status == CALL_STATUS_DIALING ||
>> call->status == CALL_STATUS_ALERTING)) {
>> vc->pending = dbus_message_ref(msg);
>> - vc->driver->hangup(vc, generic_callback, vc);
>> + vc->driver->hangup_active(vc, generic_callback, vc);
>
>And again prefer hangup_all over hangup_active to keep compatibility
>with old drivers.
Something like this then:
if (num_calls == 1 && (vc->driver->hangup_all || vc->driver->hangup_active) &&
(call->status == CALL_STATUS_ACTIVE ||
call->status == CALL_STATUS_DIALING ||
call->status == CALL_STATUS_ALERTING)) {
vc->pending = dbus_message_ref(msg);
if (vc->driver->hangup_all)
vc->driver->hangup_all(vc, generic_callback, vc);
else if (vc->driver->hangup_active)
vc->driver->hangup_active(vc, generic_callback, vc);
else
return __ofono_error_not_implemented(msg);
>This reminds me that we should treat INCOMING calls in HangupAll
>specially. It should not be handled here.
What did you have in mind?
>HangupAll should also skip waiting calls.
voicecalls_release_queue and voicecalls_release_next are used for
multiparty_hangup as well, I assume you want the same behaviour for multi-party
so that we can do these changes in voicecalls_release_queue, right?
Regards
Sjur
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
