Add indicate functionality

[iwanders]

This PR add support for the indicate functionality. This is effectively a notification that gets answered with a confirmation, see section 4.11 of the specification.

Most of this PR is mechanical and creates the indication counterparts to the *_notify functions introduced in #309 by @chrissuozzo.

• The first commit corrects the spelling of a constant to match the Bluetooth documentation.
• The second commit renames send to assemble and makes it crate public, which allows eliminating manual packet assembly in the attribute file in the next commit.
• This is the bulk of the work, it adds the indication counterparts and eliminates the manual packet assembly in notify.

I don't like that the indicate method does not wait for the CFM message, like the comment says I tried using next_gatt from the indicate method itself, but that didn't guarantee I actually got the confirmation. I got another message that was a characteristic read when I did that, perhaps that was just the incoming data from before the indication was even sent? I'm not sure if this matters much, the proposed PR is better than the current state that quietly ignores the indication registration.

Changes were tested against iOS, I can see it register for indications correctly and after sending an indication it registered to on we do see the ConfirmIndication event coming from the GattConnection. I don't really know how to test the modified notification code, but the code path is so similar that I expect it to be fine.

This functionality is necessary to implement HomeKit over BLE; the iOS device registers for 'indicate' on characteristics to be 'indicated' that they changed. The indication itself is always empty but it instructs iOS to perform a read of the characteristic over the HAP secure session (which is an attribute write, followed by read). Being relatively unfamiliar with Bluetooth it took me a while to figure out that notify != indicate. The actual changes necessary turned out to be pretty straightforward.

[iwanders]

I wrote this comment before I wrote my PR description. I actually think I have a good understanding of why this happened now... It doesn't actually have anything to do with another future also being blocked on getting the event I think.

Like I said in the last paragraph of the PR, to change a value in HomeKit, the iOS device performs a BLE write to a characteristic that triggers the HomeKit write procedure to a value, then it performs a BLE read to the characteristic to retrieve the response / result of the procedure.

In my test I send an indication immediately following the Write part of the procedure, so in between the BLE Write & BLE Reads, which is why if I block in this method, I just end up accidentally consuming that BLE Read that iOS performs to get the result, instead of the confirmation.

So thinking about this some more, I don't actually think there's a way to prevent this or have this method actually wait until the confirmation?

[chrysn]

If this method can't wait for confirmation lest it drops potential read events, what'd happen if in some place one ran:

server.my_service.my_characteristic_1.indicate(&serverconn, …).await?;
server.my_service.my_characteristic_2.indicate(&serverconn, …).await?;

Wouldn't that run into the same issue, just that this time it doesn't help that we're not waiting?

[iwanders]

In that situation, with the currently proposed code, you'd send out out two indications, for two different characteristics. I tried the exact code you showed by sending two indications consecutively. In that case iOS replies with two ConfirmIndication events and both can be seen by the user code as events on the GattConnection.

Let me make a diagram of what happened in my situation when I tried to block in this method here for the confirmation message. We didn't drop events, I merely got the wrong event in this method (HAP stands for HomeKit Accessory Protocol):

sequenceDiagram
    autonumber
    participant C as Client(iOS)
    participant S as Server(Trouble) 
    C->>S: GattConnectionEvent: Char A Write Request (Start of HAP method)
    S-->>C: Char A Write Confirm
    S->>C: Indicate on Char B
    C->>S: GattConnectionEvent: Char A Read Request  (Read result of HAP method)
    C-->>S: CFM indicate
    S-->>C: Reply with Read of A (Result of HAP method)

Loading

The problem is that the indicate / confirmation (3&5), and the gatt read request/reply (4&6) are interleaved. iOS immediately sends the characteristic read request after getting the confirmation on write. If we were to block for the next event on the connection in this method, it'd be immediately after 3, and the next incoming event is 4, the characteric read, instead of the confirmation. With indications this is always a possibility, because they are server initiated instead of client initiated.

I think the current logic of not blocking is the only way to avoid reading the wrong message. User code can still monitor whether an Confirmation event was received within a time frame after sending out an indication in case it wants to send another one.

I hope this clarifies it!

PS, first time I could actually use that mermaid diagram support that GH added, it's much better than I would've drawn in Inkscape.

[chrysn]

I've tried this out with a bluez central, and generally found Indicate working. Thanks a lot for writing this.

What I had trouble with were characteristics that had both notify and indicate in their #[characteristic()] line, but that turned out to be a widespread platform issue (with the maintainer who observed this noting that Indicate isn't much used anyway). This doesn't detract from this PR's value, it's just a warning that while it's possible to do notify and indicate selectively, the outcome with desktop peers will that neither works reliably.

[chrysn]

What feedback does the using application then get of an indication? Will other operations reliably be postponed until the confirmation has arrived (including, possibly, retransmits of the indication), and produce the error there? This would be helpful to have in the function's documentation.

[iwanders]

I'll add a note that if the client replied, it will show up as a ConfirmIndication GattEvent.

[iwanders]

I've added the note to clarify to users where the confirmations will show up, links are clickable.

[chrysn]

If this method can't wait for confirmation lest it drops potential read events, what'd happen if in some place one ran:

server.my_service.my_characteristic_1.indicate(&serverconn, …).await?;
server.my_service.my_characteristic_2.indicate(&serverconn, …).await?;

Wouldn't that run into the same issue, just that this time it doesn't help that we're not waiting?

[iwanders]

I've tried this out with a bluez central, and generally found Indicate working. Thanks a lot for writing this.

Excellent, thanks for testing!

What I had trouble with were characteristics that had both notify and indicate in their #[characteristic()] line, but that turned out to be a widespread platform issue (with the maintainer who observed this noting that Indicate isn't much used anyway).

Oh, most intriguing... So before I learned indicate != notify, I did actually bypass the should_notify check and found a notification also made iOS request the value over the HAP session, I later wondered if that was a bug in the iOS bluetooth stack because it hadn't actually subscribed to notifications, but if they treat both similarly I expect it makes sense since iOS did subscribe to indications.

[HaoboGu]

lgtm!

[HaoboGu]

/test

[iwanders]

Looks like the hardware-in-the-loop test failed 😱 Is there any way we can get the logs for those to figure out what went wrong and if it's related to this PR or an unrelated cause?

[HaoboGu]

Looks like the hardware-in-the-loop test failed 😱 Is there any way we can get the logs for those to figure out what went wrong and if it's related to this PR or an unrelated cause?

cc @lulf