code-review-facilitator

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Code 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.ino
Analyze entire project:
bash
uv run --no-project scripts/analyze_code.py --dir /path/to/project
Interactive mode (paste code):
bash
uv run --no-project scripts/analyze_code.py --interactive
Filter 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 warning

When 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 purpose
Common Issues:
IssueBadGood
God function200-line
loop()
Split into
readSensors()
,
updateDisplay()
, etc.
Mixed concernsWiFi code in sensor fileSeparate network.cpp/h
Unclear names
int x, temp1, val;
int sensorReading, temperatureC;
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行的
loop()
拆分为
readSensors()
updateDisplay()
等函数
关注点混杂传感器文件中包含WiFi代码分离为network.cpp/h
命名不清晰
int x, temp1, val;
int sensorReading, temperatureC;
重构示例:
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 strings
Memory Issues Table:
IssueProblemSolution
String fragmentationHeap corruption over timeUse char arrays, snprintf()
Stack overflowLarge local arraysUse static/global, reduce size
Buffer overflowstrcpy without boundsUse strncpy, snprintf
Memory leakmalloc without freeAvoid 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 documented
Examples:
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/network
Non-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 math
Debouncing:
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 sequencing
Pin 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 validation
Secure 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.h

8. 🔋 Power Efficiency

8. 🔋 电源效率

Check For:
□ Unused peripherals disabled
□ Appropriate sleep modes used
□ WiFi off when not needed
□ LED brightness reduced (PWM)
□ Sensor power controlled
Power 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
undefined

Code 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

自动识别的风险点

PatternSeverityAction
String +
in loop()
🔴 CriticalReplace with snprintf
delay(>100)
in loop()
🟡 WarningConsider millis()
while(1)
without yield()
🔴 CriticalAdd yield() or refactor
Hardcoded credentials🔴 CriticalMove to config.h
malloc/new
without
free/delete
🔴 CriticalTrack allocations
sprintf
(not snprintf)
🟡 WarningUse snprintf for safety
Global variables without
volatile
for ISR
🔴 CriticalAdd volatile keyword

模式严重程度处理建议
loop()中使用
String +
🔴 严重替换为snprintf
loop()中使用
delay(>100)
🟡 警告考虑改用millis()
while(1)
中未添加yield()
🔴 严重添加yield()或重构
硬编码凭证🔴 严重移至config.h
malloc/new
后未调用
free/delete
🔴 严重追踪内存分配
使用
sprintf
(而非snprintf)
🟡 警告改用snprintf以保证安全
ISR中使用的全局变量未加
volatile
🔴 严重添加volatile关键字

Review Response Template

审查回复模板

markdown
undefined
markdown
undefined

Code Review Summary

代码审查总结

Overall Assessment: ⭐⭐⭐☆☆ (3/5)
整体评分: ⭐⭐⭐☆☆ (3/5)

🔴 Critical Issues (Fix Before Use)

🔴 严重问题(使用前必须修复)

  1. Memory leak in line 45 - String concatenation in loop()
    • Current:
      String msg = "Value: " + String(val);
    • Fix: Use
      snprintf(buffer, sizeof(buffer), "Value: %d", val);
  1. 第45行内存泄漏 - loop()中使用字符串拼接
    • 当前代码:
      String msg = "Value: " + String(val);
    • 修复方案:使用
      snprintf(buffer, sizeof(buffer), "Value: %d", val);

🟡 Important Issues (Fix Soon)

🟡 重要问题(尽快修复)

  1. Missing error handling - BME280 init not checked
  2. Magic number -
    delay(1500)
    unexplained
  1. 缺失错误处理 - BME280初始化未做检查
  2. 魔法数字 -
    delay(1500)
    未加说明

🟢 Suggestions (Nice to Have)

🟢 优化建议(可选)

  1. Consider adding F() macro to Serial.print strings
  2. Function
    readAllSensors()
    could be split
  1. 考虑为Serial.print字符串添加F()宏
  2. readAllSensors()
    函数可进一步拆分

✅ Good Practices Found

✅ 已采用的最佳实践

  • Clear variable naming
  • Consistent formatting
  • Good use of constants in config.h
  • 清晰的变量命名
  • 一致的代码格式
  • 合理使用config.h中的常量

Recommended Next Steps

下一步建议

  1. Fix critical memory issue first
  2. Add sensor error handling
  3. Run memory monitoring to verify fix

---
  1. 优先修复严重内存问题
  2. 添加传感器错误处理
  3. 运行内存监控以验证修复效果

---

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);
    }
  }
}