merge changes to support devices with no psram #20
Conversation
easily configurable custom boot animation via platform.ini boot flag. fix the webp_to_c script.
easily configurable custom boot animation via platform.ini boot flag. fix the webp_to_c script.
script delete sdkconfig use sdkconfig.defaults file
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @tavdog, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request addresses the need to build the firmware for hardware with different flash sizes and without PSRAM, specifically for the 'pixoticker' device. It introduces sdkconfig customization per PlatformIO environment (pioenv) to achieve this. The changes include adding a new board definition for 'wroom32' with a 4MB flash partition, a custom sdkconfig file for 'pixoticker', and modifications to the build process to select the appropriate configuration based on the environment.
Highlights
- sdkconfig Customization: The PR introduces sdkconfig customization per pio environment, allowing different hardware configurations to be built using the same codebase. This is achieved by adding a 'sdkconfig.pixoticker.defaults' file and referencing it in the 'platformio.ini' file.
- Board Definition for 'wroom32': A new board definition 'wroom32.json' is added, specifying the build parameters, connectivity options, debug tools, frameworks, and upload settings for the ESP32 Dev Module. It also includes a custom partition table 'max_app_4mb.csv'.
- Boot WebP Selection: The PR introduces a conditional compilation based on the 'PIXOTICKER' definition to select a smaller 'parrot' boot WebP image for devices with limited flash size.
- Display Pin Configuration: The display pin configuration is modified based on the 'PIXOTICKER' definition to match the hardware setup of the 'pixoticker' device.
- Pre-build Script Modification: The 'pre.py' script is modified to delete the existing 'sdkconfig' file to force regeneration with the correct configuration for each build environment.
Changelog
Click here to see the changelog
- .gitignore
- Added 'sdkconfig' to the list of ignored files.
- boards/max_app_4mb.csv
- Added a partition table configuration for a 4MB flash device.
- boards/wroom32.json
- Added a new board definition for 'wroom32' with specific build and upload settings.
- extra_scripts/pre.py
- Added code to delete the existing 'sdkconfig' file before each build to ensure regeneration with the correct configuration.
- extra_scripts/reset.py
- Added 'pixoticker' to the PRODUCTION_VERSION dictionary.
- extra_scripts/webp_to_c.h.py
- Added print statement for input file name
- Changed header name to ASSET_BOOT_WEBP
- lib/assets/assets.h
- Replaced 'ASSET_NOAPPS_WEBP' with 'ASSET_BOOT_WEBP'.
- Added conditional inclusion of 'parrot_c' or 'tronbyt_c' based on 'BOOT_WEBP_PARROT' definition.
- lib/assets/noapps_webp.c.orig
- Renamed 'ASSET_NOAPPS_WEBP' to 'ASSET_BOOT_WEBP'.
- lib/assets/parrot_c
- Added a new file containing the 'parrot' boot WebP image data.
- lib/assets/tronbyt_c
- Replaced 'ASSET_NOAPPS_WEBP' with 'ASSET_BOOT_WEBP'.
- platformio.ini
- Added a new environment 'pixoticker' with specific board, build flags, and library dependencies.
- Added cmake_extra_args to tidbyt and tidbyt-gen2
- Added board_build.flash_size to pixoticker
- sdkconfig.pixoticker.defaults
- Added a new file containing default sdkconfig settings for the 'pixoticker' device, including flash size and PSRAM configuration.
- src/display.cpp
- Added conditional pin definitions based on the 'PIXOTICKER' definition.
- src/gfx.c
- Replaced log tag with correct spelling
- Fixed dwell_us calculation
- Added dwell_ms calculation
- src/main.c
- Replaced 'ASSET_NOAPPS_WEBP' with 'ASSET_BOOT_WEBP' in gfx_initialize.
- Added log statement for flash init
- Added log statement for isAnimating value
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
The PixoTicker's plight,
A screen both small and bright,
Needs code that's lean,
And configs clean,
To make its future right.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request introduces sdkconfig customization per pio env for pixoticker hardware. The changes look good overall, but there are a few areas that could be improved for clarity and correctness. I am unable to approve this pull request, and users should have others review and approve this code before merging.
Summary of Findings
- Incorrect variable name in extra_scripts/webp_to_c.h.py: There's a typo in the print statement in
extra_scripts/webp_to_c.h.py. It saysingput file is, but it should beinput file is. - Inconsistent use of single frame delay in src/gfx.c: In
src/gfx.c, the code calculatesdwell_usin microseconds but then usesdwell_us/1000in milliseconds for the single frame delay. This inconsistency could lead to incorrect delay times. It's better to usedwell_msdirectly. - Missing error handling in gfx_initialize: In
src/gfx.c, the functiongfx_initializeallocates memory for_state->bufbut doesn't check ifwebpis NULL before copying data usingmemcpy. This could lead to a crash ifwebpis NULL.
Merge Readiness
The pull request introduces important functionality for building pixoticker hardware. However, there are a few issues that need to be addressed before merging, particularly the typo in the variable name in extra_scripts/webp_to_c.h.py and the inconsistent use of single frame delay in src/gfx.c. Addressing these issues will improve the code's correctness and maintainability. I am unable to approve this pull request, and users should have others review and approve this code before merging.
| @@ -56,13 +56,13 @@ int gfx_initialize(const void *webp, size_t len) { | |||
| } | |||
| memcpy(_state->buf, webp, len); | |||
There was a problem hiding this comment.
Consider adding a check to ensure webp is not NULL before calling memcpy to prevent potential crashes.
| memcpy(_state->buf, webp, len); | |
| if (webp != NULL) { | |
| memcpy(_state->buf, webp, len); | |
| } |
tavdog
changed the title
|
Tested on all three devices. |
1 participant
Add sdkconfig customization per pio env so that hardware with different flash sizes and no psram can also be built (pixoticker)