substantial overhaul of MCU code - no more separate thread, just connect to signals on ports already listened to by the MidiUI thread in libardour (feedback is missing - needs a timeout connection); also reformat some big chunks of code to fit ardour coding style

git-svn-id: svn://localhost/ardour2/branches/3.0@6377 d708f5d6-7413-0410-9779-e7cbd77b26cf

libs/surfaces/mackie/controls.h

@@ -228,7 +228,7 @@ public:
 	virtual unsigned int in_use_timeout() { return _in_use_timeout; }
 
 	/// Keep track of the timeout so it can be updated with more incoming events
-	PBD::ScopedConnection in_use_connection;
+	sigc::connection in_use_connection;
 	
 private:
 	int _id;

libs/surfaces/mackie/mackie_control_protocol.h

@@ -15,6 +15,7 @@
     along with this program; if not, write to the Free Software
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 */
+
 #ifndef ardour_mackie_control_protocol_h
 #define ardour_mackie_control_protocol_h
 
@@ -26,6 +27,7 @@
 #include <glibmm/thread.h>
 
 #include "ardour/types.h"
+#include "ardour/midi_ui.h"
 #include "midi++/types.h"
 
 #include "control_protocol/control_protocol.h"
@@ -40,7 +42,6 @@
 
 namespace MIDI {
 	class Port;
-	class Parser;
 }
 
 namespace Mackie {
@@ -66,6 +67,7 @@ namespace Mackie {
 	up the relevant Strip in Surface. Then the state is retrieved from
 	the Route and encoded as the correct midi message.
 */
+
 class MackieControlProtocol 
 	: public ARDOUR::ControlProtocol
 	, public Mackie::MackieButtonHandler
@@ -112,6 +114,7 @@ class MackieControlProtocol
 	void refresh_current_bank();
 
 	// global buttons (ie button not part of strips)
+
   public:
 	// button-related signals
 	void notify_record_state_changed();
@@ -263,21 +266,6 @@ class MackieControlProtocol
 	*/
 	bool handle_strip_button(Mackie::Control &, Mackie::ButtonState, boost::shared_ptr<ARDOUR::Route>);
 
-	/// thread started. Calls monitor_work.
-	static void* _monitor_work (void* arg);
-	
-	/// Polling midi port(s) for incoming messages
-	void* monitor_work ();
-	
-	/// rebuild the set of ports for this surface
-	void update_ports();
-	
-	/// Returns true if there is pending data, false otherwise
-	bool poll_ports();
-	
-	/// Trigger the MIDI::Parser
-	void read_ports();
-
 	void add_port(MIDI::Port &, int number);
 
 	/**
@@ -324,18 +312,12 @@ class MackieControlProtocol
 	/// Sometimes the real port goes away, and we want to contain the breakage
 	Mackie::DummyPort _dummy_port;
   
-   // the thread that polls the ports for incoming midi data
-	pthread_t thread;
-  
 	/// The initial remote_id of the currently switched in bank.
 	uint32_t _current_initial_bank;
 	
-   /// protects the port list, and polling structures
+	/// protects the port list
 	Glib::Mutex update_mutex;
   
-	/// Protects set_active, and allows waiting on the poll thread
-	Glib::Cond update_cond;
-	
 	PBD::ScopedConnectionList session_connections;
 	PBD::ScopedConnectionList port_connections;
 	PBD::ScopedConnectionList route_connections;
@@ -343,14 +325,6 @@ class MackieControlProtocol
 	/// The representation of the physical controls on the surface.
   	Mackie::Surface * _surface;
 	
-	/// If a port is opened or closed, this will be
-	/// true until the port configuration is updated;
-	bool _ports_changed;
-
-	bool _polling;
-	struct pollfd * pfd;
-	int nfds;
-	
 	bool _transport_previously_rolling;
 	
 	// timer for two quick marker left presses
@@ -358,9 +332,6 @@ class MackieControlProtocol
 	
 	Mackie::JogWheel _jog_wheel;
 	
-	// Timer for controlling midi bandwidth used by automation polls
-	Mackie::Timer _automation_last;
-	
 	// last written timecode string
 	std::string _timecode_last;
 	

libs/surfaces/mackie/mackie_control_protocol_poll.cc

@@ -29,151 +29,14 @@ const char * MackieControlProtocol::default_port_name = "mcu";
 
 bool MackieControlProtocol::probe()
 {
-	if ( MIDI::Manager::instance()->port( default_port_name ) == 0 )
+	if ( MIDI::Manager::instance()->port(default_port_name)  == 0 ) {
-	{
 		info << "Mackie: No MIDI port called " << default_port_name << endmsg;
 		return false;
-	}
+	} else {
-	else
-	{
 		return true;
 	}
 }
 
-void * MackieControlProtocol::monitor_work()
-{
-	register_thread (X_("MCU"));
-
-	pthread_setcancelstate (PTHREAD_CANCEL_ENABLE, 0);
-	pthread_setcanceltype (PTHREAD_CANCEL_ASYNCHRONOUS, 0);
-
-	// read from midi ports
-	while ( _polling )
-	{
-		try
-		{
-			if ( poll_ports() )
-			{
-				try { read_ports(); }
-				catch ( exception & e ) {
-					cout << "MackieControlProtocol::poll_ports caught exception: " << e.what() << endl;
-					_ports_changed = true;
-					update_ports();
-				}
-			}
-			// poll for session data that needs to go to the unit
-			poll_session_data();
-		}
-		catch ( exception & e )
-		{
-			cout << "caught exception in MackieControlProtocol::monitor_work " << e.what() << endl;
-		}
-	}
-
-	// TODO ports and pfd and nfds should be in a separate class
-	delete[] pfd;
-	pfd = 0;
-	nfds = 0;
-
-	return (void*) 0;
-}
-
-void MackieControlProtocol::update_ports()
-{
-#ifdef DEBUG
-	cout << "MackieControlProtocol::update_ports" << endl;
-#endif
-	if ( _ports_changed )
-	{
-		Glib::Mutex::Lock ul( update_mutex );
-		// yes, this is a double-test locking paradigm, or whatever it's called
-		// because we don't *always* need to acquire the lock for the first test
-#ifdef DEBUG
-		cout << "MackieControlProtocol::update_ports lock acquired" << endl;
-#endif
-		if ( _ports_changed )
-		{
-			// create new pollfd structures
-			delete[] pfd;
-			pfd = new pollfd[_ports.size()];
-#ifdef DEBUG
-			cout << "pfd: " << pfd << endl;
-#endif
-			nfds = 0;
-			for( MackiePorts::iterator it = _ports.begin(); it != _ports.end(); ++it )
-			{
-				// add the port any handler
-				(*it)->connect_any();
-#ifdef DEBUG
-				cout << "adding pollfd for port " << (*it)->port().name() << " to pollfd " << nfds << endl;
-#endif
-				pfd[nfds].fd = (*it)->port().selectable();
-				pfd[nfds].events = POLLIN|POLLHUP|POLLERR;
-				++nfds;
-			}
-			_ports_changed = false;
-		}
-#ifdef DEBUG
-		cout << "MackieControlProtocol::update_ports signal" << endl;
-#endif
-		update_cond.signal();
-	}
-#ifdef DEBUG
-	cout << "MackieControlProtocol::update_ports finish" << endl;
-#endif
-}
-
-void MackieControlProtocol::read_ports()
-{
-	/* now read any data on the ports */
-	Glib::Mutex::Lock lock( update_mutex );
-	for ( int p = 0; p < nfds; ++p )
-	{
-		// this will cause handle_midi_any in the MackiePort to be triggered
-		// for alsa/raw ports
-		// alsa/sequencer ports trigger the midi parser off poll
-		if ( (pfd[p].revents & POLLIN) > 0 )
-		{
-			// avoid deadlocking?
-			// doesn't seem to make a difference
-			//lock.release();
-			_ports[p]->read();
-			//lock.acquire();
-		}
-	}
-}
-
-bool MackieControlProtocol::poll_ports()
-{
-	int timeout = 10; // milliseconds
-	int no_ports_sleep = 1000; // milliseconds
-
-	Glib::Mutex::Lock lock( update_mutex );
-	// if there are no ports
-	if ( nfds < 1 )
-	{
-		lock.release();
-#ifdef DEBUG
-		cout << "poll_ports no ports" << endl;
-#endif
-		usleep( no_ports_sleep * 1000 );
-		return false;
-	}
-
-	int retval = ::poll( pfd, nfds, timeout );
-	if ( retval < 0 )
-	{
-		// gdb at work, perhaps
-		if ( errno != EINTR )
-		{
-			error << string_compose(_("Mackie MIDI thread poll failed (%1)"), strerror( errno ) ) << endmsg;
-		}
-		return false;
-	}
-	
-	return retval > 0;
-}
-
 void MackieControlProtocol::handle_port_inactive( SurfacePort * port )
 {
 	// port gone away. So stop polling it ASAP
@@ -187,8 +50,6 @@ void MackieControlProtocol::handle_port_inactive( SurfacePort * port )
 			_ports.erase( it );
 		}
 	}
