JohanDegraeve / xdripswift

xdrip for iOS, written in Swift
GNU General Public License v3.0
317 stars 313 forks source link

Siri Integration #492

Closed gshaviv closed 3 months ago

gshaviv commented 6 months ago

This PR adds an AppIntent (iOS 16) that enables you to ask Siri "what is my glucose level" and Siri will reply with your glucose level and show a quick snippet chart of your recent glucose. As demonstrated in this video:

https://github.com/JohanDegraeve/xdripswift/assets/66426/8377dc66-eb85-4088-9d1c-6ef5a7b7f5b8

To add the behavior to Siri, go to the shortcuts app, tap the + button, than the add action, select in apps the xdrip app and there you'll see the "what's my glucose" action.. You can then rename it to respond to any sentence you'd like. You can repeat this setting each time a slightly different sentence to have Siri respond to several options (Siri is a bit stupid in this respect).

The PR adds a GlucoseIntent struct which conforms to AppIntent and does all the work. It also adds a GlucoseIntentResponseView which is a swiftUI view showing the response (since AppIntents require swiftUI).

callms commented 6 months ago

Nice work @gshaviv

Have you considered also adding an AppShortcutsProvider so that Siri can pick up the phrase automatically, without even needing the user to add a shortcut manually?

You can add one in just a few lines of code, see for example https://github.com/open-gluck/opengluck-swift-app/blob/799196c0d11f13fe66102880b4c0c008b3a687e5/OpenGluck/AppIntents/AppsShortcuts.swift#L6

gshaviv commented 6 months ago

To use AppShortcutsProvider the phrase needs to include the app name which in case of xdrip4iO5 is not very practical. Also tried this with some other app and there seems to be a bug in iOS 16 and Siri and it doesn't pick it up automatically as advertised.

Nice work @gshaviv

Have you considered also adding an AppShortcutsProvider so that Siri can pick up the phrase automatically, without even needing the user to add a shortcut manually?

You can add one in just a few lines of code, see for example https://github.com/open-gluck/opengluck-swift-app/blob/799196c0d11f13fe66102880b4c0c008b3a687e5/OpenGluck/AppIntents/AppsShortcuts.swift#L6

callms commented 6 months ago

I feel your pain @gshaviv, I've been struggling with AppIntents and AppShortcutsProvider all week before I finally have something that works as it should. Apple documentation and Developer Experience as its finest.

To use AppShortcutsProvider the phrase needs to include the app name which in case of xdrip4iO5 is not very practical

While this is true, did you know you can have up to three synonyms, that would allow the user to use a more friendlier name? Just include a INAlternativeAppNames key in the Info.plist, see for example https://github.com/open-gluck/opengluck-swift-app/blob/64e21f49ed71bdf7be9ad5f22db24fe46ef0d40d/OpenGluck%20Phone%20App/OpenGluck-Phone-Info.plist#L22

For this project maybe we could use something like to add xDrip and Drip (along with a pronunciation hint, because Siri is notoriously bad as understanding uncommon name, and has a strong bias towards the words it already knows make sense somehow):

    <key>INAlternativeAppNames</key>
    <array>
        <dict>
            <key>INAlternativeAppName</key>
            <string>xDrip</string>
            <key>INAlternativeAppNamePronunciationHint</key>
            <string>ex drip</string>
        </dict>
        <dict>
            <key>INAlternativeAppName</key>
            <string>Drip</string>
            <key>INAlternativeAppNamePronunciationHint</key>
            <string>drip</string>
        </dict>
    </array>

Second, and not least, supporting App Shortcuts is important not only as a way to donate the phrase to Siri (which as you pointed out might not always be practical because of the app name) but also because it integrates within iOS far beyond just Siri.

For example, searching for the App Name in Spotlight will also show up your App Shortcuts, allowing the user to learn there is something to be done. And it serves a great purpose in the Shortcuts app as well, as App Shortcuts also appear here and can be used by less advanced users as a starting point to create their own shortcut, build up on your feature with the show glucose intent already in the workflow. They just need to provide a phrase and boom, they're done. That's a pretty useful feature (and not everyone is a Shortcuts guru.)

