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

[WIP] Revive android auto #836

Closed
wants to merge 16 commits into from

Conversation

morckx
Copy link
Contributor

@morckx morckx commented May 21, 2020

What again were the issues Google had with our android auto support?
For me, it seems to work quite well.

@werman
Copy link
Contributor

werman commented May 21, 2020

The needed voice search support.

See #389

@morckx
Copy link
Contributor Author

morckx commented May 21, 2020

The needed voice search support.

Ah, thanks. I think I had got that working, too, some time ago. I hope I'll find my changes again. One of them are already i here (6812455 ).

@morckx
Copy link
Contributor Author

morckx commented May 22, 2020

The needed voice search support.

See #389

OK, playFromSearch is already working – tested with android-media-controller.

The most important thing that is still missing to get through the tests is probably a default station, which is played whenever nothing is found. Especially of course if RadioDroid has just been installed and there are no favorites and no history yet.

Maybe BBC World News? Or is there anything even less controversial and more generic?

Of course it would be nice if the Voice Search would also search the server before it takes the default station (if it doesn't find anything in History and Favorites). For this purpose it would be handy to have a static function in Utils for server requests.

In addition, MediaStore.INTENT_ACTION_MEDIA_PLAY_FROM_SEARCH should apparently also be intercepted by the activity: https://developer.android.com/guide/components/intents-common.html#PlaySearch
I don't if that's really necessary, but it would be easy in any case.

@werman
Copy link
Contributor

werman commented May 22, 2020

Thanks, I'll try to check soon.

Maybe BBC World News? Or is there anything even less controversial and more generic?

I think some plain music station would be better since it would be language independent.

@werman
Copy link
Contributor

werman commented May 23, 2020

I remember I was able to test it in the emulator, now I'm not for some reason....

Are you able to test it with in emulators?

@morckx
Copy link
Contributor Author

morckx commented May 23, 2020

I remember I was able to test it in the emulator, now I'm not for some reason....

Are you able to test it with in emulators?

Yes, I just had to copy libcrypto.so.1.0.0 and libssl.so.1.0.0 from an old computer to
/usr/local/lib64/ and added a file /etc/ld.so.conf.d/local.conf with just /usr/local/lib64 in it, after that ldconfig and it worked. In case this was your problem. And don't forget to start the head unit server from android auto (Settings->Overflow Menu->Start Head Unit Server).

Ah if you meant install Android Auto on the emulator: No I haven't managed to do it, but tried it only briefly.

Copy link
Contributor

@werman werman left a comment

Choose a reason for hiding this comment

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

Ok, it works with real Android device.

What I think is necessary:

  • Prev/Next should work for History and not only for favourites. And potentially with other lists.
  • If there is a station with broken icon url - it should not make us wait for 2s timeout on every navigation to a list with this station. I think it could be done on OkHttp level, with some interceptor, but...

app/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
RadioDroid does not run on the head unit.
To make sure that even on first start onPlayFromSearch does not fail completely.
Run in Android Studio or with
./gradlew connectedCheck

Unit tests with Mockito would be quicker, but also a lot messier in this case.
@morckx
Copy link
Contributor Author

morckx commented May 23, 2020

Ok, it works with real Android device.

What I think is necessary:

  • Prev/Next should work for History and not only for favourites. And potentially with other lists.
    Right.
  • If there is a station with broken icon url - it should not make us wait for 2s timeout on every navigation to a list with this station. I think it could be done on OkHttp level, with some interceptor, but...

Yes, and there are some more little stability issues.

@werman
Copy link
Contributor

werman commented May 23, 2020

Your instrumentation tests would clash with mine #654, better to add them afterwards.

@werman
Copy link
Contributor

werman commented May 23, 2020

It wouldn't be hard for me to merge it together, but you are using junit5 and I've made all tests junit4, if you have any objects, could you write them in the #654? I just find instrumented tests being a bit fragile in terms of "why this test doesn't run from IDE today???", so making them junit5 adds another layer of things to go wrong. And I had enough of headache even with current setup.

Copy link
Contributor Author

@morckx morckx left a comment

Choose a reason for hiding this comment

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

It wouldn't be hard for me to merge it together, but you are using junit5 and I've made all tests junit4, if you have any objects, could you write them in the #654? I just find instrumented tests being a bit fragile in terms of "why this test doesn't run from IDE today???", so making them junit5 adds another layer of things to go wrong. And I had enough of headache even with current setup.

OK, we can sort this out later, I think. I can revert my instrumentation tests for now. I've used juni5 because I also used it already for the IcyDataSource unit tests.
I could also convert the test here to a unit test with Mockito, but it's probably not worth the mess and I'll rather have a look at #654.

EDIT: There is also a junit4 compatibility library for junit5 that we can give a try then.

"tags" : "news",
"url" : "http://bbcwssc.ic.llnwd.net/stream/bbcwssc_mp1_ws-einws",
"votes" : 123
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's add some more stations here. Some criteria are probably:

  • ideally public
  • ideally receivable from all over the world
  • only music or in English

@segler-alex
Copy link
Owner

segler-alex commented May 23, 2020

instead of having a list of fallback streams, why not play a prerecorded message to the user that a stream needs to be selected first. we could also use text to speech, that way we could even add it to the translatable strings.

@morckx
Copy link
Contributor Author

morckx commented May 23, 2020

instead of having a list of fallback streams, why not play a prerecorded message to the user that a stream needs to be selected first. we could also use text to speech, that way we could even add it to the translatable strings.

Yes, I thought about that, too. I just wanted to have the fallback station(s) first, because that's probably the easiest way to the Android Auto approval.

I would then definitely continue with messages via TTS. We could just send the toasts that we're issuing now (depending on the settings) via TTS. We already have many translations for these.

The played time display seems to stay 00:00 unless MediaMetadataCompat.METADATA_KEY_DURATION is set (to unknown/infinity in our case).
According to android-media-controller auto test suite¹, icon bitmaps for
the media browser must be null and icon uris are restricted to content
uris and android resources. So there is no  supported way of using icons
here.
However, icons in the browser are extremely helpful, especially while
driving and network use is also no issue in this case – and the icons
work. Thus, introduce a settings and let the user decide.

¹https://github.com/googlesamples/android-media-controller/blob/0bbad7b22ca2c087629cce0e5b0eb61f07f3fde1/mediacontroller/src/main/java/com/example/android/mediacontroller/MediaAppTestDetails.kt#L1226
Set grid mode when showing icons is selected. Setting
CONTENT_STYLE_SUPPORTED to true is necessary in any case to make the
auto test suite happy and not crash.
@morckx morckx force-pushed the revive-android-auto branch 2 times, most recently from 3f40514 to 5487435 Compare May 28, 2020 21:07
Comment on lines +197 to +198
if (showIconsInBrowser && sharedPref.getBoolean("settings_grid_style_hud_icon_display", false)) {
Log.d(TAG, "Setting grid style for playables");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be "settings_grid_layout_hud_icon_display"? Or you have to rename the key in setting.

Comment on lines +344 to +346
<string name="settings_grid_layout_hud_icon_display">Grid layout for station icons</string>
<string name="settings_grid_layout_hud_icon_display_desc_off">Stations are displayed in list layout</string>
<string name="settings_grid_style_layout_icon_display_desc_on">Station icons are displayed in grid layout</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

When it is off, is the icon still shown? Should "icons" be added?

@segler-alex
Copy link
Owner

hi, if you are still interested. could you do another push that is not WIP? thank you for your work!!

@segler-alex
Copy link
Owner

in WIP too long, closing
please create another PR if you want to work on it.

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.

4 participants