From 8aeb609b2e266a32f7b501ea512777e0f6767dc2 Mon Sep 17 00:00:00 2001 From: eddy Date: Tue, 4 Aug 2026 18:32:15 +0200 Subject: [PATCH 01/10] fix: add SPDX license headers to firmware files --- src/client.cpp | 1 + src/ecdh_helper.h | 1 + src/main.cpp | 1 + 3 files changed, 3 insertions(+) diff --git a/src/client.cpp b/src/client.cpp index 4965916..4a85039 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-3.0-or-later #include #include #include diff --git a/src/ecdh_helper.h b/src/ecdh_helper.h index edc70de..b9b6d4f 100644 --- a/src/ecdh_helper.h +++ b/src/ecdh_helper.h @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-3.0-or-later #pragma once #include diff --git a/src/main.cpp b/src/main.cpp index f22cbdf..5b6543e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-3.0-or-later #include #include #include From c36ca10edd1493ecd4ca6b2fccf1faba8b8485a4 Mon Sep 17 00:00:00 2001 From: eddy Date: Tue, 4 Aug 2026 18:33:35 +0200 Subject: [PATCH 02/10] fix: remove outdated ble_client.py, add test_ota_notify.py to tools - Remove scripts/ble_client.py (uses old UUIDs, incompatible) - Move test_ota_notify.py to tools/ directory - Make serial port configurable via SYLDA_CLIENT_PORT env var --- scripts/ble_client.py | 87 --------------------------- tools/test_ota_notify.py | 123 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 87 deletions(-) delete mode 100644 scripts/ble_client.py create mode 100644 tools/test_ota_notify.py diff --git a/scripts/ble_client.py b/scripts/ble_client.py deleted file mode 100644 index 8d42fdd..0000000 --- a/scripts/ble_client.py +++ /dev/null @@ -1,87 +0,0 @@ -#!/usr/bin/env python3 -"""BLE client for SylDa_Server — send color commands from laptop.""" - -import asyncio -import sys -from bleak import BleakScanner, BleakClient - -SERVICE_UUID = "0000abcd-0000-1000-8000-00805f9b34fb" -CHAR_UUID = "0000abce-0000-1000-8000-00805f9b34fb" -SERVER_NAME = "SylDa_Server" - -COLORS = { - "r": ("red", b"\x00"), - "g": ("green", b"\x01"), - "y": ("yellow", b"\x02"), - "o": ("orange", b"\x03"), - "b": ("blue", b"\x04"), -} - - -async def find_server(): - print(f"Scanning for {SERVER_NAME}...") - device = await BleakScanner.find_device_by_name(SERVER_NAME, timeout=10) - if not device: - print("Not found.") - return None - print(f"Found: {device.name} ({device.address})") - return device - - -async def send_color(client, data): - await client.write_gatt_char(CHAR_UUID, data, response=True) - print(f"Sent: {COLORS[[k for k, v in COLORS.items() if v[1] == data][0]][0]}") - - -def show_menu(): - print("\n┌─────────────────────┐") - print("│ r = red │") - print("│ g = green │") - print("│ y = yellow │") - print("│ o = orange │") - print("│ b = blue │") - print("│ q = quit │") - print("└─────────────────────┘") - - -async def connect_handler(client): - print(f"Connected: {client.is_connected}") - - -async def disconnect_handler(client): - print("Disconnected from server.") - - -async def main(): - device = await find_server() - if not device: - return - - async with BleakClient( - device, - disconnected_callback=disconnect_handler, - timeout=10, - ) as client: - await connect_handler(client) - - while True: - try: - show_menu() - cmd = await asyncio.get_event_loop().run_in_executor( - None, lambda: input("> ").strip().lower() - ) - except (EOFError, KeyboardInterrupt): - break - - if cmd == "q": - break - elif cmd in COLORS: - await send_color(client, COLORS[cmd][1]) - else: - print(f"Unknown: {cmd}") - - print("Bye.") - - -if __name__ == "__main__": - asyncio.run(main()) diff --git a/tools/test_ota_notify.py b/tools/test_ota_notify.py new file mode 100644 index 0000000..19b2308 --- /dev/null +++ b/tools/test_ota_notify.py @@ -0,0 +1,123 @@ +#!/usr/bin/env python3 +"""Test OTA notify flow: writes 0x02 to server's CLIENT_OTA_STATUS via BLE, + then checks client serial for peripheral-mode entry log.""" +import sys, time, asyncio, serial, threading, os +from bleak import BleakScanner, BleakClient + +SERVICE_UUID = "4fafc201-1fb5-459e-8fcc-c5c9c331914b" +STATUS_UUID = "beb5483e-36e1-4688-b7f5-ea07361f26df" +SERVER_NAME = "SylDa_Server" + +CLIENT_PORT = os.environ.get("SYLDA_CLIENT_PORT", "/dev/serial/by-id/usb-Espressif_USB_JTAG_serial_debug_unit_44:1B:F6:F5:D1:E0-if00") +TIMEOUT_S = 25 + +class TestResult: + def __init__(self): + self.pass_marker = False + self.fail_marker = False + self.lines = [] + +def monitor_serial(stop_event, result): + try: + ser = serial.Serial(CLIENT_PORT, 115200, timeout=0.5) + print(f"[TEST] Serial opened: {CLIENT_PORT}") + while not stop_event.is_set(): + try: + data = ser.read(1024) + if data: + text = data.decode("ascii", errors="replace") + for line in text.splitlines(): + if line.strip(): + result.lines.append(line) + if "peripheral" in line.lower(): + result.pass_marker = True + stop_event.set() + print(f"[TEST] PASS: {line}") + if "OTA: received trigger" in line or "entering peripheral" in line: + result.pass_marker = True + stop_event.set() + print(f"[TEST] PASS: {line}") + except serial.SerialException: + result.fail_marker = True + stop_event.set() + print("[TEST] FAIL: serial exception") + except Exception as e: + result.fail_marker = True + stop_event.set() + print(f"[TEST] FAIL: {e}") + finally: + try: ser.close() + except: pass + +async def ble_write_02(result): + print("[TEST] Scanning for SylDa_Server...") + try: + device = await BleakScanner.find_device_by_name(SERVER_NAME, timeout=10) + except Exception as e: + print(f"[TEST] FAIL: BLE scan error: {e}") + result.fail_marker = True + return + + if device is None: + print("[TEST] FAIL: SylDa_Server not found via BLE scan") + result.fail_marker = True + return + + print(f"[TEST] Found: {device.name} ({device.address}), connecting...") + try: + async with BleakClient(device, timeout=10) as client: + print(f"[TEST] Connected, writing 0x02 to {STATUS_UUID}...") + svc = client.services.get_service(SERVICE_UUID) + if svc is None: + print("[TEST] FAIL: service not found") + result.fail_marker = True + return + char = svc.get_characteristic(STATUS_UUID) + if char is None: + print("[TEST] FAIL: CLIENT_OTA_STATUS characteristic not found") + result.fail_marker = True + return + await client.write_gatt_char(char, b'\x02', response=True) + print("[TEST] Wrote 0x02 OK, waiting for client to enter peripheral mode...") + except Exception as e: + print(f"[TEST] FAIL: BLE write error: {e}") + result.fail_marker = True + return + +async def main(): + result = TestResult() + stop = threading.Event() + + serial_thread = threading.Thread(target=monitor_serial, args=(stop, result), daemon=True) + serial_thread.start() + + await asyncio.sleep(2) + + await ble_write_02(result) + + if not stop.is_set(): + print(f"[TEST] Waiting up to {TIMEOUT_S}s for client response...") + stop.wait(TIMEOUT_S) + + stop.set() + serial_thread.join(timeout=3) + + if result.pass_marker: + print("[TEST] ===== PASS =====") + return 0 + elif result.fail_marker: + print("[TEST] ===== FAIL =====") + print("[TEST] Serial lines captured:") + for line in result.lines[-20:]: + print(f" {line}") + return 1 + else: + print("[TEST] ===== INDETERMINATE (timeout, no pass/fail marker) =====") + print("[TEST] Serial lines captured:") + for line in result.lines[-20:]: + print(f" {line}") + return 2 + +if __name__ == "__main__": + rc = asyncio.run(main()) + sys.exit(rc) From ce1a53eee69287b597628c93e77892b687c3c17c Mon Sep 17 00:00:00 2001 From: eddy Date: Tue, 4 Aug 2026 18:34:26 +0200 Subject: [PATCH 03/10] fix: use Activity Result API instead of deprecated methods - Replace onActivityResult with registerForActivityResult - Replace onRequestPermissionsResult with permissionLauncher - Remove deprecated @Suppress annotations --- .../java/nl/eddydevink/sylda/MainActivity.kt | 56 ++++++++----------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/android/app/src/main/java/nl/eddydevink/sylda/MainActivity.kt b/android/app/src/main/java/nl/eddydevink/sylda/MainActivity.kt index 8f73a52..e18839c 100644 --- a/android/app/src/main/java/nl/eddydevink/sylda/MainActivity.kt +++ b/android/app/src/main/java/nl/eddydevink/sylda/MainActivity.kt @@ -2,7 +2,6 @@ package nl.eddydevink.sylda import android.content.Intent -import android.content.pm.PackageManager import android.graphics.Color import android.net.Uri import android.os.Bundle @@ -13,14 +12,13 @@ import android.widget.Button import android.widget.ImageButton import android.widget.TextView import android.widget.Toast +import androidx.activity.result.contract.ActivityResultContracts import androidx.appcompat.app.AppCompatActivity -import androidx.core.app.ActivityCompat import androidx.core.content.ContextCompat class MainActivity : AppCompatActivity(), BleCallbacks { companion object { - private const val REQUEST_SETTINGS = 1 private const val POST_SETTINGS_RESCAN_MS = 500L private const val ON_RESUME_SCAN_DELAY_MS = 300L } @@ -34,6 +32,25 @@ class MainActivity : AppCompatActivity(), BleCallbacks { private var lastTheme: String? = null private var triedPermissions = false + private val settingsLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result -> + if (result.resultCode == SettingsActivity.RESULT_SCAN) { + triedPermissions = false + manager.disconnect() + manager.resetTriedAddresses() + manager.updateKeys() + handler.postDelayed({ manager.startScan() }, POST_SETTINGS_RESCAN_MS) + } + } + + private val permissionLauncher = registerForActivityResult(ActivityResultContracts.RequestMultiplePermissions()) { grants -> + if (grants.values.all { it }) { + manager.startScan() + } else { + triedPermissions = false + onStatusChanged(BleStatus.PermissionsDenied) + } + } + override fun onStatusChanged(status: BleStatus) { when (status) { BleStatus.Connected -> { @@ -99,10 +116,10 @@ class MainActivity : AppCompatActivity(), BleCallbacks { val settingsContainer = findViewById(R.id.settingsContainer) settingsContainer.setOnClickListener { - startActivityForResult(Intent(this, SettingsActivity::class.java), REQUEST_SETTINGS) + settingsLauncher.launch(Intent(this, SettingsActivity::class.java)) } findViewById(R.id.settingsButton).setOnClickListener { - startActivityForResult(Intent(this, SettingsActivity::class.java), REQUEST_SETTINGS) + settingsLauncher.launch(Intent(this, SettingsActivity::class.java)) } setupRelayButton() setupRelay2Button() @@ -110,18 +127,6 @@ class MainActivity : AppCompatActivity(), BleCallbacks { relay2Button.isEnabled = true } - @Deprecated("Deprecated in Java") - override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) { - super.onActivityResult(requestCode, resultCode, data) - if (requestCode == REQUEST_SETTINGS && resultCode == SettingsActivity.RESULT_SCAN) { - triedPermissions = false - manager.disconnect() - manager.resetTriedAddresses() - manager.updateKeys() - handler.postDelayed({ manager.startScan() }, POST_SETTINGS_RESCAN_MS) - } - } - override fun onResume() { super.onResume() val current = PrefsHelper.getPrefs(this).getString(PrefsHelper.PREF_THEME, "system") ?: "system" @@ -135,7 +140,7 @@ class MainActivity : AppCompatActivity(), BleCallbacks { if (!manager.hasPermissions()) { if (!triedPermissions) { triedPermissions = true - ActivityCompat.requestPermissions(this, manager.getRequiredPermissions(), 1) + permissionLauncher.launch(manager.getRequiredPermissions()) } return } @@ -150,19 +155,6 @@ class MainActivity : AppCompatActivity(), BleCallbacks { manager.stopScan() } - @Deprecated("Deprecated in Java") - override fun onRequestPermissionsResult(requestCode: Int, permissions: Array, grantResults: IntArray) { - super.onRequestPermissionsResult(requestCode, permissions, grantResults) - if (requestCode == 1) { - if (grantResults.all { it == PackageManager.PERMISSION_GRANTED }) { - manager.startScan() - } else { - triedPermissions = false - onStatusChanged(BleStatus.PermissionsDenied) - } - } - } - private fun setupRelayButton() { relay1Button.setOnClickListener(null) loadButtonLabel(relay1Button, PrefsHelper.PREF_LABEL1, R.string.button_hold) @@ -230,7 +222,7 @@ class MainActivity : AppCompatActivity(), BleCallbacks { if (isFinishing || isDestroyed || !lifecycle.currentState.isAtLeast(androidx.lifecycle.Lifecycle.State.RESUMED)) return@runOnUiThread if (espVersion.isNotEmpty() && espVersion != appVersion) { Toast.makeText(this, getString(R.string.ota_update_available), Toast.LENGTH_SHORT).show() - startActivityForResult(Intent(this, SettingsActivity::class.java), REQUEST_SETTINGS) + settingsLauncher.launch(Intent(this, SettingsActivity::class.java)) } } } From 30fc7532ae03587980e881d367fa9f266cbcdd14 Mon Sep 17 00:00:00 2001 From: eddy Date: Tue, 4 Aug 2026 18:59:14 +0200 Subject: [PATCH 04/10] docs: add design spec for issue #16 auto-detect port fix --- .../specs/2026-08-04-auto-detect-port-fix.md | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-04-auto-detect-port-fix.md diff --git a/docs/superpowers/specs/2026-08-04-auto-detect-port-fix.md b/docs/superpowers/specs/2026-08-04-auto-detect-port-fix.md new file mode 100644 index 0000000..fb05d3b --- /dev/null +++ b/docs/superpowers/specs/2026-08-04-auto-detect-port-fix.md @@ -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/... +``` From 2030b42ba27243e0dfa9f73efc18c74dbae1ec82 Mon Sep 17 00:00:00 2001 From: eddy Date: Tue, 4 Aug 2026 19:01:57 +0200 Subject: [PATCH 05/10] docs: add implementation plan for issue #16 --- .../plans/2026-08-04-auto-detect-port-fix.md | 181 ++++++++++++++++++ 1 file changed, 181 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-04-auto-detect-port-fix.md diff --git a/docs/superpowers/plans/2026-08-04-auto-detect-port-fix.md b/docs/superpowers/plans/2026-08-04-auto-detect-port-fix.md new file mode 100644 index 0000000..dfa14fa --- /dev/null +++ b/docs/superpowers/plans/2026-08-04-auto-detect-port-fix.md @@ -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) From 99fd6b21d445702abaab55faed884d666c8288d6 Mon Sep 17 00:00:00 2001 From: eddy Date: Tue, 4 Aug 2026 19:02:51 +0200 Subject: [PATCH 06/10] fix(script): filter ID response in query_id() --- tools/pio_upload_port.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/pio_upload_port.py b/tools/pio_upload_port.py index 0e94c81..43c9e25 100644 --- a/tools/pio_upload_port.py +++ b/tools/pio_upload_port.py @@ -52,7 +52,7 @@ def query_id(port, timeout=5): deadline = time.time() + timeout while time.time() < deadline: line = ser.readline().decode("utf-8", errors="ignore").strip() - if line: + if "SylDa_Server" in line or "SylDa_Client" in line: ser.close() return line ser.close() From ae5028503fd81ba4f87b6a194175113d211cc8ea Mon Sep 17 00:00:00 2001 From: eddy Date: Tue, 4 Aug 2026 19:03:21 +0200 Subject: [PATCH 07/10] fix(server): add serial mute during ID? response --- src/main.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index 5b6543e..332ff60 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -16,6 +16,18 @@ 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("*", CONFIG_LOG_DEFAULT_LEVEL); +} + #define RELAY1_PIN 1 #define RELAY2_PIN 2 #define TOUCH1_PIN 8 @@ -589,7 +601,11 @@ void loop() { Serial.println("NO_KEY"); } } else if (serialBuf == "ID?") { + Serial.flush(); + muteSerial(); Serial.println("SylDa_Server"); + delay(50); + unmuteSerial(); } else if (serialBuf == "VERSION?") { Serial.flush(); Serial.print("BUILD_NR:"); From 3c6b378b9b20f271e00806ebf0efa6c65c5c5876 Mon Sep 17 00:00:00 2001 From: eddy Date: Tue, 4 Aug 2026 19:03:54 +0200 Subject: [PATCH 08/10] fix(client): add serial mute during ID? response --- src/client.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/client.cpp b/src/client.cpp index 4a85039..3d4298c 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -13,6 +13,18 @@ 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("*", CONFIG_LOG_DEFAULT_LEVEL); +} + #define TOUCH1_PIN 8 #define TOUCH2_PIN 6 #define BUTTON_PIN 41 @@ -423,7 +435,11 @@ void loop() { Serial.println("NO_KEY"); } } else if (serialBuf == "ID?") { + Serial.flush(); + muteSerial(); Serial.println("SylDa_Client"); + delay(50); + unmuteSerial(); } else if (serialBuf == "VERSION?") { Serial.print("BUILD_NR:"); Serial.println(FIRMWARE_VERSION_CLIENT); From 7b59d36bd7715bf89a17618ad64a9eb02e242c3b Mon Sep 17 00:00:00 2001 From: eddy Date: Tue, 4 Aug 2026 19:04:37 +0200 Subject: [PATCH 09/10] fix: cast CONFIG_LOG_DEFAULT_LEVEL to esp_log_level_t --- src/client.cpp | 2 +- src/main.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 3d4298c..15746fc 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -22,7 +22,7 @@ static void muteSerial() { static void unmuteSerial() { serialMuted = false; - esp_log_level_set("*", CONFIG_LOG_DEFAULT_LEVEL); + esp_log_level_set("*", (esp_log_level_t)CONFIG_LOG_DEFAULT_LEVEL); } #define TOUCH1_PIN 8 diff --git a/src/main.cpp b/src/main.cpp index 332ff60..f795bb5 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -25,7 +25,7 @@ static void muteSerial() { static void unmuteSerial() { serialMuted = false; - esp_log_level_set("*", CONFIG_LOG_DEFAULT_LEVEL); + esp_log_level_set("*", (esp_log_level_t)CONFIG_LOG_DEFAULT_LEVEL); } #define RELAY1_PIN 1 From e0b2750c2e8b308c0ed4c187a2babb60a6349156 Mon Sep 17 00:00:00 2001 From: eddy Date: Tue, 4 Aug 2026 20:56:52 +0200 Subject: [PATCH 10/10] fix: critical security issues from code review - Thread safety: Add @Synchronized methods for ECDH buffer access - Crypto: Use X509EncodedKeySpec instead of hard-coded EC curve params - Sensitive data: Make aes_key/shared_secret private in CryptoECDH - Credential storage: Migrate PSK/UUID to EncryptedSharedPreferences --- .../nl/eddydevink/sylda/BleRelayManager.kt | 66 +++++++++++-------- .../java/nl/eddydevink/sylda/PrefsHelper.kt | 52 +++++++++++++++ .../nl/eddydevink/sylda/SettingsActivity.kt | 21 ++---- src/client.cpp | 2 +- src/ecdh_helper.h | 8 ++- src/main.cpp | 2 +- 6 files changed, 105 insertions(+), 46 deletions(-) diff --git a/android/app/src/main/java/nl/eddydevink/sylda/BleRelayManager.kt b/android/app/src/main/java/nl/eddydevink/sylda/BleRelayManager.kt index f74686d..7758380 100644 --- a/android/app/src/main/java/nl/eddydevink/sylda/BleRelayManager.kt +++ b/android/app/src/main/java/nl/eddydevink/sylda/BleRelayManager.kt @@ -22,8 +22,10 @@ import androidx.core.app.ActivityCompat import java.nio.ByteBuffer import java.nio.ByteOrder import java.security.KeyPairGenerator +import java.security.KeyFactory import java.security.SecureRandom import java.security.spec.ECGenParameterSpec +import java.security.spec.X509EncodedKeySpec import java.security.interfaces.ECPublicKey import java.util.UUID import javax.crypto.Cipher @@ -61,6 +63,25 @@ class BleRelayManager( private const val ECDH_PKT_SIZE = 97 private const val BLE_MTU = 512 private const val ECDH_TIMEOUT_MS = 5000L + + private val SECP256R1_SPKI_HEADER = byteArrayOf( + 0x30.toByte(), 0x59.toByte(), 0x30.toByte(), 0x13.toByte(), + 0x06.toByte(), 0x07.toByte(), 0x2A.toByte(), 0x86.toByte(), + 0x48.toByte(), 0xCE.toByte(), 0x3D.toByte(), 0x02.toByte(), + 0x01.toByte(), 0x06.toByte(), 0x08.toByte(), 0x2A.toByte(), + 0x86.toByte(), 0x48.toByte(), 0xCE.toByte(), 0x3D.toByte(), + 0x03.toByte(), 0x01.toByte(), 0x07.toByte(), 0x03.toByte(), + 0x42.toByte(), 0x00.toByte() + ) + + private fun decodeEcPublicKey(uncompressedKey: ByteArray): ECPublicKey { + require(uncompressedKey.size == ECDH_PUBKEY_LEN && uncompressedKey[0] == 0x04.toByte()) { + "Invalid uncompressed EC key format" + } + val spkiEncoded = SECP256R1_SPKI_HEADER + uncompressedKey + return KeyFactory.getInstance("EC") + .generatePublic(X509EncodedKeySpec(spkiEncoded)) as ECPublicKey + } } private var bluetoothGatt: BluetoothGatt? = null @@ -84,6 +105,16 @@ class BleRelayManager( private var ecdhNotifyBuf: ByteArray = ByteArray(0) private var ecdhCccdPending = false + @Synchronized + private fun setEcdhNotifyBuf(value: ByteArray) { + ecdhNotifyBuf = value.copyOf() + } + + @Synchronized + private fun getEcdhNotifyBuf(): ByteArray { + return ecdhNotifyBuf.copyOf() + } + interface OtaNotifyCallback { fun onOtaNotify(status: Int, seq: Int) } @@ -404,7 +435,7 @@ class BleRelayManager( Log.d(TAG, "OTA notify: status=$statusName seq=$seqVal raw=${value.joinToString("") { "%02X".format(it) }}") handler.post { otaNotifyCallback?.onOtaNotify(statusVal, seqVal) } } else if (characteristic.uuid == ECDH_UUID) { - ecdhNotifyBuf = value.copyOf() + setEcdhNotifyBuf(value) ecdhDeferred?.complete(true) Log.d(TAG, "ECDH notify received (${value.size} bytes)") } @@ -506,7 +537,7 @@ class BleRelayManager( return@Thread } - val resp = ecdhNotifyBuf + val resp = getEcdhNotifyBuf() if (resp.size != ECDH_PKT_SIZE) { Log.w(TAG, "ECDH: bad response size ${resp.size}") handler.post { @@ -530,28 +561,7 @@ class BleRelayManager( val ka = KeyAgreement.getInstance("ECDH") ka.init(keyPair.private) - val spec = java.security.spec.ECPublicKeySpec( - java.security.spec.ECPoint( - java.math.BigInteger(1, serverPub.copyOfRange(1, 33)), - java.math.BigInteger(1, serverPub.copyOfRange(33, 65)) - ), - java.security.spec.ECParameterSpec( - java.security.spec.EllipticCurve( - java.security.spec.ECFieldFp( - java.math.BigInteger("FFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFF", 16) - ), - java.math.BigInteger("FFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFC", 16), - java.math.BigInteger("5AC635D8AA3A93E7B3EBBD55769886BC651D06B0CC53B0F63BCE3C3E27D2604B", 16) - ), - java.security.spec.ECPoint( - java.math.BigInteger("6B17D1F2E12C4247F8BCE6E563A440F277037D812DEB33A0F4A13945D898C296", 16), - java.math.BigInteger("4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5", 16) - ), - java.math.BigInteger("FFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551", 16), - 1 - ) - ) - val serverKey = java.security.KeyFactory.getInstance("EC").generatePublic(spec) + val serverKey = decodeEcPublicKey(serverPub) ka.doPhase(serverKey, true) val shared = ka.generateSecret() @@ -606,13 +616,13 @@ class BleRelayManager( } private fun loadState() { - val prefs = PrefsHelper.getPrefs(context) - val hexKey = prefs.getString(PrefsHelper.PREF_KEY, null) + PrefsHelper.migrateCredentialsIfNeeded(context) + val hexKey = PrefsHelper.getPsk(context) psk = hexKey?.let { hexStringToBytes(it) } val hexUuid = PrefsHelper.getServerUuid(context) - ?: prefs.getString(PrefsHelper.PREF_UUID, null) + ?: PrefsHelper.getDeviceCredentialUuid(context) deviceUuid = hexUuid?.let { hexStringToBytes(it) } - seq = prefs.getInt(PrefsHelper.PREF_SEQ, 1).coerceAtMost(PrefsHelper.SEQ_MAX) + seq = PrefsHelper.getPrefs(context).getInt(PrefsHelper.PREF_SEQ, 1).coerceAtMost(PrefsHelper.SEQ_MAX) Log.d(TAG, "PSK loaded: ${psk?.size ?: "none"} bytes, UUID: ${deviceUuid?.size ?: "none"} bytes, seq=$seq") } diff --git a/android/app/src/main/java/nl/eddydevink/sylda/PrefsHelper.kt b/android/app/src/main/java/nl/eddydevink/sylda/PrefsHelper.kt index fdc6c45..f67d6c7 100644 --- a/android/app/src/main/java/nl/eddydevink/sylda/PrefsHelper.kt +++ b/android/app/src/main/java/nl/eddydevink/sylda/PrefsHelper.kt @@ -3,12 +3,16 @@ package nl.eddydevink.sylda import android.content.Context import android.content.SharedPreferences +import android.util.Log import androidx.appcompat.app.AppCompatDelegate +import androidx.security.crypto.EncryptedSharedPreferences +import androidx.security.crypto.MasterKey object PrefsHelper { private const val TAG = "PrefsHelper" const val PREFS_NAME = "relay_prefs" + private const val ENCRYPTED_PREFS_NAME = "encrypted_prefs" const val PREF_KEY = "psk" const val PREF_UUID = "device_uuid" const val PREF_SEQ = "seq_counter" @@ -20,6 +24,49 @@ object PrefsHelper { const val KEY_CLIENT_UUID = "clientUuid" const val SEQ_MAX = 1_000_000 + private var masterKey: MasterKey? = null + + private fun getMasterKey(context: Context): MasterKey { + return masterKey ?: run { + MasterKey.Builder(context) + .setKeyScheme(MasterKey.KeyScheme.AES256_GCM) + .build().also { masterKey = it } + } + } + + fun getEncryptedPrefs(context: Context): SharedPreferences { + return EncryptedSharedPreferences.create( + context, + ENCRYPTED_PREFS_NAME, + getMasterKey(context), + EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, + EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM + ) + } + + fun migrateCredentialsIfNeeded(context: Context) { + val plainPrefs = getPrefs(context) + val encPrefs = getEncryptedPrefs(context) + + val legacyKey = plainPrefs.getString(PREF_KEY, null) + val legacyUuid = plainPrefs.getString(PREF_UUID, null) + + if (legacyKey != null && !encPrefs.contains(PREF_KEY)) { + Log.i(TAG, "Migrating credentials to encrypted storage") + encPrefs.edit().apply { + putString(PREF_KEY, legacyKey) + if (legacyUuid != null) putString(PREF_UUID, legacyUuid) + apply() + } + plainPrefs.edit().apply { + remove(PREF_KEY) + remove(PREF_UUID) + apply() + } + Log.i(TAG, "Credentials migration complete") + } + } + fun getDeviceMode(context: Context): String = getPrefs(context).getString(KEY_DEVICE_MODE, "server") ?: "server" fun setDeviceMode(context: Context, mode: String) = getPrefs(context).edit().putString(KEY_DEVICE_MODE, mode).apply() fun getTransportMode(context: Context): String = getPrefs(context).getString("transportMode", "ble") ?: "ble" @@ -29,6 +76,11 @@ object PrefsHelper { fun getClientUuid(context: Context): String? = getPrefs(context).getString(KEY_CLIENT_UUID, null) fun setClientUuid(context: Context, uuid: String) = getPrefs(context).edit().putString(KEY_CLIENT_UUID, uuid).apply() + fun getPsk(context: Context): String? = getEncryptedPrefs(context).getString(PREF_KEY, null) + fun setPsk(context: Context, psk: String) = getEncryptedPrefs(context).edit().putString(PREF_KEY, psk).apply() + fun getDeviceCredentialUuid(context: Context): String? = getEncryptedPrefs(context).getString(PREF_UUID, null) + fun setDeviceCredentialUuid(context: Context, uuid: String) = getEncryptedPrefs(context).edit().putString(PREF_UUID, uuid).apply() + fun applyTheme(context: Context) { val prefs = getPrefs(context) val mode = when (prefs.getString(PREF_THEME, "system")) { diff --git a/android/app/src/main/java/nl/eddydevink/sylda/SettingsActivity.kt b/android/app/src/main/java/nl/eddydevink/sylda/SettingsActivity.kt index 91ccaec..6ede172 100644 --- a/android/app/src/main/java/nl/eddydevink/sylda/SettingsActivity.kt +++ b/android/app/src/main/java/nl/eddydevink/sylda/SettingsActivity.kt @@ -372,9 +372,8 @@ class SettingsActivity : AppCompatActivity() { private fun doKeyExchange(usbManager: UsbManager, device: UsbDevice) { showProgressDialog(getString(R.string.title_key_exchange), getString(R.string.msg_reading_key), STYLE_SPINNER) - val prefs = PrefsHelper.getPrefs(this) - val appUuid = prefs.getString(PrefsHelper.PREF_UUID, null) - val appKey = prefs.getString(PrefsHelper.PREF_KEY, null) + val appUuid = PrefsHelper.getDeviceCredentialUuid(this) + val appKey = PrefsHelper.getPsk(this) lifecycleScope.launch(Dispatchers.IO) { val result = readKeyFromEsp(usbManager, device, appUuid, appKey) @@ -384,10 +383,8 @@ class SettingsActivity : AppCompatActivity() { progressDialog = null when (result) { is KeyResult.Success -> { - prefs.edit() - .putString(PrefsHelper.PREF_UUID, result.deviceUuid) - .putString(PrefsHelper.PREF_KEY, result.hexKey) - .apply() + PrefsHelper.setDeviceCredentialUuid(this@SettingsActivity, result.deviceUuid) + PrefsHelper.setPsk(this@SettingsActivity, result.hexKey) if (PrefsHelper.getDeviceMode(this@SettingsActivity) == "server") { PrefsHelper.setServerUuid(this@SettingsActivity, result.deviceUuid) } else { @@ -838,8 +835,7 @@ class SettingsActivity : AppCompatActivity() { private fun showConflictDialog(usbManager: UsbManager, device: UsbDevice, espUuid: String, espKey: String, appKey: String) { - val prefs = PrefsHelper.getPrefs(this) - val appUuid = prefs.getString(PrefsHelper.PREF_UUID, null) + val appUuid = PrefsHelper.getDeviceCredentialUuid(this) val newAppKey = appKey.ifEmpty { generateRandomHexKey() } val newAppUuid = appUuid ?: generateRandomHex(UUID_BYTE_COUNT) @@ -946,11 +942,8 @@ class SettingsActivity : AppCompatActivity() { } } - val prefs = PrefsHelper.getPrefs(this) - prefs.edit() - .putString(PrefsHelper.PREF_UUID, newUuid) - .putString(PrefsHelper.PREF_KEY, appKey) - .apply() + PrefsHelper.setDeviceCredentialUuid(this, newUuid) + PrefsHelper.setPsk(this, appKey) if (PrefsHelper.getDeviceMode(this) == "server") { PrefsHelper.setServerUuid(this, newUuid) } else { diff --git a/src/client.cpp b/src/client.cpp index 15746fc..f49f414 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -260,7 +260,7 @@ static bool performEcdhHandshake(NimBLERemoteService *pSvc) { return false; } - memcpy(aesKey, ecdh.aes_key, AES_KEY_LEN); + memcpy(aesKey, ecdh.getAesKey(), AES_KEY_LEN); ecdh.free(); ecdhReady = true; ESP_LOGI(TAG, "ECDH: handshake complete"); diff --git a/src/ecdh_helper.h b/src/ecdh_helper.h index b9b6d4f..d8ee9c1 100644 --- a/src/ecdh_helper.h +++ b/src/ecdh_helper.h @@ -21,6 +21,9 @@ private: mbedtls_ctr_drbg_context ctr_drbg; bool initialized = false; + uint8_t shared_secret[32]; + uint8_t aes_key[AES_KEY_LEN]; + CryptoECDH(const CryptoECDH&) = delete; CryptoECDH& operator=(const CryptoECDH&) = delete; @@ -29,8 +32,9 @@ public: uint8_t public_key[ECDH_PUBKEY_LEN]; size_t pub_len = 0; - uint8_t shared_secret[32]; - uint8_t aes_key[AES_KEY_LEN]; + + const uint8_t* getAesKey() const { return aes_key; } + size_t getAesKeyLen() const { return AES_KEY_LEN; } bool init() { mbedtls_ecdh_init(&ecdh); diff --git a/src/main.cpp b/src/main.cpp index f795bb5..c330e9f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -406,7 +406,7 @@ class EcdhCharacteristicCallbacks : public NimBLECharacteristicCallbacks { return; } - memcpy(clients[idx].aesKey, clients[idx].ecdh.aes_key, AES_KEY_LEN); + memcpy(clients[idx].aesKey, clients[idx].ecdh.getAesKey(), AES_KEY_LEN); uint8_t resp[ECDH_PKT_SIZE]; memcpy(resp, clients[idx].ecdh.public_key, ECDH_PUBKEY_LEN);