Branch for review: 'riccardo/voice'

[Dan Williams]

On Sat, 2015-05-23 at 18:31 +0200, Aleksander Morgado wrote:
> Hey Dan & others,
> 
> Riccardo & Marco developed an initial draft of the AT-based voice
> support, including API, libmm-glib, mmcli, generic modem and
> Huawei-specific implementations; his work is in the "riccardo/voice"
> branch in upstream git:
> 
> http://cgit.freedesktop.org/ModemManager/ModemManager/log/?h=riccardo/voice
> 
> I've pushed some additional fixes on top of their work, mostly
> leftovers or minor coding style fixes.
> 
> Riccardo & Marco; I also rewrote a bit the commit messages, not the
> contents. Also, please review my changes and tell me what you think.
> And sorry for the late reply...
> 
> Anyway, the branch looks good enough to me; there are some things that
> it doesn't yet cover, but I believe it's a good start. Dan, what do
> you think?

> libmm-glib: added support for Modem.Voice and Call interfaces

Should mm_call_properties_set_number() check that the number is not
zero-length?  I'm not sure how much validation we want to do in
mm-call-properties.c but maybe a bit more?

Also, mm_modem_voice_list_call() -> mm_modem_voice_list_calls() (since
it should be plural).  The D-Bus API is also "ListCalls" so we should
keep it consistent.  Same for the _sync() variant, and for
ListCallContext.

Also a couple occurrences of some odd parentheses:

dictionary = (mm_call_properties_get_dictionary (properties));

Also some dead code in libmm-glib/mm-modem-voice.c that's been commented
out with //.  Actually, finalize() can just get removed from this file.

> mmcli: added Modem.Voice and Call support

Could use more g_clear_object() in various places, but I'll bet this is
a C&P from before glib-2.28 or something.  Anyway, something to keep in
mind for new code.

Also, it should be "--voice-list-calls" too like above in libmm-glib.

> broadband-modem: added voice call support

I'm not sure just checking for ATH is a great indicator of voice
support; most devices I know of will support ATH, but almost none
actually support voice.  Possibly we need plugin-specific checks
instead?  For example I don't think most of the Huawei devices support
voice unless they return ^CVOICE: 1, but they will return OK from ATH.

Should also remove the C&P dtmf_received() implementation that's
commented out.

>  build: added voice call include in libmm-glib and modified
Makefile.am

This should probably get squashed with previous commits.

> api,voice: added DTMF interface specification

How about "SendDtmf" and "DtmfReceived" instead?  Seems clearer to me.

Dan