nRF24 / RF24Mesh

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

where is duplicate frame detection implemented? #197

Closed 2bndy5 closed 3 years ago

2bndy5 commented 3 years ago

In testing my pure python port, I noticed that sequentially multicasted NETWORK_POLL messages using the same frame.id were ignored (actually only the first one was handled). I can't find where this condition (to discard duplicate frames) is checked. So, I'm asking to make sure I ported it correctly.

At first I thought it lived in the network.enqueue() method, but that only accounts for fragmentation re-assembly. I don't see anything in network.update() or mesh.update() that discards frames with a duplicate ID number.

TMRh20 commented 3 years ago

I don’t remember there being any functionality to drop duplicate frames except maybe in fragmentation/reassembly. I could be wrong but I’d have to scour the code to find out for sure. Are you sure about the consistency of this behaviour?

On Aug 4, 2021, at 5:02 PM, Brendan @.***> wrote:

 In testing my pure python port, I noticed that sequentially multicasted NETWORK_POLL messages using the same frame.id were ignored (actually only the first one was handled). I can't find where this condition (to discard duplicate frames) is checked. So, I'm asking to make sure I ported it correctly.

At first I thought it lived in the network.enqueue() method, but that only accounts for fragmentation re-assembly. I don't see anything in network.update() or mesh.update() that discards frames with a duplicate ID number.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

2bndy5 commented 3 years ago

Yes, I was running a C++ master node on RPi and trying to get CirPy non-master node on RP2040 MCU to connect to it.

At the time, my lib was only incrementing the frame ID number for user payloads. The master node on RPi would only respond to the first Poll, then ignore all subsequent frame's with the same ID. As soon as I started incrementing the frame's ID (from the CirPy node), the C++ master node started responding to each Poll message.

This would take a bit of work to reproduce...

2bndy5 commented 3 years ago

This behavior is also noted in the muticastRelay docs

TMRh20 commented 3 years ago

Hmm I’ll take your word for it, and take a closer look.

On Aug 4, 2021, at 5:38 PM, Brendan @.***> wrote:

 This behavior is also noted in the muticastRelay docs

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

2bndy5 commented 3 years ago

Nevermind, I tried to reproduce this, and the C++ master node responded to each Poll message when the frame's id number remained consistent. When I first noticed the above scenario, I was trying to use a python version of writeFast() and txStandby()... I've since reverted back to only using 1 level of the TX FIFO for reliability reasons. So, the duplicate frame's may actually have never been properly transmitted at the time.

I'm still curious how multicastRelay filters out duplicate frames though. Is it only done on the receiving nodes?

TMRh20 commented 3 years ago

I still can’t seem to find where it would filter out duplicate frames anywhere. The docs may be incorrect.

On Aug 4, 2021, at 6:00 PM, Brendan @.***> wrote:

 Nevermind, I tried to reproduce this, and the C++ master node responded to each Poll message when the frame's id number remained consistent. When I first noticed the above scenario, I was trying to use a python version of writeFast() and txStandby()... I've since reverted back to only using 1 level of the TX FIFO for reliability reasons. So, the duplicate frame's may actually have never been properly transmitted at the time.

I'm still curious how multicastRelay filters out duplicate frames though. Is it only done on the receiving nodes?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

2bndy5 commented 3 years ago

Could it be referring to inherent collision during multicasting? ~This would make a little more sense if the docs were written prior to your intuitive fragmentation feature (maniacbug days).~ @TMRh20 you wrote this 7 years ago 😆 . I can remove the part about duplicate frames on the CMake-4-Linux branch, if you agree that it's inaccurate.

TMRh20 commented 3 years ago

I’m thinking I intended to include duplicate filtering but decided against it…

On Aug 4, 2021, at 8:15 PM, Brendan @.***> wrote:

 Could it be referring to inherent collision during multicasting?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

2bndy5 commented 3 years ago

It may be too much trying for too little benefit... You'd have to keep a cache of the last handled header separate from the frame_buffer; and that only solves sequential duplicate frames.

I'll take that part out of the docs.