lib: bluetooth: ble_adv: change error codes to nrf_error #436

eivindj-nordic · 2025-10-22T07:14:03Z

Change error codes to nrf_errors.

This is the first PR for updating the error codes for BLE libraries. Starting with the ble_adv library.
Moving forward all non-BLE libraries and components will return errnos, while BLE related code as BLE services, libraries and the SoftDevice will return nrf_errors.

github-actions · 2025-10-22T07:15:37Z

You can find the documentation preview for this PR here.

applications/firmware_loader/ble_mcumgr/src/main.c

marenges · 2025-10-22T08:02:55Z

applications/firmware_loader/ble_mcumgr/src/main.c


-	if (err) {
-		LOG_ERR("Failed to initialize advertising, err %d", err);
+	if (nrf_err != NRF_SUCCESS) {


Do we need to be this explicit? I think it's easier to read if (nrf_err)

I think we should for good practice, as we don't have full control over the nrf_error values (although in practice it is very unlikely that NRF_SUCCESS will change). It also makes a clear distinction between errnos and nrf_errors.

I think we can safely stick to the simpler:

if (nrf_err) { }

lemrey · 2025-10-22T08:26:35Z

applications/firmware_loader/ble_mcumgr/src/main.c


-	if (err) {
-		LOG_ERR("Failed to initialize advertising, err %d", err);
+	if (nrf_err != NRF_SUCCESS) {


I think we can safely stick to the simpler:

if (nrf_err) { }

lemrey · 2025-10-22T08:27:34Z

include/nrf_error.h

Why are you changing this file?

Should not be part of it...

lemrey · 2025-10-22T08:28:37Z

lib/bluetooth/ble_adv/ble_adv_data.c

 	/* Check parameter consistency */
 	if (ble_adv_data->srv_list.service == NULL) {
-		return -EFAULT;
+		return NRF_ERROR_INVALID_PARAM;


Use NRF_ERROR_NULL

lemrey · 2025-10-22T08:28:54Z

lib/bluetooth/ble_adv/ble_adv_data.c

 		if (service_data->len > 0) {
 			if (service_data->data == NULL) {
-				return -EINVAL;
+				return NRF_ERROR_INVALID_PARAM;


Use NRF_ERROR_NULL when a check against NULL fails

lemrey · 2025-10-22T08:33:35Z

lib/bluetooth/ble_adv/ble_adv.c

 {
 #if CONFIG_BLE_ADV_FAST_ADVERTISING
-	int err;
+	uint32_t err;


We should decide whether we want to store nrf_error values into nrf_err or whether it can be called generically err where there is only one error.

Yes, I was thinking about that as well. For now I've only updated in the samples where it collides.

lib/bluetooth/ble_adv/ble_adv.c

anhmolt · 2025-10-23T07:44:51Z

applications/firmware_loader/ble_mcumgr/src/main.c

-		LOG_ERR("Failed to start advertising, err %d", err);
+	nrf_err = ble_adv_start(&ble_adv, BLE_ADV_MODE_FAST);
+	if (nrf_err) {
+		LOG_ERR("Failed to start advertising, nrf_error %#x", err);


Suggested change

LOG_ERR("Failed to start advertising, nrf_error %#x", err);

LOG_ERR("Failed to start advertising, nrf_error %#x", nrf_err);

anhmolt · 2025-10-23T08:01:00Z

samples/bluetooth/ble_hids_keyboard/src/main.c

 static int advertising_start(bool erase_bonds)
 {
-	int err = 0;
+	uint32_t nrf_err = 0;


Suggested change

uint32_t nrf_err = 0;

uint32_t nrf_err = NRF_SUCCESS;

anhmolt · 2025-10-23T08:09:26Z

tests/lib/bluetooth/ble_adv/src/unity_test.c


 	__cmock_sd_ble_gap_adv_set_configure_IgnoreAndReturn(NRF_ERROR_INVALID_PARAM);
 	ret = ble_adv_start(&ble_adv, BLE_ADV_MODE_SLOW);
-	TEST_ASSERT_EQUAL(-EINVAL, ret);


Name of test still ends with _einval.

Corrected, thanks!

anhmolt · 2025-10-23T08:17:10Z

include/bm/bluetooth/ble_adv.h

+ * @retval NRF_SUCCESS On success.
+ * @retval NRF_ERROR_INVALID_STATE  Library is not initialized.
+ * @retval NRF_ERROR_NULL @p ble_adv is @c NULL.
+ * @retval NRF_ERROR_INVALID_PARAM Invalid parameters.


Maybe align the error description here and for the other functions.

Suggested change

* @retval NRF_SUCCESS On success.

* @retval NRF_ERROR_INVALID_STATE Library is not initialized.

* @retval NRF_ERROR_NULL @p ble_adv is @c NULL.

* @retval NRF_ERROR_INVALID_PARAM Invalid parameters.

* @retval NRF_SUCCESS On success.

* @retval NRF_ERROR_INVALID_STATE Library is not initialized.

* @retval NRF_ERROR_NULL @p ble_adv is @c NULL.

* @retval NRF_ERROR_INVALID_PARAM Invalid parameters.

There are mant places we don't do that, but use a single space. Let's discuss offline and agree on the format for the repo.

anhmolt · 2025-10-23T08:38:24Z

lib/bluetooth/ble_adv/ble_adv_data.c

+	nrf_err = sd_ble_gap_addr_get(&device_addr);
+	if (nrf_err) {
+		LOG_ERR("Failed to get device GAP address, nrf_error %#x", nrf_err);
+		return NRF_ERROR_INVALID_ADDR;


This is the only error returned by the sd_ble_gap_addr_get apart from NRF_SUCCESS. The nRF5 SDK seems to return the error directly from sd_ble_gap_addr_get here.

anhmolt · 2025-10-23T08:40:40Z

lib/bluetooth/ble_adv/ble_adv_data.c

+	nrf_err = sd_ble_gap_device_name_get(&data[*offset + AD_DATA_OFFSET], &actual_length);
+	if (nrf_err) {
+		LOG_ERR("Failed to get device GAP name, nrf_error %#x", nrf_err);
+		return NRF_ERROR_INVALID_PARAM;


Could maybe return nrf_err here. The nRF5 SDK returns the error directly from sd_ble_gap_device_name_get.

anhmolt · 2025-10-23T08:43:35Z

lib/bluetooth/ble_adv/ble_adv_data.c

+	nrf_err = sd_ble_gap_appearance_get(&appearance);
+	if (nrf_err) {
+		LOG_ERR("Failed to get GAP appearance, nrf_error %#x", nrf_err);
+		return NRF_ERROR_INVALID_ADDR;


The nRF5 SDK seems to return directly from sd_ble_gap_appearance_get here.

lib/bluetooth/ble_adv/ble_adv.c

lemrey · 2025-10-24T11:25:19Z

lib/bluetooth/ble_adv/ble_adv.c

-			err = ble_adv_start(ble_adv, BLE_ADV_MODE_DIRECTED_HIGH_DUTY);
-			if (err) {
+			nrf_err = ble_adv_start(ble_adv, BLE_ADV_MODE_DIRECTED_HIGH_DUTY);
+			if (nrf_err != NRF_SUCCESS) {


I don't think we should be checking explictly against NRF_SUCCESS, it creates lots of noise for nothing.

Missed this here, will update.

Change error codes to nrf_errors. Signed-off-by: Eivind Jølsgard <[email protected]>

eivindj-nordic self-assigned this Oct 22, 2025

eivindj-nordic requested review from a team as code owners October 22, 2025 07:14

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 Oct 22, 2025

eivindj-nordic force-pushed the error_code_alignment_ble_adv branch 3 times, most recently from c94cb69 to 6fc813d Compare October 22, 2025 07:22

eivindj-nordic mentioned this pull request Oct 22, 2025

treewide: bluetooth: change error codes to nrf_error #437

Open

marenges reviewed Oct 22, 2025

View reviewed changes

applications/firmware_loader/ble_mcumgr/src/main.c Outdated Show resolved Hide resolved

eivindj-nordic force-pushed the error_code_alignment_ble_adv branch from 6fc813d to c2fa6b6 Compare October 22, 2025 08:01

marenges reviewed Oct 22, 2025

View reviewed changes

lemrey reviewed Oct 22, 2025

View reviewed changes

eivindj-nordic force-pushed the error_code_alignment_ble_adv branch 4 times, most recently from 031b124 to 89653ba Compare October 22, 2025 13:03

anhmolt reviewed Oct 23, 2025

View reviewed changes

eivindj-nordic force-pushed the error_code_alignment_ble_adv branch from 89653ba to ff36289 Compare October 24, 2025 07:15

eivindj-nordic added this to the v1.0.0 milestone Oct 24, 2025

lemrey reviewed Oct 24, 2025

View reviewed changes

lib: bluetooth: ble_adv: change error codes to nrf_error

9c3c4e3

Change error codes to nrf_errors. Signed-off-by: Eivind Jølsgard <[email protected]>

eivindj-nordic force-pushed the error_code_alignment_ble_adv branch from ff36289 to 9c3c4e3 Compare October 24, 2025 11:29

	LOG_ERR("Failed to start advertising, nrf_error %#x", err);
	LOG_ERR("Failed to start advertising, nrf_error %#x", nrf_err);

lib: bluetooth: ble_adv: change error codes to nrf_error #436

Are you sure you want to change the base?

lib: bluetooth: ble_adv: change error codes to nrf_error #436

Uh oh!

Conversation

eivindj-nordic commented Oct 22, 2025

Uh oh!

github-actions bot commented Oct 22, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eivindj-nordic Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eivindj-nordic Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

eivindj-nordic Oct 22, 2025 •

edited

Loading

eivindj-nordic Oct 24, 2025 •

edited

Loading