Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MIDI input/output disconnection - Ports should be persistent across connects/disconnects #79

Closed
cwilso opened this issue Sep 5, 2013 · 70 comments
Assignees
Milestone

Comments

@cwilso
Copy link
Contributor

cwilso commented Sep 5, 2013

Reported externally:

What is the exact workflow when a MIDI input disconnects? Suppose I'm listening on port 2, which is connected at start, setting onmidimessage appropriately.
Then port 2 disconnects (I receive a disconnect event on my ondisconnect handler) - What is the fate of my onmidimessage port 2 handler?

If input 2 re-connects, should I re-set the appropriate onmidimessage handler to listen for midi events on port 2, or, having set it before, the old handler (set before disconnection and reconnection) will still work?

I think that when a port disconnects, it should release the event handler and the object, and they should not become live again, even when reconnected. At the same time, the Maplike rearchitecture may change this, since that port would be the same as the previous port. Discuss.

@ghost ghost assigned cwilso Sep 5, 2013
@cristiano-belloni
Copy link

Thanks for posting that, Chris!

I think that when a port disconnects, it should release the event handler and the object, and they should not become live again, even when reconnected.

Why? Wouldn't it be more convenient to mantain the event handler? The worst it can happen, when an interface disconnects, is not getting MIDI events anymore. A web application could, for example, listen for midi message on every input and be oblivious that some of them are actually disconnected or reconnected (obviously, if it wants to know, it could still listen to the disconnect / connect events).

@jussi-kalliokoski
Copy link
Member

My idea has been that the instance would work like nothing happened (unless you choose to discard the whole thing) on disconnect -> reconnect. To me it seems that if two ports' that are of the same type and share the ID, they should behave exactly the same. I can't see that if they're from the same MIDIAccess they would even be a different instance. However, for isolation, it's probably better that different MIDIAccess instances don't share the same port instances, so that you can safely assume that if you get a port from a fresh MIDIAccess, no one has set any on<eventname> handlers on it.

@cristiano-belloni
Copy link

Actually, I don't know how devices map to MIDI ports. Let's say I plug in device1 and device2. device1 gets port 0 and device2 gets port 1. Then I disconnect device1 and plug in device3. Will device3 either take the first unallocated port (port 0) or will it take another port? or the behaviour is random/undefined?

@cristiano-belloni
Copy link

