Hi Padovan, Gustavo F. Padovan wrote: > Hi Zhenhua, > > * Zhenhua Zhang <zhenhua.zh...@intel.com> [2010-07-05 13:45:05 +0800]: > >> Add bluetooth_register_service() and bluetooth_unregister_service() >> where bluetooth profiles plugins like DUN GW can register themselves >> per adapter. It shares existing bluetooth framework to listen >> bluetooth events (new adapters, bluetoothd shutdown, etc..) >> --- >> plugins/bluetooth.c | 125 > ++++++++++++++++++++++++++++++++++++++++++--------- >> plugins/bluetooth.h | 8 +++ >> 2 files changed, 112 insertions(+), 21 deletions(-) >> >> diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c >> index 5a85eaa..3a3b5e6 100644 >> --- a/plugins/bluetooth.c >> +++ b/plugins/bluetooth.c >> @@ -38,8 +38,19 @@ >> >> static DBusConnection *connection; >> static GHashTable *uuid_hash = NULL; >> +static GHashTable *service_hash = NULL; >> static GHashTable *adapter_address_hash = NULL; >> >> +static char *bluetooth_service_type_to_str(enum >> bluetooth_service_type type) +{ + switch (type) { >> + case DUN_GATEWAY: >> + return "DUN"; >> + default: >> + return ""; >> + } >> +} >> + > > We actually don't need this function, you can put the > 'enum bluetooth_service_type' value as hash key. Sounds better.
Agree. It seems the type string is not used in other place except hash table insert/remove. So I will change that. >> void bluetooth_create_path(const char *dev_addr, const char >> *adapter_addr, char *buf, int size) { >> @@ -376,6 +387,9 @@ static void > adapter_properties_cb(DBusPendingCall *call, gpointer user_data) >> GSList *device_list = NULL; >> GSList *l; >> const char *addr; >> + GHashTableIter hash_iter; >> + gpointer key, value; >> + struct bluetooth_profile *profile; >> >> reply = dbus_pending_call_steal_reply(call); >> >> @@ -393,6 +407,12 @@ static void > adapter_properties_cb(DBusPendingCall *call, gpointer user_data) >> g_hash_table_insert(adapter_address_hash, >> g_strdup(path), g_strdup(addr)); >> >> + g_hash_table_iter_init(&hash_iter, service_hash); >> + while (g_hash_table_iter_next(&hash_iter, &key, &value)) { >> + profile = value; + profile->create(path, NULL, >> addr, NULL); > > You have to check if profile and profile->create is not NULL. > It is more > safe. Sure. I will add check for both. >> + } >> + >> for (l = device_list; l; l = l->next) { >> const char *device = l->data; >> >> @@ -492,10 +512,13 @@ static void > bluetooth_remove_all_modem(gpointer key, gpointer value, >> >> static void bluetooth_disconnect(DBusConnection *connection, void >> *user_data) { >> - if (!uuid_hash) >> - return; >> + if (uuid_hash) >> + g_hash_table_foreach(uuid_hash, bluetooth_remove_all_modem, >> + NULL); >> >> - g_hash_table_foreach(uuid_hash, > bluetooth_remove_all_modem, NULL); >> + if (service_hash) >> + g_hash_table_foreach(service_hash, bluetooth_remove_all_modem, >> + NULL); } >> >> static guint bluetooth_watch; >> @@ -503,12 +526,12 @@ static guint adapter_added_watch; >> static guint adapter_removed_watch; >> s.tatic guint property_watch; >> >> -int bluetooth_register_uuid(const char *uuid, struct >> bluetooth_profile *profile) +static int bluetooth_init() { > > So I prefer a real refcount structure here, and call this > bluetooth_ref(). What do you think? Good idea. Bluetooth_ref/unref makes more sense if we have both HFP and DUN existings on the system. Actually I have an unrelated question here. Why we don't have something like DUN_GW_UUID string so that we can easily reuse your current code? For now, we need to register service and use a different hash table to record that. >> int err; >> >> - if (uuid_hash) >> - goto done; >> + if (adapter_address_hash) >> + return 0; >> >> connection = ofono_dbus_get_connection(); >> >> @@ -536,19 +559,9 @@ int bluetooth_register_uuid(const char > *uuid, struct bluetooth_profile *profile) >> goto remove; >> } >> >> - uuid_hash = g_hash_table_new_full(g_str_hash, g_str_equal, >> - g_free, NULL); >> - >> adapter_address_hash = > g_hash_table_new_full(g_str_hash, g_str_equal, >> g_free, g_free); >> >> -done: >> - g_hash_table_insert(uuid_hash, g_strdup(uuid), profile); - >> - bluetooth_send_with_reply("/", BLUEZ_MANAGER_INTERFACE, >> "GetProperties", >> - manager_properties_cb, NULL, NULL, -1, >> - DBUS_TYPE_INVALID); >> - >> return 0; >> >> remove: >> @@ -556,14 +569,13 @@ remove: >> g_dbus_remove_watch(connection, adapter_added_watch); >> g_dbus_remove_watch(connection, adapter_removed_watch); >> g_dbus_remove_watch(connection, property_watch); >> + >> return err; >> } >> >> -void bluetooth_unregister_uuid(const char *uuid) >> +static void bluetooth_exit() > > and this one coul be bluetooth_unref() > >> { >> - g_hash_table_remove(uuid_hash, uuid); >> - >> - if (g_hash_table_size(uuid_hash)) >> + if (uuid_hash != NULL || service_hash != NULL) >> return; >> >> g_dbus_remove_watch(connection, bluetooth_watch); >> @@ -571,9 +583,80 @@ void bluetooth_unregister_uuid(const char *uuid) >> g_dbus_remove_watch(connection, adapter_removed_watch); >> g_dbus_remove_watch(connection, property_watch); >> >> - g_hash_table_destroy(uuid_hash); >> g_hash_table_destroy(adapter_address_hash); >> +} >> + >> + >> +int bluetooth_register_uuid(const char *uuid, struct >> bluetooth_profile *profile) +{ + int err; >> + >> + err = bluetooth_init(); >> + if (err) >> + return err; >> + >> + if (!uuid_hash) >> + uuid_hash = g_hash_table_new_full(g_str_hash, g_str_equal, >> + g_free, NULL); + >> + g_hash_table_insert(uuid_hash, g_strdup(uuid), profile); + >> + bluetooth_send_with_reply("/", BLUEZ_MANAGER_INTERFACE, >> "GetProperties", + manager_properties_cb, NULL, >> NULL, -1, >> + DBUS_TYPE_INVALID); >> + >> + return 0; >> +} >> + >> +void bluetooth_unregister_uuid(const char *uuid) >> +{ >> + g_hash_table_remove(uuid_hash, uuid); >> + >> + if (g_hash_table_size(uuid_hash) > 0) >> + return; >> + >> + g_hash_table_destroy(uuid_hash); >> uuid_hash = NULL; >> + bluetooth_exit(); >> +} >> + >> +int bluetooth_register_service(enum bluetooth_service_type type, >> + struct > bluetooth_profile *profile) >> +{ >> + int err; >> + char *type_str; >> + >> + err = bluetooth_init(); >> + if (err) >> + return err; >> + >> + type_str = bluetooth_service_type_to_str(type); >> + >> + if (!service_hash) >> + service_hash = > g_hash_table_new_full(g_str_hash, g_str_equal, >> + g_free, NULL); >> + >> + g_hash_table_insert(service_hash, g_strdup(type_str), profile); + >> + bluetooth_send_with_reply("/", BLUEZ_MANAGER_INTERFACE, >> "GetProperties", + manager_properties_cb, NULL, >> NULL, -1, >> + DBUS_TYPE_INVALID); >> + >> + return 0; >> +} >> + >> +void bluetooth_unregister_service(enum bluetooth_service_type type) >> +{ + char *type_str = bluetooth_service_type_to_str(type); + >> + g_hash_table_remove(service_hash, type_str); >> + >> + if (g_hash_table_size(service_hash) > 0) >> + return; >> + >> + g_hash_table_destroy(service_hash); >> + service_hash = NULL; >> + bluetooth_exit(); >> } >> >> OFONO_PLUGIN_DEFINE(bluetooth, "Bluetooth Utils Plugins", VERSION, >> diff --git a/plugins/bluetooth.h b/plugins/bluetooth.h >> index fb0d841..84707a9 100644 >> --- a/plugins/bluetooth.h >> +++ b/plugins/bluetooth.h >> @@ -28,6 +28,10 @@ >> /* Profiles bitfield */ >> #define HFP_AG 0x01 >> >> +enum bluetooth_service_type { >> + DUN_GATEWAY, >> +}; >> + >> struct bluetooth_profile { >> const char *name; >> int (*create)(const char *device, const char *dev_addr, >> @@ -40,6 +44,10 @@ int bluetooth_register_uuid(const char *uuid, >> struct bluetooth_profile *profile); >> void bluetooth_unregister_uuid(const char *uuid); >> >> +int bluetooth_register_service(enum bluetooth_service_type type, >> + struct > bluetooth_profile *profile); >> +void bluetooth_unregister_service(enum bluetooth_service_type >> type); + void bluetooth_create_path(const char *dev_addr, const >> char *adapter_addr, char > *buf, int size); >> >> -- >> 1.6.3.3 > > -- > Gustavo F. Padovan > http://padovan.org > _______________________________________________ > ofono mailing list > ofono@ofono.org > http://lists.ofono.org/listinfo/ofono Regards, Zhenhua _______________________________________________ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono