On 09/07/2020 20:52, Vijay Kumar Banerjee wrote: > > > On Fri, Jul 10, 2020, 12:11 AM Christian Mauderer <o...@c-mauderer.de > <mailto:o...@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.maude...@embedded-brains.de > <mailto:christian.maude...@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@rtems.org <mailto:devel@rtems.org> > > http://lists.rtems.org/mailman/listinfo/devel > > > _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel