code-review-facilitator
Compare original and translation side by side
🇺🇸
Original
English🇨🇳
Translation
ChineseCode Review Facilitator
代码审查辅助工具
Provides systematic code review for microcontroller projects.
为微控制器项目提供系统化的代码审查服务。
Resources
资源
This skill includes bundled tools:
- scripts/analyze_code.py - Static analyzer detecting 15+ common Arduino issues
本工具包含以下内置组件:
- scripts/analyze_code.py - 静态分析器,可检测15+种常见Arduino问题
Quick Start
快速开始
Analyze a file:
bash
uv run --no-project scripts/analyze_code.py sketch.inoAnalyze entire project:
bash
uv run --no-project scripts/analyze_code.py --dir /path/to/projectInteractive mode (paste code):
bash
uv run --no-project scripts/analyze_code.py --interactiveFilter by severity:
bash
uv run --no-project scripts/analyze_code.py sketch.ino --severity warning分析单个文件:
bash
uv run --no-project scripts/analyze_code.py sketch.ino分析整个项目:
bash
uv run --no-project scripts/analyze_code.py --dir /path/to/project交互模式(粘贴代码):
bash
uv run --no-project scripts/analyze_code.py --interactive按严重程度过滤:
bash
uv run --no-project scripts/analyze_code.py sketch.ino --severity warningWhen to Use
使用场景
- "Review my code"
- "Is this code okay?"
- "How can I improve this?"
- Before publishing to GitHub
- After completing a feature
- When code "works but feels wrong"
- “审查我的代码”
- “这段代码没问题吧?”
- “我该如何改进这段代码?”
- 发布到GitHub之前
- 完成功能开发之后
- 代码“能运行但感觉有问题”时
Review Categories
审查分类
1. 🏗️ Structure & Organization
1. 🏗️ 结构与组织
Check For:
□ Single responsibility - each function does ONE thing
□ File organization - separate concerns (config, sensors, display, network)
□ Consistent naming convention (camelCase for variables, UPPER_CASE for constants)
□ Reasonable function length (< 50 lines ideally)
□ Header comments explaining purposeCommon Issues:
| Issue | Bad | Good |
|---|---|---|
| God function | 200-line | Split into |
| Mixed concerns | WiFi code in sensor file | Separate network.cpp/h |
| Unclear names | | |
Example Refactoring:
cpp
// ❌ Bad: Everything in loop()
void loop() {
// 50 lines of sensor reading
// 30 lines of display update
// 40 lines of network code
}
// ✅ Good: Organized functions
void loop() {
SensorData data = readAllSensors();
updateDisplay(data);
if (shouldTransmit()) {
sendToServer(data);
}
handleSleep();
}检查项:
□ 单一职责 - 每个函数只完成一件事
□ 文件组织 - 分离关注点(配置、传感器、显示、网络)
□ 一致的命名规范(变量用小驼峰式,常量用大写蛇形式)
□ 合理的函数长度(理想情况下少于50行)
□ 说明用途的头部注释常见问题:
| 问题 | 反面示例 | 正面示例 |
|---|---|---|
| 上帝函数 | 200行的 | 拆分为 |
| 关注点混杂 | 传感器文件中包含WiFi代码 | 分离为network.cpp/h |
| 命名不清晰 | | |
重构示例:
cpp
// ❌ 反面示例:所有逻辑都在loop()中
void loop() {
// 50行传感器读取代码
// 30行显示更新代码
// 40行网络代码
}
// ✅ 正面示例:函数化组织
void loop() {
SensorData data = readAllSensors();
updateDisplay(data);
if (shouldTransmit()) {
sendToServer(data);
}
handleSleep();
}2. 💾 Memory Safety
2. 💾 内存安全
Critical Checks:
□ No String class in time-critical code (use char arrays)
□ Buffer sizes declared as constants
□ Array bounds checking
□ No dynamic memory allocation in loop()
□ Static buffers for frequently used stringsMemory Issues Table:
| Issue | Problem | Solution |
|---|---|---|
| String fragmentation | Heap corruption over time | Use char arrays, snprintf() |
| Stack overflow | Large local arrays | Use static/global, reduce size |
| Buffer overflow | strcpy without bounds | Use strncpy, snprintf |
| Memory leak | malloc without free | Avoid dynamic allocation |
Safe String Handling:
cpp
// ❌ Dangerous: String class in loop
void loop() {
String msg = "Temp: " + String(temp) + "C"; // Fragments heap
Serial.println(msg);
}
// ✅ Safe: Static buffer with snprintf
void loop() {
static char msg[32];
snprintf(msg, sizeof(msg), "Temp: %.1fC", temp);
Serial.println(msg);
}
// ✅ Safe: F() macro for flash strings
Serial.println(F("This string is in flash, not RAM"));Memory Monitoring:
cpp
// Add to setup() for debugging
Serial.print(F("Free heap: "));
Serial.println(ESP.getFreeHeap());
// Periodic check in loop()
if (ESP.getFreeHeap() < 10000) {
Serial.println(F("WARNING: Low memory!"));
}关键检查项:
□ 时间敏感代码中不使用String类(改用字符数组)
□ 缓冲区大小声明为常量
□ 数组边界检查
□ loop()中不使用动态内存分配
□ 频繁使用的字符串使用静态缓冲区内存问题对照表:
| 问题 | 影响 | 解决方案 |
|---|---|---|
| String碎片化 | 长期运行导致堆内存损坏 | 使用字符数组、snprintf() |
| 栈溢出 | 大型局部数组引发 | 使用静态/全局变量、减小数组大小 |
| 缓冲区溢出 | 无边界检查的strcpy | 使用strncpy、snprintf |
| 内存泄漏 | malloc后未free | 避免动态内存分配 |
安全的字符串处理:
cpp
// ❌ 危险:loop中使用String类
void loop() {
String msg = "Temp: " + String(temp) + "C"; // 造成堆碎片化
Serial.println(msg);
}
// ✅ 安全:使用静态缓冲区和snprintf
void loop() {
static char msg[32];
snprintf(msg, sizeof(msg), "Temp: %.1fC", temp);
Serial.println(msg);
}
// ✅ 安全:使用F()宏存储闪存字符串
Serial.println(F("This string is in flash, not RAM"));内存监控:
cpp
// 添加到setup()用于调试
Serial.print(F("Free heap: "));
Serial.println(ESP.getFreeHeap());
// 在loop()中定期检查
if (ESP.getFreeHeap() < 10000) {
Serial.println(F("WARNING: Low memory!"));
}3. 🔢 Magic Numbers & Constants
3. 🔢 魔法数字与常量
Check For:
□ No unexplained numbers in code
□ Pin assignments in config.h
□ Timing values named
□ Threshold values documentedExamples:
cpp
// ❌ Bad: Magic numbers everywhere
if (analogRead(A0) > 512) {
digitalWrite(4, HIGH);
delay(1500);
}
// ✅ Good: Named constants
// config.h
#define MOISTURE_SENSOR_PIN A0
#define PUMP_RELAY_PIN 4
#define MOISTURE_THRESHOLD 512 // ~50% soil moisture
#define PUMP_RUN_TIME_MS 1500 // 1.5 second watering
// main.ino
if (analogRead(MOISTURE_SENSOR_PIN) > MOISTURE_THRESHOLD) {
digitalWrite(PUMP_RELAY_PIN, HIGH);
delay(PUMP_RUN_TIME_MS);
}检查项:
□ 代码中无未解释的数字
□ 引脚分配放在config.h中
□ 计时值使用命名常量
□ 阈值添加文档说明示例:
cpp
// ❌ 反面示例:到处都是魔法数字
if (analogRead(A0) > 512) {
digitalWrite(4, HIGH);
delay(1500);
}
// ✅ 正面示例:使用命名常量
// config.h
#define MOISTURE_SENSOR_PIN A0
#define PUMP_RELAY_PIN 4
#define MOISTURE_THRESHOLD 512 // ~50%土壤湿度
#define PUMP_RUN_TIME_MS 1500 // 1.5秒浇水时间
// main.ino
if (analogRead(MOISTURE_SENSOR_PIN) > MOISTURE_THRESHOLD) {
digitalWrite(PUMP_RELAY_PIN, HIGH);
delay(PUMP_RUN_TIME_MS);
}4. ⚠️ Error Handling
4. ⚠️ 错误处理
Check For:
□ Sensor initialization verified
□ Network connections have timeouts
□ File operations check return values
□ Graceful degradation when components fail
□ User feedback for errors (LED, serial, display)Error Handling Patterns:
cpp
// ❌ Bad: Assume everything works
void setup() {
bme.begin(0x76); // What if it fails?
}
// ✅ Good: Check and handle failures
void setup() {
Serial.begin(115200);
if (!bme.begin(0x76)) {
Serial.println(F("BME280 not found!"));
errorBlink(ERROR_SENSOR); // Visual feedback
// Either halt or continue without sensor
sensorAvailable = false;
}
// WiFi with timeout
WiFi.begin(SSID, PASSWORD);
unsigned long startAttempt = millis();
while (WiFi.status() != WL_CONNECTED) {
if (millis() - startAttempt > WIFI_TIMEOUT_MS) {
Serial.println(F("WiFi failed - continuing offline"));
wifiAvailable = false;
break;
}
delay(500);
}
}检查项:
□ 传感器初始化已验证
□ 网络连接设有超时
□ 文件操作检查返回值
□ 组件故障时优雅降级
□ 错误提供用户反馈(LED、串口、显示屏)错误处理模式:
cpp
// ❌ 反面示例:假设所有操作都成功
void setup() {
bme.begin(0x76); // 失败了怎么办?
}
// ✅ 正面示例:检查并处理失败情况
void setup() {
Serial.begin(115200);
if (!bme.begin(0x76)) {
Serial.println(F("BME280 not found!"));
errorBlink(ERROR_SENSOR); // 视觉反馈
// 要么终止程序,要么无传感器继续运行
sensorAvailable = false;
}
// 带超时的WiFi连接
WiFi.begin(SSID, PASSWORD);
unsigned long startAttempt = millis();
while (WiFi.status() != WL_CONNECTED) {
if (millis() - startAttempt > WIFI_TIMEOUT_MS) {
Serial.println(F("WiFi failed - continuing offline"));
wifiAvailable = false;
break;
}
delay(500);
}
}5. ⏱️ Timing & Delays
5. ⏱️ 计时与延迟
Check For:
□ No blocking delay() in main loop (except simple projects)
□ millis() overflow handled (after 49 days)
□ Debouncing for buttons/switches
□ Rate limiting for sensors/networkNon-Blocking Pattern:
cpp
// ❌ Bad: Blocking delays
void loop() {
readSensor();
delay(1000); // Blocks everything for 1 second
}
// ✅ Good: Non-blocking with millis()
unsigned long previousMillis = 0;
const unsigned long INTERVAL = 1000;
void loop() {
unsigned long currentMillis = millis();
// Handle button immediately (responsive)
checkButton();
// Sensor reading at interval
if (currentMillis - previousMillis >= INTERVAL) {
previousMillis = currentMillis;
readSensor();
}
}
// ✅ millis() overflow safe (works after 49 days)
// The subtraction handles overflow automatically with unsigned mathDebouncing:
cpp
// Button debouncing
const unsigned long DEBOUNCE_MS = 50;
unsigned long lastDebounce = 0;
int lastButtonState = HIGH;
int buttonState = HIGH;
void checkButton() {
int reading = digitalRead(BUTTON_PIN);
if (reading != lastButtonState) {
lastDebounce = millis();
}
if ((millis() - lastDebounce) > DEBOUNCE_MS) {
if (reading != buttonState) {
buttonState = reading;
if (buttonState == LOW) {
handleButtonPress();
}
}
}
lastButtonState = reading;
}检查项:
□ 主loop中无阻塞式delay()(简单项目除外)
□ 处理millis()溢出问题(49天后)
□ 按钮/开关的消抖处理
□ 传感器/网络的速率限制非阻塞模式:
cpp
// ❌ 反面示例:阻塞式延迟
void loop() {
readSensor();
delay(1000); // 阻塞所有操作1秒
}
// ✅ 正面示例:使用millis()实现非阻塞
unsigned long previousMillis = 0;
const unsigned long INTERVAL = 1000;
void loop() {
unsigned long currentMillis = millis();
// 立即响应按钮
checkButton();
// 按间隔读取传感器
if (currentMillis - previousMillis >= INTERVAL) {
previousMillis = currentMillis;
readSensor();
}
}
// ✅ millis()溢出安全(49天后仍可正常工作)
// 无符号数的减法可自动处理溢出消抖处理:
cpp
// 按钮消抖
const unsigned long DEBOUNCE_MS = 50;
unsigned long lastDebounce = 0;
int lastButtonState = HIGH;
int buttonState = HIGH;
void checkButton() {
int reading = digitalRead(BUTTON_PIN);
if (reading != lastButtonState) {
lastDebounce = millis();
}
if ((millis() - lastDebounce) > DEBOUNCE_MS) {
if (reading != buttonState) {
buttonState = reading;
if (buttonState == LOW) {
handleButtonPress();
}
}
}
lastButtonState = reading;
}6. 🔌 Hardware Interactions
6. 🔌 硬件交互
Check For:
□ Pin modes set in setup()
□ Pull-up/pull-down resistors considered
□ Voltage levels compatible (3.3V vs 5V)
□ Current limits respected
□ Proper power sequencingPin Configuration:
cpp
// ❌ Bad: Missing or incorrect pin modes
digitalWrite(LED_PIN, HIGH); // Works by accident on some boards
// ✅ Good: Explicit configuration
void setup() {
// Outputs
pinMode(LED_PIN, OUTPUT);
pinMode(RELAY_PIN, OUTPUT);
// Inputs with pull-up (button connects to GND)
pinMode(BUTTON_PIN, INPUT_PULLUP);
// Analog input (no pinMode needed but document it)
// SENSOR_PIN is analog input - no pinMode required
// Set safe initial states
digitalWrite(RELAY_PIN, LOW); // Relay off at start
}检查项:
□ setup()中设置引脚模式
□ 考虑上拉/下拉电阻
□ 电压电平兼容(3.3V vs 5V)
□ 遵守电流限制
□ 正确的电源时序引脚配置:
cpp
// ❌ 反面示例:缺失或错误的引脚模式
digitalWrite(LED_PIN, HIGH); // 在部分开发板上偶然可用
// ✅ 正面示例:显式配置
void setup() {
// 输出引脚
pinMode(LED_PIN, OUTPUT);
pinMode(RELAY_PIN, OUTPUT);
// 带内部上拉的输入引脚(按钮接GND)
pinMode(BUTTON_PIN, INPUT_PULLUP);
// 模拟输入(无需设置pinMode但需文档说明)
// SENSOR_PIN为模拟输入 - 无需设置pinMode
// 设置安全初始状态
digitalWrite(RELAY_PIN, LOW); // 启动时继电器关闭
}7. 📡 Network & Communication
7. 📡 网络与通信
Check For:
□ Credentials not hardcoded (use config file)
□ Connection retry logic
□ Timeout handling
□ Secure connections (HTTPS where possible)
□ Data validationSecure Credential Handling:
cpp
// ❌ Bad: Credentials in main code
WiFi.begin("MyNetwork", "password123");
// ✅ Good: Separate config file (add to .gitignore)
// config.h
#ifndef CONFIG_H
#define CONFIG_H
#define WIFI_SSID "your-ssid"
#define WIFI_PASSWORD "your-password"
#define API_KEY "your-api-key"
#endif
// .gitignore
config.h检查项:
□ 凭证不硬编码(使用配置文件)
□ 连接重试逻辑
□ 超时处理
□ 尽可能使用安全连接(HTTPS)
□ 数据验证安全的凭证处理:
cpp
// ❌ 反面示例:凭证写在主代码中
WiFi.begin("MyNetwork", "password123");
// ✅ 正面示例:使用独立配置文件(添加到.gitignore)
// config.h
#ifndef CONFIG_H
#define CONFIG_H
#define WIFI_SSID "your-ssid"
#define WIFI_PASSWORD "your-password"
#define API_KEY "your-api-key"
#endif
// .gitignore
config.h8. 🔋 Power Efficiency
8. 🔋 电源效率
Check For:
□ Unused peripherals disabled
□ Appropriate sleep modes used
□ WiFi off when not needed
□ LED brightness reduced (PWM)
□ Sensor power controlledPower Optimization:
cpp
// ESP32 power management
void goToSleep(int seconds) {
WiFi.disconnect(true);
WiFi.mode(WIFI_OFF);
btStop();
esp_sleep_enable_timer_wakeup(seconds * 1000000ULL);
esp_deep_sleep_start();
}
// Sensor power control
#define SENSOR_POWER_PIN 25
void readSensorWithPowerControl() {
digitalWrite(SENSOR_POWER_PIN, HIGH); // Power on
delay(100); // Stabilization time
int value = analogRead(SENSOR_PIN);
digitalWrite(SENSOR_POWER_PIN, LOW); // Power off
return value;
}检查项:
□ 禁用未使用的外设
□ 使用合适的睡眠模式
□ 无需WiFi时关闭
□ 降低LED亮度(PWM)
□ 传感器电源可控电源优化:
cpp
// ESP32电源管理
void goToSleep(int seconds) {
WiFi.disconnect(true);
WiFi.mode(WIFI_OFF);
btStop();
esp_sleep_enable_timer_wakeup(seconds * 1000000ULL);
esp_deep_sleep_start();
}
// 传感器电源控制
#define SENSOR_POWER_PIN 25
void readSensorWithPowerControl() {
digitalWrite(SENSOR_POWER_PIN, HIGH); // 开启电源
delay(100); // 稳定时间
int value = analogRead(SENSOR_PIN);
digitalWrite(SENSOR_POWER_PIN, LOW); // 关闭电源
return value;
}Review Checklist Generator
审查清单生成器
Generate project-specific checklist:
markdown
undefined生成项目专属检查清单:
markdown
undefinedCode Review Checklist for [Project Name]
[项目名称]代码审查清单
Critical (Must Fix)
🔴 严重问题(必须修复)
- Memory: No String in loop()
- Safety: All array accesses bounds-checked
- Error: Sensor init failures handled
- 内存:loop()中未使用String类
- 安全:所有数组访问都做了边界检查
- 错误处理:传感器初始化失败已处理
Important (Should Fix)
🟡 重要问题(尽快修复)
- No magic numbers
- Non-blocking delays where possible
- Timeouts on all network operations
- 无魔法数字
- 尽可能使用非阻塞延迟
- 所有网络操作都设置了超时
Nice to Have
🟢 建议项(可选优化)
- F() macro for string literals
- Consistent naming convention
- Comments for complex logic
- 为Serial.print字符串添加F()宏
- 一致的命名规范
- 复杂逻辑添加注释
Platform-Specific (ESP32)
🎯 平台专属(ESP32)
- WiFi reconnection logic
- Brownout detector consideration
- Deep sleep properly configured
---- WiFi重连逻辑
- 考虑欠压检测
- 深度睡眠配置正确
---Code Smell Detection
代码异味检测
Automatic Red Flags
自动识别的风险点
| Pattern | Severity | Action |
|---|---|---|
| 🔴 Critical | Replace with snprintf |
| 🟡 Warning | Consider millis() |
| 🔴 Critical | Add yield() or refactor |
| Hardcoded credentials | 🔴 Critical | Move to config.h |
| 🔴 Critical | Track allocations |
| 🟡 Warning | Use snprintf for safety |
Global variables without | 🔴 Critical | Add volatile keyword |
| 模式 | 严重程度 | 处理建议 |
|---|---|---|
loop()中使用 | 🔴 严重 | 替换为snprintf |
loop()中使用 | 🟡 警告 | 考虑改用millis() |
| 🔴 严重 | 添加yield()或重构 |
| 硬编码凭证 | 🔴 严重 | 移至config.h |
| 🔴 严重 | 追踪内存分配 |
使用 | 🟡 警告 | 改用snprintf以保证安全 |
ISR中使用的全局变量未加 | 🔴 严重 | 添加volatile关键字 |
Review Response Template
审查回复模板
markdown
undefinedmarkdown
undefinedCode Review Summary
代码审查总结
Overall Assessment: ⭐⭐⭐☆☆ (3/5)
整体评分: ⭐⭐⭐☆☆ (3/5)
🔴 Critical Issues (Fix Before Use)
🔴 严重问题(使用前必须修复)
- Memory leak in line 45 - String concatenation in loop()
- Current:
String msg = "Value: " + String(val); - Fix: Use
snprintf(buffer, sizeof(buffer), "Value: %d", val);
- Current:
- 第45行内存泄漏 - loop()中使用字符串拼接
- 当前代码:
String msg = "Value: " + String(val); - 修复方案:使用
snprintf(buffer, sizeof(buffer), "Value: %d", val);
- 当前代码:
🟡 Important Issues (Fix Soon)
🟡 重要问题(尽快修复)
- Missing error handling - BME280 init not checked
- Magic number - unexplained
delay(1500)
- 缺失错误处理 - BME280初始化未做检查
- 魔法数字 - 未加说明
delay(1500)
🟢 Suggestions (Nice to Have)
🟢 优化建议(可选)
- Consider adding F() macro to Serial.print strings
- Function could be split
readAllSensors()
- 考虑为Serial.print字符串添加F()宏
- 函数可进一步拆分
readAllSensors()
✅ Good Practices Found
✅ 已采用的最佳实践
- Clear variable naming
- Consistent formatting
- Good use of constants in config.h
- 清晰的变量命名
- 一致的代码格式
- 合理使用config.h中的常量
Recommended Next Steps
下一步建议
- Fix critical memory issue first
- Add sensor error handling
- Run memory monitoring to verify fix
---- 优先修复严重内存问题
- 添加传感器错误处理
- 运行内存监控以验证修复效果
---Quick Reference Commands
快速参考命令
cpp
// Memory debugging
Serial.printf("Free heap: %d bytes\n", ESP.getFreeHeap());
Serial.printf("Min free heap: %d bytes\n", ESP.getMinFreeHeap());
// Stack high water mark (FreeRTOS)
Serial.printf("Stack remaining: %d bytes\n", uxTaskGetStackHighWaterMark(NULL));
// Find I2C devices
void scanI2C() {
for (byte addr = 1; addr < 127; addr++) {
Wire.beginTransmission(addr);
if (Wire.endTransmission() == 0) {
Serial.printf("Found device at 0x%02X\n", addr);
}
}
}cpp
// 内存调试
Serial.printf("Free heap: %d bytes\n", ESP.getFreeHeap());
Serial.printf("Min free heap: %d bytes\n", ESP.getMinFreeHeap());
// 栈剩余空间(FreeRTOS)
Serial.printf("Stack remaining: %d bytes\n", uxTaskGetStackHighWaterMark(NULL));
// 扫描I2C设备
void scanI2C() {
for (byte addr = 1; addr < 127; addr++) {
Wire.beginTransmission(addr);
if (Wire.endTransmission() == 0) {
Serial.printf("Found device at 0x%02X\n", addr);
}
}
}