-
Notifications
You must be signed in to change notification settings - Fork 1k
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
new examples and more #691
Conversation
attempting to address nRF24#636
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.
So many changes!
The only thing I've noticed is the doc changes with the documenting of GPIO and SPI functions for porting. They now show up in the main class area instead of just RF24. I think this change should be reverted and/or done in a way that it doesn't show up in the main class documentation.
I figured that's why the lines declaring the classes were commented out. I had hoped that by adding a I did look into using the Doxyfile's EXCLUDE_SYMBOLS or doxygen's |
ok the GPIO & SPI classes don't show up in the class list, but their functions still get documented in "Porting: GPIO", "Porting: SPI" module pages. I've updated the docs preview using a new pre-release candidate on my fork.
There's no reason to go easy on me here. I could see how having to review all these changes might be annoying. |
@Avamander Do you have the time to review this? |
Oops, I didn't know changing the readme link to the docs would dismiss the approving review. To host the docs using gh-pages we need to tag a commit for release. This is why I'm not too quick to merge this (& why I preemptively changed the doc link in the readme).
I've already submitted/merged changes to all other nRF24 RF24* libraries, but there needs to be a (what I like to call) "release crusade" across all these RF24* repos (except RF24Audio) before we merge my PR for tmrh20.github.io. |
Release crusade done, I just did a new release for all the RF24* libs. |
I hate to say this, but I meant this PR was supposed to be merged before the next release (if you wanted this PR's changes included in 1.3.10). |
Np, we can do another soon
… On Dec 14, 2020, at 2:18 AM, Brendan ***@***.***> wrote:
I hate to say this, but I meant this PR was supposed to be merged before the next release (if you wanted this PR's changes included in 1.3.10).
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
I took a relatively short look, but I noticed nothing significant. Everything seems good. |
* fix outdated install instruction in python wrapper docs since I rewrote all the linux examples I noticed the install instructions for the python wrapper direct the user to edit/run an example that no longer exists. This is a quick fix for just that. (changes tested/rendered correctly on my local machine) * removed atrifacts from gettingStarted.cpp about cmd args * ackPl.cpp isn't using anything from cstring.h * remove CLI artifacts from irqConfig.cpp example * rem CLI artifacts from manAck.cpp example * change call to getDynPaylSize() to getPayloadSize() in getStarted.ino * change call to getDynPaylSize() to getPayloadSize() in getStarted.cpp
hello! I'm seeing at least one breaking changes in my Python code, maybe it makes sense to call that out a little more?
(anything else?) |
Hmm. I forgot I did that. It is documented. To be honest, it should've been like that from the start. I could undo it for consistency. The Python wrapper should have it's own symantic versioning (and soon will when pyRF24 repo gets its legs).
I think you found that 1 thing that wasn't mentioned because none of the new python examples ever call |
Cool, thanks! Yeah I like the new syntax, just caught me by surprise when some previously stable code fell over. Migrating to attributes like that makes sense to me as part of a major version. Appreciate all the new work on this library, it's great! |
This is the PR I've been teasing about. All doc changes are rendered for review at https://2bndy5.github.io/RF24 -- some links that lead to nRF24.github.io/RF24 are dead until this PR gets merged.
Issues Addressed
links in tmrh20.github.io repo's index.html file need alteration for directing to docs hosted by individual reposneed to merge my PR on tmrh20.github.io repo
printPrettyDetails()
for a morehuman readable printDetails() #687
printPrettyDetails()
currently takes about 580 more bytes thanprintDetails()
REGISTER_MASK
to print_byte_register() &replace
spiTrans()
withwrite_register()
#686get_status()
transaction doesn't print to Serial whenSERIAL_DEBUG
is definedwriteBlocking()
,writeFast()
, &txStandBy()
functions could be slightly faster #678SPI transaction to the new private member
status
pinMode(IRQ_PIN, INPUT)
needed in examples #680setPALevel()
takes 1 or 2 args properlystartWrite()
return a bool #679false
if TX FIFO is full ortrue
if payload was written to TX FIFOsetPayloadSize()
#674setPayloadSize()
independentlyfrom when
openReadingPipe()
is calledpayload length is now clamped to range [1, 32]
examples with Arduino CLI #665
(takes ~ 5 minutes to complete); rf24_ATTiny examples are compiled with the SpenceKonde ATTinyCore
the solution does get rid of the warning during compilation
troubleshoot for newcomers that blindly follow unofficial tutorials
begin()
; uses before-n-aftercomparison to set the new
_is_p_variant
private memberreUseTx()
need some clarification #655reUseTX()
works behind the curtain@deprecate
)ack_payloads_enabled
private member to full potential #654writeAckPayload()
can be called before and after callingstartListening()
startCarrierWave()
may be incompatible with older nRF24L01 (non-plus variants) #640startCarrierWave()
using new_is_p_variant
private member_is_p_variant
private memberflush_rx()
marked for deprecation? #651flush_rx()
from deprecation status in docsenableAckPayload()
doesn't force-enable dynamic payloads feature for pipe 1;docs and examples show proper manipulation of dynamic payloads feature
printDetails()
(&printPrettyDetails()
) now prints"/dev/spidev<bus_number>.<bus_cs_pin>" about the CSN pin
to avoid any further confusion
Other changes not related to an open issue
writeAckPayload()
returns a bool describing if payload was or wasn't loaded into TX FIFO (courtesy of the newstatus
private member)RF24::payload_size
), rather they are truncated to 32 bytes if needed.available()
during th IRQ pin FALLING transition (information is "unreliable" per datasheet -- tests when developing interruptConfigure.ino example showed thatavailable()
returnedfalse
when it should've beentrue
). CallingwhatHappened()
beforeavailable()
seems to allow enough time for pipe information to become accurate again.RF24_TINY
is defined to customize the CSN settling times inRF24::csn()
. This should expedite the use of output values from timingSearch3pin.ino for NerdRalph's "3-pin solution".