Skip to content

esp32/machine_uart: Correctly manage UART queue and event task. #17135

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

Merged
merged 1 commit into from
May 15, 2025

Conversation

DvdGiessen
Copy link
Contributor

@DvdGiessen DvdGiessen commented Apr 16, 2025

Summary

If the driver was reinitialised while there was already an event task running the queue that task is trying to receive from would be deleted, causing it to try to take a lock that no longer existed and deadlocking the CPU.

This change ensures the task is always shut down before recreating the queue and recreates the task afterwards.

It also:

  • Allows setting an IRQ handler before the UART is initialized (like other ports allow)
  • Removes the task when the UART is deinitialized (which was previously missing).
  • Adds a check that no event task can be started when no queue exists (i.e. when the UART is already deinitialized or it's the built-in REPL UART which doesn't have a queue configured).
  • Adds a check to prevent reinitialising the UART driver unnecessarily (we only need to reinitialize it when the buffer sizes change).

Testing

Tested on three custom boards (1x original ESP32, 2x ESP32S3) to talk to three modem chips (1x Sequans, 2x different SIMCOM) all using the UART IRQ's to trigger the PPP.poll() method.

@DvdGiessen DvdGiessen force-pushed the esp32_uart_irq_queue branch from dc94627 to 5c5d75a Compare May 14, 2025 13:33
If the driver was reinitialised while there was
already an event task running the queue that task
is trying to receive from would be deleted,
causing it to try to take a lock that no longer
existed and deadlocking the CPU.

This change ensures the task is always shut down
before recreating the queue and recreates the task
afterwards.

It also allows setting an IRQ handler before the
UART is initialized (like other ports allow),
removes the task when the UART is deinitialized
(which was previously missing), adds a check that
no event task can be started when no queue exists,
and adds a check to prevent reinitialising the
UART driver unnecessarily.

Signed-off-by: Daniël van de Giessen <daniel@dvdgiessen.nl>
@DvdGiessen DvdGiessen force-pushed the esp32_uart_irq_queue branch from 5c5d75a to 155fa94 Compare May 14, 2025 13:55
Copy link
Member

@dpgeorge dpgeorge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with ESP32_GENERIC firmware, running the machine_uart_*.py tests.

Everything passes!

@dpgeorge dpgeorge merged commit 155fa94 into micropython:master May 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants