Add Serial print and println functionality

[timbell87]

Serial print and println functionality similar to that implemented by Arduino. I understand this is already in the works so I'm just placing this here to track progress.

In the mean time for any passers by trying to get this functionality to work (example tailored to Arduino Uno for simplicity):

use atmega328p_hal::usart::Usart0;
use avr_hal_generic::clock::MHz16;
use avr_hal_generic::port::mode::Floating;

static USART_MUTEX: avr_device::interrupt::Mutex<core::cell::RefCell<Option<Usart0<MHz16, Floating>>>> =
    avr_device::interrupt::Mutex::new(core::cell::RefCell::new(None));

pub fn init_serial(serial: Usart0<MHz16, Floating>) {
    avr_device::interrupt::free(|cs| {
        let serial_cell = USART_MUTEX.borrow(cs);
        *serial_cell.borrow_mut() = Some(serial);
    })
}

macro_rules! println {
    ($($arg:tt)*) => ({
        avr_device::interrupt::free(|cs| {
            let mut serial_ref = USART_MUTEX.borrow(cs).borrow_mut();
            if let Some(serial) = serial_ref.as_mut() {
                ufmt::uwriteln!(serial, $($arg)*).void_unwrap();
            }
        });
    });
}

[mutantbob]

I ended up on this page from a google search. I was trying to use the code here with

[dependencies.arduino-hal]
git = "https://github.com/rahix/avr-hal"
rev = "f84c0dff774c2292bc932b670955165161ecc7d1"
features = ["arduino-uno"]

The first thing I noticed is that Usart0 only has 1 type parameter, so Floating seems to be gone. The second thing I noticed is that I could not figure out what part of Peripherals to pass to init_serial().
Maybe a follow-up comment on this issue can inform people of the modern usage.

[mutantbob]

I eventually figured out a way to do it: https://stackoverflow.com/questions/70883797/what-is-the-rust-equivalent-of-serial-println-from-the-arduino-c-api/70883798#70883798

[Rahix]

@mutantbob, what you've linked is code to use a UART (for this, also check examples/arduino-uno/src/bin/uno-usart.rs). This is enough for simple scenarios where you just want to print from your main function. What this issue was originally about is having a standalone println!()/print!() macro for printing from anywhere in the code.

I've found a piece of code which does this for newer versions of avr-hal but I have not verified that it actually works...

Here it is:

#![no_std]
#![no_main]

use arduino_hal::prelude::*;
use panic_halt as _;

pub mod serial {
    use avr_device::interrupt::Mutex;
    use core::cell::RefCell;

    pub type Usart = arduino_hal::hal::usart::Usart0<arduino_hal::DefaultClock>;
    pub static GLOBAL_SERIAL: Mutex<RefCell<Option<Usart>>> = Mutex::new(RefCell::new(None));

    pub fn init(serial: Usart) {
        avr_device::interrupt::free(|cs| {
            GLOBAL_SERIAL.borrow(&cs).replace(Some(serial));
        })
    }

    #[macro_export]
    macro_rules! serial_println {
        ($($arg:tt)*) => {
            ::avr_device::interrupt::free(|cs| {
                if let Some(serial) = &mut *crate::serial::GLOBAL_SERIAL.borrow(&cs).borrow_mut() {
                    ::ufmt::uwriteln!(serial, $($arg)*)
                } else {
                    Ok(())
                }
            })
        }
    }
}

#[arduino_hal::entry]
fn main() -> ! {
    let dp = arduino_hal::Peripherals::take().unwrap();
    let pins = arduino_hal::pins!(dp);
    let serial = arduino_hal::default_serial!(dp, pins, 57600);

    serial::init(serial);

    serial_println!("Hello from Arduino!\r").void_unwrap();

    loop {
    }
}

[mutantbob]

In the intervening month I have complicated your example into the following:

type DefaultSerial = Usart<USART0, Pin<Input, PD0>, Pin<Output, PD1>>;
static mut SERIAL_STATIC: Mutex<RefCell<Option<DefaultSerial>>> = Mutex::new(RefCell::new(None));

macro_rules! println {
    ( $($stuff: expr),+) => {
        {
            let mut serial = avr_device::interrupt::free(|cs| { unsafe { &SERIAL_STATIC}.borrow(cs).borrow_mut().take() });
            let rval = uwriteln!(serial.as_mut().unwrap(),$($stuff),+);
            avr_device::interrupt::free(|cs|
                unsafe { &SERIAL_STATIC}.borrow(cs).replace(serial)
            );
            rval
        }
    }
}

fn initialize_serial_static(serial: DefaultSerial) {
    avr_device::interrupt::free(|cs| {
        unsafe { &SERIAL_STATIC }.borrow(&cs).replace(Some(serial));
    });
}

fn with_serial<T, F>(core: F) ->T
where
    F: FnOnce(&mut DefaultSerial) -> T,
{
    let mut serial =
        avr_device::interrupt::free(|cs| unsafe { &SERIAL_STATIC }.borrow(cs).borrow_mut().take());
    let rval = core(serial.as_mut().unwrap());
    avr_device::interrupt::free(|cs| unsafe { &SERIAL_STATIC }.borrow(cs).replace(serial));
    rval
}

extern "C" fn mqtt_callback(topic: *mut i8, payload: *mut u8, payload_length: u16) {
    let payload = unsafe { core::slice::from_raw_parts(payload, payload_length as usize) };
    let topic = unsafe { CStr::from_ptr(topic as *const i8) };

    with_serial(|serial| {
        let _ = uwrite!(serial, "got {} bytes on ", payload.len(),);
        for x in topic.to_bytes() {
            serial.write_byte(*x);
        }
        serial.write_byte(b'\n');
        for x in payload {
            serial.write_byte(*x);
        }
        serial.write_byte(b'\n');
    });
}

It is a little more complicated, because I use take() to minimize the amount of code in the interrupt-free critical section. I am not sure how important that is. It is probably irrelevant in my current app since I don't have any interrupt handlers, but it would probably become a concern in more complicated apps.

[Rahix]

@mutantbob, the code you shared has a race-condition. You can't have both "global println!()" and "no long critical section" like this unfortunately. The only way to do this would be a ring-buffer, but that adds a lot of complexity...

The problem is that your println!() implementation is now not re-entrancy safe. Consider what happens when the function is currently sending out data when an interrupt hits which again tries to println!(): Your implementation will panic in this situation.

If you cannot afford the long critical section, I would advise to instead pass the UART handle to each function that wants to send information instead of creating such a "global" println!(). Doing the latter is just going to have this race-condition bite you later...