new ControlProtocolManager API, and proper handling of view/model changes in the RC options (prefs) editor

3 files changed, 80 insertions, 35 deletions

diff --git a/gtk2_ardour/rc_option_editor.cc b/gtk2_ardour/rc_option_editor.cc
index 43fd163d2c..b514a75a80 100644
--- a/gtk2_ardour/rc_option_editor.cc
+++ b/gtk2_ardour/rc_option_editor.cc
@@ -648,6 +648,7 @@ class ControlSurfacesOptions : public OptionEditorBox
 public:
 	ControlSurfacesOptions (Gtk::Window& parent)
 		: _parent (parent)
+		, _ignore_view_change (0)
 	{
 		_store = ListStore::create (_model);
 		_view.set_model (_store);
@@ -700,9 +701,14 @@ private:
         void protocol_status_changed (ControlProtocolInfo* cpi) {
 		/* find the row */
 		TreeModel::Children rows = _store->children();
+		
 		for (TreeModel::Children::iterator x = rows.begin(); x != rows.end(); ++x) {
+			string n = ((*x)[_model.name]);
+
 			if ((*x)[_model.protocol_info] == cpi) {
+				_ignore_view_change++;
 				(*x)[_model.enabled] = (cpi->protocol || cpi->requested);
+				_ignore_view_change--;
 				break;
 			}
 		}
@@ -712,6 +718,10 @@ private:
 	{
 		TreeModel::Row r = *i;
 
+		if (_ignore_view_change) {
+			return;
+		}
+
 		ControlProtocolInfo* cpi = r[_model.protocol_info];
 		if (!cpi) {
 			return;
@@ -720,22 +730,23 @@ private:
 		bool const was_enabled = (cpi->protocol != 0);
 		bool const is_enabled = r[_model.enabled];
 
+
 		if (was_enabled != is_enabled) {
+
 			if (!was_enabled) {
-				ControlProtocolManager::instance().instantiate (*cpi);
+				ControlProtocolManager::instance().activate (*cpi);
 			} else {
 				Gtk::Window* win = r[_model.editor];
 				if (win) {
 					win->hide ();
 				}
 
-				ControlProtocolManager::instance().teardown (*cpi);
+				ControlProtocolManager::instance().deactivate (*cpi);
 					
 				if (win) {
 					delete win;
+					r[_model.editor] = 0;
 				}
-				r[_model.editor] = 0;
-				cpi->requested = false;
 			}
 		}
 
@@ -805,6 +816,7 @@ private:
 	TreeView _view;
         Gtk::Window& _parent;
         PBD::ScopedConnection protocol_status_connection;
+        uint32_t _ignore_view_change;
 };
 
 class VideoTimelineOptions : public OptionEditorBox
diff --git a/libs/ardour/ardour/control_protocol_manager.h b/libs/ardour/ardour/control_protocol_manager.h
index 519c762eee..7d3d4872e6 100644
--- a/libs/ardour/ardour/control_protocol_manager.h
+++ b/libs/ardour/ardour/control_protocol_manager.h
@@ -65,8 +65,8 @@ class ControlProtocolManager : public PBD::Stateful, public ARDOUR::SessionHandl
 	void load_mandatory_protocols ();
 	void midi_connectivity_established ();
 
-	ControlProtocol* instantiate (ControlProtocolInfo&);
-	int              teardown (ControlProtocolInfo&);
+        int activate (ControlProtocolInfo&);
+        int deactivate (ControlProtocolInfo&);
 	
 	std::list<ControlProtocolInfo*> control_protocol_info;
 
@@ -89,6 +89,8 @@ class ControlProtocolManager : public PBD::Stateful, public ARDOUR::SessionHandl
 	int control_protocol_discover (std::string path);
 	ControlProtocolDescriptor* get_descriptor (std::string path);
 	ControlProtocolInfo* cpi_by_name (std::string);
+	ControlProtocol* instantiate (ControlProtocolInfo&);
+	int teardown (ControlProtocolInfo&);
 };
 
 } // namespace
diff --git a/libs/ardour/control_protocol_manager.cc b/libs/ardour/control_protocol_manager.cc
index 2e65a8d6f8..974258a5c2 100644
--- a/libs/ardour/control_protocol_manager.cc
+++ b/libs/ardour/control_protocol_manager.cc
@@ -72,26 +72,54 @@ ControlProtocolManager::set_session (Session* s)
 
 		for (list<ControlProtocolInfo*>::iterator i = control_protocol_info.begin(); i != control_protocol_info.end(); ++i) {
 			if ((*i)->requested || (*i)->mandatory) {
-				
-				instantiate (**i);
-				(*i)->requested = false;
-
-				if ((*i)->protocol) {
-					if ((*i)->state) {
-						(*i)->protocol->set_state (*(*i)->state, Stateful::loading_state_version);
-					} else {
-						/* guarantee a call to
-						   set_state() whether we have
-						   existing state or not
-						*/
-						(*i)->protocol->set_state (XMLNode(""), Stateful::loading_state_version);
-					}
-				}
+				(void) activate (**i);
 			}
 		}
 	}
 }
 
+int
+ControlProtocolManager::activate (ControlProtocolInfo& cpi)
+{
+	ControlProtocol* cp;
+
+	cpi.requested = true;
+
+	if ((cp = instantiate (cpi)) == 0) {
+		return -1;
+	}
+
+	/* we split the set_state() and set_active() operations so that
+	   protocols that need state to configure themselves (e.g. "What device
+	   is connected, or supposed to be connected?") can get it before
+	   actually starting any interaction.
+	*/
+
+	if (cpi.state) {
+		/* force this by tweaking the internals of the state
+		 * XMLNode. Ugh.
+		 */
+		cp->set_state (*cpi.state, Stateful::loading_state_version);
+	} else {
+		/* guarantee a call to
+		   set_state() whether we have
+		   existing state or not
+		*/
+		cp->set_state (XMLNode(""), Stateful::loading_state_version);
+	}
+
+	cp->set_active (true);
+
+	return 0;
+}	
+
+int
+ControlProtocolManager::deactivate (ControlProtocolInfo& cpi)
+{
+	cpi.requested = false;
+	return teardown (cpi);
+}
+
 void
 ControlProtocolManager::session_going_away()
 {
@@ -163,6 +191,12 @@ ControlProtocolManager::teardown (ControlProtocolInfo& cpi)
 	if (cpi.mandatory) {
 		return 0;
 	}
+	
+	/* save current state */
+
+	delete cpi.state;
+	cpi.state = new XMLNode (cpi.protocol->get_state());
+	cpi.state->add_property (X_("active"), "no");
 
 	cpi.descriptor->destroy (cpi.descriptor, cpi.protocol);
 
@@ -177,8 +211,6 @@ ControlProtocolManager::teardown (ControlProtocolInfo& cpi)
 	}
 
 	cpi.protocol = 0;
-	delete cpi.state;
-	cpi.state = 0;
 	dlclose (cpi.descriptor->module);
 
 	ProtocolStatusChange (&cpi);
@@ -379,22 +411,21 @@ ControlProtocolManager::get_state ()
 
 	for (list<ControlProtocolInfo*>::iterator i = control_protocol_info.begin(); i != control_protocol_info.end(); ++i) {
 
-		XMLNode * child;
-
 		if ((*i)->protocol) {
-			child = &((*i)->protocol->get_state());
-			child->add_property (X_("active"), "yes");
-			// should we update (*i)->state here?  probably.
-			root->add_child_nocopy (*child);
+			XMLNode& child_state ((*i)->protocol->get_state());
+			child_state.add_property (X_("active"), "yes");
+			root->add_child_nocopy (child_state);
 		} else if ((*i)->state) {
-			// keep ownership clear
-			root->add_child_copy (*(*i)->state);
+			XMLNode* child_state = new XMLNode (*(*i)->state);
+			child_state->add_property (X_("active"), "no");
+			root->add_child_nocopy (*child_state);
 		} else {
-			child = new XMLNode (X_("Protocol"));
-			child->add_property (X_("name"), (*i)->name);
-			child->add_property (X_("active"), "no");
-			root->add_child_nocopy (*child);
+			XMLNode* child_state = new XMLNode (X_("Protocol"));
+			child_state->add_property (X_("name"), (*i)->name);
+			child_state->add_property (X_("active"), "no");
+			root->add_child_nocopy (*child_state);
 		}
+
 	}
 
 	return *root;