code-review-facilitator

代码审查协助工具

为微控制器项目提供系统化的代码审查。

资源

此技能包含打包的工具：

• scripts/analyze_code.py - 静态分析器，可检测 15 种以上常见的 Arduino 问题

快速开始

分析单个文件：

uv run --no-project scripts/analyze_code.py sketch.ino

分析整个项目：

uv run --no-project scripts/analyze_code.py --dir /path/to/project

交互模式（粘贴代码）：

uv run --no-project scripts/analyze_code.py --interactive

按严重程度筛选：

uv run --no-project scripts/analyze_code.py sketch.ino --severity warning

适用场景

• “审查我的代码”
• “这段代码可以吗？”
• “如何改进这段代码？”
• 在发布到 GitHub 之前
• 完成功能之后
• 代码“能运行，但感觉不太对”时

---

审查类别

1. 🏗️ 结构与组织

检查内容：

□ 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

常见问题：

问题	不佳示例	良好示例
巨型函数	200 行的 loop()	拆分为 readSensors()、updateDisplay() 等函数
关注点混杂	在传感器文件中编写 WiFi 代码	分离到 network.cpp/h
名称不清晰	int x, temp1, val;	int sensorReading, temperatureC;

重构示例：

// ❌ 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();
}

---

2. 💾 内存安全

关键检查项：

□ 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

内存问题表：

问题	后果	解决方案
String 碎片化	堆随时间推移而损坏	使用 char 数组、snprintf()
栈溢出	局部数组过大	使用 static/global，缩小数组
缓冲区溢出	strcpy 未限制边界	使用 strncpy、snprintf
内存泄漏	malloc 后未调用 free	避免动态分配

安全的字符串处理：

// ❌ 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"));

内存监控：

// 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!"));
}

---

3. 🔢 魔数与常量

检查内容：

□ No unexplained numbers in code
□ Pin assignments in config.h
□ Timing values named
□ Threshold values documented

示例：

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

---

4. ⚠️ 错误处理

检查内容：

□ Sensor initialization verified
□ Network connections have timeouts
□ File operations check return values
□ Graceful degradation when components fail
□ User feedback for errors (LED, serial, display)

错误处理模式：

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

---

5. ⏱️ 计时与延迟

检查内容：

□ No blocking delay() in main loop (except simple projects)
□ millis() overflow handled (after 49 days)
□ Debouncing for buttons/switches
□ Rate limiting for sensors/network

非阻塞模式：

// ❌ 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

消抖：

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

---

6. 🔌 硬件交互

检查内容：

□ Pin modes set in setup()
□ Pull-up/pull-down resistors considered
□ Voltage levels compatible (3.3V vs 5V)
□ Current limits respected
□ Proper power sequencing

引脚配置：

// ❌ 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
}

---

7. 📡 网络与通信

检查内容：

□ Credentials not hardcoded (use config file)
□ Connection retry logic
□ Timeout handling
□ Secure connections (HTTPS where possible)
□ Data validation

安全地处理凭据：

// ❌ 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

---

8. 🔋 能效

检查内容：

□ Unused peripherals disabled
□ Appropriate sleep modes used
□ WiFi off when not needed
□ LED brightness reduced (PWM)
□ Sensor power controlled

功耗优化：

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

---

代码审查清单生成器

生成适用于特定项目的清单：

## 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

### 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

### Platform-Specific (ESP32)
- [ ] WiFi reconnection logic
- [ ] Brownout detector consideration
- [ ] Deep sleep properly configured

---

代码异味检测

自动标记项

模式	严重程度	操作
在 loop() 中使用 String +	🔴 严重	替换为 snprintf
在 loop() 中使用 delay(>100)	🟡 警告	考虑使用 millis()
使用未调用 yield() 的 while(1)	🔴 严重	添加 yield() 或重构
凭据硬编码	🔴 严重	移至 config.h
使用 malloc/new 却未调用 free/delete	🔴 严重	跟踪分配情况
使用 sprintf（而非 snprintf）	🟡 警告	使用 snprintf 以确保安全
ISR 中的全局变量未声明为 volatile	🔴 严重	添加 volatile 关键字

---

审查回复模板

## Code Review Summary

**Overall Assessment:** ⭐⭐⭐☆☆ (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);`

### 🟡 Important Issues (Fix Soon)
1. **Missing error handling** - BME280 init not checked
2. **Magic number** - `delay(1500)` unexplained

### 🟢 Suggestions (Nice to Have)
1. Consider adding F() macro to Serial.print strings
2. Function `readAllSensors()` could be split

### ✅ Good Practices Found
- Clear variable naming
- Consistent formatting
- Good use of constants in config.h

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

---

快速参考命令

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

共享输出约定

请参阅共享 Arduino 技能约定。