All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.