crownstone / bluenet

Bluenet is the in-house firmware on Crownstone hardware. Functions: switching, dimming, energy monitoring, presence detection, indoor localization, switchcraft.
https://crownstone.rocks
91 stars 62 forks source link

Match cuckoo cpp impl with py 177097602 #113

Closed ArrowAcrobatics closed 3 years ago

ArrowAcrobatics commented 3 years ago

Delivered feature found in pivotal: The cuckoo filter is now available in both crownstone-core and (after this PR) bluenet. https://www.pivotaltracker.com/story/show/177097602

cs_TrackableParser requires further development, but that is part of the currently blocked task: https://www.pivotaltracker.com/story/show/176761262

Review of this pull request should focus on the cuckoo filter and its documentation, perhaps also to start a discussion about how to set up the host-based unit tests for this component.

ArrowAcrobatics commented 3 years ago

Regarding private parts of CuckooFilter: POD types cannot contain private members. It is nice if we have a POD underlying this object because it is used in protoocols. I could factor out the member variables into a POD struct and include that in the class. Would that be better?

struct __attribute__((__packed__)) cuckoo_filter_data_t { 
 // members
};

class CuckooFilter { 
private:
  cuckoo_filter_data_t data;
  // other stuff
public:
 // public API
};

That way the class is more a 'view' of the data. (We could even have the data field be a reference.)

vliedel commented 3 years ago

Ah, yes, the data of a cuckoo filter is sent over bluetooth, so it's protocol. This means it should be defined as struct, in the protocol dir.