arduino-libraries / MIDIUSB

A MIDI library over USB, based on PluggableUSB
GNU Lesser General Public License v2.1
483 stars 89 forks source link

uint32_t MIDI_::available(void) doesn't work #25

Open monteslu opened 7 years ago

monteslu commented 7 years ago

The current implementation always return the same value even when new messages are coming in:


uint32_t MIDI_::available(void)
{

    ring_bufferMIDI *buffer = &midi_rx_buffer;
    return (uint32_t)(MIDI_BUFFER_SIZE + buffer->head - buffer->tail) % MIDI_BUFFER_SIZE;
}

This is a semi-workaround, but not complete:


uint32_t MIDI_::available(void)
{
    ring_bufferMIDI *buffer = &midi_rx_buffer;
    uint32_t internal_buffer_size = (uint32_t)(MIDI_BUFFER_SIZE + buffer->head - buffer->tail) % MIDI_BUFFER_SIZE;
    uint32_t upstream_buffer_size = USB_Available(MIDI_RX);
    //not sure how to handle... if upstream is bigger send that number otherwise send local size
    return upstream_buffer_size > internal_buffer_size ? upstream_buffer_size : internal_buffer_size;
}
jacobrosenthal commented 7 years ago

I was poking around at this with @monteslu but after digging further I found that accept() throws away misformatted packets so you cant even add upstream to internal buffer length as that might not be accurate.

Its probably more likely that available should, like read, drain the upstream buffer as much as it can then return the accurate internal buffer size

facchinm commented 7 years ago

Correct, right now available is useless since accept is only called by read, which indeed consumes the buffered value. I remember that calling accept into available had some problems but I should benchmark again to see what the problem was. Anyway, any PR is well accepted, if you want to propose a Stream-like behaviour I'll be more than happy to merge it !

jacobrosenthal commented 7 years ago

Hrm.. are the usb timings really tight? Otherwise its hard to believe theres a performance issue.. Al the code is already polling read.. so polling available shouldnt add more than a few instruction overhead?

fab672000 commented 7 years ago

Until we get there though and have a better available() method, could you not use read() and test if something was received then eventually store the data in your own space / fifo as a workaround? I'm thinking about something like:

 rx = MidiUSB.read();
 if (rx.header != 0) {} // simulate available is true: do something now or store it for later 
 else {} // simulate available is false: nothing was available at that time