* [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.