Skip to content

Conversation

@eivindj-nordic
Copy link
Contributor

Use errnos for BM storage as it is not a BLE library or BLE subsystem.

@eivindj-nordic eivindj-nordic self-assigned this Oct 24, 2025
@eivindj-nordic eivindj-nordic requested review from a team as code owners October 24, 2025 10:08
@eivindj-nordic eivindj-nordic added this to the v1.0.0 milestone Oct 24, 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 Oct 24, 2025
@github-actions
Copy link

You can find the documentation preview for this PR here.

@eivindj-nordic eivindj-nordic force-pushed the error_code_alignment_storage branch 3 times, most recently from 4e7be0e to bcee21f Compare November 4, 2025 12:24
@eivindj-nordic eivindj-nordic requested review from a team and rghaddab as code owners November 4, 2025 12:24
@eivindj-nordic eivindj-nordic force-pushed the error_code_alignment_storage branch from bcee21f to 594ff03 Compare November 4, 2025 15:18
Copy link
Contributor

@anhmolt anhmolt left a comment

Choose a reason for hiding this comment

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

Could mention in the commit that
NRF_ERROR_INVALID_ADDR and NRF_ERROR_NULL are merged to -EFAULT, and
NRF_ERROR_INVALID_STATE and NRF_ERROR_FORBIDDEN are merged to -EPERM.
Also, is this something we want to do?

@eivindj-nordic eivindj-nordic force-pushed the error_code_alignment_storage branch from 594ff03 to dc0ef45 Compare November 5, 2025 08:28
@eivindj-nordic eivindj-nordic requested a review from a team as a code owner November 5, 2025 08:28
Copy link
Member

Choose a reason for hiding this comment

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

What is this file? Looks like it was generated by nrfutil? It does probably not belong here and will be overwritten on next SoftDevice export.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, it should not be here. @nordicjm Should this be added to .gitignore or can it be rendered in the build folder or so instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

@eivindj-nordic eivindj-nordic force-pushed the error_code_alignment_storage branch from dc0ef45 to 7783e7f Compare November 5, 2025 09:24

* Updated to use errno instead of nrf_errors.


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

* 0 on success.
* A negative errno otherwise.
*/
uint32_t result;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be int

* @retval 0 on success.
* @retval -EFAULT If @p storage is @c NULL.
* @retval -EBUSY If the implementation-specific resource is busy.
* @retval -EIO If an implementation-specific internal error occurred.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use EFAULT for generic failures. IO is more for read/write and such.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use EFAULT for NULL checks.

* @retval NRF_ERROR_BUSY If the implementation-specific resource is busy.
* @retval NRF_ERROR_INTERNAL If an implementation-specific internal error occurred.
* @retval 0 on success.
* @retval -EFAULT If @p storage is @c NULL.
Copy link
Contributor

@MirkoCovizzi MirkoCovizzi Nov 5, 2025

Choose a reason for hiding this comment

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

EINVAL? I prefer to use EINVAL here, but I know that we don't really have a standard for this.

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 the standard in the repo.

Fix compliance issue.

Signed-off-by: Eivind Jølsgard <[email protected]>
Use errnos for BM storage as it is not a BLE library or BLE subsystem.
Correct return values for BM ZMS.

Signed-off-by: Eivind Jølsgard <[email protected]>
ring_buf_get() returns the size retrieved and will never be negative.

Signed-off-by: Eivind Jølsgard <[email protected]>
@eivindj-nordic eivindj-nordic force-pushed the error_code_alignment_storage branch from 7783e7f to cd7a854 Compare November 5, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants