Merge lp:~xerosis/nuvola-player/squeezebox-integration into lp:nuvola-player/releases-1.x

[Kieran Hogg]

Thanks for the review, javascript isn't my strongest language as you can tell...

[Jiří Janoušek (fenryxo)]

Here is a review:

----8<----

6 +<p>The <strong>Squeezebox</strong> is a network music player from Logitech. It plays internet radio or digital ...

This line is too long, wrap it to 80 characters.

----8<----

40 + * @param Nuvola Nuvola JS API

This line can be removed (function does not have any arguments)

----8<----

60 + var state = Nuvola.STATE_PAUSED;

Default state should be Nuvola.STATE_NONE

----8<----

72 + var album_art = document.getElementById('ctrlCurrentArt').firstChild.firstChild.src;
73 + var artist = document.getElementById('ctrlCurrentArtist').lastChild.innerText;
74 + var song = document.getElementById('ctrlCurrentTitle').firstChild.innerText;
75 + // Squeezebox prepends the song number to the song name, remove this
76 + var song = song.split(/\.(.+)?/)[1].trim();
77 + var album = document.getElementById('ctrlCurrentAlbum').firstChild.innerText;

No need to use "var" since all variables are already declared at the beginning of the function.

----8<----

83 + var play_pause = document.getElementById('ext-gen42');
84 + var pp_pressed = play_pause.getAttribute("title");
85 + var state;
86 + if(pp_pressed == "Pause"){
87 + state = Nuvola.STATE_PLAYING;
88 + }
89 + else{
90 + state = Nuvola.STATE_PAUSED;
91 + }

If play_pause is null, line 84 raises exception. This is better:

var play_pause = document.getElementById('ext-gen42');
try{
   var pp_pressed = play_pause.getAttribute("title");
   state = (pp_pressed == "Pause") ? Nuvola.STATE_PLAYING : Nuvola.STATE_PAUSED;
}
catch(e){
// state stays Nuvola.STATE_NONE
}

review: Needs Fixing

[Jiří Janoušek (fenryxo)]

Code is now OK, but it can't be still merged because Nuvola Player is not ready for this kind of services. Currently selecting the Squeezebox service without Squeezebox installed or running on a different address would lead to error (server not found) and user might be confused. It is necessary to create local HTML page that would the tell user that this integration requires his own instance of Squezebox and provide a form to set up the address to that instance. I've subscribed you to the bug #1011097 so you will be notified about progress of a new API. When it will be ready I will help you to finish integration of Squeezebox.

Could you please join a team https://launchpad.net/~nuvola-player-devel and push your branch to lp:~nuvola-player-devel/nuvola-player/squeezebox (shared location) to make collaboration easier?

review: Disapprove

Unmerged revisions

233. By Kieran Hogg on 2012-06-10

Fixed issues from review

232. By Kieran Hogg on 2012-06-10

Add initial Squeezebox service

=== added directory 'data/nuvolaplayer/services/squeezebox'
=== added file 'data/nuvolaplayer/services/squeezebox/description.html'
--- data/nuvolaplayer/services/squeezebox/description.html	1970-01-01 00:00:00 +0000
+++ data/nuvolaplayer/services/squeezebox/description.html	2012-06-10 14:36:26 +0000
@@ -0,0 +1,12 @@
+<p>
+The <strong>Squeezebox</strong> is a network music player from Logitech. It 
+plays internet radio or digital audio streamed to it across a home network. It 
+also streams a variety of popular music services, such as Pandora, Slacker, 
+Napster, SomaFM, Mediafly, Live365, Rhapsody, Last.fm, Spotify and Sirius 
+among others.
+</p>
+<p>
+<em>Source:
+<a href="http://en.wikipedia.org/wiki/Squeezebox_(network_music_player)">
+Squeezebox on Wikipedia</a></em>
+</p>

=== added file 'data/nuvolaplayer/services/squeezebox/integration.js'
--- data/nuvolaplayer/services/squeezebox/integration.js	1970-01-01 00:00:00 +0000
+++ data/nuvolaplayer/services/squeezebox/integration.js	2012-06-10 14:36:26 +0000
@@ -0,0 +1,122 @@
+/*
+	Nuvola Player :: Cloud music integration
+	
+	This script provides integration of the Logitech Squeezebox streaming service
+
+
+	Copyright 2012 Kieran Hogg <[e-mail]>
+
+This program is free software: you can redistribute it and/or modify it 
+under the terms of the GNU General Public License version 3, as published 
+by the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful, but 
+WITHOUT ANY WARRANTY; without even the implied warranties of 
+MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR 
+PURPOSE.  See the GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License along 
+with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+/* Anonymous function is used not to pollute environment */
+(function(Nuvola){
+	/**
+	 * Creates Squeezebox integration binded to Nuvola JS API
+	 */
+	var Integration = function(){
+		/* Overwrite default commnad function */
+		Nuvola.command = Nuvola.bind(this, this.command);
+		
+		/* For debug output */
+		this.name = "Squeezebox";		
+		
+		/* Let's run */
+		this.state = Nuvola.STATE_NONE;
+		this.update();
+		this.timeout = setInterval(Nuvola.bind(this, this.update), 500);
+	};
+	
+	/**
+	 * Updates current playback state
+	 */
+	Integration.prototype.update = function(){
+		// Default state
+		var state = Nuvola.STATE_NONE;
+		var album = null;
+		var album_art =  null;
+		var artist = null;
+		var song = null;
+		var can_prev = true;
+		var can_next = true;
+		var can_thumbs_up = false;
+		var can_thumbs_down = false;
+		var can_favorite = false;
+	 		
+		try{
+			album_art = document.getElementById('ctrlCurrentArt').firstChild.firstChild.src;
+			artist = document.getElementById('ctrlCurrentArtist').lastChild.innerText;
+			song = document.getElementById('ctrlCurrentTitle').firstChild.innerText;
+			// Squeezebox prepends the song number to the song name, remove this
+			song = song.split(/\.(.+)?/)[1].trim();
+			album = document.getElementById('ctrlCurrentAlbum').firstChild.innerText;
+		}
+		catch(e){
+			console.debug("Unable to obtain song info: " + e.message);
+		}
+		
+		var play_pause =  document.getElementById('ext-gen42');
+		try {
+                    var pp_pressed = play_pause.getAttribute("title");
+                    state = (pp_pressed == "Pause") ? Nuvola.STATE_PLAYING : Nuvola.STATE_PAUSED;
+                }
+		catch (e) {
+                    //state stays Nuvola.STATE_NONE
+		
+		this.state = state;
+		// Submit data to Nuvola backend
+		Nuvola.dataChanged(
+			song, artist, album, album_art,
+			state, can_prev, can_next,
+			can_thumbs_up, can_thumbs_down, can_favorite);
+	}
+	
+	/**
+	 * Command handler
+	 * @param cmd command to execute
+	 */
+	Integration.prototype.command = function(cmd){
+		try{
+			switch(cmd){
+			case Nuvola.CMD_PLAY:
+				if(this.state != Nuvola.STATE_PLAYING) document.getElementById('ext-gen42').click();
+				break;
+			case Nuvola.CMD_PAUSE:
+				document.getElementById('ext-gen42').click();
+				break;
+			case Nuvola.CMD_TOGGLE:
+				document.getElementById('ext-gen42').click();
+				break;
+			case Nuvola.CMD_PREV_SONG:
+				document.getElementById('ext-gen40').click();
+				break;
+			case Nuvola.CMD_NEXT_SONG:
+				document.getElementById('ext-gen44').click();
+				break;
+			default:
+				// Other commands are not supported
+				throw {"message": "Not supported."};
+			}
+			console.log(this.name + ": comand '" + cmd + "' executed.");
+		}
+		catch(e){
+			// API expects exception to be a string!
+			throw (this.name + ": " + e.message);
+		}
+	}
+
+	/* Store reference */ 
+	Nuvola.integration = new Integration(); // Singleton
+	
+// Call anonymous function with Nuvola object as an argument
+})(window._Nuvola);

=== added file 'data/nuvolaplayer/services/squeezebox/metadata.conf'
--- data/nuvolaplayer/services/squeezebox/metadata.conf	1970-01-01 00:00:00 +0000
+++ data/nuvolaplayer/services/squeezebox/metadata.conf	2012-06-10 14:36:26 +0000
@@ -0,0 +1,6 @@
+name = Squeezebox
+home_page = http://localhost:9000
+sandbox_pattern = https?://(localhost:9000)
+maintainer_name = Kieran Hogg
+maintainer_link = https://launchpad.net/~xerosis
+version = 1