Skip to content
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

1.0.0 refactor #36

Closed
3 tasks done
Lindsay-Needs-Sleep opened this issue Sep 8, 2019 · 21 comments · Fixed by #54
Closed
3 tasks done

1.0.0 refactor #36

Lindsay-Needs-Sleep opened this issue Sep 8, 2019 · 21 comments · Fixed by #54

Comments

@Lindsay-Needs-Sleep
Copy link
Collaborator

Lindsay-Needs-Sleep commented Sep 8, 2019

The largest changes of this refactor are:

  • iOS, Android, and Chrome Desktop all work the same now! (With a few caveats.)
    All 3 will pass the same set of tests included in the plugin.

  • iOS and Android have scanForRoutes, stopScan, and selectRoute to replace getRouteListElement

  • iOS and Android still do not implement the full chromecast API, but a few more have been added, and existing ones have been made to match chrome desktop behavior.

Bug Fixes:

Improvements:

  • Upgraded to CAF v3, this got rid of a huge amount of deprecation notices.
  • Added eslint tests to ensure uniform js code stlying
  • Added a style checker for java
  • Updated documentation on: how to run style checkers, how to run tests.
  • Added a pull request template which instructions prospective pull requesters to run eslint and the tests
  • Repaired and added to the existing tests enforcing chrome desktop SDK behavior.
  • On app initialize you will receive any previously started+active session. (This includes navigating to a new page, or force killing the app and re-loading.)
  • Implemented startRouteScan and stopRouteScan to replace getRoutes (see why)
  • fixed selectRoute for Android 4.4.2
  • Allow native error responses to send a json object "code" and "description" parameters in addition to just the error code string.
  • Implement basic queue functionality
  • Tests can run on chrome desktop browser as well (with a couple stubs to enable missing functionality)
    • This is super useful because you can write tests and check that they work correctly on chrome first. Which is very important when chrome's implementation is what we are implementing.
  • Added a basic code example
  • Updated readme with all kinds of instructions (differences between chrome, api list, plugin-specific api list, api usage example, support+quirks, setup, how to test, how to run formatting test)
  • Added pull request template
  • Added scripts to assist in enforcing code formatting for JS and Java

Auto-tests:

  • initialize
    • receiverListener
    • sessionListener
  • leaveSession
  • stopSession
  • loadMedia Video
  • media.setVolume
  • media.play
  • media.pause
  • media.stop
  • session.setReceiverVolumeLevel
  • session.setReceiverMuted
  • requestSession
  • cordova.startRouteScan
  • cordova.stopRouteScan
  • cordova.selectRoute
  • MediaMetadata (All types with most of their special params)
  • loadMedia Audio (Did not test playback controls)
  • loadMedia Image
  • load queue
  • jump to queue item

Manual Tests:

  • Start session, reload page, call initialize, do we get session again?
  • Start session, force kill app, restart app, do we get session again?
  • Start session, join session with additional device, leave session on original, session still continues?, force kill and restart original device, original device should not rejoin the session automatically.
  • for session Leave: probably need to join the session from an additional device,then leave the session, restart app, does it rejoin the session? (it shouldn't) (pre-req rejoin on restart must work)
  • for loadmedia from external sender: start session on device, join session on other device, loadmedia from other device, is session.addMediaListener triggered?
  • for external exit: Start session, kill session with other device, requestSession should show the the join dialog, not stopCasting
@Lindsay-Needs-Sleep
Copy link
Collaborator Author

Lindsay-Needs-Sleep commented Sep 8, 2019

My first question is:
Does anyone use ‘chrome.cast.getRouteListElement()’ method?
If no one does, I would like to skip it in my refactor because it is not strictly part of the chrome sdk.

Edit: Or, maybe if you use it, it might be worth switching to the default requestSession pop up? The popUP accepts a theme, and I have a TODO which could allow the user to specify the theme via config.xml. This would allow for custom styling. (Or we could just give the option between “light” and “dark” to make it platform independent.) I understand customizability is nice, but I think it is worth noting that big apps like youtube and netflix appear to use default popup (with a custom title for netflix).

This would be nice because then we could drop that functionality all together (which is kind of messy).

I have started on re-implementing this functionality, but it is buggy. You are only allowed to join a route once per app lifecycle. Additional attempts to join routes after the current cast session have ended result in a weird state where a cast session is started (you can see it on the tv), but it comes back as the “cast session could not be started due to timeout”, so you have no session to control.

What are all your thoughts?
@anthonylavado @lewazo @emilsas (and others too I may not have included o.o)

@dkanada
Copy link
Member

dkanada commented Sep 8, 2019

Added eslint tests to ensure uniform code stlying.

I'd like to move this project from tabs to spaces at some point since all our other repos use four spaces for indentation.

Upgraded to CAF v3, this got rid of a huge amount of deprecation notices.

I don't believe this has been done yet in the sibling repo. It has a pull request open to migrate to v3 but progress has mostly halted, so if you want to continue working on this I can open a new branch with @cvium's changes.

Does anyone use ‘chrome.cast.getRouteListElement()’ method?

I haven't looked in a while but I think that's what jellyfin-web uses to build the list of devices. I agree we should remove it, but we should remove its usage in the web client first if that's the case.

@lewazo
Copy link
Contributor

lewazo commented Sep 8, 2019

As I understand it, jellyfin-web does not use this plugin at all, it directly uses the Google SDK with their own chrome.cast object here https://github.com/jellyfin/jellyfin-web/blob/master/src/scripts/site.js#L348.

The web client never deals directly with the bookeeping of the routes since they are managed by the cast framework directly.

To answer your first question @Lindsay-Needs-Sleep I do not believe the Jellyfin project uses the getRouteListElement() method, since the jellyfin-android app is the only app using this plugin. But I'm a recent contributor so I might be clueless of other clients that might use it. Maybe AndroidTV? That would be surprising since it can run the native SDK (it does not uses cordova).

IMO, using the default, standard Android mediaChooserDialog is the way to go. It would make the bookeeping of routes way easier. And yes, this functionality is quite messy as it is.

@Lindsay-Needs-Sleep
Copy link
Collaborator Author

Lindsay-Needs-Sleep commented Sep 8, 2019

@dkanda and @lewazo
Thanks for the feedback!

I won't worry about supporting chrome.cast.getRouteListElement() then. (That's a relief!)

