midi surfaces fixes (partially for PR #898)

1. do more to ensure that we do not call MidiSurface::begin_using_device()
multiple times without ::stop_using_device() in between. This reduces the risk
of duplicate signal handler connections being made (it might even eliminate it).

2. Notify all control surfaces when MIDI connectivity is established AND
disestablished. This gives them a chance to update their notion of their
current connection state. This can be important with JACK across zombification,
but also likely across backend stop&start.

These changes currntly only impact classes derived from MidiSurface but
something equivalent is required for all control surfaces

libs/ardour/ardour/control_protocol_manager.h

@@ -72,7 +72,7 @@ class LIBARDOUR_API ControlProtocolManager : public PBD::Stateful, public ARDOUR
 	void set_session (Session*);
 	void discover_control_protocols ();
 	void foreach_known_protocol (boost::function<void(const ControlProtocolInfo*)>);
-	void midi_connectivity_established ();
+	void midi_connectivity_established (bool);
 	void drop_protocols ();
 	void probe_midi_control_protocols ();
 	void probe_usb_control_protocols (bool, uint16_t, uint16_t);

libs/ardour/control_protocol_manager.cc

@@ -619,12 +619,12 @@ ControlProtocolManager::instance ()
 }
 
 void
-ControlProtocolManager::midi_connectivity_established ()
+ControlProtocolManager::midi_connectivity_established (bool yn)
 {
 	Glib::Threads::RWLock::ReaderLock lm (protocols_lock);
 
 	for (list<ControlProtocol*>::iterator p = control_protocols.begin(); p != control_protocols.end(); ++p) {
-		(*p)->midi_connectivity_established ();
+		(*p)->midi_connectivity_established (yn);
 	}
 }
 

libs/ardour/session_state.cc

@@ -355,7 +355,7 @@ Session::post_engine_init ()
 		 * could start talking to surfaces if they want to.
 		 */
 
-		ControlProtocolManager::instance().midi_connectivity_established ();
+		ControlProtocolManager::instance().midi_connectivity_established (true);
 
 		if (_is_new && !no_auto_connect()) {
 			Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock());

libs/ardour/session_transport.cc

@@ -52,6 +52,7 @@
 #include "ardour/automation_watch.h"
 #include "ardour/butler.h"
 #include "ardour/click.h"
+#include "ardour/control_protocol_manager.h"
 #include "ardour/debug.h"
 #include "ardour/disk_reader.h"
 #include "ardour/io_tasklist.h"
@@ -1868,6 +1869,8 @@ Session::engine_halted ()
 	 */
 
 	realtime_stop (false, true);
+
+	ControlProtocolManager::instance().midi_connectivity_established (false);
 }
 
 void

libs/ctrl-interface/control_protocol/control_protocol/control_protocol.h

@@ -57,7 +57,7 @@ public:
 	virtual int set_feedback (bool /*yn*/) { return 0; }
 	virtual bool get_feedback () const { return false; }
 
-	virtual void midi_connectivity_established () {}
+	virtual void midi_connectivity_established (bool) {}
 
 	virtual void stripable_selection_changed () = 0;
 

libs/ctrl-interface/midi_surface/midi_surface.cc

@@ -256,6 +256,8 @@ MIDISurface::connection_handler (std::weak_ptr<ARDOUR::Port>, std::string name1,
 	std::string ni = ARDOUR::AudioEngine::instance()->make_port_name_non_relative (std::shared_ptr<ARDOUR::Port>(_async_in)->name());
 	std::string no = ARDOUR::AudioEngine::instance()->make_port_name_non_relative (std::shared_ptr<ARDOUR::Port>(_async_out)->name());
 
+	int old_connection_state = _connection_state;
+
 	if (ni == name1 || ni == name2) {
 		if (yn) {
 			_connection_state |= InputConnected;
@@ -277,14 +279,24 @@ MIDISurface::connection_handler (std::weak_ptr<ARDOUR::Port>, std::string name1,
 	DEBUG_TRACE (DEBUG::MIDISurface, string_compose ("our ports changed connection state: %1 -> %2 connected ? %3, connection state now %4\n",
 	                                                 name1, name2, yn, _connection_state));
 
-	if ((_connection_state & (InputConnected|OutputConnected)) == (InputConnected|OutputConnected)) {
+	/* it ought o be impossible for the connection state of our ports to
+	 * change without a corresponding change in _connection_state. But
+	 * since the consequences of calling device_acquire() and
+	 * begin_using_device() are substantial, include it as a test to catch
+	 * any weird corner cases.
+	 */
+
+	if ((_connection_state != old_connection_state) && (_connection_state & (InputConnected|OutputConnected)) == (InputConnected|OutputConnected)) {
+
+                DEBUG_TRACE (DEBUG::MIDISurface, "device now connected for both input and output\n");
+
+		if (!_in_use) {
 
 			/* XXX this is a horrible hack. Without a short sleep here,
 			   something prevents the device wakeup messages from being
 			   sent and/or the responses from being received.
 			*/
 
-                DEBUG_TRACE (DEBUG::MIDISurface, "device now connected for both input and output\n");
 			g_usleep (100000);
 
 			/* may not have the device open if it was just plugged
@@ -294,6 +306,7 @@ MIDISurface::connection_handler (std::weak_ptr<ARDOUR::Port>, std::string name1,
 
 			device_acquire ();
 			begin_using_device ();
+		}
 
 	} else {
 		DEBUG_TRACE (DEBUG::MIDISurface, "Device disconnected (input or output or both) or not yet fully connected\n");
@@ -332,6 +345,15 @@ MIDISurface::write (MIDI::byte const * data, size_t size)
 	_output_port->write (data, size, 0);
 }
 
+void
+MIDISurface::midi_connectivity_established (bool yn)
+{
+	if (!yn) {
+		_connection_state = ConnectionState (0);
+		stop_using_device ();
+	}
+}
+
 bool
 MIDISurface::midi_input_handler (IOCondition ioc, MIDI::Port* port)
 {

libs/ctrl-interface/midi_surface/midi_surface/midi_surface.h

@@ -85,6 +85,7 @@ class MIDISurface : public ARDOUR::ControlProtocol
 	CONTROL_PROTOCOL_THREADS_NEED_TEMPO_MAP_DECL();
 
 	virtual bool midi_input_handler (Glib::IOCondition ioc, MIDI::Port* port);
+	void midi_connectivity_established (bool);
 
   protected:
 	bool with_pad_filter;