* [PATCH] Write discoverable mode on storage after HCI response
From: chanyeol.park @ 2012-08-08 13:03 UTC (permalink / raw)
To: linux-bluetooth
From: Chan-yeol Park <chanyeol.park@samsung.com>
Right after HCI mode command is sent, Bluez writes mode value
on the storage. but in case of DiscoverableTimeout, storage value
is not updated like adapter->mode value. So this logic is moved.
---
src/adapter.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index acb845e..1b8e3b0 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -302,7 +302,6 @@ static int set_mode(struct btd_adapter *adapter, uint8_t new_mode,
done:
modestr = mode2str(new_mode);
- write_device_mode(&adapter->bdaddr, modestr);
DBG("%s", modestr);
@@ -2244,6 +2243,12 @@ static void set_mode_complete(struct btd_adapter *adapter)
adapter->mode_sessions = NULL;
}
+ modestr = mode2str(adapter->mode);
+
+ DBG("%s", modestr);
+
+ write_device_mode(&adapter->bdaddr, modestr);
+
if (adapter->pending_mode == NULL)
return;
@@ -2268,10 +2273,6 @@ static void set_mode_complete(struct btd_adapter *adapter)
g_dbus_send_message(connection, reply);
}
- modestr = mode2str(adapter->mode);
-
- DBG("%s", modestr);
-
/* restore if the mode doesn't matches the pending */
if (err != 0) {
write_device_mode(&adapter->bdaddr, modestr);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2] Bluetooth: Another vendor specific ID for BCM20702A0 [0a5c:21f1]
From: Michal Marek @ 2012-08-08 8:37 UTC (permalink / raw)
To: Gustavo Padovan, linux-bluetooth, marcel, linux-kernel,
John Hommel
In-Reply-To: <20120806184047.GD3146@joana>
On 6.8.2012 20:40, Gustavo Padovan wrote:
> Author: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Date: Mon Aug 6 15:36:49 2012 -0300
>
> Bluetooth: Use USB_VENDOR_AND_INTERFACE_INFO() for Broadcom devices
>
> Many Broadcom devices has a vendor specific devices class, with this rule
> we match all existent and future controllers with this behavior.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index cef3bac..504231e 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -104,6 +104,9 @@ static struct usb_device_id btusb_table[] = {
> /* Foxconn - Hon Hai */
> { USB_DEVICE(0x0489, 0xe033) },
>
> + /*Broadcom devices with vendor specific id */
> + { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01) },
Hi Gustavo,
Yes, this works. Well, I don't posses the card personally, but a helpful
person at HP confirmed that the btusb driver with this patch works with
the new card. So you can add
Tested-by: John Hommel <john.hommel@hp.com>
BTW, the following ID can also be removed after adding the above match:
{ USB_DEVICE(0x0a5c, 0x21e1) },
{ USB_DEVICE(0x0a5c, 0x21e3) },
{ USB_DEVICE(0x0a5c, 0x21e6) },
{ USB_DEVICE(0x0a5c, 0x21e8) },
{ USB_DEVICE(0x0a5c, 0x21f3) },
Thanks,
Michal
^ permalink raw reply
* [RFC v3 2/2] Bluetooth: mgmt: Add device disconnect reason
From: Mikel Astiz @ 2012-08-08 7:42 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1344411753-10124-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
MGMT_EV_DEVICE_DISCONNECTED will now expose the disconnection reason to
userland, distinguishing four possible values:
0x00 Reason not known or unspecified
0x01 ACL connection timeout
0x02 ACL connection terminated by local host
0x03 ACL connection terminated by remote host
Note that the local/remote distinction just determines which side
terminated the low-level ACL connection, regardless of the disconnection
of the higher-level profiles.
This can sometimes be misleading and thus must be used with care. For
example, some hardware combinations would report a locally initiated
disconnection even if the user turned Bluetooth off in the remote side.
Signed-off-by: Mikel Astiz <mikel.astiz@bmw-carit.de>
---
include/net/bluetooth/hci_core.h | 2 +-
include/net/bluetooth/mgmt.h | 8 ++++++++
net/bluetooth/hci_event.c | 24 +++++++++++++++++++++---
net/bluetooth/mgmt.c | 9 +++++----
4 files changed, 35 insertions(+), 8 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 41d9439..4fb0323 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1004,7 +1004,7 @@ int mgmt_device_connected(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
u8 addr_type, u32 flags, u8 *name, u8 name_len,
u8 *dev_class);
int mgmt_device_disconnected(struct hci_dev *hdev, bdaddr_t *bdaddr,
- u8 link_type, u8 addr_type);
+ u8 link_type, u8 addr_type, u8 reason);
int mgmt_disconnect_failed(struct hci_dev *hdev, bdaddr_t *bdaddr,
u8 link_type, u8 addr_type, u8 status);
int mgmt_connect_failed(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 4348ee8..e8b86e0 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -405,7 +405,15 @@ struct mgmt_ev_device_connected {
__u8 eir[0];
} __packed;
+#define MGMT_DEV_DISCONN_TIMEOUT 0x01
+#define MGMT_DEV_DISCONN_LOCAL_HOST 0x02
+#define MGMT_DEV_DISCONN_REMOTE 0x03
+
#define MGMT_EV_DEVICE_DISCONNECTED 0x000C
+struct mgmt_ev_device_disconnected {
+ struct mgmt_addr_info addr;
+ __u8 reason;
+} __packed;
#define MGMT_EV_CONNECT_FAILED 0x000D
struct mgmt_ev_connect_failed {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0386e1e..1323341 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -29,6 +29,7 @@
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
+#include <net/bluetooth/mgmt.h>
/* Handle HCI Event packets */
@@ -1906,12 +1907,29 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
if (test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags) &&
(conn->type == ACL_LINK || conn->type == LE_LINK)) {
- if (ev->status != 0)
+ if (ev->status != 0) {
mgmt_disconnect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, ev->status);
- else
+ } else {
+ u8 reason = 0;
+
+ switch (ev->reason) {
+ case HCI_ERROR_CONNECTION_TIMEOUT:
+ reason = MGMT_DEV_DISCONN_TIMEOUT;
+ break;
+ case HCI_ERROR_REMOTE_USER_TERM:
+ case HCI_ERROR_REMOTE_LOW_RESOURCES:
+ case HCI_ERROR_REMOTE_POWER_OFF:
+ reason = MGMT_DEV_DISCONN_REMOTE;
+ break;
+ case HCI_ERROR_LOCAL_HOST_TERM:
+ reason = MGMT_DEV_DISCONN_LOCAL_HOST;
+ break;
+ }
+
mgmt_device_disconnected(hdev, &conn->dst, conn->type,
- conn->dst_type);
+ conn->dst_type, reason);
+ }
}
if (ev->status == 0) {
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a3329cb..05d4b83 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3077,16 +3077,17 @@ static void unpair_device_rsp(struct pending_cmd *cmd, void *data)
}
int mgmt_device_disconnected(struct hci_dev *hdev, bdaddr_t *bdaddr,
- u8 link_type, u8 addr_type)
+ u8 link_type, u8 addr_type, u8 reason)
{
- struct mgmt_addr_info ev;
+ struct mgmt_ev_device_disconnected ev;
struct sock *sk = NULL;
int err;
mgmt_pending_foreach(MGMT_OP_DISCONNECT, hdev, disconnect_rsp, &sk);
- bacpy(&ev.bdaddr, bdaddr);
- ev.type = link_to_bdaddr(link_type, addr_type);
+ bacpy(&ev.addr.bdaddr, bdaddr);
+ ev.addr.type = link_to_bdaddr(link_type, addr_type);
+ ev.reason = reason;
err = mgmt_event(MGMT_EV_DEVICE_DISCONNECTED, hdev, &ev, sizeof(ev),
sk);
--
1.7.7.6
^ permalink raw reply related
* [RFC v3 1/2] Bluetooth: Add more HCI error codes
From: Mikel Astiz @ 2012-08-08 7:42 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1344411753-10124-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Add more HCI error codes as defined in the specification.
Signed-off-by: Mikel Astiz <mikel.astiz@bmw-carit.de>
---
include/net/bluetooth/hci.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 7f19556..7253bdd 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -302,8 +302,11 @@ enum {
/* ---- HCI Error Codes ---- */
#define HCI_ERROR_AUTH_FAILURE 0x05
+#define HCI_ERROR_CONNECTION_TIMEOUT 0x08
#define HCI_ERROR_REJ_BAD_ADDR 0x0f
#define HCI_ERROR_REMOTE_USER_TERM 0x13
+#define HCI_ERROR_REMOTE_LOW_RESOURCES 0x14
+#define HCI_ERROR_REMOTE_POWER_OFF 0x15
#define HCI_ERROR_LOCAL_HOST_TERM 0x16
#define HCI_ERROR_PAIRING_NOT_ALLOWED 0x18
--
1.7.7.6
^ permalink raw reply related
* [RFC v3 0/2] Disconnect reason in Kernel API
From: Mikel Astiz @ 2012-08-08 7:42 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
This patchset (split now in two patches) extends the management API to expose the disconnect reason in the device disconnection event.
The original motivation was to distinguish the timeout case from the rest, thus the first versions of the patch provided only this information.
v3 extends the original approach to also determine which side (local host or remote) initiated the ACL disconnection. This is IMO not very useful and misleading, but has still been considered interesting by the subsystem maintainers.
In order to test these patches, the old userland patch ("[RFC BlueZ v1]: mgmt-api: Add reason to device disconnect event") can be used, in terms of log traces, even though it needs the corresponding update.
>From original patch:
During the BlueZ meeting in Brazil it was proposed to add two more values to this enum: "Connection terminated by local host" and "Connection terminated by remote host". However, after some testing, it seems the result can be quite misleading. Therefore and given that there are no known use-cases that need this information (local vs remote disconnection), these two values have been dropped.
Mikel Astiz (2):
Bluetooth: Add more HCI error codes
Bluetooth: mgmt: Add device disconnect reason
include/net/bluetooth/hci.h | 3 +++
include/net/bluetooth/hci_core.h | 2 +-
include/net/bluetooth/mgmt.h | 8 ++++++++
net/bluetooth/hci_event.c | 24 +++++++++++++++++++++---
net/bluetooth/mgmt.c | 9 +++++----
5 files changed, 38 insertions(+), 8 deletions(-)
--
1.7.7.6
^ permalink raw reply
* Re: [RFC v2] Bluetooth: mgmt: Add device disconnect reason
From: Mikel Astiz @ 2012-08-08 7:41 UTC (permalink / raw)
To: Mikel Astiz, linux-bluetooth, Mikel Astiz
In-Reply-To: <20120730110050.GA13789@x220.ger.corp.intel.com>
Hi Johan,
On Mon, Jul 30, 2012 at 1:00 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Mikel,
>
> On Fri, Jul 20, 2012, Mikel Astiz wrote:
>> During the BlueZ meeting in Brazil it was proposed to add two more
>> values to this enum: "Connection terminated by local host" and
>> "Connection terminated by remote host". However, after some testing,
>> it seems the result can be quite misleading. Therefore and given that
>> there are no known use-cases that need this information (local vs
>> remote disconnection), these two values have been dropped.
>
> I still think that it wouldn't hurt to include which side triggers the
> HCI_Disconnect() command. We can always document that this identifies
> accurately the ACL disconnection initiator but not necessarily the
> higher-level profile(s) disconnection initiator.
Sorry for the later reply. I will send an updated version soon.
Cheers,
Mikel
^ permalink raw reply
* [PATCH BlueZ V4 5/5] AVRCP: Add handler for browsing pdu
From: Vani-dineshbhai PATEL @ 2012-08-08 6:57 UTC (permalink / raw)
To: User Name, Luiz Augusto, Lucas De Marchi; +Cc: Vani, Joohi, Vani
From: Vani Patel <vani.patel@stericsson.com>
Implement generic handling of browsing
PDU IDs
---
audio/avctp.c | 27 ++++++++++++++++++--
audio/avrcp.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 97 insertions(+), 3 deletions(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index b5f84aa..7bbcecc 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -458,9 +458,9 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
gpointer data)
{
struct avctp *session = data;
- uint8_t *buf;
+ uint8_t *operands, *buf;
struct avctp_header *avctp;
- int ret, sock;
+ int ret, packet_size, operand_count, sock;
buf = (uint8_t *)malloc(session->browsing_mtu);
if (!(cond & G_IO_IN))
@@ -472,6 +472,8 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
if (ret <= 0)
goto failed;
+ DBG("Got %d bytes of data for AVCTP Browsing session %p", ret, session);
+
if ((unsigned int) ret < sizeof(struct avctp_header)) {
error("Too small AVRCP packet on browsing channel");
goto failed;
@@ -479,9 +481,28 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
avctp = (struct avctp_header *) buf;
+ DBG("AVCTP transaction %u, packet type %u, C/R %u, IPID %u, "
+ "PID 0x%04X",
+ avctp->transaction, avctp->packet_type,
+ avctp->cr, avctp->ipid, ntohs(avctp->pid));
+
if (avctp->packet_type != AVCTP_PACKET_SINGLE)
goto failed;
+ operands = buf + AVCTP_HEADER_LENGTH;
+ ret -= AVCTP_HEADER_LENGTH;
+ operand_count = ret;
+
+ packet_size = AVCTP_HEADER_LENGTH;
+ avctp->cr = AVCTP_RESPONSE;
+ if (browsing_handler)
+ packet_size += browsing_handler->cb(session, avctp->transaction,
+ operands, operand_count, browsing_handler->user_data);
+ if (packet_size !=0 ) {
+ ret = write(sock, buf, packet_size);
+ if (ret != packet_size)
+ goto failed;
+ }
free(buf);
return TRUE;
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 9d29073..deba4a6 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -94,6 +94,9 @@
#define AVRCP_ABORT_CONTINUING 0x41
#define AVRCP_SET_ABSOLUTE_VOLUME 0x50
+#define AVRCP_INVALID_BROWSING_PDU 0x00
+
+#define AVRCP_GENERAL_REJECT 0xA0
/* Capabilities for AVRCP_GET_CAPABILITIES pdu */
#define CAP_COMPANY_ID 0x02
#define CAP_EVENTS_SUPPORTED 0x03
@@ -140,6 +143,12 @@ struct avrcp_header {
#error "Unknown byte order"
#endif
+struct avrcp_browsing_header {
+ uint8_t browsing_pdu;
+ uint16_t param_len;
+} __attribute__ ((packed));
+#define AVRCP_BROWSING_HEADER_LENGTH 3
+
#define AVRCP_MTU (AVC_MTU - AVC_HEADER_LENGTH)
#define AVRCP_PDU_MTU (AVRCP_MTU - AVRCP_HEADER_LENGTH)
@@ -163,7 +172,9 @@ struct avrcp_player {
struct audio_device *dev;
unsigned int control_handler;
+ unsigned int browsing_handler;
uint16_t registered_events;
+ uint8_t transaction;
uint8_t transaction_events[AVRCP_EVENT_LAST + 1];
struct pending_pdu *pending_pdu;
@@ -1075,6 +1086,15 @@ static struct control_pdu_handler {
{ },
};
+static struct pdu_browsing_handler {
+ uint8_t browsing_pdu;
+ void (*func) (struct avrcp_player *player,
+ struct avrcp_browsing_header *pdu);
+ } browsing_handlers[] = {
+ { AVRCP_INVALID_BROWSING_PDU,
+ NULL },
+};
+
/* handle vendordep pdu inside an avctp packet */
static size_t handle_vendordep_pdu(struct avctp *session, uint8_t transaction,
uint8_t *code, uint8_t *subunit,
@@ -1134,6 +1154,49 @@ err_metadata:
return AVRCP_HEADER_LENGTH + 1;
}
+static size_t handle_browsing_pdu(struct avctp *session,
+ uint8_t transaction, uint8_t *operands,
+ size_t operand_count, void *user_data)
+{
+ struct avrcp_player *player = user_data;
+ struct pdu_browsing_handler *b_handler;
+ struct avrcp_browsing_header *avrcp_browsing = (void *) operands;
+ uint8_t status;
+
+ operand_count += AVRCP_BROWSING_HEADER_LENGTH;
+
+ for (b_handler = browsing_handlers; b_handler; b_handler++) {
+ if (b_handler->browsing_pdu == AVRCP_INVALID_BROWSING_PDU) {
+ b_handler = NULL;
+ break;
+ }
+ if (b_handler->browsing_pdu == avrcp_browsing->browsing_pdu)
+ break;
+ }
+
+ if (!b_handler) {
+ avrcp_browsing->browsing_pdu = AVRCP_GENERAL_REJECT;
+ status = AVRCP_STATUS_INVALID_COMMAND;
+ goto err;
+ }
+
+ if (!b_handler->func) {
+ status = AVRCP_STATUS_INVALID_PARAM;
+ avrcp_browsing->param_len = htons(sizeof(status));
+ goto err;
+ }
+ player->transaction = transaction;
+ b_handler->func(player, avrcp_browsing);
+ return AVRCP_BROWSING_HEADER_LENGTH + ntohs(avrcp_browsing->param_len);
+
+err:
+ avrcp_browsing->param_len = htons(sizeof(status));
+ memcpy(&operands[AVRCP_BROWSING_HEADER_LENGTH], &status,
+ (sizeof(status)));
+ return AVRCP_BROWSING_HEADER_LENGTH + sizeof(status);
+}
+
+
size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands)
{
struct avrcp_header *pdu = (void *) operands;
@@ -1233,6 +1296,10 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
avctp_unregister_pdu_handler(player->control_handler);
player->control_handler = 0;
}
+ if (player->browsing_handler) {
+ avctp_unregister_browsing_pdu_handler();
+ player->browsing_handler = 0;
+ }
break;
case AVCTP_STATE_CONNECTING:
@@ -1244,6 +1311,12 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
AVC_OP_VENDORDEP,
handle_vendordep_pdu,
player);
+ if (!player->browsing_handler)
+ player->browsing_handler =
+ avctp_register_browsing_pdu_handler(
+ handle_browsing_pdu,
+ player);
+
break;
case AVCTP_STATE_CONNECTED:
rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
--
1.7.5.4
^ permalink raw reply related
* [PATCH BlueZ V4 4/5] AVRCP: Register/Unregister Browsing handler
From: Vani-dineshbhai PATEL @ 2012-08-08 6:57 UTC (permalink / raw)
To: User Name, Luiz Augusto, Lucas De Marchi; +Cc: Vani, Joohi, Vani
From: Vani Patel <vani.patel@stericsson.com>
Add functions to register and unregister Browsing
handler
---
audio/avctp.c | 26 ++++++++++++++++++++++++++
audio/avctp.h | 8 ++++++++
2 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index 96c5605..b5f84aa 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -157,6 +157,11 @@ struct avctp_pdu_handler {
unsigned int id;
};
+struct avctp_browsing_pdu_handler {
+ avctp_browsing_pdu_cb cb;
+ void *user_data;
+};
+
static struct {
const char *name;
uint8_t avc;
@@ -176,6 +181,7 @@ 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);
@@ -1251,6 +1257,18 @@ unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
return handler->id;
}
+unsigned int avctp_register_browsing_pdu_handler(avctp_browsing_pdu_cb cb,
+ void *user_data)
+{
+ unsigned int id = 0;
+
+ browsing_handler = g_new(struct avctp_browsing_pdu_handler, 1);
+ browsing_handler->cb = cb;
+ browsing_handler->user_data = user_data;
+
+ return ++id;
+}
+
gboolean avctp_unregister_pdu_handler(unsigned int id)
{
GSList *l;
@@ -1269,6 +1287,14 @@ gboolean avctp_unregister_pdu_handler(unsigned int id)
return FALSE;
}
+gboolean avctp_unregister_browsing_pdu_handler()
+{
+ if (browsing_handler)
+ g_free(browsing_handler);
+
+ return TRUE;
+}
+
struct avctp *avctp_connect(const bdaddr_t *src, const bdaddr_t *dst)
{
struct avctp *session;
diff --git a/audio/avctp.h b/audio/avctp.h
index b80e300..3e1dabe 100644
--- a/audio/avctp.h
+++ b/audio/avctp.h
@@ -83,6 +83,10 @@ typedef size_t (*avctp_control_pdu_cb) (struct avctp *session,
typedef gboolean (*avctp_rsp_cb) (struct avctp *session, uint8_t code,
uint8_t subunit, uint8_t *operands,
size_t operand_count, void *user_data);
+typedef size_t (*avctp_browsing_pdu_cb) (struct avctp *session,
+ uint8_t transaction,
+ uint8_t *operands, size_t operand_count,
+ void *user_data);
unsigned int avctp_add_state_cb(avctp_state_cb cb, void *user_data);
gboolean avctp_remove_state_cb(unsigned int id);
@@ -98,6 +102,10 @@ unsigned int avctp_register_pdu_handler(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);
+
+gboolean avctp_unregister_browsing_pdu_handler();
int avctp_send_passthrough(struct avctp *session, uint8_t op);
int avctp_send_vendordep(struct avctp *session, uint8_t transaction,
uint8_t code, uint8_t subunit,
--
1.7.5.4
^ permalink raw reply related
* [PATCH BlueZ V3 3/5] AVRCP: Add browsing channel support
From: Vani-dineshbhai PATEL @ 2012-08-08 6:56 UTC (permalink / raw)
To: User Name, Luiz Augusto, Lucas De Marchi; +Cc: Vani, Joohi, Vani
From: Vani Patel <vani.patel@stericsson.com>
Implements browsing channel creation and release.
---
audio/avctp.c | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 215 insertions(+), 19 deletions(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index 33ca007..69bb6a8 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -41,6 +41,7 @@
#include <bluetooth/bluetooth.h>
#include <bluetooth/sdp.h>
#include <bluetooth/uuid.h>
+#include <bluetooth/l2cap.h>
#include <glib.h>
@@ -119,6 +120,7 @@ struct avctp_state_callback {
struct avctp_server {
bdaddr_t src;
GIOChannel *control_io;
+ GIOChannel *browsing_io;
GSList *sessions;
};
@@ -137,9 +139,12 @@ struct avctp {
int uinput;
GIOChannel *control_io;
+ GIOChannel *browsing_io;
guint control_io_id;
+ guint browsing_io_id;
uint16_t control_mtu;
+ uint16_t browsing_mtu;
uint8_t key_quirks[256];
GSList *handlers;
@@ -326,6 +331,17 @@ 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);
@@ -432,6 +448,42 @@ static void handle_response(struct avctp *session, struct avctp_header *avctp,
}
}
+static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
+ gpointer data)
+{
+ struct avctp *session = data;
+ uint8_t *buf;
+ struct avctp_header *avctp;
+ int ret, sock;
+
+ buf = (uint8_t *)malloc(session->browsing_mtu);
+ if (!(cond & G_IO_IN))
+ return FALSE;
+
+ sock = g_io_channel_unix_get_fd(session->browsing_io);
+ ret = read(sock, buf, session->browsing_mtu);
+
+ if (ret <= 0)
+ goto failed;
+
+ if ((unsigned int) ret < sizeof(struct avctp_header)) {
+ error("Too small AVRCP packet on browsing channel");
+ goto failed;
+ }
+
+ avctp = (struct avctp_header *) buf;
+
+ if (avctp->packet_type != AVCTP_PACKET_SINGLE)
+ goto failed;
+
+ free(buf);
+ return TRUE;
+
+failed:
+ free(buf);
+ return FALSE;
+}
+
static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
gpointer data)
{
@@ -614,6 +666,47 @@ static void init_uinput(struct avctp *session)
DBG("AVRCP: uinput initialized for %s", address);
}
+static void avctp_connect_browsing_cb(GIOChannel *chan,
+ GError *err,
+ gpointer data)
+{
+ struct avctp *session = data;
+ char address[18];
+ uint16_t imtu;
+ GError *gerr = NULL;
+
+ if (err) {
+ error("Browsing: %s", err->message);
+ g_io_channel_shutdown(chan, TRUE, NULL);
+ g_io_channel_unref(chan);
+ session->browsing_io = NULL;
+ return;
+ }
+
+ bt_io_get(chan, BT_IO_L2CAP, &gerr,
+ BT_IO_OPT_DEST, &address,
+ BT_IO_OPT_IMTU, &imtu,
+ BT_IO_OPT_INVALID);
+ if (gerr) {
+ error("%s", gerr->message);
+ g_io_channel_shutdown(chan, TRUE, NULL);
+ g_io_channel_unref(chan);
+ session->browsing_io = NULL;
+ error("%s", gerr->message);
+ g_error_free(gerr);
+ return;
+ }
+
+ if (!session->browsing_io)
+ session->browsing_io = g_io_channel_ref(chan);
+
+ session->browsing_mtu = imtu;
+
+ session->browsing_io_id = g_io_add_watch(chan,
+ G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+ (GIOFunc) session_browsing_cb, session);
+}
+
static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
{
struct avctp *session = data;
@@ -652,6 +745,32 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
(GIOFunc) session_cb, session);
}
+static void auth_browsing_cb(DBusError *derr, void *user_data)
+{
+ struct avctp *session = user_data;
+ GError *err = NULL;
+
+ if (session->browsing_io_id) {
+ g_source_remove(session->browsing_io_id);
+ session->browsing_io_id = 0;
+ }
+
+ if (derr && dbus_error_is_set(derr)) {
+ error("Browsing Access denied: %s", derr->message);
+ return;
+ }
+
+ if (!bt_io_accept(session->browsing_io, avctp_connect_browsing_cb,
+ session, NULL, &err)) {
+ error("Browsing bt_io_accept: %s", err->message);
+ if (session && session->browsing_io) {
+ g_io_channel_unref(session->browsing_io);
+ session->browsing_io = NULL;
+ }
+ g_error_free(err);
+ }
+}
+
static void auth_cb(DBusError *derr, void *user_data)
{
struct avctp *session = user_data;
@@ -730,6 +849,65 @@ static struct avctp *avctp_get_internal(const bdaddr_t *src,
return session;
}
+static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
+ struct audio_device *dev)
+{
+ if (session->control_io) {
+ error("Refusing unexpected connect from");
+ goto drop;
+ }
+
+ avctp_set_state(session, AVCTP_STATE_CONNECTING);
+ session->control_io = g_io_channel_ref(chan);
+
+ if (audio_device_request_authorization(dev, AVRCP_TARGET_UUID,
+ auth_cb, session) < 0)
+ goto drop;
+
+ session->control_io_id = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP |
+ G_IO_NVAL, session_cb, session);
+ return;
+
+drop:
+ if (!session || !session->control_io)
+ g_io_channel_shutdown(chan, TRUE, NULL);
+
+ if (session && session->control_io)
+ g_io_channel_unref(session->control_io);
+
+ if (session)
+ avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
+}
+
+static void avctp_browsing_confirm(struct avctp *session, GIOChannel *chan,
+ struct audio_device *dev)
+{
+
+ if (!session->control_io) {
+ error("Browsing: Refusing unexpected connect from");
+ goto drop;
+ }
+
+ if (session->browsing_io) {
+ error("Browsing channel already exists");
+ goto drop;
+ }
+
+ session->browsing_io = g_io_channel_ref(chan);
+
+ if (audio_device_request_authorization(dev, AVRCP_TARGET_UUID,
+ auth_browsing_cb, session) < 0)
+ goto drop;
+ return;
+
+drop:
+ if (!session || !session->control_io || !session->browsing_io)
+ g_io_channel_shutdown(chan, TRUE, NULL);
+
+ if (session && session->browsing_io)
+ g_io_channel_unref(session->browsing_io);
+}
+
static void avctp_confirm_cb(GIOChannel *chan, gpointer data)
{
struct avctp *session;
@@ -737,11 +915,13 @@ static void avctp_confirm_cb(GIOChannel *chan, gpointer data)
char address[18];
bdaddr_t src, dst;
GError *err = NULL;
+ uint16_t psm;
bt_io_get(chan, BT_IO_L2CAP, &err,
BT_IO_OPT_SOURCE_BDADDR, &src,
BT_IO_OPT_DEST_BDADDR, &dst,
BT_IO_OPT_DEST, address,
+ BT_IO_OPT_PSM, &psm,
BT_IO_OPT_INVALID);
if (err) {
error("%s", err->message);
@@ -772,40 +952,40 @@ static void avctp_confirm_cb(GIOChannel *chan, gpointer data)
goto drop;
}
- if (session->control_io) {
- error("Refusing unexpected connect from %s", address);
- goto drop;
+ switch (psm) {
+ case AVCTP_CONTROL_PSM:
+ avctp_control_confirm(session, chan, dev);
+ break;
+ case AVCTP_BROWSING_PSM:
+ avctp_browsing_confirm(session, chan, dev);
+ break;
}
- avctp_set_state(session, AVCTP_STATE_CONNECTING);
- session->control_io = g_io_channel_ref(chan);
-
- if (audio_device_request_authorization(dev, AVRCP_TARGET_UUID,
- auth_cb, session) < 0)
- goto drop;
-
- session->control_io_id = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP |
- G_IO_NVAL, session_cb, session);
return;
drop:
- if (!session || !session->control_io)
- g_io_channel_shutdown(chan, TRUE, NULL);
- if (session)
+ 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)
avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
}
-static GIOChannel *avctp_server_socket(const bdaddr_t *src, gboolean master)
+static GIOChannel *avctp_server_socket(const bdaddr_t *src, gboolean master,
+ uint8_t mode, uint16_t psm)
{
GError *err = NULL;
GIOChannel *io;
-
io = bt_io_listen(BT_IO_L2CAP, NULL, avctp_confirm_cb, NULL,
NULL, &err,
BT_IO_OPT_SOURCE_BDADDR, src,
- BT_IO_OPT_PSM, AVCTP_CONTROL_PSM,
+ BT_IO_OPT_PSM, psm,
BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
BT_IO_OPT_MASTER, master,
+ BT_IO_OPT_MODE, mode,
BT_IO_OPT_INVALID);
if (!io) {
error("%s", err->message);
@@ -825,12 +1005,25 @@ int avctp_register(const bdaddr_t *src, gboolean master)
server = g_new0(struct avctp_server, 1);
- server->control_io = avctp_server_socket(src, master);
+ server->control_io = avctp_server_socket(src, master, L2CAP_MODE_BASIC,
+ AVCTP_CONTROL_PSM);
if (!server->control_io) {
g_free(server);
return -1;
}
+ server->browsing_io = avctp_server_socket(src, master, L2CAP_MODE_ERTM,
+ AVCTP_BROWSING_PSM);
+ if (!server->browsing_io) {
+ if (server->control_io) {
+ g_io_channel_shutdown(server->control_io, TRUE, NULL);
+ g_io_channel_unref(server->control_io);
+ server->control_io = NULL;
+ }
+ g_free(server);
+ return -1;
+ }
+
bacpy(&server->src, src);
servers = g_slist_append(servers, server);
@@ -862,6 +1055,9 @@ void avctp_unregister(const bdaddr_t *src)
avctp_disconnected(server->sessions->data);
servers = g_slist_remove(servers, server);
+ g_io_channel_shutdown(server->browsing_io, TRUE, NULL);
+ g_io_channel_unref(server->browsing_io);
+ server->browsing_io = NULL;
g_io_channel_shutdown(server->control_io, TRUE, NULL);
g_io_channel_unref(server->control_io);
--
1.7.5.4
^ permalink raw reply related
* [PATCH BlueZ V3 2/5] AVRCP: Rename variables used for control channel
From: Vani-dineshbhai PATEL @ 2012-08-08 6:56 UTC (permalink / raw)
To: User Name, Luiz Augusto, Lucas De Marchi; +Cc: Vani, Joohi, Vani
From: Vani Patel <vani.patel@stericsson.com>
Prefix "control" is added to variables used for
control channel. This will improve redability
when Browsing channel is implemented
---
audio/avctp.c | 83 +++++++++++++++++++++++++++++----------------------------
audio/avctp.h | 5 ++-
audio/avrcp.c | 54 ++++++++++++++++++------------------
3 files changed, 72 insertions(+), 70 deletions(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index a20dba9..3bd021c 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -118,7 +118,7 @@ struct avctp_state_callback {
struct avctp_server {
bdaddr_t src;
- GIOChannel *io;
+ GIOChannel *control_io;
GSList *sessions;
};
@@ -136,10 +136,10 @@ struct avctp {
int uinput;
- GIOChannel *io;
- guint io_id;
+ GIOChannel *control_io;
+ guint control_io_id;
- uint16_t mtu;
+ uint16_t control_mtu;
uint8_t key_quirks[256];
GSList *handlers;
@@ -147,7 +147,7 @@ struct avctp {
struct avctp_pdu_handler {
uint8_t opcode;
- avctp_pdu_cb cb;
+ avctp_control_pdu_cb cb;
void *user_data;
unsigned int id;
};
@@ -169,7 +169,7 @@ static struct {
static GSList *callbacks = NULL;
static GSList *servers = NULL;
-static GSList *handlers = NULL;
+static GSList *control_handlers = NULL;
static uint8_t id = 0;
static void auth_cb(DBusError *derr, void *user_data);
@@ -326,15 +326,15 @@ static void avctp_disconnected(struct avctp *session)
if (!session)
return;
- if (session->io) {
- g_io_channel_shutdown(session->io, TRUE, NULL);
- g_io_channel_unref(session->io);
- session->io = NULL;
+ 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->io_id) {
- g_source_remove(session->io_id);
- session->io_id = 0;
+ if (session->control_io_id) {
+ g_source_remove(session->control_io_id);
+ session->control_io_id = 0;
if (session->state == AVCTP_STATE_CONNECTING) {
struct audio_device *dev;
@@ -445,7 +445,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->io);
+ sock = g_io_channel_unix_get_fd(session->control_io);
ret = read(sock, buf, sizeof(buf));
if (ret <= 0)
@@ -503,7 +503,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
goto done;
}
- handler = find_handler(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);
@@ -639,14 +639,14 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
DBG("AVCTP: connected to %s", address);
- if (!session->io)
- session->io = g_io_channel_ref(chan);
+ if (!session->control_io)
+ session->control_io = g_io_channel_ref(chan);
init_uinput(session);
avctp_set_state(session, AVCTP_STATE_CONNECTED);
- session->mtu = imtu;
- session->io_id = g_io_add_watch(chan,
+ 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);
}
@@ -656,9 +656,9 @@ static void auth_cb(DBusError *derr, void *user_data)
struct avctp *session = user_data;
GError *err = NULL;
- if (session->io_id) {
- g_source_remove(session->io_id);
- session->io_id = 0;
+ if (session->control_io_id) {
+ g_source_remove(session->control_io_id);
+ session->control_io_id = 0;
}
if (derr && dbus_error_is_set(derr)) {
@@ -667,7 +667,7 @@ static void auth_cb(DBusError *derr, void *user_data)
return;
}
- if (!bt_io_accept(session->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);
@@ -771,24 +771,24 @@ static void avctp_confirm_cb(GIOChannel *chan, gpointer data)
goto drop;
}
- if (session->io) {
+ if (session->control_io) {
error("Refusing unexpected connect from %s", address);
goto drop;
}
avctp_set_state(session, AVCTP_STATE_CONNECTING);
- session->io = g_io_channel_ref(chan);
+ session->control_io = g_io_channel_ref(chan);
if (audio_device_request_authorization(dev, AVRCP_TARGET_UUID,
auth_cb, session) < 0)
goto drop;
- session->io_id = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
- session_cb, session);
+ session->control_io_id = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP |
+ G_IO_NVAL, session_cb, session);
return;
drop:
- if (!session || !session->io)
+ if (!session || !session->control_io)
g_io_channel_shutdown(chan, TRUE, NULL);
if (session)
avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
@@ -824,8 +824,8 @@ int avctp_register(const bdaddr_t *src, gboolean master)
server = g_new0(struct avctp_server, 1);
- server->io = avctp_server_socket(src, master);
- if (!server->io) {
+ server->control_io = avctp_server_socket(src, master);
+ if (!server->control_io) {
g_free(server);
return -1;
}
@@ -862,8 +862,8 @@ void avctp_unregister(const bdaddr_t *src)
servers = g_slist_remove(servers, server);
- g_io_channel_shutdown(server->io, TRUE, NULL);
- g_io_channel_unref(server->io);
+ g_io_channel_shutdown(server->control_io, TRUE, NULL);
+ g_io_channel_unref(server->control_io);
g_free(server);
if (servers)
@@ -910,7 +910,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->io);
+ sk = g_io_channel_unix_get_fd(session->control_io);
if (write(sk, buf, sizeof(buf)) < 0)
return -errno;
@@ -939,7 +939,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->io);
+ sk = g_io_channel_unix_get_fd(session->control_io);
memset(buf, 0, sizeof(buf));
@@ -1034,13 +1034,13 @@ gboolean avctp_remove_state_cb(unsigned int id)
return FALSE;
}
-unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_pdu_cb cb,
+unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
void *user_data)
{
struct avctp_pdu_handler *handler;
static unsigned int id = 0;
- handler = find_handler(handlers, opcode);
+ handler = find_handler(control_handlers, opcode);
if (handler)
return 0;
@@ -1050,7 +1050,7 @@ unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_pdu_cb cb,
handler->user_data = user_data;
handler->id = ++id;
- handlers = g_slist_append(handlers, handler);
+ control_handlers = g_slist_append(control_handlers, handler);
return handler->id;
}
@@ -1059,11 +1059,12 @@ gboolean avctp_unregister_pdu_handler(unsigned int id)
{
GSList *l;
- for (l = handlers; l != NULL; l = l->next) {
+ for (l = control_handlers; l != NULL; l = l->next) {
struct avctp_pdu_handler *handler = l->data;
if (handler->id == id) {
- handlers = g_slist_remove(handlers, handler);
+ control_handlers = g_slist_remove(control_handlers,
+ handler);
g_free(handler);
return TRUE;
}
@@ -1099,14 +1100,14 @@ struct avctp *avctp_connect(const bdaddr_t *src, const bdaddr_t *dst)
return NULL;
}
- session->io = io;
+ session->control_io = io;
return session;
}
void avctp_disconnect(struct avctp *session)
{
- if (!session->io)
+ if (!session->control_io)
return;
avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
diff --git a/audio/avctp.h b/audio/avctp.h
index 34b0c1c..b80e300 100644
--- a/audio/avctp.h
+++ b/audio/avctp.h
@@ -75,7 +75,8 @@ typedef void (*avctp_state_cb) (struct audio_device *dev,
avctp_state_t new_state,
void *user_data);
-typedef size_t (*avctp_pdu_cb) (struct avctp *session, uint8_t transaction,
+typedef size_t (*avctp_control_pdu_cb) (struct avctp *session,
+ uint8_t transaction,
uint8_t *code, uint8_t *subunit,
uint8_t *operands, size_t operand_count,
void *user_data);
@@ -93,7 +94,7 @@ struct avctp *avctp_connect(const bdaddr_t *src, const bdaddr_t *dst);
struct avctp *avctp_get(const bdaddr_t *src, const bdaddr_t *dst);
void avctp_disconnect(struct avctp *session);
-unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_pdu_cb cb,
+unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
void *user_data);
gboolean avctp_unregister_pdu_handler(unsigned int id);
diff --git a/audio/avrcp.c b/audio/avrcp.c
index ca40c1e..9d29073 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -162,7 +162,7 @@ struct avrcp_player {
struct avctp *session;
struct audio_device *dev;
- unsigned int handler;
+ unsigned int control_handler;
uint16_t registered_events;
uint8_t transaction_events[AVRCP_EVENT_LAST + 1];
struct pending_pdu *pending_pdu;
@@ -255,9 +255,9 @@ static sdp_record_t *avrcp_tg_record(void)
sdp_list_t *svclass_id, *pfseq, *apseq, *root, *apseq_browsing;
uuid_t root_uuid, l2cap, avctp, avrtg;
sdp_profile_desc_t profile[1];
- sdp_list_t *aproto, *proto[2];
+ sdp_list_t *aproto_control, *proto_control[2];
sdp_record_t *record;
- sdp_data_t *psm, *version, *features, *psm_browsing;
+ sdp_data_t *psm_control, *version, *features, *psm_browsing;
sdp_list_t *aproto_browsing, *proto_browsing[2] = {0};
uint16_t lp = AVCTP_CONTROL_PSM;
uint16_t lp_browsing = AVCTP_BROWSING_PSM;
@@ -283,19 +283,19 @@ static sdp_record_t *avrcp_tg_record(void)
/* Protocol Descriptor List */
sdp_uuid16_create(&l2cap, L2CAP_UUID);
- proto[0] = sdp_list_append(0, &l2cap);
- psm = sdp_data_alloc(SDP_UINT16, &lp);
- proto[0] = sdp_list_append(proto[0], psm);
- apseq = sdp_list_append(0, proto[0]);
+ proto_control[0] = sdp_list_append(0, &l2cap);
+ psm_control = sdp_data_alloc(SDP_UINT16, &lp);
+ proto_control[0] = sdp_list_append(proto_control[0], psm_control);
+ apseq = sdp_list_append(0, proto_control[0]);
sdp_uuid16_create(&avctp, AVCTP_UUID);
- proto[1] = sdp_list_append(0, &avctp);
+ proto_control[1] = sdp_list_append(0, &avctp);
version = sdp_data_alloc(SDP_UINT16, &avctp_ver);
- proto[1] = sdp_list_append(proto[1], version);
- apseq = sdp_list_append(apseq, proto[1]);
+ proto_control[1] = sdp_list_append(proto_control[1], version);
+ apseq = sdp_list_append(apseq, proto_control[1]);
- aproto = sdp_list_append(0, apseq);
- sdp_set_access_protos(record, aproto);
+ aproto_control = sdp_list_append(0, apseq);
+ sdp_set_access_protos(record, aproto_control);
proto_browsing[0] = sdp_list_append(0, &l2cap);
psm_browsing = sdp_data_alloc(SDP_UINT16, &lp_browsing);
proto_browsing[0] = sdp_list_append(proto_browsing[0], psm_browsing);
@@ -323,12 +323,12 @@ static sdp_record_t *avrcp_tg_record(void)
sdp_list_free(proto_browsing[0], 0);
sdp_list_free(proto_browsing[1], 0);
sdp_list_free(aproto_browsing, 0);
- free(psm);
+ free(psm_control);
free(version);
- sdp_list_free(proto[0], 0);
- sdp_list_free(proto[1], 0);
+ sdp_list_free(proto_control[0], 0);
+ sdp_list_free(proto_control[1], 0);
sdp_list_free(apseq, 0);
- sdp_list_free(aproto, 0);
+ sdp_list_free(aproto_control, 0);
sdp_list_free(pfseq, 0);
sdp_list_free(root, 0);
sdp_list_free(svclass_id, 0);
@@ -1037,13 +1037,13 @@ err:
return AVC_CTYPE_REJECTED;
}
-static struct pdu_handler {
+static struct control_pdu_handler {
uint8_t pdu_id;
uint8_t code;
uint8_t (*func) (struct avrcp_player *player,
struct avrcp_header *pdu,
uint8_t transaction);
-} handlers[] = {
+} control_handlers[] = {
{ AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
avrcp_handle_get_capabilities },
{ AVRCP_LIST_PLAYER_ATTRIBUTES, AVC_CTYPE_STATUS,
@@ -1082,7 +1082,7 @@ static size_t handle_vendordep_pdu(struct avctp *session, uint8_t transaction,
void *user_data)
{
struct avrcp_player *player = user_data;
- struct pdu_handler *handler;
+ struct control_pdu_handler *handler;
struct avrcp_header *pdu = (void *) operands;
uint32_t company_id = get_company_id(pdu->company_id);
@@ -1102,7 +1102,7 @@ static size_t handle_vendordep_pdu(struct avctp *session, uint8_t transaction,
goto err_metadata;
}
- for (handler = handlers; handler; handler++) {
+ for (handler = control_handlers; handler; handler++) {
if (handler->pdu_id == pdu->pdu_id)
break;
}
@@ -1229,9 +1229,9 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
player->dev = NULL;
player->registered_events = 0;
- if (player->handler) {
- avctp_unregister_pdu_handler(player->handler);
- player->handler = 0;
+ if (player->control_handler) {
+ avctp_unregister_pdu_handler(player->control_handler);
+ player->control_handler = 0;
}
break;
@@ -1239,8 +1239,8 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
player->session = avctp_connect(&dev->src, &dev->dst);
player->dev = dev;
- if (!player->handler)
- player->handler = avctp_register_pdu_handler(
+ if (!player->control_handler)
+ player->control_handler = avctp_register_pdu_handler(
AVC_OP_VENDORDEP,
handle_vendordep_pdu,
player);
@@ -1358,8 +1358,8 @@ static void player_destroy(gpointer data)
player_abort_pending_pdu(player);
- if (player->handler)
- avctp_unregister_pdu_handler(player->handler);
+ if (player->control_handler)
+ avctp_unregister_pdu_handler(player->control_handler);
g_free(player);
}
--
1.7.5.4
^ permalink raw reply related
* [PATCH BlueZ V2 1/5] AVRCP: Add TG Record to support AVRCP Browsing
From: Vani-dineshbhai PATEL @ 2012-08-08 6:56 UTC (permalink / raw)
To: User Name, Luiz Augusto, Lucas De Marchi; +Cc: Vani, Joohi, Vani
From: Vani Patel <vani.patel@stericsson.com>
Adds SDP record to support browsing
---
audio/avctp.c | 4 ++--
audio/avctp.h | 3 ++-
audio/avrcp.c | 25 +++++++++++++++++++++----
3 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index 074eabd..a20dba9 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -802,7 +802,7 @@ static GIOChannel *avctp_server_socket(const bdaddr_t *src, gboolean master)
io = bt_io_listen(BT_IO_L2CAP, NULL, avctp_confirm_cb, NULL,
NULL, &err,
BT_IO_OPT_SOURCE_BDADDR, src,
- BT_IO_OPT_PSM, AVCTP_PSM,
+ BT_IO_OPT_PSM, AVCTP_CONTROL_PSM,
BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
BT_IO_OPT_MASTER, master,
BT_IO_OPT_INVALID);
@@ -1090,7 +1090,7 @@ struct avctp *avctp_connect(const bdaddr_t *src, const bdaddr_t *dst)
io = bt_io_connect(BT_IO_L2CAP, avctp_connect_cb, session, NULL, &err,
BT_IO_OPT_SOURCE_BDADDR, &session->server->src,
BT_IO_OPT_DEST_BDADDR, &session->dst,
- BT_IO_OPT_PSM, AVCTP_PSM,
+ BT_IO_OPT_PSM, AVCTP_CONTROL_PSM,
BT_IO_OPT_INVALID);
if (err) {
avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
diff --git a/audio/avctp.h b/audio/avctp.h
index d0cbd97..34b0c1c 100644
--- a/audio/avctp.h
+++ b/audio/avctp.h
@@ -22,7 +22,8 @@
*
*/
-#define AVCTP_PSM 23
+#define AVCTP_CONTROL_PSM 23
+#define AVCTP_BROWSING_PSM 27
#define AVC_MTU 512
#define AVC_HEADER_LENGTH 3
diff --git a/audio/avrcp.c b/audio/avrcp.c
index d925365..ca40c1e 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -190,7 +190,7 @@ static sdp_record_t *avrcp_ct_record(void)
sdp_list_t *aproto, *proto[2];
sdp_record_t *record;
sdp_data_t *psm, *version, *features;
- uint16_t lp = AVCTP_PSM;
+ uint16_t lp = AVCTP_CONTROL_PSM;
uint16_t avrcp_ver = 0x0100, avctp_ver = 0x0103;
uint16_t feat = ( AVRCP_FEATURE_CATEGORY_1 |
AVRCP_FEATURE_CATEGORY_2 |
@@ -252,13 +252,15 @@ static sdp_record_t *avrcp_ct_record(void)
static sdp_record_t *avrcp_tg_record(void)
{
- sdp_list_t *svclass_id, *pfseq, *apseq, *root;
+ sdp_list_t *svclass_id, *pfseq, *apseq, *root, *apseq_browsing;
uuid_t root_uuid, l2cap, avctp, avrtg;
sdp_profile_desc_t profile[1];
sdp_list_t *aproto, *proto[2];
sdp_record_t *record;
- sdp_data_t *psm, *version, *features;
- uint16_t lp = AVCTP_PSM;
+ sdp_data_t *psm, *version, *features, *psm_browsing;
+ sdp_list_t *aproto_browsing, *proto_browsing[2] = {0};
+ uint16_t lp = AVCTP_CONTROL_PSM;
+ uint16_t lp_browsing = AVCTP_BROWSING_PSM;
uint16_t avrcp_ver = 0x0104, avctp_ver = 0x0103;
uint16_t feat = ( AVRCP_FEATURE_CATEGORY_1 |
AVRCP_FEATURE_CATEGORY_2 |
@@ -294,6 +296,17 @@ static sdp_record_t *avrcp_tg_record(void)
aproto = sdp_list_append(0, apseq);
sdp_set_access_protos(record, aproto);
+ proto_browsing[0] = sdp_list_append(0, &l2cap);
+ psm_browsing = sdp_data_alloc(SDP_UINT16, &lp_browsing);
+ proto_browsing[0] = sdp_list_append(proto_browsing[0], psm_browsing);
+ apseq_browsing = sdp_list_append(0, proto_browsing[0]);
+
+ proto_browsing[1] = sdp_list_append(0, &avctp);
+ proto_browsing[1] = sdp_list_append(proto_browsing[1], version);
+ apseq_browsing = sdp_list_append(apseq_browsing, proto_browsing[1]);
+
+ aproto_browsing = sdp_list_append(0, apseq_browsing);
+ sdp_set_add_access_protos(record, aproto_browsing);
/* Bluetooth Profile Descriptor List */
sdp_uuid16_create(&profile[0].uuid, AV_REMOTE_PROFILE_ID);
@@ -306,6 +319,10 @@ static sdp_record_t *avrcp_tg_record(void)
sdp_set_info_attr(record, "AVRCP TG", 0, 0);
+ free(psm_browsing);
+ sdp_list_free(proto_browsing[0], 0);
+ sdp_list_free(proto_browsing[1], 0);
+ sdp_list_free(aproto_browsing, 0);
free(psm);
free(version);
sdp_list_free(proto[0], 0);
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH v2 5/9] Battery: Add Battery list to btd_device
From: Chen Ganir @ 2012-08-08 6:14 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <CAAngNManvC8Q1Rt9qtni7txOPx24++wQqVPO+c3qmZu1oc7EMQ@mail.gmail.com>
Joao,
On 08/07/2012 07:29 PM, Joao Paulo Rechi Vita wrote:
> On Wed, Jul 25, 2012 at 2:42 AM, Chen Ganir <chen.ganir@ti.com> wrote:
>> Add peer battery list to the btd_device. New property added to Device
>> called Batteries.
>> ---
>> doc/device-api.txt | 5 +++
>> profiles/batterystate/batterystate.c | 6 ++++
>> src/device.c | 66 ++++++++++++++++++++++++++++++++++
>> src/device.h | 3 ++
>> 4 files changed, 80 insertions(+)
>>
>> diff --git a/doc/device-api.txt b/doc/device-api.txt
>> index 3b84033..3d19a53 100644
>> --- a/doc/device-api.txt
>> +++ b/doc/device-api.txt
>> @@ -175,3 +175,8 @@ Properties string Address [readonly]
>> Note that this property can exhibit false-positives
>> in the case of Bluetooth 2.1 (or newer) devices that
>> have disabled Extended Inquiry Response support.
>> +
>> + array{string} Batteries [readonly]
>> +
>> + List of device battery object paths that represents the available
>> + batteries on the remote device.
>
> This property should be deprecated by the introduction of the D-Bus
> object manager (scheduled for the 5.0 release).
>
Can you please explain ? What do you mean deprecated ? What is the
proposed mechanism to replace this property ?
Thanks,
Chen Ganir.
^ permalink raw reply
* Re: [PATCH v2 9/9] Battery: Emit property changed on first read
From: Chen Ganir @ 2012-08-08 6:04 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <CAAngNMaAM4zYqhGs+sD18DqwA8Hx8ysiy=6ksCVXo9ygb5V+bQ@mail.gmail.com>
Joao,
On 08/07/2012 07:29 PM, Joao Paulo Rechi Vita wrote:
> On Wed, Jul 25, 2012 at 2:42 AM, Chen Ganir <chen.ganir@ti.com> wrote:
>> Emit battery level property changed upon connection, on first read.
>> ---
>> profiles/batterystate/batterystate.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/profiles/batterystate/batterystate.c b/profiles/batterystate/batterystate.c
>> index 718863b..4300f85 100644
>> --- a/profiles/batterystate/batterystate.c
>> +++ b/profiles/batterystate/batterystate.c
>> @@ -72,6 +72,8 @@ struct descriptor {
>> bt_uuid_t uuid; /* UUID */
>> };
>>
>> +static void emit_battery_level_changed(struct characteristic *c);
>> +
>
> No need of this forward declaration, simply move the function
> implementation before the first function that references it
> (read_batterylevel_cb).
>
I will move the function as suggested.
>> static void char_free(gpointer user_data)
>> {
>> struct characteristic *c = user_data;
>> @@ -161,6 +163,7 @@ static void read_batterylevel_cb(guint8 status, const guint8 *pdu, guint16 len,
>> }
>>
>> ch->level = value[0];
>> + emit_battery_level_changed(ch);
>> }
>>
>> static void process_batteryservice_char(struct characteristic *ch)
>> --
>> 1.7.9.5
>>
>> --
>> 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
>
>
>
Thanks,
Chen Ganir.
^ permalink raw reply
* Re: [PATCH v2 8/9] Battery: Add support for notifications
From: Chen Ganir @ 2012-08-08 6:03 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <CAAngNMaAeeUS4Os7xWvN4AAmpdYns2o4v3+9Fxjy3ZRrC72TGA@mail.gmail.com>
Joao,
On 08/07/2012 07:29 PM, Joao Paulo Rechi Vita wrote:
> On Wed, Jul 25, 2012 at 2:42 AM, Chen Ganir <chen.ganir@ti.com> wrote:
>> Add support for emitting PropertyChanged when a battery level
>> characteristic notification is sent from the peer device.
>> ---
>> doc/battery-api.txt | 5 ++
>> profiles/batterystate/batterystate.c | 107 +++++++++++++++++++++++++++++++++-
>> 2 files changed, 111 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/battery-api.txt b/doc/battery-api.txt
>> index 5d7510d..f31e7e5 100644
>> --- a/doc/battery-api.txt
>> +++ b/doc/battery-api.txt
>> @@ -16,6 +16,11 @@ Methods dict GetProperties()
>> Returns all properties for the interface. See the
>> Properties section for the available properties.
>>
>> +Signals PropertyChanged(string name, variant value)
>> +
>> + This signal indicates a changed value of the given
>> + property.
>> +
>> Properties byte Namespace [readonly]
>>
>> Namespace value from the battery format characteristic
>> diff --git a/profiles/batterystate/batterystate.c b/profiles/batterystate/batterystate.c
>> index e5b68a9..718863b 100644
>> --- a/profiles/batterystate/batterystate.c
>> +++ b/profiles/batterystate/batterystate.c
>> @@ -50,6 +50,7 @@ struct battery {
>> GAttrib *attrib; /* GATT connection */
>> guint attioid; /* Att watcher id */
>> struct att_range *svc_range; /* Battery range */
>> + guint attnotid; /* Att notifications id */
>> GSList *chars; /* Characteristics */
>> };
>>
>> @@ -104,6 +105,14 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
>> return -1;
>> }
>>
>> +static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
>> +{
>> + const struct characteristic *ch = a;
>> + const uint16_t *handle = b;
>> +
>> + return ch->attr.value_handle - *handle;
>> +}
>> +
>> static void batterystate_free(gpointer user_data)
>> {
>> struct battery *batt = user_data;
>> @@ -117,6 +126,10 @@ static void batterystate_free(gpointer user_data)
>> if (batt->attrib != NULL)
>> g_attrib_unref(batt->attrib);
>>
>> + if (batt->attrib != NULL) {
>> + g_attrib_unregister(batt->attrib, batt->attnotid);
>> + g_attrib_unref(batt->attrib);
>> + }
>>
>> dbus_connection_unref(batt->conn);
>> btd_device_unref(batt->dev);
>> @@ -158,6 +171,17 @@ static void process_batteryservice_char(struct characteristic *ch)
>> }
>> }
>>
>> +static void batterylevel_enable_notify_cb(guint8 status, const guint8 *pdu,
>> + guint16 len, gpointer user_data)
>> +{
>> + char *msg = user_data;
>> +
>> + if (status != 0)
>> + error("Could not enable battery level notification: %s", msg);
>> +
>> + g_free(msg);
>> +}
>> +
>> static void batterylevel_presentation_format_desc_cb(guint8 status,
>> const guint8 *pdu, guint16 len,
>> gpointer user_data)
>> @@ -194,6 +218,23 @@ static void process_batterylevel_desc(struct descriptor *desc)
>> char uuidstr[MAX_LEN_UUID_STR];
>> bt_uuid_t btuuid;
>>
>> + bt_uuid16_create(&btuuid, GATT_CLIENT_CHARAC_CFG_UUID);
>> +
>> + if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0 && g_strcmp0(ch->attr.uuid,
>> + BATTERY_LEVEL_UUID) == 0) {
>> + uint8_t atval[2];
>> + uint16_t val;
>> + char *msg;
>> +
>> + val = GATT_CLIENT_CHARAC_CFG_NOTIF_BIT;
>> + msg = g_strdup("Enable BatteryLevel notification");
>> +
>
> What's the point of passing "Enable BatteryLevel notification" as
> user_data for the batterylevel_enable_notify_cb() ?
>
No point. just a leftover i need to remove.
>> + att_put_u16(val, atval);
>> + gatt_write_char(ch->batt->attrib, desc->handle, atval, 2,
>> + batterylevel_enable_notify_cb, msg);
>> + return;
>> + }
>> +
>> bt_uuid16_create(&btuuid, GATT_CHARAC_FMT_UUID);
>>
>> if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0) {
>> @@ -277,6 +318,12 @@ static DBusMessage *get_properties(DBusConnection *conn, DBusMessage *msg,
>> return reply;
>> }
>>
>> +static void emit_battery_level_changed(struct characteristic *c)
>> +{
>> + emit_property_changed(c->batt->conn, c->path, BATTERY_INTERFACE,
>> + "Level", DBUS_TYPE_BYTE, &c->level);
>> +}
>> +
>> static GDBusMethodTable battery_methods[] = {
>> { GDBUS_METHOD("GetProperties",
>> NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
>> @@ -284,6 +331,11 @@ static GDBusMethodTable battery_methods[] = {
>> { }
>> };
>>
>> +static GDBusSignalTable battery_signals[] = {
>> + { GDBUS_SIGNAL("PropertyChanged",
>> + GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
>> + { }
>> +};
>>
>> static void configure_batterystate_cb(GSList *characteristics, guint8 status,
>> gpointer user_data)
>> @@ -318,7 +370,7 @@ static void configure_batterystate_cb(GSList *characteristics, guint8 status,
>>
>> if (!g_dbus_register_interface(batt->conn, ch->path,
>> BATTERY_INTERFACE,
>> - battery_methods, NULL, NULL,
>> + battery_methods, battery_signals, NULL,
>> ch, NULL)) {
>> error("D-Bus register interface %s failed",
>> BATTERY_INTERFACE);
>> @@ -346,12 +398,63 @@ static void configure_batterystate_cb(GSList *characteristics, guint8 status,
>> }
>> }
>>
>> +static void proc_batterylevel(struct characteristic *c, const uint8_t *pdu,
>> + uint16_t len, gboolean final)
>> +{
>> + uint8_t new_batt_level = 0;
>> + gboolean changed = FALSE;
>> +
>> + if (!pdu) {
>> + error("Battery level notification: Invalid pdu length");
>> + goto done;
>> + }
>> +
>
> If pdu can be NULL here, than bluetoothd would have broken when
> derreferencing it at "handle = att_get_u16(&pdu[1]);" in
> notif_handler(). If pdu can be NULL the check needs to be done there.
>
I will modify that.
>> + new_batt_level = pdu[1];
>> +
>> + if (new_batt_level != c->level)
>> + changed = TRUE;
>> +
>> + c->level = new_batt_level;
>> +
>> +done:
>
> There is no need for the done label, since 'changed' is always false
> when you 'goto' here. You could simply return instead of using goto,
> although the whole if statement is likely to be dropped from this
> function.
>
This is assuming notifications arrive only when battery level has
changed. I agree, and i will change that.
>> + if (changed)
>> + emit_battery_level_changed(c);
>> +}
>> +
>> +static void notif_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
>> +{
>> + struct battery *batt = user_data;
>> + struct characteristic *ch;
>> + uint16_t handle;
>> + GSList *l;
>> +
>
> ch->attr.uuid == BATTERY_LEVEL_UUID should be tested before anything
> else, since other profiles may have enabled notifications for other
> characteristics (hog does that, for example). When testing with hog
> devices I got "notif_handler: Unexpected handle 0x0017" for every
> input report received.
>
ch->attr.uuid is only valid later in the code, after i get the handle
out of the PDU (need to make sure the PDU is in the correct length) ,
find the characteristic in the list (make sure it is not NULL), and set
ch to l->data. I believe removing the error code will remove the
annoyance, but i see no better way to extract the characteristic without
making sure everything is valid.
>> + if (len < 3) {
>> + error("notif_handler: Bad pdu received");
>> + return;
>> + }
>> +
>> + handle = att_get_u16(&pdu[1]);
>> + l = g_slist_find_custom(batt->chars, &handle, cmp_char_val_handle);
>> + if (l == NULL) {
>> + error("notif_handler: Unexpected handle 0x%04x", handle);
>> + return;
>> + }
>> +
>> + ch = l->data;
>> + if (g_strcmp0(ch->attr.uuid, BATTERY_LEVEL_UUID) == 0) {
>> + proc_batterylevel(ch, pdu, len, FALSE);
>> + }
>> +}
>> +
>> static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
>> {
>> struct battery *batt = user_data;
>>
>> batt->attrib = g_attrib_ref(attrib);
>>
>> + batt->attnotid = g_attrib_register(batt->attrib, ATT_OP_HANDLE_NOTIFY,
>> + notif_handler, batt, NULL);
>> +
>> if (batt->chars == NULL) {
>> gatt_discover_char(batt->attrib, batt->svc_range->start,
>> batt->svc_range->end, NULL,
>> @@ -369,6 +472,8 @@ static void attio_disconnected_cb(gpointer user_data)
>> {
>> struct battery *batt = user_data;
>>
>> + g_attrib_unregister(batt->attrib, batt->attnotid);
>> + batt->attnotid = 0;
>> g_attrib_unref(batt->attrib);
>> batt->attrib = NULL;
>> }
>> --
>> 1.7.9.5
>>
>> --
>> 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
>
>
>
Thanks,
Chen Ganir.
^ permalink raw reply
* Re: [PATCH v2 5/9] Battery: Add Battery list to btd_device
From: Chen Ganir @ 2012-08-08 5:56 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <CAAngNManvC8Q1Rt9qtni7txOPx24++wQqVPO+c3qmZu1oc7EMQ@mail.gmail.com>
Joao,
On 08/07/2012 07:29 PM, Joao Paulo Rechi Vita wrote:
> On Wed, Jul 25, 2012 at 2:42 AM, Chen Ganir <chen.ganir@ti.com> wrote:
>> Add peer battery list to the btd_device. New property added to Device
>> called Batteries.
>> ---
>> doc/device-api.txt | 5 +++
>> profiles/batterystate/batterystate.c | 6 ++++
>> src/device.c | 66 ++++++++++++++++++++++++++++++++++
>> src/device.h | 3 ++
>> 4 files changed, 80 insertions(+)
>>
>> diff --git a/doc/device-api.txt b/doc/device-api.txt
>> index 3b84033..3d19a53 100644
>> --- a/doc/device-api.txt
>> +++ b/doc/device-api.txt
>> @@ -175,3 +175,8 @@ Properties string Address [readonly]
>> Note that this property can exhibit false-positives
>> in the case of Bluetooth 2.1 (or newer) devices that
>> have disabled Extended Inquiry Response support.
>> +
>> + array{string} Batteries [readonly]
>> +
>> + List of device battery object paths that represents the available
>> + batteries on the remote device.
>
> This property should be deprecated by the introduction of the D-Bus
> object manager (scheduled for the 5.0 release).
>
>> diff --git a/profiles/batterystate/batterystate.c b/profiles/batterystate/batterystate.c
>> index d3a1974..23dfab5 100644
>> --- a/profiles/batterystate/batterystate.c
>> +++ b/profiles/batterystate/batterystate.c
>> @@ -50,6 +50,7 @@ struct battery {
>> static GSList *servers;
>>
>> struct characteristic {
>> + char *path; /* object path */
>> struct gatt_char attr; /* Characteristic */
>> struct battery *batt; /* Parent Battery Service */
>> GSList *desc; /* Descriptors */
>> @@ -206,6 +207,11 @@ static void configure_batterystate_cb(GSList *characteristics, guint8 status,
>> ch->attr.value_handle = c->value_handle;
>> memcpy(ch->attr.uuid, c->uuid, MAX_LEN_UUID_STR + 1);
>> ch->batt = batt;
>> + ch->path = g_strdup_printf("%s/BATT%04X",
>> + device_get_path(batt->dev),
>> + c->handle);
>> +
>
> What's the reationale behind using the characteristic handle as an
> identifier of the battery on the object path? Can't we do anything
> better than that? I understand this avoids clashing on a device with
> multiple battery services, but I don't really like the idea of
> exposing this internal details as an identifier on the API. Maybe
> using the battery namespace + description? It is mandatory when the
> device implements more than one instance of the battery service. If
> there is only one instance we don't need an identifier at all, only
> BATT would be sufficient.
Well, I did try to use the battery namespace and description, but i had
to register the battery at a later stage, since discovering the
characteristics format descriptors and reading them was done at a later
stage. I also wanted to keep the naming convention, and avoid giving
different names according to availability of other batteries or format
descriptors. naming the batteries with the handle makes the most sense,
since it is same as naming services and characteristics obj paths. I
dont like the idea of having a single battery called BATT, multiple
batteries called BATTXXXXXX for battery with namespace and BATTYYYY for
batteries without it. This will be very confusing. Battery naming
convention should remain the same regardless of those things.
>
>> + device_add_battery(batt->dev, ch->path);
>>
>> batt->chars = g_slist_append(batt->chars, ch);
>>
>> diff --git a/src/device.c b/src/device.c
>> index cd571f7..98e431a 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -127,6 +127,10 @@ struct att_callbacks {
>> gpointer user_data;
>> };
>>
>> +struct btd_battery {
>> + char *path;
>> +};
>> +
>> struct btd_device {
>> bdaddr_t bdaddr;
>> uint8_t bdaddr_type;
>> @@ -172,6 +176,7 @@ struct btd_device {
>>
>> GIOChannel *att_io;
>> guint cleanup_id;
>> + GSList *batteries;
>> };
>>
>> static uint16_t uuid_list[] = {
>> @@ -262,6 +267,7 @@ static void device_free(gpointer user_data)
>> g_slist_free_full(device->primaries, g_free);
>> g_slist_free_full(device->attios, g_free);
>> g_slist_free_full(device->attios_offline, g_free);
>> + g_slist_free_full(device->batteries, g_free);
>>
>> attio_cleanup(device);
>>
>> @@ -433,6 +439,15 @@ static DBusMessage *get_properties(DBusConnection *conn,
>> ptr = adapter_get_path(adapter);
>> dict_append_entry(&dict, "Adapter", DBUS_TYPE_OBJECT_PATH, &ptr);
>>
>> + /* Batteries */
>> + str = g_new0(char *, g_slist_length(device->batteries) + 1);
>> + for (i = 0, l = device->batteries; l; l = l->next, i++) {
>> + struct btd_battery *b = l->data;
>> + str[i] = b->path;
>> + }
>> + dict_append_array(&dict, "Batteries", DBUS_TYPE_OBJECT_PATH, &str, i);
>> + g_free(str);
>> +
>> dbus_message_iter_close_container(&iter, &dict);
>>
>> return reply;
>> @@ -1219,6 +1234,9 @@ void device_remove(struct btd_device *device, gboolean remove_stored)
>> g_slist_free(device->drivers);
>> device->drivers = NULL;
>>
>> + g_slist_free(device->batteries);
>> + device->batteries = NULL;
>> +
>> attrib_client_unregister(device->services);
>>
>> btd_device_unref(device);
>> @@ -3141,3 +3159,51 @@ void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
>> device_set_product(device, product_id);
>> device_set_version(device, product_ver);
>> }
>> +
>> +static void batteries_changed(struct btd_device *device)
>> +{
>> + DBusConnection *conn = get_dbus_connection();
>> + char **batteries;
>> + GSList *l;
>> + int i;
>> +
>> + batteries = g_new0(char *, g_slist_length(device->batteries) + 1);
>> + for (i = 0, l = device->batteries; l; l = l->next, i++) {
>> + struct btd_battery *batt = l->data;
>> + batteries[i] = batt->path;
>> + }
>> +
>> + emit_array_property_changed(conn, device->path, DEVICE_INTERFACE,
>> + "Batteries", DBUS_TYPE_STRING, &batteries,
>> + i);
>> +
>> + g_free(batteries);
>> +}
>> +
>> +void device_add_battery(struct btd_device *device, char *path)
>> +{
>> + struct btd_battery *batt;
>> +
>> + batt = g_new0(struct btd_battery, 1);
>> + batt->path = g_strdup(path);
>> + device->batteries = g_slist_append(device->batteries, batt);
>> + batteries_changed(device);
>> +}
>> +
>> +void device_remove_battery(struct btd_device *device, char *path)
>> +{
>> + GSList *l;
>> +
>> + for (l = device->batteries; l; l = l->next) {
>> + struct btd_battery *b = l->data;
>> +
>> + if (g_strcmp0(path, b->path) == 0) {
>> + device->batteries = g_slist_remove(device->batteries, b);
>> + g_free(b->path);
>> + g_free(b);
>> + return;
>
> If you return here the property changed signal will not be emitted.
>
True. I will fix this.
>> + }
>> + }
>> +
>> + batteries_changed(device);
>> +}
>> diff --git a/src/device.h b/src/device.h
>> index 26e17f7..db71a8a 100644
>> --- a/src/device.h
>> +++ b/src/device.h
>> @@ -126,3 +126,6 @@ int device_unblock(DBusConnection *conn, struct btd_device *device,
>> void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
>> uint16_t vendor_id, uint16_t product_id,
>> uint16_t product_ver);
>> +
>> +void device_add_battery(struct btd_device *device, char *path);
>> +void device_remove_battery(struct btd_device *device, char *path);
>> --
>> 1.7.9.5
>>
>> --
>> 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
>
>
>
Thanks,
Chen Ganir.
^ permalink raw reply
* Re: [PATCH v2 4/9] Battery: Get Battery ID
From: Chen Ganir @ 2012-08-08 5:51 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <CAAngNMYTK27mHJ8JhT2=0zE4vJ_DFmY=f=XBXqJMFKKoXEC0uQ@mail.gmail.com>
Joao,
On 08/07/2012 07:29 PM, Joao Paulo Rechi Vita wrote:
> On Wed, Jul 25, 2012 at 2:42 AM, Chen Ganir <chen.ganir@ti.com> wrote:
>> Read the battery level format characteristic descriptor to get the
>> unique namespace and description values.
>> ---
>> profiles/batterystate/batterystate.c | 113 ++++++++++++++++++++++++++--------
>> 1 file changed, 86 insertions(+), 27 deletions(-)
>>
>> diff --git a/profiles/batterystate/batterystate.c b/profiles/batterystate/batterystate.c
>> index a7d2f6e..d3a1974 100644
>> --- a/profiles/batterystate/batterystate.c
>> +++ b/profiles/batterystate/batterystate.c
>> @@ -37,6 +37,8 @@
>> #include "batterystate.h"
>> #include "log.h"
>>
>> +#define BATTERY_LEVEL_UUID "00002a19-0000-1000-8000-00805f9b34fb"
>> +
>> struct battery {
>> struct btd_device *dev; /* Device reference */
>> GAttrib *attrib; /* GATT connection */
>> @@ -48,15 +50,18 @@ struct battery {
>> static GSList *servers;
>>
>> struct characteristic {
>> - struct gatt_char attr; /* Characteristic */
>> - struct battery *batt; /* Parent Battery Service */
>> + struct gatt_char attr; /* Characteristic */
>> + struct battery *batt; /* Parent Battery Service */
>> GSList *desc; /* Descriptors */
>> + uint8_t ns; /* Battery Namespace */
>> + uint16_t description; /* Battery description */
>> + uint8_t level; /* Battery last known level */
>
> The 'level' field it's not being used here. It should be added by the
> same commit that uses it.
>
I'll check it's usage and move it to the relevant patch.
>> };
>>
>> struct descriptor {
>> - struct characteristic *ch; /* Parent Characteristic */
>> - uint16_t handle; /* Descriptor Handle */
>> - bt_uuid_t uuid; /* UUID */
>> + struct characteristic *ch; /* Parent Characteristic */
>> + uint16_t handle; /* Descriptor Handle */
>> + bt_uuid_t uuid; /* UUID */
>> };
>>
>> static gint cmp_device(gconstpointer a, gconstpointer b)
>> @@ -87,6 +92,55 @@ static void batterystate_free(gpointer user_data)
>> g_free(batt);
>> }
>>
>> +static void batterylevel_presentation_format_desc_cb(guint8 status,
>> + const guint8 *pdu, guint16 len,
>> + gpointer user_data)
>> +{
>> + struct descriptor *desc = user_data;
>> + uint8_t value[ATT_MAX_MTU];
>> + int vlen;
>> +
>> + if (status != 0) {
>> + error("Presentation Format desc read failed: %s",
>> + att_ecode2str(status));
>> + return;
>> + }
>> +
>> + vlen = dec_read_resp(pdu, len, value, sizeof(value));
>> + if (!vlen) {
>> + error("Presentation Format desc read failed: Protocol error\n");
>> + return;
>> + }
>> +
>> + if (vlen < 7) {
>> + error("Presentation Format desc read failed: Invalid range");
>> + return;
>> + }
>> +
>> + desc->ch->ns = value[4];
>> + desc->ch->description = att_get_u16(&value[5]);
>> +}
>> +
>> +
>> +static void process_batterylevel_desc(struct descriptor *desc)
>> +{
>> + struct characteristic *ch = desc->ch;
>> + char uuidstr[MAX_LEN_UUID_STR];
>> + bt_uuid_t btuuid;
>> +
>> + bt_uuid16_create(&btuuid, GATT_CHARAC_FMT_UUID);
>> +
>> + if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0) {
>> + gatt_read_char(ch->batt->attrib, desc->handle, 0,
>> + batterylevel_presentation_format_desc_cb, desc);
>> + return;
>> + }
>> +
>> + bt_uuid_to_string(&desc->uuid, uuidstr, MAX_LEN_UUID_STR);
>> + DBG("Ignored descriptor %s characteristic %s", uuidstr, ch->attr.uuid);
>> +}
>> +
>> +
>> static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>> gpointer user_data)
>> {
>> @@ -120,6 +174,7 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>> desc->uuid = att_get_uuid128(&value[2]);
>>
>> ch->desc = g_slist_append(ch->desc, desc);
>> + process_batterylevel_desc(desc);
>> }
>>
>> att_data_list_free(list);
>> @@ -140,31 +195,35 @@ static void configure_batterystate_cb(GSList *characteristics, guint8 status,
>>
>> for (l = characteristics; l; l = l->next) {
>> struct gatt_char *c = l->data;
>> - struct characteristic *ch;
>> - uint16_t start, end;
>> -
>> - ch = g_new0(struct characteristic, 1);
>> - ch->attr.handle = c->handle;
>> - ch->attr.properties = c->properties;
>> - ch->attr.value_handle = c->value_handle;
>> - memcpy(ch->attr.uuid, c->uuid, MAX_LEN_UUID_STR + 1);
>> - ch->batt = batt;
>>
>> - batt->chars = g_slist_append(batt->chars, ch);
>> -
>> - start = c->value_handle + 1;
>> -
>> - if (l->next != NULL) {
>> - struct gatt_char *c = l->next->data;
>> - if (start == c->handle)
>> + if (g_strcmp0(c->uuid, BATTERY_LEVEL_UUID) == 0) {
>> + struct characteristic *ch;
>> + uint16_t start, end;
>> +
>> + ch = g_new0(struct characteristic, 1);
>> + ch->attr.handle = c->handle;
>> + ch->attr.properties = c->properties;
>> + ch->attr.value_handle = c->value_handle;
>> + memcpy(ch->attr.uuid, c->uuid, MAX_LEN_UUID_STR + 1);
>> + ch->batt = batt;
>> +
>> + batt->chars = g_slist_append(batt->chars, ch);
>> +
>> + start = c->value_handle + 1;
>> +
>> + if (l->next != NULL) {
>> + struct gatt_char *c = l->next->data;
>> + if (start == c->handle)
>> + continue;
>> + end = c->handle - 1;
>> + } else if (c->value_handle != batt->svc_range->end)
>> + end = batt->svc_range->end;
>> + else
>> continue;
>> - end = c->handle - 1;
>> - } else if (c->value_handle != batt->svc_range->end)
>> - end = batt->svc_range->end;
>> - else
>> - continue;
>>
>> - gatt_find_info(batt->attrib, start, end, discover_desc_cb, ch);
>> + gatt_find_info(batt->attrib, start, end,
>> + discover_desc_cb, ch);
>> + }
>> }
>> }
>>
>> --
>> 1.7.9.5
>>
>> --
>> 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
>
>
>
--
*Chen Ganir***| Texas Instruments | *SW Engineer* Mobile Connectivity
Solutions| Direct: (972)-9-7906080 | Mobile: (972)-52-2438-406|Zarhin
26,Ra'anana, Israel |http://www.ti.com <http://www.ti.com/>
| chen.ganir@ti.com <mailto:chen.ganir@ti.com>
^ permalink raw reply
* Re: [PATCH v2 2/9] Battery: Add connection logic
From: Chen Ganir @ 2012-08-08 5:50 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <CAAngNMaaZXDpX-voj+KLf+fmsJ-dQY_UCQY9OE8bOSi74BztjQ@mail.gmail.com>
On 08/07/2012 07:29 PM, Joao Paulo Rechi Vita wrote:
> On Wed, Jul 25, 2012 at 2:42 AM, Chen Ganir <chen.ganir@ti.com> wrote:
>> Add connection logic to the Battery Plugin. When the driver is
>> loaded, it will request a connection to the remote device and
>> release the connection request when destroyed.
>> ---
>> profiles/batterystate/batterystate.c | 78 +++++++++++++++++++++++++++++++++-
>> profiles/batterystate/batterystate.h | 3 +-
>> profiles/batterystate/manager.c | 22 +++++++++-
>> 3 files changed, 99 insertions(+), 4 deletions(-)
>>
>> diff --git a/profiles/batterystate/batterystate.c b/profiles/batterystate/batterystate.c
>> index 04c2e5e..40663f6 100644
>> --- a/profiles/batterystate/batterystate.c
>> +++ b/profiles/batterystate/batterystate.c
>> @@ -29,17 +29,29 @@
>>
>> #include "adapter.h"
>> #include "device.h"
>> +#include "gattrib.h"
>> +#include "attio.h"
>> #include "att.h"
>> #include "gattrib.h"
>> #include "gatt.h"
>> #include "batterystate.h"
>> +#include "log.h"
>>
>> struct battery {
>> struct btd_device *dev; /* Device reference */
>> + GAttrib *attrib; /* GATT connection */
>> + guint attioid; /* Att watcher id */
>> + struct att_range *svc_range; /* Battery range */
>> + GSList *chars; /* Characteristics */
>> };
>>
>> static GSList *servers;
>>
>> +struct characteristic {
>> + struct gatt_char attr; /* Characteristic */
>> + struct battery *batt; /* Parent Battery Service */
>> +};
>> +
>> static gint cmp_device(gconstpointer a, gconstpointer b)
>> {
>> const struct battery *batt = a;
>> @@ -55,20 +67,84 @@ static void batterystate_free(gpointer user_data)
>> {
>> struct battery *batt = user_data;
>>
>> + if (batt->chars != NULL)
>> + g_slist_free_full(batt->chars, g_free);
>> +
>> + if (batt->attioid > 0)
>> + btd_device_remove_attio_callback(batt->dev, batt->attioid);
>> +
>> + if (batt->attrib != NULL)
>> + g_attrib_unref(batt->attrib);
>> +
>> btd_device_unref(batt->dev);
>> g_free(batt);
>> }
>>
>> +static void configure_batterystate_cb(GSList *characteristics, guint8 status,
>> + gpointer user_data)
>> +{
>> + struct battery *batt = user_data;
>> + GSList *l;
>> +
>> + if (status != 0) {
>> + error("Discover batterystate characteristics: %s",
>> + att_ecode2str(status));
>> + return;
>> + }
>>
>> -int batterystate_register(struct btd_device *device)
>> + for (l = characteristics; l; l = l->next) {
>> + struct gatt_char *c = l->data;
>> + struct characteristic *ch;
>> +
>> + ch = g_new0(struct characteristic, 1);
>> + ch->attr.handle = c->handle;
>> + ch->attr.properties = c->properties;
>> + ch->attr.value_handle = c->value_handle;
>> + memcpy(ch->attr.uuid, c->uuid, MAX_LEN_UUID_STR + 1);
>> + ch->batt = batt;
>> +
>> + batt->chars = g_slist_append(batt->chars, ch);
>> + }
>> +}
>> +
>> +static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
>> +{
>> + struct battery *batt = user_data;
>> +
>> + batt->attrib = g_attrib_ref(attrib);
>> +
>> + if (batt->chars == NULL) {
>> + gatt_discover_char(batt->attrib, batt->svc_range->start,
>> + batt->svc_range->end, NULL,
>> + configure_batterystate_cb, batt);
>> + }
>> +}
>> +
>> +static void attio_disconnected_cb(gpointer user_data)
>> +{
>> + struct battery *batt = user_data;
>> +
>> + g_attrib_unref(batt->attrib);
>> + batt->attrib = NULL;
>> +}
>> +
>> +int batterystate_register(struct btd_device *device,
>> + struct gatt_primary *prim)
>> {
>> struct battery *batt;
>>
>> batt = g_new0(struct battery, 1);
>> batt->dev = btd_device_ref(device);
>>
>> + batt->svc_range = g_new0(struct att_range, 1);
>> + batt->svc_range->start = prim->range.start;
>> + batt->svc_range->end = prim->range.end;
>> +
>> servers = g_slist_prepend(servers, batt);
>>
>> + batt->attioid = btd_device_add_attio_callback(device,
>> + attio_connected_cb, attio_disconnected_cb,
>> + batt);
>> return 0;
>> }
>>
>> diff --git a/profiles/batterystate/batterystate.h b/profiles/batterystate/batterystate.h
>> index 9aedae7..2d30028 100644
>> --- a/profiles/batterystate/batterystate.h
>> +++ b/profiles/batterystate/batterystate.h
>> @@ -19,6 +19,5 @@
>> * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
>> *
>> */
>> -
>> -int batterystate_register(struct btd_device *device);
>> +int batterystate_register(struct btd_device *device, struct gatt_primary *prim);
>> void batterystate_unregister(struct btd_device *device);
>> diff --git a/profiles/batterystate/manager.c b/profiles/batterystate/manager.c
>> index 6718acf..62076ac 100644
>> --- a/profiles/batterystate/manager.c
>> +++ b/profiles/batterystate/manager.c
>> @@ -34,9 +34,29 @@
>>
>> #define BATTERY_SERVICE_UUID "0000180f-0000-1000-8000-00805f9b34fb"
>>
>> +static gint primary_uuid_cmp(gconstpointer a, gconstpointer b)
>> +{
>> + const struct gatt_primary *prim = a;
>> + const char *uuid = b;
>> +
>> + return g_strcmp0(prim->uuid, uuid);
>> +}
>> +
>> static int batterystate_driver_probe(struct btd_device *device, GSList *uuids)
>> {
>> - return batterystate_register(device);
>> + struct gatt_primary *prim;
>> + GSList *primaries, *l;
>> +
>> + primaries = btd_device_get_primaries(device);
>> +
>> + l = g_slist_find_custom(primaries, BATTERY_SERVICE_UUID,
>> + primary_uuid_cmp);
>
> No need to check for the BATTERY_SERVICE_UUID. If driver has been
> probed its because the remote implements this service, since it's
> declared on the .uuids field of the plugin struct.
I will remove the check.
>
>> + if (l == NULL)
>> + return -EINVAL;
>> +
>> + prim = l->data;
>> +
>> + return batterystate_register(device, prim);
>
> Getting the primary service pointer (manly used for handle range
> information could be done from inside batterystate_register() itself
> on batterystate.c instead of doing so on the manager code. This way
> the plugin keeps more self-contained.
I'll move that into the manager code as you recommended. Most of this
code was taken from other plugins, just to make sure i conform with the
current convnetions.
>
>> }
>>
>> static void batterystate_driver_remove(struct btd_device *device)
>> --
>> 1.7.9.5
>>
>> --
>> 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
>
>
>
Thanks,
Chen Ganir.
^ permalink raw reply
* Re: [PATCH v2 0/9] Add GATT Client Battery Service
From: Chen Ganir @ 2012-08-08 5:45 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: Claudio Takahasi, linux-bluetooth
In-Reply-To: <CAAngNMbXUskPXd-0AS2i_zp4mK6uH84LUmDr7bmMK68pppfqzw@mail.gmail.com>
Joao,
On 08/07/2012 07:29 PM, Joao Paulo Rechi Vita wrote:
> Hello Chen,
>
> On Mon, Aug 6, 2012 at 2:52 AM, Chen Ganir <chen.ganir@ti.com> wrote:
>> Claudio,
>>
>>
>> On 08/03/2012 03:57 PM, Claudio Takahasi wrote:
>>>
>>> Hi Chen Ganir:
>>>
>>> On Wed, Aug 1, 2012 at 3:40 AM, Chen Ganir <chen.ganir@ti.com> wrote:
>>>>
>>>> On 07/25/2012 08:42 AM, Chen Ganir wrote:
>>>>>
>>>>>
>>>>> Add suupport for LE GATT Client Battery Service.
>>>>>
>>>>> This plugin adds battery list to the btd_device, exposes DBUS API to
>>>>> list
>>>>> the
>>>>> device batteries, and allows querying for battery information. In
>>>>> addition
>>>>> this
>>>>> patch allows getting notifications for battery level changes.
>>>>>
>>>>> Look at doc/device-api.txt and doc/battery-api.txt for more information.
>>>>>
>>>>> This is version 2 of this patch set, rebased on top of the latest
>>>>> sources
>>>>> and
>>>>> fixes some issues found during testing and in the ML comments.
>>>>>
>>>>> Chen Ganir (9):
>>>>> Battery: Add Battery Service GATT Client
>>>>> Battery: Add connection logic
>>>>> Battery: Discover Characteristic Descriptors
>>>>> Battery: Get Battery ID
>>>>> Battery: Add Battery list to btd_device
>>>>> Battery: Add Battery D-BUS API
>>>>> Battery: Read Battery level characteristic
>>>>> Battery: Add support for notifications
>>>>> Battery: Emit property changed on first read
>>>>>
>>>>> Makefile.am | 10 +-
>>>>> doc/battery-api.txt | 38 +++
>>>>> doc/device-api.txt | 5 +
>>>>> profiles/batterystate/batterystate.c | 518
>>>>> ++++++++++++++++++++++++++++++++++
>>>>> profiles/batterystate/batterystate.h | 24 ++
>>>>> profiles/batterystate/main.c | 67 +++++
>>>>> profiles/batterystate/manager.c | 93 ++++++
>>>>> profiles/batterystate/manager.h | 24 ++
>>>>> src/device.c | 66 +++++
>>>>> src/device.h | 3 +
>>>>> 10 files changed, 846 insertions(+), 2 deletions(-)
>>>>> create mode 100644 doc/battery-api.txt
>>>>> create mode 100644 profiles/batterystate/batterystate.c
>>>>> create mode 100644 profiles/batterystate/batterystate.h
>>>>> create mode 100644 profiles/batterystate/main.c
>>>>> create mode 100644 profiles/batterystate/manager.c
>>>>> create mode 100644 profiles/batterystate/manager.h
>>>>>
>>>>
>>>> Ping anyone ? Did anyone get to review this ?
>>>>
>>>> Thanks,
>>>> Chen Ganir
>>>
>>>
>>>
>>> not yet.
>>> We have an INTERNAL release in two weeks, next week we will send
>>> comments in the ML.
>>>
>> Thanks. I'll be waiting.
>>
>>
>
> I've been reviewing and did some quick tests on your code. It's
> working with some minor issues, and I'll comment them on each commit.
> But first I have a few more general questions:
>
> 1. Any specific reason for calling the plugin directory and files
> 'batterystate'? I don't see any reference to this name on the
> documentation.
No specific reason. I will rename it to Battery, to correspond with the
BAS spec.
>
> 2. The spec recommends reading the battery level value with very
> little frequency. Quoting the section 3.1.1:
>
> "For example, if the expected battery life is in the order of years,
> reading the battery level value more frequently than once a week is
> not recommended."
>
> And on the same section:
>
> "The value of the Client Characteristic Configuration descriptor is
> persistent for bonded devices when not in a connection."
>
> At the moment the plugin is reading it every time it is probed, which
> is a lot more than recommended. What do you think about only enabling
> notifications after paring, and not reading the value at all and just
> wait for the notifications. If the device doesn't support
> notifications (which I *think* should be uncommon) we can read the
> value after pairing and refresh it every week or so. For this to work
> well we'll need characteristics value storage support, but that will
> improve other things as well. While we don't support that, I don't
> have other ideas to improve this.
I agree with you that we will need some storage to allow following the
specs correctly. I could just read the battery level on pairing only,
and then rely on notifications (if supported) or read battery level on
each connect. However, what happens if the device remains connected for
a long period of time (more than a week) ? I will need to add some kind
of mechanism to check how much time has passed since last reading, and
invoke battery level readout for each battery level characteristic. Do
you have any suggestions for improvement here ?
>
>
>>> BR,
>>> Claudio
>>>
>>
>> Chen Ganir.
>>
>> --
>> 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
>
>
>
Thanks for reviewing this patch set. I will try to send revised and
fixed set today or tomorrow with the fixes mentioned in your other patch
review.
Chen Ganir.
^ permalink raw reply
* Bluez/Obexd upstream test result_20120808 (bluez-4.101.178+ obexd-0.47)
From: Li, XiaX @ 2012-08-08 5:32 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]
Hi all,
QA finished upstream testing. The test is for bluez-4.101.178 and obexd-0.47. Against last, BlueZ is changed from 4.101.135 to 4.101.178, Obexd is changed from 0.46 to 0.47. Some dbus interface has been changed.
In this test, totally ran 100 case: 77 Pass, 5 Fail. Other 18 are blocked or unavailable (some cases validation method is still in investigating).
The pass rate is from 95% to 94% (pass / <pass + fail>). Pulseaudio has not caught up with the BlueZ interface changing in HFP profile.
New bugs:
===============================================
https://bugs.meego.com/show_bug.cgi?id=25593
[regression] Pulseaudio crashes after connect BT sink to alsa source
https://bugs.meego.com/show_bug.cgi?id=25595
[regression] MAP client fail to get message list from Samsung GT-i9100
Re-open bugs:
===============================================
N/A
Verified bugs:
===============================================
N/A
Testing Environment
==============================================
Hardware: netbook Eeepc 1005HA | Acer AspireOne NAV50
Image: netbook-ia32-pinetrail-tizen_20120424.2
Linux Kernel: v3.4-rc7
bluez-4.101.178.g1baf870-17.1.i586
obexd-0.47.7.g73bb0a0-1.1.i586
obexd-server-0.47.7.g73bb0a0-1.1.i586
connman-1.4-1.1.i586
ofono-1.9.8.g34b4e96-1.1.i586
Pulseaudio: d3cd82d411f9b574678870d8ccd836032a238684 (July, 9th)
For detailed test results, please see attached file.
Thanks
Li Xia
[-- Attachment #2: Bluetooth_upstream_quality_20120808.txt --]
[-- Type: text/plain, Size: 7323 bytes --]
== Detail Result ==
[PASS]: case passed <br>
[FAIL]: case failed <br>
[Block]: case is blocked by one bug <br>
[N/A]: case not available to be tested
=== Audio-A2DP ===
[PASS] A2DP_001: audio sink connect/re-connect
[PASS] A2DP_002: stereo headset playback
[PASS] A2DP_003: stereo headset voice record
[PASS] A2DP_004: Codec support: SBC
[PASS] A2DP_005: Playback music on line
[PASS] A2DP_006: SINK role, as speaker for other device music playing
With some BT headset, we ever met issue [https://bugs.meego.com/show_bug.cgi?id=25480 Bug#25480 A2DP-source: pulseaudio fails to create bluez_sink and bluez_source]
=== Audio-AVRCP ===
[PASS] AVRCP_001: Volume Up/Down
[Block] AVRCP_002: Play/Pause (by unavailable media-player)
[Block] AVRCP_003: Next/Privious (by unavailable media-player)
[PASS] AVRCP_004: Connection Establish/Release
[N/A] AVRCP_005: Audio metadata get.
We still try to find avaliable media-player to support AVRCP-1.4
=== Audio-HFP ===
[PASS] HFP_001: RFCOMM connection on AG
[PASS] HFP_002: list (ofono) hfp modem
[PASS] HFP_003: HFP modem establishment (Ofono)
[FAIL] HFP_004: Audio SNK and SRC establishment
[FAIL] HFP_005: voicecall, audio creates BT SNK/SRC
[FAIL] HFP_006: redirect AG SNK/SRC to local SRC/SNK
BlueZ interface has some regression, bug is reported as [https://bugs.meego.com/show_bug.cgi?id=25473 Bug#25473 HFP: latest bluez cannot support pulseaudio to create bluez_sink and bluez_source]
Pulseaudio has new crash bug: [https://bugs.meego.com/show_bug.cgi?id=25593 Bug#25593 Pulseaudio crashes after connect BT sink to alsa source]
[PASS] HFP_007: Check AT-commands from HF part
[PASS] HFP_008: Connect BT Headset to HFP phone
=== Audio-HSP ===
[PASS] HSP_001: Use mono headset to play music
*** DUT is a Netbook, unable to take phone ***
[N/A] HSP_002: Take incoming call by button-press
[N/A] HSP_003: Audio transfer between AG and HS
[PASS] HSP_004: Adjust Volume Up/Down
=== OBEX-FTP ===
[PASS] FTP_001: pull/push files from/to server.
[PASS] FTP_002: browse server files
[PASS] FTP_003: Client "delete file", "rename file"
[PASS] FTP_004: Server enables FTP parameter
[PASS] FTP_005: Server sets sharing root path
[PASS] FTP_006: Server handles all requirements
[PASS] FTP_007: Server has ability to set permission for FTP
[PASS] FTP_008: FTP data-rate about 30k/s~230k/s
[PASS] FTP_009: Big size file transferred stable
=== OBEX-OPP ===
[PASS] OPP_001: pull un-patterned object from server.
[PASS] OPP_002: Server enables OPP parameter
[PASS] OPP_003: During object transferring, the progress is clear
=== OBEX-PBAP ===
[PASS] PBAP_001: Client gets phone book entries from server
[PASS] PBAP_002: Client gets ICH, OCH, MCH and CCH from server
[FAIL] PBAP_003: PSE can provide PBAP daemon by enabling corresponding parameter
Case failed due to [https://bugs.meego.com/show_bug.cgi?id=25189 Bug#25189, PBAP Server has segmentation fault during responding to client PullAll]
[PASS] PBAP_004: Both sides support vCard2.1/vCard3.0
=== OBEX-SYNC ===
[PASS] SYNC_001: Server enables SYNC daemon
[PASS] SYNC_002: During sync, Server can show "syncing..."
[PASS] SYNC_003: client can set PIM fetching from INT
[PASS] SYNC_004: Server get/put entire phonebook from/to client.
[PASS] SYNC_005: Client can support vCard2.1, vCard3.0
=== OBEX-MAP MCE ===
[FAIL] MAP_001: MCE can browse message/folder list on MSE
Regression happened on client side, bug is reported as [https://bugs.meego.com/show_bug.cgi?id=25595 Bug#25595 MAP client fail to get message list from Samsung GT-i9100]
[BLOCK] MAP_002: MCE can upload local message to MSE
[BLOCK] MAP_003: MCE can delete the message on MSE side
[BLOCK] MAP_004: MCE can take use of MSE to send message
=== Network-PAN ===
[PASS] PAN_001: PANU can init nap0 device connect to NAP
[PASS] PAN_002: PANU can get ip address or assigned static ip
[PASS] PAN_003: PANU can logon internet website
[N/A] PAN_004: NAP can init bridge bnep
[N/A] PAN_005: NAP can support one or multiple PANU connection
[N/A] PAN_006: NAP can have DHCP responding to each PANU
Current connman does not support DHCP functions.
=== Network-BNEP ===
[PASS] BNEP_001: Check BNEP support on DUT
=== Networl-DUN ===
[PASS] DUN_001: GW (DUT) can parse a series of AT commands from the data terminal.
[PASS] DUN_002: DT can build up rfcomm device by bluetooth and DUN modem by Ofono.
[PASS] DUN_003: DT can use Ofono to dial up special service number ("*99#").
[PASS] DUN_004: When network connected, DT can log on website in internet.
=== SIM-SAP ===
The SAP test method is still in investigation.
[N/A] SAP_001: Server can enable a module to register Client
[N/A] SAP_002: Server can power on/off SIM Card or reset it
[N/A] SAP_003: Server can disconnect Client
[N/A] SAP_004: Server can disconnect SIM
[N/A] SAP_005: Client can connect to Server SIM
[N/A] SAP_006: Client can power on Server SIM
[N/A] SAP_007: Client can disconnect Server
=== Generic-GAP ===
[PASS] GAP_001: PSCAN/ISCAN mode setting.
[PASS] GAP_002: Active pairing to another bluetooth device.
[PASS] GAP_003: Passive pairing, accepte pair master requirement
[PASS] GAP_004: SSP supports
[PASS] GAP_005: If lost link-key, it needs re-pair
[PASS] GAP_006: reboot DUT with link-key restored, no need to re-pair
[PASS] GAP_007: Pair can be released
=== Generic-SDP ===
[PASS] SDP_001: browse available service list in local
[PASS] SDP_002: browse available service list from remote
[PASS] SDP_003: browse service by RecHandle for detail info
[PASS] SDP_004: Server can answer SDP searching request
[PASS] SDP_005: Server can add/del/update service records
=== Others-HID ===
[PASS] HID_001: Host can search nearby HID device.
[PASS] HID_002: [BAT] Host can connect to HID device.
[PASS] HID_003: Host can handle multiple human input/output devices.
=== HCI ===
[PASS] HCI_001: Build hci connection between Dev_A and Dev_B.
[PASS] HCI_002: Receive ACL data with HCI.
[PASS] HCI_003: Send ACL data with HCI.
[PASS] HCI_004: Change name of remote Dev by HCI.
[PASS] HCI_005: Change class of remote Dev by HCI.
[PASS] HCI_006: Read local HCI controller information.
[PASS] HCI_007: Read remote HCI controller information.
=== L2CAP ===
[PASS] L2CAP_001: Build l2cap connection between Dev_A and Dev_B.
[PASS] L2CAP_002: Dev_A and Dev_B take l2cap protocol to do pingpong.
=== RFCOMM ===
[PASS] RFCOMM_001: Build rfcomm connection between Dev_A and Dev_B.
[PASS] RFCOMM_002: Dev_A and Dev_B take rfcomm protocol to do pingpong.
=== Settings ===
[PASS] SET_001: [BAT] No error output during bluetoothd startup process.
=== Bluetooth Utils ===
Some basic checking for hciconfig, hcitool, sdptool commands.
[PASS] BTCMD_001: hciconfig -a
[PASS] BTCMD_002: hciconfig <adapter> piscan
[PASS] BTCMD_003: hciconfig <adapter> up/down
[PASS] BTCMD_004: hcitool scan
[PASS] BTCMD_005: sdptool browse local
[PASS] BTCMD_006: sdptool browse <ermote MAC>
[PASS] BTCMD_007: sdptool add/del
^ permalink raw reply
* [PATCH RESEND] Bluetooth: Add support for Apple MacBook Pro 9,2
From: Shea Levy @ 2012-08-07 21:21 UTC (permalink / raw)
To: linux-bluetooth
Cc: linux-kernel, Johan Hedberg, Gustavo Padovan, Marcel Holtmann,
Shea Levy
$ usb-devices
<snip>
T: Bus=04 Lev=04 Prnt=04 Port=02 Cnt=01 Dev#= 9 Spd=12 MxCh= 0
D: Ver= 2.00 Cls=ff(vend.) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=05ac ProdID=821d Rev=00.86
S: Manufacturer=Apple Inc.
S: Product=Bluetooth USB Host Controller
C: #Ifs= 4 Cfg#= 1 Atr=e0 MxPwr=0mA
I: If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=01 Prot=01 Driver=btusb
I: If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)
I: If#= 3 Alt= 0 #EPs= 0 Cls=fe(app. ) Sub=01 Prot=01 Driver=(none)
<snip>
Signed-off-by: Shea Levy <shea@shealevy.com>
---
drivers/bluetooth/btusb.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index f637c25..038acc7 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -73,6 +73,9 @@ static struct usb_device_id btusb_table[] = {
/* Apple MacBookPro8,2 */
{ USB_DEVICE(0x05ac, 0x821a) },
+ /* Apple MacBookPro9,2 */
+ { USB_DEVICE(0x05ac, 0x821d) },
+
/* Apple MacMini5,1 */
{ USB_DEVICE(0x05ac, 0x8281) },
--
1.7.11
^ permalink raw reply related
* Re: l2ping Recv failed: Connection reset by peer
From: Mike @ 2012-08-07 21:01 UTC (permalink / raw)
To: Charles Gordon; +Cc: linux-bluetooth
In-Reply-To: <CAGjArBcwtaRoecSivEaN9FiCbGfC3JhFRwXUVqq58SmQhN07sQ@mail.gmail.com>
Hi Charles,
> I'm porting bluez 4.98 to an embedded system. I have it up and running, but
> I notice that when I run l2ping, I get the message "Recv failed: Connection
> reset by peer" after l2ping runs for 5 seconds. This is very consistent.
> It doesn't matter whether I ping once a second, or do a flood ping. After 5
> seconds the l2ping program closes with the message saying the connection was
> reset by the peer.
>
> This looks like some sort of timeout. Perhaps the timeout is there for
> security reasons. I'd like to understand how to adjust the timeout, or at
> least confirm that the system is working correctly.
>
> I'm running Linux 2.6.35 and Bluez 4.98 on my embedded system, and Ubuntu
> 10.04 with Linux 2.6.32 and Bluez 4.60 on my PC.
>
> I've looked through the Bluez configuration files and I don't see anything
> that sets up security for L2CAP or sets timeouts that are relevant. I tried
> increasing the PageTimeout in /etc/bluetooth/main.conf to 81920 (50 seconds)
> on both systems, but that didn't make any difference. I also looked in the
> configuration program hciconfig for an option to set a timeout, but I didn't
> find anything.
>
> Can anyone tell me what the relevant timeout for l2ping is and how to adjust
> it, or at least why it is set for 5 seconds?
I have a feeling you're actually hitting a timeout inside the kernel.
This timer prevents devices from connecting and draining the battery
of your device. So, unless the device connects to something "useful",
it gets disconnected. It's been too long since I've looked at it, but
the timeouts are specified in "include/net/bluetooth/hci.h". I have
seen the same l2ping issues you see, so I think your setup is working
as designed.
Mike
^ permalink raw reply
* Re: [PATCH v2 9/9] Battery: Emit property changed on first read
From: Joao Paulo Rechi Vita @ 2012-08-07 16:29 UTC (permalink / raw)
To: Chen Ganir; +Cc: linux-bluetooth
In-Reply-To: <1343194947-13659-10-git-send-email-chen.ganir@ti.com>
On Wed, Jul 25, 2012 at 2:42 AM, Chen Ganir <chen.ganir@ti.com> wrote:
> Emit battery level property changed upon connection, on first read.
> ---
> profiles/batterystate/batterystate.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/profiles/batterystate/batterystate.c b/profiles/batterystate/batterystate.c
> index 718863b..4300f85 100644
> --- a/profiles/batterystate/batterystate.c
> +++ b/profiles/batterystate/batterystate.c
> @@ -72,6 +72,8 @@ struct descriptor {
> bt_uuid_t uuid; /* UUID */
> };
>
> +static void emit_battery_level_changed(struct characteristic *c);
> +
No need of this forward declaration, simply move the function
implementation before the first function that references it
(read_batterylevel_cb).
> static void char_free(gpointer user_data)
> {
> struct characteristic *c = user_data;
> @@ -161,6 +163,7 @@ static void read_batterylevel_cb(guint8 status, const guint8 *pdu, guint16 len,
> }
>
> ch->level = value[0];
> + emit_battery_level_changed(ch);
> }
>
> static void process_batteryservice_char(struct characteristic *ch)
> --
> 1.7.9.5
>
> --
> 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
--
João Paulo Rechi Vita
Openbossa Labs - INdT
^ permalink raw reply
* Re: [PATCH v2 8/9] Battery: Add support for notifications
From: Joao Paulo Rechi Vita @ 2012-08-07 16:29 UTC (permalink / raw)
To: Chen Ganir; +Cc: linux-bluetooth
In-Reply-To: <1343194947-13659-9-git-send-email-chen.ganir@ti.com>
On Wed, Jul 25, 2012 at 2:42 AM, Chen Ganir <chen.ganir@ti.com> wrote:
> Add support for emitting PropertyChanged when a battery level
> characteristic notification is sent from the peer device.
> ---
> doc/battery-api.txt | 5 ++
> profiles/batterystate/batterystate.c | 107 +++++++++++++++++++++++++++++++++-
> 2 files changed, 111 insertions(+), 1 deletion(-)
>
> diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> index 5d7510d..f31e7e5 100644
> --- a/doc/battery-api.txt
> +++ b/doc/battery-api.txt
> @@ -16,6 +16,11 @@ Methods dict GetProperties()
> Returns all properties for the interface. See the
> Properties section for the available properties.
>
> +Signals PropertyChanged(string name, variant value)
> +
> + This signal indicates a changed value of the given
> + property.
> +
> Properties byte Namespace [readonly]
>
> Namespace value from the battery format characteristic
> diff --git a/profiles/batterystate/batterystate.c b/profiles/batterystate/batterystate.c
> index e5b68a9..718863b 100644
> --- a/profiles/batterystate/batterystate.c
> +++ b/profiles/batterystate/batterystate.c
> @@ -50,6 +50,7 @@ struct battery {
> GAttrib *attrib; /* GATT connection */
> guint attioid; /* Att watcher id */
> struct att_range *svc_range; /* Battery range */
> + guint attnotid; /* Att notifications id */
> GSList *chars; /* Characteristics */
> };
>
> @@ -104,6 +105,14 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
> return -1;
> }
>
> +static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
> +{
> + const struct characteristic *ch = a;
> + const uint16_t *handle = b;
> +
> + return ch->attr.value_handle - *handle;
> +}
> +
> static void batterystate_free(gpointer user_data)
> {
> struct battery *batt = user_data;
> @@ -117,6 +126,10 @@ static void batterystate_free(gpointer user_data)
> if (batt->attrib != NULL)
> g_attrib_unref(batt->attrib);
>
> + if (batt->attrib != NULL) {
> + g_attrib_unregister(batt->attrib, batt->attnotid);
> + g_attrib_unref(batt->attrib);
> + }
>
> dbus_connection_unref(batt->conn);
> btd_device_unref(batt->dev);
> @@ -158,6 +171,17 @@ static void process_batteryservice_char(struct characteristic *ch)
> }
> }
>
> +static void batterylevel_enable_notify_cb(guint8 status, const guint8 *pdu,
> + guint16 len, gpointer user_data)
> +{
> + char *msg = user_data;
> +
> + if (status != 0)
> + error("Could not enable battery level notification: %s", msg);
> +
> + g_free(msg);
> +}
> +
> static void batterylevel_presentation_format_desc_cb(guint8 status,
> const guint8 *pdu, guint16 len,
> gpointer user_data)
> @@ -194,6 +218,23 @@ static void process_batterylevel_desc(struct descriptor *desc)
> char uuidstr[MAX_LEN_UUID_STR];
> bt_uuid_t btuuid;
>
> + bt_uuid16_create(&btuuid, GATT_CLIENT_CHARAC_CFG_UUID);
> +
> + if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0 && g_strcmp0(ch->attr.uuid,
> + BATTERY_LEVEL_UUID) == 0) {
> + uint8_t atval[2];
> + uint16_t val;
> + char *msg;
> +
> + val = GATT_CLIENT_CHARAC_CFG_NOTIF_BIT;
> + msg = g_strdup("Enable BatteryLevel notification");
> +
What's the point of passing "Enable BatteryLevel notification" as
user_data for the batterylevel_enable_notify_cb() ?
> + att_put_u16(val, atval);
> + gatt_write_char(ch->batt->attrib, desc->handle, atval, 2,
> + batterylevel_enable_notify_cb, msg);
> + return;
> + }
> +
> bt_uuid16_create(&btuuid, GATT_CHARAC_FMT_UUID);
>
> if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0) {
> @@ -277,6 +318,12 @@ static DBusMessage *get_properties(DBusConnection *conn, DBusMessage *msg,
> return reply;
> }
>
> +static void emit_battery_level_changed(struct characteristic *c)
> +{
> + emit_property_changed(c->batt->conn, c->path, BATTERY_INTERFACE,
> + "Level", DBUS_TYPE_BYTE, &c->level);
> +}
> +
> static GDBusMethodTable battery_methods[] = {
> { GDBUS_METHOD("GetProperties",
> NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
> @@ -284,6 +331,11 @@ static GDBusMethodTable battery_methods[] = {
> { }
> };
>
> +static GDBusSignalTable battery_signals[] = {
> + { GDBUS_SIGNAL("PropertyChanged",
> + GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
> + { }
> +};
>
> static void configure_batterystate_cb(GSList *characteristics, guint8 status,
> gpointer user_data)
> @@ -318,7 +370,7 @@ static void configure_batterystate_cb(GSList *characteristics, guint8 status,
>
> if (!g_dbus_register_interface(batt->conn, ch->path,
> BATTERY_INTERFACE,
> - battery_methods, NULL, NULL,
> + battery_methods, battery_signals, NULL,
> ch, NULL)) {
> error("D-Bus register interface %s failed",
> BATTERY_INTERFACE);
> @@ -346,12 +398,63 @@ static void configure_batterystate_cb(GSList *characteristics, guint8 status,
> }
> }
>
> +static void proc_batterylevel(struct characteristic *c, const uint8_t *pdu,
> + uint16_t len, gboolean final)
> +{
> + uint8_t new_batt_level = 0;
> + gboolean changed = FALSE;
> +
> + if (!pdu) {
> + error("Battery level notification: Invalid pdu length");
> + goto done;
> + }
> +
If pdu can be NULL here, than bluetoothd would have broken when
derreferencing it at "handle = att_get_u16(&pdu[1]);" in
notif_handler(). If pdu can be NULL the check needs to be done there.
> + new_batt_level = pdu[1];
> +
> + if (new_batt_level != c->level)
> + changed = TRUE;
> +
> + c->level = new_batt_level;
> +
> +done:
There is no need for the done label, since 'changed' is always false
when you 'goto' here. You could simply return instead of using goto,
although the whole if statement is likely to be dropped from this
function.
> + if (changed)
> + emit_battery_level_changed(c);
> +}
> +
> +static void notif_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
> +{
> + struct battery *batt = user_data;
> + struct characteristic *ch;
> + uint16_t handle;
> + GSList *l;
> +
ch->attr.uuid == BATTERY_LEVEL_UUID should be tested before anything
else, since other profiles may have enabled notifications for other
characteristics (hog does that, for example). When testing with hog
devices I got "notif_handler: Unexpected handle 0x0017" for every
input report received.
> + if (len < 3) {
> + error("notif_handler: Bad pdu received");
> + return;
> + }
> +
> + handle = att_get_u16(&pdu[1]);
> + l = g_slist_find_custom(batt->chars, &handle, cmp_char_val_handle);
> + if (l == NULL) {
> + error("notif_handler: Unexpected handle 0x%04x", handle);
> + return;
> + }
> +
> + ch = l->data;
> + if (g_strcmp0(ch->attr.uuid, BATTERY_LEVEL_UUID) == 0) {
> + proc_batterylevel(ch, pdu, len, FALSE);
> + }
> +}
> +
> static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
> {
> struct battery *batt = user_data;
>
> batt->attrib = g_attrib_ref(attrib);
>
> + batt->attnotid = g_attrib_register(batt->attrib, ATT_OP_HANDLE_NOTIFY,
> + notif_handler, batt, NULL);
> +
> if (batt->chars == NULL) {
> gatt_discover_char(batt->attrib, batt->svc_range->start,
> batt->svc_range->end, NULL,
> @@ -369,6 +472,8 @@ static void attio_disconnected_cb(gpointer user_data)
> {
> struct battery *batt = user_data;
>
> + g_attrib_unregister(batt->attrib, batt->attnotid);
> + batt->attnotid = 0;
> g_attrib_unref(batt->attrib);
> batt->attrib = NULL;
> }
> --
> 1.7.9.5
>
> --
> 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
--
João Paulo Rechi Vita
Openbossa Labs - INdT
^ permalink raw reply
* Re: [PATCH v2 5/9] Battery: Add Battery list to btd_device
From: Joao Paulo Rechi Vita @ 2012-08-07 16:29 UTC (permalink / raw)
To: Chen Ganir; +Cc: linux-bluetooth
In-Reply-To: <1343194947-13659-6-git-send-email-chen.ganir@ti.com>
On Wed, Jul 25, 2012 at 2:42 AM, Chen Ganir <chen.ganir@ti.com> wrote:
> Add peer battery list to the btd_device. New property added to Device
> called Batteries.
> ---
> doc/device-api.txt | 5 +++
> profiles/batterystate/batterystate.c | 6 ++++
> src/device.c | 66 ++++++++++++++++++++++++++++++++++
> src/device.h | 3 ++
> 4 files changed, 80 insertions(+)
>
> diff --git a/doc/device-api.txt b/doc/device-api.txt
> index 3b84033..3d19a53 100644
> --- a/doc/device-api.txt
> +++ b/doc/device-api.txt
> @@ -175,3 +175,8 @@ Properties string Address [readonly]
> Note that this property can exhibit false-positives
> in the case of Bluetooth 2.1 (or newer) devices that
> have disabled Extended Inquiry Response support.
> +
> + array{string} Batteries [readonly]
> +
> + List of device battery object paths that represents the available
> + batteries on the remote device.
This property should be deprecated by the introduction of the D-Bus
object manager (scheduled for the 5.0 release).
> diff --git a/profiles/batterystate/batterystate.c b/profiles/batterystate/batterystate.c
> index d3a1974..23dfab5 100644
> --- a/profiles/batterystate/batterystate.c
> +++ b/profiles/batterystate/batterystate.c
> @@ -50,6 +50,7 @@ struct battery {
> static GSList *servers;
>
> struct characteristic {
> + char *path; /* object path */
> struct gatt_char attr; /* Characteristic */
> struct battery *batt; /* Parent Battery Service */
> GSList *desc; /* Descriptors */
> @@ -206,6 +207,11 @@ static void configure_batterystate_cb(GSList *characteristics, guint8 status,
> ch->attr.value_handle = c->value_handle;
> memcpy(ch->attr.uuid, c->uuid, MAX_LEN_UUID_STR + 1);
> ch->batt = batt;
> + ch->path = g_strdup_printf("%s/BATT%04X",
> + device_get_path(batt->dev),
> + c->handle);
> +
What's the reationale behind using the characteristic handle as an
identifier of the battery on the object path? Can't we do anything
better than that? I understand this avoids clashing on a device with
multiple battery services, but I don't really like the idea of
exposing this internal details as an identifier on the API. Maybe
using the battery namespace + description? It is mandatory when the
device implements more than one instance of the battery service. If
there is only one instance we don't need an identifier at all, only
BATT would be sufficient.
> + device_add_battery(batt->dev, ch->path);
>
> batt->chars = g_slist_append(batt->chars, ch);
>
> diff --git a/src/device.c b/src/device.c
> index cd571f7..98e431a 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -127,6 +127,10 @@ struct att_callbacks {
> gpointer user_data;
> };
>
> +struct btd_battery {
> + char *path;
> +};
> +
> struct btd_device {
> bdaddr_t bdaddr;
> uint8_t bdaddr_type;
> @@ -172,6 +176,7 @@ struct btd_device {
>
> GIOChannel *att_io;
> guint cleanup_id;
> + GSList *batteries;
> };
>
> static uint16_t uuid_list[] = {
> @@ -262,6 +267,7 @@ static void device_free(gpointer user_data)
> g_slist_free_full(device->primaries, g_free);
> g_slist_free_full(device->attios, g_free);
> g_slist_free_full(device->attios_offline, g_free);
> + g_slist_free_full(device->batteries, g_free);
>
> attio_cleanup(device);
>
> @@ -433,6 +439,15 @@ static DBusMessage *get_properties(DBusConnection *conn,
> ptr = adapter_get_path(adapter);
> dict_append_entry(&dict, "Adapter", DBUS_TYPE_OBJECT_PATH, &ptr);
>
> + /* Batteries */
> + str = g_new0(char *, g_slist_length(device->batteries) + 1);
> + for (i = 0, l = device->batteries; l; l = l->next, i++) {
> + struct btd_battery *b = l->data;
> + str[i] = b->path;
> + }
> + dict_append_array(&dict, "Batteries", DBUS_TYPE_OBJECT_PATH, &str, i);
> + g_free(str);
> +
> dbus_message_iter_close_container(&iter, &dict);
>
> return reply;
> @@ -1219,6 +1234,9 @@ void device_remove(struct btd_device *device, gboolean remove_stored)
> g_slist_free(device->drivers);
> device->drivers = NULL;
>
> + g_slist_free(device->batteries);
> + device->batteries = NULL;
> +
> attrib_client_unregister(device->services);
>
> btd_device_unref(device);
> @@ -3141,3 +3159,51 @@ void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
> device_set_product(device, product_id);
> device_set_version(device, product_ver);
> }
> +
> +static void batteries_changed(struct btd_device *device)
> +{
> + DBusConnection *conn = get_dbus_connection();
> + char **batteries;
> + GSList *l;
> + int i;
> +
> + batteries = g_new0(char *, g_slist_length(device->batteries) + 1);
> + for (i = 0, l = device->batteries; l; l = l->next, i++) {
> + struct btd_battery *batt = l->data;
> + batteries[i] = batt->path;
> + }
> +
> + emit_array_property_changed(conn, device->path, DEVICE_INTERFACE,
> + "Batteries", DBUS_TYPE_STRING, &batteries,
> + i);
> +
> + g_free(batteries);
> +}
> +
> +void device_add_battery(struct btd_device *device, char *path)
> +{
> + struct btd_battery *batt;
> +
> + batt = g_new0(struct btd_battery, 1);
> + batt->path = g_strdup(path);
> + device->batteries = g_slist_append(device->batteries, batt);
> + batteries_changed(device);
> +}
> +
> +void device_remove_battery(struct btd_device *device, char *path)
> +{
> + GSList *l;
> +
> + for (l = device->batteries; l; l = l->next) {
> + struct btd_battery *b = l->data;
> +
> + if (g_strcmp0(path, b->path) == 0) {
> + device->batteries = g_slist_remove(device->batteries, b);
> + g_free(b->path);
> + g_free(b);
> + return;
If you return here the property changed signal will not be emitted.
> + }
> + }
> +
> + batteries_changed(device);
> +}
> diff --git a/src/device.h b/src/device.h
> index 26e17f7..db71a8a 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -126,3 +126,6 @@ int device_unblock(DBusConnection *conn, struct btd_device *device,
> void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
> uint16_t vendor_id, uint16_t product_id,
> uint16_t product_ver);
> +
> +void device_add_battery(struct btd_device *device, char *path);
> +void device_remove_battery(struct btd_device *device, char *path);
> --
> 1.7.9.5
>
> --
> 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
--
João Paulo Rechi Vita
Openbossa Labs - INdT
^ permalink raw reply
* Re: [PATCH v2 4/9] Battery: Get Battery ID
From: Joao Paulo Rechi Vita @ 2012-08-07 16:29 UTC (permalink / raw)
To: Chen Ganir; +Cc: linux-bluetooth
In-Reply-To: <1343194947-13659-5-git-send-email-chen.ganir@ti.com>
On Wed, Jul 25, 2012 at 2:42 AM, Chen Ganir <chen.ganir@ti.com> wrote:
> Read the battery level format characteristic descriptor to get the
> unique namespace and description values.
> ---
> profiles/batterystate/batterystate.c | 113 ++++++++++++++++++++++++++--------
> 1 file changed, 86 insertions(+), 27 deletions(-)
>
> diff --git a/profiles/batterystate/batterystate.c b/profiles/batterystate/batterystate.c
> index a7d2f6e..d3a1974 100644
> --- a/profiles/batterystate/batterystate.c
> +++ b/profiles/batterystate/batterystate.c
> @@ -37,6 +37,8 @@
> #include "batterystate.h"
> #include "log.h"
>
> +#define BATTERY_LEVEL_UUID "00002a19-0000-1000-8000-00805f9b34fb"
> +
> struct battery {
> struct btd_device *dev; /* Device reference */
> GAttrib *attrib; /* GATT connection */
> @@ -48,15 +50,18 @@ struct battery {
> static GSList *servers;
>
> struct characteristic {
> - struct gatt_char attr; /* Characteristic */
> - struct battery *batt; /* Parent Battery Service */
> + struct gatt_char attr; /* Characteristic */
> + struct battery *batt; /* Parent Battery Service */
> GSList *desc; /* Descriptors */
> + uint8_t ns; /* Battery Namespace */
> + uint16_t description; /* Battery description */
> + uint8_t level; /* Battery last known level */
The 'level' field it's not being used here. It should be added by the
same commit that uses it.
> };
>
> struct descriptor {
> - struct characteristic *ch; /* Parent Characteristic */
> - uint16_t handle; /* Descriptor Handle */
> - bt_uuid_t uuid; /* UUID */
> + struct characteristic *ch; /* Parent Characteristic */
> + uint16_t handle; /* Descriptor Handle */
> + bt_uuid_t uuid; /* UUID */
> };
>
> static gint cmp_device(gconstpointer a, gconstpointer b)
> @@ -87,6 +92,55 @@ static void batterystate_free(gpointer user_data)
> g_free(batt);
> }
>
> +static void batterylevel_presentation_format_desc_cb(guint8 status,
> + const guint8 *pdu, guint16 len,
> + gpointer user_data)
> +{
> + struct descriptor *desc = user_data;
> + uint8_t value[ATT_MAX_MTU];
> + int vlen;
> +
> + if (status != 0) {
> + error("Presentation Format desc read failed: %s",
> + att_ecode2str(status));
> + return;
> + }
> +
> + vlen = dec_read_resp(pdu, len, value, sizeof(value));
> + if (!vlen) {
> + error("Presentation Format desc read failed: Protocol error\n");
> + return;
> + }
> +
> + if (vlen < 7) {
> + error("Presentation Format desc read failed: Invalid range");
> + return;
> + }
> +
> + desc->ch->ns = value[4];
> + desc->ch->description = att_get_u16(&value[5]);
> +}
> +
> +
> +static void process_batterylevel_desc(struct descriptor *desc)
> +{
> + struct characteristic *ch = desc->ch;
> + char uuidstr[MAX_LEN_UUID_STR];
> + bt_uuid_t btuuid;
> +
> + bt_uuid16_create(&btuuid, GATT_CHARAC_FMT_UUID);
> +
> + if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0) {
> + gatt_read_char(ch->batt->attrib, desc->handle, 0,
> + batterylevel_presentation_format_desc_cb, desc);
> + return;
> + }
> +
> + bt_uuid_to_string(&desc->uuid, uuidstr, MAX_LEN_UUID_STR);
> + DBG("Ignored descriptor %s characteristic %s", uuidstr, ch->attr.uuid);
> +}
> +
> +
> static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
> gpointer user_data)
> {
> @@ -120,6 +174,7 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
> desc->uuid = att_get_uuid128(&value[2]);
>
> ch->desc = g_slist_append(ch->desc, desc);
> + process_batterylevel_desc(desc);
> }
>
> att_data_list_free(list);
> @@ -140,31 +195,35 @@ static void configure_batterystate_cb(GSList *characteristics, guint8 status,
>
> for (l = characteristics; l; l = l->next) {
> struct gatt_char *c = l->data;
> - struct characteristic *ch;
> - uint16_t start, end;
> -
> - ch = g_new0(struct characteristic, 1);
> - ch->attr.handle = c->handle;
> - ch->attr.properties = c->properties;
> - ch->attr.value_handle = c->value_handle;
> - memcpy(ch->attr.uuid, c->uuid, MAX_LEN_UUID_STR + 1);
> - ch->batt = batt;
>
> - batt->chars = g_slist_append(batt->chars, ch);
> -
> - start = c->value_handle + 1;
> -
> - if (l->next != NULL) {
> - struct gatt_char *c = l->next->data;
> - if (start == c->handle)
> + if (g_strcmp0(c->uuid, BATTERY_LEVEL_UUID) == 0) {
> + struct characteristic *ch;
> + uint16_t start, end;
> +
> + ch = g_new0(struct characteristic, 1);
> + ch->attr.handle = c->handle;
> + ch->attr.properties = c->properties;
> + ch->attr.value_handle = c->value_handle;
> + memcpy(ch->attr.uuid, c->uuid, MAX_LEN_UUID_STR + 1);
> + ch->batt = batt;
> +
> + batt->chars = g_slist_append(batt->chars, ch);
> +
> + start = c->value_handle + 1;
> +
> + if (l->next != NULL) {
> + struct gatt_char *c = l->next->data;
> + if (start == c->handle)
> + continue;
> + end = c->handle - 1;
> + } else if (c->value_handle != batt->svc_range->end)
> + end = batt->svc_range->end;
> + else
> continue;
> - end = c->handle - 1;
> - } else if (c->value_handle != batt->svc_range->end)
> - end = batt->svc_range->end;
> - else
> - continue;
>
> - gatt_find_info(batt->attrib, start, end, discover_desc_cb, ch);
> + gatt_find_info(batt->attrib, start, end,
> + discover_desc_cb, ch);
> + }
> }
> }
>
> --
> 1.7.9.5
>
> --
> 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
--
João Paulo Rechi Vita
Openbossa Labs - INdT
^ 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