OpenThread Kconfigs should more closely follow Zephyr Kconfig recommendations

[e-rk]

Is your enhancement proposal related to a problem? Please describe. The OpenThread Kconfigs make frequent use of select statement, which makes it more difficult to track enabled functionalities. Some default values, like OPENTHREAD_SHELL=y should also be reconsidered.

Describe the solution you'd like The Kconfigs should be refactored according to the guide: https://docs.zephyrproject.org/latest/guides/kconfig/tips.html

Describe alternatives you've considered None

Additional context None

[rlubos]

CC @MarekPorwisz

Do you have any specific cases of select overuse to discuss or is it a general complaint? To my knowledge, OT subsys will only select those OS features which are essential for OT to run/build (for instance first on the list SETTINGS module - OT won't run w/o it). The same goes with selects on mbedTLS configuration - the core mbedTLS functionality needed for OT to run is enforced.

An alternative is to make each OT sample to enable all of the required funcionalities manually in the prj.conf - it was done like this before, but was reworked to use select since the project files became too bulky (and this approach is error-prone to be honest). There's also an option to use imply, but there's little benefit in this case IMO, as we still get the feature enabled automatically, but there's a risk of disabling the "core" functionality needed by OpenThread.

[e-rk]

I understand the reasoning behind using select in OT Kconfigs. I actually made this issue because I was surprised by the fact that the shell was enabled in my example despite me never stating any will to enable shell in the project config. The reason is that the OPENTHREAD_SHELL uses select SHELL and defaults to y with almost no dependencies. OPENTHREAD_SHELL becomes in this case the only shell config that I have to specifically disable in my application. This wouldn't have happened, if depends on SHELL was used instead. I have nothing against OPENTHREAD_SHELL being default y if SHELL, but it could be argued that it is not necessarily a good choice.

I think that Kconfigs should follow the principle of least surprise. While it is good to have short configuration for samples, I believe that for actual product development it would be preferable to set most configs by hand to have the knowledge of what actually goes into the actual application. That way many eyes involved in development could inspect the project configuration and notice that something unneeded is making its way into the product.

I thought that this approach could be applied to other OT configs in more general sense, since it was described in the page I linked in the OP.

I wonder what would be a systemic approach to solve this conflict of interests I see between samples and actual products. Maybe there could be a config called for example SAMPLE that would be set in all samples. Then there would be helper Kconfigs that select core functionalities needed by some modules, default to y, and depend on SAMPLE among other things. That way the ease of maintaining samples could be preserved.

[jukkar]

For the OT shell, it might be good idea to use imply for "selecting" it.

[rlubos]

Well if it's a matter of OPENTHREAD_SHELL being enabled by default I think we can consider disabling it (@MarekPorwisz what do you think?). Are there any other OT features enabled by default that you have an issue with?

[MarekPorwisz]

I don't mind disabling it by default. but we'll need to remember to update thread samples. However I'm not sure 100% that is a good idea. I think Thread should be easy to use for the beginners so we propose a set of initial default values that allow quick start with thread and when people became more Zephyr/Thread aware they can tune up the setting to their need. It is quite easy to find what features are selected and disable unneeded ones. I went over all three Thread Kconfig files and it looks for me that SHELL is the only one that we could change. There are also following settings imply NET_UDP imply NET_IPV6 imply NET_CONFIG_NEED_IPV6

but those are needed by almost all use-cases. The only situations when we may not need to use them are:

• using native thread api - this is not recommended
• NCP and RCP
• Thread router that does only routing i.e. application running on it does not make any use of thread network - I don't think someone will ever make such thing (maybe thread range extender?).

[e-rk]

@rlubos OPENTHREAD_SHELL is the only option I have such problem with right now.

[rlubos]

Feel free to send a PR then, just please remember to enable it back in the samples (look for overlay-ot.conf).

[nixward]

@MarekPorwisz

"I wonder what would be a systemic approach to solve this conflict of interests I see between samples and actual products."

Note that a summary of Kconfig build options is output in the build folder in this file: zephyr/.config

I've found this very useful to review what is going into a project's build.

[rlubos]

@e-rk Is there anything else to do in the scope of this issue? If not perhaps we could close it?

[e-rk]

Yes, I think it can be closed.