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

macOS Support #534

Merged
merged 13 commits into from
Aug 12, 2024
Merged

macOS Support #534

merged 13 commits into from
Aug 12, 2024

Conversation

alexrabin-sentracam
Copy link
Contributor

@alexrabin-sentracam alexrabin-sentracam commented Jul 3, 2024

Addresses #231

This pr adds macOS support with a HUGE caveat.

This functionality only works when running the app directly from Xcode. There is a known issue with flutter that the macOS app crashes when requesting permissions in VSCode flutter/flutter#70374.

I leave this up to the author on if they want to merge this or not. But the functionality does exist.

Copy link
Contributor

@sowens-csd sowens-csd left a comment

Choose a reason for hiding this comment

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

I'm excited about MacOS support, thanks for doing the work. I'd definitely like to merge. I've only had a quick look at this PR as it is quite large but I don't understand yet how it provides MacOS support. Could you point me at the two or three relevant files so I can review? So far I mostly see file deletions but not corresponding additions.

@alexrabin-sentracam
Copy link
Contributor Author

@sowens-csd I was able to implement macOS support by using the shared Darwin approach as mentioned in the flutter docs: https://docs.flutter.dev/packages-and-plugins/developing-packages#shared-ios-and-macos-implementations. Both iOS and macOS use the same code with some slight differences based off their platforms.

Take a look at the speech_to_text/darwin/Classes/SpeechToTextPlugin.swift file for the implementation.

Copy link
Contributor

@sowens-csd sowens-csd left a comment

Choose a reason for hiding this comment

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

Thanks, very helpful. I got caught by GitHub not showing large diffs by default. Going through it now.

@alibad
Copy link

alibad commented Jul 29, 2024

@sowens-csd any plans to merge and launch it in a new version of the flutter package?

Are you aware of speech_to_text_macos?

@alexrabin-sentracam
Copy link
Contributor Author

alexrabin-sentracam commented Jul 30, 2024

@alibad "speech_to_text_macos" is no longer needed because of the shared darwin implementation in this pr

<true/>
<key>com.apple.security.device.audio-input</key>
<true/>
<key>com.apple.security.network.server</key>
Copy link

Choose a reason for hiding this comment

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

should this also include

<key>com.apple.security.network.client</key>
<true/>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To run the debug version of the audio_player_interaction on macOS the network.client. key is not needed.

Copy link

Choose a reason for hiding this comment

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

Even when speech recognition is not running locally on the device?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, when I tested locally that key is not needed when running on device.

@@ -0,0 +1,43 @@
platform :osx, '10.14'
Copy link

Choose a reason for hiding this comment

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

so it is compatible with MacOS 10.14?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The plugin can be built on macOS 10.13 and above but the speech recognition code only works on macOS 10.15 and above.

When I generated the macOS project the pod file automatically set the platform as 10.14

Copy link

Choose a reason for hiding this comment

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

so it's best be at 10.15?

@@ -1,5 +1,5 @@
# Uncomment this line to define a global platform for your project
# platform :ios, '11.0'
# platform :ios, '12.0'
Copy link

Choose a reason for hiding this comment

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

was this iOS upgrade intended?

Copy link
Contributor Author

@alexrabin-sentracam alexrabin-sentracam Jul 30, 2024

Choose a reason for hiding this comment

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

Probably not. When I generated the macOS project in the example it updated the platform numbers. Do you want me to revert it?

Copy link

Choose a reason for hiding this comment

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

I personally think it's fine, unless @sowens-csd has reasons not to

Copy link

@alibad alibad left a comment

Choose a reason for hiding this comment

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

What kind of testing was done to validate the changes in this PR work, on iOS and MacOS?

@alexrabin-sentracam
Copy link
Contributor Author

@alibad I ran the example projects on iOS and macOS on my Mac. I made sure text was returned from the plugin.

@alibad
Copy link

alibad commented Aug 11, 2024

@sowens-csd can you please approve this change? I'd like to use it as soon as the new version comes out.

@sowens-csd sowens-csd merged commit ce6fb90 into csdcorp:main Aug 12, 2024
@sowens-csd
Copy link
Contributor

sowens-csd commented Aug 12, 2024

@alexrabin-sentracam I've merged the PR and am trying to run the example on MacOS. I've tried it from Xcode after a clean build and the app appears but clicking initialize does not appear to do anything. No errors in the log.

So far I've been unable to get the debugger to stop on any lines inside the .swift. Any tips on how you tested that might help?

@alexrabin-sentracam
Copy link
Contributor Author

@sowens-csd I pulled the latest code and I was able to get it to work correctly in Xcode. What macOS version are you on? I am on macOS 14.5

@alexrabin-sentracam
Copy link
Contributor Author

@sowens-csd make sure to run

flutter pub get and cd macos && pod install It's possible your local is still pointing to the old speech_to_text_macos plugin

@sowens-csd
Copy link
Contributor

Thanks for the assist, working well now. I'll add that to the README in case anyone is as much of an idiot as I. Thanks so much for doing this work. It's going to be in the 7.0.0 release of the plugin.

@alexrabin-sentracam
Copy link
Contributor Author

Happy to help!

@alibad
Copy link

alibad commented Aug 12, 2024

Exciting! @sowens-csd when will the 7.0.0 release of the plugin take place? I'd like to pull it right away to test the MacOS behavior.

@sowens-csd
Copy link
Contributor

7.0.0-beta.1 is available now.

@alibad
Copy link

alibad commented Aug 13, 2024

I just tested the beta. Before, I would get an initialize error. Now, the app crashes without any errors logged

@sowens-csd
Copy link
Contributor

Did you run it from Xcode or VSCode? There's apparently a permissions problem that prevent Mac apps from working unless run from Xcode.

@alibad
Copy link

alibad commented Aug 13, 2024

Yes, it works from there

@alexrabin-sentracam
Copy link
Contributor Author

@sowens-csd you might want to mention higher up in the ReadMe that MacOS only works when being run from Xcode.

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.

3 participants