Integration #1

giuuuug · 2024-12-05T19:47:03Z

No description provided.

…e code of the LCD with the previious one

.gitmodules

Core/Inc/main.h

Core/Src/main.c

Core/Src/dashboard.c

Lib/LVGL9/callback/callback.c

Lib/LVGL9/log/log.c

Lib/LVGL9/log/log.h

Lib/LVGL9/callback/callback.c

Lib/LVGL9/callback/callback.h

simoneruffini · 2024-12-09T14:25:36Z

I'm not convinced on how you are structuring the lvgl folder inside lib. Here my notes:

I understand the double nesting lib/LVGL9/lvgl because in this way you know where to put the lvgl_conf.h file. Good job! I don't really like the fact that you put the version (LVGL9) as the folder name because in the future if you migrate to a new version this folder will need to be renamed -> refactor the makefile, etc... (and probably no one will ever do this). So maybe you can think of a better name.
You are adding additional code to LVGL and you are using a pattern of creating new folders for each feature ( callback/ and log/). I suggest you avoiding this if there is no particular necessity. You created 4 new files that contain less then 200 lines of code if you sum them up. You could just add a single source pair in the same place where you have log_conf.h, e.g. lvgl_user_code.h/c, lvgl_utils.h/c, lvgl_extra.h/c, choose a name that fits. This is a personal preference because I don't like creating files in projects because as a new user it gets overwhelming and messes with your head. I like the approach of having less files with more stuff in it (e.g. bsp.c/h)

dashboard.ioc

.gitmodules

simoneruffini · 2024-12-20T14:08:04Z

Core/Inc/dashboard.h

@@ -1,22 +1,25 @@
 /*INCLUDES*/

+#include "bsp.h"
+#include "ili9488.h"


In future improvements of the code you should end up in a state where the file that contains the control code doesn't depend on level drivers, i.e. you will lose the #include "ili9488.h dependency in dashboard.c because all the code you will need is in either LVGL stuff or bsp.h.

You can close this comment, just keep in mind it.

simoneruffini · 2024-12-20T14:09:26Z

Core/Inc/iwdg.h

Enable the watchdog, otherwise you will forget it!

simoneruffini · 2024-12-20T14:10:54Z

Core/Inc/dashboard.h

+#define ON  1
 #define OFF 0


where do you use this defines?

simoneruffini · 2024-12-20T14:18:37Z

Core/Src/bsp.c

+enum LCD_TFT_Device{CS, RST, DC, LCD_TFT_Device_NUM};
+static const struct GPIO_Tuple LCD_TFT_Device_to_GPIO_Tuple_map[LCD_TFT_Device_NUM] = {
+    [CS]  = {.GPIO_Port = LCD_TFT_CS_GPIO_OUT_GPIO_Port, .GPIO_Pin = LCD_TFT_CS_GPIO_OUT_Pin},
+    [DC]  = {.GPIO_Port = LCD_TFT_DC_GPIO_OUT_GPIO_Port, .GPIO_Pin = LCD_TFT_DC_GPIO_OUT_Pin},
+    [RST] = {.GPIO_Port = LCD_TFT_RST_GPIO_OUT_GPIO_Port, .GPIO_Pin = LCD_TFT_RST_GPIO_OUT_Pin},
+};


This construct here is used improperly: the array-map construct is used to make dictionaries/maps of similar things (for example LED_MONO). These GPIOs are related to one another because they are necessary to drive the LCD_TFT display but they have different functions hence they are not a LCD_TFT device. In this case I suggest you to use a struct (a handle) to group everything since it betters conveys the fact that you are making a sort of "class" for the LCD_TFT. If you will end up using the code I pushed on ili9488 this is already available with the struct ILI9488_Handle.

Core/Src/dashboard.c

simoneruffini · 2024-12-20T14:32:19Z

EEZ/global_vars.c

prefix/namespace

simoneruffini · 2024-12-20T14:32:37Z

EEZ/global_vars.h

prefix/namespace missing

simoneruffini · 2024-12-20T14:34:41Z

