Mesh-api setters for eui64 and pskd #6025
Conversation
kseverinkangas-zg
force-pushed
the
thread_api_adds
branch
4 times, most recently
from
10eab57 to
1f605ad
Compare
|
@kjbracey-arm please review |
| * \return MESH_ERROR_NONE on success. | ||
| * \return MESH_ERROR_PARAM in case of illegal parameters. | ||
| * */ | ||
| mesh_error_t device_eui64_set(uint8_t *eui64); |
There was a problem hiding this comment.
Needs to be a const pointer. Don't see the point of an error return, if it's just a null check. How can an eui64 be invalid? I guess you could fault multicast ones?
There was a problem hiding this comment.
Giving NULL is the only case of an error. This is an application api, so even a simple NULL check should be done? Could be done with assert?
There was a problem hiding this comment.
I'm just generally very down on spending code space specifically on NULL checks in APIs. If someone somehow does it, it'll crash. There are lots of other invalid things - pointers to uninitialised memory, pointers to already-freed blocks, pointers outside RAM, multicast addresses... You can't totally protect people from themselves.
With the error test, you'll return the error, and they just won't bother checking the return code anyway...
Given that the existing calls in this API don't bother with NULL checks, I don't see a reason to start now.
Anyway, minor style nit - not really that bothered. The code-in-assert thing below is critical though.
| @@ -278,14 +278,10 @@ void thread_tasklet_configure_and_connect_to_network(void) | |||
|
|
|||
| // PSKd | |||
| const char PSKd[] = MBED_CONF_MBED_MESH_API_THREAD_PSKD; | |||
| MBED_ASSERT(sizeof(PSKd) > 5 && sizeof(PSKd) < 33); | |||
| if(device_configuration.PSKd_len==0) { | |||
| MBED_ASSERT(!thread_tasklet_device_pskd_set(PSKd)); | |||
There was a problem hiding this comment.
You can't put code in an assert - it won't be called if a release build.
|
@karsev Do you want to rebase to clean the log before we trigger tests (possibly squash to one commit ? ) ? |
kseverinkangas-zg
force-pushed
the
thread_api_adds
branch
from
5f44e19 to
f63dbf5
Compare
|
/morph build |
Build : SUCCESSBuild number : 1090 Triggering tests/morph test |
Test : SUCCESSBuild number : 897 |
Exporter Build : SUCCESSBuild number : 767 |
Notes:
Description
New setters for mesh-api to be used by the Thread network.
Needed by customers when deploying devices for commissioning.
Status
IN DEVELOPMENT
Migrations
NO
Related PRs
Todos
Deploy notes
Steps to test or reproduce