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

Separate microphone and recognizer permissions #55

Merged
merged 14 commits into from
Nov 19, 2024

Conversation

msschwartz
Copy link
Contributor

When using on-device recognition, the device does not need speech recognizer permissions. By separating the permission checking we can avoid showing the following message to users:

Speech data from this app will be sent to Apple to process your requests. This will also help Apple improve its speech recognition technology.

This message is misleading since data is NOT sent to Apple when using on-device. Many users prefer to keep their data private for apps with sensitive data.

If this is something we should move forward with, we'll need to update web and Android code.

Copy link

changeset-bot bot commented Nov 17, 2024

⚠️ No Changeset found

Latest commit: eb14871

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Owner

@jamsch jamsch 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! These granular permissions APIs are definitely going to be a great improvement for the library! The only thing I'm just a bit iffy on mostly has to do with communicating the API usage in our documentation because it only really affects a semi-niche use case.

src/ExpoSpeechRecognitionModule.web.ts Outdated Show resolved Hide resolved
src/ExpoSpeechRecognitionModule.types.ts Outdated Show resolved Hide resolved
src/ExpoSpeechRecognitionModule.types.ts Outdated Show resolved Hide resolved
src/ExpoSpeechRecognitionModule.types.ts Show resolved Hide resolved
src/ExpoSpeechRecognitionModule.types.ts Outdated Show resolved Hide resolved
example/App.tsx Outdated Show resolved Hide resolved
Copy link
Owner

@jamsch jamsch left a comment

Choose a reason for hiding this comment

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

Thank you @msschwartz, this looks pretty much ready to go! When I have some time later today I'll clone your repo and test it out on iOS/Android/Web to make sure everything's working okay and update the README to document the new APIs.

@jamsch jamsch merged commit d3d3d63 into jamsch:main Nov 19, 2024
2 checks passed
jamsch added a commit that referenced this pull request Nov 26, 2024
* skip permission if not needed

* separate permissions

* dev

* Update App.tsx

* Update Podfile.lock

* Update App.tsx

* moving permission check

* deprecated

* Update example/App.tsx

Co-authored-by: jamsch <[email protected]>

* return granted response on web

* Update src/ExpoSpeechRecognitionModule.types.ts

Co-authored-by: jamsch <[email protected]>

* More docs update and removing deprecated

* Adding Android permission functions

---------

Co-authored-by: jamsch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants