* Re: [PATCHv5 08/14] tools/btgatt-client: Print type of service
From: Szymon Janc @ 2014-10-22 18:00 UTC (permalink / raw)
To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-9-git-send-email-marcin.kraglak@tieto.com>
Hi Marcin,
On Thursday 16 October 2014 12:17:20 Marcin Kraglak wrote:
> ---
> tools/btgatt-client.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
> index d900e08..5c692ff 100644
> --- a/tools/btgatt-client.c
> +++ b/tools/btgatt-client.c
> @@ -182,8 +182,9 @@ static void print_service(const bt_gatt_service_t
> *service) return;
> }
>
> - printf(COLOR_RED "service" COLOR_OFF " - start: 0x%04x, "
> + printf(COLOR_RED "service %s" COLOR_OFF " - start: 0x%04x, "
> "end: 0x%04x, uuid: ",
> + service->primary ? "primary" : "second.",
> service->start_handle, service->end_handle);
As we discussed offline, maybe having "type: foo" would be nicer instead of
"serivce second." ?
> print_uuid(service->uuid);
--
Szymon K. Janc
szymon.janc@gmail.com
^ permalink raw reply
* Re: [PATCHv5 04/14] shared/gatt: Add extra check in characteristic iterator
From: Szymon Janc @ 2014-10-22 17:49 UTC (permalink / raw)
To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-5-git-send-email-marcin.kraglak@tieto.com>
Hi Marcin,
On Thursday 16 October 2014 12:17:16 Marcin Kraglak wrote:
> Avoid incorrect reading of included service discovery results.
> ---
> src/shared/gatt-helpers.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index c18f738..58104d9 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -123,6 +123,9 @@ unsigned int bt_gatt_result_characteristic_count(struct
> bt_gatt_result *result) if (result->opcode != BT_ATT_OP_READ_BY_TYPE_RSP)
> return 0;
>
> + if (result->data_len != 21 && result->data_len != 7)
> + return 0;
This really needs to be commented (or better use meaningful defines instead of
magic values).
> +
> return result_element_count(result);
> }
>
> @@ -239,6 +242,9 @@ bool bt_gatt_iter_next_characteristic(struct
> bt_gatt_iter *iter, if (iter->result->opcode != BT_ATT_OP_READ_BY_TYPE_RSP)
> return false;
>
> + if (iter->result->data_len != 21 && iter->result->data_len != 7)
> + return false;
Ditto.
> +
> op = iter->result->op;
> pdu_ptr = iter->result->pdu + iter->pos;
--
Szymon K. Janc
szymon.janc@gmail.com
^ permalink raw reply
* Re: [PATCH BlueZ 1/8] shared/att: bt_att_cancel should not cancel pending requests/indications.
From: Arman Uguray @ 2014-10-22 15:40 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Arman Uguray, linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZJ6cR80Y0pmPyW_kgVCdssz-JHcLAij8X3+xx4DQb2+6w@mail.gmail.com>
Hi Luiz,
On Wed, Oct 22, 2014 at 7:39 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <armansito@chromium.org> wrote:
>> bt_att_cancel and bt_att_cancel_all no longer destroy any pending
>> request or indication, since this would cause a later bt_att_send to
>> erroneously send out a request or indication before a
>> response/confirmation for a pending one is received. Instead, they now
>> keep the pending operations but clear their response and destroy
>> callbacks since the upper layer is no longer interested in them.
>> ---
>> src/shared/att.c | 24 ++++++++++++++----------
>> 1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/shared/att.c b/src/shared/att.c
>> index 503e06c..c70d396 100644
>> --- a/src/shared/att.c
>> +++ b/src/shared/att.c
>> @@ -1029,15 +1029,17 @@ bool bt_att_cancel(struct bt_att *att, unsigned int id)
>> return false;
>>
>> if (att->pending_req && att->pending_req->id == id) {
>> - op = att->pending_req;
>> - att->pending_req = NULL;
>> - goto done;
>> + /* Don't cancel the pending request; remove it's handlers */
>> + att->pending_req->callback = NULL;
>> + att->pending_req->destroy = NULL;
>> + return true;
>> }
>>
>> if (att->pending_ind && att->pending_ind->id == id) {
>> - op = att->pending_ind;
>> - att->pending_ind = NULL;
>> - goto done;
>> + /* Don't cancel the pending indication; remove it's handlers */
>> + att->pending_ind->callback = NULL;
>> + att->pending_ind->destroy = NULL;
>> + return true;
>> }
>>
>> op = queue_remove_if(att->req_queue, match_op_id, UINT_TO_PTR(id));
>> @@ -1073,13 +1075,15 @@ bool bt_att_cancel_all(struct bt_att *att)
>> queue_remove_all(att->write_queue, NULL, NULL, destroy_att_send_op);
>>
>> if (att->pending_req) {
>> - destroy_att_send_op(att->pending_req);
>> - att->pending_req = NULL;
>> + /* Don't cancel the pending request; remove it's handlers */
>> + att->pending_req->callback = NULL;
>> + att->pending_req->destroy = NULL;
>> }
>>
>> if (att->pending_ind) {
>> - destroy_att_send_op(att->pending_ind);
>> - att->pending_ind = NULL;
>> + /* Don't cancel the pending indication; remove it's handlers */
>> + att->pending_ind->callback = NULL;
>> + att->pending_ind->destroy = NULL;
>> }
>>
>> return true;
>> --
>> 2.1.0.rc2.206.gedb03e5
>
> I would like to first finish with Marcin's patches, are you okay with
> v5? Btw, please make sure you follow the HACKING document when
> submitting patches so we can be consistent from now on.
>
>
> --
> Luiz Augusto von Dentz
I'm fine with Marcin's v5, though we may have to wait for v6 since I
saw some comments fly by from Szymon.
Yeah I'm following HACKING from now on. So far I used the code around
me (shared/mgmt, etc) as the style guide and might have repeated
mistakes that already existed in the code, so it might makes sense to
do a general style fix pass within src/shared at some point.
-Arman
^ permalink raw reply
* Re: [PATCHv5 00/14] Included service discovery
From: Arman Uguray @ 2014-10-22 15:35 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Marcin Kraglak, linux-bluetooth@vger.kernel.org development
In-Reply-To: <CABBYNZKQ-1s9p_18HOnKW3Sew+cMrW+6SnPJHwp-s5m5nYF3QQ@mail.gmail.com>
Hi Luiz,
On Wed, Oct 22, 2014 at 7:54 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Marcin,
>
> On Wed, Oct 22, 2014 at 9:25 AM, Marcin Kraglak
> <marcin.kraglak@tieto.com> wrote:
>> On 16 October 2014 12:17, Marcin Kraglak <marcin.kraglak@tieto.com> wrote:
>>> v3:
>>> In this version after primary service discovery,
>>> secondary services are discovered. Next included
>>> services are resolved. With this approach we
>>> don't have recursively search for included service,
>>> like it was TODO in previous proposal.
>>> There is also small coding style fix suggested by Arman.
>>>
>>> v4:
>>> If no secondary services found, continue include services search (fixed
>>> in gatt-client.c).
>>> Fixed wrong debug logs (primary->secondary).
>>> Fixed searching descriptors
>>>
>>> v5:
>>> Ignore Unsupported Group Type Error in response to secondary service
>>> discovery and continue included services discovery.
>>>
>>> Marcin Kraglak (14):
>>> shared/gatt: Add discover_secondary_services()
>>> shared/gatt: Add initial implementation of discover_included_services
>>> shared/gatt: Discover included services 128 bit UUIDS
>>> shared/gatt: Add extra check in characteristic iterator
>>> shared/gatt: Add included service iterator
>>> shared/gatt: Remove not needed function parameter
>>> shared/gatt: Distinguish Primary from Secondary services
>>> tools/btgatt-client: Print type of service
>>> shared/gatt: Discover secondary services
>>> shared/gatt: Discover included services
>>> shared/gatt: Add gatt-client include service iterator
>>> tools/btgatt-client: Print found include services
>>> shared/gatt: Fix searching descriptors
>>> shared/gatt: Add function bt_gatt_result_included_count()
>>>
>>> src/shared/gatt-client.c | 263 +++++++++++++++++++++++++++--
>>> src/shared/gatt-client.h | 18 ++
>>> src/shared/gatt-helpers.c | 418 +++++++++++++++++++++++++++++++++++++++++++---
>>> src/shared/gatt-helpers.h | 10 +-
>>> tools/btgatt-client.c | 17 +-
>>> 5 files changed, 690 insertions(+), 36 deletions(-)
>>>
>>> --
>>> 1.9.3
>>>
>
> Patches looks fine to me, but I would like Arman to ack before applying.
>
I'm fine with the patches as they are, though I saw that Szymon has
left comments on some of them (I'm guessing he's a doing a pass of the
patch set now). I'm good with it as long as his comments are
addressed.
> Btw, there is one thing I would like to see addressed, right now
> whenever we disconnect the cache is lost which doesn't use the CCC
> ability to persist across connections, this can cause us to loose
> notifications when reconnecting because the code don't remember any
> handles and this is specially bad for profiles such as HoG since we
> may loose some input events while rediscovering, in fact we should
> probably store the cache whenever the remote supports service changed
> and be able to reload them so we only have to do the discover again if
> something has changed.
>
There's an item in the top-level TODO for long-term caching of
attributes, so I guess this falls under that?
>
> --
> Luiz Augusto von Dentz
> --
> 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,
-Arman
^ permalink raw reply
* [PATCH] hciattach: bcm43xx: Use final baudrate to download the firmware
From: Loic Poulain @ 2014-10-22 15:08 UTC (permalink / raw)
To: johan.hedberg, marcel; +Cc: linux-bluetooth, Loic Poulain
Speed up bcm43xx init by setting the final baudrate before firmware
download.
---
tools/hciattach.c | 2 +-
tools/hciattach.h | 3 ++-
tools/hciattach_bcm43xx.c | 63 +++++++++++++++++++++++++++++------------------
3 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/tools/hciattach.c b/tools/hciattach.c
index bf927fc..14d3511 100644
--- a/tools/hciattach.c
+++ b/tools/hciattach.c
@@ -329,7 +329,7 @@ static int intel(int fd, struct uart_t *u, struct termios *ti)
static int bcm43xx(int fd, struct uart_t *u, struct termios *ti)
{
- return bcm43xx_init(fd, u->speed, ti, u->bdaddr);
+ return bcm43xx_init(fd, u->init_speed, u->speed, ti, u->bdaddr);
}
static int read_check(int fd, void *buf, int count)
diff --git a/tools/hciattach.h b/tools/hciattach.h
index 4810a09..2aaf075 100644
--- a/tools/hciattach.h
+++ b/tools/hciattach.h
@@ -58,4 +58,5 @@ int ath3k_init(int fd, int speed, int init_speed, char *bdaddr,
int ath3k_post(int fd, int pm);
int qualcomm_init(int fd, int speed, struct termios *ti, const char *bdaddr);
int intel_init(int fd, int init_speed, int *speed, struct termios *ti);
-int bcm43xx_init(int fd, int speed, struct termios *ti, const char *bdaddr);
+int bcm43xx_init(int fd, int def_speed, int speed, struct termios *ti,
+ const char *bdaddr);
diff --git a/tools/hciattach_bcm43xx.c b/tools/hciattach_bcm43xx.c
index cb4bfb9..3d36c20 100644
--- a/tools/hciattach_bcm43xx.c
+++ b/tools/hciattach_bcm43xx.c
@@ -154,63 +154,71 @@ static int bcm43xx_set_bdaddr(int fd, const char *bdaddr)
return 0;
}
-static int bcm43xx_set_speed(int fd, uint32_t speed)
+static int bcm43xx_set_clock(int fd, unsigned char clock)
{
- unsigned char cmd[] =
- { HCI_COMMAND_PKT, 0x18, 0xfc, 0x06, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00 };
+ unsigned char cmd[] = { HCI_COMMAND_PKT, 0x45, 0xfc, 0x01, 0x00 };
unsigned char resp[CC_MIN_SIZE];
- printf("Set Controller UART speed to %d bit/s\n", speed);
+ printf("Set Controller clock (%d)\n", clock);
- cmd[6] = (uint8_t) (speed);
- cmd[7] = (uint8_t) (speed >> 8);
- cmd[8] = (uint8_t) (speed >> 16);
- cmd[9] = (uint8_t) (speed >> 24);
+ cmd[4] = clock;
tcflush(fd, TCIOFLUSH);
if (write(fd, cmd, sizeof(cmd)) != sizeof(cmd)) {
- fprintf(stderr, "Failed to write update baudrate command\n");
+ fprintf(stderr, "Failed to write update clock command\n");
return -1;
}
if (read_hci_event(fd, resp, sizeof(resp)) < CC_MIN_SIZE) {
- fprintf(stderr, "Failed to update baudrate, invalid HCI event\n");
+ fprintf(stderr, "Failed to update clock, invalid HCI event\n");
return -1;
}
if (resp[4] != cmd[1] || resp[5] != cmd[2] || resp[6] != CMD_SUCCESS) {
- fprintf(stderr, "Failed to update baudrate, command failure\n");
+ fprintf(stderr, "Failed to update clock, command failure\n");
return -1;
}
return 0;
}
-static int bcm43xx_set_clock(int fd, unsigned char clock)
+static int bcm43xx_set_speed(int fd, struct termios *ti, uint32_t speed)
{
- unsigned char cmd[] = { HCI_COMMAND_PKT, 0x45, 0xfc, 0x01, 0x00 };
+ unsigned char cmd[] =
+ { HCI_COMMAND_PKT, 0x18, 0xfc, 0x06, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00 };
unsigned char resp[CC_MIN_SIZE];
- printf("Set Controller clock (%d)\n", clock);
+ if (speed > 3000000 && bcm43xx_set_clock(fd, BCM43XX_CLOCK_48))
+ return -1;
- cmd[4] = clock;
+ printf("Set Controller UART speed to %d bit/s\n", speed);
+
+ cmd[6] = (uint8_t) (speed);
+ cmd[7] = (uint8_t) (speed >> 8);
+ cmd[8] = (uint8_t) (speed >> 16);
+ cmd[9] = (uint8_t) (speed >> 24);
tcflush(fd, TCIOFLUSH);
if (write(fd, cmd, sizeof(cmd)) != sizeof(cmd)) {
- fprintf(stderr, "Failed to write update clock command\n");
+ fprintf(stderr, "Failed to write update baudrate command\n");
return -1;
}
if (read_hci_event(fd, resp, sizeof(resp)) < CC_MIN_SIZE) {
- fprintf(stderr, "Failed to update clock, invalid HCI event\n");
+ fprintf(stderr, "Failed to update baudrate, invalid HCI event\n");
return -1;
}
if (resp[4] != cmd[1] || resp[5] != cmd[2] || resp[6] != CMD_SUCCESS) {
- fprintf(stderr, "Failed to update clock, command failure\n");
+ fprintf(stderr, "Failed to update baudrate, command failure\n");
+ return -1;
+ }
+
+ if (set_speed(fd, ti, speed) < 0) {
+ perror("Can't set host baud rate");
return -1;
}
@@ -343,7 +351,8 @@ static int bcm43xx_locate_patch(const char *dir_name,
return ret;
}
-int bcm43xx_init(int fd, int speed, struct termios *ti, const char *bdaddr)
+int bcm43xx_init(int fd, int def_speed, int speed, struct termios *ti,
+ const char *bdaddr)
{
char chip_name[20];
char fw_path[PATH_MAX];
@@ -359,9 +368,18 @@ int bcm43xx_init(int fd, int speed, struct termios *ti, const char *bdaddr)
if (bcm43xx_locate_patch(FIRMWARE_DIR, chip_name, fw_path)) {
fprintf(stderr, "Patch not found, continue anyway\n");
} else {
+ if (bcm43xx_set_speed(fd, ti, speed))
+ return -1;
+
if (bcm43xx_load_firmware(fd, fw_path))
return -1;
+ /* Controller speed has been reset to def speed */
+ if (set_speed(fd, ti, def_speed) < 0) {
+ perror("Can't set host baud rate");
+ return -1;
+ }
+
if (bcm43xx_reset(fd))
return -1;
}
@@ -369,10 +387,7 @@ int bcm43xx_init(int fd, int speed, struct termios *ti, const char *bdaddr)
if (bdaddr)
bcm43xx_set_bdaddr(fd, bdaddr);
- if (speed > 3000000 && bcm43xx_set_clock(fd, BCM43XX_CLOCK_48))
- return -1;
-
- if (bcm43xx_set_speed(fd, speed))
+ if (bcm43xx_set_speed(fd, ti, speed))
return -1;
return 0;
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCHv5 00/14] Included service discovery
From: Luiz Augusto von Dentz @ 2014-10-22 14:54 UTC (permalink / raw)
To: Marcin Kraglak; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <CABD6X-+URkwy0eEVMoUusc3ymFb6fdnFZ0z+jfPdVNq+ibXRQQ@mail.gmail.com>
Hi Marcin,
On Wed, Oct 22, 2014 at 9:25 AM, Marcin Kraglak
<marcin.kraglak@tieto.com> wrote:
> On 16 October 2014 12:17, Marcin Kraglak <marcin.kraglak@tieto.com> wrote:
>> v3:
>> In this version after primary service discovery,
>> secondary services are discovered. Next included
>> services are resolved. With this approach we
>> don't have recursively search for included service,
>> like it was TODO in previous proposal.
>> There is also small coding style fix suggested by Arman.
>>
>> v4:
>> If no secondary services found, continue include services search (fixed
>> in gatt-client.c).
>> Fixed wrong debug logs (primary->secondary).
>> Fixed searching descriptors
>>
>> v5:
>> Ignore Unsupported Group Type Error in response to secondary service
>> discovery and continue included services discovery.
>>
>> Marcin Kraglak (14):
>> shared/gatt: Add discover_secondary_services()
>> shared/gatt: Add initial implementation of discover_included_services
>> shared/gatt: Discover included services 128 bit UUIDS
>> shared/gatt: Add extra check in characteristic iterator
>> shared/gatt: Add included service iterator
>> shared/gatt: Remove not needed function parameter
>> shared/gatt: Distinguish Primary from Secondary services
>> tools/btgatt-client: Print type of service
>> shared/gatt: Discover secondary services
>> shared/gatt: Discover included services
>> shared/gatt: Add gatt-client include service iterator
>> tools/btgatt-client: Print found include services
>> shared/gatt: Fix searching descriptors
>> shared/gatt: Add function bt_gatt_result_included_count()
>>
>> src/shared/gatt-client.c | 263 +++++++++++++++++++++++++++--
>> src/shared/gatt-client.h | 18 ++
>> src/shared/gatt-helpers.c | 418 +++++++++++++++++++++++++++++++++++++++++++---
>> src/shared/gatt-helpers.h | 10 +-
>> tools/btgatt-client.c | 17 +-
>> 5 files changed, 690 insertions(+), 36 deletions(-)
>>
>> --
>> 1.9.3
>>
Patches looks fine to me, but I would like Arman to ack before applying.
Btw, there is one thing I would like to see addressed, right now
whenever we disconnect the cache is lost which doesn't use the CCC
ability to persist across connections, this can cause us to loose
notifications when reconnecting because the code don't remember any
handles and this is specially bad for profiles such as HoG since we
may loose some input events while rediscovering, in fact we should
probably store the cache whenever the remote supports service changed
and be able to reload them so we only have to do the discover again if
something has changed.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH BlueZ 1/8] shared/att: bt_att_cancel should not cancel pending requests/indications.
From: Luiz Augusto von Dentz @ 2014-10-22 14:39 UTC (permalink / raw)
To: Arman Uguray; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1413838861-29956-2-git-send-email-armansito@chromium.org>
Hi Arman,
On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <armansito@chromium.org> wrote:
> bt_att_cancel and bt_att_cancel_all no longer destroy any pending
> request or indication, since this would cause a later bt_att_send to
> erroneously send out a request or indication before a
> response/confirmation for a pending one is received. Instead, they now
> keep the pending operations but clear their response and destroy
> callbacks since the upper layer is no longer interested in them.
> ---
> src/shared/att.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index 503e06c..c70d396 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -1029,15 +1029,17 @@ bool bt_att_cancel(struct bt_att *att, unsigned int id)
> return false;
>
> if (att->pending_req && att->pending_req->id == id) {
> - op = att->pending_req;
> - att->pending_req = NULL;
> - goto done;
> + /* Don't cancel the pending request; remove it's handlers */
> + att->pending_req->callback = NULL;
> + att->pending_req->destroy = NULL;
> + return true;
> }
>
> if (att->pending_ind && att->pending_ind->id == id) {
> - op = att->pending_ind;
> - att->pending_ind = NULL;
> - goto done;
> + /* Don't cancel the pending indication; remove it's handlers */
> + att->pending_ind->callback = NULL;
> + att->pending_ind->destroy = NULL;
> + return true;
> }
>
> op = queue_remove_if(att->req_queue, match_op_id, UINT_TO_PTR(id));
> @@ -1073,13 +1075,15 @@ bool bt_att_cancel_all(struct bt_att *att)
> queue_remove_all(att->write_queue, NULL, NULL, destroy_att_send_op);
>
> if (att->pending_req) {
> - destroy_att_send_op(att->pending_req);
> - att->pending_req = NULL;
> + /* Don't cancel the pending request; remove it's handlers */
> + att->pending_req->callback = NULL;
> + att->pending_req->destroy = NULL;
> }
>
> if (att->pending_ind) {
> - destroy_att_send_op(att->pending_ind);
> - att->pending_ind = NULL;
> + /* Don't cancel the pending indication; remove it's handlers */
> + att->pending_ind->callback = NULL;
> + att->pending_ind->destroy = NULL;
> }
>
> return true;
> --
> 2.1.0.rc2.206.gedb03e5
I would like to first finish with Marcin's patches, are you okay with
v5? Btw, please make sure you follow the HACKING document when
submitting patches so we can be consistent from now on.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH v3] obexd/mas: Add Support for MSETime filter
From: Luiz Augusto von Dentz @ 2014-10-22 14:12 UTC (permalink / raw)
To: Bharat Panda; +Cc: linux-bluetooth@vger.kernel.org, cpgs
In-Reply-To: <1413896899-29802-1-git-send-email-bharat.panda@samsung.com>
Hi,
On Tue, Oct 21, 2014 at 4:08 PM, Bharat Panda <bharat.panda@samsung.com> wrote:
> Changes made to add support for MSE local time and timezone offset
> parameter along with GetMessageListing response.
> ---
> obexd/plugins/mas.c | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/obexd/plugins/mas.c b/obexd/plugins/mas.c
> index fb97fe3..5b95b3a 100644
> --- a/obexd/plugins/mas.c
> +++ b/obexd/plugins/mas.c
> @@ -30,6 +30,7 @@
> #include <glib.h>
> #include <fcntl.h>
> #include <inttypes.h>
> +#include <sys/time.h>
>
> #include <gobex/gobex.h>
> #include <gobex/gobex-apparam.h>
> @@ -228,6 +229,34 @@ static void g_string_append_escaped_printf(GString *string,
> va_end(ap);
> }
>
> +static gchar *get_mse_timestamp(void)
> +{
> + struct timeval time_val;
> + struct tm ltime;
> + gchar *local_ts;
> + char sign;
> +
> + gettimeofday(&time_val, NULL);
> +
> + if (!localtime_r(&time_val.tv_sec, <ime))
> + return NULL;
> +
> + if (difftime(mktime(localtime(&time_val.tv_sec)),
> + mktime(gmtime(&time_val.tv_sec))) < 0)
> + sign = '+';
> + else
> + sign = '-';
> +
> + local_ts = g_strdup_printf("%04d%02d%02dT%02d%02d%02d%c%2ld%2ld",
> + ltime.tm_year + 1900, ltime.tm_mon + 1,
> + ltime.tm_mday, ltime.tm_hour,
> + ltime.tm_min, ltime.tm_sec, sign,
> + ltime.tm_gmtoff/3600,
> + (ltime.tm_gmtoff%3600)/60);
> +
> + return local_ts;
> +}
> +
> static const char *yesorno(gboolean a)
> {
> if (a)
> @@ -243,6 +272,7 @@ static void get_messages_listing_cb(void *session, int err, uint16_t size,
> {
> struct mas_session *mas = user_data;
> uint16_t max = 1024;
> + gchar *mse_time;
>
> if (err < 0 && err != -EAGAIN) {
> obex_object_set_io_flags(mas, G_IO_ERR, err);
> @@ -358,6 +388,13 @@ proceed:
> mas->outparams = g_obex_apparam_set_uint8(mas->outparams,
> MAP_AP_NEWMESSAGE,
> newmsg ? 1 : 0);
> + /* Response to report the local time of MSE */
> + mse_time = get_mse_timestamp();
> + if (mse_time) {
> + g_obex_apparam_set_string(mas->outparams,
> + MAP_AP_MSETIME, mse_time);
> + g_free(mse_time);
> + }
> }
>
> if (err != -EAGAIN)
> --
> 1.9.1
Applied, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply
* [RFC] Bluetooth : Fix hci_sync miss wakeup interrupt
From: 박찬열 @ 2014-10-22 13:48 UTC (permalink / raw)
To: linux-bluetooth
DQpfX2hjaV9jbWRfc3luY19ldigpLCBfX2hjaV9yZXFfc3luYygpIGNvdWxkIG1pc3Mgd2FrZV91
cF9pbnRlcnJ1cHQgZnJvbQ0KaGNpX3JlcV9zeW5jX2NvbXBsZXRlKCkgYmVjYXVzZSBoY2lfY21k
X3dvcmsoKSB3b3JrcXVlZSBhbmQgaXRzIHJlcG9uc2UNCmNvdWxkIGJlIGNvbXBsZXRlZCBiZWZv
cmUgdGhleSBhcmUgcmVhZHkgdG8gZ2V0IHRoZSBzaWduYWwgdGhyb3VnaA0KYWRkX3dhaXRfcXVl
dWUoKSwgc2V0X2N1cnJlbnRfc3RhdGUoVEFTS19JTlRFUlJVUFRJQkxFKS4NCg0KU2lnbmVkLW9m
Zi1ieTogQ2hhbi15ZW9sIFBhcmsgPGNoYW55ZW9sLnBhcmtAc2Ftc3VuZy5jb20+DQpTaWduZWQt
b2ZmLWJ5OiBLeXVuZ21pbiBQYXJrIDxreXVuZ21pbi5wYXJrQHNhbXN1bmcuY29tPg0KLS0tDQog
bmV0L2JsdWV0b290aC9oY2lfY29yZS5jIHwgMTggKysrKysrKysrKystLS0tLS0tDQogMSBmaWxl
IGNoYW5nZWQsIDExIGluc2VydGlvbnMoKyksIDcgZGVsZXRpb25zKC0pDQoNCmRpZmYgLS1naXQg
YS9uZXQvYmx1ZXRvb3RoL2hjaV9jb3JlLmMgYi9uZXQvYmx1ZXRvb3RoL2hjaV9jb3JlLmMNCmlu
ZGV4IGNiMDVkN2YuLmMwMDhmMWYgMTAwNjQ0DQotLS0gYS9uZXQvYmx1ZXRvb3RoL2hjaV9jb3Jl
LmMNCisrKyBiL25ldC9ibHVldG9vdGgvaGNpX2NvcmUuYw0KQEAgLTExNDcsMTMgKzExNDcsMTUg
QEAgc3RydWN0IHNrX2J1ZmYgKl9faGNpX2NtZF9zeW5jX2V2KHN0cnVjdCBoY2lfZGV2ICpoZGV2
LCB1MTYgb3Bjb2RlLCB1MzIgcGxlbiwNCg0KICAgICAgICBoZGV2LT5yZXFfc3RhdHVzID0gSENJ
X1JFUV9QRU5EOw0KDQotICAgICAgIGVyciA9IGhjaV9yZXFfcnVuKCZyZXEsIGhjaV9yZXFfc3lu
Y19jb21wbGV0ZSk7DQotICAgICAgIGlmIChlcnIgPCAwKQ0KLSAgICAgICAgICAgICAgIHJldHVy
biBFUlJfUFRSKGVycik7DQotDQogICAgICAgIGFkZF93YWl0X3F1ZXVlKCZoZGV2LT5yZXFfd2Fp
dF9xLCAmd2FpdCk7DQogICAgICAgIHNldF9jdXJyZW50X3N0YXRlKFRBU0tfSU5URVJSVVBUSUJM
RSk7DQoNCisgICAgICAgZXJyID0gaGNpX3JlcV9ydW4oJnJlcSwgaGNpX3JlcV9zeW5jX2NvbXBs
ZXRlKTsNCisgICAgICAgaWYgKGVyciA8IDApIHsNCisgICAgICAgICAgICAgICByZW1vdmVfd2Fp
dF9xdWV1ZSgmaGRldi0+cmVxX3dhaXRfcSwgJndhaXQpOw0KKyAgICAgICAgICAgICAgIHJldHVy
biBFUlJfUFRSKGVycik7DQorICAgICAgIH0NCisNCiAgICAgICAgc2NoZWR1bGVfdGltZW91dCh0
aW1lb3V0KTsNCg0KICAgICAgICByZW1vdmVfd2FpdF9xdWV1ZSgmaGRldi0+cmVxX3dhaXRfcSwg
JndhaXQpOw0KQEAgLTEyMTEsMTAgKzEyMTMsMTUgQEAgc3RhdGljIGludCBfX2hjaV9yZXFfc3lu
YyhzdHJ1Y3QgaGNpX2RldiAqaGRldiwNCg0KICAgICAgICBmdW5jKCZyZXEsIG9wdCk7DQoNCisg
ICAgICAgYWRkX3dhaXRfcXVldWUoJmhkZXYtPnJlcV93YWl0X3EsICZ3YWl0KTsNCisgICAgICAg
c2V0X2N1cnJlbnRfc3RhdGUoVEFTS19JTlRFUlJVUFRJQkxFKTsNCisNCiAgICAgICAgZXJyID0g
aGNpX3JlcV9ydW4oJnJlcSwgaGNpX3JlcV9zeW5jX2NvbXBsZXRlKTsNCiAgICAgICAgaWYgKGVy
ciA8IDApIHsNCiAgICAgICAgICAgICAgICBoZGV2LT5yZXFfc3RhdHVzID0gMDsNCg0KKyAgICAg
ICAgICAgICAgIHJlbW92ZV93YWl0X3F1ZXVlKCZoZGV2LT5yZXFfd2FpdF9xLCAmd2FpdCk7DQor
DQogICAgICAgICAgICAgICAgLyogRU5PREFUQSBtZWFucyB0aGUgSENJIHJlcXVlc3QgY29tbWFu
ZCBxdWV1ZSBpcyBlbXB0eS4NCiAgICAgICAgICAgICAgICAgKiBUaGlzIGNhbiBoYXBwZW4gd2hl
biBhIHJlcXVlc3Qgd2l0aCBjb25kaXRpb25hbHMgZG9lc24ndA0KICAgICAgICAgICAgICAgICAq
IHRyaWdnZXIgYW55IGNvbW1hbmRzIHRvIGJlIHNlbnQuIFRoaXMgaXMgbm9ybWFsIGJlaGF2aW9y
DQpAQCAtMTIyNiw5ICsxMjMzLDYgQEAgc3RhdGljIGludCBfX2hjaV9yZXFfc3luYyhzdHJ1Y3Qg
aGNpX2RldiAqaGRldiwNCiAgICAgICAgICAgICAgICByZXR1cm4gZXJyOw0KICAgICAgICB9DQoN
Ci0gICAgICAgYWRkX3dhaXRfcXVldWUoJmhkZXYtPnJlcV93YWl0X3EsICZ3YWl0KTsNCi0gICAg
ICAgc2V0X2N1cnJlbnRfc3RhdGUoVEFTS19JTlRFUlJVUFRJQkxFKTsNCi0NCiAgICAgICAgc2No
ZWR1bGVfdGltZW91dCh0aW1lb3V0KTsNCg0KICAgICAgICByZW1vdmVfd2FpdF9xdWV1ZSgmaGRl
di0+cmVxX3dhaXRfcSwgJndhaXQpOw0KLS0NCjEuOS4xIA0KICAgICAgIA0KDQogIA0KDQogICAg
ICAgICANCiAgDQo=
^ permalink raw reply
* Re: rtk_btusb issues
From: Patrick Shirkey @ 2014-10-22 13:42 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <7FC77530-F48C-436C-8954-195E9CD08E97@holtmann.org>
On Thu, October 23, 2014 12:05 am, Marcel Holtmann wrote:
> Hi Patrick,
>
>> I have an android device running the rtk_btusb driver. The original
>> driver
>> was a couple of years old and there was a problem with stability with
>> BLE.
>> In addition the localname was not found in the scan_response data.
>>
>> I upgraded the bluetooth stack by manually backporting from a recent
>> kernel. That fixed the issue wit the scan_repsonse data but there were
>> still stability issues with BLE devices (although slightly improved
>> compared to the older bluetooth stack).
>>
>> I have tried upgrading the driver to the latest version of the rtk_btusb
>> driver from the git repo and ported the hooks for bluedroid from the old
>> driver however I get issues with hci send command when loading the
>> driver.
>>
>> http://pastebin.com/hXALmXBr
>>
>> I traced the command but I am not sure where the send value is
>> generated.
>> I can see the function definition in the hci_dev struct but not the
>> location where the value is actually set.
>>
>> I thought it might be an issue specific to bluedroid so I installed
>> bluez
>> but the issue is still there so it appears to be a bug in my version of
>> the driver or something wrong with the bluetooth kernel layer.
>>
>> I went back to the older version of the driver running against bluez
>> instead of bluedroid. That gets me further along but I still cannot
>> initialise the bluetooth system on the device.
>>
>> http://pastebin.com/HcSZXiMu
>>
>> Does anyone have any suggestions for how to go about resolving this
>> situation I have got myself into?
>
> the rtk_btusb driver is not an upstream driver and that is the main
> problem. There has been recently an effort to get btusb support the
> Realtek devices, but then it went silent again.
>
> What you need is having proper Realtek support in btusb and nothing else.
> Out of tree hacked up vendor drivers are not something any of us will
> likely have a look at and trying to fix.
>
My quest is to fix the stability issue not the rtk_btusb driver. I
spotted the "new" branch in the git tree. I am testing that driver now.
Do you know if there are known issues with BLE stability on the 8723au
chipset?
--
Patrick Shirkey
Boost Hardware Ltd
^ permalink raw reply
* Re: [PATCH 3/3] bnep: Return errno instead of -1 and print error
From: Andrei Emeltchenko @ 2014-10-22 13:37 UTC (permalink / raw)
To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
In-Reply-To: <CAMv5Gbck+=b9kY-s+gd3shJsLUDAEZib5dKGM09ZBWkP==ufFw@mail.gmail.com>
Hi Grzegorz,
On Wed, Oct 22, 2014 at 02:55:54PM +0200, Grzegorz Kolodziejczyk wrote:
> Hi Andrei,
>
> On 22 October 2014 13:10, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > Make code consistent with the rest returning -errno and printing error
> > message.
> > ---
> > profiles/network/bnep.c | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> > index 4cf38d9..09d4b65 100644
> > --- a/profiles/network/bnep.c
> > +++ b/profiles/network/bnep.c
> > @@ -231,7 +231,7 @@ static int bnep_if_down(const char *devname)
> > if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
> > err = -errno;
> > error("bnep: Could not bring down %s: %s(%d)",
> > - devname, strerror(-err), -err);
> > + devname, strerror(-err), -err);
>
> Why don't you fix this indention in first patch ? (this line is added
> in previous patch)
Sorry, resend the series.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCHv5 02/14] shared/gatt: Add initial implementation of discover_included_services
From: Szymon Janc @ 2014-10-22 13:37 UTC (permalink / raw)
To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-3-git-send-email-marcin.kraglak@tieto.com>
Hi Marcin,
On Thursday 16 of October 2014 12:17:14 Marcin Kraglak wrote:
> Current implementation allow to discover included services with
> 16 bit UUID only.
> ---
> src/shared/gatt-helpers.c | 108 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 106 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 4dc787f..dcb2a2f 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -692,6 +692,82 @@ bool bt_gatt_discover_secondary_services(struct bt_att *att, bt_uuid_t *uuid,
> destroy, false);
> }
>
> +static void discover_included_cb(uint8_t opcode, const void *pdu,
> + uint16_t length, void *user_data)
> +{
> + struct bt_gatt_result *final_result = NULL;
> + struct discovery_op *op = user_data;
> + struct bt_gatt_result *cur_result;
> + uint8_t att_ecode = 0;
> + uint16_t last_handle;
> + size_t data_length;
> + bool success;
> +
> + if (opcode == BT_ATT_OP_ERROR_RSP) {
> + success = false;
set this right before goto , otherwise it looks strange to
set success to false and then jump to success label.
> + att_ecode = process_error(pdu, length);
> +
> + if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
> + op->result_head)
> + goto success;
> +
> + goto done;
> + }
> +
> + if (opcode != BT_ATT_OP_READ_BY_TYPE_RSP || !pdu || length < 6) {
> + success = false;
> + goto done;
> + }
> +
> + data_length = ((uint8_t *) pdu)[0];
pdu is const pointer so cast it to (const uint8_t *).
Also I'm not entirely sure about this byte pdu processing, maybe we should have
proper packed structs for those (not sure how feasible that would be, though)
> +
Please comment this in code.
> + if ((length - 1) % data_length || data_length != 8) {
> + success = false;
> + goto done;
> + }
> +
> + cur_result = result_create(opcode, pdu + 1, length - 1,
> + data_length, op);
> + if (!cur_result) {
> + success = false;
> + goto done;
> + }
> +
> + if (!op->result_head)
> + op->result_head = op->result_tail = cur_result;
Both branches should in braces.
> + else {
> + op->result_tail->next = cur_result;
> + op->result_tail = cur_result;
> + }
> +
> + last_handle = get_le16(pdu + length - data_length);
> + if (last_handle != op->end_handle) {
> + uint8_t pdu[6];
> +
> + put_le16(last_handle + 1, pdu);
> + put_le16(op->end_handle, pdu + 2);
> + put_le16(GATT_INCLUDE_UUID, pdu + 4);
> +
> + if (bt_att_send(op->att, BT_ATT_OP_READ_BY_TYPE_REQ,
> + pdu, sizeof(pdu),
> + discover_included_cb,
> + discovery_op_ref(op),
> + discovery_op_unref))
> + return;
> +
> + discovery_op_unref(op);
> + success = false;
goto done: missing maybe?
also I'd change labels' names a bit:
done -> failed
success -> done
> + }
> +
> +success:
> + success = true;
> + final_result = op->result_head;
> +
> +done:
> + if (op->callback)
> + op->callback(success, att_ecode, final_result, op->user_data);
> +}
> +
> bool bt_gatt_discover_included_services(struct bt_att *att,
> uint16_t start, uint16_t end,
> bt_uuid_t *uuid,
> @@ -699,8 +775,36 @@ bool bt_gatt_discover_included_services(struct bt_att *att,
> void *user_data,
> bt_gatt_destroy_func_t destroy)
> {
> - /* TODO */
> - return false;
> + struct discovery_op *op;
> + uint8_t pdu[6];
> +
> + if (!att)
> + return false;
> +
> + op = new0(struct discovery_op, 1);
> + if (!op)
> + return false;
> +
> + op->att = att;
> + op->callback = callback;
> + op->user_data = user_data;
> + op->destroy = destroy;
> + op->end_handle = end;
> +
> + put_le16(start, pdu);
> + put_le16(end, pdu + 2);
> + put_le16(GATT_INCLUDE_UUID, pdu + 4);
> +
> + if (!bt_att_send(att, BT_ATT_OP_READ_BY_TYPE_REQ,
> + pdu, sizeof(pdu),
> + discover_included_cb,
> + discovery_op_ref(op),
> + discovery_op_unref)) {
> + free(op);
> + return false;
> + }
> +
> + return true;
> }
>
> static void discover_chrcs_cb(uint8_t opcode, const void *pdu,
>
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [PATCHv5 01/14] shared/gatt: Add discover_secondary_services()
From: Szymon Janc @ 2014-10-22 13:37 UTC (permalink / raw)
To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-2-git-send-email-marcin.kraglak@tieto.com>
Hi Marcin,
On Thursday 16 of October 2014 12:17:13 Marcin Kraglak wrote:
> This pach implements searching Secondary Services in given range.
typo pach -> patch
> ---
> src/shared/gatt-helpers.c | 56 +++++++++++++++++++++++++++++++++--------------
> src/shared/gatt-helpers.h | 5 +++++
> 2 files changed, 45 insertions(+), 16 deletions(-)
>
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 55e6992..4dc787f 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -175,6 +175,7 @@ struct discovery_op {
> uint16_t end_handle;
> int ref_count;
> bt_uuid_t uuid;
> + uint16_t service_type;
> struct bt_gatt_result *result_head;
> struct bt_gatt_result *result_tail;
> bt_gatt_discovery_callback_t callback;
> @@ -487,7 +488,7 @@ static void read_by_grp_type_cb(uint8_t opcode, const void *pdu,
>
> put_le16(last_end + 1, pdu);
> put_le16(op->end_handle, pdu + 2);
> - put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> + put_le16(op->service_type, pdu + 4);
>
> if (bt_att_send(op->att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
> pdu, sizeof(pdu),
> @@ -569,7 +570,7 @@ static void find_by_type_val_cb(uint8_t opcode, const void *pdu,
>
> put_le16(last_end + 1, pdu);
> put_le16(op->end_handle, pdu + 2);
> - put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> + put_le16(op->service_type, pdu + 4);
> put_uuid_le(&op->uuid, pdu + 6);
>
> if (bt_att_send(op->att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
> @@ -594,21 +595,12 @@ done:
> op->callback(success, att_ecode, final_result, op->user_data);
> }
>
> -bool bt_gatt_discover_all_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> - bt_gatt_discovery_callback_t callback,
> - void *user_data,
> - bt_gatt_destroy_func_t destroy)
> -{
> - return bt_gatt_discover_primary_services(att, uuid, 0x0001, 0xffff,
> - callback, user_data,
> - destroy);
> -}
> -
> -bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> +static bool discover_services(struct bt_att *att, bt_uuid_t *uuid,
> uint16_t start, uint16_t end,
> bt_gatt_discovery_callback_t callback,
> void *user_data,
> - bt_gatt_destroy_func_t destroy)
> + bt_gatt_destroy_func_t destroy,
> + bool primary)
> {
> struct discovery_op *op;
> bool result;
> @@ -625,6 +617,8 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> op->callback = callback;
> op->user_data = user_data;
> op->destroy = destroy;
> + /* set service uuid to primary or secondary */
> + op->service_type = primary ? GATT_PRIM_SVC_UUID : GATT_SND_SVC_UUID;
>
> /* If UUID is NULL, then discover all primary services */
> if (!uuid) {
> @@ -632,7 +626,7 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
>
> put_le16(start, pdu);
> put_le16(end, pdu + 2);
> - put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> + put_le16(op->service_type, pdu + 4);
>
> result = bt_att_send(att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
> pdu, sizeof(pdu),
> @@ -652,7 +646,7 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
>
> put_le16(start, pdu);
> put_le16(end, pdu + 2);
> - put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> + put_le16(op->service_type, pdu + 4);
> put_uuid_le(&op->uuid, pdu + 6);
>
> result = bt_att_send(att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
> @@ -668,6 +662,36 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> return result;
> }
>
> +bool bt_gatt_discover_all_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + return bt_gatt_discover_primary_services(att, uuid, 0x0001, 0xffff,
> + callback, user_data,
> + destroy);
> +}
> +
> +bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> + uint16_t start, uint16_t end,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + return discover_services(att, uuid, start, end, callback, user_data,
> + destroy, true);
> +}
> +
> +bool bt_gatt_discover_secondary_services(struct bt_att *att, bt_uuid_t *uuid,
> + uint16_t start, uint16_t end,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + return discover_services(att, uuid, start, end, callback, user_data,
> + destroy, false);
> +}
> +
> bool bt_gatt_discover_included_services(struct bt_att *att,
> uint16_t start, uint16_t end,
> bt_uuid_t *uuid,
> diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
> index f6f4b62..8a25dea 100644
> --- a/src/shared/gatt-helpers.h
> +++ b/src/shared/gatt-helpers.h
> @@ -72,6 +72,11 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> bt_gatt_discovery_callback_t callback,
> void *user_data,
> bt_gatt_destroy_func_t destroy);
> +bool bt_gatt_discover_secondary_services(struct bt_att *att, bt_uuid_t *uuid,
> + uint16_t start, uint16_t end,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);
> bool bt_gatt_discover_included_services(struct bt_att *att,
> uint16_t start, uint16_t end,
> bt_uuid_t *uuid,
This is not related to this patch only but I have a feeling that those
functions names are getting to long. Maybe we should think of something
more compact?
--
Best regards,
Szymon Janc
^ permalink raw reply
* [PATCHv2 3/3] bnep: Return errno instead of -1 and print error
From: Andrei Emeltchenko @ 2014-10-22 13:34 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413984870-24023-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Make code consistent with the rest returning -errno and printing error
message.
---
profiles/network/bnep.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index d8883d6..09d4b65 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -520,7 +520,7 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
{
int ifindex;
struct ifreq ifr;
- int sk, err;
+ int sk, err = 0;
if (!devname || !bridge)
return -EINVAL;
@@ -535,16 +535,16 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
strncpy(ifr.ifr_name, bridge, IFNAMSIZ - 1);
ifr.ifr_ifindex = ifindex;
- err = ioctl(sk, SIOCBRDELIF, &ifr);
+ if (ioctl(sk, SIOCBRDELIF, &ifr) < 0) {
+ err = -errno;
+ error("bnep: Can't delete %s from the bridge %s: %s(%d)",
+ devname, bridge, strerror(-err), -err);
+ } else
+ info("bridge %s: interface %s removed", bridge, devname);
close(sk);
- if (err < 0)
- return err;
-
- info("bridge %s: interface %s removed", bridge, devname);
-
- return 0;
+ return err;
}
int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
--
1.9.1
^ permalink raw reply related
* [PATCHv2 2/3] bnep: trivial: code cleanup
From: Andrei Emeltchenko @ 2014-10-22 13:34 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413984870-24023-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
profiles/network/bnep.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index f8fcd1f..d8883d6 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -133,7 +133,6 @@ const char *bnep_name(uint16_t id)
int bnep_init(void)
{
ctl = socket(PF_BLUETOOTH, SOCK_RAW, BTPROTO_BNEP);
-
if (ctl < 0) {
int err = -errno;
@@ -141,7 +140,7 @@ int bnep_init(void)
warn("kernel lacks bnep-protocol support");
else
error("bnep: Failed to open control socket: %s (%d)",
- strerror(-err), -err);
+ strerror(-err), -err);
return err;
}
@@ -165,7 +164,7 @@ static int bnep_conndel(const bdaddr_t *dst)
if (ioctl(ctl, BNEPCONNDEL, &req) < 0) {
int err = -errno;
error("bnep: Failed to kill connection: %s (%d)",
- strerror(-err), -err);
+ strerror(-err), -err);
return err;
}
return 0;
@@ -184,7 +183,7 @@ static int bnep_connadd(int sk, uint16_t role, char *dev)
if (ioctl(ctl, BNEPCONNADD, &req) < 0) {
int err = -errno;
error("bnep: Failed to add device %s: %s(%d)",
- dev, strerror(-err), -err);
+ dev, strerror(-err), -err);
return err;
}
@@ -208,7 +207,7 @@ static int bnep_if_up(const char *devname)
if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
err = -errno;
error("bnep: Could not bring up %s: %s(%d)",
- devname, strerror(-err), -err);
+ devname, strerror(-err), -err);
}
close(sk);
--
1.9.1
^ permalink raw reply related
* [PATCHv2 1/3] bnep: Add error print and return errno instead of -1
From: Andrei Emeltchenko @ 2014-10-22 13:34 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <CAMv5Gbck+=b9kY-s+gd3shJsLUDAEZib5dKGM09ZBWkP==ufFw@mail.gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
profiles/network/bnep.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 927367c..f8fcd1f 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -219,7 +219,7 @@ static int bnep_if_up(const char *devname)
static int bnep_if_down(const char *devname)
{
struct ifreq ifr;
- int sk, err;
+ int sk, err = 0;
sk = socket(AF_INET, SOCK_DGRAM, 0);
@@ -229,16 +229,15 @@ static int bnep_if_down(const char *devname)
ifr.ifr_flags &= ~IFF_UP;
/* Bring down the interface */
- err = ioctl(sk, SIOCSIFFLAGS, (void *) &ifr);
+ if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
+ err = -errno;
+ error("bnep: Could not bring down %s: %s(%d)",
+ devname, strerror(-err), -err);
+ }
close(sk);
- if (err < 0) {
- error("bnep: Could not bring down %s", devname);
- return err;
- }
-
- return 0;
+ return err;
}
static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
--
1.9.1
^ permalink raw reply related
* Re: rtk_btusb issues
From: Marcel Holtmann @ 2014-10-22 13:05 UTC (permalink / raw)
To: Patrick Shirkey; +Cc: linux-bluetooth
In-Reply-To: <59774.86.105.94.79.1413974113.squirrel@boosthardware.com>
Hi Patrick,
> I have an android device running the rtk_btusb driver. The original driver
> was a couple of years old and there was a problem with stability with BLE.
> In addition the localname was not found in the scan_response data.
>
> I upgraded the bluetooth stack by manually backporting from a recent
> kernel. That fixed the issue wit the scan_repsonse data but there were
> still stability issues with BLE devices (although slightly improved
> compared to the older bluetooth stack).
>
> I have tried upgrading the driver to the latest version of the rtk_btusb
> driver from the git repo and ported the hooks for bluedroid from the old
> driver however I get issues with hci send command when loading the driver.
>
> http://pastebin.com/hXALmXBr
>
> I traced the command but I am not sure where the send value is generated.
> I can see the function definition in the hci_dev struct but not the
> location where the value is actually set.
>
> I thought it might be an issue specific to bluedroid so I installed bluez
> but the issue is still there so it appears to be a bug in my version of
> the driver or something wrong with the bluetooth kernel layer.
>
> I went back to the older version of the driver running against bluez
> instead of bluedroid. That gets me further along but I still cannot
> initialise the bluetooth system on the device.
>
> http://pastebin.com/HcSZXiMu
>
> Does anyone have any suggestions for how to go about resolving this
> situation I have got myself into?
the rtk_btusb driver is not an upstream driver and that is the main problem. There has been recently an effort to get btusb support the Realtek devices, but then it went silent again.
What you need is having proper Realtek support in btusb and nothing else. Out of tree hacked up vendor drivers are not something any of us will likely have a look at and trying to fix.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 3/3] bnep: Return errno instead of -1 and print error
From: Grzegorz Kolodziejczyk @ 2014-10-22 12:55 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1413976251-32402-3-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
On 22 October 2014 13:10, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Make code consistent with the rest returning -errno and printing error
> message.
> ---
> profiles/network/bnep.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index 4cf38d9..09d4b65 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -231,7 +231,7 @@ static int bnep_if_down(const char *devname)
> if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
> err = -errno;
> error("bnep: Could not bring down %s: %s(%d)",
> - devname, strerror(-err), -err);
> + devname, strerror(-err), -err);
Why don't you fix this indention in first patch ? (this line is added
in previous patch)
> }
>
> close(sk);
> @@ -520,7 +520,7 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
> {
> int ifindex;
> struct ifreq ifr;
> - int sk, err;
> + int sk, err = 0;
>
> if (!devname || !bridge)
> return -EINVAL;
> @@ -535,16 +535,16 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
> strncpy(ifr.ifr_name, bridge, IFNAMSIZ - 1);
> ifr.ifr_ifindex = ifindex;
>
> - err = ioctl(sk, SIOCBRDELIF, &ifr);
> + if (ioctl(sk, SIOCBRDELIF, &ifr) < 0) {
> + err = -errno;
> + error("bnep: Can't delete %s from the bridge %s: %s(%d)",
> + devname, bridge, strerror(-err), -err);
> + } else
> + info("bridge %s: interface %s removed", bridge, devname);
>
> close(sk);
>
> - if (err < 0)
> - return err;
> -
> - info("bridge %s: interface %s removed", bridge, devname);
> -
> - return 0;
> + return err;
> }
>
> int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
> --
> 1.9.1
>
> --
> 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
BR,
Grzegorz
^ permalink raw reply
* Re: [PATCH] android/pts: Update PTS files for HOGP
From: Szymon Janc @ 2014-10-22 11:50 UTC (permalink / raw)
To: Sebastian Chlad; +Cc: linux-bluetooth
In-Reply-To: <1413885347-28938-1-git-send-email-sebastian.chlad@tieto.com>
Hi Sebastian,
On Tuesday 21 of October 2014 11:55:47 Sebastian Chlad wrote:
> PICS and PIXITs updated to PTS 5.3. Regression done for Android
> 4.4.4.
> ---
> android/pics-hogp.txt | 2 +-
> android/pixit-hogp.txt | 2 +-
> android/pts-hogp.txt | 4 ++--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/android/pics-hogp.txt b/android/pics-hogp.txt
> index d7192bf..61cf56a 100644
> --- a/android/pics-hogp.txt
> +++ b/android/pics-hogp.txt
> @@ -1,6 +1,6 @@
> HOGP PICS for the PTS tool.
>
> -PTS version: 5.2
> +PTS version: 5.3
>
> * - different than PTS defaults
> # - not yet implemented/supported
> diff --git a/android/pixit-hogp.txt b/android/pixit-hogp.txt
> index 067c280..d32fe69 100644
> --- a/android/pixit-hogp.txt
> +++ b/android/pixit-hogp.txt
> @@ -1,6 +1,6 @@
> HOGP PIXIT for the PTS tool.
>
> -PTS version: 5.2
> +PTS version: 5.3
>
> * - different than PTS defaults
> & - should be set to IUT Bluetooth address
> diff --git a/android/pts-hogp.txt b/android/pts-hogp.txt
> index 19292d1..d86522d 100644
> --- a/android/pts-hogp.txt
> +++ b/android/pts-hogp.txt
> @@ -1,7 +1,7 @@
> PTS test results for HoG
>
> -PTS version: 5.2
> -Tested: 22-July-2014
> +PTS version: 5.3
> +Tested: 21-October-2014
> Android version: 4.4.4
>
> Results:
>
Patch applied, thanks.
--
Best regards,
Szymon Janc
^ permalink raw reply
* [PATCH 3/3] bnep: Return errno instead of -1 and print error
From: Andrei Emeltchenko @ 2014-10-22 11:10 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413976251-32402-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Make code consistent with the rest returning -errno and printing error
message.
---
profiles/network/bnep.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 4cf38d9..09d4b65 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -231,7 +231,7 @@ static int bnep_if_down(const char *devname)
if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
err = -errno;
error("bnep: Could not bring down %s: %s(%d)",
- devname, strerror(-err), -err);
+ devname, strerror(-err), -err);
}
close(sk);
@@ -520,7 +520,7 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
{
int ifindex;
struct ifreq ifr;
- int sk, err;
+ int sk, err = 0;
if (!devname || !bridge)
return -EINVAL;
@@ -535,16 +535,16 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
strncpy(ifr.ifr_name, bridge, IFNAMSIZ - 1);
ifr.ifr_ifindex = ifindex;
- err = ioctl(sk, SIOCBRDELIF, &ifr);
+ if (ioctl(sk, SIOCBRDELIF, &ifr) < 0) {
+ err = -errno;
+ error("bnep: Can't delete %s from the bridge %s: %s(%d)",
+ devname, bridge, strerror(-err), -err);
+ } else
+ info("bridge %s: interface %s removed", bridge, devname);
close(sk);
- if (err < 0)
- return err;
-
- info("bridge %s: interface %s removed", bridge, devname);
-
- return 0;
+ return err;
}
int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
--
1.9.1
^ permalink raw reply related
* [PATCH 2/3] bnep: trivial: code cleanup
From: Andrei Emeltchenko @ 2014-10-22 11:10 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1413976251-32402-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
profiles/network/bnep.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 1024919..4cf38d9 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -133,7 +133,6 @@ const char *bnep_name(uint16_t id)
int bnep_init(void)
{
ctl = socket(PF_BLUETOOTH, SOCK_RAW, BTPROTO_BNEP);
-
if (ctl < 0) {
int err = -errno;
@@ -141,7 +140,7 @@ int bnep_init(void)
warn("kernel lacks bnep-protocol support");
else
error("bnep: Failed to open control socket: %s (%d)",
- strerror(-err), -err);
+ strerror(-err), -err);
return err;
}
@@ -165,7 +164,7 @@ static int bnep_conndel(const bdaddr_t *dst)
if (ioctl(ctl, BNEPCONNDEL, &req) < 0) {
int err = -errno;
error("bnep: Failed to kill connection: %s (%d)",
- strerror(-err), -err);
+ strerror(-err), -err);
return err;
}
return 0;
@@ -184,7 +183,7 @@ static int bnep_connadd(int sk, uint16_t role, char *dev)
if (ioctl(ctl, BNEPCONNADD, &req) < 0) {
int err = -errno;
error("bnep: Failed to add device %s: %s(%d)",
- dev, strerror(-err), -err);
+ dev, strerror(-err), -err);
return err;
}
@@ -208,7 +207,7 @@ static int bnep_if_up(const char *devname)
if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
err = -errno;
error("bnep: Could not bring up %s: %s(%d)",
- devname, strerror(-err), -err);
+ devname, strerror(-err), -err);
}
close(sk);
--
1.9.1
^ permalink raw reply related
* [PATCH 1/3] bnep: Add error print and return errno instead of -1
From: Andrei Emeltchenko @ 2014-10-22 11:10 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
profiles/network/bnep.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 927367c..1024919 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -219,7 +219,7 @@ static int bnep_if_up(const char *devname)
static int bnep_if_down(const char *devname)
{
struct ifreq ifr;
- int sk, err;
+ int sk, err = 0;
sk = socket(AF_INET, SOCK_DGRAM, 0);
@@ -229,16 +229,15 @@ static int bnep_if_down(const char *devname)
ifr.ifr_flags &= ~IFF_UP;
/* Bring down the interface */
- err = ioctl(sk, SIOCSIFFLAGS, (void *) &ifr);
+ if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
+ err = -errno;
+ error("bnep: Could not bring down %s: %s(%d)",
+ devname, strerror(-err), -err);
+ }
close(sk);
- if (err < 0) {
- error("bnep: Could not bring down %s", devname);
- return err;
- }
-
- return 0;
+ return err;
}
static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v4 01/11] shared/hfp: Add support for HFP HF
From: Szymon Janc @ 2014-10-22 11:00 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
In-Reply-To: <1412898611-12199-2-git-send-email-lukasz.rymanowski@tieto.com>
Hi Łukasz,
On Friday 10 of October 2014 01:50:01 Lukasz Rymanowski wrote:
> This patch add struct hfp_hf plus fuctions to create an instance ref and
> unref. This code based on existing hfp_gw
> ---
> src/shared/hfp.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/hfp.h | 6 ++++
> 2 files changed, 98 insertions(+)
>
> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
> index efc981f..dbd049a 100644
> --- a/src/shared/hfp.c
> +++ b/src/shared/hfp.c
> @@ -62,6 +62,18 @@ struct hfp_gw {
> bool destroyed;
> };
>
> +struct hfp_hf {
> + int ref_count;
> + int fd;
> + bool close_on_unref;
> + struct io *io;
> + struct ringbuf *read_buf;
> + struct ringbuf *write_buf;
> +
> + bool in_disconnect;
> + bool destroyed;
> +};
> +
> struct cmd_handler {
> char *prefix;
> void *user_data;
> @@ -807,3 +819,83 @@ bool hfp_gw_disconnect(struct hfp_gw *hfp)
>
> return io_shutdown(hfp->io);
> }
> +
> +struct hfp_hf *hfp_hf_new(int fd)
> +{
> + struct hfp_hf *hfp;
> +
> + if (fd < 0)
> + return NULL;
> +
> + hfp = new0(struct hfp_hf, 1);
> + if (!hfp)
> + return NULL;
> +
> + hfp->fd = fd;
> + hfp->close_on_unref = false;
> +
> + hfp->read_buf = ringbuf_new(4096);
> + if (!hfp->read_buf) {
> + free(hfp);
> + return NULL;
> + }
> +
> + hfp->write_buf = ringbuf_new(4096);
> + if (!hfp->write_buf) {
> + ringbuf_free(hfp->read_buf);
> + free(hfp);
> + return NULL;
> + }
> +
> + hfp->io = io_new(fd);
> + if (!hfp->io) {
> + ringbuf_free(hfp->write_buf);
> + ringbuf_free(hfp->read_buf);
> + free(hfp);
> + return NULL;
> + }
> +
> + return hfp_hf_ref(hfp);
> +}
> +
> +struct hfp_hf *hfp_hf_ref(struct hfp_hf *hfp)
> +{
> + if (!hfp)
> + return NULL;
> +
> + __sync_fetch_and_add(&hfp->ref_count, 1);
> +
> + return hfp;
> +}
> +
> +void hfp_hf_unref(struct hfp_hf *hfp)
> +{
> + if (!hfp)
> + return;
> +
> + if (__sync_sub_and_fetch(&hfp->ref_count, 1))
> + return;
> +
> + io_set_write_handler(hfp->io, NULL, NULL, NULL);
> + io_set_read_handler(hfp->io, NULL, NULL, NULL);
> + io_set_disconnect_handler(hfp->io, NULL, NULL, NULL);
> +
> + io_destroy(hfp->io);
> + hfp->io = NULL;
> +
> + if (hfp->close_on_unref)
> + close(hfp->fd);
> +
> + ringbuf_free(hfp->read_buf);
> + hfp->read_buf = NULL;
> +
> + ringbuf_free(hfp->write_buf);
> + hfp->write_buf = NULL;
> +
> + if (!hfp->in_disconnect) {
> + free(hfp);
> + return;
> + }
> +
> + hfp->destroyed = true;
> +}
> diff --git a/src/shared/hfp.h b/src/shared/hfp.h
> index 743db65..50d9c4b 100644
> --- a/src/shared/hfp.h
> +++ b/src/shared/hfp.h
> @@ -76,9 +76,11 @@ typedef void (*hfp_destroy_func_t)(void *user_data);
> typedef void (*hfp_debug_func_t)(const char *str, void *user_data);
>
> typedef void (*hfp_command_func_t)(const char *command, void *user_data);
> +
Unrelated.
> typedef void (*hfp_disconnect_func_t)(void *user_data);
>
> struct hfp_gw;
> +struct hfp_hf;
I'd prefer if we have all hfp_hf stuff in same section.
>
> struct hfp_gw *hfp_gw_new(int fd);
>
> @@ -124,3 +126,7 @@ bool hfp_gw_result_get_string(struct hfp_gw_result *result, char *buf,
> bool hfp_gw_result_get_unquoted_string(struct hfp_gw_result *result, char *buf,
> uint8_t len);
> bool hfp_gw_result_has_next(struct hfp_gw_result *result);
> +
> +struct hfp_hf *hfp_hf_new(int fd);
> +struct hfp_hf *hfp_hf_ref(struct hfp_hf *hfp);
> +void hfp_hf_unref(struct hfp_hf *hfp);
>
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [PATCH v4 04/11] shared/hfp: Add register/unregister event for HFP HF
From: Szymon Janc @ 2014-10-22 11:00 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
In-Reply-To: <1412898611-12199-5-git-send-email-lukasz.rymanowski@tieto.com>
Hi Łukasz,
On Friday 10 of October 2014 01:50:04 Lukasz Rymanowski wrote:
> This patch adds API which allows to register/unregister for unsolicited
> responses.
> ---
> src/shared/hfp.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/hfp.h | 8 +++++
> 2 files changed, 116 insertions(+)
>
> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
> index b7855ed..b1cf08e 100644
> --- a/src/shared/hfp.c
> +++ b/src/shared/hfp.c
> @@ -70,6 +70,8 @@ struct hfp_hf {
> struct ringbuf *read_buf;
> struct ringbuf *write_buf;
>
> + struct queue *event_handlers;
> +
> hfp_debug_func_t debug_callback;
> hfp_destroy_func_t debug_destroy;
> void *debug_data;
> @@ -94,6 +96,18 @@ struct hfp_gw_result {
> unsigned int offset;
> };
>
> +struct hfp_hf_result {
> + const char *data;
> + unsigned int offset;
> +};
> +
> +struct event_handler {
> + char *prefix;
> + void *user_data;
> + hfp_destroy_func_t destroy;
> + hfp_hf_result_func_t callback;
> +};
> +
> static void destroy_cmd_handler(void *data)
> {
> struct cmd_handler *handler = data;
> @@ -828,6 +842,32 @@ bool hfp_gw_disconnect(struct hfp_gw *hfp)
> return io_shutdown(hfp->io);
> }
>
> +static bool match_handler_event_prefix(const void *a, const void *b)
> +{
> + const struct event_handler *handler = a;
> + const char *prefix = b;
> +
> + if (strlen(handler->prefix) != strlen(prefix))
> + return false;
> +
> + if (memcmp(handler->prefix, prefix, strlen(prefix)))
> + return false;
Why not just use strcmp() here?
I'm aware that this was based on gw code so you may also fix it there :)
> +
> + return true;
> +}
> +
> +static void destroy_event_handler(void *data)
> +{
> + struct event_handler *handler = data;
> +
> + if (handler->destroy)
> + handler->destroy(handler->user_data);
> +
> + free(handler->prefix);
> +
> + free(handler);
> +}
> +
> struct hfp_hf *hfp_hf_new(int fd)
> {
> struct hfp_hf *hfp;
> @@ -863,6 +903,15 @@ struct hfp_hf *hfp_hf_new(int fd)
> return NULL;
> }
>
> + hfp->event_handlers = queue_new();
> + if (!hfp->event_handlers) {
> + io_destroy(hfp->io);
> + ringbuf_free(hfp->write_buf);
> + ringbuf_free(hfp->read_buf);
> + free(hfp);
> + return NULL;
> + }
> +
> return hfp_hf_ref(hfp);
> }
>
> @@ -902,6 +951,9 @@ void hfp_hf_unref(struct hfp_hf *hfp)
> ringbuf_free(hfp->write_buf);
> hfp->write_buf = NULL;
>
> + queue_destroy(hfp->event_handlers, destroy_event_handler);
> + hfp->event_handlers = NULL;
> +
> if (!hfp->in_disconnect) {
> free(hfp);
> return;
> @@ -961,6 +1013,62 @@ bool hfp_hf_set_close_on_unref(struct hfp_hf *hfp, bool do_close)
> return true;
> }
>
> +bool hfp_hf_register(struct hfp_hf *hfp, hfp_hf_result_func_t callback,
> + const char *prefix,
> + void *user_data,
> + hfp_destroy_func_t destroy)
> +{
> + struct event_handler *handler;
> +
> + if (!callback)
> + return false;
> +
> + handler = new0(struct event_handler, 1);
> + if (!handler)
> + return false;
> +
> + handler->callback = callback;
> + handler->user_data = user_data;
> +
> + handler->prefix = strdup(prefix);
> + if (!handler->prefix) {
> + free(handler);
> + return false;
> + }
> +
> + if (queue_find(hfp->event_handlers, match_handler_event_prefix,
> + handler->prefix)) {
> + destroy_event_handler(handler);
> + return false;
> + }
> +
> + handler->destroy = destroy;
> +
> + return queue_push_tail(hfp->event_handlers, handler);
> +}
> +
> +bool hfp_hf_unregister(struct hfp_hf *hfp, const char *prefix)
> +{
> + struct cmd_handler *handler;
> + char *lookup_prefix;
> +
> + lookup_prefix = strdup(prefix);
> + if (!lookup_prefix)
> + return false;
This strdup seems superfluous. If this is only due to to queue api being
non-const then I'd just cast it to (void *).
> +
> + handler = queue_remove_if(hfp->event_handlers,
> + match_handler_event_prefix,
> + lookup_prefix);
> + free(lookup_prefix);
> +
> + if (!handler)
> + return false;
> +
> + destroy_event_handler(handler);
> +
> + return true;
> +}
> +
> static void hf_disconnect_watch_destroy(void *user_data)
> {
> struct hfp_hf *hfp = user_data;
> diff --git a/src/shared/hfp.h b/src/shared/hfp.h
> index a9a169b..85037b1 100644
> --- a/src/shared/hfp.h
> +++ b/src/shared/hfp.h
> @@ -68,10 +68,14 @@ enum hfp_gw_cmd_type {
> };
>
> struct hfp_gw_result;
> +struct hfp_hf_result;
As before, I'd keep hf stuff in single section.
>
> typedef void (*hfp_result_func_t)(struct hfp_gw_result *result,
> enum hfp_gw_cmd_type type, void *user_data);
>
> +typedef void (*hfp_hf_result_func_t)(struct hfp_hf_result *result,
> + void *user_data);
> +
Same here.
> typedef void (*hfp_destroy_func_t)(void *user_data);
> typedef void (*hfp_debug_func_t)(const char *str, void *user_data);
>
> @@ -138,3 +142,7 @@ bool hfp_hf_set_disconnect_handler(struct hfp_hf *hfp,
> void *user_data,
> hfp_destroy_func_t destroy);
> bool hfp_hf_disconnect(struct hfp_hf *hfp);
> +bool hfp_hf_register(struct hfp_hf *hfp, hfp_hf_result_func_t callback,
> + const char *prefix, void *user_data,
> + hfp_destroy_func_t destroy);
> +bool hfp_hf_unregister(struct hfp_hf *hfp, const char *prefix);
>
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [PATCH v4 05/11] shared/hfp: Add HFP HF parser
From: Szymon Janc @ 2014-10-22 11:00 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
In-Reply-To: <1412898611-12199-6-git-send-email-lukasz.rymanowski@tieto.com>
Hi Łukasz,
On Friday 10 of October 2014 01:50:05 Lukasz Rymanowski wrote:
> This patch adds parser for AT responses and unsolicited results.
> ---
> src/shared/hfp.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 129 insertions(+)
>
> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
> index b1cf08e..5179092 100644
> --- a/src/shared/hfp.c
> +++ b/src/shared/hfp.c
> @@ -868,6 +868,126 @@ static void destroy_event_handler(void *data)
> free(handler);
> }
>
> +static void hf_skip_whitespace(struct hfp_hf_result *result)
> +{
> + while (result->data[result->offset] == ' ')
> + result->offset++;
> +}
> +
> +static void hf_call_prefix_handler(struct hfp_hf *hfp, const char *data)
> +{
> + struct event_handler *handler;
> + const char *separators = ";:\0";
> + struct hfp_hf_result result_data;
> + char lookup_prefix[18];
> + uint8_t pref_len = 0;
> + const char *prefix;
> + int i;
> +
> + result_data.offset = 0;
> + result_data.data = data;
> +
> + hf_skip_whitespace(&result_data);
> +
> + if (strlen(data + result_data.offset) < 2)
> + return;
> +
> + prefix = data + result_data.offset;
> +
> + pref_len = strcspn(prefix, separators);
> + if (pref_len > 17 || pref_len < 2)
> + return;
> +
> + for (i = 0; i < pref_len; i++)
> + lookup_prefix[i] = toupper(prefix[i]);
> +
> + lookup_prefix[pref_len] = '\0';
> + result_data.offset += pref_len + 1;
> +
> + handler = queue_find(hfp->event_handlers, match_handler_event_prefix,
> + lookup_prefix);
> + if (!handler)
> + return;
> +
> + handler->callback(&result_data, handler->user_data);
> +}
> +
> +static char *find_cr_lf(char *str, size_t len)
> +{
> + char *ptr;
> + int count;
> + int offset;
> +
> + offset = 0;
> +
> + ptr = memchr(str, '\r', len);
> + while (ptr) {
> + /*
> + * Check if there is more data after '\r'. If so check for
> + * '\n'
> + */
> + count = ptr-str;
Style: spaces around -
> + if ((count < (int) (len - 1)) && *(ptr + 1) == '\n')
> + return ptr;
If you make count size_t then this cast is not needed.
> +
> + /* There is only '\r'? Let's try to find next one */
> + offset += count + 1;
> +
> + if (offset >= (int)len)
If you make offset size_t then this cast is not needed.
Also such casting should have space '(int) foo'.
> + return NULL;
> +
> + ptr = memchr(str + offset, '\r', len - offset);
> + }
> +
> + return NULL;
> +}
> +
> +static void hf_process_input(struct hfp_hf *hfp)
> +{
> + char *str, *ptr;
> + size_t len, count, offset;
> +
> + str = ringbuf_peek(hfp->read_buf, 0, &len);
> + if (!str)
> + return;
> +
> + offset = 0;
> +
> + ptr = find_cr_lf(str, len);
> + while (ptr) {
> + count = ptr - (str + offset);
If you would adjust str pointer instead of using str + offset everywhere
then this code would be a bit simpler to follow.
Also this would not handle wrapped string correctly. Check how this is handled
in process_input().
> + if (count == 0) {
> + /* 2 is for <cr><lf> */
> + offset += 2;
> + } else {
> + *ptr = '\0';
> + hf_call_prefix_handler(hfp, str + offset);
> + offset += count + 2;
> + }
> +
> + if (offset >= len)
> + break;
> +
> + ptr = find_cr_lf(str + offset, len - offset);
> + }
> +
> + ringbuf_drain(hfp->read_buf, offset < len ? offset : len);
> +}
> +
> +static bool hf_can_read_data(struct io *io, void *user_data)
> +{
> + struct hfp_hf *hfp = user_data;
> + ssize_t bytes_read;
> +
> + bytes_read = ringbuf_read(hfp->read_buf, hfp->fd);
> + if (bytes_read < 0)
> + return false;
> +
> + hf_process_input(hfp);
> +
> + return true;
> +}
> +
> struct hfp_hf *hfp_hf_new(int fd)
> {
> struct hfp_hf *hfp;
> @@ -912,6 +1032,15 @@ struct hfp_hf *hfp_hf_new(int fd)
> return NULL;
> }
>
> + if (!io_set_read_handler(hfp->io, hf_can_read_data, hfp,
> + read_watch_destroy)) {
> + queue_destroy(hfp->event_handlers,
> + destroy_event_handler);
> + io_destroy(hfp->io);
> + ringbuf_free(hfp->write_buf);
> + ringbuf_free(hfp->read_buf);
You are missing free(hfp); return NULL; here.
> + }
> +
> return hfp_hf_ref(hfp);
> }
>
>
--
Best regards,
Szymon Janc
^ 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