-	_ports_changed = true;
-	update_ports();
 
 	// TODO all the rebuilding of surfaces and so on
 }
@@ -219,8 +80,6 @@ void MackieControlProtocol::handle_port_init (Mackie::SurfacePort *)
 #ifdef DEBUG
 	cout << "MackieControlProtocol::handle_port_init" << endl;
 #endif
-	_ports_changed = true;
-	update_ports();
 #ifdef DEBUG
 	cout << "MackieControlProtocol::handle_port_init finish" << endl;
 #endif

libs/surfaces/mackie/mackie_port.cc

@@ -29,6 +29,8 @@
 
 #include "midi++/types.h"
 #include "midi++/port.h"
+
+#include "ardour/debug.h"
 #include "ardour/rc_configuration.h"
 
 #include "i18n.h"
@@ -37,6 +39,7 @@
 
 using namespace std;
 using namespace Mackie;
+using namespace ARDOUR;
 
 // The MCU sysex header
 MidiByteArray mackie_sysex_hdr ( 5, MIDI::sysex, 0x0, 0x0, 0x66, 0x10 );
@@ -51,20 +54,14 @@ MackiePort::MackiePort( MackieControlProtocol & mcp, MIDI::Port & port, int numb
   , _emulation( none )
   , _initialising( true )
 {
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl, "MackiePort::MackiePort\n");
-	cout << "MackiePort::MackiePort" <<endl;
-#endif
 }
 
 MackiePort::~MackiePort()
 {
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl, "MackiePort::~MackiePort\n");
-	cout << "~MackiePort" << endl;
-#endif
 	close();
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl, "~MackiePort finished\n");
-	cout << "~MackiePort finished" << endl;
-#endif
 }
 
 int MackiePort::strips() const
