Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c19a10ed1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Fixes MIDI host enumeration/configuration behavior to address issue #3544, where the MIDI host driver could be configured for an interface it didn’t actually open (leading to asserts/faults on some targets).
Changes:
- Adjusts descriptor-walk logic in
midih_open()so the computed “MIDI interface total length” is intended to stop at the next interface descriptor (instead of stepping past it). - Ensures the mount callback reports the actual MIDI streaming interface number (
p_midi->bInterfaceNumber) rather than theitf_numpassed intomidih_set_config(). - Minor comment wording cleanup for
itf_count.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
|
||
| bool found_new_interface = false; | ||
| while (tu_desc_in_bounds(p_desc, desc_end) && !found_new_interface) { | ||
| p_desc = tu_desc_next(p_desc); |
Size Difference ReportBecause TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds. Note: If there is no change, only one value is shown. Changes >1% in sizeNo entries. Changes <1% in size
No changes
|
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| frdm_kl25z/midi_rx | 18,424 → 18,396 (-28) | — | — | — | 18,424 → 18,396 (-28) | -0.2% |
| stm32c071nucleo/midi_rx | 21,040 → 21,012 (-28) | — | — | — | 21,664 → 21,636 (-28) | -0.1% |
| raspberry_pi_pico/midi_rx | 24,256 → 24,224 (-32) | — | — | — | 25,308 → 25,276 (-32) | -0.1% |
| stm32g0b1nucleo/midi_rx | 23,080 → 23,052 (-28) | — | — | — | 23,704 → 23,676 (-28) | -0.1% |
| lpcxpresso1769/midi_rx | 15,864 → 15,848 (-16) | — | — | — | 15,872 → 15,856 (-16) | -0.1% |
| ea4088_quickstart/midi_rx | 15,992 → 15,976 (-16) | — | — | — | 16,008 → 15,992 (-16) | -0.1% |
| metro_m4_express/midi_rx | 16,640 → 16,624 (-16) | — | — | — | 16,648 → 16,632 (-16) | -0.1% |
| lpcxpresso18s37/midi_rx | 18,448 → 18,432 (-16) | — | — | — | 18,886 → 18,870 (-16) | -0.1% |
| portenta_c33/midi_rx | 18,940 → 18,924 (-16) | — | — | — | 18,940 → 18,924 (-16) | -0.1% |
| ea4357/midi_rx | 19,688 → 19,672 (-16) | — | — | — | 20,134 → 20,118 (-16) | -0.1% |
There was a problem hiding this comment.
Pull request overview
Fixes the MIDI host enumeration/configuration flow (issue #3544) by ensuring the MIDI host driver does not “consume” the next interface descriptor during open(), which previously could cause the host stack to bind/configure an interface that the MIDI driver never opened.
Changes:
- Update
midih_open()descriptor-walk logic to stop at (but not advance past) the nextTUSB_DESC_INTERFACE, preventing over-sizeddrv_lenreturns. - In
midih_set_config(), report the MIDI streaming interface number from the opened interface (p_midi->bInterfaceNumber) rather than the interface number passed intoset_config. - Minor comment grammar fix (
interface→interfaces).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
fix #3544