Linux bluetooth development
 help / color / mirror / Atom feed
* Re: Query regarding creation of multiple MAS Instance ID:
From: Luiz Augusto von Dentz @ 2014-10-20 13:53 UTC (permalink / raw)
  To: Gowtham Anandha Babu
  Cc: linux-bluetooth@vger.kernel.org, Bharat Panda, Dmitry Kasatkin,
	cpgs
In-Reply-To: <000301cfec65$bdb08cf0$3911a6d0$@samsung.com>

Hi,

On Mon, Oct 20, 2014 at 3:59 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Hi Luiz,
>
>> -----Original Message-----
>> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
>> Sent: Friday, October 17, 2014 5:11 PM
>> To: Gowtham Anandha Babu
>> Cc: linux-bluetooth@vger.kernel.org; Bharat Panda; Dmitry Kasatkin;
>> cpgs@samsung.com
>> Subject: Re: Query regarding creation of multiple MAS Instance ID:
>>
>> Hi,
>>
>> On Thu, Oct 16, 2014 at 4:18 PM, Gowtham Anandha Babu
>> <gowtham.ab@samsung.com> wrote:
>> > Hi All,
>> >
>> > I am working on obexd MAP profile. Is it possible to create multiple
>> > MAS Instance in bluez?
>> > Because right now bluez has only one MAS Instance support (only one
>> > MAS_UUID), which is advertised to MCE based on the XML format
>> followed
>> > in src/profile.c.
>> > When I add one more record to MAS_RECORD XML format by changing the
>> > value as 0x01, I am not able see two instances getting loaded.
>> >                 <attribute id=\"0x0315\">                               \
>> >                         <uint8 value=\"0x01\"/>                 \
>> >                 </attribute>                                            \
>>
>> How you are doing that? You should be able to call
>> ProfileManager1.RegisterProfile if that is what you doing and it is not working
>> perhaps we have a bug somewhere. Two very important things you need to
>> check:
>>
>> 1. It cannot use the same path for both instances 2. Each instance needs a
>> different channel
>>
>> So if you are register via XML you need to take care of those details, but
>> perhaps you are using the default record by just providing the UUID, that
>> probably will not work because apparently we have hardcoded the channel,
>> if that is the case we should probably add a check if there is another instance
>> already register and use a different channel/psm.
>>
>> > Do we need to create one more UUID like OBEX_MAS1_UUID and
>> > OBEX_MAS2_UUID to support multiple mas instances?
>> > Or what could be the possible solutions?
>>
>> Each instance has to have the service class set to MAS UUID, otherwise no
>> one will be able to discover it.
>>
>> >
>> > Regards.
>> > Gowtham Anandha Babu
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe
>> > linux-bluetooth" in the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
> Here is what I did exactly after your comments,
>
> 1) Based on the flag, I managed the two MAS Instances in bluetooth.c
>
> diff --git a/obexd/plugins/bluetooth.c b/obexd/plugins/bluetooth.c
> index cf326cc..3a0c022 100644
> --- a/obexd/plugins/bluetooth.c
> +++ b/obexd/plugins/bluetooth.c
> @@ -50,6 +50,8 @@
>  #define BT_RX_MTU 32767
>  #define BT_TX_MTU 32767
>
> +gboolean MAS = FALSE;
> +
>  struct bluetooth_profile {
>         struct obex_server *server;
>         struct obex_service_driver *driver;
> @@ -269,7 +271,20 @@ static int register_profile(struct bluetooth_profile *profile)
>         char *xml;
>         int ret = 0;
>
> -       profile->path = g_strconcat("/org/bluez/obex/", profile->uuid, NULL);
> +
> +       if(!g_ascii_strcasecmp(profile->uuid, OBEX_MAS_UUID))
> +               if(!MAS) {
> +                       profile->path = g_strconcat("/org/bluez/obex/MAS1/",
> +                                                       profile->uuid, NULL);
> +                       MAS = TRUE;
> +               }
> +               else
> +                       profile->path = g_strconcat("/org/bluez/obex/MAS2/",
> +                                                       profile->uuid, NULL);
> +       else
> +               profile->path = g_strconcat("/org/bluez/obex/", profile->uuid, NULL);
> +
>         g_strdelimit(profile->path, "-", '_');
>
>         if (!g_dbus_register_interface(connection, profile->path,
> @@ -290,6 +305,10 @@ static int register_profile(struct bluetooth_profile *profile)
>
>         dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
>                                                         &profile->path);
>         dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,
>                                                         &profile->uuid);
>         dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> @@ -346,7 +365,9 @@ static const char *service2uuid(uint16_t service)
>                 return "00005005-0000-1000-8000-0002ee000001";
>         case OBEX_SYNCEVOLUTION:
>                 return "00000002-0000-1000-8000-0002ee000002";
> -       case OBEX_MAS:
> +       case OBEX_MAS1:
> +               return OBEX_MAS_UUID;
> +       case OBEX_MAS2:
>                 return OBEX_MAS_UUID;
>         case OBEX_MNS:
>                 return OBEX_MNS_UUID;
>
> 2) Created one more MAS Instance in mas.c
>
> diff --git a/obexd/plugins/mas.c b/obexd/plugins/mas.c
> index fb97fe3..fb482e2 100644
> --- a/obexd/plugins/mas.c
> +++ b/obexd/plugins/mas.c
> @@ -728,9 +728,9 @@ static int any_close(void *obj)
>         return 0;
>  }
>
> -static struct obex_service_driver mas = {
> -       .name = "Message Access server",
> -       .service = OBEX_MAS,
> +static struct obex_service_driver mas1 = {
> +       .name = "Message Access server 1",
> +       .service = OBEX_MAS1,
>         .target = MAS_TARGET,
>         .target_size = TARGET_SIZE,
>         .connect = mas_connect,
> @@ -740,6 +740,18 @@ static struct obex_service_driver mas = {
>         .disconnect = mas_disconnect,
>  };
>
> +static struct obex_service_driver mas2 = {
> +        .name = "Message Access server 2",
> +        .service = OBEX_MAS2,
> +        .target = MAS_TARGET,
> +        .target_size = TARGET_SIZE,
> +        .connect = mas_connect,
> +        .get = mas_get,
> +        .put = mas_put,
> +        .setpath = mas_setpath,
> +        .disconnect = mas_disconnect,
> +};
> +
>  static struct obex_mime_type_driver mime_map = {
>         .target = MAS_TARGET,
>         .target_size = TARGET_SIZE,
> @@ -838,10 +850,14 @@ static int mas_init(void)
>                         goto failed;
>         }
>
> -       err = obex_service_driver_register(&mas);
> +       err = obex_service_driver_register(&mas1);
>         if (err < 0)
>                 goto failed;
>
> +       err = obex_service_driver_register(&mas2);
> +       if(err < 0)
> +               goto failed;
> +
>         return 0;
>
>  failed:
> @@ -857,7 +873,8 @@ static void mas_exit(void)
>  {
>         int i;
>
> -       obex_service_driver_unregister(&mas);
> +       obex_service_driver_unregister(&mas1);
> +               obex_service_driver_unregister(&mas2);
>
>         for (i = 0; map_drivers[i] != NULL; ++i)
>                 obex_mime_type_driver_unregister(map_drivers[i]);
> @@ -865,4 +882,5 @@ static void mas_exit(void)
>         messages_exit();
>  }
>
> -OBEX_PLUGIN_DEFINE(mas, mas_init, mas_exit)
> +OBEX_PLUGIN_DEFINE(mas1, mas_init, mas_exit)
> +OBEX_PLUGIN_DEFINE(mas2, mas_init, mas_exit)
> diff --git a/obexd/src/obexd.h b/obexd/src/obexd.h
> index 42c3c4d..59de70e 100644
> --- a/obexd/src/obexd.h
> +++ b/obexd/src/obexd.h
> @@ -28,7 +28,8 @@
>  #define OBEX_IRMC      (1 << 5)
>  #define OBEX_PCSUITE   (1 << 6)
>  #define OBEX_SYNCEVOLUTION     (1 << 7)
> -#define OBEX_MAS       (1 << 8)
> +#define OBEX_MAS1      (1 << 8)
> +#define OBEX_MAS2      (1 << 10)
>  #define OBEX_MNS       (1 << 9)
>
>  gboolean plugin_init(const char *pattern, const char *exclude);
>
> 3) Assigned different channel and path for both instances
> 4)Added one more SDP record (XML) in profile.c
>
> diff --git a/src/profile.c b/src/profile.c
> index 7aca3be..ab5b5c2 100644
> --- a/src/profile.c
> +++ b/src/profile.c
> @@ -62,7 +62,8 @@
>  #define HFP_AG_DEFAULT_CHANNEL 13
>  #define SYNC_DEFAULT_CHANNEL   14
>  #define PBAP_DEFAULT_CHANNEL   15
> -#define MAS_DEFAULT_CHANNEL    16
> +#define MAS1_DEFAULT_CHANNEL   16
> +#define MAS2_DEFAULT_CHANNEL   18
>  #define MNS_DEFAULT_CHANNEL    17
>
>  #define BTD_PROFILE_PSM_AUTO   -1
> @@ -395,7 +396,7 @@
>                 </attribute>                                            \
>         </record>"
>
> -#define MAS_RECORD                                                     \
> +#define MAS_RECORD1                                                    \
>         "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>                    \
>         <record>                                                        \
>                 <attribute id=\"0x0001\">                               \
> @@ -441,6 +442,52 @@
>                 </attribute>                                            \
>         </record>"
>
> +#define MAS_RECORD2                                                    \
> +       "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>                    \
> +       <record>                                                        \
> +               <attribute id=\"0x0001\">                               \
> +                       <sequence>                                      \
> +                               <uuid value=\"0x1132\"/>                \
> +                       </sequence>                                     \
> +               </attribute>                                            \
> +               <attribute id=\"0x0004\">                               \
> +                       <sequence>                                      \
> +                               <sequence>                              \
> +                                       <uuid value=\"0x0100\"/>        \
> +                               </sequence>                             \
> +                               <sequence>                              \
> +                                       <uuid value=\"0x0003\"/>        \
> +                                       <uint8 value=\"0x%02x\"/>       \
> +                               </sequence>                             \
> +                               <sequence>                              \
> +                                       <uuid value=\"0x0008\"/>        \
> +                               </sequence>                             \
> +                       </sequence>                                     \
> +               </attribute>                                            \
> +               <attribute id=\"0x0005\">                               \
> +                       <sequence>                                      \
> +                               <uuid value=\"0x1002\" />               \
> +                       </sequence>                                     \
> +               </attribute>                                            \
> +               <attribute id=\"0x0009\">                               \
> +                       <sequence>                                      \
> +                               <sequence>                              \
> +                                       <uuid value=\"0x1134\"/>        \
> +                                       <uint16 value=\"0x%04x\" />     \
> +                               </sequence>                             \
> +                       </sequence>                                     \
> +               </attribute>                                            \
> +               <attribute id=\"0x0100\">                               \
> +                       <text value=\"%s\"/>                            \
> +               </attribute>                                            \
> +               <attribute id=\"0x0315\">                               \
> +                       <uint8 value=\"0x01\"/>                         \
> +               </attribute>                                            \
> +               <attribute id=\"0x0316\">                               \
> +                       <uint8 value=\"0x0F\"/>                         \
> +               </attribute>                                            \
> +       </record>"
> +
>  #define MNS_RECORD                                                     \
>         "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>                    \
>         <record>                                                        \
> @@ -562,6 +609,8 @@
>
>  struct ext_io;
>
> +gboolean MAS = FALSE;
> +
>  struct ext_profile {
>         struct btd_profile p;
>
> @@ -1734,10 +1787,17 @@ static char *get_pse_record(struct ext_profile *ext, struct ext_io *l2cap,
>                                                                 ext->name);
>  }
>
> -static char *get_mas_record(struct ext_profile *ext, struct ext_io *l2cap,
> +static char *get_mas_record1(struct ext_profile *ext, struct ext_io *l2cap,
> +                                                       struct ext_io *rfcomm)
> +{
> +       return g_strdup_printf(MAS_RECORD1, rfcomm->chan, ext->version,
> +                                                               ext->name);
> +}
> +
> +static char *get_mas_record2(struct ext_profile *ext, struct ext_io *l2cap,
>                                                         struct ext_io *rfcomm)
>  {
> -       return g_strdup_printf(MAS_RECORD, rfcomm->chan, ext->version,
> +       return g_strdup_printf(MAS_RECORD2, rfcomm->chan, ext->version,
>                                                                 ext->name);
>  }
>
> @@ -1949,10 +2009,17 @@ static struct default_settings {
>                 .version        = 0x0101,
>         }, {
>                 .uuid           = OBEX_MAS_UUID,
> -               .name           = "Message Access",
> -               .channel        = MAS_DEFAULT_CHANNEL,
> +               .name           = "Message Access 1",
> +               .channel        = MAS1_DEFAULT_CHANNEL,
> +               .authorize      = true,
> +               .get_record     = get_mas_record1,
> +               .version        = 0x0100
> +       }, {
> +               .uuid           = OBEX_MAS_UUID,
> +               .name           = "Message Access 2",
> +               .channel        = MAS2_DEFAULT_CHANNEL,
>                 .authorize      = true,
> -               .get_record     = get_mas_record,
> +               .get_record     = get_mas_record2,
>                 .version        = 0x0100
>         }, {
>                 .uuid           = OBEX_MNS_UUID,
> @@ -1981,8 +2048,21 @@ static void ext_set_defaults(struct ext_profile *ext)
>                 struct default_settings *settings = &defaults[i];
>                 const char *remote_uuid;
>
> +               DBG("%s ==  %s",ext->uuid, settings->uuid);
> +
>                 if (strcasecmp(ext->uuid, settings->uuid) != 0)
>                         continue;
> +
> +               if(!strcasecmp(ext->uuid, OBEX_MAS_UUID))
> +                       if(MAS) {
> +                               MAS = 0;
> +                               continue;
> +                       }
> +
> +               if(!strcasecmp(ext->uuid, OBEX_MAS_UUID))
> +                       MAS = TRUE;
> +
> +               DBG("MATCHED  %s ==  %s MAS = %d",ext->uuid, settings->uuid, MAS);
>
>                 if (settings->remote_uuid)
>                         remote_uuid = settings->remote_uuid;
> @@ -2022,6 +2102,8 @@ static void ext_set_defaults(struct ext_profile *ext)
>
>                 if (settings->name)
>                         ext->name = g_strdup(settings->name);
> +
> +               return;
>         }
>  }
>
>
> After this changes I am able to load the two MAS Instances successfully. Is it fine? What do you think?

Well the main problem then becomes how to decide how many instance it
should register and what each instance should be, in the past I
comment that I would like to see the backends evolving into D-Bus
agent/entities that register at runtime perhaps we should looking into
that direction so we don't have to hardcode the records.

-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCH] android/pts: Update PTS files for HOGP
From: Sebastian Chlad @ 2014-10-20 13:52 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sebastian Chlad

PICS and PIXITs updated to PTS 5.3. Regression done for Android
4.4.4.
---
 android/pics-hogp.txt  |  2 +-
 android/pixit-hogp.txt |  2 +-
 android/pts-hogp.txt   | 14 ++++++++------
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/android/pics-hogp.txt b/android/pics-hogp.txt
index d7192bf..61cf56a 100644
--- a/android/pics-hogp.txt
+++ b/android/pics-hogp.txt
@@ -1,6 +1,6 @@
 HOGP PICS for the PTS tool.
 
-PTS version: 5.2
+PTS version: 5.3
 
 * - different than PTS defaults
 # - not yet implemented/supported
diff --git a/android/pixit-hogp.txt b/android/pixit-hogp.txt
index 067c280..d32fe69 100644
--- a/android/pixit-hogp.txt
+++ b/android/pixit-hogp.txt
@@ -1,6 +1,6 @@
 HOGP PIXIT for the PTS tool.
 
-PTS version: 5.2
+PTS version: 5.3
 
 * - different than PTS defaults
 & - should be set to IUT Bluetooth address
diff --git a/android/pts-hogp.txt b/android/pts-hogp.txt
index 19292d1..d874c9c 100644
--- a/android/pts-hogp.txt
+++ b/android/pts-hogp.txt
@@ -1,7 +1,7 @@
 PTS test results for HoG
 
-PTS version: 5.2
-Tested: 22-July-2014
+PTS version: 5.3
+Tested: 20-October-2014
 Android version: 4.4.4
 
 Results:
@@ -54,19 +54,21 @@ TC_HGRF_BH_BV_15_I	N/A
 TC_HGRF_BH_BV_16_I	N/A
 TC_HGRF_BH_BV_17_I	N/A
 TC_HGRF_HH_BV_18_I	N/A
-TC_HGWF_RH_BV_01_I	PASS	haltest: hidhost connect <addr>
+				Note: JIRA bug #BA-175 affecting TC_HGWF_RH
+				section
+TC_HGWF_RH_BV_01_I	INC	haltest: hidhost connect <addr>
 				hidhost set_report <addr> BTHH_INPUT_REPORT
 					AAB3F8A6CD
 				hidhost disconnect <addr>
