Actually check the paired state instead of assuming it for bluetooth connection sensor

[dshokouhi]

Summary

Fixes: #2736 by actually checking the bonded state. I am also suppressing the lint message as its not valid in our use case and makes the page look like it won't compile. The main app already requests the proper permission and its not necessary for the Wear app.

Screenshots

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#

Any other notes

[jpelgrom]

Now a single device is showing up multiple times in the same attribute in the Bluetooth Connection sensor for me:

I think a check should be added to make sure that even if allowDuplicates is true, it should filter out devices where the BluetoothDevice data class is exactly the same.

[dshokouhi]

I think a check should be added to make sure that even if allowDuplicates is true, it should filter out devices where the BluetoothDevice data class is exactly the same.

I think a better way instead of adding another condition is to just call distinct() on the lists. At least in my setup it works but curious if you still get duplicates :)

[jpelgrom]

distinct works for the attributes but still results in an incorrect state / total of connected devices, because the device is still included in the list and there is no filter/distinct on the count.

[dshokouhi]

ok now i think it should be good 🤞

[jpelgrom]

Unfortunately, I don't think it is. It works for me, but Android's BluetoothDevice.equals only compares the device address, so if I understood your comments in #2736 correctly, that Mi Band would now never show up as connected again (it is paired, but the connected device isn't recognized as paired)? If it is only comparing the device address we might as well remove the allowDuplicates parameter.

Assuming my interpretation of your comments in the issue is correct, suggestion:

for (btDev in btConnectedDevices) {
    if (!allowDuplicates && devices.any { it.address == btDev.address }) continue
    
    val name = btDev.name ?: btDev.address
    val device = BluetoothDevice(
        btDev.address,
        name,
        btDev.bondState == android.bluetooth.BluetoothDevice.BOND_BONDED,
        isConnected(btDev)
    )
    if (!devices.contains(device)) devices.add(device)
}

[dshokouhi]

so if I understood your comments in #2736 correctly, that Mi Band would now never show up as connected again (it is paired, but the connected device isn't recognized as paired)?

it actually did show up as connected when I tested the latest change. In my setup the attributes have always been correct in each of the changes, which makes the testing part difficult lol. Even when I test your provided code just now I see no changes.

My Mi band is actually not a paired device (not paired to the OS but paired to its own app) so it should only show up as connected_not_paired_devices which it always has with each change.

If it is only comparing the device address we might as well remove the allowDuplicates parameter.

I always took it as allow duplicates was meant more to hide the non-paired devices. At least thats how I initially read it. With that said IMO the device address is really the only thing that counts.

[jpelgrom]

My Mi band is actually not a paired device (not paired to the OS but paired to its own app) so it should only show up as connected_not_paired_devices which it always has with each change.

In that case I didn't understand your comments, sorry. For connected, not paired devices there should indeed be no changes 🙈

If it is only comparing the device address we might as well remove the allowDuplicates parameter.

I always took it as allow duplicates was meant more to hide the non-paired devices. At least thats how I initially read it. With that said IMO the device address is really the only thing that counts.

No it was added in #2591 to stop the settings from crashing due to device addresses being included twice in the list, while still allowing this sensor to get a list with a device with the same address showing up in both the paired connected and not paired connected lists. As you can see it doesn't do anything for non-paired devices.

Considering the issue this PR solves and the confusion it creates, looking back at it I think it is no longer necessary? The app will now always check paired + connected status irregardless of the source -> duplicates should never happen.

[dshokouhi]

Considering the issue this PR solves and the confusion it creates, looking back at it I think it is no longer necessary? The app will now always check paired + connected status irregardless of the source.

one of those things were code wise it doesnt look right, until you look at the end result and realize why it was like that 😅

So I guess I should close the PR then?

[jpelgrom]

So I guess I should close the PR then?

No the PR is still good to have! 'it' was referring to allowDuplicates, not the PR.

Keep the changes from the first commit, remove allowDuplicates and always treat it as true false / always apply this:

android/common/src/main/java/io/homeassistant/companion/android/common/bluetooth/BluetoothUtils.kt

Line 36 in 90a9a27

if (!allowDuplicates && devices.any { it.address == btDev.address }) continue

[jpelgrom]

Thanks for your patience @dshokouhi ❤️

Tested and resolves the issue

[dshokouhi]

Thanks for your patience @dshokouhi ❤️

Tested and resolves the issue

Thanks for your help in testing and code suggestions 😃