@dkanada

Added eslint tests to ensure uniform code styling.

I'd like to move this project from tabs to spaces at some point since all our other repos use four spaces for indentation.

All of the js files have been styled to 4 space indentation. The java files are tabs though (since they were already pretty consistently tabs). I can change that as well. It also seems to fit with the style used on other cordova plugins as well, so this makes sense.

Upgraded to CAF v3, this got rid of a huge amount of deprecation notices.

I don't believe this has been done yet in the sibling repo. It has a pull request open to migrate to v3 but progress has mostly halted, so if you want to continue working on this I can open a new branch with @cvium's changes.

Unfortunately, the company I am working for doesn't use the jellyfin receiver, so I don't think I will be able to help here. Perhaps once my changes are a little further along we can open a branch on this repo (eg. 1.0.0)? Then include instructions in the master branch readme about how to install the newer version if desired. This will allow other cordova dev's to more easily choose the version if they want? I'm not sure what the best method is for supporting multiple major versions. At lest until jellyfin is ready to update.

@Lindsay-Needs-Sleep
Copy link
Collaborator Author

My next question is(!):

Does anyone know of a publicly hosted video which provides a non-expiring, direct link to the mp4 stream?

The video link that was in the tests didn't work. I added a very small video to the project for the tests, but it requires you start a server to host it (since the plugin does not support playing local files). I would like to change this. The extra step for testing will be annoying for other devs.

@anthonylavado
Copy link
Member

@Lindsay-Needs-Sleep What kind of video stream do you need?

@Lindsay-Needs-Sleep
Copy link
Collaborator Author

Lindsay-Needs-Sleep commented Sep 9, 2019

Any kind that the plugin should support! Doesn't matter to me!
Just something I can pass to chrome.cast.media.MediaInfo(videoUrl, 'video/mp4')

Unfortunately all the videos I have access to for work are private, so I can't use them. I have tried extracting the mp4 link from youtube and vimeo videos using vlc player, but they have expiration dates. I also don't have access to a server to properly host a video.

Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 9, 2019
(So I changed all the tabs to spaces in the java files.  Sorry commit history. o.o)
It was a lot of work, so I disable a couple of the java checkstyle rules.  They are listed in the first comment in check_style.xml.
Issue jellyfin-archive#36 progress
@Lindsay-Needs-Sleep
Copy link
Collaborator Author

I am also wondering where the most appropriate place is to log TODOs? (the kind of TODOs that I won't be able to get to for maybe months, but don't want to forget). Should I open issues for each?

