[RFC] Template prototype using jinja2 templating engine

[evenl]

Sorry for this, my bad, I had to create a new pull request:
This is an continuation of #10873

Goal

To propose the jinja2 template engine as an alternative to the COG
based solution proposed for code generation.

Rationale

The version of COG proposed to be used in Zephyr is a highly modified
version of the original COG implementation. Which means that we can
not leverage future improvements made to COG or contribute our own
modifications back to COG.

This commit evenl@d8c986c
shows what code can be removed by using a jinja instead of a COG base solution

The Jinja2 engine however can be used as a pip package dependency
without modifications.

COG is more or less straight forward python which means that a generator
must more or less be written for each case. Which means that even the
simplest things will need a lot of boiler code just to output something.

Example:

Data in is “dev_name”

Cog:

/**
 * @code{.codegen}
 *
 *     codegen.outl(“#ifndef {}_INCLUDE_H”.format(dev_name))
 *     codegen.outl(“#define {}_INCLUDE_H”.format(dev_name))
 *
 * @endcode{.codegen}
 */

Jinja2:

    #ifndef {{ dev_name }}_INCLUDE_H
    #define {{ dev_name }}_INCLUDE_H

TODO:

The Jinja2 implementation is on par with what is currently implemented using the COG
solution. But because the COG solution give the user the ability to directly write
python code directly in the template there may be a need to develop some custom filters/
helper functions for the jinja2 solution.

This code is based on this code https://github.com/galak/zephyr/tree/EDTS which is not
yet in master.

Other advantages:

    * Big active community
    * Standardized and documented (http://jinja.pocoo.org)
    * Has been in development for a long time with multiple stable releases
    * Extensive enough to be a one-stop-shop for all code generation in the Zephyr project

[codecov-io]

Codecov Report

Merging #10957 into topic-EDTS will increase coverage by 0.01%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##           topic-EDTS   #10957      +/-   ##
==============================================
+ Coverage        53.3%   53.32%   +0.01%     
==============================================
  Files             215      215              
  Lines           25908    25908              
  Branches         5711     5711              
==============================================
+ Hits            13810    13815       +5     
+ Misses           9777     9772       -5     
  Partials         2321     2321

Impacted Files	Coverage Δ
subsys/logging/log_core.c	78.4% <0%> (+2.34%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ab7823...d4c8c5b. Read the comment docs.

[erwango]

Thanks for all the changes, starting to looks nice.
2 remaining points

[erwango]

One question on particular multi compatible support.

As you could see this file is compatible with 2 STM32 I2C IP versions. Particularly, both versions use the same code for devices.
Then, what if they used slightly different code for devices V1 and devices V2.
For instance: V1 and V2 uses same data_struct_body but have different config_struct_body

How would you support that if I still want to have device instantiation all in once place? (Assumption is only one compatible would be used for a specific board, either V1 or V2, but for all types of boards, I want to use this same file.)
This is doable with codegen since we provide 'compatible' in API, so we only have to define data_struct_body, config_struct_body_v1 and config_struct_body_v2, then calling API twice:
-first with devicedeclare('compat_v1', 'config_struct_body_v1', 'data_struct_body'),
-then with devicedeclare('compat_v2', 'config_struct_body_v2', 'data_struct_body')

[evenl]

This is doable with codegen since we provide 'compatible' in API, so we only have to define data_struct_body,
config_struct_body_v1 and config_struct_body_v2, then calling API twice:
-first with devicedeclare('compat_v1', 'config_struct_body_v1', 'data_struct_body'),
-then with devicedeclare('compat_v2', 'config_struct_body_v2', 'data_struct_body')

That is not entirely true, there is a lot more parameters.

devicedeclare.device_declare( \
 *     ['st,stm32-exti', ],
 *     'CONFIG_KERNEL_INIT_PRIORITY_DEVICE',
 *     'PRE_KERNEL_1',
 *     None,
 *     'stm32_exti_init',
 *     None,
 *     device_data,
 *     None)

So what I did in my jinja2 solution is to move the parameters out of the function call and into an object, I know it's personal taste, but I think it is cleaner. But your use-case is totally supported, it's just a matter of creating another parameter object, that points to the other config_struct_body for the second device, and then do two calls as in the codegen solution.

[erwango]

That is not entirely true, there is a lot more parameters.

Indeed, my goal was not to oversimplify codegen solution. I should have used following syntax:
devicedeclare('compat_v1', 'config_struct_body_v1', 'data_struct_body', ....)

But your use-case is totally supported

Ok, great removing my NACK. But tbh I'd prefer to have things available (even as a prototype) rather than doable and will be done if RFC is approved (even if I do trust you).

[superna9999]

I'm ok for the i2c and spi stm32 drivers changes

[galak]

@evenl I updated the topic-EDTS branch, can you rebase on it.

[SebastianBoe]

@galak : IMHO, syncing the prototype with EDTS should not be done until the RFC is approved.

If the RFC is declined there is no value in syncing it with the latest EDTS changes. If the RFC is
accepted we can sync with EDTS in the PR.

[dbkinder]

+1 for docs

[evenl]

I was wondering if there is any progress regarding this topic? I think all questions/change requests has been addressed.

[SebastianBoe]

@erwango : Hi, I see you are registered as a NACK. Can you re-asses your review?

[SebastianBoe]

Hi, @galak if you could approve this RFC we can consider the RFC to be approved. We will then remove the RFC tag and rebase it onto the topic-EDTS branch as a PR.

[SebastianBoe]

Hi, since there have been several approvals I am going to consider this RFC approved.

@evenl : You may now remove the RFC tag and begin preparing this so that it can be reviewed again as a PR and then merged.

[erwango]

@SebastianBoe

.. and then merged.

Is that decided we're using jinja2 instead of codegen or other ? Have I missed something?

[SebastianBoe]

Is that decided we're using jinja2 instead of codegen or other ? Have I missed something?

Yes, that is what this RFC is for and what you have approved.
The implementation is still up for review.

[erwango]

@SebastianBoe

Yes, that is what this RFC is for and what you have approved.

Is tit really ? @carlescufi are you aligned on this?
My understanding was not this one. I've approved because I considered this proposal was matching all my requirements. This was not about "yeah lets' go with jinja2". I have not seen any github API aims as selecting a proposal over a another one.
I think this is a decision that should be taken by another process (and I'm totally open to have that discussion by any other way).
Meanwhile, if this is the current understanding, let me reconsider my review.

I would have preferred we could have the following process:
Review both RFC equally to get the best and equivalent implementations of both RFC. Once both provide satisfactions regarding reviews, a public selection can happen, which should involve the most zephyr users.

[erwango]

Commenting to withdraw my approval, since approval equals "vote for" according to @SebastianBoe .

[erwango]

Trying to remove approval, or dismiss review..

[SebastianBoe]

The RFC is no longer considered to be approved due to a misunderstanding.

Further review will be required ...

[carlescufi]

Is it really ? @carlescufi are you aligned on this?

Unfortunately the final decision on what templating system to use has been deferred to after the LTS release in order to get everything ready for the upcoming release.
Let's wait until then in order to settle the discussion.