[PATCH 0/7] RFC: proximity cleanup

[PATCH 0/7] RFC: proximity cleanup

[Andrzej Kaczmarek]

Hi,

Here are few patches to cleanup proximity profile code a bit which seem to
keep track of some data which are redundant and has minor issue with device
driver registration.

Most cleanup is done in patch #3 which removes tracking of DBusConnection
across whole plugin and simply uses get_dbus_connection() wherever necessary.
This is different than other profile plugins do, but reduces code size and
memory footprint a bit so can be done for other plugins as well if accepted.


Andrzej Kaczmarek (7):
  proximity: Change adapter driver function names
  proximity: Remove unused definitions
  proximity: Simplify DBusConnection object usage
  proximity: Move reporter device driver registration to manager
  proximity: Remove unnecessary check for GATT enabled
  proximity: Remove list of adapter devices
  proximity: Remove reporter_adapter

 profiles/proximity/immalert.c |   9 +---
 profiles/proximity/immalert.h |   2 +-
 profiles/proximity/linkloss.c |  10 +---
 profiles/proximity/linkloss.h |   2 +-
 profiles/proximity/main.c     |  10 +---
 profiles/proximity/manager.c  |  38 +++++++++-----
 profiles/proximity/manager.h  |   2 +-
 profiles/proximity/monitor.c  |  20 ++++----
 profiles/proximity/monitor.h  |   8 +--
 profiles/proximity/reporter.c | 115 +++++-------------------------------------
 profiles/proximity/reporter.h |   7 ++-
 11 files changed, 62 insertions(+), 161 deletions(-)

-- 
1.7.11.3

[PATCH 1/7] proximity: Change adapter driver function names

[Andrzej Kaczmarek]

probe and remove suffixes better describe what these functions actually do.
---
 profiles/proximity/manager.c  | 4 ++--
 profiles/proximity/reporter.c | 4 ++--
 profiles/proximity/reporter.h | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/profiles/proximity/manager.c b/profiles/proximity/manager.c
index f2e49a6..e382be1 100644
--- a/profiles/proximity/manager.c
+++ b/profiles/proximity/manager.c
@@ -90,8 +90,8 @@ static struct btd_device_driver monitor_driver = {
 
 static struct btd_adapter_driver reporter_server_driver = {
 	.name = "Proximity GATT Reporter Driver",
-	.probe = reporter_init,
-	.remove = reporter_exit,
+	.probe = reporter_adapter_probe,
+	.remove = reporter_adapter_remove,
 };
 
 static void load_config_file(GKeyFile *config)
diff --git a/profiles/proximity/reporter.c b/profiles/proximity/reporter.c
index 607de2b..a3a8c8a 100644
--- a/profiles/proximity/reporter.c
+++ b/profiles/proximity/reporter.c
@@ -256,7 +256,7 @@ static struct btd_device_driver reporter_device_driver = {
 	.remove = reporter_device_remove,
 };
 
-int reporter_init(struct btd_adapter *adapter)
+int reporter_adapter_probe(struct btd_adapter *adapter)
 {
 	struct reporter_adapter *radapter;
 	DBusConnection *conn;
@@ -286,7 +286,7 @@ int reporter_init(struct btd_adapter *adapter)
 	return 0;
 }
 
-void reporter_exit(struct btd_adapter *adapter)
+void reporter_adapter_remove(struct btd_adapter *adapter)
 {
 	struct reporter_adapter *radapter = find_reporter_adapter(adapter);
 	if (!radapter)
diff --git a/profiles/proximity/reporter.h b/profiles/proximity/reporter.h
index 5ae0eb2..ae5ddf6 100644
--- a/profiles/proximity/reporter.h
+++ b/profiles/proximity/reporter.h
@@ -36,7 +36,7 @@ enum {
 	HIGH_ALERT = 0x02,
 };
 
-int reporter_init(struct btd_adapter *adapter);
-void reporter_exit(struct btd_adapter *adapter);
+int reporter_adapter_probe(struct btd_adapter *adapter);
+void reporter_adapter_remove(struct btd_adapter *adapter);
 
 const char *get_alert_level_string(uint8_t level);
-- 
1.7.11.3

[PATCH 2/7] proximity: Remove unused definitions

[Andrzej Kaczmarek]

---
 profiles/proximity/linkloss.c | 2 --
 profiles/proximity/reporter.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/profiles/proximity/linkloss.c b/profiles/proximity/linkloss.c
index 14403cb..6ed568c 100644
--- a/profiles/proximity/linkloss.c
+++ b/profiles/proximity/linkloss.c
@@ -44,8 +44,6 @@
 #include "reporter.h"
 #include "linkloss.h"
 
-#define BLUEZ_SERVICE "org.bluez"
-
 struct link_loss_adapter {
 	struct btd_adapter *adapter;
 	uint16_t alert_lvl_value_handle;
diff --git a/profiles/proximity/reporter.c b/profiles/proximity/reporter.c
index a3a8c8a..de3770a 100644
--- a/profiles/proximity/reporter.c
+++ b/profiles/proximity/reporter.c
@@ -49,8 +49,6 @@
 #include "linkloss.h"
 #include "immalert.h"
 
-#define BLUEZ_SERVICE "org.bluez"
-
 struct reporter_adapter {
 	DBusConnection *conn;
 	struct btd_adapter *adapter;
-- 
1.7.11.3

[PATCH 3/7] proximity: Simplify DBusConnection object usage

[Andrzej Kaczmarek]

DBusConnection object can be obtained at any time by get_dbus_connection() call
so there is no need to pass and store it across profile code. Also there's no
need to ref/unref this object as by design it will live for entire lifetime of
plugin.
---
 profiles/proximity/immalert.c |  9 ++-------
 profiles/proximity/immalert.h |  2 +-
 profiles/proximity/linkloss.c |  8 ++------
 profiles/proximity/linkloss.h |  2 +-
 profiles/proximity/main.c     | 10 +---------
 profiles/proximity/manager.c  | 13 +++----------
 profiles/proximity/manager.h  |  2 +-
 profiles/proximity/monitor.c  | 20 +++++++++-----------
 profiles/proximity/monitor.h  |  8 ++++----
 profiles/proximity/reporter.c | 16 ++++------------
 10 files changed, 28 insertions(+), 62 deletions(-)

diff --git a/profiles/proximity/immalert.c b/profiles/proximity/immalert.c
index 1540b61..f4061bc 100644
--- a/profiles/proximity/immalert.c
+++ b/profiles/proximity/immalert.c
@@ -46,7 +46,6 @@
 
 struct imm_alert_adapter {
 	struct btd_adapter *adapter;
-	DBusConnection *conn;
 	GSList *connected_devices;
 };
 
@@ -125,19 +124,17 @@ const char *imm_alert_get_level(struct btd_device *device)
 static void imm_alert_emit_alert_signal(struct connected_device *condev,
 							uint8_t alert_level)
 {
-	struct imm_alert_adapter *adapter;
 	const char *path, *alert_level_str;
 
 	if (!condev)
 		return;
 
-	adapter = condev->adapter;
 	path = device_get_path(condev->device);
 	alert_level_str = get_alert_level_string(alert_level);
 
 	DBG("alert %s remote %s", alert_level_str, path);
 
-	emit_property_changed(adapter->conn, path,
+	emit_property_changed(get_dbus_connection(), path,
 			PROXIMITY_REPORTER_INTERFACE, "ImmediateAlertLevel",
 			DBUS_TYPE_STRING, &alert_level_str);
 }
@@ -233,7 +230,7 @@ set_error:
 	return ATT_ECODE_IO;
 }
 
-void imm_alert_register(struct btd_adapter *adapter, DBusConnection *conn)
+void imm_alert_register(struct btd_adapter *adapter)
 {
 	gboolean svc_added;
 	bt_uuid_t uuid;
@@ -243,7 +240,6 @@ void imm_alert_register(struct btd_adapter *adapter, DBusConnection *conn)
 
 	imadapter = g_new0(struct imm_alert_adapter, 1);
 	imadapter->adapter = adapter;
-	imadapter->conn = dbus_connection_ref(conn);
 
 	imm_alert_adapters = g_slist_append(imm_alert_adapters, imadapter);
 
@@ -283,7 +279,6 @@ void imm_alert_unregister(struct btd_adapter *adapter)
 
 	g_slist_foreach(imadapter->connected_devices, remove_condev_list_item,
 									NULL);
-	dbus_connection_unref(imadapter->conn);
 
 	imm_alert_adapters = g_slist_remove(imm_alert_adapters, imadapter);
 	g_free(imadapter);
diff --git a/profiles/proximity/immalert.h b/profiles/proximity/immalert.h
index dd28eaa..1a09fa9 100644
--- a/profiles/proximity/immalert.h
+++ b/profiles/proximity/immalert.h
@@ -21,6 +21,6 @@
  *
  */
 
-void imm_alert_register(struct btd_adapter *adapter, DBusConnection *conn);
+void imm_alert_register(struct btd_adapter *adapter);
 void imm_alert_unregister(struct btd_adapter *adapter);
 const char *imm_alert_get_level(struct btd_device *device);
diff --git a/profiles/proximity/linkloss.c b/profiles/proximity/linkloss.c
index 6ed568c..b6d01a6 100644
--- a/profiles/proximity/linkloss.c
+++ b/profiles/proximity/linkloss.c
@@ -47,7 +47,6 @@
 struct link_loss_adapter {
 	struct btd_adapter *adapter;
 	uint16_t alert_lvl_value_handle;
-	DBusConnection *conn;
 	GSList *connected_devices;
 };
 
@@ -126,7 +125,6 @@ const char *link_loss_get_alert_level(struct btd_device *device)
 
 static void link_loss_emit_alert_signal(struct connected_device *condev)
 {
-	struct link_loss_adapter *adapter = condev->adapter;
 	const char *alert_level_str, *path;
 
 	if (!condev->device)
@@ -137,7 +135,7 @@ static void link_loss_emit_alert_signal(struct connected_device *condev)
 
 	DBG("alert %s remote %s", alert_level_str, path);
 
-	emit_property_changed(adapter->conn, path,
+	emit_property_changed(get_dbus_connection(), path,
 			PROXIMITY_REPORTER_INTERFACE, "LinkLossAlertLevel",
 			DBUS_TYPE_STRING, &alert_level_str);
 }
@@ -273,7 +271,7 @@ set_error:
 	return ATT_ECODE_IO;
 }
 
-void link_loss_register(struct btd_adapter *adapter, DBusConnection *conn)
+void link_loss_register(struct btd_adapter *adapter)
 {
 	gboolean svc_added;
 	bt_uuid_t uuid;
@@ -283,7 +281,6 @@ void link_loss_register(struct btd_adapter *adapter, DBusConnection *conn)
 
 	lladapter = g_new0(struct link_loss_adapter, 1);
 	lladapter->adapter = adapter;
-	lladapter->conn = dbus_connection_ref(conn);
 
 	link_loss_adapters = g_slist_append(link_loss_adapters, lladapter);
 
@@ -329,7 +326,6 @@ void link_loss_unregister(struct btd_adapter *adapter)
 
 	g_slist_foreach(lladapter->connected_devices, remove_condev_list_item,
 			NULL);
-	dbus_connection_unref(lladapter->conn);
 
 	link_loss_adapters = g_slist_remove(link_loss_adapters, lladapter);
 	g_free(lladapter);
diff --git a/profiles/proximity/linkloss.h b/profiles/proximity/linkloss.h
index a7d83d0..0447def 100644
--- a/profiles/proximity/linkloss.h
+++ b/profiles/proximity/linkloss.h
@@ -21,6 +21,6 @@
  *
  */
 
-void link_loss_register(struct btd_adapter *adapter, DBusConnection *conn);
+void link_loss_register(struct btd_adapter *adapter);
 void link_loss_unregister(struct btd_adapter *adapter);
 const char *link_loss_get_alert_level(struct btd_device *device);
diff --git a/profiles/proximity/main.c b/profiles/proximity/main.c
index 3d5d9b2..0f57511 100644
--- a/profiles/proximity/main.c
+++ b/profiles/proximity/main.c
@@ -36,7 +36,6 @@
 #include "manager.h"
 #include "hcid.h"
 
-static DBusConnection *connection = NULL;
 static GKeyFile *config = NULL;
 
 static GKeyFile *open_config_file(const char *file)
@@ -65,16 +64,10 @@ static int proximity_init(void)
 		return -ENOTSUP;
 	}
 
-	connection = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
-	if (connection == NULL)
-		return -EIO;
-
 	config = open_config_file(CONFIGDIR "/proximity.conf");
 
-	if (proximity_manager_init(connection, config) < 0) {
-		dbus_connection_unref(connection);
+	if (proximity_manager_init(config) < 0)
 		return -EIO;
-	}
 
 	return 0;
 }
@@ -88,7 +81,6 @@ static void proximity_exit(void)
 		g_key_file_free(config);
 
 	proximity_manager_exit();
-	dbus_connection_unref(connection);
 }
 
 BLUETOOTH_PLUGIN_DEFINE(proximity, VERSION,
diff --git a/profiles/proximity/manager.c b/profiles/proximity/manager.c
index e382be1..97e9795 100644
--- a/profiles/proximity/manager.c
+++ b/profiles/proximity/manager.c
@@ -39,8 +39,6 @@
 #include "reporter.h"
 #include "manager.h"
 
-static DBusConnection *connection = NULL;
-
 static struct enabled enabled  = {
 	.linkloss = TRUE,
 	.pathloss = TRUE,
@@ -72,13 +70,12 @@ static int attio_device_probe(struct btd_device *device, GSList *uuids)
 	l = g_slist_find_custom(primaries, LINK_LOSS_UUID, primary_uuid_cmp);
 	linkloss = (l ? l->data : NULL);
 
-	return monitor_register(connection, device, linkloss, txpower,
-							immediate, &enabled);
+	return monitor_register(device, linkloss, txpower, immediate, &enabled);
 }
 
 static void attio_device_remove(struct btd_device *device)
 {
-	monitor_unregister(connection, device);
+	monitor_unregister(device);
 }
 
 static struct btd_device_driver monitor_driver = {
@@ -116,14 +113,12 @@ static void load_config_file(GKeyFile *config)
 	g_strfreev(list);
 }
 
-int proximity_manager_init(DBusConnection *conn, GKeyFile *config)
+int proximity_manager_init(GKeyFile *config)
 {
 	int ret;
 
 	load_config_file(config);
 
-	connection = dbus_connection_ref(conn);
-
 	ret = btd_register_device_driver(&monitor_driver);
 	if (ret < 0)
 		goto fail_monitor;
@@ -138,7 +133,6 @@ fail_reporter:
 	btd_unregister_device_driver(&monitor_driver);
 
 fail_monitor:
-	dbus_connection_unref(connection);
 	return ret;
 }
 
@@ -146,5 +140,4 @@ void proximity_manager_exit(void)
 {
 	btd_unregister_device_driver(&monitor_driver);
 	btd_unregister_adapter_driver(&reporter_server_driver);
-	dbus_connection_unref(connection);
 }
diff --git a/profiles/proximity/manager.h b/profiles/proximity/manager.h
index b0fe7c8..e65c31d 100644
--- a/profiles/proximity/manager.h
+++ b/profiles/proximity/manager.h
@@ -22,5 +22,5 @@
  *
  */
 
-int proximity_manager_init(DBusConnection *conn, GKeyFile *conf);
+int proximity_manager_init(GKeyFile *conf);
 void proximity_manager_exit(void);
diff --git a/profiles/proximity/monitor.c b/profiles/proximity/monitor.c
index f22d6f4..ac9dae7 100644
--- a/profiles/proximity/monitor.c
+++ b/profiles/proximity/monitor.c
@@ -66,7 +66,6 @@ enum {
 struct monitor {
 	struct btd_device *device;
 	GAttrib *attrib;
-	DBusConnection *conn;
 	struct att_range *linkloss;
 	struct att_range *txpower;
 	struct att_range *immediate;
@@ -160,7 +159,7 @@ static void linkloss_written(guint8 status, const guint8 *pdu, guint16 plen,
 
 	DBG("Link Loss Alert Level written");
 
-	emit_property_changed(monitor->conn, path,
+	emit_property_changed(get_dbus_connection(), path,
 				PROXIMITY_INTERFACE, "LinkLossAlertLevel",
 				DBUS_TYPE_STRING, &monitor->linklosslevel);
 }
@@ -289,7 +288,7 @@ static gboolean immediate_timeout(gpointer user_data)
 
 	g_free(monitor->immediatelevel);
 	monitor->immediatelevel = g_strdup("none");
-	emit_property_changed(monitor->conn, path, PROXIMITY_INTERFACE,
+	emit_property_changed(get_dbus_connection(), path, PROXIMITY_INTERFACE,
 					"ImmediateAlertLevel", DBUS_TYPE_STRING,
 					&monitor->immediatelevel);
 
@@ -304,7 +303,7 @@ static void immediate_written(gpointer user_data)
 	g_free(monitor->fallbacklevel);
 	monitor->fallbacklevel = NULL;
 
-	emit_property_changed(monitor->conn, path, PROXIMITY_INTERFACE,
+	emit_property_changed(get_dbus_connection(), path, PROXIMITY_INTERFACE,
 				"ImmediateAlertLevel",
 				DBUS_TYPE_STRING, &monitor->immediatelevel);
 
@@ -390,7 +389,7 @@ static void attio_disconnected_cb(gpointer user_data)
 
 	g_free(monitor->immediatelevel);
 	monitor->immediatelevel = g_strdup("none");
-	emit_property_changed(monitor->conn, path, PROXIMITY_INTERFACE,
+	emit_property_changed(get_dbus_connection(), path, PROXIMITY_INTERFACE,
 					"ImmediateAlertLevel", DBUS_TYPE_STRING,
 					&monitor->immediatelevel);
 }
@@ -577,7 +576,6 @@ static void monitor_destroy(gpointer user_data)
 	if (monitor->attrib)
 		g_attrib_unref(monitor->attrib);
 
-	dbus_connection_unref(monitor->conn);
 	btd_device_unref(monitor->device);
 	g_free(monitor->linkloss);
 	g_free(monitor->immediate);
@@ -588,7 +586,7 @@ static void monitor_destroy(gpointer user_data)
 	g_free(monitor);
 }
 
-int monitor_register(DBusConnection *conn, struct btd_device *device,
+int monitor_register(struct btd_device *device,
 		struct gatt_primary *linkloss, struct gatt_primary *txpower,
 		struct gatt_primary *immediate, struct enabled *enabled)
 {
@@ -604,12 +602,11 @@ int monitor_register(DBusConnection *conn, struct btd_device *device,
 
 	monitor = g_new0(struct monitor, 1);
 	monitor->device = btd_device_ref(device);
-	monitor->conn = dbus_connection_ref(conn);
 	monitor->linklosslevel = (level ? : g_strdup("high"));
 	monitor->signallevel = g_strdup("unknown");
 	monitor->immediatelevel = g_strdup("none");
 
-	if (g_dbus_register_interface(conn, path,
+	if (g_dbus_register_interface(get_dbus_connection(), path,
 				PROXIMITY_INTERFACE,
 				monitor_methods, monitor_signals,
 				NULL, monitor, monitor_destroy) == FALSE) {
@@ -661,9 +658,10 @@ int monitor_register(DBusConnection *conn, struct btd_device *device,
 	return 0;
 }
 
-void monitor_unregister(DBusConnection *conn, struct btd_device *device)
+void monitor_unregister(struct btd_device *device)
 {
 	const char *path = device_get_path(device);
 
-	g_dbus_unregister_interface(conn, path, PROXIMITY_INTERFACE);
+	g_dbus_unregister_interface(get_dbus_connection(), path,
+							PROXIMITY_INTERFACE);
 }
diff --git a/profiles/proximity/monitor.h b/profiles/proximity/monitor.h
index b71363d..191b562 100644
--- a/profiles/proximity/monitor.h
+++ b/profiles/proximity/monitor.h
@@ -28,7 +28,7 @@ struct enabled {
 	gboolean findme;
 };
 
-int monitor_register(DBusConnection *conn, struct btd_device *device,
-		struct gatt_primary *linkloss, struct gatt_primary *txpower,
-		struct gatt_primary *immediate, struct enabled *enabled);
-void monitor_unregister(DBusConnection *conn, struct btd_device *device);
+int monitor_register(struct btd_device *device, struct gatt_primary *linkloss,
+		struct gatt_primary *txpower, struct gatt_primary *immediate,
+		struct enabled *enabled);
+void monitor_unregister(struct btd_device *device);
diff --git a/profiles/proximity/reporter.c b/profiles/proximity/reporter.c
index de3770a..ff9fd0c 100644
--- a/profiles/proximity/reporter.c
+++ b/profiles/proximity/reporter.c
@@ -50,7 +50,6 @@
 #include "immalert.h"
 
 struct reporter_adapter {
-	DBusConnection *conn;
 	struct btd_adapter *adapter;
 	GSList *devices;
 };
@@ -198,7 +197,7 @@ static void unregister_reporter_device(gpointer data, gpointer user_data)
 
 	DBG("unregister on device %s", path);
 
-	g_dbus_unregister_interface(radapter->conn, path,
+	g_dbus_unregister_interface(get_dbus_connection(), path,
 					PROXIMITY_REPORTER_INTERFACE);
 
 	radapter->devices = g_slist_remove(radapter->devices, device);
@@ -212,7 +211,7 @@ static void register_reporter_device(struct btd_device *device,
 
 	DBG("register on device %s", path);
 
-	g_dbus_register_interface(radapter->conn, path,
+	g_dbus_register_interface(get_dbus_connection(), path,
 					PROXIMITY_REPORTER_INTERFACE,
 					reporter_methods, reporter_signals,
 					NULL, device, NULL);
@@ -257,24 +256,18 @@ static struct btd_device_driver reporter_device_driver = {
 int reporter_adapter_probe(struct btd_adapter *adapter)
 {
 	struct reporter_adapter *radapter;
-	DBusConnection *conn;
 
 	if (!main_opts.gatt_enabled) {
 		DBG("GATT is disabled");
 		return -ENOTSUP;
 	}
 
-	conn = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
-	if (!conn)
-		return -1;
-
 	radapter = g_new0(struct reporter_adapter, 1);
 	radapter->adapter = adapter;
-	radapter->conn = conn;
 
-	link_loss_register(adapter, radapter->conn);
+	link_loss_register(adapter);
 	register_tx_power(adapter);
-	imm_alert_register(adapter, radapter->conn);
+	imm_alert_register(adapter);
 
 	btd_register_device_driver(&reporter_device_driver);
 
@@ -297,7 +290,6 @@ void reporter_adapter_remove(struct btd_adapter *adapter)
 
 	link_loss_unregister(adapter);
 	imm_alert_unregister(adapter);
-	dbus_connection_unref(radapter->conn);
 
 	reporter_adapters = g_slist_remove(reporter_adapters, radapter);
 	g_free(radapter);
-- 
1.7.11.3

[PATCH 4/7] proximity: Move reporter device driver registration to manager

[Andrzej Kaczmarek]

Device driver for reporter is registered each time new adapter is probed
so in case multiple adapters are used it will be registered multiple
times for no reason.

This patch moves reporter device driver registration to manager so it
is registered only once, when plugin is loaded.
---
 profiles/proximity/manager.c  | 21 +++++++++++++++++++--
 profiles/proximity/reporter.c | 16 ++--------------
 profiles/proximity/reporter.h |  3 +++
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/profiles/proximity/manager.c b/profiles/proximity/manager.c
index 97e9795..4998a9e 100644
--- a/profiles/proximity/manager.c
+++ b/profiles/proximity/manager.c
@@ -85,6 +85,15 @@ static struct btd_device_driver monitor_driver = {
 	.remove = attio_device_remove,
 };
 
+
+/* device driver for tracking remote GATT client devices */
+static struct btd_device_driver reporter_device_driver = {
+	.name = "Proximity GATT Reporter Device Tracker Driver",
+	.uuids = BTD_UUIDS(GATT_UUID),
+	.probe = reporter_device_probe,
+	.remove = reporter_device_remove,
+};
+
 static struct btd_adapter_driver reporter_server_driver = {
 	.name = "Proximity GATT Reporter Driver",
 	.probe = reporter_adapter_probe,
@@ -123,13 +132,20 @@ int proximity_manager_init(GKeyFile *config)
 	if (ret < 0)
 		goto fail_monitor;
 
+	ret = btd_register_device_driver(&reporter_device_driver);
+	if (ret < 0)
+		goto fail_reporter_device;
+
 	ret = btd_register_adapter_driver(&reporter_server_driver);
 	if (ret < 0)
-		goto fail_reporter;
+		goto fail_reporter_server;
 
 	return 0;
 
-fail_reporter:
+fail_reporter_server:
+	btd_unregister_device_driver(&reporter_device_driver);
+
+fail_reporter_device:
 	btd_unregister_device_driver(&monitor_driver);
 
 fail_monitor:
@@ -139,5 +155,6 @@ fail_monitor:
 void proximity_manager_exit(void)
 {
 	btd_unregister_device_driver(&monitor_driver);
+	btd_unregister_device_driver(&reporter_device_driver);
 	btd_unregister_adapter_driver(&reporter_server_driver);
 }
diff --git a/profiles/proximity/reporter.c b/profiles/proximity/reporter.c
index ff9fd0c..c221fae 100644
--- a/profiles/proximity/reporter.c
+++ b/profiles/proximity/reporter.c
@@ -220,7 +220,7 @@ static void register_reporter_device(struct btd_device *device,
 	radapter->devices = g_slist_prepend(radapter->devices, device);
 }
 
-static int reporter_device_probe(struct btd_device *device, GSList *uuids)
+int reporter_device_probe(struct btd_device *device, GSList *uuids)
 {
 	struct reporter_adapter *radapter;
 	struct btd_adapter *adapter = device_get_adapter(device);
@@ -233,7 +233,7 @@ static int reporter_device_probe(struct btd_device *device, GSList *uuids)
 	return 0;
 }
 
-static void reporter_device_remove(struct btd_device *device)
+void reporter_device_remove(struct btd_device *device)
 {
 	struct reporter_adapter *radapter;
 	struct btd_adapter *adapter = device_get_adapter(device);
@@ -245,14 +245,6 @@ static void reporter_device_remove(struct btd_device *device)
 	unregister_reporter_device(device, radapter);
 }
 
-/* device driver for tracking remote GATT client devices */
-static struct btd_device_driver reporter_device_driver = {
-	.name = "Proximity GATT Reporter Device Tracker Driver",
-	.uuids = BTD_UUIDS(GATT_UUID),
-	.probe = reporter_device_probe,
-	.remove = reporter_device_remove,
-};
-
 int reporter_adapter_probe(struct btd_adapter *adapter)
 {
 	struct reporter_adapter *radapter;
@@ -269,8 +261,6 @@ int reporter_adapter_probe(struct btd_adapter *adapter)
 	register_tx_power(adapter);
 	imm_alert_register(adapter);
 
-	btd_register_device_driver(&reporter_device_driver);
-
 	reporter_adapters = g_slist_prepend(reporter_adapters, radapter);
 	DBG("Proximity Reporter for adapter %p", adapter);
 
@@ -283,8 +273,6 @@ void reporter_adapter_remove(struct btd_adapter *adapter)
 	if (!radapter)
 		return;
 
-	btd_unregister_device_driver(&reporter_device_driver);
-
 	g_slist_foreach(radapter->devices, unregister_reporter_device,
 								radapter);
 
diff --git a/profiles/proximity/reporter.h b/profiles/proximity/reporter.h
index ae5ddf6..c77c3af 100644
--- a/profiles/proximity/reporter.h
+++ b/profiles/proximity/reporter.h
@@ -36,6 +36,9 @@ enum {
 	HIGH_ALERT = 0x02,
 };
 
+int reporter_device_probe(struct btd_device *device, GSList *uuids);
+void reporter_device_remove(struct btd_device *device);
+
 int reporter_adapter_probe(struct btd_adapter *adapter);
 void reporter_adapter_remove(struct btd_adapter *adapter);
 
-- 
1.7.11.3

[PATCH 5/7] proximity: Remove unnecessary check for GATT enabled

[Andrzej Kaczmarek]

Proximity plugin won't be loaded if GATT is disabled so there's no point in
checking this once more when adapter is probed.
---
 profiles/proximity/reporter.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/profiles/proximity/reporter.c b/profiles/proximity/reporter.c
index c221fae..9b430f7 100644
--- a/profiles/proximity/reporter.c
+++ b/profiles/proximity/reporter.c
@@ -249,11 +249,6 @@ int reporter_adapter_probe(struct btd_adapter *adapter)
 {
 	struct reporter_adapter *radapter;
 
-	if (!main_opts.gatt_enabled) {
-		DBG("GATT is disabled");
-		return -ENOTSUP;
-	}
-
 	radapter = g_new0(struct reporter_adapter, 1);
 	radapter->adapter = adapter;
 
-- 
1.7.11.3

[PATCH 6/7] proximity: Remove list of adapter devices

[Andrzej Kaczmarek]

List of adapter devices is only used to remove them when adapter is removed.
There's no need to do this since devices will be removed before adapter is
removed anyway.
---
 profiles/proximity/reporter.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/profiles/proximity/reporter.c b/profiles/proximity/reporter.c
index 9b430f7..cf52548 100644
--- a/profiles/proximity/reporter.c
+++ b/profiles/proximity/reporter.c
@@ -51,7 +51,6 @@
 
 struct reporter_adapter {
 	struct btd_adapter *adapter;
-	GSList *devices;
 };
 
 static GSList *reporter_adapters;
@@ -192,7 +191,6 @@ static const GDBusSignalTable reporter_signals[] = {
 static void unregister_reporter_device(gpointer data, gpointer user_data)
 {
 	struct btd_device *device = data;
-	struct reporter_adapter *radapter = user_data;
 	const char *path = device_get_path(device);
 
 	DBG("unregister on device %s", path);
@@ -200,7 +198,6 @@ static void unregister_reporter_device(gpointer data, gpointer user_data)
 	g_dbus_unregister_interface(get_dbus_connection(), path,
 					PROXIMITY_REPORTER_INTERFACE);
 
-	radapter->devices = g_slist_remove(radapter->devices, device);
 	btd_device_unref(device);
 }
 
@@ -217,7 +214,6 @@ static void register_reporter_device(struct btd_device *device,
 					NULL, device, NULL);
 
 	btd_device_ref(device);
-	radapter->devices = g_slist_prepend(radapter->devices, device);
 }
 
 int reporter_device_probe(struct btd_device *device, GSList *uuids)
@@ -268,9 +264,6 @@ void reporter_adapter_remove(struct btd_adapter *adapter)
 	if (!radapter)
 		return;
 
-	g_slist_foreach(radapter->devices, unregister_reporter_device,
-								radapter);
-
 	link_loss_unregister(adapter);
 	imm_alert_unregister(adapter);
 
-- 
1.7.11.3

[PATCH 7/7] proximity: Remove reporter_adapter

[Andrzej Kaczmarek]

reporter_adapter holds only reference to btd_adapter so it's useless now.
---
 profiles/proximity/reporter.c | 65 +++----------------------------------------
 1 file changed, 4 insertions(+), 61 deletions(-)

diff --git a/profiles/proximity/reporter.c b/profiles/proximity/reporter.c
index cf52548..c331d17 100644
--- a/profiles/proximity/reporter.c
+++ b/profiles/proximity/reporter.c
@@ -49,34 +49,6 @@
 #include "linkloss.h"
 #include "immalert.h"
 
-struct reporter_adapter {
-	struct btd_adapter *adapter;
-};
-
-static GSList *reporter_adapters;
-
-static int radapter_cmp(gconstpointer a, gconstpointer b)
-{
-	const struct reporter_adapter *radapter = a;
-	const struct btd_adapter *adapter = b;
-
-	if (radapter->adapter == adapter)
-		return 0;
-
-	return -1;
-}
-
-static struct reporter_adapter *
-find_reporter_adapter(struct btd_adapter *adapter)
-{
-	GSList *l = g_slist_find_custom(reporter_adapters, adapter,
-								radapter_cmp);
-	if (!l)
-		return NULL;
-
-	return l->data;
-}
-
 const char *get_alert_level_string(uint8_t level)
 {
 	switch (level) {
@@ -188,9 +160,8 @@ static const GDBusSignalTable reporter_signals[] = {
 	{ }
 };
 
-static void unregister_reporter_device(gpointer data, gpointer user_data)
+static void unregister_reporter_device(struct btd_device *device)
 {
-	struct btd_device *device = data;
 	const char *path = device_get_path(device);
 
 	DBG("unregister on device %s", path);
@@ -201,8 +172,7 @@ static void unregister_reporter_device(gpointer data, gpointer user_data)
 	btd_device_unref(device);
 }
 
-static void register_reporter_device(struct btd_device *device,
-					struct reporter_adapter *radapter)
+static void register_reporter_device(struct btd_device *device)
 {
 	const char *path = device_get_path(device);
 
@@ -218,41 +188,21 @@ static void register_reporter_device(struct btd_device *device,
 
 int reporter_device_probe(struct btd_device *device, GSList *uuids)
 {
-	struct reporter_adapter *radapter;
-	struct btd_adapter *adapter = device_get_adapter(device);
-
-	radapter = find_reporter_adapter(adapter);
-	if (!radapter)
-		return -1;
-
-	register_reporter_device(device, radapter);
+	register_reporter_device(device);
 	return 0;
 }
 
 void reporter_device_remove(struct btd_device *device)
 {
-	struct reporter_adapter *radapter;
-	struct btd_adapter *adapter = device_get_adapter(device);
-
-	radapter = find_reporter_adapter(adapter);
-	if (!radapter)
-		return;
-
-	unregister_reporter_device(device, radapter);
+	unregister_reporter_device(device);
 }
 
 int reporter_adapter_probe(struct btd_adapter *adapter)
 {
-	struct reporter_adapter *radapter;
-
-	radapter = g_new0(struct reporter_adapter, 1);
-	radapter->adapter = adapter;
-
 	link_loss_register(adapter);
 	register_tx_power(adapter);
 	imm_alert_register(adapter);
 
-	reporter_adapters = g_slist_prepend(reporter_adapters, radapter);
 	DBG("Proximity Reporter for adapter %p", adapter);
 
 	return 0;
@@ -260,13 +210,6 @@ int reporter_adapter_probe(struct btd_adapter *adapter)
 
 void reporter_adapter_remove(struct btd_adapter *adapter)
 {
-	struct reporter_adapter *radapter = find_reporter_adapter(adapter);
-	if (!radapter)
-		return;
-
 	link_loss_unregister(adapter);
 	imm_alert_unregister(adapter);
-
-	reporter_adapters = g_slist_remove(reporter_adapters, radapter);
-	g_free(radapter);
 }
-- 
1.7.11.3

Re: [PATCH 0/7] RFC: proximity cleanup

[Anderson Lizardo]

Hi Andrzej,

On Fri, Aug 31, 2012 at 8:48 AM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> Hi,
>
> Here are few patches to cleanup proximity profile code a bit which seem to
> keep track of some data which are redundant and has minor issue with device
> driver registration.
>
> Most cleanup is done in patch #3 which removes tracking of DBusConnection
> across whole plugin and simply uses get_dbus_connection() wherever necessary.
> This is different than other profile plugins do, but reduces code size and
> memory footprint a bit so can be done for other plugins as well if accepted.

While some cleanups here are very welcome, I'm worried that you are
cleaning up things before finishing the Proximity Reporter
implementation. Power Level characteristic for instance is currently a
stub, how do you plan to store it per adapter, without the struct
reporter_adapter?

Therefore I suggest you split the "trivial" cleanups from the removal
of struct reporter_adapter.

BTW, do you have plans to add more features to the Proximity Reporter
implementation?

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 0/7] RFC: proximity cleanup

[Andrzej Kaczmarek]

Hi Anderson,

On 08/31/2012 06:17 PM, Anderson Lizardo wrote:
> Hi Andrzej,
>
> On Fri, Aug 31, 2012 at 8:48 AM, Andrzej Kaczmarek
> <andrzej.kaczmarek@tieto.com> wrote:
>> Hi,
>>
>> Here are few patches to cleanup proximity profile code a bit which seem to
>> keep track of some data which are redundant and has minor issue with device
>> driver registration.
>>
>> Most cleanup is done in patch #3 which removes tracking of DBusConnection
>> across whole plugin and simply uses get_dbus_connection() wherever necessary.
>> This is different than other profile plugins do, but reduces code size and
>> memory footprint a bit so can be done for other plugins as well if accepted.
>
> While some cleanups here are very welcome, I'm worried that you are
> cleaning up things before finishing the Proximity Reporter
> implementation. Power Level characteristic for instance is currently a
> stub, how do you plan to store it per adapter, without the struct
> reporter_adapter?

Ah, I didn't find it on TODO so assumed it's complete. But you're right, 
it probably should not be removed.

> Therefore I suggest you split the "trivial" cleanups from the removal
> of struct reporter_adapter.

Please just consider patches #6 and #7 as invalid for now, I think 
there's no point in sending #1-5 once more until there are some comments 
to fix.

> BTW, do you have plans to add more features to the Proximity Reporter
> implementation?

No plans at the moment, but perhaps I'll take a look on this in more 
details after HRP is finished (v2 will be sent soon).

BR,
Andrzej

Re: [PATCH 0/7] RFC: proximity cleanup

[Johan Hedberg]

Hi Andrzej,

On Mon, Sep 03, 2012, Andrzej Kaczmarek wrote:
> On 08/31/2012 06:17 PM, Anderson Lizardo wrote:
> >Hi Andrzej,
> >
> >On Fri, Aug 31, 2012 at 8:48 AM, Andrzej Kaczmarek
> ><andrzej.kaczmarek@tieto.com> wrote:
> >>Hi,
> >>
> >>Here are few patches to cleanup proximity profile code a bit which seem to
> >>keep track of some data which are redundant and has minor issue with device
> >>driver registration.
> >>
> >>Most cleanup is done in patch #3 which removes tracking of DBusConnection
> >>across whole plugin and simply uses get_dbus_connection() wherever necessary.
> >>This is different than other profile plugins do, but reduces code size and
> >>memory footprint a bit so can be done for other plugins as well if accepted.
> >
> >While some cleanups here are very welcome, I'm worried that you are
> >cleaning up things before finishing the Proximity Reporter
> >implementation. Power Level characteristic for instance is currently a
> >stub, how do you plan to store it per adapter, without the struct
> >reporter_adapter?
> 
> Ah, I didn't find it on TODO so assumed it's complete. But you're
> right, it probably should not be removed.
> 
> >Therefore I suggest you split the "trivial" cleanups from the removal
> >of struct reporter_adapter.
> 
> Please just consider patches #6 and #7 as invalid for now, I think
> there's no point in sending #1-5 once more until there are some
> comments to fix.
> 
> >BTW, do you have plans to add more features to the Proximity Reporter
> >implementation?
> 
> No plans at the moment, but perhaps I'll take a look on this in more
> details after HRP is finished (v2 will be sent soon).

I've applied patches 1-4. Patch 5 didn't really make sense anymore after
the btd_profile refactoring that went in just a moment ago, and as you
said patch 6 and 7 can be considered invalid for now.

Johan