From: Frederic Danis <frederic.danis@linux.intel.com>
To: Mikel Astiz <mikel.astiz.oss@gmail.com>
Cc: linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH v15 10/14] audio: Move HFP HF server to telephony.c
Date: Fri, 27 Jul 2012 14:44:33 +0200 [thread overview]
Message-ID: <50128D31.7090102@linux.intel.com> (raw)
In-Reply-To: <CANT-zCUWdfDtKpjZFX+NFnaGt0Thp5C7uzpwvXU6qpMtZzOStw@mail.gmail.com>
Hello Mikel,
On 27/07/2012 10:50, Mikel Astiz wrote:
>> + const char *connecting_uuid;
>> + const char *connecting_path;
>
> I would suggest renaming this to something like
> connecting_transport_path or connecting_transport. Otherwise the code
> is sometimes confusing IMO.
>
> Furthermore, you're actually using gateway_set_media_transport_path()
> in the internal API.
I will rename it to connecting_transport_path.
>> @@ -141,8 +130,19 @@ static void change_state(struct audio_device *dev, gateway_state_t new_state)
>>
>> void gateway_set_state(struct audio_device *dev, gateway_state_t new_state)
>
> Perhaps not directly related to your patch but... why do we have this
> function anyway as opposed to change_state?
>
> If you compare to headset_set_state, it seems really inconsistent.
>
>> {
>> + struct gateway *gw = dev->gateway;
>> +
>> switch (new_state) {
>> case GATEWAY_STATE_DISCONNECTED:
>> + if (gw->msg) {
>> + DBusMessage *reply;
>> +
>> + reply = btd_error_failed(gw->msg, "Connect failed");
>> + g_dbus_send_message(dev->conn, reply);
>> + dbus_message_unref(gw->msg);
>> + gw->msg = NULL;
>> + }
>> +
>> gateway_close(dev);
>> break;
>
> <snip>
>
>> @@ -573,6 +419,11 @@ int gateway_close(struct audio_device *device)
>> gw->sco = NULL;
>> }
>>
>> + if (gw->tel_dev) {
>> + telephony_device_disconnect(gw->tel_dev);
>> + gw->tel_dev = NULL;
>> + }
>> +
>
> Don't we also need to clear connecting_uuid and specially connecting_path?
Yes, you're right, I should clear them.
Regards
Fred
--
Frederic Danis Open Source Technology Center
frederic.danis@intel.com Intel Corporation
next prev parent reply other threads:[~2012-07-27 12:44 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-26 8:45 [PATCH v15 00/14] Add org.bluez.Telephony interface Frédéric Danis
2012-07-26 8:45 ` [PATCH v15 01/14] doc: Add telephony interface documents Frédéric Danis
2012-07-27 7:52 ` Mikel Astiz
2012-07-27 8:30 ` Frederic Danis
2012-07-26 8:45 ` [PATCH v15 02/14] audio: Move telephony drivers to D-Bus interface Frédéric Danis
2012-07-27 8:25 ` Mikel Astiz
2012-07-27 9:33 ` Frederic Danis
2012-07-30 7:58 ` Johan Hedberg
2012-07-26 8:45 ` [PATCH v15 03/14] audio: Simplify org.bluez.Headset Frédéric Danis
2012-07-26 8:45 ` [PATCH v15 04/14] audio: Remove dummy telephony driver Frédéric Danis
2012-07-26 8:45 ` [PATCH v15 05/14] audio: Remove maemo5 " Frédéric Danis
2012-07-26 8:45 ` [PATCH v15 06/14] audio: Remove maemo6 " Frédéric Danis
2012-07-26 8:45 ` [PATCH v15 07/14] audio: Remove oFono " Frédéric Danis
2012-07-26 8:45 ` [PATCH v15 08/14] audio: Move HFP/HSP AG servers to telephony.c Frédéric Danis
2012-07-26 8:45 ` [PATCH v15 09/14] audio: Send transport path to telephony agent Frédéric Danis
2012-07-26 8:45 ` [PATCH v15 10/14] audio: Move HFP HF server to telephony.c Frédéric Danis
2012-07-27 8:50 ` Mikel Astiz
2012-07-27 12:44 ` Frederic Danis [this message]
2012-07-26 8:45 ` [PATCH v15 11/14] audio: Add DUN GW to org.bluez.Telephony Frédéric Danis
2012-07-26 8:45 ` [PATCH v15 12/14] audio: Add SAP " Frédéric Danis
2012-07-26 8:45 ` [PATCH v15 13/14] adapter: Add API to get fast connectable mode Frédéric Danis
2012-07-26 8:45 ` [PATCH v15 14/14] audio: Add fast connectable to telephony interface Frédéric Danis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50128D31.7090102@linux.intel.com \
--to=frederic.danis@linux.intel.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=mikel.astiz.oss@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).