improve SX12xx/SPI read performance #29
Conversation
Instead of reading a single byte at a time - read all bytes with a single SPI transfer. This is much more performant, especially when reading longer packets and when DMA is used. Also remove the special case of read() (read_with_status()) since it's the same operation as a regular read, just with the first byte read being status.
|
First off, thank you for the effort to improve the performance of this crate. It's great to see that level of work here. I would like to understand your test environment (embedded framework, MCU, LoRa board), if you can share that information. Also, it would be good to know if you are using lora-phy for P2P or LoRaWAN. If you are using Embassy, you would have needed to take some steps to get that working with lora-phy version 2 (the GitHub main branch), since Embassy itself still uses version 1. The reason I ask is I was unable to run a simple P2P receive example with this update, and had read issues when executing a more complicated LoRaWAN case on which I am working. Both of these tests use a recent version of Embassy. We'll work that out as we move forward. For reference, this is what I used for a patch to test with Embassy, which hopefully pulls in your update in its entirety: [patch.crates-io] |
|
Atm I run a fork of lora-phy on my board, but with embassy. Only p2p. |
|
For your planning purposes, I will be unavailable from September 27 through October 23. Given the significance of this update, I doubt I will be able to adequately review and test it before I leave. I apologize. |
|
Did some quick testing and I could send P2P, but on the receiving side it seems to get stuck in the irq processing loop once it actually receives something. Same behavior with just the single read operation fix. Will investigate further at a later time. |
|
Nice work! I'm currently attempting to adapt the existing Also, would it make sense to include simpler single-buffer write/read methods in the |
Did you find the time to investigate? |
I did notice this too, that some functions are written to handle these edge cases which might make normal use of them a bit convoluted. So personally I think it could make sense and might make some parts easier to follow.
Not yet unfortunately, a bit busy elsewhere atm. But it's on my to-do list! |
|
Closing since #35 fixed main point of this PR. |
Main point of PR is to read all bytes from SX12xx chip with a single SPI transfer. This instead of reading one byte at a time.
This since reading all bytes with one operation is much more performant, especially when reading longer packets and when DMA is used.
While at this some other small things caught my eye. Like repeating some register operations a lot, so did some cleaning there too.