Re: Handset voice dial signalling patch

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi,

On Thu, Jan 14, 2010, Peter Zotov wrote:
> Johan Hedberg wrote:
> >>+	gboolean voice_dial;
> >>+	gboolean voice_dial_req;
> >
> >I'm not so sure that it'll be useful to have this state info in
> >headset.c. I suspect for the most part voice recognition will be a
> >platform specific issue that can be taken care of by the telephony
> >driver. So I'd leave out these new variables for now and add them later
> >if it turns out that there's actually some use for them in headset.c.
> >
> >After this change I think the patch could be pushed upstream.
> Fixed.

Thanks, however there are still a couple of things that I noticed (sorry
for not spotting them earlier). Once the issues in the code are fixed,
please create a patch that's appliable using git am, i.e. commit to your
own tree (with an appropriate commit message) and prepare a patch file
with git format-patch.

> +static int voice_dial(struct audio_device *device, const char *buf)
> +{
> +	if (strlen(buf) < 9)
> +		return -EINVAL;
> +
> +	gboolean start_dialing;

No variable declarations after code in the same scope. Please check your
patch with the configure script options used by bootstrap-configure and
you'll get something like the following with your patch:

audio/headset.c: In function ‘voice_dial’:
audio/headset.c:1145: error: ISO C90 forbids mixed declarations and code

> +void telephony_voice_dial_req(void *telephony_device, gboolean enable)
> +{
> +	debug("telephony-dummy: got %s voice dial request",
> +			enable ? "enable" : "disable");
> +
> +	g_dbus_emit_signal(connection, "/org/bluez/test",
> +			"org.bluez.TelephonyTest", "VoiceDial",
> +			DBUS_TYPE_INVALID);
> +
> +	telephony_voice_dial_rsp(telephony_device, CME_ERROR_NOT_SUPPORTED);
> +}

Why return ERROR_NOT_SUPPORTED instead of ERROR_NONE here? After all,
you did successfully process the command (i.e. send the signal).

Johan