(Obviously I'm talking about USB MIDI interfaces / MIDI hubs)

@cwilso
Copy link
Contributor Author

cwilso commented Sep 9, 2013

What do you mean by "gets port 0"? Gets the index of 0 in the array? The index is going away in the issue about getInputs().

@cristiano-belloni
Copy link

"gets port 0" means "is at index 0 in the live input() list". What happens in the live input list if I disconnect a device and attach another device?

Suppose that:

var inputs = midiAccess.inputs();

Results in:

inputs ===  [InputPort_forDevice1, InputPort_forDevice2]

What will the same operation result if I disconnect Device1 and connect Device3?

@cwilso
Copy link
Contributor Author

cwilso commented Sep 9, 2013

today, if you used inputs[0] in your example, it would begin by referring to device1, then it would refer to device2.

This temporal funkiness is why we're moving to Maplikes - the index identifier will go away.

@agoode
Copy link

agoode commented May 3, 2014

So, if a device goes away and comes back, all the event handlers are gone, right? It seems like this is the most sane way to avoid sending things to the wrong place and is analogous to a file descriptor being closed.

@cwilso
Copy link
Contributor Author

cwilso commented May 3, 2014

That was the rough proposal I was making above, yes. But it might make more sense to keep them alive, and restore them if the device comes back, I don't know.

@agoode
Copy link

agoode commented May 3, 2014

What does coremidi do? They seem to have the sanest OS-level API of anything.

I know Alsa permanently disconnects when hardware goes away: you have to react to the disconnect/reconnect events yourself if you want to get messages again.

@jussi-kalliokoski
Copy link
Member

Hmm. I'm not sure the file descriptor analogue works here as closing file descriptors is controlled by the program whereas disconnecting a device is nothing the program can affect. However, if the program doesn't listen to disconnect events, you might get messy situations where buffered MIDI messages all get fired simultaneously when the device is reconnected (unless we specify quite explicitly how to handle such situations). Then again in the same case, if it's an input device, it would start working seamlessly when the user reconnects which would be quite nice user experience.

My personal preference would be to just resume if the device becomes reconnected, given that the device is essentially the same before and after reconnect. But in the end, either option doesn't make anything impossible or more possible.

@cwilso cwilso added this to the V1 milestone May 6, 2014
@toyoshim
Copy link
Contributor

toyoshim commented Feb 1, 2015

Core MIDI provides a permanent access. So once the application get the handle, even the device is disconnected, and connected again, the handle continuously works. Chrome also provides permanent MIDIPort object on OS X thanks to this Core MIDI behavior.

My preference is also keeping the MIDIPort alive even after device disconnection, and the MIDIPort gets to work again once the device is reconnected. Now we have MIDIInputMap and MIDIOutputMap, and is going to have MIDIPortState. So Core MIDI like behavior looks more reasonable.

@toyoshim
Copy link
Contributor

Now Chromium implements MIDIConnectionEvent, and MIDIPort works as a permanent reference that can be reused to control the reconnected same device.

@cwilso cwilso changed the title MIDI input/output disconnection - is it permanent? MIDI input/output disconnection - Ports should be persistent across connects/disconnects Feb 23, 2015
@agoode
Copy link

agoode commented Mar 4, 2015

I've opened issue #123 for a problem I see with this new behavior. We need to be clearer about underlying system resource use.

Since I haven't seen it anywhere, let me propose a state machine using the currently defined MIDIPortStates (blank states mean no MIDIPort entry at all):

Previous state Event State System resources in use?
hardware connected connected no
disconnected hardware connected opened yes
disconnected close() no
connected hardware disconnected no
connected open() opened yes
opened hardware disconnected disconnected depends on implementation, but definitely may return unexpectedly, see issue #123
opened close() connected no

I do want to see MIDIPorts be able to disappear from the maps, in the two transitions I show above. Otherwise you'll get an ever-growing list of dead connections as you plug/unplug different hardware or start/stop other MIDI programs on the same machine.

Moreover, it is a limitation of USB that 2 identical pieces of hardware are usually indistinguishable. I definitely imagine this scenario (even on CoreMIDI):

  1. Plug device A.
  2. Plug identical device A'.
  3. open() A.
  4. open() A'.
  5. Unplug A.
  6. Unplug A'.
  7. Plug A'.
  8. Plug A.
  9. Now your handles are swapped.

We have the caveat ("some systems may not support completely unique persistent identifiers") on the id attribute, we should have the same caveat on disconnected.

@cwilso
Copy link
Contributor Author

cwilso commented Mar 4, 2015

I think my only concern is if we need to represent "disconnected and closed" as a separate state - i.e. what will happen if this disconnected port becomes reconnected. (The first blank cell in your table.) The second blank cell should be "disconnected" (remember, you may already HAVE a MIDIPort you're holding on to. But yeah, they shouldn't show up in the list then.)

On your identical-ports issue - actually, on Windows IIRC they will still show up in the same order and have the same HMIDI, based on the USB port they were plugged in to. (I kid you not.)

@bome
Copy link

bome commented Mar 4, 2015

Useful table!

But as I interpreted the states, disconnected implies close (re-connecting a disconnected port will not automatically open it), which will simplify the table considerably.

Now, arguably, a feature to auto-open a disconnected port that was open at time of disconnection can be useful (but maybe not generally expected?), but then I'd opt for a new state disconnected_pending_open or something to that effect.

Regarding disconnected ports in the map of ports: if we have semantically unambiguous states, we can leave it up to the implementation if disconnected ports appear in the map or not. I do see value to keep disconnected ports there. The implementation can purge old dead ports whenever the map grows too much.

@agoode
Copy link

agoode commented Mar 4, 2015

@cwilso:
So "disconnected and closed" should be expressed by the client remembering the id. We can't guarantee the map will persistently contain anything about these kinds of devices (across restarts for example). The id is the currently recommended way for clients to strive for persistence:

A unique ID of the port. This can be used by developers to

"connected" means "closed" in the current specification, unless I am misunderstanding the states. I wanted to avoid suggesting renaming them, but if they are confusing, we should consider it.

Also, the Windows decision of binding USB devices to their ports is a different way to work around the optional/duplication of USB serial numbers, with different effects:
http://blogs.msdn.com/b/oldnewthing/archive/2004/11/10/255047.aspx

My understanding is that if you switch the port of your MIDI device, you won't be able to find it again with the same handle on Windows. Probably the best way is to use port as a tie breaker. But you can still defeat the system by switching the 2 devices. It's the best of an unfortunate situation.

@bome:
Currently, disconnected implies neither open nor closed. (It can be either one, if I am reading things correctly.)

As for dead ports, I feel that if we are asking clients to hold onto id if they want stability, we shouldn't lull them into a false sense of security by temporarily keeping dead ports around.

What we really may need is a match (or getPortsById) method that takes an id and returns an ordered list of best matches. That way we don't ask clients to implement their matching (as hinted in section 6.1).

@toyoshim
Copy link
Contributor

toyoshim commented Mar 4, 2015

On device swap issue, I agreed that it's better to have a caveat in the spec.

But, as one DAW user, I do not expect that applications is capable to identify two same model music instruments correctly when we connect and disconnect them in such a misleading way. So, this is not a serious problem.

For long term goal, it would be more important to find a way to give a name in a standardized way against each device on all operating systems. Once I create a song with online DAW running on Chrome/Windows that is connected with a motif rack, I hope I can open the song correctly with the same online DAW running on Chrome/Mac that is connected with the same or another motif rack. But for now, it will be difficult because manufacturer, name, and so on depends on operating systems. So, I may need reconfiguration on device assignment against the song project.

@agoode
Copy link

agoode commented Mar 4, 2015

Here is a new proposed state table. The side effects column has some questions in bold that we should resolve.

Previous state Event State side effects System resources in use?
hardware connected connected no
disconnected hardware connected opened yes
disconnected close() no
connected hardware disconnected no
connected open() opened yes
connected addEventListener("onmidimessage") opened event listener is added yes
connected send() opened data is sent yes
opened hardware disconnected disconnected all event listeners removed?? maybe
opened close() connected all event listeners removed?? no

@agoode
Copy link

agoode commented Mar 4, 2015

@toyoshim I like your idea of cross-device persistent identifiers. Can you open a new issue for this? It seems like a tricky problem.

@cwilso
Copy link
Contributor Author

cwilso commented Mar 4, 2015

  1. I think "state" in your table should be "state transitions to:"
  2. I'm thinking that the difference between the first and second rows in your chart is ambiguous. We probably do need to capture the difference between "disconnected" and "not present" (the latter being disconnected and not opened). Then row 3 (disconnected, close()) transitions to "not present", as does row 4.

@agoode
Copy link

agoode commented Mar 4, 2015

I put it here for easier editing:
https://gist.github.com/agoode/3338e2dfe49afda36a9b

Feel free to fork or whatever (I am not good with gist)

@toyoshim
Copy link
Contributor

toyoshim commented Mar 4, 2015

@agoode I opened a new issue as #124

@agoode
Copy link

agoode commented Mar 4, 2015

Let me try another table:

Port present? Port opened? State name, currently
no no disconnected (or missing)
no yes disconnected
yes no connected
yes yes opened

Does this capture the current specification? If so, then I think you are proposing this change:

Port present? Port opened? State name, @cwilso proposal
no no not present (or missing)
no yes disconnected
yes no connected
yes yes opened

I suggest this change:

Port present? Port opened? State name, @agoode proposal
no no (missing)
no yes disconnected
yes no connected
yes yes opened

My argument is that I do not like the nondeterminism introduced by the optional missing in the first row. I would rather make it always missing, to reduce the number of special cases that clients must understand.

@cwilso
Copy link
Contributor Author

cwilso commented Mar 4, 2015

What does "state name: (missing)" mean? That "state" is removed? null? "missing"?

@cwilso
Copy link
Contributor Author

cwilso commented Mar 6, 2015

I see two options - names chosen optimizing for developer use, feel free to bikeshed.

Option 1:

enum MIDIPortDeviceState {
    "disconnected", // unplugged
    "connected"     // plugged
};
enum MIDIPortConnectionState {
    "open",
    "closed"
};

partial interface MIDIPort {
    readonly attribute MIDIPortDeviceState state;
    readonly attribute MIDIPortConnectionState connection;
}

if (port.state == "connected")
    console.log("device is connected!")
if (port.connection == "open")
    console.log("device is open!")

Option 2:

enum MIDIPortState {
    "disconnected", // unplugged and not open
    "disengaged",   // unplugged but open
    "closed",       // plugged but not open
    "connected"     // plugged and open
};

partial interface MIDIPort {
    readonly attribute MIDIPortState state;
}

if (port.state=="disengaged")
    console.log("Oops, your cable fell out")
if (port.state=="connected")
    console.log("you're send/receive-capable.")

I prefer option 1. I think developers are typically interested just in the device state (connected vs disconnected), since the open/closed connection status they are control of, for the most part.

We should be explicit about whether we queue up send() data that is received during disconnected state.

@bome
Copy link

bome commented Mar 6, 2015

is it common to have boolean attributes in ECMAScript?

Option 3:

partial interface MIDIPort {
    readonly attribute boolean connected;
    readonly attribute boolean open;
}

if (port.connected)
    console.log("there you are, MIDI port!")
if (port.open)
    console.log("yes, indeed I have opened the port")

For me, it's best to discard any data that is sent to a disconnected port (regardless of open or not). MIDI is a realtime protocol, queuing up MIDI data is generally bad.

@cwilso
Copy link
Contributor Author

cwilso commented Mar 7, 2015

It's not, but mostly due to concern that the list of states might need to be expanded at some point. It's not proscribed. Since MIDI is 30 years old, I doubt the states are going to expand.

So, option 3: booleans, as @bome suggests. I'm on the fence on 1 vs 3.

@agoode
Copy link

agoode commented Mar 7, 2015

My vote is on enums. If you ever end up needing to pass both states in a method, it's clearer to have 2 enums instead of 2 booleans.

makeState(true, false)
vs
makeState('connected', 'closed')

@toyoshim
Copy link
Contributor

toyoshim commented Mar 8, 2015

I like option 1 and 3 rather than 2.
For boolean vs enums, I also vote for enums. It will be promising against future improvements. For example, WebSocket has medium states, connecting and closing. If we need such states to solve unknown async problems in the future, enums is easy.

Less typing is NOT my priority because usually developers spend most time on other things, designing, reading, testing, and debugging on development. IDE is also so smart today, and developers are great typist. So it does not make much difference.

@cwilso
Copy link
Contributor Author

cwilso commented Mar 8, 2015

@agoode we won't ever have a "makeState", though - the connection (hardware) state is absolutely read-only and outside of software control, and the connection state already has open()/close().

That said, preference seems to be on the enum side - aka Option 1 - so unless someone speaks up in the next 24 hours or so, I'll write that option up in the spec.

@toyoshim
Copy link
Contributor

toyoshim commented Mar 9, 2015

As a bonus point of separating states to MIDIPortDeviceState and MIDIPortConnectionState, open() and close() can be sync methods, right? Now that they do not depend on device connection state, it can be processed immediately without any IPC or I/O operations.

@agoode
Copy link

agoode commented Mar 9, 2015

There is still I/O work that happens with open() or close(). I think you are proposing that open() and close() return immediately, and the onstatechange handler reporting the result?

open() can fail though, so promise does seem better. An example is for a device already opened by another program in exclusive mode. Both Windows and Linux can fail in this way.

@cwilso
Copy link
Contributor Author

cwilso commented Mar 9, 2015

What @agoode said. Open() can still fail. Close() returning a promise probably isn't necessary, but probably good symmetry.

@toyoshim
Copy link
Contributor

toyoshim commented Mar 9, 2015

open() should not depend on device status that happens completely in parallel. So, if the device is occupied by another application, we may want to show the device as disconnected or in another new MIDIPortConnectionState, e.g., unavailable. Otherwise, we lose major benefit of separating state to two states, that avoid many kinds of race conditions.

@toyoshim
Copy link
Contributor

toyoshim commented Mar 9, 2015

In other words, we allow open state even on disconnected state. That means open() succeeds even in disconnected state. Otherwise, we could not be consistent on some race situations. E.g., device is disconnected on opening a device.
So, why should we make open() fail only when another application exclusively locks it? There are similar race conditions. Making open() and close() asynchronous methods will introduce many race issues that are hard for developers to understand and handle correctly.

@cwilso
Copy link
Contributor Author

cwilso commented Mar 9, 2015

Hmm, it's true that we need to explicitly enable open() on disconnected ports, and define precisely how/when it does/does not fail. However, we have a need to determine, in the explicit open() case at least, whether or not we ACTUALLY got access to the port. In a exclusive-access-to-ports system (Windows or Linux, e.g.), this is important. As it is right now, open() will fail if the port is unavailable - e.g. if the port is disconnected. Similarly, an implicit open() on an output port will fail ("If the port is "disconnected", throw an InvalidStateError exception."), and on an input port it will fail to open the port (since it just runs the "open" algorithm). I'm not convinced this is bad. I'm really only concerned that if I had an input port opened, and it gets disconnected then reconnected, it becomes reopened.

@cwilso cwilso reopened this Mar 9, 2015
@cwilso
Copy link
Contributor Author

cwilso commented Mar 9, 2015

Reopened this just until the discussion on open()ing a disconnected port resolves.

@cwilso
Copy link
Contributor Author

cwilso commented Mar 9, 2015

@toyoshim I don't think "we allow open state even on disconnected state" MUST mean "open() succeeds even in disconnected state." I think we can avoid this as a race condition - it's deterministic. I think the only point here is that we MUST give some way of determining that you can ACTUALLY send/receive on a port; are you saying that is "open the port, then make sure that .state is 'connected'"?

@toyoshim
Copy link
Contributor

toyoshim commented Mar 9, 2015

Since a device can be disconnected at any time, it is impossible to ensure that a port is ready to send a data in a certain situation. Also with the same reason, it's impossible to say open() will success only on connected state because MIDIPortDeviceState may be changed at any time. Even if the browser implementation succeeded to lock the device, on returning a result to the JavaScript side, the device may be disconnected.

So only we can say is that the data will be sent if the device is connected for a certain period before and after calling send(). And incoming data will be received if the device is connected and port is in open state at that time.

Generally speaking, reliable communication needs end-to-end handshake, and MIDI does not have such mechanisms. That was my first reason why I prefer separated two states.

@cwilso
Copy link
Contributor Author

cwilso commented Mar 9, 2015

Although I agree that it's impossible to ensure a port is ready to send at all times, we need to capture in the API the state of "probably ready to send". The big problem here is on Windows/Linux, where a piece of software will definitively need to know if it was allowed access to the port (since port access is exclusive). I'm thinking that this EITHER means we ensure the port is always "closed" when "disconnected", even if it is automatically reopened when the port is reconnected (my personal preference), or we add another exposure state of "underlying system has given access".

I think the changes that are necessary are: open() should only succeed if the port is allowed access by the underlying system (i.e. Windows gives us exclusive access); disconnection should move connection to "closed", but on device connection, if there is already a Port with this id and it has an event handler, it gets automatically re-opened if possible. (in other words, "open" means Windows has given access; a disconnected port would never be "open".) Thoughts?

@toyoshim
Copy link
Contributor

toyoshim commented Mar 9, 2015

So, I'm ok to keep open() as an async method returning Promise. That can be interpreted as a process to get an access permission from the underlying system that is different concept from device state change. That idea will be easily extended for obtaining port-based sysex permission in the future.

For exclusively retained case, I'm negative to add a new state. If we introduce a new state for the exclusive case, we may want to invoke a statechange handler when the device gets to be available. But I'm afraid that only we can implement it is to poll the retained device periodically.

How about just rejecting the Promise for open() with a certain error? Probably InvalidAccessError?

@cwilso
Copy link
Contributor Author

cwilso commented Mar 9, 2015

Sorry, I should have said there are three options:

  1. open() is async, and rejects if the underlying system doesn't give you access; if an open()ed device is disconnected, it remains "open" while disconnected and is auto-reconnected. This is the current spec, I think.

  2. open is async, and rejects if the underlying system doesn't give you access. If an "open" device is disconnected, it is closed; however, when an input Port with an event handler already set is reconnected, the open algorithm is run on it to attempt to reopen. This is done prior to the state change for the device state change, so the state would reflect this reopening (or lack thereof). This would probably be cleanest in terms of defining edge cases.

  3. we add some new underlying state.

Yes, the open() rejects with InvalidAccessError today if underlying access is not given.

@toyoshim
Copy link
Contributor

Thank you Chris.
I'm +1 on option 2).

@bome
Copy link

bome commented Mar 10, 2015

The advantages of 1) are that you can determine if a disconnected port will reopen when connected (state is open), and it gives you a way to stop that (call close() on a disconnected, open port). Is it possible to call open() on a disconnected port in case 1)? If yes, that would be useful, the app can remember your settings from a previous session and just open the ports it needs when restarted. Any disconnected ports will automatically be used when plugged in.
My main problem with case 1), however, is what happens when the automatic reopen fails. In that case, the port's state would be open and connected, but still you cannot use it, because the OS did not give access to it (e.g. another software was faster opening it). Or, the port would get closed then, but the app still wants to use the port, so the programmer needs to add code to do a manual automatic reopen. And if the user needs to do that anyway, we wouldn't need to add the auto-reopen stuff at all to the API...

  1. does not give you explicit control and view of the auto-open facility. Say, a keyboarder uses an app with his keyboard A. He then purchases keyboard B, unplugs A and inserts keyboard B, and configures the app to use B from now on. Internally, web-midi will mark A for reopen, but for the app it's impossible to know that or to prevent it. Now, maybe weeks later, he wants to use A with another software. As soon as he plugs in A, its port is grabbed by webmidi and A will not work in other software (if on Windows or Linux).

