[PATCH rtems-littlevgl] Allow to pass custom lv_conf.h and lv_drv_conf.h.

[Christian Mauderer]

On 09/07/2020 20:52, Vijay Kumar Banerjee wrote:
> 
> 
> On Fri, Jul 10, 2020, 12:11 AM Christian Mauderer <oss at c-mauderer.de
> <mailto:oss at c-mauderer.de>> wrote:
> 
>     Hello Vijay,
> 
>     thanks for the review and the test.
> 
>     On 09/07/2020 19:58, Vijay Kumar Banerjee wrote:
>     > Hi,
>     >
>     > Thanks for the patch, I tested the patch and it's building fine. I
>     > just had two questions which I have inlined below.
>     >
>     > On Thu, Jul 9, 2020 at 9:13 PM Christian Mauderer
>     > <christian.mauderer at embedded-brains.de
>     <mailto:christian.mauderer at embedded-brains.de>> wrote:
>     >>
>     >> ---
>     >>  lv_conf.h => default_lv_conf.h         |  0
>     >>  lv_drv_conf.h => default_lv_drv_conf.h |  0
>     >>  lvgl.py                                | 19 ++++++++++++++++---
>     >>  wscript                                | 13 +++++++++++++
>     >>  4 files changed, 29 insertions(+), 3 deletions(-)
>     >>  rename lv_conf.h => default_lv_conf.h (100%)
>     >>  rename lv_drv_conf.h => default_lv_drv_conf.h (100%)
>     >>
>     >> diff --git a/lv_conf.h b/default_lv_conf.h
>     >> similarity index 100%
>     >> rename from lv_conf.h
>     >> rename to default_lv_conf.h
>     >> diff --git a/lv_drv_conf.h b/default_lv_drv_conf.h
>     >> similarity index 100%
>     >> rename from lv_drv_conf.h
>     >> rename to default_lv_drv_conf.h
>     >> diff --git a/lvgl.py b/lvgl.py
>     >> index c154a5e..6d55db7 100644
>     >> --- a/lvgl.py
>     >> +++ b/lvgl.py
>     >> @@ -73,6 +73,17 @@ def build(bld):
>     >>      includes.append('.')
>     >>      include_paths = []
>     >>
>     >> +    def write_stuff(stuff):
>     >> +        def stuff_writer(task):
>     >> +            task.outputs[0].write(stuff)
>     >> +        return stuff_writer
>     >> +
>     >> +    lv_conf_h='lv_conf.h'
>     >> +    lv_drv_conf_h='lv_drv_conf.h'
>     >> +
>     >> +    bld(rule=write_stuff(bld.env.LV_CONF), target=lv_conf_h)
>     >> +    bld(rule=write_stuff(bld.env.LV_DRV_CONF), target=lv_drv_conf_h)
>     >> +
>     >>      for source in sources:
>     >>          source_dir = os.path.dirname(source)
>     >>          if source_dir not in include_paths:
>     >> @@ -80,7 +91,7 @@ def build(bld):
>     >>
>     >>      bld.stlib(target = 'lvgl',
>     >>                features = 'c',
>     >> -              cflags = ['-O2', '-g'],
>     >> +              cflags = ['-O2', '-g', '-DLV_CONF_INCLUDE_SIMPLE'],
>     >>                includes = includes,
>     >>                source = sources)
>     >>
>     >> @@ -96,6 +107,8 @@ def build(bld):
>     >>      for include_path in include_paths:
>     >>          files = os.listdir(include_path)
>     >>          include_headers = [os.path.join(include_path, x) for x
>     in files if (x[-2:] == '.h')]
>     >> -        bld.install_files(os.path.join("${PREFIX}/" ,
>     arch_inc_path, include_path),
>     >> +        bld.install_files(os.path.join("${PREFIX}",
>     arch_inc_path, include_path),
>     >>                            include_headers)
>     >> -    bld.install_files('${PREFIX}/' + arch_lib_path, ["liblvgl.a"])
>     >> +    bld.install_files(os.path.join('${PREFIX}', arch_lib_path),
>     ["liblvgl.a"])
>     >> +    bld.install_files(os.path.join('${PREFIX}', arch_inc_path,
>     include_path),
>     >> +        [lv_conf_h])
>     >
>     > Should lv_drv_conf_h be installed as well?
> 
>     I haven't seen that it is used but I can install it too. I didn't test
>     all drivers.
> 
> One use case would be to check if USE_BSD_FBDEV (Or some other driver)
> is defined. The example apps assumed that both evdev and fbdev is
> defined since the conf was fixed. I think they would need some check to
> ensure it is defined.

I think it is OK for the example apps to use the default configuration
used in RTEMS. But it's a good point. I'll add the lv_drv_conf.h to the
installed headers too before I push the commit.

> 
>     >
>     >> diff --git a/wscript b/wscript
>     >> index ae91daa..0e1a51d 100644
>     >> --- a/wscript
>     >> +++ b/wscript
>     >> @@ -49,9 +49,22 @@ def options(opt):
>     >>                     dest = "drivers",
>     >>                     help = "Build without lv_drivers." +
>     >>                            "Useful for building without libbsd.")
>     >> +    opt.add_option("--lv-conf",
>     >> +                   default = "default_lv_conf.h",
>     >> +                   dest = "lv_conf",
>     >> +                   help = "Use a custom lv_conf.h instead of the
>     default one.")
>     >> +    opt.add_option("--lv-drv-conf",
>     >> +                   default = "default_lv_drv_conf.h",
>     >> +                   dest = "lv_drv_conf",
>     >> +                   help = "Use a custom lv_drv_conf.h instead of
>     the default one.")
>     >>
>     >>  def configure(conf):
>     >>      conf.env.DRIVERS = conf.options.drivers
>     >> +    with open(conf.options.lv_conf, "rb") as lv_conf:
>     >> +        conf.env.LV_CONF = lv_conf.read()
>     >> +    with open(conf.options.lv_drv_conf, "rb") as lv_drv_conf:
>     >> +        conf.env.LV_DRV_CONF = lv_drv_conf.read()
>     >> +    conf.env.BUILDINCLUDE = 'build-include'
>     >
>     > It's not clear to me where is build-include being used. It builds fine
>     > without this line, should it be removed from the patch?
>     >
> 
>     That is a part of a dead end during writing the patch. I missed to
>     remove it. Thanks for finding it.
> 
>     Best regards
> 
>     Christian
> 
>     >
>     > Thanks for working on this feature, it was much needed.
>     >
>     > Best regards,
>     > Vijay
>     >
>     >>      rtems.configure(conf)
>     >>
>     >>  def build(bld):
>     >> --
>     >> 2.26.2
>     >>
>     > _______________________________________________
>     > devel mailing list
>     > devel at rtems.org <mailto:devel at rtems.org>
>     > http://lists.rtems.org/mailman/listinfo/devel
>     >
>