@@ -91,9 +88,8 @@ int MackiePort::strips() const
 // should really be in MackiePort
 void MackiePort::open()
 {
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl, string_compose ("MackiePort::open %1\n", *this));
-	cout << "MackiePort::open " << *this << endl;
+	
-#endif
 	port().input()->sysex.connect (sysex_connection, boost::bind (&MackiePort::handle_midi_sysex, this, _1, _2, _3));
 		     
 	// make sure the device is connected
@@ -102,18 +98,13 @@ void MackiePort::open()
 
 void MackiePort::close()
 {
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl, "MackiePort::close\n");
-	cout << "MackiePort::close" << endl;
-#endif
 	
 	// disconnect signals
 	any_connection.disconnect();
 	sysex_connection.disconnect();
 	
 	// TODO emit a "closing" signal?
-#ifdef PORT_DEBUG
-	cout << "MackiePort::close finished" << endl;
-#endif
 }
 
 const MidiByteArray & MackiePort::sysex_hdr() const
@@ -149,9 +140,7 @@ MidiByteArray calculate_challenge_response( MidiByteArray::iterator begin, MidiB
 MidiByteArray MackiePort::host_connection_query( MidiByteArray & bytes )
 {
 	// handle host connection query
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl, string_compose ("host connection query: %1\n", bytes));
-	cout << "host connection query: " << bytes << endl;
-#endif
 	
 	if ( bytes.size() != 18 )
 	{
@@ -172,9 +161,7 @@ MidiByteArray MackiePort::host_connection_query( MidiByteArray & bytes )
 // not used right now
 MidiByteArray MackiePort::host_connection_confirmation( const MidiByteArray & bytes )
 {
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl, string_compose ("host_connection_confirmation: %1\n", bytes));
-	cout << "host_connection_confirmation: " << bytes << endl;
-#endif
 	
 	// decode host connection confirmation
 	if ( bytes.size() != 14 )
@@ -213,15 +200,13 @@ void MackiePort::probe_emulation (const MidiByteArray &)
 
 void MackiePort::init()
 {
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl,  "MackiePort::init\n");
-	cout << "MackiePort::init" << endl;
+
-#endif
 	init_mutex.lock();
 	_initialising = true;
 
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl, "MackiePort::init lock acquired\n");
-	cout << "MackiePort::init lock acquired" << endl;
+
-#endif
 	// emit pre-init signal
 	init_event();
 	
@@ -237,9 +222,8 @@ void MackiePort::init()
 
 void MackiePort::finalise_init( bool yn )
 {
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl, "MackiePort::finalise_init\n");
-	cout << "MackiePort::finalise_init" << endl;
+
-#endif
 	bool emulation_ok = false;
 	
 	// probing doesn't work very well, so just use a config variable
@@ -271,19 +255,18 @@ void MackiePort::finalise_init( bool yn )
 	
 	SurfacePort::active( yn );
 
-	if ( yn )
+	if (yn) {
-	{
 		active_event();
 		
 		// start handling messages from controls
 		connect_any();
 	}
+
 	_initialising = false;
 	init_cond.signal();
 	init_mutex.unlock();
-#ifdef PORT_DEBUG
+
-	cout << "MackiePort::finalise_init lock released" << endl;
+	DEBUG_TRACE (DEBUG::MackieControl, "MackiePort::finalise_init lock released\n");
-#endif
 }
 
 void MackiePort::connect_any()
@@ -296,28 +279,20 @@ void MackiePort::connect_any()
 bool MackiePort::wait_for_init()
 {
 	Glib::Mutex::Lock lock( init_mutex );
-	while ( _initialising )
+	while (_initialising) {
-	{
+		DEBUG_TRACE (DEBUG::MackieControl, "MackiePort::wait_for_active waiting\n");
-#ifdef PORT_DEBUG
-		cout << "MackiePort::wait_for_active waiting" << endl;
-#endif
 		init_cond.wait( init_mutex );
-#ifdef PORT_DEBUG
+		DEBUG_TRACE (DEBUG::MackieControl, "MackiePort::wait_for_active released\n");
-		cout << "MackiePort::wait_for_active released" << endl;
-#endif
 	}
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl, "MackiePort::wait_for_active returning\n");
-	cout << "MackiePort::wait_for_active returning" << endl;
-#endif
 	return SurfacePort::active();
 }
 
 void MackiePort::handle_midi_sysex (MIDI::Parser &, MIDI::byte * raw_bytes, size_t count )
 {
 	MidiByteArray bytes( count, raw_bytes );
-#ifdef PORT_DEBUG
+	DEBUG_TRACE (DEBUG::MackieControl, string_compose ("handle_midi_sysex: %1\n", bytes));
-	cout << "handle_midi_sysex: " << bytes << endl;
+
-#endif
 	switch( bytes[5] )
 	{
 		case 0x01:
@@ -345,10 +320,11 @@ Control & MackiePort::lookup_control( MIDI::byte * bytes, size_t count )
 	// Don't instantiate a MidiByteArray here unless it's needed for exceptions.
 	// Reason being that this method is called for every single incoming
 	// midi event, and it needs to be as efficient as possible.
+
 	Control * control = 0;
 	MIDI::byte midi_type = bytes[0] & 0xf0; //0b11110000
-	switch( midi_type )
+
-	{
+	switch (midi_type) {
 		// fader
 		case MackieMidiBuilder::midi_fader_id:
 		{
@@ -413,18 +389,16 @@ bool MackiePort::handle_control_timeout_event ( Control * control )
 // because they have similar logic flows.
 void MackiePort::handle_midi_any (MIDI::Parser &, MIDI::byte * raw_bytes, size_t count )
 {
-#ifdef DEBUG
 	MidiByteArray bytes( count, raw_bytes );
-	cout << "MackiePort::handle_midi_any " << bytes << endl;
+	DEBUG_TRACE (DEBUG::MackieControl, string_compose ("MackiePort::handle_midi_any %1\n", bytes));
-#endif
+
 	try
 	{
 		// ignore sysex messages
 		if ( raw_bytes[0] == MIDI::sysex ) return;
 
 		// sanity checking
-		if ( count != 3 )
+		if (count != 3) {
-		{
 			ostringstream os;
 			MidiByteArray mba( count, raw_bytes );
 			os << "MackiePort::handle_midi_any needs 3 bytes, but received " << mba;
@@ -436,8 +410,7 @@ void MackiePort::handle_midi_any (MIDI::Parser &, MIDI::byte * raw_bytes, size_t
 		
 		// This handles incoming bytes. Outgoing bytes
 		// are sent by the signal handlers.
-		switch ( control.type() )
+		switch (control.type()) {
-		{
 			// fader
 			case Control::type_fader:
 			{
@@ -484,17 +457,15 @@ void MackiePort::handle_midi_any (MIDI::Parser &, MIDI::byte * raw_bytes, size_t
 				// first disconnect any previous timeouts
 				control.in_use_connection.disconnect();
 				
-#if 0 // BOOSTSIGNALS
 				// now connect a new timeout to call handle_control_timeout_event
-				sigc::slot<bool> timeout_slot = sigc::bind (
+				// XXX should this use the GUI event loop (default) or the
-					mem_fun( *this, &MackiePort::handle_control_timeout_event )
+				// MIDI UI event loop ?
-					, &control
+
-				);
+				sigc::slot<bool> timeout_slot = sigc::bind 
-				control.in_use_connection = Glib::signal_timeout().connect(
+					(sigc::mem_fun( *this, &MackiePort::handle_control_timeout_event), &control);
-					timeout_slot
+
-					, control.in_use_timeout()
+				control.in_use_connection = Glib::signal_timeout().connect (timeout_slot , control.in_use_timeout());
-				);
+
-#endif				
 				// emit the control event
 				control_event( *this, control, state );
 				break;
@@ -503,12 +474,11 @@ void MackiePort::handle_midi_any (MIDI::Parser &, MIDI::byte * raw_bytes, size_t
 				cerr << "Do not understand control type " << control;
 		}
 	}
-	catch( MackieControlException & e )
+
-	{
+	catch( MackieControlException & e ) {
 		MidiByteArray bytes( count, raw_bytes );
 		cout << bytes << ' ' << e.what() << endl;
 	}
-#ifdef DEBUG
+
-	cout << "finished MackiePort::handle_midi_any " << bytes << endl;
+	DEBUG_TRACE (DEBUG::MackieControl, string_compose ("finished MackiePort::handle_midi_any %1\n", bytes));
-#endif
 }