[PATCH 2/6] bluetooth: Add register service for bluetooth

Zhang, Zhenhua zhenhua.zhang at intel.com
Sun Jul 11 20:27:06 PDT 2010


Hi Padovan,

Gustavo F. Padovan wrote:
> Hi Zhenhua,
> 
> * Zhenhua Zhang <zhenhua.zhang at 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 at ofono.org
> http://lists.ofono.org/listinfo/ofono



Regards,
Zhenhua



More information about the ofono mailing list