jussi-kalliokoski / webmidi-issues

A test repo for importing the issues from bugzilla
0 stars 0 forks source link

boolean returns from MIDIOutput methods are inappropriately synchronous #40

Closed jussi-kalliokoski closed 11 years ago

jussi-kalliokoski commented 11 years ago

Originally reported on W3C Bugzilla ISSUE-18759 Fri, 31 Aug 2012 22:36:17 GMT Reported by Chris Wilson Assigned to This bug has no owner yet - up for the taking

Both sendMIDIMessage and sendMessage "return(s) a boolean signifying whether the operation was successful."

That sounds like this is intended to be a synchronous call, which is a very bad idea (since MIDI messages may take a long time to send, on a hardware MIDI port for example). At the very least, this should state "signifying whether the message was successfully enqueued."

At most, we should probably add an onsuccess/onerror callback set to setMIDIMessage().

jussi-kalliokoski commented 11 years ago

Original comment by Jussi Kalliokoski on W3C Bugzilla. Wed, 05 Sep 2012 10:00:46 GMT

(In reply to comment #0)

Both sendMIDIMessage and sendMessage "return(s) a boolean signifying whether the operation was successful."

That sounds like this is intended to be a synchronous call, which is a very bad idea (since MIDI messages may take a long time to send, on a hardware MIDI port for example). At the very least, this should state "signifying whether the message was successfully enqueued."

At most, we should probably add an onsuccess/onerror callback set to setMIDIMessage().

Agreed. The callbacks are something we can add at a later date if there are some actual use cases for them. I'll remove the notion of returning a boolean.

jussi-kalliokoski commented 11 years ago

Original comment by Jussi Kalliokoski on W3C Bugzilla. Wed, 05 Sep 2012 12:15:22 GMT

Proposed changeset: https://dvcs.w3.org/hg/audio/rev/bf0e920450e6

jussi-kalliokoski commented 11 years ago

Original comment by Chris Wilson on W3C Bugzilla. Wed, 05 Sep 2012 17:35:10 GMT

  1. Do we want to make it explicit in the text that the operation may be "enqueues a MIDI message", rather than "send"? (I.e. that the send may not have completed.)
  2. The callback is probably most useful for the sendMIDIMessage form (since sysex may take a while) - although I do have one scenario that I wanted it for: I have a Novation Launchpad (http://us.novationmusic.com/midi-controllers-digital-dj/launchpad), which is an 8x8 array of LED-lit buttons, plus 16 additional buttons; it is only USB 1.1, so can only accept a maximum of 400 messages per second. Because there are 80 LED addresses, it takes 200 milliseconds to update a Launchpad completely. Obviously, it's kinda cool to animate it - I've written a Conway's Game of Life implementation (https://github.com/cwilso/conway/tree/MIDIBridge/WMAS) that interfaces with the Launchpad - but you don't know when the messages have finished writing, so it's hard to update appropriately. I don't just drive the animation as fast as it will go - you press a button to advance - but if I did, I might end up overrunning the buffers if I tried to push it faster than 5 updates a second. (Probably far less, of course.)

Both OSX and Windows have callbacks to implement this, but the OSX one is ONLY for Sysex Send calls, so it would be better to have this only on sendMIDIMessage if we add it.

jussi-kalliokoski commented 11 years ago

Original comment by Florian Bomers on W3C Bugzilla. Wed, 05 Sep 2012 19:34:45 GMT

I believe that it would be nice to know if an error occurred already when invoking the send method, most notable for the case that the MIDIPort was disconnected. For that, a boolean could be returned, specified as: "returns false if an error occurred, returns true if the message was enqueued for asynchronous delivery"

jussi-kalliokoski commented 11 years ago

Original comment by Jussi Kalliokoski on W3C Bugzilla. Thu, 06 Sep 2012 06:38:32 GMT

(In reply to comment #3)

  1. Do we want to make it explicit in the text that the operation may be "enqueues a MIDI message", rather than "send"? (I.e. that the send may not have completed.)

Yes.

  1. The callback is probably most useful for the sendMIDIMessage form (since sysex may take a while) - although I do have one scenario that I wanted it for: I have a Novation Launchpad (http://us.novationmusic.com/midi-controllers-digital-dj/launchpad), which is an 8x8 array of LED-lit buttons, plus 16 additional buttons; it is only USB 1.1, so can only accept a maximum of 400 messages per second. Because there are 80 LED addresses, it takes 200 milliseconds to update a Launchpad completely. Obviously, it's kinda cool to animate it - I've written a Conway's Game of Life implementation (https://github.com/cwilso/conway/tree/MIDIBridge/WMAS) that interfaces with the Launchpad - but you don't know when the messages have finished writing, so it's hard to update appropriately. I don't just drive the animation as fast as it will go - you press a button to advance - but if I did, I might end up overrunning the buffers if I tried to push it faster than 5 updates a second. (Probably far less, of course.)

Sweet! And yes, it's actually an important use case (lighting systems).

Both OSX and Windows have callbacks to implement this, but the OSX one is ONLY for Sysex Send calls, so it would be better to have this only on sendMIDIMessage if we add it.

But sendMessage can send SysEx messages too. Still, I agree, no need to complicate sendMessage with a callback.

We'll also have to clarify the enqueueing behavior. Do we want the implementations to drop messages that couldn't be sent to the port at the expected time? Or do we want the implementations to make sure that no messages are dropped?

The latter is not necessarily easy to execute and it has some drawbacks:

jussi-kalliokoski commented 11 years ago

Original comment by Chris Wilson on W3C Bugzilla. Thu, 06 Sep 2012 12:38:21 GMT

Perhaps Florian's suggestion is the best - boolean return, true if successfully enqueued, false if there was a problem (MIDI port shut down, buffer overrun).

jussi-kalliokoski commented 11 years ago

Original comment by Chris Wilson on W3C Bugzilla. Wed, 17 Oct 2012 23:46:14 GMT

This has been resolved.

jussi-kalliokoski commented 11 years ago

Original comment by Olivier Thereaux on W3C Bugzilla. Thu, 15 Nov 2012 09:57:33 GMT

(In reply to comment #7)

This has been resolved.

Closing.