Implement play-services-constellation#3359
Conversation
|
It will take a while for this to be reviewed, but I'm happy to finally see someone actually tackling the issue instead of having an LLM generate bullshit. 👏 |
|
@opstic |
|
@opstic We generally try to sort things into two modules:
The error @ale5000-git mentioned is probably due to having safe parcelable classes in Kotlin (which was never supported or tested), meaning they have automatically generated annotations that shouldn't be there. The Kotlin code also somewhat looks like it was automatically converted from Java (which is not always 100% safe, as seen here). For this API specifically, because it is an internal API of Google, you can also just add it to the |
|
@mar-v-in The @JvmField were workarounds to avoid safe-parcel-processor using reflection to access the fields, I'll be converting these into Java then. Also I'll restructure a bit to fit the sorting better. |
|
@opstic The issue is that it sees the type as |
|
@mar-v-in |
|
Okay, after some more testing I've determined it's a Java version issue. The annotations aren't passed in with Java 21 so it builds successfully. I'm still really hoping to keep the parcelables in kotlin as it would be easier for me and the ergonomics feel better but if it's necessary to be in Java and/or updating CI Java to 21 isn't acceptable I'm happy to move it over as well. I'll push the |
I hope this will never happen since it will break compatibility with old Android versions. |
|
@opstic IMPORTANT: Do not use gradle, but gradlew; since using gradle won't use the intended gradle version and maybe it will hide possible errors. NOTE: The full PS: Thanks for the good works on the PR :-) |
|
Small problem, I am cleaning up by looking at the lint results and I am pretty sure Google Messages is the oldest thing that calls this service, which only goes back to as far as Nougat for meaningful versions. Any standard method to exclude this module entirely for SDKs under Nougat? I am not a fan of putting Also I've debugged for RCS in Nougat Google Messages, it would first compare if the mnc_mcc of the phenotypes match what's in the phone, then do standard self-contained OTP provisioning (instead of UPI) while only needing to call But much of the code uses |
|
I don't see us bumping the minimum API level to 26 any time soon. Google's Play Services has minimum API level of 21. We're currently at 19 and I plan to bump to 21 soonish to be able to make wider use of Compose. Have you considered applying the annotation to the whole file (via |
|
Alright, that's much better than applying to each function at least. Thanks for the quick response! |
|
Alright, I think I'm done refactoring everything for now. The Everything should be ready for review. Also, here's a built debug APK artifact from CI that includes all the necessary changes together to make testing easier. |
This is PR 1 of 3 towards RCS support.
Related PRs: #3360, #3361
Related issue: #2994
Collectively these changes enable Google Messages to verify the phone number via UPI and retrieve the provisioning document.
In my testing, setup progresses through verification, provisioning and reaches a connected state in Google Messages.
The remaining failure occurs during Tachyon registration. One possible cause is that the final DroidGuard
tachyon_registrationchallenge is not being satisfied correctly in my test environment.Testing from people with environments that pass DroidGuard/Play Integrity would be appreciated, especially with logs/network captures if possible. Many thanks!
Description
This PR implements the Constellation service, including:
verifyPhoneNumberV1verifyPhoneNumberSingleUseverifyPhoneNumbergetIidTokengetPnvCapabilitiesgetIidTokenandverifyPhoneNumberappear to be the main APIs Google Messages would call.Most of the verification paths are hopefully implemented correctly, only the TS.43 path was tested.
Flashcall is excluded for now as it's implementation in GMS requires a hidden API in Android SDK.
The gRPC proto definitions and client implementation are also used by
play-services-asterism(PR #3360)Screenshots