nRF24 / RF24

OSI Layer 2 driver for nRF24L01 on Arduino & Raspberry Pi/Linux Devices
https://nrf24.github.io/RF24
GNU General Public License v2.0
2.22k stars 1.02k forks source link

[Request] Simplify Examples #658

Closed 2bndy5 closed 3 years ago

2bndy5 commented 3 years ago

Complex = overwhelming

Most people getting started with the nRF24L01 just need it to send data from 1 endpoint to another. The most basic GettingStarted.ino example does it both ways despite what the role variable is set to. This quickly leads to confusion about how the library should be used and how the transceiver actually works (especially since ACK payloads could be used to do the same exact thing only simpler).

Names are a guessing game

PingPair is not the most descriptive prefix to use in the example names. The first time I read it, I thought: "'pair' like in Bluetooth?" -- nope, its 'pair' like 2 devices used for 1 sketch. In fact, the most identifying part of the examples' names are abridged suffixes that require more intimate knowledge of the nRF24L01 features. e.g. "dyn" for dynamic payloads, "int"/"irq" for interrupt requests, "ack" for acknowledgment payloads... The names should be changed so that anyone can accurately guess what they're clicking on in Arduino IDE's example menu. e.g. "pingpair_dyn" to "RF24_dynamic_payloads", "pingpair_ack" to "RF24_acknowledgment_payloads"...

Laziness over physical interaction

Some examples use a digital input pin to manipulate the role variable (or even a specific feature like multicasting). Most programmers would rather be lazy (myself included). Getting up and walking over to a breadboard that's connected to the computer just to plug or unplug a wire connected to the role_pin is counterproductive. We should stick to adjusting a variable in code (or via a Serial Monitor applet) rather than using a physical GPIO pin. I understand Arduino IDE doesn't allow saving changes to examples, but it does instantly offer the option to save any modifications to an example in the default "sketches" folder (which actually helps the end-user start adapting examples to fit their desired application).

Quantity is harder to maintain

There's soooo many examples. I tried to start formatting them according to this repo's contributing guidelines, but it is overwhelming how much work needs to be done. I think there are actual duplicate examples (and even outdated file formats like *.pde files) that could be deprecated or consolidated. I can appreciate the separate folder for Linux examples, but the same problems discussed above still apply.

Inconsistencies

I found an example with different pin numbers used (as compared to most of the others). Obviously, it would be easier to upload/try different examples without having to change the wiring.

2bndy5 commented 3 years ago

the whole point of catching the keyboard interrupt was to keep the radio from listening when running the same examples (using the same addresses) on another pair of radios. What RPi version is this happening on? It probably isn't putty, rather more specific to the SSH target device.

TMRh20 commented 3 years ago

Replicated on Pi 3 Model B Rev 1.2 and Pi Model B Rev 2, both are up-to-date on software etc.

TMRh20 commented 3 years ago

I put a radio.isChipConnected(); call before the cout command, and it doesn't print anything, leaving me to believe it is the call to the radio in the interruptHandler that hangs the program. The exit(0); command isn't even being called.

TMRh20 commented 3 years ago

With the AcknowledgementPayloads example, could it be made more like the ManualAcknowledgements example in the counting? Like the receiver increments along with the sender?

I noticed on Arduino so far, the first payload is blank, which shouldn't be the case if the ack payload is written before receiving happens, which it appears to be? I'll look closer at that one later. It looks like just putting the startListening call before writing the ack payload is all that is needed:

https://github.com/2bndy5/RF24/blob/master/examples/AcknowledgementPayloads/AcknowledgementPayloads.ino#L102 https://github.com/2bndy5/RF24/blob/master/examples/AcknowledgementPayloads/AcknowledgementPayloads.ino#L204

The other examples all work seamlessly on Arduino, I didn't really look too closely at the code because they just worked exactly as expected.

TMRh20 commented 3 years ago

I guess I forgot the InterruptConfigure example which also needs radio.startListening(); called before writing teh ack payloads

2bndy5 commented 3 years ago

It looks like just putting the startListening call before writing the ack payload is all that is needed

Is this behavior from the Si24R1? I don't have this problem with the nRF24L01. If so, I wonder if flush_tx() actually works from RX mode (or vice versa) because of the diligent copy from datasheet misprints when cloning the OG.

With the AcknowledgementPayloads example, could it be made more like the ManualAcknowledgements example in the counting? Like the receiver increments along with the sender?

I tried to do this, but it seems as though the incrementing lags behind a couple tranmissions when the incoming counter differs from the initial outgoing counter. This is a change I tried to do recently. Last time you tested it the AckPl example actually incremented separate counters (without saving the incoming counter at all). I'm rather inclined to leave this flaw as is to demonstrate the 1 major disadvantage of using AckPls.

leaving me to believe it is the call to the radio in the interruptHandler that hangs the program

This makes me think it is a threading issue. I will remove the use of signal.h, but if one does ctrl+c during the linux examples' RX role, it would be preferable to run the example again (without args), then ctrl+c when prompted with the "Which radio is this?" prompt to ensure the radio is not in RX mode any more.

TMRh20 commented 3 years ago

As far as I know I don't have any Si24R1, just nrf24 clones. The payloads should be flushed during a call to startListening(), so if it works for you calling startListening() after writing the acks, then I suspect your devices are behaving oddly. The flush_tx() call should clear them all.

2bndy5 commented 3 years ago

Notice that I removed that limitation on startListening() per #654 proposal. I've never had a problem loading ACK payloads while still in TX mode (before calling startListening()). I will make the changes to the examples if that solves the problem.

another reason I like developing from python is so I can test this problem in realtime

TMRh20 commented 3 years ago

Hmm, there was a reason for it that was exposed using the previous examples if I remember correctly. I think the tx payload buffer should be flushed for some reason when switching between modes. I'll have to "play around" some to see if this is still an issue.

Hmm, I guess I neglected to update the library itself on Arduino, I've just been running the examples, so I'll have to get everything in order for more testing with all of your code.

2bndy5 commented 3 years ago

BTW, stopListening() still flushes the TX FIFO if AckPls are enabled (as it always has). Previously, startListening() only flushed the TX FIFO if AckPls were enabled, so I assumed it was because the datasheet said the SPI cmd W_ACK_PAYLOAD is meant to be used only in RX mode. However I found it beneficial to be able to "prime" the TX FIFO before switching into RX mode. If there was another reason for this limitation, I'm open to reverting the limitation.

2bndy5 commented 3 years ago

old examples now reside in my fork's "examples/old_backups" folder. The following old examples have been removed as duplicates:

  1. GettingStarted_CallResponse (duplicate of ManualAcknowledgements)
  2. GettingStarted_HandlingData (duplicate of ManualAcknowledgements & the main idea is also used in MulticeiverDemo & AcknowledgementPayloads)
  3. Transfer (duplicate of StreamingData - though Transfer uses randomly generated 32-byte payloads and different settings like channel 1, 2 Mbps, & CRC8)
  4. pingpair_ack (duplicate of AcknowledgementPayloads)
  5. pingpair_irq_simple (duplicate of InterruptConfigure, but not as insightful about maskIRQ())
  6. starping (duplicate of MulticeiverDemo)

The "examples/Usage" folder has been renamed to "examples/old_backups/recipes" as the sketches in this folder were more hardware dependent rather than focusing on just library usage. Additionally, most of the store links to the hardware referenced in ManiacBug's blog posts say that the hardware isn't offered anymore. Also, I've never heard of a "Maple" MCU board, so I'm not sure if that's still a thing...

The following old examples still reside (in "examples/old_backups" folder) because they still seem somewhat relevant:

  1. GettingStarted_HandlingFailures
  2. TransferTimeouts
  3. pingpair_dyn
  4. pingpair_irq (also fixed this one to address #680 )
  5. pingpair_multi_dyn
  6. pingpair_sleepy
  7. scanner

I didn't do an example about dynamic payloads (except for when ACK payloads are used) because usually a programmer knows what to expect in the payloads' content (including the length required to contain the contents). Nonetheless, pingpair_dyn (basic dynamic payloads) & pingpair_multi_dyn (multicasted dynamic payloads) are still available.

MulticeiverDemo is the only new example that uses a uint64_t address instead of c-strings. You may actually recognize the addresses in MulticeiverDemo from those used the datasheet's figure 10 & 12 (about multiceiving with ShockBurst & EnhancedShockBurst respectively).

TMRh20 commented 3 years ago

If there was another reason for this limitation, I'm open to reverting the limitation.

I can't seem to easily identify any issues with not flushing the tx fifo on startListening(). I'll just keep this in mind in case anything comes up, but I'm good with it as you have it.

I think that clears up most issues with the examples. I've run through a bunch in c++ and python in Linux and am having problems finding anything else to complain about.

TMRh20 commented 3 years ago

I'm ok with removing cmd args from all but streamingData and multiceiverDemo examples. It would mean less maintenance that way also. Would you want me to remove args from corresponding python examples too?

Ok, lets do that to keep things simple as possible. I'll leave it up to you on the python examples, I'm good either way.

2bndy5 commented 3 years ago

Done.

I have to admit, I immediately liked the ability to skip past the prompts & get straight to testing the behavior I wanted. I'll leave the python examples as is (they'll probably get touched up again when pyRF24 repo gets more attention). Maybe in the future we can devise a header file that does what python's std lib argparse does but for the cpp examples. Like I mentioned before, GNU C library's argp.h lacks the intuition to make it a useful tool. (e.g. auto-generated man page & automatic handling of invalid options).

I'll give the examples another round of testing and then open a PR 🎉 that summarizes all the issues I've addresses.

2bndy5 commented 3 years ago

@TMRh20 did the manualAcknowledgements example work reliably on the RPi for you? My RPi4 is still having trouble. All the other examples work well, and it doesn't seem like a power problem (some acks get through; most don't in either direction). The python example manual_acknowledgements.py works fine, so I think it might be a timing problem. Tell me its just me and I'll move on.

TMRh20 commented 3 years ago

Works fine for me.

On Nov 29, 2020, at 6:36 AM, Brendan notifications@github.com wrote:

 @TMRh20 did the manualAcknowledgements example work reliably on the RPi for you? My RPi4 is still having trouble. All the other examples work well, and it doesn't seem like a power problem (some acks get through; most don't in either direction). The python example manual_acknowledgements.py works fine, so I think it might be a timing problem. Tell me its just me and I'll move on.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

2bndy5 commented 3 years ago

ok I'll move on for now and chalk it up to a problem specific to my radio module... Unless we get reports of similar problem from someone else.