raspberrypi / pico-examples

BSD 3-Clause "New" or "Revised" License
2.83k stars 820 forks source link

Make sure there's an example of using stdio_set_chars_available_callback #461

Closed peterharperuk closed 1 month ago

peterharperuk commented 8 months ago

Make sure it works with USB See https://github.com/raspberrypi/pico-sdk/issues/1603

progvault commented 7 months ago

Seems there's a bug with how the mutex works for stdio_usb. I added a comment to the discussion in https://github.com/raspberrypi/pico-sdk/issues/1603#issuecomment-1937414069.

Edit: This does look like a bug.

I investigated a bit further and I think I understand what's going on. Inside stdio_usb.c is low_priority_work_irq() which:

  1. Grabs a mutex
  2. Calls tud_task()
  3. Releases the mutex

Only when tud_task() runs will the characters available callback occur. So the Mutex will already have ownership when the callback happens.

From what I understand, when tud_task() is called, USB packet data is received through the USB Serial Interface Engine (SIE) to the TinyUSB Stack. tud_task() causes the callback to fire once the TinyUSB Stack has data available. This occurs after ownership over the Mutex has already taken place.

The result is that the mutex_try_enter_block_until() call located in stdio_usb_in_chars() sits all day waiting for a Mutex that never gets released. A timeout occurs and the received characters never get read.

To test this theory I rearranged the code in stdio_usb.c so that tud_task() runs last. This fires the callback when noone has ownership over the Mutex. stdio_usb_in_chars() is able to take ownership over the Mutex and read the data. After making this change, the code runs correctly.

static void low_priority_worker_irq(void) {
    if (mutex_try_enter(&stdio_usb_mutex, NULL)) {
        mutex_exit(&stdio_usb_mutex);
        tud_task();
    }
static void low_priority_worker_irq(void) {
    tud_task();
    if (mutex_try_enter(&stdio_usb_mutex, NULL)) {
        mutex_exit(&stdio_usb_mutex);
    } 

This however doesn't fix the issue. It just tests the theory.

arg08 commented 7 months ago

I've proposed a fix over on the other issue: https://github.com/raspberrypi/pico-sdk/issues/1603#issuecomment-1963941166