Home Original page

Add wrappers for Android logcat broadcaster by mykola-mokhnach · Pull Request #858 · appium/java-client

@mykola-mokhnach

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

  • No changes in production code.
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@mykola-mokhnach

vmaxim

/**
* @return The list of web socket message handlers.
*/
List<T> messageHandlers();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

messageHandlers() -> getMessageHandlers()

/**
* This is the basic interface for all web socket message handlers.
*/
public interface MessagesHandler {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's strange that messages handler can not handle messages.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, I'll make it generic

* @param reason connection close reason
*/
@OnClose
public void onClose(Session session, CloseReason reason) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

session parameter is never used.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is used in connect

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

@OnClose
public void onClose(CloseReason reason) {

@mykola-mokhnach

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(new MessagesHandler<String>() {
                @Override
                public void onMessage(String message) {
                    messageSemaphore.release();
                }

                @Override
                public void onConnected() {
                    connectedSemaphore.release();
                }

                @Override
                public void onDisconnected() {
                    // ignore
                }

                @Override
                public void onError(Throwable cause) {
                    // ignore
                }
            });

it might look like

driver.addLogcatMessageListener(this::onMessage);
driver.addLogcatErrorListener(this::onError);

Mykola Mokhnach added 2 commits

April 6, 2018 14:28

@mykola-mokhnach

Make sure the hotfix appium/appium#10496 is merged to appium server before testing this PR locally

SrinivasanTarget

@SrinivasanTarget

@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

SrinivasanTarget