-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Breaking change: Add configurable bitrates, asyncio support, and expose interrupt handler when platform supports #45
Conversation
…no library * Adds configurable bitrates with GFSK modulation * Adds asyncio support so recv/send are no longer blocking * Removes TX/RX register polling in favor of pin IRQ and countio * Cleanup RPi Interrupt example, addresses adafruit#36
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This library is still in use on M0 boards, where asyncio is not available.Adding async is great, but it also would lock out those boards.
Space is also really tight on those boards, so any increase in library size would make it not fit on those boards.
(Tagging @jerryneedell and @ladyada for interest here. We may discuss this internally as well.)
So we might consider making a new version of this library with your changes, to add async capability, and keeping the old version as is. Another idea is to factor out the async code so that it would only be imported if required, but that requires some careful restructuring of the API. And when the library is frozen in, we need to avoid freezing in the the unused part.
Thank you for your consideration! Some food for thought, it looks like the 32u4 and M0 are relatively older offerings. For newcomers to the ecosystem, they may consider the RP2040 which is beefier and probably a better fit for CircuitPython in general (processing power, ram and flash). In fact, the way the RadioFruit boards are currently priced, it seems like the RP2040 could be the new baseline for projects going forward. https://www.adafruit.com/product/3076 (32u4) [$24.95] With that in mind, perhaps it makes sense to release a new library version (same name, new major rev number) that is compatible with asyncio+ boards? Would follow typical versioning conventions. |
Hi, any update on this? Alternatively, I can consider releasing this as CircuitPython_RadioHead instead. |
@mtai Yes, your changes could be a community library (in the community bundle), supported by you. Would that be OK with you? @ladyada @jerryneedell Does that make sense to you? |
Hi, to follow-up on this, haven't heard from @ladyada or @jerryneedell on what Adafruit would like to do. Would you prefer updating the Adafruit driver or have this go the community route? I am personally in favor of sending patches upstream so everyone can benefit :) |
Sorry -- I missed this discussion. I'll try to review it and comment in the next few days. |
A few quick comments/questions: @mtai I have not done any testing yet. |
I tried the exmple rfm69_1_quickstart.py on a feather_rp2040_rfm69 but I get
oops -- nevermind -- waking up slowly -- I forgot to install the library -- back to testing.... OK -- I now have the QuickStart demo running between a feather rp2040 rfm69 and a feather m4 express with rfm69 featherwing .... progress |
rx_packet = rfm69_client.recv_with_ack() | ||
# If no packet was received during the timeout then None is returned. | ||
if rx_packet is None: | ||
continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as written, this will bypass the packet transmission if no packet is received.
I changed it to
if rx_packet is not None:
# Received a packet!
# Print out the raw bytes of the packet:
print_packet(rx_packet)
``` so a packet is sent every interval as desired
With he change above, the rfm69_2_with_acks demo works. I am not very fluent in asyncio so I am having some difficulty evaluating this demo since it does not appear to be working correctly for me. |
hmm -- some progress I have not been able to get it to work in the opposite direction...yet |
Thanks for all the testing across devices! Unfortunately I only have the ESP32-S3 TFT Feather to test against. I updated the examples based on my library changes. Re rfm69_2_with_acks and missing packets, I think that's expected due to the overhead of Python managing SPI and the interrupts. I would imagine pure-C bindings would be much more efficient in terms of MCU cycles. I think you would be able to see this if we tested rfm69_throughput (Python) vs a similar setup using Arduino and RadioHead. Re rfm_3_aio_with_ack, hmm I think I see a bug in example code, try changing L107 to Let me give examples 1-3 another test spin and see if I catch anything. |
Yay! -- the fix to L107 worked!! I also now have the code running on esp32s3 boards with rfm69 featherwings. |
@mtai Given the extensive changes to the existing library and that the new library cannot support the M0 and the Raspberry Pi/Blinka boards, I think it makes most sense for this to be a new library - something like CircuitPython_RadioHead_RFM69 and you place it in the "Community Bundle". As I mentioned earlier, it would also be great to adapt this to the RFM9x -- That may be best as a separate library rather than trying to support both in one. Now that I have the demos working here, I'll take a closer look at the code itself. |
@mtai How do you execute the rfm69_throughput test? |
Re rfm69_3_aio_with_ack, great! I've had a lot of fun using this example as a ranging test to see how far I can receive signal by looking at RSSI :) I can probably update it to make the BITRATE easier to configure. Its pretty amazing how much farther you can go with lower bitrates. Re Raspberry Pi/Blinka, are you suggesting you tested it and its not working? If so, can you send me the tracebacks? I have a RPi4 lying around but I've never set it up :o Re CircuitPython_RadioHead_RFMXX, I'd be happy to adapt this for RFM9x. I don't have a need for LoRa in my projects though. If Adafruit is willing to send me 2 units of the "Adafruit LoRa Radio FeatherWing - RFM95W 900 MHz - RadioFruit", I'd be happy to add support. Re separate library for both modules, I'm thinking I'd include both radios in the same library, pulling the RadioHead common logic into its own module. In other words, 1 library supporting X radios (in this case RFM69 and RFM9X) If people are memory constrained, they can selectively copy the relevant .py files to their lib/ directory. Re rfm69_throughput, it really depends on what you want to test. This assumes use of TFT boards since I using print(). L4-L33 show my testing and % achieved vs theoretical when SENDER/RECEIVER using rfm_throughput.py on 2 Adafruit ESP32-S3 TFT boards. Based on the all the below instructions, I'll probably iterate on this example to make it easier to configure. There are 3 dimensions you can test:
What the combinations test:
Putting this together:
On the SENDER
|
Thanks for the clarifications on the throughput test. Regarding the Raspberry Pi, I have not tried it but it uses countio and countio is not supported via Blinka so I assumed it was a non-starter. I could try working around the use of countio. I'll see what I can do with it. Just to be clear, I don't work for Adafruit. I am just another user and have been involved with the RFM library development over the past several years. I don't speak for Adafuit. |
Technically speaking, on the Raspberry Pi, you don't even need countio, that was used in place of supporting Interrupts on MCUs. To test this hypothesis, I'd try the following... I'll probably need to pull a RPi4 out to actually test this E2E
|
FYI, I tried it with the above changes in a RaspBerry Pi
But the RPi is not receiving any packets. It transmits, bit does not receive. |
@mtai Still no luck getting the RPi Interrupts to work. It is not clear to me what is wrong. The example in the current library works with interrupt OK. Have you verified/fixed up the other examples. I would encourage you to create the examples exactly as you are testing them including the pin assignments. I noticed that several of your examples used the CS pin as board.CE1 which is clearly only relevant for Raspberry Pi boards. I think it would be better ti show the connections you actually used. |
I'm going to close this one as per recommendation above. @mtai please feel free to ready out if you need help getting a new library spun up and submitted for the community bundle. |
Refactor to mirror the code-structure of the original RadioHead Arduino library
Addresses
#39: Flags exposed on RHPacket
#36: RFM69.handle_interrupt exposed as a possible callback method