You could even push integration deeper, and [donate the intent to the system](https://developer.apple.com/documentation/appintents/appintent/donate()-1e60c) when the current glucose view becomes visible. This would allow iOS to learn how the user interacts with this screen, and say, provide a suggestion to add an automation if, for example, the AI on device learns that this is something does often on the first pickup in the morning.

In short, providing an AppShortcutsProvider is a relatively easy task, and with just a few lines of code you provide a great value to users.

Also tried this with some other app and there seems to be a bug in iOS 16 and Siri and it doesn't pick it up automatically as advertised

There are unfortunately a lot of caveats, some I have found is:

With XCode 15, you have also an invaluable tool as the App Shortcuts Preview, available in the Product menu. Use it to try variations of your phrases, and instantly see which are picked up by Siri.

Have a look at this section for screenshots and additional details: https://github.com/open-gluck/opengluck-swift-app#siri-phrases-dont-work

gshaviv commented 6 months ago

In the past my attempts to use app synonyms didn't work either, Siri just ignored them. Let me try again and see...

callms commented 6 months ago

In the past my attempts to use app synonyms didn't work either, Siri just ignored them. Let me try again and see...

The App Shortcuts Preview tool, new in XCode and Flexible shortcuts in iOS 17, is an invaluable tool to help you with debugging these issues. If you haven't given it a try yet, it really helps. Also make sure about how you structure the dict, and check the build log for potential errors or warnings.

gshaviv commented 6 months ago

Works perfectly now. Apparently when I tried it, it was on iOS 16.0.0 which was probably still with bugs and it was apparently fixed in one of the updates.

I've also added a note in the settings view mentioning this so the users will know they can speak to Siri: Screenshot 2023-12-31 at 15 09 52

And as you've suggested I also donated the intent on viewDidAppear of the rootViewController.

paulplant commented 6 months ago

Nice work @gshaviv ! This looks very interesting.

We'll look at getting some of the pending PRs checked and merged in shortly

paulplant commented 6 months ago

I've also added a note in the settings view mentioning this so the users will know they can speak to Siri:

This makes the (already untidy) Settings menu even more untidy I think. Maybe we could look at simply adding a "Siri Integration" menu row (or maybe a whole new section with just one row for now) and when providing the explanation in an alert when the user clicks the row...

It would just maybe fit in better with the already crowded UI of the current UIKit Settings menu

gshaviv commented 6 months ago

It's not a setting, it's a section footer as it's just a note with no user action needed.but let me know if you want still to add it as a separate section and I will

gshaviv commented 6 months ago

Actually not sure it's even possible, cause it will be a section with a title and a footer and no rows (no content) in between. I think the built in UI logic might just not show a section with zero rows, no?

paulplant commented 6 months ago

It's not a setting, it's a section footer as it's just a note with no user action needed.but let me know if you want still to add it as a separate section and I will

The section footers with explainer text are a great idea, but if we added just one it would look strange and out of context.

Don't worry, if we merge it, we can then change it to a row with actions to show the alert. It's fine.

paulplant commented 6 months ago

Actually not sure it's even possible, cause it will be a section with a title and a footer and no rows (no content) in between. I think the built in UI logic might just not show a section with zero rows, no?

It would need a view model to be created, but this is easy to do later, don't worry

paulplant commented 6 months ago

@gshaviv I've been able to use a (heavily) modified and abstracted version of your SwiftUI chart to produce real-time charts in the new live updates and widgets. My previous method of creating UIIMages from a UIView of the chart and save them through the shared-app-group worked perfectly to create the images, but live activities was very unreliable in consistently rendering them during the runtime available...

Using your SwiftUI chart idea works perfectly well.

We'll definitely get this PR merged in shortly, but just wanted to give you a heads-up that your work has helped a lot!

gshaviv commented 6 months ago

Great, glad to hear. Doing a widget was next on my todo, you beat me to it.

paulplant commented 3 months ago

@gshaviv , we'll start looking to merge this in after 5.1.0 is released.

As part of 5.1.0 I already wrote a very similar SwiftUI Glucose Chart which has many more options to make it extensible to all possible uses (widgets, complications, Watch app) so it would make sense to use this new one in your PR.

I'll pull your PR into a feature branch and we can get it all integrated before merging in.

paulplant commented 3 months ago

Hi @gshaviv , I finally got around to merging this in and rebased it on all of the latest commits in the staging-5.2 branch:

https://github.com/JohanDegraeve/xdripswift/tree/staging-5.2-siri-integration

Let's carry on with any new commits to that new branch.