Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Develop #137

Merged
merged 22 commits into from
Apr 26, 2016
Merged

Develop #137

merged 22 commits into from
Apr 26, 2016

Conversation

kaaholst
Copy link
Collaborator

@kaaholst kaaholst commented Aug 2, 2015

Implement "Play from here"

nikclayton and others added 22 commits May 25, 2015 19:38
NOTE
Only English and Danish language, other translations are missing.
TODO
This is dependant on the albumtrack sort order for the songs query,
which is available from server version 7.6

NOTE
Only English, German, Dutch and Danish language, other translations
are missing.

Signed-off-by: Kurt Aaholst <kaaholst@gmail.com>
Play from here for artists, genres and years, requires title sort
order "albumtrack", which is available from server version 7.6.

Sort order "albumtrack"  and hence play from here are disabled for
earlier server versions.
When showing all titles of a item which can be played, insert the name
of the item in the play and queue menu options.

Signed-off-by: Kurt Aaholst <kaaholst@gmail.com>
DOWNLOAD_ITEM -> DOWNLOAD
PLAYLIST_REMOVE_ITEM -> PLAYLIST_REMOVE

So now the ITEM part indicates that the string takes a item name argument.
We receive alarm playlist in AlarmActivity on a background thread. This updates
mAlarmPlaylists which is passed to and used in AlarmView.setAlarmPlaylists on
the UI thread.

This causes for a possible concurrency problem, so now mAlarmView is both
updated and passed to setAlarmPlaylists on the UI thread.

This fixes:
https://fabric.io/squeezer/android/apps/uk.org.ngo.squeezer/issues/55c6c8e5e0d514e5d6daa019
notification preference

Signed-off-by: Kurt Aaholst <kaaholst@gmail.com>
If a configurable action for song or album is not set. or is NONE, a
popup with the possible actions, and "Just once" and "Always" options,
similar to the standard Android way of resolving intents.

This requires Dialog.setOnShowListener so it is for API level 8 (froyo)
or later. API level 7 still have to select the default action via
settings.
and move them to uk.org.squeezer.framework
and move it to uk.org.squeezer.framework
in PlayableItemAction.Type
and ThemeManager.Theme
The song and album search results are displayed in a list or a grid
as selected in the song album lists respectively.
This version fixes a NoClassDefFoundError, thus eliminates the need to
reference a special fork of EventBus, and thereby also removes an Android
Studio problem, as it seems the EventBus project does not work with
Android Studio/Intellij.
move the test from src/androidTest folder to src/test.
@Override
public void run() {
mAlarmPlaylists.addAll(items);
if (start + items.size() >= count) {
mAlarmView.setAlarmPlaylists(mAlarmPlaylists);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge as is for the moment, but what if just this line was changed to be:

mAlarmView.setAlarmPlaylists(ImmutableList.copyOf(mAlarmPlaylists);

instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems like a good idea.

Note that the semantics is slightly different, as the UI is not updated until we have received all the items, which also is how it worked originally.

One drawback is that copy of the list is thrown away when AlarmView is done with it, even more so as all AlarmView does with it, is add a few category items to it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the semantics are the same the UI isn't updated until all items are received.

@nikclayton nikclayton merged commit b8aac23 into nikclayton:develop Apr 26, 2016
@@ -39,7 +39,7 @@ dependencies {
compile 'com.google.code.findbugs:jsr305:2.0.2'

// EventBus, https://github.com/greenrobot/EventBus.
compile project(':libs:eventbus:EventBus')
compile 'de.greenrobot:eventbus:2.4.1'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know that 2.4.1 actually fixes this.

Ahead of releasing 1.4.1 I've been launching Squeezer on some emulators running API 7, and they crash with

05-15 18:27:25.134 3064-3064/uk.org.ngo.squeezer W/System.err: java.lang.NoSuchMethodException
05-15 18:27:25.144 3064-3064/uk.org.ngo.squeezer W/System.err: at java.lang.Class.getDeclaredMethods(Native Method)
05-15 18:27:25.154 3064-3064/uk.org.ngo.squeezer W/System.err: at java.lang.ClassCache.getDeclaredMethods(ClassCache.java:153)
05-15 18:27:25.154 3064-3064/uk.org.ngo.squeezer W/System.err: at java.lang.Class.getDeclaredMethods(Class.java:792)
05-15 18:27:25.154 3064-3064/uk.org.ngo.squeezer W/System.err: at a.b.a.o.a(SubscriberMethodFinder.java:76)
05-15 18:27:25.165 3064-3064/uk.org.ngo.squeezer W/System.err: at a.b.a.c.register(EventBus.java:164)
05-15 18:27:25.165 3064-3064/uk.org.ngo.squeezer W/System.err: at a.b.a.c.register(EventBus.java:144)
05-15 18:27:25.165 3064-3064/uk.org.ngo.squeezer W/System.err: at uk.org.ngo.squeezer.service.SqueezeService$EventBus.register(SqueezeService.java:1691)
05-15 18:27:25.165 3064-3064/uk.org.ngo.squeezer W/System.err: at uk.org.ngo.squeezer.service.SqueezeService.onCreate(SqueezeService.java:220)
05-15 18:27:25.185 3064-3064/uk.org.ngo.squeezer W/System.err: at android.app.ActivityThread.handleCreateService(ActivityThread.java:2780)
05-15 18:27:25.185 3064-3064/uk.org.ngo.squeezer W/System.err: at android.app.ActivityThread.access$3200(ActivityThread.java:119)
05-15 18:27:25.185 3064-3064/uk.org.ngo.squeezer W/System.err: at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1917)
05-15 18:27:25.185 3064-3064/uk.org.ngo.squeezer W/System.err: at android.os.Handler.dispatchMessage(Handler.java:99)
05-15 18:27:25.185 3064-3064/uk.org.ngo.squeezer W/System.err: at android.os.Looper.loop(Looper.java:123)
05-15 18:27:25.195 3064-3064/uk.org.ngo.squeezer W/System.err: at android.app.ActivityThread.main(ActivityThread.java:4363)
05-15 18:27:25.195 3064-3064/uk.org.ngo.squeezer W/System.err: at java.lang.reflect.Method.invokeNative(Native Method)
05-15 18:27:25.195 3064-3064/uk.org.ngo.squeezer W/System.err: at java.lang.reflect.Method.invoke(Method.java:521)
05-15 18:27:25.195 3064-3064/uk.org.ngo.squeezer W/System.err: at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:860)
05-15 18:27:25.205 3064-3064/uk.org.ngo.squeezer W/System.err: at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:618)
05-15 18:27:25.205 3064-3064/uk.org.ngo.squeezer W/System.err: at dalvik.system.NativeStart.main(Native Method)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just did a few more tests based on the faq mentioned in issue 149 .

The exception is thrown if the class has methods with a parameter that is unavailable to the API level of the device.

When I launch squeezer on an API 7 (Android 2.1) emulator I get a crash on line 81 of SubscriberMethodFinder.java, which makes sense as this is the fallback ("plan b") applied by EventBus when getDeclaredMethods fails.

I then comment out the Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP section of updateOngoingNotification and the crash disappears.

I suspect (but I'm guessing here as I haven't done the test) that the crash would happen also with the EventBus fork pulled in and on EventBus 3.0 (unless we use an index).

@kaaholst
Copy link
Collaborator Author

I think it should fix it.

EventBus issue 149 references a commit, which implement the recommended workaround by @yuzeh, who's fork we previously referenced in Squeezer.

I haven't been able to trigger the problem though, so I don't know if 2.4.1 actually fixes it.

I don't understand the supplied stacktrace though. When I open SubscriberMethodFinder.java from eventbus-2.4.1-sources.jar, line 76 is: Method[] methods = clazz.getDeclaredMethods();, and this line is protected by a try-catch (Throwable) and falls back to getMethods(). So how can it throw a NoSuchMethodException?

@nikclayton
Copy link
Owner

I've got a nagging feeling this was caused by the Instant Run feature in newer versions of Android Studio (where it hot-patches new code in to the app instead of deploying a fresh APK). I turned that off, and can't reproduce.

1.4.1-beta contains 2.4.1 now (no crashes in Crashlytics that I can see), and the eventbus3 branch has 15e08b0 which is all the work (I think) necessary to use eventbus v3. I'll merge that in after 1.4.1 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants