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

serial_dma in a loop #155

Open
pdgilbert opened this issue Oct 21, 2020 · 3 comments
Open

serial_dma in a loop #155

pdgilbert opened this issue Oct 21, 2020 · 3 comments
Labels
question Further information is requested

Comments

@pdgilbert
Copy link

pdgilbert commented Oct 21, 2020

Following the example serial_dma which does not loop I have worked out an example that in a loop reads and echos a string of characters (see below). This seems uglier and more fragile than I expected, and I am wondering if I have missed something that I should be doing. (Perhaps my novice Rust status means I just don't properly appreciate beauty yet?) The difficulty is that looping seems to require variables be modified rather than re-assigned, but because of difficulty destructuring assignments the tuples returned by rx1.read_exact() and tx1.write_all() cannot be pulled apart. My work around is to keep the tuples as send and recv and just reference the parts for function calls, but I would prefer more readable code. There are more comments in the code below.

So this issue is only that it would be nice if the example serial_dma could be improved to show how to properly read/write in a loop. The code below does work, but does not handle fast typing very well.

#![deny(unsafe_code)]
#![no_main]
#![no_std]

#[cfg(debug_assertions)]
extern crate panic_semihosting;

#[cfg(not(debug_assertions))]
extern crate panic_halt;

use cortex_m::singleton;
use cortex_m_rt::entry;
use cortex_m_semihosting::hprintln;
//use nb::block;

// setup() does all  hal/MCU specific setup and returns generic hal device for use in main code.

#[cfg(feature = "stm32f3xx")]  //  eg Discovery-stm32f303
use stm32f3xx_hal::{prelude::*, 
                    stm32::Peripherals,
		    serial::{Serial, Tx, Rx}, 
		    dma::dma1, 
		    stm32::USART1 
		    };

    #[cfg(feature = "stm32f3xx")]
    fn setup() ->  (Tx<USART1>, dma1::C4, Rx<USART1>, dma1::C5)  {

       let p = Peripherals::take().unwrap();
       let mut rcc = p.RCC.constrain();  
       let clocks    = rcc.cfgr.freeze(&mut p.FLASH.constrain().acr);
       let mut gpioa = p.GPIOA.split(&mut rcc.ahb); 

       let txrx1 = Serial::usart1(
    	   p.USART1,
    	   (gpioa.pa9.into_af7( &mut gpioa.moder, &mut gpioa.afrh), 
 	    gpioa.pa10.into_af7(&mut gpioa.moder, &mut gpioa.afrh)),
    	   9600.bps(),
    	   clocks,
    	   &mut rcc.apb2,
           );

       let (tx1, rx1)  = txrx1.split();

       let dma1 = p.DMA1.split(&mut rcc.ahb);
       let (tx1_ch, rx1_ch) = (dma1.ch4, dma1.ch5);

       (tx1, tx1_ch,   rx1, rx1_ch)
       }


#[entry]
fn main() -> ! {
    
    let (tx1, tx1_ch,   rx1, rx1_ch) = setup();

    hprintln!("test write to console ...").unwrap();

    let buf = singleton!(: [u8; 15] = *b"\r\ncheck console").unwrap();
    
    let send = tx1.write_all(buf, tx1_ch);
    let x = send.wait();                         //this is 3-tuple
    let (buf, tx1_ch, tx1) = x;
    
    // But attempting to modify rather than re-assign does not work.
    //   (buf, tx1_ch, tx1) = x;
    // gives cannot ... destructuring assignments are not currently supported

    *buf = *b"\r\nSlowly type  ";  //NB. 15 characters
    
    // Note that the buf assigned next is the one that will be used below in recv() and
    //   its size is determined by the size of the argument buf (15 as set above).
    
    let (buf, tx1_ch, tx1) = tx1.write_all(buf, tx1_ch).wait();
    
    let longer_buf = singleton!(: [u8; 36] = *b"15 characters in console.  Repeat.\r\n").unwrap();
    let (_longer_buf, tx1_ch, tx1) = tx1.write_all(longer_buf, tx1_ch).wait();


    // Now read from console into  buf and echo back to console

    hprintln!("Enter 15 characters in console. Repeat.").unwrap();
    hprintln!("Use ^C in gdb to exit.").unwrap();

    // Outside of loop this repeat read buf and echo would work, even multple times:
    //   let (buf, rx1_ch, rx1) = rx1.read_exact(buf, rx1_ch).wait();
    //   let (buf, tx1_ch, tx1) = tx1.write_all( buf, tx1_ch).wait();
    // but  when moved into a loop there are problems with
    // value moved in previous iteration of loop.
    // That problem would be fixed by modifying mut variables but because of 
    // "cannot ... destructuring assignments" problem the 3-tuple needs to be kept
    // and elements referenced.


    // create recv and send structures that can be modified in loop rather than re-assigned.
    let mut recv = rx1.read_exact(buf, rx1_ch).wait();    //this is 3-tuple (buf, rx1_ch, rx1)
    let mut send = tx1.write_all(recv.0, tx1_ch).wait();  //this is 3-tuple (buf, tx1_ch, tx1)

    // Note send (write) is using buf as put into recv (read). The returned buffer in recv and
    //   the argument buffer in send are data. The argument buffer in recv may be a holding spot 
    //   to put return buffer? but it is not part of the program logic. The size of the return
    //   buffer from recv does seem to be determined by the size of the recv argument buffer.
    //   The return buffer from send seems like it should be unnecessary, but it does provide
    //   the buffer needed in the recv argument.
    
    //each pass in loop waits for input of 15 chars typed in console then echos them
    loop { 
       recv = recv.2.read_exact(send.0, recv.1).wait();   
       send = send.2.write_all( recv.0, send.1).wait(); 
       }
}
@Sh3Rm4n
Copy link
Member

Sh3Rm4n commented Oct 22, 2020

I'm not sure about that one, maybe @ra-kete has an idea?

@Sh3Rm4n Sh3Rm4n added the question Further information is requested label Oct 22, 2020
@teskje
Copy link
Collaborator

teskje commented Oct 25, 2020

The difficulty is that looping seems to require variables be modified rather than re-assigned, but because of difficulty destructuring assignments the tuples returned by rx1.read_exact() and tx1.write_all() cannot be pulled apart.

Yes, that's an unfortunate limitation. Seems like it might be resolved soon-ish though: rust-lang/rfcs#2909

Until then you'll have to do the mutable assignments manually, which is more verbose but not too bad otherwise.
To make the existing serial_dma example looping, you'd first make all the relevant variables (tx, rx, {tx,rx}_buf, {tx,rx}_channel) mutable (by adding mut to their declarations and then do something like:

loop {
    let sending = tx.write_all(tx_buf, tx_channel);
    let receiving = rx.read_exact(rx_buf, rx_channel);

    let (buf, channel, x) = sending.wait();
    tx_buf = buf;
    tx_channel = channel;
    tx = x;

    let (buf, channel, x) = receiving.wait();
    rx_buf = buf;
    rx_channel = channel;
    rx = x;

    assert_eq!(tx_buf, rx_buf);
}

To limit the verbosity you can pull the individual parts together into tuples and move these between loop iterations instead:

let mut send = (tx_buf, tx_channel, tx);
let mut recv = (rx_buf, rx_channel, rx);

loop {
    let (tx_buf, tx_channel, tx) = send;
    let (rx_buf, rx_channel, rx) = recv;

    let sending = tx.write_all(tx_buf, tx_channel);
    let receiving = rx.read_exact(rx_buf, rx_channel);

    send = sending.wait();
    recv = receiving.wait();

    assert_eq!(send.0, recv.0);
}

This is what you are doing in your example if I'm not mistaken.

Or, another alternative would be to move the Transfer objects between the loop iterations instead:

let mut sending = tx.write_all(tx_buf, tx_channel);
let mut receiving = rx.read_exact(rx_buf, rx_channel);

loop {
    let (tx_buf, tx_channel, tx) = sending.wait();
    let (rx_buf, rx_channel, rx) = receiving.wait();

    assert_eq!(tx_buf, rx_buf);

    sending = tx.write_all(tx_buf, tx_channel);
    receiving = rx.read_exact(rx_buf, rx_channel);
}

All three approaches are readable enough I would say.

I hope I didn't totally misunderstand the issue at hand here ;) If not I'd be happy to extend the serial_dma example with one of the above code snippets (probably the last one as that looks best to me personally).

@pdgilbert
Copy link
Author

I look forward to the destructing proposed by rust-lang/rfcs#2909 , but with practice I am becoming less bothered by needing to keep the structure intact while looping. Thank you for your response. If you have a chance, I have some related questions. The first is that I have been trying to set up mut variables before the loop and avoid let assignment in the loop. My intuition is that let has to find and assign memory so it will be slower. But my intuition comes from a non-Rust background, so I am wondering if my intuition is correct?

The second question is concerning .wait(). In a slightly different example were I send text then receive it, rather than receive and then send as in the example I posted above, I think I discovered that I must first start recv = rx.read_exact() then do the send().wait(), and finally do the recv.wait(). If I simply do send().wait() then rx.read_exact().wait() the read misses the transmission. I see in your examples you are more careful about this. So, if I now understand correctly, .read_exact() sets up a process to listen and .wait() causes that process to return when there is an interrupt trigger. In the send process does the write_all() fill the buffer and then the .wait() cause the buffer to be sent?

I think the start vs trigger process may be a hint to the instability (need to type slowly) in the example in my original post. Should the .read_exact() process be started immediately after the preceding rx.wait(), even before the buffer is sent on, just to be sure the process is listening if something arrives while my code does other things? Or can the listen process be made to run continuously and rx.wait() used multiple times to keep returning filled buffers? (I'm suffering from always having had an OS to look after these details.)

Finally, I am finding that different hals have different functionally, and different names for similar functionality around dma serial transfer. The embedded-dma crate seems to be either used in or base on stm32f3xx-hal? Is there any hope that might lead to a common approach in other stm32 hals?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants