From 03d9c93472574068ce9bbc9ec13dfd21b377de39 Mon Sep 17 00:00:00 2001 From: Christian Baars Date: Thu, 20 Aug 2026 13:59:47 +0200 Subject: [PATCH] MI32: harden Berry BLE client and server handling (#24970) Fix write and subscription behavior, reuse GATT discovery caches, preserve notification metadata, and harden server buffers, event headers, and BLE.info diagnostics. --- tasmota/include/xsns_62_esp32_mi.h | 2 +- .../xdrv_52_3_berry_MI32.ino | 18 +-- .../tasmota_xsns_sensor/xsns_62_esp32_mi.ino | 115 ++++++++++-------- 3 files changed, 77 insertions(+), 58 deletions(-) diff --git a/tasmota/include/xsns_62_esp32_mi.h b/tasmota/include/xsns_62_esp32_mi.h index 9552de7fa..de246287b 100644 --- a/tasmota/include/xsns_62_esp32_mi.h +++ b/tasmota/include/xsns_62_esp32_mi.h @@ -430,7 +430,7 @@ enum BLE_CLIENT_OP { BLE_OP_READ = 1, BLE_OP_WRITE, BLE_OP_SUBSCRIBE, -BLE_OP_UNSUBSCRIBE, //maybe used later +BLE_OP_UNSUBSCRIBE, BLE_OP_DISCONNECT, BLE_OP_GET_NOTIFICATION = 103, }; diff --git a/tasmota/tasmota_xdrv_driver/xdrv_52_3_berry_MI32.ino b/tasmota/tasmota_xdrv_driver/xdrv_52_3_berry_MI32.ino index 2a21438dd..9fa8a63eb 100644 --- a/tasmota/tasmota_xdrv_driver/xdrv_52_3_berry_MI32.ino +++ b/tasmota/tasmota_xdrv_driver/xdrv_52_3_berry_MI32.ino @@ -292,7 +292,8 @@ int be_BLE_run(bvm *vm) { // #else // be_map_insert_nil(vm, "bonds"); // #endif - if(MI32.mode.connected == 1 || (MI32.role & MI32_ROLE_SERVER)){ + NimBLEClient* _serverPeer = MI32.conCtx ? MI32.conCtx->serverPeer : nullptr; + if(MI32.mode.connected == 1 || _serverPeer != nullptr){ NimBLEClient* _device = nullptr; if(MI32.mode.connected == 1){ _device = NimBLEDevice::getClientByHandle(MI32.connID); @@ -311,7 +312,7 @@ int be_BLE_run(bvm *vm) { be_map_insert_bool(vm, "encrypted", _info.isEncrypted()); be_map_insert_bool(vm, "authenticated", _info.isAuthenticated()); if(_device == nullptr) { - auto _remote_client = NimBLEDevice::getServer()->getClient(_info); + auto _remote_client = _serverPeer; if(_remote_client != nullptr){ auto _name = _remote_client->getValue(NimBLEUUID((uint16_t)0x1800), NimBLEUUID((uint16_t)0x2A00)); //GAP, name if(_name){ @@ -323,8 +324,7 @@ int be_BLE_run(bvm *vm) { } ble_store_value_sec value_sec; - ble_sm_read_bond(_info.getConnHandle(), &value_sec); - if(value_sec.irk_present == 1){ + if(ble_sm_read_bond(_info.getConnHandle(), &value_sec) == 0 && value_sec.irk_present == 1){ char IRK[33]; ToHex_P(value_sec.irk,16,IRK,33); be_map_insert_str(vm, "IRK",IRK ); @@ -358,16 +358,16 @@ be_BLE_op: 1 read 2 write 3 subscribe -4 unsubscribe - maybe later +4 unsubscribe 5 disconnect -6 discover services -7 discover characteristics +6 discover services (true forces refresh) +7 discover characteristics (true forces refresh) 11 read once, then disconnect 12 write once, then disconnect 13 subscribe once, then disconnect -14 unsubscribe once, then disconnect - maybe later +14 unsubscribe once, then disconnect #server __commands @@ -402,4 +402,4 @@ MI32.set_hum(slot,float) MI32.set_temp(slot,float) MI32.widget(string[,cb]) -*/ \ No newline at end of file +*/ diff --git a/tasmota/tasmota_xsns_sensor/xsns_62_esp32_mi.ino b/tasmota/tasmota_xsns_sensor/xsns_62_esp32_mi.ino index 60b3c460d..8a606d5ec 100644 --- a/tasmota/tasmota_xsns_sensor/xsns_62_esp32_mi.ino +++ b/tasmota/tasmota_xsns_sensor/xsns_62_esp32_mi.ino @@ -55,6 +55,7 @@ void MI32ServerSetCharacteristic(NimBLEServer *pServer, std::vector MIBLEsensors; RingbufHandle_t BLERingBufferQueue = nullptr; @@ -163,6 +164,8 @@ class MI32ServerCallbacks: public NimBLEServerCallbacks { } item; item.header.length = 6; item.header.type = BLE_OP_ON_CONNECT; + item.header.returnCharUUID = 0; + item.header.handle = 0; memcpy(item.buffer,connInfo.getAddress().getVal(),6); xRingbufferSend(BLERingBufferQueue, (const void*)&item, sizeof(BLERingBufferItem_t) + 6 , pdMS_TO_TICKS(1)); MI32.infoMsg = MI32_SERV_CLIENT_CONNECTED; @@ -178,6 +181,8 @@ class MI32ServerCallbacks: public NimBLEServerCallbacks { } item; item.header.length = 0; item.header.type = BLE_OP_ON_DISCONNECT; + item.header.returnCharUUID = 0; + item.header.handle = 0; xRingbufferSend(BLERingBufferQueue, (const void*)&item, sizeof(BLERingBufferItem_t), pdMS_TO_TICKS(1)); MI32.infoMsg = MI32_SERV_CLIENT_DISCONNECTED; if(MI32.conCtx == nullptr) return; @@ -215,6 +220,8 @@ class MI32ServerCallbacks: public NimBLEServerCallbacks { memcpy(item.buffer + security_record_size, &peer_security_record, security_record_size); item.header.length = 2 * security_record_size; item.header.type = BLE_OP_ON_AUTHENTICATED; + item.header.returnCharUUID = 0; + item.header.handle = 0; xRingbufferSend(BLERingBufferQueue, (const void*)&item, sizeof(BLERingBufferItem_t) + item.header.length, pdMS_TO_TICKS(1)); MI32.infoMsg = MI32_SERV_CLIENT_AUTHENTICATED; } @@ -281,25 +288,31 @@ class MI32CharacteristicCallbacks: public NimBLECharacteristicCallbacks { void MI32notifyCB(NimBLERemoteCharacteristic* pRemoteCharacteristic, uint8_t* pData, size_t length, bool isNotify){ + const NimBLEUUID &uuid = pRemoteCharacteristic->getUUID(); + const uint8_t *uuidValue = uuid.getValue() + (uuid.bitSize() == BLE_UUID_TYPE_128 ? 12 : 0); + const uint16_t uuid16 = *reinterpret_cast(uuidValue); AddLog(LOG_LEVEL_DEBUG_MORE,PSTR("M32: notifyCB uuid=%04x handle=%u len=%u isNotify=%u"), - *reinterpret_cast(pRemoteCharacteristic->getUUID().getValue() + 12), - pRemoteCharacteristic->getHandle(), (unsigned)length, (unsigned)isNotify); - if(isNotify){ - struct{ - BLERingBufferItem_t header; - uint8_t buffer[255]; - } item; - if(length > sizeof(item.buffer)) length = sizeof(item.buffer); // Cap notification payload - item.header.length = length; - item.header.type = 103; // notification op for serv_cb dispatch in bridge mode (role==3) - memcpy(item.buffer,pData,length); - item.header.returnCharUUID = *reinterpret_cast(pRemoteCharacteristic->getUUID().getValue() + 12); - item.header.handle = pRemoteCharacteristic->getHandle(); - xRingbufferSend(BLERingBufferQueue, (const void*)&item, sizeof(BLERingBufferItem_t) + length , pdMS_TO_TICKS(5)); - MI32ReadingDone.store(true, std::memory_order_release); - MI32.infoMsg = MI32_GOT_NOTIFICATION; - return; - } + uuid16, pRemoteCharacteristic->getHandle(), (unsigned)length, (unsigned)isNotify); + struct{ + BLERingBufferItem_t header; + uint8_t buffer[255]; + } item; + if(length > sizeof(item.buffer)) length = sizeof(item.buffer); // Cap notification payload + item.header.length = length; + item.header.type = BLE_OP_GET_NOTIFICATION; + memcpy(item.buffer,pData,length); + item.header.returnCharUUID = uuid16; + item.header.handle = pRemoteCharacteristic->getHandle(); + xRingbufferSend(BLERingBufferQueue, (const void*)&item, sizeof(BLERingBufferItem_t) + length , pdMS_TO_TICKS(5)); + MI32ReadingDone.store(true, std::memory_order_release); + MI32.infoMsg = MI32_GOT_NOTIFICATION; +} + +static bool MI32SetSubscription(NimBLERemoteCharacteristic *pChr, bool subscribe, bool response){ + const bool notify = pChr->canNotify(); + if(!notify && !pChr->canIndicate()) return false; + return subscribe ? pChr->subscribe(notify, MI32notifyCB, response) + : pChr->unsubscribe(response); } static MI32AdvCallbacks MI32ScanCallbacks; @@ -1274,7 +1287,8 @@ void MI32ScanTask(void *pvParameters){ * ... next service */ void MI32ConnectionGetServices(){ - std::vector srvvector = MI32Client->getServices(true); // refresh + const bool refresh = MI32.conCtx->response || MI32Client->getServices(false).empty(); + const auto &srvvector = MI32Client->getServices(refresh); MI32.conCtx->buffer[1] = srvvector.size(); // number of services uint32_t i = 2; for (auto &srv: srvvector) { @@ -1300,7 +1314,8 @@ void MI32ConnectionGetServices(){ */ void MI32ConnectionGetCharacteristics(NimBLERemoteService* pSvc); void MI32ConnectionGetCharacteristics(NimBLERemoteService* pSvc){ - auto charvector = pSvc->getCharacteristics(); // refresh + const bool refresh = MI32.conCtx->response || pSvc->getCharacteristics(false).empty(); + const auto &charvector = pSvc->getCharacteristics(refresh); MI32.conCtx->buffer[1] = charvector.size(); // number of characteristics uint32_t i = 2; for (auto &chr: charvector) { @@ -1479,7 +1494,7 @@ static void MI32RunClientOp(){ if(pChr->canWrite() || pChr->canWriteNoResponse()){ uint8_t len = MI32.conCtx->buffer[0]; if(pChr->writeValue(MI32.conCtx->buffer + 1, len, - MI32.conCtx->response && !pChr->canWriteNoResponse())){ + MI32.conCtx->response ? pChr->canWrite() : !pChr->canWriteNoResponse())){ MI32.conCtx->handle = pChr->getHandle(); } else { MI32.conCtx->error = MI32_CONN_DID_NOT_WRITE; @@ -1489,24 +1504,21 @@ static void MI32RunClientOp(){ } break; case 3: // subscribe - if(MI32.conCtx->oneOp) MI32ReadingDone.store(false, std::memory_order_relaxed); - if(!BLERingBufferQueue){ + case 4: { // unsubscribe + const bool subscribe = MI32.conCtx->operation == BLE_OP_SUBSCRIBE; + if(subscribe && MI32.conCtx->oneOp) MI32ReadingDone.store(false, std::memory_order_relaxed); + if(subscribe && !BLERingBufferQueue){ MI32.conCtx->error = MI32_CONN_CAN_NOT_NOTIFY; break; } if(MI32.conCtx->hasArg1){ - if(pChr->canNotify()){ - if(!pChr->subscribe(true, MI32notifyCB, MI32.conCtx->response)){ - MI32.conCtx->error = MI32_CONN_CAN_NOT_NOTIFY; - } else { - // Mirror the UUID-subscribe path's bookkeeping so Berry receives the - // resolved handle (1-handle payload, big-endian then little-endian - // pair as expected by the existing decoder). - MI32.conCtx->handle = pChr->getHandle(); - MI32.conCtx->buffer[0] = 2; - MI32.conCtx->buffer[1] = pChr->getHandle() >> 8; - MI32.conCtx->buffer[2] = pChr->getHandle() & 0xff; - } + if(!MI32SetSubscription(pChr, subscribe, MI32.conCtx->response)){ + MI32.conCtx->error = MI32_CONN_CAN_NOT_NOTIFY; + } else { + MI32.conCtx->handle = pChr->getHandle(); + MI32.conCtx->buffer[0] = 2; + MI32.conCtx->buffer[1] = pChr->getHandle() >> 8; + MI32.conCtx->buffer[2] = pChr->getHandle() & 0xff; } } else { // refresh=false: getCharacteristics(true) would wipe m_vChars and @@ -1516,20 +1528,20 @@ static void MI32RunClientOp(){ uint32_t position = 1; for(auto &it: charvector){ if(it->getUUID() == MI32.conCtx->charUUID){ - if(it->canNotify()){ - if(!it->subscribe(true, MI32notifyCB, MI32.conCtx->response)){ - MI32.conCtx->error = MI32_CONN_CAN_NOT_NOTIFY; - } else { - MI32.conCtx->buffer[position++] = it->getHandle() >> 8; - MI32.conCtx->buffer[position++] = it->getHandle() & 0xff; - MI32.conCtx->handle = it->getHandle(); - } + if(!MI32SetSubscription(it, subscribe, MI32.conCtx->response)){ + MI32.conCtx->error = MI32_CONN_CAN_NOT_NOTIFY; + } else { + MI32.conCtx->buffer[position++] = it->getHandle() >> 8; + MI32.conCtx->buffer[position++] = it->getHandle() & 0xff; + MI32.conCtx->handle = it->getHandle(); } } } MI32.conCtx->buffer[0] = position - 1; + if(position == 1) MI32.conCtx->error = MI32_CONN_CAN_NOT_NOTIFY; } break; + } default: break; } @@ -1601,7 +1613,7 @@ void MI32ConnectionTask(void *pvParameters){ MI32EnsureServerInstance(pServer); MI32ServerSetCharacteristic(pServer, servicesToStart, shallStartServices); break; - case 1: case 2: case 3: case 5: case 6: case 7: // client op + case 1: case 2: case 3: case 4: case 5: case 6: case 7: // client op MI32RunClientOp(); MI32.mode.triggerBerryConnCB = 1; break; @@ -1680,11 +1692,11 @@ void MI32ServerSetAdv(NimBLEServer *pServer, std::vector& servic for (auto & pService : servicesToStart) { std::vector characteristics = pService->getCharacteristics(); for (auto & pCharacteristic : characteristics) { + if (idx + 1 >= sizeof(item.buffer)) break; // limit to 127 characteristics uint16_t handle = pCharacteristic->getHandle(); // now we have handles, so pass them to Berry //AddLog(LOG_LEVEL_DEBUG,PSTR("BLE: characteristic started %s"),pCharacteristic->toString().c_str()); - item.buffer[idx] = (uint8_t)handle>>8; + item.buffer[idx] = (uint8_t)(handle >> 8); item.buffer[idx+1] = (uint8_t)handle&0xff; - if (idx > 254) break; // limit to 127 characteristics idx += 2; } } @@ -2235,8 +2247,13 @@ void MI32HandleEveryDevice(const NimBLEAdvertisedDevice* advertisedDevice, uint8 _sensor.payload = new uint8_t[64](); } if(_sensor.payload != nullptr) { - memcpy(_sensor.payload, advertisedDevice->getPayload().data(), advertisedDevice->getPayload().size()); - _sensor.payload_len = advertisedDevice->getPayload().size(); + const auto &payload = advertisedDevice->getPayload(); + size_t payload_len = payload.size(); +#ifdef CONFIG_BT_NIMBLE_EXT_ADV + if(payload_len > 63) payload_len = 63; +#endif + memcpy(_sensor.payload, payload.data(), payload_len); + _sensor.payload_len = payload_len; bitSet(MI32.widgetSlot,_slot); MI32addHistory(_sensor.temp_history, 0, 3); // reuse temp_history as sighting history _sensor.RSSI=RSSI; @@ -2293,6 +2310,8 @@ void MI32BLELoop() if(q != nullptr){ if(q->length != 0){ memcpy(MI32.conCtx->buffer,&q->length,q->length + 1); + } else { + MI32.conCtx->buffer[0] = 0; } MI32.conCtx->returnCharUUID = q->returnCharUUID; MI32.conCtx->handle = q->handle;