* Re: [PATCH v15 02/14] audio: Move telephony drivers to D-Bus interface
From: Johan Hedberg @ 2012-07-30 7:58 UTC (permalink / raw)
To: Frédéric Danis; +Cc: linux-bluetooth
In-Reply-To: <1343292324-959-3-git-send-email-frederic.danis@linux.intel.com>
Hi Frédéric,
On Thu, Jul 26, 2012, Frédéric Danis wrote:
> +static int parse_properties(DBusMessageIter *props, const char **uuid,
> + uint16_t *version, uint16_t *features)
> +{
> + gboolean has_uuid = FALSE;
> +
> + while (dbus_message_iter_get_arg_type(props) == DBUS_TYPE_DICT_ENTRY) {
> + const char *key;
> + DBusMessageIter value, entry;
> + int var;
> +
> + dbus_message_iter_recurse(props, &entry);
> + dbus_message_iter_get_basic(&entry, &key);
> +
> + dbus_message_iter_next(&entry);
> + dbus_message_iter_recurse(&entry, &value);
> +
> + var = dbus_message_iter_get_arg_type(&value);
> + if (strcasecmp(key, "UUID") == 0) {
> + if (var != DBUS_TYPE_STRING)
> + return -EINVAL;
> + dbus_message_iter_get_basic(&value, uuid);
> + has_uuid = TRUE;
> + } else if (strcasecmp(key, "Version") == 0) {
> + if (var != DBUS_TYPE_UINT16)
> + return -EINVAL;
> + dbus_message_iter_get_basic(&value, version);
> + } else if (strcasecmp(key, "Features") == 0) {
> + if (var != DBUS_TYPE_UINT16)
> + return -EINVAL;
> + dbus_message_iter_get_basic(&value, features);
> + }
> +
> + dbus_message_iter_next(props);
> + }
> +
> + return (has_uuid) ? 0 : -EINVAL;
> +}
I suppose you could just make the above function return gboolean as it
only has two possible return values.
> +static int dev_close(struct telephony_device *tel_dev)
> +{
> + int sock;
> +
> + if (tel_dev->rfcomm) {
> + sock = g_io_channel_unix_get_fd(tel_dev->rfcomm);
> + shutdown(sock, SHUT_RDWR);
> + tel_dev->rfcomm = NULL;
> + }
Looks like you're missing a g_io_channel_unref there.
> +static void hs_newconnection_reply(DBusPendingCall *call, void *user_data)
> +{
> + struct telephony_device *tel_dev = user_data;
> + DBusMessage *reply = dbus_pending_call_steal_reply(call);
> + DBusError derr;
> +
> + dbus_error_init(&derr);
> + if (!dbus_set_error_from_message(&derr, reply)) {
> + DBG("Agent reply: file descriptor passed successfully");
> + g_io_add_watch(tel_dev->rfcomm, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
> + (GIOFunc) hs_dev_disconnect_cb, tel_dev);
> + headset_slc_complete(tel_dev->au_dev);
> + goto done;
> + }
Firstly, a more common way would be to test for positive return of
dbus_set_error_from_message and handle the error reply within the
if-clause. Secondly, please don't do callback typecasts (GIOFunc) but
instead just assign to the right type inside the callback function
itself.
> +static void get_record_cb(sdp_list_t *recs, int err, gpointer user_data)
> +{
> + struct telephony_device *tel_dev = user_data;
Here you do the right kind of handling of callback types. Why the
inconsistency?
> + sdp_get_profile_descs(recs->data, &profiles);
> + if (profiles == NULL)
> + goto failed;
I think it'd be cleaner/simpler to do:
if (sdp_get_profile_descs(...) < 0)
goto failed;
> + desc = profiles->data;
> +
> + if (sdp_uuid16_cmp(&desc->uuid, &uuid) == 0)
> + tel_dev->version = desc->version;
I don't think it's safe to assume that what's returned by
sdp_get_profile_descs is always a uuid16. Instead using sdp_uuid_cmp()
would seem more appropriate.
> +struct telephony_device *telephony_device_connecting(GIOChannel *io,
> + struct btd_device *btd_dev,
> + struct audio_device *au_dev,
> + const char *uuid)
> +{
> + struct btd_adapter *adapter;
> + struct telephony_agent *agent;
> + struct telephony_device *tel_dev;
> + uuid_t r_uuid;
> + int err;
> +
> + adapter = device_get_adapter(btd_dev);
> + agent = find_agent(adapter, NULL, NULL, uuid);
> + if (agent == NULL)
> + return NULL;
> +
> + tel_dev = g_new0(struct telephony_device, 1);
> + tel_dev->btd_dev = btd_device_ref(btd_dev);
> + tel_dev->name = g_strdup(agent->name);
> + tel_dev->path = g_strdup(agent->path);
> + tel_dev->config = agent->config;
> + tel_dev->au_dev = au_dev;
> + tel_dev->rfcomm = io;
Missing g_io_channel_ref here.
> + err = bt_search_service(&au_dev->src, &au_dev->dst, &r_uuid,
> + get_record_cb, tel_dev, NULL);
> + if (err < 0) {
> + telephony_device_disconnect(tel_dev);
> + return NULL;
> + }
> + tel_dev->pending_sdp = TRUE;
An empty line should follow after }
> +void telephony_device_disconnect(struct telephony_device *device)
> +{
> + dev_close(device);
> +
> + if (device->pending_sdp)
> + return;
Shouldn't you cancel the SDP operation here with bt_cancel_discovery?
> +gboolean telephony_get_ready_state(struct btd_adapter *adapter)
> +{
> + return find_agent(adapter, NULL, NULL, HFP_AG_UUID) ? TRUE : FALSE;
> +}
If such a function is needed just call it telephony_is_ready. It makes
the calling side look more natural: "if (telephony_is_ready(adapter))".
> +static int register_interface(struct btd_adapter *adapter)
> +{
> + const char *path;
> +
> + path = adapter_get_path(adapter);
> +
> + if (!g_dbus_register_interface(connection, path,
> + AUDIO_TELEPHONY_INTERFACE,
> + telsrv_methods, NULL,
> + NULL, adapter, path_unregister)) {
> + error("D-Bus failed to register %s interface",
> + AUDIO_TELEPHONY_INTERFACE);
> + return -1;
> + }
> +
> + DBG("Registered interface %s", AUDIO_TELEPHONY_INTERFACE);
> +
> + return 0;
> +}
> +
> +static void unregister_interface(struct btd_adapter *adapter)
> +{
> + g_dbus_unregister_interface(connection, adapter_get_path(adapter),
> + AUDIO_TELEPHONY_INTERFACE);
> +}
> +
> +int telephony_adapter_init(struct btd_adapter *adapter)
> +{
> + DBG("adapter: %p", adapter);
> +
> + return register_interface(adapter);
> +}
> +
> +void telephony_adapter_exit(struct btd_adapter *adapter)
> +{
> + struct telephony_agent *agent;
> +
> + DBG("adapter: %p", adapter);
> +
> + unregister_interface(adapter);
> +
> + while ((agent = find_agent(adapter, NULL, NULL, NULL)) != NULL) {
> + agents = g_slist_remove(agents, agent);
> + free_agent(agent);
> + }
> +}
The register_interface and unregister_interface functions above seem
unnecessary to me. Just include their code directly within
telephony_adapter_init and telephony_adapter_exit.
Johan
^ permalink raw reply
* Re: [PATCH 0/2] Support for reserving bandwidth on L2CAP socket
From: Manoj Sharma @ 2012-07-30 6:30 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth, Anurag Gupta
In-Reply-To: <1343399863.1803.10.camel@aeonflux>
Hi Marcel,
On 7/27/12, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Manoj,
>
>> >> These patches allows L2CAP socket user to reserve bandwidth in
>> >> percentage. Underlying socket reserves calculated number of
>> >> HCI credits for this L2CAP channel.
>> >
>> > this description is by far not enough. Explain why you are doing this.
>> > And make it a detailed description. Preferable with hcidump traces
>> > showing why this makes a difference at all.
>> >
>> This patch simply adds an additional L2CAP socket option for reserving
>> bandwidth.
>> The reserved bandwidth would result into reserving calculated number
>> of ACL data credits. Thus the L2CAP channels without this option set
>> would not be able to use all available ACL buffers in controller. This
>> would ensure that whenever an L2CAP channel with this option set has
>> data to send, it does not starve or wait because of other channels
>> already using all controller buffers.
>>
>> Above explanation is most suitable in case when simultaneous AVDTP
>> streaming channels and other channels (e.g. OPP, PBAP etc) are in
>> action. Such an arrangement for reserving credits would allow AVDTP
>> stream to flow to controller without any obstacle from simultaneous
>> traffic and help removing glitches in music streaming over Bluetooth.
>> Please suggest if this description is sufficient and if I should push
>> patch-set again.
>
> and what is the problem with using SO_PRIORITY for this?
>
One problem which I have faced using SO_PRIORITY is explained below.
Suppose we have 2 links A & B and link A has higher priority than link
B. And outgoing data transfer is active on both links. Now if device
on link A goes far, there would be lot of failures and number of
re-transmissions would increase for link A. Consequently at any time
host would have significant number of packets for link A, getting
accumulated due to poor quality of link.But since link A packets have
higher priority, link B packets would suffer infinitely as long as
link A packet queue in host is non-empty. Thus link B protocols may
fail due to timers expiring and finally disconnection at upper layers.
Second problem:
We have two links similar to above scenario. Say link A is being used
by AVDTP and link B is being used by OBEX. Host can come across a
situation where all controller buffers are used by OBEX and AVDTP is
waiting for a free buffer. Now due to some reason (e.g. distance) OBEX
link B goes weak. This results into delay in transmission of OBEX
packets already held by controller and consequently AVDTP packets also
get delayed which causes glitches in music streaming and user
experience goes bad.
These are the basic problems which I have faced and hence felt
necessity of a similar but different mechanism and came up with this
solution. This solution fixes both of the problems explained above.
Based on the explanation above your suggestion is required further.
> Regards
>
> Marcel
>
>
>
Best regards,
Manoj
^ permalink raw reply
* Re: [PATCH] Fix not setting class of device in adapter
From: Johan Hedberg @ 2012-07-29 16:42 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1343214893-28310-1-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Wed, Jul 25, 2012, Szymon Janc wrote:
> Set class of device in adapter when new class is received from kernel.
> This fix adapters property Class being always zero.
> ---
> lib/mgmt.h | 4 ++++
> src/adapter.c | 23 ++++++++++++++---------
> src/adapter.h | 2 +-
> src/mgmt.c | 33 +++++++++++++++++++++++++++++++--
> 4 files changed, 50 insertions(+), 12 deletions(-)
This patch doesn't compile:
src/adapter.c: In function ‘btd_adapter_class_changed’:
src/adapter.c:698:21: error: assignment makes integer from pointer without a cast [-Werror]
Please fix and resend.
Johan
^ permalink raw reply
* Re: [PATCH BlueZ 1/2] tools: Fix workaround to build with recent flex
From: Marcel Holtmann @ 2012-07-29 16:31 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
In-Reply-To: <1343543435-8429-1-git-send-email-lucas.demarchi@profusion.mobi>
Hi Lucas,
> Flex 2.5.36 no longer defines isatty(), resulting in the following build
> error:
>
> tools/lexer.c:1402:9: error: implicit declaration of function
> ‘isatty’ [-Werror=implicit-function-declaration]
>
> Include the header unistd.h so we get the definition of that function
> and at the same time keep the fix for older versions of flex, since this
> header defines _UNISTD_H.
lets remove the rfcomm.conf support from rfcomm utility and then move it
under tools/. So no more need for flex or bison.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH BlueZ v0 1/2] btio: Replace g_set_error by ERROR_FAILED macro
From: Johan Hedberg @ 2012-07-29 15:04 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1343311869-5179-1-git-send-email-claudio.takahasi@openbossa.org>
Hi Claudio,
On Thu, Jul 26, 2012, Claudio Takahasi wrote:
> This patch replaces the calls of g_set_error function by the local
> defined macro "ERROR_FAILED".
> ---
> btio/btio.c | 19 +++++++------------
> 1 files changed, 7 insertions(+), 12 deletions(-)
Both patches have been applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH BlueZ 1/3] adapter: Remove unused Broadcaster property
From: Johan Hedberg @ 2012-07-29 14:57 UTC (permalink / raw)
To: bruna.moreira; +Cc: linux-bluetooth
In-Reply-To: <50117f05.0a4bec0a.04ec.ffffcfe2@mx.google.com>
Hi Bruna,
On Thu, Jul 26, 2012, bruna.moreira@openbossa.org wrote:
> Applications that want to be Observers of certain Advertising data type
> (e.g. Manufacturer Specific Data or Service Data) will use new Observer
> D-Bus API to monitor these AD types. Therefore, the old Broadcaster
> device property is unnecessary.
> ---
> doc/adapter-api.txt | 3 +--
> src/adapter.c | 7 -------
> 2 files changed, 1 insertion(+), 9 deletions(-)
All three patches have been applied. Thanks.
Johan
^ permalink raw reply
* Re: AVDTP RECONFIGURE COMMAND & RESPONSE
From: Johan Hedberg @ 2012-07-29 14:19 UTC (permalink / raw)
To: Sathish narasimman; +Cc: linux-bluetooth
In-Reply-To: <CAOVXEJJawhMTvVLvF1E0YjPpJsOD6fNy4G8m3oxS9G=vQYP=3A@mail.gmail.com>
Hi Satish,
On Mon, Jul 23, 2012, Sathish narasimman wrote:
> i am trying to send AVDTP_RECONFIGURE command but every time i am just
> getting AVDTP_MSG_TYPE_GEN_REJECT . I saw in avdtp.c that response for
> RECONFIGURE command is always set to reply as unknown command i.e
> response is rejected.. I want to know whether reconfigure is possible
> in bluez.
AVDTP Reconfigure is an optional feature, so implementations are allowed
to return an error for it. BlueZ has opted to not support it (neither
sending nor receiving) since we in any case need to deal with
implementations that do not support it.
Johan
^ permalink raw reply
* [PATCHv2 hcidump] hci: Fix EIR data parsing
From: Andrzej Kaczmarek @ 2012-07-29 9:30 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
Data passed to ext_inquiry_data_dump are expected to start with length
octet which was consumed by get_u8.
---
parser/hci.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/parser/hci.c b/parser/hci.c
index 742a1b5..920b973 100644
--- a/parser/hci.c
+++ b/parser/hci.c
@@ -831,16 +831,19 @@ static inline void ext_inquiry_response_dump(int level, struct frame *frm)
{
void *ptr = frm->ptr;
uint32_t len = frm->len;
+ uint8_t *data;
uint8_t length;
+ data = frm->ptr;
length = get_u8(frm);
while (length > 0) {
- ext_inquiry_data_dump(level, frm, frm->ptr);
+ ext_inquiry_data_dump(level, frm, data);
frm->ptr += length;
frm->len -= length;
+ data = frm->ptr;
length = get_u8(frm);
}
--
1.7.10
^ permalink raw reply related
* bluetooth.h: fix compile issue when using in C++
From: Pacho Ramos @ 2012-07-29 7:52 UTC (permalink / raw)
To: BlueZ development
[-- Attachment #1: Type: text/plain, Size: 314 bytes --]
Hello
Today I got a report downstream showing that old problem:
http://permalink.gmane.org/gmane.linux.bluez.kernel/22306
is still not fixed.
Could you please commit debian patch to fix this?
http://patch-tracker.debian.org/patch/series/view/bluez/4.101-1/09_fix_ftbfs_with_c99.patch
Thanks a lot
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* [PATCH BlueZ 2/2] build-sys: Don't use -Werror for generated lexer.c
From: Lucas De Marchi @ 2012-07-29 6:30 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
In-Reply-To: <1343543435-8429-1-git-send-email-lucas.demarchi@profusion.mobi>
Source files that are generated by tools can't be compiled with -Werror
because we can't really fix the issue in version of the tools that was
already release and is used by distributions.
After updating to flex 2.5.36 the following error occurs:
tools/lexer.c:1599:17: error: comparison between signed and
unsigned integer expressions [-Werror=sign-compare]
Override -Werror with -Wno-error for this specific file so warning is
still emitted but we no longer fail the build.
---
Makefile.tools | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/Makefile.tools b/Makefile.tools
index 5579b86..1499bf7 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -14,11 +14,15 @@ noinst_PROGRAMS += tools/avinfo tools/ppporc \
tools/kword.c: tools/parser.h
-tools_rfcomm_SOURCES = tools/rfcomm.c tools/parser.y tools/lexer.l \
- tools/kword.h tools/kword.c
+noinst_LTLIBRARIES += tools/librfcomm-lexer.la
+tools_librfcomm_lexer_la_SOURCES = tools/lexer.l
+tools_librfcomm_lexer_la_CFLAGS = $(AM_CFLAGS) -Wno-error
+
+tools_rfcomm_SOURCES = tools/rfcomm.c tools/parser.y tools/kword.h \
+ tools/kword.c
EXTRA_tools_rfcomm_SOURCES = tools/parser.h tools/parser.c \
tools/lexer.c
-tools_rfcomm_LDADD = lib/libbluetooth-private.la
+tools_rfcomm_LDADD = lib/libbluetooth-private.la tools/librfcomm-lexer.la
tools_l2ping_LDADD = lib/libbluetooth-private.la
--
1.7.11.3
^ permalink raw reply related
* [PATCH BlueZ 1/2] tools: Fix workaround to build with recent flex
From: Lucas De Marchi @ 2012-07-29 6:30 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
Flex 2.5.36 no longer defines isatty(), resulting in the following build
error:
tools/lexer.c:1402:9: error: implicit declaration of function
‘isatty’ [-Werror=implicit-function-declaration]
Include the header unistd.h so we get the definition of that function
and at the same time keep the fix for older versions of flex, since this
header defines _UNISTD_H.
---
tools/lexer.l | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/lexer.l b/tools/lexer.l
index ff9ce81..0d449ac 100644
--- a/tools/lexer.l
+++ b/tools/lexer.l
@@ -26,9 +26,7 @@
#include <config.h>
#endif
-/* Nasty workaround, but flex defines isatty() twice */
-#define _UNISTD_H
-
+#include <unistd.h>
#include <stdio.h>
#include <errno.h>
#include <sys/socket.h>
--
1.7.11.3
^ permalink raw reply related
* Re: [PATCH BlueZ 1/3] att: Add prepare write support
From: Sergio Correia @ 2012-07-29 1:47 UTC (permalink / raw)
To: Eder Ruiz Maria; +Cc: linux-bluetooth
In-Reply-To: <1343420976-19921-1-git-send-email-eder.ruiz@openbossa.org>
Hi Eder,
On Fri, Jul 27, 2012 at 4:29 PM, Eder Ruiz Maria
<eder.ruiz@openbossa.org> wrote:
> Add functions for encoding/decoding Prepare Write Request and
> Response PDUs.
> ---
> attrib/att.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> attrib/att.h | 5 +++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/attrib/att.c b/attrib/att.c
> index 0550ac1..acfb4e0 100644
> --- a/attrib/att.c
> +++ b/attrib/att.c
> @@ -974,3 +974,58 @@ uint16_t dec_mtu_resp(const uint8_t *pdu, int len, uint16_t *mtu)
>
> return min_len;
> }
> +
> +uint16_t enc_prep_write_req(uint16_t handle, uint16_t offset,
> + const uint8_t *value, int vlen, uint8_t *pdu, int len)
> +{
> + const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle) +
> + sizeof(offset);
> +
> + if (pdu == NULL)
> + return 0;
> +
> + if (len < min_len)
> + return 0;
maybe it would be better to check the above conditions in a single if statement?
[...]
> +uint16_t dec_prep_write_resp(const uint8_t *pdu, int len, uint16_t *handle,
> + uint16_t *offset, uint8_t *value, int *vlen)
> +{
> + const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle) +
> + sizeof(*offset);
> +
> + if (pdu == NULL)
> + return 0;
> +
> + if (handle == NULL || offset == NULL || value == NULL || vlen == NULL)
> + return 0;
> +
> + if (len < min_len)
> + return 0;
> +
> + if (pdu[0] != ATT_OP_PREP_WRITE_REQ)
> + return 0;
same with the above sanity checking.
The other patches in this series could also do the same, assuming it's
the preferred way.
> +
> + *handle = att_get_u16(&pdu[1]);
> + *offset = att_get_u16(&pdu[3]);
> + *vlen = len - min_len;
> + if (*vlen > 0)
> + memcpy(value, pdu + min_len, *vlen);
> +
> + return len;
> +}
> +
> diff --git a/attrib/att.h b/attrib/att.h
> index 1c1102a..ec03be9 100644
> --- a/attrib/att.h
> +++ b/attrib/att.h
> @@ -256,3 +256,8 @@ uint16_t enc_mtu_req(uint16_t mtu, uint8_t *pdu, int len);
> uint16_t dec_mtu_req(const uint8_t *pdu, int len, uint16_t *mtu);
> uint16_t enc_mtu_resp(uint16_t mtu, uint8_t *pdu, int len);
> uint16_t dec_mtu_resp(const uint8_t *pdu, int len, uint16_t *mtu);
> +
> +uint16_t enc_prep_write_req(uint16_t handle, uint16_t offset,
> + const uint8_t *value, int vlen, uint8_t *pdu, int len);
> +uint16_t dec_prep_write_resp(const uint8_t *pdu, int len, uint16_t *handle,
> + uint16_t *offset, uint8_t *value, int *vlen);
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v4 4/7] Bluetooth: Refactor ACL connection into its own function
From: Vinicius Costa Gomes @ 2012-07-29 1:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1343428380-32705-5-git-send-email-vinicius.gomes@openbossa.org>
The hci_connect() function was starting to get too complicated to be
quickly understood. We can separate the creation of a new ACL
connection into its own function.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/hci_conn.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0a74399..1d70e9f 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -497,18 +497,10 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
return le;
}
-/* Create SCO, ACL or LE connection.
- * Device _must_ be locked */
-struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
- __u8 dst_type, __u8 sec_level, __u8 auth_type)
+static struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
+ u8 sec_level, u8 auth_type)
{
struct hci_conn *acl;
- struct hci_conn *sco;
-
- BT_DBG("%s dst %s", hdev->name, batostr(dst));
-
- if (type == LE_LINK)
- return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst);
if (!acl) {
@@ -526,6 +518,26 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
hci_acl_create_connection(acl);
}
+ return acl;
+}
+
+/* Create SCO, ACL or LE connection.
+ * Device _must_ be locked */
+struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
+ __u8 dst_type, __u8 sec_level, __u8 auth_type)
+{
+ struct hci_conn *acl;
+ struct hci_conn *sco;
+
+ BT_DBG("%s dst %s", hdev->name, batostr(dst));
+
+ if (type == LE_LINK)
+ return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
+
+ acl = hci_connect_acl(hdev, dst, sec_level, auth_type);
+ if (IS_ERR(acl))
+ return acl;
+
if (type == ACL_LINK)
return acl;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v3 4/7] Bluetooth: Refactor ACL connection into its own function
From: Vinicius Costa Gomes @ 2012-07-29 0:13 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <CAJdJm_MJM3AKu6Rrir+WkwyeyoEezjjvqQY2E14bzcUSz2NcXw@mail.gmail.com>
Hi Lizardo,
On 18:52 Fri 27 Jul, Anderson Lizardo wrote:
> Hi Vinicius,
>
> On Fri, Jul 27, 2012 at 6:32 PM, Vinicius Costa Gomes
> <vinicius.gomes@openbossa.org> wrote:
> > The hci_connect() function was starting to get too complicated to be
> > quickly understood. We can separated the creation of a new ACL
>
> small typo: separated -> separate
Ugh. Thanks.
Cheers,
--
Vinicius
^ permalink raw reply
* Re: [PATCH hcidump] hci: Fix EIR data parsing
From: Vinicius Costa Gomes @ 2012-07-28 23:42 UTC (permalink / raw)
To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1343514013-8942-1-git-send-email-andrzej.kaczmarek@tieto.com>
Hi Andrzej,
On 00:20 Sun 29 Jul, Andrzej Kaczmarek wrote:
> Data passed to ext_inquiry_data_dump are expected to start with length
> octet which was consumed by get_u8.
Good catch.
> ---
> parser/hci.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/parser/hci.c b/parser/hci.c
> index 742a1b5..adcdaf7 100644
> --- a/parser/hci.c
> +++ b/parser/hci.c
> @@ -831,16 +831,19 @@ static inline void ext_inquiry_response_dump(int level, struct frame *frm)
> {
> void *ptr = frm->ptr;
> uint32_t len = frm->len;
> + uint8_t *data_ptr;
Just a nitpick, I would only change 'data_ptr' to 'data', to make it more
consistent with the parameters names that ext_inquiry_data_dump() expects.
> uint8_t length;
>
> + data_ptr = frm->ptr;
> length = get_u8(frm);
>
> while (length > 0) {
> - ext_inquiry_data_dump(level, frm, frm->ptr);
> + ext_inquiry_data_dump(level, frm, data_ptr);
>
> frm->ptr += length;
> frm->len -= length;
>
> + data_ptr = frm->ptr;
> length = get_u8(frm);
> }
>
> --
> 1.7.10
>
> --
> 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
Cheers,
--
Vinicius
^ permalink raw reply
* [PATCH hcidump] hci: Fix EIR data parsing
From: Andrzej Kaczmarek @ 2012-07-28 22:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
Data passed to ext_inquiry_data_dump are expected to start with length
octet which was consumed by get_u8.
---
parser/hci.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/parser/hci.c b/parser/hci.c
index 742a1b5..adcdaf7 100644
--- a/parser/hci.c
+++ b/parser/hci.c
@@ -831,16 +831,19 @@ static inline void ext_inquiry_response_dump(int level, struct frame *frm)
{
void *ptr = frm->ptr;
uint32_t len = frm->len;
+ uint8_t *data_ptr;
uint8_t length;
+ data_ptr = frm->ptr;
length = get_u8(frm);
while (length > 0) {
- ext_inquiry_data_dump(level, frm, frm->ptr);
+ ext_inquiry_data_dump(level, frm, data_ptr);
frm->ptr += length;
frm->len -= length;
+ data_ptr = frm->ptr;
length = get_u8(frm);
}
--
1.7.10
^ permalink raw reply related
* Re: GSoC student introduction
From: Thiago S. A. @ 2012-07-28 4:06 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <CAMBjcSsDp2RZxAQOqu3cbWyCv7sdnhfmGWi3Ux_NPJyqKWvoYw@mail.gmail.com>
Hello again BlueZ community.
I'd like to show the status of project.
Currently it has this features:
- Dump bluetooth traffic in real time.
- Parser of events.
- Draw sequence diagram.
- Show details of events.
- Filter events by device.
In my blog [1] there is a more detailed description of project status,
including a video demonstration of tool execution.
Source code of this project is available in a github repository [0].
I appreciate any suggestions to improve the tool and any feedback
Cheers,
Thiago.
[0] https://github.com/xth1/bluez-traffic
[1] http://xth1.blogspot.com.br/search/label/GSOC
^ permalink raw reply
* Re: [PATCH] Bluetooth: Another vendor specific ID for BCM20702A0 [0a5c:21f1]
From: Marcel Holtmann @ 2012-07-27 23:29 UTC (permalink / raw)
To: Michal Marek; +Cc: linux-bluetooth, linux-kernel
In-Reply-To: <1343424081-2157-1-git-send-email-mmarek@suse.cz>
Hi Michael,
> Bus 002 Device 003: ID 0a5c:21f1 Broadcom Corp.
> Device Descriptor:
> bLength 18
> bDescriptorType 1
> bcdUSB 2.00
> bDeviceClass 255 Vendor Specific Class
> bDeviceSubClass 1
> bDeviceProtocol 1
> bMaxPacketSize0 64
> idVendor 0x0a5c Broadcom Corp.
> idProduct 0x21f1
> bcdDevice 1.12
> iManufacturer 1 Broadcom Corp
> iProduct 2 BCM20702A0
> iSerial 3 9CB70DCFF833
> bNumConfigurations 1
as usual include /sys/kernel/debug/usb/devices details for this device.
I want the full set of endpoint descriptors in a simple compact format
in the commit messages for reference.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v3 4/7] Bluetooth: Refactor ACL connection into its own function
From: Anderson Lizardo @ 2012-07-27 22:55 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <CAJdJm_MJM3AKu6Rrir+WkwyeyoEezjjvqQY2E14bzcUSz2NcXw@mail.gmail.com>
Hi Vinicius,
On Fri, Jul 27, 2012 at 6:52 PM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Does it make sense to merge the two if()'s above into one? i.e.:
>
> if (IS_ERR(acl) || type == ACL_LINK)
> return acl;
>
> It's still clear, IMHO.
Nevermind, now I see you just factored out this code on next patch.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH v3 4/7] Bluetooth: Refactor ACL connection into its own function
From: Anderson Lizardo @ 2012-07-27 22:52 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1343428380-32705-5-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Fri, Jul 27, 2012 at 6:32 PM, Vinicius Costa Gomes
<vinicius.gomes@openbossa.org> wrote:
> The hci_connect() function was starting to get too complicated to be
> quickly understood. We can separated the creation of a new ACL
small typo: separated -> separate
> connection into its own function.
>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 32 ++++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 0a74399..1d70e9f 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -497,18 +497,10 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
> return le;
> }
>
> -/* Create SCO, ACL or LE connection.
> - * Device _must_ be locked */
> -struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
> - __u8 dst_type, __u8 sec_level, __u8 auth_type)
> +static struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
> + u8 sec_level, u8 auth_type)
> {
> struct hci_conn *acl;
> - struct hci_conn *sco;
> -
> - BT_DBG("%s dst %s", hdev->name, batostr(dst));
> -
> - if (type == LE_LINK)
> - return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
>
> acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst);
> if (!acl) {
> @@ -526,6 +518,26 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
> hci_acl_create_connection(acl);
> }
>
> + return acl;
> +}
> +
> +/* Create SCO, ACL or LE connection.
> + * Device _must_ be locked */
> +struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
> + __u8 dst_type, __u8 sec_level, __u8 auth_type)
> +{
> + struct hci_conn *acl;
> + struct hci_conn *sco;
> +
> + BT_DBG("%s dst %s", hdev->name, batostr(dst));
> +
> + if (type == LE_LINK)
> + return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
> +
> + acl = hci_connect_acl(hdev, dst, sec_level, auth_type);
> + if (IS_ERR(acl))
> + return acl;
> +
> if (type == ACL_LINK)
> return acl;
Does it make sense to merge the two if()'s above into one? i.e.:
if (IS_ERR(acl) || type == ACL_LINK)
return acl;
It's still clear, IMHO.
Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH v3 7/7] Bluetooth: Add type information to the hci_connect() debug statement
From: Vinicius Costa Gomes @ 2012-07-27 22:33 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1343428380-32705-1-git-send-email-vinicius.gomes@openbossa.org>
Now that we have a "connect" function for each link type, we should be
able to indentify which function is going to be called.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/hci_conn.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 2e7b776..98670b1 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -566,7 +566,7 @@ static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, bdaddr_t *dst,
struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
__u8 dst_type, __u8 sec_level, __u8 auth_type)
{
- BT_DBG("%s dst %s", hdev->name, batostr(dst));
+ BT_DBG("%s dst %s type 0x%x", hdev->name, batostr(dst), type);
switch (type) {
case LE_LINK:
--
1.7.10.4
^ permalink raw reply related
* [PATCH v3 6/7] Bluetooth: Simplify a the connection type handling
From: Vinicius Costa Gomes @ 2012-07-27 22:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1343428380-32705-1-git-send-email-vinicius.gomes@openbossa.org>
Now that we have separate ways of doing connections for each link type,
we can do better than an "if" statement to handle each link type.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/hci_conn.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index de7df88..2e7b776 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -568,13 +568,16 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
{
BT_DBG("%s dst %s", hdev->name, batostr(dst));
- if (type == LE_LINK)
+ switch (type) {
+ case LE_LINK:
return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
-
- if (type == ACL_LINK)
+ case ACL_LINK:
return hci_connect_acl(hdev, dst, sec_level, auth_type);
+ case SCO_LINK:
+ return hci_connect_sco(hdev, dst, sec_level, auth_type);
+ }
- return hci_connect_sco(hdev, dst, sec_level, auth_type);
+ return ERR_PTR(-EINVAL);
}
/* Check link security requirement */
--
1.7.10.4
^ permalink raw reply related
* [PATCH v3 5/7] Bluetooth: Refactor SCO connection into its own function
From: Vinicius Costa Gomes @ 2012-07-27 22:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1343428380-32705-1-git-send-email-vinicius.gomes@openbossa.org>
We can do the same that we did for the other link types, for SCO
connections. The only thing that's worth noting is that as SCO
links need an ACL link, this functions uses the function that adds
an ACL link.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/hci_conn.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 1d70e9f..de7df88 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -521,29 +521,19 @@ static struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
return acl;
}
-/* Create SCO, ACL or LE connection.
- * Device _must_ be locked */
-struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
- __u8 dst_type, __u8 sec_level, __u8 auth_type)
+static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, bdaddr_t *dst,
+ u8 sec_level, u8 auth_type)
{
struct hci_conn *acl;
struct hci_conn *sco;
- BT_DBG("%s dst %s", hdev->name, batostr(dst));
-
- if (type == LE_LINK)
- return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
-
acl = hci_connect_acl(hdev, dst, sec_level, auth_type);
if (IS_ERR(acl))
return acl;
- if (type == ACL_LINK)
- return acl;
-
- sco = hci_conn_hash_lookup_ba(hdev, type, dst);
+ sco = hci_conn_hash_lookup_ba(hdev, SCO_LINK, dst);
if (!sco) {
- sco = hci_conn_add(hdev, type, dst);
+ sco = hci_conn_add(hdev, SCO_LINK, dst);
if (!sco) {
hci_conn_put(acl);
return ERR_PTR(-ENOMEM);
@@ -572,6 +562,21 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
return sco;
}
+/* Create SCO, ACL or LE connection. */
+struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
+ __u8 dst_type, __u8 sec_level, __u8 auth_type)
+{
+ BT_DBG("%s dst %s", hdev->name, batostr(dst));
+
+ if (type == LE_LINK)
+ return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
+
+ if (type == ACL_LINK)
+ return hci_connect_acl(hdev, dst, sec_level, auth_type);
+
+ return hci_connect_sco(hdev, dst, sec_level, auth_type);
+}
+
/* Check link security requirement */
int hci_conn_check_link_mode(struct hci_conn *conn)
{
--
1.7.10.4
^ permalink raw reply related
* [PATCH v3 4/7] Bluetooth: Refactor ACL connection into its own function
From: Vinicius Costa Gomes @ 2012-07-27 22:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1343428380-32705-1-git-send-email-vinicius.gomes@openbossa.org>
The hci_connect() function was starting to get too complicated to be
quickly understood. We can separated the creation of a new ACL
connection into its own function.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/hci_conn.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0a74399..1d70e9f 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -497,18 +497,10 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
return le;
}
-/* Create SCO, ACL or LE connection.
- * Device _must_ be locked */
-struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
- __u8 dst_type, __u8 sec_level, __u8 auth_type)
+static struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
+ u8 sec_level, u8 auth_type)
{
struct hci_conn *acl;
- struct hci_conn *sco;
-
- BT_DBG("%s dst %s", hdev->name, batostr(dst));
-
- if (type == LE_LINK)
- return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst);
if (!acl) {
@@ -526,6 +518,26 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
hci_acl_create_connection(acl);
}
+ return acl;
+}
+
+/* Create SCO, ACL or LE connection.
+ * Device _must_ be locked */
+struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
+ __u8 dst_type, __u8 sec_level, __u8 auth_type)
+{
+ struct hci_conn *acl;
+ struct hci_conn *sco;
+
+ BT_DBG("%s dst %s", hdev->name, batostr(dst));
+
+ if (type == LE_LINK)
+ return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
+
+ acl = hci_connect_acl(hdev, dst, sec_level, auth_type);
+ if (IS_ERR(acl))
+ return acl;
+
if (type == ACL_LINK)
return acl;
--
1.7.10.4
^ permalink raw reply related
* [PATCH v3 3/7] Bluetooth: Refactor LE connection into its own function
From: Vinicius Costa Gomes @ 2012-07-27 22:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1343428380-32705-1-git-send-email-vinicius.gomes@openbossa.org>
The code that handles LE connection is already quite separated from
the rest of the connection procedure, so we can easily put it into
its own.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/hci_conn.c | 53 +++++++++++++++++++++++++---------------------
1 file changed, 29 insertions(+), 24 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index c30c507..0a74399 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -470,6 +470,33 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src)
}
EXPORT_SYMBOL(hci_get_route);
+static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
+ u8 dst_type, u8 sec_level, u8 auth_type)
+{
+ struct hci_conn *le;
+
+ le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
+ if (!le) {
+ le = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
+ if (le)
+ return ERR_PTR(-EBUSY);
+
+ le = hci_conn_add(hdev, LE_LINK, dst);
+ if (!le)
+ return ERR_PTR(-ENOMEM);
+
+ le->dst_type = bdaddr_to_le(dst_type);
+ hci_le_create_connection(le);
+ }
+
+ le->pending_sec_level = sec_level;
+ le->auth_type = auth_type;
+
+ hci_conn_hold(le);
+
+ return le;
+}
+
/* Create SCO, ACL or LE connection.
* Device _must_ be locked */
struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
@@ -477,33 +504,11 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
{
struct hci_conn *acl;
struct hci_conn *sco;
- struct hci_conn *le;
BT_DBG("%s dst %s", hdev->name, batostr(dst));
- if (type == LE_LINK) {
- le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
- if (!le) {
- le = hci_conn_hash_lookup_state(hdev, LE_LINK,
- BT_CONNECT);
- if (le)
- return ERR_PTR(-EBUSY);
-
- le = hci_conn_add(hdev, LE_LINK, dst);
- if (!le)
- return ERR_PTR(-ENOMEM);
-
- le->dst_type = bdaddr_to_le(dst_type);
- hci_le_create_connection(le);
- }
-
- le->pending_sec_level = sec_level;
- le->auth_type = auth_type;
-
- hci_conn_hold(le);
-
- return le;
- }
+ if (type == LE_LINK)
+ return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst);
if (!acl) {
--
1.7.10.4
^ permalink raw reply related
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