FP8: some code comments and cleanup

4 files changed, 100 insertions, 53 deletions

diff --git a/libs/surfaces/faderport8/faderport8.cc b/libs/surfaces/faderport8/faderport8.cc
index b0037b72c0..9f642e8171 100644
--- a/libs/surfaces/faderport8/faderport8.cc
+++ b/libs/surfaces/faderport8/faderport8.cc
@@ -128,9 +128,11 @@ FaderPort8::FaderPort8 (Session& s)
 	ARDOUR::AudioEngine::instance()->Stopped.connect (port_connections, MISSING_INVALIDATOR, boost::bind (&FaderPort8::engine_reset, this), this);
 	ARDOUR::Port::PortDrop.connect (port_connections, MISSING_INVALIDATOR, boost::bind (&FaderPort8::engine_reset, this), this);
 
-	StripableSelectionChanged.connect (selection_connection, MISSING_INVALIDATOR, boost::bind (&FaderPort8::gui_track_selection_changed, this), this);
+	StripableSelectionChanged.connect (selection_connection, MISSING_INVALIDATOR, boost::bind (&FaderPort8::notify_gui_track_selection_changed, this), this);
 
+	/* bind button events to call libardour actions */
 	setup_actions ();
+
 	_ctrls.FaderModeChanged.connect_same_thread (modechange_connections, boost::bind (&FaderPort8::notify_fader_mode_changed, this));
 	_ctrls.MixModeChanged.connect_same_thread (modechange_connections, boost::bind (&FaderPort8::assign_strips, this, true));
 }
@@ -995,6 +997,10 @@ FaderPort8::assign_stripables (bool select_only)
 	}
 }
 
+/* ****************************************************************************
+ * Plugin selection and parameters
+ */
+
 void
 FaderPort8::assign_processor_ctrls ()
 {
@@ -1274,6 +1280,10 @@ FaderPort8::spill_plugins ()
 	assert (id == 8);
 }
 
+/* ****************************************************************************
+ * Aux Sends and Mixbus assigns
+ */
+
 void
 FaderPort8::assign_sends ()
 {
@@ -1334,13 +1344,9 @@ FaderPort8::assign_sends ()
 	assign_stripables (true);
 }
 
-void
-FaderPort8::set_periodic_display_mode (FP8Strip::DisplayMode m)
-{
-	for (uint8_t id = 0; id < 8; ++id) {
-		_ctrls.strip(id).set_periodic_display_mode (m);
-	}
-}
+/* ****************************************************************************
+ * Main stripable assignment (dispatch depending on mode)
+ */
 
 void
 FaderPort8::assign_strips (bool reset_bank)
@@ -1357,7 +1363,7 @@ FaderPort8::assign_strips (bool reset_bank)
 		case ModeTrack:
 		case ModePan:
 			assign_stripables ();
-			gui_track_selection_changed (); // update selection, automation-state
+			notify_gui_track_selection_changed (); // update selection, automation-state
 			break;
 		case ModePlugins:
 			if (_proc_params.size() > 0) {
@@ -1372,6 +1378,17 @@ FaderPort8::assign_strips (bool reset_bank)
 	}
 }
 
+/* ****************************************************************************
+ * some helper functions
+ */
+
+void
+FaderPort8::set_periodic_display_mode (FP8Strip::DisplayMode m)
+{
+	for (uint8_t id = 0; id < 8; ++id) {
+		_ctrls.strip(id).set_periodic_display_mode (m);
+	}
+}
 
 void
 FaderPort8::drop_ctrl_connections ()
@@ -1381,6 +1398,46 @@ FaderPort8::drop_ctrl_connections ()
 	_showing_well_known = 0;
 }
 
+/* functor for FP8Strip's select button */
+void
+FaderPort8::select_strip (boost::weak_ptr<Stripable> ws)
+{
+	boost::shared_ptr<Stripable> s = ws.lock();
+	if (!s) {
+		return;
+	}
+#if 1 /* single exclusive selection by default, toggle via shift */
+	if (shift_mod ()) {
+		ToggleStripableSelection (s);
+	} else {
+		SetStripableSelection (s);
+	}
+#else
+	/* tri-state selection: This allows to set the "first selected"
+	 * with a single click without clearing the selection.
+	 * Single de/select via shift.
+	 */
+	if (shift_mod ()) {
+		if (s->is_selected ()) {
+			RemoveStripableFromSelection (s);
+		} else {
+			SetStripableSelection (s);
+		}
+		return;
+	}
+	if (s->is_selected () && s != first_selected_stripable ()) {
+		set_first_selected_stripable (s);
+		notify_gui_track_selection_changed ();
+	} else {
+		ToggleStripableSelection (s);
+	}
+#endif
+}
+
+/* ****************************************************************************
+ * Assigned Stripable Callbacks
+ */
+
 void
 FaderPort8::notify_fader_mode_changed ()
 {
@@ -1411,10 +1468,6 @@ FaderPort8::notify_fader_mode_changed ()
 	notify_automation_mode_changed ();
 }
 
-/* ****************************************************************************
- * Assigned Stripable Callbacks
- */
-
 void
 FaderPort8::notify_stripable_added_or_removed ()
 {
@@ -1428,42 +1481,6 @@ FaderPort8::notify_stripable_added_or_removed ()
 	assign_strips (false);
 }
 
-/* functor for FP8Strip's select button */
-void
-FaderPort8::select_strip (boost::weak_ptr<Stripable> ws)
-{
-	boost::shared_ptr<Stripable> s = ws.lock();
-	if (!s) {
-		return;
-	}
-#if 1 /* single exclusive selection by default, toggle via shift */
-	if (shift_mod ()) {
-		ToggleStripableSelection (s);
-	} else {
-		SetStripableSelection (s);
-	}
-#else
-	/* tri-state selection: This allows to set the "first selected"
-	 * with a single click without clearing the selection.
-	 * Single de/select via shift.
-	 */
-	if (shift_mod ()) {
-		if (s->is_selected ()) {
-			RemoveStripableFromSelection (s);
-		} else {
-			SetStripableSelection (s);
-		}
-		return;
-	}
-	if (s->is_selected () && s != first_selected_stripable ()) {
-		set_first_selected_stripable (s);
-		gui_track_selection_changed ();
-	} else {
-		ToggleStripableSelection (s);
-	}
-#endif
-}
-
 /* called from static PresentationInfo::Change */
 void
 FaderPort8::notify_pi_property_changed (const PropertyChange& what_changed)
@@ -1521,8 +1538,14 @@ FaderPort8::notify_stripable_property_changed (boost::weak_ptr<Stripable> ws, co
 }
 
 void
-FaderPort8::gui_track_selection_changed (/*ARDOUR::StripableNotificationListPtr*/)
+FaderPort8::notify_gui_track_selection_changed (/*ARDOUR::StripableNotificationListPtr*/)
 {
+	if (!_device_active) {
+		/* this can be called anytime from the static
+		 * ControlProtocol::StripableSelectionChanged
+		 */
+		return;
+	}
 	automation_state_connections.drop_connections();
 
 	switch (_ctrls.fader_mode ()) {
diff --git a/libs/surfaces/faderport8/faderport8.h b/libs/surfaces/faderport8/faderport8.h
index 00ffe827df..e4809e6d2d 100644
--- a/libs/surfaces/faderport8/faderport8.h
+++ b/libs/surfaces/faderport8/faderport8.h
@@ -174,9 +174,10 @@ private:
 	void drop_ctrl_connections ();
 
 	void select_strip (boost::weak_ptr<ARDOUR::Stripable>);
+
 	void notify_pi_property_changed (const PBD::PropertyChange&);
 	void notify_stripable_property_changed (boost::weak_ptr<ARDOUR::Stripable>, const PBD::PropertyChange&);
-	void gui_track_selection_changed ();
+	void notify_gui_track_selection_changed ();
 
 	PBD::ScopedConnection selection_connection;
 	PBD::ScopedConnectionList automation_state_connections;
diff --git a/libs/surfaces/faderport8/fp8_base.h b/libs/surfaces/faderport8/fp8_base.h
index 753440c70b..c47f05b523 100644
--- a/libs/surfaces/faderport8/fp8_base.h
+++ b/libs/surfaces/faderport8/fp8_base.h
@@ -31,6 +31,16 @@ namespace ArdourSurface {
 #define fp8_context() dynamic_cast<BaseUI*>(&_base)
 #define fp8_protocol() dynamic_cast<ControlProtocol*>(&_base)
 
+/** Virtual abstracte base of the FaderPort8 control surface
+ *
+ * This is passed as handle to all elements (buttons, lights,..)
+ * to inteface common functionality for the current instance:
+ *  - sending MIDI
+ *  - global events (signals)
+ *  - thread context
+ *
+ * It is implemented by FaderPort8
+ */
 class FP8Base
 {
 public:
@@ -96,9 +106,11 @@ public:
 		 return tx_midi (d);
 	}
 
+	/* modifier keys */
 	PBD::Signal1<void, bool> ShiftButtonChange;
 	PBD::Signal1<void, bool> ARMButtonChange;
 
+	/* timer events */
 	PBD::Signal1<void, bool> BlinkIt;
 	PBD::Signal0<void> Periodic;
 
diff --git a/libs/surfaces/faderport8/fp8_button.h b/libs/surfaces/faderport8/fp8_button.h
index a817dd8e79..cddf7f471f 100644
--- a/libs/surfaces/faderport8/fp8_button.h
+++ b/libs/surfaces/faderport8/fp8_button.h
@@ -1,4 +1,5 @@
-/*
+/* FaderPort8 Button Interface
+ *
  * Copyright (C) 2017 Robin Gareus <robin@gareus.org>
  *
  * This program is free software; you can redistribute it and/or
@@ -28,6 +29,7 @@
 
 namespace ArdourSurface {
 
+/* virtual base-class and interface */
 class FP8ButtonInterface
 {
 public:
@@ -58,6 +60,10 @@ public:
 	static bool force_change; // used during init
 };
 
+/* ****************************************************************************
+ * Implementations
+ */
+
 class FP8DummyButton : public FP8ButtonInterface
 {
 public:
@@ -66,6 +72,7 @@ public:
 };
 
 
+/* common implementation */
 class FP8ButtonBase : public FP8ButtonInterface
 {
 public:
@@ -129,6 +136,7 @@ private:
 	bool _blinking;
 };
 
+/* A basic LED or RGB button, not shift sensitive */
 class FP8Button : public FP8ButtonBase
 {
 public:
@@ -169,6 +177,7 @@ protected:
 	bool     _has_color;
 };
 
+/* footswitch and encoder-press buttons */
 class FP8ReadOnlyButton : public FP8Button
 {
 public:
@@ -430,6 +439,9 @@ private:
 	sigc::connection _hold_connection;
 };
 
+/* an auto-repeat button.
+ * press + hold emits continuous "press" events.
+ */
 class FP8RepeatButton : public FP8Button
 {
 public:
@@ -485,6 +497,5 @@ private:
 	sigc::connection _press_timeout_connection;
 };
 
-
 } /* namespace */
 #endif /* _ardour_surfaces_fp8button_h_ */