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

Uart rx buffer length error #427

Open
Volkalex28 opened this issue May 30, 2024 · 8 comments
Open

Uart rx buffer length error #427

Volkalex28 opened this issue May 30, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@Volkalex28
Copy link
Contributor

I know that similar problems have already existed, but I did not find a solution there.

I'm trying to configure UartTxDriver and get the error:
"uart: uart_driver_install(1588): uart rx buffer length error"

I don't change the config

let config = Config { baudrate: Hertz(19200), parity: Parity::ParityOdd, ..Default::default() };

@Volkalex28
Copy link
Contributor Author

Volkalex28 commented May 30, 2024

version crate: 0.43.1
version esp-idf: 5.2.1
chip: ESP32S3

@Volkalex28
Copy link
Contributor Author

It looks like the problem is this

In esp-idf we have assert

ESP_RETURN_ON_FALSE((rx_buffer_size > UART_HW_FIFO_LEN(uart_num)), ESP_FAIL, UART_TAG, "uart rx buffer length error");

but in esp-idf-hal we have this uart_driver_install call

esp!(unsafe {
    uart_driver_install(
        UART::port(),
        if rx.is_some() {
            config.rx_fifo_size as _
        } else {
            0
        },
        if tx.is_some() {
            config.tx_fifo_size as _
        } else {
            0
        },
        config.queue_size as _,
        queue.map(|q| q as *mut _).unwrap_or(ptr::null_mut()),
        InterruptType::to_native(config.intr_flags) as i32,
    )
})?;

It turns out that if don’t have an rx pin, then we are trying to set the buffer length to 0, but esp-idf does not allow us to do this

@ivmarkov
Copy link
Collaborator

ivmarkov commented Jun 7, 2024

@Volkalex28 Would you like to contribute a PR that addresses this issue?

@Volkalex28
Copy link
Contributor Author

@ivmarkov In general, yes
I also have a change for ResetReason. In from impl, remove the panic and replace it with ResetReason::Unknown. Do you want to discuss this elsewhere?

@ivmarkov
Copy link
Collaborator

ivmarkov commented Jun 9, 2024

Let's have two separate PRs for these if you don't mind.

If you want to discuss or in general see what is going on - join the Matrix room which is linked in the readme.

@ivmarkov
Copy link
Collaborator

@Volkalex28 what happened? Are you still interested in contributing a PR?

@Volkalex28
Copy link
Contributor Author

@ivmarkov Sorry for delay. I didn't have time to do this at all. I think I'll do it tomorrow!

@ivmarkov
Copy link
Collaborator

No rush. It is just that we are prepping for a release these days, so if you want the fixes to go in, you might need to hurry up.

But we can always do a patch-level release later.

@Vollbrecht Vollbrecht added the bug Something isn't working label Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Todo
Development

No branches or pull requests

3 participants