Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node: Add lv_font_conv to node package #1764

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

node: Add lv_font_conv to node package #1764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9fd014d
node: Add lv_font_conv to node package
FintasticMan May 19, 2023
de04b9f
workflows: Remove packages from npm command
FintasticMan Jun 21, 2023
1f9e873
docker: Install node modules locally
FintasticMan Jun 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ jobs:
run: |
sudo apt-get -y install ninja-build

- name: Install lv_font_conv
run:
npm i -g [email protected]

- name: Checkout source files
uses: actions/checkout@v3
with:
submodules: recursive

- name: Install npm packages
run:
npm i

- name: Get InfiniSim repo
run: |
git clone https://github.com/InfiniTimeOrg/InfiniSim.git --depth 1 --branch main
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,6 @@ src/arm-none-eabi

# clangd
.cache/

# npm
node_modules/
5 changes: 2 additions & 3 deletions doc/buildAndProgram.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@ To build this project, you'll need:
python -m pip install -r tools/mcuboot/requirements.txt
```
- A reasonably recent version of CMake (I use 3.16.5)
- lv_font_conv, to generate the font .c files
- see [lv_font_conv](https://github.com/lvgl/lv_font_conv#install-the-script)
- lv_font_conv and lv_img_conv, to generate the font .c files
- install npm (commonly done via the package manager, ensure node's version is at least 12)
- install lv_font_conv: `npm install lv_font_conv`
- install the modules: `npm install`

## Build steps

Expand Down
1 change: 0 additions & 1 deletion docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ RUN apt-get update -qq \
RUN pip3 install adafruit-nrfutil
RUN pip3 install -Iv cryptography==3.3
RUN pip3 install cbor
RUN npm i [email protected] -g

# build.sh knows how to compile
COPY build.sh /opt/
Expand Down
2 changes: 2 additions & 0 deletions docker/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ main() {
[ ! -d "$TOOLS_DIR/$NRF_SDK_VER" ] && GetNrfSdk
[ ! -d "$TOOLS_DIR/mcuboot" ] && GetMcuBoot

npm i
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why move the install from the docker file where it ran once to the build script where it has to run each build.

please try to move it back to the dockerfile

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To do that, it would need to clone the InfiniTime repo during the Docker image build. This doesn't really make sense, and I think that it doesn't make sense to need to rebuild the Docker image just to use updated npm packages. It also shouldn't affect the build time much, because if the packages are up to date it doesn't do anything.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it would require me to have a working internet connection for each build. Previously that was a requirement for just the docker image creation

I think you could use a COPY packages.json command and delete it after the npm command again

Good point. In this case, npm actually runs even if there's no internet connection as long as the packages are up-to-date with the package-lock.json.

You're right, I could just download the package.json and package-lock.json from the repo and use those in the Docker image itself. I personally think that installing the modules in the repo and running npm i for every build is a better solution.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also prefer to have all dependencies installed during the build of the Docker image rather than during the build of the project. This ensures that once the Docker image is generated and tested, it will be able to build the project no matter what.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After looking into it, npm doesn't support installing packages globally from a package.json. If we do want the Docker container to contain the npm packages, we'll have to install them like we do now, without a package.json.


mkdir -p "$BUILD_DIR"

CmakeGenerate
Expand Down
118 changes: 118 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"lv_font_conv": "^1.5.2"
}
}
3 changes: 2 additions & 1 deletion src/displayapp/fonts/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ set(FONTS jetbrains_mono_42 jetbrains_mono_76 jetbrains_mono_bold_20
jetbrains_mono_extrabold_compressed lv_font_sys_48
open_sans_light fontawesome_weathericons)
find_program(LV_FONT_CONV "lv_font_conv" NO_CACHE REQUIRED
HINTS "${CMAKE_SOURCE_DIR}/node_modules/.bin")
HINTS "${CMAKE_SOURCE_DIR}/node_modules/.bin"
HINTS "${InfiniTime_DIR}/node_modules/.bin")
message(STATUS "Using ${LV_FONT_CONV} to generate font files")
configure_file(${CMAKE_CURRENT_LIST_DIR}/jetbrains_mono_bold_20.c_zero.patch
${CMAKE_CURRENT_BINARY_DIR}/jetbrains_mono_bold_20.c_zero.patch COPYONLY)
Expand Down
3 changes: 2 additions & 1 deletion src/resources/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@

find_program(LV_FONT_CONV "lv_font_conv" NO_CACHE REQUIRED
HINTS "${CMAKE_SOURCE_DIR}/node_modules/.bin")
HINTS "${CMAKE_SOURCE_DIR}/node_modules/.bin"
HINTS "${InfiniTime_DIR}/node_modules/.bin")
message(STATUS "Using ${LV_FONT_CONV} to generate font files")

find_program(LV_IMG_CONV "lv_img_conv.py" NO_CACHE REQUIRED
Expand Down