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

feat: add spi rx dma (and migrate to embedded-dma) #165

Closed

Conversation

ForsakenHarmony
Copy link
Contributor

No description provided.


// Tell DMA to request from serial
channel.cselr().modify(|_, w| {
w.$dmacst().bits(0b0010) // TODO: Fix this, not valid for DMA2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to this it seems to always be map2

src/serial.rs Outdated Show resolved Hide resolved
@korken89
Copy link
Collaborator

korken89 commented Mar 8, 2021

Hi, what is the status of this PR?
It is marked as Draft, but it sort of seems like it's close to ready?

@korken89
Copy link
Collaborator

Ping again :)

@ForsakenHarmony
Copy link
Contributor Author

@korken89 it is / was marked as a draft because it depends on the SPI slave PR

@MathiasKoch
Copy link
Collaborator

marked as a draft because it depends on the SPI slave PR

Just merged that one for you :)

@korken89
Copy link
Collaborator

Thanks @MathiasKoch! I'll review this today!

@ForsakenHarmony
Copy link
Contributor Author

Ah, right I also noticed the Frame Reader is in a bit of a weird state with this

Not sure if the character_match should be used / moved to a different place

@korken89
Copy link
Collaborator

@ForsakenHarmony Not quite sure I am following, could you expand on that?

@ForsakenHarmony
Copy link
Contributor Author

See the failing test

error[E0599]: no method named `is_character_match` found for mutable reference `&mut Rx<stm32l4xx_hal::pac::USART2>` in the current scope
Error:    --> examples/rtic_frame_serial_dma.rs:115:28
    |
115 |         if cx.resources.rx.is_character_match(true) {
    |                            ^^^^^^^^^^^^^^^^^^ method not found in `&mut Rx<stm32l4xx_hal::pac::USART2>`

rx is consumed by the dma with the new API, so is_character_match has to be moved somewhere else

I also haven't found the FrameReader API in any of the other stm32-rs repos so far, so I'm not entirely sure how it works / is supposed to work

@korken89
Copy link
Collaborator

Ah, I see waht you mean. The frame reader works like this:

  1. Select a frame delimiter character, lets say 'a'
  2. The DMA will read into the buffer until it is full or an 'a' is then received (hence the character match)

E.g.: The following is received: sdfsdfaqweqwea.
Then we get 2 frames with sdfsdfa and qweqwea from the driver, assuming they fit in the buffer size provided.

A common use-case is if you encode data with COBS (\0 delimited), and then the DMA does all the work to detect frames for you.

So we need to find someway to make it work, what are you suggestions?

@ForsakenHarmony
Copy link
Contributor Author

@korken89 added a trait for the character match and added it to the FrameReader
let me know what you think

Signed-off-by: Leah <github.leah@hrmny.sh>
@ForsakenHarmony ForsakenHarmony marked this pull request as ready for review April 27, 2021 16:59
matching_character,
}
}
}

impl<BUFFER, PAYLOAD, CHANNEL, const N: usize> FrameReader<BUFFER, RxDma<PAYLOAD, CHANNEL>, N>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I suppose it would also be possible to just implement the trait here

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is the right place :)

@korken89
Copy link
Collaborator

Super, we are getting close! :D
I think your comment on the frame reader is correct as well.

One question, the DMA for SPI is setup for RX are you planning to support TX and TX/RX transaction, or is this left out for now?

@korken89
Copy link
Collaborator

korken89 commented May 2, 2021

Could you have a look at fixing the failing tests?
I think we can merge this as soon as possible, and we can extend the missing features in coming PRs.

@korken89
Copy link
Collaborator

Hi, @ForsakenHarmony , I provided fixes for the errors here: #230

One questions has popped up for me, how is it planned that this will be used?
From what I understand from the code I cannot see how to perform a SPI transfer using DMA based on this, as there is no way to send data (which drives the SPI read transfer).
Could you provide an usage example indicating this please?

@ForsakenHarmony
Copy link
Contributor Author

ForsakenHarmony commented May 26, 2021

Sorry, I keep missing these GitHub notifications, I have too many

https://github.com/ForsakenHarmony/roboclub-lighting/blob/main/stm/src/main.rs#L225-L228

Here's how I'm using it, I've only used it in slave mode, which means a master sends data and clock

@korken89
Copy link
Collaborator

korken89 commented May 26, 2021

Ahh, makes sense - thanks!
I'll continue adding the SPI master Tx and RxTx support then in the parallel PR.

korken89 added a commit that referenced this pull request Jun 20, 2021
SPI RX/TX DMA and move to `embedded-dma` (continuation of #165)
@korken89
Copy link
Collaborator

This was merged as a part of the work in #230.
Thanks for starting this PR!

@korken89 korken89 closed this Jun 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants