Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing return codes for security manager #93

Open
sampaioletti opened this issue Feb 19, 2024 · 4 comments · Fixed by #105
Open

Add missing return codes for security manager #93

sampaioletti opened this issue Feb 19, 2024 · 4 comments · Fixed by #105

Comments

@sampaioletti
Copy link
Contributor

Add missing return codes for security manager to BLEReturnCode

pub fn return_code_to_string(rc: u32) -> Option<&'static str> {

https://mynewt.apache.org/latest/network/ble_hs/ble_hs_return_codes.html#return-codes-security-manager-us

As a secondary point might be useful to consider returning BLEReturnCode in some additional places. i.e.

callback: impl Fn(&BLEConnDesc, c_int) + Send + Sync + 'static,

instead of c_int, which then begs the question should BLEReturnCode inner be a i32 or is u32 correct.

pub struct BLEReturnCode(pub u32);

@taks
Copy link
Owner

taks commented Feb 20, 2024

Can you create a PR?

@sampaioletti
Copy link
Contributor Author

sampaioletti commented Feb 20, 2024 via email

@sampaioletti
Copy link
Contributor Author

What is the reasoning behind the wrapped type for BLEReturnCode vs a more rusty Enum style error? Not a critique, just an ask.

@taks
Copy link
Owner

taks commented Feb 20, 2024

At the time of implementation,
if I was not sure whether the value could be converted to a BLEReturnCode, I used the value as it is.

It is better to replace BLEReturnCode if it can be replaced.

sampaioletti added a commit to sampaioletti/esp32-nimble that referenced this issue Feb 20, 2024
taks added a commit that referenced this issue Feb 20, 2024
Fixes Add missing return codes for security manager #93
@taks taks linked a pull request Feb 29, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants