* Re: [PATCH 1/1] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Alan Ott @ 2010-07-09 3:51 UTC (permalink / raw)
To: Marcel Holtmann
Cc: David S Miller, Jiri Kosina, Michael Poole, Bastien Nocera,
Eric Dumazet, linux-bluetooth, linux-kernel, netdev
In-Reply-To: <1278623485.10421.73.camel@localhost.localdomain>
On 07/08/2010 05:11 PM, Marcel Holtmann wrote:
> I looked at this and I am bit worried that this should not be done in
> this detail in the HIDP driver. Essentially HIDP is a pure transport
> driver. It should not handle all these details. Can we make this a bit
> easier for the transport drivers to support such features?
>
> Regards
>
> Marcel
>
Hi Marcel,
I put these changes (most notably the addition of hidp_get_raw_report())
in hidp because that's where the parallel function
hidp_output_raw_report() was already located. I figured the input should
go with the output. That said, if there's a better place for both of
them (input and output) to go, let me know where you think it should be,
and I'll get them moved into the proper spot.
I'm not sure what you mean about HIDP being a pure transport driver.
Alan.
^ permalink raw reply
* Re: [PATCH] Bluetooth: Send ConfigReq after send a ConnectionRsp
From: Marcel Holtmann @ 2010-07-08 23:12 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth, Gustavo F. Padovan
In-Reply-To: <1278630498-20401-1-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> The extended L2CAP features requires that one should initiate a
> ConfigReq after send the ConnectionRsp. This patch changes the behaviour
> of the configuration process of our stack.
>
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> ---
> net/bluetooth/l2cap.c | 44 +++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 41 insertions(+), 3 deletions(-)
patch has been applied. Thanks.
Regards
Marcel
^ permalink raw reply
* [PATCH] Bluetooth: Send ConfigReq after send a ConnectionRsp
From: Gustavo F. Padovan @ 2010-07-08 23:08 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
From: Gustavo F. Padovan <padovan@profusion.mobi>
The extended L2CAP features requires that one should initiate a
ConfigReq after send the ConnectionRsp. This patch changes the behaviour
of the configuration process of our stack.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/l2cap.c | 44 +++++++++++++++++++++++++++++++++++++++++---
1 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index a3dfee9..e366be0 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -74,6 +74,7 @@ static void __l2cap_sock_close(struct sock *sk, int reason);
static void l2cap_sock_close(struct sock *sk);
static void l2cap_sock_kill(struct sock *sk);
+static int l2cap_build_conf_req(struct sock *sk, void *data);
static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
u8 code, u8 ident, u16 dlen, void *data);
@@ -548,6 +549,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
}
} else if (sk->sk_state == BT_CONNECT2) {
struct l2cap_conn_rsp rsp;
+ char buf[128];
rsp.scid = cpu_to_le16(l2cap_pi(sk)->dcid);
rsp.dcid = cpu_to_le16(l2cap_pi(sk)->scid);
@@ -570,6 +572,17 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
l2cap_send_cmd(conn, l2cap_pi(sk)->ident,
L2CAP_CONN_RSP, sizeof(rsp), &rsp);
+
+ if (l2cap_pi(sk)->conf_state & L2CAP_CONF_REQ_SENT ||
+ rsp.result != L2CAP_CR_SUCCESS) {
+ bh_unlock_sock(sk);
+ continue;
+ }
+
+ l2cap_pi(sk)->conf_state |= L2CAP_CONF_REQ_SENT;
+ l2cap_send_cmd(conn, l2cap_get_ident(conn), L2CAP_CONF_REQ,
+ l2cap_build_conf_req(sk, buf), buf);
+ l2cap_pi(sk)->num_conf_req++;
}
bh_unlock_sock(sk);
@@ -1897,6 +1910,8 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
if (sk->sk_state == BT_CONNECT2 && bt_sk(sk)->defer_setup) {
struct l2cap_conn_rsp rsp;
+ struct l2cap_conn *conn = l2cap_pi(sk)->conn;
+ u8 buf[128];
sk->sk_state = BT_CONFIG;
@@ -1907,6 +1922,16 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
l2cap_send_cmd(l2cap_pi(sk)->conn, l2cap_pi(sk)->ident,
L2CAP_CONN_RSP, sizeof(rsp), &rsp);
+ if (l2cap_pi(sk)->conf_state & L2CAP_CONF_REQ_SENT) {
+ release_sock(sk);
+ return 0;
+ }
+
+ l2cap_pi(sk)->conf_state |= L2CAP_CONF_REQ_SENT;
+ l2cap_send_cmd(conn, l2cap_get_ident(conn), L2CAP_CONF_REQ,
+ l2cap_build_conf_req(sk, buf), buf);
+ l2cap_pi(sk)->num_conf_req++;
+
release_sock(sk);
return 0;
}
@@ -2613,7 +2638,7 @@ static int l2cap_parse_conf_req(struct sock *sk, void *data)
}
}
- if (pi->num_conf_rsp || pi->num_conf_req)
+ if (pi->num_conf_rsp || pi->num_conf_req > 1)
goto done;
switch (pi->mode) {
@@ -2857,7 +2882,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
struct l2cap_chan_list *list = &conn->chan_list;
struct l2cap_conn_req *req = (struct l2cap_conn_req *) data;
struct l2cap_conn_rsp rsp;
- struct sock *sk, *parent;
+ struct sock *parent, *uninitialized_var(sk);
int result, status = L2CAP_CS_NO_INFO;
u16 dcid = 0, scid = __le16_to_cpu(req->scid);
@@ -2966,6 +2991,15 @@ sendresp:
L2CAP_INFO_REQ, sizeof(info), &info);
}
+ if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_REQ_SENT) &&
+ result == L2CAP_CR_SUCCESS) {
+ u8 buf[128];
+ l2cap_pi(sk)->conf_state |= L2CAP_CONF_REQ_SENT;
+ l2cap_send_cmd(conn, l2cap_get_ident(conn), L2CAP_CONF_REQ,
+ l2cap_build_conf_req(sk, buf), buf);
+ l2cap_pi(sk)->num_conf_req++;
+ }
+
return 0;
}
@@ -2998,9 +3032,13 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
sk->sk_state = BT_CONFIG;
l2cap_pi(sk)->ident = 0;
l2cap_pi(sk)->dcid = dcid;
- l2cap_pi(sk)->conf_state |= L2CAP_CONF_REQ_SENT;
l2cap_pi(sk)->conf_state &= ~L2CAP_CONF_CONNECT_PEND;
+ if (l2cap_pi(sk)->conf_state & L2CAP_CONF_REQ_SENT)
+ break;
+
+ l2cap_pi(sk)->conf_state |= L2CAP_CONF_REQ_SENT;
+
l2cap_send_cmd(conn, l2cap_get_ident(conn), L2CAP_CONF_REQ,
l2cap_build_conf_req(sk, req), req);
l2cap_pi(sk)->num_conf_req++;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 9/20] drivers/bluetooth: Use kzalloc
From: Marcel Holtmann @ 2010-07-08 22:01 UTC (permalink / raw)
To: Julia Lawall; +Cc: linux-bluetooth, linux-kernel, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1005132201470.6282@ask.diku.dk>
Hi Julia,
> Use kzalloc rather than the combination of kmalloc and memset.
>
patch has been applied to bluetooth-next-2.6 tree. Thanks.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 20/37] drivers/bluetooth: Use kmemdup
From: Marcel Holtmann @ 2010-07-08 21:57 UTC (permalink / raw)
To: Julia Lawall; +Cc: linux-bluetooth, linux-kernel, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1005152318590.21345@ask.diku.dk>
Hi Julia,
> Use kmemdup when some other buffer is immediately copied into the
> allocated region.
patch has been applied to bluetooth-next-2.6 tree. Thanks.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Bluetooth: btmrvl: process interrupt in main thread to
From: Marcel Holtmann @ 2010-07-08 21:52 UTC (permalink / raw)
To: Bing Zhao; +Cc: linux-bluetooth, Amitkumar Karwar
In-Reply-To: <1275003517-18563-1-git-send-email-bzhao@marvell.com>
Hi Bing,
> When driver is sending a command or data and the firmware is also
> sending a sleep event, sometimes it is observed that driver will
> continue to send the command/data to firmware right after processing
> sleep event. Once sleep event is processed driver is not supposed to
> send anything because firmware is in sleep state after that. Previously
> interrupt processing was done in SDIO interrupt callback handler.
> Now it is done in btmrvl driver main thread to solve the
> cross-sending properly.
>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> Signed-off-by: Bing Zhao <bzhao@marvell.com>
> ---
> drivers/bluetooth/btmrvl_drv.h | 1 +
> drivers/bluetooth/btmrvl_main.c | 5 ++-
> drivers/bluetooth/btmrvl_sdio.c | 97 ++++++++++++++++++++-------------------
> 3 files changed, 55 insertions(+), 48 deletions(-)
patch has been applied. Thanks.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/3] Bluetooth: Remove extraneous white space
From: Marcel Holtmann @ 2010-07-08 21:49 UTC (permalink / raw)
To: Ron Shaffer; +Cc: linux-bluetooth
In-Reply-To: <1275062027-27872-2-git-send-email-rshaffer@codeaurora.org>
Hi Ron,
> Deleted extraneous white space from the end of several lines
>
> Signed-off-by: Ron Shaffer <rshaffer@codeaurora.org>
> ---
> include/net/bluetooth/hci_core.h | 14 +++++++-------
> 1 files changed, 7 insertions(+), 7 deletions(-)
patch has been applied. Thanks.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/3] Bluetooth: Reassigned copyright to Code Aurora Forum
From: Marcel Holtmann @ 2010-07-08 21:49 UTC (permalink / raw)
To: Ron Shaffer; +Cc: linux-bluetooth
In-Reply-To: <1275062027-27872-3-git-send-email-rshaffer@codeaurora.org>
Hi Ron,
> Qualcomm, Inc. has reassigned rights to Code Aurora Forum. Accordingly,
> as files are modified by Code Aurora Forum members, the copyright
> statement will be updated.
>
> Signed-off-by: Ron Shaffer <rshaffer@codeaurora.org>
> ---
> include/net/bluetooth/hci_core.h | 2 +-
> net/bluetooth/hci_conn.c | 2 +-
> net/bluetooth/hci_event.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
patch has been applied. Thanks.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: Synchronize SCO/eSCO connection requests to ACL state
From: Marcel Holtmann @ 2010-07-08 21:49 UTC (permalink / raw)
To: Ron Shaffer; +Cc: linux-bluetooth
In-Reply-To: <1275062027-27872-4-git-send-email-rshaffer@codeaurora.org>
Hi Ron,
> Certain headsets such as the Motorola H350 will reject SCO and eSCO
> connection requests while the ACL is transitioning from sniff mode
> to active mode. Add synchronization so that SCO and eSCO connection
> requests will wait until the ACL has fully transitioned to active mode.
>
> Signed-off-by: Ron Shaffer <rshaffer@codeaurora.org>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_conn.c | 18 ++++++++++++++++++
> net/bluetooth/hci_event.c | 23 ++++++++++++++++++++++-
> 3 files changed, 41 insertions(+), 1 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index fd53323..c4a37fc 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -250,6 +250,7 @@ enum {
> HCI_CONN_ENCRYPT_PEND,
> HCI_CONN_RSWITCH_PEND,
> HCI_CONN_MODE_CHANGE_PEND,
> + HCI_CONN_SCO_PEND,
> };
>
> static inline void hci_conn_hash_init(struct hci_dev *hdev)
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 9bf4308..e900f85 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -117,9 +117,18 @@ void hci_add_sco(struct hci_conn *conn, __u16 handle)
> {
> struct hci_dev *hdev = conn->hdev;
> struct hci_cp_add_sco cp;
> + struct hci_conn *acl = conn->link;
>
> BT_DBG("%p", conn);
>
> + if (acl->mode == HCI_CM_SNIFF &&
> + test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
> + set_bit(HCI_CONN_SCO_PEND, &conn->pend);
> + return;
> + }
> +
> + clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
> +
> conn->state = BT_CONNECT;
> conn->out = 1;
>
> @@ -135,9 +144,18 @@ void hci_setup_sync(struct hci_conn *conn, __u16 handle)
> {
> struct hci_dev *hdev = conn->hdev;
> struct hci_cp_setup_sync_conn cp;
> + struct hci_conn *acl = conn->link;
>
> BT_DBG("%p", conn);
>
> + if (acl->mode == HCI_CM_SNIFF &&
> + test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
> + set_bit(HCI_CONN_SCO_PEND, &conn->pend);
> + return;
> + }
> +
> + clear_bit(HCI_CONN_SCO_PEND, &conn->pend);
> +
I really would prefer test_and_clear_bit() here.
> conn->state = BT_CONNECT;
> conn->out = 1;
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 3af537a..7692db6 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -615,6 +615,7 @@ static void hci_cs_add_sco(struct hci_dev *hdev, __u8 status)
> acl = hci_conn_hash_lookup_handle(hdev, handle);
> if (acl && (sco = acl->link)) {
> sco->state = BT_CLOSED;
> + clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
>
> hci_proto_connect_cfm(sco, status);
> hci_conn_del(sco);
> @@ -760,6 +761,7 @@ static void hci_cs_setup_sync_conn(struct hci_dev *hdev, __u8 status)
> acl = hci_conn_hash_lookup_handle(hdev, handle);
> if (acl && (sco = acl->link)) {
> sco->state = BT_CLOSED;
> + clear_bit(HCI_CONN_SCO_PEND, &sco->pend);
>
> hci_proto_connect_cfm(sco, status);
> hci_conn_del(sco);
> @@ -795,6 +797,7 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
> {
> struct hci_cp_exit_sniff_mode *cp;
> struct hci_conn *conn;
> + struct hci_conn *sco;
>
> BT_DBG("%s status 0x%x", hdev->name, status);
>
> @@ -808,9 +811,17 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
> hci_dev_lock(hdev);
>
> conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(cp->handle));
> - if (conn)
> + if (conn) {
> clear_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend);
>
> + sco = conn->link;
> + if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND, &sco->pend)) {
> + hci_proto_connect_cfm(sco, status);
> + hci_conn_del(sco);
> + }
> + }
> + }
> +
Something is wrong here. The } are not matching up.
> hci_dev_unlock(hdev);
> }
>
> @@ -1463,6 +1474,7 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
> {
> struct hci_ev_mode_change *ev = (void *) skb->data;
> struct hci_conn *conn;
> + struct hci_conn *sco;
>
> BT_DBG("%s status %d", hdev->name, ev->status);
>
> @@ -1478,6 +1490,15 @@ static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb
> conn->power_save = 1;
> else
> conn->power_save = 0;
> + } else {
> + sco = conn->link;
> + if (sco && test_and_clear_bit(HCI_CONN_SCO_PEND,
> + &sco->pend)) {
> + if (lmp_esco_capable(hdev))
> + hci_setup_sync(sco, conn->handle);
> + else
> + hci_add_sco(sco, conn->handle);
> + }
> }
> }
>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/8 v2]Bluetooth: Fix warning: variable 'tty' set but not used
From: Justin P. Mattock @ 2010-07-08 21:25 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth, gustavo, linux-kernel
In-Reply-To: <1278623228.10421.70.camel@localhost.localdomain>
On 07/08/2010 02:07 PM, Marcel Holtmann wrote:
> Hi Justin,
>
>> Resend of this patch to fix whitespace issues
>> The patch below fixes a warning message
>> when using gcc 4.6.0
>>
>> CC [M] drivers/bluetooth/hci_ldisc.o
>> drivers/bluetooth/hci_ldisc.c: In function 'hci_uart_send_frame':
>> drivers/bluetooth/hci_ldisc.c:213:21: warning: variable 'tty' set but not used
>>
>>
>> Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
>> Reviewed-By: Gustavo F. Padovan<gustavo@padovan.org>
>
> patch has been applied to bluetooth-next-2.6 tree. Thanks.
>
> Regards
>
> Marcel
>
>
>
cool..
Justin P. Mattock
^ permalink raw reply
* Re: [PATCH 1/1] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Marcel Holtmann @ 2010-07-08 21:11 UTC (permalink / raw)
To: Alan Ott
Cc: David S Miller, Jiri Kosina, Michael Poole, Bastien Nocera,
Eric Dumazet, linux-bluetooth, linux-kernel, netdev
In-Reply-To: <1276467601-9066-1-git-send-email-alan@signal11.us>
Hi Alan,
> This patch adds support or getting and setting feature reports for bluetooth
> HID devices from HIDRAW.
>
> Signed-off-by: Alan Ott <alan@signal11.us>
> ---
> net/bluetooth/hidp/core.c | 121 +++++++++++++++++++++++++++++++++++++++++++--
> net/bluetooth/hidp/hidp.h | 8 +++
> 2 files changed, 125 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index bfe641b..0f068a0 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -36,6 +36,7 @@
> #include <linux/file.h>
> #include <linux/init.h>
> #include <linux/wait.h>
> +#include <linux/mutex.h>
> #include <net/sock.h>
>
> #include <linux/input.h>
> @@ -313,6 +314,93 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep
> return hidp_queue_report(session, buf, rsize);
> }
>
> +static int hidp_get_raw_report(struct hid_device *hid,
> + unsigned char report_number,
> + unsigned char *data, size_t count,
> + unsigned char report_type)
> +{
> + struct hidp_session *session = hid->driver_data;
> + struct sk_buff *skb;
> + size_t len;
> + int numbered_reports = hid->report_enum[report_type].numbered;
> +
> + switch (report_type) {
> + case HID_FEATURE_REPORT:
> + report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_FEATURE;
> + break;
> + case HID_INPUT_REPORT:
> + report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_INPUT;
> + break;
> + case HID_OUTPUT_REPORT:
> + report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_OUPUT;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (mutex_lock_interruptible(&session->report_mutex))
> + return -ERESTARTSYS;
> +
> + /* Set up our wait, and send the report request to the device. */
> + session->waiting_report_type = report_type & HIDP_DATA_RTYPE_MASK;
> + session->waiting_report_number = numbered_reports ? report_number : -1;
> + set_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
> + data[0] = report_number;
> + if (hidp_send_ctrl_message(hid->driver_data, report_type, data, 1))
> + goto err_eio;
> +
> + /* Wait for the return of the report. The returned report
> + gets put in session->report_return. */
> + while (test_bit(HIDP_WAITING_FOR_RETURN, &session->flags)) {
> + int res;
> +
> + res = wait_event_interruptible_timeout(session->report_queue,
> + !test_bit(HIDP_WAITING_FOR_RETURN, &session->flags),
> + 5*HZ);
> + if (res == 0) {
> + /* timeout */
> + goto err_eio;
> + }
> + if (res < 0) {
> + /* signal */
> + goto err_restartsys;
> + }
> + }
> +
> + skb = session->report_return;
> + if (skb) {
> + if (numbered_reports) {
> + /* Strip off the report number. */
> + size_t rpt_len = skb->len-1;
> + len = rpt_len < count ? rpt_len : count;
> + memcpy(data, skb->data+1, len);
> + } else {
> + len = skb->len < count ? skb->len : count;
> + memcpy(data, skb->data, len);
> + }
> +
> + kfree_skb(skb);
> + session->report_return = NULL;
> + } else {
> + /* Device returned a HANDSHAKE, indicating protocol error. */
> + len = -EIO;
> + }
> +
> + clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
> + mutex_unlock(&session->report_mutex);
> +
> + return len;
> +
> +err_restartsys:
> + clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
> + mutex_unlock(&session->report_mutex);
> + return -ERESTARTSYS;
> +err_eio:
> + clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
> + mutex_unlock(&session->report_mutex);
> + return -EIO;
> +}
> +
> static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
> unsigned char report_type)
> {
> @@ -367,6 +455,10 @@ static void hidp_process_handshake(struct hidp_session *session,
> case HIDP_HSHK_ERR_INVALID_REPORT_ID:
> case HIDP_HSHK_ERR_UNSUPPORTED_REQUEST:
> case HIDP_HSHK_ERR_INVALID_PARAMETER:
> + if (test_bit(HIDP_WAITING_FOR_RETURN, &session->flags)) {
> + clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
> + wake_up_interruptible(&session->report_queue);
> + }
> /* FIXME: Call into SET_ GET_ handlers here */
> break;
>
> @@ -403,9 +495,11 @@ static void hidp_process_hid_control(struct hidp_session *session,
> }
> }
>
> -static void hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
> +/* Returns true if the passed-in skb should be freed by the caller. */
> +static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
> unsigned char param)
> {
> + int done_with_skb = 1;
> BT_DBG("session %p skb %p len %d param 0x%02x", session, skb, skb->len, param);
>
> switch (param) {
> @@ -417,7 +511,6 @@ static void hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
>
> if (session->hid)
> hid_input_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len, 0);
> -
> break;
>
> case HIDP_DATA_RTYPE_OTHER:
> @@ -429,12 +522,27 @@ static void hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
> __hidp_send_ctrl_message(session,
> HIDP_TRANS_HANDSHAKE | HIDP_HSHK_ERR_INVALID_PARAMETER, NULL, 0);
> }
> +
> + if (test_bit(HIDP_WAITING_FOR_RETURN, &session->flags) &&
> + param == session->waiting_report_type) {
> + if (session->waiting_report_number < 0 ||
> + session->waiting_report_number == skb->data[0]) {
> + /* hidp_get_raw_report() is waiting on this report. */
> + session->report_return = skb;
> + done_with_skb = 0;
> + clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
> + wake_up_interruptible(&session->report_queue);
> + }
> + }
> +
> + return done_with_skb;
> }
>
> static void hidp_recv_ctrl_frame(struct hidp_session *session,
> struct sk_buff *skb)
> {
> unsigned char hdr, type, param;
> + int free_skb = 1;
>
> BT_DBG("session %p skb %p len %d", session, skb, skb->len);
>
> @@ -454,7 +562,7 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
> break;
>
> case HIDP_TRANS_DATA:
> - hidp_process_data(session, skb, param);
> + free_skb = hidp_process_data(session, skb, param);
> break;
>
> default:
> @@ -463,7 +571,8 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
> break;
> }
>
> - kfree_skb(skb);
> + if (free_skb)
> + kfree_skb(skb);
> }
>
> static void hidp_recv_intr_frame(struct hidp_session *session,
> @@ -797,6 +906,7 @@ static int hidp_setup_hid(struct hidp_session *session,
> hid->dev.parent = hidp_get_device(session);
> hid->ll_driver = &hidp_hid_driver;
>
> + hid->hid_get_raw_report = hidp_get_raw_report;
> hid->hid_output_raw_report = hidp_output_raw_report;
>
> err = hid_add_device(hid);
> @@ -857,6 +967,9 @@ int hidp_add_connection(struct hidp_connadd_req *req, struct socket *ctrl_sock,
> skb_queue_head_init(&session->ctrl_transmit);
> skb_queue_head_init(&session->intr_transmit);
>
> + mutex_init(&session->report_mutex);
> + init_waitqueue_head(&session->report_queue);
> +
> session->flags = req->flags & (1 << HIDP_BLUETOOTH_VENDOR_ID);
> session->idle_to = req->idle_to;
>
> diff --git a/net/bluetooth/hidp/hidp.h b/net/bluetooth/hidp/hidp.h
> index 8d934a1..00e71dd 100644
> --- a/net/bluetooth/hidp/hidp.h
> +++ b/net/bluetooth/hidp/hidp.h
> @@ -80,6 +80,7 @@
> #define HIDP_VIRTUAL_CABLE_UNPLUG 0
> #define HIDP_BOOT_PROTOCOL_MODE 1
> #define HIDP_BLUETOOTH_VENDOR_ID 9
> +#define HIDP_WAITING_FOR_RETURN 10
>
> struct hidp_connadd_req {
> int ctrl_sock; // Connected control socket
> @@ -154,6 +155,13 @@ struct hidp_session {
> struct sk_buff_head ctrl_transmit;
> struct sk_buff_head intr_transmit;
>
> + /* Used in hidp_get_raw_report() */
> + int waiting_report_type; /* HIDP_DATA_RTYPE_* */
> + int waiting_report_number; /* -1 for not numbered */
> + struct mutex report_mutex;
> + struct sk_buff *report_return;
> + wait_queue_head_t report_queue;
> +
> /* Report descriptor */
> __u8 *rd_data;
> uint rd_size;
I looked at this and I am bit worried that this should not be done in
this detail in the HIDP driver. Essentially HIDP is a pure transport
driver. It should not handle all these details. Can we make this a bit
easier for the transport drivers to support such features?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/1] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Marcel Holtmann @ 2010-07-08 21:08 UTC (permalink / raw)
To: Andrei Emeltchenko
Cc: David Miller, ospite, alan, jkosina, mdpoole, hadess,
eric.dumazet, linux-bluetooth, linux-kernel, netdev
In-Reply-To: <AANLkTiktvYWdZmpQs_k7mJRqMCqgfhy1b0T8tX8emmtU@mail.gmail.com>
Hi Andrei,
> >> On Sun, 13 Jun 2010 18:20:01 -0400
> >> Alan Ott <alan@signal11.us> wrote:
> >>
> >>> This patch adds support or getting and setting feature reports for bluetooth
> >>> HID devices from HIDRAW.
> >>>
> >>> Signed-off-by: Alan Ott <alan@signal11.us>
> >>> ---
> >>
> >> Ping.
> >
> > We effectively don't have a bluetooth maintainer at the current point
> > in time. I've tried to let patches sit for a while hoping the listed
> > maintainer would do something, at least occaisionally, but that simply
> > isn't happening.
> > So I'll just pick patches up directly as I find time to review them,
> > but I have to warn that for me it's going to be done in a very low
> > priority way because I really don't find bluetooth all that exciting. :-)
>
> This would be good. We have a backlog of bluetooth kernel patches
> waiting for several months.
> This takes too much time to return to them again and again...
>
> Please keep this ML informed.
as soon as Ville is back from vacation, he should be helping out with
patch review and maintenance.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/8 v2]Bluetooth: Fix warning: variable 'tty' set but not used
From: Marcel Holtmann @ 2010-07-08 21:07 UTC (permalink / raw)
To: Justin P. Mattock; +Cc: linux-bluetooth, gustavo, linux-kernel
In-Reply-To: <1276565200-5583-1-git-send-email-justinmattock@gmail.com>
Hi Justin,
> Resend of this patch to fix whitespace issues
> The patch below fixes a warning message
> when using gcc 4.6.0
>
> CC [M] drivers/bluetooth/hci_ldisc.o
> drivers/bluetooth/hci_ldisc.c: In function 'hci_uart_send_frame':
> drivers/bluetooth/hci_ldisc.c:213:21: warning: variable 'tty' set but not used
>
>
> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
> Reviewed-By: Gustavo F. Padovan <gustavo@padovan.org>
patch has been applied to bluetooth-next-2.6 tree. Thanks.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Adding support for bluetooth controller shipped with iMac i5
From: Marcel Holtmann @ 2010-07-08 21:05 UTC (permalink / raw)
To: Cyril Lacoux (Yack); +Cc: linux-bluetooth
In-Reply-To: <201006162028.59280.clacoux@ifeelgood.org>
Hi,
> > please show the output of /proc/bus/usb/devices or usb-devices script
> > for this device.
> >
> > And I want a proper kernel patch from git format-patch.
>
> Ok,
>
> Here is the output of command `git format-patch -1 --stdout` against current
> bluetooth-next-2.6 tree - hope that's ok.
please create a proper commit message and include the usb-devices output
in that message.
Also the HID part in the source code comment makes no sense. Why are you
using that one. Copy&paste from the previous one?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] bluetooth: btmrvl_sdio: silence warning
From: Marcel Holtmann @ 2010-07-08 21:02 UTC (permalink / raw)
To: Kulikov Vasiliy
Cc: Kernel Janitors, Bing Zhao, Ben Hutchings,
André Goddard Rosa, Julia Lawall, linux-bluetooth,
linux-kernel
In-Reply-To: <1278316883-29784-1-git-send-email-segooon@gmail.com>
Hi Kulikov,
> Clone checking of ret to simplify the code.
>
> This patch silences a compiler warning:
> drivers/bluetooth/btmrvl_sdio.c: In function ‘btmrvl_sdio_verify_fw_download’:
> drivers/bluetooth/btmrvl_sdio.c:80: warning: ‘fws1’ may be used uninitialized in this function
> drivers/bluetooth/btmrvl_sdio.c:80: note: ‘fws1’ was declared here
>
> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
patch has been applied to bluetooth-next-2.6 tree. Thanks.
Regards
Marcel
^ permalink raw reply
* Re: Proposed API for HDP
From: José Antonio Santos Cadenas @ 2010-07-08 20:30 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <20100708191755.GD26759@vigoh>
El Thursday 08 July 2010 21:17:55 Gustavo F. Padovan escribi=F3:
> > > ---------------------------------------------------------------------=
=2D-
> > > ---------
> > >=20
> > > Service org.bluez
> > > Interface org.bluez.HealthDeviceApplication
> > > Object path [variable
> > > prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
> >=20
> > That is more like the org.bluez.HealthService as mentioned above. So
> > lets combine them. I don't see a need for splitting these.
> >=20
> > > Methods:
> > > array GetProperties()
> > > =09
> > > Gets the information of the remote application published on its
> > > SDP record. The returned data format is as follows:
> > > =09
> > > {
> > > =09
> > > "end_points": [
> > > =09
> > > "mdepid": uint8,
> > > "role" : "source" or "sink" ,
> > > "specs" : [{
> > > =09
> > > "dtype" : uint16,
> > > "description" : string, (optional)
> > > }]
> > > =09
> > > ]
> > > =09
> > > }
> > > =09
> > > object Connect(path local_application_id)
> > > =09
> > > Connects the local application with the remote application.
> > > =09
> > > Only the bus client that created the local session will be able
> > > to create connections using it.
> > > =09
> > > If the Device is already connected with an other application an
> > > org.bluez.Error.AlreadyConnected error will be received.
> > > =09
> > > Possible errors: org.bluez.Error.InvalidArguments
> > > =09
> > > org.bluez.Error.AlreadyConnected
> > > org.bluez.Error.HealthError
> > > =09
> > > void Disconnect()
> > > =09
> > > Disconnect from the remote application the state will also be
> > > deleted. And no future reconnections will be possible. For
> > > keeping the state the method Pause of the health link should be
> > > used.
> > > =09
> > > Possible errors: org.bluez.Error.InvalidArguments
> > > =09
> > > org.bluez.Error.NotFound
> > > org.bluez.Error.HealthError
> >=20
> > Do we need Connect() and Disconnect() here. Can we just not create these
> > connections in the background based of a reference counting via the
> > channels?
> >=20
> > > boolean Echo(array{byte})
> > > =09
> > > Sends an echo petition to the remote intance. Returns True if
> > > response matches with the buffer sent. If some error is detected
> > > False value is returned and the associated MCL is closed.
> > > =09
> > > path OpenDataChannel(byte mdepid, string conf)
> > > =09
> > > Creates a new data channel with the indicated config to the
> > > remote MCAP Data End Point (MDEP).
> > > The configuration should indicate the channel quality of
> > > service using one of this values "reliable", "streaming", "any".
> > > =09
> > > Returns the data channel path.
> > > =09
> > > Possible errors: org.bluez.Error.InvalidArguments
> > > =09
> > > org.bluez.Error.HealthError
> > > =09
> > > void ReconnectDataChannel(path data_channel)
> > > =09
> > > Reconnects a previously created data channel indicated by its
> > > path.
> > > =09
> > > Possible errors: org.bluez.Error.InvalidArguments
> > > =09
> > > org.bluez.Error.HealthError
> > > org.bluez.Error.NotFound
> > > =09
> > > int GetDataChannelFileDescriptor(path data_channel)
> > > =09
> > > Gets a file descriptor where data can be read or written.
> > > =09
> > > Possible errors: org.bluez.Error.InvalidArguments
> > > =09
> > > org.bluez.Error.NotFound
> > > org.bluez.Error.HealthError
> > > =09
> > > void DeleteDataChannel(path data_channel)
> > > =09
> > > Deletes a data channel so it will not be available to use.
> > > =09
> > > Possible errors: org.bluez.Error.InvalidArguments
> > > =09
> > > org.bluez.Error.NotFound
> > > org.bluez.Error.HealthError
> > > =09
> > > void DeleteAllDataChannels()
> > > =09
> > > Deletes all data channels so they will not be available for
> > > future use. Typically this function is called when the
> > > connection with the remote device will be closed permanently.
> > > =09
> > > Possible errors: org.bluez.Error.HealthError
> >=20
> > This actually means also Disconnect() to me. So clear the extra work of
> > connect and disconnect can be done in the background and invisible for
> > the user.
> >=20
> > > dict GetDataChannelStatus()
> > > =09
> > > Return a dictionary with all the data channels that can be used
> > > to send data right now. The dictionary is formed like follows:
> > > =09
> > > {
> > > =09
> > > "reliable": [channel_path_r1, channel_path_r2, ...],
> > > "streaming" : [channel_path_s1, channel_path_s2, ...]
> > > =09
> > > }
> > > =09
> > > The fist reliable data channel will always be the first data
> > > channel in reliable array.
>=20
> You don't need this method, DataChannel could be a Property and then you
> get this via GetProperties().
The problem is that properties are public and any process could access the=
=20
data channel status of the link even when they did not created the applicat=
ion=20
that is using the data channels. BTW this method can be removed because all=
=20
the data channel creation and deletion are notified to the agent so it alre=
ady=20
has this information.
^ permalink raw reply
* Re: Proposed API for HDP
From: Santiago Carot-Nemesio @ 2010-07-08 19:50 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: José Antonio Santos Cadenas, linux-bluetooth
In-Reply-To: <1278616520.10421.67.camel@localhost.localdomain>
Hi,
El jue, 08-07-2010 a las 16:15 -0300, Marcel Holtmann escribió:
> Hi Jose,
>
> > > > Health Device Profile hierarchy
> > > > ===============================
> > > >
> > > > Service org.bluez
> > > > Interface org.bluez.HealthAdapter
> > > > Object path [variable prefix]/{hci0,hci1,...}
> > >
> > > so I changed my mind here. Basing this on the local adapter is rather
> > > pointless.
> > >
> > > Lets just do org.bluez.HealthManager on /org/bluez object path. There is
> > > no need that the calling application knows anything about the specific
> > > adapters in our system. We properly separate them anyway during paring.
> > >
> > > Only the application that does the initial pairing with a remote health
> > > device needs to know which adapter to use. For the actual health
> > > application it is pointless since it will be notified about the paired
> > > health device initially.
> >
> > In case we don't use an adapter how can we know were the connections are
> > waited. ¿In all the adapters? Remember that this is going to create an SDP
> > record and also open l2cap sockets waiting data channels.
>
> so you only need to wait on the adapters that have been paired with a
> remote device. However also it is just okay to listen via all adapters.
> I don't see a direct need to differentiate this.
>
> > > > Methods:
> > > > path CreateApplication(object path, dict config)
> > > >
> > > > Returns the path of the new created application. The path
> > > > parameter is the path of the object with the callbacks to
> > > > notify events (see org.bluez.HealthAgent at the end of this
> > > > document)
> > > > This petition starts an mcap instance and also register a proper
> > > > record in the SDP if is needed.
> > > >
> > > > Dict is defined as bellow:
> > > > {
> > > >
> > > > "end_points" : [{ (optional)
> > > >
> > > > "role" : ("source" or "sink"), (mandatory)
> > > > "specs" :[{ (mandatory)
> > > >
> > > > "data_type" : uint16, (mandatory)
> > > > "description" : string, (optional)
> > > >
> > > > }]
> > > >
> > > > }]
> > > >
> > > > }
> > > >
> > > > Application will be closed by the call or implicitly when the
> > > > programs leaves the bus.
> > > >
> > > > Possible errors: org.bluez.Error.InvalidArguments
> > > >
> > > > void ReleaseApplication(path application)
> > > >
> > > > Closes the HDP application identified by the object path. Also
> > > > application will be closed if the process that started it leaves
> > > > the bus. If there is a SDP record associated to this application
> > > > it will be removed.
> > > >
> > > > Possible errors: org.bluez.Error.InvalidArguments
> > > >
> > > > org.bluez.Error.NotFound
> > >
> > > Since we now make this as part of a generic manager, the method class
> > > RegisterApplication and UnregisterApplication are a lot better choice.
> > >
> > > > array GetRemoteApplications(path application)
> > > >
> > > > This method will return an array with the paths of all the
> > > > remote instances found in remote devices.
> > > >
> > > > Possible errors: org.bluez.Error.InvalidArguments
> > > >
> > > > org.bluez.Error.NotFound
> > >
> > > We don't wanna do that. When you register your application the first
> > > callback via the agent should be telling what remote instances are
> > > available.
> > >
> > > This has the advantage that the code flow for the application is
> > > simpler. It just has to listen to that update. And if you register your
> > > application before pairing with a new device, it will still work. So no
> > > extra work to listen for new devices and bootstrapping an existing list.
> > >
> > > > -------------------------------------------------------------------------
> > > > -------
> > > >
> > > > Service org.bluez
> > > > Interface org.bluez.HealthDevice
> > > > Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > >
> > > This is not really a health device. As mentioned yesterday, we can have
> > > multiple health service per remote device. So we should be using here
> > > are org.bluez.HealthService for every SDP record for HDP inside the
> > > remote device.
> > >
> > > So potential object paths are .../hci0/dev_xxxxxxxx/{hdp0,hdp1,...} and
> > > so on. This way we clearly map health service and not bother with remote
> > > device details that might implement multiple functions.
> >
> > This object is created on each adapter for refreshing the SDP records. It is
> > supposed to search again for HDP records on the device and notify them to the
> > proper agents.
>
> That can be done automatically via existing BlueZ D-Bus APIs. I don't
> think there is need to have specific functionality.
>
> > > > Methods:
> > > > void Refresh()
> > > >
> > > > This method searches for HDP applications in the remote device
> > > > and notifies them to the appropriate agents.
> > >
> > > I might have called this Update(), but that is a minor detail.
> >
> > Ok
> >
> > >
> > > > -------------------------------------------------------------------------
> > > > -------
> > > >
> > > > Service org.bluez
> > > > Interface org.bluez.HealthDeviceApplication
> > > > Object path [variable
> > > > prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
> > >
> > > That is more like the org.bluez.HealthService as mentioned above. So
> > > lets combine them. I don't see a need for splitting these.
> >
> > As I mentioned above, I think next methods and the previous one are different.
> > Is a little bit ugly but I think that is necessary to have a way for check
> > again the SDP records looking for new remote instances (or applications).
> >
> > >
> > > > Methods:
> > > > array GetProperties()
> > > >
> > > > Gets the information of the remote application published on its
> > > > SDP record. The returned data format is as follows:
> > > >
> > > > {
> > > >
> > > > "end_points": [
> > > >
> > > > "mdepid": uint8,
> > > > "role" : "source" or "sink" ,
> > > > "specs" : [{
> > > >
> > > > "dtype" : uint16,
> > > > "description" : string, (optional)
> > > > }]
> > > >
> > > > ]
> > > >
> > > > }
> > > >
> > > > object Connect(path local_application_id)
> > > >
> > > > Connects the local application with the remote application.
> > > >
> > > > Only the bus client that created the local session will be able
> > > > to create connections using it.
> > > >
> > > > If the Device is already connected with an other application an
> > > > org.bluez.Error.AlreadyConnected error will be received.
> > > >
> > > > Possible errors: org.bluez.Error.InvalidArguments
> > > >
> > > > org.bluez.Error.AlreadyConnected
> > > > org.bluez.Error.HealthError
> > > >
> > > > void Disconnect()
> > > >
> > > > Disconnect from the remote application the state will also be
> > > > deleted. And no future reconnections will be possible. For
> > > > keeping the state the method Pause of the health link should be
> > > > used.
> > > >
> > > > Possible errors: org.bluez.Error.InvalidArguments
> > > >
> > > > org.bluez.Error.NotFound
> > > > org.bluez.Error.HealthError
> > >
> > > Do we need Connect() and Disconnect() here. Can we just not create these
> > > connections in the background based of a reference counting via the
> > > channels?
> >
> > This functions offer an abstraction to create a mcl and removing it from
> > cache. As status is maintained even when the connection is off. So Connect is
> > something like start keeping state and Disconnect is like stop keeping state.
>
> I don't see a need here. That can be hidden from the application. I
> would make the channel look persistent and not bother with manual
> connect of the MCL entity.
>
> > > > boolean Echo(array{byte})
> > > >
> > > > Sends an echo petition to the remote intance. Returns True if
> > > > response matches with the buffer sent. If some error is detected
> > > > False value is returned and the associated MCL is closed.
> > > >
> > > > path OpenDataChannel(byte mdepid, string conf)
> > > >
> > > > Creates a new data channel with the indicated config to the
> > > > remote MCAP Data End Point (MDEP).
> > > > The configuration should indicate the channel quality of
> > > > service using one of this values "reliable", "streaming", "any".
> > > >
> > > > Returns the data channel path.
> > > >
> > > > Possible errors: org.bluez.Error.InvalidArguments
> > > >
> > > > org.bluez.Error.HealthError
> > > >
> > > > void ReconnectDataChannel(path data_channel)
> > > >
> > > > Reconnects a previously created data channel indicated by its
> > > > path.
> > > >
> > > > Possible errors: org.bluez.Error.InvalidArguments
> > > >
> > > > org.bluez.Error.HealthError
> > > > org.bluez.Error.NotFound
> > > >
> > > > int GetDataChannelFileDescriptor(path data_channel)
> > > >
> > > > Gets a file descriptor where data can be read or written.
> > > >
> > > > Possible errors: org.bluez.Error.InvalidArguments
> > > >
> > > > org.bluez.Error.NotFound
> > > > org.bluez.Error.HealthError
> > > >
> > > > void DeleteDataChannel(path data_channel)
> > > >
> > > > Deletes a data channel so it will not be available to use.
> > > >
> > > > Possible errors: org.bluez.Error.InvalidArguments
> > > >
> > > > org.bluez.Error.NotFound
> > > > org.bluez.Error.HealthError
> > > >
> > > > void DeleteAllDataChannels()
> > > >
> > > > Deletes all data channels so they will not be available for
> > > > future use. Typically this function is called when the
> > > > connection with the remote device will be closed permanently.
> > > >
> > > > Possible errors: org.bluez.Error.HealthError
> > >
> > > This actually means also Disconnect() to me. So clear the extra work of
> > > connect and disconnect can be done in the background and invisible for
> > > the user.
> >
> > It is not just the same. Probably the explanations is not very pointless. This
> > is a way for deleting all data channels but keeping the connection active
> > (waiting for more channel creation)
>
> That is fine, but way? We can use and idle timeout to keep the
> connection around and disconnect it after inactivity. That is a good way
> of handling this anyway. And if disconnect, then the other side just
> needs to reestablish it again. Happens. I don't want the upper layer to
> worry about these details.
Just to clear this point, Why do we want to delete all data channels and
preserve active the MCL connection?, it is because the health device
doesnt want to send any data but it still may want to send
synchronization commands (CSP).
>
> > >
> > > > dict GetDataChannelStatus()
> > > >
> > > > Return a dictionary with all the data channels that can be used
> > > > to send data right now. The dictionary is formed like follows:
> > > >
> > > > {
> > > >
> > > > "reliable": [channel_path_r1, channel_path_r2, ...],
> > > > "streaming" : [channel_path_s1, channel_path_s2, ...]
> > > >
> > > > }
> > > >
> > > > The fist reliable data channel will always be the first data
> > > > channel in reliable array.
> > > >
> > > > HealthAgent hierarchy
> > > > =====================
> > > >
> > > > (this object is implemented by the HDP user in order to receive
> > > > notifications)
> > > >
> > > > Service unique name
> > > > Interface org.bluez.HealthAgent
> > > > Object path freely definable
> > > >
> > > > Methods:
> > > > void DeviceApplicationDiscovered(object path)
> > > >
> > > > This method is called when a device containing an hdp
> > > > application is connected. The object path is the application
> > > > path. The method will be called one time for each
> > > > application.
> > >
> > > I think this should be ServiceDiscovered and map to a HealthService.
> >
> > Ok
> >
> > >
> > > > void DeviceConnected(object path)
> > > >
> > > > This method is called whenever a new connection has been
> > > > established over the control channel of the current HDP
> > > > application. The object path paremeter contains the object path
> > > > of the connected HealthDevice.
> > >
> > > Don't see a useful need for this. I don't want to expose HealthDevice
> > > details anyway.
> >
> > What this tries to mean is that this HealthService has connected with the
> > local Application so the application can open data channels with it now.
>
> And why does it care? It only cares when a channel has been created.
> What is the benefit about telling about a physical link that has no
> users yet.
>
> >
> > >
> > > > void DevicePaused(object path)
> > > >
> > > > This method is called when a MCL is closed. Future reconnections
> > > > will be notified using the DeviceRestarted callback.
> > > > All data channels associated to this device will be closed and
> > > > a reconnection will be needed before using them again.
> > > >
> > > > void DeviceResumed(object path)
> > > >
> > > > This method is called whenever a MCL is reconnected. All data
> > > > channels associated are still closed but they will be able to be
> > > > reconnected skipping the configuration process.
> > > >
> > > > void DeviceDisconnected(object path)
> > > >
> > > > This method is called when a remote device is disconnected or
> > > > removed from MCAP cache. Any future reconnections will fail.
> > > > Also all data channels associated to this device will be closed.
> > >
> > > Why bother with this. We can do this on channel level.
> >
> > The problem is that in some cases (when the remote is not publishing a record)
> > it is not possible to open data channels nor reconnecting the mcl so it is not
> > possible to make this automatically. The application should be concerned about
> > this issues to avoid this kind of operation in this cases.
>
> I can follow the reason. We can figure this out intelligent for the
> application. If we can't then neither will the application.
>
> >
> > >
> > > > void CreatedDataChannel(object path, path data_channel, string conf)
> > > >
> > > > This method is called when a new data channel is created.
> > > >
> > > > The path contains the object path of the HealthDeviceApplication
> > > > where the new connection is created, the data_channel is the
> > > > path for identify the data channel and conf is the quality of
> > > > service of the data channel ("reliable" or "streaming").
> > >
> > > DataChannelCreated please.
> > >
> > > > void DataChannelReconnected(object path, path data_channel, string
> > conf)
> > > >
> > > > This method is called when a closed data channel is reconnected
> > > > by the remote device.
> > > >
> > > > Conf will be "reliable" or "streaming".
> > > >
> > > > TThe path contains the object path of the
> > > > HealthDeviceApplication where the new connection is reconnected,
> > > > the data_channel is the path for identify the data channel and
> > > > conf is the quality of service of the data channel ("reliable"
> > > > or "streaming").
> > > >
> > > > void DeletedDataChannel(object path, path data_channel)
> > > >
> > > > This method is called when a data channel is deleted.
> > > >
> > > > After this call the data channel path will not be valid and can
> > > > be reused for future creation of data channels.
> > >
> > > DataChannelRemoved. We always map create with remove.
> > >
> > > > void DeletedAllDataChannels(object path)
> > > >
> > > > This method is called when all data channels are deleted.
> > > >
> > > > The path contains the object path of the HealthDeviceApplication
> > > > where the data channels are deleted.
> > >
> > > That is pointless. You will get separate callbacks for each channel
> > > anyway.
> >
> > We tried to avoid calling may times to the same callback when a delete all
> > operation is done, but of course it is not necessary.
>
> I would normally agree, but about how many channels are we talking here.
> 5, 10 or 200. Since I assume this is mostly 2-3 channels sending it
> multiple times on cleanup just makes the code flow inside the
> application a lot simpler. That is what we want. The extra payload on
> the D-Bus can be neglected her in favor of a simpler application work
> flow.
>
> Regards
>
> Marcel
>
>
> --
> 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
^ permalink raw reply
* Re: Proposed API for HDP
From: Gustavo F. Padovan @ 2010-07-08 19:17 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: José Antonio Santos Cadenas, linux-bluetooth
In-Reply-To: <1278610788.10421.51.camel@localhost.localdomain>
>
> > --------------------------------------------------------------------------------
> >
> > Service org.bluez
> > Interface org.bluez.HealthDeviceApplication
> > Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
> >
>
> That is more like the org.bluez.HealthService as mentioned above. So
> lets combine them. I don't see a need for splitting these.
>
> > Methods:
> >
> > array GetProperties()
> >
> > Gets the information of the remote application published on its
> > SDP record. The returned data format is as follows:
> >
> > {
> > "end_points": [
> > "mdepid": uint8,
> > "role" : "source" or "sink" ,
> > "specs" : [{
> > "dtype" : uint16,
> > "description" : string, (optional)
> > }]
> > ]
> > }
> >
> > object Connect(path local_application_id)
> >
> > Connects the local application with the remote application.
> >
> > Only the bus client that created the local session will be able
> > to create connections using it.
> >
> > If the Device is already connected with an other application an
> > org.bluez.Error.AlreadyConnected error will be received.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> > org.bluez.Error.AlreadyConnected
> > org.bluez.Error.HealthError
> >
> > void Disconnect()
> >
> > Disconnect from the remote application the state will also be
> > deleted. And no future reconnections will be possible. For
> > keeping the state the method Pause of the health link should be
> > used.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> > org.bluez.Error.NotFound
> > org.bluez.Error.HealthError
>
> Do we need Connect() and Disconnect() here. Can we just not create these
> connections in the background based of a reference counting via the
> channels?
>
> > boolean Echo(array{byte})
> >
> > Sends an echo petition to the remote intance. Returns True if
> > response matches with the buffer sent. If some error is detected
> > False value is returned and the associated MCL is closed.
> >
> > path OpenDataChannel(byte mdepid, string conf)
> >
> > Creates a new data channel with the indicated config to the
> > remote MCAP Data End Point (MDEP).
> > The configuration should indicate the channel quality of
> > service using one of this values "reliable", "streaming", "any".
> >
> > Returns the data channel path.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> > org.bluez.Error.HealthError
> >
> > void ReconnectDataChannel(path data_channel)
> >
> > Reconnects a previously created data channel indicated by its
> > path.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> > org.bluez.Error.HealthError
> > org.bluez.Error.NotFound
> >
> > int GetDataChannelFileDescriptor(path data_channel)
> >
> > Gets a file descriptor where data can be read or written.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> > org.bluez.Error.NotFound
> > org.bluez.Error.HealthError
> >
> > void DeleteDataChannel(path data_channel)
> >
> > Deletes a data channel so it will not be available to use.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> > org.bluez.Error.NotFound
> > org.bluez.Error.HealthError
> >
> > void DeleteAllDataChannels()
> >
> > Deletes all data channels so they will not be available for
> > future use. Typically this function is called when the
> > connection with the remote device will be closed permanently.
> >
> > Possible errors: org.bluez.Error.HealthError
>
> This actually means also Disconnect() to me. So clear the extra work of
> connect and disconnect can be done in the background and invisible for
> the user.
>
> > dict GetDataChannelStatus()
> >
> > Return a dictionary with all the data channels that can be used
> > to send data right now. The dictionary is formed like follows:
> >
> > {
> > "reliable": [channel_path_r1, channel_path_r2, ...],
> > "streaming" : [channel_path_s1, channel_path_s2, ...]
> > }
> >
> > The fist reliable data channel will always be the first data
> > channel in reliable array.
> >
You don't need this method, DataChannel could be a Property and then you
get this via GetProperties().
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: Proposed API for HDP
From: Marcel Holtmann @ 2010-07-08 19:15 UTC (permalink / raw)
To: José Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <201007082033.04466.santoscadenas@gmail.com>
Hi Jose,
> > > Health Device Profile hierarchy
> > > ===============================
> > >
> > > Service org.bluez
> > > Interface org.bluez.HealthAdapter
> > > Object path [variable prefix]/{hci0,hci1,...}
> >
> > so I changed my mind here. Basing this on the local adapter is rather
> > pointless.
> >
> > Lets just do org.bluez.HealthManager on /org/bluez object path. There is
> > no need that the calling application knows anything about the specific
> > adapters in our system. We properly separate them anyway during paring.
> >
> > Only the application that does the initial pairing with a remote health
> > device needs to know which adapter to use. For the actual health
> > application it is pointless since it will be notified about the paired
> > health device initially.
>
> In case we don't use an adapter how can we know were the connections are
> waited. ¿In all the adapters? Remember that this is going to create an SDP
> record and also open l2cap sockets waiting data channels.
so you only need to wait on the adapters that have been paired with a
remote device. However also it is just okay to listen via all adapters.
I don't see a direct need to differentiate this.
> > > Methods:
> > > path CreateApplication(object path, dict config)
> > >
> > > Returns the path of the new created application. The path
> > > parameter is the path of the object with the callbacks to
> > > notify events (see org.bluez.HealthAgent at the end of this
> > > document)
> > > This petition starts an mcap instance and also register a proper
> > > record in the SDP if is needed.
> > >
> > > Dict is defined as bellow:
> > > {
> > >
> > > "end_points" : [{ (optional)
> > >
> > > "role" : ("source" or "sink"), (mandatory)
> > > "specs" :[{ (mandatory)
> > >
> > > "data_type" : uint16, (mandatory)
> > > "description" : string, (optional)
> > >
> > > }]
> > >
> > > }]
> > >
> > > }
> > >
> > > Application will be closed by the call or implicitly when the
> > > programs leaves the bus.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > void ReleaseApplication(path application)
> > >
> > > Closes the HDP application identified by the object path. Also
> > > application will be closed if the process that started it leaves
> > > the bus. If there is a SDP record associated to this application
> > > it will be removed.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.NotFound
> >
> > Since we now make this as part of a generic manager, the method class
> > RegisterApplication and UnregisterApplication are a lot better choice.
> >
> > > array GetRemoteApplications(path application)
> > >
> > > This method will return an array with the paths of all the
> > > remote instances found in remote devices.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.NotFound
> >
> > We don't wanna do that. When you register your application the first
> > callback via the agent should be telling what remote instances are
> > available.
> >
> > This has the advantage that the code flow for the application is
> > simpler. It just has to listen to that update. And if you register your
> > application before pairing with a new device, it will still work. So no
> > extra work to listen for new devices and bootstrapping an existing list.
> >
> > > -------------------------------------------------------------------------
> > > -------
> > >
> > > Service org.bluez
> > > Interface org.bluez.HealthDevice
> > > Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> >
> > This is not really a health device. As mentioned yesterday, we can have
> > multiple health service per remote device. So we should be using here
> > are org.bluez.HealthService for every SDP record for HDP inside the
> > remote device.
> >
> > So potential object paths are .../hci0/dev_xxxxxxxx/{hdp0,hdp1,...} and
> > so on. This way we clearly map health service and not bother with remote
> > device details that might implement multiple functions.
>
> This object is created on each adapter for refreshing the SDP records. It is
> supposed to search again for HDP records on the device and notify them to the
> proper agents.
That can be done automatically via existing BlueZ D-Bus APIs. I don't
think there is need to have specific functionality.
> > > Methods:
> > > void Refresh()
> > >
> > > This method searches for HDP applications in the remote device
> > > and notifies them to the appropriate agents.
> >
> > I might have called this Update(), but that is a minor detail.
>
> Ok
>
> >
> > > -------------------------------------------------------------------------
> > > -------
> > >
> > > Service org.bluez
> > > Interface org.bluez.HealthDeviceApplication
> > > Object path [variable
> > > prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
> >
> > That is more like the org.bluez.HealthService as mentioned above. So
> > lets combine them. I don't see a need for splitting these.
>
> As I mentioned above, I think next methods and the previous one are different.
> Is a little bit ugly but I think that is necessary to have a way for check
> again the SDP records looking for new remote instances (or applications).
>
> >
> > > Methods:
> > > array GetProperties()
> > >
> > > Gets the information of the remote application published on its
> > > SDP record. The returned data format is as follows:
> > >
> > > {
> > >
> > > "end_points": [
> > >
> > > "mdepid": uint8,
> > > "role" : "source" or "sink" ,
> > > "specs" : [{
> > >
> > > "dtype" : uint16,
> > > "description" : string, (optional)
> > > }]
> > >
> > > ]
> > >
> > > }
> > >
> > > object Connect(path local_application_id)
> > >
> > > Connects the local application with the remote application.
> > >
> > > Only the bus client that created the local session will be able
> > > to create connections using it.
> > >
> > > If the Device is already connected with an other application an
> > > org.bluez.Error.AlreadyConnected error will be received.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.AlreadyConnected
> > > org.bluez.Error.HealthError
> > >
> > > void Disconnect()
> > >
> > > Disconnect from the remote application the state will also be
> > > deleted. And no future reconnections will be possible. For
> > > keeping the state the method Pause of the health link should be
> > > used.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.NotFound
> > > org.bluez.Error.HealthError
> >
> > Do we need Connect() and Disconnect() here. Can we just not create these
> > connections in the background based of a reference counting via the
> > channels?
>
> This functions offer an abstraction to create a mcl and removing it from
> cache. As status is maintained even when the connection is off. So Connect is
> something like start keeping state and Disconnect is like stop keeping state.
I don't see a need here. That can be hidden from the application. I
would make the channel look persistent and not bother with manual
connect of the MCL entity.
> > > boolean Echo(array{byte})
> > >
> > > Sends an echo petition to the remote intance. Returns True if
> > > response matches with the buffer sent. If some error is detected
> > > False value is returned and the associated MCL is closed.
> > >
> > > path OpenDataChannel(byte mdepid, string conf)
> > >
> > > Creates a new data channel with the indicated config to the
> > > remote MCAP Data End Point (MDEP).
> > > The configuration should indicate the channel quality of
> > > service using one of this values "reliable", "streaming", "any".
> > >
> > > Returns the data channel path.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.HealthError
> > >
> > > void ReconnectDataChannel(path data_channel)
> > >
> > > Reconnects a previously created data channel indicated by its
> > > path.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.HealthError
> > > org.bluez.Error.NotFound
> > >
> > > int GetDataChannelFileDescriptor(path data_channel)
> > >
> > > Gets a file descriptor where data can be read or written.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.NotFound
> > > org.bluez.Error.HealthError
> > >
> > > void DeleteDataChannel(path data_channel)
> > >
> > > Deletes a data channel so it will not be available to use.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.NotFound
> > > org.bluez.Error.HealthError
> > >
> > > void DeleteAllDataChannels()
> > >
> > > Deletes all data channels so they will not be available for
> > > future use. Typically this function is called when the
> > > connection with the remote device will be closed permanently.
> > >
> > > Possible errors: org.bluez.Error.HealthError
> >
> > This actually means also Disconnect() to me. So clear the extra work of
> > connect and disconnect can be done in the background and invisible for
> > the user.
>
> It is not just the same. Probably the explanations is not very pointless. This
> is a way for deleting all data channels but keeping the connection active
> (waiting for more channel creation)
That is fine, but way? We can use and idle timeout to keep the
connection around and disconnect it after inactivity. That is a good way
of handling this anyway. And if disconnect, then the other side just
needs to reestablish it again. Happens. I don't want the upper layer to
worry about these details.
> >
> > > dict GetDataChannelStatus()
> > >
> > > Return a dictionary with all the data channels that can be used
> > > to send data right now. The dictionary is formed like follows:
> > >
> > > {
> > >
> > > "reliable": [channel_path_r1, channel_path_r2, ...],
> > > "streaming" : [channel_path_s1, channel_path_s2, ...]
> > >
> > > }
> > >
> > > The fist reliable data channel will always be the first data
> > > channel in reliable array.
> > >
> > > HealthAgent hierarchy
> > > =====================
> > >
> > > (this object is implemented by the HDP user in order to receive
> > > notifications)
> > >
> > > Service unique name
> > > Interface org.bluez.HealthAgent
> > > Object path freely definable
> > >
> > > Methods:
> > > void DeviceApplicationDiscovered(object path)
> > >
> > > This method is called when a device containing an hdp
> > > application is connected. The object path is the application
> > > path. The method will be called one time for each
> > > application.
> >
> > I think this should be ServiceDiscovered and map to a HealthService.
>
> Ok
>
> >
> > > void DeviceConnected(object path)
> > >
> > > This method is called whenever a new connection has been
> > > established over the control channel of the current HDP
> > > application. The object path paremeter contains the object path
> > > of the connected HealthDevice.
> >
> > Don't see a useful need for this. I don't want to expose HealthDevice
> > details anyway.
>
> What this tries to mean is that this HealthService has connected with the
> local Application so the application can open data channels with it now.
And why does it care? It only cares when a channel has been created.
What is the benefit about telling about a physical link that has no
users yet.
>
> >
> > > void DevicePaused(object path)
> > >
> > > This method is called when a MCL is closed. Future reconnections
> > > will be notified using the DeviceRestarted callback.
> > > All data channels associated to this device will be closed and
> > > a reconnection will be needed before using them again.
> > >
> > > void DeviceResumed(object path)
> > >
> > > This method is called whenever a MCL is reconnected. All data
> > > channels associated are still closed but they will be able to be
> > > reconnected skipping the configuration process.
> > >
> > > void DeviceDisconnected(object path)
> > >
> > > This method is called when a remote device is disconnected or
> > > removed from MCAP cache. Any future reconnections will fail.
> > > Also all data channels associated to this device will be closed.
> >
> > Why bother with this. We can do this on channel level.
>
> The problem is that in some cases (when the remote is not publishing a record)
> it is not possible to open data channels nor reconnecting the mcl so it is not
> possible to make this automatically. The application should be concerned about
> this issues to avoid this kind of operation in this cases.
I can follow the reason. We can figure this out intelligent for the
application. If we can't then neither will the application.
>
> >
> > > void CreatedDataChannel(object path, path data_channel, string conf)
> > >
> > > This method is called when a new data channel is created.
> > >
> > > The path contains the object path of the HealthDeviceApplication
> > > where the new connection is created, the data_channel is the
> > > path for identify the data channel and conf is the quality of
> > > service of the data channel ("reliable" or "streaming").
> >
> > DataChannelCreated please.
> >
> > > void DataChannelReconnected(object path, path data_channel, string
> conf)
> > >
> > > This method is called when a closed data channel is reconnected
> > > by the remote device.
> > >
> > > Conf will be "reliable" or "streaming".
> > >
> > > TThe path contains the object path of the
> > > HealthDeviceApplication where the new connection is reconnected,
> > > the data_channel is the path for identify the data channel and
> > > conf is the quality of service of the data channel ("reliable"
> > > or "streaming").
> > >
> > > void DeletedDataChannel(object path, path data_channel)
> > >
> > > This method is called when a data channel is deleted.
> > >
> > > After this call the data channel path will not be valid and can
> > > be reused for future creation of data channels.
> >
> > DataChannelRemoved. We always map create with remove.
> >
> > > void DeletedAllDataChannels(object path)
> > >
> > > This method is called when all data channels are deleted.
> > >
> > > The path contains the object path of the HealthDeviceApplication
> > > where the data channels are deleted.
> >
> > That is pointless. You will get separate callbacks for each channel
> > anyway.
>
> We tried to avoid calling may times to the same callback when a delete all
> operation is done, but of course it is not necessary.
I would normally agree, but about how many channels are we talking here.
5, 10 or 200. Since I assume this is mostly 2-3 channels sending it
multiple times on cleanup just makes the code flow inside the
application a lot simpler. That is what we want. The extra payload on
the D-Bus can be neglected her in favor of a simpler application work
flow.
Regards
Marcel
^ permalink raw reply
* Re: Proposed API for HDP
From: Gustavo F. Padovan @ 2010-07-08 19:13 UTC (permalink / raw)
To: José Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <201007082036.00289.santoscadenas@gmail.com>
Hi José,
* José Antonio Santos Cadenas <santoscadenas@gmail.com> [2010-07-08 20:36:00 +0200]:
> El Thursday 08 July 2010 19:54:30 Gustavo F. Padovan escribió:
> > Hi José,
> >
> > * José Antonio Santos Cadenas <santoscadenas@gmail.com> [2010-07-08
> > 19:12:31 +0200]:
> >
> > <snip>
> >
> > > Service org.bluez
> > > Interface org.bluez.HealthDeviceApplication
> > > Object path [variable
> > > prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
> > >
> > > Methods:
> > > array GetProperties()
> > >
> > > Gets the information of the remote application published on its
> > > SDP record. The returned data format is as follows:
> > >
> > > {
> > >
> > > "end_points": [
> > >
> > > "mdepid": uint8,
> > > "role" : "source" or "sink" ,
> > > "specs" : [{
> > >
> > > "dtype" : uint16,
> > > "description" : string, (optional)
> > > }]
> > >
> > > ]
> > >
> > > }
> > >
> > > object Connect(path local_application_id)
> > >
> > > Connects the local application with the remote application.
> > >
> > > Only the bus client that created the local session will be able
> > > to create connections using it.
> > >
> > > If the Device is already connected with an other application an
> > > org.bluez.Error.AlreadyConnected error will be received.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.AlreadyConnected
> > > org.bluez.Error.HealthError
> > >
> > > void Disconnect()
> > >
> > > Disconnect from the remote application the state will also be
> > > deleted. And no future reconnections will be possible. For
> > > keeping the state the method Pause of the health link should be
> > > used.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.NotFound
> > > org.bluez.Error.HealthError
> > >
> > > boolean Echo(array{byte})
> > >
> > > Sends an echo petition to the remote intance. Returns True if
> > > response matches with the buffer sent. If some error is detected
> > > False value is returned and the associated MCL is closed.
> > >
> > > path OpenDataChannel(byte mdepid, string conf)
> > >
> > > Creates a new data channel with the indicated config to the
> > > remote MCAP Data End Point (MDEP).
> > > The configuration should indicate the channel quality of
> > > service using one of this values "reliable", "streaming", "any".
> > >
> > > Returns the data channel path.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.HealthError
> > >
> > > void ReconnectDataChannel(path data_channel)
> > >
> > > Reconnects a previously created data channel indicated by its
> > > path.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.HealthError
> > > org.bluez.Error.NotFound
> >
> > So are we really going to expose the temporally disconnect and
> > reconnection to the applicantion level? The application should not
> > care if the data channel is there or not.
>
> We changed our mind about this issue because there are cases were an implicit
> reconnection will not be possible (when the remote do not publish an SDP
> record) so if the application is concerned about this issues could try to
> avoid this kind of failure waiting for the remote to reconnect the data
> channel.
Is this better than simplify the API removing the Reconnection stuff?
On average how much time takes to reconnect achieve the timeout?
BTW, failure can occur on OpenDataChannel as well, that just happens.
>
> >
> > > int GetDataChannelFileDescriptor(path data_channel)
> > >
> > > Gets a file descriptor where data can be read or written.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.NotFound
> > > org.bluez.Error.HealthError
> >
> > What about the fd-passing feature of D-Bus here?
>
> There is an error here, the return value is not an integer is a file
> descriptor.
No. The fd should go to the Agent automatically, once you try to
open a data channel a callback in the Application should be registered.
The callback is a function in the Agent API with the fd as parameter,
that will be called once the connection in the lower layer is done.
See the NewConnection method from the HandsfreeAgent Interface.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: What is the motivation for conn->power_save
From: Nick Pelly @ 2010-07-08 18:37 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <AANLkTil8Hloz5I7Gpnp9M-y2W3jkLKeca4wz7dgFKA0V@mail.gmail.com>
On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
<andrei.emeltchenko.news@gmail.com> wrote:
> Hi,
>
> On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org> w=
rote:
>> Hi Nick,
>>
>>> >>> >> If I understand correctly, conn->power_save prevents the host st=
ack
>>> >>> >> from requesting active mode if it was not the host stack that
>>> >>> >> requested sniff mode.
>>> >>> >>
>>> >>> >> I don't understand the motivation for this. If we have ACL data =
to
>>> >>> >> send, then it seems like a good idea for the host stack to expli=
citly
>>> >>> >> request active mode, regardless of the reason that we entered sn=
iff
>>> >>> >> mode.
>>> >>> >>
>>> >>> >> We want to enter active mode more aggressively when setting up S=
CO
>>> >>> >> connections, to avoid a 5 second delay with certain sniff modes.=
But
>>> >>> >> the conn->power_save code is getting in the way and doesn't appe=
ar to
>>> >>> >> be useful in the first place.
>>> >>> >
>>> >>> > we have discussed this a few times. And if you lock through the c=
ode
>>> >>> > history then you see that initially we just took devices out of s=
niff
>>> >>> > mode if we had to send data. However with HID devices this falls =
flat on
>>> >>> > its face. They need to stay in control of sniff mode if they init=
iated
>>> >>> > it. Some of them crash and others just drain the battery. With sn=
iff
>>> >>> > mode you can send small amounts of data even while in sniff and f=
or HID
>>> >>> > that is sometimes used. So the remote side better not interfere.
>>> >>> >
>>> >>> > What we really need is a socket option where we can control this =
on a
>>> >>> > per socket basis if we take devices out of sniff mode. And one ex=
tra
>>> >>> > case might be when we try to establish a SCO channel, because the=
n it is
>>> >>> > clearly not an HID device. However even A2DP has this sort of pro=
blems
>>> >>> > sometimes where the stream setup takes time.
>>> >>>
>>> >>> Makes sense. Thanks for the explanation.
>>> >>
>>> >> this means you will be working on a patch for this :)
>>>
>>> Actually, I want to put a patch together for a socket option to not
>>> use power_save (so that we *always* exit sniff mode when sending ACL
>>> data). We're seeing this problem with the Plantronics Voyager 855
>>> which enters sniff mode during A2DP.
>>>
>>> Given that power_save is just for HID devices, my preferred design is
>>> to disable power_save by default, and have an L2CAP socket option to
>>> turn on power_save that would be used by HID L2CAP sockets.
>>> Unfortunately this would require userspace HID code to use the new
>>> socket option to keep current behavior. But it seems preferable to the
>>> alternative of having every single other L2CAP socket use a new socket
>>> option just to disable power_save for the sake of HID.
>>
>> actually you still mix up the meaning of the power_save option. From the
>> stack side, the automatic sniff mode is off by default. You have to
>> enable via sysfs first. The power_save option is just to control when
>> sniff mode is activated and the remote enters sniff mode, that we are
>> not getting out of it if we have ACL data.
>>
>> In conclusion, I am fine with a socket option that allows to control
>> sniff mode (preferable with interval details) and sniff subrate details
>> so that we can use them for that ACL link.
>>
>> So go ahead and work on this. We can fix userspace easily since a new
>> socket can be detected easily with newer kernels.
>
> We have found that some Nokia BT headsets stop working after idle
> period when they enter
> "power save" mode.
>
> Do we have any solution for this problem? So far I see good enough
> patch :-)) below:
>
> http://android.git.kernel.org/?p=3Dkernel/common.git;a=3Dblobdiff;f=3Dnet=
/bluetooth/hci_conn.c;h=3Dfa8b412205cd89546b4a325c558c68040eeaf491;hp=3D0cf=
25114a3f576fc2788a549eb96d0087dd39b44;hb=3Dd8488237646920cd71ef43e5f3ae1001=
c6f4cf7b;hpb=3D3f68e5050c5ae559f56d5da9202cb88928d42b36
>
> - =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
> + =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF /* || !conn->power_save */=
)
>
> Nick have you done socket option for "power_save"?
No. We'll do it once we need it for HID support.
Nick
^ permalink raw reply
* Re: Proposed API for HDP
From: José Antonio Santos Cadenas @ 2010-07-08 18:36 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <20100708175430.GB26759@vigoh>
El Thursday 08 July 2010 19:54:30 Gustavo F. Padovan escribió:
> Hi José,
>
> * José Antonio Santos Cadenas <santoscadenas@gmail.com> [2010-07-08
> 19:12:31 +0200]:
>
> <snip>
>
> > Service org.bluez
> > Interface org.bluez.HealthDeviceApplication
> > Object path [variable
> > prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
> >
> > Methods:
> > array GetProperties()
> >
> > Gets the information of the remote application published on its
> > SDP record. The returned data format is as follows:
> >
> > {
> >
> > "end_points": [
> >
> > "mdepid": uint8,
> > "role" : "source" or "sink" ,
> > "specs" : [{
> >
> > "dtype" : uint16,
> > "description" : string, (optional)
> > }]
> >
> > ]
> >
> > }
> >
> > object Connect(path local_application_id)
> >
> > Connects the local application with the remote application.
> >
> > Only the bus client that created the local session will be able
> > to create connections using it.
> >
> > If the Device is already connected with an other application an
> > org.bluez.Error.AlreadyConnected error will be received.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> >
> > org.bluez.Error.AlreadyConnected
> > org.bluez.Error.HealthError
> >
> > void Disconnect()
> >
> > Disconnect from the remote application the state will also be
> > deleted. And no future reconnections will be possible. For
> > keeping the state the method Pause of the health link should be
> > used.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> >
> > org.bluez.Error.NotFound
> > org.bluez.Error.HealthError
> >
> > boolean Echo(array{byte})
> >
> > Sends an echo petition to the remote intance. Returns True if
> > response matches with the buffer sent. If some error is detected
> > False value is returned and the associated MCL is closed.
> >
> > path OpenDataChannel(byte mdepid, string conf)
> >
> > Creates a new data channel with the indicated config to the
> > remote MCAP Data End Point (MDEP).
> > The configuration should indicate the channel quality of
> > service using one of this values "reliable", "streaming", "any".
> >
> > Returns the data channel path.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> >
> > org.bluez.Error.HealthError
> >
> > void ReconnectDataChannel(path data_channel)
> >
> > Reconnects a previously created data channel indicated by its
> > path.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> >
> > org.bluez.Error.HealthError
> > org.bluez.Error.NotFound
>
> So are we really going to expose the temporally disconnect and
> reconnection to the applicantion level? The application should not
> care if the data channel is there or not.
We changed our mind about this issue because there are cases were an implicit
reconnection will not be possible (when the remote do not publish an SDP
record) so if the application is concerned about this issues could try to
avoid this kind of failure waiting for the remote to reconnect the data
channel.
>
> > int GetDataChannelFileDescriptor(path data_channel)
> >
> > Gets a file descriptor where data can be read or written.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> >
> > org.bluez.Error.NotFound
> > org.bluez.Error.HealthError
>
> What about the fd-passing feature of D-Bus here?
There is an error here, the return value is not an integer is a file
descriptor.
Thanks for the comments.
Regards.
Jose.
^ permalink raw reply
* Re: Proposed API for HDP
From: José Antonio Santos Cadenas @ 2010-07-08 18:33 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1278610788.10421.51.camel@localhost.localdomain>
Hi Marcel,
El Thursday 08 July 2010 19:39:48 Marcel Holtmann escribi=F3:
> Hi Jose,
>=20
> > Health Device Profile hierarchy
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D
> >=20
> > Service org.bluez
> > Interface org.bluez.HealthAdapter
> > Object path [variable prefix]/{hci0,hci1,...}
>=20
> so I changed my mind here. Basing this on the local adapter is rather
> pointless.
>=20
> Lets just do org.bluez.HealthManager on /org/bluez object path. There is
> no need that the calling application knows anything about the specific
> adapters in our system. We properly separate them anyway during paring.
>=20
> Only the application that does the initial pairing with a remote health
> device needs to know which adapter to use. For the actual health
> application it is pointless since it will be notified about the paired
> health device initially.
In case we don't use an adapter how can we know were the connections are=20
waited. =BFIn all the adapters? Remember that this is going to create an SD=
P=20
record and also open l2cap sockets waiting data channels.
>=20
> > Methods:
> > path CreateApplication(object path, dict config)
> > =09
> > Returns the path of the new created application. The path
> > parameter is the path of the object with the callbacks to
> > notify events (see org.bluez.HealthAgent at the end of this
> > document)
> > This petition starts an mcap instance and also register a proper
> > record in the SDP if is needed.
> > =09
> > Dict is defined as bellow:
> > {
> > =09
> > "end_points" : [{ (optional)
> > =09
> > "role" : ("source" or "sink"), (mandatory)
> > "specs" :[{ (mandatory)
> > =09
> > "data_type" : uint16, (mandatory)
> > "description" : string, (optional)
> > =09
> > }]
> > =09
> > }]
> > =09
> > }
> > =09
> > Application will be closed by the call or implicitly when the
> > programs leaves the bus.
> > =09
> > Possible errors: org.bluez.Error.InvalidArguments
> > =09
> > void ReleaseApplication(path application)
> > =09
> > Closes the HDP application identified by the object path. Also
> > application will be closed if the process that started it leaves
> > the bus. If there is a SDP record associated to this application
> > it will be removed.
> > =09
> > Possible errors: org.bluez.Error.InvalidArguments
> > =09
> > org.bluez.Error.NotFound
>=20
> Since we now make this as part of a generic manager, the method class
> RegisterApplication and UnregisterApplication are a lot better choice.
>=20
> > array GetRemoteApplications(path application)
> > =09
> > This method will return an array with the paths of all the
> > remote instances found in remote devices.
> > =09
> > Possible errors: org.bluez.Error.InvalidArguments
> > =09
> > org.bluez.Error.NotFound
>=20
> We don't wanna do that. When you register your application the first
> callback via the agent should be telling what remote instances are
> available.
>=20
> This has the advantage that the code flow for the application is
> simpler. It just has to listen to that update. And if you register your
> application before pairing with a new device, it will still work. So no
> extra work to listen for new devices and bootstrapping an existing list.
>=20
> > -----------------------------------------------------------------------=
=2D-
> > -------
> >=20
> > Service org.bluez
> > Interface org.bluez.HealthDevice
> > Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
>=20
> This is not really a health device. As mentioned yesterday, we can have
> multiple health service per remote device. So we should be using here
> are org.bluez.HealthService for every SDP record for HDP inside the
> remote device.
>=20
> So potential object paths are .../hci0/dev_xxxxxxxx/{hdp0,hdp1,...} and
> so on. This way we clearly map health service and not bother with remote
> device details that might implement multiple functions.
This object is created on each adapter for refreshing the SDP records. It i=
s=20
supposed to search again for HDP records on the device and notify them to t=
he=20
proper agents.
>=20
> > Methods:
> > void Refresh()
> > =09
> > This method searches for HDP applications in the remote device
> > and notifies them to the appropriate agents.
>=20
> I might have called this Update(), but that is a minor detail.
Ok
>=20
> > -----------------------------------------------------------------------=
=2D-
> > -------
> >=20
> > Service org.bluez
> > Interface org.bluez.HealthDeviceApplication
> > Object path [variable
> > prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
>=20
> That is more like the org.bluez.HealthService as mentioned above. So
> lets combine them. I don't see a need for splitting these.
As I mentioned above, I think next methods and the previous one are differe=
nt.=20
Is a little bit ugly but I think that is necessary to have a way for check=
=20
again the SDP records looking for new remote instances (or applications).
>=20
> > Methods:
> > array GetProperties()
> > =09
> > Gets the information of the remote application published on its
> > SDP record. The returned data format is as follows:
> > =09
> > {
> > =09
> > "end_points": [
> > =09
> > "mdepid": uint8,
> > "role" : "source" or "sink" ,
> > "specs" : [{
> > =09
> > "dtype" : uint16,
> > "description" : string, (optional)
> > }]
> > =09
> > ]
> > =09
> > }
> > =09
> > object Connect(path local_application_id)
> > =09
> > Connects the local application with the remote application.
> > =09
> > Only the bus client that created the local session will be able
> > to create connections using it.
> > =09
> > If the Device is already connected with an other application an
> > org.bluez.Error.AlreadyConnected error will be received.
> > =09
> > Possible errors: org.bluez.Error.InvalidArguments
> > =09
> > org.bluez.Error.AlreadyConnected
> > org.bluez.Error.HealthError
> > =09
> > void Disconnect()
> > =09
> > Disconnect from the remote application the state will also be
> > deleted. And no future reconnections will be possible. For
> > keeping the state the method Pause of the health link should be
> > used.
> > =09
> > Possible errors: org.bluez.Error.InvalidArguments
> > =09
> > org.bluez.Error.NotFound
> > org.bluez.Error.HealthError
>=20
> Do we need Connect() and Disconnect() here. Can we just not create these
> connections in the background based of a reference counting via the
> channels?
This functions offer an abstraction to create a mcl and removing it from=20
cache. As status is maintained even when the connection is off. So Connect =
is=20
something like start keeping state and Disconnect is like stop keeping stat=
e.
>=20
> > boolean Echo(array{byte})
> > =09
> > Sends an echo petition to the remote intance. Returns True if
> > response matches with the buffer sent. If some error is detected
> > False value is returned and the associated MCL is closed.
> > =09
> > path OpenDataChannel(byte mdepid, string conf)
> > =09
> > Creates a new data channel with the indicated config to the
> > remote MCAP Data End Point (MDEP).
> > The configuration should indicate the channel quality of
> > service using one of this values "reliable", "streaming", "any".
> > =09
> > Returns the data channel path.
> > =09
> > Possible errors: org.bluez.Error.InvalidArguments
> > =09
> > org.bluez.Error.HealthError
> > =09
> > void ReconnectDataChannel(path data_channel)
> > =09
> > Reconnects a previously created data channel indicated by its
> > path.
> > =09
> > Possible errors: org.bluez.Error.InvalidArguments
> > =09
> > org.bluez.Error.HealthError
> > org.bluez.Error.NotFound
> > =09
> > int GetDataChannelFileDescriptor(path data_channel)
> > =09
> > Gets a file descriptor where data can be read or written.
> > =09
> > Possible errors: org.bluez.Error.InvalidArguments
> > =09
> > org.bluez.Error.NotFound
> > org.bluez.Error.HealthError
> > =09
> > void DeleteDataChannel(path data_channel)
> > =09
> > Deletes a data channel so it will not be available to use.
> > =09
> > Possible errors: org.bluez.Error.InvalidArguments
> > =09
> > org.bluez.Error.NotFound
> > org.bluez.Error.HealthError
> > =09
> > void DeleteAllDataChannels()
> > =09
> > Deletes all data channels so they will not be available for
> > future use. Typically this function is called when the
> > connection with the remote device will be closed permanently.
> > =09
> > Possible errors: org.bluez.Error.HealthError
>=20
> This actually means also Disconnect() to me. So clear the extra work of
> connect and disconnect can be done in the background and invisible for
> the user.
It is not just the same. Probably the explanations is not very pointless. T=
his=20
is a way for deleting all data channels but keeping the connection active=20
(waiting for more channel creation)
>=20
> > dict GetDataChannelStatus()
> > =09
> > Return a dictionary with all the data channels that can be used
> > to send data right now. The dictionary is formed like follows:
> > =09
> > {
> > =09
> > "reliable": [channel_path_r1, channel_path_r2, ...],
> > "streaming" : [channel_path_s1, channel_path_s2, ...]
> > =09
> > }
> > =09
> > The fist reliable data channel will always be the first data
> > channel in reliable array.
> >=20
> > HealthAgent hierarchy
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> >=20
> > (this object is implemented by the HDP user in order to receive
> > notifications)
> >=20
> > Service unique name
> > Interface org.bluez.HealthAgent
> > Object path freely definable
> >=20
> > Methods:
> > void DeviceApplicationDiscovered(object path)
> > =09
> > This method is called when a device containing an hdp
> > application is connected. The object path is the application
> > path. The method will be called one time for each
> > application.
>=20
> I think this should be ServiceDiscovered and map to a HealthService.
Ok
>=20
> > void DeviceConnected(object path)
> > =09
> > This method is called whenever a new connection has been
> > established over the control channel of the current HDP
> > application. The object path paremeter contains the object path
> > of the connected HealthDevice.
>=20
> Don't see a useful need for this. I don't want to expose HealthDevice
> details anyway.
What this tries to mean is that this HealthService has connected with the=20
local Application so the application can open data channels with it now.
>=20
> > void DevicePaused(object path)
> > =09
> > This method is called when a MCL is closed. Future reconnections
> > will be notified using the DeviceRestarted callback.
> > All data channels associated to this device will be closed and
> > a reconnection will be needed before using them again.
> > =09
> > void DeviceResumed(object path)
> > =09
> > This method is called whenever a MCL is reconnected. All data
> > channels associated are still closed but they will be able to be
> > reconnected skipping the configuration process.
> > =09
> > void DeviceDisconnected(object path)
> > =09
> > This method is called when a remote device is disconnected or
> > removed from MCAP cache. Any future reconnections will fail.
> > Also all data channels associated to this device will be closed.
>=20
> Why bother with this. We can do this on channel level.
The problem is that in some cases (when the remote is not publishing a reco=
rd)=20
it is not possible to open data channels nor reconnecting the mcl so it is =
not=20
possible to make this automatically. The application should be concerned ab=
out=20
this issues to avoid this kind of operation in this cases.
>=20
> > void CreatedDataChannel(object path, path data_channel, string conf)
> > =09
> > This method is called when a new data channel is created.
> > =09
> > The path contains the object path of the HealthDeviceApplication
> > where the new connection is created, the data_channel is the
> > path for identify the data channel and conf is the quality of
> > service of the data channel ("reliable" or "streaming").
>=20
> DataChannelCreated please.
>=20
> > void DataChannelReconnected(object path, path data_channel, string=20
conf)
> > =09
> > This method is called when a closed data channel is reconnected
> > by the remote device.
> > =09
> > Conf will be "reliable" or "streaming".
> > =09
> > TThe path contains the object path of the
> > HealthDeviceApplication where the new connection is reconnected,
> > the data_channel is the path for identify the data channel and
> > conf is the quality of service of the data channel ("reliable"
> > or "streaming").
> > =09
> > void DeletedDataChannel(object path, path data_channel)
> > =09
> > This method is called when a data channel is deleted.
> > =09
> > After this call the data channel path will not be valid and can
> > be reused for future creation of data channels.
>=20
> DataChannelRemoved. We always map create with remove.
>=20
> > void DeletedAllDataChannels(object path)
> > =09
> > This method is called when all data channels are deleted.
> > =09
> > The path contains the object path of the HealthDeviceApplication
> > where the data channels are deleted.
>=20
> That is pointless. You will get separate callbacks for each channel
> anyway.
We tried to avoid calling may times to the same callback when a delete all=
=20
operation is done, but of course it is not necessary.
>=20
> And the agent is missing a Release method for unforseen terminations.
> See other agents for that detail.
I'll have a look.
>=20
> Regards
>=20
> Marcel
Thanks for your comments.
Regards
Jose
^ permalink raw reply
* Re: Proposed API for HDP
From: Gustavo F. Padovan @ 2010-07-08 17:54 UTC (permalink / raw)
To: José Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <201007081912.31407.santoscadenas@gmail.com>
Hi José,
* José Antonio Santos Cadenas <santoscadenas@gmail.com> [2010-07-08 19:12:31 +0200]:
<snip>
>
> Service org.bluez
> Interface org.bluez.HealthDeviceApplication
> Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
>
> Methods:
>
> array GetProperties()
>
> Gets the information of the remote application published on its
> SDP record. The returned data format is as follows:
>
> {
> "end_points": [
> "mdepid": uint8,
> "role" : "source" or "sink" ,
> "specs" : [{
> "dtype" : uint16,
> "description" : string, (optional)
> }]
> ]
> }
>
> object Connect(path local_application_id)
>
> Connects the local application with the remote application.
>
> Only the bus client that created the local session will be able
> to create connections using it.
>
> If the Device is already connected with an other application an
> org.bluez.Error.AlreadyConnected error will be received.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.AlreadyConnected
> org.bluez.Error.HealthError
>
> void Disconnect()
>
> Disconnect from the remote application the state will also be
> deleted. And no future reconnections will be possible. For
> keeping the state the method Pause of the health link should be
> used.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.NotFound
> org.bluez.Error.HealthError
>
> boolean Echo(array{byte})
>
> Sends an echo petition to the remote intance. Returns True if
> response matches with the buffer sent. If some error is detected
> False value is returned and the associated MCL is closed.
>
> path OpenDataChannel(byte mdepid, string conf)
>
> Creates a new data channel with the indicated config to the
> remote MCAP Data End Point (MDEP).
> The configuration should indicate the channel quality of
> service using one of this values "reliable", "streaming", "any".
>
> Returns the data channel path.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.HealthError
>
> void ReconnectDataChannel(path data_channel)
>
> Reconnects a previously created data channel indicated by its
> path.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.HealthError
> org.bluez.Error.NotFound
So are we really going to expose the temporally disconnect and
reconnection to the applicantion level? The application should not
care if the data channel is there or not.
>
> int GetDataChannelFileDescriptor(path data_channel)
>
> Gets a file descriptor where data can be read or written.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.NotFound
> org.bluez.Error.HealthError
What about the fd-passing feature of D-Bus here?
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: Proposed API for HDP
From: Marcel Holtmann @ 2010-07-08 17:39 UTC (permalink / raw)
To: José Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <201007081912.31407.santoscadenas@gmail.com>
Hi Jose,
> Health Device Profile hierarchy
> ===============================
>
> Service org.bluez
> Interface org.bluez.HealthAdapter
> Object path [variable prefix]/{hci0,hci1,...}
so I changed my mind here. Basing this on the local adapter is rather
pointless.
Lets just do org.bluez.HealthManager on /org/bluez object path. There is
no need that the calling application knows anything about the specific
adapters in our system. We properly separate them anyway during paring.
Only the application that does the initial pairing with a remote health
device needs to know which adapter to use. For the actual health
application it is pointless since it will be notified about the paired
health device initially.
> Methods:
>
> path CreateApplication(object path, dict config)
>
> Returns the path of the new created application. The path
> parameter is the path of the object with the callbacks to
> notify events (see org.bluez.HealthAgent at the end of this
> document)
> This petition starts an mcap instance and also register a proper
> record in the SDP if is needed.
>
> Dict is defined as bellow:
> {
> "end_points" : [{ (optional)
> "role" : ("source" or "sink"), (mandatory)
> "specs" :[{ (mandatory)
> "data_type" : uint16, (mandatory)
> "description" : string, (optional)
> }]
> }]
> }
>
> Application will be closed by the call or implicitly when the
> programs leaves the bus.
>
> Possible errors: org.bluez.Error.InvalidArguments
>
> void ReleaseApplication(path application)
>
> Closes the HDP application identified by the object path. Also
> application will be closed if the process that started it leaves
> the bus. If there is a SDP record associated to this application
> it will be removed.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.NotFound
Since we now make this as part of a generic manager, the method class
RegisterApplication and UnregisterApplication are a lot better choice.
> array GetRemoteApplications(path application)
>
> This method will return an array with the paths of all the
> remote instances found in remote devices.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.NotFound
We don't wanna do that. When you register your application the first
callback via the agent should be telling what remote instances are
available.
This has the advantage that the code flow for the application is
simpler. It just has to listen to that update. And if you register your
application before pairing with a new device, it will still work. So no
extra work to listen for new devices and bootstrapping an existing list.
> --------------------------------------------------------------------------------
>
> Service org.bluez
> Interface org.bluez.HealthDevice
> Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
This is not really a health device. As mentioned yesterday, we can have
multiple health service per remote device. So we should be using here
are org.bluez.HealthService for every SDP record for HDP inside the
remote device.
So potential object paths are .../hci0/dev_xxxxxxxx/{hdp0,hdp1,...} and
so on. This way we clearly map health service and not bother with remote
device details that might implement multiple functions.
> Methods:
>
> void Refresh()
>
> This method searches for HDP applications in the remote device
> and notifies them to the appropriate agents.
I might have called this Update(), but that is a minor detail.
> --------------------------------------------------------------------------------
>
> Service org.bluez
> Interface org.bluez.HealthDeviceApplication
> Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
>
That is more like the org.bluez.HealthService as mentioned above. So
lets combine them. I don't see a need for splitting these.
> Methods:
>
> array GetProperties()
>
> Gets the information of the remote application published on its
> SDP record. The returned data format is as follows:
>
> {
> "end_points": [
> "mdepid": uint8,
> "role" : "source" or "sink" ,
> "specs" : [{
> "dtype" : uint16,
> "description" : string, (optional)
> }]
> ]
> }
>
> object Connect(path local_application_id)
>
> Connects the local application with the remote application.
>
> Only the bus client that created the local session will be able
> to create connections using it.
>
> If the Device is already connected with an other application an
> org.bluez.Error.AlreadyConnected error will be received.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.AlreadyConnected
> org.bluez.Error.HealthError
>
> void Disconnect()
>
> Disconnect from the remote application the state will also be
> deleted. And no future reconnections will be possible. For
> keeping the state the method Pause of the health link should be
> used.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.NotFound
> org.bluez.Error.HealthError
Do we need Connect() and Disconnect() here. Can we just not create these
connections in the background based of a reference counting via the
channels?
> boolean Echo(array{byte})
>
> Sends an echo petition to the remote intance. Returns True if
> response matches with the buffer sent. If some error is detected
> False value is returned and the associated MCL is closed.
>
> path OpenDataChannel(byte mdepid, string conf)
>
> Creates a new data channel with the indicated config to the
> remote MCAP Data End Point (MDEP).
> The configuration should indicate the channel quality of
> service using one of this values "reliable", "streaming", "any".
>
> Returns the data channel path.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.HealthError
>
> void ReconnectDataChannel(path data_channel)
>
> Reconnects a previously created data channel indicated by its
> path.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.HealthError
> org.bluez.Error.NotFound
>
> int GetDataChannelFileDescriptor(path data_channel)
>
> Gets a file descriptor where data can be read or written.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.NotFound
> org.bluez.Error.HealthError
>
> void DeleteDataChannel(path data_channel)
>
> Deletes a data channel so it will not be available to use.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.NotFound
> org.bluez.Error.HealthError
>
> void DeleteAllDataChannels()
>
> Deletes all data channels so they will not be available for
> future use. Typically this function is called when the
> connection with the remote device will be closed permanently.
>
> Possible errors: org.bluez.Error.HealthError
This actually means also Disconnect() to me. So clear the extra work of
connect and disconnect can be done in the background and invisible for
the user.
> dict GetDataChannelStatus()
>
> Return a dictionary with all the data channels that can be used
> to send data right now. The dictionary is formed like follows:
>
> {
> "reliable": [channel_path_r1, channel_path_r2, ...],
> "streaming" : [channel_path_s1, channel_path_s2, ...]
> }
>
> The fist reliable data channel will always be the first data
> channel in reliable array.
>
> HealthAgent hierarchy
> =====================
>
> (this object is implemented by the HDP user in order to receive notifications)
>
> Service unique name
> Interface org.bluez.HealthAgent
> Object path freely definable
>
> Methods:
>
> void DeviceApplicationDiscovered(object path)
>
> This method is called when a device containing an hdp
> application is connected. The object path is the application
> path. The method will be called one time for each
> application.
I think this should be ServiceDiscovered and map to a HealthService.
> void DeviceConnected(object path)
>
> This method is called whenever a new connection has been
> established over the control channel of the current HDP
> application. The object path paremeter contains the object path
> of the connected HealthDevice.
Don't see a useful need for this. I don't want to expose HealthDevice
details anyway.
> void DevicePaused(object path)
>
> This method is called when a MCL is closed. Future reconnections
> will be notified using the DeviceRestarted callback.
> All data channels associated to this device will be closed and
> a reconnection will be needed before using them again.
>
> void DeviceResumed(object path)
>
> This method is called whenever a MCL is reconnected. All data
> channels associated are still closed but they will be able to be
> reconnected skipping the configuration process.
>
> void DeviceDisconnected(object path)
>
> This method is called when a remote device is disconnected or
> removed from MCAP cache. Any future reconnections will fail.
> Also all data channels associated to this device will be closed.
Why bother with this. We can do this on channel level.
> void CreatedDataChannel(object path, path data_channel, string conf)
>
> This method is called when a new data channel is created.
>
> The path contains the object path of the HealthDeviceApplication
> where the new connection is created, the data_channel is the
> path for identify the data channel and conf is the quality of
> service of the data channel ("reliable" or "streaming").
DataChannelCreated please.
> void DataChannelReconnected(object path, path data_channel, string conf)
>
> This method is called when a closed data channel is reconnected
> by the remote device.
>
> Conf will be "reliable" or "streaming".
>
> TThe path contains the object path of the
> HealthDeviceApplication where the new connection is reconnected,
> the data_channel is the path for identify the data channel and
> conf is the quality of service of the data channel ("reliable"
> or "streaming").
>
> void DeletedDataChannel(object path, path data_channel)
>
> This method is called when a data channel is deleted.
>
> After this call the data channel path will not be valid and can
> be reused for future creation of data channels.
DataChannelRemoved. We always map create with remove.
> void DeletedAllDataChannels(object path)
>
> This method is called when all data channels are deleted.
>
> The path contains the object path of the HealthDeviceApplication
> where the data channels are deleted.
That is pointless. You will get separate callbacks for each channel
anyway.
And the agent is missing a Release method for unforseen terminations.
See other agents for that detail.
Regards
Marcel
^ 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