V2 takes into account Johan's comments presented so far.
I will start with some introduction.
*** How MAP obexd plugin works? ***
Service driver and MIME driver are split between mas.c and messages-*.c.
mas.c makes the actual obexd plugin, i.e. there are OBEX_PLUGIN_DEFINE,
obex_service_driver and obex_mime_type_driver there.
In order to support different message storages the actual data accessing
is done in messages-*.c files (a symbolic link messages.c to one of them
is made during ./configure phase, as it is also done in PBAP plugin). I
will refer to this one as "backend".
There is only one mime driver here, as OBEX request type and OBEX type
header is used in MAP merely to select operation to perform. This
operation in MAP specification is called function. Processing of the
incoming headers and response is done in the same fashion for each one of
them, thus one driver handling all requests to MAP target is enough.
Backend in many ways resembles service plugin/mime type driver pair.
So there are messages_init() and messages_exit() to give backend the
chance to do its own initialization during MAP startup. Converting it to
plugin on its own wouldn't make much sense, as it is fully functional
only in context of MAP plugin (i.e. mas.c). Calling messages_init()
manually from MAP plugin init() function also allows MAP plugin to stop
from loading if the messages backend is non-functional.
There is also messages_connect() and messages_disconnect() which are
called from service driver ->connect() and ->disconnect() respectively.
Session awareness is needed for doing connection-specific processing, for
example storing (un)read status per client (which is required by MAP
specification). The message_connect() call returns pointer which is in
turn passed to all other in-session functions of backend.
The low-level processing which would be the same regardless of storage in
use are done in mas.c. When receiving get/put request, mas.c determines
operation to perform (MAP function) and parses application parameters
headers. Then as in case of other obexd services
obex_(get|put)_stream_start gets called which in turn triggers MIME
driver's ->open().
As I mentioned, the MIME driver functionality is split between mas.c and
backend. So actual MIME driver open() elects appropriate callback for MAP
function in question and calls messages_open() which does the rest of the
job of preparing I/O for the request. It however does not pass the same
data it gets, as some preliminary processing has been already done - so
message_open() gets requested opcode (MAP function), input parameters,
storage for output parameters, OBEX name header value and callback
address. This callback is used to return data asynchronously.
The data in callback is returned in a structure specific to selected MAP
function and optionally wakes the I/O. Callback code takes the structure,
formats it into a stream for sending and appends this to output buffer.
So for example, in GetMessagesListing case, callback gets struct with
metadata of messages in folder and makes an XML from it.
Simultaneously MIME driver read()-s are called (as driven by OpenOBEX
streaming mode) and they pass to the caller what's in the buffer, or
return -EAGAIN if buffer is empty and callback did not set the flag
informing that the operation is already finished.
At the end of the request or at any time if the request is aborted MIME
driver close() is called which in turn calls messages_close() to free
it's data or stop any pending actions (for example D-Bus calls if backend
uses D-Bus).
==========================================
So to answer Johan's doubts.
Current design makes some things easier. For instance request abortion,
as there is messages_open() and messages_close() and close already knows
what to close because of fid (function id) presence. In case of what
Johan wants either I would have to make 14 functions in global name space
(7 MAP functions, equivalent of close and open for any single one of
them) or use 7
identical "open" functions and one single close, which will rely on
hidden function id stored internally in backend. Latter would make ugly
asymmetry in plugin code, as in MIME driver open() there would be called
one of those 7 functions and in close() only messages_close() (or
cancel?) for which relation to one of previous functions would not be
clear from mas.c perspective.
And of course each of these "open" functions and also each of
these "close" functions would have the same prototype and would do the
same thing from the caller view.
What I'm trying to say is that the abstraction is rewarding also from the
service plugin point, and putting backend logic behind something almost
like mime driver is well... logical, as it almost follows mime driver
work flow, it is just a little bit too crippled to be real mime driver on
its own so mime->open() does what it can (it does the low level OBEX
things) and then calls for assistance from messages_open(). And
messages_open does the rest of opening but not using low level mime-like
open() semantics, but MAP semantics.
Err... I said "also" because implementing backend is easier too. It can
simply return error for any MAP function it does not want to support
right away from messages_open and it also makes it immune to further
addition of new MAP functions.
And even though MAP calls its functions, well, functions, they are not
exactly the same as C functions they are more like an opcodes selecting
what to do with data presented. And this data is always of the same
structure.
The way it is now, MIME driver and thus plugin code is kept short and sweet.
Hi,
On Fri, Mar 11, 2011, Johan Hedberg wrote:
> This would be similar to how we handle the telephony driver in BlueZ and
> how the PBAP phonebook back-end is handled in obexd. I'm not saying this
> as justification for the approach but just as a place to look for
> examples. The main justification is to remove one unnecessary
> abstraction layer (the numeric function id) and to remove the need to
> fit each MAP request into the same C function signature (messages_open).
>
> Trying to fit everything into the same mold forces you to create very
> generic structs like mas_request which then end up needing a
> private_data pointer (for request specific data)
Sorry, I mixed this with some other structs in your code that have this
sort of private data and layering. However, the main point about telling
the back-end which function to call indirectly through an integer
instead of just calling the function directly remains (even if the
function signatures would be the same or very similar).
Johan
Hi Slawek,
On Fri, Mar 11, 2011, Slawomir Bochenski wrote:
> >> +enum messages_function_id {
> >> + ? ? MFID_INVALID = 0,
> >> + ? ? MFID_SET_NOTIFICATION_REGISTRATION,
> >> + ? ? MFID_GET_FOLDER_LISTING,
> >> + ? ? MFID_GET_MESSAGES_LISTING,
> >> + ? ? MFID_GET_MESSAGE,
> >> + ? ? MFID_SET_MESSAGE_STATUS,
> >> + ? ? MFID_PUSH_MESSAGE,
> >> + ? ? MFID_UPDATE_INBOX,
> >> +};
> >
> > Since "function" here doesn't seem to be referring to a C function but
> > to the type of request, could you call it e.g. request_type instead? Is
> > there a reason you you want to have this enum instead of e.g. storing
> > the original value of the type header?
>
> Function here refers to function as used in MAP specification (see
> chapter 5: Message Access Profile Functions). Functions in MAP are an
> abstraction above OBEX requests and types. There is one case where one
> type is used for two functions (the difference is whether it is GET or
> PUT request).
>
> In 3 (of 9 total) cases type is used solely for selecting function, as
> there is no real object transmitted (well, to be exact the body in
> this cases consist of single filler byte).
>
> And those MAP functions also accept input parameters and return output
> ones, so the name "function" is quite appropriate.
>
> As checking for this MAP function called is needed in more places it
> is convenient to store it as a number instead of doing expensive
> string comparison each time.
Ok, this makes sense (this was a good chance for me to catch up on the
MAP spec ;). However only as far as the MAP core is concerned. On the
MAP back-end side the only relevant purpose of the id seems to be to
pick the right internal function to call. Therefore, I think it'd make
more sense to have these request specific functions exported by the
back-end and have the MAP core call them directly.
This would be similar to how we handle the telephony driver in BlueZ and
how the PBAP phonebook back-end is handled in obexd. I'm not saying this
as justification for the approach but just as a place to look for
examples. The main justification is to remove one unnecessary
abstraction layer (the numeric function id) and to remove the need to
fit each MAP request into the same C function signature (messages_open).
Trying to fit everything into the same mold forces you to create very
generic structs like mas_request which then end up needing a
private_data pointer (for request specific data) and all this layering
just increases the overall complexity of the code.
Johan
>> +enum messages_function_id {
>> + ? ? MFID_INVALID = 0,
>> + ? ? MFID_SET_NOTIFICATION_REGISTRATION,
>> + ? ? MFID_GET_FOLDER_LISTING,
>> + ? ? MFID_GET_MESSAGES_LISTING,
>> + ? ? MFID_GET_MESSAGE,
>> + ? ? MFID_SET_MESSAGE_STATUS,
>> + ? ? MFID_PUSH_MESSAGE,
>> + ? ? MFID_UPDATE_INBOX,
>> +};
>
> Since "function" here doesn't seem to be referring to a C function but
> to the type of request, could you call it e.g. request_type instead? Is
> there a reason you you want to have this enum instead of e.g. storing
> the original value of the type header?
Function here refers to function as used in MAP specification (see
chapter 5: Message Access Profile Functions). Functions in MAP are an
abstraction above OBEX requests and types. There is one case where one
type is used for two functions (the difference is whether it is GET or
PUT request).
In 3 (of 9 total) cases type is used solely for selecting function, as
there is no real object transmitted (well, to be exact the body in
this cases consist of single filler byte).
And those MAP functions also accept input parameters and return output
ones, so the name "function" is quite appropriate.
As checking for this MAP function called is needed in more places it
is convenient to store it as a number instead of doing expensive
string comparison each time.
--
Slawomir Bochenski
Hi Slawek,
On Fri, Mar 11, 2011, Slawomir Bochenski wrote:
> There is also first part of mas.c <-> backend API. The mas_request
> structure will be used when calling backend functions.
> ---
> plugins/mas.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> plugins/messages.h | 15 +++++++++++++
> 2 files changed, 72 insertions(+), 0 deletions(-)
I've pushed 2/4 and 3/4 upstream, but there's at least issue one with
this last patch:
> +enum messages_function_id {
> + MFID_INVALID = 0,
> + MFID_SET_NOTIFICATION_REGISTRATION,
> + MFID_GET_FOLDER_LISTING,
> + MFID_GET_MESSAGES_LISTING,
> + MFID_GET_MESSAGE,
> + MFID_SET_MESSAGE_STATUS,
> + MFID_PUSH_MESSAGE,
> + MFID_UPDATE_INBOX,
> +};
Since "function" here doesn't seem to be referring to a C function but
to the type of request, could you call it e.g. request_type instead? Is
there a reason you you want to have this enum instead of e.g. storing
the original value of the type header?
Johan
There is also first part of mas.c <-> backend API. The mas_request
structure will be used when calling backend functions.
---
plugins/mas.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
plugins/messages.h | 15 +++++++++++++
2 files changed, 72 insertions(+), 0 deletions(-)
diff --git a/plugins/mas.c b/plugins/mas.c
index a84b8fd..106544d 100644
--- a/plugins/mas.c
+++ b/plugins/mas.c
@@ -25,6 +25,7 @@
#include <config.h>
#endif
+#include <string.h>
#include <errno.h>
#include <glib.h>
#include <openobex/obex.h>
@@ -38,6 +39,14 @@
#include "messages.h"
+#define EVENT_TYPE "x-bt/MAP-event-report"
+#define MESSAGE_TYPE "x-bt/message"
+#define FOLDER_LISTING_TYPE "x-obex/folder-listing"
+#define MESSAGES_LISTING_TYPE "x-bt/MAP-msg-listing"
+#define NOTIFICATION_TYPE "x-bt/MAP-NotificationRegistration"
+#define STATUS_TYPE "x-bt/messageStatus"
+#define UPDATE_TYPE "x-bt/MAP-messageUpdate"
+
/* Channel number according to bluez doc/assigned-numbers.txt */
#define MAS_CHANNEL 16
@@ -99,6 +108,14 @@ static void mas_clean(struct mas_session *mas)
g_free(mas);
}
+static void reset_request(struct mas_session *mas)
+{
+ if (mas->request) {
+ g_free(mas->request);
+ mas->request = NULL;
+ }
+}
+
static void *mas_connect(struct obex_session *os, int *err)
{
struct mas_session *mas;
@@ -139,6 +156,21 @@ static int mas_get(struct obex_session *os, obex_object_t *obj,
if (type == NULL)
return -EBADR;
+ mas->request = g_new0(struct mas_request, 1);
+
+ /* NOTE: type is case-insensitive! */
+ if (g_ascii_strcasecmp(type, FOLDER_LISTING_TYPE) == 0)
+ mas->request->fid = MFID_GET_FOLDER_LISTING;
+ else if (g_ascii_strcasecmp(type, MESSAGES_LISTING_TYPE) == 0)
+ mas->request->fid = MFID_GET_MESSAGES_LISTING;
+ else if (g_ascii_strcasecmp(type, MESSAGE_TYPE) == 0)
+ mas->request->fid = MFID_GET_MESSAGE;
+ else {
+ DBG("Incorrect type: %s", type);
+ ret = -EBADR;
+ goto failed;
+ }
+
*stream = FALSE;
ret = obex_get_stream_start(os, name);
@@ -148,6 +180,8 @@ static int mas_get(struct obex_session *os, obex_object_t *obj,
return 0;
failed:
+ reset_request(mas);
+
return ret;
}
@@ -163,6 +197,23 @@ static int mas_put(struct obex_session *os, obex_object_t *obj, void *user_data)
if (type == NULL)
return -EBADR;
+ mas->request = g_new0(struct mas_request, 1);
+
+ /* NOTE: type is case-insensitive! */
+ if (g_ascii_strcasecmp(type, NOTIFICATION_TYPE) == 0)
+ mas->request->fid = MFID_SET_NOTIFICATION_REGISTRATION;
+ else if (g_ascii_strcasecmp(type, STATUS_TYPE) == 0)
+ mas->request->fid = MFID_SET_MESSAGE_STATUS;
+ else if (g_ascii_strcasecmp(type, MESSAGE_TYPE) == 0)
+ mas->request->fid = MFID_PUSH_MESSAGE;
+ else if (g_ascii_strcasecmp(type, UPDATE_TYPE) == 0)
+ mas->request->fid = MFID_UPDATE_INBOX;
+ else {
+ DBG("Incorrect type: %s", type);
+ ret = -EBADR;
+ goto failed;
+ }
+
ret = obex_put_stream_start(os, name);
if (ret < 0)
goto failed;
@@ -170,6 +221,8 @@ static int mas_put(struct obex_session *os, obex_object_t *obj, void *user_data)
return 0;
failed:
+ reset_request(mas);
+
return ret;
}
@@ -228,8 +281,12 @@ static ssize_t any_read(void *obj, void *buf, size_t count,
static int any_close(void *obj)
{
+ struct mas_session *mas = obj;
+
DBG("");
+ reset_request(mas);
+
return 0;
}
diff --git a/plugins/messages.h b/plugins/messages.h
index c510244..91e7b3b 100644
--- a/plugins/messages.h
+++ b/plugins/messages.h
@@ -20,3 +20,18 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*
*/
+
+enum messages_function_id {
+ MFID_INVALID = 0,
+ MFID_SET_NOTIFICATION_REGISTRATION,
+ MFID_GET_FOLDER_LISTING,
+ MFID_GET_MESSAGES_LISTING,
+ MFID_GET_MESSAGE,
+ MFID_SET_MESSAGE_STATUS,
+ MFID_PUSH_MESSAGE,
+ MFID_UPDATE_INBOX,
+};
+
+struct mas_request {
+ enum messages_function_id fid;
+};
--
1.7.1
As procedures when communicating with message backend will be the same
regardless of the actual type in request, there is only one MIME driver,
thus .mimetype is left NULL.
---
plugins/mas.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/plugins/mas.c b/plugins/mas.c
index aefd291..a84b8fd 100644
--- a/plugins/mas.c
+++ b/plugins/mas.c
@@ -33,6 +33,7 @@
#include "log.h"
#include "obex.h"
#include "service.h"
+#include "mimetype.h"
#include "dbus.h"
#include "messages.h"
@@ -195,6 +196,43 @@ static int mas_setpath(struct obex_session *os, obex_object_t *obj,
return 0;
}
+static void *any_open(const char *name, int oflag, mode_t mode,
+ void *driver_data, size_t *size, int *err)
+{
+ struct mas_session *mas = driver_data;
+
+ DBG("");
+
+ *err = 0;
+
+ return mas;
+}
+
+static ssize_t any_write(void *object, const void *buf, size_t count)
+{
+ DBG("");
+
+ return count;
+}
+
+static ssize_t any_read(void *obj, void *buf, size_t count,
+ uint8_t *hi, unsigned int *flags)
+{
+ DBG("");
+
+ *hi = OBEX_HDR_BODY;
+ *flags = 0;
+
+ return 0;
+}
+
+static int any_close(void *obj)
+{
+ DBG("");
+
+ return 0;
+}
+
static struct obex_service_driver mas = {
.name = "Message Access server",
.service = OBEX_MAS,
@@ -209,10 +247,24 @@ static struct obex_service_driver mas = {
.disconnect = mas_disconnect,
};
+static struct obex_mime_type_driver mime_map = {
+ .target = MAS_TARGET,
+ .target_size = TARGET_SIZE,
+ .mimetype = NULL,
+ .open = any_open,
+ .close = any_close,
+ .read = any_read,
+ .write = any_write,
+};
+
static int mas_init(void)
{
int err;
+ err = obex_mime_type_driver_register(&mime_map);
+ if (err < 0)
+ goto failed_mime;
+
err = obex_service_driver_register(&mas);
if (err < 0)
goto failed_mas_reg;
@@ -220,12 +272,16 @@ static int mas_init(void)
return 0;
failed_mas_reg:
+ obex_mime_type_driver_unregister(&mime_map);
+
+failed_mime:
return err;
}
static void mas_exit(void)
{
obex_service_driver_unregister(&mas);
+ obex_mime_type_driver_unregister(&mime_map);
}
OBEX_PLUGIN_DEFINE(mas, mas_init, mas_exit)
--
1.7.1
This adds basic service functionality in MAP code, accompanied by proper
record for SDP.
RFCOMM channel of choice is 16 which is in accordance with
doc/assigned-numbers.txt.
---
plugins/mas.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 189 insertions(+), 0 deletions(-)
diff --git a/plugins/mas.c b/plugins/mas.c
index 0eab43b..aefd291 100644
--- a/plugins/mas.c
+++ b/plugins/mas.c
@@ -25,18 +25,207 @@
#include <config.h>
#endif
+#include <errno.h>
+#include <glib.h>
+#include <openobex/obex.h>
+
#include "plugin.h"
#include "log.h"
+#include "obex.h"
+#include "service.h"
+#include "dbus.h"
#include "messages.h"
+/* Channel number according to bluez doc/assigned-numbers.txt */
+#define MAS_CHANNEL 16
+
+#define MAS_RECORD "<?xml version=\"1.0\" encoding=\"UTF-8\" ?> \
+<record> \
+ <attribute id=\"0x0001\"> \
+ <sequence> \
+ <uuid value=\"0x1132\"/> \
+ </sequence> \
+ </attribute> \
+ \
+ <attribute id=\"0x0004\"> \
+ <sequence> \
+ <sequence> \
+ <uuid value=\"0x0100\"/> \
+ </sequence> \
+ <sequence> \
+ <uuid value=\"0x0003\"/> \
+ <uint8 value=\"%u\" name=\"channel\"/> \
+ </sequence> \
+ <sequence> \
+ <uuid value=\"0x0008\"/> \
+ </sequence> \
+ </sequence> \
+ </attribute> \
+ \
+ <attribute id=\"0x0009\"> \
+ <sequence> \
+ <sequence> \
+ <uuid value=\"0x1134\"/> \
+ <uint16 value=\"0x0100\" name=\"version\"/> \
+ </sequence> \
+ </sequence> \
+ </attribute> \
+ \
+ <attribute id=\"0x0100\"> \
+ <text value=\"%s\" name=\"name\"/> \
+ </attribute> \
+ \
+ <attribute id=\"0x0315\"> \
+ <uint8 value=\"0x00\"/> \
+ </attribute> \
+ \
+ <attribute id=\"0x0316\"> \
+ <uint8 value=\"0x0F\"/> \
+ </attribute> \
+</record>"
+
+struct mas_session {
+ struct mas_request *request;
+};
+
+static const uint8_t MAS_TARGET[TARGET_SIZE] = {
+ 0xbb, 0x58, 0x2b, 0x40, 0x42, 0x0c, 0x11, 0xdb,
+ 0xb0, 0xde, 0x08, 0x00, 0x20, 0x0c, 0x9a, 0x66 };
+
+static void mas_clean(struct mas_session *mas)
+{
+ g_free(mas);
+}
+
+static void *mas_connect(struct obex_session *os, int *err)
+{
+ struct mas_session *mas;
+
+ DBG("");
+
+ *err = 0;
+
+ mas = g_new0(struct mas_session, 1);
+
+ manager_register_session(os);
+
+ return mas;
+}
+
+static void mas_disconnect(struct obex_session *os, void *user_data)
+{
+ struct mas_session *mas = user_data;
+
+ DBG("");
+
+ manager_unregister_session(os);
+
+ mas_clean(mas);
+}
+
+static int mas_get(struct obex_session *os, obex_object_t *obj,
+ gboolean *stream, void *user_data)
+{
+ struct mas_session *mas = user_data;
+ const char *type = obex_get_type(os);
+ const char *name = obex_get_name(os);
+ int ret;
+
+ DBG("GET: name %s type %s mas %p",
+ name, type, mas);
+
+ if (type == NULL)
+ return -EBADR;
+
+ *stream = FALSE;
+
+ ret = obex_get_stream_start(os, name);
+ if (ret < 0)
+ goto failed;
+
+ return 0;
+
+failed:
+ return ret;
+}
+
+static int mas_put(struct obex_session *os, obex_object_t *obj, void *user_data)
+{
+ struct mas_session *mas = user_data;
+ const char *type = obex_get_type(os);
+ const char *name = obex_get_name(os);
+ int ret;
+
+ DBG("PUT: name %s type %s mas %p", name, type, mas);
+
+ if (type == NULL)
+ return -EBADR;
+
+ ret = obex_put_stream_start(os, name);
+ if (ret < 0)
+ goto failed;
+
+ return 0;
+
+failed:
+ return ret;
+}
+
+static int mas_setpath(struct obex_session *os, obex_object_t *obj,
+ void *user_data)
+{
+ const char *name;
+ uint8_t *nonhdr;
+
+ if (OBEX_ObjectGetNonHdrData(obj, &nonhdr) != 2) {
+ error("Set path failed: flag and constants not found!");
+ return -EBADR;
+ }
+
+ name = obex_get_name(os);
+
+ DBG("SETPATH: name %s nonhdr 0x%x%x", name, nonhdr[0], nonhdr[1]);
+
+ if ((nonhdr[0] & 0x02) != 0x02) {
+ DBG("Error: requested directory creation");
+ return -EBADR;
+ }
+
+ return 0;
+}
+
+static struct obex_service_driver mas = {
+ .name = "Message Access server",
+ .service = OBEX_MAS,
+ .channel = MAS_CHANNEL,
+ .record = MAS_RECORD,
+ .target = MAS_TARGET,
+ .target_size = TARGET_SIZE,
+ .connect = mas_connect,
+ .get = mas_get,
+ .put = mas_put,
+ .setpath = mas_setpath,
+ .disconnect = mas_disconnect,
+};
+
static int mas_init(void)
{
+ int err;
+
+ err = obex_service_driver_register(&mas);
+ if (err < 0)
+ goto failed_mas_reg;
+
return 0;
+
+failed_mas_reg:
+ return err;
}
static void mas_exit(void)
{
+ obex_service_driver_unregister(&mas);
}
OBEX_PLUGIN_DEFINE(mas, mas_init, mas_exit)
--
1.7.1