Skip to content

Conversation

@eivindj-nordic
Copy link
Contributor

Add bond management service.

@eivindj-nordic eivindj-nordic self-assigned this Aug 18, 2025
@eivindj-nordic eivindj-nordic requested review from a team as code owners August 18, 2025 12:12
@eivindj-nordic eivindj-nordic added the DNM Do not merge label Aug 18, 2025
@eivindj-nordic eivindj-nordic added this to the v0.9.0 milestone Aug 19, 2025
@eivindj-nordic eivindj-nordic requested a review from a team as a code owner August 25, 2025 11:46
@github-actions
Copy link

You can find the documentation preview for this PR here.

@eivindj-nordic eivindj-nordic removed this from the v0.9.0 milestone Sep 2, 2025
@github-actions github-actions bot added changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. doc-required PR must not be merged without tech writer approval. labels Sep 29, 2025
@eivindj-nordic eivindj-nordic removed the DNM Do not merge label Oct 3, 2025
@eivindj-nordic eivindj-nordic added this to the v1.0.0 milestone Oct 13, 2025
@eivindj-nordic eivindj-nordic added the DNM Do not merge label Oct 21, 2025
@eivindj-nordic eivindj-nordic force-pushed the ble_bms branch 2 times, most recently from 0dd951c to 8a689af Compare November 7, 2025 13:03
@eivindj-nordic eivindj-nordic requested a review from a team as a code owner November 7, 2025 13:03
@eivindj-nordic
Copy link
Contributor Author

Added unit tests. Coverage is not perfect, though I have to prioritize other tasks.
image

Events from the service are forwarded through the event handler specified during initialization.
For a full list of events see the :c:enum:`ble_bms_evt_type` enum.


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra newline.

| Header file: :file:`include/bluetooth/services/ble_bms.h`
| Source files: :file:`subsys/bluetooth/services/ble_bms/`

:ref:`Battery Service API reference <api_ble_bms>`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
:ref:`Battery Service API reference <api_ble_bms>`
:ref:`Bond Management Service API reference <api_ble_bms>`

default 512

config BLE_BMS_PEERS_TO_DELETE_ON_DISCONNECT_MAX
int "Maximum nmber of peers to delete pending peer disconnect"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int "Maximum nmber of peers to delete pending peer disconnect"
int "Maximum number of peers to delete pending peer disconnect"

}
}


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra newline.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, comments addressed.


/** @brief BMS event types. */
enum ble_bms_evt_type {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

* authorization code is not valid.
*/
uint16_t ble_bms_on_qwr_evt(struct ble_bms *bms, struct ble_qwr *qwr,
const struct ble_qwr_evt *evt);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad alignment

cmake_minimum_required(VERSION 3.20.0)

find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
project(ble_lbs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong name

default 10

module=BLE_BMS_SAMPLE
module-dep=LOG
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
module-dep=LOG

let's not keep re-adding this thing that has never done anything

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

LOG_ERR("Failed to delete peer, nrf_error %#x", err);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

goto idle;
}

const bool erase_bonds = bm_buttons_is_pressed(BOARD_PIN_BTN_1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

declare variables at top of scope

default "ABCD"

module=BLE_BMS
module-dep=LOG
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
module-dep=LOG


menu "BLE BMS sample"

config QWR_MEM_BUFF_SIZE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefix application/sample Kconfigs with APP_ so that they won't possibly conflict with future Kconfigs added to subsystems

default 512

config BLE_BMS_PEERS_TO_DELETE_ON_DISCONNECT_MAX
int "Maximum number of peers to delete pending peer disconnect"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not clear to me what this does

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int "Maximum number of peers to delete pending peer disconnect"
int "Maximum number of peers to delete upon a peer disconnect"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewritten as suggested by @MirkoCovizzi.

break;

case BLE_GAP_EVT_DISCONNECTED:
LOG_INF("Peer disconnected, reason %d",
Copy link
Contributor

@MirkoCovizzi MirkoCovizzi Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see, these values (BLE_HCI_STATUS_CODES) are defined as hex.

Suggested change
LOG_INF("Peer disconnected, reason %d",
LOG_INF("Peer disconnected, reason %#x",

Copy link

@peknis peknis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering about the capitalization. Server or server?

Deleting the bonds
==================

The Server deletes bonding information on client's request right away when there is no active Bluetooth® Low Energy connection associated with a bond.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to capitalize the Server?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it should be capitalized. In fact, the counterpart client is not, in the same sentence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was capitalized in nRF5 documentation, though I agree it is not needed.

@eivindj-nordic eivindj-nordic force-pushed the ble_bms branch 2 times, most recently from 52d0aac to 78af194 Compare November 28, 2025 12:30
int "Softdevice event observer priority"
default 0

config USE_AUTHORIZATION_CODE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add prefix to these, "use authorization code" has no context, e.g. BLE_BMS_USE_AUTHORIZATION_CODE, fix in whole PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, updated.

bool "Use authorization code"
default y

config AUTHORIZATION_CODE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed the update?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems so, updated now...

Copy link
Contributor

@MechanicV1 MechanicV1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving for tests !

@eivindj-nordic eivindj-nordic force-pushed the ble_bms branch 7 times, most recently from 446c21a to e693175 Compare December 1, 2025 16:54
Add bond management service.

Signed-off-by: Eivind Jølsgard <[email protected]>
@eivindj-nordic eivindj-nordic added the doc-postponed PR requires documentation updates, tech writer review happens later. label Dec 1, 2025
@eivindj-nordic
Copy link
Contributor Author

doc-postponed: missing changelog entries.

@eivindj-nordic eivindj-nordic merged commit 82aa365 into nrfconnect:main Dec 1, 2025
19 of 20 checks passed
@eivindj-nordic
Copy link
Contributor Author

Release notes changelog added in #557.

@eivindj-nordic eivindj-nordic removed changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. doc-postponed PR requires documentation updates, tech writer review happens later. labels Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-required PR must not be merged without tech writer approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants