Re: [PATCH] voicecall: fix release and answer existing call validation

Re: [PATCH] voicecall: fix release and answer existing call validation

[Denis Kenzior]

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

Hi Michael,

On 04/19/2012 05:10 PM, Mike Brudevold wrote:
> From: Michael Brudevold <michael.brudevold@logicpd.com>
> 
> Release and answer should be allowed when there are held calls as well as the
> existing check for waiting calls.  Most implementations use AT+CHLD=1 which
> will resume the other call, be it waiting or held.  This follows the HFP,
> GSM 07.07 and 3GPP 27.007 specs.
> ---
>  src/voicecall.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 

This isn't really a bug, but an explicit choice we made with the API.
See the previous discussion here:
http://lists.ofono.org/pipermail/ofono/2011-January/008029.html.

For HFP this is further made funny by the fact that CHLD=1X is supposed
to retrieve the held call automatically for you by the AG according to
section 4.32.1, making CHLD=1 behavior in question somewhat redundant.

However, I do see that this could be useful, so I suggest we add a new
ReleaseAndSwap method.  I prefer this approach especially since the
current D-Bus API is frozen and I don't want to modify the behavior of
existing methods.

Regards,
-Denis

[PATCH] voicecall: fix release and answer existing call validation

[Mike Brudevold]

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

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

Release and answer should be allowed when there are held calls as well as the
existing check for waiting calls.  Most implementations use AT+CHLD=1 which
will resume the other call, be it waiting or held.  This follows the HFP,
GSM 07.07 and 3GPP 27.007 specs.
---
 src/voicecall.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index 263f1ee..2d261bf 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -1684,7 +1684,7 @@ static DBusMessage *manager_release_and_answer(DBusConnection *conn,
 	if (vc->pending || vc->dial_req || vc->pending_em)
 		return __ofono_error_busy(msg);
 
-	if (!voicecalls_have_waiting(vc))
+	if (!(voicecalls_have_waiting(vc) || voicecalls_have_held(vc)))
 		return __ofono_error_failed(msg);
 
 	if (vc->driver->release_all_active == NULL)
-- 
1.7.5.4

Re: [PATCH] voicecall: fix release and answer existing call validation

[Mike]

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

Hi Denis,

On Thu, Apr 19, 2012 at 8:42 AM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Michael,
>
> On 04/19/2012 05:10 PM, Mike Brudevold wrote:
>> From: Michael Brudevold <michael.brudevold@logicpd.com>
>>
>> Release and answer should be allowed when there are held calls as well as the
>> existing check for waiting calls.  Most implementations use AT+CHLD=1 which
>> will resume the other call, be it waiting or held.  This follows the HFP,
>> GSM 07.07 and 3GPP 27.007 specs.
>> ---
>>  src/voicecall.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>
> This isn't really a bug, but an explicit choice we made with the API.
> See the previous discussion here:
> http://lists.ofono.org/pipermail/ofono/2011-January/008029.html.
>
> For HFP this is further made funny by the fact that CHLD=1X is supposed
> to retrieve the held call automatically for you by the AG according to
> section 4.32.1, making CHLD=1 behavior in question somewhat redundant.
>
> However, I do see that this could be useful, so I suggest we add a new
> ReleaseAndSwap method.  I prefer this approach especially since the
> current D-Bus API is frozen and I don't want to modify the behavior of
> existing methods.
>
> Regards,
> -Denis

Sounds good to me. Clearly not having the ability to do this is
annoying to more than just me!  My first solution was as you proposed
before where the active call was hungup, but that left the held call
on hold.  Not having read the spec fully yet left me a bit confused as
to why I would want that behavior.  So, the preferred solution is
getting this all done in one step, and ReleaseAndSwap should do
nicely.

Mike