-
Notifications
You must be signed in to change notification settings - Fork 8.1k
bluetooth: ANS: Add Alert Notification Service #95578
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.
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
base: main
Are you sure you want to change the base?
Conversation
108bcfa
to
7a19efc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements the Alert Notification Service (ANS) for Bluetooth subsystem, enabling devices to send alert notifications to connected BLE clients. The implementation follows the Bluetooth ANS 1.0 specification and includes support for both new alert and unread alert notifications across multiple categories.
- Implements complete ANS service with support for 10 alert categories (simple alert, email, news, call, etc.)
- Adds configurable category support through Kconfig options for both new and unread alert types
- Includes a sample application demonstrating ANS usage with periodic alert notifications
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
subsys/bluetooth/services/ans.c | Core ANS service implementation with GATT characteristics and API functions |
subsys/bluetooth/services/Kconfig.ans | Configuration options for enabling specific alert categories |
subsys/bluetooth/services/Kconfig | Integration of ANS Kconfig into main services menu |
subsys/bluetooth/services/CMakeLists.txt | Build system integration for ANS service |
samples/bluetooth/peripheral_ans/src/main.c | Sample application demonstrating ANS usage |
samples/bluetooth/peripheral_ans/sample.yaml | Test configuration for sample application |
samples/bluetooth/peripheral_ans/prj.conf | Project configuration for sample |
samples/bluetooth/peripheral_ans/README.rst | Documentation for sample usage |
samples/bluetooth/peripheral_ans/CMakeLists.txt | Build configuration for sample |
include/zephyr/bluetooth/services/ans.h | Public API header for ANS service |
include/zephyr/bluetooth/att.h | Addition of BT_ATT_ERR_CMD_NOT_SUP error code |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR - It's overall good quality, and most of my comments are related some potential missing features or Zephyr specific formatting or coding guidelines
7a19efc
to
57067c7
Compare
ff12748
to
47ddfac
Compare
Thanks for the review. I replied to a few comments and applied your suggestions. I also refactored the transmit functions since they got too nested. |
47ddfac
to
4c0db0f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some things that I think needs to be fixed :)
9d85235
to
2b3c021
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments, but otherwise LGTM.
Please also have a look at the issues reported by Sonar
} | ||
k_sleep(K_SECONDS(1)); | ||
|
||
ret = bt_ans_set_unread_count(NULL, BT_ANS_CAT_SIMPLE_ALERT, ++num_unread); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
num_unread
can overflow here, can't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this and #95578 (comment) it will overflow and loop back around to 0 (essentially restarting the sample). I thought this was fine for the sample. I'll add a comment to the static declaration of the variables however stating this behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's fine to overflow, but ideally we explicitly state this to avoid any confusion :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
7afa20c
to
728b1e5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good for now - The spec itself is too unclear about how to handle multiple connections, so that part is still somewhat unclear in the implementation as well.
It would have been nice with some testing of this service, but that would also require a client implementation
CI issue not related to this PR; see #96787 |
728b1e5
to
b8b126e
Compare
Just rebased |
@jhedberg or @alwa-nordic could you review when you have a chance? Thanks |
menuconfig BT_ANS | ||
bool "GATT Alert Notification Service" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add " [EXPERIMENTAL]" to the end of the description to align with select EXPERIMENTAL
menu "Unread Alert Categories" | ||
|
||
config BT_ANS_UNALRT_CAT_SIMPLE_ALERT | ||
bool "Support Simple Alert" | ||
|
||
config BT_ANS_UNALRT_CAT_EMAIL | ||
bool "Support Email" | ||
|
||
config BT_ANS_UNALRT_CAT_NEWS | ||
bool "Support News" | ||
|
||
config BT_ANS_UNALRT_CAT_CALL | ||
bool "Support Call" | ||
|
||
config BT_ANS_UNALRT_CAT_MISSED_CALL | ||
bool "Support Missed Call" | ||
|
||
config BT_ANS_UNALRT_CAT_SMS_MMS | ||
bool "Support SMS/MMS" | ||
|
||
config BT_ANS_UNALRT_CAT_VOICE_MAIL | ||
bool "Support Voice Mail" | ||
|
||
config BT_ANS_UNALRT_CAT_SCHEDULE | ||
bool "Support Schedule" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels slightly overkill to have Kconfig options for these. Did you consider having the app provide them when initializing ANS? I suppose one core reason may be that our stack-internal services don't (generally?) public exposed initialization APIs, so as soon as you need some parametrization you end up having to do this through Kconfig even though Kconfig might not otherwise be the most intuitive tool for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we did discuss it here: #95578 (comment).
To recap, I do agree, but I think since the service requires that at least one topic be enabled at all times, that it may warrant the use of the Kconfig to initially set the bitmaps.
Add alert notification service (ANS) to Bluetooth subsystem and accompanying sample. Signed-off-by: Sean Kyer <[email protected]>
Add Alert Notification Service (ANS) and enabling Kconfig to 4.3 release notes. Signed-off-by: Sean Kyer <[email protected]>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for docs / sample, thanks!
Add alert notification service (ANS) to Bluetooth subsystem and accompanying sample.
Based on spec described in https://www.bluetooth.com/specifications/specs/alert-notification-service-1-0/ with bitfields taken from https://btprodspecificationrefs.blob.core.windows.net/gatt-specification-supplement/GATT_Specification_Supplement.pdf.
To interact with this sample, download a BLE app like ADI Attach or nRF connect. Once you subscribe to the unread alert and new alert notification characteristics, you need to send a "write" to the Alert Notification Control Point characteristic to enable the categories you would like to be notified on.
In the basic example you would send: [0,0] and [1,0] to the control point and once you have subscribed to notifications you will then start receiving the messages. Further commands are explained in https://btprodspecificationrefs.blob.core.windows.net/gatt-specification-supplement/GATT_Specification_Supplement.pdf section 3.12.
It is the responsibility of the app to interpret whatever a 'new notification' or 'unread notification' is and use the APIs to update them accordingly. The service must save the most recent value for each enabled category in case the control point sends the 'immediate' request, which relays the most up to date values for the specified category.