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

Add 'adb_response' attribute to Android TV / Fire TV #23960

Merged
merged 4 commits into from May 24, 2019

Conversation

JeffLIrion
Copy link
Contributor

@JeffLIrion JeffLIrion commented May 18, 2019

Description:

This stores the output, if any, of the androidtv.adb_command service in a state attribute -- 'adb_response' -- so that it can be used in automations / scripts.

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.
  • I have followed the [development checklist][dev-checklist]

@JeffLIrion
Copy link
Contributor Author

It can be the case that this adb_response attribute is really long (for example, the response from dumpsys power). But (a) I don't think this hurts anything and (b) this is a more advanced feature anyways for users who want to send an ADB command and parse its output, so it's up to the user to send reasonable commands and manage the output.

This ensures that the `'adb_response'` attribute contains the response to the latest command
@JeffLIrion
Copy link
Contributor Author

@rytilahti are there any other changes you'd like me to make?

return self._adb_response

response = self.aftv.adb_shell(cmd)
if isinstance(response, str) and response.strip():
Copy link
Member

Choose a reason for hiding this comment

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

This feels a bit hacky.. Is there some case where the adb_shell() call would be returning something else than a string? If not, it'd be just easier to check for that here? Otherwise I think this would be better handled in the backend lib.

Other than that it looks fine to me; I don't have a device at hand, so I cannot really do any testing on functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's either a string or None. I've been using this version of the component, it works as expected.

Copy link
Member

Choose a reason for hiding this comment

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

I think I'd personally prefer then checking against Noneness here, but it's fine as it is. Let's merge this, thanks! 🎉

@rytilahti rytilahti merged commit 14d1695 into home-assistant:dev May 24, 2019
@balloob balloob mentioned this pull request Jun 4, 2019
alandtse pushed a commit to alandtse/home-assistant that referenced this pull request Oct 12, 2019
…23960)

* Add 'adb_response' attribute to Android TV / Fire TV

* Use None instead of empty string for empty ADB responses

* Initialize self._adb_response as None, not empty string

* Update the state after sending an ADB command

This ensures that the `'adb_response'` attribute contains the response to the latest command
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants