Hi Denis,

sorry for responding late.

On 14 October 2010 10:47, Denis Kenzior <denk...@gmail.com> wrote:
> On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
>> This provides a way for other atoms to send DTMF tones during a call.
>> ---
>>  src/ofono.h     |    4 ++++
>>  src/voicecall.c |   16 ++++++++++++++++
>>  2 files changed, 20 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/ofono.h b/src/ofono.h
>> index 6c7f649..6efd9ac 100644
>> --- a/src/ofono.h
>> +++ b/src/ofono.h
>> @@ -218,6 +218,10 @@ int __ofono_voicecall_dial(struct ofono_voicecall *vc,
>>                               ofono_voicecall_dial_cb_t cb, void *user_data);
>>  void __ofono_voicecall_dial_cancel(struct ofono_voicecall *vc);
>>
>> +int __ofono_voicecall_send_tone(struct ofono_voicecall *vc,
>> +                             const char *tone_str,
>> +                             ofono_voicecall_cb_t cb, void *user_data);
>> +
>
> So this one is structured according to __ofono_voicecall_dial, should we
> also have __ofono_voicecall_send_tone_cancel?
>
>>  #include <ofono/sms.h>
>>
>>  struct sms;
>> diff --git a/src/voicecall.c b/src/voicecall.c
>> index 7b5fe3b..45e19ce 100644
>> --- a/src/voicecall.c
>> +++ b/src/voicecall.c
>> @@ -2326,3 +2326,19 @@ void __ofono_voicecall_dial_cancel(struct 
>> ofono_voicecall *vc)
>>
>>       vc->dial_req->cb = NULL;
>>  }
>> +
>> +int __ofono_voicecall_send_tone(struct ofono_voicecall *vc,
>> +                             const char *tone_str,
>> +                             ofono_voicecall_cb_t cb, void *user_data)
>> +{
>> +     if (!vc->driver->send_tones)
>> +             return -ENOSYS;
>> +
>> +     /* Send DTMFs only if we have at least one connected call */
>> +     if (!voicecalls_can_dtmf(vc))
>> +             return -ENOENT;
>
> I'm a bit worried that we never check for BUSY conditions here, in
> particular since we can get into this situation:
>
> Send DTMF
> Cancel
> Send DTMF
>
> Which results in two DTMF operations being outstanding.
>
>> +
>> +     vc->driver->send_tones(vc, tone_str, cb, user_data);
>> +
>> +     return 0;
>> +}
>
> We also don't busy out any D-Bus operations when Send DTMF is in progress...

So I gave this some thought now and made an implementation based on
the __ofono_voicecall_dial api, but I think that this old approach is
actually working better for us.  There are a couple of scenarios
(let's consider only atmodem first)

1. Send DTMF - Cancel - Send DTMF

in this case the second Send DTMF is going to return an error from the
driver (not from core voicecall.c).  I don't think it's a big deal,
but we could think of something like cancelling the driver .send_tones
call, because the operation can potentially take a while -- this would
need a little change in the driver api.

2. Send DTMF - DBus command arrives,

Now any DBus command (other than SendTone) will actually be sent to
the driver and executed.  I think that's the correct thing to do --
the user needs to be able to end the call at any time.  The tones
being emitted should not actually interfere with anything.  The only
thing we need to do is respond to the STK command with an error if the
call is terminated before all tones finished being emitted.

3. Modem is doing something and a Send DTMF stk command arrives,

In this case the DTMFs will be delayed because of command
serialisation in gatchat / gisi.  Should we return "terminal busy" to
the card instead?

What do you think?  What behaviour do we want?

The handling may be different in case of isimodem, so this would need
checking anyway.

Best regards
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

Reply via email to