funbiscuit / embedded-cli-rs

CLI in Rust with autocompletion, subcommands, options, help and history for embedded systems (like Arduino or STM32)
Apache License 2.0
72 stars 4 forks source link

Hardfault on STM32 chip #14

Open peopleskai opened 3 months ago

peopleskai commented 3 months ago

Hi,

I'm did a prototype for a STM32 chip using the ardiuno example as a basis. I have the same LedCommend

// Enum to represent commands and subcommands that can be issued to control LEDs.
// Tripple slash comments are used for generating the help for the CLI.
// Contains two subcommands and their functionality is defined inside on_led()
#[derive(Debug, Command)]
pub enum LedCommand {
    /// Get current LED value
    Get,

    /// Set LED value
    Set {
        /// LED brightness
        value: u8,
    },
}

/// Function to process LED commands, This function is called once the Base CLI detects an LED command.
pub fn on_led<I: embedded_io::Write>(
    cli: &mut CliHandle<'_, Writer<I>, CliError>,
    state: &mut AppState,
    id: u8,
    command: LedCommand,
) -> Result<(), CliError> {
    state.num_commands += 1; // Increment command count for state tracking

    if id as usize >= state.led_brightness.len() {
        uwrite!(cli.writer(), "{}{}{}\n", "LED", id, " not found")?;
        // print command error
        cli_print_error_code(cli, type_name::<LedCommand>(), CliError::CommandError)?;
        return Ok(());
    } else {
        match command {
            LedCommand::Get => {
                uwrite!(
                    cli.writer(),
                    "{}{}{}{}\n",
                    "Current LED",
                    id,
                    " brightness: ",
                    state.led_brightness[id as usize]
                )?;
            }
            LedCommand::Set { value } => {
                state.led_brightness[id as usize] = value;
                uwrite!(
                    cli.writer(),
                    "{}{}{}{}\n",
                    "Setting LED",
                    id,
                    " brightness to ",
                    state.led_brightness[id as usize]
                )?;
            }
        }
    }

But this causes a hardfault, look at the call stack i can this

  1. token.rs:125 - if let Some(pos) = self.tokens.as_bytes().iter().position(|&b| b == 0) {
  2. arguments.rs:108 - let raw = self.tokens.next()?;
  3. cli.rs:365 - let res = handler.process(&mut handle, command);

The CLI com

The interesting thing is, if we clone mainline locally and compile it this hardfault goes away. Want to see if you have any insight into this issue or its an issue fixed already and just need to make a new release.

funbiscuit commented 3 months ago

Hi! That's interesting, can you provide some more info? At what moment does it hard fault? After any input and pressing enter, or some specific input?

peopleskai commented 3 months ago

Sorry i see the original post the CLI command is cut off, it is led --id 1 set 3. The crash happens when it process CR character.

Another interesting observation to point out is that when I remove value: u8 from the Led Set command. and call led --id set the program doesn't crash anymore.

funbiscuit commented 3 months ago

Can you also include base command (that has id flag) and how you provide bytes to cli instance? And what buffer sizes do you use?

peopleskai commented 3 months ago

This is the base command and how we provide the buffer for CLI.

/// Defines CLI Commands and reference to subcommands, args and options.
/// Triplle slash comments above command names are used to define command names
/// and help documentation for the command directly from the comments.
#[derive(Debug, Command)]
pub enum BaseCommand<'a> {
    /// Control LEDs
    Led {
        /// LED id
        #[arg(long)]
        id: u8,

        /// Subcommand for LED control
        #[command(subcommand)]
        command: LedCommand,
    },

    /// Control ADC
    Adc {
        /// ADC id
        #[arg(long)]
        id: u8,

        /// Subcommand for ADC control
        #[command(subcommand)]
        command: AdcCommand<'a>,
    },

    /// Show some status
    Status,
}

// --- main loop that feeds data to CLI ---

    // Creating static buffers for command history to avoid using stack memory
    let (command_buffer, history_buffer) = unsafe {
        static mut COMMAND_BUFFER: [u8; CMD_BUFFER_SIZE] = [0; CMD_BUFFER_SIZE];
        static mut HISTORY_BUFFER: [u8; HISTORY_BUFFER_SIZE] = [0; HISTORY_BUFFER_SIZE];
        (COMMAND_BUFFER.as_mut(), HISTORY_BUFFER.as_mut())
    };

    // Setup CLI with command buffer, history buffer, and a writer for output
    let mut cli = CliBuilder::default()
        .writer(writer)
        .command_buffer(command_buffer)
        .history_buffer(history_buffer)
        .build()
        .ok()
        .expect("Failed to build CLI");

    // Setting the CLI prompt
    let _ = cli.set_prompt("main$ ");

    // Global state used throughout the application
    // This is used for the status command and the led simulation command
    let mut state = AppState {
        led_brightness: [0; 4],
        num_commands: 0,
    };

    let _ = cli.write(|writer| {
        // storing big text in progmem
        // for small text it's usually better to use normal &str literals
        uwrite!(
            writer,
            "{}",
            "Cli is running.
        Type \"help\" for a list of commands.
        Use backspace and tab to remove chars and autocomplete.
        Use up and down for history navigation.
        Use left and right to move inside input."
        )?;
        Ok(())
    });

    // Loop continuously processing incoming bytes from UART and executing corresponding CLI commands
    loop {
        freertos_rust::CurrentTask::delay(freertos_rust::Duration::ms(2));

        // Single byte buffer to read one byte at a time
        let mut buffer: [u8; 1] = [0; 1];
        // Single byte buffer to read one byte at a time
        let bytes_read = embedded_io::Read::read(uart_rx, &mut buffer).unwrap();
        defmt::dbg!("cli bytes read: {}", bytes_read);

        let result = cli.process_byte::<BaseCommand<'_>, _>(
            // byte_read,
            buffer[0],
            &mut BaseCommand::processor(|cli, command| match command {
                BaseCommand::Led { id, command } => on_led(cli, &mut state, id, command),
                BaseCommand::Adc { id, command } => on_adc(cli, &mut state, id, command),
                BaseCommand::Status => on_status(cli, &mut state),
            }),
        );
       ...
   }

