* [RFCv1 13/20] Bluetooth: Handle physical link completion
From: Mat Martineau @ 2012-09-11 18:21 UTC (permalink / raw)
To: linux-bluetooth, gustavo; +Cc: sunnyk
In-Reply-To: <1347387691-5285-1-git-send-email-mathewm@codeaurora.org>
Several different actions may be taken when an AMP physical link
becomes available. A channel being created on an AMP controller must
continue the connection process. A channel being moved needs to
either send a move request or a move response. A failed physical link
will revert to using a BR/EDR controller if possible.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap_core.c | 166 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 166 insertions(+)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 0d57ab3..9d7fab0 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -977,6 +977,19 @@ static void l2cap_send_conn_req(struct l2cap_chan *chan)
l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_REQ, sizeof(req), &req);
}
+static void l2cap_send_create_chan_req(struct l2cap_chan *chan, u8 amp_id)
+{
+ struct l2cap_create_chan_req req;
+ req.scid = cpu_to_le16(chan->scid);
+ req.psm = chan->psm;
+ req.amp_id = amp_id;
+
+ chan->ident = l2cap_get_ident(chan->conn);
+
+ l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CREATE_CHAN_REQ,
+ sizeof(req), &req);
+}
+
static void l2cap_move_setup(struct l2cap_chan *chan)
{
struct sk_buff *skb;
@@ -4128,6 +4141,23 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
return 0;
}
+static void l2cap_send_move_chan_req(struct l2cap_chan *chan, u8 dest_amp_id)
+{
+ struct l2cap_move_chan_req req;
+ u8 ident;
+
+ BT_DBG("chan %p, dest_amp_id %d", chan, dest_amp_id);
+
+ ident = l2cap_get_ident(chan->conn);
+ chan->ident = ident;
+
+ req.icid = cpu_to_le16(chan->scid);
+ req.dest_amp_id = dest_amp_id;
+
+ l2cap_send_cmd(chan->conn, ident, L2CAP_MOVE_CHAN_REQ, sizeof(req),
+ &req);
+}
+
static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
u16 icid, u16 result)
{
@@ -4290,6 +4320,142 @@ static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
}
}
+static void l2cap_do_create(struct l2cap_chan *chan, int result,
+ u8 local_amp_id, u8 remote_amp_id)
+{
+ if (!test_bit(CONF_CONNECT_PEND, &chan->conf_state)) {
+ struct l2cap_conn_rsp rsp;
+ char buf[128];
+ rsp.scid = cpu_to_le16(chan->dcid);
+ rsp.dcid = cpu_to_le16(chan->scid);
+
+ /* Incoming channel on AMP */
+ if (result == L2CAP_CR_SUCCESS) {
+ /* Send successful response */
+ rsp.result = cpu_to_le16(L2CAP_CR_SUCCESS);
+ rsp.status = cpu_to_le16(L2CAP_CS_NO_INFO);
+ } else {
+ /* Send negative response */
+ rsp.result = cpu_to_le16(L2CAP_CR_NO_MEM);
+ rsp.status = cpu_to_le16(L2CAP_CS_NO_INFO);
+ }
+
+ l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CREATE_CHAN_RSP,
+ sizeof(rsp), &rsp);
+
+ if (result == L2CAP_CR_SUCCESS) {
+ __l2cap_state_change(chan, BT_CONFIG);
+ set_bit(CONF_REQ_SENT, &chan->conf_state);
+ l2cap_send_cmd(chan->conn, l2cap_get_ident(chan->conn),
+ L2CAP_CONF_REQ,
+ l2cap_build_conf_req(chan, buf), buf);
+ chan->num_conf_req++;
+ }
+ } else {
+ /* Outgoing channel on AMP */
+ if (result == L2CAP_CR_SUCCESS) {
+ chan->local_amp_id = local_amp_id;
+ l2cap_send_create_chan_req(chan, remote_amp_id);
+ } else {
+ /* Revert to BR/EDR connect */
+ l2cap_send_conn_req(chan);
+ }
+ }
+}
+
+static void l2cap_do_move_initiate(struct l2cap_chan *chan, u8 local_amp_id,
+ u8 remote_amp_id)
+{
+ l2cap_move_setup(chan);
+ chan->move_id = local_amp_id;
+ chan->move_state = L2CAP_MOVE_WAIT_RSP;
+
+ l2cap_send_move_chan_req(chan, remote_amp_id);
+ __set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+}
+
+static void l2cap_do_move_respond(struct l2cap_chan *chan, int result)
+{
+ struct hci_chan *hchan = NULL;
+
+ /* Placeholder - get hci_chan for logical link */
+
+ if (hchan) {
+ if (hchan->state == BT_CONNECTED) {
+ /* Logical link is ready to go */
+ chan->hs_hcon = hchan->conn;
+ chan->hs_hcon->l2cap_data = chan->conn;
+ chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
+ l2cap_send_move_chan_rsp(chan->conn, chan->ident,
+ chan->dcid, L2CAP_MR_SUCCESS);
+
+ l2cap_logical_cfm(chan, hchan, L2CAP_MR_SUCCESS);
+ } else {
+ /* Wait for logical link to be ready */
+ chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_CFM;
+ }
+ } else {
+ /* Logical link not available */
+ l2cap_send_move_chan_rsp(chan->conn, chan->ident, chan->dcid,
+ L2CAP_MR_NOT_ALLOWED);
+ }
+}
+
+static void l2cap_do_move_cancel(struct l2cap_chan *chan, int result)
+{
+ if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
+ u8 rsp_result;
+ if (result == -EINVAL)
+ rsp_result = L2CAP_MR_BAD_ID;
+ else
+ rsp_result = L2CAP_MR_NOT_ALLOWED;
+
+ l2cap_send_move_chan_rsp(chan->conn, chan->ident, chan->dcid,
+ rsp_result);
+ }
+
+ chan->move_role = L2CAP_MOVE_ROLE_NONE;
+ chan->move_state = L2CAP_MOVE_STABLE;
+
+ /* Restart data transmission */
+ l2cap_ertm_send(chan);
+}
+
+void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
+ u8 remote_amp_id)
+{
+ BT_DBG("chan %p, result %d, local_amp_id %d, remote_amp_id %d",
+ chan, result, local_amp_id, remote_amp_id);
+
+ l2cap_chan_lock(chan);
+
+ if (chan->state == BT_DISCONN || chan->state == BT_CLOSED) {
+ l2cap_chan_unlock(chan);
+ return;
+ }
+
+ if (chan->state != BT_CONNECTED) {
+ l2cap_do_create(chan, result, local_amp_id, remote_amp_id);
+ } else if (result != L2CAP_MR_SUCCESS) {
+ l2cap_do_move_cancel(chan, result);
+ } else {
+ switch (chan->move_role) {
+ case L2CAP_MOVE_ROLE_INITIATOR:
+ l2cap_do_move_initiate(chan, local_amp_id,
+ remote_amp_id);
+ break;
+ case L2CAP_MOVE_ROLE_RESPONDER:
+ l2cap_do_move_respond(chan, result);
+ break;
+ default:
+ l2cap_do_move_cancel(chan, result);
+ break;
+ }
+ }
+
+ l2cap_chan_unlock(chan);
+}
+
static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
struct l2cap_cmd_hdr *cmd,
u16 cmd_len, void *data)
--
1.7.12
--
Mat Martineau
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [RFCv1 14/20] Bluetooth: Flag ACL frames as complete for AMP controllers
From: Mat Martineau @ 2012-09-11 18:21 UTC (permalink / raw)
To: linux-bluetooth, gustavo; +Cc: sunnyk
In-Reply-To: <1347387691-5285-1-git-send-email-mathewm@codeaurora.org>
AMP controllers expect to transmit only "complete" ACL frames. These
frames have both the "start" and "cont" bits set. AMP does not allow
fragmented ACLs.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
include/net/bluetooth/hci.h | 1 +
net/bluetooth/l2cap_core.c | 14 ++++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 0f28f70..0732721 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -196,6 +196,7 @@ enum {
#define ACL_START_NO_FLUSH 0x00
#define ACL_CONT 0x01
#define ACL_START 0x02
+#define ACL_COMPLETE 0x03
#define ACL_ACTIVE_BCAST 0x04
#define ACL_PICO_BCAST 0x08
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 9d7fab0..4a5c42b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -734,10 +734,20 @@ static void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
u16 flags;
BT_DBG("chan %p, skb %p len %d priority %u", chan, skb, skb->len,
- skb->priority);
+ skb->priority);
+
+ if (chan->hs_hcon && (chan->move_state == L2CAP_MOVE_STABLE ||
+ chan->move_state == L2CAP_MOVE_WAIT_PREPARE)) {
+ if (chan->hs_hchan)
+ hci_send_acl(chan->hs_hchan, skb, ACL_COMPLETE);
+ else
+ kfree_skb(skb);
+
+ return;
+ }
if (!test_bit(FLAG_FLUSHABLE, &chan->flags) &&
- lmp_no_flush_capable(hcon->hdev))
+ lmp_no_flush_capable(hcon->hdev))
flags = ACL_START_NO_FLUSH;
else
flags = ACL_START;
--
1.7.12
--
Mat Martineau
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [RFCv1 15/20] Bluetooth: Do not send data during channel move
From: Mat Martineau @ 2012-09-11 18:21 UTC (permalink / raw)
To: linux-bluetooth, gustavo; +Cc: sunnyk
In-Reply-To: <1347387691-5285-1-git-send-email-mathewm@codeaurora.org>
Outgoing ERTM data is queued during a channel move. The ERTM state
machine is partially reset at the start of a move, and must be
resynchronized with the remote state machine at the end of the move.
Data is not sent so that there are no state transitions between the
partial reset and the resync.
Streaming mode frames are dropped during a move.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap_core.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 4a5c42b..0a7df20 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -921,6 +921,10 @@ static void l2cap_send_sframe(struct l2cap_chan *chan,
if (!control->sframe)
return;
+ if (chan->move_state != L2CAP_MOVE_STABLE &&
+ chan->move_state != L2CAP_MOVE_WAIT_PREPARE)
+ return;
+
if (test_and_clear_bit(CONN_SEND_FBIT, &chan->conn_state) &&
!control->poll)
control->final = 1;
@@ -1765,6 +1769,10 @@ static void l2cap_streaming_send(struct l2cap_chan *chan,
BT_DBG("chan %p, skbs %p", chan, skbs);
+ if (chan->move_state != L2CAP_MOVE_STABLE &&
+ chan->move_state != L2CAP_MOVE_WAIT_PREPARE)
+ return;
+
skb_queue_splice_tail_init(skbs, &chan->tx_q);
while (!skb_queue_empty(&chan->tx_q)) {
@@ -1807,6 +1815,10 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state))
return 0;
+ if (chan->move_state != L2CAP_MOVE_STABLE &&
+ chan->move_state != L2CAP_MOVE_WAIT_PREPARE)
+ return 0;
+
while (chan->tx_send_head &&
chan->unacked_frames < chan->remote_tx_win &&
chan->tx_state == L2CAP_TX_STATE_XMIT) {
@@ -1872,6 +1884,10 @@ static void l2cap_ertm_resend(struct l2cap_chan *chan)
if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state))
return;
+ if (chan->move_state != L2CAP_MOVE_STABLE &&
+ chan->move_state != L2CAP_MOVE_WAIT_PREPARE)
+ return;
+
while (chan->retrans_list.head != L2CAP_SEQ_LIST_CLEAR) {
seq = l2cap_seq_list_pop(&chan->retrans_list);
--
1.7.12
--
Mat Martineau
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [RFCv1 16/20] Bluetooth: Configure appropriate timeouts for AMP controllers
From: Mat Martineau @ 2012-09-11 18:21 UTC (permalink / raw)
To: linux-bluetooth, gustavo; +Cc: sunnyk
In-Reply-To: <1347387691-5285-1-git-send-email-mathewm@codeaurora.org>
The L2CAP spec recommends specific retransmit and monitor timeouts for
ERTM channels that are on AMP controllers. These timeouts are
calculated from the AMP controller's best effort flush timeout.
BR/EDR controllers use the default retransmit and monitor timeouts.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap_core.c | 47 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 41 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 0a7df20..e7c2efc 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2924,6 +2924,44 @@ static inline bool __l2cap_efs_supported(struct l2cap_chan *chan)
return enable_hs && chan->conn->feat_mask & L2CAP_FEAT_EXT_FLOW;
}
+static void __l2cap_set_ertm_timeouts(struct l2cap_chan *chan,
+ struct l2cap_conf_rfc *rfc)
+{
+ if (chan->local_amp_id && chan->hs_hcon) {
+ u64 ertm_to = chan->hs_hcon->hdev->amp_be_flush_to;
+
+ /* Class 1 devices have must have ERTM timeouts
+ * exceeding the Link Supervision Timeout. The
+ * default Link Supervision Timeout for AMP
+ * controllers is 10 seconds.
+ *
+ * Class 1 devices use 0xffffffff for their
+ * best-effort flush timeout, so the clamping logic
+ * will result in a timeout that meets the above
+ * requirement. ERTM timeouts are 16-bit values, so
+ * the maximum timeout is 65.535 seconds.
+ */
+
+ /* Convert timeout to milliseconds and round */
+ ertm_to = div_u64(ertm_to + 999, 1000);
+
+ /* This is the recommended formula for class 2 devices
+ * that start ERTM timers when packets are sent to the
+ * controller.
+ */
+ ertm_to = 3 * ertm_to + 500;
+
+ if (ertm_to > 0xffff)
+ ertm_to = 0xffff;
+
+ rfc->retrans_timeout = cpu_to_le16((u16) ertm_to);
+ rfc->monitor_timeout = rfc->retrans_timeout;
+ } else {
+ rfc->retrans_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
+ rfc->monitor_timeout = __constant_cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
+ }
+}
+
static inline void l2cap_txwin_setup(struct l2cap_chan *chan)
{
if (chan->tx_win > L2CAP_DEFAULT_TX_WINDOW &&
@@ -2990,8 +3028,8 @@ done:
case L2CAP_MODE_ERTM:
rfc.mode = L2CAP_MODE_ERTM;
rfc.max_transmit = chan->max_tx;
- rfc.retrans_timeout = 0;
- rfc.monitor_timeout = 0;
+
+ __l2cap_set_ertm_timeouts(chan, &rfc);
size = min_t(u16, L2CAP_DEFAULT_MAX_PDU_SIZE, chan->conn->mtu -
L2CAP_EXT_HDR_SIZE -
@@ -3223,10 +3261,7 @@ done:
rfc.max_pdu_size = cpu_to_le16(size);
chan->remote_mps = size;
- rfc.retrans_timeout =
- __constant_cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
- rfc.monitor_timeout =
- __constant_cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
+ __l2cap_set_ertm_timeouts(chan, &rfc);
set_bit(CONF_MODE_DONE, &chan->conf_state);
--
1.7.12
--
Mat Martineau
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [RFCv1 17/20] Bluetooth: Ignore BR/EDR packet size constraints when fragmenting for AMP
From: Mat Martineau @ 2012-09-11 18:21 UTC (permalink / raw)
To: linux-bluetooth, gustavo; +Cc: sunnyk
In-Reply-To: <1347387691-5285-1-git-send-email-mathewm@codeaurora.org>
When operating over BR/EDR, ERTM accounts for the maximum over-the-air
packet size when setting the PDU size. AMP controllers do not use the
same over-the-air packets, so the PDU size should only be based on the
HCI MTU of the AMP controller.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap_core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index e7c2efc..3fd2eac 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2230,7 +2230,9 @@ static int l2cap_segment_sdu(struct l2cap_chan *chan,
/* PDU size is derived from the HCI MTU */
pdu_len = chan->conn->mtu;
- pdu_len = min_t(size_t, pdu_len, L2CAP_BREDR_MAX_PAYLOAD);
+ /* Constrain PDU size for BR/EDR connections */
+ if (!chan->hs_hcon)
+ pdu_len = min_t(size_t, pdu_len, L2CAP_BREDR_MAX_PAYLOAD);
/* Adjust for largest possible L2CAP overhead. */
if (chan->fcs)
--
1.7.12
--
Mat Martineau
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [RFCv1 18/20] Bluetooth: Send create channel request instead of connect for AMP
From: Mat Martineau @ 2012-09-11 18:21 UTC (permalink / raw)
To: linux-bluetooth, gustavo; +Cc: sunnyk
In-Reply-To: <1347387691-5285-1-git-send-email-mathewm@codeaurora.org>
When the channel policy is set to prefer AMP, then an L2CAP channel is
set up using the "create channel" command rather than the "connect"
command. A physical link is also set up before sending "create
channel".
Behavior is unchanged if enable_hs is false.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap_core.c | 84 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 78 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 3fd2eac..bde1303 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -56,7 +56,10 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn,
struct l2cap_chan *chan, int err);
static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
- struct sk_buff_head *skbs, u8 event);
+ struct sk_buff_head *skbs, u8 event);
+
+static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
+ u8 status);
/* ---- L2CAP channels ---- */
@@ -554,6 +557,13 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
hci_conn_put(conn->hcon);
}
+ if (chan->hs_hchan) {
+ chan->hs_hchan = NULL;
+ chan->hs_hcon = NULL;
+
+ /* Placeholder - free logical link */
+ }
+
if (chan->ops->teardown)
chan->ops->teardown(chan, err);
@@ -1100,8 +1110,19 @@ static void l2cap_do_start(struct l2cap_chan *chan)
return;
if (l2cap_chan_check_security(chan) &&
- __l2cap_no_conn_pending(chan))
- l2cap_send_conn_req(chan);
+ __l2cap_no_conn_pending(chan)) {
+ if (enable_hs &&
+ (conn->fixed_chan_mask & L2CAP_FC_A2MP) &&
+ chan->chan_policy == BT_CHANNEL_POLICY_AMP_PREFERRED) {
+ set_bit(CONF_CONNECT_PEND, &chan->conf_state);
+
+ /* Placeholder
+ amp_create_phylink();
+ */
+ } else {
+ l2cap_send_conn_req(chan);
+ }
+ }
} else {
struct l2cap_info_req req;
req.type = __constant_cpu_to_le16(L2CAP_IT_FEAT_MASK);
@@ -1196,8 +1217,17 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
continue;
}
- l2cap_send_conn_req(chan);
+ if (enable_hs &&
+ (conn->fixed_chan_mask & L2CAP_FC_A2MP) &&
+ chan->chan_policy == BT_CHANNEL_POLICY_AMP_PREFERRED) {
+ set_bit(CONF_CONNECT_PEND, &chan->conf_state);
+ /* Placeholder
+ amp_create_phylink();
+ */
+ } else {
+ l2cap_send_conn_req(chan);
+ }
} else if (chan->state == BT_CONNECT2) {
struct l2cap_conn_rsp rsp;
char buf[128];
@@ -1225,8 +1255,18 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHEN_PEND);
}
+ if (rsp.result == __constant_cpu_to_le16(L2CAP_CR_SUCCESS) &&
+ chan->local_amp_id) {
+ /* Placeholder - uncomment when amp functions
+ * are available
+ amp_accept_physical(chan, chan->local_amp_id);
+ */
+ l2cap_chan_unlock(chan);
+ continue;
+ }
+
l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP,
- sizeof(rsp), &rsp);
+ sizeof(rsp), &rsp);
if (test_bit(CONF_REQ_SENT, &chan->conf_state) ||
rsp.result != L2CAP_CR_SUCCESS) {
@@ -3308,6 +3348,18 @@ done:
rfc.mode = chan->mode;
}
+ if (test_bit(CONF_LOC_CONF_PEND, &chan->conf_state) &&
+ chan->local_amp_id) {
+ struct hci_chan *hchan = NULL;
+
+ /* Placeholder - get hci_chan for logical link */
+
+ if (hchan && hchan->state == BT_CONNECTED) {
+ l2cap_logical_cfm(chan, hchan,
+ L2CAP_MR_SUCCESS);
+ }
+ }
+
if (result == L2CAP_CONF_SUCCESS)
set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
}
@@ -6282,7 +6334,18 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
if (chan->state == BT_CONNECT) {
if (!status) {
- l2cap_send_conn_req(chan);
+ if (enable_hs &&
+ (conn->fixed_chan_mask & L2CAP_FC_A2MP) &&
+ chan->chan_policy == BT_CHANNEL_POLICY_AMP_PREFERRED) {
+ set_bit(CONF_CONNECT_PEND,
+ &chan->conf_state);
+
+ /* Placeholder
+ amp_create_phylink();
+ */
+ } else {
+ l2cap_send_conn_req(chan);
+ }
} else {
__set_chan_timer(chan, L2CAP_DISC_TIMEOUT);
}
@@ -6302,6 +6365,15 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
if (parent)
parent->sk_data_ready(parent, 0);
} else {
+ if (chan->local_amp_id) {
+ /* Placeholder - accept physical
+ * link
+ amp_accept_physical(chan,
+ chan->local_amp_id);
+ */
+ continue;
+ }
+
__l2cap_state_change(chan, BT_CONFIG);
res = L2CAP_CR_SUCCESS;
stat = L2CAP_CS_NO_INFO;
--
1.7.12
--
Mat Martineau
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [RFCv1 19/20] Bluetooth: Do not retransmit data during a channel move
From: Mat Martineau @ 2012-09-11 18:21 UTC (permalink / raw)
To: linux-bluetooth, gustavo; +Cc: sunnyk
In-Reply-To: <1347387691-5285-1-git-send-email-mathewm@codeaurora.org>
Do not retransmit previously-sent data when a "receiver ready" s-frame
with the "final" flag is received during a move.
The ERTM state machines will resynchronize at the end of a channel
move, and the state machine needs to avoid state changes during a
move.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap_core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index bde1303..35baa27 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5580,8 +5580,9 @@ static int l2cap_rx_state_recv(struct l2cap_chan *chan,
if (control->final) {
clear_bit(CONN_REMOTE_BUSY, &chan->conn_state);
- if (!test_and_clear_bit(CONN_REJ_ACT,
- &chan->conn_state)) {
+ if (!test_and_clear_bit(CONN_REJ_ACT, &chan->conn_state)
+ && (chan->move_state == L2CAP_MOVE_STABLE ||
+ chan->move_state == L2CAP_MOVE_WAIT_PREPARE)) {
control->final = 0;
l2cap_retransmit_all(chan, control);
}
--
1.7.12
--
Mat Martineau
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [RFCv1 20/20] Bluetooth: Start channel move when socket option is changed
From: Mat Martineau @ 2012-09-11 18:21 UTC (permalink / raw)
To: linux-bluetooth, gustavo; +Cc: sunnyk
In-Reply-To: <1347387691-5285-1-git-send-email-mathewm@codeaurora.org>
Channel moves are triggered by changes to the BT_CHANNEL_POLICY
sockopt when an ERTM or streaming-mode channel is connected.
Moves are only started if enable_hs is true.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/l2cap_core.c | 20 ++++++++++++++++++++
net/bluetooth/l2cap_sock.c | 5 +++++
3 files changed, 26 insertions(+)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index cfbde8f..b6a57c8 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -799,5 +799,6 @@ void l2cap_chan_set_defaults(struct l2cap_chan *chan);
int l2cap_ertm_init(struct l2cap_chan *chan);
void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan);
void l2cap_chan_del(struct l2cap_chan *chan, int err);
+void l2cap_move_start(struct l2cap_chan *chan);
#endif /* __L2CAP_H */
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 35baa27..06942a8 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4435,6 +4435,26 @@ static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
}
}
+void l2cap_move_start(struct l2cap_chan *chan)
+{
+ BT_DBG("chan %p", chan);
+
+ if (chan->local_amp_id == 0) {
+ if (chan->chan_policy != BT_CHANNEL_POLICY_AMP_PREFERRED)
+ return;
+ chan->move_role = L2CAP_MOVE_ROLE_INITIATOR;
+ chan->move_state = L2CAP_MOVE_WAIT_PREPARE;
+ /* Placeholder - start physical link setup */
+ } else {
+ chan->move_role = L2CAP_MOVE_ROLE_INITIATOR;
+ chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
+ chan->move_id = 0;
+ l2cap_move_start(chan);
+ l2cap_send_move_chan_req(chan, 0);
+ __set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
+ }
+}
+
static void l2cap_do_create(struct l2cap_chan *chan, int result,
u8 local_amp_id, u8 remote_amp_id)
{
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 13f6a98..f77f5a3 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -725,6 +725,11 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
}
chan->chan_policy = (u8) opt;
+
+ if (sk->sk_state == BT_CONNECTED &&
+ chan->move_role == L2CAP_MOVE_ROLE_NONE)
+ l2cap_move_start(chan);
+
break;
default:
--
1.7.12
--
Mat Martineau
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* Re: [PATCHv3 00/19] Bluetooth: Create AMP physical link
From: Mat Martineau @ 2012-09-11 18:23 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1346933997-14369-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Andrei -
On Thu, 6 Sep 2012, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> This set of patches enhances A2MP protocol and creates physical
> link between AMP controllers. This is further iteration towards
> Bluetooth High Speed.
>
> Changes:
> * p3: Use hci_conn for representing physical link(type AMP_LINK) instead of
> struct phy_link, refactoring.
> * p2: Remove HCI callbacks and use amp_mgr global list, refactor code.
> * p1: Fixed locking issues, added basic logical link preparation.
> * v3: Remove workqueue from callback processing; change callback functions
> names according to reviewers recommendations; create global amp_mgr_list to
> have lookup to amp manager, physical and logical links so for those HCI events
> which might be identified by __handler__ we have lookup; remove extensive
> hexdump from gen_amp_key.
> * v2: Fix typos and bugs, add functionality: now physical connection
> might be established.
> * v1: Fix typos, change debug prints, refactor code for better
> splitting functionality.
>
This doesn't apply to the current bluetooth-next HEAD, could you
rebase?
Patch 18 (AMP: Send Create Chan Req) also conflicts with my signaling
patch set, so it would help if you dropped it for now and added that
functionality after the signaling code is in.
Thanks,
--
Mat Martineau
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH 02/10] battery: Implement Generic device battery
From: Joao Paulo Rechi Vita @ 2012-09-11 18:27 UTC (permalink / raw)
To: chen.ganir; +Cc: linux-bluetooth
In-Reply-To: <1347349100-24228-3-git-send-email-chen.ganir@ti.com>
On Tue, Sep 11, 2012 at 4:38 AM, <chen.ganir@ti.com> wrote:
> From: Chen Ganir <chen.ganir@ti.com>
>
> Add implementation for the generic battery in bluetooth device.
> This patch adds new D-Bus interface for adding/removing/changing
> peer device battery status, allowing management of remote device
> batteries.
> ---
> src/device.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/device.h | 15 +++++
> test/test-device | 13 ++++
> 3 files changed, 212 insertions(+)
>
> diff --git a/src/device.c b/src/device.c
> index 02ef35e..ce4d467 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -166,6 +166,7 @@ struct btd_device {
>
> gboolean authorizing;
> gint ref;
> + GSList *batteries;
>
> GIOChannel *att_io;
> guint cleanup_id;
> @@ -180,6 +181,26 @@ static uint16_t uuid_list[] = {
> 0
> };
>
> +struct device_battery {
> + DBusConnection *conn;
> + uint16_t level;
> + char *path;
> + struct btd_device *device;
> + RefreshBattFunc refresh_func;
> +};
> +
> +static void battery_free(gpointer user_data)
> +{
> + struct device_battery *b = user_data;
> +
> + g_dbus_unregister_interface(b->conn, b->path, BATTERY_INTERFACE);
> + dbus_connection_unref(b->conn);
> + btd_device_unref(b->device);
> + g_free(b->path);
> + g_free(b);
> +
> +}
> +
> static void browse_request_free(struct browse_req *req)
> {
> if (req->listener_id)
> @@ -259,6 +280,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, battery_free);
>
> attio_cleanup(device);
>
> @@ -433,6 +455,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 device_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;
> @@ -3263,3 +3294,156 @@ void btd_profile_disconnected(struct btd_profile *profile,
> {
> device->conns = g_slist_remove(device->conns, profile);
> }
> +
> +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 device_battery *batt = l->data;
> + batteries[i] = batt->path;
> + }
> +
> + emit_array_property_changed(conn, device->path, DEVICE_INTERFACE,
> + "Batteries", DBUS_TYPE_OBJECT_PATH,
> + &batteries, i);
> +
> + g_free(batteries);
> +}
> +
> +static DBusMessage *refresh_batt_level(DBusConnection *conn,
> + DBusMessage *msg, void *data)
> +{
> + struct device_battery *b = data;
> +
> + if (!b->refresh_func)
> + return btd_error_not_supported(msg);
> +
> + b->refresh_func(b);
> +
> + return dbus_message_new_method_return(msg);
> +}
> +
> +static DBusMessage *get_batt_properties(DBusConnection *conn,
> + DBusMessage *msg, void *data)
> +{
> + struct device_battery *b = data;
> + DBusMessageIter iter;
> + DBusMessageIter dict;
> + DBusMessage *reply;
> +
> + reply = dbus_message_new_method_return(msg);
> + if (reply == NULL)
> + return NULL;
> +
> + dbus_message_iter_init_append(reply, &iter);
> +
> + dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> + DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
> + DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
> + DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
> +
> + dict_append_entry(&dict, "Level", DBUS_TYPE_UINT16, &b->level);
> +
> + dbus_message_iter_close_container(&iter, &dict);
> +
> + return reply;
> +}
> +
> +static GDBusMethodTable battery_methods[] = {
> + { GDBUS_METHOD("GetProperties",
> + NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
> + get_batt_properties) },
> + { GDBUS_METHOD("Refresh",
> + NULL, NULL,
> + refresh_batt_level) },
> + { }
> +};
> +
> +static GDBusSignalTable battery_signals[] = {
> + { GDBUS_SIGNAL("PropertyChanged",
> + GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
> + { }
> +};
> +
> +struct device_battery *btd_device_add_battery(struct btd_device *device)
> +{
> + struct device_battery *batt;
> + DBusConnection *conn = get_dbus_connection();
> +
> + batt = g_new0(struct device_battery, 1);
> + batt->path = g_strdup_printf("%s/BATT%04X", device->path,
> + g_slist_length(device->batteries));
> + batt->conn = dbus_connection_ref(conn);
> +
> + if (!g_dbus_register_interface(batt->conn, batt->path,
> + BATTERY_INTERFACE, battery_methods, battery_signals,
> + NULL, batt, NULL)) {
> + error("D-Bus register interface %s failed", BATTERY_INTERFACE);
> + dbus_connection_unref(batt->conn);
> + g_free(batt->path);
> + g_free(batt);
> + return NULL;
> + }
> +
> + batt->device = btd_device_ref(device);
> + device->batteries = g_slist_append(device->batteries, batt);
> + batteries_changed(device);
> +
> + return batt;
> +}
> +
> +void btd_device_remove_battery(struct device_battery *batt)
> +{
> + struct btd_device *dev = batt->device;
> +
> + dev->batteries = g_slist_remove(dev->batteries, batt);
> +
> + battery_free(batt);
> +
> + batteries_changed(dev);
> +}
> +
> +gboolean btd_device_set_battery_opt(struct device_battery *batt,
> + BatteryOption opt1, ...)
> +{
> + va_list args;
> + BatteryOption opt = opt1;
> + int level;
> +
> + if (!batt)
> + return FALSE;
> +
> + va_start(args, opt1);
> +
> + while (opt != BATTERY_OPT_INVALID) {
> + switch (opt) {
> + case BATTERY_OPT_LEVEL:
> + level = va_arg(args, int);
> + if (level != batt->level) {
> + batt->level = level;
> + emit_property_changed(batt->conn, batt->path,
> + BATTERY_INTERFACE, "Level",
> + DBUS_TYPE_UINT16, &level);
> + }
> + break;
> + case BATTERY_OPT_REFRESH_FUNC:
> + batt->refresh_func = va_arg(args, RefreshBattFunc);
> + break;
> + default:
> + error("Unknown option %d", opt);
> + return FALSE;
> + }
> +
> + opt = va_arg(args, int);
> + }
> +
> + va_end(args);
> +
> + return TRUE;
> +
> +}
Please remove this empty line also.
> diff --git a/src/device.h b/src/device.h
> index f1d95c6..66eb5e8 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -23,8 +23,10 @@
> */
>
> #define DEVICE_INTERFACE "org.bluez.Device"
> +#define BATTERY_INTERFACE "org.bluez.Battery"
>
> struct btd_device;
> +struct device_battery;
Since this symbol is being exported, shouldn't it be prefixed with btd_ as well?
>
> typedef enum {
> AUTH_TYPE_PINCODE,
> @@ -54,6 +56,14 @@ struct btd_profile {
> void (*adapter_remove) (struct btd_adapter *adapter);
> };
>
> +typedef void (*RefreshBattFunc) (struct device_battery *batt);
I don't think we use CamelCase for anything other than D-Bus method names.
> +
> +typedef enum {
> + BATTERY_OPT_INVALID = 0,
> + BATTERY_OPT_LEVEL,
> + BATTERY_OPT_REFRESH_FUNC,
> +} BatteryOption;
Fix CamelCase usage here and on uses of this type as well.
> +
> void btd_profile_foreach(void (*func)(struct btd_profile *p, void *data),
> void *data);
>
> @@ -158,3 +168,8 @@ 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);
> +
> +struct device_battery *btd_device_add_battery(struct btd_device *device);
> +void btd_device_remove_battery(struct device_battery *batt);
> +gboolean btd_device_set_battery_opt(struct device_battery *batt,
> + BatteryOption opt1, ...);
> diff --git a/test/test-device b/test/test-device
> index 63a96d3..7edb7b8 100755
> --- a/test/test-device
> +++ b/test/test-device
> @@ -37,6 +37,7 @@ if (len(args) < 1):
> print("")
> print(" list")
> print(" services <address>")
> + print(" batteries <address>")
> print(" create <address>")
> print(" remove <address|path>")
> print(" disconnect <address>")
> @@ -205,5 +206,17 @@ if (args[0] == "services"):
> print(path)
> sys.exit(0)
>
> +if (args[0] == "batteries"):
> + if (len(args) < 2):
> + print("Need address parameter")
> + else:
> + path = adapter.FindDevice(args[1])
> + device = dbus.Interface(bus.get_object("org.bluez", path),
> + "org.bluez.Device")
> + properties = device.GetProperties()
> + for path in properties["Batteries"]:
> + print(path)
> + sys.exit(0)
> +
> print("Unknown command")
> sys.exit(1)
> --
> 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
* Agenda (v2) for Linux Wireless (802.11/Bluetooth/NFC) Mini-Summit 2012 (Barcelona!)
From: John W. Linville @ 2012-09-11 20:17 UTC (permalink / raw)
To: linux-wireless, linux-bluetooth, alexander.smirnov
OK, making progress...still, we need some more discussion topics...
We have one room on the first day, and three rooms on the second day.
I figure that the Bluetooth and 802.11 groups should each get a
break-out room for their own in-depth discussons on the second day and
the third room can be used for more general topics and presentations.
Below is my strawman agenda proposal. Please make suggestions for
improvements!
Day 1
-----
Everyone together in a single room...
Time Topic Speaker
---- ----- -------
0930 Welcome/Intro/Agenda John Linville
1000 802.15.4 stack, 6lowpan Alexander Smirnov
1100 Break
1130 Bluetooth: state of the union Marcel Holtmann
1230 Lunch
1400 NFC "state of the union", etc. Ortiz / Elias / Chlad
1500 Break
...
1800 Adjourn
Day 2
-----
802.11 break-out room...
Time Topic Speaker
---- ----- -------
1000 Regulatory updates Luis Rodriguez
1100 Break
1130 Virtual Bluetooth AMP Controller for mac80211 Andrei Emeltchenko
1230 Lunch
1400 IEEE802.11/WFA Update & HotSpot 2.0 Jouni Malinen
...
1800 Adjourn
Bluetooth break-out room...
Time Topic Speaker
---- ----- -------
1000 Bluetooth Broadcaster/Observer API Lizardo & Delfes
1100 Break
1130 status and roadmap of Bluetooth LE Joao Paulo Rechi Vita
1230 Lunch
...
1800 Adjourn
Presentation room...
Time Topic Speaker
---- ----- -------
1000 wireless community networks Nick Kossifidis
1100 Break
1130 "a short overview what we do in ConnMan" Wagner / Flykt
1230 Lunch
1400 oFono / mobile wireless Denis Kenzior
1500 Break
...
1800 Adjourn
So...suggestions??
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH] Add support for Logitech Harmony Adapter for PS3
From: Bastien Nocera @ 2012-09-11 20:17 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: David Dillow, linux-bluetooth
In-Reply-To: <CABBYNZKggzJ7bzQWVmWi30kxOUR8bVCyMLL=SM1=Oe74YPwvKg@mail.gmail.com>
Em Tue, 2012-09-11 às 17:21 +0300, Luiz Augusto von Dentz escreveu:
> Hi Bastien,
>
> On Mon, Sep 10, 2012 at 10:03 PM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > Hi Bastien,
> >
> > On Mon, Sep 10, 2012 at 4:58 PM, Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> >> Hi Bastien,
> >>
> >> On Mon, Sep 10, 2012 at 4:47 PM, Bastien Nocera <hadess@hadess.net> wrote:
> >>> Em Mon, 2012-09-10 às 16:11 +0300, Luiz Augusto von Dentz escreveu:
> >>>> Hi David, Bastien,
> >>>>
> >>>> So we are plannin to rid of the fakehid.c in favor of implementing it
> >>>> properly inside the kernel similarly to what was done to wiimote, so
> >>>> is there any obstacle for doing that?
> >>>>
> >>>> The kernel seems to already have some support for sixaxis in
> >>>> drivers/hid/hid-sony.c, so I suppose the following would enable us to
> >>>> use it:
> >>>
> >>> It won't. They're not the same hardware.
> >>
> >> What hardware is that then? And why wouldn't the kernel be able to
> >> support even if it is a different driver?
> >
> > So what exactly are the difference between 0x0268 and 0x0306? And why
> > sixpair.c save as 0x0268 while fakeinput.c uses 0x0306?
> >
> > Also, after fixing sixpair.c to be able to compile it does add to the
> > storage but it does not create any object until bluetoothd is
> > restarted and even after restart it does not allow the device to
> > connect because it does has the record (although this can be fixed by
> > automatically add the UUID once we find out it is attempt to connect).
>
> So apparently the 0x0306 device is the br remote controller, not the
> sixaxis joystick, sorry about the confusion.
Completely different protocols for the devices, indeed.
> Anyway regardless of
> being a different device I thing we should move this code to kernel as
> it was done for wiimote.
Certainly, but until somebody writes the code (I already have at least 3
drivers to submit to the kernel that I've not had time to handle,
include one Bluetooth device), I think it would be nice not to drop
support for the existing hardware. The cost of David's patch is close to
none.
There's also a bunch of features that I'm not sure how you'd handle,
like forcing a disconnect timeout for the device (a patch which Johan
merged a couple of days ago).
Cheers
^ permalink raw reply
* Re: [RFCv1 01/15] Bluetooth: Set Exended Flowspec flag for HS chan
From: Mat Martineau @ 2012-09-11 20:23 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <20120911081506.GB2597@aemeltch-MOBL1>
Andrei -
On Tue, 11 Sep 2012, Andrei Emeltchenko wrote:
> Hi Mat,
>
> On Mon, Sep 10, 2012 at 04:49:36PM -0700, Mat Martineau wrote:
>>
>> Andrei -
>>
>> On Tue, 4 Sep 2012, Andrei Emeltchenko wrote:
>>
>>> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>>>
>>> For AMP we always assume EFS L2CAP option.
>>>
>>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>>> ---
>>> include/net/bluetooth/l2cap.h | 2 ++
>>> net/bluetooth/l2cap_core.c | 7 ++++++-
>>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>>> index 161be83..3ec8679 100644
>>> --- a/include/net/bluetooth/l2cap.h
>>> +++ b/include/net/bluetooth/l2cap.h
>>> @@ -508,6 +508,8 @@ struct l2cap_chan {
>>> __u32 remote_acc_lat;
>>> __u32 remote_flush_to;
>>>
>>> + __u8 ctrl_id;
>>> +
>>
>> How about "remote_amp_id" instead of ctrl_id? The channel move code
>> tracks local_amp_id.
>> Why does the remote controller id need to be
>> tracked?
>
> I think you are right, I think it shall be local AMP controller id. So we
> can keep this name ctrl_id for local one or do you think local_amp_id is
> better?
local_amp_id seems better to me. I used that in my patch set posted
today (patch 01/20).
--
Mat Martineau
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH 05/10] battery: Discover Characteristic Descriptors
From: Joao Paulo Rechi Vita @ 2012-09-11 20:52 UTC (permalink / raw)
To: chen.ganir; +Cc: linux-bluetooth
In-Reply-To: <1347349100-24228-6-git-send-email-chen.ganir@ti.com>
On Tue, Sep 11, 2012 at 4:38 AM, <chen.ganir@ti.com> wrote:
> From: Chen Ganir <chen.ganir@ti.com>
>
> Discover all characteristic descriptors, and build a descriptor
> list. Presentation Format Descriptor and Client Characteristic
> Configuration descriptors are searched.
> ---
> profiles/battery/battery.c | 73 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> index 37cbce4..3a24a7b 100644
> --- a/profiles/battery/battery.c
> +++ b/profiles/battery/battery.c
> @@ -51,6 +51,13 @@ static GSList *servers;
> struct characteristic {
> struct gatt_char attr; /* Characteristic */
> struct battery *batt; /* Parent Battery Service */
> + GSList *desc; /* Descriptors */
Wrong alignment here.
> +};
> +
> +struct descriptor {
> + struct characteristic *ch; /* Parent Characteristic */
> + uint16_t handle; /* Descriptor Handle */
> + bt_uuid_t uuid; /* UUID */
> };
>
> static gint cmp_device(gconstpointer a, gconstpointer b)
> @@ -64,12 +71,21 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
> return -1;
> }
>
> +static void char_free(gpointer user_data)
> +{
> + struct characteristic *c = user_data;
> +
> + g_slist_free_full(c->desc, g_free);
> +
> + g_free(c);
> +}
> +
> static void battery_free(gpointer user_data)
> {
> struct battery *batt = user_data;
>
> if (batt->chars != NULL)
> - g_slist_free_full(batt->chars, g_free);
> + g_slist_free_full(batt->chars, char_free);
>
> if (batt->attioid > 0)
> btd_device_remove_attio_callback(batt->dev, batt->attioid);
> @@ -82,7 +98,47 @@ static void battery_free(gpointer user_data)
> g_free(batt);
> }
>
> +static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
> + gpointer user_data)
> +{
> + struct characteristic *ch = user_data;
> + struct att_data_list *list;
> + uint8_t format;
> + int i;
> +
> + if (status != 0) {
> + error("Discover all characteristic descriptors failed [%s]: %s",
> + ch->attr.uuid, att_ecode2str(status));
> + return;
> + }
> +
> + list = dec_find_info_resp(pdu, len, &format);
> + if (list == NULL)
> + return;
> +
> + for (i = 0; i < list->num; i++) {
> + struct descriptor *desc;
> + uint8_t *value;
> +
> + value = list->data[i];
> + desc = g_new0(struct descriptor, 1);
> + desc->handle = att_get_u16(value);
> + desc->ch = ch;
> +
> + if (format == 0x01)
> + desc->uuid = att_get_uuid16(&value[2]);
> + else
> + desc->uuid = att_get_uuid128(&value[2]);
> +
> + ch->desc = g_slist_append(ch->desc, desc);
> + }
> +
> + att_data_list_free(list);
> +}
> +
> +
Remove extra blank line here.
> static void configure_battery_cb(GSList *characteristics, guint8 status,
> +
Remove extra blank line here.
> gpointer user_data)
> {
> struct battery *batt = user_data;
> @@ -97,6 +153,7 @@ static void configure_battery_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;
> @@ -106,6 +163,20 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
> 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;
> +
> + 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
* Re: [PATCH 07/10] battery: Add Battery to device
From: Joao Paulo Rechi Vita @ 2012-09-11 21:40 UTC (permalink / raw)
To: chen.ganir; +Cc: linux-bluetooth
In-Reply-To: <1347349100-24228-8-git-send-email-chen.ganir@ti.com>
On Tue, Sep 11, 2012 at 4:38 AM, <chen.ganir@ti.com> wrote:
> From: Chen Ganir <chen.ganir@ti.com>
>
> Add/Remove battery from device
> ---
> profiles/battery/battery.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> index d1e0b6e..31f2297 100644
> --- a/profiles/battery/battery.c
> +++ b/profiles/battery/battery.c
> @@ -49,8 +49,9 @@ struct battery {
> static GSList *servers;
>
> struct characteristic {
> - struct gatt_char attr; /* Characteristic */
> - struct battery *batt; /* Parent Battery Service */
> + struct device_battery *devbatt; /* device_battery pointer */
> + struct gatt_char attr; /* Characteristic */
> + struct battery *batt; /* Parent Battery Service */
Just a minor pick here: the comment alignment of attr and batt should
have been fixed on the previous commit, to keep it consistent along
the series.
> GSList *desc; /* Descriptors */
> uint8_t ns; /* Battery Namespace */
> uint16_t description; /* Battery description */
> @@ -79,6 +80,8 @@ static void char_free(gpointer user_data)
>
> g_slist_free_full(c->desc, g_free);
>
> + btd_device_remove_battery(c->devbatt);
> +
> g_free(c);
> }
>
> @@ -160,12 +163,12 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
> if (status != 0) {
> error("Discover all characteristic descriptors failed [%s]: %s",
> ch->attr.uuid, att_ecode2str(status));
> - return;
> + goto update_char;
> }
>
> list = dec_find_info_resp(pdu, len, &format);
> if (list == NULL)
> - return;
> + goto update_char;
>
> for (i = 0; i < list->num; i++) {
> struct descriptor *desc;
> @@ -186,6 +189,9 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
> }
>
> att_data_list_free(list);
> +
> +update_char:
> + ch->devbatt = btd_device_add_battery(ch->batt->dev);
If I understood correctly, adding a battery to the device is a
consequence of finding the "Battery Level" characteristic, and not
related to the descriptors discovery. So the call to
btd_device_add_battery() should be done inside the
configure_battery_cb() instead of the discover_desc_cb(). This way
you'll not need all these goto's.
> }
>
>
> --
> 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 08/10] battery: Read Battery level characteristic
From: Joao Paulo Rechi Vita @ 2012-09-11 21:50 UTC (permalink / raw)
To: chen.ganir; +Cc: linux-bluetooth
In-Reply-To: <1347349100-24228-9-git-send-email-chen.ganir@ti.com>
On Tue, Sep 11, 2012 at 4:38 AM, <chen.ganir@ti.com> wrote:
> From: Chen Ganir <chen.ganir@ti.com>
>
> Implement support for reading the battery level characteristic on
> connection establishment.
> ---
> profiles/battery/battery.c | 87 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 85 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> index 31f2297..a93b352 100644
> --- a/profiles/battery/battery.c
> +++ b/profiles/battery/battery.c
> @@ -55,6 +55,7 @@ struct characteristic {
> GSList *desc; /* Descriptors */
> uint8_t ns; /* Battery Namespace */
> uint16_t description; /* Battery description */
> + uint8_t level;
Alignment problem here.
> };
>
> struct descriptor {
> @@ -103,6 +104,80 @@ static void battery_free(gpointer user_data)
> g_free(batt);
> }
>
> +static void read_batterylevel_cb(guint8 status, const guint8 *pdu, guint16 len,
> + gpointer user_data)
> +{
> + struct characteristic *ch = user_data;
> + uint8_t value[ATT_MAX_MTU];
> + int vlen;
> +
> + if (status != 0) {
> + error("Failed to read Battery Level:%s", att_ecode2str(status));
> + return;
> + }
> +
> + vlen = dec_read_resp(pdu, len, value, sizeof(value));
> + if (!vlen) {
> + error("Failed to read Battery Level: Protocol error\n");
> + return;
> + }
> +
> + if (vlen < 1) {
> + error("Failed to read Battery Level: Wrong pdu len");
> + return;
> + }
> +
> + ch->level = value[0];
> + btd_device_set_battery_opt(ch->devbatt, BATTERY_OPT_LEVEL, ch->level,
> + BATTERY_OPT_INVALID);
> +}
> +
> +static void process_batteryservice_char(struct characteristic *ch)
> +{
> + if (g_strcmp0(ch->attr.uuid, BATTERY_LEVEL_UUID) == 0) {
> + gatt_read_char(ch->batt->attrib, ch->attr.value_handle, 0,
> + read_batterylevel_cb, ch);
> + }
> +}
> +
> +static gint device_battery_cmp(gconstpointer a, gconstpointer b)
> +{
> + const struct characteristic *ch = a;
> + const struct device_battery *batt = b;
> +
> + if (batt == ch->devbatt)
> + return 0;
> +
> + return -1;
> +}
> +
> +static struct characteristic *find_battery_char(struct device_battery *db)
> +{
> + GSList *l, *b;
> +
> + for (l = servers; l != NULL; l = g_slist_next(l)) {
> + struct battery *batt = l->data;
> +
> + b = g_slist_find_custom(batt->chars, db, device_battery_cmp);
> + if (!b)
> + return NULL;
> +
> + return b->data;
> + }
> +
> + return NULL;
> +}
> +
> +static void batterylevel_refresh_cb(struct device_battery *batt)
> +{
> + struct characteristic *ch;
> +
> + ch = find_battery_char(batt);
> +
> + if (ch)
> + process_batteryservice_char(ch);
> +}
> +
> static void batterylevel_presentation_format_desc_cb(guint8 status,
> const guint8 *pdu, guint16 len,
> gpointer user_data)
> @@ -151,7 +226,6 @@ static void process_batterylevel_desc(struct descriptor *desc)
> 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)
> {
> @@ -192,8 +266,11 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>
> update_char:
> ch->devbatt = btd_device_add_battery(ch->batt->dev);
> -}
> + btd_device_set_battery_opt(ch->devbatt, BATTERY_OPT_REFRESH_FUNC,
> + batterylevel_refresh_cb, BATTERY_OPT_INVALID);
>
> + process_batteryservice_char(ch);
There two call also should be part of configure_battery_cb() instead
of discover_desc_cb().
> +}
>
> static void configure_battery_cb(GSList *characteristics, guint8 status,
>
> @@ -252,6 +329,12 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
> gatt_discover_char(batt->attrib, batt->svc_range->start,
> batt->svc_range->end, NULL,
> configure_battery_cb, batt);
> + } else {
> + GSList *l;
> + for (l = batt->chars; l; l = l->next) {
> + struct characteristic *c = l->data;
> + process_batteryservice_char(c);
> + }
> }
> }
>
> --
> 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: Agenda (v2) for Linux Wireless (802.11/Bluetooth/NFC) Mini-Summit 2012 (Barcelona!)
From: G.W. Haywood @ 2012-09-11 21:51 UTC (permalink / raw)
To: John W. Linville; +Cc: linux-wireless, linux-bluetooth, alexander.smirnov
In-Reply-To: <20120911201729.GJ7015@tuxdriver.com>
Hi there,
On Tue, 11 Sep 2012, John W. Linville wrote:
> So...suggestions??
Dates?
--
73,
Ged.
^ permalink raw reply
* Re: [PATCH 09/10] battery: Add support for notifications
From: Joao Paulo Rechi Vita @ 2012-09-11 22:08 UTC (permalink / raw)
To: chen.ganir; +Cc: linux-bluetooth
In-Reply-To: <1347349100-24228-10-git-send-email-chen.ganir@ti.com>
On Tue, Sep 11, 2012 at 4:38 AM, <chen.ganir@ti.com> wrote:
> From: Chen Ganir <chen.ganir@ti.com>
>
> Add support for emitting PropertyChanged when a battery level
> characteristic notification is sent from the peer device.
> ---
> profiles/battery/battery.c | 95 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 93 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> index a93b352..95f548e 100644
> --- a/profiles/battery/battery.c
> +++ b/profiles/battery/battery.c
> @@ -43,6 +43,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 */
> };
>
> @@ -56,6 +57,7 @@ struct characteristic {
> uint8_t ns; /* Battery Namespace */
> uint16_t description; /* Battery description */
> uint8_t level;
> + gboolean canNotify;
> };
>
> struct descriptor {
> @@ -86,6 +88,14 @@ static void char_free(gpointer user_data)
> g_free(c);
> }
>
> +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 battery_free(gpointer user_data)
> {
> struct battery *batt = user_data;
> @@ -99,6 +109,10 @@ static void battery_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);
> + }
You're using the same condition twice here (batt->attrib != NULL).
Looks like a copy & paste error. Make sure you check for attnotid
before checking attrib, and that you don't unref attrib twice.
> btd_device_unref(batt->dev);
> g_free(batt->svc_range);
> g_free(batt);
> @@ -140,6 +154,18 @@ static void process_batteryservice_char(struct characteristic *ch)
> }
> }
>
> +static void batterylevel_enable_notify_cb(guint8 status, const guint8 *pdu,
> + guint16 len, gpointer user_data)
> +{
> + struct characteristic *ch = (struct characteristic *)user_data;
> +
> + if (status != 0) {
> + error("Could not enable batt level notification.");
> + ch->canNotify = FALSE;
> + process_batteryservice_char(ch);
I don't think is necessary to call process_batteryservice_char() here,
since you already called it during the characteristics discovery.
> + }
> +}
> +
> static gint device_battery_cmp(gconstpointer a, gconstpointer b)
> {
> const struct characteristic *ch = a;
> @@ -178,6 +204,19 @@ static void batterylevel_refresh_cb(struct device_battery *batt)
> process_batteryservice_char(ch);
> }
>
> +static void enable_battery_notification(struct characteristic *ch,
> + uint16_t handle)
> +{
> + uint8_t atval[2];
> + uint16_t val;
> +
> + val = GATT_CLIENT_CHARAC_CFG_NOTIF_BIT;
> +
> + att_put_u16(val, atval);
> + gatt_write_char(ch->batt->attrib, handle, atval, 2,
> + batterylevel_enable_notify_cb, ch);
> +}
> +
> static void batterylevel_presentation_format_desc_cb(guint8 status,
> const guint8 *pdu, guint16 len,
> gpointer user_data)
> @@ -207,13 +246,20 @@ static void batterylevel_presentation_format_desc_cb(guint8 status,
> 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_CLIENT_CHARAC_CFG_UUID);
> +
> + if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0 && g_strcmp0(ch->attr.uuid,
> + BATTERY_LEVEL_UUID) == 0) {
> + enable_battery_notification(ch, desc->handle);
> + return;
> + }
> +
> bt_uuid16_create(&btuuid, GATT_CHARAC_FMT_UUID);
>
> if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0) {
> @@ -319,12 +365,54 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
> }
> }
>
> +static void proc_batterylevel(struct characteristic *c, const uint8_t *pdu,
> + uint16_t len, gboolean final)
> +{
> + if (!pdu) {
> + error("Battery level notification: Invalid pdu length");
> + return;
> + }
> +
> + c->level = pdu[1];
> +
> + btd_device_set_battery_opt(c->devbatt, BATTERY_OPT_LEVEL, c->level,
> + BATTERY_OPT_INVALID);
> +}
> +
> +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;
> +
> + 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,
> @@ -333,7 +421,8 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
> GSList *l;
> for (l = batt->chars; l; l = l->next) {
> struct characteristic *c = l->data;
> - process_batteryservice_char(c);
> + if (!c->canNotify)
> + process_batteryservice_char(c);
> }
> }
> }
> @@ -342,6 +431,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: Agenda (v2) for Linux Wireless (802.11/Bluetooth/NFC) Mini-Summit 2012 (Barcelona!)
From: John W. Linville @ 2012-09-12 1:10 UTC (permalink / raw)
To: G.W. Haywood; +Cc: linux-wireless, linux-bluetooth, alexander.smirnov
In-Reply-To: <Pine.LNX.4.64.1209112250460.8356@mail5.jubileegroup.co.uk>
On Tue, Sep 11, 2012 at 10:51:43PM +0100, G.W. Haywood wrote:
> Hi there,
>
> On Tue, 11 Sep 2012, John W. Linville wrote:
>
> >So...suggestions??
>
> Dates?
Sorry, I guess I was talking to the choir... :-)
This event is on 8-9 November 2012, just after LinuxCon Europe
in Barcelona. Anyone attending needs to also register to attend
LinuxCon Europe:
http://events.linuxfoundation.org/events/linuxcon-europe
Thanks,
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH BlueZ v3 06/15] gdbus: Implement DBus.Properties.Set method
From: Lucas De Marchi @ 2012-09-12 3:34 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1346961700.21200.116.camel@aeonflux>
Hi Marcel,
On Thu, Sep 6, 2012 at 5:01 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Lucas,
>
>> Contrary to Get() and GetAll(), Set is asynchronous so we pass on the
>> DBusMessage so user is able to create the response. It's the only use of
>> this parameter.
>> ---
>> gdbus/gdbus.h | 7 +++++++
>> gdbus/object.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>> 2 files changed, 64 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
>> index b2e78c4..3e4aa16 100644
>> --- a/gdbus/gdbus.h
>> +++ b/gdbus/gdbus.h
>> @@ -31,6 +31,8 @@ extern "C" {
>> #include <dbus/dbus.h>
>> #include <glib.h>
>>
>> +typedef enum GDBusPropertySetReturn GDBusPropertySetReturn;
>> +
>> typedef enum GDBusMethodFlags GDBusMethodFlags;
>> typedef enum GDBusSignalFlags GDBusSignalFlags;
>> typedef enum GDBusPropertyFlags GDBusPropertyFlags;
>> @@ -69,6 +71,10 @@ typedef DBusMessage * (* GDBusMethodFunction) (DBusConnection *connection,
>> typedef gboolean (*GDBusPropertyGetter)(const GDBusPropertyTable *property,
>> DBusMessageIter *iter, void *data);
>>
>> +typedef DBusMessage *(*GDBusPropertySetter)(const GDBusPropertyTable *property,
>> + DBusMessageIter *value,
>> + DBusMessage *msg, void *data);
>> +
>
> I am not really happy with this. We just need a unique handle here since
> the return value is either success or an error. I rather don't send
> messages around for no other reason to create the error.
>
> Inside the authorization code I am using GDBusPendingReply as unique
> token. What about using the same approach?
Sounds reasonable... I'll send an update on the next days.
Thanks
Lucas De Marchi
^ permalink raw reply
* Re: [PATCH 01/10] battery: Add generic device battery documentation
From: Chen Ganir @ 2012-09-12 4:45 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <CAAngNMaSq2e7XXdLWOSbegOcgmqFaLzGZmGivRn9ERo-YL57JQ@mail.gmail.com>
Joao,
On 09/11/2012 06:34 PM, Joao Paulo Rechi Vita wrote:
> Hello Chen,
>
> On Tue, Sep 11, 2012 at 4:38 AM, <chen.ganir@ti.com> wrote:
>> From: Chen Ganir <chen.ganir@ti.com>
>>
>> Add documentation for the generic battery D-Bus interface.
>> ---
>> doc/battery-api.txt | 33 +++++++++++++++++++++++++++++++++
>> doc/device-api.txt | 5 +++++
>> 2 files changed, 38 insertions(+)
>> create mode 100644 doc/battery-api.txt
>>
>> diff --git a/doc/battery-api.txt b/doc/battery-api.txt
>> new file mode 100644
>> index 0000000..d5ef5ed
>> --- /dev/null
>> +++ b/doc/battery-api.txt
>> @@ -0,0 +1,33 @@
>> +BlueZ D-Bus Battery API description
>> +****************************************
>> +
>> + Texas Instruments, Inc. <chen.ganir@ti.com>
>> +
>> +Device Battery hierarchy
>> +=====================================
>> +
>> +Service org.bluez
>> +Interface org.bluez.Battery
>> +Object path [variable prefix]/{hci0,..}/dev_XX_XX_XX_XX_XX_XX/BATTYYYY
>> +YYYY is numeric value between 0 and 9999.
>> +
>> +Methods dict GetProperties()
>> +
>> + Returns all properties for the interface. See the
>> + Properties section for the available properties.
>> +
>> + void Refresh()
>> +
>> + Refresh the batterty level. If the battery level changed, the
>> + PropertyChanged signal will be sent with the new value.
>> +
>> +Signals PropertyChanged(string name, variant value)
>> +
>> + This signal indicates a changed value of the given
>> + property.
>> +
>> +Properties uint16 Level [readonly]
>> +
>> + Battery level (0-100).
>> +
>> +
>
> Can you please remove these two blank lines on the end of the file?
>
Ok.
>> diff --git a/doc/device-api.txt b/doc/device-api.txt
>> index 1f0dc96..c98d539 100644
>> --- a/doc/device-api.txt
>> +++ b/doc/device-api.txt
>> @@ -179,3 +179,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{object} Batteries [readonly]
>> +
>> + List of device battery object paths that represents the available
>> + batteries on the remote 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 for the review.
--
BR,
Chen Ganir
^ permalink raw reply
* Re: [PATCH 02/10] battery: Implement Generic device battery
From: Chen Ganir @ 2012-09-12 4:48 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <CAAngNMbUQzxAdwo2JUjzrqkLGXFhpyRVqtz03u+3t3nfXcyxpg@mail.gmail.com>
Joao,
On 09/11/2012 09:27 PM, Joao Paulo Rechi Vita wrote:
> On Tue, Sep 11, 2012 at 4:38 AM, <chen.ganir@ti.com> wrote:
>> From: Chen Ganir <chen.ganir@ti.com>
>>
>> Add implementation for the generic battery in bluetooth device.
>> This patch adds new D-Bus interface for adding/removing/changing
>> peer device battery status, allowing management of remote device
>> batteries.
>> ---
>> src/device.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/device.h | 15 +++++
>> test/test-device | 13 ++++
>> 3 files changed, 212 insertions(+)
>>
>> diff --git a/src/device.c b/src/device.c
>> index 02ef35e..ce4d467 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -166,6 +166,7 @@ struct btd_device {
>>
>> gboolean authorizing;
>> gint ref;
>> + GSList *batteries;
>>
>> GIOChannel *att_io;
>> guint cleanup_id;
>> @@ -180,6 +181,26 @@ static uint16_t uuid_list[] = {
>> 0
>> };
>>
>> +struct device_battery {
>> + DBusConnection *conn;
>> + uint16_t level;
>> + char *path;
>> + struct btd_device *device;
>> + RefreshBattFunc refresh_func;
>> +};
>> +
>> +static void battery_free(gpointer user_data)
>> +{
>> + struct device_battery *b = user_data;
>> +
>> + g_dbus_unregister_interface(b->conn, b->path, BATTERY_INTERFACE);
>> + dbus_connection_unref(b->conn);
>> + btd_device_unref(b->device);
>> + g_free(b->path);
>> + g_free(b);
>> +
>> +}
>> +
>> static void browse_request_free(struct browse_req *req)
>> {
>> if (req->listener_id)
>> @@ -259,6 +280,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, battery_free);
>>
>> attio_cleanup(device);
>>
>> @@ -433,6 +455,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 device_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;
>> @@ -3263,3 +3294,156 @@ void btd_profile_disconnected(struct btd_profile *profile,
>> {
>> device->conns = g_slist_remove(device->conns, profile);
>> }
>> +
>> +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 device_battery *batt = l->data;
>> + batteries[i] = batt->path;
>> + }
>> +
>> + emit_array_property_changed(conn, device->path, DEVICE_INTERFACE,
>> + "Batteries", DBUS_TYPE_OBJECT_PATH,
>> + &batteries, i);
>> +
>> + g_free(batteries);
>> +}
>> +
>> +static DBusMessage *refresh_batt_level(DBusConnection *conn,
>> + DBusMessage *msg, void *data)
>> +{
>> + struct device_battery *b = data;
>> +
>> + if (!b->refresh_func)
>> + return btd_error_not_supported(msg);
>> +
>> + b->refresh_func(b);
>> +
>> + return dbus_message_new_method_return(msg);
>> +}
>> +
>> +static DBusMessage *get_batt_properties(DBusConnection *conn,
>> + DBusMessage *msg, void *data)
>> +{
>> + struct device_battery *b = data;
>> + DBusMessageIter iter;
>> + DBusMessageIter dict;
>> + DBusMessage *reply;
>> +
>> + reply = dbus_message_new_method_return(msg);
>> + if (reply == NULL)
>> + return NULL;
>> +
>> + dbus_message_iter_init_append(reply, &iter);
>> +
>> + dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
>> + DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
>> + DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
>> + DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
>> +
>> + dict_append_entry(&dict, "Level", DBUS_TYPE_UINT16, &b->level);
>> +
>> + dbus_message_iter_close_container(&iter, &dict);
>> +
>> + return reply;
>> +}
>> +
>> +static GDBusMethodTable battery_methods[] = {
>> + { GDBUS_METHOD("GetProperties",
>> + NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
>> + get_batt_properties) },
>> + { GDBUS_METHOD("Refresh",
>> + NULL, NULL,
>> + refresh_batt_level) },
>> + { }
>> +};
>> +
>> +static GDBusSignalTable battery_signals[] = {
>> + { GDBUS_SIGNAL("PropertyChanged",
>> + GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
>> + { }
>> +};
>> +
>> +struct device_battery *btd_device_add_battery(struct btd_device *device)
>> +{
>> + struct device_battery *batt;
>> + DBusConnection *conn = get_dbus_connection();
>> +
>> + batt = g_new0(struct device_battery, 1);
>> + batt->path = g_strdup_printf("%s/BATT%04X", device->path,
>> + g_slist_length(device->batteries));
>> + batt->conn = dbus_connection_ref(conn);
>> +
>> + if (!g_dbus_register_interface(batt->conn, batt->path,
>> + BATTERY_INTERFACE, battery_methods, battery_signals,
>> + NULL, batt, NULL)) {
>> + error("D-Bus register interface %s failed", BATTERY_INTERFACE);
>> + dbus_connection_unref(batt->conn);
>> + g_free(batt->path);
>> + g_free(batt);
>> + return NULL;
>> + }
>> +
>> + batt->device = btd_device_ref(device);
>> + device->batteries = g_slist_append(device->batteries, batt);
>> + batteries_changed(device);
>> +
>> + return batt;
>> +}
>> +
>> +void btd_device_remove_battery(struct device_battery *batt)
>> +{
>> + struct btd_device *dev = batt->device;
>> +
>> + dev->batteries = g_slist_remove(dev->batteries, batt);
>> +
>> + battery_free(batt);
>> +
>> + batteries_changed(dev);
>> +}
>> +
>> +gboolean btd_device_set_battery_opt(struct device_battery *batt,
>> + BatteryOption opt1, ...)
>> +{
>> + va_list args;
>> + BatteryOption opt = opt1;
>> + int level;
>> +
>> + if (!batt)
>> + return FALSE;
>> +
>> + va_start(args, opt1);
>> +
>> + while (opt != BATTERY_OPT_INVALID) {
>> + switch (opt) {
>> + case BATTERY_OPT_LEVEL:
>> + level = va_arg(args, int);
>> + if (level != batt->level) {
>> + batt->level = level;
>> + emit_property_changed(batt->conn, batt->path,
>> + BATTERY_INTERFACE, "Level",
>> + DBUS_TYPE_UINT16, &level);
>> + }
>> + break;
>> + case BATTERY_OPT_REFRESH_FUNC:
>> + batt->refresh_func = va_arg(args, RefreshBattFunc);
>> + break;
>> + default:
>> + error("Unknown option %d", opt);
>> + return FALSE;
>> + }
>> +
>> + opt = va_arg(args, int);
>> + }
>> +
>> + va_end(args);
>> +
>> + return TRUE;
>> +
>> +}
>
> Please remove this empty line also.
>
Thanks, missed that as well.
>> diff --git a/src/device.h b/src/device.h
>> index f1d95c6..66eb5e8 100644
>> --- a/src/device.h
>> +++ b/src/device.h
>> @@ -23,8 +23,10 @@
>> */
>>
>> #define DEVICE_INTERFACE "org.bluez.Device"
>> +#define BATTERY_INTERFACE "org.bluez.Battery"
>>
>> struct btd_device;
>> +struct device_battery;
>
> Since this symbol is being exported, shouldn't it be prefixed with btd_ as well?
>
I will rename it to struct btd_battery. Is that ok ?
>>
>> typedef enum {
>> AUTH_TYPE_PINCODE,
>> @@ -54,6 +56,14 @@ struct btd_profile {
>> void (*adapter_remove) (struct btd_adapter *adapter);
>> };
>>
>> +typedef void (*RefreshBattFunc) (struct device_battery *batt);
>
> I don't think we use CamelCase for anything other than D-Bus method names.
>
We use it in gattrib.h for function pointers. What is the correct
convention for function pointers ? same as in adapter_ops ?
>> +
>> +typedef enum {
>> + BATTERY_OPT_INVALID = 0,
>> + BATTERY_OPT_LEVEL,
>> + BATTERY_OPT_REFRESH_FUNC,
>> +} BatteryOption;
>
> Fix CamelCase usage here and on uses of this type as well.
>
btio.h also uses this convention. What should be the correct convention ?
>> +
>> void btd_profile_foreach(void (*func)(struct btd_profile *p, void *data),
>> void *data);
>>
>> @@ -158,3 +168,8 @@ 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);
>> +
>> +struct device_battery *btd_device_add_battery(struct btd_device *device);
>> +void btd_device_remove_battery(struct device_battery *batt);
>> +gboolean btd_device_set_battery_opt(struct device_battery *batt,
>> + BatteryOption opt1, ...);
>> diff --git a/test/test-device b/test/test-device
>> index 63a96d3..7edb7b8 100755
>> --- a/test/test-device
>> +++ b/test/test-device
>> @@ -37,6 +37,7 @@ if (len(args) < 1):
>> print("")
>> print(" list")
>> print(" services <address>")
>> + print(" batteries <address>")
>> print(" create <address>")
>> print(" remove <address|path>")
>> print(" disconnect <address>")
>> @@ -205,5 +206,17 @@ if (args[0] == "services"):
>> print(path)
>> sys.exit(0)
>>
>> +if (args[0] == "batteries"):
>> + if (len(args) < 2):
>> + print("Need address parameter")
>> + else:
>> + path = adapter.FindDevice(args[1])
>> + device = dbus.Interface(bus.get_object("org.bluez", path),
>> + "org.bluez.Device")
>> + properties = device.GetProperties()
>> + for path in properties["Batteries"]:
>> + print(path)
>> + sys.exit(0)
>> +
>> print("Unknown command")
>> sys.exit(1)
>> --
>> 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,
--
BR,
Chen Ganir
^ permalink raw reply
* Re: [PATCH 05/10] battery: Discover Characteristic Descriptors
From: Chen Ganir @ 2012-09-12 4:49 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <CAAngNMaudTwxYNXqSsYFW+tU7OOTiPkseDKHux64HLzxp-5ALw@mail.gmail.com>
Joao,
On 09/11/2012 11:52 PM, Joao Paulo Rechi Vita wrote:
> On Tue, Sep 11, 2012 at 4:38 AM, <chen.ganir@ti.com> wrote:
>> From: Chen Ganir <chen.ganir@ti.com>
>>
>> Discover all characteristic descriptors, and build a descriptor
>> list. Presentation Format Descriptor and Client Characteristic
>> Configuration descriptors are searched.
>> ---
>> profiles/battery/battery.c | 73 +++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 72 insertions(+), 1 deletion(-)
>>
>> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
>> index 37cbce4..3a24a7b 100644
>> --- a/profiles/battery/battery.c
>> +++ b/profiles/battery/battery.c
>> @@ -51,6 +51,13 @@ static GSList *servers;
>> struct characteristic {
>> struct gatt_char attr; /* Characteristic */
>> struct battery *batt; /* Parent Battery Service */
>> + GSList *desc; /* Descriptors */
>
> Wrong alignment here.
>
Thanks.
>> +};
>> +
>> +struct descriptor {
>> + struct characteristic *ch; /* Parent Characteristic */
>> + uint16_t handle; /* Descriptor Handle */
>> + bt_uuid_t uuid; /* UUID */
>> };
>>
>> static gint cmp_device(gconstpointer a, gconstpointer b)
>> @@ -64,12 +71,21 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
>> return -1;
>> }
>>
>> +static void char_free(gpointer user_data)
>> +{
>> + struct characteristic *c = user_data;
>> +
>> + g_slist_free_full(c->desc, g_free);
>> +
>> + g_free(c);
>> +}
>> +
>> static void battery_free(gpointer user_data)
>> {
>> struct battery *batt = user_data;
>>
>> if (batt->chars != NULL)
>> - g_slist_free_full(batt->chars, g_free);
>> + g_slist_free_full(batt->chars, char_free);
>>
>> if (batt->attioid > 0)
>> btd_device_remove_attio_callback(batt->dev, batt->attioid);
>> @@ -82,7 +98,47 @@ static void battery_free(gpointer user_data)
>> g_free(batt);
>> }
>>
>> +static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>> + gpointer user_data)
>> +{
>> + struct characteristic *ch = user_data;
>> + struct att_data_list *list;
>> + uint8_t format;
>> + int i;
>> +
>> + if (status != 0) {
>> + error("Discover all characteristic descriptors failed [%s]: %s",
>> + ch->attr.uuid, att_ecode2str(status));
>> + return;
>> + }
>> +
>> + list = dec_find_info_resp(pdu, len, &format);
>> + if (list == NULL)
>> + return;
>> +
>> + for (i = 0; i < list->num; i++) {
>> + struct descriptor *desc;
>> + uint8_t *value;
>> +
>> + value = list->data[i];
>> + desc = g_new0(struct descriptor, 1);
>> + desc->handle = att_get_u16(value);
>> + desc->ch = ch;
>> +
>> + if (format == 0x01)
>> + desc->uuid = att_get_uuid16(&value[2]);
>> + else
>> + desc->uuid = att_get_uuid128(&value[2]);
>> +
>> + ch->desc = g_slist_append(ch->desc, desc);
>> + }
>> +
>> + att_data_list_free(list);
>> +}
>> +
>> +
>
> Remove extra blank line here.
>
>> static void configure_battery_cb(GSList *characteristics, guint8 status,
>> +
>
> Remove extra blank line here.
>
Ok, will remove both of them.
>> gpointer user_data)
>> {
>> struct battery *batt = user_data;
>> @@ -97,6 +153,7 @@ static void configure_battery_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;
>> @@ -106,6 +163,20 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
>> 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;
>> +
>> + 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
>
>
>
Thanks,
--
BR,
Chen Ganir
^ permalink raw reply
* Re: [PATCH 07/10] battery: Add Battery to device
From: Chen Ganir @ 2012-09-12 4:54 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <CAAngNMbnRc-JxTP_aGwVbyov3bsd=dVgAC=GkjMM9mnzgtNPQQ@mail.gmail.com>
On 09/12/2012 12:40 AM, Joao Paulo Rechi Vita wrote:
> On Tue, Sep 11, 2012 at 4:38 AM, <chen.ganir@ti.com> wrote:
>> From: Chen Ganir <chen.ganir@ti.com>
>>
>> Add/Remove battery from device
>> ---
>> profiles/battery/battery.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
>> index d1e0b6e..31f2297 100644
>> --- a/profiles/battery/battery.c
>> +++ b/profiles/battery/battery.c
>> @@ -49,8 +49,9 @@ struct battery {
>> static GSList *servers;
>>
>> struct characteristic {
>> - struct gatt_char attr; /* Characteristic */
>> - struct battery *batt; /* Parent Battery Service */
>> + struct device_battery *devbatt; /* device_battery pointer */
>> + struct gatt_char attr; /* Characteristic */
>> + struct battery *batt; /* Parent Battery Service */
>
> Just a minor pick here: the comment alignment of attr and batt should
> have been fixed on the previous commit, to keep it consistent along
> the series.
>
You are correct. That should have been like that, but i missed it. Will
fix it.
>> GSList *desc; /* Descriptors */
>> uint8_t ns; /* Battery Namespace */
>> uint16_t description; /* Battery description */
>> @@ -79,6 +80,8 @@ static void char_free(gpointer user_data)
>>
>> g_slist_free_full(c->desc, g_free);
>>
>> + btd_device_remove_battery(c->devbatt);
>> +
>> g_free(c);
>> }
>>
>> @@ -160,12 +163,12 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>> if (status != 0) {
>> error("Discover all characteristic descriptors failed [%s]: %s",
>> ch->attr.uuid, att_ecode2str(status));
>> - return;
>> + goto update_char;
>> }
>>
>> list = dec_find_info_resp(pdu, len, &format);
>> if (list == NULL)
>> - return;
>> + goto update_char;
>>
>> for (i = 0; i < list->num; i++) {
>> struct descriptor *desc;
>> @@ -186,6 +189,9 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>> }
>>
>> att_data_list_free(list);
>> +
>> +update_char:
>> + ch->devbatt = btd_device_add_battery(ch->batt->dev);
>
> If I understood correctly, adding a battery to the device is a
> consequence of finding the "Battery Level" characteristic, and not
> related to the descriptors discovery. So the call to
> btd_device_add_battery() should be done inside the
> configure_battery_cb() instead of the discover_desc_cb(). This way
> you'll not need all these goto's.
>
You are correct. It can be done there.
>> }
>>
>>
>> --
>> 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,
--
BR,
Chen Ganir
^ permalink raw reply
* Re: [PATCH 08/10] battery: Read Battery level characteristic
From: Chen Ganir @ 2012-09-12 4:55 UTC (permalink / raw)
To: Joao Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <CAAngNMYezceDToeNVC5a96_odOzDWnLRBQ0GrJJUd84s0hRqiw@mail.gmail.com>
On 09/12/2012 12:50 AM, Joao Paulo Rechi Vita wrote:
> On Tue, Sep 11, 2012 at 4:38 AM, <chen.ganir@ti.com> wrote:
>> From: Chen Ganir <chen.ganir@ti.com>
>>
>> Implement support for reading the battery level characteristic on
>> connection establishment.
>> ---
>> profiles/battery/battery.c | 87 +++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 85 insertions(+), 2 deletions(-)
>>
>> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
>> index 31f2297..a93b352 100644
>> --- a/profiles/battery/battery.c
>> +++ b/profiles/battery/battery.c
>> @@ -55,6 +55,7 @@ struct characteristic {
>> GSList *desc; /* Descriptors */
>> uint8_t ns; /* Battery Namespace */
>> uint16_t description; /* Battery description */
>> + uint8_t level;
>
> Alignment problem here.
>
Thanks.
>> };
>>
>> struct descriptor {
>> @@ -103,6 +104,80 @@ static void battery_free(gpointer user_data)
>> g_free(batt);
>> }
>>
>> +static void read_batterylevel_cb(guint8 status, const guint8 *pdu, guint16 len,
>> + gpointer user_data)
>> +{
>> + struct characteristic *ch = user_data;
>> + uint8_t value[ATT_MAX_MTU];
>> + int vlen;
>> +
>> + if (status != 0) {
>> + error("Failed to read Battery Level:%s", att_ecode2str(status));
>> + return;
>> + }
>> +
>> + vlen = dec_read_resp(pdu, len, value, sizeof(value));
>> + if (!vlen) {
>> + error("Failed to read Battery Level: Protocol error\n");
>> + return;
>> + }
>> +
>> + if (vlen < 1) {
>> + error("Failed to read Battery Level: Wrong pdu len");
>> + return;
>> + }
>> +
>> + ch->level = value[0];
>> + btd_device_set_battery_opt(ch->devbatt, BATTERY_OPT_LEVEL, ch->level,
>> + BATTERY_OPT_INVALID);
>> +}
>> +
>> +static void process_batteryservice_char(struct characteristic *ch)
>> +{
>> + if (g_strcmp0(ch->attr.uuid, BATTERY_LEVEL_UUID) == 0) {
>> + gatt_read_char(ch->batt->attrib, ch->attr.value_handle, 0,
>> + read_batterylevel_cb, ch);
>> + }
>> +}
>> +
>> +static gint device_battery_cmp(gconstpointer a, gconstpointer b)
>> +{
>> + const struct characteristic *ch = a;
>> + const struct device_battery *batt = b;
>> +
>> + if (batt == ch->devbatt)
>> + return 0;
>> +
>> + return -1;
>> +}
>> +
>> +static struct characteristic *find_battery_char(struct device_battery *db)
>> +{
>> + GSList *l, *b;
>> +
>> + for (l = servers; l != NULL; l = g_slist_next(l)) {
>> + struct battery *batt = l->data;
>> +
>> + b = g_slist_find_custom(batt->chars, db, device_battery_cmp);
>> + if (!b)
>> + return NULL;
>> +
>> + return b->data;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static void batterylevel_refresh_cb(struct device_battery *batt)
>> +{
>> + struct characteristic *ch;
>> +
>> + ch = find_battery_char(batt);
>> +
>> + if (ch)
>> + process_batteryservice_char(ch);
>> +}
>> +
>> static void batterylevel_presentation_format_desc_cb(guint8 status,
>> const guint8 *pdu, guint16 len,
>> gpointer user_data)
>> @@ -151,7 +226,6 @@ static void process_batterylevel_desc(struct descriptor *desc)
>> 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)
>> {
>> @@ -192,8 +266,11 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>>
>> update_char:
>> ch->devbatt = btd_device_add_battery(ch->batt->dev);
>> -}
>> + btd_device_set_battery_opt(ch->devbatt, BATTERY_OPT_REFRESH_FUNC,
>> + batterylevel_refresh_cb, BATTERY_OPT_INVALID);
>>
>> + process_batteryservice_char(ch);
>
> There two call also should be part of configure_battery_cb() instead
> of discover_desc_cb().
>
True. Once i move the btd_device_add_battery call, i'll move those as well.
>> +}
>>
>> static void configure_battery_cb(GSList *characteristics, guint8 status,
>>
>> @@ -252,6 +329,12 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
>> gatt_discover_char(batt->attrib, batt->svc_range->start,
>> batt->svc_range->end, NULL,
>> configure_battery_cb, batt);
>> + } else {
>> + GSList *l;
>> + for (l = batt->chars; l; l = l->next) {
>> + struct characteristic *c = l->data;
>> + process_batteryservice_char(c);
>> + }
>> }
>> }
>>
>> --
>> 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,
--
BR,
Chen Ganir
^ 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