eg. implement queues, deprecation notices, enable and satisfy java styling rules I temporarily disabled (because there were just way too many nit-picky ones and I just don't have that much time atm. >.<)

Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 9, 2019
finally got the tests to early terminate on error (It's ugly.  We should consider switching to mocha.)
Work done for Issue jellyfin-archive#36
@anthonylavado
Copy link
Member

Yeah, Issues are best.

I’ll see what I can do on the video URL space. Sounds like something from the Internet Archive would work.

Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 9, 2019
… selecting the route do work. So I am going to commit so that they are in the history.

Also removed the private _sessions array in chrome.cast.js.  The app side can only ever maintain 1 session, so it is strange for the client side to track multiple sessions. (If it does have multiple sessions, all but the latest will be dead to it anyways.
Relevant to Issue jellyfin-archive#36
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 11, 2019
…as been force quit and restarted. This was rough.

Added defaults to _sessionListener and _receiverListener.
Issue jellyfin-archive#36
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 11, 2019
…as been force quit and restarted. This was rough.

Added defaults to _sessionListener and _receiverListener.
Issue jellyfin-archive#36
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 11, 2019
@Lindsay-Needs-Sleep
Copy link
Collaborator Author

I think I will keep the top post as kind of progress tracker. But so that we have history and conversations make sense, here is what I just updated it to:

I am working on a refactor that simplifies a few things and much more closely follows the behavior of the desktop chrome SDK.

Done:

Tests that need to be written:

  • (manual test) Start session, force kill app, restart app, do we get session again?
  • (manual test) Start session, join session with additional device, leave session on original, session still continues?, force kill and restart original device, original device should not rejoin the session automatically.
  • (auto-ish) Everything else to do with media and playback.

Working on:

  • Adding to the test suite as I fix features
  • Confirming media functionality
  • Remove getRoutes functionality (I just worked so hard on it, I am reluctant to remove now. :p)

Could use help on:

  • Adding to the test suite. It would be nice if someone could write a couple more tests for untested but used functionality. (Since I don’t know which parts of the plugin everyone uses.)
  • I have basically no experience with iOS, so all of that. (Make sure it still works with slight javascript changes, and passes all the tests.)
  • getting a hosted video url for tests
    )

TODO later (for 1.0.0):

  • finish confirming pre-existing functionality

TODO later (post 1.0.0)

Opinions?

@anthonylavado
Copy link
Member

Tagging @emilsas and @ghenry22, who use the iOS side more. Any input?

@anthonylavado
Copy link
Member

@Lindsay-Needs-Sleep There should be something you can use for testing here: https://archive.org/details/CosmosLaundromatFirstCycle

Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 12, 2019
- following the chrome SDK behavior for setVolume more closely
- need to trigger sessionUpdate on receiver volume change
- added method which accepts Integer for setMediaVolume (in case client puts exactly 0 or 1)
Allowed null arguments to be received on the Android side
- since all our android method entry points accept nullable objects this should be allowed
- in particular, this allows us to have one setMediaVolume (instead of setMediaVolume and setMediaMuted) to accept null args which mean "don't update the level/mute state"
Session handling fixes
- fixed how session.status is added to session
- fixed leave session (was passing true to endSession, oops)
- changed "SESSION_ERROR" instances in android to "session_error" to match the SDK directly
Cleaned up and improved tests
- more tests checked for accurate status updates
- made some tests more readable with callOrder
Cleaning
- removed ChromecastConnection.Callback since it was exactly the same as Runnable, just use that.
Streamlined the session end event callback process
- Chromecast.java has only one location which triggers the client's session update listener (with end status)
- ChromecastSession.java detects external reasons for disconnect with onApplicationDisconnected (eg. timeout and someone else stealing the cast)
- ChromecastConnection.java detects self-triggered reasons for disconnect (eg. sessionEnd, sessionLeave)
  (in which case it notifies ChromecastSession.java that a session has ended, which then tells Chromecast.java)
Simplified receiverListener
- just pass a boolean to _receiverUpdate
Untested fix:
- maybe fixed the message functions and updates (needs a test)

Issue jellyfin-archive#36 (1.0.0 development)
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 12, 2019
@Lindsay-Needs-Sleep
Copy link
Collaborator Author

(@anthonylavado Thanks for the link, the video works great! )

Here are changes made to first comment to reflect recent progress:

Has good (provides decent coverage) tests:

  • initialize
    • receiverListener
    • sessionListener
  • leaveSession
  • stopSession
  • media.setVolume
  • session.setReceiverVolumeLevel
  • session.setReceiverMuted
  • requestSession
  • cordova.startRouteScan
  • cordova.stopRouteScan
  • cordova.selectRoute

Tests that need to be written/improved:

  • (manual test) Start session, force kill app, restart app, do we get session again?
  • (manual test) Start session, join session with additional device, leave session on original, session still continues?, force kill and restart original device, original device should not rejoin the session automatically.
  • loadMedia (what update listeners should be triggered, and with status changes?)
  • play (what update listeners should be triggered, and with status changes?)
  • pause (what update listeners should be triggered, and with status changes?)
  • stop (what update listeners should be triggered, and with status changes?)
  • messages (no tests at all)
  • (probably missing some others. Load images or audio?)

@Lindsay-Needs-Sleep
Copy link
Collaborator Author

I think I have kind of changed my mind about the getRoutes functionality, here's why:

  • It appears that at least some people are interested in the getRoutes function (chrome.cast.getRouteListElement() returns empty object on iOS #22)
  • I do think it could be nice for users who want to display with html to keep their app look consistent.
  • It is useful for the automated tests. Without it, there is no way to start a session without user interaction.
  • I have already tested it and it is working (Might as well keep it right? :p)

@anthonylavado @lewazo What do you think?

@lewazo
Copy link
Contributor

lewazo commented Sep 13, 2019

The freedom you get from being able to customize the dialog how you want using HTML and CSS is a very nice feature IMO. Like you said, it's already there and it works. Not much would need to be done in order to keep it working if modifications are done since it relies mostly on the routeAdded callback which is called directly by the framework.

Might as well keep it!

@anthonylavado
Copy link
Member

@Lindsay-Needs-Sleep I would say @dkanada and/or @thornbill are the better resources to ask. They’ll know more towards our usage of the plug-in in the Jellyfin Android app.

Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 15, 2019
…o switch to MediaRouter.selectRoute()

Add isNearbyDevice to routes output from startRouteScan (mostly so testing can avoid attempting to join these routes, since they require manual input.  But could be useful to the user as well.)
Remove unnecessary chrome.cast.js global vars.
Added polyfill for Promise to tests since Android 4.4.3- does not have Promise natively.
Contributing to Issue jellyfin-archive#36
Relevant to Issue #22
@Lindsay-Needs-Sleep
Copy link
Collaborator Author

@lewazo I should mention that I have changed the getRoutes functionality to these functions instead:
cordova.startRouteScan
cordova.stopRouteScan

The reasoning is here: #22 (comment)

Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 15, 2019
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 15, 2019
…e are emitting twice each update. In the case that we are creating a new media object, it is impossible for there to be any listeners on it yet, so no point in emitting.

Towards issue jellyfin-archive#36
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 15, 2019
Since we are keeping scanForRoutes, might as well use it while checking for available receivers.
Make the JSON creator functions static so they aren't state dependent.
Issue jellyfin-archive#36
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 15, 2019
Add isCastGroup to routes returned to startRouteScan.
Relevant to Issue #22
Improvement for issue jellyfin-archive#36
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 15, 2019
@Lindsay-Needs-Sleep Lindsay-Needs-Sleep changed the title Android plugin refactor, fix many issues 1.0.0 refactor Sep 17, 2019
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 22, 2019
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 22, 2019
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 26, 2019
… with "code" and "description" parameters instead of just the error code string.

Make selectRoute use the full error object.
Towards issue jellyfin-archive#36
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 28, 2019
startRouteScan also has it's error callback called with a 'cancel' error when the scan is stopped.
update documentation
Issue jellyfin-archive#36
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 28, 2019
…fail test. Added the receiver volume tests and set up the session describe.

Issue jellyfin-archive#36
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Sep 29, 2019
Do this because a route scan may have been triggered, then the user navigated away from the page before stopRouteScan was called.
The new page load will call setup and stop any scan.
Issue jellyfin-archive#36
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Oct 1, 2019
…sting auto tests to mocha and misc improvements/fixes.

Fixed some bugs:
    - typo in ChromecastUtilities "CANCELED"
    - session building, in particulate the media array was not being built correctly

Improvements:
    - Improved many of the tests to be more specific and encourage following desktop chrome behavior more closely
    - Update the readme with more accurate information/documentation/and info about Mocha tests
    - Improved the utils functions (callOrder and waitForAllCalls)
    - Added pre-checks to match chrome behavior.  (Checks for existing session and valid sessionId)
    - followed the _update pattern completely in chrome.cast
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Oct 1, 2019
…sting auto tests to mocha and misc improvements/fixes.

Fixed some bugs:
    - typo in ChromecastUtilities "CANCELED"
    - session building, in particulate the media array was not being built correctly

Improvements:
    - Improved many of the tests to be more specific and encourage following desktop chrome behavior more closely
    - Update the readme with more accurate information/documentation/and info about Mocha tests
    - Improved the utils functions (callOrder and waitForAllCalls)
    - Added pre-checks to match chrome behavior.  (Checks for existing session and valid sessionId)
    - followed the _update pattern completely in chrome.cast
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Oct 4, 2019
Fix Issue jellyfin-archive#23 for Android and part of #3
Also some better null checking.
And better images handling.
Add RepeatMode to media output.
Issue jellyfin-archive#36
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Oct 17, 2019
Move some object generation methods to Utilities
Disable method length rule.  We are grown ups and can decide if we want to use ridiculously long methods or not. :)
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Oct 19, 2019
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Oct 22, 2019
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Oct 22, 2019
Lindsay-Needs-Sleep added a commit to miloproductionsinc/cordova-plugin-chromecast that referenced this issue Oct 22, 2019
@dkanada
Copy link
Member

dkanada commented Oct 31, 2019

So you have definitely changed a lot of things, but it would still be great to have them all in master. Do you have a plan for that other than a huge pull request?

@Lindsay-Needs-Sleep
Copy link
Collaborator Author

Lindsay-Needs-Sleep commented Oct 31, 2019

Just a big pull request is all I was thinking.

I’m just not sure if I should create the pull request now or after iOS is complete. (Because iOS is definitely going to 100% broken after all my changes.)

My company has hired someone to get iOS up to speed. But it is a lot of work. I’m not sure if they would be open to assistance or not. (Also depending on if assistance is available?)

I could start the pull request sooner as well if people want to be able start working off of this sooner. I am definitely open to suggestions on how to proceed.

@dkanada
Copy link
Member

dkanada commented Oct 31, 2019

If you could do some of the smaller items in separate PRs that might help get your changes through faster. Things like the testing framework change or the PR template should be easier to review. We might want to add a CI job here to build every PR before it gets merged, if you're familiar with Azure I can add the job to our pipeline.

@Lindsay-Needs-Sleep
Copy link
Collaborator Author

Lindsay-Needs-Sleep commented Oct 31, 2019

I don’t think I will have time to pull it all apart into different PRs.

Pulling out the PR template wouldn’t be too bad. But separating the test framework change would be huge, and would destroy the commit history, (which, in this case, could offer useful insight for answering future questions of “why was it done like this?”)

I am not familiar with Azure or implementing CI jobs. That does sound like a good idea though!
How would it handle testing a chromecast device though? Only like the 1st 2 tests will pass without a valid chromecast on the network.
The styling stuff should work though I guess!
Or would it just test that plugin builds without errors for a cordova ios/android project?

anthonylavado pushed a commit that referenced this issue Feb 9, 2020
* Update cast library so that it doesn't conflict with the androidx libraries.
Fixes issue#26 #26

* Update cast library so that it doesn't conflict with the androidx libraries.
Fixes issue#26 #26

* Move js files to www to follow the standard-ish plugin format

* Add eslint test (so that the code can have a unified [standard-ish] style)  (Will satisfy the eslint test in another commit.)

* Satisfy ESLint.  (To [commit history], I'm sorry.)

# Conflicts:
#	tests/tests.js
#	www/chrome.cast.js

* Updated the jasmine tests to use the newer cordova-plugin-test-framework version. (Most tests fail.  Will try to fix in another commit.)

# Conflicts:
#	README.md
#	tests/tests.js

* Switch to MediaRouteChooserDialog to simplify the dialog building process. (Chromecast.java)
We shouldn't prevent requestSession from going through if there is no receiver.  It handles this fine.  It just shows that it is searching for a chromecast.  This follows the behavior of chrome browser cast SDK. (chrome.cast.js)
Remove unused imports. (Chromecast.java)
Try to be a bit more safe with the threads. (Chromecast.java)
Just pass the Chromecast instance to the callback on initialization.  (Chromecast.java and ChromecastMediaRouterCallback.java)

* We should not be logging to javascript all the time.  It is an abuse of power and annoying.  Use the cordova log utility instead.

* WIP: Got the test framework to run.  Basically all the tests are broken though.
Updated the README.md with instructions on how to run these tests.
We shouldn't load the tests.js file when the user is using the plugin for production use.
We shouldn't include the test framework as a dependency.  (Even a dependency of the test plugin.)
Added a pull_request_template.md

* WIP: Got the first couple tests working.
Managed to match the desktop chrome SDK behavior regarding receiver updates on start up.
Added some additional tests to cover more of the basic situations with receiver updates.
Simplified all the get route stuff (will fix it up properly later)

* WIP: Got request session somewhat working
capture routeUnselected reason

* WIP: Add a video that will actually play and fix the load media test

* .gitignore update

* Got all the tests to pass

* remove receiverAvailable because it does nothing
stop spamming emaiAllRoutes

* Version bump to 1.0.0

* Should make sure assets are in the plugin folder so that we don't pollute the top level of www

* reject and catch don't work great because jasmine does not give very good stack trace.  So remove them and check error as soon as possible to the criminal code.

* Update package.json to match licensing

* Fix on requestSession cancel handling to match the documentation and chrome desktop chromecast behavior.  We are supposed to receive a callback to the error handler.  Not have it silenced.  This is important if you have started connection animations.  If you never receive a callback you don't know when to stop the animations, and whether to transition back to the unconnected state or to a connected stated.  If you wish to ignore the cancel error, just filter it out in your error callback. eg. if (err.code === 'cancel') { //ignore }

* Forgot to remove unused references to receiverAvailable related vars.

* Update readme

* Integrate receiverListener test into initialize.  Having cross-test dependencies is not great.  Though, we are on our way to a monolith test this way.  :/  Pick our poison I guess.

* rename for next commit

* Update to cast v3 sender to remove deprecated functions.
Session is automatically "rejoined" when the webview goes to a new page and calls initialize (this follows chromecast desktop sdk)
Do not call sessionListener on requestSession success (this does not follow chromecast sdk desktop behavior)

* Added a java file style checker that enforces 4 spaces.
(So I changed all the tabs to spaces in the java files.  Sorry commit history. o.o)
It was a lot of work, so I disable a couple of the java checkstyle rules.  They are listed in the first comment in check_style.xml.
Issue #36 progress

* Added stop casting dialog.
finally got the tests to early terminate on error (It's ugly.  We should consider switching to mocha.)
Work done for Issue #36

* Apparently my changes for scanning for routes, stopping the scan, and selecting the route do work.  So I am going to commit so that they are in the history.
Also removed the private _sessions array in chrome.cast.js.  The app side can only ever maintain 1 session, so it is strange for the client side to track multiple sessions. (If it does have multiple sessions, all but the latest will be dead to it anyways.
Relevant to Issue #36

* fix createAppImagesObject

* Added tests for session Leave and session Stop.  Cleaned up their functionality to match chrome desktop SDK a bit more.
Add names to constructors in chrome.cast.js to improve out in test.js test().toBeDefined()

* Added package.json script to automatically fix the js errors it can.

* Got the plugin to automatically resume sessions after being the app has been force quit and restarted.  This was rough.
Added defaults to _sessionListener and _receiverListener.
Issue #36

* Should technically mark the package version as a "dev" version until completely stable.

* (android) Make the constants match the desktop chrome SDK.
Issue #36

* Fixed all kinds of problems with setVolume (media and receiver)
- following the chrome SDK behavior for setVolume more closely
- need to trigger sessionUpdate on receiver volume change
- added method which accepts Integer for setMediaVolume (in case client puts exactly 0 or 1)
Allowed null arguments to be received on the Android side
- since all our android method entry points accept nullable objects this should be allowed
- in particular, this allows us to have one setMediaVolume (instead of setMediaVolume and setMediaMuted) to accept null args which mean "don't update the level/mute state"
Session handling fixes
- fixed how session.status is added to session
- fixed leave session (was passing true to endSession, oops)
- changed "SESSION_ERROR" instances in android to "session_error" to match the SDK directly
Cleaned up and improved tests
- more tests checked for accurate status updates
- made some tests more readable with callOrder
Cleaning
- removed ChromecastConnection.Callback since it was exactly the same as Runnable, just use that.
Streamlined the session end event callback process
- Chromecast.java has only one location which triggers the client's session update listener (with end status)
- ChromecastSession.java detects external reasons for disconnect with onApplicationDisconnected (eg. timeout and someone else stealing the cast)
- ChromecastConnection.java detects self-triggered reasons for disconnect (eg. sessionEnd, sessionLeave)
  (in which case it notifies ChromecastSession.java that a session has ended, which then tells Chromecast.java)
Simplified receiverListener
- just pass a boolean to _receiverUpdate
Untested fix:
- maybe fixed the message functions and updates (needs a test)

Issue #36 (1.0.0 development)

* Got a remote video for testing instead!  Wonderful!  Thanks @anthonylavado!
Issue #36

* SessionManager.startSession did not work reliably on android 4.4.2, so switch to MediaRouter.selectRoute()
Add isNearbyDevice to routes output from startRouteScan (mostly so testing can avoid attempting to join these routes, since they require manual input.  But could be useful to the user as well.)
Remove unnecessary chrome.cast.js global vars.
Added polyfill for Promise to tests since Android 4.4.3- does not have Promise natively.
Contributing to Issue #36
Relevant to Issue #22

* Fixed up mediaLoaded.  It should be called whenever a new media is loaded, even by an external sender.
Towards issue #36

* _mediaUpdated event is already fired by Media.prototype._update, so we are emitting twice each update.  In the case that we are creating a new media object, it is impossible for there to be any listeners on it yet, so no point in emitting.
Towards issue #36

* Only ChromecastSession needs to hold a reference to session.
Since we are keeping scanForRoutes, might as well use it while checking for available receivers.
Make the JSON creator functions static so they aren't state dependent.
Issue #36

* (android) (Refactor) Move JSONObject creator functions to utilities.

* (android) (Refactor) Decouple the 3 way dependency between Chromecast.java, ChromecastConnection.java, and ChromecastSession.java a little bit.
Move more JSON creation methods to utilities (createSessionObject with state, and createRoutesArray).
connection.selectRoute does not use routeName anymore.
Chromecast.java shouldn't know about CastSession if we can avoid it.

* Improve selectRoute with some error handling.
Add isCastGroup to routes returned to startRouteScan.
Relevant to Issue #22
Improvement for issue #36

* Improve loadMediaVideo test
Issue #36

* Add test to check for video finished state

* (android) Remove sendJavascript and replace with an event callback that is set during setup.
Part of issue #36
Fixes issue #41 for android

* Update package script to have the standard "npm test"

* Move the Api isAvailable check to a central location

* (android) Add some notes and error detection for Android 4.4 mysterious, intermittent failure to create a session during selectRoute.

* Add check to make sure initialize has been successfully called before most functions are able to work.

* Should make sure _session exists before trying to use it since this event can be called at any time

* Improvements for selectRoute and requestSession.
Make unique callbacks for selectRoute and requestSession.
Enable retrying to join a route for selectRoute under certain situations.  See issues:
#49
#48

* Improve error handling to allow native responses to send a JSONObject with "code" and "description" parameters instead of just the error code string.
Make selectRoute use the full error object.
Towards issue #36

* Protect against user passing null as _receiverListener or _sessionListener

* Improved startRouteScan/stopRouteScan to handle multiple calls better.
startRouteScan also has it's error callback called with a 'cancel' error when the scan is stopped.
update documentation
Issue #36

* (android) Remove e.printStackTrace when building JSONObjects.
Since we expect those exceptions to be thrown often they are basically just annoying.  If a user has experiences unexpected responses they won't help immediately anyways (since there are so many and so frequent).

* Kill any route scan that may be going on when setup is called.
Do this because a route scan may have been triggered, then the user navigated away from the page before stopRouteScan was called.
The new page load will call setup and stop any scan.
Issue #36

* Only update session.addMediaListnener (MEDIA_LOADED event) when an external sender loads media.
Handle PLAYER_STATE_LOADING, returning buffering is better than unknown.
Remove calls to super because they do nothing.
session.getApplicationMetadata() throws IllegalStateException if called before connected, catch it like the others.

* Issue #50 Issue #36 Converted the existing auto tests to mocha and misc improvements/fixes.

Fixed some bugs:
    - typo in ChromecastUtilities "CANCELED"
    - session building, in particulate the media array was not being built correctly

Improvements:
    - Improved many of the tests to be more specific and encourage following desktop chrome behavior more closely
    - Update the readme with more accurate information/documentation/and info about Mocha tests
    - Improved the utils functions (callOrder and waitForAllCalls)
    - Added pre-checks to match chrome behavior.  (Checks for existing session and valid sessionId)
    - followed the _update pattern completely in chrome.cast

* Added ability to run tests on chrome to confirm that functionality matches chrome desktop behavior.

* Fix chai.js, fix typos in readme, fix css import, rename runner since it is actually just the custom reporter

* Fixed up Metadata, added some tests.
Fix Issue #23 for Android and part of miloproductionsinc#3
Also some better null checking.
And better images handling.
Add RepeatMode to media output.
Issue #36

* Issue #23 Fix up the metadata to handle more than strings (ChromecastSession and ChromecastUtilities).
Provide better translations between desktop chrome metadata names and the android names (ChromecastUtilities).
Add tests for loading audio and images.
Should manually trigger media update event after media load because sometimes the MEDIA_UPDATE event fires before the client is able to get a reference to media to addUpdateListener.

* Include Mocha fix PR #4051 because I just tried it out and it's nice (fixes a bug with filtering by passes/failures).

* Fix tests html, and improve utils output of callOrder functions a bit

* Fix crash because of NullPointerException. miloproductionsinc#3

* Added base queue functions.  Towards issue #36
Move some object generation methods to Utilities
Disable method length rule.  We are grown ups and can decide if we want to use ridiculously long methods or not. :)

* Improve tests styling a bit

* In tests_auto.js improve error output and move setup items to setup.

* Added beginning of manual tests (requestSession)
Towards issue #36

* Added the restart app / rejoin session manual test
Removed test skipping/memory for manual tests
Move some functions to utils so we can use them from multiple locations

* Added more manual Tests.  Added all the major ones where multiple devices interact via the same session.
Towards issue #36

* finished auto tests

* Added a bit more specific instructions to manual tests

* Auto tests are fixed

* manual and auto tests completed

* Auto and manual tests completed

* auto and manual tests are finished

* Fix bug.  Js failure if trying to unset media on non-existent session.

* Add tests that specifically test that initialize will return any active session on new page loads.  (Or will not return a session that has been left (session.leave).)  Keep track of test progress through page reloads and app restarts via cookies.

* Improve manual instructions a bit and fix forgotten closing quotes in instruction.

* Apparently ios deletes cookies during app restart. So switch to localstorage to persist the data through the app restart tests.

* Move the sessionRejoin event to after the initialize success callback.  The order of events for initialize when a session is present should be:
1) initialize callback success
2) RECEIVER_LISTENER (false)
3) RECEIVER_LISTENER (true)
4) SESSION_LISTENER (session)

