Add Roku driver

[cbuelvasc]

Proposed changes

• Added Roku Driver to support Appium in Roku.

Types of changes

What types of changes does your code introduce to Appium?
Put an x in the boxes that apply

• 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)
• Documentation Update (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

• I have read the CONTRIBUTING doc
• I have signed the CLA
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)
• Any dependent changes have been merged and published in downstream modules

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

[KazuCocoa]

Do you have any documentation about it?
It would be great to the repository or documentation to link from https://github.com/appium/appium/blob/d37090e67e88d2f59113b62174f1dcd1d2e8611f/docs/en/about-appium/platform-support.md

[mykola-mokhnach]

@cbuelvasc Could you please add dependabot to the driver repository to make sure there are no outdated dependencies?

[mykola-mokhnach]

With basedriver version 8 we can only include this driver to v2 Appium branch. Otherwise it is not compatible with Appium v1

[cbuelvasc]

documentation

Add changes in platform-support.md, thank you for the advice

[cbuelvasc]

@cbuelvasc Could you please add dependabot to the driver repository to make sure there are no outdated dependencies?

Add dependabot for both repositories https://github.com/cbuelvasc/appium-roku-driver and https://github.com/cbuelvasc/appium-ecp-master.

Thank you for the advice

[cbuelvasc]

With basedriver version 8 we can only include this driver to v2 Appium branch. Otherwise it is not compatible with Appium v1

Please help me with that
Can you tell me how to get the support from version 1?

[mykola-mokhnach]

Can you tell me how to get the support from version 1?

Simply use base driver with major version 7

[cbuelvasc]

Can you tell me how to get the support from version 1?

Simply use base driver with major version 7

Fixed the issue

[warrior7474]

cbuelvasc:appium-roku-driver

[mykola-mokhnach]

I was checking https://github.com/dlenroc/appium-roku-driver by @dlenroc
It has a good readme and the internal implementation itself seems to be decent, even though it is only compatible to Appium 2.0
@cbuelvasc Maybe it makes sense to keep everything as is and only have that driver available for Appium 2.0. Appium 1.0 will be EOLed sooner or later and adding new dependencies to it unnecessarily increases the overall package size plus add troubles related to third-party packages maintenance.
What do you think @KazuCocoa @jlipps ?

[cbuelvasc]

I was checking https://github.com/dlenroc/appium-roku-driver by @dlenroc
It has a good readme and the internal implementation itself seems to be decent, even though it is only compatible to Appium 2.0
@cbuelvasc Maybe it makes sense to keep everything as is and only have that driver available for Appium 2.0. Appium 1.0 will be EOLed sooner or later and adding new dependencies to it unnecessarily increases the overall package size plus add troubles related to third-party packages maintenance.
What do you think @KazuCocoa @jlipps ?

Hello guys, I will review the documentation that you comment and I will improve mine a lot, I have the willingness to start at the point that you think is more convenient v1 or v2 or both.

I appreciate your comments.

[KazuCocoa]

I'd second the idea @mykola-mokhnach to work with 2.0. Appium 2.0 is more realistic to use nowadays. (e.g. In a flutter driver case, 2.0 was not enough to use.)

