Update platform and libraries #22

cedel1 · 2023-03-21T19:59:06Z

update and fix platform version
fix Microchip library (wrong usage of Wire library)
fix minor typo

cedel1

Maybe you want to review by individual commits - they follow on another in logical steps. Sorry for the amount of comments, I tried to be as clear as possible.

cedel1 · 2023-03-23T16:47:16Z

platformio.ini

-    -DLOAD_GLCD
-    -DLOAD_FONT2
-    -DLOAD_FONT4
-    -DLOAD_FONT6
-    -DLOAD_FONT7
-    -DLOAD_FONT8
-    -DLOAD_GFXFF


Removing these unused fonts (either here in platformio.ini and/or in settings of the included TFT_eSPI library in User_settings.h) saves flash space - couple percent (depending on what stat you look at).

BTW (unrelated) the largest resources currently seem to be the included ChevyRegular fonts.

cedel1 · 2023-03-23T16:49:16Z

platformio.ini

@@ -12,13 +12,12 @@
 src_dir = smartpower3

 [env:esp32dev]
-;upload_port = /dev/ttyUSB0
-platform = espressif32
+platform = [email protected]


Specifying exact version prevents ambiguity when somebody else tries to build the project - why the project works for somebody and not for somebody else. Please see the comment related to beginTransmission.

cedel1 · 2023-03-23T16:49:54Z

smartpower3/screen.cpp

@@ -96,7 +96,7 @@ void Screen::run()
 			}
 		}
 #endif
-	};


A typo? Does not make sense to me if you look at the surrounded code.

cedel1 · 2023-03-23T17:00:10Z

libraries/PAC193x/Microchip_PAC193x.cpp

@@ -74,8 +77,6 @@ void Microchip_PAC193x::Read(uint8_t reg_address, int Nbytes, uint8_t *pBuffer)
 	if (errorCode != 0){
 		errorCode = (-1);
 	}
-
-	_wire->beginTransmission(I2C_ADDRESS); 


This solves the main "no power" issue - #10 .

According to docs/info resources, this is a mistake. beginTransmission method should only be used before writing to I2C (as opposed to reading). It worked with older Espressif32 platform, but after upgrade to newer version, Wire/I2C library got upgraded from v1 (which ignored the issue) to v2 which has tighter checks and it blocks here without compiler errors or printing any errors. Tricky...

See espressif/arduino-esp32#6674 .

That's why I fixed the platform version, so if anything similar should happen in the future, it should be obvious it is related to the change of version.

cedel1 · 2023-03-23T17:08:43Z

libraries/PAC193x/Microchip_PAC193x.cpp

@@ -213,6 +214,9 @@ int16_t Microchip_PAC193x::UpdateSampleRateLat(){
        case 3:
            sampleRateVal = 8;
            break;
+        // This should not happen but compiler requirements
+        default:
+            sampleRateVal = (SlowStatus) ? 8 : 1024;


Complier warns that sampleRateVal may be used uninitialized. According PAC193x docs, the switch cases should cover all the possible returned values, but how could the compiler know?

So I've solved it with this default that should work with any hardware revision, regardless of how the Slow/Alert pin of PAC193x is connected - just in case.

cedel1 · 2023-03-23T17:11:58Z

platformio.ini

@@ -22,3 +22,5 @@ monitor_speed = 115200
 upload_speed = 921600
 build_type = debug
 monitor_filters = esp32_exception_decoder
+lib_deps =
+    madhephaestus/ESP32Encoder@~0.10.1


This seems to be pretty straightforward update. I've also updated the relevant submodule to this version.

cedel1 · 2023-03-23T17:14:07Z

platformio.ini

@@ -24,3 +24,6 @@ build_type = debug
 monitor_filters = esp32_exception_decoder
 lib_deps =
    madhephaestus/ESP32Encoder@~0.10.1
+    ArduinoNVS@~2.8  ; local version, since Platformio library manager seems


See the comment in the code. There have been no updates of this library. Added here to prevent ambiguity.

cedel1 · 2023-03-23T17:15:38Z

platformio.ini

@@ -17,14 +17,42 @@ board = esp32dev
 framework = arduino
 lib_extra_dirs = libraries
 board_build.partitions = odroid.csv
-;build_flags =   -DSCREEN_CAPTURE


just moved to the end of new build_flags section.

cedel1 · 2023-03-23T17:51:04Z

platformio.ini

+;   [email protected]         ; local Hardkernel modified version of the library from https://github.com/leeseungcheol/TFT_eSPI
+    bodmer/[email protected]  ; upstream version of library that requires change to the main code
+                            ; because of the hardware - at lease display color inversion
+; If you decide to use upstream version of TFT_eSPI, following settings must be used,
+; otherwise settings have to be set/copied over in User_Setup.h file of the library each time it is updated
+build_flags =
+; TFT_eSPI library settings
+    -DUSER_SETUP_LOADED
+    -DILI9488_DRIVER
+    -DTFT_SDA_READ
+    -DTFT_MOSI=23
+    -DTFT_SCLK=18
+    -DTFT_CS=5
+    -DTFT_DC=19
+    -DTFT_RST=16
+    -DTFT_BL=17
+    -DLOAD_GLCD
+    -DLOAD_FONT2
+    -DLOAD_FONT4
+    -DLOAD_FONT6
+    -DLOAD_FONT7
+    -DLOAD_FONT8
+    -DLOAD_GFXFF
+    -DSMOOTH_FONT
+    -DSPI_FREQUENCY=55000000
+    -DSPI_READ_FREQUENCY=20000000
+    -DSPI_TOUCH_FREQUENCY=2500000
+    -DTOUCH_CS=-1  ; this disables compiler warning - pin should not be set


This is the most controversial change that requires double-checking, preferably by somebody with deeper knowledge of the SP3 hardware.

SP3 used own modified TFT_eSPI library from https://github.com/leeseungcheol/TFT_eSPI, which has not been touched in 2 years. It contains non-upstream changes to ILI9488 display controller initialization code and settings.

There are 2 notable changes from upstream that I could see:

Display inversion mode is turned on. This does not have to be done (i think) in the driver init code - can be safely turned on in the code by invertDisplay method. It does the same thing, only slightly later - writes the same value to the same display controller register.

SPI 3 wire communication mode is turned on and different read method is used based on TFT_SDA_READ setting. I didn't find (yet?) if it is turned on in the upstream library somewhere else. TFT_SDA_READ setting is present in the upstream library and there were changes in the upstream code since the Hardkernel version was forked - but the upstream version still works for me with the proper settings.

So what I've done is set the upstream version to be used in the project and added the invertDisplay call. This also allowed moving the library settings into platformio.ini - that has the advantage that the library can be updated rather freely (without having to modify User_settings.h each time a new library version is used and having own clone of the library).
As a backup, the original hardkernel library version is kept as a git submodule (didn't update that) and can be turned on in the settings (comment out the upstream version, remove comment from local version and remove displayInvert call in display init method).

My whole point is that upstream library version should be used if at all technically possible, especially when the library project is alive (which it is, in this case). That will ease you of the burden of having to maintain your own clone, incorporating upstream bug fixes, improvements etc.

If not possible for some technical reason, than I would prefer the changes to the library code to be made general (could be used, turned on/off by anybody) and pushed upstream - again for the same reasons of not having to maintain basically the same library. Having own clone is the last, least preferable possibility.

To make myself clear: I am not 100% sure if this concrete update to upstream version of this library does not misuse the hardware in any way. That's why I consider thorough review of this part necessary. It works for me, but your mileage may vary. So until reviewed, use at your own risk.

cedel1 · 2023-03-23T18:01:22Z

smartpower3/screen.cpp

@@ -7,6 +7,7 @@ uint8_t baud_idx;
 Screen::Screen()
 {
 	tft.init();
+	tft.invertDisplay(true);


see the other large comment - used with upstream library

cedel1 · 2023-03-23T18:07:29Z

README.md

+## Notes on building the firmware using Platformio
+
+### Build environment, libraries and their versions
+#### Platform
+Platform build environment pulls in some more general libraries, for example bus and filesystem libraries (Wire/I2C,
+SPI, SPIFFS etc.). As these may change between (major) versions, it is advisable to fix platform version
+in ```platformio.ini``` file.
+
+That is not to say that you shouldn't use the latest platform version - on the contrary. But 
+it gives you a known working starting point - if you can compile the original firmware code without any changes 
+to the codebase and version settings, flash it into the device and it works as expected, than any problem is **likely**
+related to any update or changes you may have made. Should you make a change and things went wrong, then you have
+a good idea what change exactly caused the problem and work from there.
+
+#### Libraries
+Some of the more specific hardware libraries required to build the project can be included into the firmware build 
+in two ways. The original way of using ```git submodule``` (using source and version fixed in git) or by linking 
+them via ```platformio.ini```.
+
+If you remove ```lib_deps``` information from ```platformio.ini```, the submodules at their checked out versions will be used. 
+Or you can use library manager in Platformio (for VSCode or other with similar functionality), download the same library 
+at different version and then see which one works best for your use-case. There are more options - see documentation 
+on setting up your project and configuration options (https://docs.platformio.org/en/latest/projectconf/index.html) and
+notes in ```platformio.ini``` provided with the project.
+
+Again, as is the case with platform settings, having a known version working configuration and updating from there can
+help pinpoint any problem you may encounter.


Updating the README could make changes easier for anybody trying to build the project, so they don't have to run into the same roadblocks as I did ;)

bablokb · 2023-03-23T18:25:36Z

This all sounds reasonable. Thanks for all of your work.

This was referenced Mar 21, 2023

add library and platform version constraints required for functional result #21

Closed

Problems compiling and no power #10

Closed

Update espressif32 platform and fix PAC193x library

454bbd3

cedel1 force-pushed the update_platform_and_libraries branch from ee3509d to 454bbd3 Compare March 23, 2023 01:29

cedel1 added 6 commits March 23, 2023 03:09

update ESP32Encoder lib to v 0.10.1

f464db1

Add ArduinoNVS lib_dep version and comment

f6a6fcf

TFT_eSPI - add Hardkernel modified version of library to lib_deps

e8a3544

Use upstream version of TFT_eSPI and related settings

0b609a6

Update project README

ce5db66

save some flash space - remove unused fonts from compilation

7929f8d

cedel1 commented Mar 23, 2023

View reviewed changes

cedel1 marked this pull request as ready for review March 23, 2023 18:09

cedel1 changed the title ~~WIP: Update platform and libraries~~ Update platform and libraries Mar 23, 2023

hardkernel merged commit 67dbd2b into hardkernel:master Mar 24, 2023

cedel1 deleted the update_platform_and_libraries branch March 24, 2023 10:22

cedel1 mentioned this pull request Mar 25, 2023

Channel data fetch freezes for 2^32 microseconds #17

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update platform and libraries #22

Update platform and libraries #22

Uh oh!

cedel1 commented Mar 21, 2023

Uh oh!

cedel1 left a comment •

edited

Loading

Uh oh!

cedel1 Mar 23, 2023

Uh oh!

cedel1 Mar 23, 2023

Uh oh!

cedel1 Mar 23, 2023

Uh oh!

cedel1 Mar 23, 2023

Uh oh!

cedel1 Mar 23, 2023

Uh oh!

cedel1 Mar 23, 2023

Uh oh!

cedel1 Mar 23, 2023

Uh oh!

cedel1 Mar 23, 2023

Uh oh!

cedel1 Mar 23, 2023 •

edited

Loading

Uh oh!

cedel1 Mar 23, 2023

Uh oh!

cedel1 Mar 23, 2023

Uh oh!

bablokb commented Mar 23, 2023

Uh oh!

Uh oh!

@@ @@ -96,7 +96,7 @@ void Screen::run() @@
               			}
               		}
               #endif
-              	};

Update platform and libraries #22

Update platform and libraries #22

Uh oh!

Conversation

cedel1 commented Mar 21, 2023

Uh oh!

cedel1 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cedel1 Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bablokb commented Mar 23, 2023

Uh oh!

Uh oh!

cedel1 left a comment •

edited

Loading

cedel1 Mar 23, 2023 •

edited

Loading