Bluetooth: Immediate Alert Service #43574
Conversation
szymon-czapracki
requested review from
Vudentz,
alwa-nordic,
andyross,
dcpleung,
jhedberg and
nashif
as code owners
github-actions
bot
added
area: API
|
@MariuszSkamra FYI |
szymon-czapracki
force-pushed
the
ias
branch
4 times, most recently
from
b2fbbea to
a4c1fe0
Compare
include/linker/common-rom.ld
Outdated
There was a problem hiding this comment.
Not at all opposed to allow statically registered callbacks, but I am not sure how this is properly added.
I see that for e.g. bt_conn_cb, it is added to not only this file, but also some of the ESP32 linker.ld files.
Do you know how this is properly added? If adding it to only here, does it then not work on e.g. the ESP32 devices?
There was a problem hiding this comment.
Unfortunately we have to manually add this in ESP32/S2/C3 as well. We are still to figure out a way to overcome this.
There was a problem hiding this comment.
Just an update. This issue regarding ESP32 linker script would/will be fixed in this #43659.
include/bluetooth/services/ias.h
Outdated
There was a problem hiding this comment.
What is the advantage of doing it this way?
There was a problem hiding this comment.
Callbacks are stored in ROM instead of RAM :)
subsys/bluetooth/services/ias.c
Outdated
There was a problem hiding this comment.
Why is it called a sample? How about "implementation"?
subsys/bluetooth/services/ias.c
Outdated
There was a problem hiding this comment.
What if the ongoing alert has been stopped by the user? In that case, this should start a new one, rather than just returning.
subsys/bluetooth/services/ias.c
Outdated
There was a problem hiding this comment.
Setting alert_level should be outside the loop.
alwa-nordic
left a comment
alwa-nordic
left a comment
There was a problem hiding this comment.
The IAS spec is not clear on behavior when multiple peers are connected. But I think clearing the alert when any peer disconnects is wrong.
The Alert Level characteristic is not readable. This makes me think that each GATT client should have a separate alert level.
Thalley
left a comment
Thalley
left a comment
There was a problem hiding this comment.
LGTM on the condition that we mark it as experimental in the Kconfig as well, so that we may revisit the API later more easily
There was a problem hiding this comment.
Needs a select EXPERIMENTAL
There was a problem hiding this comment.
There should also be " [EXPERIMENTAL]" at the end of the description.
szymon-czapracki
force-pushed
the
ias
branch
2 times, most recently
from
b52abe7 to
a601be6
Compare
|
@szymon-czapracki please rebase |
This commits adds IAS for zephyr bluetooth. Signed-off-by: Szymon Czapracki <[email protected]>
szymon-czapracki
dismissed stale reviews from asbjornsabo, jhedberg, MariuszSkamra, and Thalley
via
83ec366
Done |
MariuszSkamra
requested review from
MariuszSkamra,
Thalley,
asbjornsabo and
jhedberg
10 participants
Add Immediate Alert Service