ESP8266 Community Forum

- Tue Oct 25, 2016 5:09 pm #57122

the node timeout value isn't being applied to the equation as expected ...
lastrecieved (4289636273) + timeout (10000000) = 4299636273 ... but that doesn't match the results ...
because 4299636273 is NOT less than 4289641320 (but 4289636273 is) ...

@sfranzyshen:
I think the answer to this problem comes in the max value of uint32_t which is 4,294,967,295.
However, as you pointed out 'lastreceieved+timeout' yielded 4,299,636,273 which is greater than the max of uint32_t.

Hence, the comparison operation: "connection->lastRecieved + NODE_TIMEOUT" can roll-over uint32_t and yield unwieldy results. I would use the following:

Code: Select allvoid ICACHE_FLASH_ATTR easyMesh::manageConnections( void ) {
    debugMsg( GENERAL, "manageConnections():\n");
    uint32_t nowNodeTime;
    uint32_t timeOut=NODE_TIMEOUT/1000;                     // Resolves it to a milli-second count
    uint32_t _lastReceived;

    SimpleList<meshConnectionType>::iterator connection = _connections.begin();
    while ( connection != _connections.end() ) {
        _lastReceived=connection->lastRecieved/1000;        // Resolves it to a milli-second count
        nowNodeTime = getNodeTime()/1000;                   // Resolves it to a milli-second count
        if ( ( _lastReceived+timeOut ) < nowNodeTime ) {
            debugMsg( CONNECTION, "manageConnections(): dropping %d timeout=%u + last=%u (%u) <
              now=%u\n", connection->chipId, NODE_TIMEOUT, connection->lastRecieved,
              connection->lastRecieved+NODE_TIMEOUT, nowNodeTime );

            connection = closeConnection( connection );
            continue;
        }
    }
}

The above code would detect time-outs on a milli-second basis and serves the purpose quite well. This prevents the disrupts caused by the roll-overs.

EDIT: Had this reply typed in early in the day, forgot to post

- Tue Oct 25, 2016 6:29 pm #57127 I hope we can get this to be reliable because there are some good things in it. Unfortunately I don't have the skill, or the time, to do much more than poking at it from the edges. I'm able to do testing. My testing so far has been at home but I can put up a network at work, where I have a larger space and complete access to whatever I need.

- Tue Oct 25, 2016 6:56 pm #57128

I hope we can get this to be reliable because there are some good things in it. Unfortunately I don't have the skill, or the time, to do much more than poking at it from the edges. I'm able to do testing. My testing so far has been at home but I can put up a network at work, where I have a larger space and complete access to whatever I need.

Every piece of this puzzle is important! So please keep poking at it ... I'll keep at it ... and eventually this will work ... and be worth it

- Wed Oct 26, 2016 12:53 am #57133

@sfranzyshen:
I think the answer to this problem comes in the max value of uint32_t which is 4,294,967,295.
However, as you pointed out 'lastreceieved+timeout' yielded 4,299,636,273 which is greater than the max of uint32_t.

Hence, the comparison operation: "connection->lastRecieved + NODE_TIMEOUT" can roll-over uint32_t and yield unwieldy results. I would use the following:

Yes! but rather than truncate the values ... it's better to change the equation to be rollover safe ...
// The trick is to always calculate the time difference, and not compare the two time values.
instead of doing (last+timeout < now) i did (now - last > timeout) So lets say last is 4294967290 (just before rollover), and now is 10 (just after rollover). Then now - last is actual 16 (not -4294967280) since the result will be calculated as an unsigned long (which can't be negative, so itself will roll around).