search

coil-kt / coil

Image loading for Android and Compose Multiplatform.
https://coil-kt.github.io/coil/
Apache License 2.0
10.81k stars 662 forks source link

Update NetworkObserver to call connectivityManager.registerDefaultNetworkCallback instead of connectivityManager.registerNetworkCallback #2374

Closed daniellevass closed 3 months ago

daniellevass commented 3 months ago

Hi,

We'd like to update the NetworkObserver to call connectivityManager.registerDefaultNetworkCallback instead of connectivityManager.registerNetworkCallback to ensure images load across our Android Automotive app regardless of the network assigned.

Android Automotive OEMs use a feature called Per-application network selection (PANS) to separate network traffic amongst various cellular network interfaces. OEMs generally prioritize critical car functionality to an "OEM-paid" network and keep entertainment functionality to a "Customer-paid" network.

Calling registerNetworkCallback is not recommended to track the default network assigned - Google recommend using registerDefaultNetworkCallback to track the default network assigned to the app.

I'm happy to open a pull request for this change, if there are no concerns with this?

Thank you,

Danielle.

colinrtwhite commented 3 months ago

Thanks for the context. I think it's possible changing NetworkObserver to use registerDefaultNetworkCallback could have side effects for existing users so I think we should probably add ImageLoader.Builder.observeDefaultNetwork(true/false) to allow opting into the change. By default we only load over the default network, but it's possible a user could be supplying a custom SocketFactory to use a different network. PR welcome!

I think this is also a good reason to make NetworkObserver public in Coil 3.x so the behaviour can be custom.

daniellevass commented 3 months ago

Thank you so much for your quick feedback! I've opened a PR over here : https://github.com/coil-kt/coil/pull/2379 - any feedback on the PR welcome 🙇‍♀️ I ran the ./test.sh locally, but it looks like CI won't let me know if I did everything okay until someone approves the build 🙈

colinrtwhite commented 3 months ago

This will be available in 2.8.0 - unfortunately I don't have a timeline for when the next release will be.

daniellevass commented 1 week ago

hi @colinrtwhite - apologies for the post on an old thread! Do you have any updates on when the next release might be? Would it be possible to make a patch release (2.7.1) with the fix instead of a minor release (2.8.0)? Is there anything I can do to help facilitate making a release sooner? We'd really appreciate being able to access this fix sooner rather than later! Thank you 🙇‍♀️