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

[3/3] DMA Move API: Introduce DMA buffer objects #1856

Merged
merged 15 commits into from
Aug 20, 2024

Conversation

Dominaezzz
Copy link
Collaborator

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • My changes are in accordance to the esp-rs API guidelines

Extra:

Pull Request Details 📖

Description

The (not so) final PR in the DMA move API saga in #1785 (previously #1512).
(Also closes #1798 by removing the struct in question).
The code in this PR could do with several enhancements but I've left them for a future PR (which is in the works) as this PR is big and catastrophic enough. I didn't want to go changing too much in one go, making it tough to review.

This PR does a couple things:

  • Introduces DmaTxBuf struct. Which is a buffer and descriptor combo used for sending data with the DMA.
  • Introduces DmaRxBuf struct. Which is a buffer and descriptor combo used for receiving data with the DMA.
  • Introduces DmaTxRxBuf struct. Which is a buffer and descriptor combo used for full duplex DMA.
  • Migrates the SpiDma struct to a move only API. (This is to ensure soundness in the presence of mem::forget).
  • Introduces a SpiDmaTransfer struct to represent an in-progress spi dma transfer.
  • Provides a SpiDmaBus and SpiDmaAsyncBus structs which wrap the SpiDma driver (and some dma bufs) to provide a convenient API that works with slices and doesn't require moving things around.
  • Added HIL tests

I'm happy to rename things. 🙂

Future planned enhancements:

  • Improving error handling/propagation.
    • There's a handful of errors that can occur that the existing driver doesn't expose and exposing them in the code as-is is awkward due to fact that the current driver supports asymmetric transfers, which the hardware reports errors for, because as far as the SPI peripheral is concerned, all transfers are symmetric. In a future PR, I'll remove support for asymmetric transfers in the core move based API, but leave it in the SpiDmaBus layer. Then error propagation can be implemented.
    • The semantics need to be decided for when the DMA transfer fails before the SPI transfer is done. SPI transfers can't be cancelled according to the TRM, so do we just wait until the SPI peripheral to finish sending garbage? Maybe setting SPI_USR to zero would actually stop the transfer but it's not documented....
  • Move/split/refactor the SPI specific SpiDmaTransfer struct to the dma module as a generic DmaTransfer (name pending) struct. I didn't do this in this PR due to error propagation issues I described above. Asymmetric transfers as they are need special care.
  • Introduce some traits for DmaTxBuf, DmaRxBuf and DmaTxRxBuf to implement as I highly anticipate other buffer types that enforce things like alignment and burst mode, or specific buffer type for external memory, or a buf type that temporarily combines multiple bufs into one buf, etc.
  • Where the SpiDmaBus and SpiDmaAsyncBus does chunking, the implementation can be enhanced to fill up a dma buf with the next chunk whilst the previous chunk is in flight, eliminating the latency gaps between each chunk. Haven't done this here as this PR is big enough.
  • Allow SpiDmaBus and SpiDmaAsyncBus to work in half duplex mode. Doing this right now leads to too much duplication, I'm exploring alternatives in Is the Half/Full duplex type state distinction really needed on the Spi driver? #1853 .
  • Allow quick conversions between DmaTxBuf and DmaRxBuf (by just moving the descriptors and buffer), as this will make it trivial to implement zero-copy peripheral to peripheral transfer. Imagine receiving a JPEG from LCD_CAM into a DmaRxBuf, then convert it into a DmaTxBuf and give it to the AES to encrypt it before storing it to an SD card.
  • Add some convenience macros to create dma buffers.

Testing

I've ran the related HIL tests and examples on an S3 and C6.

@bjoernQ
Copy link
Contributor

bjoernQ commented Jul 25, 2024

I guess FlashSafeDma could be removed with these changes

Dominic Fischer added 2 commits July 27, 2024 14:15
# Conflicts:
#	esp-hal/src/dma/mod.rs
#	esp-hal/src/soc/mod.rs
#	esp-hal/src/spi/master.rs
#	hil-test/tests/spi_full_duplex_dma.rs
@Dominaezzz Dominaezzz force-pushed the new_spi_dma branch 2 times, most recently from 8828e91 to 3f34276 Compare July 27, 2024 21:36
@bjoernQ
Copy link
Contributor

bjoernQ commented Jul 30, 2024

Apparently, this breaks the qspi_flash example on ESP32-C6 for me (accessing a GD25Q64C)

On current main (81f3776aa79d5b6d0098ecf499abaf03e0a6ef56) it works fine and prints the expected output. On this branch it just freezes

@bjoernQ
Copy link
Contributor

bjoernQ commented Jul 30, 2024

Seems like dma_write is somehow broken - changing spi_loopback_dma to use dma_write instead of dma_transfer works on current main but freezes with this branch

@Dominaezzz
Copy link
Collaborator Author

Apparently, this breaks the qspi_flash example on ESP32-C6 for me (accessing a GD25Q64C)

On current main (81f3776aa79d5b6d0098ecf499abaf03e0a6ef56) it works fine and prints the expected output. On this branch it just freezes

I sadly don't have that chip to test. Does it freeze in the loop or in the first few commands?

@bjoernQ
Copy link
Contributor

bjoernQ commented Jul 30, 2024

Apparently, this breaks the qspi_flash example on ESP32-C6 for me (accessing a GD25Q64C)
On current main (81f3776aa79d5b6d0098ecf499abaf03e0a6ef56) it works fine and prints the expected output. On this branch it just freezes

I sadly don't have that chip to test. Does it freeze in the loop or in the first few commands?

Seems like it freezes in the first call to wait - you can also run the example without anything attached (maybe a logic analyzer)

@Dominaezzz
Copy link
Collaborator Author

Whilst I made a silly mistake when creating the SpiDmaTransfer struct, I also found an existing bug in SpiDma driver.
writes with a buffer of length zero will return a DmaTransferRx who's is_done() will never return true, even though wait() will quickly return.
Long story short, the channel's EOF mode (emit EOF interrupt when descriptor data is removed from the DMA FIFO, rather than when the descriptor data enters the DMA FIFO) means the TX channel never completes.

I've worked around this problem for now but I'm not sure what the long term solution for this should be. Perhaps transfer with data length zero shouldn't be allowed? Not sure.

@bjoernQ
Copy link
Contributor

bjoernQ commented Jul 31, 2024

Thanks!

Unfortunately, on this branch

  • cargo xtask run-example examples esp32c6 qspi_flash still freezes
  • cargo xtask run-example examples esp32c6 spi_loopback_dma freezes when changing the example to just do a write instead of transfer (same for just dma_read)

Both is working on main currently

@Dominaezzz Dominaezzz mentioned this pull request Jul 31, 2024
5 tasks
@Dominaezzz
Copy link
Collaborator Author

The workaround didn't work out, the GDMA channel isn't behaving like I expect so I've reverted to the behaviour on main for now.

@bjoernQ
Copy link
Contributor

bjoernQ commented Aug 2, 2024

Thanks! The spi_loopback_dma works fine again also when using dma_write/dma_read

The qspi_flash example (when using an SPI flash chip) also doesn't freeze anymore but there seem to be still issues compared to main

Output on main (as expected):

[48, 65, 6c, 6c, 6f, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff]
Hello!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!................................................................
[48, 65, 6c, 6c, 6f, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, 21, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff]
Hello!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!................................................................

Output on this branch:

[48, 0, 65, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff]
H.elllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllll................................................................
[48, 0, 65, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, 6c, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff, ff]
H.elllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllll................................................................

I know you don't have that SPI flash chip but maybe a logic-analyzer will reveal the problem, too?

It also seems to be a problem with write since running the example from main and commenting out writing on this branch reads the correct data apparently

@Dominaezzz
Copy link
Collaborator Author

Thanks! Yeah I can see it in my logical analyzer. It appears the first write works as expected but subsequent writes don't.

@Dominaezzz
Copy link
Collaborator Author

Well this is annoying, the issue is due to timing.... (Originally found in #489 it seems!)

The ESP32 chips insist on having a (undocumented) delay between starting the DMA and starting the peripheral.

Now that I've moved the descriptor setup/initialization out of the SPI driver, the delay from doing it is gone, which means the DMA doesn't have enough time to fill the SPI async fifo before the SPI peripheral starts sending data.

I was aware that LCD_CAM had this issue but it seems it's a general thing.

Note: The reason dma_transfer works and dma_write doesn't, is because the RX channel is setup after the TX one, which functions as a delay... 🙂.

I've added a delay to fix this but I'm open to alternatives

Copy link
Contributor

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! - Code looks good to me and everything works fine (again)

# Conflicts:
#	esp-hal/src/dma/mod.rs
#	esp-hal/src/lib.rs
#	esp-hal/src/spi/master.rs
esp-hal/src/dma/mod.rs Show resolved Hide resolved
esp-hal/src/dma/mod.rs Show resolved Hide resolved
esp-hal/src/lib.rs Show resolved Hide resolved
Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firstly, I'm very sorry for taking so long to get to this! I really appreciate the effort your putting in and I will make it a priority to review your PRs in a more timely fashion.

Overall, the PR, and particularly he SPI driver improvements are really nice. I am also glad that FlashSafeDma is now dead :D. I do have a couple of concerns which I think are worth discussing.

I think it's unlikely that a new user is going to want to use the move-based API initially. I think that the most common path would be to use SpiDmaBus or SpiDmaAsyncBuswhich begs the question, should this be the default behaviour of SpiDma?

Perhaps we could have a with_dma() call that includes a set of descriptors (how it works before this PR), and a seperate with_dma_split_descriptors (I'm sure there is a better way to name this) that gives you a move based SPI where you passed the descriptors to allow for queuing etc.

What do you think?

@Dominaezzz
Copy link
Collaborator Author

Thanks for having a look. (I appreciate you've been on leave and also ill for a bit)

Yes I see where you're coming from, make the common case the default path.
There's the complication that SpiDma(Async)Bus doesn't support half duplex but I should be able to work around that with some where clauses.

Dominic Fischer added 2 commits August 19, 2024 22:14
# Conflicts:
#	hil-test/tests/spi_full_duplex_dma.rs
#	hil-test/tests/spi_half_duplex_write.rs
@Dominaezzz
Copy link
Collaborator Author

I couldn't figure out the where clauses 😅. I've added a with_buffers(...) method to make it easy to go the default path.

.with_dma(channel)
.with_buffers(dma_tx_buf, dma_rx_buf);

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't figure out the where clauses 😅. I've added a with_buffers(...) method to make it easy to go the default path.

.with_dma(channel)
.with_buffers(dma_tx_buf, dma_rx_buf);

I think this works well!

This LGTM, thanks for sticking with me, I think the end result is really nice! Looking forward to the future PRs.

@MabezDev MabezDev added this pull request to the merge queue Aug 20, 2024
Merged via the queue into esp-rs:main with commit 41f9925 Aug 20, 2024
18 checks passed
@MabezDev MabezDev mentioned this pull request Aug 20, 2024
@Dominaezzz Dominaezzz deleted the new_spi_dma branch August 20, 2024 12:43
SergioGasquez added a commit to SergioGasquez/esp-hal that referenced this pull request Sep 9, 2024
* Add self-testing mode for `TWAI` peripheral. (esp-rs#1929)

* Add self-testing mode for `TWAI` peripheral

* changelog entry

* fix docs build

* fix async example

* Restore example to original state

fix comment

* `NoAck` -> `SelfTest`

* DMA: Don't require implementors of Read/WriteBuffer to be Sealed (esp-rs#1921)

* DMA: Don't require implementors of Read/WriteBuffer to be Sealed

* CHANGELOG

* mark dma::ReadBuffer and dma::WriteBuffer traits unsafe

* Reset peripherals on driver construction (where missing) (esp-rs#1893)

* Reset peripherals on driver contruction (where missing)

* Don't enable and reset SHA in HMAC ctor

* changelog

* Don't reset the TIMG0

* Deny missing docs at the package level, adding exceptions for relevant modules (esp-rs#1931)

* Fix an infinite loop in interrupt executors (esp-rs#1936)

* Add failing test

Fix the name of the test fn

* Fix interrupt executor looping

* Fix formatting

* Fix changelog reference

* Move changelog to the right crate

* Remove dead code

* Fix `i2c` + get rid of unused constants/enums (esp-rs#1940)

* Slightly clean up embassy HIL tests (esp-rs#1937)

* Implemented queue_msg_waiting. (esp-rs#1925)

* Implemented queue_msg_waiting.

* Fmt.

* Adjusted changelog.

* Fixed CI.

* Fixed pointer mutability.

* Update to latest release (`0.6.0`) for `embassy-executor` in `esp-embassy-hal` (fixes esp-rs#1941) (esp-rs#1942)

* Updated to latest release (`0.6.0`) for `embassy-executor`

* update changelog

* update hil-test version of embassy-executor to 0.6.0

* update embassy-executor in `examples`

* reflect esp_hal change in `OneShotTimer` to not have a lifetime.

* update changelog

* revert OneShotTimer changes

* ESP32C6: Make `ADC` usable after `TRNG` deinitialization (esp-rs#1945)

* Make `ADC` usable after `TRNG` deinicialization (esp32c6)

* Changelog entry

* Adding `TWAI` HIL test (esp-rs#1946)

* Adding `TWAI` HIL test

* add `Frame` trait

* mutability

* Update probe-rs, prebuild xtask for HIL tests (esp-rs#1939)

* Update probe-rs

* Differentiate jobs

* Explicitly print that probe-rs's execution failed

* Do not capture stdin/stderr

* Pre-build xtask binary

* Use current_directory

* Print more info when a file can't be read

* Do not erase flash after a failure

* get_time: fail faster

* Make xtask runnable

* Removing raw addresses manipulations - part 3 (esp-rs#1892)

* WIP state

* More fixes

* Roll back `esp-storage` changes

* Small fixes

Will not work, needs another patch for PACs

* update pacs dep

* Lint

* Get rid of unnecessary if-else

fix

* New pacs version

* make contribution docs more visible (esp-rs#1947)

* Clean up i2s_async test, add option to repeat (esp-rs#1951)

* Simplify I2S async test

* Allow running tests repeatedly

* Fail at the first mismatch

* Clean up

* Further clean up timers/executors test (esp-rs#1953)

* Further clean up embassy_timers_executors

* Do not delay for so long

* Print timer values on assert failure

* Clean up some more

* Retry test a few times to counteract probe-rs halting us

* Fix formatting

* Fix GPIO Touch pin I/O (esp-rs#1956)

* Do not reset `UsbSerialJtag` peripheral (esp-rs#1961)

* Fix typos

* Add a function to detect debugger connection

* Do not reset USB peripheral

* Changelog

* Fix different register names

* Reuse xtensa_lx::is_debugger_attached

* Improve SYSTIMER API (esp-rs#1871)

* Improve SYSTIMER API

* Remove config object

* fix things

* Allow erasure of unit and comparator numbers

* Merge fail

---------

Co-authored-by: Dominic Fischer <git@dominicfischer.me>

* Simplify initialization APIs (esp-rs#1957)

* Accept more types in embassy::init

* Apply the same treatment to esp-wifi

* Changelog

* Clean up

* Add doc examples

* Fix Alarm generic parameters

* Some xtask/metadata cleanups (esp-rs#1965)

* Clean up almost all clippy violations

* Remove redundant variable from context

* Do not clone configs

* Do not collect all config symbols into a vec needlessly

* Do not allocate so many strings

* Implement Sniffer API (esp-rs#1935)

* Implemented queue_msg_waiting.

* Fmt.

* Adjusted changelog.

* Fixed CI.

* Fixed pointer mutability.

* Implemented experimental sniffer api.

* Fixed CI..

* Added safety comment.

* Featured gated, PromiscuousPkt

* Format.

* Adjusted imports.

* Added injection example.

* Made RxControlInfo::from_raw public.

* Format.

* Added sniffer example.

* Add more SPI DMA (full-duplex) HIL tests (blocking and async) (esp-rs#1952)

* Add more SPI DMA HIL tests (blocking and async)

* move test repetitions into loops instead, add a description about why PCNT is used, import embedded_hal_async::spi

* clean up

* Add basic HIL test for GPIO that can be configured as  pin for (esp-rs#1963)

* Patch typo in debug assist register name and used patched esp-pacs (esp-rs#1968)

* Disable RTT polling in HIL tests by default (esp-rs#1960)

* Disable defmt-rtt by default

* Update i2s test based on changes done to async

* fmt

* Update readme

* Update more tests

* Refactor SHA to use trait. Implement Digest traits for SHA (esp-rs#1908)

* feat(SHA): Refactor SHA to use trait. Implement Digest traits for SHA

* Fix CI. Fix wrong sha mode for esp32

* Save hash register for interleaving operation

An example (wip) `sha_fuzz.rs` was added to test different functionalities of the SHA driver and to ensure proper functionning under all cases.

* Use random data when testing SHA

* fix(SHA): Buffer words until a full block before writing to memory

This fixes interleaving operations by buffering words into the SHA context until a full block can be processed.

* Fix(SHA): Use correct length padding for SHA384 and SHA512.

- This fixes a long running issue with SHA384 and SHA512, where some digest of specific sizes wouldn't compute correctly, by changing the padding length of the size field.

* Re-export digest for convenience

* Remove completed TODO

* Remove SHA peripheral requirement.

- Document safety of the SHA driver.

---------

Co-authored-by: Scott Mabin <scott@mabez.dev>

* Fix 1GB elfs (esp-rs#1962)

* [3/3] DMA Move API: Introduce DMA buffer objects (esp-rs#1856)

* [3/3] DMA Move API: Introduce DMA buffer objects

* Remove FlashSafeDma

* Add async HIL test

* Handle set_length(0) correctly

* Fix tx/rx booleans

* Unlucky

* Preserve previous blocking semantics

* Add delay between starting DMA TX and SPI driver

* Update CHANGELOG

* merge tidy

* Add with_buffers builder

---------

Co-authored-by: Dominic Fischer <git@dominicfischer.me>

* Run HIL tests as part of PR checks (esp-rs#1959)

* Run HIL tests as part of PR checks

* Cancel pending HIL runs

* Only run for ready PRs

* Remove `free(self)` in HMAC which goes against esp-hal API guidelines (esp-rs#1972)

* Remove `free(self)` which goes against esp-hal API guidelines

* changelog

* correct changelog sections (esp-rs#1974)

* Remove redundant WithDmaSpi traits (esp-rs#1975)

Co-authored-by: Dominic Fischer <git@dominicfischer.me>

* Fix S2 systimers (esp-rs#1979)

* Add basic systimer interrupt tests

* Remove unnecessary condition

* Fix edge interrupt bitmasks

* Modify target_conf in critical section

* Remove unnecessary fn call

* Fix test

* Add failing test case

* Fix S2 systimer interrupts being fired unexpectedly

* Add changelog entry

* Format

* Fix C2 delays (esp-rs#1981)

* Re-enable delay tests on S2 and C2

* Systimer: use fn instead of constant to retrieve tick freq

* Reformulate delay using current_time

* Take actual XTAL into account

* Re-enable tests

* Fix changelog

* Disable defmt

* Remove unused esp32 code

* Update esp-hal/src/delay.rs

Co-authored-by: Jesse Braham <jessebraham@users.noreply.github.com>

---------

Co-authored-by: Jesse Braham <jessebraham@users.noreply.github.com>

* Get rid of `missing docs` in a number of modules (esp-rs#1967)

* Get rid of missing docs in a number of modules

* address reviews

* Address the rest of reviews

* remove all remaining `allows`

* are you serious?

* Add tests to ensure that we don't reset current_time drivers (esp-rs#1978)

* tell cargo about all our custom cfgs (esp-rs#1988)

* tell cargo about all our custom lints

* fixup the unexpected cfg lints, including remove clic

* HIL: Multiple featuresets & conditionally enable generic-queue feature (esp-rs#1989)

* Conditionally enable generic-queue feature

* Allow specifying multiple feature sets and run all of them

* parl_io: use ReadBuffer/WriteBuffer for async DMA (esp-rs#1996)

* parl_io: use ReadBuffer/WriteBuffer for async DMA

* CHANGELOG

* Use uhubctl to disable and enable usb ports (esp-rs#1997)

* Remove files after test (esp-rs#1993)

* Refactor SPI tests & re-enable S3 and some S2 (esp-rs#1990)

* Deduplicate spi_full_duplex_dma_async

* Refactor SPI tests

* Separate out PCNT tests

* Re-enable test on S3

* Re-enable some S2 tests

* gpio: Make AnyPin, AnyInputOnlyPin, DummyPin available from gpio module (esp-rs#1918)

Making these available straight from `gpio` aligns it with other Embassy
implementations (mainly nrf and stm32).

Signed-off-by: Priit Laes <plaes@plaes.org>

* Clean up SHA, RSA, mandate `#[must_use]` on Futures (esp-rs#2000)

* Janitor go brr

* Clean up SHA

* Use max CPU speed

* RSA cleanup part 1

* Clean up nonsense comments

* Mark all futures as must_use

* Prefer `cfg_if` (esp-rs#2003)

* SPI DMA: use `State` for both blocking and async operations (esp-rs#1985)

* use `State` for both blocking and async operations, remove async version of SpiDmaBus in favour of being generic over the mode

* reuse wait_for_idle more

* changelog

* rename generic params for consistency

* Add duplex mode to SpiDmaBus

* implement HalfDuplexReadWrite for SpiDmaBus

* Docs on new async APIs

* Limit half duplex transfers to the capacity of the DmaBuf

* docs

* rebase tests

* address review comments

* remove duplex traits from spi

* fix tests

* spi docs rejig

* s/InUse/TemporarilyRemoved/g

* Re-add feature gate for software_interrupt3 (esp-rs#2011)

* Fix (esp-rs#2013)

* Implement timer conversion for some arrays (esp-rs#2012)

* Test and fix async RSA (esp-rs#2002)

* RSA cleanup & API consistency change, part 2

* RSA cleanup & API consistency change, part 3

* Add async tests

* Fix async for ESP32

* Merge impl blocks

* Backtrack on some mutability changes

* Use Acquire/Release ordering

* Fwd to write_multi_start instead of duplicating impl

* Only reserve the interrupt when executors are needed (esp-rs#2014)

* forward spi methods to SpiDmaBus (esp-rs#2016)

* forward spi methods to SpiDmaBus

* changelog

* Fix defmt compatibility (esp-rs#2017)

* Fix defmt compatibility

* Update tests to cover macros

* Remove unneeded logs (esp-rs#2022)

* HIL: Don't skip cleanup (esp-rs#2024)

* Don't skip cleanup

* Make sure the power is off for a short while

* Use newly published versions of all PACs in `esp-hal` (esp-rs#2025)

* Use newly published versions of all PACs in `esp-hal`

* Address additional review comments

* Version 0.20.0 (esp-rs#2038)

* Update package dependencies and bump version numbers

* Update `CHANGELOG.md` for each package to be published

* Remember to update `xtensa-lx-rt` too :)

* Add and use TrapFrame::new() in esp-wifi

* Bump `xtensa-lx-rt` by minor instead of patch, as there are breaking changes

---------

Co-authored-by: Dániel Buga <bugadani@gmail.com>

* Fix before_snippet failing in release (esp-rs#2040)

* Fix before_snippet failing in release

* Fix esp-hal-embassy comment

* Prepare v0.20.1 release (esp-rs#2046)

* Try to be more helpful (esp-rs#2044)

* Begin next release cycle (esp-rs#2039)

Co-authored-by: Scott Mabin <scott@mabez.dev>

* fix: Fix nightly errors (esp-rs#1934)

* Disable object's unnecessary features in proc macro that loads LP code (esp-rs#2018)

* Disable object/decompress

* Only enable elf support in object

* Random cleanups in non-checked packages (esp-rs#2034)

* Deduplicate feature check macros

* Re-enable rust-analyzer for most of the workspace

* Cargo fix

* Turn off defmt

* Only build xtask

* Clippy pls

* Fix CI

* Fix paths

* Always create doc directory first

* Revert r-a

* Update esp-hal-procmacros/src/lp_core.rs

Co-authored-by: Dominic Fischer <14130965+Dominaezzz@users.noreply.github.com>

---------

Co-authored-by: Dominic Fischer <14130965+Dominaezzz@users.noreply.github.com>

* QSPI tests (esp-rs#2015)

* Add QSPI tests

* Simplify

* Add qspi_write_read test

* Clean up gigantic GPIO eyesore (esp-rs#2048)

* Save/restore coprocessor state on stack (esp-rs#2057)

* Whitespace

* Save and restore coprocessor enable state

* release prep xtensa-lx-rt@0.17.1 (esp-rs#2060)

* Protect SYSTIMER/TIMG shared registers (esp-rs#2051)

* Protect SYSTIMER/TIMG shared registers

* some review comments

* more review comments

* more review comments

* bring back portable_atomic

---------

Co-authored-by: Dominic Fischer <git@dominicfischer.me>

* automatically apply status:needs-attention to new esp-hal issues (esp-rs#2030)

* Improve CP0-disabled error message (esp-rs#2061)

* Improve CP0-disabled error message

* CHANGELOG.md

* Rework hal initialization (esp-rs#1970)

* Rework hal initialization

* Turn sw interrupt control into a virtual peripheral

* Return a tuple instead of a named struct

* Fix docs

* Remove SystemClockControl

* Move software interrupts under interrupt

* Re-document what's left in system

* Update time docs

* Update sw int docs

* Introduce Config

* Fix tests

* Remove redundant inits

* Doc

* Clean up examples&tests

* Update tests

* Add changelog entry

* Start migration guide

* Restore some convenience-imports

* Remove Config from prelude

* Fix hil-test xtask instruction (esp-rs#2062)

* Fix hil-test xtask instruction

* Fix another mention

* [esp-metadata] Make clap dependency optional (esp-rs#2055)

* Provide ehal impls for DummyPin (esp-rs#2019)

Co-authored-by: Scott Mabin <scott@mabez.dev>

* storage: Clean up ROM function declarations (esp-rs#2058)

* Clean up external function declarations

* Tweak syntax

* Fix various SPI/DMA issues (esp-rs#2065)

* Add failing test

* Fix enabled interrupt

* Fix using the correct waker

* Changelog

* Enable test on more devices that have SPI3

* WPA2 ENTERPRISE (esp-rs#2004)

* WPA2 ENTERPRISE

* Defmt, Clippy, Changelog

* Defmt, again

* Clippy, again

* Mention corresponding JIRA ticket

* Rename

* fmt

* Use Mutex in scheduler

* Adapt wifi_delete_queue

* Adapt log level

* Bump to esp-wifi 0.9.0 (esp-rs#2066)

* Remove NoPinType (esp-rs#2068)

* Make esp-wifi build on stable, again. Bump to 0.9.1 (esp-rs#2067)

* Make esp-wifi build on stable, again. Bump to 0.9.1

* CHANGELOG.md

* MSRV check esp-wifi

* ESP32-S2 doesn't support Bluetooth

* Remove lazy_static in favor of OnceLock (esp-rs#2063)

* [esp-metadata] Remove lazy_static in favor of OnceLock

* [esp-wifishark] Remove lazy_static in favor of normal initialisation

* [ieee802154-sniffer] Shorten SelectorConfig initialisation

* [ieee802154-sniffer] Remove lazy_static in favor of normal initialisation

* Remove most trait implementation features from `esp-hal` (esp-rs#2070)

* Eliminate esp-hal's `ufmt` feature

* Eliminate esp-hal's `embedded-hal-02` feature

* Eliminate esp-hal's `embedded-hal` feature

* Eliminate esp-hal's `embedded-io` feature

* Eliminate esp-hal's `async` feature

* Update `CHANGELOG.md`

* Remove `async` from required features for HIL tests

* Update migration guide

* Adding `I2C` HIL test (esp-rs#2023)

* i2c hil test

* pin

* fmt

* Test

* WIP (gpio test left)

* Finalize the CODE part (to be cleaned up)

fmt

* Smaller cleanup

* cleanup

* rebase

* fix

* getting last chips ready

* Addressing reviews

* Remove Gpio type aliasses (esp-rs#2073)

* Remove Gpio type aliasses

* Clean up examples

* Remove the need to manually pass clocks around (esp-rs#1999)

* Clean up passing clocks to drivers

* Update changelog

* Initialise Clocks in a critical section

* Fix calling now() before init

* Fix doc

* Fix esp-wifi migration guide

* Add safety comment

* Update tests

* Remove gpio dispatch macro-defining proc macro (esp-rs#2069)

* Keep a single PinType trait

* Merge impl blocks

* Deduplicate usb pad workaround

* Deduplicate some bit manipulation

* Remove gpio dispatch proc macro

* Inline PinType into GpioProperties

* Remove AnyInputOnlyPin (esp-rs#2071)

* Remove AnyInputOnlyPin

* Add section to migration guide

* Remove unnecessary enum

Co-authored-by: Dominic Fischer <14130965+Dominaezzz@users.noreply.github.com>

---------

Co-authored-by: Dominic Fischer <14130965+Dominaezzz@users.noreply.github.com>
Co-authored-by: Jesse Braham <jessebraham@users.noreply.github.com>

* Accept ErasedPin in AnyPin (esp-rs#2072)

Co-authored-by: Jesse Braham <jessebraham@users.noreply.github.com>

* fix issue handler, don't rebuild on main the merge queue checks this for us (esp-rs#2077)

* Fix nightly warnings (esp-rs#2082)

* Build examples in debug mode (esp-rs#2078)

* Build examples in debug mode

* Allow building psram examples in debug mode in CI

* Don't rebuild tests, try to avoid rebuilding dependencies

* Improve SHA driver API (esp-rs#2049)

Co-authored-by: Dominic Fischer <git@dominicfischer.me>

* [esp-hal-procmacros] Update to proc-macro-error2 (esp-rs#2090)

* lcd_cam: fix wrong buffer length used if 16bit and len<=8192 (esp-rs#2085)

* lcd_cam: fix wrong buffer length used if 16bit and len<=8192

* changelog

* Implement sleep and wakeup functionalities for ESP32C2 esp-rs#1920 (esp-rs#1922)

* i2c: fix embedded-hal transactions (esp-rs#2028)

* i2c: fix embedded-hal transactions

* changelog+fmt

* small naming cleanup

* i2c: fix 1 byte reads

* typo

* small cleanup and add a few internal docs

* update changelog

* rebase & CHANGELOG

* extract next op conversion

* fix `setup_read()` logic for 0 length reads.

* return error for 0 length reads and 0 length  writes where start=false

* comment about max_len in setup_write()

* filter out 0 length read operations in `transaction()`

* Short circuit for problematic 0 lengths in read_operation and write_operation

* don't short circuit a 0 length write operation if stop=true

* handle write_read when the read bufer is empty

* Optionally type-erased GPIO drivers (esp-rs#2075)

* Remove type erased gpio structs

* Implement Peripheral for ErasedPin

* Simpler type erasing, accept ErasedPin in pin drivers, remove type erased drivers

* Reformulate pin drivers using Flex

* Erase gpio types by default

* Accept any pin in AnyPin

* Add changelog and migration guide

* Fix tests and examples

* Undo rename of clone_unchecked

* Rename `esp_hal::time::current_time` to `esp_hal::time::now` (esp-rs#2091)

* rename esp_hal::time::current_time to esp_hal::time::uptime

* changelog

* move more things to init

* s/uptime/now/g

* Add missing #[doc(hidden)] in xtensa-lx-rt-proc-macros (esp-rs#2097)

* Enable ESP32 HIL (esp-rs#1977)

* Enable ESP32 HIL

* RMT fixed

* SPI DMA partially works, _pcnt tests not working

* bckup

* finish

* readme and cleanup

* rebase + cleanup

* RMT S2 pin typo + clean forgotten comments

* review comments

* update 10000

* indentation

* replace cfg gate with cfg_if

* esp-wifi: other crates also provide `strchr` (littlefs2-sys) (esp-rs#2096)

* esp-wifi: other crates also provide strchr (littlefs2-sys)

* esp-wifi: other crates also provide strchr (littlefs2-sys)

* changelog

* fmt :-(

* Reordered RX-TX pairs to be consistent (esp-rs#2074)

* feat: Update rx-tx order in i2s

* feat: Update rx-tx order in dma macros

* feat: Update rx-tx order in spi

* feat: Update rx-tx order in aes

* feat: Update rx-tx order in mem2mem

* feat: Update rx-tx order in twai and split methods

* feat: Update rx-tx order in twai

* feat: Update rx-tx order in twai and uart docs

* docs: Add sentence about order

* docs: Update changelog

* feat: Update rx-tx order in embassy_interrupt_spi_dma tests

* style: Rustfmt

* docs: Migrating guide

* fix: Typo

Co-authored-by: Dániel Buga <bugadani@gmail.com>

* fix: Diff

Co-authored-by: Dániel Buga <bugadani@gmail.com>

* fix: Tests rx-tx order

* fix: Update new_with_default_pins order

* feat: Update rx/tx order in hil_test::common_test_pins!

* feat: Update dma_extmem2mem example

* fix: Revert deleted input arg

* style: rustfmt

* feat: Disable test_asymmetric_dma_transfer for S2

---------

Co-authored-by: Dániel Buga <bugadani@gmail.com>

* Random additional GPIO cleanups, implement Peripheral for drivers (esp-rs#2094)

* Reuse enable_iomux_clk_gate

* Remove public functions

* Remove set_to_input

* Deduplicate constructor

* Deduplicate is_listening

* Hide PinFuture better

* Deduplicate set_int_enable

* Align macro indentation

* Typo

* Slightly simplify the touch_into macro

* Implement the AnalogPin trait directly

* Provide default impls for simple forwarding methods

* Newtype ErasedPin

* Merge rtc_pin macros

* Fmt

* Changelog

* Fix migration guide

* Fix example

* Fix ETM

* Make additional memory available as `dram2_uninit` (esp-rs#2079)

* Make additional memory available as `dram2_uninit`

* CHANGELOG.md

* Update esp-println version in usage section (esp-rs#2100)

* Add integration with bt-hci crate (esp-rs#1971)

* Add integration with bt-hci crate

Implementing traits from bt-hci allows the BleConnector to
be used with the Trouble BLE stack.

* use packed based read interface

* Improve example to allow another connection after disconnect

* update trouble version

* Workaround for spurious command complete events

* fix formatting

* ignore notify errors in example

* fix clippy warnings

* remove async feature from hal dependency

* remove deprecated feature from example

* Adopt to api changes

* Api fix for esp32

* Set rust-version of esp-wifi

* bump MSRV to 1.77 for CI and esp-hal

* Add changelog entry

* ensure that clock init happens after rtc domain is initialized (esp-rs#2104)

* Prepare esp-backtrace 0.14.1 (esp-rs#2107)

* esp-wifi uses global allocator, esp-alloc supports multiple regions (esp-rs#2099)

* esp-wifi uses global allocator, esp-alloc supports multiple regions

* CHANGELOG.md

* Apply suggestions

* Use `alloc` when linting esp-wifi

* Make coex example build for ESP32

* Re-enable some wifi examples for ESP32-S2

* Optionally depend on `esp-alloc` (by default)

* Rename INSTANCE -> HEAP

* feat: Add issue templates

---------

Signed-off-by: Priit Laes <plaes@plaes.org>
Co-authored-by: Kirill Mikhailov <62840029+playfulFence@users.noreply.github.com>
Co-authored-by: liebman <liebman@zod.com>
Co-authored-by: Juraj Sadel <juraj.sadel@espressif.com>
Co-authored-by: Jesse Braham <jessebraham@users.noreply.github.com>
Co-authored-by: Dániel Buga <bugadani@gmail.com>
Co-authored-by: Frostie314159 <frostie.neuenhausen@gmail.com>
Co-authored-by: Sycrosity <72102935+Sycrosity@users.noreply.github.com>
Co-authored-by: Scott Mabin <scott@mabez.dev>
Co-authored-by: Fan Jiang <ProfFan@users.noreply.github.com>
Co-authored-by: Dominic Fischer <14130965+Dominaezzz@users.noreply.github.com>
Co-authored-by: Dominic Fischer <git@dominicfischer.me>
Co-authored-by: Anthony Grondin <104731965+AnthonyGrondin@users.noreply.github.com>
Co-authored-by: Priit Laes <plaes@plaes.org>
Co-authored-by: Björn Quentin <bjoernQ@users.noreply.github.com>
Co-authored-by: Gnome! <david2005thomas@Gmail.com>
Co-authored-by: M4tsuri <me@m4tsuri.com>
Co-authored-by: Szybet <53944559+Szybet@users.noreply.github.com>
Co-authored-by: Ulf Lilleengen <ulf.lilleengen@gmail.com>
}

/// Returns the buf as a mutable slice than can be written.
pub fn as_mut_slice(&mut self) -> &mut [u8] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In its current form this function is a footgun. It gives access to the whole buffer, but it does not update the length.

Should we

  • ... set the maximum length when this is called?
  • ... only return a subset of the buffer, controlled by set_length?
  • ... document that the user should probably call set_length?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this is confusing, which is why I didn't implement Deref/AsRef/Borrow.

I know I definitely want users to be able to access the whole buffer regardless of length somehow, how this is done I'm not too fussed about.

Definitely not the first option at least. Since there will be no way to edit the buffer without changing its length.

This is issue worthy

In its current form this function is a footgun.

I hope you didn't shoot your foot recently 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope you didn't shoot your foot recently

No, I did not :) but I'm reading through this code, because for some reason that one failing test fails because it overwrites the TX buffer, and I'm wondering why that may be :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more options:

  • A closure based API where the user has to return the number of bytes "used".
  • We remove as_mut_slice and rely on fill only.
  • We consider DmaTxBuf etc short lived, slice manipulation should happen outside before passing it into DmaTxBuf

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 this pull request may close these issues.

Bugs in FlashSafeDma<SpiDma>
6 participants