-
-
Notifications
You must be signed in to change notification settings - Fork 271
Fix handling of signaling message responses #5115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
danxuliu
wants to merge
6
commits into
master
Choose a base branch
from
fix-handling-of-signaling-message-responses
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The interface is now passed to "sendMessage" instead of a serialized message, which will make possible to customize the "id" property (still to be added) if needed before sending the message. Note that, for consistency, the interface was applied to all "overall" data objects, even if some of them are only for received messages (like "ErrorOverallWebSocketMessage"). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
WebSocket messages may contain an "id" property used to match a response message with its request. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
With the external signaling server, if the "id" property is set when sending a signaling message the response to that specific message will be identified by the same id. Now a callback can be passed to "sendMessage", which automatically adds an id to the message and makes possible to run the callback when/if the response to the message is received. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When joining a room, the signaling server confirms that joining the room was successful by sending a "room" message. Until now the Android app assumed that "room" messages were sent only in that case, but a "room" message can be sent also if the properties of the room change. When the room is joined while the call activity is active the call is also joined, so if the properties of the room changed while in a call the Android app rejoined the call, which caused strange issues due to how it was done. For example, when a recording started additional guests appeared in the UI. To solve that now only the "room" message that actually confirms that the join was successful, which can be identified by setting an ID in the request to join the room, is treated as such. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
/backport to stable-21.1 |
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/5115-talk.apk |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #4635
When the external signaling server is used it is possible to assign an id to a message to then be able to identify the response in the messages sent by the signaling server. For example, when joining a room, the signaling server confirms that joining the room was successful with a
room
message.Until now the Android app assumed that
room
messages were sent only in that case, but aroom
message can be sent also if the properties of the room change. When the room is joined and the call activity is active the call is also joined, so if the properties of the room changed while in a call, the Android app rejoined the call. However, this was not done in a clean way; previous event handlers were replaced without tearing down them first, and that caused strange issues. For example, when a recording started additional guests appeared in the UI. That would be the most common/visible case, but there could be others too when something caused aroom
message to be sent while in a call.To solve that now only the
room
event that is sent as a response of joining a room is treated as such. Currently otherroom
messages are ignored, although they could be used to update the properties of the room and things like that, but that would be a future improvement.TODO
How to test
Result with this pull request
Nothing strange happens in the Android app
Result without this pull request
A guest user appears in the call view of the Android app