app: Refactor most modules to use SYS_INIT() #126
Conversation
SeppoTakalo
force-pushed
the
sys_init
branch
2 times, most recently
from
563e3f4 to
cbdaf6b
Compare
SeppoTakalo
force-pushed
the
sys_init
branch
2 times, most recently
from
4f648cb to
d8329ef
Compare
MarkusLassila
left a comment
MarkusLassila
left a comment
There was a problem hiding this comment.
Very nice changes!
| return 0; | ||
| } | ||
|
|
||
| int sm_at_mqtt_uninit(void) |
There was a problem hiding this comment.
We could actually do something useful and close the MQTT socket in here.
There was a problem hiding this comment.
I fail to see why it would be usefull to start network traffic when shutdown, or sleep, is requested.
On XSLEEP=2 all sockets remain active, on all other modes, we reset. So it is not usefull to trigger TCP close-down sequence, which causes network attaching and TX traffic.
Refactor modules to use SYS_INIT() instead of separate init functions that are called from sm_at_init(). When the only work is to initialize k_work handlers, use static macros for initializing. Remove all uninitialization functions which don't seem to do anything usefull that would be visible from outside of the device. The uninit is only used just before powering off, so there is no point of cleaning the memory. Signed-off-by: Seppo Takalo <[email protected]>
Instead of returning from main() and wasting the allocated stack space, run the Serial Modem's work queue from it. Previous main() is now ns_main() and runs from SYS_INIT() using order APPLICATION+100, so it should be last of the init functions. Other modules should use SYS_INIT(..,APPLICATION, 0). Signed-off-by: Seppo Takalo <[email protected]>
| SYS_INIT(sm_cmux_init, APPLICATION, 0); | ||
|
|
||
| void sm_cmux_uninit(void) | ||
| static void sm_cmux_uninit(void) |
There was a problem hiding this comment.
If this is only used from stop_work_fn, should we just copy this code into that function. The motivation I have is to avoid using old sm_cmux_uninit naming which gives certain impressions of its use from a centralized Serial Modem uninitialization.
| if (k_current_get() == &sm_work_q.thread && !urc) { | ||
| /* Send only from Serial Modem work queue to guarantee URC ordering. | ||
| * But only if the work queue is running. | ||
| * During statup, we need to use the system workqueue |
There was a problem hiding this comment.
| * During statup, we need to use the system workqueue | |
| * During startup, we need to use the system workqueue |
| LOG_MODULE_REGISTER(sm_at_host, CONFIG_SM_LOG_LEVEL); | ||
|
|
||
| #define SM_SYNC_STR "Ready\r\n" | ||
| #define SM_SYNC_ERR_STR "INIT ERROR\r\n" |
There was a problem hiding this comment.
This is not used anywhere anymore
| /**@brief API to initialize GNSS AT commands handler | ||
| */ | ||
| int sm_at_gnss_init(void) | ||
| static int sm_at_gnss_init(void) |
There was a problem hiding this comment.
Maybe you could fix error log on line 853 to:
"Could not set GNSS event handler"
| ret = start_execute(); | ||
| exit: | ||
| if (ret) { | ||
| LOG_ERR("Failed to start SM (%d). It's not operational!!!", ret); |
There was a problem hiding this comment.
This was an important log previously (added earlier this year) to make it very clear that user can expect SM not to work at all.
All module initialization failures was getting here. It's not that many cases in practice but some. Now the failures are unnoticed, or there is only an error log which gets a bit more hidden in the boot sequence and SM still starts.
We are also missing sending "INIT ERROR" over the uart in these cases which would make failure very clear.
These are sort of drawbacks of the SYS_INIT approach.
In the refactored form, we don't have many cases where this error log is relevant anymore. Maybe sm_uart_handler_enable and sm_at_send_str(SM_SYNC_STR) failures.
3 participants
Refactor modules to use SYS_INIT() instead of separate init functions that are called from sm_at_init().
Remove all uninitialization functions which don't seem to do anything useful that would be visible from outside of the device. The uninit is only used just before powering off, so there is no point of cleaning the memory.
Use main() for running the SM work queue
Instead of returning from main() and wasting the allocated stack space,
run the Serial Modem's work queue from it. This saves 4 kB of RAM.
Previous main() is now sm_main() and runs from SYS_INIT() using
order APPLICATION+100, so it should be last of the init functions.
Other modules should use SYS_INIT(..,APPLICATION, 0).