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] Move REGISTER_MASK to print_byte_register() & replace spiTrans() with write_register() #686

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

Comments

@2bndy5
Copy link
Member

2bndy5 commented Nov 17, 2020

I'm trying to reduce the code size of getDynamicPayloadSize() by using the read_register() function, but the use of REGISTER_MASK is the only thing in my way. In fact it would only be useful in the print_byte_register() as that is the only time that a macro defined in nRF24L01.h is not passed to read_register(). So, this line would be changed to:

        printf_P(PSTR(" 0x%02x"), read_register(reg++ & REGISTER_MASK));

Same thing applies to write_register(); every call to write_register() passes a macro defined from nRF24L01.h, so REGISTER_MASK is completely useless.

I'd also like to get rid of RF24::spiTrans() because we can get the same job done with write_register() by adding a bool is_cmd_only=false parameter to the write_register() declaration. This would affect:

  1. flush_tx()
  2. flush_rx()
  3. reUseTX()
  4. get_status()

I don't really see the usefulness of returning the status byte for the first 2 affected functions (see also the additional context below). Its completely useless in the python wrapper because the macros in nRF24L01.h aren't exposed to python. In fact, reUseTX() ignores the returned status byte already, but I will modify flush_tx(), flush_rx(), and get_status() to continue returning the status byte for backward compatibility.

Of course write_register() would need some tweaking:

void RF24::write_register(uint8_t reg, uint8_t value, bool is_cmd_only)
{
    if (is_cmd_only) {
        IF_SERIAL_DEBUG(printf_P(PSTR("write_register(%02x)\r\n"), reg));
        beginTransaction();
        status = _SPI.transfer(W_REGISTER | reg);
        endTransaction();
    }
    else {
        IF_SERIAL_DEBUG(printf_P(PSTR("write_register(%02x,%02x)\r\n"), reg, value));
        #if defined(RF24_LINUX)
        beginTransaction();
        uint8_t * prx = spi_rxbuff;
        uint8_t * ptx = spi_txbuff;
        *ptx++ = (W_REGISTER | reg);
        *ptx = value;

        _SPI.transfernb((char *)spi_txbuff, (char *)spi_rxbuff, 2);
        status = *prx++; // status is 1st byte of receive buffer
        endTransaction();
        #else // !defined(RF24_LINUX)

        beginTransaction();
        status = _SPI.transfer(W_REGISTER | reg);
        _SPI.transfer(value);
        endTransaction();
        #endif // !defined(RF24_LINUX)
    }
}

Additional context

  • Remember [Request] writeBlocking(), writeFast(), & txStandBy() functions could be slightly faster #678 proposes that the status byte be saved in every SPI transaction, meaning any RF24::function can access the status byte after any SPI transaction.
  • Notice from the datasheet that W_REGISTER | reg will not affect the SPI cmds as the bit that is asserted by W_REGISTER is already asserted in all of the SPI cmds. Only exception is the ACTIVATE cmd, but that is only used by the toggle_features() function which already directly accesses the SPI bus.
  • Before raising this issue, I've already reduced writeAckPayload() to use
    write_payload(current, len, W_ACK_PAYLOAD | (pipe & 0x07))`.
    There was nothing special about writeAckPayload() that made it differ from write_payload().
  • I've also taken liberty with not truncating dynamic payloads according to the static payloads setting. Both read_payload() & write_payload() now use:
    uint8_t blank_len = 0;
    if (!dynamic_payloads_enabled) {
        data_len = rf24_min(data_len, payload_size);
        blank_len = payload_size - data_len;
    }
    else {
        data_len = rf24_min(data_len, 32);
    }
    Although care should be taken to not read/write any payload with a length of 0 (per datasheet) before even calling read_payload() & write_payload()
@2bndy5 2bndy5 self-assigned this Nov 17, 2020
@TMRh20
Copy link
Member

TMRh20 commented Nov 17, 2020

Pretty detailed but seems to make sense & if it all works, it works. 👍

@2bndy5
Copy link
Member Author

2bndy5 commented Nov 17, 2020

Pretty detailed

Sorry it started out simple, but I kept finding small changes that needed a thread for changelog reasons

2bndy5 added a commit to 2bndy5/RF24Revamped that referenced this issue Nov 20, 2020
address 2 new issues
1. nRF24#687 added printPrettyDetails(); updated python wrapper and keywords.txt
2. nRF24#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__)
@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
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants