NordicPlayground / nRF51-ble-bcast-mesh

Other
324 stars 121 forks source link

version overflow #8

Closed victorpasse closed 9 years ago

victorpasse commented 9 years ago

This could be a solution to the problem with version number ofverflow

in mesh_srv.c old code:

/* new version */
/**@TODO: Handle version overflow */
if (version > ch_md->version_number || uninitialized)

New code:

/* new version */
uint16_t seperation;
if(version >= ch_md->version_number)
    seperation=version-ch_md->version_number;
else
    seperation=(UINT16_MAX-ch_md->version_number)+version+1;
if ((seperation<(UINT16_MAX>>1) && seperation>0) || uninitialized)

If the received version is <(UINT16_MAX>>1) more than current version then the received version is considered a new version else a previous version

trond-snekvik commented 9 years ago

This works, but you'll have to be on guard for new nodes in the mesh. Consider a mesh that has been running for some time, and made it to version 64000 (right below overflow). Then, a new node joins the mesh, making enquieries about the variable by broadcasting its version 0-variable. The rest of the mesh would then assume that this new node knows more than they do, and change their values to the new, useless version, erasing any previous state in the mesh.

Treating the 0-version specially would solve this pretty fast, but I think a Lollipop sequencing scheme might be a better fit, where we treat the first N versions as beginner-versions, that will never be able to overwrite any larger ones. Then we can cycle at N+1 and up (the yummy, round part of the lollipop). That would cover cases where a new node has been assigned a value by some setup-system or something, bumping its version number before it's introduced to the mesh.

uint16_t separation = (new_ver >= old_ver)?
    (new_ver - old_ver) : 
    (-(old_ver - N) + (new_ver - N) - N);

if ((old_ver < N && new_ver >= old_ver) || 
    (old_ver >= N && separation < (UINT16_MAX - N)/2) || 
    uninitialized)
{

Any thoughts?

victorpasse commented 9 years ago

This seems to be a better solution, I will use your variant

olskyy commented 9 years ago

sounds good!

+1!

trond-snekvik commented 9 years ago

The solution I proposed is now part of the master branch (as of release v0.6.2), and version should now cycle like I described above. Thank you @victorpasse for reporting :)