Uh oh!
There was an error while loading. Please reload this page.
Add wrappers for Android logcat broadcaster - #858
Merged
Conversation
mykola-mokhnach
commented
Apr 2, 2018
ContributorAuthor
@KazuCocoa FYI |
vmaxim
reviewed
Apr 3, 2018
| /** | ||
| * @return The list of web socket message handlers. | ||
| */ | ||
| List<T> messageHandlers(); |
| /** | ||
| * This is the basic interface for all web socket message handlers. | ||
| */ | ||
| public interface MessagesHandler { |
There was a problem hiding this comment.
It's strange that messages handler can not handle messages.
ContributorAuthor
There was a problem hiding this comment.
yep, I'll make it generic
| * @param reason connection close reason | ||
| */ | ||
| @OnClose | ||
| public void onClose(Session session, CloseReason reason) { |
ContributorAuthor
There was a problem hiding this comment.
it is used in connect
There was a problem hiding this comment.
@OnClose
public void onClose(CloseReason reason) {
mykola-mokhnach
commented
Apr 3, 2018
ContributorAuthor
I was also thinking about having separate callbacks for each event instead of interface implementation, which is more Java 8'ish. for example: Instead of driver.addLogcatListener(newMessagesHandler<String>() {
@OverridepublicvoidonMessage(Stringmessage) {
messageSemaphore.release();
}
@OverridepublicvoidonConnected() {
connectedSemaphore.release();
}
@OverridepublicvoidonDisconnected() {
// ignore
}
@OverridepublicvoidonError(Throwablecause) {
// ignore
}
});it might look like driver.addLogcatMessageListener(this::onMessage);
driver.addLogcatErrorListener(this::onError); |
mykola-mokhnach
commented
Apr 6, 2018
ContributorAuthor
Make sure the hotfix appium/appium#10496 is merged to appium server before testing this PR locally |
SrinivasanTarget
approved these changes
Apr 10, 2018
SrinivasanTarget
commented
Apr 10, 2018
Member
@mykola-mokhnach Unfortunately there is a conflict. Can you please resolve this? I will get this in. |
… into ws # Conflicts: # src/main/java/io/appium/java_client/android/AndroidDriver.java
mykola-mokhnach
commented
Apr 11, 2018
ContributorAuthor
@SrinivasanTarget The conflict is resolved |
SrinivasanTarget
approved these changes
Apr 13, 2018
6 tasks
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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Change list
Added basic classes to support web sockets interaction in general and to interact with logcat messages broadcaster that has been implemented for Android server.
Types of changes