* Give slightly longer timeout to accommodate slower devices

* Fix session being returned twice on app restart / session rejoin

* Fix for iOS 9.  iOS 9 does not support [NSTimer scheduledTimerWithTimeInterval].  Also clean up start/stop scan and selectroute.  Make it so that startScan has a hope of sending updates as available routes change.  selectRoute now also actually tries for 15 seconds.

* Style changes.  Xcode is determined to do this.

* (ios) ensure that the scan is running while attempting to selectRoute.

* Make manual test buttons bigger for ios.

* Forgot to apply some button ids

* Updated readme with api description and example js

* version update 1.0.0

* (ios) Added instructions for meeting ios app distribution requirements

* Update google cast library to 4.4.6 which supports iOS 9.0+

* (ios) remove unused emitAllRoutes

* [non-change] typo and empty lines change

* (ios) we don't use sendJavascript anymore

* (ios) Move device list building to utilities. Simplify receiverListener methods.

* (ios) media must be an array when returned in session

* (ios) decrease cast utilties code duplication

* (ios) fix startTime and preloadTime, check for invalidTimeInterval and NaN

* (ios) Let ChromecastSession handle all the rejoining logic (also change ChromecastSession initialization to happen only ).  Create rety function.  Store appId in user preferences.  In ChromecastSession, make currentSession a private variable.

* (ios) Modify the way scans keep track of whether or not to stop a scan.  make scanCommand a private variable.

* (ios) [no change] Remove sendScan (forgot to remove on commit "(ios) Move device list building to utilities.")

* (ios) [no change] Remove createSessionObject (the one with no "status" parameter) (forgot to remove on commit "(ios) Let ChromecastSession handle all the rejoining logic...")

* (ios) create one endSession function

* (ios) try to join selected route for 5 seconds

* (ios) WIP remove isAlive param, it is never used in these contexts, just passed around

* (ios) Add CDVPlugin event onReset so that we can stop scans from running when we have changed/refreshed the page

* (ios) WIP fix requestSession bug where it was not getting the devices (because of commit "Move device list building to utilities. Simplify receiverListener methods.")

* (test) Ensure that chromecast is initialized in before of describe test block

* (ios) WIP Calculate the session's status except for the case when we are doing session.leave

* (ios) WIP Added endSessionWithCallback to reduce code duplication by allowing requestSession's stop casting option to use the same endSesssion controls

* (test) WIP session.stop should happen in a specific order

* (test) WIP Remove some unintended copy pasta

* (test) (ios) WIP ensure there is no idelReason when media is active

* (ios) WIP remove isRequesting and create slighlty unified requestDelegate creators to avoid code duplication

* (ios) WIP fix/unify setMediaVolumeWithCommand so that we have just one function for simplicity and so that it does not return until both the volume and mute state have been updated

* (ios) WIP Add createLoadMediaRequestDelegate to unify the load request delegates.  Use didReceiveQueueItemIDs to detect when new media has loaded (external and internal).  Use queueFetchItemsForIDs to ensure that the media items data does exist before calling the loadMediaCallback.  Remove unused remoteMediaClientListener events.

* (ios) WIP Use lastMedia to keep track of when the next item in a queue begins playing and to send a manual MEDIA_UPDATE event showing the previous media as finished

* (ios) WIP Switch to using isQueueJumping to determine the reason for the queue advancing to another item

* (ios) WIP Allow storing multiple callbacks for when the session ends.  Also don't call the endSessionCallbacks until the session has actually ended (but before the SESSION_UPDATE event).

* (ios) WIP Forgot to commit these changes with their relevant commit.  (Sorry, did a huge amout of fixes and then tried to make commits for each section of change, to have a slightly more informative git history.  But it is difficult...)

* (ios) WIP fix crash when textTrackStyle.backgroundColor does not exist

* (ios) WIP Fix contentID output.  For queues, only contentURL is available, but for loadMedia media, only contentID is available

* (ios) WIP bug fix, selectRoute should only return the "Leave/Stop current session before selecting" error message when a current connected session exists

* (ios) WIP sometimes initialize -> tryRejoin can happen before didEndCastSession can update/erase the currentSession.  This just provides some additional protection against that.

* (ios) WIP For some reason didResumeCastSession is called randomly even once we already have the session. (Maybe it temporarily/very quickly loses and regains connection).  Whatever the reason, if we already  have the session, we shouldn't trigger the SESSION_LISTENER event. (Also no point in "setSession" either, since it is the same session.

* (ios) WIP detect join session fail and return an error

* (test) WIP fix test "ueue should start the next item automatically when previous one finishes" so that it does not re-use var i (causing issue if we need to hit the media listener more than once).  And make it so that the current play time of the item must be close to it's start time once it is reported as playing.

* (test) WIP fix copypasta

* (test) WIP allow requestSession's stop casting option to trigger the update listener and the success/cancel event in any order.  (Potentially it can happen in either order for chrome desktop since the success/cancel event happens when the casting dialog hides.  Theoretically it is possible to have the dialog hide before the session ends.)

* (test) WIP auto tests should have a before which ensures that initialization has occurred so that we can use ".only"

* (test) WIP wrap media functions in a describe to improve ".only" use for testing just media functions

* [no change] formatting. Change indent for tests required from last 2 commits.

* [no change] satisfy eslint and miniscule performance boost