We've did some testing on which commit after the v0.2.1 fixed the hardfault and the we found the last "utf8 short support" a83b84efa0a6faa062f724e063d9f096974b9197 fixes the hardfault.

Would it be possible to cut a release with the latest commit?

funbiscuit commented 3 months ago

Thanks for the sources, I want to investigate first what was the problem before publishing a release. Can you for now use main branch using patch directive in your Cargo.toml?

peopleskai commented 3 months ago

Look at the commit a83b84e, I think the chagne from use a [u8] to &str in ArgsIter probably fixed the problem? I don't understand the code enough to understand the root cause yet.

I did do more debugging, and stepped through the code a bit. I tried to look through the logic but haven't determine the root cause for the crash yet. Wanted to share screenshots of the call stack.

CLI command tested: "led --id 1 set 1"

There are 9 calls to the suspect function ArgsIter.next(). In the screenshots we can see the process buffer have the correct ASCII codes.

  1. 1st call loop 1
  2. 2nd call loop 2
  3. 3rd call loop 3
  4. 4th call loop 4
  5. 5th call loop 5
  6. 6th call loop 6
  7. 7th call loop 7
  8. 8th call loop 8
  9. 9th call loop 9
  10. 9th call's hardfault line loop 9 crash line
funbiscuit commented 3 months ago

Thanks for the debug screenshots. From them it actually seems like it's not error in library but somewhere else. This is because self.tokens is 100% valid byte slice (on last screen we see that it is 1 byte long and starts at 15th position in COMMAND_BUFFER, which is expected. But somehow calling iter().position(..) on this slice causes a hard fault. Can you provide more details about environment?

  1. What mcu do you use?
  2. What history buffer size?
  3. Do you upload optimized binary or not?
  4. What optimizations are used (size or speed)?

Ideally - upload somewhere whole project that can be flashed to mcu.