library and services event handling and cleanup by eivindj-nordic · Pull Request #447 · nrfconnect/sdk-nrf-bm

eivindj-nordic · 2025-10-28T09:48:37Z

No description provided.

github-actions · 2025-10-28T09:49:47Z

You can find the documentation preview for this PR here.

lemrey

Had a brief look

lemrey · 2025-10-29T12:59:18Z

 	 */
-	BLE_BAS_EVT_NOTIFICATION_DISABLED
+	BLE_BAS_EVT_NOTIFICATION_DISABLED,
+


extra new line

lemrey · 2025-10-29T13:00:35Z

+		/**
+		 * @brief Connection handle for which the event applies.
+		 *
+		 * Used for BLE_BAS_EVT_NOTIFICATION_ENABLED and BLE_BAS_EVT_NOTIFICATION_DISABLED.
+		 */
+		uint16_t conn_handle;


The conn_handle should be part of the outermost structure, because otherwise it's impossible to know which peer any message belongs to.

lemrey · 2025-10-29T13:01:06Z

+	/**
+	 * @brief Error event.
+	 */
+	BLE_NUS_EVT_ERROR,


nit: I would keep the _ERROR event last in the enum everywhere.

It is the first everywhere so that it has the same value for all libraries and services. Having it at the end will give different values, and it will change if another event is added.

Sure but currently there is no advantage for that. Do you have something in mind?

Nothing in particular with the use of the value, no.

ok, I am a bit unsure whether we should have it first then; typically I would expect it to be last..

Moved last.

lemrey · 2025-10-29T13:01:51Z

+		/** @ref BLE_BAS_EVT_ERROR event data. */
+		struct {
+			/** Error reason. */
+			uint32_t reason;
+		} error;


nit: I would keep this last in the list.

Order kept the same as the events.

Do you need to update here?

lemrey · 2025-10-29T13:03:20Z

@@ -55,6 +62,11 @@ struct ble_lbs_evt {
 			/** Value to write */
 			uint8_t value;
 		} led_write;


I think the conn_handle should be in the outermost structure here too.

lemrey · 2025-10-29T13:05:49Z

+			/** Connection handle. */
+			uint16_t conn_handle;


MirkoCovizzi · 2025-11-06T13:35:26Z

+      if (nrf_err) {
+         LOG_ERR("pm_register() failed, nrf_error 0x%x", nrf_err);
+         return nrf_err;
      }


I realize that this is missing a return NRF_SUCCESS.

Added in #437, where the file is changed.

lemrey · 2025-11-14T11:44:23Z

+		/** @ref BLE_BAS_EVT_ERROR event data. */
+		struct {
+			/** Error reason. */
+			uint32_t reason;
+		} error;


Do you need to update here?

eivindj-nordic · 2025-11-14T11:54:01Z

Do you need to update here?

Done.

Add error event to NUS service. Align event handler with other services. Align event type field name to evt_type. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

Add error event to BAS service. The event is currently unused, but is added to align with other services. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

* Formatting fixes to doxygen * Add error logging where error event is sent. This makes it easier to debug. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

Add error event to HRS service. The event is currently unused, but is added to align with other services. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

Add error event to LBS service. The event is currently unused, but is added to align with other services. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

* Add error logging where the error event is sent to the application. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

* Add error event to ble_conn_params library. * Change conn_params_event id field to evt_type to align with other. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

Align event handler for ble_gq with other libraries. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

Align error event at end of enum. Doxygen format fixes. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

eivindj-nordic self-assigned this Oct 28, 2025

eivindj-nordic requested review from a team as code owners October 28, 2025 09:48

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 28, 2025

eivindj-nordic force-pushed the error_handlers branch 3 times, most recently from 3a36230 to cfe69f7 Compare October 29, 2025 11:44

lemrey reviewed Oct 29, 2025

View reviewed changes

eivindj-nordic force-pushed the error_handlers branch 4 times, most recently from 73b576d to 342b13a Compare November 3, 2025 12:33

lemrey added this to the v1.0.0 milestone Nov 5, 2025

MirkoCovizzi reviewed Nov 6, 2025

View reviewed changes

eivindj-nordic force-pushed the error_handlers branch from 342b13a to 16cf730 Compare November 12, 2025 12:56

github-actions Bot removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Nov 12, 2025

b-gent approved these changes Nov 13, 2025

View reviewed changes

eivindj-nordic requested review from MirkoCovizzi and lemrey November 14, 2025 06:55

eivindj-nordic force-pushed the error_handlers branch from 16cf730 to d64c37b Compare November 14, 2025 10:59

lemrey reviewed Nov 14, 2025

View reviewed changes

eivindj-nordic force-pushed the error_handlers branch from d64c37b to b0babb9 Compare November 14, 2025 11:53

lemrey approved these changes Nov 14, 2025

View reviewed changes

eivindj-nordic force-pushed the error_handlers branch 2 times, most recently from e6ab2d5 to c2396cc Compare November 17, 2025 11:09

eivindj-nordic force-pushed the error_handlers branch 6 times, most recently from 0cbafcc to 9e0a309 Compare November 17, 2025 12:31

anhmolt reviewed Nov 17, 2025

View reviewed changes

eivindj-nordic force-pushed the error_handlers branch from 9e0a309 to 221a91e Compare November 17, 2025 13:23

eivindj-nordic requested a review from anhmolt November 18, 2025 09:58

eivindj-nordic force-pushed the error_handlers branch from 221a91e to 31e0b3e Compare November 18, 2025 11:13

anhmolt reviewed Nov 18, 2025

View reviewed changes

eivindj-nordic added 10 commits November 18, 2025 13:05

bluetooth: services: ble_nus: add error event and align event handler

170eb2a

Add error event to NUS service. Align event handler with other services. Align event type field name to evt_type. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

bluetooth: services: ble_bas: add error event

961af01

Add error event to BAS service. The event is currently unused, but is added to align with other services. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

bluetooth: services: ble_cgms: formating fixes and error logging

c14a997

* Formatting fixes to doxygen * Add error logging where error event is sent. This makes it easier to debug. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

bluetooth: services: ble_hids: formating fixes and error logging

f3d4fc5

* Formatting fixes to doxygen * Add error logging where error event is sent. This makes it easier to debug. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

bluetooth: services: ble_hrs: add error event

b153125

Add error event to HRS service. The event is currently unused, but is added to align with other services. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

bluetooth: services: ble_lbs: add error event

ebcbf48

Add error event to LBS service. The event is currently unused, but is added to align with other services. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

lib: bluetooth: ble_adv: error logging

68f077c

* Add error logging where the error event is sent to the application. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

lib: bluetooth: ble_conn_params: Error event and change id to evt_type

e8f11df

* Add error event to ble_conn_params library. * Change conn_params_event id field to evt_type to align with other. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

lib: bluetooth: ble_gq: align event handler

ecd5d69

Align event handler for ble_gq with other libraries. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

include: bm: bluetooth: ble_qwr: align error event and doxygen fixes

d4331bd

Align error event at end of enum. Doxygen format fixes. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

eivindj-nordic force-pushed the error_handlers branch from 31e0b3e to d4331bd Compare November 18, 2025 12:05

eivindj-nordic requested a review from anhmolt November 18, 2025 13:04

anhmolt approved these changes Nov 19, 2025

View reviewed changes

eivindj-nordic merged commit 6e05db7 into nrfconnect:main Nov 19, 2025
18 checks passed

Conversation

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

Uh oh!

Uh oh!

github-actions Bot commented Oct 28, 2025

Uh oh!

lemrey left a comment

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!

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!

Uh oh!

eivindj-nordic commented Nov 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

eivindj-nordic commented Oct 28, 2025 •

edited

Loading