* (ios) Have the plugin start onload.   This is supposed to be required to make session resume after app restart work correctly.

* (ios) Fix issue miloproductionsinc#5  Added tests for the issue.

* (ios) [no change] remove unused function

* (test) format the test homepage to look a bit nicer

* (test) Do not require the receiver updates for the before initialize (because chrome broswer will not send receiver updates on the second call), also it is nice for the before to be simple since it is not meant to be testing initialize.

* (ios) Fix communication problem with Android.  Android still requires the contentID to be set on media items (ios says it's deprecated), but without it, android counts ios loaded media messages to be malformed.  Maybe Android needs to update the google cast library.  Also removed some code duplication.   Had 2 methods that were both building almost identical MediaInformation items.

* (test) allow media after resume to be buffering or playing

* (android) Improve/simplify queue item handling

* (android) Add a try catch on initialization so that devices incapable of using chromecast do not crash when this plugin is included

* (test) Change audioUrl because old one is gone.  Fix readme typo.

* Improve queueReloadCallback handling.  Unset it when an error occurs.  Only set it for external loads that have more than 0 items.

* (android) remove annoying e.printStackTrace's for invalid (but optional) JSON parsing when creating a MediaInfo object

* Added 2 tests:
1) Ensure that we can get media updates after automatically rejoining a session after page reload and app restart (test_manual_primary_1.js) (second part of miloproductionsinc#5)
2) Ensure that you can't issue media control commands to an outdated media object after a new media object has been loaded (test_auto.js)

To fix #1: (chrome.cast.js) _currentMedia object was removed to ensure that we only send mediaUpdate events to the media stored in the session object.  All media object management is handle through the session object as well now.

To fix #2: (chrome.cast.js) we make sure to invalidate the media.mediaSessionId whenever new media is detected.  For chrome desktop, the new media will always have a different mediaSessionId, but for us, (on android at least), newly loaded media will have the same mediaSessionId, hence the invalidation of the previous media's mediaSessionId.  Additionally, in the case of new media, the old media object stored in session is removed and a new one added (instead of just updating like we were doing previously).

* (ios) build returned objects line-by-line instead of all at once to ease debugging (better stack traces)

* Handle invalid application receiver ids better

* (test) Improve duration checking

* (android) queues should only load 2-3 items into media.items.  If current item index is i, you load items [i-1, i, i+1] *exclude i-1 if it is < 0, and exclude i+1 if it is >= queue.length.  (aka. don't wrap the items.)  This is to match chrome desktop behavior.

Fixes a bug where larger queues (maybe 20+) result in a loop and queueLoad never returns.  This is also safer because it should not result in a crash due to eating all the memory for extremely large queues.

* (ios) fix orderId on queue itmes

* Improve error output from waitForAllCalls

* [no change] just a little cleanup, return nothing instead of return false for successfull preChecks (chrome.cast.js)

* (ios) Fix miloproductionsinc#6  This should have been fixed with miloproductionsinc#5 but the test that covered this case was effectively skipped.  So the test now works properly and you can control the media after an app restart.

* Add a little protection from js errors if a media update event is sent before the client has received the session.  (Can happen on occaision when resuming a session on page reload or app restart).

* (ios) move retry utility function to CastUtilities

* (test) should have a timeout for this test

* (ios) Fixes miloproductionsinc#8
ios does not have mediaStatus on didResumeCastSession if initialize is called immediately after app restart.  So retry for 2 seconds to see if we can get any media.  If not, we will assume the session does not have any media loaded.

* (ios) remove NSLog's

* (ios) (wip) remame objc files to have a namespace

* (ios) Fix imports and usage after file + class rename

* (ios) improve error handling for loadMediaCallback

* Fix miloproductionsinc#9  Try to keep the media up to date by forcing it to be an empty array whenever media doesn't exist (instead of setting mediaSessionId to 'invalidated').

Co-authored-by: Sel-en-ium <[email protected]>
Co-authored-by: annenkovaa905 <[email protected]>
dkanada added a commit that referenced this issue Feb 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants