* [PATCH 5/6] avdtp: Move connection_lost function to avoid forward declaration
From: Szymon Janc @ 2013-10-10 8:53 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1381395221-5067-1-git-send-email-szymon.janc@tieto.com>
---
profiles/audio/avdtp.c | 53 +++++++++++++++++++++++++-------------------------
1 file changed, 26 insertions(+), 27 deletions(-)
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index bf15935..e5d4139 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -439,7 +439,6 @@ static gboolean avdtp_parse_rej(struct avdtp *session,
uint8_t transaction, uint8_t signal_id,
void *buf, int size);
static int process_queue(struct avdtp *session);
-static void connection_lost(struct avdtp *session, int err);
static void avdtp_sep_set_state(struct avdtp *session,
struct avdtp_local_sep *sep,
avdtp_state_t state);
@@ -1137,6 +1136,32 @@ static void avdtp_free(void *data)
g_free(session);
}
+static void connection_lost(struct avdtp *session, int err)
+{
+ struct avdtp_server *server = session->server;
+ char address[18];
+
+ ba2str(device_get_address(session->device), address);
+ DBG("Disconnected from %s", address);
+
+ if (err != EACCES)
+ avdtp_cancel_authorization(session);
+
+ g_slist_foreach(session->streams, (GFunc) release_stream, session);
+ session->streams = NULL;
+
+ finalize_discovery(session, err);
+
+ avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
+
+ if (session->ref > 0)
+ return;
+
+ server->sessions = g_slist_remove(server->sessions, session);
+ btd_device_unref(session->device);
+ avdtp_free(session);
+}
+
static gboolean disconnect_timeout(gpointer user_data)
{
struct avdtp *session = user_data;
@@ -1180,32 +1205,6 @@ static void set_disconnect_timer(struct avdtp *session)
session);
}
-static void connection_lost(struct avdtp *session, int err)
-{
- struct avdtp_server *server = session->server;
- char address[18];
-
- ba2str(device_get_address(session->device), address);
- DBG("Disconnected from %s", address);
-
- if (err != EACCES)
- avdtp_cancel_authorization(session);
-
- g_slist_foreach(session->streams, (GFunc) release_stream, session);
- session->streams = NULL;
-
- finalize_discovery(session, err);
-
- avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
-
- if (session->ref > 0)
- return;
-
- server->sessions = g_slist_remove(server->sessions, session);
- btd_device_unref(session->device);
- avdtp_free(session);
-}
-
void avdtp_unref(struct avdtp *session)
{
if (!session)
--
1.8.4
^ permalink raw reply related
* [PATCH 4/6] avdtp: Rename avdtp_callbacks to state_callbacks
From: Szymon Janc @ 2013-10-10 8:53 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1381395221-5067-1-git-send-email-szymon.janc@tieto.com>
This better describes what list contains.
---
profiles/audio/avdtp.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index e83245e..bf15935 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -425,7 +425,7 @@ struct avdtp {
static GSList *servers = NULL;
-static GSList *avdtp_callbacks = NULL;
+static GSList *state_callbacks = NULL;
static int send_request(struct avdtp *session, gboolean priority,
struct avdtp_stream *stream, uint8_t signal_id,
@@ -710,7 +710,7 @@ static void avdtp_set_state(struct avdtp *session,
session->state = new_state;
- for (l = avdtp_callbacks; l != NULL; l = l->next) {
+ for (l = state_callbacks; l != NULL; l = l->next) {
struct avdtp_state_callback *cb = l->data;
if (session->device != cb->dev)
@@ -3874,7 +3874,8 @@ unsigned int avdtp_add_state_cb(struct btd_device *dev,
state_cb->id = ++id;
state_cb->user_data = user_data;
- avdtp_callbacks = g_slist_append(avdtp_callbacks, state_cb);
+ state_callbacks = g_slist_append(state_callbacks,
+ state_cb);
return state_cb->id;
}
@@ -3883,10 +3884,10 @@ gboolean avdtp_remove_state_cb(unsigned int id)
{
GSList *l;
- for (l = avdtp_callbacks; l != NULL; l = l->next) {
+ for (l = state_callbacks; l != NULL; l = l->next) {
struct avdtp_state_callback *cb = l->data;
if (cb && cb->id == id) {
- avdtp_callbacks = g_slist_remove(avdtp_callbacks, cb);
+ state_callbacks = g_slist_remove(state_callbacks, cb);
g_free(cb);
return TRUE;
}
--
1.8.4
^ permalink raw reply related
* [PATCH 3/6] avdtp: Remove unused avdtp_stream_setup_active function
From: Szymon Janc @ 2013-10-10 8:53 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1381395221-5067-1-git-send-email-szymon.janc@tieto.com>
It is not used anywhere and can be removed.
---
profiles/audio/avdtp.c | 5 -----
profiles/audio/avdtp.h | 1 -
2 files changed, 6 deletions(-)
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 1f29050..e83245e 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -3857,11 +3857,6 @@ gboolean avdtp_has_stream(struct avdtp *session, struct avdtp_stream *stream)
return g_slist_find(session->streams, stream) ? TRUE : FALSE;
}
-gboolean avdtp_stream_setup_active(struct avdtp *session)
-{
- return session->stream_setup;
-}
-
void avdtp_set_device_disconnect(struct avdtp *session, gboolean dev_dc)
{
session->device_disconnect = dev_dc;
diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
index e19dfc3..3bf7503 100644
--- a/profiles/audio/avdtp.h
+++ b/profiles/audio/avdtp.h
@@ -307,7 +307,6 @@ int avdtp_error_posix_errno(struct avdtp_error *err);
struct btd_adapter *avdtp_get_adapter(struct avdtp *session);
struct btd_device *avdtp_get_device(struct avdtp *session);
-gboolean avdtp_stream_setup_active(struct avdtp *session);
void avdtp_set_device_disconnect(struct avdtp *session, gboolean dev_dc);
int avdtp_init(struct btd_adapter *adapter);
--
1.8.4
^ permalink raw reply related
* [PATCH 2/6] avdtp: Remove unused avdtp_is_connected function
From: Szymon Janc @ 2013-10-10 8:53 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1381395221-5067-1-git-send-email-szymon.janc@tieto.com>
It is not used anywhere and can be removed.
---
profiles/audio/avdtp.c | 19 -------------------
profiles/audio/avdtp.h | 2 --
2 files changed, 21 deletions(-)
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index fc896c7..1f29050 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -3164,25 +3164,6 @@ static gboolean avdtp_parse_rej(struct avdtp *session,
}
}
-gboolean avdtp_is_connected(struct btd_device *device)
-{
- struct avdtp_server *server;
- struct avdtp *session;
-
- server = find_server(servers, device_get_adapter(device));
- if (!server)
- return FALSE;
-
- session = find_session(server->sessions, device);
- if (!session)
- return FALSE;
-
- if (session->state != AVDTP_SESSION_STATE_DISCONNECTED)
- return TRUE;
-
- return FALSE;
-}
-
struct avdtp_service_capability *avdtp_stream_get_codec(
struct avdtp_stream *stream)
{
diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
index b8aaf1d..e19dfc3 100644
--- a/profiles/audio/avdtp.h
+++ b/profiles/audio/avdtp.h
@@ -218,8 +218,6 @@ struct avdtp *avdtp_get(struct btd_device *device);
void avdtp_unref(struct avdtp *session);
struct avdtp *avdtp_ref(struct avdtp *session);
-gboolean avdtp_is_connected(struct btd_device *device);
-
struct avdtp_service_capability *avdtp_service_cap_new(uint8_t category,
void *data, int size);
--
1.8.4
^ permalink raw reply related
* [PATCH 1/6] avdtp: Fix typos in errors from avdtp_strerror
From: Szymon Janc @ 2013-10-10 8:53 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
---
profiles/audio/avdtp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index fe63a40..fc896c7 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -3807,7 +3807,7 @@ const char *avdtp_strerror(struct avdtp_error *err)
case AVDTP_BAD_ROHC_FORMAT:
return "Bad Header Compression Format";
case AVDTP_BAD_CP_FORMAT:
- return "Bad Content Protetion Format";
+ return "Bad Content Protection Format";
case AVDTP_BAD_MULTIPLEXING_FORMAT:
return "Bad Multiplexing Format";
case AVDTP_UNSUPPORTED_CONFIGURATION:
@@ -3815,7 +3815,7 @@ const char *avdtp_strerror(struct avdtp_error *err)
case AVDTP_BAD_STATE:
return "Bad State";
default:
- return "Unknow error";
+ return "Unknown error";
}
}
--
1.8.4
^ permalink raw reply related
* Re: [PATCH v4 2/2] Bluetooth: Refactor hci_connect_le
From: Marcel Holtmann @ 2013-10-10 8:31 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1381231278-20534-2-git-send-email-andre.guedes@openbossa.org>
Hi Andrei,
> This patch does some code refactoring in hci_connect_le() by moving
> the exception code into if statements and letting the main flow in
> first level of function scope. It also adds extra comments to improve
> the code readability.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 51 +++++++++++++++++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 18 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v4 1/2] Bluetooth: Use HCI request for LE connection
From: Marcel Holtmann @ 2013-10-10 8:31 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1381231278-20534-1-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch introduces a new helper, which uses the HCI request
> framework, for creating LE connectons. All the handling is now
> done by this function so we can remove the hci_cs_le_create_conn()
> event handler.
>
> This patch also removes the old hci_le_create_connection() since
> it is not used anymore.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 91 ++++++++++++++++++++++++++++++++++-------------
> net/bluetooth/hci_event.c | 31 ----------------
> 2 files changed, 67 insertions(+), 55 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Andrei Emeltchenko @ 2013-10-10 8:16 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <982246CD-1B58-457A-85C3-4EA584226E3C@holtmann.org>
Hi Marcel,
On Thu, Oct 10, 2013 at 10:10:37AM +0200, Marcel Holtmann wrote:
> Hi Andrei,
>
> >>>>> These types of warnings are disabled in BlueZ makeifiles
> >>>>>
> >>>>> main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
> >>>>> main.c:67:2: warning: (near initialization for 'options[1].short_name')
> >>>>> [-Wmissing-field-initializers]
> >>>>> ---
> >>>>> android/Android.mk | 3 +++
> >>>>> 1 file changed, 3 insertions(+)
> >>>>>
> >>>>> diff --git a/android/Android.mk b/android/Android.mk
> >>>>> index ec820ac..5498b41 100644
> >>>>> --- a/android/Android.mk
> >>>>> +++ b/android/Android.mk
> >>>>> @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
> >>>>>
> >>>>> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> >>>>>
> >>>>> +# to suppress the "warning: missing initializer near initialization.." warning
> >>>>> +LOCAL_CFLAGS += -Wno-missing-field-initializers
> >>>>> +
> >>>>
> >>>> why are we doing this and not fixing the actual warning?
> >>>>
> >>>
> >>> Do you want me to initialize structures like
> >>>
> >>> {NULL, NULL, NULL, ... }
> >>>
> >>> this way it can be fixed.
> >>
> >> that is ugly and gets really complicated in the code over time. So I prefer we do not do that.
> >>
> >> Which structures does this affect. All of them?
> >
> > You can just try following patch and see:
> >
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index 5bfa29d..4d6a42c 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -15,7 +15,6 @@ AC_DEFUN([COMPILER_FLAGS], [
> > if (test "$USE_MAINTAINER_MODE" = "yes"); then
> > with_cflags="$with_cflags -Wall -Werror -Wextra"
> > with_cflags="$with_cflags -Wno-unused-parameter"
> > - with_cflags="$with_cflags -Wno-missing-field-initializers"
> > with_cflags="$with_cflags -Wdeclaration-after-statement"
> > with_cflags="$with_cflags -Wmissing-declarations"
> > with_cflags="$with_cflags -Wredundant-decls"
>
> can we then have a global variable in Android.mk that lists all of our
> warning exceptions. Re-assigning them over and over again manually seems
> a rather bad idea.
We can create ANDROID_CFLAGS and include it to other targets. Is this
good?
Best regards
Andrei Emeltchenko
^ permalink raw reply
* [PATCH] emulator: Fix AMP emulation build error
From: Szymon Janc @ 2013-10-10 8:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
This fix build with GCC 4.6.3.
CC emulator/amp.o
emulator/amp.c: In function ‘send_packet’:
emulator/amp.c:172:7: error: ignoring return value of ‘write’,
declared with attribute warn_unused_result [-Werror=unused-result]
cc1: all warnings being treated as errors
make[1]: *** [emulator/amp.o] Error
---
emulator/amp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/emulator/amp.c b/emulator/amp.c
index e80d9a8..714854b 100644
--- a/emulator/amp.c
+++ b/emulator/amp.c
@@ -169,7 +169,8 @@ static void reset_defaults(struct bt_amp *amp)
static void send_packet(struct bt_amp *amp, const void *data, uint16_t len)
{
- write(amp->vhci_fd, data, len);
+ if (write(amp->vhci_fd, data, len) < 0)
+ fprintf(stderr, "Write to /dev/vhci failed\n");
}
static void send_event(struct bt_amp *amp, uint8_t event,
--
1.8.4
^ permalink raw reply related
* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Marcel Holtmann @ 2013-10-10 8:10 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <20131010080657.GF23879@aemeltch-MOBL1>
Hi Andrei,
>>>>> These types of warnings are disabled in BlueZ makeifiles
>>>>>
>>>>> main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
>>>>> main.c:67:2: warning: (near initialization for 'options[1].short_name')
>>>>> [-Wmissing-field-initializers]
>>>>> ---
>>>>> android/Android.mk | 3 +++
>>>>> 1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/android/Android.mk b/android/Android.mk
>>>>> index ec820ac..5498b41 100644
>>>>> --- a/android/Android.mk
>>>>> +++ b/android/Android.mk
>>>>> @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
>>>>>
>>>>> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
>>>>>
>>>>> +# to suppress the "warning: missing initializer near initialization.." warning
>>>>> +LOCAL_CFLAGS += -Wno-missing-field-initializers
>>>>> +
>>>>
>>>> why are we doing this and not fixing the actual warning?
>>>>
>>>
>>> Do you want me to initialize structures like
>>>
>>> {NULL, NULL, NULL, ... }
>>>
>>> this way it can be fixed.
>>
>> that is ugly and gets really complicated in the code over time. So I prefer we do not do that.
>>
>> Which structures does this affect. All of them?
>
> You can just try following patch and see:
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index 5bfa29d..4d6a42c 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -15,7 +15,6 @@ AC_DEFUN([COMPILER_FLAGS], [
> if (test "$USE_MAINTAINER_MODE" = "yes"); then
> with_cflags="$with_cflags -Wall -Werror -Wextra"
> with_cflags="$with_cflags -Wno-unused-parameter"
> - with_cflags="$with_cflags -Wno-missing-field-initializers"
> with_cflags="$with_cflags -Wdeclaration-after-statement"
> with_cflags="$with_cflags -Wmissing-declarations"
> with_cflags="$with_cflags -Wredundant-decls"
can we then have a global variable in Android.mk that lists all of our warning exceptions. Re-assigning them over and over again manually seems a rather bad idea.
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Andrei Emeltchenko @ 2013-10-10 8:07 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <7EFBFA1F-E02A-454D-B93E-1252D4288C87@holtmann.org>
Hi Marcel,
On Thu, Oct 10, 2013 at 09:35:40AM +0200, Marcel Holtmann wrote:
> Hi Andrei,
>
> >>> These types of warnings are disabled in BlueZ makeifiles
> >>>
> >>> main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
> >>> main.c:67:2: warning: (near initialization for 'options[1].short_name')
> >>> [-Wmissing-field-initializers]
> >>> ---
> >>> android/Android.mk | 3 +++
> >>> 1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/android/Android.mk b/android/Android.mk
> >>> index ec820ac..5498b41 100644
> >>> --- a/android/Android.mk
> >>> +++ b/android/Android.mk
> >>> @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
> >>>
> >>> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> >>>
> >>> +# to suppress the "warning: missing initializer near initialization.." warning
> >>> +LOCAL_CFLAGS += -Wno-missing-field-initializers
> >>> +
> >>
> >> why are we doing this and not fixing the actual warning?
> >>
> >
> > Do you want me to initialize structures like
> >
> > {NULL, NULL, NULL, ... }
> >
> > this way it can be fixed.
>
> that is ugly and gets really complicated in the code over time. So I prefer we do not do that.
>
> Which structures does this affect. All of them?
You can just try following patch and see:
diff --git a/acinclude.m4 b/acinclude.m4
index 5bfa29d..4d6a42c 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -15,7 +15,6 @@ AC_DEFUN([COMPILER_FLAGS], [
if (test "$USE_MAINTAINER_MODE" = "yes"); then
with_cflags="$with_cflags -Wall -Werror -Wextra"
with_cflags="$with_cflags -Wno-unused-parameter"
- with_cflags="$with_cflags -Wno-missing-field-initializers"
with_cflags="$with_cflags -Wdeclaration-after-statement"
with_cflags="$with_cflags -Wmissing-declarations"
with_cflags="$with_cflags -Wredundant-decls"
Best regards
Andrei Emeltchenko
^ permalink raw reply related
* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Marcel Holtmann @ 2013-10-10 7:35 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <20131010065825.GC23879@aemeltch-MOBL1>
Hi Andrei,
>>> These types of warnings are disabled in BlueZ makeifiles
>>>
>>> main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
>>> main.c:67:2: warning: (near initialization for 'options[1].short_name')
>>> [-Wmissing-field-initializers]
>>> ---
>>> android/Android.mk | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/android/Android.mk b/android/Android.mk
>>> index ec820ac..5498b41 100644
>>> --- a/android/Android.mk
>>> +++ b/android/Android.mk
>>> @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
>>>
>>> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
>>>
>>> +# to suppress the "warning: missing initializer near initialization.." warning
>>> +LOCAL_CFLAGS += -Wno-missing-field-initializers
>>> +
>>
>> why are we doing this and not fixing the actual warning?
>>
>
> Do you want me to initialize structures like
>
> {NULL, NULL, NULL, ... }
>
> this way it can be fixed.
that is ugly and gets really complicated in the code over time. So I prefer we do not do that.
Which structures does this affect. All of them?
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 03/15] android: Add Socket Bluetooth HAL template
From: Marcel Holtmann @ 2013-10-10 7:33 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <20131010065631.GB23879@aemeltch-MOBL1>
Hi Andrei,
>>> +static bt_status_t listen(btsock_type_t type, const char *service_name,
>>> + const uint8_t *uuid, int chan,
>>> + int *sock, int flags)
>>> +{
>>> + if ((uuid == NULL && chan <= 0) || sock == NULL) {
>>
>> we are moving away from uuid == NULL checks. Use !uuid instead.
>
> I do not like myself these kind of checks but since this seems to be BlueZ
> style. Should I change comparison val != NULL and val != 0 to !val ? Like
> in kernel?
yes, we are slowly moving towards val and !val checks. Especially for new code that coding style has to be enforced now.
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Andrei Emeltchenko @ 2013-10-10 6:58 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <E4E245F7-8EE6-4FE2-AC99-3246F1541C99@holtmann.org>
Hi Marcel,
On Wed, Oct 09, 2013 at 09:11:04PM +0200, Marcel Holtmann wrote:
> Hi Andrei,
>
> > These types of warnings are disabled in BlueZ makeifiles
> >
> > main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
> > main.c:67:2: warning: (near initialization for 'options[1].short_name')
> > [-Wmissing-field-initializers]
> > ---
> > android/Android.mk | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/android/Android.mk b/android/Android.mk
> > index ec820ac..5498b41 100644
> > --- a/android/Android.mk
> > +++ b/android/Android.mk
> > @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
> >
> > LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> >
> > +# to suppress the "warning: missing initializer near initialization.." warning
> > +LOCAL_CFLAGS += -Wno-missing-field-initializers
> > +
>
> why are we doing this and not fixing the actual warning?
>
Do you want me to initialize structures like
{NULL, NULL, NULL, ... }
this way it can be fixed.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCHv3 03/15] android: Add Socket Bluetooth HAL template
From: Andrei Emeltchenko @ 2013-10-10 6:56 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <CCFB8A73-18EC-4FE3-9C92-8FBD91B9F6B9@holtmann.org>
Hi Marcel,
On Wed, Oct 09, 2013 at 09:14:05PM +0200, Marcel Holtmann wrote:
> > +static bt_status_t listen(btsock_type_t type, const char *service_name,
> > + const uint8_t *uuid, int chan,
> > + int *sock, int flags)
> > +{
> > + if ((uuid == NULL && chan <= 0) || sock == NULL) {
>
> we are moving away from uuid == NULL checks. Use !uuid instead.
I do not like myself these kind of checks but since this seems to be BlueZ
style. Should I change comparison val != NULL and val != 0 to !val ? Like
in kernel?
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Lucas De Marchi @ 2013-10-09 22:55 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Andrei Emeltchenko, BlueZ development
In-Reply-To: <E4E245F7-8EE6-4FE2-AC99-3246F1541C99@holtmann.org>
On Wed, Oct 9, 2013 at 4:11 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andrei,
>
>> These types of warnings are disabled in BlueZ makeifiles
>>
>> main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
>> main.c:67:2: warning: (near initialization for 'options[1].short_name')
>> [-Wmissing-field-initializers]
>> ---
>> android/Android.mk | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/android/Android.mk b/android/Android.mk
>> index ec820ac..5498b41 100644
>> --- a/android/Android.mk
>> +++ b/android/Android.mk
>> @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
>>
>> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
>>
>> +# to suppress the "warning: missing initializer near initialization.." warning
>> +LOCAL_CFLAGS += -Wno-missing-field-initializers
>> +
>
> why are we doing this and not fixing the actual warning?
I guess because this is a stupid warning.
We do the same in autotools because otherwise we can't initialize
structs and rely on compiler setting the rest to 0. This is
particularly annoying on d-bus tables (see the exists and flags on the
end of the GDBusPropertyTable struct). I'm sure it's annoying in other
places, too.
Lucas De Marchi
^ permalink raw reply
* Re: [PATCHv3 14/15] android: Add makefile for hciconfig
From: Marcel Holtmann @ 2013-10-09 19:59 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1381243883-2745-15-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
>
> ---
> android/Android.mk | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/android/Android.mk b/android/Android.mk
> index e7a70d0..09c4579 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -123,3 +123,37 @@ $(foreach file,$(lib_headers), $(shell ln -sf ../$(file) $(LOCAL_PATH)/../lib/bl
> LOCAL_MODULE := libbluetooth
>
> include $(BUILD_SHARED_LIBRARY)
> +
> +#
> +# hciconfig
> +#
why are we building this one. Only btmgmt should be needed for testing. If at all.
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 13/15] android: Handle mgmt changed events
From: Marcel Holtmann @ 2013-10-09 19:58 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1381243883-2745-14-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Add code handling changing adapter settings.
> ---
> android/main.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 154 insertions(+)
>
> diff --git a/android/main.c b/android/main.c
> index a2ed8a4..c181d3a 100644
> --- a/android/main.c
> +++ b/android/main.c
> @@ -127,6 +127,159 @@ static void load_link_keys_complete(uint8_t status, uint16_t length,
> DBG("status %u", status);
> }
>
> +static void mgmt_local_name_changed_event(uint16_t index, uint16_t length,
> + const void *param, void *user_data)
> +{
> + struct bt_adapter *adapter = user_data;
> + const struct mgmt_cp_set_local_name *rp = param;
> +
> + if (length < sizeof(*rp)) {
> + error("Wrong size of local name changed parameters");
> + return;
> + }
> +
> + if (adapter != default_adapter) {
> + error("Wrong adapter %p", adapter);
> + return;
> + }
this check is rather useless if the events are all registered per adapter. With mgmt you can register events for a specific index.
> +
> + if (!g_strcmp0(adapter->short_name, (const char *) rp->short_name) &&
> + !g_strcmp0(adapter->name, (const char *) rp->name))
> + return;
Don't bother with the short name.
> +
> + DBG("name: %s short name: %s", rp->name, rp->short_name);
> +
> + g_free(adapter->name);
> + adapter->name = g_strdup((const char *) rp->name);
> +
> + g_free(adapter->short_name);
> + adapter->short_name = g_strdup((const char *) rp->short_name);
> +
> + /* TODO Update services if needed */
> +}
> +
> +static void settings_changed_connectable(struct bt_adapter *adapter)
> +{
> + /* TODO */
> +}
> +
> +static void settings_changed_discoverable(struct bt_adapter *adapter)
> +{
> + /* TODO */
> +}
> +
> +static void settings_changed(struct bt_adapter *adapter, uint32_t settings)
> +{
> + uint32_t changed_mask;
> +
> + changed_mask = adapter->current_settings ^ settings;
> +
> + adapter->current_settings = settings;
> +
> + DBG("0x%08x", changed_mask);
> +
> + if (changed_mask & MGMT_SETTING_POWERED) {
> + info("Powered");
> +
> + if (adapter->current_settings & MGMT_SETTING_POWERED)
> + adapter_start(adapter);
> + else
> + adapter_stop(adapter);
> + }
> +
> + /* Seems not needed for Android */
> + if (changed_mask & MGMT_SETTING_PAIRABLE)
> + DBG("Pairable");
We should always enable pairable when setting up the adapter.
> +
> + /*
> + * There are only 2 scan modes:
> + * CONNECTABLE and CONNECTABLE_DISCOVERABLE
> + */
I can not make heads or tails out of this comment.
> + if (changed_mask & MGMT_SETTING_CONNECTABLE) {
> + DBG("Connectable");
> +
> + settings_changed_connectable(adapter);
> + }
> +
> + if (changed_mask & MGMT_SETTING_DISCOVERABLE) {
> + DBG("Discoverable");
> +
> + settings_changed_discoverable(adapter);
> + }
> +}
> +
> +static void new_settings_callback(uint16_t index, uint16_t length,
> + const void *param, void *user_data)
> +{
> + struct bt_adapter *adapter = user_data;
> + uint32_t settings;
> +
> + if (length < sizeof(settings)) {
> + error("Wrong size of new settings parameters");
> + return;
> + }
> +
> + if (adapter != default_adapter) {
> + error("Wrong adapter %p", adapter);
> + return;
> + }
> +
> + settings = bt_get_le32(param);
> +
> + DBG("settings: 0x%8.8x -> 0x%8.8x", adapter->current_settings,
> + settings);
> +
> + if (settings == adapter->current_settings)
> + return;
> +
> + settings_changed(adapter, settings);
> +}
> +
> +static void mgmt_dev_class_changed_event(uint16_t index, uint16_t length,
> + const void *param, void *user_data)
> +{
> + struct bt_adapter *adapter = user_data;
> + const struct mgmt_cod *rp = param;
> + uint32_t dev_class;
> +
> + if (length < sizeof(*rp)) {
> + error("Wrong size of class of device changed parameters");
> + return;
> + }
> +
> + if (adapter != default_adapter) {
> + error("Wrong adapter %p", adapter);
> + return;
> + }
> +
> + dev_class = rp->val[0] | (rp->val[1] << 8) | (rp->val[2] << 16);
> +
> + if (dev_class == adapter->dev_class)
> + return;
> +
> + DBG("Class: 0x%06x", dev_class);
> +
> + adapter->dev_class = dev_class;
> +
> + /* TODO: Inform prop change: Class */
> +
> + /* TODO: Gatt attrib set*/
> +}
> +
> +static void register_mgmt_handlers(struct bt_adapter *adapter)
> +{
> + mgmt_register(adapter->mgmt, MGMT_EV_NEW_SETTINGS, adapter->dev_id,
> + new_settings_callback, adapter, NULL);
> +
> + mgmt_register(adapter->mgmt, MGMT_EV_CLASS_OF_DEV_CHANGED,
> + adapter->dev_id, mgmt_dev_class_changed_event,
> + adapter, NULL);
> +
> + mgmt_register(adapter->mgmt, MGMT_EV_LOCAL_NAME_CHANGED,
> + adapter->dev_id, mgmt_local_name_changed_event,
> + adapter, NULL);
> +}
> +
> static void load_link_keys(struct bt_adapter *adapter, GSList *keys)
> {
> struct mgmt_cp_load_link_keys *cp;
> @@ -187,6 +340,7 @@ static void read_info_complete(uint8_t status, uint16_t length,
> adapter->current_settings = btohs(rp->current_settings);
>
> /* TODO: Register all event notification handlers */
> + register_mgmt_handlers(default_adapter);
>
> if (adapter->current_settings & MGMT_SETTING_POWERED)
> adapter_start(adapter);
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 12/15] android: Implement read_info_complete callback
From: Marcel Holtmann @ 2013-10-09 19:54 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1381243883-2745-13-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> Handle read info complete callback from mgmt interface.
> ---
> android/main.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 89 insertions(+), 12 deletions(-)
>
> diff --git a/android/main.c b/android/main.c
> index a100013..a2ed8a4 100644
> --- a/android/main.c
> +++ b/android/main.c
> @@ -54,6 +54,7 @@
> #include "log.h"
> #include "hcid.h"
> #include "sdpd.h"
> +#include "bt_adapter.h"
>
> #include "lib/bluetooth.h"
> #include "lib/mgmt.h"
> @@ -69,6 +70,8 @@ static struct mgmt *mgmt_if = NULL;
> static uint8_t mgmt_version = 0;
> static uint8_t mgmt_revision = 0;
>
> +struct bt_adapter *default_adapter = NULL;
> +
> static int sdp_start(void)
> {
> DBG("");
> @@ -118,34 +121,108 @@ static GOptionEntry options[] = {
> { NULL }
> };
>
> -static void read_info_complete(uint8_t status, uint16_t length,
> +static void load_link_keys_complete(uint8_t status, uint16_t length,
> const void *param, void *user_data)
> {
> - /* TODO: Store Controller information */
> -
> - /**
> - * Register all event notification handlers for controller.
> - *
> - * The handlers are registered after a succcesful read of the
> - * controller info. From now on they can track updates and
> - * notifications.
> - */
> + DBG("status %u", status);
> }
>
> +static void load_link_keys(struct bt_adapter *adapter, GSList *keys)
> +{
> + struct mgmt_cp_load_link_keys *cp;
> + size_t key_len = g_slist_length(keys);
> + struct mgmt_link_key_info *key;
> + size_t len;
> +
> + DBG("");
> +
> + len = sizeof(*cp) + key_len * sizeof(*key);
> + cp = g_try_malloc0(len);
> + if (cp == NULL) {
> + error("%s: Not enough memory for link keys loading", __func__);
> + return;
> + }
I am really fine with just using g_malloc0 and aborting the program in case of memory allocation errors.
> +
> + cp->debug_keys = 0;
> + cp->key_count = htobs(key_len);
> +
> + mgmt_send(adapter->mgmt, MGMT_OP_LOAD_LINK_KEYS, adapter->dev_id, len,
> + cp, load_link_keys_complete, adapter, NULL);
> +
> + free(cp);
> +}
> +
> +static void read_info_complete(uint8_t status, uint16_t length,
> + const void *param, void *user_data)
> +{
> + struct bt_adapter *adapter = user_data;
> + const struct mgmt_rp_read_info *rp = param;
> +
> + DBG("");
> +
> + if (status != MGMT_STATUS_SUCCESS) {
> + error("Failed to read info for index %u: %s (0x%02x)",
> + adapter->dev_id, mgmt_errstr(status), status);
> + goto failed;
> + }
> +
> + if (length < sizeof(*rp)) {
> + error("Too small read info complete response");
> + goto failed;
> + }
> +
> + if (bacmp(&rp->bdaddr, BDADDR_ANY) == 0) {
> + error("No Bluetooth address for index %u", adapter->dev_id);
> + goto failed;
> + }
> +
> + /* Store adapter information */
> + bacpy(&adapter->bdaddr, &rp->bdaddr);
> + adapter->dev_class = rp->dev_class[0] | (rp->dev_class[1] << 8) |
> + (rp->dev_class[2] << 16);
> + adapter->name = g_strdup((const char *) rp->name);
> + adapter->short_name = g_strdup((const char *) rp->short_name);
Don't bother with the short name.
> +
> + adapter->supported_settings = btohs(rp->supported_settings);
> + adapter->current_settings = btohs(rp->current_settings);
> +
> + /* TODO: Register all event notification handlers */
> +
> + if (adapter->current_settings & MGMT_SETTING_POWERED)
> + adapter_start(adapter);
> +
> + /* dummy link keys loading */
The comment is wrong here. Just load the link keys. Even an empty list of keys needs to be loaded.
> + load_link_keys(adapter, NULL);
> +
> + return;
> +
> +failed:
> + default_adapter = NULL;
> +}
>
> static void mgmt_index_added_event(uint16_t index, uint16_t length,
> const void *param, void *user_data)
> {
> info("%s: index %u", __func__, index);
>
> + if (default_adapter != NULL) {
if (default_adapter)
> + DBG("skip event for index %d", index);
If you copy code from src/adapter.c that uses %u for the index, then be consistent.
> + return;
> + }
> +
> + default_adapter = bt_adapter_new(index, mgmt_if);
> + if (default_adapter == NULL) {
And here again, just assume memory allocation succeeds.
> + error("Unable to create new adapter for index %u", index);
> + return;
> + }
> +
> DBG("sending read info command for index %u", index);
>
> if (mgmt_send(mgmt_if, MGMT_OP_READ_INFO, index, 0, NULL,
> - read_info_complete, NULL, NULL) > 0)
> + read_info_complete, default_adapter, NULL) > 0)
> return;
>
> error("Failed to read adapter info for index %u", index);
> -
> }
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 11/15] android: Add cap to bind to port < 1024
From: Marcel Holtmann @ 2013-10-09 19:48 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1381243883-2745-12-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> For SDP server we need to bind to lower port, acquire this capability.
> ---
> android/main.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> configure.ac | 4 ++++
> 2 files changed, 75 insertions(+)
>
> diff --git a/android/main.c b/android/main.c
> index 7968ed0..a100013 100644
> --- a/android/main.c
> +++ b/android/main.c
> @@ -32,6 +32,22 @@
> #include <stdlib.h>
> #include <stdbool.h>
> #include <string.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sys/prctl.h>
> +#include <linux/capability.h>
> +
> +/**
> + * Include <sys/capability.h> for host build and
> + * also for Android 4.3 when it is added to bionic
> + */
why focus on anything before Android 4.3?
> +#if !defined(ANDROID) || (PLATFORM_SDK_VERSION > 17)
> +#include <sys/capability.h>
> +#endif
> +
> +#if defined(ANDROID)
> +#include <private/android_filesystem_config.h>
> +#endif
>
> #include <glib.h>
>
> @@ -279,6 +295,58 @@ static void cleanup_mgmt_interface(void)
> mgmt_if = NULL;
> }
>
> +static bool android_set_aid_and_cap(void)
> +{
> + struct __user_cap_header_struct header;
> + struct __user_cap_data_struct cap;
> +#if defined(ANDROID)
> + gid_t groups[] = {AID_NET_BT, AID_NET_BT_ADMIN, AID_NET_ADMIN};
> +#endif
> +
> + DBG("pid %d uid %d gid %d", getpid(), getuid(), getgid());
> +
> + header.version = _LINUX_CAPABILITY_VERSION;
> +
> + prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0);
> +
> +#if defined(ANDROID)
> + if (setgid(AID_BLUETOOTH) < 0)
> + warn("%s: setgid(): %s", __func__, strerror(errno));
> +
> + if (setuid(AID_BLUETOOTH) < 0)
> + warn("%s: setuid(): %s", __func__, strerror(errno));
> +#endif
> +
> + header.version = _LINUX_CAPABILITY_VERSION;
> + header.pid = 0;
> +
> + cap.effective = cap.permitted =
> + CAP_TO_MASK(CAP_SETGID) |
> + CAP_TO_MASK(CAP_NET_RAW) |
> + CAP_TO_MASK(CAP_NET_ADMIN) |
> + CAP_TO_MASK(CAP_NET_BIND_SERVICE);
> + cap.inheritable = 0;
> +
> + if (capset(&header, &cap) < 0) {
> + error("%s: capset(): %s", __func__, strerror(errno));
> + return false;
> + }
> +
> +#if defined(ANDROID)
> + if (setgroups(sizeof(groups)/sizeof(groups[0]), groups) < 0)
> + warn("%s: setgroups: %s", __func__, strerror(errno));
> +#endif
> + if (capget(&header, &cap) < 0)
> + error("%s: capget(): %s", __func__, strerror(errno));
> + else
> + DBG("Caps: eff: 0x%x, perm: 0x%x, inh: 0x%x", cap.effective,
> + cap.permitted, cap.inheritable);
> +
> + DBG("pid %d uid %d gid %d", getpid(), getuid(), getgid());
> +
> + return true;
> +}
> +
> int main(int argc, char *argv[])
> {
> GOptionContext *context;
> @@ -312,6 +380,9 @@ int main(int argc, char *argv[])
> sigaction(SIGINT, &sa, NULL);
> sigaction(SIGTERM, &sa, NULL);
>
> + if (android_set_aid_and_cap() == false)
Please check with if (!android…())
> + exit(EXIT_FAILURE);
> +
I prefer return EXIT_FAILURE;
> init_mgmt_interface();
> sdp_start();
>
> diff --git a/configure.ac b/configure.ac
> index 7b1f64a..5406434 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -247,4 +247,8 @@ AC_ARG_ENABLE(android, AC_HELP_STRING([--enable-android],
> [enable_android=${enableval}])
> AM_CONDITIONAL(ANDROID, test "${enable_android}" = "yes")
>
> +if (test "${android_daemon}" = "yes"); then
> + AC_CHECK_LIB(cap, capget, dummy=yes, AC_MSG_ERROR(libcap is required))
> +fi
> +
> AC_OUTPUT(Makefile src/bluetoothd.8 lib/bluez.pc)
REgards
Marcel
^ permalink raw reply
* Re: [PATCHv3 10/15] android: sdp: Reuse BlueZ SDP server in Android
From: Marcel Holtmann @ 2013-10-09 19:45 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1381243883-2745-11-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andei,
> Reuse existing SDP server code in Android GPL daemon.
> ---
> Makefile.android | 7 +++++--
> android/Android.mk | 7 +++++++
> android/bt_adapter.c | 2 --
> android/main.c | 28 ++++++++++++++++++++++++++++
> 4 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile.android b/Makefile.android
> index 9a2c486..d576b52 100644
> --- a/Makefile.android
> +++ b/Makefile.android
> @@ -2,9 +2,12 @@ if ANDROID
> noinst_PROGRAMS += android/bluetoothd
>
> android_bluetoothd_SOURCES = android/main.c src/log.c \
> + src/sdpd-database.c src/sdpd-server.c \
> + src/sdpd-service.c src/sdpd-request.c \
> src/shared/util.h src/shared/util.c \
> - src/shared/mgmt.h src/shared/mgmt.c
> -android_bluetoothd_LDADD = @GLIB_LIBS@
> + src/shared/mgmt.h src/shared/mgmt.c \
> + android/bt_adapter.h android/bt_adapter.c
> +android_bluetoothd_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
> endif
>
> EXTRA_DIST += android/Android.mk android/log.c
> diff --git a/android/Android.mk b/android/Android.mk
> index 8f95a66..e7a70d0 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -17,6 +17,11 @@ LOCAL_SRC_FILES := \
> log.c \
> ../src/shared/mgmt.c \
> ../src/shared/util.c \
> + bt_adapter.c \
> + ../src/sdpd-database.c \
> + ../src/sdpd-service.c \
> + ../src/sdpd-request.c \
> + ../src/sdpd-server.c \
>
> LOCAL_C_INCLUDES := \
> $(call include-path-for, glib) \
> @@ -25,6 +30,7 @@ LOCAL_C_INCLUDES := \
> LOCAL_C_INCLUDES += \
> $(LOCAL_PATH)/../ \
> $(LOCAL_PATH)/../src \
> + $(LOCAL_PATH)/../lib \
>
> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
>
> @@ -39,6 +45,7 @@ LOCAL_CFLAGS += -DSOCK_CLOEXEC=02000000 -DSOCK_NONBLOCK=04000
>
> LOCAL_SHARED_LIBRARIES := \
> libglib \
> + libbluetooth \
>
> LOCAL_MODULE := bluetoothd
> LOCAL_REQUIRED_MODULES := libbluetooth
> diff --git a/android/bt_adapter.c b/android/bt_adapter.c
> index e21d50c..6b758bf 100644
> --- a/android/bt_adapter.c
> +++ b/android/bt_adapter.c
> @@ -45,8 +45,6 @@ void adapter_start(struct bt_adapter *adapter)
>
> /* TODO: CB: report scan mode */
>
> - /* TODO: SDP start here */
> -
> /* TODO: CB: report state on */
> }
>
> diff --git a/android/main.c b/android/main.c
> index 3580ac7..7968ed0 100644
> --- a/android/main.c
> +++ b/android/main.c
> @@ -36,6 +36,8 @@
> #include <glib.h>
>
> #include "log.h"
> +#include "hcid.h"
The hcid.h include is bogus. That should not be needed. If it is, then we need to fix that first.
> +#include "sdpd.h"
>
> #include "lib/bluetooth.h"
> #include "lib/mgmt.h"
> @@ -43,12 +45,36 @@
>
> #define SHUTDOWN_GRACE_SECONDS 10
>
> +struct main_opts main_opts;
> +
> static GMainLoop *event_loop;
> static struct mgmt *mgmt_if = NULL;
>
> static uint8_t mgmt_version = 0;
> static uint8_t mgmt_revision = 0;
>
> +static int sdp_start(void)
> +{
> + DBG("");
> +
> + /* TODO: add logic */
> +
> + /* sdpd-server use these settings */
> + memset(&main_opts, 0, sizeof(main_opts));
Then fix sdpd-server to not use these. I do not want this included.
> +
> + /* Use params: mtu = 0, flags = 0 */
> + return start_sdp_server(0, 0);
> +}
> +
> +static void sdp_stop(void)
> +{
> + DBG("");
> +
> + /* TODO: add logic */
> +
> + stop_sdp_server();
> +}
> +
Right now sdp_start and sdp_stop look like pointless wrappers around nothing.
> static gboolean quit_eventloop(gpointer user_data)
> {
> g_main_loop_quit(event_loop);
> @@ -287,11 +313,13 @@ int main(int argc, char *argv[])
> sigaction(SIGTERM, &sa, NULL);
>
> init_mgmt_interface();
> + sdp_start();
>
> DBG("Entering main loop");
>
> g_main_loop_run(event_loop);
>
> + sdp_stop();
> cleanup_mgmt_interface();
> g_main_loop_unref(event_loop);
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 09/15] android: Add Android Makefile for libbluetooth
From: Marcel Holtmann @ 2013-10-09 19:43 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1381243883-2745-10-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> Build libbluetooth library for Android.
> ---
> android/Android.mk | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/android/Android.mk b/android/Android.mk
> index f5fd863..8f95a66 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -41,6 +41,7 @@ LOCAL_SHARED_LIBRARIES := \
> libglib \
>
> LOCAL_MODULE := bluetoothd
> +LOCAL_REQUIRED_MODULES := libbluetooth
>
> include $(BUILD_EXECUTABLE)
>
> @@ -63,3 +64,55 @@ LOCAL_MODULE_TAGS := optional
> LOCAL_MODULE_CLASS := SHARED_LIBRARIES
>
> include $(BUILD_SHARED_LIBRARY)
> +
> +#
> +# libbluetooth library
> +#
> +
> +include $(CLEAR_VARS)
> +
> +LOCAL_SRC_FILES := \
> + ../lib/bluetooth.c \
> + ../lib/sdp.c \
> + ../lib/hci.c \
> + ../lib/uuid.c \
why are we building a shared library here. What is it useful for. Even within BlueZ we do not install libbluetooth anymore.
> +
> +LOCAL_C_INCLUDES := \
> + $(LOCAL_PATH)/../lib/ \
> + $(LOCAL_PATH)/../src/shared \
The library clearly does not include anything from src/shared/. So what is that doing here.
> +
> +LOCAL_C_INCLUDES += \
> + $(call include-path-for, glib) \
> + $(call include-path-for, glib)/glib \
The library clearly does not need GLib. So why are we doing that here.
> +
> +LOCAL_CFLAGS += -O3
> +
> +# to suppress the "pointer of type 'void *' used in arithmetic" warning
> +LOCAL_CFLAGS += -Wno-pointer-arith
> +
> +# to suppress the "missing initializer near initialization" warning
> +LOCAL_CFLAGS += -Wno-missing-field-initializers
Lets fix the actual warnings.
> +
> +# Define missing flags for Android 4.2
> +LOCAL_CFLAGS += -DSOCK_CLOEXEC=02000000 -DSOCK_NONBLOCK=04000
And are we sure that these flags are needed for the library.
> +
> +lib_headers := \
> + bluetooth.h \
> + hci.h \
> + hci_lib.h \
> + sco.h \
> + l2cap.h \
> + sdp.h \
> + sdp_lib.h \
> + rfcomm.h \
> + bnep.h \
> + cmtp.h \
> + hidp.h \
I would really prefer we do not install these.
> +
> +$(shell mkdir -p $(LOCAL_PATH)/../lib/bluetooth)
> +
> +$(foreach file,$(lib_headers), $(shell ln -sf ../$(file) $(LOCAL_PATH)/../lib/bluetooth/$(file)))
> +
> +LOCAL_MODULE := libbluetooth
> +
> +include $(BUILD_SHARED_LIBRARY)
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 08/15] android: Add adapter and device struct for BlueZ daemon
From: Marcel Holtmann @ 2013-10-09 19:39 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1381243883-2745-9-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> Adapter structure in BlueZ daemon keeps track of default adapter
> and device structure keeps track about found devices.
> ---
> android/bt_adapter.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
> android/bt_adapter.h | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 116 insertions(+)
> create mode 100644 android/bt_adapter.c
> create mode 100644 android/bt_adapter.h
>
> diff --git a/android/bt_adapter.c b/android/bt_adapter.c
> new file mode 100644
> index 0000000..e21d50c
> --- /dev/null
> +++ b/android/bt_adapter.c
> @@ -0,0 +1,56 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2013 Intel Corporation. All rights reserved.
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#include "bt_adapter.h"
the include of its own header should always be the last one. Never the first one.
> +#include "log.h"
> +#include "src/shared/mgmt.h"
> +
> +struct bt_adapter *bt_adapter_new(uint16_t index, struct mgmt *mgmt_if)
> +{
> + struct bt_adapter *adapter;
> +
> + adapter = g_try_new0(struct bt_adapter, 1);
> + if (!adapter)
> + return NULL;
Since this all brand new code, use g_new0 and we just exit on memory allocation failures for our own data structures.
> +
> + adapter->dev_id = index;
> + adapter->mgmt = mgmt_ref(mgmt_if);
> +
> + return adapter;
> +}
> +
> +void adapter_start(struct bt_adapter *adapter)
Why does this now have no bt_ prefix?
> +{
> + DBG("enabled %u", adapter->dev_id);
> +
> + /* TODO: CB: report scan mode */
> +
> + /* TODO: SDP start here */
> +
> + /* TODO: CB: report state on */
> +}
> +
> +void adapter_stop(struct bt_adapter *adapter)
> +{
> + DBG("disabled %u", adapter->dev_id);
> +}
> diff --git a/android/bt_adapter.h b/android/bt_adapter.h
> new file mode 100644
> index 0000000..6877cc7
> --- /dev/null
> +++ b/android/bt_adapter.h
> @@ -0,0 +1,60 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2013 Intel Corporation. All rights reserved.
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <glib.h>
> +
> +#include "lib/bluetooth.h"
> +
> +struct bt_device {
> + int refcnt;
> +
> + bdaddr_t bdaddr;
> + uint8_t bdaddr_type;
> + uint32_t cod;
> + char *name;
> +};
Why is bt_device declared in bt_adapter. And why are these public in the first place. Please hide these details.
> +
> +struct bt_adapter {
> + int refcnt;
> +
> + uint16_t dev_id;
And now we are falling back to calling this dev_id. Do we need the index at all actually?
> + struct mgmt *mgmt;
> + bdaddr_t bdaddr;
> + uint32_t dev_class;
> +
> + char *name;
> + char *short_name;
If we are not using the short name, then don't bother storing it. Only implement things we really need.
> +
> + uint32_t supported_settings;
> + uint32_t current_settings;
> +
> + GList *found_devices;
> +};
> +
> +struct bt_adapter *bt_adapter_new(uint16_t index, struct mgmt *mgmt_if);
> +
> +void adapter_start(struct bt_adapter *adapter);
> +void adapter_stop(struct bt_adapter *adapter);
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 07/15] android: Create HAL API header skeleton
From: Marcel Holtmann @ 2013-10-09 19:34 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1381243883-2745-8-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> Header describes the protocol between Android HAL threads and BlueZ
> daemon.
> ---
> android/hal_msg.h | 255 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 255 insertions(+)
> create mode 100644 android/hal_msg.h
>
> diff --git a/android/hal_msg.h b/android/hal_msg.h
> new file mode 100644
> index 0000000..c6bc883
> --- /dev/null
> +++ b/android/hal_msg.h
> @@ -0,0 +1,255 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2013 Intel Corporation. All rights reserved.
> + *
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#ifndef __packed
> +#define __packed __attribute__((packed))
> +#endif
nowhere in the code we are using __packed. That is more a kernel thing. Just use the gcc style directly.
> +
> +typedef struct {
> + uint8_t b[6];
> +} __packed __bdaddr_t;
Just use a uint8_t bdaddr[6] instead.
> +
> +struct hal_msg_hdr {
> + uint8_t service_id;
> + uint8_t opcode;
> + uint16_t len;
> + uint8_t payload[0];
I would not include the payload[0] here.
> +} __packed;
> +
> +#define HAL_SERVICE_ID_CORE 0
> +#define HAL_SERVICE_ID_BLUETOOTH 1
> +#define HAL_SERVICE_ID_SOCK 2
> +#define HAL_SERVICE_ID_HIDHOST 3
> +#define HAL_SERVICE_ID_PAN 4
> +#define HAL_SERVICE_ID_HANDSFREE 5
> +#define HAL_SERVICE_ID_AD2P 6
> +#define HAL_SERVICE_ID_HEALTH 7
> +#define HAL_SERVICE_ID_AVRCP 8
> +#define HAL_SERVICE_ID_GATT 9
> +
> +/* Core Service */
> +
> +#define HAL_MSG_OP_ERROR 0x00
> +struct hal_msg_rp_error {
> + uint8_t status;
> +} __packed;
using RSP, rsp, CMD, cmd, EVT and EVT seems to be clearer I think.
> +
> +#define HAL_MSG_OP_REGISTER_MODULE 0x01
> +struct hal_msg_cp_register_module {
> + uint8_t service_id;
> +} __packed;
> +struct hal_msg_rp_register_module {
> + uint8_t service_id;
> +} __packed;
> +
> +#define HAL_MSG_OP_UNREGISTER_MODULE 0x02
> +struct hal_msg_cp_unregister_module {
> + uint8_t service_id;
> +} __packed;
> +
> +/* Bluetooth Core HAL API */
> +
> +#define HAL_MSG_OP_BT_ENABLE 0x01
> +
> +#define HAL_MSG_OP_BT_DISABLE 0x02
> +
> +#define HAL_MSG_OP_BT_GET_ADAPTER_PROPS 0x03
> +
> +#define HAL_MSG_OP_BT_GET_ADAPTER_PROP 0x04
> +struct hal_msg_cp_bt_get_adapter_prop {
> + uint8_t type;
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_SET_ADAPTER_PROP 0x05
> +struct hal_msg_cp_bt_set_adapter_prop {
> + uint8_t type;
> + uint16_t len;
> + uint8_t val[0];
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_GET_REMOTE_DEVICE_PROPS 0x06
> +struct hal_msg_cp_bt_get_remote_device_props {
> + __bdaddr_t bdaddr;
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_GET_REMOTE_DEVICE_PROP 0x07
> +struct hal_msg_cp_bt_get_remote_device_prop {
> + __bdaddr_t bdaddr;
> + uint8_t type;
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_SET_REMOTE_DEVICE_PROP 0x08
> +struct hal_msg_cp_bt_set_remote_device_prop {
> + __bdaddr_t bdaddr;
> + uint8_t type;
> + uint16_t len;
Lets make these all align properly.
> + uint8_t val[0];
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_GET_REMOTE_SERVICE_REC 0x09
> +struct hal_msg_cp_bt_get_remote_service_rec {
> + __bdaddr_t bdaddr;
> + uint8_t uuid[16];
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_GET_REMOTE_SERVICE 0x0a
> +struct hal_msg_cp_bt_get_remote_service {
> + __bdaddr_t bdaddr;
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_START_DISCOVERY 0x0b
> +
> +#define HAL_MSG_OP_BT_CANCEL_DISCOVERY 0x0c
> +
> +#define HAL_MSG_OP_BT_CREATE_BOND 0x0d
> +struct hal_msg_cp_bt_create_bond {
> + __bdaddr_t bdaddr;
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_REMOVE_BOND 0x0d
> +struct hal_msg_cp_bt_remove_bond {
> + __bdaddr_t bdaddr;
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_CANCEL_BOND 0x0f
> +struct hal_msg_cp_bt_cancel_bond {
> + __bdaddr_t bdaddr;
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_PIN_REPLY 0x10
> +struct hal_msg_cp_bt_pin_reply {
> + __bdaddr_t bdaddr;
> + uint8_t accept;
> + uint8_t pin_len;
> + uint8_t pin_code[16];
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_SSP_REPLY 0x11
> +struct hal_msg_cp_bt_ssp_reply {
> + __bdaddr_t bdaddr;
> + uint8_t ssp_variant;
> + uint8_t accept;
> + uint32_t passkey;
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_DUT_MODE_CONF 0x12
> +struct hal_msg_cp_bt_dut_mode_conf {
> + uint8_t enable;
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_DUT_MODE_SEND 0x13
> +struct hal_msg_cp_bt_dut_mode_send {
> + uint16_t opcode;
> + uint8_t len;
> + uint8_t data[0];
> +} __packed;
> +
> +#define HAL_MSG_OP_BT_LE_TEST_MODE 0x14
> +struct hal_msg_cp_bt_le_test_mode {
> + uint16_t opcode;
> + uint8_t len;
> + uint8_t data[0];
> +} __packed;
> +
> +/* Notifications and confirmations */
> +
> +#define HAL_MSG_EV_BT_ERROR 0x80
> +
> +#define HAL_MSG_EV_BT_ADAPTER_STATE_CHANGED 0x81
> +struct hal_msg_ev_bt_adapter_state_changed {
> + uint8_t state;
> +} __packed;
> +
> +#define HAL_MSG_EV_BT_ADAPTER_PROPS_CHANGED 0x82
> +struct hal_property {
> + uint8_t type;
> + uint16_t len;
> + uint8_t val[0];
> +} __packed;
> +struct hal_msg_ev_bt_adapter_props_changed {
> + uint8_t status;
> + uint8_t num_props;
> + struct hal_property props[0];
> +} __packed;
> +
> +#define HAL_MSG_EV_BT_REMOTE_DEVICE_PROPS 0x83
> +struct hal_msg_ev_bt_remote_device_props {
> + uint8_t status;
> + __bdaddr_t bdaddr;
> + uint8_t num_props;
> + struct hal_property props[0];
> +} __packed;
> +
> +#define HAL_MSG_EV_BT_DEVICE_FOUND 0x84
> +struct hal_msg_ev_bt_device_found {
> + uint8_t num_props;
> + struct hal_property props[0];
> +} __packed;
> +
> +#define HAL_MSG_EV_BT_DISCOVERY_STATE_CHANGED 0x85
> +struct hal_msg_ev_bt_discovery_state_changed {
> + uint8_t state;
> +} __packed;
> +
> +#define HAL_MSG_EV_BT_PIN_REQUEST 0x86
> +struct hal_msg_ev_bt_pin_request {
> + __bdaddr_t bdaddr;
> + uint8_t name[249 - 1];
> + uint8_t class_of_dev[3];
> +} __packed;
> +
> +#define HAL_MSG_EV_BT_SSP_REQUEST 0x87
> +struct hal_msg_ev_bt_ssp_request {
> + __bdaddr_t bdaddr;
> + uint8_t name[249 - 1];
> + uint8_t class_of_dev[3];
> + uint8_t pairing_variant;
> + uint32_t passkey;
> +} __packed;
> +
> +#define HAL_MSG_EV_BT_BOND_STATE_CHANGED 0x88
> +struct hal_msg_ev_bt_bond_state_changed {
> + uint8_t status;
> + __bdaddr_t bdaddr;
> + uint8_t state;
> +} __packed;
> +
> +#define HAL_MSG_EV_BT_ACL_STATE_CHANGED 0x89
> +struct hal_msg_ev_bt_acl_state_changed {
> + uint8_t status;
> + __bdaddr_t bdaddr;
> + uint8_t state;
> +} __packed;
> +
> +#define HAL_MSG_EV_BT_DUT_MODE_RECEIVE 0x8a
> +struct hal_msg_ev_bt_dut_mode_receive {
> + uint16_t opcode;
> + uint8_t len;
> + uint8_t data[0];
> +} __packed;
> +
> +#define HAL_MSG_EV_BT_LE_TEST_MODE 0x8b
> +struct hal_msg_ev_bt_le_test_mode {
> + uint8_t status;
> + uint16_t num_packets;
> +} __packed;
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 06/15] android: Add basic mgmt initialization sequence
From: Marcel Holtmann @ 2013-10-09 19:30 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1381243883-2745-7-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> Initialize bluetooth controller via mgmt interface.
> ---
> Makefile.android | 4 +-
> android/Android.mk | 11 +++
> android/main.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 203 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile.android b/Makefile.android
> index e161e6d..9a2c486 100644
> --- a/Makefile.android
> +++ b/Makefile.android
> @@ -1,7 +1,9 @@
> if ANDROID
> noinst_PROGRAMS += android/bluetoothd
>
> -android_bluetoothd_SOURCES = android/main.c src/log.c
> +android_bluetoothd_SOURCES = android/main.c src/log.c \
> + src/shared/util.h src/shared/util.c \
> + src/shared/mgmt.h src/shared/mgmt.c
> android_bluetoothd_LDADD = @GLIB_LIBS@
> endif
>
> diff --git a/android/Android.mk b/android/Android.mk
> index 2cabff4..f5fd863 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -15,10 +15,15 @@ include $(CLEAR_VARS)
> LOCAL_SRC_FILES := \
> main.c \
> log.c \
> + ../src/shared/mgmt.c \
> + ../src/shared/util.c \
>
> LOCAL_C_INCLUDES := \
> $(call include-path-for, glib) \
> $(call include-path-for, glib)/glib \
> +
> +LOCAL_C_INCLUDES += \
> + $(LOCAL_PATH)/../ \
> $(LOCAL_PATH)/../src \
do we need these nested includes actually? We could also just fix the includes. BlueZ historically has not been good with clear includes. I started to fix this, but it seems I have not gotten to all of them yet.
>
> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> @@ -26,6 +31,12 @@ LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> # to suppress the "warning: missing initializer near initialization.." warning
> LOCAL_CFLAGS += -Wno-missing-field-initializers
>
> +# to suppress the "pointer of type 'void *' used in arithmetic" warning
> +LOCAL_CFLAGS += -Wno-pointer-arith
Why do we need to suppress these warning. Can we not just fix them.
> +
> +# Define missing flags for Android 4.2
> +LOCAL_CFLAGS += -DSOCK_CLOEXEC=02000000 -DSOCK_NONBLOCK=04000
> +
This thing is dangerous. Do we really bother with Android 4.2 support and can not rely on a newer version that has this fixed properly in bionic.
> LOCAL_SHARED_LIBRARIES := \
> libglib \
>
> diff --git a/android/main.c b/android/main.c
> index f75b0a8..3580ac7 100644
> --- a/android/main.c
> +++ b/android/main.c
> @@ -25,6 +25,7 @@
> #include <config.h>
> #endif
>
> +#include <stdbool.h>
> #include <signal.h>
> #include <stdint.h>
> #include <stdio.h>
> @@ -36,9 +37,17 @@
>
> #include "log.h"
>
> +#include "lib/bluetooth.h"
> +#include "lib/mgmt.h"
> +#include "src/shared/mgmt.h"
> +
> #define SHUTDOWN_GRACE_SECONDS 10
>
> static GMainLoop *event_loop;
> +static struct mgmt *mgmt_if = NULL;
> +
> +static uint8_t mgmt_version = 0;
> +static uint8_t mgmt_revision = 0;
>
> static gboolean quit_eventloop(gpointer user_data)
> {
> @@ -67,6 +76,183 @@ static GOptionEntry options[] = {
> { NULL }
> };
>
> +static void read_info_complete(uint8_t status, uint16_t length,
> + const void *param, void *user_data)
> +{
> + /* TODO: Store Controller information */
> +
> + /**
> + * Register all event notification handlers for controller.
> + *
> + * The handlers are registered after a succcesful read of the
> + * controller info. From now on they can track updates and
> + * notifications.
> + */
This is not our comment style.
> +}
> +
> +
> +static void mgmt_index_added_event(uint16_t index, uint16_t length,
> + const void *param, void *user_data)
> +{
> + info("%s: index %u", __func__, index);
> +
> + DBG("sending read info command for index %u", index);
> +
> + if (mgmt_send(mgmt_if, MGMT_OP_READ_INFO, index, 0, NULL,
> + read_info_complete, NULL, NULL) > 0)
> + return;
> +
> + error("Failed to read adapter info for index %u", index);
> +
> +}
I prefer if we focus on one controller index and one controller index only. There is no need to bother reading information from controllers that we will never use.
> +
> +static void mgmt_index_removed_event(uint16_t index, uint16_t length,
> + const void *param, void *user_data)
> +{
> + info("%s: index %u", __func__, index);
> +}
> +
> +static void read_index_list_complete(uint8_t status, uint16_t length,
> + const void *param, void *user_data)
> +{
> + const struct mgmt_rp_read_index_list *rp = param;
> + uint16_t num;
> + int i;
> +
> + info(__func__);
> +
> + if (status != MGMT_STATUS_SUCCESS) {
> + error("%s: Failed to read index list: %s (0x%02x)",
> + __func__, mgmt_errstr(status), status);
> + return;
> + }
> +
> + if (length < sizeof(*rp)) {
> + error("%s: Wrong size of read index list response", __func__);
> + return;
> + }
> +
> + num = btohs(rp->num_controllers);
> +
> + DBG("%s: Number of controllers: %d", __func__, num);
> +
> + if (num * sizeof(uint16_t) + sizeof(*rp) != length) {
> + error("%s: Incorrect pkt size for index list rsp", __func__);
> + return;
> + }
> +
> + for (i = 0; i < num; i++) {
> + uint16_t index;
> +
> + index = btohs(rp->index[i]);
> +
> + DBG("%s: Found index %u", __func__, index);
> +
> + /**
> + * Use index added event notification.
> + */
> + mgmt_index_added_event(index, 0, NULL, NULL);
> + }
Same here. Lets just pick one controller and be done with it.
> +}
> +
> +
Avoid double empty lines.
> +static void read_commands_complete(uint8_t status, uint16_t length,
> + const void *param, void *user_data)
> +{
> + const struct mgmt_rp_read_commands *rp = param;
> + uint16_t num_commands, num_events;
> +
> + info(__func__);
These can not be info() ever. Make them DBG().
> +
> + if (status != MGMT_STATUS_SUCCESS) {
> + error("Failed to read supported commands: %s (0x%02x)",
> + mgmt_errstr(status), status);
> + return;
> + }
> +
> + if (length < sizeof(*rp)) {
> + error("Wrong size of read commands response");
> + return;
> + }
> +
> + num_commands = btohs(rp->num_commands);
> + num_events = btohs(rp->num_events);
> +
> + DBG("Number of commands: %d", num_commands);
> + DBG("Number of events: %d", num_events);
> +}
When copying code from src/adapter.c can we please at least be smart about it. Code that has no benefit should not be run. If it is currently not used, then leave it out for now.
> +
> +static void read_version_complete(uint8_t status, uint16_t length,
> + const void *param, void *user_data)
> +{
> + const struct mgmt_rp_read_version *rp = param;
> +
> + info(__func__);
> +
> + if (status != MGMT_STATUS_SUCCESS) {
> + error("Failed to read version information: %s (0x%02x)",
> + mgmt_errstr(status), status);
> + return;
> + }
> +
> + if (length < sizeof(*rp)) {
> + error("Wrong size response");
> + return;
> + }
> +
> + mgmt_version = rp->version;
> + mgmt_revision = btohs(rp->revision);
> +
> + info("Bluetooth management interface %u.%u initialized",
> + mgmt_version, mgmt_revision);
> +
> + if (mgmt_version < 1) {
> + error("Version 1.0 or later of management interface required");
> + abort();
> + }
> +
> + DBG("sending read supported commands command");
> +
> + mgmt_send(mgmt_if, MGMT_OP_READ_COMMANDS, MGMT_INDEX_NONE, 0, NULL,
> + read_commands_complete, NULL, NULL);
> +
> + mgmt_register(mgmt_if, MGMT_EV_INDEX_ADDED, MGMT_INDEX_NONE,
> + mgmt_index_added_event, NULL, NULL);
> + mgmt_register(mgmt_if, MGMT_EV_INDEX_REMOVED, MGMT_INDEX_NONE,
> + mgmt_index_removed_event, NULL, NULL);
> +
> + DBG("sending read index list command");
> +
> + if (mgmt_send(mgmt_if, MGMT_OP_READ_INDEX_LIST, MGMT_INDEX_NONE, 0,
> + NULL, read_index_list_complete, NULL, NULL) > 0)
> + return;
> +
> + error("Failed to read controller index list");
> +}
> +
> +static bool init_mgmt_interface(void)
> +{
If you are not using the return value, then do not return it.
> + mgmt_if = mgmt_new_default();
> + if (mgmt_if == NULL) {
> + error("Failed to access management interface");
> + return false;
> + }
> +
> + if (mgmt_send(mgmt_if, MGMT_OP_READ_VERSION, MGMT_INDEX_NONE, 0, NULL,
> + read_version_complete, NULL, NULL) == 0) {
> + error("Error sending READ_VERSION mgmt command");
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static void cleanup_mgmt_interface(void)
> +{
> + mgmt_unref(mgmt_if);
> + mgmt_if = NULL;
> +}
> +
> int main(int argc, char *argv[])
> {
> GOptionContext *context;
> @@ -100,10 +286,13 @@ int main(int argc, char *argv[])
> sigaction(SIGINT, &sa, NULL);
> sigaction(SIGTERM, &sa, NULL);
>
> + init_mgmt_interface();
> +
> DBG("Entering main loop");
>
> g_main_loop_run(event_loop);
>
> + cleanup_mgmt_interface();
> g_main_loop_unref(event_loop);
>
> info("Exit");
Regards
Marcel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox