Changing WebUI to support end-user setting display BLE advertised device name vs current model_id

[plambrechtsen]

Changing WebUI to include display device name as a parameter.

Description:

When a device is created in Home Assistant using automatic discovery it is named by the model_id. This can be changed using the build parameter ForceDeviceName but this change introduces this as a setting in WebUI. So now devices can be named by their BLE advertised device name rather than the model_id.
Also added a xxWSD0xMMCDiscovery DiscoveryFromList including the RSSI so that all values are named nicely by default when the displayDeviceName flag is true.
When the flag is false the existing behavior is unchanged.

In Home Assistant the devices will be named as well as the entities by either their model_id or nevice name

Model ID	Device Name

Checklist:

• The pull request is done against the latest development branch
• Only one feature/fix was added per PR and the code change compiles without warnings
• I accept the DCO.

[DigiH]

I'll leave the final review to @1technophile, and I only skimmed over this as I'm getting ready for some travels tomorrow ;) but just a few comments.

Also changed "Display Metric" to "Display temperature" with a select drop down as that makes far more sense to me.

I think this should definitely stay Metric and Imperial (Metric false). While temperature with Celsius and Fahrenheit is definitely the most common occurrence of this distinction, there are also other units like length in millimetres or inches for the Mopeka sensors for example, for which a temperature setting wouldn't make sense at all. Also for consistency, as the runtime MQTT command to change this setting is aptly called {"displayMetric":true/false}

Any published JAON should always still keep both Metric and Imperial properties, and this setting is only really applicable to BLE and RTL_433 devices being displayed in the WebUI display and the LilyGo OLED display it mimics.

I hope this makes sense.

[1technophile]

I agree with @DigiH we should keep the setting as Metric/Imperial with Metric being the default.

[plambrechtsen]

Hi @1technophile and @DigiH I have cleaned up this PR to just be the Display Name drop down option in the WebUI. Hopefully the screenshots clarify the reasoning behind making the change. ☺️

[plambrechtsen]

Something I have noticed @1technophile with the LYWSD03MMC in BTHome mode when I changed the tag to "cots" so that it will enable continuous scan as well as enabling adaptive scan then this logic is followed. https://github.com/1technophile/OpenMQTTGateway/blob/development/main/gatewayBT.cpp#L1420-L1436
What happens is the stateBTMeasures(false); is called, and I have been getting a stack overflow segfault.

I think I have narrowed it down to

N: Passive continuous scanning required, parameters adapted
***ERROR*** A stack overflow in task procBLETask has been detected.

Backtrace: 0x4008344d:0x3ffebc30 0x400960d9:0x3ffebc50 0x40097065:0x3ffebc70 0x400981b9:0x3ffebcf0 0x40097204:0x3ffebd20 0x400971b4:0x3ffebd50 0x4012c5dd:0x3ffca314 |<-CORRUPTED

ELF file SHA256: 874b75fd5

Guru Meditation Error: Core  1 panic'ed (Unhandled debug exception).
Debug exception reason: Stack canary watchpoint triggered (procBLETask) 
Core  1 register dump:
PC      : 0x401712d0  PS      : 0x00060236  A0      : 0x8017030e  A1      : 0x3ffeb9e0
A2      : 0x3ffebb0c  A3      : 0x00000000  A4      : 0x000007d0  A5      : 0x60000000  
A6      : 0x3ffedea0  A7      : 0x3ffc9db8  A8      : 0x00000022  A9      : 0x3ffeba04
A10     : 0x00000000  A11     : 0x3ffeba7c  A12     : 0x00000001  A13     : 0x3ffeba88  
A14     : 0x3ffedf18  A15     : 0x00000000  SAR     : 0x0000000a  EXCCAUSE: 0x00000001
EXCVADDR: 0x00000000  LBEG    : 0x401712d0  LEND    : 0x401712d4  LCOUNT  : 0x00000021  


Backtrace: 0x401712cd:0x3ffeb9e0 0x4017030b:0x3ffebab0 0x40136895:0x3ffebad0 0x40135afd:0x3ffebb00 0x400829e9:0x3ffebb50 0x4008507c:0x3ffebb70 0x4008344d:0x3ffebc30 0x400960d9:0x3ffebc50 0x40097065:0x3ffebc70 0x400981b9:0x3ffebcf0 0x40097204:0x3ffebd20 0x400971b4:0x3ffebd50 0x4012c5dd:0x3ffca314 |<-CORRUPTED

ELF file SHA256: 874b75fd5

Re-entered core dump! Exception happened during core dump!
Rebooting...
ets Jun  8 2016 00:22:57

rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:2
load:0x3fff0030,len:4688
load:0x40078000,len:15448
load:0x40080400,len:4
ho 8 tail 4 room 4
load:0x40080404,len:3196
entry 0x400805a4
N: 
************* WELCOME TO OpenMQTTGateway **************

I think it's because when the enqueueJsonObject(jo, QueueSemaphoreTimeOutTask); is called at https://github.com/1technophile/OpenMQTTGateway/blob/development/main/gatewayBT.cpp#L169 because another JSON_MSG_BUFFER sized buffer is allocated the ESP32 runs out of ram.

I am using a Wemos D1 ESP32.

PLATFORM: Espressif 32 (53.3.11+sha.ccf2dba) > Espressif ESP32 Dev Module
HARDWARE: ESP32 240MHz, 320KB RAM, 4MB Flash
DEBUG: Current (cmsis-dap) External (cmsis-dap, esp-bridge, esp-prog, iot-bus-jtag, jlink, minimodule, olimex-arm-usb-ocd, olimex-arm-usb-ocd-h, olimex-arm-usb-tiny-h, olimex-jtag-tiny, tumpa)
PACKAGES:
 - framework-arduinoespressif32 @ 3.1.1
 - framework-arduinoespressif32-libs @ 5.3.0+sha.cfea4f7c98
 - tool-esptoolpy @ 4.8.5
 - tool-mkfatfs @ 2.0.1
 - tool-mklittlefs @ 3.2.0
 - tool-mkspiffs @ 2.230.0 (2.30)
 - tool-riscv32-esp-elf-gdb @ 14.2.0+20240403
 - tool-xtensa-esp-elf-gdb @ 14.2.0+20240403
 - toolchain-xtensa-esp-elf @ 13.2.0+20240530
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 50 compatible libraries
Scanning dependencies...
Dependency Graph
|-- PicoMQTT @ 1.1.1
|-- ArduinoJson @ 6.18.5
|-- ArduinoLog @ 1.0.3+sha.f634509
|-- WiFiManager @ 2.0.17+sha.1ac24f4
|-- NimBLE-Arduino @ 2.3.2+sha.67be40c
|-- TheengsDecoder @ 1.9.9+sha.79346df
|-- LEDManager
|-- EEPROM @ 3.1.1
|-- Ethernet @ 3.1.1
|-- WiFi @ 3.1.1
|-- SPI @ 3.1.1
|-- TheengsUtils
|-- Wire @ 3.1.1
|-- FS @ 3.1.1
|-- SPIFFS @ 3.1.1
|-- ArduinoOTA @ 3.1.1
|-- DNSServer @ 3.1.1
|-- ESPmDNS @ 3.1.1
|-- HTTPClient @ 3.1.1
|-- HTTPUpdate @ 3.1.1
|-- Preferences @ 3.1.1
|-- NetworkClientSecure @ 3.1.1
|-- WebServer @ 3.1.1

Commenting out the stateBTMeasures call as I don't think it's actually needed resolves(ish) the problem.

[DigiH]

It should likely also be noted in the Decoder documentation page for the LYWSD03MMC/MJWSD05MMC_ that if a custom name is chosen during the firmware installation with the BTHome v2 format, that the name must start with "ATC", as otherwise the decoder will not recognise the devices.

Personally, I always found the model_id with the amended three octets of the MAC address ideal for uniquely identifying devices, as a very quick edit in Home Assistant allows for renaming of the title as well as all the properties to proper individual fully written out labels, e. g. for my Inkbird TH1

As most users will likely also rename these labels as well even when the discovery naming has been changed to use the broadcast name, this change is only really applicable for device identifiication.

Not having had a proper look at the code yet, how does this setting affect devices which do not broadcast a name at all, or which do broadcast a name, but generally only during an active scan, but they are already recognised by the decoder during passive scanning and therefor discovered during passive scanning with their model_id, when intervalacts is higher than interval. This would then appear for the setting to not work properly.

These considerations are definitely something to test out with this new settings option, as not to break any discovery.

Also with your exception mentioned above, did you manually set iwntervalacts to the same short interval as interval when switching to continuous scanning and/or adaptive scanning, as adaptive/continous scanning would not have done that automatically as the LYWSD03MMC definitely requires active scanning, but by no means continuous scanning, but definitely requires an active scan to be able to get a broadcast name if this setting is set to discovery with the name … :)

All without me really looking properly at the code, but with active scanning required to get a broadcast device name, and using this broadcast name for initial auto-discovery I think there will be a few more safeguards required to make this setting work correctly and smoothly in all possible scanning situations for all users.

[DigiH]

To shorten my ramblings above ;)

• This setting will only reliably work for devices which are defined as requiring active scanning to be recognised by Decoder. That is roughly 1/3 of decoders.

• It might also work for devices which are recognised through passive scanning by Decoder, only if and when intervalacts is set to the same value as interval and if they do broadcast a name during active scanning. Otherwise they will still be discovered with their model_id.

• Devices which never broadcast any name will always be discovered by their model_id.

[plambrechtsen]

Hi @1technophile and @DigiH I think this PR is ready to go. It now supports displayDeviceName as a drop down in the WebUI and it doesn't change the default behaviour but once you change over in the WebUI then when the OMG is rebooted the naming of the devices changes to the bluetooth value.
Also created a xxWSD0xMMCDiscovery DiscoveryFromList add added in rssi so there is nothing I need to change in OMG or HA when the device is detected.
Last change I made was to comment out stateBTMeasures when active or continuous scanning is enabled. As it causes a stack overflow as per the above.

[DigiH]

@plambrechtsen How did you address the concerns above about this possibly only reliably working with 30% of devices? Do you have other BLE devices which allow for passive scanning you can test this with?

I'm also a bit confused about the new discovery addition. Individual device discoveries should only really exist for devices which have additional connection only properties, but not for an rssi discovery of just one single device (group). AFAIK it was a deliberate decision not to discover rssi, but if this changed it should be properly implemented for all devices. Currently, adding an rssi discovery, or any non-decoder included property of other devices, can easily be achieved by manually publishing such discovery messages.

@1technophile will be better suited to fully comment on this though

[1technophile]

typo instead

[1technophile]

I suggest to increase the stack size of the task:

OpenMQTTGateway/main/gatewayBT.cpp

Line 873 in ddb9cc6

9500, /* Stack size in bytes */

and see if you still get the issue

[1technophile]

Same a s above

[1technophile]

@DigiH Are you okay with this ?

[DigiH]

Just an afterthought?

Has this been tested against the common occurrence of users having lots of devices already discovered with the previous model_id entity names , placed on dashboards and used in automations. Now with the change to broadcast name, would the new discovery-entries now not create new entity names, for applicable devices, breaking all the previous ones because of the new uniq_id?

Just some theoretical scenario thinking again from me :)