EEZ/src/ui/screens.h

Prefix/namespace missing on exported functions (eg: GUI, UI, EEZ_UI ...)
IMHO create is a bit cringe as a name in a fuction

simoneruffini · 2024-12-20T14:36:33Z

Lib/LVGL/lvgl_callbacks.c

+             mem_mon.used_pct,
+             (unsigned long)mem_mon.free_biggest_size);
+
+    HAL_UART_Transmit(&LCD_TFT_USART_Handle, (uint8_t *)buffer, strlen(buffer), HAL_MAX_DELAY);


This should move to an interrupt based approach with DMA in the future otherwise when you do loggin the application will run veeeeery slowly.

simoneruffini · 2024-12-20T14:36:49Z

Lib/LVGL/lvgl_callbacks.h

+#define BYTES_PER_PIXEL (LV_COLOR_FORMAT_GET_SIZE(LV_COLOR_FORMAT_RGB888))
+#define BUFFER_SIZE     (VERTICAL_RES * HORIZONTAL_RES * BYTES_PER_PIXEL / 7)


Namespace/prefix

simoneruffini · 2024-12-20T14:46:54Z

.gitmodules

 [submodule "Lib/PCA9555"]
 	path = Lib/PCA9555
 	url = https://github.com/squadracorsepolito/PCA9555.git


remove this lines

giuuuug added 2 commits December 5, 2024 19:40

feat: Libraries and drivers are now included. Basic integration of th…

eb72731

…e code of the LCD with the previious one

chore: Implemented the SPI DMA function

a6206ba

simoneruffini assigned simoneruffini and giuuuug and unassigned simoneruffini Dec 5, 2024

feat: code cleanup

887f852

simoneruffini reviewed Dec 9, 2024

View reviewed changes

Lib/LVGL9/callback/callback.c Outdated Show resolved Hide resolved

simoneruffini reviewed Dec 9, 2024

View reviewed changes

Lib/LVGL9/callback/callback.c Outdated Show resolved Hide resolved

Lib/LVGL9/callback/callback.h Outdated Show resolved Hide resolved

Lib/LVGL9/callback/callback.h Outdated Show resolved Hide resolved

simoneruffini reviewed Dec 9, 2024

View reviewed changes

dashboard.ioc Outdated Show resolved Hide resolved

giuuuug added 2 commits December 16, 2024 15:47

feat: Added SC24_ILI9488_DRIVER as a submodule

1d7797c

feat: Removed old submodule Drivers/ILI9488_DRIVER

99f5009

simoneruffini reviewed Dec 16, 2024

View reviewed changes

.gitmodules Outdated Show resolved Hide resolved

giuuuug added 5 commits December 20, 2024 09:24

fix: Submodules elimated

43f046a

fix: submodules added

db0d5c9

fix: brought SCan to branch build

ae81e3c

fix: issue resolved

57f8730

feat: code abstraction improved

5ee0448

giuuuug force-pushed the integration branch from 5589adb to 5ee0448 Compare December 20, 2024 12:54

simoneruffini reviewed Dec 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integration #1

Integration #1

giuuuug commented Dec 5, 2024

simoneruffini commented Dec 9, 2024

simoneruffini Dec 20, 2024

simoneruffini Dec 20, 2024

simoneruffini Dec 20, 2024

simoneruffini Dec 20, 2024

simoneruffini Dec 20, 2024

simoneruffini Dec 20, 2024

simoneruffini Dec 20, 2024

simoneruffini Dec 20, 2024

simoneruffini Dec 20, 2024

simoneruffini Dec 20, 2024

		#define BYTES_PER_PIXEL (LV_COLOR_FORMAT_GET_SIZE(LV_COLOR_FORMAT_RGB888))
		#define BUFFER_SIZE (VERTICAL_RES * HORIZONTAL_RES * BYTES_PER_PIXEL / 7)

Integration #1

Are you sure you want to change the base?

Integration #1

Conversation

giuuuug commented Dec 5, 2024

simoneruffini commented Dec 9, 2024

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