nRF24 / RF24Mesh

OSI Layer 7 Mesh Networking for RF24Network & nrf24L01+ & nrf52x devices
http://nrf24.github.io/RF24Mesh
GNU General Public License v2.0
421 stars 154 forks source link

Enhance requestAddress by looping through poll responses #238

Closed TMRh20 closed 1 month ago

TMRh20 commented 1 month ago

Old Behaviour Example:

New Behaviour Example:

Still testing this new code, but it basically involves moving one bracket and changing some return statements to continues.

2bndy5 commented 1 month ago

It's hard to believe we didn't pick up on this before. I can't think of any disadvantages. Maybe users might observe a slower mesh connecting performance when the mesh is well populated, but this feels like the proper approach regardless.

TMRh20 commented 1 month ago

Haha, I know right? The whole polling system was built on having multiple polls, so why we didn't exploit that fact before is beyond me. I think it should offer better performance in most situations, since one problem with a populated mesh is receiving polls from the wrong 'level' when poll responses are delayed.

2bndy5 commented 1 month ago

Looking at the whole function body, I think we can optimize the while loop that collects poll responses. https://github.com/nRF24/RF24Mesh/blob/7c30debee4b91d63c27847213eaf10d413a10212/RF24Mesh.cpp#L325-L356

#if defined(MESH_DEBUG)
    bool goodSignal = radio.testRPD();
#endif
    while (millis() - timr < 55 && pollCount < MESH_MAXPOLLS) {
        if (network.update() == NETWORK_POLL) {
            uint16_t contact = 0;
            memcpy(&contact, &network.frame_buffer[0], sizeof(contact));

            // Drop duplicate polls to help prevent duplicate requests
            bool isDupe = false;
            for (uint8_t i = 0; i < pollCount; ++i) {
                if (contact == contactNode[i]) {
                    isDupe = true;
                    break;
                }
            }
            if (!isDupe) {
                contactNode[pollCount] = contact;
                ++pollCount;
            }

            IF_MESH_DEBUG(printf_P(PSTR("%u: MSH Poll %c -64dbm\n"), millis(), (goodSignal ? '>' : '<')));
        } // end if

        MESH_CALLBACK
    } // end while

    IF_MESH_DEBUG(printf_P(PSTR("%u: MSH Got poll from level %d count %d\n"), millis(), level, pollCount));
    if (!pollCount) return 0;
2bndy5 commented 1 month ago

BTW, I think it is more performant to

uint32_t timeout = millis() + 225;
while (millis() < timeout) { /* ... */ }

instead of

uint32_t timr = millis();
while (millis() - timr < 225) { /* ... */ }

because the arithmetic for calculating the time diff is abstracted away from each iteration of the loop.

TMRh20 commented 1 month ago

Updated per your suggestions but slightly modified:

2bndy5 commented 1 month ago

FYI, I cherry picked the merge commit from this PR onto the 1.x branch. If I overstepped (or somehow screwed it up), then it can be reverted.

TMRh20 commented 1 month ago

I keep forgetting about the 1.x branch. Glad you remembered!

@2bndy5 Are we about due for a release crusade? Its been around a year since the last release...

2bndy5 commented 1 month ago

Yeah, we probably should. Although, the only real changes that affect Arduino platform (which are directly influenced by our tagged commits/releases) are the network layers. The RF24 lib itself just has doc & example updates; the rest is specific to Linux (https://github.com/nRF24/RF24/compare/v1.4.8...master).

I gotta get back to investigating the piwheels' 32-bit build problem with pyRF24 deployments (nRF24/pyRF24#61)...