Support users changing device names while running, #21

Also work around Java's strange habit of instantiating
CoreMidiDeviceProvider hundreds of times, which was wasting many
cycles rebuilding the device map repeatedly on any MIDI environment
change.

CHANGELOG.md

@@ -15,6 +15,22 @@ This change log follows the conventions of
   closing transmitters and receivers.
 - We now close any devices (and their transmitters or receivers) when
   the underlying CoreMIDI device disappears.
+- It turns out that Java instantiates many, many instances of our
+  `CoreMidiDeviceProvider` class, each of which was getting added to
+  the listener list to be notified when the MIDI environment changed,
+  causing the device list to be rebuilt hundreds of times more than it
+  needed to be. We now work around this by only calling the listener
+  on the most-recently constructed instance.
+
+### Added
+
+- We now allow you to distinguish between multiple devices of the same
+  type by showing the user-assigned name of the device (and of the
+  port, if there is more than one port on the device), as discussed in
+  [Issue 21](https://github.com/DerekCook/CoreMidi4J/issues/21).
+- If the user changes a device or port name while Java is running, the
+  device information is properly updated, although the device itself
+  retains its object-level identity and opened state.
 
 
 ## [1.0] - 2017-05-14

CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiClient.java

@@ -14,8 +14,10 @@
 
 package uk.co.xfactorylibrarians.coremidi4j;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * CoreMidiClient class
@@ -26,7 +28,15 @@ public class CoreMidiClient {
 
   private final int midiClientReference;
 
-  private final List<CoreMidiNotification> notificationListeners = new ArrayList<CoreMidiNotification>();
+  private final Set<CoreMidiNotification> notificationListeners =
+          Collections.newSetFromMap(new ConcurrentHashMap<CoreMidiNotification, Boolean>());
+
+  /**
+   * Keeps track of the latest {@link CoreMidiDeviceProvider} added to our listener list; this is the only one that
+   * we want to call when the MIDI environment changes, since we only need to update the device map once, and Java
+   * creates a vast number of instances of our device provider.
+   */
+  private CoreMidiNotification mostRecentDeviceProvider = null;
 
   /**
    * Constructor for class
@@ -77,6 +87,97 @@ public CoreMidiOutputPort outputPortCreate(final String name) throws CoreMidiExc
 
   }
 
+  /**
+   * Used to make sure we are only running one callback delivery loop at a time without having to serialize the process
+   * in a way that will block the actual CoreMidi callback.
+   */
+
+  private final AtomicBoolean runningCallbacks = new AtomicBoolean(false);
+
+  /**
+   * Used to count the number of CoreMidi environment change callbacks we have received, so that if additional ones
+   * come in while we are delivering callback messages to our listeners, we know to start another round at the end.
+   */
+
+  private final AtomicInteger callbackCount = new AtomicInteger( 0);
+
+  /**
+   * Check whether we are already in the process of delivering callbacks to our listeners; if not, start a background
+   * thread to do so, and at the end of that process, see if additional callbacks were attempted while it was going on.
+   */
+
+  private void deliverCallbackToListeners() {
+
+    final int initialCallbackCount = callbackCount.get();
+
+    if (runningCallbacks.compareAndSet(false, true)) {
+
+      new Thread(new Runnable() {
+
+        @Override
+        public void run() {
+
+          try {
+
+            int currentCallbackCount = initialCallbackCount;
+            while ( currentCallbackCount > 0 ) {  // Loop until no new callbacks occur while delivering a set.
+
+              // Iterate through the listeners (if any) and call them to advise that the environment has changed.
+              final Set<CoreMidiNotification> listeners = Collections.unmodifiableSet(new HashSet<>(notificationListeners));
+
+              // First notify the CoreMidiDeviceProvider object itself, so that the device map is
+              // updated before any other listeners, from client code, are called.
+              if (mostRecentDeviceProvider != null) {
+
+                try {
+
+                  mostRecentDeviceProvider.midiSystemUpdated();
+
+                } catch (CoreMidiException e) {
+
+                  throw new RuntimeException("Problem delivering MIDI environment change notification to CoreMidiDeviceProvider" , e);
+
+                }
+
+              }
+
+              // Finally, notify any registered client code listeners, now that the device map is properly up to date.
+              for ( CoreMidiNotification listener : listeners ) {
+
+                try {
+
+                  listener.midiSystemUpdated();
+
+                } catch (CoreMidiException e) {
+
+                  throw new RuntimeException("Problem delivering MIDI environment change notification" , e);
+
+                }
+
+              }
+
+              synchronized (CoreMidiClient.this) {
+
+                // We have handled however many callbacks occurred before this iteration started
+                currentCallbackCount = callbackCount.addAndGet( -currentCallbackCount );
+
+              }
+
+            }
+
+          } finally {
+
+            runningCallbacks.set(false);   // We are terminating.
+
+          }
+
+        }
+
+      }).start();
+
+    }
+  }
+
   /**
    * The message callback for receiving notifications about changes in the MIDI environment from the JNI code
    * 
@@ -91,41 +192,42 @@ public void notifyCallback() throws CoreMidiException  {
 
     synchronized(this) {
 
-      // Iterate through the listeners (if any) and call them to advise that the environment has changed    	
-      for ( CoreMidiNotification listener : notificationListeners ) {
-
-        listener.midiSystemUpdated();
-
-      }
+      callbackCount.incrementAndGet();  // Record that a callback has come in.
+      deliverCallbackToListeners();  // Try to deliver callback notifications to our listeners.
 
     }
 
   }
 
   /**
-   * Adds a notification listener to the listener list maintained by this class
+   * Adds a notification listener to the listener set maintained by this class
    * 
    * @param listener	The CoreMidiNotification listener to add
    * 
    */
 
   public void addNotificationListener(CoreMidiNotification listener) {
 
-    // Need to ensure that any CoreMidiDeviceProvider objects are at the head of the notification list, so that the device map is updated before other listeners are called
-    if ( listener instanceof CoreMidiDeviceProvider ) {
+    if ( listener != null ) {
 
-      notificationListeners.add(0,listener);
+      // Our CoreMidiDeviceProvider is a special case, we only want to notify a single instance of that, even though
+      // Java keeps creating new ones. So keep track of the most recent instance registered, do not add it to the list.
+      if (listener instanceof CoreMidiDeviceProvider) {
 
-    } else {
+        mostRecentDeviceProvider = listener;
 
-      notificationListeners.add(listener);
+      } else {
+
+        notificationListeners.add(listener);
+
+      }
 
     }
 
   }
 
   /**
-   * Removes a notification listener from the listener list maintained by this class
+   * Removes a notification listener from the listener set maintained by this class
    * 
    * @param listener	The CoreMidiNotification listener to remove
    * 

CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDestination.java

@@ -34,7 +34,7 @@
 
 public class CoreMidiDestination implements MidiDevice {
 
-  private final CoreMidiDeviceInfo info;
+  private CoreMidiDeviceInfo info;
   private final AtomicBoolean isOpen;  // Tracks whether we are conneted to CoreMIDI and can be used
   private final AtomicLong startTime;  // The system time in microseconds when the port was opened
   private final Set<CoreMidiReceiver> receivers;
@@ -69,6 +69,15 @@ public Info getDeviceInfo() {
 
   }
 
+  /**
+   * Changes the MIDI Info object; can only be done by this package as a result of a MIDI environment change event.
+   */
+  void updateDeviceInfo(CoreMidiDeviceInfo info) {
+
+    this.info = info;
+
+  }
+
   /**
    * Opens the Core MIDI Device
    * 

CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDeviceProvider.java

@@ -45,11 +45,15 @@ private static final class MidiProperties {
    * 
    */
 
-  private void initialise() throws CoreMidiException {
+  private synchronized void initialise() throws CoreMidiException {
 
-    midiProperties.client = new CoreMidiClient("Core MIDI Provider");
-    midiProperties.output = midiProperties.client.outputPortCreate("Core Midi Provider Output");
-    buildDeviceMap();
+    if ( midiProperties.client == null ) {
+
+      midiProperties.client = new CoreMidiClient("Core MIDI Provider");
+      midiProperties.output = midiProperties.client.outputPortCreate("Core Midi Provider Output");
+      buildDeviceMap();
+
+    }
 
   }
 
@@ -100,10 +104,15 @@ private void buildDeviceMap() throws CoreMidiException {
       // Keep track of the IDs of all the devices we see
       devicesSeen.add(uniqueID);
 
-      // If the unique ID of the end point is not in the map then create a CoreMidiSource object and add it to the map
+      // If the unique ID of the end point is not in the map then create a CoreMidiSource object and add it to the map.
       if ( !midiProperties.deviceMap.containsKey(uniqueID) ) {
 
-        midiProperties.deviceMap.put(uniqueID,new CoreMidiSource(getMidiDeviceInfo(endPointReference)));
+        midiProperties.deviceMap.put(uniqueID, new CoreMidiSource(getMidiDeviceInfo(endPointReference)));
+
+      } else {  // We already know about the device, but may need to update its information (e.g. user renamed it).
+
+        CoreMidiSource existingDevice = (CoreMidiSource) midiProperties.deviceMap.get(uniqueID);
+        existingDevice.updateDeviceInfo(getMidiDeviceInfo(endPointReference));
 
       }
 
@@ -119,11 +128,16 @@ private void buildDeviceMap() throws CoreMidiException {
       // Keep track of the IDs of all the devices we see
       devicesSeen.add(uniqueID);
 
-      // If the unique ID of the end point is not in the map then create a CoreMidiDestination object and add it to the map
+      // If the unique ID of the end point is not in the map then create a CoreMidiDestination object and add it to the map.
       if ( !midiProperties.deviceMap.containsKey(uniqueID) ) {
 
         midiProperties.deviceMap.put(uniqueID, new CoreMidiDestination(getMidiDeviceInfo(endPointReference)));
 
+      } else {  // We already know about the device, but may need to update its information (e.g. user renamed it).
+
+        CoreMidiDestination existingDevice = (CoreMidiDestination) midiProperties.deviceMap.get(uniqueID);
+        existingDevice.updateDeviceInfo(getMidiDeviceInfo(endPointReference));
+
       }
 
     }
@@ -314,9 +328,9 @@ public boolean isDeviceSupported(final MidiDevice.Info info) {
 
   /**
    * Called when a change in the MIDI environment occurs
-   * 
+   *
    * @throws CoreMidiException if a problem occurs rebuilding the map of available MIDI devices
-   * 
+   *
    */
 
   public void midiSystemUpdated() throws CoreMidiException {
@@ -327,7 +341,9 @@ public void midiSystemUpdated() throws CoreMidiException {
   }
 
   /**
-   * Adds a notification listener to the listener list maintained by this class
+   * Adds a notification listener to the listener set maintained by this class. If it is a
+   * {@link CoreMidiDeviceProvider}, only keep the most recent one, since Java will create many,
+   * and we only want to update the device map once when the MIDI environment changes.
    * 
    * @param listener	The CoreMidiNotification listener to add
    * 

CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiSource.java

@@ -35,7 +35,7 @@
 
 public class CoreMidiSource implements MidiDevice {
 
-  private final CoreMidiDeviceInfo info;
+  private CoreMidiDeviceInfo info;
   private final AtomicBoolean isOpen;
   private final AtomicReference<CoreMidiInputPort> input;
   private final Set<CoreMidiTransmitter> transmitters;
@@ -77,6 +77,15 @@ public Info getDeviceInfo() {
 
   }
 
+  /**
+   * Changes the MIDI Info object; can only be done by this package as a result of a MIDI environment change event.
+   */
+  void updateDeviceInfo(CoreMidiDeviceInfo info) {
+
+    this.info = info;
+
+  }
+
   /**
    * Opens the Core MIDI Device
    *