-TC_HGWF_RH_BV_02_I	PASS	haltest: hidhost connect <addr>
+TC_HGWF_RH_BV_02_I	INC	haltest: hidhost connect <addr>
 				hidhost set_report <addr> BTHH_OUTPUT_REPORT
 					EF907856341200
 				hidhost disconnect <addr>
-TC_HGWF_RH_BV_03_I	PASS	haltest: hidhost connect <addr>
+TC_HGWF_RH_BV_03_I	INC	haltest: hidhost connect <addr>
 				hidhost set_report <addr> BTHH_OUTPUT_REPORT
 					EF907856341200
 				hidhost disconnect <addr>
-TC_HGWF_RH_BV_04_I	PASS	haltest: hidhost connect <addr>
+TC_HGWF_RH_BV_04_I	INC	haltest: hidhost connect <addr>
 				hidhost set_report <addr> BTHH_FEATURE_REPORT
 					EA453F2D87
 				hidhost disconnect <addr>
-- 
1.8.5.3


^ permalink raw reply related

* Re: [PATCH ] audio/avrcp: Add Support for PLAYBACK_POS_CHANGED_EVENT
From: Luiz Augusto von Dentz @ 2014-10-20 13:46 UTC (permalink / raw)
  To: Bharat Bhusan Panda; +Cc: linux-bluetooth@vger.kernel.org, cpgs
In-Reply-To: <018701cfec67$7ad7bfa0$70873ee0$@samsung.com>

Hi,

On Mon, Oct 20, 2014 at 4:11 PM, Bharat Bhusan Panda
<bharat.panda@samsung.com> wrote:
> Hi Luiz,
>
>> > As per AVRCP 1.5 spec, 6.7.2, page 57.
>> >
>> > EVENT_PLAYBACK_POS_CHANGED shall be notified in the following
>> conditions:
>> >
>> >         1. TG has reached the registered playback Interval time.
>> >         2. Changed PLAY STATUS.
>> >         3. Changed Current Track.
>> >         4. Reached end or beginning of track.
>> > ---
>> >  profiles/audio/avrcp.c | 49
>> > +++++++++++++++++++++++++++++++++++++++++++++++++
>> >  profiles/audio/avrcp.h |  1 +
>> >  profiles/audio/media.c | 23 ++++++++++++++---------
>> >  3 files changed, 64 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c index
>> > 5c3c4f9..77f1dcb 100644
>> > --- a/profiles/audio/avrcp.c
>> > +++ b/profiles/audio/avrcp.c
>> > @@ -198,6 +198,7 @@ struct pending_list_items {  struct avrcp_player {
>> >         struct avrcp_server *server;
>> >         GSList *sessions;
>> > +       uint16_t pos_timer;
>> >         uint16_t id;
>> >         uint8_t scope;
>> >         uint64_t uid;
>> > @@ -627,6 +628,7 @@ void avrcp_player_event(struct avrcp_player
>> *player, uint8_t id,
>> >         uint8_t buf[AVRCP_HEADER_LENGTH + 9];
>> >         struct avrcp_header *pdu = (void *) buf;
>> >         uint16_t size;
>> > +       uint32_t *pos = NULL;
>> >         GSList *l;
>> >         int attr;
>> >         int val;
>> > @@ -673,6 +675,18 @@ void avrcp_player_event(struct avrcp_player
>> *player, uint8_t id,
>> >                 pdu->params[size++] = attr;
>> >                 pdu->params[size++] = val;
>> >                 break;
>> > +       case AVRCP_EVENT_PLAYBACK_POS_CHANGED:
>> > +               size = 5;
>> > +               pos = (uint32_t *) data;
>> > +
>> > +               be32toh(*pos);
>> > +               memcpy(&pdu->params[1], pos, sizeof(uint32_t));
>> > +
>> > +               if (player->pos_timer > 0) {
>> > +                       g_source_remove(player->pos_timer);
>> > +                       player->pos_timer = 0;
>> > +               }
>> > +               break;
>>
>> Note that it is meant to be playback interval, so if it is not playing we should
>> not send anything.
> Yes, will change it accordingly.
>>
>> >         default:
>> >                 error("Unknown event %u", id);
>> >                 return;
>> > @@ -1429,6 +1443,19 @@ static bool handle_passthrough(struct avctp
>> *conn, uint8_t op, bool pressed,
>> >         return handler->func(session);  }
>> >
>> > +static gboolean interval_timeout(gpointer user_data) {
>> > +       uint32_t pos;
>> > +       struct avrcp_player *mp = user_data;
>> > +
>> > +       pos = player_get_position(mp);
>> > +       mp->pos_timer = 0;
>> > +       avrcp_player_event(mp, AVRCP_EVENT_PLAYBACK_POS_CHANGED,
>> > +                                               &pos);
>> > +
>> > +       return FALSE;
>> > +}
>> > +
>> >  static uint8_t avrcp_handle_register_notification(struct avrcp *session,
>> >                                                 struct avrcp_header *pdu,
>> >                                                 uint8_t transaction)
>> > @@ -1436,6 +1463,8 @@ static uint8_t
>> avrcp_handle_register_notification(struct avrcp *session,
>> >         struct avrcp_player *player = target_get_player(session);
>> >         struct btd_device *dev = session->dev;
>> >         uint16_t len = ntohs(pdu->params_len);
>> > +       uint32_t interval;
>> > +       uint32_t pos;
>> >         uint64_t uid;
>> >         GList *settings;
>> >
>> > @@ -1467,6 +1496,20 @@ static uint8_t
>> avrcp_handle_register_notification(struct avrcp *session,
>> >         case AVRCP_EVENT_TRACK_REACHED_START:
>> >                 len = 1;
>> >                 break;
>> > +       case AVRCP_EVENT_PLAYBACK_POS_CHANGED:
>> > +               len = 5;
>> > +
>> > +               interval = htole32(pdu->params[1]);
>> > +               player->pos_timer = g_timeout_add_seconds(interval,
>> > +
>> > + interval_timeout, player);
>> > +
>> > +               pos = player_get_position(player);
>> > +               be32toh(pos);
>> > +               /* Fill the value for sending INTERIM response */
>> > +               memcpy(&pdu->params[1], &pos, sizeof(uint32_t));
>> > +
>> > +               break;
>> > +
>> >         case AVRCP_EVENT_SETTINGS_CHANGED:
>> >                 len = 1;
>> >                 settings = player_list_settings(player); @@ -2949,6
>> > +2992,11 @@ static void player_destroy(gpointer data)
>> >         if (player->destroy)
>> >                 player->destroy(player->user_data);
>> >
>> > +       if (player->pos_timer != 0) {
>> > +               g_source_remove(player->pos_timer);
>> > +               player->pos_timer = 0;
>> > +       }
>> > +
>> >         g_slist_free(player->sessions);
>> >         g_free(player->path);
>> >         g_free(player->change_path);
>> > @@ -3407,6 +3455,7 @@ static void target_init(struct avrcp *session)
>> >                                 (1 << AVRCP_EVENT_TRACK_CHANGED) |
>> >                                 (1 << AVRCP_EVENT_TRACK_REACHED_START) |
>> >                                 (1 << AVRCP_EVENT_TRACK_REACHED_END) |
>> > +                               (1 <<
>> > + AVRCP_EVENT_PLAYBACK_POS_CHANGED) |
>> >                                 (1 << AVRCP_EVENT_SETTINGS_CHANGED);
>> >
>> >         if (target->version < 0x0104)
>> > diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h index
>> > 6ac5294..4816e4b 100644
>> > --- a/profiles/audio/avrcp.h
>> > +++ b/profiles/audio/avrcp.h
>> > @@ -74,6 +74,7 @@
>> >  #define AVRCP_EVENT_TRACK_CHANGED              0x02
>> >  #define AVRCP_EVENT_TRACK_REACHED_END          0x03
>> >  #define AVRCP_EVENT_TRACK_REACHED_START                0x04
>> > +#define        AVRCP_EVENT_PLAYBACK_POS_CHANGED        0x05
>> >  #define AVRCP_EVENT_SETTINGS_CHANGED           0x08
>> >  #define AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED  0x0a
>> >  #define AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED   0x0b
>> > diff --git a/profiles/audio/media.c b/profiles/audio/media.c index
>> > ef7b910..cc19c61 100644
>> > --- a/profiles/audio/media.c
>> > +++ b/profiles/audio/media.c
>> > @@ -1305,6 +1305,9 @@ static gboolean set_status(struct media_player
>> *mp, DBusMessageIter *iter)
>> >         mp->status = g_strdup(value);
>> >
>> >         avrcp_player_event(mp->player, AVRCP_EVENT_STATUS_CHANGED,
>> > mp->status);
>> > +       avrcp_player_event(mp->player,
>> > +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED,
>> > +                                       &mp->position);
>> >
>> >         return TRUE;
>> >  }
>> > @@ -1312,7 +1315,6 @@ static gboolean set_status(struct media_player
>> > *mp, DBusMessageIter *iter)  static gboolean set_position(struct
>> > media_player *mp, DBusMessageIter *iter)  {
>> >         uint64_t value;
>> > -       const char *status;
>> >
>> >         if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_INT64)
>> >                         return FALSE;
>> > @@ -1321,11 +1323,6 @@ static gboolean set_position(struct
>> > media_player *mp, DBusMessageIter *iter)
>> >
>> >         value /= 1000;
>> >
>> > -       if (value > get_position(mp))
>> > -               status = "forward-seek";
>> > -       else
>> > -               status = "reverse-seek";
>> > -
>>
>> Not sure what is reasoning of removing these, the remote side may not
>> register for position change and in that case it would not get any seek event
>> to synchronize the position.
>
> Yes, PLAYBACK_STATUS_CHANGED and POSITION_CHANGED both should be sent in this case.
> If only PLAYBACK_STATUS_CHANGED is sent, the CT will query for position if it didn’t receive " POSITION_CHANGED " event after PLAYBACK_STATUS_CHANGED.

Exactly, and that means with POSITION_CHANGED you have more traffic
because it first send a changed which is normally followed by register
notification and interim response versus only get play status and
response.

>>
>> >         mp->position = value;
>> >         g_timer_start(mp->timer);
>> >
>> > @@ -1334,6 +1331,9 @@ static gboolean set_position(struct media_player
>> *mp, DBusMessageIter *iter)
>> >         if (!mp->position) {
>> >                 avrcp_player_event(mp->player,
>> >
>> > AVRCP_EVENT_TRACK_REACHED_START, NULL);
>> > +               avrcp_player_event(mp->player,
>> > +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED,
>> > +                                       &mp->position);
>> >                 return TRUE;
>> >         }
>> >
>> > @@ -1344,11 +1344,13 @@ static gboolean set_position(struct
>> media_player *mp, DBusMessageIter *iter)
>> >         if (mp->position == UINT32_MAX || mp->position >= mp->duration) {
>> >                 avrcp_player_event(mp->player,
>> AVRCP_EVENT_TRACK_REACHED_END,
>> >
>> > NULL);
>> > +               avrcp_player_event(mp->player,
>> > +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED,
>> > +                                       &mp->position);
>> >                 return TRUE;
>> >         }
>> > -
>> > -       /* Send a status change to force resync the position */
>> > -       avrcp_player_event(mp->player, AVRCP_EVENT_STATUS_CHANGED,
>> status);
>> > +       avrcp_player_event(mp->player,
>> AVRCP_EVENT_PLAYBACK_POS_CHANGED,
>> > +
>> > + &mp->position);
>> >
>> >         return TRUE;
>> >  }
>> > @@ -1456,6 +1458,7 @@ static gboolean parse_player_metadata(struct
>> media_player *mp,
>> >         int ctype;
>> >         gboolean title = FALSE;
>> >         uint64_t uid;
>> > +       uint32_t pos;
>> >
>> >         ctype = dbus_message_iter_get_arg_type(iter);
>> >         if (ctype != DBUS_TYPE_ARRAY)
>> > @@ -1521,9 +1524,11 @@ static gboolean parse_player_metadata(struct
>> media_player *mp,
>> >         mp->position = 0;
>> >         g_timer_start(mp->timer);
>> >         uid = get_uid(mp);
>> > +       pos = get_position(mp);
>> >
>> >         avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED,
>> &uid);
>> >         avrcp_player_event(mp->player,
>> > AVRCP_EVENT_TRACK_REACHED_START, NULL);
>> > +       avrcp_player_event(mp->player,
>> > + AVRCP_EVENT_PLAYBACK_POS_CHANGED, &pos);
>> >
>> >         return TRUE;
>> >  }
>> > --
>> > 1.9.1
>>
>> Overall EVENT_PLAYBACK_POS_CHANGED cause more problems than it
>> solves, because position is already part of the play status and that normally
>> has to be queried anyway, also self synchronizing is way more efficient since
>> we can't estimate the speed when the playback is fast-foward or rewind, and
>> even if we could that would generate a lot of traffic depending on what
>> interval the remote side wants to be updated.
>
> Playback position was not a part of play status though, it was trying resync the position by sending forward-seek or reverse seek as status changed event.
> In some control devices, it expects a playback_pos_changed event followed by TRACK_REACHED_START, TRACK_REACHED_END, PLAYBACK_STATUS_CHANGED and also when interval of registered for getting the playback position.
> Your input on the same will be highly valuable.

It is part of GetPlayStatus which also contains the duration, anyway
afaik position changed event is not mandatory, so the remote control
has to deal with stacks that do not have it which in the end leads to
synchronize via state changes by querying play status, if it cannot do
it you might have IOP problems with a lot stacks including iOS and
Android which does not support position changed.

-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH v4 2/2] core: Add subscription API for Manufacturer Specific Data
From: Johan Hedberg @ 2014-10-20 13:42 UTC (permalink / raw)
  To: Alfonso Acosta; +Cc: linux-bluetooth
In-Reply-To: <1413290765-20398-3-git-send-email-fons@spotify.com>

Hi Alfonso,

On Tue, Oct 14, 2014, Alfonso Acosta wrote:
> This patch allows plugins to be notified whenever an adapter receives
> Manufacturer Specific Data in the advertising reports from a device.
> 
> This can happen when new device is discovered or when we autoconnect
> to it.
> ---
>  src/adapter.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  src/adapter.h | 10 ++++++++++
>  2 files changed, 54 insertions(+)

This patch also doesn't compile (the error from the first patch might
have gone away, but then there's this new one):

src/adapter.c:4654:6: error: no previous declaration for ‘adapter_msd_notify’ [-Werror=missing-declarations]
 void adapter_msd_notify(struct btd_adapter *adapter, struct btd_device *dev,
      ^
cc1: all warnings being treated as errors
make[1]: *** [src/bluetoothd-adapter.o] Error 1


Johan

^ permalink raw reply

* Re: [PATCH v4 1/2] core: Add Manufacturer Specific Data EIR field
From: Johan Hedberg @ 2014-10-20 13:41 UTC (permalink / raw)
  To: Alfonso Acosta; +Cc: linux-bluetooth
In-Reply-To: <1413290765-20398-2-git-send-email-fons@spotify.com>

Hi Alfonso,

On Tue, Oct 14, 2014, Alfonso Acosta wrote:
> Add data structure and parsing support.
> ---
>  src/eir.c | 22 ++++++++++++++++++++++
>  src/eir.h |  8 ++++++++
>  2 files changed, 30 insertions(+)

This patch doesn't compile:

In file included from android/bluetooth.c:45:0:
./src/eir.h:53:15: error: ‘HCI_MAX_EIR_LENGTH’ undeclared here (not in a function)
  uint8_t data[HCI_MAX_EIR_LENGTH];
               ^
make[1]: *** [android/bluetooth.o] Error 1

Johan

^ permalink raw reply

* [PATCH 2/2] android/pts: Update PICS, PIXIT and test results for MPS
From: Marcin Kraglak @ 2014-10-20 13:33 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1413812032-26631-1-git-send-email-marcin.kraglak@tieto.com>

---
 android/pics-mps.txt  | 2 +-
 android/pixit-mps.txt | 2 +-
 android/pts-mps.txt   | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/android/pics-mps.txt b/android/pics-mps.txt
index 6fa9858..ab937a6 100644
--- a/android/pics-mps.txt
+++ b/android/pics-mps.txt
@@ -1,6 +1,6 @@
 MPS PICS for the PTS tool.
 
-PTS version: 5.2
+PTS version: 5.3
 
 * - different than PTS defaults
 # - not yet implemented/supported
diff --git a/android/pixit-mps.txt b/android/pixit-mps.txt
index 088efe9..56f5122 100644
--- a/android/pixit-mps.txt
+++ b/android/pixit-mps.txt
@@ -1,6 +1,6 @@
 MPS PIXIT for the PTS tool.
 
-PTS version: 5.2
+PTS version: 5.3
 
 * - different than PTS defaults
 & - should be set to IUT Bluetooth address
diff --git a/android/pts-mps.txt b/android/pts-mps.txt
index 2f661bb..46d2510 100644
--- a/android/pts-mps.txt
+++ b/android/pts-mps.txt
@@ -1,7 +1,7 @@
 PTS test results for MPS
 
-PTS version: 5.2
-Tested: 31.07.2014
+PTS version: 5.3
+Tested: 20.10.2014
 Android version: 4.4.4
 
 Results:
@@ -50,5 +50,5 @@ TC_SRC_TG_HFAV_CLH_MD_BV_04_I		PASS	Do not use AOSP Music player.
 						Use e.g. MortPlayer or Poweramp
 TC_SRC_TG_HFAV_CLH_MD_BV_05_I		PASS
 TC_SRC_TG_HFAV_CLH_MD_BV_06_I		PASS
-TC_PAIRING_HF_SNK_CT			PASS
+TC_PAIRING_HF_SNK_CT			N/A	Pairing helper for MD tests
 -------------------------------------------------------------------------------
-- 
1.9.3


^ permalink raw reply related

* [PATCH 1/2] android/bluetooth: Fix Supported feature bitmask for MPMD scenarios
From: Marcin Kraglak @ 2014-10-20 13:33 UTC (permalink / raw)
  To: linux-bluetooth

Set A2DP-SRC_AVRCP-TG instead of A2DP-SNK_AVRCP-CT_DUN-DT bit.
It was affecting TC_SDP_CTH_SD_BV_01_I PTS test case.
---
 android/bluetooth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 827e205..03eb1a1 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -70,7 +70,7 @@
  */
 #define MPS_DEFAULT_MPMD ((1ULL << 1) | (1ULL << 3) | (1ULL << 5) | \
 				(1ULL << 6) | (1ULL << 8) | (1ULL << 10) | \
-				(1ULL << 12) | (1ULL << 15) | (1ULL << 18))
+				(1ULL << 12) | (1ULL << 15) | (1ULL << 17))
 
 /*
  * bits in bitmask as defined in table 6.5 of Multi Profile Specification
-- 
1.9.3


^ permalink raw reply related

* Re: how to set PSCAN mode in .conf?
From: Marcel Holtmann @ 2014-10-20 13:27 UTC (permalink / raw)
  To: Jeff Chua; +Cc: linux-bluetooth
In-Reply-To: <CAAJw_ZuK6SUy22R133CezsqoU4+dTzxmsipBAp7rCvYBo+zrxw@mail.gmail.com>

Hi Jeff,

> latest bluez doesn't read hcid.conf, so how can I enable "pscan" in
> the conf file?
> 
> I know "hciconfig hci0 pscan" works, but I would still like to set
> that via the .conf file.

the mgmt interface has a connectable option and with BlueZ 5 that will be enabled by default for < 3.17 kernel. With >= 3.17 kernels the kernel automatically enables it based on our BR/EDR device list.

Regards

Marcel


^ permalink raw reply

* RE: [PATCH ] audio/avrcp: Add Support for PLAYBACK_POS_CHANGED_EVENT
From: Bharat Bhusan Panda @ 2014-10-20 13:11 UTC (permalink / raw)
  To: 'Luiz Augusto von Dentz'; +Cc: linux-bluetooth, cpgs
In-Reply-To: <CABBYNZLTMaJQ2hO_LoCgZQz-EwCQ9LuD9mDgK-w54XDc3wNCvw@mail.gmail.com>

Hi Luiz,

> > As per AVRCP 1.5 spec, 6.7.2, page 57.
> >
> > EVENT_PLAYBACK_POS_CHANGED shall be notified in the following
> conditions:
> >
> >         1. TG has reached the registered playback Interval time.
> >         2. Changed PLAY STATUS.
> >         3. Changed Current Track.
> >         4. Reached end or beginning of track.
> > ---
> >  profiles/audio/avrcp.c | 49
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  profiles/audio/avrcp.h |  1 +
> >  profiles/audio/media.c | 23 ++++++++++++++---------
> >  3 files changed, 64 insertions(+), 9 deletions(-)
> >
> > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c index
> > 5c3c4f9..77f1dcb 100644
> > --- a/profiles/audio/avrcp.c
> > +++ b/profiles/audio/avrcp.c
> > @@ -198,6 +198,7 @@ struct pending_list_items {  struct avrcp_player {
> >         struct avrcp_server *server;
> >         GSList *sessions;
> > +       uint16_t pos_timer;
> >         uint16_t id;
> >         uint8_t scope;
> >         uint64_t uid;
> > @@ -627,6 +628,7 @@ void avrcp_player_event(struct avrcp_player
> *player, uint8_t id,
> >         uint8_t buf[AVRCP_HEADER_LENGTH + 9];
> >         struct avrcp_header *pdu = (void *) buf;
> >         uint16_t size;
> > +       uint32_t *pos = NULL;
> >         GSList *l;
> >         int attr;
> >         int val;
> > @@ -673,6 +675,18 @@ void avrcp_player_event(struct avrcp_player
> *player, uint8_t id,
> >                 pdu->params[size++] = attr;
> >                 pdu->params[size++] = val;
> >                 break;
> > +       case AVRCP_EVENT_PLAYBACK_POS_CHANGED:
> > +               size = 5;
> > +               pos = (uint32_t *) data;
> > +
> > +               be32toh(*pos);
> > +               memcpy(&pdu->params[1], pos, sizeof(uint32_t));
> > +
> > +               if (player->pos_timer > 0) {
> > +                       g_source_remove(player->pos_timer);
> > +                       player->pos_timer = 0;
> > +               }
> > +               break;
> 
> Note that it is meant to be playback interval, so if it is not playing we should
> not send anything.
Yes, will change it accordingly.
> 
> >         default:
> >                 error("Unknown event %u", id);
> >                 return;
> > @@ -1429,6 +1443,19 @@ static bool handle_passthrough(struct avctp
> *conn, uint8_t op, bool pressed,
> >         return handler->func(session);  }
> >
> > +static gboolean interval_timeout(gpointer user_data) {
> > +       uint32_t pos;
> > +       struct avrcp_player *mp = user_data;
> > +
> > +       pos = player_get_position(mp);
> > +       mp->pos_timer = 0;
> > +       avrcp_player_event(mp, AVRCP_EVENT_PLAYBACK_POS_CHANGED,
> > +                                               &pos);
> > +
> > +       return FALSE;
> > +}
> > +
> >  static uint8_t avrcp_handle_register_notification(struct avrcp *session,
> >                                                 struct avrcp_header *pdu,
> >                                                 uint8_t transaction)
> > @@ -1436,6 +1463,8 @@ static uint8_t
> avrcp_handle_register_notification(struct avrcp *session,
> >         struct avrcp_player *player = target_get_player(session);
> >         struct btd_device *dev = session->dev;
> >         uint16_t len = ntohs(pdu->params_len);
> > +       uint32_t interval;
> > +       uint32_t pos;
> >         uint64_t uid;
> >         GList *settings;
> >
> > @@ -1467,6 +1496,20 @@ static uint8_t
> avrcp_handle_register_notification(struct avrcp *session,
> >         case AVRCP_EVENT_TRACK_REACHED_START:
> >                 len = 1;
> >                 break;
> > +       case AVRCP_EVENT_PLAYBACK_POS_CHANGED:
> > +               len = 5;
> > +
> > +               interval = htole32(pdu->params[1]);
> > +               player->pos_timer = g_timeout_add_seconds(interval,
> > +
> > + interval_timeout, player);
> > +
> > +               pos = player_get_position(player);
> > +               be32toh(pos);
> > +               /* Fill the value for sending INTERIM response */
> > +               memcpy(&pdu->params[1], &pos, sizeof(uint32_t));
> > +
> > +               break;
> > +
> >         case AVRCP_EVENT_SETTINGS_CHANGED:
> >                 len = 1;
> >                 settings = player_list_settings(player); @@ -2949,6
> > +2992,11 @@ static void player_destroy(gpointer data)
> >         if (player->destroy)
> >                 player->destroy(player->user_data);
> >
> > +       if (player->pos_timer != 0) {
> > +               g_source_remove(player->pos_timer);
> > +               player->pos_timer = 0;
> > +       }
> > +
> >         g_slist_free(player->sessions);
> >         g_free(player->path);
> >         g_free(player->change_path);
> > @@ -3407,6 +3455,7 @@ static void target_init(struct avrcp *session)
> >                                 (1 << AVRCP_EVENT_TRACK_CHANGED) |
> >                                 (1 << AVRCP_EVENT_TRACK_REACHED_START) |
> >                                 (1 << AVRCP_EVENT_TRACK_REACHED_END) |
> > +                               (1 <<
> > + AVRCP_EVENT_PLAYBACK_POS_CHANGED) |
> >                                 (1 << AVRCP_EVENT_SETTINGS_CHANGED);
> >
> >         if (target->version < 0x0104)
> > diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h index
> > 6ac5294..4816e4b 100644
> > --- a/profiles/audio/avrcp.h
> > +++ b/profiles/audio/avrcp.h
> > @@ -74,6 +74,7 @@
> >  #define AVRCP_EVENT_TRACK_CHANGED              0x02
> >  #define AVRCP_EVENT_TRACK_REACHED_END          0x03
> >  #define AVRCP_EVENT_TRACK_REACHED_START                0x04
> > +#define        AVRCP_EVENT_PLAYBACK_POS_CHANGED        0x05
> >  #define AVRCP_EVENT_SETTINGS_CHANGED           0x08
> >  #define AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED  0x0a
> >  #define AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED   0x0b
> > diff --git a/profiles/audio/media.c b/profiles/audio/media.c index
> > ef7b910..cc19c61 100644
> > --- a/profiles/audio/media.c
> > +++ b/profiles/audio/media.c
> > @@ -1305,6 +1305,9 @@ static gboolean set_status(struct media_player
> *mp, DBusMessageIter *iter)
> >         mp->status = g_strdup(value);
> >
> >         avrcp_player_event(mp->player, AVRCP_EVENT_STATUS_CHANGED,
> > mp->status);
> > +       avrcp_player_event(mp->player,
> > +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED,
> > +                                       &mp->position);
> >
> >         return TRUE;
> >  }
> > @@ -1312,7 +1315,6 @@ static gboolean set_status(struct media_player
> > *mp, DBusMessageIter *iter)  static gboolean set_position(struct
> > media_player *mp, DBusMessageIter *iter)  {
> >         uint64_t value;
> > -       const char *status;
> >
> >         if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_INT64)
> >                         return FALSE;
> > @@ -1321,11 +1323,6 @@ static gboolean set_position(struct
> > media_player *mp, DBusMessageIter *iter)
> >
> >         value /= 1000;
> >
> > -       if (value > get_position(mp))
> > -               status = "forward-seek";
> > -       else
> > -               status = "reverse-seek";
> > -
> 
> Not sure what is reasoning of removing these, the remote side may not
> register for position change and in that case it would not get any seek event
> to synchronize the position.

Yes, PLAYBACK_STATUS_CHANGED and POSITION_CHANGED both should be sent in this case.
If only PLAYBACK_STATUS_CHANGED is sent, the CT will query for position if it didn’t receive " POSITION_CHANGED " event after PLAYBACK_STATUS_CHANGED.
> 
> >         mp->position = value;
> >         g_timer_start(mp->timer);
> >
> > @@ -1334,6 +1331,9 @@ static gboolean set_position(struct media_player
> *mp, DBusMessageIter *iter)
> >         if (!mp->position) {
> >                 avrcp_player_event(mp->player,
> >
> > AVRCP_EVENT_TRACK_REACHED_START, NULL);
> > +               avrcp_player_event(mp->player,
> > +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED,
> > +                                       &mp->position);
> >                 return TRUE;
> >         }
> >
> > @@ -1344,11 +1344,13 @@ static gboolean set_position(struct
> media_player *mp, DBusMessageIter *iter)
> >         if (mp->position == UINT32_MAX || mp->position >= mp->duration) {
> >                 avrcp_player_event(mp->player,
> AVRCP_EVENT_TRACK_REACHED_END,
> >
> > NULL);
> > +               avrcp_player_event(mp->player,
> > +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED,
> > +                                       &mp->position);
> >                 return TRUE;
> >         }
> > -
> > -       /* Send a status change to force resync the position */
> > -       avrcp_player_event(mp->player, AVRCP_EVENT_STATUS_CHANGED,
> status);
> > +       avrcp_player_event(mp->player,
> AVRCP_EVENT_PLAYBACK_POS_CHANGED,
> > +
> > + &mp->position);
> >
> >         return TRUE;
> >  }
> > @@ -1456,6 +1458,7 @@ static gboolean parse_player_metadata(struct
> media_player *mp,
> >         int ctype;
> >         gboolean title = FALSE;
> >         uint64_t uid;
> > +       uint32_t pos;
> >
> >         ctype = dbus_message_iter_get_arg_type(iter);
> >         if (ctype != DBUS_TYPE_ARRAY)
> > @@ -1521,9 +1524,11 @@ static gboolean parse_player_metadata(struct
> media_player *mp,
> >         mp->position = 0;
> >         g_timer_start(mp->timer);
> >         uid = get_uid(mp);
> > +       pos = get_position(mp);
> >
> >         avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED,
> &uid);
> >         avrcp_player_event(mp->player,
> > AVRCP_EVENT_TRACK_REACHED_START, NULL);
> > +       avrcp_player_event(mp->player,
> > + AVRCP_EVENT_PLAYBACK_POS_CHANGED, &pos);
> >
> >         return TRUE;
> >  }
> > --
> > 1.9.1
> 
> Overall EVENT_PLAYBACK_POS_CHANGED cause more problems than it
> solves, because position is already part of the play status and that normally
> has to be queried anyway, also self synchronizing is way more efficient since
> we can't estimate the speed when the playback is fast-foward or rewind, and
> even if we could that would generate a lot of traffic depending on what
> interval the remote side wants to be updated.

Playback position was not a part of play status though, it was trying resync the position by sending forward-seek or reverse seek as status changed event.
In some control devices, it expects a playback_pos_changed event followed by TRACK_REACHED_START, TRACK_REACHED_END, PLAYBACK_STATUS_CHANGED and also when interval of registered for getting the playback position.
Your input on the same will be highly valuable.

Best Regards,
Bharat


^ permalink raw reply

* RE: Query regarding creation of multiple MAS Instance ID:
From: Gowtham Anandha Babu @ 2014-10-20 12:59 UTC (permalink / raw)
  To: 'Luiz Augusto von Dentz'
  Cc: linux-bluetooth, 'Bharat Panda',
	'Dmitry Kasatkin', cpgs
In-Reply-To: <CABBYNZKPex1xe0yJQkfOopBWj3QRiwF4cQd=1Jxj=xOLV69QJw@mail.gmail.com>

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> Sent: Friday, October 17, 2014 5:11 PM
> To: Gowtham Anandha Babu
> Cc: linux-bluetooth@vger.kernel.org; Bharat Panda; Dmitry Kasatkin;
> cpgs@samsung.com
> Subject: Re: Query regarding creation of multiple MAS Instance ID:
> 
> Hi,
> 
> On Thu, Oct 16, 2014 at 4:18 PM, Gowtham Anandha Babu
> <gowtham.ab@samsung.com> wrote:
> > Hi All,
> >
> > I am working on obexd MAP profile. Is it possible to create multiple
> > MAS Instance in bluez?
> > Because right now bluez has only one MAS Instance support (only one
> > MAS_UUID), which is advertised to MCE based on the XML format
> followed
> > in src/profile.c.
> > When I add one more record to MAS_RECORD XML format by changing the
> > value as 0x01, I am not able see two instances getting loaded.
> >                 <attribute id=\"0x0315\">                               \
> >                         <uint8 value=\"0x01\"/>                 \
> >                 </attribute>                                            \
> 
> How you are doing that? You should be able to call
> ProfileManager1.RegisterProfile if that is what you doing and it is not working
> perhaps we have a bug somewhere. Two very important things you need to
> check:
> 
> 1. It cannot use the same path for both instances 2. Each instance needs a
> different channel
> 
> So if you are register via XML you need to take care of those details, but
> perhaps you are using the default record by just providing the UUID, that
> probably will not work because apparently we have hardcoded the channel,
> if that is the case we should probably add a check if there is another instance
> already register and use a different channel/psm.
> 
> > Do we need to create one more UUID like OBEX_MAS1_UUID and
> > OBEX_MAS2_UUID to support multiple mas instances?
> > Or what could be the possible solutions?
> 
> Each instance has to have the service class set to MAS UUID, otherwise no
> one will be able to discover it.
> 
> >
> > Regards.
> > Gowtham Anandha Babu
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-bluetooth" in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> --
> Luiz Augusto von Dentz

Here is what I did exactly after your comments, 

1) Based on the flag, I managed the two MAS Instances in bluetooth.c

diff --git a/obexd/plugins/bluetooth.c b/obexd/plugins/bluetooth.c
index cf326cc..3a0c022 100644
--- a/obexd/plugins/bluetooth.c
+++ b/obexd/plugins/bluetooth.c
@@ -50,6 +50,8 @@
 #define BT_RX_MTU 32767
 #define BT_TX_MTU 32767
 
