Skip to content
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

[Request] writeBlocking(), writeFast(), & txStandBy() functions could be slightly faster #678

Closed
2bndy5 opened this issue Nov 7, 2020 · 0 comments · Fixed by #691
Closed
Assignees

Comments

@2bndy5
Copy link
Member

2bndy5 commented Nov 7, 2020

Currently, writeBlocking(), writeFast(), & txStandBy() functions have a while loop that fetches the status byte twice when waiting for an ACK response. If we save the status byte from every SPI transaction as a private member (uint8_t), we could simply refresh the status byte only when needed. This would reduce the polling of the SPI bus when waiting for a ACK packet.

The following source code snippet is from writeBlocking()

    while ((get_status() & (_BV(TX_FULL)))) {
        if (get_status() & _BV(MAX_RT)) {
            return 0;
        }
         ... // timeout algorithm removed for brevity
    }

Saving the status byte from all SPI transactions can reduce the need to refresh the status byte inside this while statement. There is an identical while loop in writeFast(). Both txStandBy() functions can also be optimized as there are similar while loops that fetch the status byte inside the loop.

Solution

Simply save the status byte from every SPI transaction as a private member. This should be fairly straight forward, and since there are several basic SPI functions that either return the status byte (write_*()) or ignore it (read_*()), I'm not going to get into a lot of detail here.

Alternative

If adding another byte worth of private member data is overstepping, we could instead use a locally scoped variable to save the status byte. This is the lazy way, and it doesn't open up further development opportunities (see below) concerning information from the status byte.

Additional context

Saving the status byte from every SPI transaction would actually be my first step towards implementing payload sizes on a per pipe basis as getting the payload size for the next available payload would require knowing which pipe received the payload (specifically concerning getPayloadSize())

Additionally I think the end-user would be better served to know if startWrite() & startFastWrite() were able to write to the TX FIFO at all by returning a boolean using the status byte's TX_FULL flag in bit position 0. Personally, I find it weird that this library has a rxFifoFull() but no txFifoFull(). This could be another issue for another time as I keep looking into the partially blocking write() functions.

@2bndy5 2bndy5 self-assigned this Nov 7, 2020
@2bndy5 2bndy5 changed the title [Request] writeBlocking(), writeFast(), txStandBy() functions could be slightly faster [Request] writeBlocking() & writeFast() functions could be slightly faster Nov 8, 2020
@2bndy5 2bndy5 changed the title [Request] writeBlocking() & writeFast() functions could be slightly faster [Request] writeBlocking(), writeFast(), & txStandBy() functions could be slightly faster Nov 8, 2020
2bndy5 added a commit to 2bndy5/RF24Revamped that referenced this issue Nov 10, 2020
addresses nRF24#678 nRF24#679 nRF24#677

removed comments in build_linux.yml about building/testing the python wrapper

added a fail fast prompt to the streaming examples when failures >= 100
2bndy5 added a commit to 2bndy5/RF24Revamped that referenced this issue Nov 10, 2020
addresses nRF24#678 nRF24#679 nRF24#677

removed comments in build_linux.yml about building/testing the python wrapper

added a fail fast prompt to the streaming examples when failures >= 100
@2bndy5 2bndy5 linked a pull request Nov 29, 2020 that will close this issue
2bndy5 added a commit that referenced this issue Dec 14, 2020
* don't hide EN_ACK_PAY reqs; adjust examples & docs

* removed duplicated calls in examples/tests

* too many examples

* update docs about reUseTx()

* deprecate enableDynamicAck()

* clarify reUseTx()

* typo in func ref

* append setAutoAck() about pipe 0

* consistent casing in "auto-ack"

* fix typo

* revert this "see also" changes

* use ack_payloads_enabled to full potential

* undeprecate flush_rx()

* isPVariant return internal var; default dyn_pl on

addresses the following issues:
- #401
- #641
- #660

* oops, readdress #401 #641 #660

* address #640

* deprecate isAckPayloadAvailable() #664

* try to ignore return value; trigger forked actions

attempting to address #636

* this is why I can't have nice things

* retrying to ignore return value; fix init order

* add CI for avr compiling examples

* build doxygen with latest release ver number

* revert doxyfile, fix doxygen.yml, make2 = install

* build examples changes, doxygen.yml bad indent

* remove unnecessary pipe chars in doxygen.yml

* linux CI changed, pde->ino files

* platform specific examples; fix WiringPi build

* fix using void return value

* remove gemma & lilypad; fix certain example vars

* wiringPi may be broken #669

* 8 paragraphs that say keep auto-ack enabled

#667

* reverted examples folders; exclude maple & 3pin

* exclude didn't work

* trigger arduino build on workflow changes

* explicit examples' paths

* no wildcards in explicit paths

* no quotes in yaml sequence

* try using a pipe char

* arduino build "needs" A-style check

* applied arduino IDE's auto formatting

* remove format checker

* fine let the example terminate

* all official arduino cores to build_arduino.yml

* backup examples; new GettingStarted

* no explicit examples; compile all

* new InterruptConfigure example

* // about when IRQ flags get cleared; & a fix typo

* new StreamingData & MulticeiverDemo examples

* reduce streamData compilation size

* tested GetStarted, AckPayloads, Streaming

* IRQ & Multicceiver examples tested

* undo #646; add broken FakeBLE example

* fix arduino build CI

* forgot to use pipe char

* typo

* 2nd job should run on ubuntu-latest

* fqbn arch-type is wrong?!

* uncomment #define IRQ_PIN from testing artifacts

* use SpenceKonde core released zip file

* use SpenceKonde board definitions

* use correct vendor name?!

* use lower case?!

* platform name should match the fqbn

* what am I doing wrong? use SpenceKonde attiny core

* under "platforms" not "with"

* yml formatted "platforms" options

* "with: platforms:"

* source-path uses relative path

* use trailing / in source-path

* clear advice from per1234 about yml syntax

* fixing source-path and version number

* use direct download link to SpenceKonde release

* adjust source-path again

* no source-path

* redo-ing #646

* timingSearch3pin includes stdio

* don't ignore rf24_ATTiny examples

* separate job for separate ATTiny examples

* timingSearch3pin uses redefined spi funcs

* drop testing on attiny43, install core from json

* keywords updated removed duplicate examples

* CallResponse was duplicate; (+) ManualAck example

* doc updates #671 tested new manAck example

* FakeBLE crc calc not checking out

* add per1234's action to check formatting

* did not find expected alphanumeric char?!

* add manualAcknowledgements example to docs

* don't remove comment block prefix

* restored comment prefix in old examples also

* less confusing comments about addresses

* clarify docs on return value for all write-related

* remove broken fakeBLE example

* forgot to remove fakeBLE from arduino workflow

* test deploy to gh-pages

* bad yml format

* something's wrong with the doxygen build CI

* lack of new line in end of  doxyfile

* remove debug prompt; deploy on publish released

* deploy on any release event

* fix main page useful links

* only clear RX_DR flag in read()

* typo

* doc updates; new python examples

* new gettingstarted example for linux

* add micros() to applicable utilities compatiblity

* define micros() in arch_config.h

* printDetails() in linux

* pipe number not printing correctly

* oops it was a non-printable char

* avoid keyboard interrupt

* build wiringPi examples also

* linux build CI for SPIDEV

* ignore return value in utilty/SPIDEV/interrupt.c

* trying rf24_micros()

* ammend example to use rf24_micros()

* let micros() be defined in examples for linux

* need cstdio

* ported arduino examples to linux (& python)

* oops

* should probably declare i huh

* copy n paste much?

* don't use stoi for a char

* typo, expose digitalRead(), ifdef RF24_WIRINGPI

* ifdef WIRINGPI didn't help, fix inline condition

* testing round 1; fix pyRf24/setup.py

* bug fixes in py examples prompt fixes in linux

* fix irqConf for arduino, rename stream_data.py

* copy n paste artifact

* debugging streamingData

* testing round 2

* oops

* timing output changes; printDetails() also

* debugging IRQ configure example

* debugging IRQ_config.py also

* declare timer stuff for micros in manACK example

