Merge pull request 'fix: auto-detect upload port fails with active log output' (#24) from fix/issue-16-auto-detect-port into main
All checks were successful
CI / build (push) Successful in 58s
All checks were successful
CI / build (push) Successful in 58s
This commit is contained in:
commit
ec310c1bcb
5 changed files with 366 additions and 1 deletions
181
docs/superpowers/plans/2026-08-04-auto-detect-port-fix.md
Normal file
181
docs/superpowers/plans/2026-08-04-auto-detect-port-fix.md
Normal file
|
|
@ -0,0 +1,181 @@
|
||||||
|
# Auto-detect Upload Port Fix Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Fix auto-detect upload port failure when active log output interferes with ID? response detection.
|
||||||
|
|
||||||
|
**Architecture:** Two-part fix — script-side filtering continues reading until ID match, firmware-side serial mute pauses all output during ID? response.
|
||||||
|
|
||||||
|
**Tech Stack:** Python 3 + pyserial (script), ESP32-S3 + Arduino + ESP-IDF logging (firmware)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## File Structure
|
||||||
|
|
||||||
|
| File | Action | Purpose |
|
||||||
|
|------|--------|---------|
|
||||||
|
| `tools/pio_upload_port.py` | Modify | Filter ID responses in query_id() |
|
||||||
|
| `src/main.cpp` | Modify | Add serial mute for server |
|
||||||
|
| `src/client.cpp` | Modify | Add serial mute for client |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 1: Script-side ID filtering
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `tools/pio_upload_port.py:46-61`
|
||||||
|
|
||||||
|
- [ ] **Step 1: Update query_id() to filter for ID response**
|
||||||
|
|
||||||
|
Replace lines 46-61 in `tools/pio_upload_port.py`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def query_id(port, timeout=5):
|
||||||
|
try:
|
||||||
|
ser = serial.Serial(port, 115200, timeout=0.5)
|
||||||
|
time.sleep(0.1)
|
||||||
|
ser.reset_input_buffer()
|
||||||
|
ser.write(b"ID?\n")
|
||||||
|
deadline = time.time() + timeout
|
||||||
|
while time.time() < deadline:
|
||||||
|
line = ser.readline().decode("utf-8", errors="ignore").strip()
|
||||||
|
if "SylDa_Server" in line or "SylDa_Client" in line:
|
||||||
|
ser.close()
|
||||||
|
return line
|
||||||
|
ser.close()
|
||||||
|
except (serial.SerialException, OSError):
|
||||||
|
pass
|
||||||
|
return None
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add tools/pio_upload_port.py
|
||||||
|
git commit -m "fix(script): filter ID response in query_id()"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 2: Server firmware serial mute
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `src/main.cpp`
|
||||||
|
|
||||||
|
- [ ] **Step 1: Add serialMuted flag and mute helper**
|
||||||
|
|
||||||
|
After line 17 (`static const char *TAG = "server";`), add:
|
||||||
|
|
||||||
|
```cpp
|
||||||
|
static bool serialMuted = false;
|
||||||
|
|
||||||
|
static void muteSerial() {
|
||||||
|
serialMuted = true;
|
||||||
|
esp_log_level_set("*", ESP_LOG_NONE);
|
||||||
|
}
|
||||||
|
|
||||||
|
static void unmuteSerial() {
|
||||||
|
serialMuted = false;
|
||||||
|
esp_log_level_set("*", CONFIG_LOG_DEFAULT_LEVEL);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Update ID? handler to use mute**
|
||||||
|
|
||||||
|
Replace lines 591-592:
|
||||||
|
|
||||||
|
```cpp
|
||||||
|
} else if (serialBuf == "ID?") {
|
||||||
|
Serial.flush();
|
||||||
|
muteSerial();
|
||||||
|
Serial.println("SylDa_Server");
|
||||||
|
delay(50);
|
||||||
|
unmuteSerial();
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 3: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add src/main.cpp
|
||||||
|
git commit -m "fix(server): add serial mute during ID? response"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 3: Client firmware serial mute
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `src/client.cpp`
|
||||||
|
|
||||||
|
- [ ] **Step 1: Add serialMuted flag and mute helper**
|
||||||
|
|
||||||
|
After line 15 (`static const char *TAG = "client";`), add:
|
||||||
|
|
||||||
|
```cpp
|
||||||
|
static bool serialMuted = false;
|
||||||
|
|
||||||
|
static void muteSerial() {
|
||||||
|
serialMuted = true;
|
||||||
|
esp_log_level_set("*", ESP_LOG_NONE);
|
||||||
|
}
|
||||||
|
|
||||||
|
static void unmuteSerial() {
|
||||||
|
serialMuted = false;
|
||||||
|
esp_log_level_set("*", CONFIG_LOG_DEFAULT_LEVEL);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Update ID? handler to use mute**
|
||||||
|
|
||||||
|
Replace lines 425-426:
|
||||||
|
|
||||||
|
```cpp
|
||||||
|
} else if (serialBuf == "ID?") {
|
||||||
|
Serial.flush();
|
||||||
|
muteSerial();
|
||||||
|
Serial.println("SylDa_Client");
|
||||||
|
delay(50);
|
||||||
|
unmuteSerial();
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 3: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add src/client.cpp
|
||||||
|
git commit -m "fix(client): add serial mute during ID? response"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 4: Build and verify
|
||||||
|
|
||||||
|
- [ ] **Step 1: Build both environments**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
pio run -e s3lite-server
|
||||||
|
pio run -e s3lite-client
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: Both compile without errors.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Verify firmware version bump**
|
||||||
|
|
||||||
|
Check `version.txt` exists with correct format.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Push branch**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git push -u origin fix/issue-16-auto-detect-port
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Testing Checklist
|
||||||
|
|
||||||
|
After implementation, test with actual hardware:
|
||||||
|
|
||||||
|
1. Connect both server and client via USB
|
||||||
|
2. Run `pio run -e s3lite-client -t upload` — should auto-detect client
|
||||||
|
3. Run `pio run -e s3lite-server -t upload` — should auto-detect server
|
||||||
|
4. Verify log output resumes after upload completes
|
||||||
|
5. Test during active ECDH handshake (worst case)
|
||||||
152
docs/superpowers/specs/2026-08-04-auto-detect-port-fix.md
Normal file
152
docs/superpowers/specs/2026-08-04-auto-detect-port-fix.md
Normal file
|
|
@ -0,0 +1,152 @@
|
||||||
|
# Auto-detect Upload Port Fix
|
||||||
|
|
||||||
|
**Issue:** #16
|
||||||
|
**Date:** 2026-08-04
|
||||||
|
**Status:** Approved
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
The post-script `tools/pio_upload_port.py` sends `ID?` to the ESP and reads the first non-empty response to determine device type (server/client). The client responds correctly with `SylDa_Client`, but during upload there was active log output (ECDH handshake) on the serial port. The `query_id()` function reads the first log message instead of the ID response, causing auto-detect to fail.
|
||||||
|
|
||||||
|
Root cause: `query_id()` blindly takes the first non-empty line, even if it's log output.
|
||||||
|
|
||||||
|
## Solution
|
||||||
|
|
||||||
|
Two-part fix for maximum robustness:
|
||||||
|
|
||||||
|
1. **Script-side:** Filter responses — keep reading until a line containing `SylDa_Server` or `SylDa_Client` appears
|
||||||
|
2. **Firmware-side:** Pause all serial output during `ID?` response
|
||||||
|
|
||||||
|
No protocol changes. Both parts work independently but together provide robustness.
|
||||||
|
|
||||||
|
## Architecture
|
||||||
|
|
||||||
|
```
|
||||||
|
┌─────────────────────┐ ID?\n ┌─────────────────────┐
|
||||||
|
│ pio_upload_port │ ─────────────▶│ Firmware (ESP32) │
|
||||||
|
│ (Python script) │ │ │
|
||||||
|
│ │◀─────────────│ serialMuted=true │
|
||||||
|
│ query_id() reads │ SylDa_* │ Serial.flush() │
|
||||||
|
│ until match │ │ response │
|
||||||
|
└─────────────────────┘ │ serialMuted=false │
|
||||||
|
└─────────────────────┘
|
||||||
|
```
|
||||||
|
|
||||||
|
## Components
|
||||||
|
|
||||||
|
### 1. Script: `tools/pio_upload_port.py`
|
||||||
|
|
||||||
|
**Change:** `query_id()` function filters for expected ID instead of returning first non-empty line.
|
||||||
|
|
||||||
|
**Before:**
|
||||||
|
```python
|
||||||
|
def query_id(port, timeout=5):
|
||||||
|
# ...
|
||||||
|
while time.time() < deadline:
|
||||||
|
line = ser.readline().decode("utf-8", errors="ignore").strip()
|
||||||
|
if line:
|
||||||
|
ser.close()
|
||||||
|
return line # Returns first non-empty, even if log
|
||||||
|
# ...
|
||||||
|
```
|
||||||
|
|
||||||
|
**After:**
|
||||||
|
```python
|
||||||
|
def query_id(port, timeout=5):
|
||||||
|
try:
|
||||||
|
ser = serial.Serial(port, 115200, timeout=0.5)
|
||||||
|
time.sleep(0.1)
|
||||||
|
ser.reset_input_buffer()
|
||||||
|
ser.write(b"ID?\n")
|
||||||
|
deadline = time.time() + timeout
|
||||||
|
while time.time() < deadline:
|
||||||
|
line = ser.readline().decode("utf-8", errors="ignore").strip()
|
||||||
|
if "SylDa_Server" in line or "SylDa_Client" in line:
|
||||||
|
ser.close()
|
||||||
|
return line
|
||||||
|
ser.close()
|
||||||
|
except (serial.SerialException, OSError):
|
||||||
|
pass
|
||||||
|
return None
|
||||||
|
```
|
||||||
|
|
||||||
|
**No other changes needed** — the rest of the script already uses `expected_id in resp` checks.
|
||||||
|
|
||||||
|
### 2. Firmware: `src/client.cpp` and `src/server.cpp`
|
||||||
|
|
||||||
|
**Changes:**
|
||||||
|
|
||||||
|
1. Global `serialMuted` flag
|
||||||
|
2. Serial mute during `ID?` handling
|
||||||
|
3. Wrapper macros for existing Serial calls
|
||||||
|
|
||||||
|
**Implementation:**
|
||||||
|
|
||||||
|
```cpp
|
||||||
|
// Global flag (file scope)
|
||||||
|
static bool serialMuted = false;
|
||||||
|
|
||||||
|
// Wrapper macros
|
||||||
|
#define SERIAL_PRINTLN(msg) if (!serialMuted) Serial.println(msg)
|
||||||
|
#define SERIAL_PRINT(msg) if (!serialMuted) Serial.print(msg)
|
||||||
|
|
||||||
|
// In serial command handling
|
||||||
|
} else if (serialBuf == "ID?") {
|
||||||
|
Serial.flush();
|
||||||
|
serialMuted = true;
|
||||||
|
Serial.println("SylDa_Client"); // or SylDa_Server
|
||||||
|
delay(50); // Ensure response transmitted
|
||||||
|
serialMuted = false;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Existing `Serial.println()` calls:** Replace with `SERIAL_PRINTLN()` macro.
|
||||||
|
|
||||||
|
**ESP_LOG macros:** Unchanged — these use IDF logging, not Arduino Serial.
|
||||||
|
|
||||||
|
### 3. Server firmware
|
||||||
|
|
||||||
|
Identical implementation to client, with `SylDa_Server` response.
|
||||||
|
|
||||||
|
## Data Flow
|
||||||
|
|
||||||
|
1. Script opens serial port, sends `ID?\n`
|
||||||
|
2. Firmware receives command:
|
||||||
|
- `Serial.flush()` empties TX buffer
|
||||||
|
- `serialMuted = true` blocks new output
|
||||||
|
- `Serial.println("SylDa_Client")` — only output
|
||||||
|
- `delay(50)` ensures transmission
|
||||||
|
- `serialMuted = false` releases mute
|
||||||
|
3. Script reads lines until `SylDa_*` appears (timeout 5s)
|
||||||
|
4. Match → port selected
|
||||||
|
|
||||||
|
## Error Handling
|
||||||
|
|
||||||
|
| Scenario | Behavior |
|
||||||
|
|----------|----------|
|
||||||
|
| No response | Script timeout → manual port selection (existing) |
|
||||||
|
| Wrong response | Script detects mismatch → manual selection (existing) |
|
||||||
|
| Multiple ports, both respond | Script matches correct one, selects it |
|
||||||
|
| Firmware mute stuck | Impossible — mute released immediately after response |
|
||||||
|
| Buffer overflow | `Serial.flush()` prevents this |
|
||||||
|
|
||||||
|
## Testing
|
||||||
|
|
||||||
|
1. Connect both server and client simultaneously
|
||||||
|
2. Run `pio run -e s3lite-client -t upload` — should auto-detect client
|
||||||
|
3. Run `pio run -e s3lite-server -t upload` — should auto-detect server
|
||||||
|
4. Verify log output resumes after `ID?` response
|
||||||
|
5. Test with active ECDH handshake in progress
|
||||||
|
|
||||||
|
## Files Changed
|
||||||
|
|
||||||
|
- `tools/pio_upload_port.py` — `query_id()` filtering
|
||||||
|
- `src/client.cpp` — serial mute implementation
|
||||||
|
- `src/server.cpp` — serial mute implementation
|
||||||
|
|
||||||
|
## Rollback
|
||||||
|
|
||||||
|
If issues arise, revert to manual port selection:
|
||||||
|
```bash
|
||||||
|
pio run -e s3lite-client -t upload --upload-port /dev/serial/by-id/...
|
||||||
|
```
|
||||||
|
|
@ -13,6 +13,18 @@
|
||||||
|
|
||||||
static const char *TAG = "client";
|
static const char *TAG = "client";
|
||||||
|
|
||||||
|
static bool serialMuted = false;
|
||||||
|
|
||||||
|
static void muteSerial() {
|
||||||
|
serialMuted = true;
|
||||||
|
esp_log_level_set("*", ESP_LOG_NONE);
|
||||||
|
}
|
||||||
|
|
||||||
|
static void unmuteSerial() {
|
||||||
|
serialMuted = false;
|
||||||
|
esp_log_level_set("*", (esp_log_level_t)CONFIG_LOG_DEFAULT_LEVEL);
|
||||||
|
}
|
||||||
|
|
||||||
#define TOUCH1_PIN 8
|
#define TOUCH1_PIN 8
|
||||||
#define TOUCH2_PIN 6
|
#define TOUCH2_PIN 6
|
||||||
#define BUTTON_PIN 41
|
#define BUTTON_PIN 41
|
||||||
|
|
@ -423,7 +435,11 @@ void loop() {
|
||||||
Serial.println("NO_KEY");
|
Serial.println("NO_KEY");
|
||||||
}
|
}
|
||||||
} else if (serialBuf == "ID?") {
|
} else if (serialBuf == "ID?") {
|
||||||
|
Serial.flush();
|
||||||
|
muteSerial();
|
||||||
Serial.println("SylDa_Client");
|
Serial.println("SylDa_Client");
|
||||||
|
delay(50);
|
||||||
|
unmuteSerial();
|
||||||
} else if (serialBuf == "VERSION?") {
|
} else if (serialBuf == "VERSION?") {
|
||||||
Serial.print("BUILD_NR:");
|
Serial.print("BUILD_NR:");
|
||||||
Serial.println(FIRMWARE_VERSION_CLIENT);
|
Serial.println(FIRMWARE_VERSION_CLIENT);
|
||||||
|
|
|
||||||
16
src/main.cpp
16
src/main.cpp
|
|
@ -16,6 +16,18 @@
|
||||||
|
|
||||||
static const char *TAG = "server";
|
static const char *TAG = "server";
|
||||||
|
|
||||||
|
static bool serialMuted = false;
|
||||||
|
|
||||||
|
static void muteSerial() {
|
||||||
|
serialMuted = true;
|
||||||
|
esp_log_level_set("*", ESP_LOG_NONE);
|
||||||
|
}
|
||||||
|
|
||||||
|
static void unmuteSerial() {
|
||||||
|
serialMuted = false;
|
||||||
|
esp_log_level_set("*", (esp_log_level_t)CONFIG_LOG_DEFAULT_LEVEL);
|
||||||
|
}
|
||||||
|
|
||||||
#define RELAY1_PIN 1
|
#define RELAY1_PIN 1
|
||||||
#define RELAY2_PIN 2
|
#define RELAY2_PIN 2
|
||||||
#define TOUCH1_PIN 8
|
#define TOUCH1_PIN 8
|
||||||
|
|
@ -589,7 +601,11 @@ void loop() {
|
||||||
Serial.println("NO_KEY");
|
Serial.println("NO_KEY");
|
||||||
}
|
}
|
||||||
} else if (serialBuf == "ID?") {
|
} else if (serialBuf == "ID?") {
|
||||||
|
Serial.flush();
|
||||||
|
muteSerial();
|
||||||
Serial.println("SylDa_Server");
|
Serial.println("SylDa_Server");
|
||||||
|
delay(50);
|
||||||
|
unmuteSerial();
|
||||||
} else if (serialBuf == "VERSION?") {
|
} else if (serialBuf == "VERSION?") {
|
||||||
Serial.flush();
|
Serial.flush();
|
||||||
Serial.print("BUILD_NR:");
|
Serial.print("BUILD_NR:");
|
||||||
|
|
|
||||||
|
|
@ -52,7 +52,7 @@ def query_id(port, timeout=5):
|
||||||
deadline = time.time() + timeout
|
deadline = time.time() + timeout
|
||||||
while time.time() < deadline:
|
while time.time() < deadline:
|
||||||
line = ser.readline().decode("utf-8", errors="ignore").strip()
|
line = ser.readline().decode("utf-8", errors="ignore").strip()
|
||||||
if line:
|
if "SylDa_Server" in line or "SylDa_Client" in line:
|
||||||
ser.close()
|
ser.close()
|
||||||
return line
|
return line
|
||||||
ser.close()
|
ser.close()
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue