Highlighted the important user messages (BANNED, BADAUTH, BADTIME) wi…

…th color. Stopped the scrobbling thread from calling stopIfIdle() if there was a failure, because that call would have caused infinite loops (and a failure implies that we will definitely not be idle). Refactored the code that reconfigures the UI in response to notifications from the service. Updated app version number to release value. Added a TODO item. Removed a TODO item that I've decided not to do.
JJC1138 · Nov 25, 2008 · 8c75316 · 8c75316
1 parent 79cf483
commit 8c75316
Show file tree

Hide file tree

Showing 7 changed files with 103 additions and 32 deletions.
diff --git a/AndroidManifest.xml b/AndroidManifest.xml
@@ -2,7 +2,7 @@
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
 	package="net.jjc1138.android.scrobbler"
 	android:versionCode="1"
-	android:versionName="0.9">
+	android:versionName="1.0">
 	<permission android:name="net.jjc1138.android.scrobbler.privateservices"
 		android:protectionLevel="signature" />
 	<uses-permission android:name="net.jjc1138.android.scrobbler.privateservices" />

diff --git a/TODO.txt b/TODO.txt
@@ -1,7 +1,5 @@
  Pre-release:
-Update checking.
 Make an icon.
-Highlight the important user messages (BANNED, BADAUTH, BADTIME) with color.
 Add notifications for important handshake failures that require user action (BANNED, BADAUTH, BADTIME).
 
  Would be nice:
@@ -12,3 +10,4 @@ Replace magic strings with constants so that they can be refactored easily.
 Make it so that when scrobbling is disabled we still keep track of what's going, but just don't enqueue.
 Check UI with QVGA resolution.
 Support for the getting metadata automatically from the device's Video MediaStore.
+If we've received a BADTIME response then disable scrobbling and re-enable it after the time has been updated.
diff --git a/res/layout/main.xml b/res/layout/main.xml
@@ -91,6 +91,20 @@
 	style="?android:listSeparatorTextViewStyle"
 	android:layout_marginTop="15sp"
 	/>
+<TextView android:id="@+id/scrobble_user_error"
+	android:layout_width="wrap_content"
+	android:layout_height="wrap_content"
+	android:textColor="#f00"
+	android:textSize="16sp"
+	android:visibility="gone"
+	/>
+<TextView android:id="@+id/update_link"
+	android:layout_width="wrap_content"
+	android:layout_height="wrap_content"
+	android:text="@string/update_link"
+	style="@style/webLink"
+	android:visibility="gone"
+	/>
 <TextView android:id="@+id/queue_status"
 	android:layout_width="wrap_content"
 	android:layout_height="wrap_content"

diff --git a/res/values/strings.xml b/res/values/strings.xml
@@ -21,6 +21,7 @@
 <string name="scrobbling_in_progress">Scrobbling is in progress...</string>
 <string name="scrobbling_ok">The last scrobble was successful.</string>
 <string name="scrobbling_banned">This version of this application is no longer allowed to scrobble. Please try upgrading to a newer version if one is available.</string>
+<string name="update_link">Check for new version</string>
 <string name="scrobbling_badauth">Scrobbling failed because the username or password is incorrect. Please check them.</string>
 <string name="scrobbling_badtime">Scrobbling failed because the time or the timezone on this device appears to be incorrect. Please check them.</string>
 <string name="scrobbling_failed_net">Scrobbling failed because of a network error.</string>

diff --git a/res/values/styles.xml b/res/values/styles.xml
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <resources>
 <style name="webLink">
-	<item name="android:focusable">true</item>item>
+	<item name="android:focusable">true</item>
 	<item name="android:textColor">@color/link</item>
 	<!-- This is partly for legibility against the black background, and partly to make it easier to tap with a finger: -->
 	<item name="android:textSize">18sp</item>

