search

aws-samples / amazon-sumerian-arcore-starter-app

A sample Android project that demonstrates how to create a simple augmented reality experience using Google's ARCore with the Amazon Sumerian service.
Apache License 2.0
53 stars 31 forks source link

Adding support for Microphone input for Lex (in Sumerian) #10

Closed saltysoup closed 5 years ago

saltysoup commented 5 years ago

Description of changes: Added microphone permissions in the manifest file, helper class for checking microphone permission granted by users and chrome client for WebView to handle getusermedia API from Sumerian (to have android app pass microphone input to Lex).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

mike-starr commented 5 years ago

Could you remove the .DS_Store and files added to the .idea directory from the commit? These are client-specific items that shouldn't be committed. The .gitignore file should be set up to exclude them automatically, but looks like they made it in somehow.

saltysoup commented 5 years ago

hi @mike-starr, not sure how i had multiple . files but tracked them in gitignore and deleted from repo.

mike-starr commented 5 years ago

I think that may have made things worse. You should revert your changes to the .gitignore file - those are resulting in important files being deleted (the pull request template) and the addition of some files (built binaries) that should not be committed.

Additionally, you've a change to strings.xml that changes the project name and a change to MainActivity.java that changes the default scene URL, which also should not be included.

The MicPermissionHelper.java file has a header with a Google copyright notice. Did this file come from a Google open sourced project somewhere, or did you write it yourself?

saltysoup commented 5 years ago

hi @mike-starr, reverted the .gitignore (for some reason i had another copy at root as well) and changed the project name and scene url back to default in strings.xml and MainActivity.java.

re: MicPermissionHelper.java, i created it to reference the changes in the manifest file for requesting microphone/record_audio permission. The google copyright notice was there because i wanted to import the same packages in the mic helper class, but must've got included in there by accident. I've since removed references to it and retained Amazon notice

mike-starr commented 5 years ago

There are still a good number of unnecessary changes in this PR. For this to be merged, the only files in the PR should be the ones required for the mic changes. Do not submit changes to project or system files.

mike-starr commented 5 years ago

Closing this for now - please feel free to re-submit after resolving the issues.