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

avoid SPI repetition during fragmentation #182

Closed
2bndy5 opened this issue Jul 24, 2021 · 11 comments · Fixed by #176
Closed

avoid SPI repetition during fragmentation #182

2bndy5 opened this issue Jul 24, 2021 · 11 comments · Fixed by #176

Comments

@2bndy5
Copy link
Member

2bndy5 commented Jul 24, 2021

I'm asking because it seems there is a lot of repetitive overhead when sending each fragment of a message going to the same to_node & to_pipe (referring to the call to logicalToPhysicalAddress() in _write()). Additionally, managing the auto-ack feature for pipe 0, writing the same TX address, and manually toggling TX/RX mode for each fragment seems like it would be unnecessarily time consuming.

I understand that there's additional considerations concerning networkFlags & FLAG_FAST_FRAG and the 3 manual retries (which invokes a 2 ms delay on failed attempts) for each fragment, but I'm just looking for an expert opinion. I thought the whole point of using RF24::writeFast() was to fill up the TX FIFO (blocking only when TX FIFO is full), and let the radio do it's thing (during RF24::txStandby()). Maybe I'm reading the source code wrong or just overthinking it again.

@TMRh20
Copy link
Member

TMRh20 commented Jul 24, 2021 via email

@2bndy5
Copy link
Member Author

2bndy5 commented Jul 24, 2021

I believe the networkFlags & FLAG_FAST_FRAG takes care of not repetitively toggling RX/TX mode. Some repetitively toggling the auto-ack feature on pipe 0 is also avoided using networkFlags & FLAG_FAST_FRAG, but not all; write_to_pipe() seems to keep setting auto-ack for pipe 0 based on the multicast parameter invoked from logicalToPhysicalAddress() results (which would always be the same during fragmentation of an individual message).
write_to_pipe() also does radio.openWritingPipe(pipe_address(node, pipe)); for every fragment, although the TX address should be the same for all fragments of the same message.

There's also the case of waiting for NETWORK_ACK messages for each fragment in _write(). I didn't mention this in the OP, but I don't think (from what I've seen) fragments get a NETWORK_ACK message even though message fragments use a message_type value within 64 < message_type < 192.

  • I'm having trouble narrowing down where/what actually triggers sending the NETWORK_ACK message. I understand it's sent in write(uint16_t to_node, uint8_t sendType), but I don't see anything in update() that would cause write(uint16_t to_node, uint8_t sendType) to send a NETWORK_ACK. Is it possible that NETWORK_ACKs are solely meant for user-defined messages?

I should enable SERIAL_DEBUG in RF24 to see the actual repetition of unnecessary SPI transactions during fragmentation. I also put together a RPi/pyRF24Network/examples/dev_test.py script for faster debugging convenience (I do plan on removing this dev_test.py before opening #176 for review).

@TMRh20
Copy link
Member

TMRh20 commented Jul 24, 2021

Network acks are handled in the bool RF24Network::write(uint16_t to_node, uint8_t directTo) function. It would seem that they should be handled in the update function, but they are delivered by the last node to send route the data. ie: Node 00 sends to node 011, node 01 will send the network ack to 00 upon delivery. They should be enabled for payload types 65 through 191. You are correct that fragments don't get a network ack if I remember correctly.

@2bndy5
Copy link
Member Author

2bndy5 commented Jul 24, 2021

Node 00 sends to node 011, node 01 will send the network ack to 00 upon delivery

This is so explicitly insightful! I'm adding a comment in the write(to_node, directTo) (and NETWORK_ACK's doc) that should help others trying to understand network acks.

@2bndy5
Copy link
Member Author

2bndy5 commented Jul 24, 2021

did a quick test with SERIAL_DEBUG enabled in RF24 lib. The following results are from that dev_test.py script I mentioned:

>>> network.write(header, bytes(range(26)))
write_register(00,0e)    # CONFIG - stopListening() called
write_register(02,3f)    # EN_RXADDR - side effect of stopListening()
141381: NET Sending id 1 from 00 to 01 type 148
141381: FRG frame size 32
141381: FRG frame 00 00 01 00 01 00 94 02 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F 10 11 12 13 14 15 16 17
141381: MAC Sending to 01 via 01 on pipe 5
write_register(01,3f)    # EN_AA - pipe 0 is enabled
141381: NET Pipe 5 on node 01 has address cccccc3ce3   # SPI transaction to write TX address not output
[Writing 32 bytes 0 blanks]
141382: NET Sending id 1 from 00 to 01 type 150
141382: FRG frame size 10
141382: FRG frame 00 00 01 00 01 00 96 00 18 19
141382: MAC Sending to 01 via 01 on pipe 5
write_register(01,3f)    # EN_AA - pipe 0 is enabled
141382: NET Pipe 5 on node 01 has address cccccc3ce3   # SPI transaction to write TX address not output
[Writing 10 bytes 0 blanks]
write_register(07,10)    # STATUS - clearing the MAX_RT flag
write_register(00,0f)    # CONFIG - startListening() called
write_register(07,70)    # STATUS - side effect of startListening()
write_register(01,3e)    # EN_AA - pipe 0 is disabled
True

I've added comments to the output that should help decipher what is going on.