diff --git a/src/net/jjc1138/android/scrobbler/ScrobblerConfig.java b/src/net/jjc1138/android/scrobbler/ScrobblerConfig.java
@@ -40,6 +40,8 @@ public class ScrobblerConfig extends Activity {
 	private TextView view_user_page;
 
 	private LinearLayout settingsChanged;
+	private TextView scrobble_user_error;
+	private TextView update_link;
 	private TextView queue_status;
 	private TextView scrobble_when;
 	private Button scrobble_now;
@@ -49,6 +51,14 @@ public class ScrobblerConfig extends Activity {
 
 	private final Handler handler = new Handler();
 
+	private static void show(View v) {
+		v.setVisibility(View.VISIBLE);
+	}
+
+	private static void hide(View v) {
+		v.setVisibility(View.GONE);
+	}
+
 	private final IScrobblerServiceNotificationHandler.Stub notifier =
 		new IScrobblerServiceNotificationHandler.Stub() {
 			@Override
@@ -64,41 +74,72 @@ public void run() {
 							new ChoiceFormat(getString(R.string.tracks_ready))
 								.format(queueSize), queueSize));
 
-						// TODOLATER BADTIME should also prevent further
-						// handshakes according to the spec., but we don't yet
-						// have a way of resetting from BADTIME when the time is
-						// updated.
-						scrobble_now.setVisibility(
-							(!scrobbling && queueSize > 0 &&
-								lastScrobbleResult != ScrobblerService.BANNED &&
-								lastScrobbleResult != ScrobblerService.BADAUTH)
-								? View.VISIBLE : View.GONE);
+						// This is perhaps a tad verbose, but at least it's easy
+						// to read:
 
-						scrobble_status.setVisibility(
-							(!scrobbling && lastScrobbleResult == 
-								ScrobblerService.NOT_YET_ATTEMPTED) ?
-
-							View.GONE : View.VISIBLE);
+						if (queueSize > 0) {
+							show(scrobble_now);
+						} else {
+							hide(scrobble_now);
+						}
 
 						if (scrobbling) {
 							scrobble_status.setText(
 								getString(R.string.scrobbling_in_progress));
+							hide(scrobble_user_error);
+							hide(update_link);
+							hide(scrobble_now);
+							show(scrobble_status);
+						} else if (lastScrobbleResult ==
+							ScrobblerService.NOT_YET_ATTEMPTED) {
+
+							hide(scrobble_user_error);
+							hide(update_link);
+							hide(scrobble_status);
 						} else {
+							// TODOLATER BADTIME should also prevent further
+							// handshakes according to the spec., but we don't
+							// yet have a way of resetting from BADTIME when the
+							// time is updated.
+							if (lastScrobbleResult == ScrobblerService.BANNED ||
+								lastScrobbleResult == ScrobblerService.BADAUTH)
+							{
+								hide(scrobble_now);
+							}
+							if (lastScrobbleResult ==
+									ScrobblerService.BANNED ||
+								lastScrobbleResult ==
+									ScrobblerService.BADAUTH ||
+								lastScrobbleResult ==
+									ScrobblerService.BADTIME) {
+
+								show(scrobble_user_error);
+								hide(scrobble_status);
+							} else {
+								hide(scrobble_user_error);
+								show(scrobble_status);
+							}
+							if (lastScrobbleResult == ScrobblerService.BANNED) {
+								show(update_link);
+							} else {
+								hide(update_link);
+							}
+
 							switch (lastScrobbleResult) {
 							case ScrobblerService.OK:
 								scrobble_status.setText(getString(
 									R.string.scrobbling_ok));
 								break;
 							case ScrobblerService.BANNED:
-								scrobble_status.setText(getString(
+								scrobble_user_error.setText(getString(
 									R.string.scrobbling_banned));
 								break;
 							case ScrobblerService.BADAUTH:
-								scrobble_status.setText(getString(
+								scrobble_user_error.setText(getString(
 									R.string.scrobbling_badauth));
 								break;
 							case ScrobblerService.BADTIME:
-								scrobble_status.setText(getString(
+								scrobble_user_error.setText(getString(
 									R.string.scrobbling_badtime));
 								break;
 							case ScrobblerService.FAILED_NET:
@@ -159,15 +200,19 @@ public void onTextChanged(CharSequence s, int start, int before,
 		view_user_page = (TextView) findViewById(R.id.view_user_page);
 
 		settingsChanged = (LinearLayout) findViewById(R.id.settings_changed);
+		scrobble_user_error = (TextView) findViewById(R.id.scrobble_user_error);
+		update_link = (TextView) findViewById(R.id.update_link);
 		queue_status = (TextView) findViewById(R.id.queue_status);
 		scrobble_when = (TextView) findViewById(R.id.scrobble_when);
 		scrobble_now = (Button) findViewById(R.id.scrobble_now);
 		scrobble_status = (TextView) findViewById(R.id.scrobble_status);
 
-		Spannable text = (Spannable) sign_up.getText();
-		text.setSpan(new UnderlineSpan(), 0, text.length(), 0);
-		text = (Spannable) view_user_page.getText();
-		text.setSpan(new UnderlineSpan(), 0, text.length(), 0);
+		for (TextView tv : new TextView[] {
+			sign_up, view_user_page, update_link
+		}) {
+			Spannable text = (Spannable) tv.getText();
+			text.setSpan(new UnderlineSpan(), 0, text.length(), 0);
+		}
 
 		scrobbleWaiting =
 			MessageFormat.format(getString(R.string.scrobble_when),
@@ -223,6 +268,14 @@ public void onClick(View v) {
 						URLEncoder.encode(username.getText().toString())))));
 			}
 		});
+		update_link.setOnClickListener(new View.OnClickListener() {
+			@Override
+			public void onClick(View v) {
+				startActivity((new Intent(Intent.ACTION_VIEW,
+					Uri.parse("market://search?q=pname:" +
+						URLEncoder.encode(getPackageName())))));
+			}
+		});
 	}
 
 	protected void settingsChanged() {

diff --git a/src/net/jjc1138/android/scrobbler/ScrobblerService.java b/src/net/jjc1138/android/scrobbler/ScrobblerService.java
@@ -1013,8 +1013,10 @@ private void submit() throws IOException {
 
 		@Override
 		public void run() {
+			Log.v(LOG_TAG, "Scrobbling process started.");
 			if (prefs.getString("username", "").length() == 0) {
 				inProgress = false;
+				Log.v(LOG_TAG, "Cannot scrobble because there is no username.");
 				updateAllClients();
 				return;
 			}
@@ -1025,6 +1027,8 @@ public void run() {
 				// changed, but we'll need to monitor for time changes to
 				// implement that.
 				inProgress = false;
+				Log.v(LOG_TAG,
+					"Refusing to scrobble because of an uncorrected error.");
 				updateAllClients();
 				return;
 			}
@@ -1048,6 +1052,13 @@ public void run() {
 
 				inProgress = false;
 				lastScrobbleResult = OK;
+
+				handler.post(new Runnable() {
+					@Override
+					public void run() {
+						stopIfIdle();
+					}
+				});
 			} catch (IOException e) {
 				Runnable retry = new Runnable() {
 					@Override
@@ -1084,13 +1095,6 @@ public void run() {
 			}
 
 			updateAllClients();
-
-			handler.post(new Runnable() {
-				@Override
-				public void run() {
-					stopIfIdle();
-				}
-			});
 		}
 
 		public boolean inProgress() {