[PATCH v3 0/8] MAP client: notification support

[PATCH v3 0/8] MAP client: notification support

[Christian Fetzer]

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

Changelog:
v3: Added user_data parameter to dispatcher. This is needed for accessing struct map_data
    in the notification handler.
v2: Rebased since the first patchset didn't apply cleanly anymore.

---

This patchset adds the necessary blocks for receiving notifications in the MAP client.
So far this covers only bluetooth protocol related additions, leaving out end user D-Bus API changes.

The patchset consists of the following parts:
- Message Notification Server (MNS)
  Once connected, the MNS will receive XML based event reports from the MSE and parse them into a struct
  for further processing.

- RegisterNotification function
  The function sends the SetNotificationRegistration command to the MSE which will then try to connect
  to the MNS.

- Notification dispatcher (that dispatches event reports by their MAS instance id to the correct client instance)
  The message access part of every client instance registers a notification handler function.
  The MNS will call the associated handler for all received event report.

Since the dispatching needs to be done on the mas instance id, the message access part of the client
needs to be aware of the instance id of the server it connects to. Unfortunately this information is only
available as SDP attribute.

Due to the current transport layer abstraction in obexd, the MAP client has no direct access to SDP.
Therefore I have extended obc_transport to allow concrete transports (like Bluetooth) to specify
a get_service_record function and made it available through obc_session_get_service_record.
This way, transports can provide additional service information to the client implementations.
The bluetooth transport will now always do a sdp lookup (if the driver's obc_driver->force_service_lookup is set).
In addition it will cache sdp record associated with the connection and make it accessible in get_service_record.

Christian Fetzer (8):
  obexd: Add get_service_record to obc_transport
  obexd: Provide SDP record in get_service_record
  obexd: Read MAP client SDP attributes
  obexd: Add Message Notification Server (MNS)
  obexd: Add RegisterNotifications function
  obexd: Add MAP notification dispatcher
  obexd: Register MAP notification handler
  obexd: Notify registered notification handlers

 Makefile.obexd                |   4 +
 doc/obex-api.txt              |   6 +
 lib/sdp.h                     |   2 +
 obexd/client/bluetooth.c      |  51 ++++++-
 obexd/client/driver.h         |   1 +
 obexd/client/map-dispatcher.c |  88 +++++++++++
 obexd/client/map-dispatcher.h |  47 ++++++
 obexd/client/map-event.h      |  42 +++++
 obexd/client/map.c            | 146 +++++++++++++++++-
 obexd/client/mns.c            | 346 ++++++++++++++++++++++++++++++++++++++++++
 obexd/client/session.c        |  11 +-
 obexd/client/session.h        |   2 +
 obexd/client/transport.h      |   2 +
 obexd/plugins/bluetooth.c     |   2 +
 obexd/src/obexd.h             |   1 +
 src/profile.c                 |  55 +++++++
 16 files changed, 795 insertions(+), 11 deletions(-)
 create mode 100644 obexd/client/map-dispatcher.c
 create mode 100644 obexd/client/map-dispatcher.h
 create mode 100644 obexd/client/map-event.h
 create mode 100644 obexd/client/mns.c

-- 
1.8.1.5

[PATCH v3 1/8] obexd: Add get_service_record to obc_transport

[Christian Fetzer]

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

This adds a get_service_record function pointer to obc_transport,
which allows transports to provide the session specific service record.
This record can be queried using obc_session_get_service_record.
---
 obexd/client/session.c   | 6 ++++++
 obexd/client/session.h   | 2 ++
 obexd/client/transport.h | 1 +
 3 files changed, 9 insertions(+)

diff --git a/obexd/client/session.c b/obexd/client/session.c
index f677bcb..809279a 100644
--- a/obexd/client/session.c
+++ b/obexd/client/session.c
@@ -827,6 +827,12 @@ fail:
 	return NULL;
 }
 
+const void *obc_session_get_service_record(struct obc_session *session)
+{
+	if (session->id > 0 && session->transport != NULL)
+		return session->transport->get_service_record(session->id);
+}
+
 const char *obc_session_get_owner(struct obc_session *session)
 {
 	if (session == NULL)
diff --git a/obexd/client/session.h b/obexd/client/session.h
index 402c042..8a3d629 100644
--- a/obexd/client/session.h
+++ b/obexd/client/session.h
@@ -54,6 +54,8 @@ const char *obc_session_get_target(struct obc_session *session);
 const char *obc_session_register(struct obc_session *session,
 						GDBusDestroyFunction destroy);
 
+const void *obc_session_get_service_record(struct obc_session *session);
+
 guint obc_session_queue(struct obc_session *session,
 				struct obc_transfer *transfer,
 				session_callback_t func, void *user_data,
diff --git a/obexd/client/transport.h b/obexd/client/transport.h
index 5140840..eb29bf3 100644
--- a/obexd/client/transport.h
+++ b/obexd/client/transport.h
@@ -31,6 +31,7 @@ struct obc_transport {
 				obc_transport_func func, void *user_data);
 	int (*getpacketopt) (GIOChannel *io, int *tx_mtu, int *rx_mtu);
 	void (*disconnect) (guint id);
+	const void *(*get_service_record)(guint id);
 };
 
 int obc_transport_register(struct obc_transport *transport);
-- 
1.8.1.5

[PATCH v3 2/8] obexd: Provide SDP record in get_service_record

[Christian Fetzer]

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

This caches the SDP record of the active session in the Bluetooth transport
and makes the data available for clients in get_service_record.
The cached SDP record can be retrieved using obc_session_get_service_record.

To have the SDP record also available when directly connecting by port,
the profile implementation has to specify force_service_lookup in its
obc_driver struct.

Profiles like MAP specify additional SDP attributes that are of interest
for the profile implementation.
---
 obexd/client/bluetooth.c | 51 +++++++++++++++++++++++++++++++++++++++++-------
 obexd/client/driver.h    |  1 +
 obexd/client/session.c   |  5 +++--
 obexd/client/transport.h |  1 +
 4 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/obexd/client/bluetooth.c b/obexd/client/bluetooth.c
index a832a3f..07c2228 100644
--- a/obexd/client/bluetooth.c
+++ b/obexd/client/bluetooth.c
@@ -51,7 +51,9 @@ struct bluetooth_session {
 	bdaddr_t src;
 	bdaddr_t dst;
 	uint16_t port;
+	gboolean force_service_lookup;
 	sdp_session_t *sdp;
+	sdp_record_t *sdp_record;
 	GIOChannel *io;
 	char *service;
 	obc_transport_func func;
@@ -82,6 +84,9 @@ static void session_destroy(struct bluetooth_session *session)
 	if (session->sdp)
 		sdp_close(session->sdp);
 
+	if (session->sdp_record)
+		sdp_record_free(session->sdp_record);
+
 	g_free(session->service);
 	g_free(session);
 }
@@ -160,7 +165,7 @@ static void search_callback(uint8_t type, uint16_t status,
 		sdp_record_t *rec;
 		sdp_list_t *protos;
 		sdp_data_t *data;
-		int recsize, ch = -1;
+		int recsize, ch = -1, psm = -1;
 
 		recsize = 0;
 		rec = sdp_extract_pdu(rsp, bytesleft, &recsize);
@@ -183,15 +188,31 @@ static void search_callback(uint8_t type, uint16_t status,
 		data = sdp_data_get(rec, 0x0200);
 		/* PSM must be odd and lsb of upper byte must be 0 */
 		if (data != NULL && (data->val.uint16 & 0x0101) == 0x0001)
-			ch = data->val.uint16;
-
-		sdp_record_free(rec);
+			psm = data->val.uint16;
+
+		/* Preferably connect using L2CAP directly instead of RFCOMM.
+		 * If the session has already a port specified, use that to
+		 * connect. */
+		if (session->port == 0) {
+			if (psm > 0)
+				port = psm;
+			else if (ch > 0)
+				port = ch;
+		} else if (session->port > 0 &&
+				(session->port == ch || session->port == psm)) {
+			port = session->port;
+		}
 
-		if (ch > 0) {
-			port = ch;
+		/* Cache the sdp record associated with the service that we
+		 * attempt to connect. This allows reading it's application
+		 * specific parameters with get_service_record. */
+		if (port > 0) {
+			session->sdp_record = rec;
 			break;
 		}
 
+		sdp_record_free(rec);
+
 		scanned += recsize;
 		rsp += recsize;
 		bytesleft -= recsize;
@@ -362,7 +383,7 @@ static int session_connect(struct bluetooth_session *session)
 
 	DBG("session %p", session);
 
-	if (session->port > 0) {
+	if (session->port > 0 && !session->force_service_lookup) {
 		session->io = transport_connect(&session->src, &session->dst,
 							session->port,
 							transport_callback,
@@ -379,6 +400,7 @@ static int session_connect(struct bluetooth_session *session)
 
 static guint bluetooth_connect(const char *source, const char *destination,
 				const char *service, uint16_t port,
+				gboolean force_service_lookup,
 				obc_transport_func func, void *user_data)
 {
 	struct bluetooth_session *session;
@@ -398,6 +420,7 @@ static guint bluetooth_connect(const char *source, const char *destination,
 	session->func = func;
 	session->port = port;
 	session->user_data = user_data;
+	session->force_service_lookup = force_service_lookup;
 
 	session->service = g_strdup(service);
 	str2ba(destination, &session->dst);
@@ -459,11 +482,25 @@ static int bluetooth_getpacketopt(GIOChannel *io, int *tx_mtu, int *rx_mtu)
 	return 0;
 }
 
+static const void *get_bluetooth_service_record(guint id)
+{
+	GSList *l;
+
+	for (l = sessions; l; l = l->next) {
+		struct bluetooth_session *session = l->data;
+
+		if (session->id == id)
+			return session->sdp_record;
+	}
+	return NULL;
+}
+
 static struct obc_transport bluetooth = {
 	.name = "Bluetooth",
 	.connect = bluetooth_connect,
 	.getpacketopt = bluetooth_getpacketopt,
 	.disconnect = bluetooth_disconnect,
+	.get_service_record = get_bluetooth_service_record,
 };
 
 int bluetooth_init(void)
diff --git a/obexd/client/driver.h b/obexd/client/driver.h
index f1c0646..08ea09b 100644
--- a/obexd/client/driver.h
+++ b/obexd/client/driver.h
@@ -26,6 +26,7 @@ struct obc_driver {
 	const char *uuid;
 	void *target;
 	gsize target_len;
+	gboolean force_service_lookup;
 	int (*probe) (struct obc_session *session);
 	void (*remove) (struct obc_session *session);
 };
diff --git a/obexd/client/session.c b/obexd/client/session.c
index 809279a..514b95d 100644
--- a/obexd/client/session.c
+++ b/obexd/client/session.c
@@ -407,8 +407,9 @@ static int session_connect(struct obc_session *session,
 	}
 
 	session->id = transport->connect(session->source, session->destination,
-					driver->uuid, session->channel,
-					transport_func, callback);
+						driver->uuid, session->channel,
+						driver->force_service_lookup,
+						transport_func, callback);
 	if (session->id == 0) {
 		obc_session_unref(callback->session);
 		g_free(callback);
diff --git a/obexd/client/transport.h b/obexd/client/transport.h
index eb29bf3..b131c50 100644
--- a/obexd/client/transport.h
+++ b/obexd/client/transport.h
@@ -28,6 +28,7 @@ struct obc_transport {
 	const char *name;
 	guint (*connect) (const char *source, const char *destination,
 				const char *service, uint16_t port,
+				gboolean force_service_lookup,
 				obc_transport_func func, void *user_data);
 	int (*getpacketopt) (GIOChannel *io, int *tx_mtu, int *rx_mtu);
 	void (*disconnect) (guint id);
-- 
1.8.1.5

[PATCH v3 3/8] obexd: Read MAP client SDP attributes

[Christian Fetzer]

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

Parse SDP attributes mas_instance_id and supported_message_types from
the transport's service attributes as soon as a connection is
established.
---
 lib/sdp.h          |  2 ++
 obexd/client/map.c | 35 +++++++++++++++++++++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/lib/sdp.h b/lib/sdp.h
index baae15f..4ef2176 100644
--- a/lib/sdp.h
+++ b/lib/sdp.h
@@ -294,6 +294,8 @@ extern "C" {
 #define SDP_ATTR_SUPPORTED_FUNCTIONS		0x0312
 #define SDP_ATTR_TOTAL_IMAGING_DATA_CAPACITY	0x0313
 #define SDP_ATTR_SUPPORTED_REPOSITORIES		0x0314
+#define SDP_ATTR_MAS_INSTANCE_ID		0x0315
+#define SDP_ATTR_SUPPORTED_MESSAGE_TYPES	0x0316
 
 #define SDP_ATTR_SPECIFICATION_ID		0x0200
 #define SDP_ATTR_VENDOR_ID			0x0201
diff --git a/obexd/client/map.c b/obexd/client/map.c
index 2b665ec..669ba9b 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -39,6 +39,8 @@
 #include "transfer.h"
 #include "session.h"
 #include "driver.h"
+#include "sdp.h"
+#include "sdp_lib.h"
 
 #define OBEX_MAS_UUID \
 	"\xBB\x58\x2B\x40\x42\x0C\x11\xDB\xB0\xDE\x08\x00\x20\x0C\x9A\x66"
@@ -94,6 +96,8 @@ struct map_data {
 	struct obc_session *session;
 	DBusMessage *msg;
 	GHashTable *messages;
+	int16_t mas_instance_id;
+	uint8_t supported_message_types;
 };
 
 #define MAP_MSG_FLAG_PRIORITY	0x01
@@ -1699,6 +1703,30 @@ static void map_free(void *data)
 	g_free(map);
 }
 
+static void parse_service_record(struct map_data *map)
+{
+	sdp_data_t *data;
+	const sdp_record_t *rec = obc_session_get_service_record(map->session);
+
+	if (!rec)
+		return;
+
+	/* MAS instance id */
+	map->mas_instance_id = -1;
+	data = sdp_data_get(rec, SDP_ATTR_MAS_INSTANCE_ID);
+	if (data != NULL)
+		map->mas_instance_id = data->val.uint8;
+	else
+		DBG("Failed to read MAS instance id");
+
+	/* Supported Message Types */
+	data = sdp_data_get(rec, SDP_ATTR_SUPPORTED_MESSAGE_TYPES);
+	if (data != NULL)
+		map->supported_message_types = data->val.uint8;
+	else
+		DBG("Failed to read supported message types");
+}
+
 static int map_probe(struct obc_session *session)
 {
 	struct map_data *map;
@@ -1706,8 +1734,6 @@ static int map_probe(struct obc_session *session)
 
 	path = obc_session_get_path(session);
 
-	DBG("%s", path);
-
 	map = g_try_new0(struct map_data, 1);
 	if (!map)
 		return -ENOMEM;
@@ -1716,6 +1742,10 @@ static int map_probe(struct obc_session *session)
 	map->messages = g_hash_table_new_full(g_str_hash, g_str_equal, NULL,
 								map_msg_remove);
 
+	parse_service_record(map);
+
+	DBG("%s, instance id %d", path, map->mas_instance_id);
+
 	if (!g_dbus_register_interface(conn, path, MAP_INTERFACE, map_methods,
 					NULL, NULL, map, map_free)) {
 		map_free(map);
@@ -1740,6 +1770,7 @@ static struct obc_driver map = {
 	.uuid = MAS_UUID,
 	.target = OBEX_MAS_UUID,
 	.target_len = OBEX_MAS_UUID_LEN,
+	.force_service_lookup = TRUE,
 	.probe = map_probe,
 	.remove = map_remove
 };
-- 
1.8.1.5

[PATCH v3 4/8] obexd: Add Message Notification Server (MNS)

[Christian Fetzer]

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

This implements the MAP Message Notification Server (MNS)
which is part of the MCE.
---
 Makefile.obexd            |   3 +
 obexd/client/map-event.h  |  42 ++++++
 obexd/client/mns.c        | 348 ++++++++++++++++++++++++++++++++++++++++++++++
 obexd/plugins/bluetooth.c |   2 +
 obexd/src/obexd.h         |   1 +
 src/profile.c             |  55 ++++++++
 6 files changed, 451 insertions(+)
 create mode 100644 obexd/client/map-event.h
 create mode 100644 obexd/client/mns.c

diff --git a/Makefile.obexd b/Makefile.obexd
index ae05ae9..5824e0a 100644
--- a/Makefile.obexd
+++ b/Makefile.obexd
@@ -48,6 +48,9 @@ obexd_builtin_sources += obexd/plugins/mas.c obexd/src/map_ap.h \
 				obexd/plugins/messages.h \
 				obexd/plugins/messages-dummy.c
 
+obexd_builtin_modules += mns
+obexd_builtin_sources += obexd/client/mns.c obexd/src/map_ap.h \
+				obexd/client/map-event.h
 
 libexec_PROGRAMS += obexd/src/obexd
 
diff --git a/obexd/client/map-event.h b/obexd/client/map-event.h
new file mode 100644
index 0000000..749f1e0
--- /dev/null
+++ b/obexd/client/map-event.h
@@ -0,0 +1,42 @@
+/*
+ *
+ *  OBEX
+ *
+ *  Copyright (C) 2013  BMW Car IT GmbH. All rights reserved.
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+enum map_event_type {
+	MAP_ET_NEW_MESSAGE,
+	MAP_ET_DELIVERY_SUCCESS,
+	MAP_ET_SENDING_SUCCESS,
+	MAP_ET_DELIVERY_FAILURE,
+	MAP_ET_SENDING_FAILURE,
+	MAP_ET_MEMORY_FULL,
+	MAP_ET_MEMORY_AVAILABLE,
+	MAP_ET_MESSAGE_DELETED,
+	MAP_ET_MESSAGE_SHIFT
+};
+
+struct map_event {
+	enum map_event_type type;
+	char *handle;
+	char *folder;
+	char *old_folder;
+	char *msg_type;
+};
diff --git a/obexd/client/mns.c b/obexd/client/mns.c
new file mode 100644
index 0000000..81550ef
--- /dev/null
+++ b/obexd/client/mns.c
@@ -0,0 +1,348 @@
+/*
+ *
+ *  OBEX Server
+ *
+ *  Copyright (C) 2013  BMW Car IT GmbH. All rights reserved.
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <string.h>
+#include <errno.h>
+#include <glib.h>
+#include <fcntl.h>
+#include <inttypes.h>
+
+#include <gobex/gobex.h>
+#include <gobex/gobex-apparam.h>
+
+#include "obexd.h"
+#include "plugin.h"
+#include "log.h"
+#include "obex.h"
+#include "service.h"
+#include "mimetype.h"
+#include "map_ap.h"
+#include "map-event.h"
+
+struct mns_session {
+	GString *buffer;
+	GObexApparam *inparams;
+	uint8_t mas_instance_id;
+};
+
+static const uint8_t MNS_TARGET[TARGET_SIZE] = {
+			0xbb, 0x58, 0x2b, 0x41, 0x42, 0x0c, 0x11, 0xdb,
+			0xb0, 0xde, 0x08, 0x00, 0x20, 0x0c, 0x9a, 0x66  };
+
+static int get_params(struct obex_session *os, struct mns_session *mns)
+{
+	const uint8_t *buffer;
+	ssize_t size;
+
+	size = obex_get_apparam(os, &buffer);
+	if (size < 0)
+		size = 0;
+
+	mns->inparams = g_obex_apparam_decode(buffer, size);
+	if (mns->inparams == NULL) {
+		DBG("Error when parsing parameters!");
+		return -EBADR;
+	}
+
+	return 0;
+}
+
+static void reset_request(struct mns_session *mns)
+{
+	if (mns->buffer) {
+		g_string_free(mns->buffer, TRUE);
+		mns->buffer = NULL;
+	}
+
+	if (mns->inparams) {
+		g_obex_apparam_free(mns->inparams);
+		mns->inparams = NULL;
+	}
+}
+
+static void mns_session_free(struct mns_session *mns)
+{
+	reset_request(mns);
+	g_free(mns);
+}
+
+static void *mns_connect(struct obex_session *os, int *err)
+{
+	struct mns_session *mns;
+
+	DBG("");
+
+	mns = g_new0(struct mns_session, 1);
+
+	manager_register_session(os);
+
+	return mns;
+}
+
+static void mns_disconnect(struct obex_session *os, void *user_data)
+{
+	struct mns_session *mns = user_data;
+
+	DBG("");
+
+	manager_unregister_session(os);
+
+	mns_session_free(mns);
+}
+
+static int mns_put(struct obex_session *os, void *user_data)
+{
+	struct mns_session *mns = user_data;
+	const char *type = obex_get_type(os);
+	const char *name = obex_get_name(os);
+	int ret;
+
+	DBG("PUT: name %s type %s mns %p", name, type, mns);
+
+	if (type == NULL)
+		return -EBADR;
+
+	ret = get_params(os, mns);
+	if (ret < 0)
+		goto failed;
+
+	ret = obex_put_stream_start(os, name);
+	if (ret < 0)
+		goto failed;
+
+	return 0;
+
+failed:
+	reset_request(mns);
+
+	return ret;
+}
+
+static void parse_event_report_type(struct map_event *event, const char *value)
+{
+	if (!g_ascii_strcasecmp(value, "NewMessage"))
+		event->type = MAP_ET_NEW_MESSAGE;
+	else if (!g_ascii_strcasecmp(value, "DeliverySuccess"))
+		event->type = MAP_ET_DELIVERY_SUCCESS;
+	else if (!g_ascii_strcasecmp(value, "SendingSuccess"))
+		event->type = MAP_ET_SENDING_SUCCESS;
+	else if (!g_ascii_strcasecmp(value, "DeliveryFailure"))
+		event->type = MAP_ET_DELIVERY_FAILURE;
+	else if (!g_ascii_strcasecmp(value, "SendingFailure"))
+		event->type = MAP_ET_SENDING_FAILURE;
+	else if (!g_ascii_strcasecmp(value, "MemoryFull"))
+		event->type = MAP_ET_MEMORY_FULL;
+	else if (!g_ascii_strcasecmp(value, "MemoryAvailable"))
+		event->type = MAP_ET_MEMORY_AVAILABLE;
+	else if (!g_ascii_strcasecmp(value, "MessageDeleted"))
+		event->type = MAP_ET_MESSAGE_DELETED;
+	else if (!g_ascii_strcasecmp(value, "MessageShift"))
+		event->type = MAP_ET_MESSAGE_SHIFT;
+}
+
+static void parse_event_report_handle(struct map_event *event,
+							const char *value)
+{
+	event->handle = g_strdup(value);
+}
+
+static void parse_event_report_folder(struct map_event *event,
+							const char *value)
+{
+	event->folder = g_strdup(value);
+}
+
+static void parse_event_report_old_folder(struct map_event *event,
+							const char *value)
+{
+	event->old_folder = g_strdup(value);
+}
+
+static void parse_event_report_msg_type(struct map_event *event,
+							const char *value)
+{
+	event->msg_type = g_strdup(value);
+}
+
+static struct map_event_report_parser {
+	const char *name;
+	void (*func) (struct map_event *event, const char *value);
+} event_report_parsers[] = {
+		{ "type", parse_event_report_type },
+		{ "handle", parse_event_report_handle },
+		{ "folder", parse_event_report_folder },
+		{ "old_folder", parse_event_report_old_folder },
+		{ "msg_type", parse_event_report_msg_type },
+		{ }
+};
+
+static void event_report_element(GMarkupParseContext *ctxt,
+				const gchar *element, const gchar **names,
+				const gchar **values, gpointer user_data,
+								GError **gerr)
+{
+	struct map_event *event = user_data;
+	const gchar *key;
+	gint i;
+
+	if (strcasecmp("event", element) != 0)
+		return;
+
+	for (i = 0, key = names[i]; key; key = names[++i]) {
+		struct map_event_report_parser *parser;
+
+		for (parser = event_report_parsers; parser && parser->name;
+								parser++) {
+			if (strcasecmp(key, parser->name) == 0) {
+				parser->func(event, values[i]);
+				break;
+			}
+		}
+	}
+}
+
+static const GMarkupParser event_report_parser = {
+	event_report_element,
+	NULL,
+	NULL,
+	NULL,
+	NULL
+};
+
+static void map_event_free(struct map_event *event)
+{
+	g_free(event->handle);
+	g_free(event->folder);
+	g_free(event->old_folder);
+	g_free(event->msg_type);
+	g_free(event);
+}
+
+static void *event_report_open(const char *name, int oflag, mode_t mode,
+				void *driver_data, size_t *size, int *err)
+{
+	struct mns_session *mns = driver_data;
+
+	DBG("");
+
+	g_obex_apparam_get_uint8(mns->inparams, MAP_AP_MASINSTANCEID,
+							&mns->mas_instance_id);
+
+	mns->buffer = g_string_new("");
+
+	if (*err < 0)
+		return NULL;
+	else
+		return mns;
+}
+
+static int event_report_close(void *obj)
+{
+	struct mns_session *mns = obj;
+	GMarkupParseContext *ctxt;
+	struct map_event *event;
+
+	event = g_new0(struct map_event, 1);
+	ctxt = g_markup_parse_context_new(&event_report_parser, 0, event,
+									NULL);
+	g_markup_parse_context_parse(ctxt, mns->buffer->str, mns->buffer->len,
+									NULL);
+	g_markup_parse_context_free(ctxt);
+
+	DBG("Received event report for instance %d", mns->mas_instance_id);
+	DBG("type=%x, handle=%s, folder=%s, old_folder=%s, msg_type=%s",
+				event->type, event->handle, event->folder,
+					event->old_folder, event->msg_type);
+
+	map_event_free(event);
+
+	reset_request(mns);
+
+	return 0;
+}
+
+static ssize_t event_report_write(void *obj, const void *buf, size_t count)
+{
+	struct mns_session *mns = obj;
+
+	DBG("");
+
+	g_string_append_len(mns->buffer, buf, count);
+	return count;
+}
+
+static struct obex_service_driver mns = {
+	.name = "Message Notification server",
+	.service = OBEX_MNS,
+	.target = MNS_TARGET,
+	.target_size = TARGET_SIZE,
+	.connect = mns_connect,
+	.put = mns_put,
+	.disconnect = mns_disconnect,
+};
+
+static struct obex_mime_type_driver mime_event_report = {
+	.target = MNS_TARGET,
+	.target_size = TARGET_SIZE,
+	.mimetype = "x-bt/MAP-event-report",
+	.open = event_report_open,
+	.close = event_report_close,
+	.write = event_report_write,
+};
+
+static struct obex_mime_type_driver *mas_drivers[] = {
+	&mime_event_report,
+	NULL
+};
+
+static int mns_init(void)
+{
+	int err;
+
+	err = obex_mime_type_driver_register(&mime_event_report);
+	if (err < 0)
+		goto fail_mime_event;
+
+	err = obex_service_driver_register(&mns);
+	if (err < 0)
+		goto fail_mns_reg;
+
+	return 0;
+
+fail_mns_reg:
+	obex_mime_type_driver_unregister(&mime_event_report);
+fail_mime_event:
+	return err;
+}
+
+static void mns_exit(void)
+{
+	obex_service_driver_unregister(&mns);
+	obex_mime_type_driver_unregister(&mime_event_report);
+}
+
+OBEX_PLUGIN_DEFINE(mns, mns_init, mns_exit)
diff --git a/obexd/plugins/bluetooth.c b/obexd/plugins/bluetooth.c
index f80faa4..b9e9c91 100644
--- a/obexd/plugins/bluetooth.c
+++ b/obexd/plugins/bluetooth.c
@@ -336,6 +336,8 @@ static const char *service2uuid(uint16_t service)
 		return "00000002-0000-1000-8000-0002ee000002";
 	case OBEX_MAS:
 		return OBEX_MAS_UUID;
+	case OBEX_MNS:
+		return OBEX_MNS_UUID;
 	}
 
 	return NULL;
diff --git a/obexd/src/obexd.h b/obexd/src/obexd.h
index 1c9b2b3..42c3c4d 100644
--- a/obexd/src/obexd.h
+++ b/obexd/src/obexd.h
@@ -29,6 +29,7 @@
 #define OBEX_PCSUITE	(1 << 6)
 #define OBEX_SYNCEVOLUTION	(1 << 7)
 #define OBEX_MAS	(1 << 8)
+#define OBEX_MNS	(1 << 9)
 
 gboolean plugin_init(const char *pattern, const char *exclude);
 void plugin_cleanup(void);
diff --git a/src/profile.c b/src/profile.c
index 29f9ee6..55e72dd 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -435,6 +435,44 @@
 		</attribute>						\
 	</record>"
 
+#define MNS_RECORD							\
+	"<?xml version=\"1.0\" encoding=\"UTF-8\" ?>			\
+	<record>							\
+		<attribute id=\"0x0001\">				\
+			<sequence>					\
+				<uuid value=\"0x1133\"/>		\
+			</sequence>					\
+		</attribute>						\
+		<attribute id=\"0x0004\">				\
+			<sequence>					\
+				<sequence>				\
+					<uuid value=\"0x0100\"/>	\
+				</sequence>				\
+				<sequence>				\
+					<uuid value=\"0x0003\"/>	\
+					<uint8 value=\"0x%02x\"/>	\
+				</sequence>				\
+				<sequence>				\
+					<uuid value=\"0x0008\"/>	\
+				</sequence>				\
+			</sequence>					\
+		</attribute>						\
+		<attribute id=\"0x0009\">				\
+			<sequence>					\
+				<sequence>				\
+					<uuid value=\"0x1134\"/>	\
+					<uint16 value=\"0x%04x\"/>	\
+				</sequence>				\
+			</sequence>					\
+		</attribute>						\
+		<attribute id=\"0x0100\">				\
+			<text value=\"%s\"/>				\
+		</attribute>						\
+		<attribute id=\"0x0200\">				\
+			<uint16 value=\"%u\" name=\"psm\"/>		\
+		</attribute>						\
+	</record>"
+
 #define SYNC_RECORD							\
 	"<?xml version=\"1.0\" encoding=\"UTF-8\" ?>			\
 	<record>							\
@@ -1709,6 +1747,20 @@ static char *get_mas_record(struct ext_profile *ext, struct ext_io *l2cap,
 								ext->name);
 }
 
+static char *get_mns_record(struct ext_profile *ext, struct ext_io *l2cap,
+							struct ext_io *rfcomm)
+{
+	uint16_t psm = 0;
+	uint8_t chan = 0;
+
+	if (l2cap)
+		psm = l2cap->psm;
+	if (rfcomm)
+		chan = rfcomm->chan;
+
+	return g_strdup_printf(MNS_RECORD, chan, ext->version, ext->name, psm);
+}
+
 static char *get_sync_record(struct ext_profile *ext, struct ext_io *l2cap,
 							struct ext_io *rfcomm)
 {
@@ -1899,6 +1951,9 @@ static struct default_settings {
 		.uuid		= OBEX_MNS_UUID,
 		.name		= "Message Notification",
 		.channel	= MNS_DEFAULT_CHANNEL,
+		.psm		= BTD_PROFILE_PSM_AUTO,
+		.get_record	= get_mns_record,
+		.version	= 0x0100
 	},
 };
 
-- 
1.8.1.5

[PATCH v3 5/8] obexd: Add RegisterNotifications function

[Christian Fetzer]

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

This allows applications to register for all different types
of MAP event reports.

In response to this call, the MSE should connect to the local MNS
instance.
---
 doc/obex-api.txt   |  6 ++++
 obexd/client/map.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)

diff --git a/doc/obex-api.txt b/doc/obex-api.txt
index 759c4d8..ef5d85e 100644
--- a/doc/obex-api.txt
+++ b/doc/obex-api.txt
@@ -651,6 +651,12 @@ Methods		void SetFolder(string name)
 			Possible errors: org.bluez.obex.Error.InvalidArguments
 					 org.bluez.obex.Error.Failed
 
+		void RegisterNotifications(boolean)
+
+			Register / unregister reception of notifications.
+
+			Possible errors: org.bluez.obex.Error.InvalidArguments
+					 org.bluez.obex.Error.Failed
 
 Filter:		uint16 Offset:
 
diff --git a/obexd/client/map.c b/obexd/client/map.c
index 669ba9b..5b69978 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -1512,6 +1512,7 @@ static DBusMessage *push_message(struct map_data *map,
 
 	transfer = obc_transfer_put("x-bt/message", folder, filename,
 								NULL, 0, &err);
+
 	if (transfer == NULL) {
 		g_obex_apparam_free(apparam);
 		goto fail;
@@ -1654,6 +1655,87 @@ static DBusMessage *map_push_message(DBusConnection *connection,
 	return push_message(map, message, filename, folder, apparam);
 }
 
+static void notification_registration_cb(struct obc_session *session,
+				struct obc_transfer *transfer,
+				GError *err, void *user_data)
+{
+	struct map_data *map = user_data;
+	DBusMessage *reply;
+
+	if (err != NULL) {
+		reply = g_dbus_create_error(map->msg,
+						ERROR_INTERFACE ".Failed",
+						"%s", err->message);
+		goto done;
+	}
+
+	reply = dbus_message_new_method_return(map->msg);
+
+done:
+	g_dbus_send_message(conn, reply);
+	dbus_message_unref(map->msg);
+}
+
+static DBusMessage *set_notification_registration(struct map_data *map,
+							DBusMessage *message,
+							GObexApparam *apparam)
+{
+	struct obc_transfer *transfer;
+	GError *err = NULL;
+	DBusMessage *reply;
+	char contents[2];
+
+	contents[0] = FILLER_BYTE;
+	contents[1] = '\0';
+
+	transfer = obc_transfer_put("x-bt/MAP-NotificationRegistration", NULL,
+					NULL, contents, sizeof(contents), &err);
+
+	if (transfer == NULL) {
+		g_obex_apparam_free(apparam);
+		goto fail;
+	}
+
+	obc_transfer_set_apparam(transfer, apparam);
+
+	if (obc_session_queue(map->session, transfer,
+				notification_registration_cb, map, &err)) {
+		map->msg = dbus_message_ref(message);
+		return NULL;
+	}
+
+fail:
+	reply = g_dbus_create_error(message, ERROR_INTERFACE ".Failed", "%s",
+								err->message);
+	g_error_free(err);
+	return reply;
+}
+
+
+static DBusMessage *map_register_notifications(DBusConnection *connection,
+					DBusMessage *message, void *user_data)
+{
+	struct map_data *map = user_data;
+	GObexApparam *apparam;
+	gboolean status;
+
+	if (map->mas_instance_id < 0)
+		return g_dbus_create_error(message,
+					ERROR_INTERFACE ".Failed",
+					"Unknown MAS instance id");
+
+	if (dbus_message_get_args(message, NULL, DBUS_TYPE_BOOLEAN, &status,
+					DBUS_TYPE_INVALID) == FALSE)
+		return g_dbus_create_error(message,
+					ERROR_INTERFACE ".InvalidArguments",
+					NULL);
+
+	apparam = g_obex_apparam_set_uint8(NULL, MAP_AP_NOTIFICATIONSTATUS,
+							status ? 0x01 : 0x00);
+
+	return set_notification_registration(map, message, apparam);
+}
+
 static const GDBusMethodTable map_methods[] = {
 	{ GDBUS_ASYNC_METHOD("SetFolder",
 				GDBUS_ARGS({ "name", "s" }), NULL,
@@ -1680,6 +1762,10 @@ static const GDBusMethodTable map_methods[] = {
 			GDBUS_ARGS({ "transfer", "o" },
 						{ "properties", "a{sv}" }),
 			map_push_message) },
+	{ GDBUS_ASYNC_METHOD("RegisterNotifications",
+			GDBUS_ARGS({ "status", "b" }),
+			NULL,
+			map_register_notifications) },
 	{ }
 };
 
-- 
1.8.1.5

[PATCH v3 6/8] obexd: Add MAP notification dispatcher

[Christian Fetzer]

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

The MAP specification allows to reuse one MNS instance for all local
MAS client instances. This dispatching of event reports to the
correct MAS client instance is done by the instance id.

The dispatcher component allows MAP client instances to
register a notification handler (map_aquire_mns).
Events reports are then forwarded by the notification server
using (mns_dispatch_notification).
---
 Makefile.obexd                |  1 +
 obexd/client/map-dispatcher.c | 88 +++++++++++++++++++++++++++++++++++++++++++
 obexd/client/map-dispatcher.h | 47 +++++++++++++++++++++++
 3 files changed, 136 insertions(+)
 create mode 100644 obexd/client/map-dispatcher.c
 create mode 100644 obexd/client/map-dispatcher.h

diff --git a/Makefile.obexd b/Makefile.obexd
index 5824e0a..02901a3 100644
--- a/Makefile.obexd
+++ b/Makefile.obexd
@@ -73,6 +73,7 @@ obexd_src_obexd_SOURCES = $(gdbus_sources) $(btio_sources) $(gobex_sources) \
 			obexd/client/ftp.h obexd/client/ftp.c \
 			obexd/client/opp.h obexd/client/opp.c \
 			obexd/client/map.h obexd/client/map.c \
+			obexd/client/map-dispatcher.h obexd/client/map-dispatcher.c \
 			obexd/client/transfer.h obexd/client/transfer.c \
 			obexd/client/transport.h obexd/client/transport.c \
 			obexd/client/dbus.h obexd/client/dbus.c \
diff --git a/obexd/client/map-dispatcher.c b/obexd/client/map-dispatcher.c
new file mode 100644
index 0000000..911e062
--- /dev/null
+++ b/obexd/client/map-dispatcher.c
@@ -0,0 +1,88 @@
+/*
+ *
+ *  OBEX
+ *
+ *  Copyright (C) 2013  BMW Car IT GmbH. All rights reserved.
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <glib.h>
+
+#include "log.h"
+#include "map-dispatcher.h"
+
+static GSList *mappings = NULL;
+
+struct mns_mapping {
+	int mas_instance_id;
+	map_handle_notification_func handler;
+	void *user_data;
+};
+
+gboolean map_acquire_mns(int mas_instance_id,
+			map_handle_notification_func handler, void *user_data)
+{
+	struct mns_mapping *mapping;
+
+	mapping = g_try_new0(struct mns_mapping, 1);
+	if (mapping == NULL)
+		return FALSE;
+
+	mapping->mas_instance_id = mas_instance_id;
+	mapping->handler = handler;
+	mapping->user_data = user_data;
+
+	mappings = g_slist_append(mappings, mapping);
+	DBG("Added MAP MNS mapping for instance %d", mas_instance_id);
+
+	return TRUE;
+}
+
+static struct mns_mapping *find_mapping(int mas_instance_id)
+{
+	GSList *list;
+
+	for (list = mappings; list; list = list->next) {
+		struct mns_mapping *mapping = list->data;
+		if (mapping->mas_instance_id == mas_instance_id)
+			return mapping;
+	}
+
+	DBG("Cannot find MAP MNS mapping for instance %d", mas_instance_id);
+	return NULL;
+}
+
+void map_release_mns(int mas_instance_id)
+{
+	struct mns_mapping *mapping = find_mapping(mas_instance_id);
+	if (mapping) {
+		mappings = g_slist_remove(mappings, mapping);
+		DBG("Removed MAP MNS mapping for instance %d", mas_instance_id);
+	}
+}
+
+void mns_dispatch_notification(int mas_instance_id, struct map_event *event)
+{
+	struct mns_mapping *mapping = find_mapping(mas_instance_id);
+	if (mapping)
+		mapping->handler(event, mapping->user_data);
+}
diff --git a/obexd/client/map-dispatcher.h b/obexd/client/map-dispatcher.h
new file mode 100644
index 0000000..3664e01
--- /dev/null
+++ b/obexd/client/map-dispatcher.h
@@ -0,0 +1,47 @@
+/*
+ *
+ *  OBEX
+ *
+ *  Copyright (C) 2013  BMW Car IT GmbH. All rights reserved.
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include "map-event.h"
+
+/* Handle notification in map client.
+ *
+ * event: Event report.
+ *
+ * Callback shall be called for every received event.
+ */
+typedef void (*map_handle_notification_func)(struct map_event *event,
+							void *user_data);
+
+/* Registers client notification handler callback for events that are
+ * addressed to the given mas instance id.
+ */
+gboolean map_acquire_mns(int mas_instance_id, map_handle_notification_func,
+							void *user_data);
+
+/* Unregisters client notification handler callback.
+ */
+void map_release_mns(int mas_instance_id);
+
+/* Dispatch notification to a registered notification handler callback.
+ */
+void mns_dispatch_notification(int mas_instance_id, struct map_event *event);
-- 
1.8.1.5

[PATCH v3 7/8] obexd: Register MAP notification handler

[Christian Fetzer]

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

---
 obexd/client/map.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/obexd/client/map.c b/obexd/client/map.c
index 5b69978..7e73471 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -41,6 +41,7 @@
 #include "driver.h"
 #include "sdp.h"
 #include "sdp_lib.h"
+#include "map-dispatcher.h"
 
 #define OBEX_MAS_UUID \
 	"\xBB\x58\x2B\x40\x42\x0C\x11\xDB\xB0\xDE\x08\x00\x20\x0C\x9A\x66"
@@ -98,6 +99,7 @@ struct map_data {
 	GHashTable *messages;
 	int16_t mas_instance_id;
 	uint8_t supported_message_types;
+	gboolean mns_acquired;
 };
 
 #define MAP_MSG_FLAG_PRIORITY	0x01
@@ -1711,6 +1713,12 @@ fail:
 	return reply;
 }
 
+static void map_handle_notification(struct map_event *event, void *user_data)
+{
+	DBG("event: type=%x, handle=%s, folder=%s, old_folder=%s, msg_type=%s",
+				event->type, event->handle, event->folder,
+					event->old_folder, event->msg_type);
+}
 
 static DBusMessage *map_register_notifications(DBusConnection *connection,
 					DBusMessage *message, void *user_data)
@@ -1730,9 +1738,23 @@ static DBusMessage *map_register_notifications(DBusConnection *connection,
 					ERROR_INTERFACE ".InvalidArguments",
 					NULL);
 
+	if (map->mns_acquired == status)
+		return g_dbus_create_error(message,
+					ERROR_INTERFACE ".InvalidArguments",
+					NULL);
+
 	apparam = g_obex_apparam_set_uint8(NULL, MAP_AP_NOTIFICATIONSTATUS,
 							status ? 0x01 : 0x00);
 
+	if (status) {
+		map->mns_acquired = TRUE;
+		map_acquire_mns(map->mas_instance_id, &map_handle_notification,
+									map);
+	} else {
+		map->mns_acquired = FALSE;
+		map_release_mns(map->mas_instance_id);
+	}
+
 	return set_notification_registration(map, message, apparam);
 }
 
@@ -1784,6 +1806,9 @@ static void map_free(void *data)
 {
 	struct map_data *map = data;
 
+	if (map->mns_acquired)
+		map_release_mns(map->mas_instance_id);
+
 	obc_session_unref(map->session);
 	g_hash_table_unref(map->messages);
 	g_free(map);
-- 
1.8.1.5

[PATCH v3 8/8] obexd: Notify registered notification handlers

[Christian Fetzer]

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

---
 obexd/client/mns.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/obexd/client/mns.c b/obexd/client/mns.c
index 81550ef..f16d235 100644
--- a/obexd/client/mns.c
+++ b/obexd/client/mns.c
@@ -41,7 +41,7 @@
 #include "service.h"
 #include "mimetype.h"
 #include "map_ap.h"
-#include "map-event.h"
+#include "map-dispatcher.h"
 
 struct mns_session {
 	GString *buffer;
@@ -266,6 +266,8 @@ static int event_report_close(void *obj)
 	GMarkupParseContext *ctxt;
 	struct map_event *event;
 
+	DBG("");
+
 	event = g_new0(struct map_event, 1);
 	ctxt = g_markup_parse_context_new(&event_report_parser, 0, event,
 									NULL);
@@ -273,11 +275,7 @@ static int event_report_close(void *obj)
 									NULL);
 	g_markup_parse_context_free(ctxt);
 
-	DBG("Received event report for instance %d", mns->mas_instance_id);
-	DBG("type=%x, handle=%s, folder=%s, old_folder=%s, msg_type=%s",
-				event->type, event->handle, event->folder,
-					event->old_folder, event->msg_type);
-
+	mns_dispatch_notification(mns->mas_instance_id, event);
 	map_event_free(event);
 
 	reset_request(mns);
-- 
1.8.1.5

Re: [PATCH v3 3/8] obexd: Read MAP client SDP attributes

[Marcel Holtmann]

Hi Christian,

> Parse SDP attributes mas_instance_id and supported_message_types from
> the transport's service attributes as soon as a connection is
> established.
> ---
> lib/sdp.h          |  2 ++
> obexd/client/map.c | 35 +++++++++++++++++++++++++++++++++--
> 2 files changed, 35 insertions(+), 2 deletions(-)

please stop mixing lib/ changes with anything else.

Actually any kind of patch that crosses multiple directories without having a really good reason for it is not a good idea. So do not do that.

Regards

Marcel

Re: [PATCH v3 4/8] obexd: Add Message Notification Server (MNS)

[Marcel Holtmann]

Hi Christian,

> This implements the MAP Message Notification Server (MNS)
> which is part of the MCE.
> ---
> Makefile.obexd            |   3 +
> obexd/client/map-event.h  |  42 ++++++
> obexd/client/mns.c        | 348 ++++++++++++++++++++++++++++++++++++++++++++++
> obexd/plugins/bluetooth.c |   2 +
> obexd/src/obexd.h         |   1 +
> src/profile.c             |  55 ++++++++
> 6 files changed, 451 insertions(+)
> create mode 100644 obexd/client/map-event.h
> create mode 100644 obexd/client/mns.c

same here. Why is this all intermixed with each other. Please learn how to split patches up properly into small digestible ones.

Even additions to the build system can be done after adding the sources. And at no time intermix src/ with obexd/ directory.

Regards

Marcel

Re: [PATCH v3 5/8] obexd: Add RegisterNotifications function

[Marcel Holtmann]

Hi Christian,

> This allows applications to register for all different types
> of MAP event reports.
> 
> In response to this call, the MSE should connect to the local MNS
> instance.
> ---
> doc/obex-api.txt   |  6 ++++
> obexd/client/map.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 92 insertions(+)

never ever intermix doc/ changes with actual code. You really need to learn on how to split patches properly. There is no reason at all that this are not two patches.


> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
> index 759c4d8..ef5d85e 100644
> --- a/doc/obex-api.txt
> +++ b/doc/obex-api.txt
> @@ -651,6 +651,12 @@ Methods		void SetFolder(string name)
> 			Possible errors: org.bluez.obex.Error.InvalidArguments
> 					 org.bluez.obex.Error.Failed
> 
> +		void RegisterNotifications(boolean)
> +
> +			Register / unregister reception of notifications.
> +
> +			Possible errors: org.bluez.obex.Error.InvalidArguments
> +					 org.bluez.obex.Error.Failed

Why don't you describe the boolean variable and what it does. If this is an on/off switch, then we are not doing this stupid thing here.

Simple things like EnableNotifications and DisableNotifications would be better. Also this needs a bit detailed explanations why you thing this is the right API for handling this. I am not even convinced we should be doing it like this.

Regards

Marcel

Re: [PATCH v3 5/8] obexd: Add RegisterNotifications function

[Christian Fetzer]

Hi Marcel,

On 03/11/2013 05:19 PM, Marcel Holtmann wrote:
> Hi Christian,
>
>> This allows applications to register for all different types
>> of MAP event reports.
>>
>> In response to this call, the MSE should connect to the local MNS
>> instance.
>> ---
>> doc/obex-api.txt   |  6 ++++
>> obexd/client/map.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 92 insertions(+)
> never ever intermix doc/ changes with actual code. You really need to learn on how to split patches properly. There is no reason at all that this are not two patches.
>
>
>> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
>> index 759c4d8..ef5d85e 100644
>> --- a/doc/obex-api.txt
>> +++ b/doc/obex-api.txt
>> @@ -651,6 +651,12 @@ Methods		void SetFolder(string name)
>> 			Possible errors: org.bluez.obex.Error.InvalidArguments
>> 					 org.bluez.obex.Error.Failed
>>
>> +		void RegisterNotifications(boolean)
>> +
>> +			Register / unregister reception of notifications.
>> +
>> +			Possible errors: org.bluez.obex.Error.InvalidArguments
>> +					 org.bluez.obex.Error.Failed
> Why don't you describe the boolean variable and what it does. If this is an on/off switch, then we are not doing this stupid thing here.
>
> Simple things like EnableNotifications and DisableNotifications would be better. Also this needs a bit detailed explanations why you thing this is the right API for handling this. I am not even convinced we should be doing it like this.
>
> Regards
>
> Marcel
Thanks for your comments. I'll split the patches and send an updated 
patch set.

Yes, RegisterNotification simply enables / disabled receiving event 
reports. I can change it as suggested.
The reason for having an API function instead of enabling it implicitly 
is that the application can control
for which device it wants to receive event reports.

Event reports can be received for multiple MAP sessions on the same device.
This is used for example when the phone exposes different E-Mail 
accounts as different MAP instances.
But according to the specification, we need to have one MNS (obex 
server) instance for each remote device.

The current code is limited to exactly one static MNS instance, and you 
can receive notifications only for one device.
In the future we could eventually work around that limitation by 
dynamically instantiating MNS instances when
needed.

Br,
Christian

Re: [PATCH v3 5/8] obexd: Add RegisterNotifications function

[Marcel Holtmann]

Hi Christian,

>>> This allows applications to register for all different types
>>> of MAP event reports.
>>> 
>>> In response to this call, the MSE should connect to the local MNS
>>> instance.
>>> ---
>>> doc/obex-api.txt   |  6 ++++
>>> obexd/client/map.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 92 insertions(+)
>> never ever intermix doc/ changes with actual code. You really need to learn on how to split patches properly. There is no reason at all that this are not two patches.
>> 
>> 
>>> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
>>> index 759c4d8..ef5d85e 100644
>>> --- a/doc/obex-api.txt
>>> +++ b/doc/obex-api.txt
>>> @@ -651,6 +651,12 @@ Methods		void SetFolder(string name)
>>> 			Possible errors: org.bluez.obex.Error.InvalidArguments
>>> 					 org.bluez.obex.Error.Failed
>>> 
>>> +		void RegisterNotifications(boolean)
>>> +
>>> +			Register / unregister reception of notifications.
>>> +
>>> +			Possible errors: org.bluez.obex.Error.InvalidArguments
>>> +					 org.bluez.obex.Error.Failed
>> Why don't you describe the boolean variable and what it does. If this is an on/off switch, then we are not doing this stupid thing here.
>> 
>> Simple things like EnableNotifications and DisableNotifications would be better. Also this needs a bit detailed explanations why you thing this is the right API for handling this. I am not even convinced we should be doing it like this.
>> 
>> Regards
>> 
>> Marcel
> Thanks for your comments. I'll split the patches and send an updated patch set.
> 
> Yes, RegisterNotification simply enables / disabled receiving event reports. I can change it as suggested.
> The reason for having an API function instead of enabling it implicitly is that the application can control
> for which device it wants to receive event reports.
> 
> Event reports can be received for multiple MAP sessions on the same device.
> This is used for example when the phone exposes different E-Mail accounts as different MAP instances.
> But according to the specification, we need to have one MNS (obex server) instance for each remote device.
> 
> The current code is limited to exactly one static MNS instance, and you can receive notifications only for one device.
> In the future we could eventually work around that limitation by dynamically instantiating MNS instances when
> needed.

maybe it would a good idea to write this up in an email that just proposes the API changes and additions and we can discuss based on that. However you have to explain on how this API is supposed to be used.

Regards

Marcel

Re: [PATCH v3 5/8] obexd: Add RegisterNotifications function

[Luiz Augusto von Dentz]

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

Hi Christian,


On Wed, Mar 13, 2013 at 1:27 PM, Marcel Holtmann <marcel@holtmann.org>wrote:

> Hi Christian,
>
> >>> This allows applications to register for all different types
> >>> of MAP event reports.
> >>>
> >>> In response to this call, the MSE should connect to the local MNS
> >>> instance.
> >>> ---
> >>> doc/obex-api.txt   |  6 ++++
> >>> obexd/client/map.c | 86
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>> 2 files changed, 92 insertions(+)
> >> never ever intermix doc/ changes with actual code. You really need to
> learn on how to split patches properly. There is no reason at all that this
> are not two patches.
> >>
> >>
> >>> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
> >>> index 759c4d8..ef5d85e 100644
> >>> --- a/doc/obex-api.txt
> >>> +++ b/doc/obex-api.txt
> >>> @@ -651,6 +651,12 @@ Methods                void SetFolder(string name)
> >>>                     Possible errors:
> org.bluez.obex.Error.InvalidArguments
> >>>                                      org.bluez.obex.Error.Failed
> >>>
> >>> +           void RegisterNotifications(boolean)
> >>> +
> >>> +                   Register / unregister reception of notifications.
> >>> +
> >>> +                   Possible errors:
> org.bluez.obex.Error.InvalidArguments
> >>> +                                    org.bluez.obex.Error.Failed
>

Why we are not doing this automatically? The application can opt out by not
connecting to signals, but I see no reason why not to be connected to
register the notification channel automatically when connecting the MAP
session if the remote support it.

-- 
Luiz Augusto von Dentz

[-- Attachment #2: Type: text/html, Size: 2307 bytes --]

Re: [PATCH v3 5/8] obexd: Add RegisterNotifications function

[Christian Fetzer]

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

Hi Luiz,

On 03/13/2013 08:57 PM, Luiz Augusto von Dentz wrote:
> Hi Christian,
>
>
> On Wed, Mar 13, 2013 at 1:27 PM, Marcel Holtmann <marcel@holtmann.org 
> <mailto:marcel@holtmann.org>> wrote:
>
>     Hi Christian,
>
>     >>> This allows applications to register for all different types
>     >>> of MAP event reports.
>     >>>
>     >>> In response to this call, the MSE should connect to the local MNS
>     >>> instance.
>     >>> ---
>     >>> doc/obex-api.txt   |  6 ++++
>     >>> obexd/client/map.c | 86
>     ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>     >>> 2 files changed, 92 insertions(+)
>     >> never ever intermix doc/ changes with actual code. You really
>     need to learn on how to split patches properly. There is no reason
>     at all that this are not two patches.
>     >>
>     >>
>     >>> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
>     >>> index 759c4d8..ef5d85e 100644
>     >>> --- a/doc/obex-api.txt
>     >>> +++ b/doc/obex-api.txt
>     >>> @@ -651,6 +651,12 @@ Methods    void SetFolder(string name)
>     >>>                     Possible errors:
>     org.bluez.obex.Error.InvalidArguments
>     >>>  org.bluez.obex.Error.Failed
>     >>>
>     >>> +           void RegisterNotifications(boolean)
>     >>> +
>     >>> +                   Register / unregister reception of
>     notifications.
>     >>> +
>     >>> +                   Possible errors:
>     org.bluez.obex.Error.InvalidArguments
>     >>> +  org.bluez.obex.Error.Failed
>
>
> Why we are not doing this automatically? The application can opt out 
> by not connecting to signals, but I see no reason why not to be 
> connected to register the notification channel automatically when 
> connecting the MAP session if the remote support it.
>
> -- 
> Luiz Augusto von Dentz

as I tried to explain, the purpose of the function in the current patch 
is to select the one device for that you will receive notifications.
I agree, it would be better to hide the notification connection setup 
and rather do it implicitly.
That needs to go hand in hand with notification support for multiple 
devices; with removing the function, you loose the possibility to select 
a device.

I'll send a suggestion for multiple device notification support in a 
separate email and try to explain the relevant spec details.
Then we can discuss it.

Br,
Christian

[-- Attachment #2: Type: text/html, Size: 4863 bytes --]

Re: [PATCH v3 5/8] obexd: Add RegisterNotifications function

[Christian Fetzer]

Hi Luiz,

On 03/13/2013 08:57 PM, Luiz Augusto von Dentz wrote:
> Hi Christian,
>
>
> On Wed, Mar 13, 2013 at 1:27 PM, Marcel Holtmann <marcel@holtmann.org 
> <mailto:marcel@holtmann.org>> wrote:
>
>     Hi Christian,
>
>     >>> This allows applications to register for all different types
>     >>> of MAP event reports.
>     >>>
>     >>> In response to this call, the MSE should connect to the local MNS
>     >>> instance.
>     >>> ---
>     >>> doc/obex-api.txt   |  6 ++++
>     >>> obexd/client/map.c | 86
>     ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>     >>> 2 files changed, 92 insertions(+)
>     >> never ever intermix doc/ changes with actual code. You really
>     need to learn on how to split patches properly. There is no reason
>     at all that this are not two patches.
>     >>
>     >>
>     >>> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
>     >>> index 759c4d8..ef5d85e 100644
>     >>> --- a/doc/obex-api.txt
>     >>> +++ b/doc/obex-api.txt
>     >>> @@ -651,6 +651,12 @@ Methods    void SetFolder(string name)
>     >>>                     Possible errors:
>     org.bluez.obex.Error.InvalidArguments
>     >>>  org.bluez.obex.Error.Failed
>     >>>
>     >>> +           void RegisterNotifications(boolean)
>     >>> +
>     >>> +                   Register / unregister reception of
>     notifications.
>     >>> +
>     >>> +                   Possible errors:
>     org.bluez.obex.Error.InvalidArguments
>     >>> +  org.bluez.obex.Error.Failed
>
>
> Why we are not doing this automatically? The application can opt out 
> by not connecting to signals, but I see no reason why not to be 
> connected to register the notification channel automatically when 
> connecting the MAP session if the remote support it.
>
> -- 
> Luiz Augusto von Dentz

as I tried to explain, the purpose of the function in the current patch 
is to select the one device for that you will receive notifications.
I agree, it would be better to hide the notification connection setup 
and rather do it implicitly.
That needs to go hand in hand with notification support for multiple 
devices; with removing the function, you loose the possibility to select 
a device.

I'll send a suggestion for multiple device notification support in a 
separate email and try to explain the relevant spec details.
Then we can continue the discussion there.

Br,
Christian