On Tue, Oct 20, 2009 at 7:05 AM, Zhao Forrest <forrest.zhao@xxxxxxxxx> wrote:
>>> --- 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.
If it's consensus the right way would be exposing SyncEvolution
through ProviderName, please add a comment to the code telling that,
so someone remember to fix it by the time obexd supports it.
--
João Paulo Rechi Vita
MSc Computer Science Student
Computer Engineer
IC / Unicamp
http://jprvita.wordpress.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
