Conversation
sync: main back into develop
build: add compile validation workflow
ha-llm-bot
left a comment
There was a problem hiding this comment.
Reviewed for develop to main promotion.
ha-llm-bot
left a comment
There was a problem hiding this comment.
Reviewed for develop to main promotion.
Summary of ChangesHello, 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! This pull request significantly enhances the development and CI/CD workflow by introducing a robust compile-only validation mechanism for the firmware. It streamlines the build process by managing temporary configuration files, updates core ESP32 sleep API calls to modern standards, and explicitly defines project dependencies, all while thoroughly documenting these changes for improved clarity and maintainability. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable compile-only validation workflow, complete with a Python helper script and updated documentation. The changes also modernize the ESP sleep API calls and correctly declare PlatformIO library dependencies, which are great improvements for maintainability and reproducibility. My review includes one suggestion to enhance the robustness of the new compile check script.
| def main() -> int: | ||
| repo_root = Path(__file__).resolve().parent.parent | ||
| src_dir = repo_root / "src" | ||
| config_path = src_dir / "config.h" | ||
| template_path = src_dir / "config_template.h" | ||
|
|
||
| created_temp_config = False | ||
| if not config_path.exists(): | ||
| shutil.copyfile(template_path, config_path) | ||
| created_temp_config = True | ||
| print(f"created temporary config: {config_path}") | ||
| else: | ||
| print(f"using existing local config: {config_path}") | ||
|
|
||
| try: | ||
| result = subprocess.run( | ||
| [sys.executable, "-m", "platformio", "run"], | ||
| cwd=repo_root, | ||
| check=False, | ||
| ) | ||
| return result.returncode | ||
| finally: | ||
| if created_temp_config and config_path.exists(): | ||
| config_path.unlink() | ||
| print(f"removed temporary config: {config_path}") |
There was a problem hiding this comment.
The script doesn't handle the case where src/config_template.h is missing and will crash with a FileNotFoundError. It would be more robust to add an explicit check for the template file at the beginning of the function and exit with a user-friendly error message. This improves the script's robustness and user experience in a clear failure scenario.
def main() -> int:
repo_root = Path(__file__).resolve().parent.parent
src_dir = repo_root / "src"
config_path = src_dir / "config.h"
template_path = src_dir / "config_template.h"
if not template_path.exists():
print(f"error: template config not found: {template_path}", file=sys.stderr)
return 1
created_temp_config = False
if not config_path.exists():
shutil.copyfile(template_path, config_path)
created_temp_config = True
print(f"created temporary config: {config_path}")
else:
print(f"using existing local config: {config_path}")
try:
result = subprocess.run(
[sys.executable, "-m", "platformio", "run"],
cwd=repo_root,
check=False,
)
return result.returncode
finally:
if created_temp_config and config_path.exists():
config_path.unlink()
print(f"removed temporary config: {config_path}")
Summary
Verification