From 7829fa7662873098f4f9ffa25286ce9b33e5329c Mon Sep 17 00:00:00 2001 From: David Lee <247393336+davelee98@users.noreply.github.com> Date: Fri, 17 Jul 2026 21:52:08 -0400 Subject: [PATCH 1/2] fix(ble): defer NimBLE onConnect/onDisconnect work to loop() (race audit #1-3,6) The Bluedroid->NimBLE migration left heavyweight, state-mutating work running inline inside the ESP32 server callbacks on the NimBLE host task, racing the Arduino loop() task. Make the callbacks flag-only and service the flags from loop(), mirroring the existing bleRestartAdvertisingPending pattern. - onDisconnect: EPD session teardown (bbepSleep/delay/SPI.end + rail cut), partial + pipe cleanup, and resetPipeWriteState() ran on the host task while loop() streamed SPI via bbepWriteData and processed pipe frames. The only guard (epdRefreshInProgress) covered the refresh phase, not streaming. Now sets bleDisconnectCleanupPending; serviced by serviceBleDisconnectCleanup() in loop() (preserving the epdRefreshInProgress deferral), run before the advertising restart. (#1 critical, #2 high, #6 medium) - onConnect: updatemsdata() polled I2C and mutated the shared advertisement vector that loop() also drives on its 60s cadence -> heap corruption. Now sets msdUpdatePending; loop() calls updatemsdata(). (#3 high) - updatemsdata(): drop the connected-branch that rebuilt *advertisementData but never pushed it via setAdvertisementData() -- dead work, no behavior change. All ESP32-guarded; nRF/Bluefruit path untouched. Builds clean on esp32-s3-N16R8, esp32-c3-N16, nrf52840custom. Co-Authored-By: Claude Opus 4.8 --- src/ble_init.cpp | 2 ++ src/ble_init.h | 4 ++++ src/display_service.cpp | 40 ++++++++++++++++++--------------------- src/esp32_ble_callbacks.h | 25 ++++++++++-------------- src/main.cpp | 31 +++++++++++++++++++++++++++++- 5 files changed, 64 insertions(+), 38 deletions(-) diff --git a/src/ble_init.cpp b/src/ble_init.cpp index f7cb2ce..d612852 100644 --- a/src/ble_init.cpp +++ b/src/ble_init.cpp @@ -208,6 +208,8 @@ void ble_init() { #ifdef TARGET_ESP32 volatile bool bleRestartAdvertisingPending = false; volatile bool esp32BleNotifySubscribed = false; +volatile bool bleDisconnectCleanupPending = false; +volatile bool msdUpdatePending = false; void esp32_ble_clear_handles(void) { pServer = nullptr; diff --git a/src/ble_init.h b/src/ble_init.h index 766683b..fc85e75 100644 --- a/src/ble_init.h +++ b/src/ble_init.h @@ -36,6 +36,10 @@ void esp32_ble_clear_handles(void); bool esp32_ble_notify_enabled(void); extern volatile bool bleRestartAdvertisingPending; extern volatile bool esp32BleNotifySubscribed; +// Set flag-only by MyBLEServerCallbacks (NimBLE host task); serviced from loop() +// so the heavyweight teardown / I2C+advertisement work never races loop(). +extern volatile bool bleDisconnectCleanupPending; +extern volatile bool msdUpdatePending; #endif #endif diff --git a/src/display_service.cpp b/src/display_service.cpp index e54d8b8..609a1ff 100644 --- a/src/display_service.cpp +++ b/src/display_service.cpp @@ -1716,28 +1716,24 @@ void updatemsdata(){ memcpy(prev_msd_payload, msd_payload, 16); advertisementData->setManufacturerData(msd_payload, 16); BLEAdvertising *pAdvertising = (pServer != nullptr) ? pServer->getAdvertising() : BLEDevice::getAdvertising(); - if (pAdvertising != nullptr) { - if (pServer != nullptr && pServer->getConnectedCount() > 0) { - *advertisementData = BLEAdvertisementData(); - advertisementData->setName(("OD" + getChipIdHex()).c_str()); - advertisementData->setFlags(0x06); - advertisementData->setManufacturerData(msd_payload, 16); - } else { - pAdvertising->stop(); - BLEAdvertisementData freshAdvertisementData; - static String savedDeviceName = ""; - if (savedDeviceName.length() == 0) savedDeviceName = "OD" + getChipIdHex(); - freshAdvertisementData.setName(savedDeviceName.c_str()); - freshAdvertisementData.setFlags(0x06); - freshAdvertisementData.setManufacturerData(msd_payload, 16); - *advertisementData = freshAdvertisementData; - // setAdvertisementData() must be the last data call before start(): - // enableScanResponse()/setPreferredParams() reset NimBLE's custom-data - // flag and would make start() drop this manufacturer-data payload. - pAdvertising->setAdvertisementData(freshAdvertisementData); - delay(50); - pAdvertising->start(); - } + // Only rebuild+restart advertising while disconnected. The former connected + // branch rebuilt *advertisementData but never pushed it via + // setAdvertisementData(), so it was dead work — dropped. + if (pAdvertising != nullptr && !(pServer != nullptr && pServer->getConnectedCount() > 0)) { + pAdvertising->stop(); + BLEAdvertisementData freshAdvertisementData; + static String savedDeviceName = ""; + if (savedDeviceName.length() == 0) savedDeviceName = "OD" + getChipIdHex(); + freshAdvertisementData.setName(savedDeviceName.c_str()); + freshAdvertisementData.setFlags(0x06); + freshAdvertisementData.setManufacturerData(msd_payload, 16); + *advertisementData = freshAdvertisementData; + // setAdvertisementData() must be the last data call before start(): + // enableScanResponse()/setPreferredParams() reset NimBLE's custom-data + // flag and would make start() drop this manufacturer-data payload. + pAdvertising->setAdvertisementData(freshAdvertisementData); + delay(50); + pAdvertising->start(); } } opendisplay_mdns_update_msd_txt(); diff --git a/src/esp32_ble_callbacks.h b/src/esp32_ble_callbacks.h index 95b2d10..194d59c 100644 --- a/src/esp32_ble_callbacks.h +++ b/src/esp32_ble_callbacks.h @@ -48,7 +48,10 @@ class MyBLEServerCallbacks : public BLEServerCallbacks { writeSerial("=== BLE CLIENT CONNECTED (ESP32) ==="); rebootFlag = 0; esp32BleNotifySubscribed = false; - updatemsdata(); + // Flag-only: updatemsdata() polls I2C and mutates the shared advertisement + // vector, which loop() also drives (60 s cadence) — running it inline on the + // NimBLE host task would corrupt the heap. Serviced from loop() instead. + msdUpdatePending = true; } void onDisconnect(NimBLEServer* pServer, NimBLEConnInfo& connInfo, int reason) override { (void)pServer; @@ -56,20 +59,12 @@ class MyBLEServerCallbacks : public BLEServerCallbacks { (void)reason; writeSerial("=== BLE CLIENT DISCONNECTED (ESP32) ==="); esp32BleNotifySubscribed = false; - if (epdRefreshInProgress) { - writeSerial("EPD refresh in progress — deferring cleanup/advertising to main loop"); - } else { - // ACTIVE-only-teardown invariant: a WARM (post-successful-refresh) panel - // SURVIVES disconnect and keeps its keep-alive window, so the cleanups - // below no-op on power when WARM and only tear down a mid-transfer - // (PWR_ACTIVE) session. No logic change needed for keep-alive. - if (directWriteActive) cleanupDirectWriteState(true); - // Partial sessions (0x76 or pipe-partial) power the panel without - // setting directWriteActive; release it here instead of waiting on - // the 15-min partial watchdog. - cleanupPartialWriteOnDisconnect(); - } - resetPipeWriteState(); // clear any pipe transfer + reorder queue on disconnect + // Flag-only: the session teardown below (EPD force-off with SPI.end()/rail + // cut, partial + pipe cleanup) is heavyweight, state-mutating work that races + // loop()'s SPI streaming and pipe-frame processing on the Arduino task. Defer + // it — and the epdRefreshInProgress deferral — to loop() where it is + // single-task-safe (see serviceBleDisconnectCleanup in main.cpp). + bleDisconnectCleanupPending = true; bleRestartAdvertisingPending = true; } }; diff --git a/src/main.cpp b/src/main.cpp index 2945bf1..3ed740f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -265,6 +265,26 @@ static void flushResponseQueueToBle() { } } } + +// Services the deferred BLE-disconnect session teardown flagged by +// MyBLEServerCallbacks::onDisconnect. Runs on the loop() task so the heavyweight +// EPD force-off (bbepSleep/delay/SPI.end/rail cut) and partial/pipe cleanup never +// race SPI streaming or pipe-frame processing on the NimBLE host task. Deferred +// while a refresh is mid-flight — the same epdRefreshInProgress gate the callback +// used to apply inline; the flag stays set until a later pass clears it. +static void serviceBleDisconnectCleanup() { + if (!bleDisconnectCleanupPending || epdRefreshInProgress) return; + bleDisconnectCleanupPending = false; + // ACTIVE-only-teardown invariant: a WARM (post-successful-refresh) panel + // SURVIVES disconnect and keeps its keep-alive window, so the cleanups below + // no-op on power when WARM and only tear down a mid-transfer (PWR_ACTIVE) + // session. No logic change needed for keep-alive. + if (directWriteActive) cleanupDirectWriteState(true); + // Partial sessions (0x76 or pipe-partial) power the panel without setting + // directWriteActive; release it here instead of waiting on the 15-min watchdog. + cleanupPartialWriteOnDisconnect(); + resetPipeWriteState(); // clear any pipe transfer + reorder queue on disconnect +} #endif void loop() { @@ -285,7 +305,8 @@ void loop() { return; } // A connect+drop entirely inside one poll gap leaves the radio dark for the - // rest of the window; the flag is otherwise only serviced past this return. + // rest of the window; the flags are otherwise only serviced past this return. + serviceBleDisconnectCleanup(); // tear down before re-advertising if (bleRestartAdvertisingPending) { esp32_restart_ble_advertising(); } @@ -340,6 +361,14 @@ void loop() { } } flushResponseQueueToBle(); + // Service the flag-only BLE callbacks on this (single) task. Cleanup runs + // before the advertising restart so a disconnected session is fully torn down + // before the radio re-arms. + serviceBleDisconnectCleanup(); + if (msdUpdatePending) { + msdUpdatePending = false; + updatemsdata(); + } if (bleRestartAdvertisingPending) { esp32_restart_ble_advertising(); } From b83d824eafa86b775c8555b1fe6b7b66204192ca Mon Sep 17 00:00:00 2001 From: David Lee <247393336+davelee98@users.noreply.github.com> Date: Fri, 17 Jul 2026 23:12:11 -0400 Subject: [PATCH 2/2] Replace protocol magic numbers with canonical named constants Follow-up to the header vendoring (3f09f6d): sweep the remaining hand-coded protocol literals across the BLE command handlers and use the constants from the vendored opendisplay_protocol.h. Value-for-value, no wire-behavior change; all CI envs build clean. - encryption.cpp: auth-status frames -> RESP_ACK/RESP_AUTHENTICATE + AUTH_STATUS_*; CCM envelope sizes -> BLE_CMD_HEADER_SIZE/ ENCRYPTION_NONCE_SIZE/ENCRYPTION_TAG_SIZE (handshake nonces left as-is). - communication.cpp: handler response codes -> RESP_* (incl. RESP_AUTH_REQUIRED); config limits -> CONFIG_CHUNK_SIZE*/ MAX_CONFIG_CHUNKS/MAX_RESPONSE_DATA_SIZE; envelope sizes. - device_control.cpp / buzzer_control.cpp: LED, deep-sleep, and buzzer ack/nack frames -> RESP_*. - display_service.cpp: direct-write/refresh frames -> RESP_DIRECT_WRITE_*; pipe-start errors -> OD_ERR_PIPE_START_*; deleted the local ERR_* shadow copies of OD_ERR_PARTIAL_* in favor of the canonical names. Pipe/partial-write opcode-echo bytes (0x76/0x80/0x81/0x82), the pipe data-phase error codes, and the START response-flags bit stay raw with TODO(protocol) markers: the canonical header defines no RESP_* mirror or data-phase error namespace for them yet. Co-Authored-By: Claude Opus 4.8 --- src/buzzer_control.cpp | 18 +++--- src/communication.cpp | 82 ++++++++++++------------- src/device_control.cpp | 18 +++--- src/display_service.cpp | 129 +++++++++++++++++++++++----------------- src/encryption.cpp | 32 +++++----- 5 files changed, 151 insertions(+), 128 deletions(-) diff --git a/src/buzzer_control.cpp b/src/buzzer_control.cpp index 55da11c..a8ba8c0 100644 --- a/src/buzzer_control.cpp +++ b/src/buzzer_control.cpp @@ -268,19 +268,19 @@ void initPassiveBuzzers(void) { void handleBuzzerActivate(uint8_t* data, uint16_t len) { if (len < 3) { - uint8_t err[] = {0xFF, 0x77, 0x01, 0x00}; + uint8_t err[] = {RESP_NACK, RESP_BUZZER_ACK, 0x01, 0x00}; sendResponse(err, sizeof(err)); return; } uint8_t inst = data[0]; if (inst >= globalConfig.passive_buzzer_count) { - uint8_t err[] = {0xFF, 0x77, 0x02, 0x00}; + uint8_t err[] = {RESP_NACK, RESP_BUZZER_ACK, 0x02, 0x00}; sendResponse(err, sizeof(err)); return; } PassiveBuzzerConfig* b = &globalConfig.passive_buzzers[inst]; if (b->drive_pin == 0xFF) { - uint8_t err[] = {0xFF, 0x77, 0x03, 0x00}; + uint8_t err[] = {RESP_NACK, RESP_BUZZER_ACK, 0x03, 0x00}; sendResponse(err, sizeof(err)); return; } @@ -291,7 +291,7 @@ void handleBuzzerActivate(uint8_t* data, uint16_t len) { } uint8_t pattern_count = data[2]; if (pattern_count == 0) { - uint8_t err[] = {0xFF, 0x77, 0x04, 0x00}; + uint8_t err[] = {RESP_NACK, RESP_BUZZER_ACK, 0x04, 0x00}; sendResponse(err, sizeof(err)); return; } @@ -299,26 +299,26 @@ void handleBuzzerActivate(uint8_t* data, uint16_t len) { uint16_t scan = 3; for (uint8_t pi = 0; pi < pattern_count; pi++) { if (scan >= len) { - uint8_t err[] = {0xFF, 0x77, 0x05, 0x00}; + uint8_t err[] = {RESP_NACK, RESP_BUZZER_ACK, 0x05, 0x00}; sendResponse(err, sizeof(err)); return; } uint8_t nsteps = data[scan++]; uint32_t need = (uint32_t)nsteps * 2u; if (scan + need > len) { - uint8_t err[] = {0xFF, 0x77, 0x05, 0x00}; + uint8_t err[] = {RESP_NACK, RESP_BUZZER_ACK, 0x05, 0x00}; sendResponse(err, sizeof(err)); return; } scan = (uint16_t)(scan + need); } if (scan != len) { - uint8_t err[] = {0xFF, 0x77, 0x06, 0x00}; + uint8_t err[] = {RESP_NACK, RESP_BUZZER_ACK, 0x06, 0x00}; sendResponse(err, sizeof(err)); return; } if (len > sizeof(s_buzzer.melody)) { - uint8_t err[] = {0xFF, 0x77, 0x05, 0x00}; + uint8_t err[] = {RESP_NACK, RESP_BUZZER_ACK, 0x05, 0x00}; sendResponse(err, sizeof(err)); return; } @@ -344,7 +344,7 @@ void handleBuzzerActivate(uint8_t* data, uint16_t len) { // Start the first step, then ACK "accepted & started" immediately. buzzer_run(); - uint8_t ok[] = {0x00, 0x77, 0x00, 0x00}; + uint8_t ok[] = {RESP_ACK, RESP_BUZZER_ACK, 0x00, 0x00}; sendResponse(ok, sizeof(ok)); } diff --git a/src/communication.cpp b/src/communication.cpp index c2d1b3a..f533900 100644 --- a/src/communication.cpp +++ b/src/communication.cpp @@ -184,9 +184,9 @@ void sendResponse(uint8_t* response, uint16_t len) { uint8_t status = (len >= 3 && !pipeDataAck) ? response[2] : 0x00; // Encrypt all authenticated responses except auth/version handshakes and FE/FF status. // Direct-write / partial-write / LED acks must be encrypted too; LAN/BLE clients decrypt every response. - if (command != CMD_AUTHENTICATE && command != CMD_FIRMWARE_VERSION && status != 0xFE && status != 0xFF) { - uint8_t nonce[16]; - uint8_t auth_tag[12]; + if (command != CMD_AUTHENTICATE && command != CMD_FIRMWARE_VERSION && status != RESP_AUTH_REQUIRED && status != RESP_NACK) { + uint8_t nonce[ENCRYPTION_NONCE_SIZE]; + uint8_t auth_tag[ENCRYPTION_TAG_SIZE]; uint16_t encrypted_len = 0; if (encryptResponse(response, len, encrypted_response, &encrypted_len, nonce, auth_tag)) { if (!quietAck) { @@ -198,7 +198,7 @@ void sendResponse(uint8_t* response, uint16_t len) { len = encrypted_len; } else { writeSerial("WARNING: Failed to encrypt response, sending unencrypted error response", true); - errorResponse[0] = 0xFF; + errorResponse[0] = RESP_NACK; errorResponse[1] = (uint8_t)(command & 0xFF); errorResponse[2] = 0x00; response = errorResponse; @@ -259,8 +259,8 @@ void handleReadMSD() { writeSerial("=== READ MSD COMMAND (0x0044) ===", true); uint8_t response[2 + 16]; uint16_t responseLen = 0; - response[responseLen++] = 0x00; - response[responseLen++] = 0x44; + response[responseLen++] = RESP_ACK; + response[responseLen++] = RESP_MSD_READ; memcpy(&response[responseLen], msd_payload, sizeof(msd_payload)); responseLen += sizeof(msd_payload); sendResponse(response, responseLen); @@ -336,8 +336,8 @@ void handleFirmwareVersion() { if (shaLen > 40) shaLen = 40; uint8_t response[2 + 1 + 1 + 1 + 40]; uint16_t offset = 0; - response[offset++] = 0x00; - response[offset++] = 0x43; + response[offset++] = RESP_ACK; + response[offset++] = RESP_FIRMWARE_VERSION; response[offset++] = major; response[offset++] = minor; response[offset++] = shaLen; @@ -362,20 +362,20 @@ void handleReadConfig() { const uint16_t maxChunks = (MAX_CONFIG_SIZE + 93) / 94; while (remaining > 0 && chunkNumber < maxChunks) { uint16_t responseLen = 0; - configReadResponseBuffer[responseLen++] = 0x00; - configReadResponseBuffer[responseLen++] = 0x40; + configReadResponseBuffer[responseLen++] = RESP_ACK; + configReadResponseBuffer[responseLen++] = RESP_CONFIG_READ; configReadResponseBuffer[responseLen++] = chunkNumber & 0xFF; configReadResponseBuffer[responseLen++] = (chunkNumber >> 8) & 0xFF; if (chunkNumber == 0) { configReadResponseBuffer[responseLen++] = configLen & 0xFF; configReadResponseBuffer[responseLen++] = (configLen >> 8) & 0xFF; } - uint16_t maxDataSize = 100 - responseLen; + uint16_t maxDataSize = MAX_RESPONSE_DATA_SIZE - responseLen; uint16_t chunkSize = (remaining < maxDataSize) ? remaining : maxDataSize; if (chunkSize == 0) break; memcpy(configReadResponseBuffer + responseLen, configData + offset, chunkSize); responseLen += chunkSize; - if (responseLen > 100 || responseLen == 0) break; + if (responseLen > MAX_RESPONSE_DATA_SIZE || responseLen == 0) break; sendResponse(configReadResponseBuffer, responseLen); offset += chunkSize; remaining -= chunkSize; @@ -387,7 +387,7 @@ void handleReadConfig() { #endif } } else { - uint8_t errorResponse[] = {0xFF, 0x40, 0x00, 0x00}; + uint8_t errorResponse[] = {RESP_NACK, RESP_CONFIG_READ, 0x00, 0x00}; sendResponse(errorResponse, sizeof(errorResponse)); } } @@ -397,38 +397,38 @@ void handleWriteConfig(uint8_t* data, uint16_t len) { if (isEncryptionEnabled() && !isAuthenticated()) { bool rewriteAllowed = (securityConfig.flags & (1 << 0)) != 0; if (!rewriteAllowed) { - uint8_t response[] = {0x00, (uint8_t)(CMD_CONFIG_WRITE & 0xFF), 0xFE}; + uint8_t response[] = {RESP_ACK, (uint8_t)(CMD_CONFIG_WRITE & 0xFF), RESP_AUTH_REQUIRED}; sendResponseUnencrypted(response, sizeof(response)); return; } secureEraseConfig(); } - if (len > 200) { + if (len > CONFIG_CHUNK_SIZE) { chunkedWriteState.active = true; chunkedWriteState.receivedSize = 0; chunkedWriteState.expectedChunks = 0; chunkedWriteState.receivedChunks = 0; - if (len >= 202) { + if (len >= CONFIG_CHUNK_SIZE_WITH_PREFIX) { chunkedWriteState.totalSize = data[0] | (data[1] << 8); - chunkedWriteState.expectedChunks = (chunkedWriteState.totalSize + 200 - 1) / 200; - uint16_t chunkDataSize = ((len - 2) < 200) ? (len - 2) : 200; + chunkedWriteState.expectedChunks = (chunkedWriteState.totalSize + CONFIG_CHUNK_SIZE - 1) / CONFIG_CHUNK_SIZE; + uint16_t chunkDataSize = ((len - 2) < CONFIG_CHUNK_SIZE) ? (len - 2) : CONFIG_CHUNK_SIZE; memcpy(chunkedWriteState.buffer, data + 2, chunkDataSize); chunkedWriteState.receivedSize = chunkDataSize; chunkedWriteState.receivedChunks = 1; } else { - uint16_t chunkSize = (len < 200) ? len : 200; + uint16_t chunkSize = (len < CONFIG_CHUNK_SIZE) ? len : CONFIG_CHUNK_SIZE; chunkedWriteState.totalSize = len; chunkedWriteState.expectedChunks = 1; memcpy(chunkedWriteState.buffer, data, chunkSize); chunkedWriteState.receivedSize = chunkSize; chunkedWriteState.receivedChunks = 1; } - uint8_t ackResponse[] = {0x00, 0x41, 0x00, 0x00}; + uint8_t ackResponse[] = {RESP_ACK, RESP_CONFIG_WRITE, 0x00, 0x00}; sendResponse(ackResponse, sizeof(ackResponse)); return; } - uint8_t responseOk[] = {0x00, 0x41, 0x00, 0x00}; - uint8_t responseErr[] = {0xFF, 0x41, 0x00, 0x00}; + uint8_t responseOk[] = {RESP_ACK, RESP_CONFIG_WRITE, 0x00, 0x00}; + uint8_t responseErr[] = {RESP_NACK, RESP_CONFIG_WRITE, 0x00, 0x00}; bool ok = saveConfig(data, len); if (ok) { reloadConfigAfterSave(); @@ -438,8 +438,8 @@ void handleWriteConfig(uint8_t* data, uint16_t len) { void handleClearConfig(void) { writeSerial("=== CLEAR CONFIG COMMAND (0x0045) ==="); - uint8_t responseOk[] = {0x00, 0x45, 0x00, 0x00}; - uint8_t responseErr[] = {0xFF, 0x45, 0x00, 0x00}; + uint8_t responseOk[] = {RESP_ACK, RESP_CONFIG_CLEAR, 0x00, 0x00}; + uint8_t responseErr[] = {RESP_NACK, RESP_CONFIG_CLEAR, 0x00, 0x00}; if (!clearStoredConfig()) { sendResponse(responseErr, sizeof(responseErr)); @@ -452,7 +452,7 @@ void handleClearConfig(void) { void handleWriteConfigChunk(uint8_t* data, uint16_t len) { if (!chunkedWriteState.active) { - uint8_t errorResponse[] = {0xFF, 0x42, 0x00, 0x00}; + uint8_t errorResponse[] = {RESP_NACK, RESP_CONFIG_CHUNK, 0x00, 0x00}; sendResponse(errorResponse, sizeof(errorResponse)); return; } @@ -460,15 +460,15 @@ void handleWriteConfigChunk(uint8_t* data, uint16_t len) { bool rewriteAllowed = (securityConfig.flags & (1 << 0)) != 0; if (!rewriteAllowed) { chunkedWriteState.active = false; - uint8_t response[] = {0x00, (uint8_t)(CMD_CONFIG_CHUNK & 0xFF), 0xFE}; + uint8_t response[] = {RESP_ACK, (uint8_t)(CMD_CONFIG_CHUNK & 0xFF), RESP_AUTH_REQUIRED}; sendResponseUnencrypted(response, sizeof(response)); return; } secureEraseConfig(); } - if (len == 0 || len > 200 || chunkedWriteState.receivedSize + len > 4096 || chunkedWriteState.receivedChunks >= 20) { + if (len == 0 || len > CONFIG_CHUNK_SIZE || chunkedWriteState.receivedSize + len > 4096 || chunkedWriteState.receivedChunks >= MAX_CONFIG_CHUNKS) { chunkedWriteState.active = false; - uint8_t errorResponse[] = {0xFF, 0x42, 0x00, 0x00}; + uint8_t errorResponse[] = {RESP_NACK, RESP_CONFIG_CHUNK, 0x00, 0x00}; sendResponse(errorResponse, sizeof(errorResponse)); return; } @@ -476,8 +476,8 @@ void handleWriteConfigChunk(uint8_t* data, uint16_t len) { chunkedWriteState.receivedSize += len; chunkedWriteState.receivedChunks++; if (chunkedWriteState.receivedChunks >= chunkedWriteState.expectedChunks) { - uint8_t ok[] = {0x00, 0x42, 0x00, 0x00}; - uint8_t err[] = {0xFF, 0x42, 0x00, 0x00}; + uint8_t ok[] = {RESP_ACK, RESP_CONFIG_CHUNK, 0x00, 0x00}; + uint8_t err[] = {RESP_NACK, RESP_CONFIG_CHUNK, 0x00, 0x00}; bool saved = saveConfig(chunkedWriteState.buffer, chunkedWriteState.receivedSize); if (saved) { reloadConfigAfterSave(); @@ -487,7 +487,7 @@ void handleWriteConfigChunk(uint8_t* data, uint16_t len) { chunkedWriteState.receivedSize = 0; chunkedWriteState.receivedChunks = 0; } else { - uint8_t ackResponse[] = {0x00, 0x42, 0x00, 0x00}; + uint8_t ackResponse[] = {RESP_ACK, RESP_CONFIG_CHUNK, 0x00, 0x00}; sendResponse(ackResponse, sizeof(ackResponse)); } } @@ -529,27 +529,27 @@ void imageDataWritten(BLEConnHandle conn_hdl, BLECharPtr chr, uint8_t* data, uin if (isEncryptionEnabled()) { if (!isAuthenticated()) { writeSerial("ERROR: Command requires authentication (encryption enabled)"); - uint8_t response[] = {0x00, (uint8_t)(command & 0xFF), 0xFE}; + uint8_t response[] = {RESP_ACK, (uint8_t)(command & 0xFF), RESP_AUTH_REQUIRED}; sendResponseUnencrypted(response, sizeof(response)); return; } - if (len < 2 + 16 + 12) { + if (len < BLE_CMD_HEADER_SIZE + ENCRYPTION_NONCE_SIZE + ENCRYPTION_TAG_SIZE) { writeSerial("ERROR: Unencrypted command received when encryption is enabled"); - uint8_t response[] = {0x00, (uint8_t)(command & 0xFF), 0xFE}; + uint8_t response[] = {RESP_ACK, (uint8_t)(command & 0xFF), RESP_AUTH_REQUIRED}; sendResponseUnencrypted(response, sizeof(response)); return; } - uint8_t nonce_full[16]; - uint8_t auth_tag[12]; + uint8_t nonce_full[ENCRYPTION_NONCE_SIZE]; + uint8_t auth_tag[ENCRYPTION_TAG_SIZE]; static uint8_t plaintext[512]; uint16_t plaintext_len = 0; - memcpy(nonce_full, data + 2, 16); - memcpy(auth_tag, data + len - 12, 12); + memcpy(nonce_full, data + BLE_CMD_HEADER_SIZE, ENCRYPTION_NONCE_SIZE); + memcpy(auth_tag, data + len - ENCRYPTION_TAG_SIZE, ENCRYPTION_TAG_SIZE); - uint16_t encrypted_data_len = len - 2 - 16 - 12; + uint16_t encrypted_data_len = len - BLE_CMD_HEADER_SIZE - ENCRYPTION_NONCE_SIZE - ENCRYPTION_TAG_SIZE; if (!quietCmd) { static char data_buf[256]; @@ -565,9 +565,9 @@ void imageDataWritten(BLEConnHandle conn_hdl, BLECharPtr chr, uint8_t* data, uin writeSerial(nonce_buf); } - if (!decryptCommand(data + 2 + 16, encrypted_data_len, plaintext, &plaintext_len, nonce_full, auth_tag, command)) { + if (!decryptCommand(data + BLE_CMD_HEADER_SIZE + ENCRYPTION_NONCE_SIZE, encrypted_data_len, plaintext, &plaintext_len, nonce_full, auth_tag, command)) { writeSerial("ERROR: Decryption failed"); - uint8_t response[] = {0x00, (uint8_t)(command & 0xFF), 0xFF}; + uint8_t response[] = {RESP_ACK, (uint8_t)(command & 0xFF), RESP_NACK}; sendResponseUnencrypted(response, sizeof(response)); return; } diff --git a/src/device_control.cpp b/src/device_control.cpp index c1858e0..c58ee80 100644 --- a/src/device_control.cpp +++ b/src/device_control.cpp @@ -376,13 +376,13 @@ void processLedFlash() { void handleLedActivate(uint8_t* data, uint16_t len) { if (len < 1) { - uint8_t errorResponse[] = {0xFF, 0x73, 0x01, 0x00}; + uint8_t errorResponse[] = {RESP_NACK, RESP_LED_ACTIVATE_ACK, 0x01, 0x00}; sendResponse(errorResponse, sizeof(errorResponse)); return; } uint8_t ledInstance = data[0]; if (ledInstance >= globalConfig.led_count) { - uint8_t errorResponse[] = {0xFF, 0x73, 0x02, 0x00}; + uint8_t errorResponse[] = {RESP_NACK, RESP_LED_ACTIVATE_ACK, 0x02, 0x00}; sendResponse(errorResponse, sizeof(errorResponse)); return; } @@ -393,7 +393,7 @@ void handleLedActivate(uint8_t* data, uint16_t len) { uint8_t mode = (uint8_t)(led->reserved[0] & 0x0F); if (mode != 1) { led_stop_internal(true); - uint8_t successResponse[] = {0x00, 0x73, 0x00, 0x00}; + uint8_t successResponse[] = {RESP_ACK, RESP_LED_ACTIVATE_ACK, 0x00, 0x00}; sendResponse(successResponse, sizeof(successResponse)); return; } @@ -407,18 +407,18 @@ void handleLedActivate(uint8_t* data, uint16_t len) { led_load_config(led); led_run_step(); - uint8_t successResponse[] = {0x00, 0x73, 0x00, 0x00}; + uint8_t successResponse[] = {RESP_ACK, RESP_LED_ACTIVATE_ACK, 0x00, 0x00}; sendResponse(successResponse, sizeof(successResponse)); } void handleLedStop(uint8_t* data, uint16_t len) { if (s_led.active && len >= 1 && data[0] != s_led.instance) { - uint8_t errorResponse[] = {0xFF, 0x75, 0x02, 0x00}; + uint8_t errorResponse[] = {RESP_NACK, RESP_LED_STOP_ACK, 0x02, 0x00}; sendResponse(errorResponse, sizeof(errorResponse)); return; } led_stop_internal(true); - uint8_t successResponse[] = {0x00, 0x75, 0x00, 0x00}; + uint8_t successResponse[] = {RESP_ACK, RESP_LED_STOP_ACK, 0x00, 0x00}; sendResponse(successResponse, sizeof(successResponse)); } @@ -718,7 +718,7 @@ void handleDeepSleepCommand(const uint8_t* payload, uint16_t payloadLen) { if (overrideSeconds != 0) { writeSerial("0x0052 duration payload ignored (D-FF hard power off has no timer)", true); } - uint8_t ok[] = {0x00, 0x52, 0x00, 0x00}; + uint8_t ok[] = {RESP_ACK, RESP_DEEP_SLEEP, 0x00, 0x00}; sendResponse(ok, sizeof(ok)); delay(100); powerLatchPowerOff(); @@ -726,13 +726,13 @@ void handleDeepSleepCommand(const uint8_t* payload, uint16_t payloadLen) { } if (globalConfig.power_option.power_mode != 1) { writeSerial("Device not battery powered - 0x0052 rejected", true); - uint8_t errorResponse[] = {0xFF, 0x52, 0x02, 0x00}; + uint8_t errorResponse[] = {RESP_NACK, RESP_DEEP_SLEEP, 0x02, 0x00}; sendResponse(errorResponse, sizeof(errorResponse)); return; } if (globalConfig.power_option.deep_sleep_time_seconds == 0) { writeSerial("Deep sleep disabled in config - 0x0052 rejected", true); - uint8_t errorResponse[] = {0xFF, 0x52, 0x01, 0x00}; + uint8_t errorResponse[] = {RESP_NACK, RESP_DEEP_SLEEP, 0x01, 0x00}; sendResponse(errorResponse, sizeof(errorResponse)); return; } diff --git a/src/display_service.cpp b/src/display_service.cpp index 609a1ff..060449a 100644 --- a/src/display_service.cpp +++ b/src/display_service.cpp @@ -63,13 +63,20 @@ volatile bool epdRefreshInProgress = false; extern uint32_t displayed_etag; -static const uint8_t ERR_ETAG_MISMATCH = 0x01u; -static const uint8_t ERR_RECT_OOB = 0x03u; -static const uint8_t ERR_RECT_ALIGN = 0x04u; -static const uint8_t ERR_PARTIAL_FLAGS = 0x05u; -static const uint8_t ERR_PARTIAL_STREAM = 0x06u; -static const uint8_t ERR_PARTIAL_UNSUPPORTED = 0x07u; - +// 0x76 partial-write error codes come from the canonical opendisplay_protocol.h; +// use OD_ERR_PARTIAL_* directly at the call sites rather than shadowing them here +// (the OD_ERR_PIPE_START_* family reuses the same byte values with DIFFERENT +// meanings, so a local copy is a drift/mix-up hazard). For reference: +// OD_ERR_PARTIAL_ETAG_MISMATCH 0x01 old_etag != displayed etag +// OD_ERR_PARTIAL_RECT_OOB 0x03 rectangle out of panel bounds +// OD_ERR_PARTIAL_RECT_ALIGN 0x04 x / width not a multiple of 8 +// OD_ERR_PARTIAL_FLAGS 0x05 bad / unsupported flags +// OD_ERR_PARTIAL_STREAM 0x06 stream / length error +// OD_ERR_PARTIAL_UNSUPPORTED 0x07 partial write unsupported (e.g. not 1bpp) + +// TODO(protocol): the canonical header defines no partial-write flag constant; +// the 0x76 path reuses the bit0=compressed convention. Add an OD_PARTIAL_FLAG_* +// (or reuse a shared flag) upstream in opendisplay-protocol, then drop this local. static const uint8_t PARTIAL_FLAG_COMPRESSED = 0x01u; static const uint8_t PARTIAL_ALLOWED_FLAGS = PARTIAL_FLAG_COMPRESSED; @@ -2019,7 +2026,7 @@ void handleDirectWriteStart(uint8_t* data, uint16_t len) { memcpy(&directWriteDecompressedTotal, data, 4); if (directWriteDecompressedTotal != directWriteTotalBytes) { cleanupDirectWriteState(false); - uint8_t errorResponse[] = {0xFF, 0x70}; + uint8_t errorResponse[] = {RESP_NACK, RESP_DIRECT_WRITE_START_ACK}; sendResponse(errorResponse, sizeof(errorResponse)); return; } @@ -2029,13 +2036,13 @@ void handleDirectWriteStart(uint8_t* data, uint16_t len) { uint32_t compressedDataLen = len - 4; if (!zlib_stream_to_direct_write(data + 4, compressedDataLen, false)) { cleanupDirectWriteState(false); - uint8_t errorResponse[] = {0xFF, 0x70}; + uint8_t errorResponse[] = {RESP_NACK, RESP_DIRECT_WRITE_START_ACK}; sendResponse(errorResponse, sizeof(errorResponse)); return; } directWriteCompressedReceived = compressedDataLen; } - uint8_t ackResponse[] = {0x00, 0x70}; + uint8_t ackResponse[] = {RESP_ACK, RESP_DIRECT_WRITE_START_ACK}; sendResponse(ackResponse, sizeof(ackResponse)); } @@ -2046,7 +2053,7 @@ void handlePartialWriteStart(uint8_t* data, uint16_t len) { imageWriteLogReset(); if (len < 17) { - send_direct_write_nack(0x76, ERR_PARTIAL_STREAM, false); + send_direct_write_nack(0x76, OD_ERR_PARTIAL_STREAM, false); return; } @@ -2059,12 +2066,12 @@ void handlePartialWriteStart(uint8_t* data, uint16_t len) { uint16_t rectH = ((uint16_t)data[15] << 8) | data[16]; if ((flags & ~PARTIAL_ALLOWED_FLAGS) != 0) { - send_direct_write_nack(0x76, ERR_PARTIAL_FLAGS, false); + send_direct_write_nack(0x76, OD_ERR_PARTIAL_FLAGS, false); return; } if (oldEtag == 0 || oldEtag != displayed_etag || newEtag == 0) { - send_direct_write_nack(0x76, ERR_ETAG_MISMATCH, false); + send_direct_write_nack(0x76, OD_ERR_PARTIAL_ETAG_MISMATCH, false); return; } @@ -2074,27 +2081,31 @@ void handlePartialWriteStart(uint8_t* data, uint16_t len) { // bb_epaper partial refresh support is effectively non-existent for // 2bpp+ panels, and physical panels may not support that mode either. // This protocol uses two 1bpp controller planes as old/new image memory. - send_direct_write_nack(0x76, ERR_PARTIAL_UNSUPPORTED, false); + send_direct_write_nack(0x76, OD_ERR_PARTIAL_UNSUPPORTED, false); return; } if (rectW == 0 || rectH == 0 || (uint32_t)rectX + rectW > dispW || (uint32_t)rectY + rectH > dispH) { - send_direct_write_nack(0x76, ERR_RECT_OOB, false); + send_direct_write_nack(0x76, OD_ERR_PARTIAL_RECT_OOB, false); return; } if ((rectX & 7u) != 0 || (rectW & 7u) != 0) { - send_direct_write_nack(0x76, ERR_RECT_ALIGN, false); + send_direct_write_nack(0x76, OD_ERR_PARTIAL_RECT_ALIGN, false); return; } uint32_t planeBytes = calc_controller_plane_bytes(rectW, rectH); uint32_t expectedLogicalSize = planeBytes * 2u; + // TODO(protocol): 0x76 (partial-write) has no RESP_* mirror in the canonical + // header, so the opcode-echo byte in these frames — and in the send_direct_write_nack(0x76, ...) + // calls below — stays a raw literal. Add RESP_PARTIAL_WRITE_START upstream in + // opendisplay-protocol, then replace the raw 0x76 here. if (expectedLogicalSize == 0) { - uint8_t errResponse[] = {0xFF, 0x76}; + uint8_t errResponse[] = {RESP_NACK, 0x76}; sendResponse(errResponse, sizeof(errResponse)); return; } @@ -2121,12 +2132,12 @@ void handlePartialWriteStart(uint8_t* data, uint16_t len) { if (len > 17) { uint16_t initLen = len - 17; if (!partial_consume_bytes(data + 17, (uint32_t)initLen)) { - send_direct_write_nack(0x76, ERR_PARTIAL_STREAM, true); + send_direct_write_nack(0x76, OD_ERR_PARTIAL_STREAM, true); return; } } - uint8_t ackResponse[] = {0x00, 0x76}; + uint8_t ackResponse[] = {RESP_ACK, 0x76}; sendResponse(ackResponse, sizeof(ackResponse)); } @@ -2140,11 +2151,11 @@ void handleDirectWriteData(uint8_t* data, uint16_t len) { if (len == 0) return; imageWriteLogChunk(data, len); if (!partial_consume_bytes(data, (uint32_t)len)) { - send_direct_write_nack(0x71, ERR_PARTIAL_STREAM, true); + send_direct_write_nack(RESP_DIRECT_WRITE_DATA_ACK, OD_ERR_PARTIAL_STREAM, true); return; } imageWriteLogProgress(partialCtx.bytes_received, partialCtx.expected_stream_size); - uint8_t ackResponse[] = {0x00, 0x71}; + uint8_t ackResponse[] = {RESP_ACK, RESP_DIRECT_WRITE_DATA_ACK}; sendResponse(ackResponse, sizeof(ackResponse)); return; } @@ -2153,12 +2164,12 @@ void handleDirectWriteData(uint8_t* data, uint16_t len) { if (directWriteCompressed) { if (!handleDirectWriteCompressedData(data, len)) { cleanupDirectWriteState(true); - uint8_t errorResponse[] = {0xFF, 0x71}; + uint8_t errorResponse[] = {RESP_NACK, RESP_DIRECT_WRITE_DATA_ACK}; sendResponse(errorResponse, sizeof(errorResponse)); } else { directWriteCompressedReceived += len; imageWriteLogProgress(directWriteBytesWritten, directWriteTotalBytes); - uint8_t ackResponse[] = {0x00, 0x71}; + uint8_t ackResponse[] = {RESP_ACK, RESP_DIRECT_WRITE_DATA_ACK}; sendResponse(ackResponse, sizeof(ackResponse)); } return; @@ -2182,7 +2193,7 @@ void handleDirectWriteData(uint8_t* data, uint16_t len) { if (directWriteBytesWritten >= directWriteTotalBytes) { handleDirectWriteEnd(nullptr, 0); } else { - uint8_t ackResponse[] = {0x00, 0x71}; + uint8_t ackResponse[] = {RESP_ACK, RESP_DIRECT_WRITE_DATA_ACK}; sendResponse(ackResponse, sizeof(ackResponse)); } } @@ -2194,20 +2205,20 @@ void handleDirectWriteEnd(uint8_t* data, uint16_t len) { if (pipeState.active) return; if (partialCtx.active) { if (data != nullptr && len > 1) { - send_direct_write_nack(0x72, ERR_PARTIAL_STREAM, true); + send_direct_write_nack(RESP_DIRECT_WRITE_END_ACK, OD_ERR_PARTIAL_STREAM, true); return; } if (partialCtx.compressed) { if (partialCtx.bytes_received == 0 || !zlib_stream_to_partial_write(nullptr, 0, true)) { - send_direct_write_nack(0x72, ERR_PARTIAL_STREAM, true); + send_direct_write_nack(RESP_DIRECT_WRITE_END_ACK, OD_ERR_PARTIAL_STREAM, true); return; } } else if (partialCtx.bytes_written != partialCtx.expected_stream_size) { - send_direct_write_nack(0x72, ERR_PARTIAL_STREAM, true); + send_direct_write_nack(RESP_DIRECT_WRITE_END_ACK, OD_ERR_PARTIAL_STREAM, true); return; } imageWriteLogFinish(partialCtx.bytes_received, partialCtx.expected_stream_size); - uint8_t ackResponse[] = {0x00, 0x72}; + uint8_t ackResponse[] = {RESP_ACK, RESP_DIRECT_WRITE_END_ACK}; sendResponse(ackResponse, sizeof(ackResponse)); int refreshMode = REFRESH_PARTIAL; if (data != nullptr && len >= 1 && data[0] == REFRESH_FULL) refreshMode = REFRESH_FULL; @@ -2215,11 +2226,11 @@ void handleDirectWriteEnd(uint8_t* data, uint16_t len) { bool refreshSuccess = partial_write_to_panel(refreshMode); if (refreshSuccess) { displayed_etag = partialCtx.new_etag; - uint8_t validatedResponse[] = {0x00, 0x73}; + uint8_t validatedResponse[] = {RESP_ACK, RESP_DIRECT_WRITE_REFRESH_SUCCESS}; sendResponse(validatedResponse, sizeof(validatedResponse)); } else { displayed_etag = 0; - uint8_t timeoutResponse[] = {0x00, 0x74}; + uint8_t timeoutResponse[] = {RESP_ACK, RESP_DIRECT_WRITE_REFRESH_TIMEOUT}; sendResponse(timeoutResponse, sizeof(timeoutResponse)); } cleanup_partial_write_state(); @@ -2306,11 +2317,11 @@ static void directWriteFinishAndRefresh(uint8_t* data, uint16_t len, uint8_t end // of diffing against the wrong, now-outdated base image. if (hasNewEtag && newEtag != 0) displayed_etag = newEtag; else displayed_etag = 0; - uint8_t refreshResponse[] = {0x00, 0x73}; + uint8_t refreshResponse[] = {RESP_ACK, RESP_DIRECT_WRITE_REFRESH_SUCCESS}; sendResponse(refreshResponse, sizeof(refreshResponse)); } else { if (hasNewEtag) displayed_etag = 0; - uint8_t timeoutResponse[] = {0x00, 0x74}; + uint8_t timeoutResponse[] = {RESP_ACK, RESP_DIRECT_WRITE_REFRESH_TIMEOUT}; sendResponse(timeoutResponse, sizeof(timeoutResponse)); } } @@ -2381,10 +2392,18 @@ static void pipeBuildAckPayload(uint8_t* out) { out[4] = (uint8_t)((mask >> 24) & 0xFF); } +// TODO(protocol): the canonical opendisplay_protocol.h defines no RESP_* mirror +// for the pipe-write opcodes, so the response opcode-echo byte in the helpers +// below (0x80 / 0x81 / 0x82) stays a raw literal. It also defines no data-phase +// pipe error namespace (only OD_ERR_PIPE_START_* for the 0x80 START), so the +// sendPipeNack() error codes (0x03 over-size/overflow, 0x04 out-of-window) are +// raw too. Add RESP_PIPE_WRITE_{START,DATA,END} + an OD_ERR_PIPE_DATA_* set +// upstream in opendisplay-protocol, then replace those literals here. + // {0x00,0x81, highest_seen, ack_mask LE(4)} via sendResponse (auto-encrypts when // authenticated). Resets both cadence counters. static void sendPipeAck(void) { - uint8_t r[7] = {0x00, 0x81, 0, 0, 0, 0, 0}; + uint8_t r[7] = {RESP_ACK, 0x81, 0, 0, 0, 0, 0}; pipeBuildAckPayload(r + 2); sendResponse(r, sizeof(r)); pipeState.frames_since_ack = 0; @@ -2400,7 +2419,7 @@ static void sendPipeAck(void) { // with refreshDisplay=true: sleep a powered controller cleanly, cut power, resume // touch) instead of leaving the panel powered until the next transfer. static void sendPipeNack(uint8_t err) { - uint8_t r[8] = {0xFF, 0x81, err, 0, 0, 0, 0, 0}; + uint8_t r[8] = {RESP_NACK, 0x81, err, 0, 0, 0, 0, 0}; pipeBuildAckPayload(r + 3); sendResponse(r, sizeof(r)); pipeState.error = true; @@ -2417,7 +2436,7 @@ static void sendPipeNack(uint8_t err) { // {0xFF,0x80, err, 0x00}. Caller owns any session teardown. static void sendPipeStartNack(uint8_t err) { - uint8_t r[4] = {0xFF, 0x80, err, 0x00}; + uint8_t r[4] = {RESP_NACK, 0x80, err, 0x00}; sendResponse(r, sizeof(r)); } @@ -2482,7 +2501,7 @@ void handlePipeWriteStart(uint8_t* data, uint16_t len) { // Fixed 10-byte payload (opcode already stripped by the dispatcher): // ver(1)+flags(1)+req_w(1)+req_n(1)+client_max_frame(2)+total_size(4). // Tolerate trailing bytes (future fields). - if (len < 10) { sendPipeStartNack(0x01); return; } + if (len < 10) { sendPipeStartNack(OD_ERR_PIPE_START_BAD_HEADER); return; } uint8_t ver = data[0]; uint8_t flags = data[1]; uint8_t req_w = data[2]; @@ -2491,16 +2510,16 @@ void handlePipeWriteStart(uint8_t* data, uint16_t len) { uint32_t total_size = (uint32_t)data[6] | ((uint32_t)data[7] << 8) | ((uint32_t)data[8] << 16) | ((uint32_t)data[9] << 24); - if (ver != PIPE_VERSION) { sendPipeStartNack(0x01); return; } + if (ver != PIPE_VERSION) { sendPipeStartNack(OD_ERR_PIPE_START_BAD_HEADER); return; } // Defined flags: bit0 zlib compression, bit1 partial-region refresh. Any other bit unsupported. - if ((flags & ~(PIPE_FLAG_COMPRESSED | PIPE_FLAG_PARTIAL)) != 0) { sendPipeStartNack(0x02); return; } + if ((flags & ~(PIPE_FLAG_COMPRESSED | PIPE_FLAG_PARTIAL)) != 0) { sendPipeStartNack(OD_ERR_PIPE_START_UNKNOWN_FLAG); return; } bool compressed = (flags & PIPE_FLAG_COMPRESSED) != 0; bool partial = (flags & PIPE_FLAG_PARTIAL) != 0; // Partial START appends a 12-byte LE extension after total_size (payload len 22 vs 10): // [old_etag:4][x:2][y:2][w:2][h:2]. LE, unlike 0x76's big-endian layout. - if (partial && len < 22) { sendPipeStartNack(0x01); return; } + if (partial && len < 22) { sendPipeStartNack(OD_ERR_PIPE_START_BAD_HEADER); return; } uint32_t old_etag = 0; uint16_t rectX = 0, rectY = 0, rectW = 0, rectH = 0; uint32_t planeBytes = 0; @@ -2519,17 +2538,17 @@ void handlePipeWriteStart(uint8_t* data, uint16_t len) { uint16_t dispH = globalConfig.displays[0].pixel_height; // 5: two 1bpp controller planes are the partial mechanism; seeed/IT8951 has no equivalent. if (getBitsPerPixel() != 1 || seeed_driver_used() || e1004_panel_used()) { - displayed_etag = 0; sendPipeStartNack(0x06); return; + displayed_etag = 0; sendPipeStartNack(OD_ERR_PIPE_START_PARTIAL_UNSUPPORTED); return; } // 6: etag gate — nonzero and must match what is currently on the panel. if (old_etag == 0 || old_etag != displayed_etag) { - displayed_etag = 0; sendPipeStartNack(0x05); return; + displayed_etag = 0; sendPipeStartNack(OD_ERR_PIPE_START_ETAG_MISMATCH); return; } // 7: rectangle must be non-empty, in-bounds, and x/width byte-aligned (1bpp packing). if (rectW == 0 || rectH == 0 || (uint32_t)rectX + rectW > dispW || (uint32_t)rectY + rectH > dispH || (rectX & 7u) != 0 || (rectW & 7u) != 0) { - displayed_etag = 0; sendPipeStartNack(0x07); return; + displayed_etag = 0; sendPipeStartNack(OD_ERR_PIPE_START_RECT_INVALID); return; } } @@ -2541,11 +2560,11 @@ void handlePipeWriteStart(uint8_t* data, uint16_t len) { if (planeBytes == 0 || total_size != planeBytes * 2u) { // Plan 1.2: every partial-request NACK at steps 5-8 clears the etag // (parity with send_direct_write_nack). - displayed_etag = 0; sendPipeStartNack(0x03); return; + displayed_etag = 0; sendPipeStartNack(OD_ERR_PIPE_START_SIZE_MISMATCH); return; } } else { directWriteComputeGeometry(compressed); - if (total_size != directWriteTotalBytes) { sendPipeStartNack(0x03); return; } + if (total_size != directWriteTotalBytes) { sendPipeStartNack(OD_ERR_PIPE_START_SIZE_MISMATCH); return; } } // Effective values (min-rule, plan 1.1). Floors at 1; N <= W; frame <= 244. @@ -2605,7 +2624,11 @@ void handlePipeWriteStart(uint8_t* data, uint16_t len) { // commands sequentially from its callback task, so activation below completes // before any queued 0x0081 write is handed to the dispatcher. // Device maxima; flags bit0 SET (selective repeat), bit1 = partial accepted (plan 1.2). - uint8_t resp[8] = {0x00, 0x80, PIPE_VERSION, PIPE_MAX_W, PIPE_MAX_N, + // TODO(protocol): 0x80 has no RESP_* mirror (see sendPipeAck), and the START + // response-flags bit0 "selective repeat" has no canonical constant (distinct from + // the request-side PIPE_FLAG_*). Add RESP_PIPE_WRITE_START + a response-flag + // constant upstream, then replace the raw 0x80 / 0x01 here. + uint8_t resp[8] = {RESP_ACK, 0x80, PIPE_VERSION, PIPE_MAX_W, PIPE_MAX_N, (uint8_t)(PIPE_MAX_FRAME & 0xFF), (uint8_t)((PIPE_MAX_FRAME >> 8) & 0xFF), (uint8_t)(0x01 | (partial ? PIPE_FLAG_PARTIAL : 0))}; sendResponse(resp, sizeof(resp)); @@ -2728,12 +2751,12 @@ void handlePipeWriteData(uint8_t* data, uint16_t len) { void handlePipeWriteEnd(uint8_t* data, uint16_t len) { if (!pipeState.active) { - uint8_t n[2] = {0xFF, 0x82}; // no active pipe transfer + uint8_t n[2] = {RESP_NACK, 0x82}; // no active pipe transfer sendResponse(n, sizeof(n)); return; } if (pipeState.error) { - uint8_t n[2] = {0xFF, 0x82}; // a fatal error already NACKed this transfer + uint8_t n[2] = {RESP_NACK, 0x82}; // a fatal error already NACKed this transfer sendResponse(n, sizeof(n)); // sendPipeNack already released the panel hardware at NACK time; this // re-run is a defensive no-op (partialCtx / directWriteActive already down). @@ -2755,7 +2778,7 @@ void handlePipeWriteEnd(uint8_t* data, uint16_t len) { incomplete = true; } if (incomplete) { - uint8_t n[2] = {0xFF, 0x82}; + uint8_t n[2] = {RESP_NACK, 0x82}; sendResponse(n, sizeof(n)); displayed_etag = 0; cleanup_partial_write_state(); @@ -2763,7 +2786,7 @@ void handlePipeWriteEnd(uint8_t* data, uint16_t len) { return; } imageWriteLogFinish(partialCtx.bytes_received, partialCtx.expected_stream_size); - uint8_t ackResponse[] = {0x00, 0x82}; + uint8_t ackResponse[] = {RESP_ACK, 0x82}; sendResponse(ackResponse, sizeof(ackResponse)); // Refresh selector rides the END tail (plan 1.4): 0->FULL, 1->FAST, 2/absent->PARTIAL. int refreshMode = REFRESH_PARTIAL; @@ -2774,11 +2797,11 @@ void handlePipeWriteEnd(uint8_t* data, uint16_t len) { bool refreshSuccess = partial_write_to_panel(refreshMode); if (refreshSuccess) { displayed_etag = newEtag; - uint8_t validatedResponse[] = {0x00, 0x73}; + uint8_t validatedResponse[] = {RESP_ACK, RESP_DIRECT_WRITE_REFRESH_SUCCESS}; sendResponse(validatedResponse, sizeof(validatedResponse)); } else { displayed_etag = 0; - uint8_t timeoutResponse[] = {0x00, 0x74}; + uint8_t timeoutResponse[] = {RESP_ACK, RESP_DIRECT_WRITE_REFRESH_TIMEOUT}; sendResponse(timeoutResponse, sizeof(timeoutResponse)); } cleanup_partial_write_state(); @@ -2791,7 +2814,7 @@ void handlePipeWriteEnd(uint8_t* data, uint16_t len) { bool incomplete = (pipeState.queued_count > 0); if (!pipeState.compressed && directWriteBytesWritten < directWriteTotalBytes) incomplete = true; if (incomplete) { - uint8_t n[2] = {0xFF, 0x82}; + uint8_t n[2] = {RESP_NACK, 0x82}; sendResponse(n, sizeof(n)); // Mid-stream abort with the panel powered: use the legacy mid-stream // variant (refreshDisplay=true → sleep the controller cleanly, cut power, @@ -3120,6 +3143,6 @@ static void send_direct_write_nack(uint8_t opcode, uint8_t error, bool cleanupSt if (partialCtx.active) cleanup_partial_write_state(); else cleanupDirectWriteState(false); } - uint8_t errResponse[] = {0xFF, opcode, error, 0x00}; + uint8_t errResponse[] = {RESP_NACK, opcode, error, 0x00}; sendResponse(errResponse, sizeof(errResponse)); } diff --git a/src/encryption.cpp b/src/encryption.cpp index 02c3c0d..b5ab62e 100644 --- a/src/encryption.cpp +++ b/src/encryption.cpp @@ -512,7 +512,7 @@ void secure_random(uint8_t* output, size_t len) { bool handleAuthenticate(uint8_t* data, uint16_t len) { if (!isEncryptionEnabled()) { - uint8_t response[] = {0x00, 0x50, 0x03}; + uint8_t response[] = {RESP_ACK, RESP_AUTHENTICATE, AUTH_STATUS_NOT_CONFIG}; sendResponse(response, sizeof(response)); return false; } @@ -521,7 +521,7 @@ bool handleAuthenticate(uint8_t* data, uint16_t len) { uint32_t timeSinceLastAuth = (currentTime - encryptionSession.last_auth_time) / 1000; if (timeSinceLastAuth < 60) { if (encryptionSession.auth_attempts >= 10) { - uint8_t response[] = {0x00, 0x50, 0x04}; + uint8_t response[] = {RESP_ACK, RESP_AUTHENTICATE, AUTH_STATUS_RATE_LIMIT}; sendResponse(response, sizeof(response)); return false; } @@ -541,7 +541,7 @@ bool handleAuthenticate(uint8_t* data, uint16_t len) { uint8_t device_id[4]; getAuthDeviceIdBytes(device_id); uint8_t response[2 + 1 + 16 + 4]; - response[0] = 0x00; response[1] = 0x50; response[2] = 0x00; + response[0] = RESP_ACK; response[1] = RESP_AUTHENTICATE; response[2] = AUTH_STATUS_CHALLENGE; memcpy(response + 3, encryptionSession.pending_server_nonce, 16); memcpy(response + 19, device_id, 4); sendResponse(response, sizeof(response)); @@ -555,7 +555,7 @@ bool handleAuthenticate(uint8_t* data, uint16_t len) { memcpy(challenge_response, data + 16, 16); if (currentTime - encryptionSession.server_nonce_time > 30000) { writeSerial("ERROR: Server nonce expired"); - uint8_t response[] = {0x00, 0x50, 0xFF}; + uint8_t response[] = {RESP_ACK, RESP_AUTHENTICATE, AUTH_STATUS_ERROR}; sendResponse(response, sizeof(response)); return false; } @@ -568,13 +568,13 @@ bool handleAuthenticate(uint8_t* data, uint16_t len) { uint8_t expected_response[16]; if (!aes_cmac(securityConfig.encryption_key, challenge_input, 36, expected_response)) { writeSerial("ERROR: Failed to compute expected CMAC"); - uint8_t response[] = {0x00, 0x50, 0xFF}; + uint8_t response[] = {RESP_ACK, RESP_AUTHENTICATE, AUTH_STATUS_ERROR}; sendResponse(response, sizeof(response)); return false; } if (!constantTimeCompare(challenge_response, expected_response, 16)) { writeSerial("ERROR: Authentication failed (wrong key)"); - uint8_t response[] = {0x00, 0x50, 0x01}; + uint8_t response[] = {RESP_ACK, RESP_AUTHENTICATE, AUTH_STATUS_FAILED}; sendResponse(response, sizeof(response)); memset(encryptionSession.pending_server_nonce, 0, 16); return false; @@ -584,7 +584,7 @@ bool handleAuthenticate(uint8_t* data, uint16_t len) { if (!deriveSessionKey(securityConfig.encryption_key, client_nonce, encryptionSession.pending_server_nonce, encryptionSession.session_key)) { writeSerial("ERROR: Failed to derive session key"); - uint8_t response[] = {0x00, 0x50, 0xFF}; + uint8_t response[] = {RESP_ACK, RESP_AUTHENTICATE, AUTH_STATUS_ERROR}; sendResponse(response, sizeof(response)); return false; } @@ -596,7 +596,7 @@ bool handleAuthenticate(uint8_t* data, uint16_t len) { } if (!session_id_valid) { writeSerial("ERROR: Session ID is invalid (all zeros)!"); - uint8_t response[] = {0x00, 0x50, 0xFF}; + uint8_t response[] = {RESP_ACK, RESP_AUTHENTICATE, AUTH_STATUS_ERROR}; sendResponse(response, sizeof(response)); return false; } @@ -617,19 +617,19 @@ bool handleAuthenticate(uint8_t* data, uint16_t len) { if (!aes_cmac(encryptionSession.session_key, server_input, 36, server_response)) { writeSerial("ERROR: Failed to compute server response"); clearEncryptionSession(); - uint8_t response[] = {0x00, 0x50, 0xFF}; + uint8_t response[] = {RESP_ACK, RESP_AUTHENTICATE, AUTH_STATUS_ERROR}; sendResponse(response, sizeof(response)); return false; } uint8_t response[2 + 1 + 16]; - response[0] = 0x00; response[1] = 0x50; response[2] = 0x00; + response[0] = RESP_ACK; response[1] = RESP_AUTHENTICATE; response[2] = AUTH_STATUS_SUCCESS; memcpy(response + 3, server_response, 16); sendResponse(response, sizeof(response)); writeSerial("Authentication successful, session established"); return true; } writeSerial("ERROR: Invalid authentication request format (len=" + String(len) + ")"); - uint8_t response[] = {0x00, 0x50, 0xFF}; + uint8_t response[] = {RESP_ACK, RESP_AUTHENTICATE, AUTH_STATUS_ERROR}; sendResponse(response, sizeof(response)); return false; } @@ -662,7 +662,7 @@ bool decryptCommand(uint8_t* ciphertext, uint16_t ciphertext_len, uint8_t* plain static uint8_t decrypted_with_length[512]; bool success = aes_ccm_decrypt(encryptionSession.session_key, nonce, 13, ad, 2, ciphertext, encrypted_len, - decrypted_with_length, auth_tag, 12); + decrypted_with_length, auth_tag, ENCRYPTION_TAG_SIZE); if (success) { uint8_t payload_length = decrypted_with_length[0]; if (payload_length > encrypted_len - 1) { @@ -707,13 +707,13 @@ bool encryptResponse(uint8_t* plaintext, uint16_t plaintext_len, uint8_t* cipher uint16_t total_payload_len = 1 + payload_len; bool success = aes_ccm_encrypt(encryptionSession.session_key, nonce_ccm, 13, ad, 2, payload_with_length, total_payload_len, - ciphertext + 2 + 16, auth_tag, 12); + ciphertext + BLE_CMD_HEADER_SIZE + ENCRYPTION_NONCE_SIZE, auth_tag, ENCRYPTION_TAG_SIZE); if (!success) return false; ciphertext[0] = plaintext[0]; ciphertext[1] = plaintext[1]; - memcpy(ciphertext + 2, nonce, 16); - memcpy(ciphertext + 2 + 16 + total_payload_len, auth_tag, 12); - *ciphertext_len = 2 + 16 + total_payload_len + 12; + memcpy(ciphertext + BLE_CMD_HEADER_SIZE, nonce, ENCRYPTION_NONCE_SIZE); + memcpy(ciphertext + BLE_CMD_HEADER_SIZE + ENCRYPTION_NONCE_SIZE + total_payload_len, auth_tag, ENCRYPTION_TAG_SIZE); + *ciphertext_len = BLE_CMD_HEADER_SIZE + ENCRYPTION_NONCE_SIZE + total_payload_len + ENCRYPTION_TAG_SIZE; updateEncryptionSessionActivity(); return true; }