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.

Sign up for GitHub

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

Jump to bottom

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

Open
1 task done
JF002 opened this issue Oct 23, 2023 · 4 comments · May be fixed by #1968
Open
1 task done

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

JF002 opened this issue Oct 23, 2023 · 4 comments · May be fixed by #1968
Labels
bug Something isn't working

Comments

@JF002
Copy link
Collaborator

JF002 commented Oct 23, 2023

Verification

  • I searched for similar bug reports (including closed issues) and found none was relevant.

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
@JF002 JF002 added the bug Something isn't working label Oct 23, 2023
@KaffeinatedKat
Copy link
Contributor

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
Copy link

jmlich commented Oct 27, 2023 •
edited

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
Copy link
Contributor

minacode commented Nov 5, 2023

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

@Nobody2303
Copy link

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

agreed 🤝

jmlich added a commit to jmlich/InfiniTime that referenced this issue Dec 28, 2023
@jmlich
Fixes InfiniTimeOrg#1895: use just 2 bytes header according to GATT s…
277089a 
…pecification
jmlich added a commit to jmlich/InfiniTime that referenced this issue Jan 15, 2024
@jmlich
header size is 3 only for customHuami category
a696c8f 
header size should be 2 according to specification

fixes: InfiniTimeOrg#1895
jmlich added a commit to jmlich/InfiniTime that referenced this issue Jan 15, 2024
@jmlich
header size is 3 only for customHuami category
2429fca 
header size should be 2 according to specification

fixes: InfiniTimeOrg#1895
@jmlich jmlich linked a pull request Jan 15, 2024 that will close this issue
Open
jmlich added a commit to jmlich/InfiniTime that referenced this issue Jan 15, 2024
@jmlich
header size is 3 only for customHuami category
347487b 
header size should be 2 according to specification

fixes: InfiniTimeOrg#1895
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

header size is 3 only for customHuami category jmlich/InfiniTime
5 participants
@JF002 @jmlich @minacode @Nobody2303 @KaffeinatedKat