From a0598ef3699c1b502a499d76c26bd48d48355384 Mon Sep 17 00:00:00 2001 From: Kay Sievers Date: Sun, 19 Apr 2020 11:44:15 +0200 Subject: MIDI - Add flow control to incoming packet stream Larger SysEx transfers get corrupted by incoming packets. This changes the FIFOs not to overwrite their data. MIDI should not be a transport that drops packets. A potentially blocking device is easier to detect and handle than a device that silently corrupts the packet stream at random overflows, especially when SysEx messages are involved. --- src/class/midi/midi_device.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) (limited to 'src/class') diff --git a/src/class/midi/midi_device.c b/src/class/midi/midi_device.c index 56a27fbce..589d0123f 100644 --- a/src/class/midi/midi_device.c +++ b/src/class/midi/midi_device.c @@ -85,6 +85,18 @@ bool tud_midi_n_mounted (uint8_t itf) return midi->ep_in && midi->ep_out; } +static void _prep_out_transaction (midid_interface_t* p_midi) +{ + // skip if previous transfer not complete + if ( usbd_edpt_busy(TUD_OPT_RHPORT, p_midi->ep_out) ) + return; + + // Prepare for incoming data but only allow what we can store in the ring buffer. + uint16_t max_read = tu_fifo_remaining(&p_midi->rx_ff); + if ( max_read >= CFG_TUD_MIDI_EPSIZE ) + usbd_edpt_xfer(TUD_OPT_RHPORT, p_midi->ep_out, p_midi->epout_buf, CFG_TUD_MIDI_EPSIZE); +} + //--------------------------------------------------------------------+ // READ API //--------------------------------------------------------------------+ @@ -138,11 +150,17 @@ void tud_midi_n_read_flush (uint8_t itf, uint8_t jack_id) { (void) jack_id; tu_fifo_clear(&_midid_itf[itf].rx_ff); + + if (tud_ready() && &_midid_itf[itf].ep_out != 0) + _prep_out_transaction(&_midid_itf[itf]); } bool tud_midi_n_receive (uint8_t itf, uint8_t packet[4]) { - return tu_fifo_read_n(&_midid_itf[itf].rx_ff, packet, 4); + if (tud_ready() && &_midid_itf[itf].ep_out != 0) + _prep_out_transaction(&_midid_itf[itf]); + + return tu_fifo_read_n(&_midid_itf[itf].rx_ff, packet, 4) == 4; } void midi_rx_done_cb(midid_interface_t* midi, uint8_t const* buffer, uint32_t bufsize) { @@ -267,8 +285,8 @@ void midid_init(void) midid_interface_t* midi = &_midid_itf[i]; // config fifo - tu_fifo_config(&midi->rx_ff, midi->rx_ff_buf, CFG_TUD_MIDI_RX_BUFSIZE, 1, true); - tu_fifo_config(&midi->tx_ff, midi->tx_ff_buf, CFG_TUD_MIDI_TX_BUFSIZE, 1, true); + tu_fifo_config(&midi->rx_ff, midi->rx_ff_buf, CFG_TUD_MIDI_RX_BUFSIZE, 1, false); + tu_fifo_config(&midi->tx_ff, midi->tx_ff_buf, CFG_TUD_MIDI_TX_BUFSIZE, 1, false); #if CFG_FIFO_MUTEX tu_fifo_config_mutex(&midi->rx_ff, osal_mutex_create(&midi->rx_ff_mutex)); @@ -358,7 +376,7 @@ bool midid_open(uint8_t rhport, tusb_desc_interface_t const * desc_itf, uint16_t *p_length = drv_len; // Prepare for incoming data - TU_ASSERT( usbd_edpt_xfer(rhport, p_midi->ep_out, p_midi->epout_buf, CFG_TUD_MIDI_EPSIZE), false); + _prep_out_transaction(p_midi); return true; } @@ -381,6 +399,7 @@ bool midid_control_request(uint8_t rhport, tusb_control_request_t const * p_requ bool midid_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes) { + (void) rhport; (void) result; uint8_t itf = 0; @@ -399,7 +418,8 @@ bool midid_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32 midi_rx_done_cb(p_midi, p_midi->epout_buf, xferred_bytes); // prepare for next - TU_ASSERT( usbd_edpt_xfer(rhport, p_midi->ep_out, p_midi->epout_buf, CFG_TUD_MIDI_EPSIZE), false ); + _prep_out_transaction(p_midi); + } else if ( ep_addr == p_midi->ep_in ) { maybe_transmit(p_midi, itf); } -- cgit v1.3.1 From d57312602dd9aadc47ae8eae943f51b585e0a228 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 20 Apr 2020 16:09:17 +0700 Subject: add extra comma to HID_REPORT_ID this make the template with Report ID look less weird to the user --- examples/device/hid_composite/src/usb_descriptors.c | 4 ++-- src/class/hid/hid.h | 4 ++-- src/class/hid/hid_device.h | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) (limited to 'src/class') diff --git a/examples/device/hid_composite/src/usb_descriptors.c b/examples/device/hid_composite/src/usb_descriptors.c index 6bde2ff7a..02cbc17f6 100644 --- a/examples/device/hid_composite/src/usb_descriptors.c +++ b/examples/device/hid_composite/src/usb_descriptors.c @@ -73,8 +73,8 @@ uint8_t const * tud_descriptor_device_cb(void) uint8_t const desc_hid_report[] = { - TUD_HID_REPORT_DESC_KEYBOARD( HID_REPORT_ID(REPORT_ID_KEYBOARD), ), - TUD_HID_REPORT_DESC_MOUSE ( HID_REPORT_ID(REPORT_ID_MOUSE), ) + TUD_HID_REPORT_DESC_KEYBOARD( HID_REPORT_ID(REPORT_ID_KEYBOARD) ), + TUD_HID_REPORT_DESC_MOUSE ( HID_REPORT_ID(REPORT_ID_MOUSE) ) }; // Invoked when received GET HID REPORT DESCRIPTOR diff --git a/src/class/hid/hid.h b/src/class/hid/hid.h index 848274037..8803e4b66 100644 --- a/src/class/hid/hid.h +++ b/src/class/hid/hid.h @@ -413,8 +413,8 @@ enum { #define HID_REPORT_SIZE(x) HID_REPORT_ITEM(x, 7, RI_TYPE_GLOBAL, 1) #define HID_REPORT_SIZE_N(x, n) HID_REPORT_ITEM(x, 7, RI_TYPE_GLOBAL, n) -#define HID_REPORT_ID(x) HID_REPORT_ITEM(x, 8, RI_TYPE_GLOBAL, 1) -#define HID_REPORT_ID_N(x) HID_REPORT_ITEM(x, 8, RI_TYPE_GLOBAL, n) +#define HID_REPORT_ID(x) HID_REPORT_ITEM(x, 8, RI_TYPE_GLOBAL, 1), +#define HID_REPORT_ID_N(x) HID_REPORT_ITEM(x, 8, RI_TYPE_GLOBAL, n), #define HID_REPORT_COUNT(x) HID_REPORT_ITEM(x, 9, RI_TYPE_GLOBAL, 1) #define HID_REPORT_COUNT_N(x, n) HID_REPORT_ITEM(x, 9, RI_TYPE_GLOBAL, n) diff --git a/src/class/hid/hid_device.h b/src/class/hid/hid_device.h index f5e29d8a2..efdde9569 100644 --- a/src/class/hid/hid_device.h +++ b/src/class/hid/hid_device.h @@ -93,17 +93,17 @@ TU_ATTR_WEAK bool tud_hid_set_idle_cb(uint8_t idle_rate); * HID Report Descriptor Template * * Convenient for declaring popular HID device (keyboard, mouse, consumer, - * gamepad etc...). Templates take "HID_REPORT_ID(n)," as input, leave + * gamepad etc...). Templates take "HID_REPORT_ID(n)" as input, leave * empty if multiple reports is not used * * - Only 1 report: no parameter * uint8_t const report_desc[] = { TUD_HID_REPORT_DESC_KEYBOARD() }; * - * - Multiple Reports: "HID_REPORT_ID(ID)," must be passed to template + * - Multiple Reports: "HID_REPORT_ID(ID)" must be passed to template * uint8_t const report_desc[] = * { - * TUD_HID_REPORT_DESC_KEYBOARD( HID_REPORT_ID(1), ) , - * TUD_HID_REPORT_DESC_MOUSE ( HID_REPORT_ID(2), ) + * TUD_HID_REPORT_DESC_KEYBOARD( HID_REPORT_ID(1) ) , + * TUD_HID_REPORT_DESC_MOUSE ( HID_REPORT_ID(2) ) * }; *--------------------------------------------------------------------*/ -- cgit v1.3.1 From 3b83813f017dfd08e19a7a3bc2dc575779b1eadd Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 22 Apr 2020 00:25:08 +0700 Subject: clean up --- .github/ISSUE_TEMPLATE/bug_report.md | 13 ++++++++----- .github/ISSUE_TEMPLATE/feature_request.md | 7 +++++-- .github/ISSUE_TEMPLATE/question.md | 2 +- README.md | 4 ---- src/class/midi/midi_device.c | 5 +++-- 5 files changed, 17 insertions(+), 14 deletions(-) (limited to 'src/class') diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index c4d2757d2..5c70569d0 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -7,17 +7,20 @@ assignees: '' --- -**Set up (mandatory):** -Provide details of your setup help us to reproduce the issue as quick as possible +**Set up** +[Mandatory] Provide details of your setup help us to reproduce the issue as quick as possible - **PC OS** : Ubuntu 18.04 / Windows 10/ macOS 10.15 - **Board** : Feather nRF52840 Express - **Firmware**: examples/device/cdc_msc -**Bug Description** -Describe what the bug is. +**Describe the bug** +A clear and concise description of what the bug is. -**Reproduce** +**To reproduce** Steps to reproduce the behavior: 1. Go to '...' 2. Click on '....' 3. See error + +**Log & screenshots** +If applicable, add screenshots and TinyUSB's log to help explain your problem. To enable logging, add `LOG=2` to your make command if building with stock examples or set `CFG_TUSB_DEBUG=2` in your tusb_config.h. \ No newline at end of file diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md index cb211eeb4..f34ff49ed 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.md +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -7,5 +7,8 @@ assignees: '' --- -**Feature Description** -Describe your feature +**Is your feature request related to a problem? Please describe.** +A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] + +**Describe the solution you'd like** +A clear and concise description of what you want to happen. diff --git a/.github/ISSUE_TEMPLATE/question.md b/.github/ISSUE_TEMPLATE/question.md index 3a9fd205c..d12c9785d 100644 --- a/.github/ISSUE_TEMPLATE/question.md +++ b/.github/ISSUE_TEMPLATE/question.md @@ -7,4 +7,4 @@ assignees: '' --- -**Question Description** +**Describe what the question is** diff --git a/README.md b/README.md index a8a533fd4..a7551155d 100644 --- a/README.md +++ b/README.md @@ -73,10 +73,6 @@ TinyUSB is completely thread-safe by pushing all ISR events into a central queue - **FreeRTOS** - **Mynewt** Due to the newt package build system, Mynewt examples are better to be on its [own repo](https://github.com/hathach/mynewt-tinyusb-example) -## Compiler & IDE - -The stack is developed with GCC compiler and should be compilable with others. The `examples` folder provides Makefile and Segger Embedded Studio build support. [Here are example build instructions](examples/readme.md). - ## Getting Started [Here are the details for getting started](docs/getting_started.md) with the stack. diff --git a/src/class/midi/midi_device.c b/src/class/midi/midi_device.c index 589d0123f..f8db63b73 100644 --- a/src/class/midi/midi_device.c +++ b/src/class/midi/midi_device.c @@ -88,13 +88,14 @@ bool tud_midi_n_mounted (uint8_t itf) static void _prep_out_transaction (midid_interface_t* p_midi) { // skip if previous transfer not complete - if ( usbd_edpt_busy(TUD_OPT_RHPORT, p_midi->ep_out) ) - return; + if ( usbd_edpt_busy(TUD_OPT_RHPORT, p_midi->ep_out) ) return; // Prepare for incoming data but only allow what we can store in the ring buffer. uint16_t max_read = tu_fifo_remaining(&p_midi->rx_ff); if ( max_read >= CFG_TUD_MIDI_EPSIZE ) + { usbd_edpt_xfer(TUD_OPT_RHPORT, p_midi->ep_out, p_midi->epout_buf, CFG_TUD_MIDI_EPSIZE); + } } //--------------------------------------------------------------------+ -- cgit v1.3.1