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

Add "Find my phone" app. #2053

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Conversation

vchigrin
Copy link

This change set adds implementation of client for BLE "Immediate alert service", allowing companion apps get notifications from PineTime watch when requested by user.

Fixes: #343

@vchigrin
Copy link
Author

This pull request is result of joint work of @jmlich and @vchigrin, most of the code is written by Jozef (personal contributions can be found by commit history).

@vchigrin
Copy link
Author

That is how that app looks like
photo_2024-04-25_23-57-58
After user presses "Mild" or "High" button phone starts ringing and vibrating. User can stop this either by pressing "None" button on the PineTime, or by pressing button in GadgetBridge companion app on the phone.

To make this work some changes in GadgetBridge application were required, I suppose to upstream this after ensuring that this functionality will be accepted in InfiniTime.

Copy link

github-actions bot commented Apr 25, 2024

Build size and comparison to main:

Section Size Difference
text 374400B 1584B
data 948B 0B
bss 22584B 48B

@vchigrin
Copy link
Author

vchigrin commented May 1, 2024

Corresponding PR to GadgetBridge
https://codeberg.org/Freeyourgadget/Gadgetbridge/pulls/3746

@NitroNils
Copy link

And reverse? Find my watch? Make PineTime blink display and vibrate?
Don't know what this would take. Just a logical follow up 😁

@jmlich
Copy link
Contributor

jmlich commented May 4, 2024

Find my watch already works. It is Alert Notification Service. In Amazfish -> Settings -> Debug page -> Phone Call.
Also Immediate Alert Service is implemented and shows None/Mild/High levels, but this is not right to find watch.

@crazycrystals
Copy link

Sorry if its offtopic, but is it also possible to have something like a signal strength indicator so you can see how close the device is and triangulate it?
Doing a ring with an app could be broken by do not disturb.

@jmlich
Copy link
Contributor

jmlich commented May 6, 2024

The signal strength or more precisely RSSI (Received Signal Strength Indication) is specified/measured in BLE and Nimble can provide the value. I am not sure if it is reasonable reachable in the code.

src/FreeRTOSConfig.h Outdated Show resolved Hide resolved
@vchigrin

This comment was marked as outdated.

@devnoname120
Copy link

Previous attempt: #1193

@devnoname120
Copy link

FYI here are some notes about the limitations of the Immediate alert service standard: #1193 (comment)

@vchigrin
Copy link
Author

Well, you have a point. Indeed, using ImmediateAlertService has some drawbacks. Problems you mentioned are not solved here - in this app button returns to original state after timeout in few seconds, there are no any complex communications.

So having this PR and your work maintainers have a choice - either use standard BLE service, having some drawbacks, or develop custom solution, which may give better user experience. I'll be happy with any of these choices.

Or may be this functionality is out of "product vision" of the InfiniTime and it will not be merged - I don't know...

If I can help in any way to bringing this functionality to main branch, let me know, please.

@devnoname120
Copy link

@vchigrin As far as I see your approach matches better what they were looking for! And no matter what the UI code will be useful. We will have to wait for @JF002 to chime on this 😊

@vchigrin
Copy link
Author

Just noted that corresponding issue at present is the leader by "thumb-up" reactions :)
https://github.com/InfiniTimeOrg/InfiniTime/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc

@JF002 @mark9064 I'll be very happy to assist with merging this PR. Is there anything I can do to help with review?

@mark9064 mark9064 added new app This thread is about a new app new feature This thread is about a new feature labels Nov 26, 2024
@mark9064
Copy link
Member

I agree that the BLE spec sucks completely here. Both the IAS and FMP have annoying limitations that makes them kind of useless :(

I think implementing just the IAS is probably our best bet, but we should probably bend the rules a bit and expect the central to keep ringing even on disconnection etc. I honestly have no clue who the IAS is designed for TBH.

As for this PR, I think the UI needs some work. The "None-Mild-High" buttons aren't immediately clear in functionality to the user. Though the specification leaves it vague, I think we could assign more precise meaning to the levels- whether the companion app chooses to do something else doesn't really matter as we don't get to control the meaning of levels anyway. Maybe Stop, Vibrate and Ring? Or maybe we should simplify it to just Stop and Ring (and skip ever sending the mild level)?

(as an aside, I also wanted to remove all client roles from InfiniTime as they're kind of silly but the spec doesn't allow notify either- sending a notification to the central makes sooooo much more sense to me. So that puts a hole in that plan which I'm a little sad about. Oh well)

@vchigrin vchigrin force-pushed the findmyphone branch 2 times, most recently from b7117c2 to 3e0c1f1 Compare November 27, 2024 22:37
@vchigrin
Copy link
Author

Updated. I've removed "mild" alert since "Vibrate" label is too long for this button size. If you think 3 buttons will be better, I think we should consider emojis/icons...

At present UI looks like this.
photo_2024-11-28_01-43-12

PS: There "build-simulator" check fails since simulator repo does not have ImmediateAlertClient class. What is correct way handle situations like this? Should I open PR with necessary changes to simulator repo beforehand?

@mark9064
Copy link
Member

I think the two button design looks quite good, what about you?
Not sure if anything should be done with the space above, I'm pretty bad at visual design 😅
Now this PR is feature complete it is a good time to do the InfiniSim PR 👍

I'll try to review soon but things are looking busy- I'll get to it when I can

@vchigrin
Copy link
Author

Made InfiniTimeOrg/InfiniSim#165

Copy link
Member

@mark9064 mark9064 left a comment

Choose a reason for hiding this comment

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

I think this app could be improved by some UI rework:

  • The app should be aware of whether BLE is connected and whether the connected device supports the IAS. If not, it should show an appropriate message and disable the buttons, otherwise they can be enabled. It should also respond to connection changes, so it'll need a Refresh() method to check the bluetooth conditions continuously and change the UI as needed
  • It should maintain state (only while the app is open) - so it should show a message like "Device alerting" after a successful IAS write, and "Alert stopped" after setting the no alert level. On app load since the IAS state is unknown, it would probably be best to display a simple "Ready"/"Disconnected"/"Alert service unavailable" message and replace this message after sending an alert
  • Since signal may be bad when using this service, it should explicitly handle message send failures, and report back a different message to when the service isn't present or BLE is unavailable

Other than those points the rest looks good 👍

doc/ble.md Outdated Show resolved Hide resolved
src/components/ble/ImmediateAlertClient.h Outdated Show resolved Hide resolved
src/displayapp/screens/FindMyPhone.cpp Outdated Show resolved Hide resolved
src/displayapp/screens/FindMyPhone.cpp Outdated Show resolved Hide resolved
src/components/ble/ImmediateAlertClient.cpp Outdated Show resolved Hide resolved
src/components/ble/ImmediateAlertClient.cpp Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
doc/ble.md Outdated Show resolved Hide resolved
src/components/ble/ImmediateAlertClient.cpp Outdated Show resolved Hide resolved
src/components/ble/ImmediateAlertClient.cpp Show resolved Hide resolved
@vchigrin vchigrin force-pushed the findmyphone branch 3 times, most recently from 2006948 to f33578a Compare December 8, 2024 00:56
@vchigrin vchigrin requested a review from mark9064 December 8, 2024 01:09
@mark9064
Copy link
Member

mark9064 commented Dec 9, 2024

Thank you for all the changes, I will try to review them properly soon! Apologies in advance if it takes me a while to get round to it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new app This thread is about a new app new feature This thread is about a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Find My Phone" functionality with Gadgetbridge
6 participants