-
Notifications
You must be signed in to change notification settings - Fork 775
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
Bugs fixes and enhancements #2
base: master
Are you sure you want to change the base?
Conversation
prefixed with this new private method to ensure proper operation when multiple SPI devices are in use.
compatibility.
merge cleanup (printDetails).
US/Internationally legal while ensuring no spectrum bleed.
Thanks, I look forward to checking it out. Have you merged my master in prior to this? -----Original Message----- My master has changes which include a write() bug fix and dynamic timeout adjustment, multi SPI device compatibility, additional printDetails() debugging output (nice improvements BTW), some comment clean ups, a US/internationally friendly default channel, and some minor changes to pingpair for my own local testing. The pingpair changes can be ignored. The bug fix on write() now monitors the proper radio register. Also included in this code is deadlock timeout logic which dynamically adjusts based on current ARD/ARC hardware configuration. IMOHO, the bugfix and dynamic timeout adjustment alone is worth the pull. Cheers, Reply to this email directly or view it on GitHub: |
Ok, I see now that you did merge the master in. Yay. That should make it a little easier.
Regarding a bug in 'write()'. I do not see a bug in write() in the master branch of my repo. "status = read_register(OBSERVE_TX,&observe_tx,1);" is a perfectly legitimate way of reading the status. It's that way so that in SERIAL_DEBUG mode I can print out the value of observe_tx, otherwise it's ignored. There definitely was a bug in the version you had, but you had a pretty old drop. I had fixed the bug in there already. For the SPI stuff, it's not clear what is being done here. What is the user benefit of b762847 ? |
Request to merge codes ...
Radio is now allowed to enter standby. This allows for the removal of needless delays within the driver. Additional timings optimizations. If applications desire additional power savings, they should manually power down the radio as needed. Multicast write support added. ::write timeout monitoring logic changed to emcompass worst timeouts for retries/timeouts. Should never trigger anyways...but better safe. Fixes write retries timeout bug. Proper hardware delay added to powerUp. Was previously part of startWrite; which needlessly added overhead to every write/startWrite. Allows for query of hardware channel. PA documentation fixes.
calculated based on current hardware configuration. There is room for optimization via caching.
loss of resolution and the possibility of a premature timeout. The write monitoring loop now uses us rather than ms to be more compatible with the us result returned from getMaxTimeout.
a bool for multicasting. Documentation updates.
artificial delay. Examples use more options. Added a new multicast examples based on pingpair_dyn; pingpair_multi_dyn.
RF24.h. Fixed some radio power up/down issues in code which relied on side effect behavior of radio powering down after every TX. Applications now take responsibility for power management.
timeout in tests.
these flushed, it should do it. Otherwise, properly behaving applications should need this. Also, by not flushing it opens the door for additional retransmission features down the road.
slows things down. Its now where it needs to be. Dang.
What about this PR? I'm considering gcopeland version but I think that It should be merged |
This PR would be great to merge in, especially the multi SPI support (the commit you called out, @maniacbug). Currently your RF24 library assumes it’s the only thing using SPI, and that the NRF is the only slaved device. If you have multiple devices that you want to communicate with, your library will likely break because it doesn’t properly reset the SPI configuration before attempting to communicate with the radio (so it’ll get whatever configuration was last set, which’ll be for another device, and may not match the NRF’s spec). |
My master has changes which include a write() bug fix and dynamic timeout adjustment, multi SPI device compatibility, additional printDetails() debugging output (nice improvements BTW), some comment clean ups, a US/internationally friendly default channel, and some minor changes to pingpair for my own local testing. The pingpair changes can be ignored.
The bug fix on write() now monitors the proper radio register. Also included in this code is deadlock timeout logic which dynamically adjusts based on current ARD/ARC hardware configuration.
IMOHO, the bugfix and dynamic timeout adjustment alone is worth the pull.
Cheers,