As I suspected, unnecessary repetition includes

  • the TX address is written (this is a detrimental slow-down)
  • auto-ack on pipe 0 enabled (doesn't hurt as much as writing 5 bytes, but still...)
  • don't forget that the call logicalToPhysicalAddress() in write(to_node, directTo)

@TMRh20
Copy link
Member

TMRh20 commented Jul 25, 2021 via email

@2bndy5
Copy link
Member Author

2bndy5 commented Jul 25, 2021

It would be a fair bit of work as well

Kinda, but I shy from no worthy challenge. I've been brainstorming...

  • The fragging algorithm can be abstracted into a private function named "frag_n_send" (or something more obvious) that write_to_pipe() calls on when the frame_size member is larger than MAX_FRAME_SIZE.
  • The networkFlags & FLAG_FAST_FRAG statements can probably stay as is to not break any unknown external systems that rely on RF24Network. I imagine that this change will ultimately nullify the current internal necessity of FLAG_FAST_FRAG.
  • The hardest part might be managing the difference between the frame_buffer (present in write_to_pipe() & thus accessible by the new frag_n_send()) and the actual message fragments; I'm trying to think up a way that's most inexpensive on memory (with regard to both Arduino and Linux).

I'll probably start a new branch based on the CMake-4-Linux branch so undoing this change won't be difficult if it doesn't work out.

@2bndy5
Copy link
Member Author

2bndy5 commented Jul 27, 2021

I didn't realize the frame_buffer is limited to 32 bytes (should've guessed though). Thus, it does not make sense to move the fragging algorithm to write_to_pipe().

Alternative idea to avoid the repetition

Add a new flag called FLAG_FIRST_FRAG for use in the networkFlags member as the networkFlags is allocated to 8 bits (current flags only use 4 LSB). This new flag can only be used to avoid the unnecessary SPI transactions because the call to logicalToPhysicalAddress() is baked into the workflow as a message is passed from

  1. write(header, msg, ...) (where fragmentation occurs) to
  2. _write() (where args for logicalToPhysicalAddress() are assembled) to
  3. write(toNode, directTo) (where logicalToPhysicalAddress() is called) to
  4. write_to_pipe() (where all the repeated SPI transactions occur)

So, to summarize, leave code as is, use a new flag FLAG_FIRST_FRAG asserted on the first fragment in step 1 than get immediately de-asserted in step 4.

  • Perform the SPI transactions in step 4 only when the new FLAG_FIRST_FRAG and FAST_FAST_FRAG are assert at the same time.
  • If FLAG_FIRST_FRAG is de-asserted and FAST_FAST_FRAG is asserted, then don't do SPI transactions in step 4.
  • If both FLAG_FIRST_FRAG and FAST_FAST_FRAG are de-asserted, then always do SPI transactions in step 4.

This should look like in psuedo-code (for step 4)

#define FLAG_FIRST_FRAG 16
uint8_t assertedFlags = networkFlags & (FLAG_FIRST_FRAG + FAST_FAST_FRAG);
if (assertedFlags > FLAG_FIRST_FRAG || !assertedFlags) {
    networkFlags &= ~FLAG_FIRST_FRAG;
    // do SPI transactions
}

@TMRh20
Copy link
Member

TMRh20 commented Jul 27, 2021 via email

@2bndy5 2bndy5 changed the title Would it make sense to move fragmentation from write() to write_to_pipe()? avoid SPI repetition during fragmentation Jul 29, 2021
@2bndy5
Copy link
Member Author

2bndy5 commented Jul 29, 2021

well, the new flag works on Arduino platform, but I just thought of way to not use a new flag at all: use the outgoing header's message type. The math/comparison logic would be simpler at least...

@2bndy5
Copy link
Member Author

2bndy5 commented Jul 29, 2021

this issue has been addressed without the need for a new flag

>>> network.write(header, bytes(range(99)))
write_register(00,0e)
write_register(02,3f)
67305: NET Sending id 1 from 01 to 00 type 148
67305: FRG frame size 32
67305: FRG frame 01 00 00 00 01 00 94 05 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F 10 11 12 13 14 15 16 17
67305: MAC Sending to 00 via 00 on pipe 1
write_register(01,3f)
67305: NET Pipe 1 on node 00 has address cccccccc3c
[Writing 32 bytes 0 blanks]
67306: NET Sending id 1 from 01 to 00 type 149
67306: FRG frame size 32
67306: FRG frame 01 00 00 00 01 00 95 04 18 19 1A 1B 1C 1D 1E 1F 20 21 22 23 24 25 26 27 28 29 2A 2B 2C 2D 2E 2F
67306: MAC Sending to 00 via 00 on pipe 1
[Writing 32 bytes 0 blanks]
67306: NET Sending id 1 from 01 to 00 type 149
67306: FRG frame size 32
67306: FRG frame 01 00 00 00 01 00 95 03 30 31 32 33 34 35 36 37 38 39 3A 3B 3C 3D 3E 3F 40 41 42 43 44 45 46 47
67306: MAC Sending to 00 via 00 on pipe 1
[Writing 32 bytes 0 blanks]
67306: NET Sending id 1 from 01 to 00 type 149
67307: FRG frame size 32
67307: FRG frame 01 00 00 00 01 00 95 02 48 49 4A 4B 4C 4D 4E 4F 50 51 52 53 54 55 56 57 58 59 5A 5B 5C 5D 5E 5F
67307: MAC Sending to 00 via 00 on pipe 1
[Writing 32 bytes 0 blanks]
67307: NET Sending id 1 from 01 to 00 type 150
67307: FRG frame size 11
67307: FRG frame 01 00 00 00 01 00 96 00 60 61 62
67307: MAC Sending to 00 via 00 on pipe 1
[Writing 11 bytes 0 blanks]
write_register(00,0f)
write_register(07,70)
write_register(01,3e)
True

Notice that the necessary configuring SPI transactions are only performed before and after all fragments are transmitted. This is indicated by the following 2 lines

write_register(01,3f)
67305: NET Pipe 1 on node 00 has address cccccccc3c

@2bndy5 2bndy5 linked a pull request Jul 29, 2021 that will close this issue
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.

2 participants