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

Remove pycryptodome requirement for Android TV #22552

Merged
merged 2 commits into from Apr 6, 2019

Conversation

JeffLIrion
Copy link
Contributor

@JeffLIrion JeffLIrion commented Mar 30, 2019

Description:

This removes the pycryptodome requirement from androidtv.

Originally, I wanted to implement the set_volume_method, but this proved to be more difficult than expected, so I changed the focus of this pull request.

Fixes issue: #22726, #22769

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.

@ghost ghost added the in progress label Mar 30, 2019
Copy link
Contributor

@dgomes dgomes left a comment

Choose a reason for hiding this comment

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

Looks good, but supporting library must be released first

@JeffLIrion
Copy link
Contributor Author

Looks good, but supporting library must be released first

I know, I just want to make sure that this works as expected before publishing androidtv 0.0.15 on pypi.

@JeffLIrion JeffLIrion changed the title WIP: Improved volume support for Android TV Remove pycryptodome requirement for Android TV Apr 4, 2019
@JeffLIrion
Copy link
Contributor Author

@dgomes the tests should pass, and it does say:

ci/circleci: build — Your tests passed on CircleCI!

But 5 tests say they are still expected. Is this a bug with Circle CI? Or do I need to rebase? Or is it something else???

@dgomes
Copy link
Contributor

dgomes commented Apr 4, 2019

We are undergoing major code rebase, I would advise you to rebase.

@JeffLIrion
Copy link
Contributor Author

JeffLIrion commented Apr 5, 2019

@dgomes

Am I supposed to modify the manifest.json file?

Should I do that in a separate PR so that this PR can get merged into the 0.91.1 release?

@JeffLIrion
Copy link
Contributor Author

Could we please merge this in so that #22726 can be closed. And if there's going to be a 0.91.2 release, it would be nice if this fix could be included.

@dgomes dgomes added this to the 0.91.2 milestone Apr 6, 2019
@dgomes dgomes merged commit a747eaa into home-assistant:dev Apr 6, 2019
@ghost ghost removed the in progress label Apr 6, 2019
@JeffLIrion
Copy link
Contributor Author

Thanks, @dgomes!

unibeck pushed a commit to unibeck/home-assistant that referenced this pull request Apr 7, 2019
* Bump androidtv to 0.0.15

* Bump androidtv to 0.0.15 in manifest.json
@pvizeli pvizeli removed this from the 0.91.2 milestone Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants