Skip to content

Use NetworkRequest transport types for auto-send constraints#6971

Merged
seadowg merged 2 commits intogetodk:masterfrom
grzesiek2010:COLLECT-6937
Dec 3, 2025
Merged

Use NetworkRequest transport types for auto-send constraints#6971
seadowg merged 2 commits intogetodk:masterfrom
grzesiek2010:COLLECT-6937

Conversation

@grzesiek2010
Copy link
Member

@grzesiek2010 grzesiek2010 commented Nov 19, 2025

Closes #6937

Why is this the best possible solution? Were any other approaches considered?

The issue states that we should keep the current code for older Android versions and only use the new approach on newer ones. However, it seems we can use setRequiredNetworkRequest on all API levels, as it performs its own version check and internally uses the appropriate implementation:

        fun setRequiredNetworkRequest(
            networkRequest: NetworkRequest,
            networkType: NetworkType
        ): Builder {
            if (Build.VERSION.SDK_INT >= 28) {
                if (
                    Build.VERSION.SDK_INT >= 31 &&
                        NetworkRequest30.getNetworkSpecifier(networkRequest) != null
                ) {
                    throw IllegalArgumentException(
                        "NetworkRequests with NetworkSpecifiers set aren't supported."
                    )
                }
                requiredNetworkRequest = NetworkRequestCompat(networkRequest)
                requiredNetworkType = NetworkType.NOT_REQUIRED
            } else {
                requiredNetworkType = networkType
            }
            return this
        }

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

For users on Android 9 (API 28) and above, auto-send behavior becomes more accurate and dependable. Instead of relying on “metered vs unmetered” detection, which incorrectly triggers sending on metered Wi-Fi hotspots when “Cellular only” is selected, the app now uses explicit network transport types (Wi-Fi vs cellular).

For users on older devices (API 27 and below), behavior remains unchanged because the existing metered-based logic is retained.

Do we need any specific form for testing your changes? If so, please attach one.

No.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

Before submitting this PR, please make sure you have:

  • added or modified tests for any new or changed behavior
  • run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
  • added a comment above any new strings describing it for translators
  • added any new strings with date formatting to DateFormatsTest
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

@grzesiek2010 grzesiek2010 marked this pull request as ready for review November 19, 2025 22:11
@grzesiek2010 grzesiek2010 requested a review from seadowg November 19, 2025 22:11
networkConstraint: Scheduler.NetworkType?
) {
val constraintNetworkType = when (networkConstraint) {
val networkRequest = NetworkRequest.Builder().apply {
Copy link
Member

Choose a reason for hiding this comment

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

I think we still need to add the NetworkCapabilities.NET_CAPABILITY_INTERNET capability here right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why do you think so? I think it's not needed as we specify NetworkType.

Copy link
Member

Choose a reason for hiding this comment

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

It looks to me like (when calling from API 28 and above) NetworkType is ignored. Given that, my guess would be that we still need to specify NET_CAPABILITY_INTERNET on the NetworkRequest.

Copy link
Member Author

@grzesiek2010 grzesiek2010 Dec 2, 2025

Choose a reason for hiding this comment

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

Yes, the old network type is used only on older Android versions, but once we switch to using NetworkRequest directly (newer Android versions), specifying the transport (WIFI / CELLULAR) should already imply the type of network we want.

Copy link
Member

Choose a reason for hiding this comment

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

specifying the transport (WIFI / CELLULAR) should already imply the type of network we want.

Wifi and cellular connections don't necessarily have internet access though. Reading the docs, it would seem to me that we'd still want NET_CAPABILITY_INTERNET or even potentially NET_CAPABILITY_VALIDATED as part of the request.

@grzesiek2010 grzesiek2010 requested a review from seadowg December 1, 2025 12:25
networkConstraint: Scheduler.NetworkType?
) {
val constraintNetworkType = when (networkConstraint) {
val networkRequest = NetworkRequest.Builder().apply {
Copy link
Member

Choose a reason for hiding this comment

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

It looks to me like (when calling from API 28 and above) NetworkType is ignored. Given that, my guess would be that we still need to specify NET_CAPABILITY_INTERNET on the NetworkRequest.

@grzesiek2010 grzesiek2010 requested a review from seadowg December 2, 2025 12:41
Copy link
Member

@seadowg seadowg left a comment

Choose a reason for hiding this comment

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

@seadowg seadowg merged commit 298e4dc into getodk:master Dec 3, 2025
7 checks passed
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.

Improve "Cellular only" setting for Android 9+ users

2 participants