linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


  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).