taks / esp32-nimble

A wrapper for the ESP32 NimBLE Bluetooth stack.
Apache License 2.0
106 stars 32 forks source link

Add missing return codes for security manager #93

Open sampaioletti opened 6 months ago

sampaioletti commented 6 months ago

Add missing return codes for security manager to BLEReturnCode https://github.com/taks/esp32-nimble/blob/8f55ab516bc2854ac7304ff7875e8aa0066546f0/src/ble_return_code.rs#L39

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.

https://github.com/taks/esp32-nimble/blob/8f55ab516bc2854ac7304ff7875e8aa0066546f0/src/server/ble_server.rs#L89

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

https://github.com/taks/esp32-nimble/blob/8f55ab516bc2854ac7304ff7875e8aa0066546f0/src/ble_return_code.rs#L2

taks commented 6 months ago

Can you create a PR?

sampaioletti commented 6 months ago

Yeah I should have some time this week. I'll try and get something put together.

-Sam


From: taks @.> Sent: Monday, February 19, 2024 7:04:29 PM To: taks/esp32-nimble @.> Cc: sam @.>; Author @.> Subject: Re: [taks/esp32-nimble] Add missing return codes for security manager (Issue #93)

Can you create a PR?

— Reply to this email directly, view it on GitHubhttps://github.com/taks/esp32-nimble/issues/93#issuecomment-1953371888, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACDGQ52OOV4KF665VUIGCMDYUQAC3AVCNFSM6AAAAABDP7BB7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJTGM3TCOBYHA. You are receiving this because you authored the thread.Message ID: @.***>

sampaioletti commented 6 months ago

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

taks commented 6 months ago

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.