Skip to content

Conversation

@lemrey
Copy link
Contributor

@lemrey lemrey commented Dec 15, 2025

No description provided.

@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Dec 15, 2025
@github-actions
Copy link

You can find the documentation preview for this PR here.

@lemrey lemrey force-pushed the services-cleanup branch 2 times, most recently from 92761c1 to 1b81cd8 Compare December 15, 2025 12:05
@lemrey lemrey force-pushed the services-cleanup branch 3 times, most recently from f01a41e to ad177b5 Compare December 16, 2025 13:12
@lemrey lemrey marked this pull request as ready for review December 16, 2025 13:14
@lemrey lemrey requested review from a team as code owners December 16, 2025 13:14
@lemrey lemrey force-pushed the services-cleanup branch 2 times, most recently from 09cfa57 to 29fae85 Compare December 17, 2025 08:50
Remove stray doxygen comment.

Signed-off-by: Emanuele Di Santo <[email protected]>
@lemrey lemrey force-pushed the services-cleanup branch 3 times, most recently from 8663a9b to b451cef Compare January 6, 2026 14:15
@lemrey lemrey requested a review from a team as a code owner January 6, 2026 14:15
@github-actions github-actions bot added doc-required PR must not be merged without tech writer approval. and removed changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Jan 6, 2026
lemrey added 6 commits January 6, 2026 15:18
Remove outdated doxygen documentation.

Signed-off-by: Emanuele Di Santo <[email protected]>
Remove unnecessary cast.

Signed-off-by: Emanuele Di Santo <[email protected]>
Remove unnecessary cast.

Signed-off-by: Emanuele Di Santo <[email protected]>
Do not translate the errors from SoftDevice calls, instead return
those directly.

Signed-off-by: Emanuele Di Santo <[email protected]>
SoftDevice will return an error if they are not, no need to check that
ourselves in the service implementation.

Signed-off-by: Emanuele Di Santo <[email protected]>
Use compound literal for the notification size.

Signed-off-by: Emanuele Di Santo <[email protected]>
This function is mostly called automatically by the SoftDevice handler,
so only assert if the parameters are NULL.

Signed-off-by: Emanuele Di Santo <[email protected]>
lemrey added 4 commits January 6, 2026 15:20
Remove NULL checks in the BLE event handler. This handler is mostly
called by the SoftDevice handler automatically, just use asserts.

Signed-off-by: Emanuele Di Santo <[email protected]>
Rename to `nus_instance` for clarity and consistency with other services.

Signed-off-by: Emanuele Di Santo <[email protected]>
Remove checks on the parameters to the SoftDevice _hvx() call.
All SoftDevice APIs implement error checking already, just call
the SoftDevice function directly.

Signed-off-by: Emanuele Di Santo <[email protected]>
Remove error translation for the SoftDevice _hvx() call.
Return the SoftDevice error directly. The return value of the call
invoking _hvx() is anyway only checked against zero,
its actual value is not used.

Signed-off-by: Emanuele Di Santo <[email protected]>
Comment on lines -290 to -295
case NRF_SUCCESS:
if (hvx_len != len) {
LOG_ERR("Notified %d of %d bytes", hvx_len, len);
return NRF_ERROR_INVALID_PARAM;
}
return NRF_SUCCESS;
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 is a change in behavior, where earlier we returned an error now we return success.

Comment on lines -314 to -329
if (*len > BLE_NUS_MAX_DATA_LEN) {
return NRF_ERROR_INVALID_PARAM;
}

if (conn_handle == BLE_CONN_HANDLE_INVALID) {
return NRF_ERROR_NOT_FOUND;
}

ctx = ble_nus_client_context_get(nus, conn_handle);
if (ctx == NULL) {
return NRF_ERROR_NOT_FOUND;
}

if (!ctx->is_notification_enabled) {
return NRF_ERROR_INVALID_PARAM;
}
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 is also gone and a change in behavior

Copy link
Contributor Author

@lemrey lemrey left a comment

Choose a reason for hiding this comment

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

There will be a follow up PR to update samples not to send notifications blindly. There were a bunch of checks in the application and in the service to ignore / override errors. This patch removes such checks in the services.

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.

2 participants