code-review-facilitator

コードレビューのファシリテーター

マイクロコントローラープロジェクトの体系的なコードレビューを提供します。

リソース

このスキルには、以下のツールが含まれています。

• scripts/analyze_code.py — Arduino でよくある 15 件以上の問題を検出する静的アナライザー

クイックスタート

ファイルを解析する：

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 またはグローバルを使用し、サイズを小さくする
バッファーオーバーフロー	境界チェックなしの strcpy	strncpy、snprintf を使用する
メモリリーク	malloc に対する free の呼び出しがない	動的メモリ割り当てを避ける

安全な String の扱い：

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

---

コードスメルの検出

自動検出される危険なパターン

パターン	重要度	対応
String + in loop()	🔴 Critical	snprintf に置き換える
delay(>100) in loop()	🟡 Warning	millis() の使用を検討する
yield() を含まない while(1)	🔴 Critical	yield() を追加するか、リファクタリングする
ハードコードされた認証情報	🔴 Critical	config.h に移動する
free/delete のない malloc/new	🔴 Critical	割り当てを追跡する
sprintf（snprintf ではない）	🟡 Warning	安全のため snprintf を使用する
ISR 用に volatile が指定されていないグローバル変数	🔴 Critical	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 スキルの共通規約を参照してください。