Re: [PATCH 3/4] bluetooth: Add bluetooth server support

[Frederic Danis <frederic.danis@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 9194 bytes --]

Le 27/01/2011 17:26, Gustavo F. Padovan a écrit :
> Hi Frédéric,
>
> * Frédéric Danis<frederic.danis@linux.intel.com>  [2011-01-27 16:05:39 +0100]:
>
>> ---
>>   Makefile.am         |    1 +
>>   plugins/bluetooth.c |  340 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   plugins/bluetooth.h |    9 ++
>>   3 files changed, 350 insertions(+), 0 deletions(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 9b77e63..c0efe28 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -318,6 +318,7 @@ builtin_sources += plugins/bluetooth.c plugins/bluetooth.h
>>   builtin_modules += hfp
>>   builtin_sources += plugins/hfp.c plugins/bluetooth.h
>>
>> +builtin_sources += $(btio_sources)
>>   builtin_cflags += @BLUEZ_CFLAGS@
>>   builtin_libadd += @BLUEZ_LIBS@
>>   endif
>> diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c
>> index 4da662a..86d4c25 100644
>> --- a/plugins/bluetooth.c
>> +++ b/plugins/bluetooth.c
>> @@ -35,18 +35,36 @@
>>
>>   #include<ofono/dbus.h>
>>
>> +#include<btio.h>
>>   #include "bluetooth.h"
>>
>>   static DBusConnection *connection;
>>   static GHashTable *uuid_hash = NULL;
>>   static GSList *adapter_list = NULL;
>>   static gint bluetooth_refcount;
>> +static GSList *server_list = NULL;
>>
>>   struct adapter_address {
>>   	char *adapter;
>>   	char *address;
>>   };
>>
>> +struct server {
>> +	guint8 channel;
>> +	char *sdp_record;
>> +	GIOChannel *io;
>> +	GHashTable *adapter_hash;
>> +	ConnectFunc connect_cb;
>> +	gpointer user_data;
>> +	GSList *client_list;
>> +};
>> +
>> +struct cb_data {
>> +	struct server *server;
>> +	char *path;
>> +	guint source;
>> +};
>> +
>>   void bluetooth_create_path(const char *dev_addr, const char *adapter_addr,
>>   				char *buf, int size)
>>   {
>> @@ -429,6 +447,277 @@ static gboolean property_changed(DBusConnection *connection, DBusMessage *msg,
>>   	return TRUE;
>>   }
>>
>> +static void remove_record(char *path, guint handle, struct server *server)
>> +{
>> +	DBusMessage *msg;
>> +
>> +	msg = dbus_message_new_method_call(BLUEZ_SERVICE, path,
>> +					BLUEZ_SERVICE_INTERFACE,
>> +					"RemoveRecord");
>> +	if (msg == NULL) {
>> +		ofono_error("Unable to allocate D-Bus RemoveRecord message");
>> +		return;
>> +	}
>> +
>> +	dbus_message_append_args(msg, DBUS_TYPE_UINT32,&handle,
>> +					DBUS_TYPE_INVALID);
>> +	g_dbus_send_message(connection, msg);
>> +
>> +	ofono_info("Unregistered handle for %s, channel %d: 0x%x", path,
>> +			server->channel, handle);
>> +}
>> +
>> +static void server_stop(struct server *server)
>> +{
>> +	while (server->client_list) {
>> +		g_source_remove((guint) server->client_list->data);
>> +		server->client_list = g_slist_remove(server->client_list,
>> +						server->client_list->data);
>> +	}
>> +
>> +	g_hash_table_foreach_remove(server->adapter_hash,
>> +					(GHRFunc) remove_record, server);
>> +
>> +	if (server->io != NULL) {
>> +		g_io_channel_shutdown(server->io, TRUE, NULL);
>> +		g_io_channel_unref(server->io);
>> +		server->io = NULL;
>> +	}
>> +}
>> +
>> +static void cb_data_destroy(gpointer data)
>> +{
>> +	struct cb_data *cb_data = data;
>> +
>> +	if (cb_data->path != NULL)
>> +		g_free(cb_data->path);
>> +	g_free(cb_data);
>> +}
>> +
>> +static gboolean client_event(GIOChannel *chan, GIOCondition cond, gpointer data)
>> +{
>> +	struct cb_data *cb_data = data;
>> +	struct server *server = cb_data->server;
>> +
>> +	server->client_list = g_slist_remove(server->client_list,
>> +						(void *) cb_data->source);
>> +
>> +	cb_data_destroy(cb_data);
>> +
>> +	return FALSE;
>> +}
>> +
>> +static void confirm_event(GIOChannel *io, gpointer user_data)
>> +{
>> +	struct server *server = user_data;
>> +	struct cb_data *client_data;
>> +	GError *err = NULL;
>> +	char laddress[18], raddress[18];
>> +	guint8 channel;
>> +
>> +	bt_io_get(io, BT_IO_RFCOMM,&err, BT_IO_OPT_SOURCE, laddress,
>> +					BT_IO_OPT_DEST, raddress,
>> +					BT_IO_OPT_CHANNEL,&channel,
>> +					BT_IO_OPT_INVALID);
>> +	if (err) {
>> +		ofono_error("%s", err->message);
>> +		g_error_free(err);
>> +		return;
>> +	}
>> +
>> +	ofono_info("New connection for %s on  channel %u from: %s,", laddress,
>> +							channel, raddress);
>> +
>> +	if (!bt_io_accept(io, server->connect_cb, server->user_data,
>> +						NULL,&err)) {
>> +		ofono_error("%s", err->message);
>> +		g_error_free(err);
>> +		g_io_channel_unref(io);
>> +		return;
>> +	}
>> +
>> +	client_data = g_try_new0(struct cb_data, 1);
>> +	if (client_data == NULL) {
>> +		ofono_error("Unable to allocate client cb_data structure");
>> +		return;
>> +	}
>> +
>> +	client_data->server = server;
>> +	client_data->source = g_io_add_watch(io,
>> +					G_IO_HUP | G_IO_ERR | G_IO_NVAL,
>> +					client_event, client_data);
>> +	server->client_list = g_slist_prepend(server->client_list,
>> +						(void *)client_data->source);
>> +}
>> +
>> +static void add_record_cb(DBusPendingCall *call, gpointer user_data)
>> +{
>> +	struct cb_data *cb_data = user_data;
>> +	DBusMessage *reply = dbus_pending_call_steal_reply(call);
>> +	DBusError derr;
>> +	guint32 handle;
>> +
>> +	dbus_error_init(&derr);
>> +
>> +	if (dbus_set_error_from_message(&derr, reply)) {
>> +		ofono_error("Replied with an error: %s, %s",
>> +					derr.name, derr.message);
>> +		dbus_error_free(&derr);
>> +		g_free(cb_data->path);
>> +		goto done;
>> +	}
>> +
>> +	dbus_message_get_args(reply, NULL, DBUS_TYPE_UINT32,&handle,
>> +					DBUS_TYPE_INVALID);
>> +
>> +	g_hash_table_insert(cb_data->server->adapter_hash, cb_data->path,
>> +				(void *) handle);
>> +
>> +	ofono_info("Registered handle for %s, channel %d: 0x%x", cb_data->path,
>> +			cb_data->server->channel, handle);
>> +
>> +done:
>> +	/* Do not free cb_data->path, it is used in adapter_hash */
>> +	g_free(cb_data);
>> +	dbus_message_unref(reply);
>> +}
>> +
>> +static void server_adapter_properties_cb(DBusPendingCall *call,
>> +						gpointer user_data)
>> +{
>> +	struct cb_data *cb_data = user_data;
>> +	const char *path = cb_data->path;
>> +	DBusMessage *reply;
>> +	const char *addr;
>> +	struct server *server = cb_data->server;
>> +
>> +	reply = dbus_pending_call_steal_reply(call);
>> +
>> +	if (dbus_message_is_error(reply, DBUS_ERROR_SERVICE_UNKNOWN)) {
>> +		DBG("Bluetooth daemon is apparently not available.");
>> +		goto done;
>> +	}
>> +
>> +	bluetooth_parse_properties(reply, "Address", parse_string,&addr, NULL);
>> +
>> +	DBG("Adapter Address: %s, Path: %s", addr, path);
>> +	adapter_list = adapter_address_add(adapter_list, path, addr);
>> +
>> +	DBG("Add record on %s", path);
>> +
>> +	bluetooth_send_with_reply(path, BLUEZ_SERVICE_INTERFACE,
>> +				"AddRecord", add_record_cb,
>> +				cb_data, NULL, -1,
>> +				DBUS_TYPE_STRING,&server->sdp_record,
>> +				DBUS_TYPE_INVALID);
>> +
>> +done:
>> +	dbus_message_unref(reply);
>> +}
>> +
>> +static void server_parse_adapters(DBusMessageIter *array, gpointer user_data)
>> +{
>> +	struct server *server = user_data;
>> +	DBusMessageIter value;
>> +	struct cb_data *cb_data;
>> +
>> +	if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
>> +		return;
>> +
>> +	dbus_message_iter_recurse(array,&value);
>> +
>> +	while (dbus_message_iter_get_arg_type(&value)
>> +			== DBUS_TYPE_OBJECT_PATH) {
>> +		const char *path;
>> +
>> +		dbus_message_iter_get_basic(&value,&path);
>> +
>> +		DBG("Calling GetProperties on %s", path);
>> +
>> +		cb_data = g_try_new0(struct cb_data, 1);
>> +		if (cb_data == NULL) {
>> +			ofono_error("Unable to allocate cb_data structure");
>> +			return;
>> +		}
>> +
>> +		cb_data->server = server;
>> +		cb_data->path = g_strdup(path);
>> +
>> +		bluetooth_send_with_reply(path, BLUEZ_ADAPTER_INTERFACE,
>> +				"GetProperties", server_adapter_properties_cb,
>> +				cb_data, NULL, -1, DBUS_TYPE_INVALID);
>> +
>> +		dbus_message_iter_next(&value);
>> +	}
>> +}
>> +
>> +static void server_manager_properties_cb(DBusPendingCall *call,
>> +						gpointer user_data)
>> +{
>> +	DBusMessage *reply;
>> +
>> +	reply = dbus_pending_call_steal_reply(call);
>> +
>> +	if (dbus_message_is_error(reply, DBUS_ERROR_SERVICE_UNKNOWN)) {
>> +		DBG("Bluetooth daemon is apparently not available.");
>> +		goto done;
>> +	}
>> +
>> +	bluetooth_parse_properties(reply, "Adapters", server_parse_adapters,
>> +						user_data, NULL);
>> +
>> +done:
>> +	dbus_message_unref(reply);
>> +}
>
> What's going on here? You are duplicating many line of codes that we already
> have in bluetooth.c. Can't you just adapt the code we have there?
>

Most of the code is same, but the end of callbacks do not finish in same 
way.
If this code is not duplicate, this will complicate the server and/or 
client code to manage that it is called form the other path.
With this code the client and server path are isolated.

-- 
Frederic Danis                            Open Source Technology Centre
frederic.danis(a)intel.com                              Intel Corporation