[Bose Soundtouch] Improved thing status check #5968
Conversation
| @@ -76,6 +82,8 @@ | |||
| private CommandExecutor commandExecutor; | |||
|
|
|||
| private PresetContainer presetContainer; | |||
|
|
|||
| private final Counter missedPongsCount = new Counter(MAX_MISSED_PONGS_COUNT); | |||
There was a problem hiding this comment.
I find it rather irritating that the "missed pongs" counter counts backwards. If it has value "2", the natural interpretation would imho be that we are missing 2 pongs already.
Wouldn't a simple int fully do the job here, starting at 0 and increasing it on every miss?
There was a problem hiding this comment.
I agree that a simple int would do the job and fit our purposes better.
| this.session.getRemote().sendPing(null); | ||
| this.missedPongsCount.countDown(); | ||
| } catch (IOException | NullPointerException | UnsupportedOperationException e) { | ||
| logger.warn("Sending websocket PING failed."); |
There was a problem hiding this comment.
This should be no warning here, only debug.
Furthermore, the message only applies to the IOException and NullPointerException, while the UnsupportedOperationException actually means that we missed some pongs (but actually not for the ping that we just sent, right? So does that make sense here at all?).
There was a problem hiding this comment.
I'll change the debug and the try/catch behavior here together with the removal of the Counter.
|
|
||
| public synchronized void countDown() { | ||
| if (currentValue == 0) { | ||
| throw new UnsupportedOperationException("Cannot count down below 0!"); |
There was a problem hiding this comment.
UnsupportedOperationException seems an unsuitable choice here. IllegalStateException might be better, but imho simply removing this class and using an int as a counter and comparing it against the max would be much better readable.
There was a problem hiding this comment.
Totally agree.
…heck. Signed-off-by: Alexander Kostadinov <alexander.g.kostadinov@gmail.com>
alex-kostadinov
force-pushed
the
feature/bose/implement-ws-ping-pong
branch
from
c04c164
to
4269087
Compare
|
@kaikreuzer Requested changes are pushed. |
…heck. (eclipse-archived#5968) Signed-off-by: Alexander Kostadinov <alexander.g.kostadinov@gmail.com>
…heck. (eclipse-archived#5968) Signed-off-by: Alexander Kostadinov <alexander.g.kostadinov@gmail.com>
kaikreuzer
changed the title
There is an issue that things representing BOSE SoundTouch players (SoundTouch 20 for example) don't go OFFLINE shortly after the ethernet or power cable is disconnected.
The reason is that doing
session.getRemote().sendString("HELLO")does not promise that the remote party actually responds. This actually results in the fact that it takes random time for the thing to go OFFLINE (e.g. 20 minutes).So, in order to reliably check the player's status, we PING the player by doing
session.getRemote().sendPing()and make use ofWebSocketFrameListener#onWebSocketFrame(Frame frame)which notifies about a received respective PONG. If two consecutively sent PINGs don't get a respective PONG, thing goes OFFLINE.