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.

Sign up for GitHub

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

Jump to bottom

feat: add carplay and android auto support #255

Merged
merged 29 commits into from
Oct 15, 2024
Merged

feat: add carplay and android auto support #255

merged 29 commits into from
Oct 15, 2024

Conversation

illuminati1911
Copy link
Contributor

@illuminati1911 illuminati1911 commented Sep 10, 2024 •
edited
Loading

Adds support for Apple CarPlay and Android Auto.

  • Added BaseAutoSceneDelegate for iOS and AndroidAutoBaseScreen for Android on a plugin level to manage setting up the base functionality for each platform.
  • Added example of setting up each platform in the SampleApp. iOS SampleApp has a new separate target for CarPlay support SampleAppCarPlay.
  • Added separate native module NavAutoModule to manage communication between RN code and the map instance.
  • Added a generic event channel so that user actions can be sent from native code to RN. e.g. user presses a button in CarPlay window and this event is sent to RN code.
  • Added a function to NavAutoModule to check whether CarPlay or Android Auto screen has been initialized and is available.

Resolves #177

  • Tests pass
  • Appropriate changes to documentation are included in the PR

jokerttu and others added 13 commits September 3, 2024 13:49
@jokerttu
feat: navigation support for auto
900bdf7 
Supports to show and control NavigationView on Android Auto and CarPlay
@illuminati1911
fix: fix ios build
f72c949
@illuminati1911
feat: change carplay map init setup to match android auto
04e31ab
@illuminati1911
feat: added android auto base screen
afce7d4
@jokerttu
fix: revert keys.plist removal from the project
671cbd6
@illuminati1911
fix: fix missing windows ref in app delegate
2fcbf24
@jokerttu
feat: support for navigation auto custom events
80f6785
@jokerttu @illuminati1911
feat: separate ios build targets
049e91e 
* feat: separate build target for carplay
* chore: move iOS example files under SampleApp folder

Co-authored-by: Ville Välimaa <[email protected]>
@illuminati1911
chore: add new carplay build target to podfile
b220b83
@jokerttu
fix: event transformer callback response handling
e1b3214
@jokerttu @illuminati1911
feat: notify on auto screen availibility changes (#5)
1a64117 
* feat: notify on auto screen availibility changes

* refactor: remove unused line of code

* refactor: rename onAutoScreen callback

---------

Co-authored-by: Ville Välimaa <[email protected]>
@illuminati1911
fix: fix carplay session attach management
c2ecac7
@illuminati1911
feat: add basecarscenedelegate and enable panning for carlay
4baf805
@jokerttu jokerttu mentioned this pull request Sep 12, 2024
Merged
2 tasks
@illuminati1911 illuminati1911 force-pushed the feat/navigation-for-auto branch from 37ba285 to fc64ecd Compare September 19, 2024 05:45
@illuminati1911 illuminati1911 force-pushed the feat/navigation-for-auto branch from fc64ecd to 7270b10 Compare September 19, 2024 05:47
@illuminati1911 illuminati1911 marked this pull request as ready for review September 19, 2024 05:50
@illuminati1911 illuminati1911 force-pushed the feat/navigation-for-auto branch from ff64a73 to 7702c2f Compare September 19, 2024 06:09
@ArturoSalazarB16
Copy link
Contributor

ArturoSalazarB16 commented Sep 20, 2024 •
edited
Loading

Hi! Thanks a lot for working on this Ville/Joonas. Moving forward, is it possible to create unitary PRs to make reviewing easier? and would also be easier to debug/investigate any possible issues.

Example: for this particular case, would it have been possible to split out Android Auto Vs CarPlay changes? we'd have different folks reviewing each platform, so that would also make it easier for them to track.

To not delay the feature, please don't block on this comment. It's more feedback for future changes.

@ArturoSalazarB16
Copy link
Contributor

Hi @ShirwaM! If you get some bandwidth, can you please take a look at the iOS portion of the PR? (And also if you have any feedback around Android Auto implementation that would be amazing).

thanks!
Arturo

ArturoSalazarB16
ArturoSalazarB16 reviewed Sep 20, 2024
View reviewed changes
Copy link
Contributor

@ArturoSalazarB16 ArturoSalazarB16 left a comment

Choose a reason for hiding this comment

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

Hi! Sharing some initial comments, will take another pass today

ANDROIDAUTO.md Outdated Show resolved Hide resolved
ANDROIDAUTO.md Outdated Show resolved Hide resolved
ANDROIDAUTO.md Outdated Show resolved Hide resolved
android/src/main/java/com/google/android/react/navsdk/AndroidAutoBaseScreen.java Outdated Show resolved Hide resolved
android/src/main/java/com/google/android/react/navsdk/AndroidAutoBaseScreen.java Show resolved Hide resolved
android/src/main/java/com/google/android/react/navsdk/AndroidAutoBaseScreen.java Outdated Show resolved Hide resolved
android/src/main/java/com/google/android/react/navsdk/AndroidAutoBaseScreen.java Outdated Show resolved Hide resolved
android/src/main/java/com/google/android/react/navsdk/NavAutoModule.java Show resolved Hide resolved
ArturoSalazarB16
ArturoSalazarB16 approved these changes Sep 20, 2024
View reviewed changes
Copy link
Contributor

@ArturoSalazarB16 ArturoSalazarB16 left a comment

Choose a reason for hiding this comment

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

LGTM - to unblock the change. But please let's await for @ShirwaM to provide feedback before merging.

src/shared/useModuleListeners.ts
return eventEmitterRef.current;
}, [dispatcher]);

const updateListeners = useCallback(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

For some of these new files, let's please start building our unit test coverage.

src/auto/useNavigationAuto.ts Show resolved Hide resolved
ShirwaM
ShirwaM requested changes Oct 4, 2024
View reviewed changes
Copy link
Member

@ShirwaM ShirwaM left a comment

Choose a reason for hiding this comment

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

LGTM pending a few minor change requests. Thank you!

example/android/app/src/main/java/com/sampleapp/SampleAndroidAutoScreen.java Outdated Show resolved Hide resolved
example/android/app/src/main/java/com/sampleapp/SampleAndroidAutoScreen.java Outdated Show resolved Hide resolved
example/android/app/src/main/java/com/sampleapp/SampleAndroidAutoScreen.java Outdated Show resolved Hide resolved
example/android/app/src/main/java/com/sampleapp/SampleAndroidAutoScreen.java Outdated Show resolved Hide resolved
@jokerttu jokerttu requested a review from ShirwaM October 7, 2024 10:53
zioncalvo
zioncalvo approved these changes Oct 14, 2024
View reviewed changes
Copy link

@zioncalvo zioncalvo left a comment

Choose a reason for hiding this comment

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

LGTM for iOS specific changes

@jokerttu jokerttu merged commit 933209a into googlemaps:main Oct 15, 2024
13 checks passed
@jokerttu jokerttu deleted the feat/navigation-for-auto branch October 15, 2024 09:20
@googlemaps-bot googlemaps-bot mentioned this pull request Oct 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zioncalvo zioncalvo zioncalvo approved these changes

@ArturoSalazarB16 ArturoSalazarB16 ArturoSalazarB16 approved these changes

@ShirwaM ShirwaM ShirwaM approved these changes

@jokerttu jokerttu Awaiting requested review from jokerttu

@caio1985 caio1985 Awaiting requested review from caio1985

Assignees

@ArturoSalazarB16 ArturoSalazarB16

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Apple CarPlay support
5 participants
@illuminati1911 @ArturoSalazarB16 @ShirwaM @jokerttu @zioncalvo