Hi Nicolas,
Sorry for the delay, but thanks for the patches!
I've applied the first two, which were obvious bugfixes. Thanks!
As for the calendar ID patch, some issues:
1) Pulling data from the Blackberry should go through the btohll() macro.
+ case CALFC_CALENDAR_ID:
+ if( btohs(field->size) == 8 ) {
+ CalendarID = field->u.uint64;
+ }
+ else {
2) Calendar::Clear() should set the new CalendarID to -1 as a default,
instead of in the constructor, so that the user can reset
the record to default values himself.
3) If I understand the logic of your patch correctly, it saves a copy
of one of the calendar records as it goes from BB -> VCal.
Then when new data comes from VCal -> BB, it uses this
saved record as the Calendar ID.
What happens if there really are multiple calendars? Won't this
overwrite things, or potentially put certain calendar data in
the wrong calendar?
What if there are no new records in the BB -> VCal direction?
Will the VCal -> BB step have a Calendar ID, or just the default?
I think we need to think about how to sync multiple calendars into another
application. Does opensync support such a concept? I don't think it does.
Perhaps we need a plugin config setting, to let the user decide what
calendar he wishes to sync, instead of just assuming a default.
Is there an extra database in your device that lists the available
calendars? It would be good to add a parser for that, so we can connect
CalendarID's with email addresses. If you can post a 'btool -d' hex dump
of the records, maybe we can analyze the format together.
- Chris
On Wed, May 26, 2010 at 07:35:07PM +0200, Nicolas wrote:
> Hi,
>
> I have finished my test and you find my patch in my git repository :
>
> http://repo.or.cz/w/barry/progweb.git
>
> I have splitted my commit in several steps.
>
> Work for me and should be good with older devices.
>
> Regards,
>
> Nicolas
>
>
>
> Le mardi 25 mai 2010 ?? 23:04 -0400, Chris Frey a ??crit :
> > On Mon, May 24, 2010 at 10:44:09PM +0200, Nicolas wrote:
> > > Hi Chris,
> > >
> > > I have noticed two little issues.
> > >
> > > First, in :
> > > void BuildField(Data &data, size_t &size, uint8_t type, uint32_t value)
> > > I read :
> > > uint32_t store = htobs(value);
> > > inside of :
> > > uint32_t store = htobl(value);
> >
> > Hi Nicolas,
> >
> > That looks like a true bug. Good catch! I would accept a patch just
> > to fix that, if you want. I notice that your git repo says that
> > your current patch isn't ready to be included yet.
> >
> >
> > > Second, SetRecordByIndex replaces some fields even if I haven't set a
> > > new values. So I can lose some informations.
> > > Sample, in calendar we have to set the CalendarID field (new field see
> > > my git) otherwise, the event is moved in default calendar :(
> > >
> > > Read my patch and most of my comment in GIT, I think that you understand
> > > the issue.
> >
> > I see that you've added an extra 64 bit BuildField. That's fine with me
> > too.
> >
> > I don't see the change in SetRecordByIndex().
> >
> > I don't have a new device, so I can only determine where the new CalendarID
> > field is based on your parsing code. It looks like it is part of the
> > record data itself. So you are correct to add another class member to the
> > Calendar record class.
> >
> > I am hoping that there is a default value for CalendarID. The constructor
> > should set this default value, and if the value is still default when
> > building the record, it should skip it, in order to be backward compatible
> > with older devices.
> >
> > - Chris
> >
> >
> > ------------------------------------------------------------------------------
> >
> > _______________________________________________
> > Barry-devel mailing list
> > Barry-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/barry-devel
> >
>
>
>
> ------------------------------------------------------------------------------
>
> _______________________________________________
> Barry-devel mailing list
> Barry-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/barry-devel
------------------------------------------------------------------------------
_______________________________________________
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel
