home-assistant / android

:iphone: Home Assistant Companion for Android
https://companion.home-assistant.io/
Apache License 2.0
2.35k stars 656 forks source link

Breaking change: Improve sensor mobile_data #4696

Closed frimtec closed 1 month ago

frimtec commented 1 month ago

Summary

Improve gathering data for sensor mobile_data. The currently used API for the mobile_data sensor is marked as @UnsupportedAppUsage and is therefore not supported on newer devices (e.g. Pixel 9). As of SDK 26 there is an API TelephonyManager#isDataEnabled that is supported on all devices.

Screenshots

Frontend not affected due to this change.

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#435 The existing documentation introduced with above pull request is still fully valid.

Any other notes

dshokouhi commented 1 month ago

I see that permissions are changing here for a sensor, that will be a breaking change for users who have that sensor enabled but not the permission granted. Is the sensor not updating properly? on my pixel 9 pro xl its showing the correct state.

frimtec commented 1 month ago

The state for the sensor "mobile_data" on my pixel 9 Pro is always "on" even when mobile data is disabled. I could also reproduce that behavior on a pixel 7 of a friend. With this fix it works again on my pixel 9, as I was used it with my old pixel 4.

I have currently no idea why it still works on your pixel 9 pro xl.

The declaration of the old Android API Settings.Global.MOBILE_DATA is as follows :

        @UnsupportedAppUsage
        @Readable
        public static final String MOBILE_DATA = "mobile_data";

@UnsupportedAppUsage means: Marks elements as internal and not intended for external use. Avoid using unsupported APIs to ensure app stability and compatibility.

Therefore I think it makes more sense to use the public API if available (as of SDK 26).

As the new API requires permission READ_PHONE_STATE, you are right this could be a breaking change. I updated the requiredPermissions() method to make the change as small as possible.

dshokouhi commented 1 month ago

My bad i did not realize that the sensor state had stopped working, I had incorrectly assumed as I had mobile data enabled that the sensor had the correct state 🙈 Now I wonder how long it was broken for 😂

dshokouhi commented 1 month ago

Now I wonder how long it was broken for 😂

Looks like my Pixel watch had the correct state with the old method. Surprisingly it's on the same version of Android as my phone. Guess it shows the platform differences there lol. Thanks again for the PR 🙏Screenshot_20241004-151327.png

frimtec commented 1 month ago

Looks like my Pixel watch had the correct state with the old method. Surprisingly it's on the same version of Android as my phone.

It is not the android version that is relevant whether it works or not, but the device itself. E.g. the old API still works up to android 15 on the android emulator. But more and more devices do not support the old API because it violates the security model. With the old API you can access the phone state without the required permission READ_PHONE_STATE.

Thx for merging the pull request 👍