Even though it's not particularly pretty, an additional state will solve all these problems:

  • open: the port is successfully opened and ready to use. Exclusive ports are reserved to be used by this process.
  • autoOpen: the port will be opened as soon as the port becomes available. If the port fails to open, it stays in autoOpen state. Disconnecting an open port, or opening a disconnected port, will set its state to autoOpen. Closing an autoOpen port will set it to closed.
  • closed: port cannot be used

@cwilso
Copy link
Contributor Author

cwilso commented Mar 10, 2015

Indeed, that's why I separated the states to begin with, and somehow failed to noticed I'd messed up that value - so thus I give you option 2a):

open() is async, and rejects if the underlying system doesn't give you access. If an "open" device is disconnected, its connection state transitions to "pending"; when a Port with connection state "pending" is reconnected (.state=>"connected"), the open algorithm is run on it to attempt to reopen. If this connection fails (e.g. the Port is reserved by something else in the underlying system, and therefore unavailable), the connection state moves to "closed", else it transitions back to "open". This is done prior to the state change event for the device state change, so the state would reflect this reopening (or lack thereof).

close() is allowed on ports that are "pending", and will quickly transition to "closed". open() is allowed on disconnected ports, but it will resolve quickly - but only transition the state to "pending", not to "open". (In effect, it is marking the port for auto-opening.)

@bome
Copy link

bome commented Mar 10, 2015

+1 for naming it "pending", +1 for adding pending state...

@toyoshim
Copy link
Contributor

@cwilso Do you mean MIDIPortConnectionState has three states, "open", "pending", and "closed", and MIDIPortConnectionState has two states, "connected", "disconnected"?
If so, I'm fine with that idea.

@cwilso
Copy link
Contributor Author

cwilso commented Mar 10, 2015

@toyoshim yes, precisely. I'll write that up.

@cwilso cwilso closed this as completed in 386c60b Mar 10, 2015
@PozEnergy
Copy link

I am a musician and developer working on multiple WebMIDI projects and greatly appreciate all those here and elsewhere have done to implement it.

Should the last reference showing on this page talking about MIDIPortConnectionState refer to MIDIPortDeviceState as follows?

Do you mean MIDIPortConnectionState has three states, "open", "pending", and "closed", and MIDIPortDeviceState has two states, "connected", "disconnected"?

@cwilso
Copy link
Contributor Author

cwilso commented Nov 17, 2022

@PozEnergy Yes, that's correct. (That was what @toyoshim meant I'm sure, and what I put in the spec, but you're right, he wrote it incorrectly in the comment above.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants