API Call proposal

[Dan Williams]

On Wed, 2015-04-22 at 17:36 +0200, Riccardo Vangelisti wrote:
> Hi all,
> 
> I attach'd a new patch and schema with your latest suggestions.
> 
> Note that Voice.StateChanged signal has "reason" argument but, by the 
> way, I had to create the MMCallStateChangedReason property in order to 
> permits user to retrieve that information also from the call object.
> This property is the same that indicates why and how a call terminates. 
> So, I've created an enum for each state trasition.

I would just call it MMCallStateReason.

> Also, I've removed Audio property as Aleksander suggested.

Yeah, we can add it later when we actually know what we need here.

More comments below...

> If you doesn't have any other suggestions, my colleague and I start to 
> develop in our branch: 
> https://bitbucket.org/riccardovangelisti/modemmanager/branch/voicecall
> 
> 
> Riccardo Vangelisti - Sadel SpA
> Software Development
> Via Serenari 1, Castel Maggiore (BO)
> 
> Il 22/04/2015 11:23, Aleksander Morgado ha scritto:
> > Hey Dan & Riccardo,
> >
> > Some comments from my side.
> >
> >> For states:
> >>
> >> + * MM_CALL_STATE_UNKNOWN: default state for a new outgoing call
> >>
> >> Should we have a MM_CALL_STATE_DIALING in between here for outgoing
> >> calls?  The call isn't really RINGING until the dialing request is
> >> complete, and that can sometimes take 10 seconds or more while network
> >> capacity is being allocated and the circuit set up, especially in busy
> >> cells.
> >>
> > Is this worth? I think it could be cleaner to have DIALING state
> > applicable only to ongoing calls (i.e. you ATD... and you wait for the
> > OK/NOCARRIER/BUSY/NODIALTONE response), and then RINGING only for
> > incoming calls (i.e. you're receiving a new call and the device should
> > ring).

Discussed this with Aleksander on IRC and came to an agreement; we'd
like to see a DIALING state (which would only be relevant for outgoing
calls), and then RINGING_IN and RINGING_OUT states.  The RINGING_IN
state would be when a call is waiting to be answered locally, and _OUT
would be when it is ringing on the other end, *if the modem supports
notifying that*, which would be important to note in the documentation.

> >> I think we also need STATE_HELD to indicate when the call is on hold
> >> (see +CHLD docs), and STATE_WAITING (+CCWA).
> >>
> >> Ok, I ignored these command! I'll add these states in the next patch.
> >>
> > I actually would only add these whenever we support holding/waiting the call...

We agreed these should be added now, otherwise the state enum will be
out-of-order.  We know we'll want them eventually, even if they don't
get implemented quite yet, so we might as well add them now.

> >> The "Call.Audio" property concerns me a bit, I don't think it provides
> >> enough detail.  What is /dev/ttyUSB3 supposed to be as the "Audio"
> >> value?  I know on the one Huawei that I have that supports voice, the
> >> serial device just spits out and accepts 16-bit PCM frames, but how
> >> would that be indicated here?  I don't think apps should require a
> >> parallel database of modem voice details.  Also, what does "analog"
> >> actually mean?  Where does the audio come from or go to?  I think this
> >> part needs more specification and possibly more detail in the API.
> >>
> >> I will reply to this as soon as possible. In the meanwhile, what are your
> >> suggestions?
> >> Note that in my case the modem provides an analog output that must not be
> >> handled by software.
> >>
> > I would try to avoid guessing; and therefore I would just target for
> > now the specific device that is going to be supported first. If this
> > device for now doesn't need anything software-wise (i.e. the audio
> > circuit is totally independent), then either we add a property showing
> > a value which says "external" or the like, or even, we completely
> > ignore this and we don't even add a property. Whenever new
> > implementations that require other info are implemented, then we can
> > add new properties. What do you guys think? I'm just trying to keep it
> > simple.

Aleksander and I discussed the possibilities here and decided that a
dict ("a{sv}") would be best, but that can be added later.

> >> On to Voice...
> >>
> >> There are also some meta operations that can be done, like call
> >> transfer, join, multiparty, etc.  Ofono created a VoiceCallManager API
> >> for that purpose, which might be useful to review for .Voice API
> >> additions.
> >>
> >> It's true, but maybe these methods could be added later.
> >> My goal is to start a call as soon as possible...
> >>
> > I also agree with this. We should write a minimal API for now, not
> > targeting transfer/join/multiparty...

Agreed.

> > But looking at Ofono's API is also something that would be valuable.
> > Riccardo, could you take a look at this API and compare it with the
> > proposal we're discussing to see if there are inconsistencies?
> > Android's API would also be interesting to check.
> >
> >> DeleteCall needs more explanation; obviously it will terminate (hang up,
> >> disconnect) the call and release any resources.  Basically it's just
> >> like Hangup() right?
> >>
> >> I thought to use "DeleteCall" in order to remove a terminated call from the
> >> Voice list property.
> >> Obviously if the call is active, this method should also terminate the call.
> >>
> > Yeah, so the thing here is that we'd be leaving in the list of calls
> > managed by the Voice API all the call objects that have been generated
> > in this session, and this includes calls that have been terminated
> > with a HangUp. So, to let the users of the API clear the list or
> > specific objects from DBus, the DeleteCall() method would be available
> > (same as we do e.g. in Messaging API with non-stored sent SMS
> > messages).

Ok, I didn't get that the calls would stick around after they ended,
until explicitly deleted.  But also, should this really work the same
way as SMS?  I feel like calls are much more transient, and anything
that wanted a "call log" would be listening to the D-Bus interface
already and writing this stuff down.  I just worry a bit that we'll be
left with a lot of calls in the list and that something will forget to
clean them up.

But OK.

> >> For CallAdded, instead of a boolean, how about just re-using
> >> MM_CALL_DIRECTION?  Also, is the direction argument that interesting
> >> here?  eg, what would a client do when it receives the CallAdded signal
> >> + direction, if it *doesn't* retrieve the call object properties?
> >> Possibly the argument could be removed if we expect clients to get all
> >> the properties, since eventually we'll have CLIP, phone number,
> >> multiparty, emergency indicator, etc.
> >>
> >> Direction is more correct than the bool value, but I prefer to remove the
> >> property and force user to read the call object properties.
> >>
> > Yes, the signal should only expose the path of the new call object
> > being exposed in DBus. Clients should load the properties they are
> > interested in, and that would include the direction.

Agreed.

Looking better, thanks!

Dan