* [RFC 0/1] avctp: Set browsing channel connection for ct
@ 2013-01-22 10:34 Alexandros Antonopoulos
2013-01-22 10:34 ` [RFC 1/1] " Alexandros Antonopoulos
0 siblings, 1 reply; 3+ messages in thread
From: Alexandros Antonopoulos @ 2013-01-22 10:34 UTC (permalink / raw)
To: linux-bluetooth
A few comments:
Handling channel creation for AVRCP:
For AVRCP >= 1.4 the browsing channel can be connected after the
control channel is connected successfully.
It's not clear for me what bluez should do in case the browsing
channel connection fails. There is nothing stopping bluez for still
providing avrcp 1.3 functionality over the control channel.
If the choice is to allow the use of the control channel even if the
browsing one fails then bluez needs the logic in
avrcp.c:state_changed(). If not then both channels must be destroyed
and the session must not be initialized.
State AVCTP_STATE_CONNECTED in state_changed has also to differentiate
between AVRCP <= 1.3 and AVRCP >= 1.4. In AVRCP <= 1.3 after the
control channel is created the avrcp session can be initialized.
In AVRCP >= 1.4 we need first to try and connect the browsing channel.
The session can be created if:
- The state is AVCTP_STATE_BROWSING_CONNECTED (if the browsing channel
was connected successfully),
- The state is AVCTP_STATE_CONNECTED and old_state is
AVCTP_STATE_BROWSING_CONNECTING (if the browsing channel connection
failed),
- The state is AVCTP_STATE_CONNECTED but avctp_connect_browsing fails.
Handling the feature mask:
Since bluez will support AVRCP 1.5 is the AVRCP_FEATURE_BROWSING
actually required? It was checked on the target side during the session
init.
Alexandros Antonopoulos (1):
avctp: Set browsing channel connection for ct
profiles/audio/avctp.c | 11 +++++++++++
profiles/audio/avctp.h | 4 +++-
profiles/audio/avrcp.c | 28 +++++++++++++++++++++++-----
profiles/audio/device.c | 4 ++++
4 files changed, 41 insertions(+), 6 deletions(-)
--
1.8.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [RFC 1/1] avctp: Set browsing channel connection for ct
2013-01-22 10:34 [RFC 0/1] avctp: Set browsing channel connection for ct Alexandros Antonopoulos
@ 2013-01-22 10:34 ` Alexandros Antonopoulos
2013-01-22 16:01 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 3+ messages in thread
From: Alexandros Antonopoulos @ 2013-01-22 10:34 UTC (permalink / raw)
To: linux-bluetooth
Add two extra states for the browsing channel and handle the connection
sequence in avrcp.c
---
profiles/audio/avctp.c | 11 +++++++++++
profiles/audio/avctp.h | 4 +++-
profiles/audio/avrcp.c | 28 +++++++++++++++++++++++-----
profiles/audio/device.c | 4 ++++
4 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index e65594d..753f8fe 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -476,6 +476,12 @@ static void avctp_set_state(struct avctp *session, avctp_state_t new_state)
case AVCTP_STATE_CONNECTED:
DBG("AVCTP Connected");
break;
+ case AVCTP_STATE_BROWSING_CONNECTING:
+ DBG("AVCTP Browsing Connecting");
+ break;
+ case AVCTP_STATE_BROWSING_CONNECTED:
+ DBG("AVCTP Browsing Connected");
+ break;
default:
error("Invalid AVCTP state %d", new_state);
return;
@@ -913,9 +919,12 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
(GIOFunc) session_browsing_cb, session);
+ avctp_set_state(session, AVCTP_STATE_BROWSING_CONNECTED);
return;
fail:
+ avctp_set_state(session, AVCTP_STATE_CONNECTED);
+
if (session->browsing) {
avctp_channel_destroy(session->browsing);
session->browsing = NULL;
@@ -1648,6 +1657,8 @@ int avctp_connect_browsing(struct avctp *session)
if (session->browsing != NULL)
return 0;
+ avctp_set_state(session, AVCTP_STATE_BROWSING_CONNECTING);
+
io = bt_io_connect(avctp_connect_browsing_cb, session, NULL, &err,
BT_IO_OPT_SOURCE_BDADDR,
adapter_get_address(session->server->adapter),
diff --git a/profiles/audio/avctp.h b/profiles/audio/avctp.h
index c25a3b3..d1fe693 100644
--- a/profiles/audio/avctp.h
+++ b/profiles/audio/avctp.h
@@ -67,7 +67,9 @@ struct avctp;
typedef enum {
AVCTP_STATE_DISCONNECTED = 0,
AVCTP_STATE_CONNECTING,
- AVCTP_STATE_CONNECTED
+ AVCTP_STATE_CONNECTED,
+ AVCTP_STATE_BROWSING_CONNECTING,
+ AVCTP_STATE_BROWSING_CONNECTED
} avctp_state_t;
typedef void (*avctp_state_cb) (struct audio_device *dev,
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 636d3e4..6cab45f 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2182,12 +2182,9 @@ static void session_tg_init(struct avrcp *session)
(1 << AVRCP_EVENT_TRACK_REACHED_END) |
(1 << AVRCP_EVENT_SETTINGS_CHANGED);
- if (session->version >= 0x0104) {
+ if (session->version >= 0x0104)
avrcp_register_notification(session,
AVRCP_EVENT_VOLUME_CHANGED);
- if (session->features & AVRCP_FEATURE_BROWSING)
- avctp_connect_browsing(session->conn);
- }
session->control_id = avctp_register_pdu_handler(session->conn,
AVC_OP_VENDORDEP,
@@ -2341,6 +2338,7 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
{
struct avrcp_server *server;
struct avrcp *session;
+ uint8_t browsing_failed = false;
server = find_server(servers, device_get_adapter(dev->btd_dev));
if (!server)
@@ -2367,8 +2365,28 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
if (session == NULL)
break;
- session->init(session);
+ if (session->version <= 0x0103) {
+ session->init(session);
+ break;
+ }
+
+ /*AVCRP >= 1.4*/
+ if (old_state == AVCTP_STATE_CONNECTING) {
+ if (avctp_connect_browsing(session->conn) != 0)
+ browsing_failed = true;
+ } else if (old_state == AVCTP_STATE_BROWSING_CONNECTING) {
+ browsing_failed = true;
+ }
+ if (browsing_failed)
+ session->init(session);
+ break;
+ case AVCTP_STATE_BROWSING_CONNECTED:
+ if (session == NULL)
+ break;
+
+ session->init(session);
+ break;
default:
return;
}
diff --git a/profiles/audio/device.c b/profiles/audio/device.c
index d4ba6d2..2aa22bb 100644
--- a/profiles/audio/device.c
+++ b/profiles/audio/device.c
@@ -289,6 +289,10 @@ static void device_avctp_cb(struct audio_device *dev,
break;
case AVCTP_STATE_CONNECTED:
break;
+ case AVCTP_STATE_BROWSING_CONNECTING:
+ break;
+ case AVCTP_STATE_BROWSING_CONNECTED:
+ break;
}
}
--
1.8.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC 1/1] avctp: Set browsing channel connection for ct
2013-01-22 10:34 ` [RFC 1/1] " Alexandros Antonopoulos
@ 2013-01-22 16:01 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2013-01-22 16:01 UTC (permalink / raw)
To: Alexandros Antonopoulos; +Cc: linux-bluetooth
Hi Alexandros,
On Tue, Jan 22, 2013 at 12:34 PM, Alexandros Antonopoulos
<alexandros.antonopoulos@oss.bmw-carit.de> wrote:
> Add two extra states for the browsing channel and handle the connection
> sequence in avrcp.c
>
> ---
> profiles/audio/avctp.c | 11 +++++++++++
> profiles/audio/avctp.h | 4 +++-
> profiles/audio/avrcp.c | 28 +++++++++++++++++++++++-----
> profiles/audio/device.c | 4 ++++
> 4 files changed, 41 insertions(+), 6 deletions(-)
>
> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> index e65594d..753f8fe 100644
> --- a/profiles/audio/avctp.c
> +++ b/profiles/audio/avctp.c
> @@ -476,6 +476,12 @@ static void avctp_set_state(struct avctp *session, avctp_state_t new_state)
> case AVCTP_STATE_CONNECTED:
> DBG("AVCTP Connected");
> break;
> + case AVCTP_STATE_BROWSING_CONNECTING:
> + DBG("AVCTP Browsing Connecting");
> + break;
> + case AVCTP_STATE_BROWSING_CONNECTED:
> + DBG("AVCTP Browsing Connected");
> + break;
> default:
> error("Invalid AVCTP state %d", new_state);
> return;
> @@ -913,9 +919,12 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
> G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
> (GIOFunc) session_browsing_cb, session);
>
> + avctp_set_state(session, AVCTP_STATE_BROWSING_CONNECTED);
> return;
>
> fail:
> + avctp_set_state(session, AVCTP_STATE_CONNECTED);
> +
> if (session->browsing) {
> avctp_channel_destroy(session->browsing);
> session->browsing = NULL;
> @@ -1648,6 +1657,8 @@ int avctp_connect_browsing(struct avctp *session)
> if (session->browsing != NULL)
> return 0;
>
> + avctp_set_state(session, AVCTP_STATE_BROWSING_CONNECTING);
> +
> io = bt_io_connect(avctp_connect_browsing_cb, session, NULL, &err,
> BT_IO_OPT_SOURCE_BDADDR,
> adapter_get_address(session->server->adapter),
> diff --git a/profiles/audio/avctp.h b/profiles/audio/avctp.h
> index c25a3b3..d1fe693 100644
> --- a/profiles/audio/avctp.h
> +++ b/profiles/audio/avctp.h
> @@ -67,7 +67,9 @@ struct avctp;
> typedef enum {
> AVCTP_STATE_DISCONNECTED = 0,
> AVCTP_STATE_CONNECTING,
> - AVCTP_STATE_CONNECTED
> + AVCTP_STATE_CONNECTED,
> + AVCTP_STATE_BROWSING_CONNECTING,
> + AVCTP_STATE_BROWSING_CONNECTED
> } avctp_state_t;
>
> typedef void (*avctp_state_cb) (struct audio_device *dev,
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 636d3e4..6cab45f 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -2182,12 +2182,9 @@ static void session_tg_init(struct avrcp *session)
> (1 << AVRCP_EVENT_TRACK_REACHED_END) |
> (1 << AVRCP_EVENT_SETTINGS_CHANGED);
>
> - if (session->version >= 0x0104) {
> + if (session->version >= 0x0104)
> avrcp_register_notification(session,
> AVRCP_EVENT_VOLUME_CHANGED);
> - if (session->features & AVRCP_FEATURE_BROWSING)
> - avctp_connect_browsing(session->conn);
> - }
>
> session->control_id = avctp_register_pdu_handler(session->conn,
> AVC_OP_VENDORDEP,
> @@ -2341,6 +2338,7 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
> {
> struct avrcp_server *server;
> struct avrcp *session;
> + uint8_t browsing_failed = false;
>
> server = find_server(servers, device_get_adapter(dev->btd_dev));
> if (!server)
> @@ -2367,8 +2365,28 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
> if (session == NULL)
> break;
>
> - session->init(session);
> + if (session->version <= 0x0103) {
> + session->init(session);
> + break;
> + }
> +
> + /*AVCRP >= 1.4*/
> + if (old_state == AVCTP_STATE_CONNECTING) {
> + if (avctp_connect_browsing(session->conn) != 0)
> + browsing_failed = true;
> + } else if (old_state == AVCTP_STATE_BROWSING_CONNECTING) {
> + browsing_failed = true;
> + }
>
> + if (browsing_failed)
> + session->init(session);
> + break;
> + case AVCTP_STATE_BROWSING_CONNECTED:
> + if (session == NULL)
> + break;
> +
> + session->init(session);
> + break;
> default:
> return;
> }
> diff --git a/profiles/audio/device.c b/profiles/audio/device.c
> index d4ba6d2..2aa22bb 100644
> --- a/profiles/audio/device.c
> +++ b/profiles/audio/device.c
> @@ -289,6 +289,10 @@ static void device_avctp_cb(struct audio_device *dev,
> break;
> case AVCTP_STATE_CONNECTED:
> break;
> + case AVCTP_STATE_BROWSING_CONNECTING:
> + break;
> + case AVCTP_STATE_BROWSING_CONNECTED:
> + break;
> }
> }
>
> --
> 1.8.1
>
This patch has been applied, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-01-22 16:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-22 10:34 [RFC 0/1] avctp: Set browsing channel connection for ct Alexandros Antonopoulos
2013-01-22 10:34 ` [RFC 1/1] " Alexandros Antonopoulos
2013-01-22 16:01 ` Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.