Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate bluetooth sensors to common and add to wear OS #3168

Merged
merged 3 commits into from
Jan 11, 2023

Conversation

dshokouhi
Copy link
Member

@dshokouhi dshokouhi commented Dec 20, 2022

Summary

Migrates over the bluetooth sensors so they can be used in Wear OS.

I know that previously I had mentioned we need to have settings and notifications for these sensors to work properly, I came to notice that a lot of users in the request (#2183) just wanted something to work and use it right away. Given the interest I wanted to see if we can get some testers to do some battery life testing.

Its important to note that I have not tested this on my own watch yet (I plan to do so this weekend or next week but maybe sooner), testing was done on the phone but the code should still work on Wear OS so would be good to get some early feedback on this.

This will also tell us how badly we would need settings and commands for these sensors or even if a dedicated tile to control the transmitter/monitor will suffice.

Testing for functionality is now complete, the 4 sensor do indeed function as expected. I was able to confirm that my phone could pick up the ble transmitter from watch and the beacon monitor from the watch was able to pick up the transmitter from other devices.

{"type":"update_sensor_states","data":[{"unique_id":"bluetooth_state","state":true,"type":"binary_sensor","icon":"mdi:bluetooth","attributes":{}}]}

{"type":"update_sensor_states","data":[{"unique_id":"bluetooth_connection","state":1,"type":"sensor","icon":"mdi:bluetooth","attributes":{"connected_not_paired_devices":[],"connected_paired_devices":["D4:3A:2C:99:F1:20 (Pixel 7 Pro)"],"paired_devices":["D4:3A:2C:99:F1:20 (Pixel 7 Pro)"]}}]}

{"type":"update_sensor_states","data":[{"unique_id":"ble_emitter","state":"Transmitting","type":"sensor","icon":"mdi:bluetooth","attributes":{"Advertise mode":"lowPower","Measured power":-59,"Supports transmitter":true,"Transmitting power":"ultraLow","id":"a7fb89b6-1810-480b-b378-1466fe737f8a_100_1"}}]}

{"type":"update_sensor_states","data":[{"unique_id":"beacon_monitor","state":"Monitoring","type":"sensor","icon":"mdi:bluetooth","attributes":{"50765cb7-d9ea-4e21-99a4-fa879613a492_15841_33291":17.18}}]}

Still need to do battery life testing and would be nice to get some user feedback

Battery life testing complete, the watch does not drain super fast. On my pixel watch it drained 25% in 8 hours, that seems very reasonable IMO. Maybe a tile to control the transmitter and monitor might make sense since you can leave it on for an extended period of time?

Screenshots

Link to pull request in Documentation repository

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

Any other notes

@dshokouhi
Copy link
Member Author

Had several users confirm things are working well for them and their intention is to just leave the sensors enabled as the battery drain seemed minimal to them. Marked as ready for review.

Docs have been updated to mention sensor settings are not functional on Wear OS.

@dshokouhi dshokouhi linked an issue Jan 4, 2023 that may be closed by this pull request
Copy link
Member

@jpelgrom jpelgrom left a comment

Choose a reason for hiding this comment

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

The Wear app manifest should be updated to include the various Bluetooth permissions that may be required (presumably they're granted by default on Wear OS as no one has complained yet, despite the merged manifest only including 2/5?).

@dshokouhi
Copy link
Member Author

The Wear app manifest should be updated to include the various Bluetooth permissions that may be required (presumably they're granted by default on Wear OS as no one has complained yet, despite the merged manifest only including 2/5?).

Good call, I also forgot to register for the BT intents. Will add it all now to match the phone app.

@dshokouhi
Copy link
Member Author

match the phone app.

Actually we should not match the permissions as we are not targeting SDK 33 so some of the new BT permissions do not exist yet. I realized this because the sensors that are working are not getting enabled because the app does not need all the new BT restrictions just yet. Will adjust for the current state of the app and the fact that there are no Wear OS devices running SDK 33. We should only worry about the 3 new permissions once we Wear OS app changes its target SDK.

The phone app already targets SDK 33 so the new permissions and restrictions on old permissions work as expected.

@jpelgrom
Copy link
Member

jpelgrom commented Jan 7, 2023

we are not targeting SDK 33 so some of the new BT permissions do not exist yet

The new permissions are for SDK 31+ actually. The app targets SDK 32 currently so they should work, a bit unexpected that they don't and impact SDK <31 but then again there aren't actually any watches/emulators with SDK >= 31.

When using the Bluetooth permissions like in the main manifest, sensors do stay enabled for me in the emulator and aren't behaving differently (but that isn't a 100% confirmation as the emulator doesn't really expose Bluetooth data). Are you sure that the sensors not being enabled was because of the maxSdkVersion attribute?

@dshokouhi
Copy link
Member Author

When using the Bluetooth permissions like in the main manifest, sensors do stay enabled for me in the emulator and aren't behaving differently (but that isn't a 100% confirmation as the emulator doesn't really expose Bluetooth data). Are you sure that the sensors not being enabled was because of the maxSdkVersion attribute?

So what I did for my test was to copy over the 5 BT permissions including the maxSdkVersion attribute. The transmitter and monitor sensors did not stay enabled even after granting the permissions. The initially turned on and sent a state but then turned off and stayed off until I made the most recent changes committed here.

@dshokouhi
Copy link
Member Author

Ran another test and now its behaving as expected, weird. Will update again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add iBeacon capability to the Wear OS app
3 participants