Appium 1.0 requires installing all dependencies in all users even when a user wants only a few drivers. (So Appium 2.0 is coming.) A similar naming driver is already in npm. (I also checked https://github.com/dlenroc/appium-roku-driver first, and thought it was this PR's one). I don't know how mature this driver (or used by some companies?), but it would be greater to consider using it as Appium 2.0.

[cbuelvasc]

I'd second the idea @mykola-mokhnach to work with 2.0. Appium 2.0 is more realistic to use nowadays. (e.g. In a flutter driver case, 2.0 was not enough to use.)

Appium 1.0 requires installing all dependencies in all users even when a user wants only a few drivers. (So Appium 2.0 is coming.) A similar naming driver is already in npm. (I also checked https://github.com/dlenroc/appium-roku-driver first, and thought it was this PR's one). I don't know how mature this driver (or used by some companies?), but it would be greater to consider using it as Appium 2.0.

@KazuCocoa I have also had the opportunity to review the repository https://github.com/dlenroc/appium-roku-driver and a few days ago it was created, some time ago I had my public repository and I had to make it private while developing it in its entirety Since some people cloned it, you can check the age of my repository and you can see that it takes around 8 - 9 months.

I work for a company in which I have been advancing the development of several drivers as my own initiative which has been very well received, among them the WebOS driver for LG smart televisions, as well as the Tizen-TV driver for Samsung smart televisions, due to because the Tizen driver that is currently in Appium does not support televisions.

My repositories are https://github.com/cbuelvasc/appium-roku-driver and https://github.com/cbuelvasc/appium-ecp-master
Npm packages are https://www.npmjs.com/package/appium-roku-driver and https://www.npmjs.com/package/appium-ecp

I am attentive to your comments, thanks very much.

[mykola-mokhnach]

What we could do to find a happy middle is to remove the driver from dependencies, but keep the information about it in the platform selector. This way one could simply install the necessary npm module manually if needed and Appium will dynamically pick it up, like, for example, OpenCV is currently loaded.

BTW, https://github.com/cbuelvasc/appium-roku-driver/blob/master/package.json is missing the appium entry, which makes it incompatible with Appium 2.0

[cbuelvasc]

What we could do to find a happy middle is to remove the driver from dependencies, but keep the information about it in the platform selector. This way one could simply install the necessary npm module manually if needed and Appium will dynamically pick it up, like, for example, OpenCV is currently loaded.

BTW, https://github.com/cbuelvasc/appium-roku-driver/blob/master/package.json is missing the appium entry, which makes it incompatible with Appium 2.0

appium entry

Sorry, @mykola-mokhnach, I had understood in the comments you made earlier to download the version to make it compatible, that was the first step, then I am guided by the guidelines they have in https://github.com/appium/appium-android- driver, in this repo there is not the part that you mention that I should add additional for the appium entry.

Excuse me, I'm a little disoriented

Fixed the entry:

"appium": {
    "driverName": "roku",
    "automationName": "Roku",
    "platformNames": [
      "Roku"
    ],
    "mainClass": "RokuDriver"
  }

[mykola-mokhnach]

I'm fine to merge the PR after https://github.com/appium/appium/pull/15227/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R53 is removed from package.json

[cbuelvasc]

I'm fine to merge the PR after https://github.com/appium/appium/pull/15227/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R53 is removed from package.json

Fixed package.json

Thanks very much, guys 👍

[dlenroc]

Hi, sorry for the intrusion,
I think it's good to have references to existing drivers that can be used, but it might be good to prefix drivers that aren't maintained by Appium or the original organization in a way that lets users know exactly what they're using, and the organization/appium could use a regular namespace someday.

For example this driver can use automationName in one of the following formats: cbuelvasc-roku, cbuelvasc/roku, @cbuelvasc/roku, etc.

@mykola-mokhnach, @KazuCocoa, @jlipps what do you think about this (considering future architecture, if makes sense) ?

---

PS: I looked through driver code and seems that most of the commands are almost entirely taken from by my repo if not considering rewriting in JS and Appium 1 support what is good.

If the point was Appium 1 support, I may update my driver to support it (internally we use a v1 version that is injected in Appium using patch-package)

[mykola-mokhnach]

For example this driver can use automationName in one of the following formats: cbuelvasc-roku, cbuelvasc/roku, @cbuelvasc/roku, etc.

Is this a proposal for Appium 2.0?

[dlenroc]

@mykola-mokhnach yes, my concern is in how it will work in appium-2 after merging this PR and if will not be naming conflicts.

[mykola-mokhnach]

Appium2 works differently. It is only necessary to add the corresponding appium entry into package.json of your driver. Then Appium 2 picks it up automatically as soon as this driver is installed via Appium CLI and added to its internal registry. Appium CLI itself supports multiple sources, like NPM, GitHub repository or a local folder. Read https://appiumpro.com/editions/122-installing-appium-20-and-the-driver-and-plugins-cli for more details

[dlenroc]

@mykola-mokhnach thanks, then everything is fine 👍🏼, ie drivers installed from CLI will not conflict with those from DRIVER_MAP/AUTOMATION_NAMES -- because of ~/.appium folder where actual drivers/plugins/linking are stored.

[jlipps]

Hi all thanks for this, I've been having trouble getting time to dig in but will have a look soon

[jlipps]

It has a good readme and the internal implementation itself seems to be decent, even though it is only compatible to Appium 2.0

I think we should only worry about making this an Appium 2.0 driver. It's a brand new driver so there's no existing code changes to worry about, and creating a new driver with a support requirement for Appium 1.x seems like a recipe for pain.

[jlipps]

@dlenroc

then everything is fine 👍🏼, ie drivers installed from CLI will not conflict with those from DRIVER_MAP/AUTOMATION_NAMES -- because of ~/.appium folder where actual drivers/plugins/linking are stored.

That's right. With Appium 2.0, you can have many drivers which provide the same automationName, but you can only ever appium driver install one of them at a time. If somebody wants to namespace their driver, i.e., cbuelvasc:roku, that's fine, but it's not a requirement and since we can't enforce it I don't really see a reason for it.

[jlipps]

@dlenroc @cbuelvasc I'm a little bit confused on the existence of two roku drivers. Can someone give me an explanation of the two and how they are related?

In any case the nice thing about making these Appium 2.0 drivers is that there is no reason both can't exist. The community will just use the one that works better for them.

[jlipps]

@jlipps @mykola-mokhnach my inclination would be to say that this is an Appium 2.0 driver. At some point, if we want to review the code and make it an official Appium 2.0 driver (and I think we should have an official Roku driver implementation--maybe it's this one!), then we would simply want to update drivers.js in the 2.0 branch. But this PR to master doesn't need to exist IMO.

[dlenroc]

Initially, I developed a Roku driver for internal use (>1 year ago), and recently I decided to rewrite it in TypeScript and made it open-source as well as removing all commands that were not roku specific (ie to make it pure without android/ios/youiengine commands).

I was notified that someone mentions me in that PR, and after looking I found out that @cbuelvasc did a similar driver for his work (and is inspired by mine implementation), so I asked here if that will not cause conflicts on installation with Appium 2 or will need to choose a different name to avoids conflicts.

@jlipps Both realizations are totally unrelated and not offered by Roku company (official driver is not compatible with appium / webdriver protocol)

In case if Appium wants to make one of the realizations default or provide one, I'm ok with any decision

[cbuelvasc]

I do not fully understand the whole problem, I consider that both can coexist, my intention is not to compete with anyone, if you check my repo and the date on which it is created and the work I have been doing, it is already 9 months. I found out why someone here mentioned the @dlenroc driver

I clarify @dlenroc my diver is not inspired by its implementation, in fact, I have taken as a basis the driver https://github.com/appium/appium-android-driver they can say that it is a faithful copy of the structure and the naming of methods, variables, etc.

I have no problems if they decide or not to accept the PR, I understand that the issue of doing it by this means is complex, in fact, this is the first of three PR that I have, I had planned to upload the driver for LG TV WebOS and Tizen TV.

[cbuelvasc]

I have a proposal if it seems to everyone @mykola-mokhnach @jlipps @KazuCocoa especially @dlenroc, we can unify the driver so we join forces, I have all the availability to do it, we can follow the way in which the community drivers are already elaborated.

In this way, we all win including the community.

[dlenroc]

@cbuelvasc Yes we can, I looked at both your repos and seems that till Mar 25 you implemented the following appium commands (commands that can be used via appium clients)

getPageSource
sendKey
removeApp
installApp
launchApp 
isAppInstalled 
closeApp
+ create/deleteSession

⬆️ - those commands and even more are supported by my realization

and after that time you started to be inspire by my implementation (unfortunately not all taken commands/realizations was integrated correctly), so if you're agreed with that we can contribute to my repository (or you can continue with your version, it's up to you), or if Appium team want we may use those drivers as a base for an official driver provided under Appium.

Of course, if Appium wants, they can also merge this PR as well, I agree with any decision that is made.

[jlipps]

Thank you for this explanation. I think we will be agreeable to merging something in here to make it available for Appium v1 users, though I would like to promote it much more as an Appium v2 driver.

And yes, if @cbuelvasc and @dlenroc can work together to contribute to one repository and one driver, this will be much less confusing for the community, and will make this PR more sensible!

[cbuelvasc]

@jlipps thank you for your words and I agree, I have some doubts regarding being able to create a joint driver that is official and that both @dlenroc and I can work on each other, in case @dlenroc is willing for us to work together and Let's start from scratch:

@mykola-mokhnach @jlipps @KazuCocoa

1. Do you consider that it is appropriate that the structure is maintained following already created drivers?, That is, in my case I handle a structure very similar to appium-android-driver

2. What do you recommend using for the construction of the driver, vanilla Javascript or Typescript? I'm thinking of the community

3. Regarding the handling of names and distribution in appium and npm:

• For example: appium-roku-driver or @ something / appium-roku-driver
• For example: appium-ecp or node-roku, in my case take the name of the client that Roku exposes on its platform https://developer.roku.com/es-cr/docs/developer-program/debugging/external-control-api.md i.e. something like appium-adb equivalent to appium-android-driver

[KazuCocoa]

1. appium-base-driver is a root driver format. So, basically, you can follow it. A new one like https://github.com/appium/appium-mac2-driver may be easy to catch up with since it is simpler than existing Android/iOS ones.
2. We do not have. You can go in your preferred way.
3. Afaik, Appium 2.0 allows installing modules from git as well as npm. Their usage is also in package.json. So, for now, we don't restrict naming rules. But basically, Appium team will delegate or close issues in this Appium org repo which are not our maintained modules. So, please note that where users should report issues in your readme.
   e.g. https://github.com/truongsinh/appium-flutter-driver is a community-based driver.

[jlipps]

Do you consider that it is appropriate that the structure is maintained following already created drivers?, That is, in my case I handle a structure very similar to appium-android-driver

The structure doesn't have to mimic other drivers exactly, they have each evolved independently. Each driver should be as simple as possible to start and can go from there. As @KazuCocoa pointed out a simpler model to start from might be good.

What do you recommend using for the construction of the driver, vanilla Javascript or Typescript? I'm thinking of the community

I'm open to either, personally I'd love to see a move to typescript in new projects, I'm not sure of the typing requirements that this will cause, but if you also want to create typings for shared libraries like BaseDriver that could be really useful. I know that there will be others in the future who will want to create typescript drivers as well.

Regarding the handling of names and distribution in appium and npm

If it's a third party driver, appium-roku-driver. If it's a first party driver, then @appium/roku-driver. So it depends on whether we adopt this driver as an officially maintained appium driver.

For example: appium-ecp or node-roku

Either of these names would be OK, however what is the point in having it be a separate library? Is it that you would like to make it available outside of Appium? My recommendation would be to not create extra packages unless and until there is a concrete use case for breaking them out. And even then, it would be good to follow a monorepo format (see https://github.com/appium/appium-plugins). We are trying to get away from so many repos and packages.