Skip to contentSkip to conversation

android: Add skeleton of BlueZ Android daemon

PATCH In review Frederic Danis 5 participants 15 messages Sep 20, 2013 – Sep 27, 2013

Subsystems (2)
Bluetooth drivers Kbuild
lore
Share & tools
lore

Frederic Danis

author
#0 lore raw

PATCH 1/2 android: Add skeleton of BlueZ Android daemon

Define local mapping to glib path, otherwise this has to be inside central place in the build repository.

Retrieve Bluetooth version from configure.ac.

---
 .gitignore         |    2 +
 Android.mk         |    9 ++++
 Makefile.am        |    1 +
 Makefile.android   |    7 ++++
 android/Android.mk |   24 +++++++++++
 android/main.c     |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 configure.ac       |    5 +++
 7 files changed, 167 insertions(+)
 create mode 100644 Android.mk
 create mode 100644 Makefile.android
 create mode 100644 android/Android.mk
 create mode 100644 android/main.c
.gitignore +2 -0
index 8a25a3e..331a18b 100644
@@ -98,3 +98,5 @@ unit/test-gobex-packet
 unit/test-gobex-transfer
 unit/test-*.log
 unit/test-*.trs
+
+android/bluezd
Android.mk new file +9 -0
new file mode 100644
index 0000000..5ec3bb7
@@ -0,0 +1,9 @@
+LOCAL_PATH := $(call my-dir)
+
+# Retrieve BlueZ version from configure.ac file
+BLUEZ_VERSION := $(shell grep AC_INIT $(LOCAL_PATH)/configure.ac | tr -d ' ' | sed -e 's/.*(.*,\(.*\))/\1/')
+
+# Specify pathmap for glib
+pathmap_INCL += glib:external/bluetooth/glib
+
+include $(call all-subdir-makefiles)
Makefile.am +1 -0
index 4e4b1c5..2c048bd 100644
@@ -179,6 +179,7 @@ test_scripts =
 
 include Makefile.tools
 include Makefile.obexd
+include Makefile.android
 
 if HID2HCI
 rulesdir = @UDEV_DIR@/rules.d
Makefile.android new file +7 -0
new file mode 100644
index 0000000..7b901ff
@@ -0,0 +1,7 @@
+
+if ANDROID_DAEMON
+noinst_PROGRAMS += android/bluezd
+
+android_bluezd_SOURCES = android/main.c
+android_bluezd_LDADD = @GLIB_LIBS@
+endif
android/Android.mk new file +24 -0
new file mode 100644
index 0000000..50f3c36
@@ -0,0 +1,24 @@
+LOCAL_PATH := $(call my-dir)
+
+#
+# Android BlueZ daemon (bluezd)
+#
+
+include $(CLEAR_VARS)
+
+LOCAL_SRC_FILES := \
+	main.c \
+
+LOCAL_C_INCLUDES := \
+	$(call include-path-for, glib) \
+	$(call include-path-for, glib)/glib \
+	$(LOCAL_PATH)/../src \
+
+LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
+
+LOCAL_SHARED_LIBRARIES := \
+	libglib \
+
+LOCAL_MODULE := bluezd
+
+include $(BUILD_EXECUTABLE)
android/main.c new file +119 -0

119 lines not on this page · raw message

configure.ac +5 -0
index 41c2935..3b7a5d9 100644
@@ -242,4 +242,9 @@ AC_DEFINE_UNQUOTED(CONFIGDIR, "${configdir}",
 			[Directory for the configuration files])
 AC_SUBST(CONFIGDIR, "${configdir}")
 
+AC_ARG_ENABLE(android-daemon, AC_HELP_STRING([--enable-android-daemon],
+			[enable BlueZ Android daemon]),
+					[android_daemon=${enableval}])
+AM_CONDITIONAL(ANDROID_DAEMON, test "${android_daemon}" = "yes")
+
 AC_OUTPUT(Makefile src/bluetoothd.8 lib/bluez.pc)
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH 2/2 android: Android version of log.c

Add logging system to BlueZ Android daemon.
Android build will use android/log.c file while autotools build will use src/log.c instead.

---
 Makefile.android   |    2 +-
 android/Android.mk |    1 +
 android/log.c      |  184 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 android/main.c     |   39 +++++++++++
 4 files changed, 225 insertions(+), 1 deletion(-)
 create mode 100644 android/log.c
Makefile.android +1 -1
index 7b901ff..8f65dbf 100644
@@ -2,6 +2,6 @@
 if ANDROID_DAEMON
 noinst_PROGRAMS += android/bluezd
 
-android_bluezd_SOURCES = android/main.c
+android_bluezd_SOURCES = android/main.c src/log.c
 android_bluezd_LDADD = @GLIB_LIBS@
 endif
android/Android.mk +1 -0
index 50f3c36..25099b3 100644
@@ -8,6 +8,7 @@ include $(CLEAR_VARS)
 
 LOCAL_SRC_FILES := \
 	main.c \
+	log.c \
 
 LOCAL_C_INCLUDES := \
 	$(call include-path-for, glib) \
android/log.c new file +184 -0

184 lines not on this page · raw message

android/main.c +39 -0
index 37a64c1..ef62b3d 100644
@@ -33,6 +33,7 @@
 
 #include <glib.h>
 
+#include "log.h"
 #include "hcid.h"
 
 #define SHUTDOWN_GRACE_SECONDS 10

@@ -58,19 +59,45 @@ static void signal_handler(int sig)
 	case SIGINT:
 	case SIGTERM:
 		if (__terminated == 0) {
+			info("Terminating");
 			g_timeout_add_seconds(SHUTDOWN_GRACE_SECONDS,
 							quit_eventloop, NULL);
 		}
 
 		__terminated = 1;
 		break;
+
+	case SIGUSR2:
+		__btd_toggle_debug();
+		break;
 	}
 }
 
+static char *option_debug = NULL;
 static gboolean option_detach = TRUE;
 static gboolean option_version = FALSE;
 
+static void free_options(void)
+{
+	g_free(option_debug);
+	option_debug = NULL;
+}
+
+static gboolean parse_debug(const char *key, const char *value,
+				gpointer user_data, GError **error)
+{
+	if (value)
+		option_debug = g_strdup(value);
+	else
+		option_debug = g_strdup("*");
+
+	return TRUE;
+}
+
 static GOptionEntry options[] = {
+	{ "debug", 'd', G_OPTION_FLAG_OPTIONAL_ARG,
+				G_OPTION_ARG_CALLBACK, parse_debug,
+				"Specify debug options to enable", "DEBUG" },
 	{ "nodetach", 'n', G_OPTION_FLAG_REVERSE,
 				G_OPTION_ARG_NONE, &option_detach,
 				"Run with logging in foreground", NULL },

@@ -110,10 +137,22 @@ int main(int argc, char *argv[])
 	sa.sa_handler = signal_handler;
 	sigaction(SIGINT, &sa, NULL);
 	sigaction(SIGTERM, &sa, NULL);
+	sigaction(SIGUSR2, &sa, NULL);
+
+	__btd_log_init(option_debug, option_detach);
+
+	/* no need to keep parsed option in memory */
+	free_options();
+
+	DBG("Entering main loop");
 
 	g_main_loop_run(event_loop);
 
 	g_main_loop_unref(event_loop);
 
+	info("Exit");
+
+	__btd_log_cleanup();
+
 	return 0;
 }
Signature · 1 line
1.7.9.5
maintainer
#2 lore raw

Re: [PATCH 2/2] android: Android version of log.c

Hi Fred,

Quoted 76 lines

Add logging system to BlueZ Android daemon.
Android build will use android/log.c file while autotools build will use src/log.c instead.
---

Makefile.android   |    2 +-
android/Android.mk |    1 +
android/log.c      |  184 ++++++++++++++++++++++++++++++++++++++++++++++++++++
android/main.c     |   39 +++++++++++
4 files changed, 225 insertions(+), 1 deletion(-)

create mode 100644 android/log.c

Makefile.android +0 -0
index 7b901ff..8f65dbf 100644
@@ -2,6 +2,6 @@

if ANDROID_DAEMON
noinst_PROGRAMS += android/bluezd

Makefile.android +1 -1
-android_bluezd_SOURCES = android/main.c
+android_bluezd_SOURCES = android/main.c src/log.c

android_bluezd_LDADD = @GLIB_LIBS@
endif

android/Android.mk +1 -0
index 50f3c36..25099b3 100644
@@ -8,6 +8,7 @@ include $(CLEAR_VARS)
 
 LOCAL_SRC_FILES := \
 	main.c \
+	log.c \
 
 LOCAL_C_INCLUDES := \
 	$(call include-path-for, glib) \
android/log.c new file +35 -0
new file mode 100644
index 0000000..1f98196
@@ -0,0 +1,184 @@
+/*
+ *
+ *  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
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdarg.h>
+#include <unistd.h>
+#include <sys/uio.h>
+
+#include <glib.h>
+

android/log.c line 45

Quoted 10 lines
android/log.c new file +10 -0
+#include "log.h"
+
+#define LOG_DEBUG	3
+#define LOG_INFO	4
+#define LOG_WARN	5
+#define LOG_ERR		6
+
+const char tag[] = "BlueZ";
+int system_fd;
+int detached;

Local variables should be static.

Quoted 5 lines
android/log.c new file +5 -0
+
+static void android_log(int pri, const char *fmt, va_list ap)
+{
+	char *msg;
+	struct iovec vec[3];

android/log.c line 60

Quoted 10 lines
android/log.c new file +10 -0
+
+	msg = g_strdup_vprintf(fmt, ap);
+
+	if (!detached) {
+		vec[0].iov_base = (void *) msg;
+		vec[0].iov_len = strlen(msg) + 1;
+		vec[1].iov_base = "\n";
+		vec[1].iov_len = 1;
+		writev(STDERR_FILENO, vec, 2);
+	}

Lets not worry about logging to stderr at all.

Quoted 137 lines
android/log.c new file +124 -0
+
+	if (system_fd == -1)
+		goto done;
+
+	vec[0].iov_base = (unsigned char *) &pri;
+	vec[0].iov_len = 1;
+	vec[1].iov_base = (void *) tag;
+	vec[1].iov_len = strlen(tag) + 1;
+	vec[2].iov_base = (void *) msg;
+	vec[2].iov_len = strlen(msg) + 1;
+
+	writev(system_fd, vec, 3);
+
+done:
+	g_free(msg);
+}
+
+void info(const char *format, ...)
+{
+	va_list ap;
+
+	va_start(ap, format);
+
+	android_log(LOG_INFO, format, ap);
+
+	va_end(ap);
+}
+
+void warn(const char *format, ...)
+{
+	va_list ap;
+
+	va_start(ap, format);
+
+	android_log(LOG_WARN, format, ap);
+
+	va_end(ap);
+}
+
+void error(const char *format, ...)
+{
+	va_list ap;
+
+	va_start(ap, format);
+
+	android_log(LOG_ERR, format, ap);
+
+	va_end(ap);
+}
+
+void btd_debug(const char *format, ...)
+{
+	va_list ap;
+
+	va_start(ap, format);
+
+	android_log(LOG_DEBUG, format, ap);
+
+	va_end(ap);
+}
+
+extern struct btd_debug_desc __start___debug[];
+extern struct btd_debug_desc __stop___debug[];
+
+static char **enabled = NULL;
+
+static gboolean is_enabled(struct btd_debug_desc *desc)
+{
+	int i;
+
+	if (enabled == NULL)
+		return 0;
+
+	for (i = 0; enabled[i] != NULL; i++)
+		if (desc->file != NULL && g_pattern_match_simple(enabled[i],
+							desc->file) == TRUE)
+			return 1;
+
+	return 0;
+}
+
+void __btd_enable_debug(struct btd_debug_desc *start,
+					struct btd_debug_desc *stop)
+{
+	struct btd_debug_desc *desc;
+
+	if (start == NULL || stop == NULL)
+		return;
+
+	for (desc = start; desc < stop; desc++) {
+		if (is_enabled(desc))
+			desc->flags |= BTD_DEBUG_FLAG_PRINT;
+	}
+}
+
+void __btd_toggle_debug(void)
+{
+	struct btd_debug_desc *desc;
+
+	for (desc = __start___debug; desc < __stop___debug; desc++)
+		desc->flags |= BTD_DEBUG_FLAG_PRINT;
+}
+
+void __btd_log_init(const char *debug, int detach)
+{
+	if (debug != NULL)
+		enabled = g_strsplit_set(debug, ":, ", 0);
+
+	__btd_enable_debug(__start___debug, __stop___debug);
+
+	detached = detach;
+
+	system_fd = open("/dev/log/system", O_WRONLY);
+
+	info("Bluetooth daemon %s", VERSION);
+}
+
+void __btd_log_cleanup(void)
+{
+	close(system_fd);
+	system_fd = -1;
+
+	g_strfreev(enabled);
+}
android/main.c +1 -0
index 37a64c1..ef62b3d 100644
@@ -33,6 +33,7 @@
 
 #include <glib.h>
 
+#include "log.h"
 #include "hcid.h"
 
 #define SHUTDOWN_GRACE_SECONDS 10

android/main.c static void signal_handler(int sig) · line 62

Quoted 4 lines
android/main.c +1 -0
@@ -58,19 +59,45 @@ static void signal_handler(int sig)
 	case SIGINT:
 	case SIGTERM:
 		if (__terminated == 0) {
+			info("Terminating");

Since you are in a signal handler and not using signalfd, this is racy.

android/main.c static void signal_handler(int sig) · line 73

Quoted 11 lines
android/main.c +4 -0
 			g_timeout_add_seconds(SHUTDOWN_GRACE_SECONDS,
 							quit_eventloop, NULL);
 		}
 
 		__terminated = 1;
 		break;
+
+	case SIGUSR2:
+		__btd_toggle_debug();
+		break;
 	}

Lets leave this handling out right now. I know that bluetoothd historically supported it, but that is historic baggage. Neither oFono nor ConnMan does it that way. So just do no bother with this. If it becomes useful, we will add it.

Quoted 32 lines
android/main.c +21 -0
 }
 
+static char *option_debug = NULL;
 static gboolean option_detach = TRUE;
 static gboolean option_version = FALSE;
 
+static void free_options(void)
+{
+	g_free(option_debug);
+	option_debug = NULL;
+}
+
+static gboolean parse_debug(const char *key, const char *value,
+				gpointer user_data, GError **error)
+{
+	if (value)
+		option_debug = g_strdup(value);
+	else
+		option_debug = g_strdup("*");
+
+	return TRUE;
+}
+
 static GOptionEntry options[] = {
+	{ "debug", 'd', G_OPTION_FLAG_OPTIONAL_ARG,
+				G_OPTION_ARG_CALLBACK, parse_debug,
+				"Specify debug options to enable", "DEBUG" },
 	{ "nodetach", 'n', G_OPTION_FLAG_REVERSE,
 				G_OPTION_ARG_NONE, &option_detach,
 				"Run with logging in foreground", NULL },

@@ -110,10 +137,22 @@ int main(int argc, char *argv[])
 	sa.sa_handler = signal_handler;

android/main.c int main(int argc, char *argv[]) · line 147

Quoted 10 lines
android/main.c +8 -0
 	sigaction(SIGINT, &sa, NULL);
 	sigaction(SIGTERM, &sa, NULL);
+	sigaction(SIGUSR2, &sa, NULL);
+
+	__btd_log_init(option_debug, option_detach);
+
+	/* no need to keep parsed option in memory */
+	free_options();
+
+	DBG("Entering main loop");

Don't do this debug. It is not helpful. The main() is so short, no point in logging that.

android/main.c int main(int argc, char *argv[]) · line 158

Quoted 11 lines
android/main.c +4 -0
 	g_main_loop_run(event_loop);
 
 	g_main_loop_unref(event_loop);
 
+	info("Exit");
+
+	__btd_log_cleanup();
+
 	return 0;
 }

Regards

Marcel

#3 lore raw

Re: [PATCH 2/2] android: Android version of log.c

Hi all,

Quoted 10 lines · Marcel Holtmann

Hi Fred,

Quoted 9 lines
android/log.c new file +9 -0
+
+static void android_log(int pri, const char *fmt, va_list ap)
+{
+	char *msg;
+	struct iovec vec[3];
+
+	msg = g_strdup_vprintf(fmt, ap);
+
+	if (!detached) {
Quoted 8 lines

android/log.c line 60

Quoted 6 lines
android/log.c new file +6 -0
+		vec[0].iov_base = (void *) msg;
+		vec[0].iov_len = strlen(msg) + 1;
+		vec[1].iov_base = "\n";
+		vec[1].iov_len = 1;
+		writev(STDERR_FILENO, vec, 2);
+	}

Lets not worry about logging to stderr at all.

Then I believe this might be simplified a lot. We might use __android_log_print - like functions in this case.

Best regards
Andrei Emeltchenko

author
#4 lore raw

Re: [PATCH 2/2] android: Android version of log.c

In reply to Marcel Holtmann

Hello Marcel,

On 21/09/2013 19:20, Marcel Holtmann wrote:
<snip>

Quoted 6 lines
Quoted 6 lines
android/main.c +2 -0
@@ -110,10 +137,22 @@ int main(int argc, char *argv[])
 	sa.sa_handler = signal_handler;
 	sigaction(SIGINT, &sa, NULL);
 	sigaction(SIGTERM, &sa, NULL);
+	sigaction(SIGUSR2, &sa, NULL);
+
Quoted 8 lines

android/main.c int main(int argc, char *argv[]) · line 147

Quoted 6 lines
android/main.c +6 -0
+	__btd_log_init(option_debug, option_detach);
+
+	/* no need to keep parsed option in memory */
+	free_options();
+
+	DBG("Entering main loop");

Don't do this debug. It is not helpful. The main() is so short, no point in logging that.

I need at least one DBG call to be able to build, otherwise I got "undefined reference to `__start___debug'" error messages. So, this one seems ok to me.

Regards

Fred

Signature · 2 lines
Frederic Danis                            Open Source Technology Center
[email protected]                              Intel Corporation
maintainer
#5 lore raw

Re: [PATCH 2/2] android: Android version of log.c

Hi Fred,

Quoted 16 lines

<snip>

Quoted 6 lines
Quoted 6 lines
android/main.c +2 -0
@@ -110,10 +137,22 @@ int main(int argc, char *argv[])
 	sa.sa_handler = signal_handler;
 	sigaction(SIGINT, &sa, NULL);
 	sigaction(SIGTERM, &sa, NULL);
+	sigaction(SIGUSR2, &sa, NULL);
+
Quoted 8 lines

android/main.c int main(int argc, char *argv[]) · line 147

Quoted 6 lines
android/main.c +6 -0
+	__btd_log_init(option_debug, option_detach);
+
+	/* no need to keep parsed option in memory */
+	free_options();
+
+	DBG("Entering main loop");

Don't do this debug. It is not helpful. The main() is so short, no point in logging that.

Quoted 2 lines

I need at least one DBG call to be able to build, otherwise I got "undefined reference to `__start___debug'" error messages. So, this one seems ok to me.

fair enough for the beginning. Just remove it once you have more than one DBG in the source code and this is no longer needed.

Regards

Marcel

maintainer
#6 lore raw
In reply to Frederic Danis

Hi Fred,

Quoted 13 lines

Define local mapping to glib path, otherwise this has to be inside central place in the build repository.

Retrieve Bluetooth version from configure.ac.
---

.gitignore         |    2 +
Android.mk         |    9 ++++
Makefile.am        |    1 +
Makefile.android   |    7 ++++
android/Android.mk |   24 +++++++++++
android/main.c     |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac       |    5 +++
7 files changed, 167 insertions(+)
Quoted 4 lines

create mode 100644 Android.mk
create mode 100644 Makefile.android
create mode 100644 android/Android.mk
create mode 100644 android/main.c

lets split this out a little bit. Code additions should not be intermixed with additions to the build system and especially configure options.

I rather not have a top-level Android.mk. It should be enough to provide an android/Android.mk.

.gitignore unit/test-gobex-packet · line 102

Quoted 10 lines
.gitignore +0 -0
index 8a25a3e..331a18b 100644
@@ -98,3 +98,5 @@ unit/test-gobex-packet

unit/test-gobex-transfer
unit/test-*.log
unit/test-*.trs

.gitignore +2 -0
+
+android/bluezd

If we keep using prefix btd_ for certain set of functions, then it might be better if the daemon is build as android/bluetoothd or android/btd.

<snip>

Quoted 4 lines
.gitignore +4 -0
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif

.gitignore

Quoted 10 lines
.gitignore +10 -0
+
+#include <signal.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <glib.h>
+
+#include "hcid.h"

I do not think we need to include hcid.h here. If we for some reason still do, then please try to cleanup that first.

Quoted 9 lines
.gitignore +9 -0
+
+#define SHUTDOWN_GRACE_SECONDS 10
+
+static GMainLoop *event_loop;
+
+void btd_exit(void)
+{
+	g_main_loop_quit(event_loop);
+}

.gitignore

Quoted 10 lines
.gitignore +10 -0
+
+static gboolean quit_eventloop(gpointer user_data)
+{
+	btd_exit();
+	return FALSE;
+}
+
+static void signal_handler(int sig)
+{
+	static unsigned int __terminated = 0;

Make this static volatile bool __terminated = false instead. Remember that you are actually using signals here and not signalfd.

Quoted 11 lines
.gitignore +11 -0
+
+	switch (sig) {
+	case SIGINT:
+	case SIGTERM:
+		if (__terminated == 0) {
+			g_timeout_add_seconds(SHUTDOWN_GRACE_SECONDS,
+							quit_eventloop, NULL);
+		}
+
+		__terminated = 1;
+		break;

.gitignore

Quoted 10 lines
.gitignore +10 -0
+	}
+}
+
+static gboolean option_detach = TRUE;
+static gboolean option_version = FALSE;
+
+static GOptionEntry options[] = {
+	{ "nodetach", 'n', G_OPTION_FLAG_REVERSE,
+				G_OPTION_ARG_NONE, &option_detach,
+				"Run with logging in foreground", NULL },

Do we need this option at all. I think we should always run in foreground and the system manager need to make sure we are spawned.

Quoted 18 lines
.gitignore +18 -0
+	{ "version", 'v', 0, G_OPTION_ARG_NONE, &option_version,
+				"Show version information and exit", NULL },
+	{ NULL }
+};
+
+int main(int argc, char *argv[])
+{
+	GOptionContext *context;
+	GError *err = NULL;
+	struct sigaction sa;
+
+	context = g_option_context_new(NULL);
+	g_option_context_add_main_entries(context, options, NULL);
+
+	if (g_option_context_parse(context, &argc, &argv, &err) == FALSE) {
+		if (err != NULL) {
+			g_printerr("%s\n", err->message);
+			g_error_free(err);

.gitignore

Quoted 10 lines
.gitignore +10 -0
+		} else
+			g_printerr("An unknown error occurred\n");
+		exit(1);
+	}
+
+	g_option_context_free(context);
+
+	if (option_version == TRUE) {
+		printf("%s\n", VERSION);
+		exit(0);

I prefer if we start using return EXIT_SUCCESS and in error case EXIT_FAILURE.

Quoted 20 lines
.gitignore +15 -0
+	}
+
+	event_loop = g_main_loop_new(NULL, FALSE);
+
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_handler = signal_handler;
+	sigaction(SIGINT, &sa, NULL);
+	sigaction(SIGTERM, &sa, NULL);
+
+	g_main_loop_run(event_loop);
+
+	g_main_loop_unref(event_loop);
+
+	return 0;
+}
configure.ac +0 -0
index 41c2935..3b7a5d9 100644

configure.ac AC_DEFINE_UNQUOTED(CONFIGDIR, "${configdir}", · line 249

Quoted 8 lines
configure.ac +5 -0
@@ -242,4 +242,9 @@ AC_DEFINE_UNQUOTED(CONFIGDIR, "${configdir}",
 			[Directory for the configuration files])
 AC_SUBST(CONFIGDIR, "${configdir}")
 
+AC_ARG_ENABLE(android-daemon, AC_HELP_STRING([--enable-android-daemon],
+			[enable BlueZ Android daemon]),
+					[android_daemon=${enableval}])
+AM_CONDITIONAL(ANDROID_DAEMON, test "${android_daemon}" = "yes")
+

This needs to be a separate patch. And it should just say --enable-android.

In addition it needs to be added to bootstrap-configure and distcheck handling.

Regards

Marcel

#7 lore raw

Hi Marcel,

Quoted 18 lines · Marcel Holtmann
Quoted 13 lines

Define local mapping to glib path, otherwise this has to be inside central place in the build repository.

Retrieve Bluetooth version from configure.ac.
---

.gitignore         |    2 +
Android.mk         |    9 ++++
Makefile.am        |    1 +
Makefile.android   |    7 ++++
android/Android.mk |   24 +++++++++++
android/main.c     |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac       |    5 +++
7 files changed, 167 insertions(+)
Quoted 4 lines

create mode 100644 Android.mk
create mode 100644 Makefile.android
create mode 100644 android/Android.mk
create mode 100644 android/main.c

Quoted 3 lines

lets split this out a little bit. Code additions should not be intermixed with additions to the build system and especially configure options.

I agree about keeping separate build system changes such as adding completely new makefiles or adding configure options. However, what I think doesn't hurt (and the guidance I've given) is to at least include the matching build system change to the same commit that adds new c/h files. This way you don't end up having "dead code" in the tree in between commits that just lies around there but can't be compiled and wont be included in any release tarballs.

Johan

#8 lore raw
In reply to Marcel Holtmann
Quoted 20 lines · Marcel Holtmann

Hi Fred,

Quoted 13 lines

Define local mapping to glib path, otherwise this has to be inside central place in the build repository.

Retrieve Bluetooth version from configure.ac.
---

.gitignore         |    2 +
Android.mk         |    9 ++++
Makefile.am        |    1 +
Makefile.android   |    7 ++++
android/Android.mk |   24 +++++++++++
android/main.c     |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac       |    5 +++
7 files changed, 167 insertions(+)
Quoted 4 lines

create mode 100644 Android.mk
create mode 100644 Makefile.android
create mode 100644 android/Android.mk
create mode 100644 android/main.c

Quoted 4 lines

lets split this out a little bit. Code additions should not be intermixed with additions to the build system and especially configure options.

I rather not have a top-level Android.mk. It should be enough to provide an android/Android.mk.

I believe this way we cannot build our project.

Best regards
Andrei Emeltchenko

author
#9 lore raw
In reply to Marcel Holtmann

Hello Marcel,

Quoted 24 lines · Marcel Holtmann

Hi Fred,

Quoted 13 lines

Define local mapping to glib path, otherwise this has to be inside central place in the build repository.

Retrieve Bluetooth version from configure.ac.
---

.gitignore         |    2 +
Android.mk         |    9 ++++
Makefile.am        |    1 +
Makefile.android   |    7 ++++
android/Android.mk |   24 +++++++++++
android/main.c     |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac       |    5 +++
7 files changed, 167 insertions(+)
Quoted 4 lines

create mode 100644 Android.mk
create mode 100644 Makefile.android
create mode 100644 android/Android.mk
create mode 100644 android/main.c

lets split this out a little bit. Code additions should not be intermixed with additions to the build system and especially configure options.

I rather not have a top-level Android.mk. It should be enough to provide an android/Android.mk.

Quoted 12 lines

.gitignore unit/test-gobex-packet · line 102

Quoted 10 lines
.gitignore +0 -0
index 8a25a3e..331a18b 100644
@@ -98,3 +98,5 @@ unit/test-gobex-packet

unit/test-gobex-transfer
unit/test-*.log
unit/test-*.trs

.gitignore +2 -0
+
+android/bluezd

If we keep using prefix btd_ for certain set of functions, then it might be better if the daemon is build as android/bluetoothd or android/btd.

OK, I will rename it android/bluetoothd

Quoted 19 lines

<snip>

Quoted 4 lines
.gitignore +4 -0
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif

.gitignore

Quoted 10 lines
.gitignore +10 -0
+
+#include <signal.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <glib.h>
+
+#include "hcid.h"

I do not think we need to include hcid.h here. If we for some reason still do, then please try to cleanup that first.

OK, I will remove it

Quoted 14 lines
Quoted 14 lines
.gitignore +13 -0
+
+#define SHUTDOWN_GRACE_SECONDS 10
+
+static GMainLoop *event_loop;
+
+void btd_exit(void)
+{
+	g_main_loop_quit(event_loop);
+}
+
+static gboolean quit_eventloop(gpointer user_data)
+{
+	btd_exit();
Quoted 8 lines

.gitignore

Quoted 6 lines
.gitignore +6 -0
+	return FALSE;
+}
+
+static void signal_handler(int sig)
+{
+	static unsigned int __terminated = 0;

Make this static volatile bool __terminated = false instead. Remember that you are actually using signals here and not signalfd.

OK, I will do this.

Quoted 16 lines
Quoted 16 lines
.gitignore +15 -0
+
+	switch (sig) {
+	case SIGINT:
+	case SIGTERM:
+		if (__terminated == 0) {
+			g_timeout_add_seconds(SHUTDOWN_GRACE_SECONDS,
+							quit_eventloop, NULL);
+		}
+
+		__terminated = 1;
+		break;
+	}
+}
+
+static gboolean option_detach = TRUE;
Quoted 8 lines

.gitignore

Quoted 6 lines
.gitignore +6 -0
+static gboolean option_version = FALSE;
+
+static GOptionEntry options[] = {
+	{ "nodetach", 'n', G_OPTION_FLAG_REVERSE,
+				G_OPTION_ARG_NONE, &option_detach,
+				"Run with logging in foreground", NULL },

Do we need this option at all. I think we should always run in foreground and the system manager need to make sure we are spawned.

I think this is useful when debugging in conjonction with debug option and logging to console.

Quoted 23 lines
Quoted 23 lines
.gitignore +22 -0
+	{ "version", 'v', 0, G_OPTION_ARG_NONE, &option_version,
+				"Show version information and exit", NULL },
+	{ NULL }
+};
+
+int main(int argc, char *argv[])
+{
+	GOptionContext *context;
+	GError *err = NULL;
+	struct sigaction sa;
+
+	context = g_option_context_new(NULL);
+	g_option_context_add_main_entries(context, options, NULL);
+
+	if (g_option_context_parse(context, &argc, &argv, &err) == FALSE) {
+		if (err != NULL) {
+			g_printerr("%s\n", err->message);
+			g_error_free(err);
+		} else
+			g_printerr("An unknown error occurred\n");
+		exit(1);
+	}
Quoted 8 lines

.gitignore

Quoted 6 lines
.gitignore +6 -0
+
+	g_option_context_free(context);
+
+	if (option_version == TRUE) {
+		printf("%s\n", VERSION);
+		exit(0);

I prefer if we start using return EXIT_SUCCESS and in error case EXIT_FAILURE.

OK, I will do this.

Quoted 32 lines
Quoted 21 lines
.gitignore +15 -0
+	}
+
+	event_loop = g_main_loop_new(NULL, FALSE);
+
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_handler = signal_handler;
+	sigaction(SIGINT, &sa, NULL);
+	sigaction(SIGTERM, &sa, NULL);
+
+	g_main_loop_run(event_loop);
+
+	g_main_loop_unref(event_loop);
+
+	return 0;
+}
configure.ac +0 -0
index 41c2935..3b7a5d9 100644

configure.ac AC_DEFINE_UNQUOTED(CONFIGDIR, "${configdir}", · line 249

Quoted 8 lines
configure.ac +5 -0
@@ -242,4 +242,9 @@ AC_DEFINE_UNQUOTED(CONFIGDIR, "${configdir}",
 			[Directory for the configuration files])
 AC_SUBST(CONFIGDIR, "${configdir}")
 
+AC_ARG_ENABLE(android-daemon, AC_HELP_STRING([--enable-android-daemon],
+			[enable BlueZ Android daemon]),
+					[android_daemon=${enableval}])
+AM_CONDITIONAL(ANDROID_DAEMON, test "${android_daemon}" = "yes")
+

This needs to be a separate patch. And it should just say --enable-android.

Quoted 1 line

In addition it needs to be added to bootstrap-configure and distcheck handling.

OK

Regards

Fred

Signature · 2 lines
Frederic Danis                            Open Source Technology Center
[email protected]                              Intel Corporation
#10 lore raw

Hi Marcel

Quoted 34 lines · Frederic Danis

Hello Marcel,

Quoted 30 lines · Marcel Holtmann

Hi Fred,

Define local mapping to glib path, otherwise this has to be inside

Quoted 14 lines

central
place in the build repository.

Retrieve Bluetooth version from configure.ac.
---

.gitignore         |    2 +
Android.mk         |    9 ++++
Makefile.am        |    1 +
Makefile.android   |    7 ++++
android/Android.mk |   24 +++++++++++
android/main.c     |  119 ++++++++++++++++++++++++++++++**
++++++++++++++++++++++
configure.ac       |    5 +++
7 files changed, 167 insertions(+)
Quoted 5 lines

create mode 100644 Android.mk create mode 100644 Makefile.android create mode 100644 android/Android.mk create mode 100644 android/main.c

lets split this out a little bit. Code additions should not be intermixed with additions to the build system and especially configure options.

I rather not have a top-level Android.mk. It should be enough to provide an android/Android.mk.

BTW, do you expect to have only one Android.mk for the whole project or

this is OK for you to have smaller Android.mk in different subfolders like /src/shared etc. Both ways are valid.

BR
Lukasz

#11 lore raw
In reply to Frederic Danis

Hi Marcel,

Quoted 32 lines · Frederic Danis

Hello Marcel,

Quoted 29 lines · Marcel Holtmann

Hi Fred,

Quoted 15 lines

Define local mapping to glib path, otherwise this has to be inside central
place in the build repository.

Retrieve Bluetooth version from configure.ac.
---

.gitignore         |    2 +
Android.mk         |    9 ++++
Makefile.am        |    1 +
Makefile.android   |    7 ++++
android/Android.mk |   24 +++++++++++
android/main.c     |  119
++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac       |    5 +++
7 files changed, 167 insertions(+)
Quoted 4 lines

create mode 100644 Android.mk create mode 100644 Makefile.android create mode 100644 android/Android.mk create mode 100644 android/main.c

lets split this out a little bit. Code additions should not be intermixed with additions to the build system and especially configure options.

I rather not have a top-level Android.mk. It should be enough to provide an android/Android.mk.

BTW, do you expect to have only one Android.mk for the whole project or
this is OK for you to have smaller Android.mk in different subfolders like /src/shared etc. Both ways are valid.

Br
Lukasz

#12 lore raw

Hi All,

Quoted 34 lines · Lukasz Rymanowski

Hi Marcel,

Quoted 30 lines · Frederic Danis

Hello Marcel,

Quoted 27 lines · Marcel Holtmann

Hi Fred,

Quoted 15 lines

Define local mapping to glib path, otherwise this has to be inside central
place in the build repository.

Retrieve Bluetooth version from configure.ac.
---

.gitignore         |    2 +
Android.mk         |    9 ++++
Makefile.am        |    1 +
Makefile.android   |    7 ++++
android/Android.mk |   24 +++++++++++
android/main.c     |  119
++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac       |    5 +++
7 files changed, 167 insertions(+)
Quoted 4 lines

create mode 100644 Android.mk create mode 100644 Makefile.android create mode 100644 android/Android.mk create mode 100644 android/main.c

lets split this out a little bit. Code additions should not be intermixed with additions to the build system and especially configure options.

Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

I rather not have a top-level Android.mk. It should be enough to provide an android/Android.mk.

BTW, do you expect to have only one Android.mk for the whole project or
this is OK for you to have smaller Android.mk in different subfolders like /src/shared etc. Both ways are valid.

While having a single huge Android makefile is somehow make sense it would have several disadvantages:
- we cannot use local build with "mm" for building for example only tools or libs
- Android.mk will become very large and difficult to read

Best regards
Andrei Emeltchenko

maintainer
#13 lore raw

Hi Andrei,

Quoted 31 lines
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Quoted 15 lines

Define local mapping to glib path, otherwise this has to be inside central
place in the build repository.

Retrieve Bluetooth version from configure.ac.
---

.gitignore         |    2 +
Android.mk         |    9 ++++
Makefile.am        |    1 +
Makefile.android   |    7 ++++
android/Android.mk |   24 +++++++++++
android/main.c     |  119
++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac       |    5 +++
7 files changed, 167 insertions(+)
Quoted 4 lines

create mode 100644 Android.mk create mode 100644 Makefile.android create mode 100644 android/Android.mk create mode 100644 android/main.c

lets split this out a little bit. Code additions should not be intermixed with additions to the build system and especially configure options.

Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

I rather not have a top-level Android.mk. It should be enough to provide an android/Android.mk.

BTW, do you expect to have only one Android.mk for the whole project or this is OK for you to have smaller Android.mk in different subfolders like /src/shared etc. Both ways are valid.

Quoted 5 lines

While having a single huge Android makefile is somehow make sense it would have several disadvantages:
- we cannot use local build with "mm" for building for example only tools or libs
- Android.mk will become very large and difficult to read

I have no idea what this means. Lets start with a single android/Android.mk and then go from there. I am strictly against cluttering the whole project with Android specific make files.

Regards

Marcel

#14 lore raw

Hi Marcel,

Quoted 39 lines · Marcel Holtmann

Hi Andrei,

Quoted 31 lines
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Quoted 15 lines

Define local mapping to glib path, otherwise this has to be inside central
place in the build repository.

Retrieve Bluetooth version from configure.ac.
---

.gitignore         |    2 +
Android.mk         |    9 ++++
Makefile.am        |    1 +
Makefile.android   |    7 ++++
android/Android.mk |   24 +++++++++++
android/main.c     |  119
++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac       |    5 +++
7 files changed, 167 insertions(+)
Quoted 4 lines

create mode 100644 Android.mk create mode 100644 Makefile.android create mode 100644 android/Android.mk create mode 100644 android/main.c

lets split this out a little bit. Code additions should not be intermixed with additions to the build system and especially configure options.

Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

I rather not have a top-level Android.mk. It should be enough to provide an android/Android.mk.

BTW, do you expect to have only one Android.mk for the whole project or this is OK for you to have smaller Android.mk in different subfolders like /src/shared etc. Both ways are valid.

Quoted 5 lines

While having a single huge Android makefile is somehow make sense it would have several disadvantages:
- we cannot use local build with "mm" for building for example only tools or libs
- Android.mk will become very large and difficult to read

Quoted 3 lines

I have no idea what this means. Lets start with a single android/Android.mk and then go from there. I am strictly against cluttering the whole project with Android specific make files.

OK, we will use the single Android makefile, it is actually not that big.

Best regards
Andrei Emeltchenko