Provide logger on a serial port

[robamu]

I was wondering if it is possible to provide a logger for a serial port or whether you already though about this. I'm new to Rust, so right now I am trying to figure out how to define a static logger which will needs to be able to be set by the user depending on which serial port could be used.

[DoumanAsh]

Serial is more of a general purpose interface
You would generally need to setup everything yourself not only for writing log onto serial, but also receiving.
It is especially problematic if you use serial to transfer something else.
Not to mention serial ports are device dependent so it would be very complicated to make it generic (I'm not sure what is current state of rust embedded, maybe there is trait for serial)

In any case I think debugging is better done with semihosting, which should be disabled once debugging is no longer necessary.

If you still think of using serial, which is likely the case for when you want to log even in production, I suggest you to come up with example of such logging and then we can consider how to integrate it in a generic way

[robamu]

Yes, ITM/Semihosting and RTT are superior for many reasons, but a simple log on a serial port is probably the easiest way to print something to a console without additional hardware or setup (at least with my Nucleo board)

But I'm probably also a bit biased due to my C/C++ background and because I'm used to get debug printout that way :-)

In any case, I managed to write an abstraction which allows to use the serial port of the STM32H743ZI which also sends to the USB port as a logger:

pub mod serial {
    use log::{Level, Metadata, Record, LevelFilter};
    use cortex_m::interrupt::{self, Mutex};
    use core::{cell::RefCell};
    use stm32h7xx_hal::{serial, device};
    use core::fmt::Write;

    pub struct SerLogger {
        level: Level,
        serial: Mutex<RefCell<Option<serial::Serial<device::USART3>>>>
    }
    
    static SER_LOGGER: SerLogger = SerLogger {
        level: Level::Info,
        serial: Mutex::new(RefCell::new(None))
    };
    
    pub fn init(
        serial: serial::Serial<device::USART3>
    ) {
        interrupt::free(|cs| {
            SER_LOGGER.serial.borrow(cs).replace(Some(serial));
        });
        log::set_logger(&SER_LOGGER).map(|()| log::set_max_level(LevelFilter::Info)).unwrap();
    }
    
    
    impl log::Log for SerLogger {
        fn enabled(&self, metadata: &Metadata) -> bool {
            metadata.level() <= self.level
        }
    
        fn log(&self, record: &Record) {
            interrupt::free(|cs| {
                let mut tx_ref = self.serial.borrow(cs).borrow_mut();
                let tx = tx_ref.as_mut().unwrap();
                writeln!(tx, "{} - {}\r", record.level(), record.args()).unwrap();
            })
        }
    
        fn flush(&self) {}
    }    
}

And making it generic would be the next step for me but I'm not sure how to do that. All I need is the core::fmt::Write trait implemented by the serial TX, but if I specify the serial type as dyn core::fmt::Write I get the error that Option requires to know the size of the trait. I think Box is usually used for that and I can' t use it in a no_std environment. Do you have an idea how to solve this?

[DoumanAsh]

You can provide it as &'a mut core::fmt::Write
Ideally it should be 'static but if not available we can just transmute it

[DoumanAsh]

On side note you can use my logger if you implement cortex_m_log::printer::Printer trait

[DoumanAsh]

I've introduced generic printer for any core::fmt::Write dcea8f1

You can try to use it by just creating it with any instance of object that implements core::fmt::Write

[robamu]

Thanks, I will try it out

[robamu]

I was now able to create a printer on the stack

    let mut ser_printer = printer::generic::GenericPrinter::new(serial);
    ser_printer.println(format_args!("Hello World\r"));

However, I think having a global logger is still kind of tricky. For the following code

    static SER_LOGGER: Logger<GenericPrinter<&'static core::fmt::Write>> = Logger {
        level: LevelFilter::Info,
        inner: unsafe {
            interrupt::free(|cs| {
                let serial_ref = SERIAL_REF.borrow(cs).borrow_mut();
                let serial = serial_ref.as_mut().unwrap();
                GenericPrinter::new(serial)
        })
        },
    };

I still get a depracation error because I did not use dyn. Using &'a mut core::fmt::Write instead of &'static core::fmt::Write, I get an undeclared lifetime error. I guess the nature of the embedded environment forces me to use the concrete type of the serial peripheral. I could probably circumvent this issue by ussing the alloc crate but I have not tried this yet.

So now the code would look like this

    static SER_LOGGER: Logger<GenericPrinter<serial::Serial<device::USART3>>> = Logger {
        level: LevelFilter::Info,
        inner: unsafe {
            interrupt::free(|cs| {
                let serial_ref = SERIAL_REF.borrow(cs).borrow_mut();
                let serial = serial_ref.as_mut().unwrap();
                GenericPrinter::new(serial)
        })
        },
    };

Here, I get the issue that GenericPrinter does not implement Sync. I'm now not sure how to solve this..

[DoumanAsh]

@robamu Well GenericPrinter is there to work with any sort of type so I don't think it matters whether it is concrete or not.
Regarding Sync it is probably because serial is not Sync
See https://docs.rs/stm32h7xx-hal/0.10.0/stm32h7xx_hal/device/struct.USART3.html#impl-Sync

You do not need your logger to be global, just set it via trick_init which fixes lifetime.
As long as you declare your logger at the begging of main, it should be safe

[DoumanAsh]

I made GenericPrinter Sync unconditionally as it uses interrupt free mode always
2c4fa47

[robamu]

I think the second <W> is missing.

//Because we use `InterruptFree` mode
unsafe impl<W> Sync for GenericPrinter<W> {
}

Thanks for the hints, I managed to make it work now like this:

    let serial = serial::Serial::usart3(
        dp.USART3,
        serial::config::Config::default().baudrate(115200.bps()),
        ccdr.peripheral.USART3,
        &ccdr.clocks
    ).unwrap();

    // Configure the timer to trigger an update every second
    let mut timer = timer::Timer::tim1(dp.TIM1, ccdr.peripheral.TIM1, &ccdr.clocks);
    timer.start(1.hz());

    let ser_printer = printer::generic::GenericPrinter::new(serial);

    let cortex_logger = cortex_m_log::log::Logger {
        level: LevelFilter::Info,
        inner: ser_printer
    };

    unsafe { 
        cortex_m_log::log::trick_init(&cortex_logger).unwrap();
    }
    // Configure the serial port as a logger
    //logging::serial::init(serial);
    info!("Serial logger example application\r");
    loop {
        info!("Hello, I'm a periodic printout\r");
        warn!("Hello, I'm a warning!\r");
        block!(timer.wait()).unwrap();
    }

The only caveat is that the printer consumes the serial peripheral now and it can't be shared anymore. I guess it makes sense that the peripheral is not used by anything else now because there is no IRQ protection

[DoumanAsh]

@robamu the problem is that you cannot have multiple mutable reference to it, so by value makes sense.
As alternative it should be possible to take pointer and wrap it into some sort of smart type that provides Write
But you need to guarantee that pointer remains valid (i.e. value is not moved anywhere)

P.s. Sorry for bad commit. Let me know how you want it to look and I'll just final version to your needs