InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.64k stars 902 forks source link

AlertNotification Service/Client are not compliant to the spec #1895

Open JF002 opened 8 months ago

JF002 commented 8 months ago

Verification

What happened?

AlertNotification Service/Client use a 3 bytes header, the spec defines a 2 bytes header

What should happen instead?

AlertNotification Service/Client should comply to the BLE spec.

Reproduction steps

N/A

More details?

This issue was reported by the Amazfish community : InfiniTime expects the header of all notifications is a 3 bytes header (here and here).

However, the GATT spec specifies a header of 2 bytes.

Now, even if InfiniTime is not 100% compliant, all companion apps have currently implemented this 3 bytes header, and changing this might break the compatibility with those companion apps so I'm not sure if we should fix this. If we do want to change the header size, we'll have to communicate with companion app developers to ensure smooth transition.

It might be possible to support both 2 and 3 bytes header by checking the value of the 3rd byte of the buffer : it can be ignored if it's equal to 0x00 or lower than 0x20.

The documentation should however be completed to mention that this is not completely compliant to the spec of the AlertNotificationService.

Version

<=1.13.0

Companion app

All

KaffeinatedKat commented 8 months ago

It might be possible to support both 2 and 3 bytes header by checking the value of the 3rd byte of the buffer : it can be ignored if it's equal to 0x00 or lower than 0x20.

This might be a good temporary solution. Support both while transitioning all the companion apps, then the support for the 3 byte header can be removed once all the apps have moved over to comply with the new header size

jmlich commented 8 months ago

For Amazfish is it one line to change. Line https://github.com/piggz/harbour-amazfish/blob/master/daemon/src/devices/pinetimejfdevice.cpp#L103 to

        alert->incomingCall(QByteArray::fromHex("0301"), caller);
minacode commented 7 months ago

I vote to change it, because not being standard compliant only gets worse with time, right?

Nobody2303 commented 7 months ago

I vote to change it, because not being standard compliant only gets worse with time, right?

agreed 🤝