+gboolean MAS = FALSE;
+
 struct bluetooth_profile {
 	struct obex_server *server;
 	struct obex_service_driver *driver;
@@ -269,7 +271,20 @@ static int register_profile(struct bluetooth_profile *profile)
 	char *xml;
 	int ret = 0;
 
-	profile->path = g_strconcat("/org/bluez/obex/", profile->uuid, NULL);
+
+	if(!g_ascii_strcasecmp(profile->uuid, OBEX_MAS_UUID))
+		if(!MAS) {
+			profile->path = g_strconcat("/org/bluez/obex/MAS1/", 
+							profile->uuid, NULL);
+			MAS = TRUE;
+		}
+		else
+			profile->path = g_strconcat("/org/bluez/obex/MAS2/", 
+							profile->uuid, NULL);
+	else
+		profile->path = g_strconcat("/org/bluez/obex/", profile->uuid, NULL);
+
 	g_strdelimit(profile->path, "-", '_');
 
 	if (!g_dbus_register_interface(connection, profile->path,
@@ -290,6 +305,10 @@ static int register_profile(struct bluetooth_profile *profile)
 
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
 							&profile->path);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,
 							&profile->uuid);
 	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
@@ -346,7 +365,9 @@ static const char *service2uuid(uint16_t service)
 		return "00005005-0000-1000-8000-0002ee000001";
 	case OBEX_SYNCEVOLUTION:
 		return "00000002-0000-1000-8000-0002ee000002";
-	case OBEX_MAS:
+	case OBEX_MAS1:
+		return OBEX_MAS_UUID;
+	case OBEX_MAS2:
 		return OBEX_MAS_UUID;
 	case OBEX_MNS:
 		return OBEX_MNS_UUID;

2) Created one more MAS Instance in mas.c

diff --git a/obexd/plugins/mas.c b/obexd/plugins/mas.c
index fb97fe3..fb482e2 100644
--- a/obexd/plugins/mas.c
+++ b/obexd/plugins/mas.c
@@ -728,9 +728,9 @@ static int any_close(void *obj)
 	return 0;
 }
 
-static struct obex_service_driver mas = {
-	.name = "Message Access server",
-	.service = OBEX_MAS,
+static struct obex_service_driver mas1 = {
+	.name = "Message Access server 1",
+	.service = OBEX_MAS1,
 	.target = MAS_TARGET,
 	.target_size = TARGET_SIZE,
 	.connect = mas_connect,
@@ -740,6 +740,18 @@ static struct obex_service_driver mas = {
 	.disconnect = mas_disconnect,
 };
 
+static struct obex_service_driver mas2 = {
+        .name = "Message Access server 2",
+        .service = OBEX_MAS2,
+        .target = MAS_TARGET,
+        .target_size = TARGET_SIZE,
+        .connect = mas_connect,
+        .get = mas_get,
+        .put = mas_put,
+        .setpath = mas_setpath,
+        .disconnect = mas_disconnect,
+};
+
 static struct obex_mime_type_driver mime_map = {
 	.target = MAS_TARGET,
 	.target_size = TARGET_SIZE,
@@ -838,10 +850,14 @@ static int mas_init(void)
 			goto failed;
 	}
 
-	err = obex_service_driver_register(&mas);
+	err = obex_service_driver_register(&mas1);
 	if (err < 0)
 		goto failed;
 
+	err = obex_service_driver_register(&mas2);
+	if(err < 0)
+		goto failed;
+
 	return 0;
 
 failed:
@@ -857,7 +873,8 @@ static void mas_exit(void)
 {
 	int i;
 
-	obex_service_driver_unregister(&mas);
+	obex_service_driver_unregister(&mas1);
+        	obex_service_driver_unregister(&mas2);
 
 	for (i = 0; map_drivers[i] != NULL; ++i)
 		obex_mime_type_driver_unregister(map_drivers[i]);
@@ -865,4 +882,5 @@ static void mas_exit(void)
 	messages_exit();
 }
 
-OBEX_PLUGIN_DEFINE(mas, mas_init, mas_exit)
+OBEX_PLUGIN_DEFINE(mas1, mas_init, mas_exit)
+OBEX_PLUGIN_DEFINE(mas2, mas_init, mas_exit)
diff --git a/obexd/src/obexd.h b/obexd/src/obexd.h
index 42c3c4d..59de70e 100644
--- a/obexd/src/obexd.h
+++ b/obexd/src/obexd.h
@@ -28,7 +28,8 @@
 #define OBEX_IRMC	(1 << 5)
 #define OBEX_PCSUITE	(1 << 6)
 #define OBEX_SYNCEVOLUTION	(1 << 7)
-#define OBEX_MAS	(1 << 8)
+#define OBEX_MAS1	(1 << 8)
+#define OBEX_MAS2	(1 << 10)
 #define OBEX_MNS	(1 << 9)
 
 gboolean plugin_init(const char *pattern, const char *exclude);

3) Assigned different channel and path for both instances 
4)Added one more SDP record (XML) in profile.c

diff --git a/src/profile.c b/src/profile.c
index 7aca3be..ab5b5c2 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -62,7 +62,8 @@
 #define HFP_AG_DEFAULT_CHANNEL	13
 #define SYNC_DEFAULT_CHANNEL	14
 #define PBAP_DEFAULT_CHANNEL	15
-#define MAS_DEFAULT_CHANNEL	16
+#define MAS1_DEFAULT_CHANNEL	16
+#define MAS2_DEFAULT_CHANNEL	18
 #define MNS_DEFAULT_CHANNEL	17
 
 #define BTD_PROFILE_PSM_AUTO	-1
@@ -395,7 +396,7 @@
 		</attribute>						\
 	</record>"
 
-#define MAS_RECORD							\
+#define MAS_RECORD1							\
 	"<?xml version=\"1.0\" encoding=\"UTF-8\" ?>			\
 	<record>							\
 		<attribute id=\"0x0001\">				\
@@ -441,6 +442,52 @@
 		</attribute>						\
 	</record>"
 
+#define MAS_RECORD2							\
+	"<?xml version=\"1.0\" encoding=\"UTF-8\" ?>			\
+	<record>							\
+		<attribute id=\"0x0001\">				\
+			<sequence>					\
+				<uuid value=\"0x1132\"/>		\
+			</sequence>					\
+		</attribute>						\
+		<attribute id=\"0x0004\">				\
+			<sequence>					\
+				<sequence>				\
+					<uuid value=\"0x0100\"/>	\
+				</sequence>				\
+				<sequence>				\
+					<uuid value=\"0x0003\"/>	\
+					<uint8 value=\"0x%02x\"/>	\
+				</sequence>				\
+				<sequence>				\
+					<uuid value=\"0x0008\"/>	\
+				</sequence>				\
+			</sequence>					\
+		</attribute>						\
+		<attribute id=\"0x0005\">				\
+			<sequence>					\
+				<uuid value=\"0x1002\" />		\
+			</sequence>					\
+		</attribute>						\
+		<attribute id=\"0x0009\">				\
+			<sequence>					\
+				<sequence>				\
+					<uuid value=\"0x1134\"/>	\
+					<uint16 value=\"0x%04x\" />	\
+				</sequence>				\
+			</sequence>					\
+		</attribute>						\
+		<attribute id=\"0x0100\">				\
+			<text value=\"%s\"/>				\
+		</attribute>						\
+		<attribute id=\"0x0315\">				\
+			<uint8 value=\"0x01\"/>				\
+		</attribute>						\
+		<attribute id=\"0x0316\">				\
+			<uint8 value=\"0x0F\"/>				\
+		</attribute>						\
+	</record>"
+
 #define MNS_RECORD							\
 	"<?xml version=\"1.0\" encoding=\"UTF-8\" ?>			\
 	<record>							\
@@ -562,6 +609,8 @@
 
 struct ext_io;
 
+gboolean MAS = FALSE;
+
 struct ext_profile {
 	struct btd_profile p;
 
@@ -1734,10 +1787,17 @@ static char *get_pse_record(struct ext_profile *ext, struct ext_io *l2cap,
 								ext->name);
 }
 
-static char *get_mas_record(struct ext_profile *ext, struct ext_io *l2cap,
+static char *get_mas_record1(struct ext_profile *ext, struct ext_io *l2cap,
+							struct ext_io *rfcomm)
+{
+	return g_strdup_printf(MAS_RECORD1, rfcomm->chan, ext->version,
+								ext->name);
+}
+
+static char *get_mas_record2(struct ext_profile *ext, struct ext_io *l2cap,
 							struct ext_io *rfcomm)
 {
-	return g_strdup_printf(MAS_RECORD, rfcomm->chan, ext->version,
+	return g_strdup_printf(MAS_RECORD2, rfcomm->chan, ext->version,
 								ext->name);
 }
 
@@ -1949,10 +2009,17 @@ static struct default_settings {
 		.version	= 0x0101,
 	}, {
 		.uuid		= OBEX_MAS_UUID,
-		.name		= "Message Access",
-		.channel	= MAS_DEFAULT_CHANNEL,
+		.name		= "Message Access 1",
+		.channel	= MAS1_DEFAULT_CHANNEL,
+		.authorize	= true,
+		.get_record	= get_mas_record1,
+		.version	= 0x0100
+	}, {
+		.uuid		= OBEX_MAS_UUID,
+		.name		= "Message Access 2",
+		.channel	= MAS2_DEFAULT_CHANNEL,
 		.authorize	= true,
-		.get_record	= get_mas_record,
+		.get_record	= get_mas_record2,
 		.version	= 0x0100
 	}, {
 		.uuid		= OBEX_MNS_UUID,
@@ -1981,8 +2048,21 @@ static void ext_set_defaults(struct ext_profile *ext)
 		struct default_settings *settings = &defaults[i];
 		const char *remote_uuid;
 
+		DBG("%s ==  %s",ext->uuid, settings->uuid);
+
 		if (strcasecmp(ext->uuid, settings->uuid) != 0)
 			continue;
+		
+		if(!strcasecmp(ext->uuid, OBEX_MAS_UUID))
+			if(MAS) {
+				MAS = 0;
+				continue;
+			}
+		
+		if(!strcasecmp(ext->uuid, OBEX_MAS_UUID))
+			MAS = TRUE;
+
+		DBG("MATCHED  %s ==  %s MAS = %d",ext->uuid, settings->uuid, MAS);
 
 		if (settings->remote_uuid)
 			remote_uuid = settings->remote_uuid;
@@ -2022,6 +2102,8 @@ static void ext_set_defaults(struct ext_profile *ext)
 
 		if (settings->name)
 			ext->name = g_strdup(settings->name);
+		
+		return;
 	}
 }


After this changes I am able to load the two MAS Instances successfully. Is it fine? What do you think?

Regards,
Gowtham Anandha Babu 


^ permalink raw reply related

* how to set PSCAN mode in .conf?
From: Jeff Chua @ 2014-10-20 12:56 UTC (permalink / raw)
  To: linux-bluetooth

latest bluez doesn't read hcid.conf, so how can I enable "pscan" in
the conf file?

I know "hciconfig hci0 pscan" works, but I would still like to set
that via the .conf file.

Thanks,
Jeff

^ permalink raw reply

* Re: [PATCH] hciattach: Add sleep|nosleep keyword
From: Luiz Augusto von Dentz @ 2014-10-20 12:38 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1413550093-16448-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Fri, Oct 17, 2014 at 3:48 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> ---
>  tools/hciattach.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/hciattach.c b/tools/hciattach.c
> index d8ef7e7..808bbac 100644
> --- a/tools/hciattach.c
> +++ b/tools/hciattach.c
> @@ -1276,7 +1276,9 @@ static void usage(void)
>  {
>         printf("hciattach - HCI UART driver initialization utility\n");
>         printf("Usage:\n");
> -       printf("\thciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed] <tty> <type | id> [speed] [flow|noflow] [bdaddr]\n");
> +       printf("\thciattach [-n] [-p] [-b] [-r] [-t timeout] [-s initial_speed]"
> +                       " <tty> <type | id> [speed] [flow|noflow]"
> +                       " [sleep|nosleep] [bdaddr]\n");
>         printf("\thciattach -l\n");
>  }
>
> --
> 1.9.1

Applied, thanks.

-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH 1/6] bnep: Avoid double error print for bnep_connadd()
From: Luiz Augusto von Dentz @ 2014-10-20 12:37 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1413550450-16577-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Fri, Oct 17, 2014 at 3:54 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> This avoids double printing the same error with bnep connection add
> ioctl.
> ---
>  profiles/network/bnep.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index 136709d..035beb1 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -316,10 +316,8 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
>         setsockopt(sk, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));
>
>         sk = g_io_channel_unix_get_fd(session->io);
> -       if (bnep_connadd(sk, session->src, session->iface)) {
> -               error("bnep conn could not be added");
> +       if (bnep_connadd(sk, session->src, session->iface) < 0)
>                 goto failed;
> -       }
>
>         if (bnep_if_up(session->iface)) {
>                 error("could not up %s", session->iface);
> @@ -556,14 +554,14 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
>  int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
>                                                 const bdaddr_t *addr)
>  {
> +       int err;
> +
>         if (!bridge || !iface || !addr)
>                 return -EINVAL;
>
> -       if (bnep_connadd(sk, dst, iface) < 0) {
> -               error("Can't add connection to the bridge %s: %s(%d)",
> -                                               bridge, strerror(errno), errno);
> -               return -errno;
> -       }
> +       err = bnep_connadd(sk, dst, iface);
> +       if (err < 0)
> +               return err;
>
>         if (bnep_add_to_bridge(iface, bridge) < 0) {
>                 error("Can't add %s to the bridge %s: %s(%d)",
> --
> 1.9.1

Applied, thanks.

-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH v2 0/5] Introduce shared/gatt-server.
From: Luiz Augusto von Dentz @ 2014-10-20 12:09 UTC (permalink / raw)
  To: Arman Uguray; +Cc: BlueZ development
In-Reply-To: <CAHrH25SLjFr4iaKy8WJAfrz0oZpqENpkBJesNd4_xa_br+E6cA@mail.gmail.com>

Hi Arman,

On Thu, Oct 16, 2014 at 9:49 PM, Arman Uguray <armansito@chromium.org> wrote:
> On Mon, Oct 13, 2014 at 2:09 PM, Arman Uguray <armansito@chromium.org> wrote:
>> *v2:
>>   - Split read_by_grp_type_cb into two functions by moving the response PDU
>>     encoding loop into a helper function.
>>
>> *v1:
>>   - Make gatt-db external to gatt-server, as initially discussed.
>>   - Also implement Read By Group Type request.
>>
>> Arman Uguray (5):
>>   shared/att: Drop the connection if a request is received while one is
>>     pending.
>>   shared/att: Respond with ERROR_REQUEST_NOT_SUPPORTED for unhandled
>>     requests.
>>   shared/gatt-server: Introduce shared/gatt-server.
>>   shared/gatt-server: Support Exchange MTU request.
>>   shared/gatt-server: Support Read By Group Type request.
>>
>>  Makefile.am              |   1 +
>>  src/shared/att.c         | 124 +++++++++++------
>>  src/shared/gatt-server.c | 356 +++++++++++++++++++++++++++++++++++++++++++++++
>>  src/shared/gatt-server.h |  40 ++++++
>>  4 files changed, 481 insertions(+), 40 deletions(-)
>>  create mode 100644 src/shared/gatt-server.c
>>  create mode 100644 src/shared/gatt-server.h
>>
>> --
>> 2.1.0.rc2.206.gedb03e5
>>
>
> ping. Luiz made comments on one of the patches but I believe they've
> been addressed.
> --

This is now pushed, note that Ive made a couple changes both in the
code and the commit message, please next time follow 50/72 format for
the commit message (check HACKING if you don't know what Im talking
about).


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH BlueZ] android/avrcp-lib: Use cpu_to_* and *_to_cpu
From: Luiz Augusto von Dentz @ 2014-10-20 12:05 UTC (permalink / raw)
  To: linux-bluetooth@vger.kernel.org
In-Reply-To: <1413536909-13049-1-git-send-email-luiz.dentz@gmail.com>

Hi,

On Fri, Oct 17, 2014 at 12:08 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> For PDU which have packed struct it is not necessary to use put_*
> and get_* since it should be already aligned.
> ---
>  android/avrcp-lib.c | 136 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 68 insertions(+), 68 deletions(-)
>
> diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> index 2c3d2e9..cff37a8 100644
> --- a/android/avrcp-lib.c
> +++ b/android/avrcp-lib.c
> @@ -929,7 +929,7 @@ static bool parse_attributes(uint32_t *params, uint16_t params_len,
>
>         for (i = 0; i < number && params_len >= sizeof(*attrs); i++,
>                                         params_len -= sizeof(*attrs)) {
> -               attrs[i] = get_be32(&params[i]);
> +               attrs[i] = be32_to_cpu(params[i]);
>
>                 if (attrs[i] == AVRCP_MEDIA_ATTRIBUTE_ILLEGAL ||
>                                 attrs[i] > AVRCP_MEDIA_ATTRIBUTE_LAST)
> @@ -988,7 +988,7 @@ static ssize_t register_notification(struct avrcp *session, uint8_t transaction,
>
>         req = (void *) params;
>
> -       interval = get_be32(&req->interval);
> +       interval = be32_to_cpu(req->interval);
>
>         return player->ind->register_notification(session, transaction,
>                                                         req->event, interval,
> @@ -1037,7 +1037,7 @@ static ssize_t set_addressed(struct avrcp *session, uint8_t transaction,
>
>         req = (void *) params;
>
> -       id = get_be16(&req->id);
> +       id = be16_to_cpu(req->id);
>
>         return player->ind->set_addressed(session, transaction, id,
>                                                         player->user_data);
> @@ -1271,7 +1271,7 @@ static ssize_t set_browsed(struct avrcp *session, uint8_t transaction,
>
>         req = (void *) params;
>
> -       id = get_be16(&req->id);
> +       id = be16_to_cpu(req->id);
>
>         return player->ind->set_browsed(session, transaction, id,
>                                                         player->user_data);
> @@ -1301,16 +1301,16 @@ static ssize_t get_folder_items(struct avrcp *session, uint8_t transaction,
>         if (req->scope > AVRCP_MEDIA_NOW_PLAYING)
>                 return -EBADRQC;
>
> -       start = get_be32(&req->start);
> -       end = get_be32(&req->end);
> +       start = be32_to_cpu(req->start);
> +       end = be32_to_cpu(req->end);
>
>         if (start > end)
>                 return -ERANGE;
>
> -       number = get_be16(&req->number);
> +       number = be16_to_cpu(req->number);
>
>         for (i = 0; i < number; i++) {
> -               attrs[i] = get_be32(&req->attrs[i]);
> +               attrs[i] = be32_to_cpu(req->attrs[i]);
>
>                 if (attrs[i] == AVRCP_MEDIA_ATTRIBUTE_ILLEGAL ||
>                                 attrs[i] > AVRCP_MEDIA_ATTRIBUTE_LAST)
> @@ -1341,8 +1341,8 @@ static ssize_t change_path(struct avrcp *session, uint8_t transaction,
>
>         req = (void *) params;
>
> -       counter = get_be16(&req->counter);
> -       uid = get_be64(&req->uid);
> +       counter = be16_to_cpu(req->counter);
> +       uid = be64_to_cpu(req->uid);
>
>         return player->ind->change_path(session, transaction, counter,
>                                         req->direction, uid, player->user_data);
> @@ -1372,11 +1372,11 @@ static ssize_t get_item_attributes(struct avrcp *session, uint8_t transaction,
>         if (req->scope > AVRCP_MEDIA_NOW_PLAYING)
>                 return -EBADRQC;
>
> -       uid = get_be64(&req->uid);
> -       counter = get_be16(&req->counter);
> +       uid = be64_to_cpu(req->uid);
> +       counter = be16_to_cpu(req->counter);
>
>         for (i = 0; i < req->number; i++) {
> -               attrs[i] = get_be32(&req->attrs[i]);
> +               attrs[i] = be32_to_cpu(req->attrs[i]);
>
>                 if (attrs[i] == AVRCP_MEDIA_ATTRIBUTE_ILLEGAL ||
>                                 attrs[i] > AVRCP_MEDIA_ATTRIBUTE_LAST)
> @@ -1411,8 +1411,8 @@ static ssize_t play_item(struct avrcp *session, uint8_t transaction,
>         if (req->scope > AVRCP_MEDIA_NOW_PLAYING)
>                 return -EBADRQC;
>
> -       uid = get_be64(&params[1]);
> -       counter = get_be16(&params[9]);
> +       uid = be64_to_cpu(req->uid);
> +       counter = be16_to_cpu(req->counter);
>
>         return player->ind->play_item(session, transaction, req->scope, uid,
>                                                 counter, player->user_data);
> @@ -1438,7 +1438,7 @@ static ssize_t search(struct avrcp *session, uint8_t transaction,
>
>         req = (void *) params;
>
> -       len = get_be16(&req->len);
> +       len = be16_to_cpu(req->len);
>         if (!len)
>                 return -EINVAL;
>
> @@ -1474,8 +1474,8 @@ static ssize_t add_to_now_playing(struct avrcp *session, uint8_t transaction,
>         if (req->scope > AVRCP_MEDIA_NOW_PLAYING)
>                 return -EBADRQC;
>
> -       uid = get_be64(&req->uid);
> -       counter = get_be16(&req->counter);
> +       uid = be64_to_cpu(req->uid);
> +       counter = be16_to_cpu(req->counter);
>
>         return player->ind->add_to_now_playing(session, transaction, req->scope,
>                                                         uid, counter,
> @@ -1859,7 +1859,7 @@ int avrcp_register_notification(struct avrcp *session, uint8_t event,
>                 return -EINVAL;
>
>         req.event = event;
> -       put_be32(interval, &req.interval);
> +       req.interval = cpu_to_be32(interval);
>
>         iov.iov_base = &req;
>         iov.iov_len = sizeof(req);
> @@ -2345,8 +2345,8 @@ static gboolean get_play_status_rsp(struct avctp *conn,
>
>         rsp = (void *) pdu->params;
>
> -       duration = get_be32(&rsp->duration);
> -       position = get_be32(&rsp->position);
> +       duration = be32_to_cpu(rsp->duration);
> +       position = be32_to_cpu(rsp->position);
>         status = rsp->status;
>         err = 0;
>
> @@ -2432,9 +2432,9 @@ static int parse_attribute_list(uint8_t *params, uint16_t params_len,
>         for (i = 0; i < number && params_len >= sizeof(*item); i++) {
>                 item = (void *) params;
>
> -               item->attr = get_be32(&item->attr);
> -               item->charset = get_be16(&item->charset);
> -               item->len = get_be16(&item->len);
> +               item->attr = be32_to_cpu(item->attr);
> +               item->charset = be16_to_cpu(item->charset);
> +               item->len = be16_to_cpu(item->len);
>
>                 params_len -= sizeof(*item);
>                 params += sizeof(*item);
> @@ -2584,7 +2584,7 @@ int avrcp_set_addressed_player(struct avrcp *session, uint16_t player_id)
>         struct iovec iov;
>         struct set_addressed_req req;
>
> -       put_be16(player_id, &req.id);
> +       req.id = cpu_to_be16(player_id);
>
>         iov.iov_base = &req;
>         iov.iov_len = sizeof(req);
> @@ -2658,8 +2658,8 @@ static gboolean set_browsed_rsp(struct avctp *conn, uint8_t *operands,
>
>         rsp = (void *) pdu->params;
>
> -       counter = get_be16(&rsp->counter);
> -       items = get_be32(&rsp->items);
> +       counter = be16_to_cpu(rsp->counter);
> +       items = be32_to_cpu(rsp->items);
>
>         path = parse_folder_list(rsp->data, pdu->params_len - sizeof(*rsp),
>                                                                 rsp->depth);
> @@ -2676,12 +2676,12 @@ done:
>  int avrcp_set_browsed_player(struct avrcp *session, uint16_t player_id)
>  {
>         struct iovec iov;
> -       uint8_t pdu[2];
> +       struct set_browsed_req req;
>
> -       put_be16(player_id, pdu);
> +       req.id = cpu_to_be16(player_id);
>
> -       iov.iov_base = pdu;
> -       iov.iov_len = sizeof(pdu);
> +       iov.iov_base = &req;
> +       iov.iov_len = sizeof(req);
>
>         return avrcp_send_browsing_req(session, AVRCP_SET_BROWSED_PLAYER,
>                                         &iov, 1, set_browsed_rsp, session);
> @@ -2721,8 +2721,8 @@ static gboolean get_folder_items_rsp(struct avctp *conn,
>
>         rsp = (void *) pdu->params;
>
> -       counter = get_be16(&rsp->counter);
> -       number = get_be16(&rsp->number);
> +       counter = be16_to_cpu(rsp->counter);
> +       number = be16_to_cpu(rsp->number);
>         params = rsp->data;
>
>         /* FIXME: Add proper parsing for each item type */
> @@ -2744,8 +2744,8 @@ int avrcp_get_folder_items(struct avrcp *session, uint8_t scope,
>         int i;
>
>         req.scope = scope;
> -       put_be32(start, &req.start);
> -       put_be32(end, &req.end);
> +       req.start = cpu_to_be32(start);
> +       req.end = cpu_to_be32(end);
>         req.number = number;
>
>         iov[0].iov_base = &req;
> @@ -2757,7 +2757,7 @@ int avrcp_get_folder_items(struct avrcp *session, uint8_t scope,
>                                                 session);
>
>         for (i = 0; i < number; i++)
> -               put_be32(attrs[i], &attrs[i]);
> +               attrs[i] = cpu_to_be32(attrs[i]);
>
>         iov[1].iov_base = attrs;
>         iov[1].iov_len = number * sizeof(*attrs);
> @@ -2798,7 +2798,7 @@ static gboolean change_path_rsp(struct avctp *conn, uint8_t *operands,
>
>         rsp = (void *) pdu->params;
>
> -       items = get_be32(&rsp->items);
> +       items = be32_to_cpu(rsp->items);
>
>  done:
>         player->cfm->change_path(session, err, items, player->user_data);
> @@ -2812,9 +2812,9 @@ int avrcp_change_path(struct avrcp *session, uint8_t direction, uint64_t uid,
>         struct iovec iov;
>         struct change_path_req req;
>
> -       put_be16(counter, &req.counter);
> +       req.counter = cpu_to_be16(counter);
>         req.direction = direction;
> -       put_be64(uid, &req.uid);
> +       req.uid = cpu_to_be64(uid);
>
>         iov.iov_base = &req;
>         iov.iov_len = sizeof(req);
> @@ -2867,8 +2867,8 @@ int avrcp_get_item_attributes(struct avrcp *session, uint8_t scope,
>         int i;
>
>         req.scope = scope;
> -       put_be64(uid, &req.uid);
> -       put_be16(counter, &req.counter);
> +       req.uid = cpu_to_be64(uid);
> +       req.counter = cpu_to_be16(counter);
>         req.number = number;
>
>         iov[0].iov_base = &req;
> @@ -2887,7 +2887,7 @@ int avrcp_get_item_attributes(struct avrcp *session, uint8_t scope,
>                 if (attrs[i] > AVRCP_MEDIA_ATTRIBUTE_LAST ||
>                                 attrs[i] == AVRCP_MEDIA_ATTRIBUTE_ILLEGAL)
>                         return -EINVAL;
> -               put_be32(attrs[i], &attrs[i]);
> +               attrs[i] = cpu_to_be32(attrs[i]);
>         }
>
>         iov[1].iov_base = attrs;
> @@ -2935,8 +2935,8 @@ int avrcp_play_item(struct avrcp *session, uint8_t scope, uint64_t uid,
>                 return -EINVAL;
>
>         req.scope = scope;
> -       put_be64(uid, &req.uid);
> -       put_be16(counter, &req.counter);
> +       req.uid = cpu_to_be64(uid);
> +       req.counter = cpu_to_be16(counter);
>
>         iov.iov_base = &req;
>         iov.iov_len = sizeof(req);
> @@ -2978,8 +2978,8 @@ static gboolean search_rsp(struct avctp *conn, uint8_t *operands,
>
>         rsp = (void *) pdu->params;
>
> -       counter = get_be16(&rsp->counter);
> -       items = get_be32(&rsp->items);
> +       counter = be16_to_cpu(rsp->counter);
> +       items = be32_to_cpu(rsp->items);
>
>         err = 0;
>
> @@ -3000,8 +3000,8 @@ int avrcp_search(struct avrcp *session, const char *string)
>
>         len = strnlen(string, UINT8_MAX);
>
> -       put_be16(AVRCP_CHARSET_UTF8, &req.charset);
> -       put_be16(len, &req.len);
> +       req.charset = cpu_to_be16(AVRCP_CHARSET_UTF8);
> +       req.len = cpu_to_be16(len);
>
>         iov[0].iov_base = &req;
>         iov[0].iov_len = sizeof(req);
> @@ -3050,8 +3050,8 @@ int avrcp_add_to_now_playing(struct avrcp *session, uint8_t scope, uint64_t uid,
>                 return -EINVAL;
>
>         req.scope = scope;
> -       put_be64(uid, &req.uid);
> -       put_be16(counter, &req.counter);
> +       req.uid = cpu_to_be64(uid);
> +       req.counter = cpu_to_be16(counter);
>
>         iov.iov_base = &req;
>         iov.iov_len = sizeof(req);
> @@ -3136,7 +3136,7 @@ int avrcp_get_player_attribute_text_rsp(struct avrcp *session,
>                         len = strlen(text[i]);
>
>                 val[i].attr = attrs[i];
> -               put_be16(AVRCP_CHARSET_UTF8, &val[i].charset);
> +               val[i].charset = cpu_to_be16(AVRCP_CHARSET_UTF8);
>                 val[i].len = len;
>
>                 iov[i + 1].iov_base = &val[i];
> @@ -3177,8 +3177,8 @@ int avrcp_get_play_status_rsp(struct avrcp *session, uint8_t transaction,
>         struct iovec iov;
>         struct get_play_status_rsp rsp;
>
> -       put_be32(duration, &rsp.duration);
> -       put_be32(position, &rsp.position);
> +       rsp.duration = cpu_to_be32(duration);
> +       rsp.position = cpu_to_be32(position);
>         rsp.status = status;
>
>         iov.iov_base = &rsp;
> @@ -3210,7 +3210,7 @@ int avrcp_get_player_values_text_rsp(struct avrcp *session,
>                         len = strlen(text[i]);
>
>                 val[i].attr = values[i];
> -               put_be16(AVRCP_CHARSET_UTF8, &val[i].charset);
> +               val[i].charset = cpu_to_be16(AVRCP_CHARSET_UTF8);
>                 val[i].len = len;
>
>                 iov[i + 1].iov_base = &val[i];
> @@ -3314,8 +3314,8 @@ int avrcp_register_notification_rsp(struct avrcp *session, uint8_t transaction,
>                         return -EINVAL;
>
>                 player = data;
> -               put_be16(player[0], &player[0]);
> -               put_be16(player[1], &player[1]);
> +               player[0] = cpu_to_be16(player[0]);
> +               player[1] = cpu_to_be16(player[1]);
>
>                 break;
>         case AVRCP_EVENT_SETTINGS_CHANGED:
> @@ -3398,9 +3398,9 @@ int avrcp_set_browsed_player_rsp(struct avrcp *session, uint8_t transaction,
>                                         AVRCP_SET_BROWSED_PLAYER, status);
>
>         rsp.status = status;
> -       put_be16(counter, &rsp.counter);
> -       put_be32(items, &rsp.items);
> -       put_be16(AVRCP_CHARSET_UTF8, &rsp.charset);
> +       rsp.counter = cpu_to_be16(counter);
> +       rsp.items = cpu_to_be32(items);
> +       rsp.charset = cpu_to_be16(AVRCP_CHARSET_UTF8);
>         rsp.depth = depth;
>
>         iov[0].iov_base = &rsp;
> @@ -3420,7 +3420,7 @@ int avrcp_set_browsed_player_rsp(struct avrcp *session, uint8_t transaction,
>                 iov[i * 2 + 2].iov_base = (void *) folders[i];
>                 iov[i * 2 + 2].iov_len = len[i];
>
> -               put_be16(len[i], &len[i]);
> +               len[i] = cpu_to_be16(len[i]);
>
>                 iov[i * 2 + 1].iov_base = &len[i];
>                 iov[i * 2 + 1].iov_len = sizeof(len[i]);
> @@ -3446,8 +3446,8 @@ int avrcp_get_folder_items_rsp(struct avrcp *session, uint8_t transaction,
>                                         AVRCP_GET_FOLDER_ITEMS, status);
>
>         rsp.status = status;
> -       put_be16(counter, &rsp.counter);
> -       put_be16(number, &rsp.number);
> +       rsp.counter = cpu_to_be16(counter);
> +       rsp.number = cpu_to_be16(number);
>
>         iov[0].iov_base = &rsp;
>         iov[0].iov_len = sizeof(rsp);
> @@ -3478,7 +3478,7 @@ int avrcp_change_path_rsp(struct avrcp *session, uint8_t transaction,
>                                                                         status);
>
>         rsp.status = status;
> -       put_be32(items, &rsp.items);
> +       rsp.items = cpu_to_be32(items);
>
>         iov.iov_base = &rsp;
>         iov.iov_len = sizeof(rsp);
> @@ -3503,9 +3503,9 @@ static bool pack_attribute_list(struct iovec *iov, uint8_t number,
>                 if (text[i])
>                         len = strlen(text[i]);
>
> -               put_be32(attrs[i], &val[i].attr);
> -               put_be16(AVRCP_CHARSET_UTF8, &val[i].charset);
> -               put_be16(len, &val[i].len);
> +               val[i].attr = cpu_to_be32(attrs[i]);
> +               val[i].charset = cpu_to_be16(AVRCP_CHARSET_UTF8);
> +               val[i].len = cpu_to_be16(len);
>
>                 iov[i].iov_base = &val[i];
>                 iov[i].iov_len = sizeof(val[i]);
> @@ -3563,8 +3563,8 @@ int avrcp_search_rsp(struct avrcp *session, uint8_t transaction, uint8_t status,
>                                                                 status);
>
>         rsp.status = status;
> -       put_be16(counter, &rsp.counter);
> -       put_be32(items, &rsp.items);
> +       rsp.counter = cpu_to_be16(counter);
> +       rsp.items = cpu_to_be32(items);
>
>         iov.iov_base = &rsp;
>         iov.iov_len = sizeof(rsp);
> --
> 1.9.3

Pushed.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH ] audio/avrcp: Add Support for PLAYBACK_POS_CHANGED_EVENT
From: Luiz Augusto von Dentz @ 2014-10-20 11:17 UTC (permalink / raw)
  To: Bharat Panda; +Cc: linux-bluetooth@vger.kernel.org, cpgs
In-Reply-To: <1412610595-16393-1-git-send-email-bharat.panda@samsung.com>

Hi,

On Mon, Oct 6, 2014 at 6:49 PM, Bharat Panda <bharat.panda@samsung.com> wrote:
> As per AVRCP 1.5 spec, 6.7.2, page 57.
>
> EVENT_PLAYBACK_POS_CHANGED shall be notified in the following conditions:
>
>         1. TG has reached the registered playback Interval time.
>         2. Changed PLAY STATUS.
>         3. Changed Current Track.
>         4. Reached end or beginning of track.
> ---
>  profiles/audio/avrcp.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  profiles/audio/avrcp.h |  1 +
>  profiles/audio/media.c | 23 ++++++++++++++---------
>  3 files changed, 64 insertions(+), 9 deletions(-)
>
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 5c3c4f9..77f1dcb 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -198,6 +198,7 @@ struct pending_list_items {
>  struct avrcp_player {
>         struct avrcp_server *server;
>         GSList *sessions;
> +       uint16_t pos_timer;
>         uint16_t id;
>         uint8_t scope;
>         uint64_t uid;
> @@ -627,6 +628,7 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>         uint8_t buf[AVRCP_HEADER_LENGTH + 9];
>         struct avrcp_header *pdu = (void *) buf;
>         uint16_t size;
> +       uint32_t *pos = NULL;
>         GSList *l;
>         int attr;
>         int val;
> @@ -673,6 +675,18 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>                 pdu->params[size++] = attr;
>                 pdu->params[size++] = val;
>                 break;
> +       case AVRCP_EVENT_PLAYBACK_POS_CHANGED:
> +               size = 5;
> +               pos = (uint32_t *) data;
> +
> +               be32toh(*pos);
> +               memcpy(&pdu->params[1], pos, sizeof(uint32_t));
> +
> +               if (player->pos_timer > 0) {
> +                       g_source_remove(player->pos_timer);
> +                       player->pos_timer = 0;
> +               }
> +               break;

Note that it is meant to be playback interval, so if it is not playing
we should not send anything.

>         default:
>                 error("Unknown event %u", id);
>                 return;
> @@ -1429,6 +1443,19 @@ static bool handle_passthrough(struct avctp *conn, uint8_t op, bool pressed,
>         return handler->func(session);
>  }
>
> +static gboolean interval_timeout(gpointer user_data)
> +{
> +       uint32_t pos;
> +       struct avrcp_player *mp = user_data;
> +
> +       pos = player_get_position(mp);
> +       mp->pos_timer = 0;
> +       avrcp_player_event(mp, AVRCP_EVENT_PLAYBACK_POS_CHANGED,
> +                                               &pos);
> +
> +       return FALSE;
> +}
> +
>  static uint8_t avrcp_handle_register_notification(struct avrcp *session,
>                                                 struct avrcp_header *pdu,
>                                                 uint8_t transaction)
> @@ -1436,6 +1463,8 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
>         struct avrcp_player *player = target_get_player(session);
>         struct btd_device *dev = session->dev;
>         uint16_t len = ntohs(pdu->params_len);
> +       uint32_t interval;
> +       uint32_t pos;
>         uint64_t uid;
>         GList *settings;
>
> @@ -1467,6 +1496,20 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
>         case AVRCP_EVENT_TRACK_REACHED_START:
>                 len = 1;
>                 break;
> +       case AVRCP_EVENT_PLAYBACK_POS_CHANGED:
> +               len = 5;
> +
> +               interval = htole32(pdu->params[1]);
> +               player->pos_timer = g_timeout_add_seconds(interval,
> +                                                               interval_timeout, player);
> +
> +               pos = player_get_position(player);
> +               be32toh(pos);
> +               /* Fill the value for sending INTERIM response */
> +               memcpy(&pdu->params[1], &pos, sizeof(uint32_t));
> +
> +               break;
> +
>         case AVRCP_EVENT_SETTINGS_CHANGED:
>                 len = 1;
>                 settings = player_list_settings(player);
> @@ -2949,6 +2992,11 @@ static void player_destroy(gpointer data)
>         if (player->destroy)
>                 player->destroy(player->user_data);
>
> +       if (player->pos_timer != 0) {
> +               g_source_remove(player->pos_timer);
> +               player->pos_timer = 0;
> +       }
> +
>         g_slist_free(player->sessions);
>         g_free(player->path);
>         g_free(player->change_path);
> @@ -3407,6 +3455,7 @@ static void target_init(struct avrcp *session)
>                                 (1 << AVRCP_EVENT_TRACK_CHANGED) |
>                                 (1 << AVRCP_EVENT_TRACK_REACHED_START) |
>                                 (1 << AVRCP_EVENT_TRACK_REACHED_END) |
> +                               (1 << AVRCP_EVENT_PLAYBACK_POS_CHANGED) |
>                                 (1 << AVRCP_EVENT_SETTINGS_CHANGED);
>
>         if (target->version < 0x0104)
> diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h
> index 6ac5294..4816e4b 100644
> --- a/profiles/audio/avrcp.h
> +++ b/profiles/audio/avrcp.h
> @@ -74,6 +74,7 @@
>  #define AVRCP_EVENT_TRACK_CHANGED              0x02
>  #define AVRCP_EVENT_TRACK_REACHED_END          0x03
>  #define AVRCP_EVENT_TRACK_REACHED_START                0x04
> +#define        AVRCP_EVENT_PLAYBACK_POS_CHANGED        0x05
>  #define AVRCP_EVENT_SETTINGS_CHANGED           0x08
>  #define AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED  0x0a
>  #define AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED   0x0b
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index ef7b910..cc19c61 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -1305,6 +1305,9 @@ static gboolean set_status(struct media_player *mp, DBusMessageIter *iter)
>         mp->status = g_strdup(value);
>
>         avrcp_player_event(mp->player, AVRCP_EVENT_STATUS_CHANGED, mp->status);
> +       avrcp_player_event(mp->player,
> +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED,
> +                                       &mp->position);
>
>         return TRUE;
>  }
> @@ -1312,7 +1315,6 @@ static gboolean set_status(struct media_player *mp, DBusMessageIter *iter)
>  static gboolean set_position(struct media_player *mp, DBusMessageIter *iter)
>  {
>         uint64_t value;
> -       const char *status;
>
>         if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_INT64)
>                         return FALSE;
> @@ -1321,11 +1323,6 @@ static gboolean set_position(struct media_player *mp, DBusMessageIter *iter)
>
>         value /= 1000;
>
> -       if (value > get_position(mp))
> -               status = "forward-seek";
> -       else
> -               status = "reverse-seek";
> -

Not sure what is reasoning of removing these, the remote side may not
register for position change and in that case it would not get any
seek event to synchronize the position.

>         mp->position = value;
>         g_timer_start(mp->timer);
>
> @@ -1334,6 +1331,9 @@ static gboolean set_position(struct media_player *mp, DBusMessageIter *iter)
>         if (!mp->position) {
>                 avrcp_player_event(mp->player,
>                                         AVRCP_EVENT_TRACK_REACHED_START, NULL);
> +               avrcp_player_event(mp->player,
> +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED,
> +                                       &mp->position);
>                 return TRUE;
>         }
>
> @@ -1344,11 +1344,13 @@ static gboolean set_position(struct media_player *mp, DBusMessageIter *iter)
>         if (mp->position == UINT32_MAX || mp->position >= mp->duration) {
>                 avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_REACHED_END,
>                                                                         NULL);
> +               avrcp_player_event(mp->player,
> +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED,
> +                                       &mp->position);
>                 return TRUE;
>         }
> -
> -       /* Send a status change to force resync the position */
> -       avrcp_player_event(mp->player, AVRCP_EVENT_STATUS_CHANGED, status);
> +       avrcp_player_event(mp->player, AVRCP_EVENT_PLAYBACK_POS_CHANGED,
> +                                                               &mp->position);
>
>         return TRUE;
>  }
> @@ -1456,6 +1458,7 @@ static gboolean parse_player_metadata(struct media_player *mp,
>         int ctype;
>         gboolean title = FALSE;
>         uint64_t uid;
> +       uint32_t pos;
>
>         ctype = dbus_message_iter_get_arg_type(iter);
>         if (ctype != DBUS_TYPE_ARRAY)
> @@ -1521,9 +1524,11 @@ static gboolean parse_player_metadata(struct media_player *mp,
>         mp->position = 0;
>         g_timer_start(mp->timer);
>         uid = get_uid(mp);
> +       pos = get_position(mp);
>
>         avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED, &uid);
>         avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_REACHED_START, NULL);
> +       avrcp_player_event(mp->player, AVRCP_EVENT_PLAYBACK_POS_CHANGED, &pos);
>
>         return TRUE;
>  }
> --
> 1.9.1

Overall EVENT_PLAYBACK_POS_CHANGED cause more problems than it solves,
because position is already part of the play status and that normally
has to be queried anyway, also self synchronizing is way more
efficient since we can't estimate the speed when the playback is
fast-foward or rewind, and even if we could that would generate a lot
of traffic depending on what interval the remote side wants to be
updated.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH 1/2] android/pts: Update PICS and PIXIT for GATT
From: Szymon Janc @ 2014-10-20 10:11 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413463262-4090-1-git-send-email-marcin.kraglak@tieto.com>

Hi Marcin,

On Thursday 16 of October 2014 14:41:01 Marcin Kraglak wrote:
> ---
>  android/pics-gatt.txt  | 2 +-
>  android/pixit-gatt.txt | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/android/pics-gatt.txt b/android/pics-gatt.txt
> index 0cd67b6..6ff9c55 100644
> --- a/android/pics-gatt.txt
> +++ b/android/pics-gatt.txt
> @@ -1,6 +1,6 @@
>  GATT PICS for the PTS tool.
>  
> -PTS version: 5.2
> +PTS version: 5.3
>  
>  * - different than PTS defaults
>  
> diff --git a/android/pixit-gatt.txt b/android/pixit-gatt.txt
> index 08a80a3..191e2c9 100644
> --- a/android/pixit-gatt.txt
> +++ b/android/pixit-gatt.txt
> @@ -1,6 +1,6 @@
>  GATT PIXIT for the PTS tool.
>  
> -PTS version: 5.2
> +PTS version: 5.3
>  
>  * - different than PTS defaults
>  & - should be set to IUT Bluetooth address
> 

Both patches applied, thanks.

-- 
Best regards, 
Szymon Janc

^ permalink raw reply

* Re: [PATCH] android/pts: Update PTS files for HFP
From: Szymon Janc @ 2014-10-20 10:10 UTC (permalink / raw)
  To: Sebastian Chlad; +Cc: linux-bluetooth
In-Reply-To: <1413546696-25699-1-git-send-email-sebastian.chlad@tieto.com>

Hi Sebastian,

On Friday 17 of October 2014 13:51:36 Sebastian Chlad wrote:
> PICS and PIXITs updated to PTS 5.3. Regression done for Android
> 4.4.4.
> ---
>  android/pics-hfp.txt  | 22 ++++++++++++++++++++--
>  android/pixit-hfp.txt |  5 ++++-
>  android/pts-hfp.txt   | 14 ++++++++------
>  3 files changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/android/pics-hfp.txt b/android/pics-hfp.txt
> index acbd25c..65f5112 100644
> --- a/android/pics-hfp.txt
> +++ b/android/pics-hfp.txt
> @@ -1,6 +1,6 @@
>  HFP PICS for the PTS tool.
>  
> -PTS version: 5.2
> +PTS version: 5.3
>  
>  * - different than PTS defaults
>  # - not yet implemented/supported
> @@ -14,6 +14,7 @@ Parameter Name	Selected	Description
>  -------------------------------------------------------------------------------
>  TSPC_HFP_0_1	False		Version: Hands-Free Profile v1.5 (O.1)
>  TSPC_HFP_0_2	True (*)	Version: Hands-Free Profile v1.6 (O.1)
> +TSPC_HFP_0_3	False		Version: Hands-Free Profile v1.7 (O.1)
>  -------------------------------------------------------------------------------
>  O.1: It is mandatory to support only one of the adopted versions.
>  -------------------------------------------------------------------------------
> @@ -83,8 +84,10 @@ TSPC_HFP_2_21c	True (*)	Enhanced Call Status with limited network
>  					notification (C.4)
>  TSPC_HFP_2_22	False		Support for automatic link loss recovery (O)
>  TSPC_HFP_2_23	True (*)	Individual Indicator Activation (C.9)
> -TSPC_HFP_2_24	True (*)s	Wide Band Speech service (C.8)
> +TSPC_HFP_2_24	True (*)	Wide Band Speech service (C.8)
>  TSPC_HFP_2_25	False		Support roaming function (O)
> +TSPC_HFP_2_26	False		HF Indicators (C.11)
> +TSPC_HFP_2_27	False		Support CVSD eSCO s4 setting (C.12)
>  -------------------------------------------------------------------------------
>  C.1:  The AG must support one of item TSPC_HFP_2_4a or TSPC_HFP_2_4b
>  C.2:  Mandatory if TSPC_HFP_2_12is TRUE; otherwise excluded
> @@ -96,6 +99,10 @@ C.7:  Mandatory if TSPC_HFP_2_24 otherwise excluded
>  C.8:  Excluded if TSPC_HFP_0_1 otherwise optional
>  C.9:  Excluded if TSPC_HFP_0_1 otherwise mandatory
>  C.10: Mandatory if TSPC_HFP_2_24 otherwise optional
> +C.11: Optional IF HFP v1.5 (TSPC_HFP_0_1) OR HFP v1.6 (TSPC_HFP_0_2) is NOT
> +	supported, otherwise Excluded.
> +C.12: Excluded IF HFP v1.5 (TSPC_HFP_0_1) OR HFP v1.6 (TSPC_HFP_0_2) is
> +	supported, otherwise Mandatory.
>  -------------------------------------------------------------------------------
>  
>  
> @@ -161,6 +168,8 @@ TSPC_HFP_3_21b	False		Enhanced Call Control (C.2)
>  TSPC_HFP_3_22	False		Support for automatic link loss recovery (O)
>  TSPC_HFP_3_23	False		Individual Indicator Activation (C.6)
>  TSPC_HFP_3_24	False		Wide Band Speech service (C.6)
> +TSPC_HFP_3_25	False		HF Indicators (C.8)
> +TSPC_HFP_3_36	False		Support CVSD eSCO S4 setting (C.9)
>  -------------------------------------------------------------------------------
>  C.1: Mandatory if TSPC_HFP_3_12; otherwise excluded
>  C.2: Optional if TSPC_HFP_3_12; otherwise excluded
> @@ -169,6 +178,10 @@ C.4: Mandatory if TSPC_HFP_3_18a, otherwise optional
>  C.5: Mandatory if TSPC_HFP_3_24 otherwise excluded
>  C.6: Excluded if TSPC_HFP_0_1 otherwise optional
>  C.7: Mandatory if TSPC_HFP_3_24 otherwise optional
> +C.8: Optional IF HFP v1.5 (TSPC_HFP_0_1) OR HFP v1.6 (TSPC_HFP_0_2) is NOT
> +	supported, otherwise Excluded.
> +C.9: Excluded IF HFP v1.5 (TSPC_HFP_0_1) OR HFP v1.6 (TSPC_HFP_0_2) is
> +	supported, otherwise Mandatory.
>  -------------------------------------------------------------------------------
>  
>  
> @@ -178,9 +191,14 @@ Parameter Name	Selected	Description
>  -------------------------------------------------------------------------------
>  TSPC_HFP_4_1	True		CVSD audio coding over SCO (M)
>  TSPC_HFP_4_2	True (*)	mSBC audio coding over eSCO (C.1)
> +TSPC_HFP_4_3	True (*)	CVSD audio coding over eSCO (Initiating) (C.2)
> +TSPC_HFP_4_2	True (*)	CVSD audio coding over eSCO (Accepting) (C.2)
>  -------------------------------------------------------------------------------
>  C.1: Mandatory if Wide band speech service is supported TSPC_HFP_2_24 or
>  	TSPC_HFP_3_24, otherwise excluded
> +C.2: Mandatory IF TPSC_HFP_2_3b (eSCO support in Audio Connection - AG) OR
> +	TSPC_HFP_3_3b (eSCO support in Audio Connection - HF); otherwise
> +	Excluded.
>  -------------------------------------------------------------------------------
>  
>  
> diff --git a/android/pixit-hfp.txt b/android/pixit-hfp.txt
> index eaccc75..bf1b7e5 100644
> --- a/android/pixit-hfp.txt
> +++ b/android/pixit-hfp.txt
> @@ -1,6 +1,6 @@
>  HFP PIXIT for the PTS tool.
>  
> -PTS version: 5.2
> +PTS version: 5.3
>  
>  * - different than PTS defaults
>  & - should be set to IUT Bluetooth address
> @@ -35,4 +35,7 @@ TSPX_no_fail_verdict					FALSE
>  TSPX_network_supports_correct_call_and_callstatus	TRUE
>  TSPX_secure_simple_pairing_pass_key_confirmation	FALSE
>  TSPX_AG_match_tester_BRSF_codec_negotiation_bit		FALSE
> +TSPX_HFP_Unsupported_HF_Indicator_1			99
> +TSPX_HFP_Supported_HF_Indiccator_1			1
> +TSPX_Automation						FALSE
>  -------------------------------------------------------------------------------
> diff --git a/android/pts-hfp.txt b/android/pts-hfp.txt
> index a281d6b..8d75766 100644
> --- a/android/pts-hfp.txt
> +++ b/android/pts-hfp.txt
> @@ -1,7 +1,7 @@
>  PTS test results for HFP
>  
> -PTS version: 5.2
> -Tested: 14-Jul-2014
> +PTS version: 5.3
> +Tested: 16-October-2014
>  Android version: 4.4.4
>  
>  Results:
> @@ -28,6 +28,7 @@ TC_AG_ACS_BV_08_I	PASS
>  TC_AG_ACS_BV_10_I	N/A
>  TC_AG_ACS_BV_11_I	PASS
>  TC_AG_ACS_BI_14_I	PASS
> +TC_AG_ACS_BI_16_I	N/A
>  TC_AG_ACR_BV_01_I	PASS
>  TC_AG_ACR_BV_02_I	PASS
>  TC_AG_CLI_BV_01_I	PASS
> @@ -66,7 +67,7 @@ TC_AG_ENO_BV_02_I	N/A
>  TC_AG_VRA_BV_01_I	PASS
>  TC_AG_VRA_BV_02_I	PASS
>  TC_AG_VRA_BI_01_I	PASS
> -TC_AG_VRD_BV_01_I	PASS
> +TC_AG_VRD_BV_01_I	N/A
>  TC_AG_VTG_BV_01_I	N/A
>  TC_AG_TDC_BV_01_I	PASS
>  TC_AG_RSV_BV_01_I	PASS
> @@ -98,6 +99,8 @@ TC_AG_SLC_BV_04_C	PASS
>  TC_AG_SLC_BV_05_I	PASS
>  TC_AG_SLC_BV_06_I	PASS
>  TC_AG_SLC_BV_07_I	PASS
> +TC_AG_SLC_BV_09_I	N/A
> +TC_AG_SLC_BV_10_I	N/A
>  TC_AG_ACC_BV_08_I	INC	Possible PTS issue #12039
>  TC_AG_ACC_BV_09_I	PASS
>  TC_AG_ACC_BV_10_I	INC	Possible PTS issue #12039
> @@ -120,7 +123,8 @@ TC_AG_IID_BV_04_I	PASS
>  TC_AG_IIC_BV_01_I	PASS
>  TC_AG_IIC_BV_02_I	PASS
>  TC_AG_IIC_BV_03_I	PASS
> -
> +TC_AG_HFI_BV_02_I	N/A
> +TC_AG_HFI_BV_03_I	N/A
>  TC_HF_OOR_BV_01_I	N/A
>  TC_HF_OOR_BV_02_I	N/A
>  TC_HF_TRS_BV_01_I	N/A
> @@ -223,7 +227,6 @@ TC_HF_SDP_BV_03_C	N/A
>  TC_HF_ATAH_BV_01_I	N/A
>  TC_HF_OCA_BV_01_I	N/A
>  TC_HF_IIA_BV_04_I	N/A
> -
>  TC_AG_COD_BV_02_I	PASS
>  TC_AG_ATAH_BV_03_I	PASS
>  TC_AG_ATA_BV_03_I	PASS
> @@ -236,7 +239,6 @@ TC_AG_ICA_BV_09_I	PASS
>  TC_AG_VRA_BV_03_I	PASS
>  TC_AG_OCA_BV_01_I	PASS
>  TC_AG_TCA_BV_06_I	PASS
> -
>  TC_HF_ATAH_BV_03_I	N/A
>  TC_HF_ATA_BV_03_I	N/A
>  TC_HF_ATH_BV_09_I	N/A
> 

Applied, thanks.

-- 
Best regards, 
Szymon Janc

^ permalink raw reply

* Re: [PATCH] android/tester: Add missing hook removal
From: Szymon Janc @ 2014-10-20 10:10 UTC (permalink / raw)
  To: Jakub Tyszkowski; +Cc: linux-bluetooth
In-Reply-To: <1413547812-12345-1-git-send-email-jakub.tyszkowski@tieto.com>

Hi Jakub,

On Friday 17 of October 2014 14:10:12 Jakub Tyszkowski wrote:
> Missing hook removal was resulting in memory leak:
> 936 bytes in 39 blocks are definitely lost in loss record 42 of 45
> ==15026==    at 0x4C2AB80: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==15026==    by 0x407D60: btdev_add_hook (btdev.c:3226)
> ==15026==    by 0x40EB75: read_info_callback (tester-main.c:364)
> ==15026==    by 0x4142B5: request_complete (mgmt.c:245)
> ==15026==    by 0x41441A: can_read_data (mgmt.c:349)
> ==15026==    by 0x41663C: read_callback (io-glib.c:170)
> ==15026==    by 0x5083CE4: g_main_context_dispatch (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
> ==15026==    by 0x5084047: ??? (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
> ==15026==    by 0x5084309: g_main_loop_run (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0)
> ==15026==    by 0x4162D0: tester_run (tester.c:815)
> ==15026==    by 0x40263E: main (tester-main.c:2716)
> ---
>  android/tester-main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/android/tester-main.c b/android/tester-main.c
> index a804f11..fc1de06 100644
> --- a/android/tester-main.c
> +++ b/android/tester-main.c
> @@ -212,6 +212,9 @@ static void test_post_teardown(const void *test_data)
>  {
>  	struct test_data *data = tester_get_data();
>  
> +	/* remove hook for encryption change */
> +	hciemu_del_hook(data->hciemu, HCIEMU_HOOK_POST_EVT, 0x08);
> +
>  	hciemu_unref(data->hciemu);
>  	data->hciemu = NULL;
>  
> 

Applied, thanks.

-- 
Best regards, 
Szymon Janc

^ permalink raw reply

* Re: [PATCH] android/pts: Update PTS files for DIS
From: Szymon Janc @ 2014-10-20 10:08 UTC (permalink / raw)
  To: Sebastian Chlad; +Cc: linux-bluetooth
In-Reply-To: <1413551299-32329-1-git-send-email-sebastian.chlad@tieto.com>

Hi Sebastian,

On Friday 17 of October 2014 15:08:19 Sebastian Chlad wrote:
> PICS and PIXITs updated to PTS 5.3. Regression done for Android
> 4.4.4.
> ---
>  android/pics-dis.txt  | 2 +-
>  android/pixit-dis.txt | 4 ++--
>  android/pts-dis.txt   | 7 +++++--
>  3 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/android/pics-dis.txt b/android/pics-dis.txt
> index 0a1ab3f..169173d 100644
> --- a/android/pics-dis.txt
> +++ b/android/pics-dis.txt
> @@ -1,6 +1,6 @@
>  DIS PICS for the PTS tool.
>  
> -PTS version: 5.2
> +PTS version: 5.3
>  
>  * - different than PTS defaults
>  
> diff --git a/android/pixit-dis.txt b/android/pixit-dis.txt
> index c29ec36..71729fe 100644
> --- a/android/pixit-dis.txt
> +++ b/android/pixit-dis.txt
> @@ -1,6 +1,6 @@
>  DIS PIXIT for the PTS tool.
>  
> -PTS version: 5.2
> +PTS version: 5.3
>  
>  * - different than PTS defaults
>  & - should be set to IUT Bluetooth address
> @@ -19,7 +19,7 @@ TSPX_delete_link_key					FALSE
>  TSPX_pin_code						0000
>  TSPX_use_dynamic_pin					FALSE
>  TSPX_delete_ltk						FALSE
> -TSPX_security_enabled					TRUE
> +TSPX_security_enabled					TRUE (*)
>  TSPX_iut_setup_att_over_br_edr				FALSE
>  TSPX_tester_appearance					0000
>  -------------------------------------------------------------------------------
> diff --git a/android/pts-dis.txt b/android/pts-dis.txt
> index 8489430..215baa2 100644
> --- a/android/pts-dis.txt
> +++ b/android/pts-dis.txt
> @@ -1,7 +1,7 @@
>  PTS test results for DIS
>  
> -PTS version: 5.2
> -Tested: 29-July-2014
> +PTS version: 5.3
> +Tested: 18-October-2014
>  Android version: 4.4.4
>  
>  Results:
> @@ -11,6 +11,9 @@ INC	test is inconclusive
>  N/A	test is disabled due to PICS setup
>  NONE	test result is none
>  
> +NOTE: DIS testing might require some extra properties to be set. Please refer
> +to the README file in android folder. Section: Customization.
> +
>  -------------------------------------------------------------------------------
>  Test Name		Result	Notes
>  -------------------------------------------------------------------------------
> 

Patch applied, thanks.

-- 
Best regards, 
Szymon Janc

^ permalink raw reply

* Re: [PATCH] android/pts: Update PAN PTS tests
From: Szymon Janc @ 2014-10-20 10:08 UTC (permalink / raw)
  To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
In-Reply-To: <1413560573-3261-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>

Hi Grzegorz,

On Friday 17 of October 2014 17:42:53 Grzegorz Kolodziejczyk wrote:
> Checked and tested against PTS 5.3
> ---
>  android/pics-pan.txt  | 2 +-
>  android/pixit-pan.txt | 2 +-
>  android/pts-pan.txt   | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/android/pics-pan.txt b/android/pics-pan.txt
> index 20b91c7..7c4939a 100644
> --- a/android/pics-pan.txt
> +++ b/android/pics-pan.txt
> @@ -1,6 +1,6 @@
>  PAN PICS for the PTS tool.
>  
> -PTS version: 5.2
> +PTS version: 5.3
>  
>  * - different than PTS defaults
>  # - not yet implemented/supported
> diff --git a/android/pixit-pan.txt b/android/pixit-pan.txt
> index a4233c6..680abb2 100644
> --- a/android/pixit-pan.txt
> +++ b/android/pixit-pan.txt
> @@ -1,6 +1,6 @@
>  PAN PIXIT for the PTS tool.
>  
> -PTS version: 5.2
> +PTS version: 5.3
>  
>  * - different than PTS defaults
>  & - should be set to IUT or PTS Bluetooth address respectively
> diff --git a/android/pts-pan.txt b/android/pts-pan.txt
> index 0af186c..dac1dec 100644
> --- a/android/pts-pan.txt
> +++ b/android/pts-pan.txt
> @@ -1,7 +1,7 @@
>  PTS test results for PAN
>  
> -PTS version: 5.2
> -Tested: 05-August-2014
> +PTS version: 5.3
> +Tested: 17-October-2014
>  Android version: 4.4.4
>  
>  Results:
> 

Applied. Thanks.

-- 
Best regards, 
Szymon Janc

^ permalink raw reply

* Re: [PATCH 1/2] android/pts: Update PICS and PIXIT for IOPT
From: Szymon Janc @ 2014-10-20 10:07 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413793642-5431-1-git-send-email-marcin.kraglak@tieto.com>

Hi Marcin,

On Monday 20 of October 2014 10:27:21 Marcin Kraglak wrote:
> ---
>  android/pics-iopt.txt  | 2 +-
>  android/pixit-iopt.txt | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/android/pics-iopt.txt b/android/pics-iopt.txt
> index 61b5697..0b7446e 100644
> --- a/android/pics-iopt.txt
> +++ b/android/pics-iopt.txt
> @@ -1,6 +1,6 @@
>  IOPT PICS for the PTS tool.
>  
> -PTS version: 5.2
> +PTS version: 5.3
>  
>  * - different than PTS defaults
>  # - not yet implemented/supported
> diff --git a/android/pixit-iopt.txt b/android/pixit-iopt.txt
> index 85a8fcb..53837de 100644
> --- a/android/pixit-iopt.txt
> +++ b/android/pixit-iopt.txt
> @@ -1,6 +1,6 @@
>  IOPT PIXIT for the PTS tool.
>  
> -PTS version: 5.2
> +PTS version: 5.3
>  
>  * - different than PTS defaults
>  & - should be set to IUT Bluetooth address
> 

Applied. Thanks.

-- 
Best regards, 
Szymon Janc

^ permalink raw reply

* Re: [PATCH bluetooth-next 0/5] Move skb delivery out of IPHC.
From: Martin Townsend @ 2014-10-20 10:06 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Martin Townsend, linux-bluetooth, linux-wpan, marcel,
	jukka.rissanen
In-Reply-To: <20141020091043.GC5404@omega>

Hi Alex,

I think if I put the skb free stuff in lowpan_give_skb_to_devices then it should all be fixed.

- lowpan_process_data or now lowpan_header_decompress will either
     - always free the skb on error and return an error code
     - or return 0.  So there is no chance of NET_RX_FOO codes being returned as we have moved skb deilvery out of this path.

- This means that lowpan_recv can now checks to see if ret < 0, I could change this to ret != 0 if you want but the outcome is the same, if decompression fails then skb has been freed so goto drop.

- This leaves the last part lowpan_give_skb_to_devices must now free or consume the skb (which I've just added).

Hopefully this sorts out the original problem? or have I missed something.

My next patch series will remove all the kfree_skb's from decompression but I don't think this fixes anything just makes the code more maintainable in that we don't have to worry about making sure all error paths are covered.

Let me know what you think and then I'll send v2.

- Martin.



On 20/10/14 10:10, Alexander Aring wrote:
> On Mon, Oct 20, 2014 at 10:02:17AM +0100, Martin Townsend wrote:
>> Hi Alex,
>>
>> I've completely forgotten what the original problem was it was so long ago :)  , let me dig through the old emails and see if I can jog my memory. 
>>
>> My intention was to do the kfree_skb patch in my next submission but I'll see what I can do.
>>
> yea, the problem a general error handling issue in mainly two parts:
>
>  - detection errors. Means the NET_RX_FOO conversion to errno. But you
>    put out the netif_rx call outside of lowpan_header_create which is
>    very well. I approve that one. Now it's important that the "lowpan
>    give to upper layer" functions not returning ERRNO's and NET_RX_FOO.
>
>    Again, errno is a negative value and NET_RX_FOO is 0 or 1. We can't
>    check on a error with if (ret < 0) or (ret == NET_RX_DROP). Also
>    (ret != NET_RX_SUCCESS) will not work because it's "1" and "0"
>    indicate successful or sometimes not successful because NET_RX_DROP
>    was returned and this is "0". Lot of confusing here.
>
>    For the "lowpan give to upper layer" functions doesn't matter if
>    errno is returned or NET_RX_FOO, but don't a mix of both!
>    The "lowpan_header_create" should return errno's.
>
>  - Second issue was a complete wrong reaction on error handling and
>    memory managment with "kfree_skb" which needs a fix of the above one
>    at first to introduce a correct error handling.
>
>
> Now I see a little bit the fix of the first one. But not the second one.
> Or should these things all fine now, otherwise I will take a more
> careful review.
>
> - Alex
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 3/5] android/map-client: Add support for get remote MAS instances
From: Grzegorz Kolodziejczyk @ 2014-10-20 10:01 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1413383281-17292-4-git-send-email-grzegorz.kolodziejczyk@tieto.com>

Hi,

On 15 October 2014 16:27, Grzegorz Kolodziejczyk
<grzegorz.kolodziejczyk@tieto.com> wrote:
>
> This allows to get remote mas instances. In case of wrong sdp record
> fail status will be returned in notification.
> ---
>  android/map-client.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 130 insertions(+), 1 deletion(-)
>
> diff --git a/android/map-client.c b/android/map-client.c
> index 142c680..12f600b 100644
> --- a/android/map-client.c
> +++ b/android/map-client.c
> @@ -30,21 +30,150 @@
>  #include <stdint.h>
>  #include <glib.h>
>
> +#include "lib/sdp.h"
> +#include "lib/sdp_lib.h"
> +#include "src/sdp-client.h"
> +
>  #include "ipc.h"
>  #include "lib/bluetooth.h"
>  #include "map-client.h"
>  #include "src/log.h"
>  #include "hal-msg.h"
> +#include "ipc-common.h"
> +#include "utils.h"
> +#include "src/shared/util.h"
>
>  static struct ipc *hal_ipc = NULL;
>  static bdaddr_t adapter_addr;
>
> +static int fill_mce_inst(void *buf, int32_t id, int32_t scn, int32_t msg_type,
> +                                       const void *name, uint8_t name_len)
> +{
> +       struct hal_map_client_mas_instance *inst = buf;
> +
> +       inst->id = id;
> +       inst->scn = scn;
> +       inst->msg_types = msg_type;
> +       inst->name_len = name_len;
> +
> +       if (name_len)
> +               memcpy(inst->name, name, name_len);
> +
> +       return sizeof(*inst) + name_len;
> +}
> +
> +static void map_client_sdp_search_cb(sdp_list_t *recs, int err, gpointer data)
> +{
> +       uint8_t buf[IPC_MTU];
> +       struct hal_ev_map_client_remote_mas_instances *ev = (void *) buf;
> +       bdaddr_t *dst = data;
> +       sdp_list_t *list, *protos;
> +       uint8_t status;
> +       int32_t id, scn, msg_type, name_len, num_instances = 0;
> +       char *name;
> +       size_t size;
> +
> +       size = sizeof(*ev);
> +       bdaddr2android(dst, &ev->bdaddr);
> +
> +       if (err < 0) {
> +               error("mce: Unable to get SDP record: %s", strerror(-err));
> +               status = HAL_STATUS_FAILED;
> +               goto fail;
> +       }
> +
> +       for (list = recs; list != NULL; list = list->next) {
> +               sdp_record_t *rec = list->data;
> +               sdp_data_t *data;
> +
> +               data = sdp_data_get(rec, SDP_ATTR_MAS_INSTANCE_ID);
> +               if (!data) {
> +                       error("mce: cannot get mas instance id");
> +                       continue;
> +               }
> +
> +               id = data->val.uint8;
> +
> +               data = sdp_data_get(rec, SDP_ATTR_SVCNAME_PRIMARY);
> +               if (!data) {
> +                       error("mce: cannot get mas instance name");
> +                       continue;
> +               }
> +
> +               name = data->val.str;
> +               name_len = data->unitSize;
> +
> +               data = sdp_data_get(rec, SDP_ATTR_SUPPORTED_MESSAGE_TYPES);
> +               if (!data) {
> +                       error("mce: cannot get mas instance msg type");
> +                       continue;
> +               }
> +
> +               msg_type = data->val.uint8;
> +
> +               if (sdp_get_access_protos(rec, &protos) < 0) {
> +                       error("mce: cannot get mas instance sdp protocol list");
> +                       continue;
> +               }
> +
> +               scn = sdp_get_proto_port(protos, RFCOMM_UUID);
> +
> +               sdp_list_foreach(protos, (sdp_list_func_t) sdp_list_free, NULL);
> +               sdp_list_free(protos, NULL);
> +
> +               if (!scn) {
> +                       error("mce: cannot get mas instance rfcomm channel");
> +                       continue;
> +               }
> +
> +               size += fill_mce_inst(buf + size, id, scn, msg_type, name,
> +                                                               name_len);
> +               num_instances++;
> +       }
> +
> +       status = HAL_STATUS_SUCCESS;
> +
> +fail:
> +       ev->num_instances = num_instances;
> +       ev->status = status;
> +
> +       ipc_send_notif(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT,
> +                       HAL_EV_MAP_CLIENT_REMOTE_MAS_INSTANCES, size, buf);
> +
> +       free(dst);


This "free(dst)" should also be removed while using bt_destroy function.

>
> +}
> +
>  static void handle_get_instances(const void *buf, uint16_t len)
>  {
> +       const struct hal_cmd_map_client_get_instances *cmd = buf;
> +       uint8_t status;
> +       bdaddr_t *dst;
> +       uuid_t uuid;
> +
>         DBG("");
>
> +       dst = new0(bdaddr_t, 1);
> +       if (!dst) {
> +               error("mce: Fail to allocate cb data");
> +               status = HAL_STATUS_FAILED;
> +               goto failed;
> +       }
> +
> +       android2bdaddr(&cmd->bdaddr, dst);
> +       sdp_uuid16_create(&uuid, MAP_MSE_SVCLASS_ID);
> +
> +       if (bt_search_service(&adapter_addr, dst, &uuid,
> +                               map_client_sdp_search_cb, dst, free, 0)) {
> +               error("mce: Failed to search SDP details");
> +               status = HAL_STATUS_FAILED;
> +               goto failed;
> +       }
> +
> +       status = HAL_STATUS_SUCCESS;
> +
> +failed:
>         ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT,
> -                       HAL_OP_MAP_CLIENT_GET_INSTANCES, HAL_STATUS_FAILED);
> +                               HAL_OP_MAP_CLIENT_GET_INSTANCES, status);
>  }
>
>  static const struct ipc_handler cmd_handlers[] = {
> --
> 1.9.3
>

Best regards,
Grzegorz

^ permalink raw reply

* RE: Use HFP as sco sink and source at the same time
From: Zheng, Wu @ 2014-10-20  9:16 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: 'linux-bluetooth@vger.kernel.org'
In-Reply-To: <CABBYNZKoNVhj91NQ1e3PF2ORaAcMs4NsX300bj0cLic1=Joicw@mail.gmail.com>

SGkgTHVpeiwNCg0KVGhhbmtzIGZvciB5b3VyIHJlc3BvbnNlLg0KDQpGb3IgaHR0cHM6Ly9idWdz
LnRpemVuLm9yZy9qaXJhL2Jyb3dzZS9UQy0xNjAsIHdlIGNhbiBnZXQgdGhlIHJlbGF0ZWQgbG9n
IG9mIHB1bHNlYXVkaW8uDQoNCltwdWxzZWF1ZGlvXSByb3V0ZXIuYzogcm91dGluZyAnZGlhbGVy
JyA9PiAnQmx1ZXRvb3RoIE1vbm8gSGFuZHNmcmVlJw0KRDogW3B1bHNlYXVkaW9dIGF1ZGlvbWdy
LmM6IHNraXAgcmVnaXN0ZXJpbmcgbm9kZXMgd2hpbGUgdGhlIGRvbWFpbiBpcyBkb3duDQpEOiBb
cHVsc2VhdWRpb10gY29uc3RyYWluLmM6IG5vZGUgJ0JsdWV0b290aCBNb25vIEhhbmRzZnJlZScg
YWRkZWQgdG8gY29uc3RyYWluIHByb2ZpbGUvYmx1ZXpfY2FyZC4wMF8xNl80NF9GRF8zNV9EMg0K
RDogW3B1bHNlYXVkaW9dIGRpc2NvdmVyLmM6IGNhcmQgJ2JsdWV6X2NhcmQuMDBfMTZfNDRfRkRf
MzVfRDInIGRlZmF1bHQgcHJvZmlsZSAnb2ZmJw0KRDogW3B1bHNlYXVkaW9dIGRpc2NvdmVyLmM6
IHNjaGVkdWxpbmcgY2FyZCBjaGVjaw0KSTogW3B1bHNlYXVkaW9dIG1vZHVsZS5jOiBMb2FkZWQg
Im1vZHVsZS1ibHVlejUtZGV2aWNlIiAoaW5kZXg6ICMyMTsgYXJndW1lbnQ6ICJwYXRoPS9vcmcv
Ymx1ZXovaGNpMC9kZXZfMDBfMTZfNDRfRkRfMzVfRDIiKS4NCkQ6IFtwdWxzZWF1ZGlvXSBibHVl
ejUtdXRpbC5jOiBUcmFuc3BvcnQgL29yZy9ibHVlei9oY2kwL2Rldl8wMF8xNl80NF9GRF8zNV9E
Mi9mZDEgYXZhaWxhYmxlIGZvciBwcm9maWxlIGEyZHBfc2luaw0KRDogW3B1bHNlYXVkaW9dIGRp
c2NvdmVyLmM6IGNhcmQgY2hlY2sgc3RhcnRzDQpEOiBbcHVsc2VhdWRpb10gZGlzY292ZXIuYzog
Y2FyZCAnYmx1ZXpfY2FyZC4wMF8xNl80NF9GRF8zNV9EMicgaGFzIG5vIHNpbmtzL3NvdXJjZXMu
IERvIHJvdXRpbmcgLi4uDQpEOiBbcHVsc2VhdWRpb10gcm91dGVyLmM6IHVzaW5nICdwaG9uZScg
cm91dGVyIGdyb3VwIHdoZW4gcm91dGluZyAnZGlhbGVyJw0KRDogW3B1bHNlYXVkaW9dIGNvbnN0
cmFpbi5jOiBhcHBseWluZyBjb25zdHJhaW4gcHJvZmlsZS9ibHVlel9jYXJkLjAwXzE2XzQ0X0ZE
XzM1X0QyDQpEOiBbcHVsc2VhdWRpb10gY29uc3RyYWluLmM6ICAgIHVuYmxvY2tpbmcgJ0JsdWV0
b290aCBNb25vIEhhbmRzZnJlZScgaW4gdGFibGUgJ2RlZmF1bHRfZHJpdmVyJw0KRDogW3B1bHNl
YXVkaW9dIGNvbnN0cmFpbi5jOiAgICB1bmJsb2NraW5nICdCbHVldG9vdGggTW9ubyBIYW5kc2Zy
ZWUnIGluIHRhYmxlICdwaG9uZScNCkQ6IFtwdWxzZWF1ZGlvXSBjb25zdHJhaW4uYzogICAgYmxv
Y2tpbmcgJ0JsdWV0b290aCBTdGVyZW8gSGVhZHBob25lJyBpbiB0YWJsZSAnZGVmYXVsdF9kcml2
ZXInDQpEOiBbcHVsc2VhdWRpb10gY29uc3RyYWluLmM6ICAgIHVuYmxvY2tpbmcgJ0JsdWV0b290
aCBNb25vIEhhbmRzZnJlZScgaW4gdGFibGUgJ3Bob25lJw0KRDogW3B1bHNlYXVkaW9dIHJvdXRl
ci5jOiByb3V0aW5nICdkaWFsZXInID0+ICdCbHVldG9vdGggTW9ubyBIYW5kc2ZyZWUnDQpEOiBb
cHVsc2VhdWRpb10gYXVkaW9tZ3IuYzogaWdub3JpbmcgZGVmYXVsdCByb3V0ZSBkaWFsZXIgPT4g
Qmx1ZXRvb3RoIE1vbm8gSGFuZHNmcmVlOiBpbmNvbXBsZXRlIGlucHV0IG9yIG91dHB1dA0KRDog
W3B1bHNlYXVkaW9dIHN3aXRjaC5jOiBjaGFuZ2luZyBwcm9maWxlICdvZmYnID0+ICdoc3AnDQpX
OiBbcHVsc2VhdWRpb10gbW9kdWxlLWJsdWV6NS1kZXZpY2UuYzogUmVmdXNlZCB0byBzd2l0Y2gg
cHJvZmlsZSB0byBoc3A6IE5vdCBjb25uZWN0ZWQNCkQ6IFtwdWxzZWF1ZGlvXSBzd2l0Y2guYzog
Y2FuJ3Qgcm91dGUgdG8gJ0JsdWV0b290aCBNb25vIEhhbmRzZnJlZSc6IG5vIHNpbmsNCkQ6IFtw
dWxzZWF1ZGlvXSB2b2x1bWUuYzogcmVzZXQgdm9sdW1lIGNsYXNzZXMgb24gbm9kZSAnQmx1ZXRv
b3RoIE1vbm8gSGFuZHNmcmVlJw0KRDogW3B1bHNlYXVkaW9dIHZvbHVtZS5jOiBhZGQgdm9sdW1l
IGNsYXNzIDcgKFBob25lKSB0byBub2RlICdCbHVldG9vdGggTW9ubyBIYW5kc2ZyZWUnIChjbG1h
c2sgMHgwKQ0KRDogW3B1bHNlYXVkaW9dIHJvdXRlci5jOiB1c2luZyAncGhvbmUnIHJvdXRlciBn
cm91cCB3aGVuIHJvdXRpbmcgJ0NhciBLaXQnDQpEOiBbcHVsc2VhdWRpb10gcm91dGVyLmM6IHJv
dXRpbmcgJ0NhciBLaXQnID0+ICdCbHVldG9vdGggTW9ubyBIYW5kc2ZyZWUnDQpEOiBbcHVsc2Vh
dWRpb10gYXVkaW9tZ3IuYzogaWdub3JpbmcgZGVmYXVsdCByb3V0ZSBDYXIgS2l0ID0+IEJsdWV0
b290aCBNb25vIEhhbmRzZnJlZTogaW5jb21wbGV0ZSBpbnB1dCBvciBvdXRwdXQNCkQ6IFtwdWxz
ZWF1ZGlvXSBzd2l0Y2guYzogY2hhbmdpbmcgcHJvZmlsZSAnb2ZmJyA9PiAnaHNwJw0KVzogW3B1
bHNlYXVkaW9dIG1vZHVsZS1ibHVlejUtZGV2aWNlLmM6IFJlZnVzZWQgdG8gc3dpdGNoIHByb2Zp
bGUgdG8gaHNwOiBOb3QgY29ubmVjdGVkDQpEOiBbcHVsc2VhdWRpb10gc3dpdGNoLmM6IGNhbid0
IHJvdXRlIHRvICdCbHVldG9vdGggTW9ubyBIYW5kc2ZyZWUnOiBubyBzaW5rDQoNCk5vdCBtYWtl
IHN1cmUgaWYgdGhlIEJsdWV0b290aCBoYW5kbGUgdG8gcHVsc2VhdWRpbyBjYW4gd29yayB3ZWxs
IG9yIG5vdO+8nw0KDQpXaGF0IGRvIHlvdSB0aGluaz8NCg0KQmVzdCBSZWdhcmRzDQpaaGVuZyBX
dQ0KDQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBMdWl6IEF1Z3VzdG8gdm9u
IERlbnR6IFttYWlsdG86bHVpei5kZW50ekBnbWFpbC5jb21dIA0KU2VudDogTW9uZGF5LCBPY3Rv
YmVyIDIwLCAyMDE0IDU6MDMgUE0NClRvOiBaaGVuZywgV3UNCkNjOiBWb24gRGVudHosIEx1aXo7
IGxpbnV4LWJsdWV0b290aEB2Z2VyLmtlcm5lbC5vcmcNClN1YmplY3Q6IFJlOiBVc2UgSEZQIGFz
IHNjbyBzaW5rIGFuZCBzb3VyY2UgYXQgdGhlIHNhbWUgdGltZQ0KDQpIaSwNCg0KT24gTW9uLCBP
Y3QgMjAsIDIwMTQgYXQgMTE6MTUgQU0sIFpoZW5nLCBXdSA8d3UuemhlbmdAaW50ZWwuY29tPiB3
cm90ZToNCj4gSGkgTHVpeiwNCj4NCj4gSEZQIGlzIHVzZWQgb24gSVZJIGFuZCBtZWV0IHNvbWUg
aXNzdWVzLg0KPg0KPiBFWEFDVCBTVEVQUyBMRUFESU5HIFRPIFBST0JMRU06DQo+ID09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQo+IDEu
IHBhaXIgQlQgcGhvbmUgYnkgYmx1ZXRvb3RoY3RsDQo+IDIuIGxhdW5jaCBkaWFsZXIgYXBwDQo+
IDMuIG1ha2Ugdm9pY2UgY2FsbCBhbmQgYW5zd2VyLCB0aGUgdm9pY2UgY2FuIGJlIGhlYXJkIGZy
b20gSVZJIHNwZWFrZXIgDQo+IDQuIHBhaXIgYW5kIGNvbm5lY3QgQlQgaGVhZHNldCB3aXRoIGJs
dWV0b290aGN0bA0KPg0KPiBFWFBFQ1RFRCBPVVRDT01FOg0KPiA9PT09PT09PT09PT09PT09PT09
DQo+IFZvaWNlIHNlbmQgb3V0IGZyb20gdGhlIEJUIGhlYWRzZXQNCj4gQUNUVUFMIE9VVENPTUUN
Cj4gPT09PT09PT09PT09PT09PT09PQ0KPiBzb3VuZCBzdGlsbCBoZWFyZCBmcm9tIElWSSBzcGVh
a2VyLCBkb2Vzbid0IGF1dG8gc3dpdGNoIHRvIEJUIGhhbmRzZXQNCj4NCj4gSSBkb24ndCBtYWtl
IHN1cmUgaWYgQmx1ZXogY2FuIHN1cHBvcnQgaXQgb3Igbm90Pw0KPg0KPiBRdWVzdGlvbjogdGhl
IFNDT3MgY29ubmVjdGlvbiBvZiBIRlAgb24gSVZJIGNhbiBiZSB1c2VkIGFzIFNDTyBzaW5rIGFu
ZCBzb3VyY2UgYXQgdGhlIHNhbWUgdGltZT8NCj4NCj4gSXQgaXMgdGhlIGlzc3VlIG9mIGh0dHBz
Oi8vYnVncy50aXplbi5vcmcvamlyYS9icm93c2UvVEMtMTYwLg0KDQpZZXMsIGJ5IGRlc2lnbiBp
dCBpcyBkdXBsZXggc28gYm90aCBzaW5rIGFuZCBzb3VyY2Ugd29ya3Mgc2ltdWx0YW5lb3VzbHks
IHBlcmhhcHMgeW91IGFyZSB0YWxraW5nIGFib3V0IG9mIHNlbmRpbmcgdGhlIG1pY3JvcGhvbmUg
ZGF0YSB0byB0aGUgb3RoZXIgZW5kIHdoaWNoIGhhcyBwcm9iYWJseSBub3RoaW5nIHRvIGRvIHdp
dGggQmx1ZXRvb3RoIGJ1dCB3aXRoIGF1ZGlvIHJvdXRpbmcgbm90IGJlaW5nIHByb3Blcmx5IGNv
bmZpZ3VyZWQuDQoNCg0KLS0NCkx1aXogQXVndXN0byB2b24gRGVudHoNCg==

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox