* [PATCH 0/7] RFC: proximity cleanup
@ 2012-08-31 12:48 Andrzej Kaczmarek
2012-08-31 12:48 ` [PATCH 1/7] proximity: Change adapter driver function names Andrzej Kaczmarek
` (7 more replies)
0 siblings, 8 replies; 11+ messages in thread
From: Andrzej Kaczmarek @ 2012-08-31 12:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/7] proximity: Change adapter driver function names 2012-08-31 12:48 [PATCH 0/7] RFC: proximity cleanup Andrzej Kaczmarek @ 2012-08-31 12:48 ` Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 2/7] proximity: Remove unused definitions Andrzej Kaczmarek ` (6 subsequent siblings) 7 siblings, 0 replies; 11+ messages in thread From: Andrzej Kaczmarek @ 2012-08-31 12:48 UTC (permalink / raw) To: linux-bluetooth; +Cc: 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/7] proximity: Remove unused definitions 2012-08-31 12:48 [PATCH 0/7] RFC: proximity cleanup Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 1/7] proximity: Change adapter driver function names Andrzej Kaczmarek @ 2012-08-31 12:48 ` Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 3/7] proximity: Simplify DBusConnection object usage Andrzej Kaczmarek ` (5 subsequent siblings) 7 siblings, 0 replies; 11+ messages in thread From: Andrzej Kaczmarek @ 2012-08-31 12:48 UTC (permalink / raw) To: linux-bluetooth; +Cc: 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/7] proximity: Simplify DBusConnection object usage 2012-08-31 12:48 [PATCH 0/7] RFC: proximity cleanup Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 1/7] proximity: Change adapter driver function names Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 2/7] proximity: Remove unused definitions Andrzej Kaczmarek @ 2012-08-31 12:48 ` Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 4/7] proximity: Move reporter device driver registration to manager Andrzej Kaczmarek ` (4 subsequent siblings) 7 siblings, 0 replies; 11+ messages in thread From: Andrzej Kaczmarek @ 2012-08-31 12:48 UTC (permalink / raw) To: linux-bluetooth; +Cc: 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/7] proximity: Move reporter device driver registration to manager 2012-08-31 12:48 [PATCH 0/7] RFC: proximity cleanup Andrzej Kaczmarek ` (2 preceding siblings ...) 2012-08-31 12:48 ` [PATCH 3/7] proximity: Simplify DBusConnection object usage Andrzej Kaczmarek @ 2012-08-31 12:48 ` Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 5/7] proximity: Remove unnecessary check for GATT enabled Andrzej Kaczmarek ` (3 subsequent siblings) 7 siblings, 0 replies; 11+ messages in thread From: Andrzej Kaczmarek @ 2012-08-31 12:48 UTC (permalink / raw) To: linux-bluetooth; +Cc: 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/7] proximity: Remove unnecessary check for GATT enabled 2012-08-31 12:48 [PATCH 0/7] RFC: proximity cleanup Andrzej Kaczmarek ` (3 preceding siblings ...) 2012-08-31 12:48 ` [PATCH 4/7] proximity: Move reporter device driver registration to manager Andrzej Kaczmarek @ 2012-08-31 12:48 ` Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 6/7] proximity: Remove list of adapter devices Andrzej Kaczmarek ` (2 subsequent siblings) 7 siblings, 0 replies; 11+ messages in thread From: Andrzej Kaczmarek @ 2012-08-31 12:48 UTC (permalink / raw) To: linux-bluetooth; +Cc: 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 6/7] proximity: Remove list of adapter devices 2012-08-31 12:48 [PATCH 0/7] RFC: proximity cleanup Andrzej Kaczmarek ` (4 preceding siblings ...) 2012-08-31 12:48 ` [PATCH 5/7] proximity: Remove unnecessary check for GATT enabled Andrzej Kaczmarek @ 2012-08-31 12:48 ` Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 7/7] proximity: Remove reporter_adapter Andrzej Kaczmarek 2012-08-31 16:17 ` [PATCH 0/7] RFC: proximity cleanup Anderson Lizardo 7 siblings, 0 replies; 11+ messages in thread From: Andrzej Kaczmarek @ 2012-08-31 12:48 UTC (permalink / raw) To: linux-bluetooth; +Cc: 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 7/7] proximity: Remove reporter_adapter 2012-08-31 12:48 [PATCH 0/7] RFC: proximity cleanup Andrzej Kaczmarek ` (5 preceding siblings ...) 2012-08-31 12:48 ` [PATCH 6/7] proximity: Remove list of adapter devices Andrzej Kaczmarek @ 2012-08-31 12:48 ` Andrzej Kaczmarek 2012-08-31 16:17 ` [PATCH 0/7] RFC: proximity cleanup Anderson Lizardo 7 siblings, 0 replies; 11+ messages in thread From: Andrzej Kaczmarek @ 2012-08-31 12:48 UTC (permalink / raw) To: linux-bluetooth; +Cc: 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/7] RFC: proximity cleanup 2012-08-31 12:48 [PATCH 0/7] RFC: proximity cleanup Andrzej Kaczmarek ` (6 preceding siblings ...) 2012-08-31 12:48 ` [PATCH 7/7] proximity: Remove reporter_adapter Andrzej Kaczmarek @ 2012-08-31 16:17 ` Anderson Lizardo 2012-09-03 7:36 ` Andrzej Kaczmarek 7 siblings, 1 reply; 11+ messages in thread From: Anderson Lizardo @ 2012-08-31 16:17 UTC (permalink / raw) To: Andrzej Kaczmarek; +Cc: linux-bluetooth 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/7] RFC: proximity cleanup 2012-08-31 16:17 ` [PATCH 0/7] RFC: proximity cleanup Anderson Lizardo @ 2012-09-03 7:36 ` Andrzej Kaczmarek 2012-09-03 11:33 ` Johan Hedberg 0 siblings, 1 reply; 11+ messages in thread From: Andrzej Kaczmarek @ 2012-09-03 7:36 UTC (permalink / raw) To: Anderson Lizardo; +Cc: linux-bluetooth@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/7] RFC: proximity cleanup 2012-09-03 7:36 ` Andrzej Kaczmarek @ 2012-09-03 11:33 ` Johan Hedberg 0 siblings, 0 replies; 11+ messages in thread From: Johan Hedberg @ 2012-09-03 11:33 UTC (permalink / raw) To: Andrzej Kaczmarek; +Cc: Anderson Lizardo, linux-bluetooth@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-09-03 11:33 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-31 12:48 [PATCH 0/7] RFC: proximity cleanup Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 1/7] proximity: Change adapter driver function names Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 2/7] proximity: Remove unused definitions Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 3/7] proximity: Simplify DBusConnection object usage Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 4/7] proximity: Move reporter device driver registration to manager Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 5/7] proximity: Remove unnecessary check for GATT enabled Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 6/7] proximity: Remove list of adapter devices Andrzej Kaczmarek 2012-08-31 12:48 ` [PATCH 7/7] proximity: Remove reporter_adapter Andrzej Kaczmarek 2012-08-31 16:17 ` [PATCH 0/7] RFC: proximity cleanup Anderson Lizardo 2012-09-03 7:36 ` Andrzej Kaczmarek 2012-09-03 11:33 ` Johan Hedberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).