Re: [PATCH] Implements the OBEX server/SyncML client binding for syncEvolution (http://syncevolution.org/).

Zhao Forrest
2009-10-20T09:05:31Z

>> --- a/src/main.c
>> +++ b/src/main.c
>> @@ -53,6 +53,7 @@
>>  #define OPP_CHANNEL    9
>>  #define FTP_CHANNEL    10
>>  #define PBAP_CHANNEL   15
>> +#define SYNCEVOLUTION_CHANNEL  16
>>  #define PCSUITE_CHANNEL        24
>
> Do we want to call this "SYNCEVOLUTION" or "SYNCML"? Both has pros and
> cons. Treating it as generic SyncML capability hides the detail that
> it's currently based on SyncEvolution. Exposing SyncEvolution itself
> would allow to activate multiple different SyncML implementations at the
> same time.
>
> My preference would be to keep it as is, but I wanted to ask anyway.
I have the same idea as you do, and prefer to keep it as "SYNCEVOLUTION".

>
> Off topic: what is the "PC Suite Services server"?
>
It's Nokia OBEX PC Suite Services. Johan may know more details about it.

>> +       { "syncevolution", 'e', 0, G_OPTION_ARG_NONE, &option_syncevolution,
>> +                               "Enable OBEX server for Syncevolution client" },
>
> Mind the spelling: "syncevolution" (lower case) is the command line
> tool, "SyncEvolution" (camel case) the project. "Syncevolution" is a
> typo ;-)
>
Will fix it ASAP.

> Is there documentation that should be updated together with introducing
> the new feature?
The patch does not introduce any new d-bus API to obexd, so no doc need to
be updated.

>
>> +       if (option_syncevolution == TRUE) {
>> +               services |= OBEX_SYNCEVOLUTION;
>> +               bluetooth_init(OBEX_SYNCEVOLUTION, "OBEX server for"
>> +                               " Syncevolution client", NULL,
>> +                               SYNCEVOLUTION_CHANNEL, TRUE, FALSE, FALSE,
>> +                               NULL);
>> +       }
>> +
>
> Which string is going to appear in the service description (the SDP
> part)? This one here? Mentioning "SyncML" would be useful. So I suggest
> "OBEX server for SyncML, using SyncEvolution". Leave out the
> "SyncEvolution client", because the term client is a) overloaded
> (SyncML/OBEX/D-Bus) and b) SyncEvolution could be both client and server
> in this context (SAN => SyncML client, SyncML message => SyncML server).
>
OK. Will expose service name as "OBEX server for SyncML, using SyncEvolution"
since ProviderName attribute is not supported by obexd now.