| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
@Riksu9000 i'd like to get your thoughts about whats already here. thanks! |
Sorry, something went wrong.
There was a problem hiding this comment.
I like this solution. The README should be updated as a part of this PR by removing the old instructions and writing something about this new method.
Sorry, something went wrong.
Also, removed feature existance cheking (since it now depends on a font, so may end up being inside (only) a font not being used currently - which is an allowed usage)
|
and now the font .c files are generated at compile time with CMake. |
Sorry, something went wrong.
|
I've been thinking that the font "features" probably isn't going to be very useful to us, since we only ship a single firmware, and that's likely not going to be changing. The other use case would be if fork maintainers pushed their changes in this repo, which I don't think should be our responsibility to handle either. |
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you for this PR, that's definitely something I want to add tot he project!
The generation of the fonts in CMake and then in Python and in Github Actions look good to me!
If you don't mind, I think we should make 2 addition:
I can help with those points if you want to ;)
Sorry, something went wrong.
There was a problem hiding this comment.
I noticed that changing the font configuration will regenerate all the fonts, even if only one was changed. Not a big issue, but maybe worth considering if something can be done about that.
Sorry, something went wrong.
Added a bit of changes here (use CMAKE_CURRENT_LIST_DIR to reach things, instead of assuming paths), |
Sorry, something went wrong.
Also, I did (except when I forgot once) compile before pushing, and here watching the checks status, so it's weird that the simulator check succeeded. so I had a look, and in the simulator workflow here I manually generated the scripts - I just removed it, so it will fail here too (until InfiniTimeOrg/InfiniSim#28 is merged). |
Sorry, something went wrong.
There was a problem hiding this comment.
This looks totally fine to me!
There's just 1 last issue : the build in the docker container does not work properly. See this log message:
[ 35%] Generating displayapp/fonts/jetbrains_mono_42.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_76.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_76.c
[ 35%] Generating displayapp/fonts/lv_font_navi_80.c
[ 35%] Generating displayapp/fonts/open_sans_light.c
[ 35%] Generating displayapp/fonts/open_sans_light.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_extrabold_compressed.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_extrabold_compressed.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_bold_20.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_42.c
[ 35%] Generating displayapp/fonts/lv_font_sys_48.c
[ 36%] Generating displayapp/fonts/jetbrains_mono_bold_20.c
[ 36%] Generating displayapp/fonts/lv_font_navi_80.c
File " File "/sources/src/displayapp/fonts/generate.py/sources/src/displayapp/fonts/generate.py", line ", line 21
21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
File "/sources/src/displayapp/fonts/generate.py File "/sources/src/displayapp/fonts/generate.py", line 21
File " def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
/sources/src/displayapp/fonts/generate.py def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
File " File "/sources/src/displayapp/fonts/generate.py", line /sources/src/displayapp/fonts/generate.py21", line
21 def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
File "/sources/src/displayapp/fonts/generate.py", line ", line 21
21 def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
^
SyntaxError : invalid syntax
^
SyntaxError ^
SyntaxError ^
File "SyntaxError : invalid syntax/sources/src/displayapp/fonts/generate.py
", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
: def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
invalid syntax
: invalid syntax
^
^
SyntaxError^
: SyntaxErrorinvalid syntaxSyntaxError:
: File "invalid syntaxinvalid syntax
/sources/src/displayapp/fonts/generate.py
", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
File " /sources/src/displayapp/fonts/generate.py ", line 21
^
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
^
SyntaxError: invalid syntax
File "/sources/src/displayapp/fonts/generate.py", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
^
SyntaxError: invalid syntax
^
SyntaxError: invalid syntax
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:65: recipe for target 'src/displayapp/fonts/jetbrains_mono_extrabold_compressed.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_extrabold_compressed.c] Error 1
make[2]: *** Waiting for unfinished jobs....
src/CMakeFiles/pinetime-app.dir/build.make:73: recipe for target 'src/displayapp/fonts/jetbrains_mono_76.c' failed
SyntaxErrormake[2]: *** [src/displayapp/fonts/jetbrains_mono_76.c] Error 1
: File " [ 36%] Generating displayapp/fonts/lv_font_sys_48.c
invalid syntaxmake[2]: *** Waiting for unfinished jobs....
/sources/src/displayapp/fonts/generate.py
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:73: recipe for target 'src/displayapp/fonts/jetbrains_mono_76.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_76.c] Error 1
", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
src/CMakeFiles/pinetime-app.dir/build.make:77: recipe for target 'src/displayapp/fonts/jetbrains_mono_42.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_42.c] Error 1
^
SyntaxError : invalid syntax
^
SyntaxError: invalid syntax
src/CMakeFiles/pinetime-app.dir/build.make:65: recipe for target 'src/displayapp/fonts/jetbrains_mono_extrabold_compressed.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_extrabold_compressed.c] Error 1
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:77: recipe for target 'src/displayapp/fonts/jetbrains_mono_42.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_42.c] Error 1
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:61: recipe for target 'src/displayapp/fonts/lv_font_navi_80.c' failed
make[2]: *** [src/displayapp/fonts/lv_font_navi_80.c] Error 1
src/CMakeFiles/pinetime-app.dir/build.make:69: recipe for target 'src/displayapp/fonts/jetbrains_mono_bold_20.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_bold_20.c] Error 1
src/CMakeFiles/pinetime-app.dir/build.make:85: recipe for target 'src/displayapp/fonts/open_sans_light.c' failed
make[2]: *** [src/displayapp/fonts/open_sans_light.c] Error 1
src/CMakeFiles/pinetime-app.dir/build.make:81: recipe for target 'src/displayapp/fonts/lv_font_sys_48.c' failed
make[2]: *** [src/displayapp/fonts/lv_font_sys_48.c] Error 1
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:85: recipe for target 'src/displayapp/fonts/open_sans_light.c' failed
make[2]: *** [src/displayapp/fonts/open_sans_light.c] Error 1
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:69: recipe for target 'src/displayapp/fonts/jetbrains_mono_bold_20.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_bold_20.c] Error 1
src/CMakeFiles/pinetime-app.dir/build.make:61: recipe for target 'src/displayapp/fonts/lv_font_navi_80.c' failed
make[2]: *** [src/displayapp/fonts/lv_font_navi_80.c] Error 1
CMakeFiles/Makefile2:409: recipe for target 'src/CMakeFiles/pinetime-app.dir/all' failed
make[1]: *** [src/CMakeFiles/pinetime-app.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
File "/sources/src/displayapp/fonts/generate.py", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
^
SyntaxError: invalid syntax
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:81: recipe for target 'src/displayapp/fonts/lv_font_sys_48.c' failed
make[2]: *** [src/displayapp/fonts/lv_font_sys_48.c] Error 1
CMakeFiles/Makefile2:214: recipe for target 'src/CMakeFiles/pinetime-mcuboot-app.dir/all' failed
make[1]: *** [src/CMakeFiles/pinetime-mcuboot-app.dir/all] Error 2
I think this is caused by the version of Python : the docker image uses Python 2.7.17, while my computer is running Python 3.10.
Do you think it would cause any issue if we called python3 instead of python in the cmake command that generates the font?
Sorry, something went wrong.
|
@yehoshuapw I've just tried the new docker image based on ubunutu 20.04, and I still got the same issues with the Python script. infinitime@cacd01d36e16:/$ lsb_release -a No LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.4 LTS Release: 20.04 Codename: focal infinitime@cacd01d36e16:/$ python --version Python 2.7.18 infinitime@cacd01d36e16:/$ python3 --version Python 3.8.10 I'm not sure why it works for you and not for me... Did I miss anything? PS : Please excuse me for the time it takes to finish this review... Those little details about Docker and the CI are taking more time than I expected... |
Sorry, something went wrong.
|
Ubuntu 22.04 is released and it has a package called python-is-python3 which symlinks python to python3. Would this be acceptable? |
Sorry, something went wrong.
Yes, probably! As long as it still builds the firmware and generates the DFU images, that fine for me :) EDIT : this seems to work: diff --git a/docker/Dockerfile b/docker/Dockerfile
index 1943177a..e2867c51 100644
--- a/docker/Dockerfile
+++ b/docker/Dockerfile
@@ -1,4 +1,4 @@
-FROM ubuntu:20.04
+FROM ubuntu:22.04
ARG DEBIAN_FRONTEND=noninteractive
RUN apt-get update -qq \
@@ -15,11 +15,11 @@ RUN apt-get update -qq \
unzip \
wget \
curl \
-# aarch64 packages
+ python-is-python3 \
+ # aarch64 packages
libffi-dev \
libssl-dev \
python3-dev \
- python \
git \
apt-utils \
&& curl -sL https://deb.nodesource.com/setup_18.x | bash - \
@@ -30,6 +30,7 @@ RUN apt-get update -qq \
RUN pip3 install adafruit-nrfutil
RUN pip3 install -Iv cryptography==3.3
+RUN pip3 install cbor
RUN npm i lv_font_conv@1.5.2 -g
# build.sh knows how to compile
|
Sorry, something went wrong.
I must have made some mistake (left the python2 version? left .c files in the build dir?), because indeed the image I built here does have the "python" version as 2.7. updated as you suggested, and seems to work. (building the image and then infinitime, from the start) (I'm also wondering if any dist except for arch (which I don't even use) doesn't have a "python3" anymore without installing some extra things. nonetheless having a newer version which works tends to be worth it) |
Sorry, something went wrong.
On Manjaro, I still have python and python3, but since python 3 is support to replace python 2, it's probably better to assume that python is Python 3
Thanks, that looks very good to me :) |
Sorry, something went wrong.
|
Now... How do we handle that in InfiniSim ? |
Sorry, something went wrong.
I opened a PR there too, InfiniTimeOrg/InfiniSim#28 which seems to work. |
Sorry, something went wrong.
|
I think nearly identical to here. Need to install npm and the lv-font generator in the CI. And need to add those generated fonts to InfiniSim. I'll need to have a thorough look at this PR and see how and where to fonts.c files are generated. Maybe I can make the font-generating CMakeLists.txt file generic enough so I just need to do add_subdirectory in InfiniSim and link to a infinisim-fonts target or something like that 🤔 |
Sorry, something went wrong.
in build/displayapp/fonts (or wherever the build dir is)
I'm new at CMake, so go for it - and if you want to make any edits here, you are welcome to. |
Sorry, something went wrong.
|
I'll merge this PR as it's working as expected in InfiniTime. We'll follow up with the integration in InfiniSim in anothe PR (probably in the InfiniSim project).
It should be possible to create a 'font' target or a GenerateFont() function that we'll be able to include in InfiniSim, so we don't have to duplicate the whole code needed to generate the fonts. @yehoshuapw I'm happy to see you've already started work in InfiniSim too! Thanks again for this nice PR! 🥇 |
Sorry, something went wrong.
In InfiniTimeOrg#1097 new font generation capabilites were added. Generalize the font creation to make it possible to reuse the `displayapp/fonts/CMakeLists.txt` file for `InfiniSim` and just add the new cmake file to the project and link against the new `infinitime_fonts` target. In the following a list of changes. Allow non-global installed `lv_font_conv` executable installed with ```sh npm install lv_font_conv@1.5.2 ``` In CMake we search for `lv_font_conv` executable. Add the found executable to the python script `generate.py`, to remove the need for `lv_font_conv` to be in the path. Search for `python3` executable, if CMake version 3.12 is available. Otherwise use `python` as hard coded executable. Instead of adding the generated fonts to `SOURCE_FILES` variable, create a static library `infinitime_fonts` instead. Link this library to the executables instead.
Since InfiniTimeOrg/InfiniTime#1097 the fonts are generated. Support this new way of using fonts.
In InfiniTimeOrg#1097 new font generation capabilites were added. Generalize the font creation to make it possible to reuse the `displayapp/fonts/CMakeLists.txt` file for `InfiniSim` and just add the new cmake file to the project and link against the new `infinitime_fonts` target. In the following a list of changes. Allow non-global installed `lv_font_conv` executable installed with ```sh npm install lv_font_conv@1.5.2 ``` In CMake we search for `lv_font_conv` executable. Add the found executable to the python script `generate.py`, to remove the need for `lv_font_conv` to be in the path. Search for `python3` executable, if CMake version 3.12 is available. Otherwise use `python` as hard coded executable. Instead of adding the generated fonts to `SOURCE_FILES` variable, create a static library `infinitime_fonts`. Link this library to the executables instead. Use `add_custom_target()` together with `add_custom_command()` to generate the font.c files once (like the original PR does).
Since InfiniTimeOrg/InfiniTime#1097 the fonts are generated. Support this new way of using fonts. When InfiniTime isn't a subdirectory of InfiniSim we need to provide the binary dir when using `add_subdirectory()`. Use `fonts`. Update README with instructions to install `lv_font_conv`.
Since InfiniTimeOrg/InfiniTime#1097 the fonts are generated. Support this new way of using fonts. When InfiniTime isn't a subdirectory of InfiniSim we need to provide the binary dir when using `add_subdirectory()`. Use `fonts`. Update README with instructions to install `lv_font_conv`.
In #1097 new font generation capabilites were added. Generalize the font creation to make it possible to reuse the `displayapp/fonts/CMakeLists.txt` file for `InfiniSim` and just add the new cmake file to the project and link against the new `infinitime_fonts` target. In the following a list of changes. Allow non-global installed `lv_font_conv` executable installed with ```sh npm install lv_font_conv@1.5.2 ``` In CMake we search for `lv_font_conv` executable. Add the found executable to the python script `generate.py`, to remove the need for `lv_font_conv` to be in the path. Search for `python3` executable, if CMake version 3.12 is available. Otherwise use `python` as hard coded executable. Instead of adding the generated fonts to `SOURCE_FILES` variable, create a static library `infinitime_fonts`. Link this library to the executables instead. Use `add_custom_target()` together with `add_custom_command()` to generate the font.c files once (like the original PR does).
| Back | FazBrowse Home | New Git URL |
closes #964 (supersedes)
this adds a script src/displayapp/fonts/generate.py to generate the font files (using lv_font_conv) from a singular config file (src/displayapp/fonts/fonts.json).
the json has each font, and it's relevant options (size,bpp,compress, and "patches") and sources.
in addition there's also a "feature" feature, allowing extra sources with a extra flag (and the config has an example usage of adding hebrew, which won't do anything unless -e hebrew is given)
currently, this manages to generate exactly the same as what is already in place.
TODO:
integrate this into the CMake (and add lv_font_conv to the build images).
(my CMake-fu is not that strong, so I'll get to that tomorrow - but opening this to get comments about the rest of it from anyone interested)