From 7dfd982643e16cc3c1f9b9f1531a5587faeb7d06 Mon Sep 17 00:00:00 2001 From: Synced Synapse Date: Tue, 28 May 2019 19:44:02 +0100 Subject: [PATCH] Tweak connection threads This PR fixes some issues with connections and threading. Specifically, the change in #618 introduced threading in `HostConnection`, which had some issues. To fix them, the following changes were made: - A specific TCP listener thread is used and manually started, instead of using one of the threads in the pool. The TCP listener thread is a long lived thread that should always be running (as long as the connection is through TCP), blocked listening on the TCP socket, so it shouldn't be managed in a pool, where, theoretically, it can be paused and reused. - Changed the number of threads to 5. We shouldn't need more than this, otherwise we can overwhelm some Kodi hardware. - Had to sprinkle some `synchronized` to avoid race conditions. For instance, through a TCP connection, as soon as Kore is opened (on the remote screen) it will call at least `GetActivePlayers`, `GetProperties`, `Ping`. If the TCP socket isn't set up yet, each of these calls would create a socket (and a TCP listener thread), so we would open 3 or more sockets when we should just open 1. A `synchronized` in `executeThroughTcp` prevents this. The others prevent similar issues. Aditionally: - Tweaked the playlist fetching code, so that it happens exclusively in `HostConnectionObserver` and when PlaylistFragment is notified of changes, it already gets the playlist data. This somewhat simplifies the code, and makes it more consistent with the Player Observer code; - Change `EventServerConnection` to accept a Handler on which to post the result of the connection, so that the caller can control on which thread the result is called; - Calls to the various `RegisterObserver` loose the reply immediately parameter, as it was always true. --- .../eventclient/EventServerConnection.java | 24 ++- .../kore/host/HostConnectionObserver.java | 183 ++++++++++-------- .../java/org/xbmc/kore/host/HostManager.java | 10 +- .../xbmc/kore/host/actions/GetPlaylist.java | 10 +- .../org/xbmc/kore/jsonrpc/HostConnection.java | 52 ++--- .../ConnectionObserversManagerService.java | 4 +- .../org/xbmc/kore/ui/BaseMediaActivity.java | 6 +- ...olumeControllerDialogFragmentListener.java | 4 +- .../sections/remote/NowPlayingFragment.java | 8 +- .../ui/sections/remote/PlaylistFragment.java | 29 +-- .../ui/sections/remote/RemoteActivity.java | 7 +- .../ui/sections/remote/RemoteFragment.java | 4 +- 12 files changed, 190 insertions(+), 151 deletions(-) diff --git a/app/src/main/java/org/xbmc/kore/eventclient/EventServerConnection.java b/app/src/main/java/org/xbmc/kore/eventclient/EventServerConnection.java index 71ba9d3..74bb1c6 100644 --- a/app/src/main/java/org/xbmc/kore/eventclient/EventServerConnection.java +++ b/app/src/main/java/org/xbmc/kore/eventclient/EventServerConnection.java @@ -81,13 +81,17 @@ public class EventServerConnection { /** * Constructor. Starts the thread that keeps the connection alive. Make sure to call quit() when done. * @param hostInfo Host to connect to + * @param callback Callback to call with the connection result + * @param callbackHandler Handler on which to call the callback */ - public EventServerConnection(final HostInfo hostInfo, final EventServerConnectionCallback callback) { + public EventServerConnection(final HostInfo hostInfo, + final EventServerConnectionCallback callback, + final Handler callbackHandler) { this.hostInfo = hostInfo; LogUtils.LOGD(TAG, "Starting EventServer Thread"); // Handler thread that will keep pinging and send the requests to Kodi - handlerThread = new HandlerThread("EventServerConnection", Process.THREAD_PRIORITY_DEFAULT); + handlerThread = new HandlerThread("EventServerConnection", Process.THREAD_PRIORITY_BACKGROUND); handlerThread.start(); // Get the HandlerThread's Looper and use it for our Handler @@ -103,12 +107,22 @@ public class EventServerConnection { LogUtils.LOGD(TAG, "Got an UnknownHostException, disabling EventServer"); hostInetAddress = null; } - callback.OnConnectResult(hostInetAddress != null); + // Call the callback on the caller's thread + callbackHandler.post(new Runnable() { + @Override + public void run() { + callback.OnConnectResult(hostInetAddress != null); + } + }); + if (hostInetAddress != null) { + // Start pinging + commHandler.postDelayed(pingRunnable, PING_INTERVAL); + } else { + quitHandlerThread(handlerThread); + } } }); - // Start pinging - commHandler.postDelayed(pingRunnable, PING_INTERVAL); } diff --git a/app/src/main/java/org/xbmc/kore/host/HostConnectionObserver.java b/app/src/main/java/org/xbmc/kore/host/HostConnectionObserver.java index ec396d0..e0c3a75 100644 --- a/app/src/main/java/org/xbmc/kore/host/HostConnectionObserver.java +++ b/app/src/main/java/org/xbmc/kore/host/HostConnectionObserver.java @@ -57,17 +57,22 @@ public class HostConnectionObserver public interface PlaylistEventsObserver { /** + * Notifies that a playlist has been cleared * @param playlistId of playlist that has been cleared */ void playlistOnClear(int playlistId); - void playlistChanged(int playlistId); - /** - * @param playlists the available playlists on the server + * Notifies about the available playlists on Kodi + * @param playlists Available playlists */ void playlistsAvailable(ArrayList playlists); + /** + * Notifies that an error occured when fetching playlists + * @param errorCode Error code + * @param description Error description + */ void playlistOnError(int errorCode, String description); } @@ -164,8 +169,11 @@ public class HostConnectionObserver private List applicationEventsObservers = new ArrayList<>(); private List playlistEventsObservers = new ArrayList<>(); + // This controls the frequency with wich the playlist is checked. + // It's checked everytime it reaches 0, being reset afterwards + private int checkPlaylistFrequencyCounter = 0; + // Associate the Handler with the UI thread - int checkPlaylistCounter = 0; private Handler checkerHandler = new Handler(Looper.getMainLooper()); private Runnable httpCheckerRunnable = new Runnable() { @Override @@ -183,11 +191,15 @@ public class HostConnectionObserver if (!applicationEventsObservers.isEmpty()) getApplicationProperties(); - if (!playlistEventsObservers.isEmpty() && checkPlaylistCounter > 1) { - checkPlaylist(); - checkPlaylistCounter = 0; + if (!playlistEventsObservers.isEmpty()) { + if (checkPlaylistFrequencyCounter <= 0) { + // Check playlist and reset the frequency counter + checkPlaylist(); + checkPlaylistFrequencyCounter = 1; + } else { + checkPlaylistFrequencyCounter--; + } } - checkPlaylistCounter++; checkerHandler.postDelayed(this, HTTP_NOTIFICATION_CHECK_INTERVAL); } @@ -234,26 +246,18 @@ public class HostConnectionObserver } }; - public class HostState { - private int lastCallResult = PlayerEventsObserver.PLAYER_NO_RESULT; - private PlayerType.GetActivePlayersReturnType lastGetActivePlayerResult = null; - private PlayerType.PropertyValue lastGetPropertiesResult = null; - private ListType.ItemsAll lastGetItemResult = null; - private boolean volumeMuted = false; - private int volumeLevel = -1; // -1 indicates no volumeLevel known - private int lastErrorCode; - private String lastErrorDescription; - - public int getVolumeLevel() { - return volumeLevel; - } - - public boolean isVolumeMuted() { - return volumeMuted; - } + private static class HostState { + int lastCallResult = PlayerEventsObserver.PLAYER_NO_RESULT; + PlayerType.GetActivePlayersReturnType lastGetActivePlayerResult = null; + PlayerType.PropertyValue lastGetPropertiesResult = null; + ListType.ItemsAll lastGetItemResult = null; + boolean volumeMuted = false; + int volumeLevel = -1; // -1 indicates no volumeLevel known + int lastErrorCode; + String lastErrorDescription; + ArrayList lastGetPlaylistResults = null; } - - public HostState hostState; + private HostState hostState; public HostConnectionObserver(HostConnection connection) { this.hostState = new HostState(); @@ -264,14 +268,15 @@ public class HostConnectionObserver * Registers a new observer that will be notified about player events * @param observer Observer */ - public void registerPlayerObserver(PlayerEventsObserver observer, boolean replyImmediately) { + public void registerPlayerObserver(PlayerEventsObserver observer) { if (this.connection == null) return; if (!playerEventsObservers.contains(observer)) playerEventsObservers.add(observer); - if (replyImmediately) replyWithLastResult(observer); + // Reply immediatelly + replyWithLastResult(observer); if (playerEventsObservers.size() == 1) { // If this is the first observer, start checking through HTTP or register us @@ -297,8 +302,7 @@ public class HostConnectionObserver " observers."); if (playerEventsObservers.isEmpty()) { - // No more observers, so unregister us from the host connection, or stop - // the http checker thread + // No more observers. If through TCP unregister us from the host connection if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) { connection.unregisterPlayerNotificationsObserver(this); connection.unregisterSystemNotificationsObserver(this); @@ -311,22 +315,16 @@ public class HostConnectionObserver /** * Registers a new observer that will be notified about application events * @param observer Observer - * @param replyImmediately Wether to immediatlely issue a reply with the current status */ - public void registerApplicationObserver(ApplicationEventsObserver observer, boolean replyImmediately) { + public void registerApplicationObserver(ApplicationEventsObserver observer) { if (this.connection == null) return; if (!applicationEventsObservers.contains(observer)) applicationEventsObservers.add(observer); - if (replyImmediately) { - if( hostState.volumeLevel == -1 ) { - getApplicationProperties(); - } else { - observer.applicationOnVolumeChanged(hostState.volumeLevel, hostState.volumeMuted); - } - } + // Reply immediatelly + replyWithLastResult(observer); if (applicationEventsObservers.size() == 1) { // If this is the first observer, start checking through HTTP or register us @@ -350,8 +348,7 @@ public class HostConnectionObserver " observers."); if (applicationEventsObservers.isEmpty()) { - // No more observers, so unregister us from the host connection, or stop - // the http checker thread + // No more observers. If through TCP unregister us from the host connection if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) { connection.unregisterApplicationNotificationsObserver(this); } @@ -361,29 +358,27 @@ public class HostConnectionObserver /** * Registers a new observer that will be notified about playlist events * @param observer Observer - * @param replyImmediately Whether to immediately issue a request if there are playlists available */ - public void registerPlaylistObserver(PlaylistEventsObserver observer, boolean replyImmediately) { + public void registerPlaylistObserver(PlaylistEventsObserver observer) { if (this.connection == null) return; - if ( ! playlistEventsObservers.contains(observer) ) { + if (!playlistEventsObservers.contains(observer) ) { playlistEventsObservers.add(observer); } + // Reply immediatelly + replyWithLastResult(observer); + if (playlistEventsObservers.size() == 1) { if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) { connection.registerPlaylistNotificationsObserver(this, checkerHandler); } - startCheckerHandler(); } - - if (replyImmediately) - checkPlaylist(); } - public void unregisterPlaylistObserver(PlayerEventsObserver observer) { + public void unregisterPlaylistObserver(PlaylistEventsObserver observer) { playlistEventsObservers.remove(observer); LogUtils.LOGD(TAG, "Unregistering playlist observer " + observer.getClass().getSimpleName() + @@ -391,11 +386,11 @@ public class HostConnectionObserver " observers."); if (playlistEventsObservers.isEmpty()) { - // No more observers, so unregister us from the host connection, or stop - // the http checker thread + // No more observers. If through TCP unregister us from the host connection if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) { connection.unregisterPlaylistNotificationsObserver(this); } + hostState.lastGetPlaylistResults = null; } } @@ -407,6 +402,8 @@ public class HostConnectionObserver observer.observerOnStopObserving(); playerEventsObservers.clear(); + playlistEventsObservers.clear(); + applicationEventsObservers.clear(); if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) { connection.unregisterPlayerNotificationsObserver(this); @@ -416,7 +413,7 @@ public class HostConnectionObserver connection.unregisterPlaylistNotificationsObserver(this); checkerHandler.removeCallbacks(tcpCheckerRunnable); } - hostState.lastCallResult = PlayerEventsObserver.PLAYER_NO_RESULT; + hostState = new HostState(); } @Override @@ -522,6 +519,11 @@ public class HostConnectionObserver @Override public void onPlaylistCleared(Playlist.OnClear notification) { + if (hostState.lastGetPlaylistResults != null) + hostState.lastGetPlaylistResults.clear(); + else + hostState.lastGetPlaylistResults = new ArrayList<>(); + for (PlaylistEventsObserver observer : playlistEventsObservers) { observer.playlistOnClear(notification.playlistId); } @@ -529,16 +531,12 @@ public class HostConnectionObserver @Override public void onPlaylistItemAdded(Playlist.OnAdd notification) { - for (PlaylistEventsObserver observer : playlistEventsObservers) { - observer.playlistChanged(notification.playlistId); - } + checkPlaylist(); } @Override public void onPlaylistItemRemoved(Playlist.OnRemove notification) { - for (PlaylistEventsObserver observer : playlistEventsObservers) { - observer.playlistChanged(notification.playlistId); - } + checkPlaylist(); } private void startCheckerHandler() { @@ -577,7 +575,6 @@ public class HostConnectionObserver }, checkerHandler); } - private ArrayList prevGetPlaylistResults = new ArrayList<>(); private boolean isCheckingPlaylist = false; private void checkPlaylist() { if (isCheckingPlaylist) @@ -588,10 +585,12 @@ public class HostConnectionObserver connection.execute(new GetPlaylist(connection), new ApiCallback>() { @Override public void onSuccess(ArrayList result) { + LogUtils.LOGD(TAG, "Checked playlist, got results: " + result.size()); isCheckingPlaylist = false; if (result.isEmpty()) { - callPlaylistsOnClear(prevGetPlaylistResults); + callPlaylistsOnClear(hostState.lastGetPlaylistResults); + hostState.lastGetPlaylistResults = result; return; } @@ -599,19 +598,19 @@ public class HostConnectionObserver observer.playlistsAvailable(result); } - // Handle onClear for HTTP only connections - for (GetPlaylist.GetPlaylistResult getPlaylistResult : result) { - for (int i = 0; i < prevGetPlaylistResults.size(); i++) { - if (getPlaylistResult.id == prevGetPlaylistResults.get(i).id) { - prevGetPlaylistResults.remove(i); - break; + // Handle cleared playlists + if (hostState.lastGetPlaylistResults != null) { + for (GetPlaylist.GetPlaylistResult getPlaylistResult : result) { + for (int i = 0; i < hostState.lastGetPlaylistResults.size(); i++) { + if (getPlaylistResult.id == hostState.lastGetPlaylistResults.get(i).id) { + hostState.lastGetPlaylistResults.remove(i); + break; + } } } + callPlaylistsOnClear(hostState.lastGetPlaylistResults); } - - callPlaylistsOnClear(prevGetPlaylistResults); - - prevGetPlaylistResults = result; + hostState.lastGetPlaylistResults = result; } @Override @@ -622,10 +621,11 @@ public class HostConnectionObserver observer.playlistOnError(errorCode, description); } } - }, new Handler()); + }, checkerHandler); } private void callPlaylistsOnClear(ArrayList clearedPlaylists) { + if (clearedPlaylists == null) return; for (GetPlaylist.GetPlaylistResult getPlaylistResult : clearedPlaylists) { for (PlaylistEventsObserver observer : playlistEventsObservers) { observer.playlistOnClear(getPlaylistResult.id); @@ -935,11 +935,11 @@ public class HostConnectionObserver } /** - * Replies to the observer with the last result we got. + * Replies to the player observer with the last result we got. * If we have no result, nothing will be called on the observer interface. - * @param observer Observer to call with last result + * @param observer Player observer to call with last result */ - public void replyWithLastResult(PlayerEventsObserver observer) { + private void replyWithLastResult(PlayerEventsObserver observer) { switch (hostState.lastCallResult) { case PlayerEventsObserver.PLAYER_CONNECTION_ERROR: notifyConnectionError(hostState.lastErrorCode, hostState.lastErrorDescription, observer); @@ -957,16 +957,41 @@ public class HostConnectionObserver } } + /** + * Replies to the application observer with the last result we got. + * If we have no result, nothing will be called on the observer interface. + * @param observer Application observer to call with last result + */ + private void replyWithLastResult(ApplicationEventsObserver observer) { + if (hostState.volumeLevel == -1) { + getApplicationProperties(); + } else { + observer.applicationOnVolumeChanged(hostState.volumeLevel, hostState.volumeMuted); + } + } + /** + * Replies to the playlist observer with the last result we got. + * If we have no result, nothing will be called on the observer interface. + * @param observer Playlist observer to call with last result + */ + private void replyWithLastResult(PlaylistEventsObserver observer) { + if (hostState.lastGetPlaylistResults != null) + observer.playlistsAvailable(hostState.lastGetPlaylistResults); + else + checkPlaylist(); + } + /** * Forces a refresh of the current cached results */ - public void forceRefreshResults() { - LogUtils.LOGD(TAG, "Forcing a refresh"); + public void refreshWhatsPlaying() { + LogUtils.LOGD(TAG, "Forcing a refresh of what's playing"); forceReply = true; checkWhatsPlaying(); } - public HostState getHostState() { - return hostState; + public void refreshPlaylists() { + LogUtils.LOGD(TAG, "Forcing a refresh of playlists"); + checkPlaylist(); } } diff --git a/app/src/main/java/org/xbmc/kore/host/HostManager.java b/app/src/main/java/org/xbmc/kore/host/HostManager.java index 4429f6d..e23d68c 100644 --- a/app/src/main/java/org/xbmc/kore/host/HostManager.java +++ b/app/src/main/java/org/xbmc/kore/host/HostManager.java @@ -198,10 +198,14 @@ public class HostManager { */ public HostConnection getConnection() { if (currentHostConnection == null) { - currentHostInfo = getHostInfo(); + synchronized (this) { + if (currentHostConnection == null) { + currentHostInfo = getHostInfo(); - if (currentHostInfo != null) { - currentHostConnection = new HostConnection(currentHostInfo); + if (currentHostInfo != null) { + currentHostConnection = new HostConnection(currentHostInfo); + } + } } } return currentHostConnection; diff --git a/app/src/main/java/org/xbmc/kore/host/actions/GetPlaylist.java b/app/src/main/java/org/xbmc/kore/host/actions/GetPlaylist.java index d77be48..6352fd4 100644 --- a/app/src/main/java/org/xbmc/kore/host/actions/GetPlaylist.java +++ b/app/src/main/java/org/xbmc/kore/host/actions/GetPlaylist.java @@ -22,6 +22,7 @@ import org.xbmc.kore.jsonrpc.HostConnection; import org.xbmc.kore.jsonrpc.method.Playlist; import org.xbmc.kore.jsonrpc.type.ListType; import org.xbmc.kore.jsonrpc.type.PlaylistType; +import org.xbmc.kore.utils.LogUtils; import java.util.ArrayList; import java.util.HashMap; @@ -34,6 +35,7 @@ import java.util.concurrent.ExecutionException; * available. */ public class GetPlaylist implements Callable> { + private static final String TAG = LogUtils.makeLogTag(GetPlaylist.class); private final static String[] propertiesToGet = new String[] { ListType.FieldsAll.ART, @@ -62,7 +64,7 @@ public class GetPlaylist implements Callable> clientCallbacks = new HashMap<>(); @@ -209,7 +214,7 @@ public class HostConnection { // Start with the default host protocol this.protocol = hostInfo.getProtocol(); // Create a single threaded executor - this.executorService = Executors.newFixedThreadPool(10); + this.executorService = Executors.newFixedThreadPool(5); // Set timeout this.connectTimeout = connectTimeout; } @@ -226,7 +231,7 @@ public class HostConnection { * Overrides the protocol for this connection * @param protocol {@link #PROTOCOL_HTTP} or {@link #PROTOCOL_TCP} */ - public void setProtocol(int protocol) { + public synchronized void setProtocol(int protocol) { if (!isValidProtocol(protocol)) { throw new IllegalArgumentException("Invalid protocol specified."); } @@ -393,8 +398,7 @@ public class HostConnection { * @param method The remote method to invoke * @param The type of the return value of the method * @return the future result of the method call. API errors will be wrapped in - * an {@link java.util.concurrent.ExecutionException ExecutionException} like - * regular futures. + * an {@link ExecutionException} like regular futures. */ public Future execute(ApiMethod method) { final ApiFuture future = new ApiFuture<>(); @@ -418,9 +422,9 @@ public class HostConnection { * @param callable executed using an {@link ExecutorService} * @param apiCallback used to return the result of the callable * @param handler used to execute the {@link ApiCallback} methods - * @param + * @param The callable return type */ - public void execute(Callable callable, final ApiCallback apiCallback, final Handler handler) { + public void execute(final Callable callable, final ApiCallback apiCallback, final Handler handler) { final Future future = executorService.submit(callable); executorService.execute(new Runnable() { @Override @@ -559,7 +563,7 @@ public class HostConnection { /** * Initializes this class OkHttpClient */ - public OkHttpClient getOkHttpClient() { + public synchronized OkHttpClient getOkHttpClient() { if (httpClient == null) { httpClient = new OkHttpClient(); httpClient.setConnectTimeout(connectTimeout, TimeUnit.MILLISECONDS); @@ -693,15 +697,16 @@ public class HostConnection { private void executeThroughTcp(final ApiMethod method) { String methodId = String.valueOf(method.getId()); try { - // TODO: Validate if this shouldn't be enclosed by a synchronized. - if (socket == null) { - // Open connection to the server and setup reader thread - socket = openTcpConnection(hostInfo); - startListenerThread(socket); - } + synchronized (this) { + if (socket == null) { + // Open connection to the server and setup reader thread + socket = openTcpConnection(hostInfo); + startListenerThread(socket); + } - // Write request - sendTcpRequest(socket, method.toJsonString()); + // Write request + sendTcpRequest(socket, method.toJsonString()); + } } catch (final ApiException e) { callErrorCallback(methodId, e); } @@ -712,7 +717,7 @@ public class HostConnection { * This method calls connect() so that any errors are cathced * @param hostInfo Host info * @return Connection set up - * @throws ApiException + * @throws ApiException Exception if open is unsucessful */ private Socket openTcpConnection(HostInfo hostInfo) throws ApiException { try { @@ -731,7 +736,6 @@ public class HostConnection { } } - /** * Send a TCP request * @param socket Socket to write to @@ -752,12 +756,12 @@ public class HostConnection { } private void startListenerThread(final Socket socket) { - executorService.execute(new Runnable() { + tcpListenerThread = new Thread(new Runnable() { @Override public void run() { Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND); try { - LogUtils.LOGD(TAG, "Starting socket listener thread..."); + LogUtils.LOGD(TAG, "Starting TCP socket listener thread from thread: " + Thread.currentThread().getName()); // We're going to read from the socket. This will be a blocking call and // it will keep on going until disconnect() is called on this object. // Note: Mind the objects used here: we use createParser because it doesn't @@ -774,11 +778,13 @@ public class HostConnection { callErrorCallback(null, new ApiException(ApiException.INVALID_JSON_RESPONSE_FROM_HOST, e)); } catch (IOException e) { LogUtils.LOGW(TAG, "Error reading from socket.", e); - disconnect(); callErrorCallback(null, new ApiException(ApiException.IO_EXCEPTION_WHILE_READING_RESPONSE, e)); - } + } finally { + disconnect(); + } } }); + tcpListenerThread.start(); } private boolean shouldIgnoreTcpResponse(ObjectNode jsonResponse) { @@ -1113,7 +1119,7 @@ public class HostConnection { * Cleans up used resources. * This method should always be called if the protocol used is TCP, so we can shutdown gracefully */ - public void disconnect() { + public synchronized void disconnect() { if (protocol == PROTOCOL_HTTP) return; diff --git a/app/src/main/java/org/xbmc/kore/service/ConnectionObserversManagerService.java b/app/src/main/java/org/xbmc/kore/service/ConnectionObserversManagerService.java index d5f6deb..28a3ec3 100644 --- a/app/src/main/java/org/xbmc/kore/service/ConnectionObserversManagerService.java +++ b/app/src/main/java/org/xbmc/kore/service/ConnectionObserversManagerService.java @@ -81,13 +81,13 @@ public class ConnectionObserversManagerService extends Service if (hostConnectionObserver == null) { hostConnectionObserver = connectionObserver; - hostConnectionObserver.registerPlayerObserver(this, true); + hostConnectionObserver.registerPlayerObserver(this); } else if (hostConnectionObserver != connectionObserver) { // There has been a change in hosts. // Unregister the previous one and register the current one hostConnectionObserver.unregisterPlayerObserver(this); hostConnectionObserver = connectionObserver; - hostConnectionObserver.registerPlayerObserver(this, true); + hostConnectionObserver.registerPlayerObserver(this); } // If we get killed after returning from here, restart diff --git a/app/src/main/java/org/xbmc/kore/ui/BaseMediaActivity.java b/app/src/main/java/org/xbmc/kore/ui/BaseMediaActivity.java index 079cca2..4d8adc9 100644 --- a/app/src/main/java/org/xbmc/kore/ui/BaseMediaActivity.java +++ b/app/src/main/java/org/xbmc/kore/ui/BaseMediaActivity.java @@ -447,10 +447,10 @@ public abstract class BaseMediaActivity extends BaseActivity if (hostConnectionObserver == null) return; - hostConnectionObserver.registerApplicationObserver(this, true); - hostConnectionObserver.registerPlayerObserver(this, true); + hostConnectionObserver.registerApplicationObserver(this); + hostConnectionObserver.registerPlayerObserver(this); - hostConnectionObserver.forceRefreshResults(); + hostConnectionObserver.refreshWhatsPlaying(); } private void updateNowPlayingPanel(PlayerType.PropertyValue getPropertiesResult, diff --git a/app/src/main/java/org/xbmc/kore/ui/generic/VolumeControllerDialogFragmentListener.java b/app/src/main/java/org/xbmc/kore/ui/generic/VolumeControllerDialogFragmentListener.java index 6d3141a..546596d 100644 --- a/app/src/main/java/org/xbmc/kore/ui/generic/VolumeControllerDialogFragmentListener.java +++ b/app/src/main/java/org/xbmc/kore/ui/generic/VolumeControllerDialogFragmentListener.java @@ -138,8 +138,8 @@ public class VolumeControllerDialogFragmentListener extends AppCompatDialogFragm return; } - hostConnectionObserver.registerApplicationObserver(this, true); - hostConnectionObserver.forceRefreshResults(); + hostConnectionObserver.registerApplicationObserver(this); + hostConnectionObserver.refreshWhatsPlaying(); } private void setListeners() { diff --git a/app/src/main/java/org/xbmc/kore/ui/sections/remote/NowPlayingFragment.java b/app/src/main/java/org/xbmc/kore/ui/sections/remote/NowPlayingFragment.java index e891448..a79c2cc 100644 --- a/app/src/main/java/org/xbmc/kore/ui/sections/remote/NowPlayingFragment.java +++ b/app/src/main/java/org/xbmc/kore/ui/sections/remote/NowPlayingFragment.java @@ -232,8 +232,8 @@ public class NowPlayingFragment extends Fragment @Override public void onResume() { super.onResume(); - hostConnectionObserver.registerPlayerObserver(this, true); - hostConnectionObserver.registerApplicationObserver(this, true); + hostConnectionObserver.registerPlayerObserver(this); + hostConnectionObserver.registerApplicationObserver(this); } @Override @@ -331,7 +331,7 @@ public class NowPlayingFragment extends Fragment public void onSuccess(String result) { if (!isAdded()) return; // Force a refresh - hostConnectionObserver.forceRefreshResults(); + hostConnectionObserver.refreshWhatsPlaying(); } @Override @@ -346,7 +346,7 @@ public class NowPlayingFragment extends Fragment @Override public void onSuccess(String result) { if (!isAdded()) return; - hostConnectionObserver.forceRefreshResults(); + hostConnectionObserver.refreshWhatsPlaying(); } @Override diff --git a/app/src/main/java/org/xbmc/kore/ui/sections/remote/PlaylistFragment.java b/app/src/main/java/org/xbmc/kore/ui/sections/remote/PlaylistFragment.java index cfbe333..ce42da9 100644 --- a/app/src/main/java/org/xbmc/kore/ui/sections/remote/PlaylistFragment.java +++ b/app/src/main/java/org/xbmc/kore/ui/sections/remote/PlaylistFragment.java @@ -182,8 +182,8 @@ public class PlaylistFragment extends Fragment public void onResume() { super.onResume(); - hostConnectionObserver.registerPlayerObserver(this, true); - hostConnectionObserver.registerPlaylistObserver(this, true); + hostConnectionObserver.registerPlayerObserver(this); + hostConnectionObserver.registerPlaylistObserver(this); } @Override @@ -246,7 +246,7 @@ public class PlaylistFragment extends Fragment public void onError(int errorCode, String description) { refreshingPlaylist = false; - displayErrorGettingPlaylistMessage(description); + playerOnConnectionError(errorCode, description); } }, callbackHandler); } @@ -366,7 +366,7 @@ public class PlaylistFragment extends Fragment Iterator it = playlists.keySet().iterator(); while (it.hasNext()) { String key = it.next(); - if ( playlists.get(key).getPlaylistResult.id == playlistId ) { + if (playlists.get(key).getPlaylistResult.id == playlistId) { it.remove(); playlistsBar.setHasPlaylistAvailable(key, false); playlistsBar.setIsPlaying(key, false); @@ -375,16 +375,13 @@ public class PlaylistFragment extends Fragment displayPlaylist(); } - @Override - public void playlistChanged(int playlistId) { - refreshPlaylist(new GetPlaylist(hostManager.getConnection(), playlistId)); - } - @Override public void playlistsAvailable(ArrayList playlists) { updatePlaylists(playlists); - if ( playerState == PLAYER_STATE.PLAYING ) // if item is currently playing displaying is already handled by playerOnPlay callback + if ((playerState == PLAYER_STATE.PLAYING) && + (hostManager.getConnection().getProtocol() == HostConnection.PROTOCOL_TCP)) + // if item is currently playing displaying is already handled by playerOnPlay callback return; // BUG: When playing movies playlist stops, audio tab gets selected when it contains a playlist. @@ -398,7 +395,7 @@ public class PlaylistFragment extends Fragment @Override public void playlistOnError(int errorCode, String description) { - displayErrorGettingPlaylistMessage(description); + playerOnConnectionError(errorCode, description); } private void updatePlaylists(ArrayList playlists) { @@ -488,16 +485,6 @@ public class PlaylistFragment extends Fragment } } - /** - * Displays an error on the info panel - * @param details Details message - */ - private void displayErrorGettingPlaylistMessage(String details) { - switchToPanel(R.id.info_panel); - infoTitle.setText(R.string.error_getting_playlist); - infoMessage.setText(String.format(getString(R.string.error_message), details)); - } - /** * Displays empty playlist */ diff --git a/app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteActivity.java b/app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteActivity.java index 882d973..fc6300b 100644 --- a/app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteActivity.java +++ b/app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteActivity.java @@ -176,9 +176,10 @@ public class RemoteActivity extends BaseActivity public void onResume() { super.onResume(); hostConnectionObserver = hostManager.getHostConnectionObserver(); - hostConnectionObserver.registerPlayerObserver(this, true); - // Force a refresh, mainly to update the time elapsed on the fragments - hostConnectionObserver.forceRefreshResults(); + hostConnectionObserver.registerPlayerObserver(this); + // Force a refresh, specifically to update the time elapsed on the fragments + hostConnectionObserver.refreshWhatsPlaying(); + hostConnectionObserver.refreshPlaylists(); // Check whether we should keep the remote activity above the lockscreen boolean keepAboveLockscreen = PreferenceManager diff --git a/app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteFragment.java b/app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteFragment.java index 2a656f8..f484d5a 100644 --- a/app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteFragment.java +++ b/app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteFragment.java @@ -224,7 +224,7 @@ public class RemoteFragment extends Fragment @Override public void onResume() { super.onResume(); - hostConnectionObserver.registerPlayerObserver(this, true); + hostConnectionObserver.registerPlayerObserver(this); if (eventServerConnection == null) eventServerConnection = createEventServerConnection(); } @@ -265,7 +265,7 @@ public class RemoteFragment extends Fragment eventServerConnection = null; } } - }); + }, callbackHandler); } /**