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

Allow dma::Transfer.peripheral() for serial::Rx #588

Merged
merged 1 commit into from
May 6, 2023

Conversation

qwerty19106
Copy link
Contributor

@qwerty19106 qwerty19106 commented Mar 14, 2023

  • Impl Transfer.is_idle() and Transfer.is_rx_not_empty() for serial::Rx and uart::Rx.
  • Relax trait bounds this methods.

See #586 for details.

@qwerty19106 qwerty19106 changed the title Allow Transfer.peripheral() for serial::Rx Allow dma::Transfer.peripheral() for serial::Rx Mar 14, 2023
@qwerty19106 qwerty19106 changed the title Allow dma::Transfer.peripheral() for serial::Rx [NOT MERGE] Allow dma::Transfer.peripheral() for serial::Rx Mar 14, 2023
@qwerty19106 qwerty19106 marked this pull request as ready for review March 14, 2023 14:45
@qwerty19106
Copy link
Contributor Author

It is ready for review.
I will add serial-dma-rx.rs a little later.

@qwerty19106
Copy link
Contributor Author

I add rtic-serial-dma-rx-idle example

@qwerty19106 qwerty19106 changed the title [NOT MERGE] Allow dma::Transfer.peripheral() for serial::Rx Allow dma::Transfer.peripheral() for serial::Rx Mar 16, 2023
@burrbull
Copy link
Member

I'm not happy in existence of fn peripheral which was introduced in #396 as it potentially break API safety.
I still think this was a mistake.

cc @therealprof your comments?

@qwerty19106
Copy link
Contributor Author

The fn peripheral problem seems to be delayed.
I can remove fn peripheral part of this PR to expedite the acceptance of this PR.
@burrbull

@rursprung
Copy link
Contributor

FYI: i had introduced the API you've marked as deprecated in #556 (see #550 for background). the original ticket also describes why i did this with a ReadBuffer instead of a WriteBuffer (it was a read use-case for me). if there was something wrong with that then i'm sorry (i'm absolutely no expert on STM32, HALs or this specific repo). but it'd just be good if the use-case i described there will still work one way or the other with a clean API afterwards.

@qwerty19106
Copy link
Contributor Author

@rursprung
A WriteBuffer is

Trait for buffers that can be given to DMA for writing.

Moreover the WriteBuffer::write_buffer method takes mutable reference unlike ReadBuffer:

unsafe fn write_buffer(&self) -> (*mut Self::Word, usize);

Thus DMA read word from peripheral and write to WriteBuffer.
We should use WriteBuffer for PeripheralToMemory, and ReadBuffer for MemoryToPeripheral.

Probably your example compiles with this bug because your buffer implements both WriteBuffer and ReadBuffer.

@qwerty19106 qwerty19106 force-pushed the dma_rx_peripheral branch 4 times, most recently from 3260776 to 699733b Compare May 6, 2023 13:31
@burrbull
Copy link
Member

burrbull commented May 6, 2023

Maybe it is better to implement RxISR for Transfer instead of using inherent methods.

@qwerty19106
Copy link
Contributor Author

I removed the controversial peripheral() code and add RxISR implementation for Transfer.

@burrbull
Copy link
Member

burrbull commented May 6, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented May 6, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit bf445ea into stm32-rs:master May 6, 2023
@qwerty19106 qwerty19106 deleted the dma_rx_peripheral branch May 7, 2023 07:01
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.

3 participants