Infineat external resources #1324

JF002 · 2022-09-11T14:24:51Z

This branch contains #1024 merged with develop. I've also added support for external resources.

@dmlls Would you like to have a look at those changes? If it's OK for you, I can add those commit to your PR and eventually merge it :)

# Conflicts: # src/displayapp/screens/Symbols.h # src/displayapp/screens/settings/SettingWatchFace.cpp # src/displayapp/screens/settings/SettingWatchFace.h

…d fonts from the ilesystem

dmlls · 2022-09-15T19:53:16Z

@JF002 looks good!

Maybe a stupid question, but should the Actions build run as is on the PineTime? I've tried it but it crashes. There's probably something obvious I'm missing :)

Also, just as a note, the case when the fonts are not loaded and falls back to the system fonts will cause misalignments. But this is probably fine, since at that point there is already something not working properly.

JF002 · 2022-09-17T17:16:52Z

@dmlls Thanks for the review!

Maybe a stupid question, but should the Actions build run as is on the PineTime? I've tried it but it crashes. There's probably something obvious I'm missing :)

I don't think you're missing anything obvious... I've just tried the DFU from the CI build (https://github.com/InfiniTimeOrg/InfiniTime/actions/runs/3032845417) and it works fine on my setup, even if the resources are not loaded yet. What happens when "it crashes" ? It boots and resets a few seconds later?

EDIT : note that both files from the CI build are intended to be run with the bootloader. Most of the companion apps support the DFU ZIP file, and Amazfish needs the MCUBoot image (Amazfish implemented the support for the zip file recently, so it should be available soon!). If you want to flash it via SWD, you need to flash the MCUBoot image at offset 0x8000 (and the bootloader at 0x00).

Also, just as a note, the case when the fonts are not loaded and falls back to the system fonts will cause misalignments. But this is probably fine, since at that point there is already something not working properly.

I implemented this fallback mode to ensure that the watch wouldn't crash if the resources are not installed when running this code. And I confirm, the alignment is not perfect with the system font. That's something we can improve if we want the fallback mode to look good, indeed!

dmlls · 2022-09-20T20:48:11Z

@JF002 not sure what I was doing wrong, but installing the DFU via Gadgetbdrige worked neatly ;) Thanks!

I implemented this fallback mode to ensure that the watch wouldn't crash if the resources are not installed when running this code.

Makes total sense!

And I confirm, the alignment is not perfect with the system font. That's something we can improve if we want the fallback mode to look good, indeed!

In the original PR I fixed the alignment (see 2ad51e6). Only the date and BLE icon needed to be adjusted, resulting in:

However this fix cannot be applied directly here as there were some small naming and layout changes. Nothing too complicated to implement, but this and next week will be super busy, so I don't want to block the PR.

What if we leave it like this for now and I'll come back to it later? :)

NeroBurner

not a complete review, I ran out of time

NeroBurner · 2022-09-20T21:14:14Z

src/displayapp/DisplayAppRecovery.cpp

@@ -2,6 +2,7 @@
 #include <FreeRTOS.h>
 #include <task.h>
 #include <libraries/log/nrf_log.h>
+#include <components/fs/FS.h>


including our headers should be done with "", excluding other libs (like lvgl, nrf, std-lib) should be done with <>

Suggested change

#include <components/fs/FS.h>

#include "components/fs/FS.h"

NeroBurner · 2022-09-20T21:17:45Z

src/displayapp/screens/CheckboxList.h

+#include "displayapp/Apps.h"
+#include "components/settings/Settings.h"
+
+#define MAXLISTITEMS 4


we could make this define into a constexpr size_t and move it from the global namespace into the private CheckboxList one

Done! However, since this constant is also used in the ctor(), I added it as a public field of the class.

NeroBurner · 2022-09-20T21:19:03Z

src/displayapp/screens/CheckboxList.h

+        uint8_t (Controllers::Settings::*GetOptionIndex)() const;
+        std::array<const char*, MAXLISTITEMS> options;
+
+        lv_obj_t* cbOption[MAXLISTITEMS];


we could use a std::array here as well

Suggested change

lv_obj_t* cbOption[MAXLISTITEMS];

std::array<lv_obj_t*, MAXLISTITEMS> cbOption;

NeroBurner · 2022-09-20T21:20:35Z

src/displayapp/screens/CheckboxList.h

+        lv_point_t pageIndicatorBasePoints[2];
+        lv_point_t pageIndicatorPoints[2];


although more to type I like std::array 😁

Suggested change

lv_point_t pageIndicatorBasePoints[2];

lv_point_t pageIndicatorPoints[2];

std::array<lv_point_t, 2> pageIndicatorBasePoints;

std::array<lv_point_t, 2> pageIndicatorPoints;

NeroBurner · 2022-09-20T21:22:16Z

src/displayapp/screens/Clock.h

@@ -42,12 +43,14 @@ namespace Pinetime {
        Controllers::Settings& settingsController;
        Controllers::HeartRateController& heartRateController;
        Controllers::MotionController& motionController;
+        Controllers::FS& fs;


optional suggestion: we could rename it to filesystem to be more explicit

see comment (far) above

NeroBurner · 2022-09-20T21:25:40Z

src/displayapp/DisplayAppRecovery.cpp

@@ -24,7 +25,8 @@ DisplayApp::DisplayApp(Drivers::St7789& lcd,
                       Pinetime::Controllers::TimerController& timerController,
                       Pinetime::Controllers::AlarmController& alarmController,
                       Pinetime::Controllers::BrightnessController& brightnessController,
-                       Pinetime::Controllers::TouchHandler& touchHandler)
+                       Pinetime::Controllers::TouchHandler& touchHandler,
+                       Pinetime::Controllers::FS& filesystem)


we're using fs for the filesystem controller everywhere else (I think). I like the explicitness of filesystem

would you suggest to use filesystem instead of fs in the future?

Yes, I agree with you, filesystem is more explicit!

NeroBurner · 2022-09-20T21:28:15Z

src/displayapp/screens/WatchFaceInfineat.h

@@ -0,0 +1,144 @@
+#pragma once
+
+#include <lvgl/src/lv_core/lv_obj.h>


I think we're using lvgl.h directly, instead of including from the internals of the lvgl library

Suggested change

#include <lvgl/src/lv_core/lv_obj.h>

#include <lvgl/lvgl.h>

…tem, use std::array instead of raw array,...

JF002 · 2022-09-27T16:10:12Z

@dmlls Thanks for your feedback! I'll (try to) merge those changes in your PR so that you get credits for this new watchface, and then merge it in develop, as I need those changes to merge the G7710 watchface!
We'll still be able to improve the fallback display when you (or someone else) have the time to work on it!

…ems).

…finiTime#1024) DisplayApp needs the FileSystem as parameter to the constructor.

JF002 · 2022-09-27T19:28:39Z

Merged in #1024

dmlls added 30 commits June 11, 2022 16:56

Implement watchface Infineat

ae705f2

Add Infineat watchface to settings

17d37d5

Add 'circle' symbol & Pine64 logo

a89fca7

Add Infineat settings to settings controller

c2ecd4c

Add 'Bebas Neue' and 'Teko' fonts

da2cf8e

Add lowercase days of the week

b5fbcf0

Fix typo

8f54332

Update BLE controller in Infineat watchface

be16ef0

Build on push to 'colors-rebase' branch

03790d4

Fixes in side cover toggling

a890f85

Add whitespace char to Teko font

f1f7fb7

Fix aligment of time labels

6eb5b99

Change Pine64 logo to grayscale

fcdf6de

Small color adjustments

f9d2b78

Implement CheckboxList screen

b561e7f

Implement SettingWatchFace list

a3bfbb5

All ready for PR

8942791

Bump settings version (#1)

b15f90a

Make function DayOfWeekShortToStringLow const

f5e9055

Make Pine64 logo background transparent

dd0d2a4

Fix bleIcon in Infineat watchface after upstream rebase

a4c3558

Use existing fonts in Infineat watchface

f6f6f95

Make Pine64 logo darker

9122547

Implement battery indicator

b5bf6c5

Implement charging battery animation

cb2131e

Better alignment of the date label

603af7c

Show battery indicator when side cover hidden

20b31fd

Simplify code

4e9fe3f

Fix date alignment

2ad51e6

Make notification color white when side cover off

18cff28

JF002 added 5 commits September 11, 2022 14:59

Merge branch 'develop' into infineat-external-resources

ada2c09

# Conflicts: # src/displayapp/screens/Symbols.h # src/displayapp/screens/settings/SettingWatchFace.cpp # src/displayapp/screens/settings/SettingWatchFace.h

Infineat : Add support for external resources, and read the images an…

4cf73fc

…d fonts from the ilesystem

Fix recovery firmware : add FS in parameters of DisplayAppRecovery ctor.

0cd1d6f

Fix submodule for date library

970d5b3

Fix formatting

58bb0e7

NeroBurner requested changes Sep 20, 2022

View reviewed changes

A few minors changes following the code review : rename fs -> filesys…

56f315b

…tem, use std::array instead of raw array,...

CheckBoxList : remove unused constant MAXLISTITEMS (replaced by MaxIt…

2400110

…ems).

JF002 added a commit to InfiniTimeOrg/InfiniSim that referenced this pull request Sep 27, 2022

Since #1324 (InfiniTimeOrg/InfiniTime#1324) / #1024 (InfiniTimeOrg/In…

4e60d77

…finiTime#1024) DisplayApp needs the FileSystem as parameter to the constructor.

Fix formatting in CheckBoxList.cpp.

58586d0

JF002 mentioned this pull request Sep 27, 2022

DisplayApp now takes the filesystem as ctor arguments InfiniTimeOrg/InfiniSim#69

Merged

JF002 merged commit 6393281 into develop Sep 27, 2022

NeroBurner deleted the infineat-external-resources branch September 27, 2022 21:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Infineat external resources #1324

Infineat external resources #1324

JF002 commented Sep 11, 2022

dmlls commented Sep 15, 2022

JF002 commented Sep 17, 2022 •

edited

Loading

dmlls commented Sep 20, 2022

NeroBurner left a comment

NeroBurner Sep 20, 2022

JF002 Sep 27, 2022

NeroBurner Sep 20, 2022

JF002 Sep 27, 2022

NeroBurner Sep 20, 2022

JF002 Sep 27, 2022

NeroBurner Sep 20, 2022

JF002 Sep 27, 2022

NeroBurner Sep 20, 2022

JF002 Sep 27, 2022

NeroBurner Sep 20, 2022

JF002 Sep 27, 2022

NeroBurner Sep 20, 2022

JF002 Sep 27, 2022

JF002 commented Sep 27, 2022

JF002 commented Sep 27, 2022

	lv_obj_t* cbOption[MAXLISTITEMS];
	std::array<lv_obj_t*, MAXLISTITEMS> cbOption;

		lv_point_t pageIndicatorBasePoints[2];
		lv_point_t pageIndicatorPoints[2];

		@@ -0,0 +1,144 @@
		#pragma once

		#include <lvgl/src/lv_core/lv_obj.h>

Infineat external resources #1324

Infineat external resources #1324

Conversation

JF002 commented Sep 11, 2022

dmlls commented Sep 15, 2022

JF002 commented Sep 17, 2022 • edited Loading

dmlls commented Sep 20, 2022

NeroBurner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JF002 commented Sep 27, 2022

JF002 commented Sep 27, 2022

JF002 commented Sep 17, 2022 •

edited

Loading