-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add library installation before sketch builds in CI scripts #6
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.
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds library installation functionality to CI scripts before building Arduino sketches, introduces a new board configuration for the Twin AIoT Module, and fixes documentation formatting issues.
- Adds automated library installation from ci.json files before sketch compilation
- Introduces configuration for a new ESP32-S3 based board variant called "twinaiot"
- Corrects RST formatting issues in documentation
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
variants/twinaiot/pins_arduino.h | Defines pin mappings and hardware constants for the new Twin AIoT Module board |
docs/en/guides/core_compatibility.rst | Fixes RST formatting by replacing backticks with proper double backticks and correcting documentation URLs |
boards.txt | Adds complete board configuration for the Twin AIoT Module with ESP32-S3 target and various menu options |
.github/scripts/tests_build.sh | Integrates library installation call before individual sketch builds |
.github/scripts/sketch_utils.sh | Implements comprehensive install_libs function with support for both named libraries and GitHub URLs |
.github/scripts/on-push.sh | Adds library installation before sketch building in the push workflow |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
variants/twinaiot/pins_arduino.h
Outdated
static const uint8_t SDA = 39; | ||
static const uint8_t SCL = 40; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I2C pins SDA and SCL are assigned the same values as UART TX and RX pins (39 and 40). This creates a pin conflict where the same GPIO pins are defined for different protocols, which will cause hardware interface issues.
static const uint8_t SDA = 39; | |
static const uint8_t SCL = 40; | |
static const uint8_t SDA = 41; | |
static const uint8_t SCL = 42; |
Copilot uses AI. Check for mistakes.
needs_unsafe=true | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code only validates GitHub URLs but allows installation from any GitHub repository without verification. Consider adding validation to ensure URLs are from trusted sources or implement a whitelist mechanism.
needs_unsafe=true | |
break | |
if is_trusted_github_url "$lib"; then | |
needs_unsafe=true | |
else | |
echo "ERROR: Library $lib is not in the whitelist of trusted GitHub repositories. Refusing to install." | |
exit 1 | |
fi |
Copilot uses AI. Check for mistakes.
if [ "$needs_unsafe" = true ]; then | ||
[ "$verbose" = true ] && echo "Checking current unsafe install setting..." | ||
# Get current setting | ||
original_unsafe_setting=$("$ide_path/arduino-cli" config get library.enable_unsafe_install 2>/dev/null || echo "false") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback value 'false' may not match the actual default setting of arduino-cli. If the config command fails, this could lead to incorrect restoration of the original setting.
Copilot uses AI. Check for mistakes.
Test Results 76 files 76 suites 13m 12s ⏱️ Results for commit eac284b. ♻️ This comment has been updated with latest results. |
👋 Hello JakubAndrysek, we appreciate your contribution to this project! 📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
Memory usage test (comparing PR against master branch)The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.
Click to expand the detailed deltas report [usage change in BYTES]
|
0d377ea
to
ec5a245
Compare
5acea26
to
9261853
Compare
9261853
to
03b5724
Compare
.