moxie0
commented
6 years ago This is not what I'm looking for. Again, what I'm looking for is:
1) Replace Thread.sleep() with an interface. 2) An implementation of that interface in the java module that uses thead.sleep 3) An implementation of that interface in the android module that uses an alarm manager. 4) The caller passes in the implementation during construction, the websocket class connection logic is completely unchanged except for references to thread.sleep
Adds a way to periodically wake the WebSocketConnection and avoid issues when the device enters sleep, disrupting timing functions like Thread.sleep() and Object.wait().
// FREEBIE
Description
This is a reimplementation of #45, so please see the discussion there, as well as in WhisperSystems/Signal-Android#6644 and WhisperSystems/Signal-Android#6732, for more details.
This started out as an attempt at a different implementation, more aligned with the desired goal of maintaining the existing interface for the
WebSocketConnection. I didn't see how that was possible, but I was hoping to find some way using thelibsignal-service-androidartifact, which is currently empty. Sadly, the current PR turned out mostly the same as #45.The basic difference, is that the code that sets up the periodic alarm, now resides in the
libsignal-service-androidartifact, in the form of theWebSocketAlarmclass. Although it would be preferable, to make this part ofWebSocketConnection, I don't see how that is possible aslibsignal-service-java, depends on neitherlibsignal-service-android, nor on any Android-related stuff. The only way I can see around this, would be to migrateWebSocketConnection, along with other classes, that depend on it, to thelibsignal-service-androidartifact, but that would be an unnecessary major-scale modification, I would prefer to avoid.Even worse, the code that uses
WebSocketConnection, namelyMessageRetrievalService, needs to know about the need to use theWebSocketAlarmclass, and explicitly use it, if necessary. Again, it would be desirable for theWebSocketConnectionto take care of itself, but I don't see how that is possible as it doesn't have access toTextSecurePreferencesand hence knowledge of whether GCM is disabled and neither does it have access to aContext, that it can use to set up the alarm.I apologize in advance for submitting a patch, while acknowledging that it basically has the same shortcomings as the previously submitted PRs. As I've said, I can't see how the desired approach is possible, at least as far as I understand it, but I thought I'd give it another go, as there are now three separate parties submitting PRs on this, which implies it might be important to many people. Although I don't think the effort succeeded, I'm submitting it, in case it proves useful to someone. Feel free to close this without further explanation.