feat(console): Added runtime component registration support in console_simple_init #402
Conversation
espressif-abhikroy
requested review from
euripedesrocha,
gabsuren,
igrr and
david-cermak
espressif-abhikroy
force-pushed
the
components/console_simple_init_plugin
branch
2 times, most recently
from
4a1a9a3 to
8bccb89
Compare
|
Just one note, overall this component seems pretty generic and not specifically tied to esp-protocols. Perhaps we can move it to https://github.com/espressif/idf-extra-components in the future? Implementations of commands specific to protocols should stay here, of course. |
espressif-abhikroy
force-pushed
the
components/console_simple_init_plugin
branch
from
6dd55e9 to
3f60801
Compare
espressif-abhikroy
force-pushed
the
components/console_simple_init_plugin
branch
from
3f60801 to
057873c
Compare
| .plugin_regd_fn = &cmd_registration_function | ||
| };̌ | ||
| ``` | ||
| 2. Add the `WHOLE_ARCHIVE` flag to CMakeLists.txt of the component. |
There was a problem hiding this comment.
I think this potentially links more object files than necessary. We only need to ensure that the component registration callback is linked, but the component may also contain other object files. Following the "user shouldn't pay for what they don't use" principle I think we could try to have a more light-weight solution.
This is probably not a blocker for this PR specifically, we can always update the documentation later. But for other console components you create we might need to revise the WHOLE_ARCHIVE approach. (Also, not a blocker for releasing the first versions.)
|
Just one remark, LGTM otherwise! |
Added support for runtime component registration in console_simple_init.