* typo

* doc read() for python; add flush_rx() to py wrap

* remove cap on how many bytes to read in python!!!

* don't cap bytes read() from RX FIFO in C++

* supposed to be decode() not encode()

* fix typos; better python doc about read()

* py wrapper omits len args for buf-related funcs

* fix streaming_data.py master()

* duplicate prompt in ackPay.py

* prompt format changes

* big commit

- reverted defaults about Dynamic Payloads and Dynamic ACKs per @TMRh20 suggestions about Si24R1 in #658. This reverts #660 & #661
- ammended docs & all examples about above suggestions
- re-wrote InterruptConfigure examples to use attachInterrupt(); py example uses RPi.GPIO equivalant.
- removed MKR-based boards from Arduino build CI as pin 2 on those boards don't support interrupt requests, and InterruptConfigure.ino uses pin 2 for exactly that.
- added building of python wrapper to Linux workflow; it now also runs pylint on python examples
- removed my attempt to port digitalRead() to various Linux drivers as it is no longer needed in the interruptConfigure.cpp example.
- addressed #674 about RX_PW_Px registers manipulation.
- remove and ignore my VSCode folder as it is machine-specific
- removed contradicting "examples_linux" ignores and added "examples_linux/__pycache__"  folder
- RF24::begin() now brute forces a soft reset. It does not reset addresses set to the pipes, but it does close all pipes. This helps address #401
- fixed some typos/copy-n-paste issues in the docs.
- added instructions on how to run the python example scripts

* Linux build CI use pip3 to install RPi.GPIO

* workflow uses python 3.7 x86

* workflow uses python 3.x x86

* python needs x32 called in workflow

* fix address assignment in linux examples

* don't check python examples in workflow

* define INPUT in RPi-BCM driver

* remove GPIO.cleanup() from py examples

* payloadSize is an attribute in python

* fix python examples about getPLsize()

* use RuntimeError in python examples when begin() is false

* testing round 3

* streamData uses write(); python examples need 3.7+

* fix ellapsedTime in streamingData

* status byte saved as private member

addresses #678 #679 #677

removed comments in build_linux.yml about building/testing the python wrapper

added a fail fast prompt to the streaming examples when failures >= 100

* simplify logic about _is_p_variant

* testing round 4 & address #680

* fix irq examples

* fix compilation errors for IRQ examples

* fix irq example for linux

* tweak IRQ py example; writeAckPayload returns bool

* final tweaks to IRQ examples; some doc updates

* leave closeReadingPipe(0) in startListening()

* arduino examples work

address 2 new issues
1. #687 added printPrettyDetails(); updated python wrapper and keywords.txt
2. #686 centralizing all SPI access to write_*() and read_*() functions

Also reverted changes to gitignore about examples_linux folder while adding/ignoring some things about building the python wrapper (venv, *.pyc, __pycache__)

* remove volatile from radio declaration

* datasheets; fix man_ack.py example

updated comment proceeding radio object in python examples

documented enums and linked references to them. Also linked references in COMMON_ISSUES.md & main page's changelog.

* ATTiny examples should work on SpenceKonde core

* exclude ATTinyx313 from timingSearch3pin example

* oops, bad indent

* Astyle indent for #ifdef is 0

* (+) CLI args to examples. pwr down radio on ctrl+c

* fail if no valid arg is specified

* fix parsing args in python examples

* make py examples behave like c++ examples

* found a bug in py examples' -r arg

* testing round 5

* adjust manpage descriptions

* don't use signal.h in cpp examples

* del cli args from all linux examples except 2

* edit doxyfile OUTPUT_DIR

* only run doxygen on master branch

* redirect doc links to nRF24 org

* missed a link

* remove use of timeout bool in manAck.ino

* self-reviewed PR changes

* ATTinyCore adjustments

- use macros from SpenceKonde ATTinyCore
- add macros for CSN settling times for when using the ATTinyCore

* some more self-reviewed PR changes

* exclude root README from doxygen output

* exclude GPIO & SPI classes from doc'd classes

* add Arduino & Linux build badges to readme

* Change link to docs in readme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant