nrf_rpc: Add function for single group initialization #1593
Conversation
Vge0rge
changed the title
Vge0rge
force-pushed
the
54h20_psa_rng
branch
from
a943a29 to
cd4ad91
Compare
Vge0rge
force-pushed
the
54h20_psa_rng
branch
3 times, most recently
from
1351882 to
7a4cfd2
Compare
Vge0rge
force-pushed
the
54h20_psa_rng
branch
2 times, most recently
from
6d9476d to
326b004
Compare
| return 0; | ||
| } | ||
|
|
||
| int nrf_rpc_init(nrf_rpc_err_handler_t err_handler) |
There was a problem hiding this comment.
The err_handler is never assigned to global handler.
There was a problem hiding this comment.
Good catch, I changed that.
nrf_rpc/nrf_rpc.c
Outdated
| static void default_err_handler(const struct nrf_rpc_err_report *report) | ||
| { | ||
| NRF_RPC_ERR("nRF RPC error %d ocurred. See nRF RPC logs for more details", report->code); | ||
| k_oops(); |
There was a problem hiding this comment.
This should be OS-independent code. k_oops() is a Zephyr function. Please pass it through OS porting layer.
There was a problem hiding this comment.
Regarding this, do you refer to this OS porting layer do you refer to the nrf_rpc one?
So what you are proposing is to add another function here:
https://github.com/nrfconnect/sdk-nrf/blob/main/subsys/nrf_rpc/nrf_rpc_os.c
Which handles the error case?
There was a problem hiding this comment.
Do you have an opinion on this Pawel? I can do what I am describing here if you have the same understanding as me
Vge0rge
force-pushed
the
54h20_psa_rng
branch
from
326b004 to
fa5ea93
Compare
pdunaj
dismissed
doki-nordic’s stale review
reviewer is away. nitpicks were addressed
nrf_rpc/nrf_rpc.c
Outdated
|
|
||
| NRF_RPC_ASSERT(transport != NULL); | ||
| if(group->data->transport_initialized) { |
| for (NRF_RPC_AUTO_ARR_FOR(iter, group, &nrf_rpc_groups_array, | ||
| const struct nrf_rpc_group)) { | ||
|
|
||
| transport_init_single(receive_cb, group); |
There was a problem hiding this comment.
This is intentional though, because I saw in the previous implementation they didn't return an error when a single initialization failed.
The logic here:
Line 420 in 4929937
Should be the same with the logic in the function transport_init_single basically.
The error that I return in this function is the logic which exists here:
Line 458 in 4929937
nrf_rpc/nrf_rpc.c
Outdated
| @@ -1144,20 +1162,67 @@ int nrf_rpc_init(nrf_rpc_err_handler_t err_handler) | |||
| return err; | |||
| } | |||
|
|
|||
| for (i = 0; i < CONFIG_NRF_RPC_CMD_CTX_POOL_SIZE; i++) { | |||
| for (int i = 0; i < CONFIG_NRF_RPC_CMD_CTX_POOL_SIZE; i++) { | |||
nrf_rpc/nrf_rpc.c
Outdated
| int nrf_rpc_init_group(const struct nrf_rpc_group *group) | ||
| { | ||
| int err = nrf_rpc_prepare_init(); | ||
| if(err < 0) { |
nrf_rpc/nrf_rpc.c
Outdated
| NRF_RPC_DBG("Initializing nRF RPC module"); | ||
|
|
||
| err = nrf_rpc_prepare_init(); | ||
| if(err < 0) { |
|
I assume that @doki-nordic have seen no problems with the actual change. |
|
Hi @Vge0rge , please just fix the error handling and I will approve and merge. |
Vge0rge
force-pushed
the
54h20_psa_rng
branch
2 times, most recently
from
fefd809 to
d078aad
Compare
Adds the function nrf_rpc_init_group to allow initializing an nrf_rpc group individually. This is useful when groups need to be initialized in different boot stages. nrf_rpc_init now takes into account that a group could could be initialized using the nrf_rpc_init_group and avoids initializiting again. Signed-off-by: Georgios Vasilakis <[email protected]>
Vge0rge
force-pushed
the
54h20_psa_rng
branch
from
d078aad to
2873f50
Compare
Adds the function nrf_rpc_init_group to allow initializing an nrf_rpc group individually. This is useful when groups need to be initialized in different boot stages.
nrf_rpc_init now takes into account that a group could could be initialized using the nrf_rpc_init_group and avoids initializiting again.