Skip to content

Protocol constant cleanup + NimBLE callback race fix (stacked on #107) #108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jonasniesner merged 2 commits into from
Jul 19, 2026
Merged

Protocol constant cleanup + NimBLE callback race fix (stacked on #107) #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7829fa7
fix(ble): defer NimBLE onConnect/onDisconnect work to loop() (race au…
davelee98 Jul 18, 2026
b83d824
Replace protocol magic numbers with canonical named constants
davelee98 Jul 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/ble_init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/ble_init.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
18 changes: 9 additions & 9 deletions src/buzzer_control.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -291,34 +291,34 @@ 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;
}

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;
}
Expand All @@ -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));
}

Expand Down
82 changes: 41 additions & 41 deletions src/communication.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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));
}
}
Expand All @@ -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();
Expand All @@ -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));
Expand All @@ -452,32 +452,32 @@ 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;
}
if (chunkedWriteState.receivedChunks == 1 && isEncryptionEnabled() && !isAuthenticated()) {
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;
}
memcpy(chunkedWriteState.buffer + chunkedWriteState.receivedSize, data, 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();
Expand All @@ -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));
}
}
Expand Down Expand Up @@ -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];
Expand All @@ -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;
}
Expand Down
18 changes: 9 additions & 9 deletions src/device_control.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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));
}

Expand Down Expand Up @@ -718,21 +718,21 @@ 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();
return;
}
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;
}
Expand Down
Loading