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

Reply via email to