* [PATCH BlueZ 06/16] AVRCP: Simplify state_changed callback
From: Luiz Augusto von Dentz @ 2012-10-15 14:05 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350309936-31588-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Move session creation and destroy to their own functions
---
audio/avrcp.c | 91 ++++++++++++++++++++++++++++++++++-------------------------
1 file changed, 53 insertions(+), 38 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index f31372b..7188039 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1352,15 +1352,63 @@ static struct avrcp *find_session(GSList *list, struct audio_device *dev)
return NULL;
}
-static void state_changed(struct audio_device *dev, avctp_state_t old_state,
- avctp_state_t new_state, void *user_data)
+static struct avrcp *session_create(struct avrcp_server *server,
+ struct audio_device *dev)
{
- struct avrcp_server *server;
struct avrcp *session;
const sdp_record_t *rec;
sdp_list_t *list;
sdp_profile_desc_t *desc;
+ session = g_new0(struct avrcp, 1);
+ session->server = server;
+ session->conn = avctp_connect(&dev->src, &dev->dst);
+ session->dev = dev;
+ session->player = g_slist_nth_data(server->players, 0);
+
+ server->sessions = g_slist_append(server->sessions, session);
+
+ rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
+ if (rec == NULL)
+ return session;
+
+ if (sdp_get_profile_descs(rec, &list) < 0)
+ return session;
+
+ desc = list->data;
+ session->version = desc->version;
+ sdp_get_int_attr(rec, SDP_ATTR_SUPPORTED_FEATURES, &session->features);
+
+ sdp_list_free(list, free);
+
+ return session;
+}
+
+static void session_destroy(struct avrcp *session)
+{
+ struct avrcp_server *server = session->server;
+ struct avrcp_player *player = session->player;
+
+ server->sessions = g_slist_remove(server->sessions, session);
+
+ if (session->control_id > 0)
+ avctp_unregister_pdu_handler(session->control_id);
+
+ if (session->browsing_id > 0)
+ avctp_unregister_browsing_pdu_handler(session->browsing_id);
+
+ if (player != NULL)
+ player->sessions = g_slist_remove(player->sessions, session);
+
+ g_free(session);
+}
+
+static void state_changed(struct audio_device *dev, avctp_state_t old_state,
+ avctp_state_t new_state, void *user_data)
+{
+ struct avrcp_server *server;
+ struct avrcp *session;
+
server = find_server(servers, &dev->src);
if (!server)
return;
@@ -1372,47 +1420,14 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
if (session == NULL)
break;
- server->sessions = g_slist_remove(server->sessions, session);
+ session_destroy(session);
- if (session->control_id > 0)
- avctp_unregister_pdu_handler(session->control_id);
-
- if (session->browsing_id > 0)
- avctp_unregister_browsing_pdu_handler(
- session->browsing_id);
-
- if (session->player != NULL)
- session->player->sessions = g_slist_remove(
- session->player->sessions,
- session);
-
- g_free(session);
break;
case AVCTP_STATE_CONNECTING:
if (session != NULL)
break;
- session = g_new0(struct avrcp, 1);
- session->server = server;
- session->conn = avctp_connect(&dev->src, &dev->dst);
- session->dev = dev;
- session->player = g_slist_nth_data(server->players, 0);
-
- server->sessions = g_slist_append(server->sessions, session);
-
- rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
- if (rec == NULL)
- return;
-
- if (sdp_get_profile_descs(rec, &list) < 0)
- return;
-
- desc = list->data;
- session->version = desc->version;
- sdp_get_int_attr(rec, SDP_ATTR_SUPPORTED_FEATURES,
- &session->features);
-
- sdp_list_free(list, free);
+ session_create(server, dev);
break;
case AVCTP_STATE_CONNECTED:
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ 05/16] AVRCP: Fix crash on disconnect
From: Luiz Augusto von Dentz @ 2012-10-15 14:05 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350309936-31588-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
In case of multiple session being active the code was registering one
PDU hanlder per AVRCP session and given the session pointer as user data
causing the following:
24 bytes in 1 blocks are definitely lost in loss record 370 of 893
at 0x4A0884D: malloc (vg_replace_malloc.c:263)
by 0x4C803FE: g_malloc (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x12EE9D: avctp_register_browsing_pdu_handler (avctp.c:1259)
by 0x12FD7B: state_changed (avrcp.c:1402)
by 0x12D391: avctp_set_state (avctp.c:403)
by 0x12E6B4: avctp_confirm_cb (avctp.c:871)
by 0x1606A3: server_cb (btio.c:254)
by 0x4C7A824: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4C7AB57: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4C7AF51: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x120EA1: main (main.c:551)
To fix this the PDU handlers are now per AVCTP session/channel so each
AVRCP session can register its own handler and pass itself as user
data.
---
audio/avctp.c | 180 ++++++++++++++++++++++++++++++++++++----------------------
audio/avctp.h | 10 ++--
audio/avrcp.c | 56 +++++++++---------
audio/avrcp.h | 1 +
4 files changed, 148 insertions(+), 99 deletions(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index aa9a1ca..228d5b7 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -137,6 +137,7 @@ struct avctp_channel {
uint16_t imtu;
uint16_t omtu;
uint8_t *buffer;
+ GSList *handlers;
};
struct avctp {
@@ -148,6 +149,9 @@ struct avctp {
int uinput;
guint auth_id;
+ unsigned int passthrough_id;
+ unsigned int unit_id;
+ unsigned int subunit_id;
struct avctp_channel *control;
struct avctp_channel *browsing;
@@ -186,9 +190,7 @@ static struct {
static GSList *callbacks = NULL;
static GSList *servers = NULL;
-static GSList *control_handlers = NULL;
static uint8_t id = 0;
-static struct avctp_browsing_pdu_handler *browsing_handler = NULL;
static void auth_cb(DBusError *derr, void *user_data);
@@ -346,6 +348,7 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
g_source_remove(chan->watch);
g_free(chan->buffer);
+ g_slist_free_full(chan->handlers, g_free);
g_free(chan);
}
@@ -454,6 +457,7 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
uint8_t *operands;
struct avctp_header *avctp;
int sock, ret, packet_size, operand_count;
+ struct avctp_browsing_pdu_handler *handler;
if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
goto failed;
@@ -480,10 +484,18 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
packet_size = AVCTP_HEADER_LENGTH;
avctp->cr = AVCTP_RESPONSE;
- packet_size += browsing_handler->cb(session, avctp->transaction,
+ handler = g_slist_nth_data(browsing->handlers, 0);
+ if (handler == NULL) {
+ DBG("handler not found");
+ packet_size += avrcp_browsing_general_reject(operands);
+ goto send;
+ }
+
+ packet_size += handler->cb(session, avctp->transaction,
operands, operand_count,
- browsing_handler->user_data);
+ handler->user_data);
+send:
if (packet_size != 0) {
ret = write(sock, buf, packet_size);
if (ret != packet_size)
@@ -570,7 +582,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
goto done;
}
- handler = find_handler(control_handlers, avc->opcode);
+ handler = find_handler(control->handlers, avc->opcode);
if (!handler) {
DBG("handler not found for 0x%02x", avc->opcode);
packet_size += avrcp_handle_vendor_reject(&code, operands);
@@ -774,6 +786,19 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
(GIOFunc) session_cb, session);
+ session->passthrough_id = avctp_register_pdu_handler(session,
+ AVC_OP_PASSTHROUGH,
+ handle_panel_passthrough,
+ NULL);
+ session->unit_id = avctp_register_pdu_handler(session,
+ AVC_OP_UNITINFO,
+ handle_unit_info,
+ NULL);
+ session->subunit_id = avctp_register_pdu_handler(session,
+ AVC_OP_SUBUNITINFO,
+ handle_subunit_info,
+ NULL);
+
init_uinput(session);
avctp_set_state(session, AVCTP_STATE_CONNECTED);
@@ -896,12 +921,6 @@ static void avctp_browsing_confirm(struct avctp *session, GIOChannel *chan,
return;
}
- if (browsing_handler == NULL) {
- error("Browsing: Handler not registered");
- g_io_channel_shutdown(chan, TRUE, NULL);
- return;
- }
-
if (bt_io_accept(chan, avctp_connect_browsing_cb, session, NULL,
&err))
return;
@@ -994,10 +1013,6 @@ static GIOChannel *avctp_server_socket(const bdaddr_t *src, gboolean master,
return io;
}
-static unsigned int passthrough_id = 0;
-static unsigned int unit_id = 0;
-static unsigned int subunit_id = 0;
-
int avctp_register(const bdaddr_t *src, gboolean master)
{
struct avctp_server *server;
@@ -1026,18 +1041,6 @@ int avctp_register(const bdaddr_t *src, gboolean master)
servers = g_slist_append(servers, server);
- if (!passthrough_id)
- passthrough_id = avctp_register_pdu_handler(AVC_OP_PASSTHROUGH,
- handle_panel_passthrough, NULL);
-
- if (!unit_id)
- unit_id = avctp_register_pdu_handler(AVC_OP_UNITINFO, handle_unit_info,
- NULL);
-
- if (!subunit_id)
- subunit_id = avctp_register_pdu_handler(AVC_OP_SUBUNITINFO,
- handle_subunit_info, NULL);
-
return 0;
}
@@ -1061,24 +1064,6 @@ void avctp_unregister(const bdaddr_t *src)
g_io_channel_shutdown(server->control_io, TRUE, NULL);
g_io_channel_unref(server->control_io);
g_free(server);
-
- if (servers)
- return;
-
- if (passthrough_id) {
- avctp_unregister_pdu_handler(passthrough_id);
- passthrough_id = 0;
- }
-
- if (unit_id) {
- avctp_unregister_pdu_handler(unit_id);
- passthrough_id = 0;
- }
-
- if (subunit_id) {
- avctp_unregister_pdu_handler(subunit_id);
- subunit_id = 0;
- }
}
int avctp_send_passthrough(struct avctp *session, uint8_t op)
@@ -1230,13 +1215,18 @@ gboolean avctp_remove_state_cb(unsigned int id)
return FALSE;
}
-unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
- void *user_data)
+unsigned int avctp_register_pdu_handler(struct avctp *session, uint8_t opcode,
+ avctp_control_pdu_cb cb,
+ void *user_data)
{
+ struct avctp_channel *control = session->control;
struct avctp_pdu_handler *handler;
static unsigned int id = 0;
- handler = find_handler(control_handlers, opcode);
+ if (control == NULL)
+ return 0;
+
+ handler = find_handler(control->handlers, opcode);
if (handler)
return 0;
@@ -1246,36 +1236,63 @@ unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
handler->user_data = user_data;
handler->id = ++id;
- control_handlers = g_slist_append(control_handlers, handler);
+ control->handlers = g_slist_append(control->handlers, handler);
return handler->id;
}
-unsigned int avctp_register_browsing_pdu_handler(avctp_browsing_pdu_cb cb,
- void *user_data)
+unsigned int avctp_register_browsing_pdu_handler(struct avctp *session,
+ avctp_browsing_pdu_cb cb,
+ void *user_data)
{
- unsigned int id = 0;
+ struct avctp_channel *browsing = session->browsing;
+ struct avctp_browsing_pdu_handler *handler;
+ static unsigned int id = 0;
+
+ if (browsing == NULL)
+ return 0;
+
+ if (browsing->handlers != NULL)
+ return 0;
+
+ handler = g_new(struct avctp_browsing_pdu_handler, 1);
+ handler->cb = cb;
+ handler->user_data = user_data;
+ handler->id = ++id;
- browsing_handler = g_new(struct avctp_browsing_pdu_handler, 1);
- browsing_handler->cb = cb;
- browsing_handler->user_data = user_data;
- browsing_handler->id = ++id;
+ browsing->handlers = g_slist_append(browsing->handlers, handler);
- return browsing_handler->id;
+ return handler->id;
}
gboolean avctp_unregister_pdu_handler(unsigned int id)
{
GSList *l;
- for (l = control_handlers; l != NULL; l = l->next) {
- struct avctp_pdu_handler *handler = l->data;
+ for (l = servers; l; l = l->next) {
+ struct avctp_server *server = l->data;
+ GSList *s;
- if (handler->id == id) {
- control_handlers = g_slist_remove(control_handlers,
- handler);
- g_free(handler);
- return TRUE;
+ for (s = server->sessions; s; s = s->next) {
+ struct avctp *session = s->data;
+ struct avctp_channel *control = session->control;
+ GSList *h;
+
+ if (control == NULL)
+ continue;
+
+ for (h = control->handlers; h; h = h->next) {
+ struct avctp_pdu_handler *handler = h->data;
+
+ if (handler->id != id)
+ continue;
+
+ control->handlers = g_slist_remove(
+ control->handlers,
+ handler);
+ g_free(handler);
+ return TRUE;
+ }
}
}
@@ -1284,12 +1301,37 @@ gboolean avctp_unregister_pdu_handler(unsigned int id)
gboolean avctp_unregister_browsing_pdu_handler(unsigned int id)
{
- if (browsing_handler->id != id)
- return FALSE;
+ GSList *l;
- g_free(browsing_handler);
- browsing_handler = NULL;
- return TRUE;
+ for (l = servers; l; l = l->next) {
+ struct avctp_server *server = l->data;
+ GSList *s;
+
+ for (s = server->sessions; s; s = s->next) {
+ struct avctp *session = l->data;
+ struct avctp_channel *browsing = session->browsing;
+ GSList *h;
+
+ if (browsing == NULL)
+ continue;
+
+ for (h = browsing->handlers; h; h = h->next) {
+ struct avctp_browsing_pdu_handler *handler =
+ h->data;
+
+ if (handler->id != id)
+ continue;
+
+ browsing->handlers = g_slist_remove(
+ browsing->handlers,
+ handler);
+ g_free(handler);
+ return TRUE;
+ }
+ }
+ }
+
+ return FALSE;
}
struct avctp *avctp_connect(const bdaddr_t *src, const bdaddr_t *dst)
diff --git a/audio/avctp.h b/audio/avctp.h
index 6d39a20..41cf6c5 100644
--- a/audio/avctp.h
+++ b/audio/avctp.h
@@ -98,12 +98,14 @@ struct avctp *avctp_get(const bdaddr_t *src, const bdaddr_t *dst);
int avctp_connect_browsing(struct avctp *session);
void avctp_disconnect(struct avctp *session);
-unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
- void *user_data);
+unsigned int avctp_register_pdu_handler(struct avctp *session, uint8_t opcode,
+ avctp_control_pdu_cb cb,
+ void *user_data);
gboolean avctp_unregister_pdu_handler(unsigned int id);
-unsigned int avctp_register_browsing_pdu_handler(avctp_browsing_pdu_cb cb,
- void *user_data);
+unsigned int avctp_register_browsing_pdu_handler(struct avctp *session,
+ avctp_browsing_pdu_cb cb,
+ void *user_data);
gboolean avctp_unregister_browsing_pdu_handler(unsigned int id);
int avctp_send_passthrough(struct avctp *session, uint8_t op);
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 1ed6a24..f31372b 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -184,8 +184,8 @@ struct avrcp {
uint16_t version;
int features;
- unsigned int control_handler;
- unsigned int browsing_handler;
+ unsigned int control_id;
+ unsigned int browsing_id;
uint16_t registered_events;
uint8_t transaction;
uint8_t transaction_events[AVRCP_EVENT_LAST + 1];
@@ -1228,6 +1228,19 @@ static struct browsing_pdu_handler {
{ },
};
+size_t avrcp_browsing_general_reject(uint8_t *operands)
+{
+ struct avrcp_browsing_header *pdu = (void *) operands;
+ uint8_t status;
+
+ pdu->pdu_id = AVRCP_GENERAL_REJECT;
+ status = AVRCP_STATUS_INVALID_COMMAND;
+
+ pdu->param_len = htons(sizeof(status));
+ memcpy(pdu->params, &status, (sizeof(status)));
+ return AVRCP_BROWSING_HEADER_LENGTH + sizeof(status);
+}
+
static size_t handle_browsing_pdu(struct avctp *conn,
uint8_t transaction, uint8_t *operands,
size_t operand_count, void *user_data)
@@ -1235,7 +1248,6 @@ static size_t handle_browsing_pdu(struct avctp *conn,
struct avrcp *session = user_data;
struct browsing_pdu_handler *handler;
struct avrcp_browsing_header *pdu = (void *) operands;
- uint8_t status;
DBG("AVRCP Browsing PDU 0x%02X, len 0x%04X", pdu->pdu_id,
pdu->param_len);
@@ -1245,20 +1257,12 @@ static size_t handle_browsing_pdu(struct avctp *conn,
break;
}
- if (handler == NULL || handler->func == NULL) {
- pdu->pdu_id = AVRCP_GENERAL_REJECT;
- status = AVRCP_STATUS_INVALID_COMMAND;
- goto err;
- }
+ if (handler == NULL || handler->func == NULL)
+ return avrcp_browsing_general_reject(operands);
session->transaction = transaction;
handler->func(session, pdu, transaction);
return AVRCP_BROWSING_HEADER_LENGTH + ntohs(pdu->param_len);
-
-err:
- pdu->param_len = htons(sizeof(status));
- memcpy(pdu->params, &status, (sizeof(status)));
- return AVRCP_BROWSING_HEADER_LENGTH + sizeof(status);
}
size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands)
@@ -1370,12 +1374,12 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
server->sessions = g_slist_remove(server->sessions, session);
- if (session->control_handler)
- avctp_unregister_pdu_handler(session->control_handler);
+ if (session->control_id > 0)
+ avctp_unregister_pdu_handler(session->control_id);
- if (session->browsing_handler)
+ if (session->browsing_id > 0)
avctp_unregister_browsing_pdu_handler(
- session->browsing_handler);
+ session->browsing_id);
if (session->player != NULL)
session->player->sessions = g_slist_remove(
@@ -1394,15 +1398,6 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
session->dev = dev;
session->player = g_slist_nth_data(server->players, 0);
- session->control_handler = avctp_register_pdu_handler(
- AVC_OP_VENDORDEP,
- handle_vendordep_pdu,
- session);
- session->browsing_handler =
- avctp_register_browsing_pdu_handler(
- handle_browsing_pdu,
- session);
-
server->sessions = g_slist_append(server->sessions, session);
rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
@@ -1429,6 +1424,15 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
if (session->features & AVRCP_FEATURE_BROWSING)
avctp_connect_browsing(session->conn);
}
+
+ session->control_id = avctp_register_pdu_handler(session->conn,
+ AVC_OP_VENDORDEP,
+ handle_vendordep_pdu,
+ session);
+ session->browsing_id = avctp_register_browsing_pdu_handler(
+ session->conn,
+ handle_browsing_pdu,
+ session);
default:
return;
}
diff --git a/audio/avrcp.h b/audio/avrcp.h
index 42d902b..6c651dd 100644
--- a/audio/avrcp.h
+++ b/audio/avrcp.h
@@ -105,3 +105,4 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data);
size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands);
+size_t avrcp_browsing_general_reject(uint8_t *operands);
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ 04/16] AVRCP: Don't respond with errors when no player is registered
From: Luiz Augusto von Dentz @ 2012-10-15 14:05 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350309936-31588-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Some devices w.g. Sony MW600 will stop using certain commands if an
error happen, so the code now just fake a player and once a real
player is registered it takes place of the fake one.
---
audio/avrcp.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 63 insertions(+), 14 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 295a1e1..1ed6a24 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -477,6 +477,17 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
return;
}
+static void *player_get_metadata(struct avrcp_player *player, uint32_t attr)
+{
+ if (player != NULL)
+ return player->cb->get_metadata(attr, player->user_data);
+
+ if (attr == AVRCP_MEDIA_ATTRIBUTE_TITLE)
+ return "";
+
+ return NULL;
+}
+
static uint16_t player_write_media_attribute(struct avrcp_player *player,
uint32_t id, uint8_t *buf,
uint16_t *pos,
@@ -489,7 +500,7 @@ static uint16_t player_write_media_attribute(struct avrcp_player *player,
DBG("%u", id);
- value = player->cb->get_metadata(id, player->user_data);
+ value = player_get_metadata(player, id);
if (value == NULL) {
*offset = 0;
return 0;
@@ -588,6 +599,9 @@ static int player_set_attribute(struct avrcp_player *player,
{
DBG("Change attribute: %u %u", attr, val);
+ if (player == NULL)
+ return -ENOENT;
+
return player->cb->set_setting(attr, val, player->user_data);
}
@@ -597,6 +611,9 @@ static int player_get_attribute(struct avrcp_player *player, uint8_t attr)
DBG("attr %u", attr);
+ if (player == NULL)
+ return -ENOENT;
+
value = player->cb->get_setting(attr, player->user_data);
if (value < 0)
DBG("attr %u not supported by player", attr);
@@ -653,7 +670,7 @@ static uint8_t avrcp_handle_list_player_attributes(struct avrcp *session,
uint16_t len = ntohs(pdu->params_len);
unsigned int i;
- if (len != 0 || player == NULL) {
+ if (len != 0) {
pdu->params_len = htons(1);
pdu->params[0] = AVRCP_STATUS_INVALID_PARAM;
return AVC_CTYPE_REJECTED;
@@ -685,7 +702,7 @@ static uint8_t avrcp_handle_list_player_values(struct avrcp *session,
uint16_t len = ntohs(pdu->params_len);
unsigned int i;
- if (len != 1 || player == NULL)
+ if (len != 1)
goto err;
if (player_get_attribute(player, pdu->params[0]) < 0)
@@ -707,6 +724,15 @@ err:
return AVC_CTYPE_REJECTED;
}
+static GList *player_list_metadata(struct avrcp_player *player)
+{
+ if (player != NULL)
+ return player->cb->list_metadata(player->user_data);
+
+ return g_list_prepend(NULL,
+ GUINT_TO_POINTER(AVRCP_MEDIA_ATTRIBUTE_TITLE));
+}
+
static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
struct avrcp_header *pdu,
uint8_t transaction)
@@ -719,7 +745,7 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
GList *attr_ids;
uint16_t offset;
- if (len < 9 || identifier != 0 || player == NULL)
+ if (len < 9 || identifier != 0)
goto err;
nattr = pdu->params[8];
@@ -732,7 +758,7 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
* Return all available information, at least
* title must be returned if there's a track selected.
*/
- attr_ids = player->cb->list_metadata(player->user_data);
+ attr_ids = player_list_metadata(player);
len = g_list_length(attr_ids);
} else {
unsigned int i;
@@ -788,8 +814,7 @@ static uint8_t avrcp_handle_get_current_player_value(struct avrcp *session,
uint8_t *settings;
unsigned int i;
- if (player == NULL || len <= 1 || pdu->params[0] != len - 1 ||
- player == NULL)
+ if (len <= 1 || pdu->params[0] != len - 1)
goto err;
/*
@@ -922,6 +947,14 @@ err:
return AVC_CTYPE_REJECTED;
}
+static uint32_t player_get_position(struct avrcp_player *player)
+{
+ if (player == NULL)
+ return 0;
+
+ return player->cb->get_position(player->user_data);
+}
+
static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
struct avrcp_header *pdu,
uint8_t transaction)
@@ -932,15 +965,15 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
uint32_t duration;
void *pduration;
- if (len != 0 || player == NULL) {
+ if (len != 0) {
pdu->params_len = htons(1);
pdu->params[0] = AVRCP_STATUS_INVALID_PARAM;
return AVC_CTYPE_REJECTED;
}
- position = player->cb->get_position(player->user_data);
- pduration = player->cb->get_metadata(AVRCP_MEDIA_ATTRIBUTE_DURATION,
- player->user_data);
+ position = player_get_position(player);
+ pduration = player_get_metadata(player,
+ AVRCP_MEDIA_ATTRIBUTE_DURATION);
if (pduration != NULL)
duration = htonl(GPOINTER_TO_UINT(pduration));
else
@@ -957,6 +990,22 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
return AVC_CTYPE_STABLE;
}
+static uint64_t player_get_uid(struct avrcp_player *player)
+{
+ if (player == NULL)
+ return UINT64_MAX;
+
+ return player->cb->get_status(player->user_data);
+}
+
+static uint8_t player_get_status(struct avrcp_player *player)
+{
+ if (player == NULL)
+ return AVRCP_PLAY_STATUS_STOPPED;
+
+ return player->cb->get_status(player->user_data);
+}
+
static uint8_t avrcp_handle_register_notification(struct avrcp *session,
struct avrcp_header *pdu,
uint8_t transaction)
@@ -970,18 +1019,18 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
* one is applicable only for EVENT_PLAYBACK_POS_CHANGED. See AVRCP
* 1.3 spec, section 5.4.2.
*/
- if (len != 5 || player == NULL)
+ if (len != 5)
goto err;
switch (pdu->params[0]) {
case AVRCP_EVENT_STATUS_CHANGED:
len = 2;
- pdu->params[1] = player->cb->get_status(player->user_data);
+ pdu->params[1] = player_get_status(player);
break;
case AVRCP_EVENT_TRACK_CHANGED:
len = 9;
- uid = player->cb->get_uid(player->user_data);
+ uid = player_get_uid(player);
memcpy(&pdu->params[1], &uid, sizeof(uint64_t));
break;
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ 03/16] AVRCP: Register to AVCTP state changes without depending on player
From: Luiz Augusto von Dentz @ 2012-10-15 14:05 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350309936-31588-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
It is not longer necessary to have a player to be able to register the
extra pdu handlers.
---
audio/avrcp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index aa62987..295a1e1 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1467,6 +1467,9 @@ int avrcp_register(const bdaddr_t *src, GKeyFile *config)
servers = g_slist_append(servers, server);
+ if (!avctp_id)
+ avctp_id = avctp_add_state_cb(state_changed, NULL);
+
return 0;
}
@@ -1528,9 +1531,6 @@ struct avrcp_player *avrcp_register_player(const bdaddr_t *src,
player->user_data = user_data;
player->destroy = destroy;
- if (!avctp_id)
- avctp_id = avctp_add_state_cb(state_changed, NULL);
-
server->players = g_slist_append(server->players, player);
/* Assign player to session without current player */
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ 02/16] AVCTP: Allocate memory to hold incoming/outgoing PDUs
From: Luiz Augusto von Dentz @ 2012-10-15 14:05 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350309936-31588-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This makes possible to the handler to respond asyncronous as the memory
remains valid after it returns.
In addition to that it uses the MTU to calculate the buffer size
necessary.
---
audio/avctp.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index 092a5e2..aa9a1ca 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -134,7 +134,9 @@ struct avctp_rsp_handler {
struct avctp_channel {
GIOChannel *io;
guint watch;
- uint16_t mtu;
+ uint16_t imtu;
+ uint16_t omtu;
+ uint8_t *buffer;
};
struct avctp {
@@ -343,6 +345,7 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
if (chan->watch)
g_source_remove(chan->watch);
+ g_free(chan->buffer);
g_free(chan);
}
@@ -446,7 +449,9 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
gpointer data)
{
struct avctp *session = data;
- uint8_t buf[1024], *operands;
+ struct avctp_channel *browsing = session->browsing;
+ uint8_t *buf = browsing->buffer;
+ uint8_t *operands;
struct avctp_header *avctp;
int sock, ret, packet_size, operand_count;
@@ -455,7 +460,7 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
sock = g_io_channel_unix_get_fd(chan);
- ret = read(sock, buf, sizeof(buf));
+ ret = read(sock, buf, sizeof(browsing->imtu));
if (ret <= 0)
goto failed;
@@ -496,7 +501,9 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
gpointer data)
{
struct avctp *session = data;
- uint8_t buf[1024], *operands, code, subunit;
+ struct avctp_channel *control = session->control;
+ uint8_t *buf = control->buffer;
+ uint8_t *operands, code, subunit;
struct avctp_header *avctp;
struct avc_header *avc;
int ret, packet_size, operand_count, sock;
@@ -507,7 +514,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
sock = g_io_channel_unix_get_fd(chan);
- ret = read(sock, buf, sizeof(buf));
+ ret = read(sock, buf, control->imtu);
if (ret <= 0)
goto failed;
@@ -688,7 +695,7 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
{
struct avctp *session = data;
char address[18];
- uint16_t imtu;
+ uint16_t imtu, omtu;
GError *gerr = NULL;
if (err) {
@@ -699,6 +706,7 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
bt_io_get(chan, &gerr,
BT_IO_OPT_DEST, &address,
BT_IO_OPT_IMTU, &imtu,
+ BT_IO_OPT_OMTU, &omtu,
BT_IO_OPT_INVALID);
if (gerr) {
error("%s", gerr->message);
@@ -713,7 +721,9 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
if (session->browsing == NULL)
session->browsing = avctp_channel_create(chan);
- session->browsing->mtu = imtu;
+ session->browsing->imtu = imtu;
+ session->browsing->omtu = omtu;
+ session->browsing->buffer = g_malloc0(MAX(imtu, omtu));
session->browsing->watch = g_io_add_watch(session->browsing->io,
G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
(GIOFunc) session_browsing_cb, session);
@@ -731,7 +741,7 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
{
struct avctp *session = data;
char address[18];
- uint16_t imtu;
+ uint16_t imtu, omtu;
GError *gerr = NULL;
if (err) {
@@ -743,6 +753,7 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
bt_io_get(chan, &gerr,
BT_IO_OPT_DEST, &address,
BT_IO_OPT_IMTU, &imtu,
+ BT_IO_OPT_IMTU, &omtu,
BT_IO_OPT_INVALID);
if (gerr) {
avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
@@ -756,7 +767,9 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
if (session->control == NULL)
session->control = avctp_channel_create(chan);
- session->control->mtu = imtu;
+ session->control->imtu = imtu;
+ session->control->omtu = omtu;
+ session->control->buffer = g_malloc0(MAX(imtu, omtu));
session->control->watch = g_io_add_watch(session->control->io,
G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
(GIOFunc) session_cb, session);
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ 01/16] AVCTP: Simplify channel handling
From: Luiz Augusto von Dentz @ 2012-10-15 14:05 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Make both control and browsing channels to use the same structure to
store its channel information.
---
audio/avctp.c | 155 +++++++++++++++++++++++++++++++---------------------------
1 file changed, 82 insertions(+), 73 deletions(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index 53c28ec..092a5e2 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -131,6 +131,12 @@ struct avctp_rsp_handler {
void *user_data;
};
+struct avctp_channel {
+ GIOChannel *io;
+ guint watch;
+ uint16_t mtu;
+};
+
struct avctp {
struct avctp_server *server;
bdaddr_t dst;
@@ -139,14 +145,10 @@ struct avctp {
int uinput;
- GIOChannel *control_io;
- GIOChannel *browsing_io;
- guint control_io_id;
- guint browsing_io_id;
guint auth_id;
- uint16_t control_mtu;
- uint16_t browsing_mtu;
+ struct avctp_channel *control;
+ struct avctp_channel *browsing;
uint8_t key_quirks[256];
GSList *handlers;
@@ -333,6 +335,17 @@ static struct avctp_pdu_handler *find_handler(GSList *list, uint8_t opcode)
return NULL;
}
+static void avctp_channel_destroy(struct avctp_channel *chan)
+{
+ g_io_channel_shutdown(chan->io, TRUE, NULL);
+ g_io_channel_unref(chan->io);
+
+ if (chan->watch)
+ g_source_remove(chan->watch);
+
+ g_free(chan);
+}
+
static void avctp_disconnected(struct avctp *session)
{
struct avctp_server *server;
@@ -340,31 +353,15 @@ static void avctp_disconnected(struct avctp *session)
if (!session)
return;
- if (session->browsing_io) {
- g_io_channel_shutdown(session->browsing_io, TRUE, NULL);
- g_io_channel_unref(session->browsing_io);
- session->browsing_io = NULL;
- }
-
- if (session->browsing_io_id) {
- g_source_remove(session->browsing_io_id);
- session->browsing_io_id = 0;
- }
-
- if (session->control_io) {
- g_io_channel_shutdown(session->control_io, TRUE, NULL);
- g_io_channel_unref(session->control_io);
- session->control_io = NULL;
- }
+ if (session->browsing)
+ avctp_channel_destroy(session->browsing);
- if (session->control_io_id) {
- g_source_remove(session->control_io_id);
- session->control_io_id = 0;
+ if (session->control)
+ avctp_channel_destroy(session->control);
- if (session->auth_id != 0) {
- btd_cancel_authorization(session->auth_id);
- session->auth_id = 0;
- }
+ if (session->auth_id != 0) {
+ btd_cancel_authorization(session->auth_id);
+ session->auth_id = 0;
}
if (session->uinput >= 0) {
@@ -420,7 +417,7 @@ static void avctp_set_state(struct avctp *session, avctp_state_t new_state)
}
}
-static void handle_response(struct avctp *session, struct avctp_header *avctp,
+static void control_response(struct avctp *session, struct avctp_header *avctp,
struct avc_header *avc, uint8_t *operands,
size_t operand_count)
{
@@ -456,7 +453,7 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
goto failed;
- sock = g_io_channel_unix_get_fd(session->browsing_io);
+ sock = g_io_channel_unix_get_fd(chan);
ret = read(sock, buf, sizeof(buf));
if (ret <= 0)
@@ -508,7 +505,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
goto failed;
- sock = g_io_channel_unix_get_fd(session->control_io);
+ sock = g_io_channel_unix_get_fd(chan);
ret = read(sock, buf, sizeof(buf));
if (ret <= 0)
@@ -548,7 +545,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
avc->opcode, operand_count);
if (avctp->cr == AVCTP_RESPONSE) {
- handle_response(session, avctp, avc, operands, operand_count);
+ control_response(session, avctp, avc, operands, operand_count);
return TRUE;
}
@@ -676,6 +673,16 @@ static void init_uinput(struct avctp *session)
DBG("AVRCP: uinput initialized for %s", address);
}
+static struct avctp_channel *avctp_channel_create(GIOChannel *io)
+{
+ struct avctp_channel *chan;
+
+ chan = g_new0(struct avctp_channel, 1);
+ chan->io = g_io_channel_ref(io);
+
+ return chan;
+}
+
static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
gpointer data)
{
@@ -686,10 +693,7 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
if (err) {
error("Browsing: %s", err->message);
- g_io_channel_shutdown(chan, TRUE, NULL);
- g_io_channel_unref(chan);
- session->browsing_io = NULL;
- return;
+ goto fail;
}
bt_io_get(chan, &gerr,
@@ -700,20 +704,28 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
error("%s", gerr->message);
g_io_channel_shutdown(chan, TRUE, NULL);
g_io_channel_unref(chan);
- session->browsing_io = NULL;
g_error_free(gerr);
- return;
+ goto fail;
}
DBG("AVCTP Browsing: connected to %s", address);
- if (!session->browsing_io)
- session->browsing_io = g_io_channel_ref(chan);
+ if (session->browsing == NULL)
+ session->browsing = avctp_channel_create(chan);
- session->browsing_mtu = imtu;
- session->browsing_io_id = g_io_add_watch(chan,
+ session->browsing->mtu = imtu;
+ session->browsing->watch = g_io_add_watch(session->browsing->io,
G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
- (GIOFunc) session_browsing_cb, session); }
+ (GIOFunc) session_browsing_cb, session);
+
+ return;
+
+fail:
+ if (session->browsing) {
+ avctp_channel_destroy(session->browsing);
+ session->browsing = NULL;
+ }
+}
static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
{
@@ -741,16 +753,17 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
DBG("AVCTP: connected to %s", address);
- if (!session->control_io)
- session->control_io = g_io_channel_ref(chan);
+ if (session->control == NULL)
+ session->control = avctp_channel_create(chan);
+
+ session->control->mtu = imtu;
+ session->control->watch = g_io_add_watch(session->control->io,
+ G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+ (GIOFunc) session_cb, session);
init_uinput(session);
avctp_set_state(session, AVCTP_STATE_CONNECTED);
- session->control_mtu = imtu;
- session->control_io_id = g_io_add_watch(chan,
- G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
- (GIOFunc) session_cb, session);
}
static void auth_cb(DBusError *derr, void *user_data)
@@ -760,9 +773,9 @@ static void auth_cb(DBusError *derr, void *user_data)
session->auth_id = 0;
- if (session->control_io_id) {
- g_source_remove(session->control_io_id);
- session->control_io_id = 0;
+ if (session->control->watch > 0) {
+ g_source_remove(session->control->watch);
+ session->control->watch = 0;
}
if (derr && dbus_error_is_set(derr)) {
@@ -771,7 +784,7 @@ static void auth_cb(DBusError *derr, void *user_data)
return;
}
- if (!bt_io_accept(session->control_io, avctp_connect_cb, session,
+ if (!bt_io_accept(session->control->io, avctp_connect_cb, session,
NULL, &err)) {
error("bt_io_accept: %s", err->message);
g_error_free(err);
@@ -836,14 +849,14 @@ static struct avctp *avctp_get_internal(const bdaddr_t *src,
static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
struct audio_device *dev)
{
- if (session->control_io) {
+ if (session->control != NULL) {
error("Control: Refusing unexpected connect");
g_io_channel_shutdown(chan, TRUE, NULL);
return;
}
avctp_set_state(session, AVCTP_STATE_CONNECTING);
- session->control_io = g_io_channel_ref(chan);
+ session->control = avctp_channel_create(chan);
session->auth_id = btd_request_authorization(&dev->src, &dev->dst,
AVRCP_TARGET_UUID,
@@ -851,7 +864,7 @@ static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
if (session->auth_id == 0)
goto drop;
- session->control_io_id = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP |
+ session->control->watch = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP |
G_IO_NVAL, session_cb, session);
return;
@@ -864,7 +877,7 @@ static void avctp_browsing_confirm(struct avctp *session, GIOChannel *chan,
{
GError *err = NULL;
- if (session->control_io == NULL || session->browsing_io) {
+ if (session->control == NULL || session->browsing != NULL) {
error("Browsing: Refusing unexpected connect");
g_io_channel_shutdown(chan, TRUE, NULL);
return;
@@ -911,8 +924,8 @@ static void avctp_confirm_cb(GIOChannel *chan, gpointer data)
DBG("AVCTP: incoming connect from %s", address);
session = avctp_get_internal(&src, &dst);
- if (!session)
- goto drop;
+ if (session == NULL)
+ return;
dev = manager_get_device(&src, &dst, FALSE);
if (!dev) {
@@ -942,13 +955,7 @@ static void avctp_confirm_cb(GIOChannel *chan, gpointer data)
return;
drop:
- if (session && session->browsing_io)
- g_io_channel_unref(session->browsing_io);
-
- if (session && session->control_io)
- g_io_channel_unref(session->control_io);
-
- if (session && psm == AVCTP_CONTROL_PSM)
+ if (psm == AVCTP_CONTROL_PSM)
avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
}
@@ -1086,7 +1093,7 @@ int avctp_send_passthrough(struct avctp *session, uint8_t op)
operands[0] = op & 0x7f;
operands[1] = 0;
- sk = g_io_channel_unix_get_fd(session->control_io);
+ sk = g_io_channel_unix_get_fd(session->control->io);
if (write(sk, buf, sizeof(buf)) < 0)
return -errno;
@@ -1115,7 +1122,7 @@ static int avctp_send(struct avctp *session, uint8_t transaction, uint8_t cr,
if (session->state != AVCTP_STATE_CONNECTED)
return -ENOTCONN;
- sk = g_io_channel_unix_get_fd(session->control_io);
+ sk = g_io_channel_unix_get_fd(session->control->io);
memset(buf, 0, sizeof(buf));
@@ -1299,7 +1306,8 @@ struct avctp *avctp_connect(const bdaddr_t *src, const bdaddr_t *dst)
return NULL;
}
- session->control_io = io;
+ session->control = avctp_channel_create(io);
+ g_io_channel_unref(io);
return session;
}
@@ -1312,7 +1320,7 @@ int avctp_connect_browsing(struct avctp *session)
if (session->state != AVCTP_STATE_CONNECTED)
return -ENOTCONN;
- if (session->browsing_io != NULL)
+ if (session->browsing != NULL)
return 0;
io = bt_io_connect(avctp_connect_browsing_cb, session, NULL, &err,
@@ -1327,14 +1335,15 @@ int avctp_connect_browsing(struct avctp *session)
return -EIO;
}
- session->browsing_io = io;
+ session->browsing = avctp_channel_create(io);
+ g_io_channel_unref(io);
return 0;
}
void avctp_disconnect(struct avctp *session)
{
- if (!session->control_io)
+ if (session->state == AVCTP_STATE_DISCONNECTED)
return;
avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
--
1.7.11.4
^ permalink raw reply related
* Re: [PATCHv2 5/6] Bluetooth: AMP: Get amp_mgr reference in HS hci_conn
From: Andrei Emeltchenko @ 2012-10-15 13:21 UTC (permalink / raw)
To: Gustavo Padovan, linux-bluetooth
In-Reply-To: <20121015124903.GA6498@joana>
Hi Gustavo,
On Mon, Oct 15, 2012 at 09:49:03AM -0300, Gustavo Padovan wrote:
> Hi Andrei,
>
> * Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-10-15 11:58:43 +0300]:
>
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > When assigning amp_mgr in hci_conn (type AMP_LINK) get also reference.
> > In hci_conn_del those references would be put for both types.
> >
> > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> > ---
> > net/bluetooth/amp.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
> > index 59da0f1..e525e23 100644
> > --- a/net/bluetooth/amp.c
> > +++ b/net/bluetooth/amp.c
> > @@ -123,9 +123,11 @@ struct hci_conn *phylink_add(struct hci_dev *hdev, struct amp_mgr *mgr,
> > hcon->attempt++;
> > hcon->handle = __next_handle(mgr);
> > hcon->remote_id = remote_id;
> > - hcon->amp_mgr = mgr;
> > hcon->out = out;
> >
> > + hcon->amp_mgr = mgr;
> > + amp_mgr_get(mgr);
>
> You did not take the comments from Marcel in this patch, please make the
> change to amp_mgr_get() before sending this patch.
I was thinking of making separate patch since this might be used in
several places so I would change it once instead of splitting this change
to several patches.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCHv2 6/6] Bluetooth: Zero bredr pointer when chan is deleted
From: Gustavo Padovan @ 2012-10-15 12:50 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1350291524-13258-7-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-10-15 11:58:44 +0300]:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> If BREDR L2CAP chan is deleted and this chan is the channel through
> which High Speed traffic is routed to AMP then zero pointer to
> the chan in amp_mgr to prevent accessing it.
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
> net/bluetooth/l2cap_core.c | 4 ++++
> 1 file changed, 4 insertions(+)
All patches but patch 5 were applied to bluetooth-next. Thanks.
Gustavo
^ permalink raw reply
* Re: [PATCHv2 5/6] Bluetooth: AMP: Get amp_mgr reference in HS hci_conn
From: Gustavo Padovan @ 2012-10-15 12:49 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1350291524-13258-6-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-10-15 11:58:43 +0300]:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> When assigning amp_mgr in hci_conn (type AMP_LINK) get also reference.
> In hci_conn_del those references would be put for both types.
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> ---
> net/bluetooth/amp.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
> index 59da0f1..e525e23 100644
> --- a/net/bluetooth/amp.c
> +++ b/net/bluetooth/amp.c
> @@ -123,9 +123,11 @@ struct hci_conn *phylink_add(struct hci_dev *hdev, struct amp_mgr *mgr,
> hcon->attempt++;
> hcon->handle = __next_handle(mgr);
> hcon->remote_id = remote_id;
> - hcon->amp_mgr = mgr;
> hcon->out = out;
>
> + hcon->amp_mgr = mgr;
> + amp_mgr_get(mgr);
You did not take the comments from Marcel in this patch, please make the
change to amp_mgr_get() before sending this patch.
Gustavo
^ permalink raw reply
* Re: [PATCH BlueZ] media: Fix leaking endpoint in case of failure to register SEP
From: Johan Hedberg @ 2012-10-15 10:13 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1350294687-20718-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Mon, Oct 15, 2012, Luiz Augusto von Dentz wrote:
> This can happen in case of AudioSink and/or AudioSource are disabled in
> audio.conf.
> ---
> audio/media.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied. Thanks.
This finally fixed the leak I've been seeing for a long time. Btw, this
happens also with no audio.conf at all as the default is to disable
AudioSource, so it's a bit misleading to imply (as you do in your commit
message) that this would happen only because of something in audio.conf.
Johan
^ permalink raw reply
* [PATCH BlueZ] media: Fix leaking endpoint in case of failure to register SEP
From: Luiz Augusto von Dentz @ 2012-10-15 9:51 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This can happen in case of AudioSink and/or AudioSource are disabled in
audio.conf.
---
audio/media.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/audio/media.c b/audio/media.c
index 5c26035..f2b5b2f 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -769,7 +769,7 @@ static struct media_endpoint *media_endpoint_create(struct media_adapter *adapte
}
if (!succeeded) {
- g_free(endpoint);
+ media_endpoint_destroy(endpoint);
return NULL;
}
--
1.7.11.4
^ permalink raw reply related
* [PATCHv2 6/6] Bluetooth: Zero bredr pointer when chan is deleted
From: Andrei Emeltchenko @ 2012-10-15 8:58 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350291524-13258-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
If BREDR L2CAP chan is deleted and this chan is the channel through
which High Speed traffic is routed to AMP then zero pointer to
the chan in amp_mgr to prevent accessing it.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
net/bluetooth/l2cap_core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 978b203..32b3fae 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -531,6 +531,7 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
BT_DBG("chan %p, conn %p, err %d", chan, conn, err);
if (conn) {
+ struct amp_mgr *mgr = conn->hcon->amp_mgr;
/* Delete from channel list */
list_del(&chan->list);
@@ -540,6 +541,9 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
if (chan->chan_type != L2CAP_CHAN_CONN_FIX_A2MP)
hci_conn_put(conn->hcon);
+
+ if (mgr && mgr->bredr_chan == chan)
+ mgr->bredr_chan = NULL;
}
chan->ops->teardown(chan, err);
--
1.7.9.5
^ permalink raw reply related
* [PATCHv2 5/6] Bluetooth: AMP: Get amp_mgr reference in HS hci_conn
From: Andrei Emeltchenko @ 2012-10-15 8:58 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350291524-13258-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
When assigning amp_mgr in hci_conn (type AMP_LINK) get also reference.
In hci_conn_del those references would be put for both types.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
net/bluetooth/amp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 59da0f1..e525e23 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -123,9 +123,11 @@ struct hci_conn *phylink_add(struct hci_dev *hdev, struct amp_mgr *mgr,
hcon->attempt++;
hcon->handle = __next_handle(mgr);
hcon->remote_id = remote_id;
- hcon->amp_mgr = mgr;
hcon->out = out;
+ hcon->amp_mgr = mgr;
+ amp_mgr_get(mgr);
+
return hcon;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCHv2 4/6] Bluetooth: Send EFS Conf Rsp only for BR/EDR chan
From: Andrei Emeltchenko @ 2012-10-15 8:58 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350291524-13258-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Do not send EFS Configuration Response for High Speed channel yet.
It will be sent after receiving Logical Link Complete event.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
net/bluetooth/l2cap_core.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index e7bec6c..978b203 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3715,7 +3715,11 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
/* check compatibility */
- l2cap_send_efs_conf_rsp(chan, rsp, cmd->ident, flags);
+ /* Send rsp for BR/EDR channel */
+ if (!chan->ctrl_id)
+ l2cap_send_efs_conf_rsp(chan, rsp, cmd->ident, flags);
+ else
+ chan->ident = cmd->ident;
}
unlock:
@@ -3764,7 +3768,11 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
/* check compatibility */
- l2cap_send_efs_conf_rsp(chan, buf, cmd->ident, 0);
+ if (!chan->ctrl_id)
+ l2cap_send_efs_conf_rsp(chan, buf, cmd->ident,
+ 0);
+ else
+ chan->ident = cmd->ident;
}
goto done;
--
1.7.9.5
^ permalink raw reply related
* [PATCHv2 3/6] Bluetooth: AMP: Drop packets when no l2cap conn exist
From: Andrei Emeltchenko @ 2012-10-15 8:58 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350291524-13258-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
High Speed hci_conn should always have l2cap_conn associated with it.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
net/bluetooth/l2cap_core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 5fd236f..e7bec6c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5579,6 +5579,10 @@ int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
struct l2cap_hdr *hdr;
int len;
+ /* For AMP controller do not create l2cap conn */
+ if (!conn && hcon->hdev->dev_type != HCI_BREDR)
+ goto drop;
+
if (!conn)
conn = l2cap_conn_add(hcon, 0);
--
1.7.9.5
^ permalink raw reply related
* [PATCHv2 2/6] Bluetooth: AMP: Handle complete frames in l2cap
From: Andrei Emeltchenko @ 2012-10-15 8:58 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350291524-13258-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Check flags type in switch statement and handle new frame
type ACL_COMPLETE used for High Speed data over AMP.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
include/net/bluetooth/hci.h | 1 +
net/bluetooth/l2cap_core.c | 15 ++++++++++-----
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 77b6a19..88cbbda 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -198,6 +198,7 @@ enum {
#define ACL_START_NO_FLUSH 0x00
#define ACL_CONT 0x01
#define ACL_START 0x02
+#define ACL_COMPLETE 0x03
#define ACL_ACTIVE_BCAST 0x04
#define ACL_PICO_BCAST 0x08
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 2fb37de..5fd236f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5576,6 +5576,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
{
struct l2cap_conn *conn = hcon->l2cap_data;
+ struct l2cap_hdr *hdr;
+ int len;
if (!conn)
conn = l2cap_conn_add(hcon, 0);
@@ -5585,10 +5587,10 @@ int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
BT_DBG("conn %p len %d flags 0x%x", conn, skb->len, flags);
- if (!(flags & ACL_CONT)) {
- struct l2cap_hdr *hdr;
- int len;
-
+ switch (flags) {
+ case ACL_START:
+ case ACL_START_NO_FLUSH:
+ case ACL_COMPLETE:
if (conn->rx_len) {
BT_ERR("Unexpected start frame (len %d)", skb->len);
kfree_skb(conn->rx_skb);
@@ -5630,7 +5632,9 @@ int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb->len),
skb->len);
conn->rx_len = len - skb->len;
- } else {
+ break;
+
+ case ACL_CONT:
BT_DBG("Cont: frag len %d (expecting %d)", skb->len, conn->rx_len);
if (!conn->rx_len) {
@@ -5658,6 +5662,7 @@ int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
l2cap_recv_frame(conn, conn->rx_skb);
conn->rx_skb = NULL;
}
+ break;
}
drop:
--
1.7.9.5
^ permalink raw reply related
* [PATCHv2 1/6] Bluetooth: AMP: Use Loglink handle in ACL Handle field
From: Andrei Emeltchenko @ 2012-10-15 8:58 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350291524-13258-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
For AMP HCI controller use Logical Link handle in HCI ACL
Handle field.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
net/bluetooth/hci_core.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 2e72c41..0ec776a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2162,7 +2162,18 @@ static void hci_queue_acl(struct hci_chan *chan, struct sk_buff_head *queue,
skb->data_len = 0;
bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
- hci_add_acl_hdr(skb, conn->handle, flags);
+
+ switch (hdev->dev_type) {
+ case HCI_BREDR:
+ hci_add_acl_hdr(skb, conn->handle, flags);
+ break;
+ case HCI_AMP:
+ hci_add_acl_hdr(skb, chan->handle, flags);
+ break;
+ default:
+ BT_ERR("%s unknown dev_type %d", hdev->name, hdev->dev_type);
+ return;
+ }
list = skb_shinfo(skb)->frag_list;
if (!list) {
--
1.7.9.5
^ permalink raw reply related
* [PATCHv2 0/6] AMP link fixes
From: Andrei Emeltchenko @ 2012-10-15 8:58 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1350051005-6015-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Set of patches handling AMP link fixes
Changelog:
* v2: Removed patch "AMP: Zero amp_mgr pointer..", taken Marcel's
comments.
Andrei Emeltchenko (6):
Bluetooth: AMP: Use Loglink handle in ACL Handle field
Bluetooth: AMP: Handle complete frames in l2cap
Bluetooth: AMP: Drop packets when no l2cap conn exist
Bluetooth: Send EFS Conf Rsp only for BR/EDR chan
Bluetooth: AMP: Get amp_mgr reference in HS hci_conn
Bluetooth: Zero bredr pointer when chan is deleted
include/net/bluetooth/hci.h | 1 +
net/bluetooth/amp.c | 4 +++-
net/bluetooth/hci_core.c | 13 ++++++++++++-
net/bluetooth/l2cap_core.c | 35 ++++++++++++++++++++++++++++-------
4 files changed, 44 insertions(+), 9 deletions(-)
--
1.7.9.5
^ permalink raw reply
* Re: [RFCv1 5/7] Bluetooth: AMP: Get amp_mgr reference in HS hci_conn
From: Andrei Emeltchenko @ 2012-10-15 8:31 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1350086005.19673.14.camel@aeonflux>
Hi Marcel,
On Sat, Oct 13, 2012 at 01:53:25AM +0200, Marcel Holtmann wrote:
> Hi Andrei,
>
> > When assigning amp_mgr in hci_conn (type AMP_LINK) get also reference.
> > In hci_conn_del those references would be put for both types.
> >
> > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > ---
> > net/bluetooth/amp.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
>
> > diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
> > index 59da0f1..e525e23 100644
> > --- a/net/bluetooth/amp.c
> > +++ b/net/bluetooth/amp.c
> > @@ -123,9 +123,11 @@ struct hci_conn *phylink_add(struct hci_dev *hdev, struct amp_mgr *mgr,
> > hcon->attempt++;
> > hcon->handle = __next_handle(mgr);
> > hcon->remote_id = remote_id;
> > - hcon->amp_mgr = mgr;
> > hcon->out = out;
> >
> > + hcon->amp_mgr = mgr;
> > + amp_mgr_get(mgr);
> > +
>
> We could discuss that amp_mgt_get(mgr) should just return mgr and you
> can write this like this:
>
> hcon->amp_mgr = amp_mgr_get(mgr);
Good idea. I will make it in the following patches.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [pulseaudio-discuss] how to troubleshoot bluetooth audio issues?
From: Oleksij Rempel @ 2012-10-14 17:24 UTC (permalink / raw)
To: Tanu Kaskinen; +Cc: pulseaudio-discuss, linux-bluetooth
In-Reply-To: <1350230390.30695.22.camel@x230>
Am 14.10.2012 17:59, schrieb Tanu Kaskinen:
> On Sun, 2012-10-14 at 17:29 +0200, Oleksij Rempel wrote:
>> Hallo all, (Pulseaudio and Bluetooth devs)
>>
>> i have Samsung HS3000 bluetooth headset. Mostly it works but there are
>> different issue with it:
>>
>> - volume level seems to be set to max (at really uncomfortable level)
>> - at the moment i trying to change volume with pavucontrol or after it i
>> get bad sound. Some times sort of cracky or trashy sound, some times
>> just white noise.
>> - play/stop key do not works with linux (not pulse issue)
>>
>> how to troubleshoot this issues? what tools should i use and where
>> should it start?
>
> Regarding the broken audio: I would suspect that the headset is buggy,
hehe, nice try :) This headset works perfectly with my
smartphone/cyanogenmod.
> because the volume handling code should be pretty well tested in
> pulseaudio, and I don't think bluetoothd touches the audio signal at
> all. To confirm that, I think it's somehow possible to capture the audio
> that is sent to the headset, but I don't know how. Maybe the bluez
> people can answer that (I'm a pulseaudio guy myself).
>
> Are you using the headset in the HSP or A2DP mode, btw? If A2DP, then
> that would suggest a pulseaudio bug, since I don't think the headset
> will get any notification via A2DP when you change the volume in
> pavucontrol.
You right, i played with different profiles. HSP controls volume on
pulse, but minimal volume is still too loud. A2DP use internal volume
control and i can set it as silent as i need.
I made some more tests with different kernels. It looks more like kernel
issue. I get different oopses and it looks like 3.5.0 sounds better then
current master git.
--
Regards,
Oleksij
^ permalink raw reply
* Re: [pulseaudio-discuss] how to troubleshoot bluetooth audio issues?
From: Tanu Kaskinen @ 2012-10-14 15:59 UTC (permalink / raw)
To: Oleksij Rempel; +Cc: pulseaudio-discuss, linux-bluetooth
In-Reply-To: <507ADA75.9040204@fisher-privat.net>
On Sun, 2012-10-14 at 17:29 +0200, Oleksij Rempel wrote:
> Hallo all, (Pulseaudio and Bluetooth devs)
>
> i have Samsung HS3000 bluetooth headset. Mostly it works but there are
> different issue with it:
>
> - volume level seems to be set to max (at really uncomfortable level)
> - at the moment i trying to change volume with pavucontrol or after it i
> get bad sound. Some times sort of cracky or trashy sound, some times
> just white noise.
> - play/stop key do not works with linux (not pulse issue)
>
> how to troubleshoot this issues? what tools should i use and where
> should it start?
Regarding the broken audio: I would suspect that the headset is buggy,
because the volume handling code should be pretty well tested in
pulseaudio, and I don't think bluetoothd touches the audio signal at
all. To confirm that, I think it's somehow possible to capture the audio
that is sent to the headset, but I don't know how. Maybe the bluez
people can answer that (I'm a pulseaudio guy myself).
Are you using the headset in the HSP or A2DP mode, btw? If A2DP, then
that would suggest a pulseaudio bug, since I don't think the headset
will get any notification via A2DP when you change the volume in
pavucontrol.
--
Tanu
^ permalink raw reply
* how to troubleshoot bluetooth audio issues?
From: Oleksij Rempel @ 2012-10-14 15:29 UTC (permalink / raw)
To: pulseaudio-discuss, linux-bluetooth
Hallo all, (Pulseaudio and Bluetooth devs)
i have Samsung HS3000 bluetooth headset. Mostly it works but there are
different issue with it:
- volume level seems to be set to max (at really uncomfortable level)
- at the moment i trying to change volume with pavucontrol or after it i
get bad sound. Some times sort of cracky or trashy sound, some times
just white noise.
- play/stop key do not works with linux (not pulse issue)
how to troubleshoot this issues? what tools should i use and where
should it start?
--
Regards,
Oleksij
^ permalink raw reply
* Re: [PATCH] heartrate: Convert to DBus.Properties
From: Johan Hedberg @ 2012-10-13 19:31 UTC (permalink / raw)
To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1350147379-10912-1-git-send-email-andrzej.kaczmarek@tieto.com>
H Andrzej,
On Sat, Oct 13, 2012, Andrzej Kaczmarek wrote:
> ---
> doc/heartrate-api.txt | 7 +---
> profiles/heartrate/heartrate.c | 73 ++++++++++++++++++++++++------------------
> test/test-heartrate | 5 ++-
> 3 files changed, 46 insertions(+), 39 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* [PATCH] heartrate: Convert to DBus.Properties
From: Andrzej Kaczmarek @ 2012-10-13 16:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
---
doc/heartrate-api.txt | 7 +---
profiles/heartrate/heartrate.c | 73 ++++++++++++++++++++++++------------------
test/test-heartrate | 5 ++-
3 files changed, 46 insertions(+), 39 deletions(-)
diff --git a/doc/heartrate-api.txt b/doc/heartrate-api.txt
index 39a0cbd..d1d5191 100644
--- a/doc/heartrate-api.txt
+++ b/doc/heartrate-api.txt
@@ -28,12 +28,7 @@ Service org.bluez
Interface org.bluez.HeartRate
Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
-Methods dict GetProperties()
-
- Returns all properties for the interface. See the
- Properties section for the available properties.
-
- Reset()
+Methods Reset()
Restart the accumulation of energy expended from zero.
diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 28c1932..9a92960 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -641,43 +641,47 @@ static const GDBusMethodTable heartrate_manager_methods[] = {
{ }
};
-static DBusMessage *get_properties(DBusConnection *conn, DBusMessage *msg,
- void *data)
+static gboolean property_get_location(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
{
struct heartrate *hr = data;
- DBusMessageIter iter;
- DBusMessageIter dict;
- DBusMessage *reply;
- gboolean has_reset;
+ char *loc;
- reply = dbus_message_new_method_return(msg);
- if (reply == NULL)
- return NULL;
+ if (!hr->has_location)
+ return FALSE;
- dbus_message_iter_init_append(reply, &iter);
+ loc = g_strdup(location2str(hr->location));
- dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
- DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
- DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
- DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+ if (loc == NULL)
+ return FALSE;
- if (hr->has_location) {
- char *loc = g_strdup(location2str(hr->location));
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &loc);
- if (loc) {
- dict_append_entry(&dict, "Location",
- DBUS_TYPE_STRING, &loc);
- g_free(loc);
- }
- }
+ g_free(loc);
- has_reset = !!hr->hrcp_val_handle;
- dict_append_entry(&dict, "ResetSupported", DBUS_TYPE_BOOLEAN,
- &has_reset);
+ return TRUE;
+}
- dbus_message_iter_close_container(&iter, &dict);
+static gboolean property_exists_location(const GDBusPropertyTable *property,
+ void *data)
+{
+ struct heartrate *hr = data;
+
+ if (!hr->has_location || location2str(hr->location) == NULL)
+ return FALSE;
+
+ return TRUE;
+}
+
+static gboolean property_get_reset_supported(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct heartrate *hr = data;
+ dbus_bool_t has_reset = !!hr->hrcp_val_handle;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &has_reset);
- return reply;
+ return TRUE;
}
static DBusMessage *hrcp_reset(DBusConnection *conn, DBusMessage *msg,
@@ -704,13 +708,17 @@ static DBusMessage *hrcp_reset(DBusConnection *conn, DBusMessage *msg,
}
static const GDBusMethodTable heartrate_device_methods[] = {
- { GDBUS_METHOD("GetProperties",
- NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
- get_properties) },
{ GDBUS_METHOD("Reset", NULL, NULL, hrcp_reset) },
{ }
};
+static const GDBusPropertyTable heartrate_device_properties[] = {
+ { "Location", "s", property_get_location, NULL,
+ property_exists_location },
+ { "ResetSupported", "b", property_get_reset_supported },
+ { }
+};
+
int heartrate_adapter_register(struct btd_adapter *adapter)
{
struct heartrate_adapter *hradapter;
@@ -772,8 +780,9 @@ int heartrate_device_register(struct btd_device *device,
device_get_path(device),
HEART_RATE_INTERFACE,
heartrate_device_methods,
- NULL, NULL, hr,
- destroy_heartrate)) {
+ NULL,
+ heartrate_device_properties,
+ hr, destroy_heartrate)) {
error("D-Bus failed to register %s interface",
HEART_RATE_INTERFACE);
destroy_heartrate(hr);
diff --git a/test/test-heartrate b/test/test-heartrate
index 316375d..a7d05b4 100755
--- a/test/test-heartrate
+++ b/test/test-heartrate
@@ -80,7 +80,10 @@ if __name__ == "__main__":
watcher = Watcher(bus, path)
- properties = heartrate.GetProperties()
+ dev_prop = dbus.Interface(bus.get_object("org.bluez", device_path),
+ "org.freedesktop.DBus.Properties")
+
+ properties = dev_prop.GetAll("org.bluez.HeartRate")
if "Location" in properties:
print("Sensor location: %s" % properties["Location"])
--
1.7.11.3
^ permalink raw reply related
* Re: [PATCH 1/2] heartrate: Fix registration of notification handler
From: Johan Hedberg @ 2012-10-13 16:49 UTC (permalink / raw)
To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1350141502-4385-1-git-send-email-andrzej.kaczmarek@tieto.com>
Hi Andrzej,
On Sat, Oct 13, 2012, Andrzej Kaczmarek wrote:
> Notification handler is registered only when CCC is written during
> descriptors discovery, i.e. at least one watcher is registered before
> device is connected. This means there will be no handler registered in
> case watcher is registered after device already connected.
> This is side-effect of 74a9fc7.
>
> This patch registers handler immediately when measurement characteristic
> is discovered so it does not matter when watcher is registered.
>
> ccc_write_cb() is reduntant in this case so it's removed.
> ---
> profiles/heartrate/heartrate.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
Both patches have been applied. Thanks.
Johan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox