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.
This commit is contained in:
Synced Synapse 2019-05-28 19:44:02 +01:00 committed by Martijn Brekhof
parent 4ac37f3f02
commit 7dfd982643
12 changed files with 190 additions and 151 deletions

View File

@ -81,13 +81,17 @@ public class EventServerConnection {
/** /**
* Constructor. Starts the thread that keeps the connection alive. Make sure to call quit() when done. * Constructor. Starts the thread that keeps the connection alive. Make sure to call quit() when done.
* @param hostInfo Host to connect to * @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; this.hostInfo = hostInfo;
LogUtils.LOGD(TAG, "Starting EventServer Thread"); LogUtils.LOGD(TAG, "Starting EventServer Thread");
// Handler thread that will keep pinging and send the requests to Kodi // 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(); handlerThread.start();
// Get the HandlerThread's Looper and use it for our Handler // 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"); LogUtils.LOGD(TAG, "Got an UnknownHostException, disabling EventServer");
hostInetAddress = null; 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);
} }

View File

@ -57,17 +57,22 @@ public class HostConnectionObserver
public interface PlaylistEventsObserver { public interface PlaylistEventsObserver {
/** /**
* Notifies that a playlist has been cleared
* @param playlistId of playlist that has been cleared * @param playlistId of playlist that has been cleared
*/ */
void playlistOnClear(int playlistId); 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<GetPlaylist.GetPlaylistResult> playlists); void playlistsAvailable(ArrayList<GetPlaylist.GetPlaylistResult> playlists);
/**
* Notifies that an error occured when fetching playlists
* @param errorCode Error code
* @param description Error description
*/
void playlistOnError(int errorCode, String description); void playlistOnError(int errorCode, String description);
} }
@ -164,8 +169,11 @@ public class HostConnectionObserver
private List<ApplicationEventsObserver> applicationEventsObservers = new ArrayList<>(); private List<ApplicationEventsObserver> applicationEventsObservers = new ArrayList<>();
private List<PlaylistEventsObserver> playlistEventsObservers = new ArrayList<>(); private List<PlaylistEventsObserver> 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 // Associate the Handler with the UI thread
int checkPlaylistCounter = 0;
private Handler checkerHandler = new Handler(Looper.getMainLooper()); private Handler checkerHandler = new Handler(Looper.getMainLooper());
private Runnable httpCheckerRunnable = new Runnable() { private Runnable httpCheckerRunnable = new Runnable() {
@Override @Override
@ -183,11 +191,15 @@ public class HostConnectionObserver
if (!applicationEventsObservers.isEmpty()) if (!applicationEventsObservers.isEmpty())
getApplicationProperties(); getApplicationProperties();
if (!playlistEventsObservers.isEmpty() && checkPlaylistCounter > 1) { if (!playlistEventsObservers.isEmpty()) {
checkPlaylist(); if (checkPlaylistFrequencyCounter <= 0) {
checkPlaylistCounter = 0; // Check playlist and reset the frequency counter
checkPlaylist();
checkPlaylistFrequencyCounter = 1;
} else {
checkPlaylistFrequencyCounter--;
}
} }
checkPlaylistCounter++;
checkerHandler.postDelayed(this, HTTP_NOTIFICATION_CHECK_INTERVAL); checkerHandler.postDelayed(this, HTTP_NOTIFICATION_CHECK_INTERVAL);
} }
@ -234,26 +246,18 @@ public class HostConnectionObserver
} }
}; };
public class HostState { private static class HostState {
private int lastCallResult = PlayerEventsObserver.PLAYER_NO_RESULT; int lastCallResult = PlayerEventsObserver.PLAYER_NO_RESULT;
private PlayerType.GetActivePlayersReturnType lastGetActivePlayerResult = null; PlayerType.GetActivePlayersReturnType lastGetActivePlayerResult = null;
private PlayerType.PropertyValue lastGetPropertiesResult = null; PlayerType.PropertyValue lastGetPropertiesResult = null;
private ListType.ItemsAll lastGetItemResult = null; ListType.ItemsAll lastGetItemResult = null;
private boolean volumeMuted = false; boolean volumeMuted = false;
private int volumeLevel = -1; // -1 indicates no volumeLevel known int volumeLevel = -1; // -1 indicates no volumeLevel known
private int lastErrorCode; int lastErrorCode;
private String lastErrorDescription; String lastErrorDescription;
ArrayList<GetPlaylist.GetPlaylistResult> lastGetPlaylistResults = null;
public int getVolumeLevel() {
return volumeLevel;
}
public boolean isVolumeMuted() {
return volumeMuted;
}
} }
private HostState hostState;
public HostState hostState;
public HostConnectionObserver(HostConnection connection) { public HostConnectionObserver(HostConnection connection) {
this.hostState = new HostState(); this.hostState = new HostState();
@ -264,14 +268,15 @@ public class HostConnectionObserver
* Registers a new observer that will be notified about player events * Registers a new observer that will be notified about player events
* @param observer Observer * @param observer Observer
*/ */
public void registerPlayerObserver(PlayerEventsObserver observer, boolean replyImmediately) { public void registerPlayerObserver(PlayerEventsObserver observer) {
if (this.connection == null) if (this.connection == null)
return; return;
if (!playerEventsObservers.contains(observer)) if (!playerEventsObservers.contains(observer))
playerEventsObservers.add(observer); playerEventsObservers.add(observer);
if (replyImmediately) replyWithLastResult(observer); // Reply immediatelly
replyWithLastResult(observer);
if (playerEventsObservers.size() == 1) { if (playerEventsObservers.size() == 1) {
// If this is the first observer, start checking through HTTP or register us // If this is the first observer, start checking through HTTP or register us
@ -297,8 +302,7 @@ public class HostConnectionObserver
" observers."); " observers.");
if (playerEventsObservers.isEmpty()) { if (playerEventsObservers.isEmpty()) {
// No more observers, so unregister us from the host connection, or stop // No more observers. If through TCP unregister us from the host connection
// the http checker thread
if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) { if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) {
connection.unregisterPlayerNotificationsObserver(this); connection.unregisterPlayerNotificationsObserver(this);
connection.unregisterSystemNotificationsObserver(this); connection.unregisterSystemNotificationsObserver(this);
@ -311,22 +315,16 @@ public class HostConnectionObserver
/** /**
* Registers a new observer that will be notified about application events * Registers a new observer that will be notified about application events
* @param observer Observer * @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) if (this.connection == null)
return; return;
if (!applicationEventsObservers.contains(observer)) if (!applicationEventsObservers.contains(observer))
applicationEventsObservers.add(observer); applicationEventsObservers.add(observer);
if (replyImmediately) { // Reply immediatelly
if( hostState.volumeLevel == -1 ) { replyWithLastResult(observer);
getApplicationProperties();
} else {
observer.applicationOnVolumeChanged(hostState.volumeLevel, hostState.volumeMuted);
}
}
if (applicationEventsObservers.size() == 1) { if (applicationEventsObservers.size() == 1) {
// If this is the first observer, start checking through HTTP or register us // If this is the first observer, start checking through HTTP or register us
@ -350,8 +348,7 @@ public class HostConnectionObserver
" observers."); " observers.");
if (applicationEventsObservers.isEmpty()) { if (applicationEventsObservers.isEmpty()) {
// No more observers, so unregister us from the host connection, or stop // No more observers. If through TCP unregister us from the host connection
// the http checker thread
if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) { if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) {
connection.unregisterApplicationNotificationsObserver(this); connection.unregisterApplicationNotificationsObserver(this);
} }
@ -361,29 +358,27 @@ public class HostConnectionObserver
/** /**
* Registers a new observer that will be notified about playlist events * Registers a new observer that will be notified about playlist events
* @param observer Observer * @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) if (this.connection == null)
return; return;
if ( ! playlistEventsObservers.contains(observer) ) { if (!playlistEventsObservers.contains(observer) ) {
playlistEventsObservers.add(observer); playlistEventsObservers.add(observer);
} }
// Reply immediatelly
replyWithLastResult(observer);
if (playlistEventsObservers.size() == 1) { if (playlistEventsObservers.size() == 1) {
if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) { if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) {
connection.registerPlaylistNotificationsObserver(this, checkerHandler); connection.registerPlaylistNotificationsObserver(this, checkerHandler);
} }
startCheckerHandler(); startCheckerHandler();
} }
if (replyImmediately)
checkPlaylist();
} }
public void unregisterPlaylistObserver(PlayerEventsObserver observer) { public void unregisterPlaylistObserver(PlaylistEventsObserver observer) {
playlistEventsObservers.remove(observer); playlistEventsObservers.remove(observer);
LogUtils.LOGD(TAG, "Unregistering playlist observer " + observer.getClass().getSimpleName() + LogUtils.LOGD(TAG, "Unregistering playlist observer " + observer.getClass().getSimpleName() +
@ -391,11 +386,11 @@ public class HostConnectionObserver
" observers."); " observers.");
if (playlistEventsObservers.isEmpty()) { if (playlistEventsObservers.isEmpty()) {
// No more observers, so unregister us from the host connection, or stop // No more observers. If through TCP unregister us from the host connection
// the http checker thread
if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) { if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) {
connection.unregisterPlaylistNotificationsObserver(this); connection.unregisterPlaylistNotificationsObserver(this);
} }
hostState.lastGetPlaylistResults = null;
} }
} }
@ -407,6 +402,8 @@ public class HostConnectionObserver
observer.observerOnStopObserving(); observer.observerOnStopObserving();
playerEventsObservers.clear(); playerEventsObservers.clear();
playlistEventsObservers.clear();
applicationEventsObservers.clear();
if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) { if (connection.getProtocol() == HostConnection.PROTOCOL_TCP) {
connection.unregisterPlayerNotificationsObserver(this); connection.unregisterPlayerNotificationsObserver(this);
@ -416,7 +413,7 @@ public class HostConnectionObserver
connection.unregisterPlaylistNotificationsObserver(this); connection.unregisterPlaylistNotificationsObserver(this);
checkerHandler.removeCallbacks(tcpCheckerRunnable); checkerHandler.removeCallbacks(tcpCheckerRunnable);
} }
hostState.lastCallResult = PlayerEventsObserver.PLAYER_NO_RESULT; hostState = new HostState();
} }
@Override @Override
@ -522,6 +519,11 @@ public class HostConnectionObserver
@Override @Override
public void onPlaylistCleared(Playlist.OnClear notification) { public void onPlaylistCleared(Playlist.OnClear notification) {
if (hostState.lastGetPlaylistResults != null)
hostState.lastGetPlaylistResults.clear();
else
hostState.lastGetPlaylistResults = new ArrayList<>();
for (PlaylistEventsObserver observer : playlistEventsObservers) { for (PlaylistEventsObserver observer : playlistEventsObservers) {
observer.playlistOnClear(notification.playlistId); observer.playlistOnClear(notification.playlistId);
} }
@ -529,16 +531,12 @@ public class HostConnectionObserver
@Override @Override
public void onPlaylistItemAdded(Playlist.OnAdd notification) { public void onPlaylistItemAdded(Playlist.OnAdd notification) {
for (PlaylistEventsObserver observer : playlistEventsObservers) { checkPlaylist();
observer.playlistChanged(notification.playlistId);
}
} }
@Override @Override
public void onPlaylistItemRemoved(Playlist.OnRemove notification) { public void onPlaylistItemRemoved(Playlist.OnRemove notification) {
for (PlaylistEventsObserver observer : playlistEventsObservers) { checkPlaylist();
observer.playlistChanged(notification.playlistId);
}
} }
private void startCheckerHandler() { private void startCheckerHandler() {
@ -577,7 +575,6 @@ public class HostConnectionObserver
}, checkerHandler); }, checkerHandler);
} }
private ArrayList<GetPlaylist.GetPlaylistResult> prevGetPlaylistResults = new ArrayList<>();
private boolean isCheckingPlaylist = false; private boolean isCheckingPlaylist = false;
private void checkPlaylist() { private void checkPlaylist() {
if (isCheckingPlaylist) if (isCheckingPlaylist)
@ -588,10 +585,12 @@ public class HostConnectionObserver
connection.execute(new GetPlaylist(connection), new ApiCallback<ArrayList<GetPlaylist.GetPlaylistResult>>() { connection.execute(new GetPlaylist(connection), new ApiCallback<ArrayList<GetPlaylist.GetPlaylistResult>>() {
@Override @Override
public void onSuccess(ArrayList<GetPlaylist.GetPlaylistResult> result) { public void onSuccess(ArrayList<GetPlaylist.GetPlaylistResult> result) {
LogUtils.LOGD(TAG, "Checked playlist, got results: " + result.size());
isCheckingPlaylist = false; isCheckingPlaylist = false;
if (result.isEmpty()) { if (result.isEmpty()) {
callPlaylistsOnClear(prevGetPlaylistResults); callPlaylistsOnClear(hostState.lastGetPlaylistResults);
hostState.lastGetPlaylistResults = result;
return; return;
} }
@ -599,19 +598,19 @@ public class HostConnectionObserver
observer.playlistsAvailable(result); observer.playlistsAvailable(result);
} }
// Handle onClear for HTTP only connections // Handle cleared playlists
for (GetPlaylist.GetPlaylistResult getPlaylistResult : result) { if (hostState.lastGetPlaylistResults != null) {
for (int i = 0; i < prevGetPlaylistResults.size(); i++) { for (GetPlaylist.GetPlaylistResult getPlaylistResult : result) {
if (getPlaylistResult.id == prevGetPlaylistResults.get(i).id) { for (int i = 0; i < hostState.lastGetPlaylistResults.size(); i++) {
prevGetPlaylistResults.remove(i); if (getPlaylistResult.id == hostState.lastGetPlaylistResults.get(i).id) {
break; hostState.lastGetPlaylistResults.remove(i);
break;
}
} }
} }
callPlaylistsOnClear(hostState.lastGetPlaylistResults);
} }
hostState.lastGetPlaylistResults = result;
callPlaylistsOnClear(prevGetPlaylistResults);
prevGetPlaylistResults = result;
} }
@Override @Override
@ -622,10 +621,11 @@ public class HostConnectionObserver
observer.playlistOnError(errorCode, description); observer.playlistOnError(errorCode, description);
} }
} }
}, new Handler()); }, checkerHandler);
} }
private void callPlaylistsOnClear(ArrayList<GetPlaylist.GetPlaylistResult> clearedPlaylists) { private void callPlaylistsOnClear(ArrayList<GetPlaylist.GetPlaylistResult> clearedPlaylists) {
if (clearedPlaylists == null) return;
for (GetPlaylist.GetPlaylistResult getPlaylistResult : clearedPlaylists) { for (GetPlaylist.GetPlaylistResult getPlaylistResult : clearedPlaylists) {
for (PlaylistEventsObserver observer : playlistEventsObservers) { for (PlaylistEventsObserver observer : playlistEventsObservers) {
observer.playlistOnClear(getPlaylistResult.id); 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. * 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) { switch (hostState.lastCallResult) {
case PlayerEventsObserver.PLAYER_CONNECTION_ERROR: case PlayerEventsObserver.PLAYER_CONNECTION_ERROR:
notifyConnectionError(hostState.lastErrorCode, hostState.lastErrorDescription, observer); 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 * Forces a refresh of the current cached results
*/ */
public void forceRefreshResults() { public void refreshWhatsPlaying() {
LogUtils.LOGD(TAG, "Forcing a refresh"); LogUtils.LOGD(TAG, "Forcing a refresh of what's playing");
forceReply = true; forceReply = true;
checkWhatsPlaying(); checkWhatsPlaying();
} }
public HostState getHostState() { public void refreshPlaylists() {
return hostState; LogUtils.LOGD(TAG, "Forcing a refresh of playlists");
checkPlaylist();
} }
} }

View File

@ -198,10 +198,14 @@ public class HostManager {
*/ */
public HostConnection getConnection() { public HostConnection getConnection() {
if (currentHostConnection == null) { if (currentHostConnection == null) {
currentHostInfo = getHostInfo(); synchronized (this) {
if (currentHostConnection == null) {
currentHostInfo = getHostInfo();
if (currentHostInfo != null) { if (currentHostInfo != null) {
currentHostConnection = new HostConnection(currentHostInfo); currentHostConnection = new HostConnection(currentHostInfo);
}
}
} }
} }
return currentHostConnection; return currentHostConnection;

View File

@ -22,6 +22,7 @@ import org.xbmc.kore.jsonrpc.HostConnection;
import org.xbmc.kore.jsonrpc.method.Playlist; import org.xbmc.kore.jsonrpc.method.Playlist;
import org.xbmc.kore.jsonrpc.type.ListType; import org.xbmc.kore.jsonrpc.type.ListType;
import org.xbmc.kore.jsonrpc.type.PlaylistType; import org.xbmc.kore.jsonrpc.type.PlaylistType;
import org.xbmc.kore.utils.LogUtils;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashMap; import java.util.HashMap;
@ -34,6 +35,7 @@ import java.util.concurrent.ExecutionException;
* available. * available.
*/ */
public class GetPlaylist implements Callable<ArrayList<GetPlaylist.GetPlaylistResult>> { public class GetPlaylist implements Callable<ArrayList<GetPlaylist.GetPlaylistResult>> {
private static final String TAG = LogUtils.makeLogTag(GetPlaylist.class);
private final static String[] propertiesToGet = new String[] { private final static String[] propertiesToGet = new String[] {
ListType.FieldsAll.ART, ListType.FieldsAll.ART,
@ -62,7 +64,7 @@ public class GetPlaylist implements Callable<ArrayList<GetPlaylist.GetPlaylistRe
/** /**
* Use this to get the first non-empty playlist * Use this to get the first non-empty playlist
* @param hostConnection * @param hostConnection {@link HostConnection} to use
*/ */
public GetPlaylist(HostConnection hostConnection) { public GetPlaylist(HostConnection hostConnection) {
this.hostConnection = hostConnection; this.hostConnection = hostConnection;
@ -70,7 +72,7 @@ public class GetPlaylist implements Callable<ArrayList<GetPlaylist.GetPlaylistRe
/** /**
* Use this to get a playlist for a specific playlist type * Use this to get a playlist for a specific playlist type
* @param hostConnection * @param hostConnection {@link HostConnection} to use
* @param playlistType should be one of the types from {@link org.xbmc.kore.jsonrpc.type.PlaylistType.GetPlaylistsReturnType}. * @param playlistType should be one of the types from {@link org.xbmc.kore.jsonrpc.type.PlaylistType.GetPlaylistsReturnType}.
* If null the first non-empty playlist is returned. * If null the first non-empty playlist is returned.
*/ */
@ -81,8 +83,8 @@ public class GetPlaylist implements Callable<ArrayList<GetPlaylist.GetPlaylistRe
/** /**
* Use this to get a playlist for a specific playlist id * Use this to get a playlist for a specific playlist id
* @param hostConnection * @param hostConnection {@link HostConnection} to use
* @param playlistId * @param playlistId Kodi's playlist id
*/ */
public GetPlaylist(HostConnection hostConnection, int playlistId) { public GetPlaylist(HostConnection hostConnection, int playlistId) {
this.hostConnection = hostConnection; this.hostConnection = hostConnection;

View File

@ -130,8 +130,13 @@ public class HostConnection {
*/ */
private Socket socket = null; private Socket socket = null;
/**
* Listener thread that will be listening on the TCP socket
*/
private Thread tcpListenerThread = null;
/** /**
* {@link java.util.HashMap} that will hold the {@link MethodCallInfo} with the information * {@link HashMap} that will hold the {@link MethodCallInfo} with the information
* necessary to respond to clients (TCP only) * necessary to respond to clients (TCP only)
*/ */
private final HashMap<String, MethodCallInfo<?>> clientCallbacks = new HashMap<>(); private final HashMap<String, MethodCallInfo<?>> clientCallbacks = new HashMap<>();
@ -209,7 +214,7 @@ public class HostConnection {
// Start with the default host protocol // Start with the default host protocol
this.protocol = hostInfo.getProtocol(); this.protocol = hostInfo.getProtocol();
// Create a single threaded executor // Create a single threaded executor
this.executorService = Executors.newFixedThreadPool(10); this.executorService = Executors.newFixedThreadPool(5);
// Set timeout // Set timeout
this.connectTimeout = connectTimeout; this.connectTimeout = connectTimeout;
} }
@ -226,7 +231,7 @@ public class HostConnection {
* Overrides the protocol for this connection * Overrides the protocol for this connection
* @param protocol {@link #PROTOCOL_HTTP} or {@link #PROTOCOL_TCP} * @param protocol {@link #PROTOCOL_HTTP} or {@link #PROTOCOL_TCP}
*/ */
public void setProtocol(int protocol) { public synchronized void setProtocol(int protocol) {
if (!isValidProtocol(protocol)) { if (!isValidProtocol(protocol)) {
throw new IllegalArgumentException("Invalid protocol specified."); throw new IllegalArgumentException("Invalid protocol specified.");
} }
@ -393,8 +398,7 @@ public class HostConnection {
* @param method The remote method to invoke * @param method The remote method to invoke
* @param <T> The type of the return value of the method * @param <T> The type of the return value of the method
* @return the future result of the method call. API errors will be wrapped in * @return the future result of the method call. API errors will be wrapped in
* an {@link java.util.concurrent.ExecutionException ExecutionException} like * an {@link ExecutionException} like regular futures.
* regular futures.
*/ */
public <T> Future<T> execute(ApiMethod<T> method) { public <T> Future<T> execute(ApiMethod<T> method) {
final ApiFuture<T> future = new ApiFuture<>(); final ApiFuture<T> future = new ApiFuture<>();
@ -418,9 +422,9 @@ public class HostConnection {
* @param callable executed using an {@link ExecutorService} * @param callable executed using an {@link ExecutorService}
* @param apiCallback used to return the result of the callable * @param apiCallback used to return the result of the callable
* @param handler used to execute the {@link ApiCallback} methods * @param handler used to execute the {@link ApiCallback} methods
* @param <T> * @param <T> The callable return type
*/ */
public <T> void execute(Callable<T> callable, final ApiCallback<T> apiCallback, final Handler handler) { public <T> void execute(final Callable<T> callable, final ApiCallback<T> apiCallback, final Handler handler) {
final Future<T> future = executorService.submit(callable); final Future<T> future = executorService.submit(callable);
executorService.execute(new Runnable() { executorService.execute(new Runnable() {
@Override @Override
@ -559,7 +563,7 @@ public class HostConnection {
/** /**
* Initializes this class OkHttpClient * Initializes this class OkHttpClient
*/ */
public OkHttpClient getOkHttpClient() { public synchronized OkHttpClient getOkHttpClient() {
if (httpClient == null) { if (httpClient == null) {
httpClient = new OkHttpClient(); httpClient = new OkHttpClient();
httpClient.setConnectTimeout(connectTimeout, TimeUnit.MILLISECONDS); httpClient.setConnectTimeout(connectTimeout, TimeUnit.MILLISECONDS);
@ -693,15 +697,16 @@ public class HostConnection {
private <T> void executeThroughTcp(final ApiMethod<T> method) { private <T> void executeThroughTcp(final ApiMethod<T> method) {
String methodId = String.valueOf(method.getId()); String methodId = String.valueOf(method.getId());
try { try {
// TODO: Validate if this shouldn't be enclosed by a synchronized. synchronized (this) {
if (socket == null) { if (socket == null) {
// Open connection to the server and setup reader thread // Open connection to the server and setup reader thread
socket = openTcpConnection(hostInfo); socket = openTcpConnection(hostInfo);
startListenerThread(socket); startListenerThread(socket);
} }
// Write request // Write request
sendTcpRequest(socket, method.toJsonString()); sendTcpRequest(socket, method.toJsonString());
}
} catch (final ApiException e) { } catch (final ApiException e) {
callErrorCallback(methodId, e); callErrorCallback(methodId, e);
} }
@ -712,7 +717,7 @@ public class HostConnection {
* This method calls connect() so that any errors are cathced * This method calls connect() so that any errors are cathced
* @param hostInfo Host info * @param hostInfo Host info
* @return Connection set up * @return Connection set up
* @throws ApiException * @throws ApiException Exception if open is unsucessful
*/ */
private Socket openTcpConnection(HostInfo hostInfo) throws ApiException { private Socket openTcpConnection(HostInfo hostInfo) throws ApiException {
try { try {
@ -731,7 +736,6 @@ public class HostConnection {
} }
} }
/** /**
* Send a TCP request * Send a TCP request
* @param socket Socket to write to * @param socket Socket to write to
@ -752,12 +756,12 @@ public class HostConnection {
} }
private void startListenerThread(final Socket socket) { private void startListenerThread(final Socket socket) {
executorService.execute(new Runnable() { tcpListenerThread = new Thread(new Runnable() {
@Override @Override
public void run() { public void run() {
Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND); Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
try { 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 // 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. // 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 // 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)); callErrorCallback(null, new ApiException(ApiException.INVALID_JSON_RESPONSE_FROM_HOST, e));
} catch (IOException e) { } catch (IOException e) {
LogUtils.LOGW(TAG, "Error reading from socket.", e); LogUtils.LOGW(TAG, "Error reading from socket.", e);
disconnect();
callErrorCallback(null, new ApiException(ApiException.IO_EXCEPTION_WHILE_READING_RESPONSE, e)); callErrorCallback(null, new ApiException(ApiException.IO_EXCEPTION_WHILE_READING_RESPONSE, e));
} } finally {
disconnect();
}
} }
}); });
tcpListenerThread.start();
} }
private boolean shouldIgnoreTcpResponse(ObjectNode jsonResponse) { private boolean shouldIgnoreTcpResponse(ObjectNode jsonResponse) {
@ -1113,7 +1119,7 @@ public class HostConnection {
* Cleans up used resources. * Cleans up used resources.
* This method should always be called if the protocol used is TCP, so we can shutdown gracefully * 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) if (protocol == PROTOCOL_HTTP)
return; return;

View File

@ -81,13 +81,13 @@ public class ConnectionObserversManagerService extends Service
if (hostConnectionObserver == null) { if (hostConnectionObserver == null) {
hostConnectionObserver = connectionObserver; hostConnectionObserver = connectionObserver;
hostConnectionObserver.registerPlayerObserver(this, true); hostConnectionObserver.registerPlayerObserver(this);
} else if (hostConnectionObserver != connectionObserver) { } else if (hostConnectionObserver != connectionObserver) {
// There has been a change in hosts. // There has been a change in hosts.
// Unregister the previous one and register the current one // Unregister the previous one and register the current one
hostConnectionObserver.unregisterPlayerObserver(this); hostConnectionObserver.unregisterPlayerObserver(this);
hostConnectionObserver = connectionObserver; hostConnectionObserver = connectionObserver;
hostConnectionObserver.registerPlayerObserver(this, true); hostConnectionObserver.registerPlayerObserver(this);
} }
// If we get killed after returning from here, restart // If we get killed after returning from here, restart

View File

@ -447,10 +447,10 @@ public abstract class BaseMediaActivity extends BaseActivity
if (hostConnectionObserver == null) if (hostConnectionObserver == null)
return; return;
hostConnectionObserver.registerApplicationObserver(this, true); hostConnectionObserver.registerApplicationObserver(this);
hostConnectionObserver.registerPlayerObserver(this, true); hostConnectionObserver.registerPlayerObserver(this);
hostConnectionObserver.forceRefreshResults(); hostConnectionObserver.refreshWhatsPlaying();
} }
private void updateNowPlayingPanel(PlayerType.PropertyValue getPropertiesResult, private void updateNowPlayingPanel(PlayerType.PropertyValue getPropertiesResult,

View File

@ -138,8 +138,8 @@ public class VolumeControllerDialogFragmentListener extends AppCompatDialogFragm
return; return;
} }
hostConnectionObserver.registerApplicationObserver(this, true); hostConnectionObserver.registerApplicationObserver(this);
hostConnectionObserver.forceRefreshResults(); hostConnectionObserver.refreshWhatsPlaying();
} }
private void setListeners() { private void setListeners() {

View File

@ -232,8 +232,8 @@ public class NowPlayingFragment extends Fragment
@Override @Override
public void onResume() { public void onResume() {
super.onResume(); super.onResume();
hostConnectionObserver.registerPlayerObserver(this, true); hostConnectionObserver.registerPlayerObserver(this);
hostConnectionObserver.registerApplicationObserver(this, true); hostConnectionObserver.registerApplicationObserver(this);
} }
@Override @Override
@ -331,7 +331,7 @@ public class NowPlayingFragment extends Fragment
public void onSuccess(String result) { public void onSuccess(String result) {
if (!isAdded()) return; if (!isAdded()) return;
// Force a refresh // Force a refresh
hostConnectionObserver.forceRefreshResults(); hostConnectionObserver.refreshWhatsPlaying();
} }
@Override @Override
@ -346,7 +346,7 @@ public class NowPlayingFragment extends Fragment
@Override @Override
public void onSuccess(String result) { public void onSuccess(String result) {
if (!isAdded()) return; if (!isAdded()) return;
hostConnectionObserver.forceRefreshResults(); hostConnectionObserver.refreshWhatsPlaying();
} }
@Override @Override

View File

@ -182,8 +182,8 @@ public class PlaylistFragment extends Fragment
public void onResume() { public void onResume() {
super.onResume(); super.onResume();
hostConnectionObserver.registerPlayerObserver(this, true); hostConnectionObserver.registerPlayerObserver(this);
hostConnectionObserver.registerPlaylistObserver(this, true); hostConnectionObserver.registerPlaylistObserver(this);
} }
@Override @Override
@ -246,7 +246,7 @@ public class PlaylistFragment extends Fragment
public void onError(int errorCode, String description) { public void onError(int errorCode, String description) {
refreshingPlaylist = false; refreshingPlaylist = false;
displayErrorGettingPlaylistMessage(description); playerOnConnectionError(errorCode, description);
} }
}, callbackHandler); }, callbackHandler);
} }
@ -366,7 +366,7 @@ public class PlaylistFragment extends Fragment
Iterator<String> it = playlists.keySet().iterator(); Iterator<String> it = playlists.keySet().iterator();
while (it.hasNext()) { while (it.hasNext()) {
String key = it.next(); String key = it.next();
if ( playlists.get(key).getPlaylistResult.id == playlistId ) { if (playlists.get(key).getPlaylistResult.id == playlistId) {
it.remove(); it.remove();
playlistsBar.setHasPlaylistAvailable(key, false); playlistsBar.setHasPlaylistAvailable(key, false);
playlistsBar.setIsPlaying(key, false); playlistsBar.setIsPlaying(key, false);
@ -375,16 +375,13 @@ public class PlaylistFragment extends Fragment
displayPlaylist(); displayPlaylist();
} }
@Override
public void playlistChanged(int playlistId) {
refreshPlaylist(new GetPlaylist(hostManager.getConnection(), playlistId));
}
@Override @Override
public void playlistsAvailable(ArrayList<GetPlaylist.GetPlaylistResult> playlists) { public void playlistsAvailable(ArrayList<GetPlaylist.GetPlaylistResult> playlists) {
updatePlaylists(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; return;
// BUG: When playing movies playlist stops, audio tab gets selected when it contains a playlist. // 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 @Override
public void playlistOnError(int errorCode, String description) { public void playlistOnError(int errorCode, String description) {
displayErrorGettingPlaylistMessage(description); playerOnConnectionError(errorCode, description);
} }
private void updatePlaylists(ArrayList<GetPlaylist.GetPlaylistResult> playlists) { private void updatePlaylists(ArrayList<GetPlaylist.GetPlaylistResult> 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 * Displays empty playlist
*/ */

View File

@ -176,9 +176,10 @@ public class RemoteActivity extends BaseActivity
public void onResume() { public void onResume() {
super.onResume(); super.onResume();
hostConnectionObserver = hostManager.getHostConnectionObserver(); hostConnectionObserver = hostManager.getHostConnectionObserver();
hostConnectionObserver.registerPlayerObserver(this, true); hostConnectionObserver.registerPlayerObserver(this);
// Force a refresh, mainly to update the time elapsed on the fragments // Force a refresh, specifically to update the time elapsed on the fragments
hostConnectionObserver.forceRefreshResults(); hostConnectionObserver.refreshWhatsPlaying();
hostConnectionObserver.refreshPlaylists();
// Check whether we should keep the remote activity above the lockscreen // Check whether we should keep the remote activity above the lockscreen
boolean keepAboveLockscreen = PreferenceManager boolean keepAboveLockscreen = PreferenceManager

View File

@ -224,7 +224,7 @@ public class RemoteFragment extends Fragment
@Override @Override
public void onResume() { public void onResume() {
super.onResume(); super.onResume();
hostConnectionObserver.registerPlayerObserver(this, true); hostConnectionObserver.registerPlayerObserver(this);
if (eventServerConnection == null) if (eventServerConnection == null)
eventServerConnection = createEventServerConnection(); eventServerConnection = createEventServerConnection();
} }
@ -265,7 +265,7 @@ public class RemoteFragment extends Fragment
eventServerConnection = null; eventServerConnection = null;
} }
} }
}); }, callbackHandler);
} }
/** /**