Skip to content

Commit 1acf968

Browse files
2bndy52bndy5
authored
Refactor some SPI transactions (#988)
* Use `read_register()` to write 1 byte SPI commands * replace `reg | R_REGISTER` with just `reg` * don't `reg & REGISTER_MASK` * `writeAckPayload()` does not need to check for static payload size * reviewed debug output
1 parent bae5111 commit 1acf968

File tree

3 files changed

+43
-60
lines changed

3 files changed

+43
-60
lines changed

‎RF24.cpp

+41-56
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ void RF24::read_register(uint8_t reg, uint8_t* buf, uint8_t len)
154154
uint8_t* ptx = spi_txbuff;
155155
uint8_t size = static_cast<uint8_t>(len + 1); // Add register value to transmit buffer
156156

157-
*ptx++ = (R_REGISTER | reg);
157+
*ptx++ = reg;
158158

159159
while (len--) {
160160
*ptx++ = RF24_NOP; // Dummy operation, just for reading
@@ -179,13 +179,13 @@ void RF24::read_register(uint8_t reg, uint8_t* buf, uint8_t len)
179179

180180
beginTransaction();
181181
#if defined(RF24_SPI_PTR)
182-
status = _spi->transfer(R_REGISTER | reg);
182+
status = _spi->transfer(reg);
183183
while (len--) {
184184
*buf++ = _spi->transfer(0xFF);
185185
}
186186

187187
#else // !defined(RF24_SPI_PTR)
188-
status = _SPI.transfer(R_REGISTER | reg);
188+
status = _SPI.transfer(reg);
189189
while (len--) {
190190
*buf++ = _SPI.transfer(0xFF);
191191
}
@@ -206,7 +206,7 @@ uint8_t RF24::read_register(uint8_t reg)
206206

207207
uint8_t* prx = spi_rxbuff;
208208
uint8_t* ptx = spi_txbuff;
209-
*ptx++ = (R_REGISTER | reg);
209+
*ptx++ = reg;
210210
*ptx++ = RF24_NOP; // Dummy operation, just for reading
211211

212212
#if defined(RF24_RP2)
@@ -223,11 +223,11 @@ uint8_t RF24::read_register(uint8_t reg)
223223

224224
beginTransaction();
225225
#if defined(RF24_SPI_PTR)
226-
status = _spi->transfer(R_REGISTER | reg);
226+
status = _spi->transfer(reg);
227227
result = _spi->transfer(0xff);
228228

229229
#else // !defined(RF24_SPI_PTR)
230-
status = _SPI.transfer(R_REGISTER | reg);
230+
status = _SPI.transfer(reg);
231231
result = _SPI.transfer(0xff);
232232

233233
#endif // !defined(RF24_SPI_PTR)
@@ -247,7 +247,7 @@ void RF24::write_register(uint8_t reg, const uint8_t* buf, uint8_t len)
247247
uint8_t* ptx = spi_txbuff;
248248
uint8_t size = static_cast<uint8_t>(len + 1); // Add register value to transmit buffer
249249

250-
*ptx++ = (W_REGISTER | (REGISTER_MASK & reg));
250+
*ptx++ = (W_REGISTER | reg);
251251
while (len--) {
252252
*ptx++ = *buf++;
253253
}
@@ -282,54 +282,36 @@ void RF24::write_register(uint8_t reg, const uint8_t* buf, uint8_t len)
282282

283283
/****************************************************************************/
284284

285-
void RF24::write_register(uint8_t reg, uint8_t value, bool is_cmd_only)
285+
void RF24::write_register(uint8_t reg, uint8_t value)
286286
{
287-
if (is_cmd_only) {
288-
if (reg != RF24_NOP) { // don't print the get_status() operation
289-
IF_RF24_DEBUG(printf_P(PSTR("write_register(%02x)\r\n"), reg));
290-
}
291-
beginTransaction();
292-
#if defined(RF24_LINUX)
293-
status = _SPI.transfer(W_REGISTER | reg);
294-
#else // !defined(RF24_LINUX) || defined (RF24_RP2)
295-
#if defined(RF24_SPI_PTR)
296-
status = _spi->transfer(W_REGISTER | reg);
297-
#else // !defined (RF24_SPI_PTR)
298-
status = _SPI.transfer(W_REGISTER | reg);
299-
#endif // !defined (RF24_SPI_PTR)
300-
#endif // !defined(RF24_LINUX) || defined(RF24_RP2)
301-
endTransaction();
302-
}
303-
else {
304-
IF_RF24_DEBUG(printf_P(PSTR("write_register(%02x,%02x)\r\n"), reg, value));
287+
IF_RF24_DEBUG(printf_P(PSTR("write_register(%02x,%02x)\r\n"), reg, value));
305288
#if defined(RF24_LINUX) || defined(RF24_RP2)
306-
beginTransaction();
307-
uint8_t* prx = spi_rxbuff;
308-
uint8_t* ptx = spi_txbuff;
309-
*ptx++ = (W_REGISTER | reg);
310-
*ptx = value;
289+
beginTransaction();
290+
uint8_t* prx = spi_rxbuff;
291+
uint8_t* ptx = spi_txbuff;
292+
*ptx++ = (W_REGISTER | reg);
293+
*ptx = value;
311294

312295
#if defined(RF24_RP2)
313-
_spi->transfernb((const uint8_t*)spi_txbuff, spi_rxbuff, 2);
296+
_spi->transfernb((const uint8_t*)spi_txbuff, spi_rxbuff, 2);
314297
#else // !defined(RF24_RP2)
315-
_SPI.transfernb(reinterpret_cast<char*>(spi_txbuff), reinterpret_cast<char*>(spi_rxbuff), 2);
298+
_SPI.transfernb(reinterpret_cast<char*>(spi_txbuff), reinterpret_cast<char*>(spi_rxbuff), 2);
316299
#endif // !defined(RF24_RP2)
317300

318-
status = *prx++; // status is 1st byte of receive buffer
319-
endTransaction();
301+
status = *prx++; // status is 1st byte of receive buffer
302+
endTransaction();
320303
#else // !defined(RF24_LINUX) && !defined(RF24_RP2)
321304

322-
beginTransaction();
305+
beginTransaction();
323306
#if defined(RF24_SPI_PTR)
324-
status = _spi->transfer(W_REGISTER | reg);
325-
_spi->transfer(value);
307+
status = _spi->transfer(W_REGISTER | reg);
308+
_spi->transfer(value);
326309
#else // !defined(RF24_SPI_PTR)
327-
status = _SPI.transfer(W_REGISTER | reg);
328-
_SPI.transfer(value);
310+
status = _SPI.transfer(W_REGISTER | reg);
311+
_SPI.transfer(value);
329312
#endif // !defined(RF24_SPI_PTR)
330-
endTransaction();
313+
endTransaction();
331314
#endif // !defined(RF24_LINUX) && !defined(RF24_RP2)
332-
}
333315
}
334316

335317
/****************************************************************************/
@@ -348,7 +330,7 @@ void RF24::write_payload(const void* buf, uint8_t data_len, const uint8_t writeT
348330
}
349331

350332
//printf("[Writing %u bytes %u blanks]",data_len,blank_len);
351-
IF_RF24_DEBUG(printf("[Writing %u bytes %u blanks]\n", data_len, blank_len););
333+
IF_RF24_DEBUG(printf_P("[Writing %u bytes %u blanks]\n", data_len, blank_len););
352334

353335
#if defined(RF24_LINUX) || defined(RF24_RP2)
354336
beginTransaction();
@@ -420,7 +402,7 @@ void RF24::read_payload(void* buf, uint8_t data_len)
420402

421403
//printf("[Reading %u bytes %u blanks]",data_len,blank_len);
422404

423-
IF_RF24_DEBUG(printf("[Reading %u bytes %u blanks]\n", data_len, blank_len););
405+
IF_RF24_DEBUG(printf_P("[Reading %u bytes %u blanks]\n", data_len, blank_len););
424406

425407
#if defined(RF24_LINUX) || defined(RF24_RP2)
426408
beginTransaction();
@@ -486,23 +468,25 @@ void RF24::read_payload(void* buf, uint8_t data_len)
486468

487469
uint8_t RF24::flush_rx(void)
488470
{
489-
write_register(FLUSH_RX, RF24_NOP, true);
471+
read_register(FLUSH_RX, (uint8_t*)nullptr, 0);
472+
IF_RF24_DEBUG(printf_P("[Flushing RX FIFO]"););
490473
return status;
491474
}
492475

493476
/****************************************************************************/
494477

495478
uint8_t RF24::flush_tx(void)
496479
{
497-
write_register(FLUSH_TX, RF24_NOP, true);
480+
read_register(FLUSH_TX, (uint8_t*)nullptr, 0);
481+
IF_RF24_DEBUG(printf_P("[Flushing RX FIFO]"););
498482
return status;
499483
}
500484

501485
/****************************************************************************/
502486

503487
uint8_t RF24::get_status(void)
504488
{
505-
write_register(RF24_NOP, RF24_NOP, true);
489+
read_register(RF24_NOP, (uint8_t*)nullptr, 0);
506490
return status;
507491
}
508492

@@ -545,7 +529,7 @@ void RF24::print_address_register(const char* name, uint8_t reg, uint8_t qty)
545529
name);
546530
while (qty--) {
547531
uint8_t* buffer = new uint8_t[addr_width];
548-
read_register(reg++ & REGISTER_MASK, buffer, addr_width);
532+
read_register(reg++, buffer, addr_width);
549533

550534
printf_P(PSTR(" 0x"));
551535
uint8_t* bufptr = buffer + addr_width;
@@ -564,7 +548,7 @@ uint8_t RF24::sprintf_address_register(char* out_buffer, uint8_t reg, uint8_t qt
564548
uint8_t offset = 0;
565549
uint8_t* read_buffer = new uint8_t[addr_width];
566550
while (qty--) {
567-
read_register(reg++ & REGISTER_MASK, read_buffer, addr_width);
551+
read_register(reg++, read_buffer, addr_width);
568552
uint8_t* bufptr = read_buffer + addr_width;
569553
while (--bufptr >= read_buffer) {
570554
offset += sprintf_P(out_buffer + offset, PSTR("%02X"), *bufptr);
@@ -1325,7 +1309,8 @@ bool RF24::writeBlocking(const void* buf, uint8_t len, uint32_t timeout)
13251309
void RF24::reUseTX()
13261310
{
13271311
write_register(NRF_STATUS, _BV(MAX_RT)); //Clear max retry flag
1328-
write_register(REUSE_TX_PL, RF24_NOP, true);
1312+
read_register(REUSE_TX_PL, (uint8_t*)nullptr, 0);
1313+
IF_RF24_DEBUG(printf_P("[Reusing payload in TX FIFO]"););
13291314
ce(LOW); //Re-Transfer packet
13301315
ce(HIGH);
13311316
}
@@ -1690,7 +1675,7 @@ void RF24::enableDynamicPayloads(void)
16901675
//toggle_features();
16911676
write_register(FEATURE, read_register(FEATURE) | _BV(EN_DPL));
16921677

1693-
IF_RF24_DEBUG(printf("FEATURE=%i\r\n", read_register(FEATURE)));
1678+
IF_RF24_DEBUG(printf_P("FEATURE=%i\r\n", read_register(FEATURE)));
16941679

16951680
// Enable dynamic payload on all pipes
16961681
//
@@ -1710,7 +1695,7 @@ void RF24::disableDynamicPayloads(void)
17101695
//toggle_features();
17111696
write_register(FEATURE, 0);
17121697

1713-
IF_RF24_DEBUG(printf("FEATURE=%i\r\n", read_register(FEATURE)));
1698+
IF_RF24_DEBUG(printf_P("FEATURE=%i\r\n", read_register(FEATURE)));
17141699

17151700
// Disable dynamic payload on all pipes
17161701
//
@@ -1731,7 +1716,7 @@ void RF24::enableAckPayload(void)
17311716
if (!ack_payloads_enabled) {
17321717
write_register(FEATURE, read_register(FEATURE) | _BV(EN_ACK_PAY) | _BV(EN_DPL));
17331718

1734-
IF_RF24_DEBUG(printf("FEATURE=%i\r\n", read_register(FEATURE)));
1719+
IF_RF24_DEBUG(printf_P("FEATURE=%i\r\n", read_register(FEATURE)));
17351720

17361721
// Enable dynamic payload on pipes 0 & 1
17371722
write_register(DYNPD, read_register(DYNPD) | _BV(DPL_P1) | _BV(DPL_P0));
@@ -1748,7 +1733,7 @@ void RF24::disableAckPayload(void)
17481733
if (ack_payloads_enabled) {
17491734
write_register(FEATURE, static_cast<uint8_t>(read_register(FEATURE) & ~_BV(EN_ACK_PAY)));
17501735

1751-
IF_RF24_DEBUG(printf("FEATURE=%i\r\n", read_register(FEATURE)));
1736+
IF_RF24_DEBUG(printf_P("FEATURE=%i\r\n", read_register(FEATURE)));
17521737

17531738
ack_payloads_enabled = false;
17541739
}
@@ -1764,7 +1749,7 @@ void RF24::enableDynamicAck(void)
17641749
//toggle_features();
17651750
write_register(FEATURE, read_register(FEATURE) | _BV(EN_DYN_ACK));
17661751

1767-
IF_RF24_DEBUG(printf("FEATURE=%i\r\n", read_register(FEATURE)));
1752+
IF_RF24_DEBUG(printf_P("FEATURE=%i\r\n", read_register(FEATURE)));
17681753
}
17691754

17701755
/****************************************************************************/
@@ -1774,7 +1759,7 @@ bool RF24::writeAckPayload(uint8_t pipe, const void* buf, uint8_t len)
17741759
if (ack_payloads_enabled) {
17751760
const uint8_t* current = reinterpret_cast<const uint8_t*>(buf);
17761761

1777-
write_payload(current, len, W_ACK_PAYLOAD | (pipe & 0x07));
1762+
write_register(W_ACK_PAYLOAD | (pipe & 0x07), current, rf24_min(len, static_cast<uint8_t>(32)));
17781763
return !(status & _BV(TX_FULL));
17791764
}
17801765
return 0;

‎RF24.h

+1-3
Original file line numberDiff line numberDiff line change
@@ -1929,12 +1929,10 @@ class RF24
19291929
*
19301930
* @param reg Which register. Use constants from nRF24L01.h
19311931
* @param value The new value to write
1932-
* @param is_cmd_only if this parameter is true, then the `reg` parameter
1933-
* is written, and the `value` param is ignored.
19341932
* @return Nothing. Older versions of this function returned the status
19351933
* byte, but that it now saved to a private member on all SPI transactions.
19361934
*/
1937-
void write_register(uint8_t reg, uint8_t value, bool is_cmd_only = false);
1935+
void write_register(uint8_t reg, uint8_t value);
19381936

19391937
/**
19401938
* Write the transmit payload

‎examples/rf24_ATTiny/timingSearch3pin/timingSearch3pin.ino

+1-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ void csn(bool mode) {
7979
/****************************************************************************/
8080
uint8_t read_register(uint8_t reg) {
8181
csn(LOW);
82-
SPI.transfer(R_REGISTER | reg);
82+
SPI.transfer(reg);
8383
uint8_t result = SPI.transfer(0xff);
8484
csn(HIGH);
8585
return result;

0 commit comments

Comments
 (0)