Remote recording control #20
Conversation
…orage permission for api 29
# Conflicts: # app/src/main/java/net/sourceforge/opencamera/MyApplicationInterface.java
| private final ServerSocket mRpcSocket; | ||
| private final Properties mConfig; | ||
| private final RemoteRpcRequestHandler mRequestHandler; | ||
| private final AtomicBoolean mIsExecuting = new AtomicBoolean(); |
There was a problem hiding this comment.
Looks like volatile boolean should be enough here.
| } | ||
|
|
||
| public void stopExecuting() { | ||
| mIsExecuting.set(false); |
There was a problem hiding this comment.
NB! It is safe to .stopExecuting even in case mIsExecuting is already false
| @@ -1494,6 +1513,12 @@ protected void onPause() { | |||
| super.onPause(); // docs say to call this before freeing other things | |||
| this.app_is_paused = true; | |||
|
|
|||
| // Stop Remote controller for OpenCamera Sensors | |||
| if (mRpcServer != null && mRpcServer.isExecuting()) { | |||
There was a problem hiding this comment.
| if (mRpcServer != null && mRpcServer.isExecuting()) { | |
| if (mRpcServer != null) { |
There was a problem hiding this comment.
Looks like it the check is redundant
| Log.d(TAG, "App rpc listener thread started"); | ||
|
|
||
| } catch (IOException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
| e.printStackTrace(); | |
| e.printStackTrace(); | |
| mRpcServer = null; |
There was a problem hiding this comment.
If we do not need broken object.
| "\n" + | ||
| message + | ||
| "\n" + | ||
| config.getProperty("CHUNK_END_DELIMITER"); |
There was a problem hiding this comment.
Advice: do not read property every time, store value in private field on initialization, if the property value does not change between application restarts.
| mRpcSocket.setReuseAddress(true); | ||
| mRpcSocket.setSoTimeout(SOCKET_WAIT_TIME_MS); | ||
|
|
||
| if (MyDebug.LOG) { |
There was a problem hiding this comment.
Missing handshake. At least some string with version and available features\options string . Or each reply should contain this kind of server identification. (f.e. like User Agent string from browser or HTTP reply header from server)
| BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream)); | ||
|
|
||
| String inputLine; | ||
| while (!clientSocket.isClosed() && (inputLine = reader.readLine()) != null) { |
There was a problem hiding this comment.
Advice: check the exit condition (mIsExecuting) here too. Server must shutdown immediately despite the input length.
| if (MyDebug.LOG) { | ||
| Log.d(TAG, "closing connection to client"); | ||
| } | ||
| outputByte.close(); |
| } | ||
|
|
||
| // Stopped accepting requests, close the server socket | ||
| try { |
There was a problem hiding this comment.
Advice: enclose the whole processing loop into try/finally to close the socket in the finally block.
| private final double mAvgDurationNs; | ||
|
|
||
| // Accepts a sorted collection | ||
| private static double median(List<Long> numList) { |
There was a problem hiding this comment.
If this function is actually useful now and the performance is not critical, just call Collections.sort. It is safe to be called on small sorted List, but will guarantee the sorted order.
No description provided.