[FEATURE] Add setUniqueId(const char * uniqueId) to HADevice Class

[JacobChrist]

Issue

I am creating a global instance of a HADevice. If I set the uniqueId to a string it works great, however this is not unique enough if I want to have two or more identical devices connected to HA. It is possible to use my mac address after the fact but a mac address alone makes it hard to identify the device in MQTT Explorer.

Resolution

Create an overload of HADevice::setUniqueId() that takes a char* that allows setting of the uniquId using string + the mac address once it has become avaible.

bool HADevice::setUniqueId(const char* uniqueId)
{
    if (_uniqueId) {
        return false; // unique ID cannot be changed at runtime once it's set
    }

    char* dst = new char[strlen(uniqueId) + 1]; // include null terminator
    strcpy(dst, uniqueId);

    _uniqueId = dst;
    _ownsUniqueId = true;
    _serializer->set(AHATOFSTR(HADeviceIdentifiersProperty), _uniqueId);
    return true;
}

This allows setting my uniqueId as such:

    ///////////////////////////////////////////
    // Set device ID as MAC address
    ///////////////////////////////////////////
    byte mac[WL_MAC_ADDR_LENGTH];
    WiFi.macAddress(mac);
    char mac_string[30];
    snprintf(mac_string,30,"FlowBot-%02x%02x%02x%02x%02x%02x", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5], mac[6]);
    Serial.printf("%s\n", mac_string);
    device.setUniqueId(mac_string);

Which results in a readable MQTT Explorer:

I'm happy to do a pull request for this change if acceptable.

[matthewmcneill]

I think also have problems with this, where I have multiple arduino Nano temperature sensors running the same code. In my case HA will discover the first one, and then HA complains that the other sensors (even with different device IDs i.e. mac address) are not using a unique id for the sensors. I can only view one of them.

2024-08-08 14:54:51.753 ERROR (MainThread) [homeassistant.components.sensor] Platform mqtt does not generate unique IDs. ID cbTemperature already exists - ignoring sensor.cb_iot_ha_01_temperature_c

I want to pre/append the mac to the sensor uniqueid too, but since the constuctor takes a const* char it is very difficult to create IDs dynamically since the String variables need to persist beyond the scope of the local function. I've been getting junk in my MQTT topics due to dangling memory.

[JacobChrist]

EDIT: Note to future readers, don't do this (see below if your curious why its not needed)

@matthewmcneill Did you try my fix? If your having trouble with the string going away you might just be able to make it a static when you declare the string such as this:

    static char mac_string[30];
    snprintf(mac_string,30,"FlowBot-%02x%02x%02x%02x%02x%02x", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5], mac[6]);
    Serial.printf("%s\n", mac_string);
    device.setUniqueId(mac_string);

[matthewmcneill]

Hi Jacob, yes, thanks for this. What I was doing was creating an object structure for ha - I had multiple modbus devices I wanted to handle identically, and so I had to declare and create all these entites in the object initialisation - even before I had a mac address. I also needed to create entity names with global scope in HA to prevent clashes.

Your code does not work easily in the object constructors, so In the end I wrote a small module that stores static strings in a way that they don't change address as a workaround.

See here for more info: #264 (comment)

Your suggested changes to the library are definitely the best way forward IMHO, so that it does not rely on the user to maintain const char* strings.

Matt

[matthewmcneill]

Your solution shoulld be applied to any and every field where string parameters are passed to the library IMHO. Same for the name etc.

[JacobChrist]

@matthewmcneill "so I had to declare and create all these entites in the object initialisation - even before I had a mac address"

Yeah, I was having the same exact problem you were having and I was my work around was to hard code the mac in the uniqueID at construction. This means custom code for every board and having discover the mac address and write it down, alter the code and re-upload. I'm just too lazy for all that madness. I wish you could get the mac address earlier so this solution wasn't needed (and there is probably a way but this seemed like the path of least resistance).

As far as applying it to every field, I don't understand why this would be necessary (yet). I am not a fan of using new in embedded projects, but as long as the memory is never deleted its probably okay.

Also, I was mistaken about my comment above, the static keyword is not needed because the string passed to setUniqueId() is copied into memory allocated with the new. Its been a few weeks since I have looked at this.

I guess I should have posted an expanded picture from MQTT, but this solution does uniquely identify every field in the tree because they are created after setUniqueId() is called. Resulting in the same work around that you were going for to overcome the bug in HA that doesn't see the entire path for a given entity.

I'm using the whole mac address right now, but probably the bottom 8-24 bits are sufficient to prevent crashes (depending on how many devices you want to hang on HA of the same type). I'm not sure if my router could even handle 256 unique devices on WiFi. A quick google search and I saw a comment that most routers can handle 16-32 clients but I think my router can handle up to 255. When I look I currently have 26 clients.

I'm using this with a a Pi Pico and I have purchased them on tape and the mac address pretty much increment by one as you pull them off the tape. The top 24 of the mac's address is the vendor, so they will probably rarely change (unless the vendor burns through 16.7M mac address, which is probably not unreasonable since Rasp Pi foundation appears to have move 61M units so far).

[matthewmcneill]

I used the chip unique ID instead of the MAC address.

String getUniqueChipID() {

#ifdef ARDUINO_ARCH_SAMD
  // Get the unique device ID (for SAMD-based boards)
  uint32_t uniqueID = *(uint32_t*)0x0080A00C;

  // Convert the ID to a string (hexadecimal representation)
  char uniqueIDStr[9]; // 8 hex digits + null terminator
  sprintf(uniqueIDStr, "%08X", uniqueID);

  return String(uniqueIDStr);
#endif

#ifdef ARDUINO_ARCH_ESP32
  uint64_t chipid = ESP.getEfuseMac(); // The chip ID is essentially its MAC address(length: 6 bytes).
  uint16_t chip = (uint16_t)(chipid >> 32);

  // Convert the ID to a string (hexadecimal representation)
  char uniqueIDStr[9]; // 8 hex digits + null terminator
  sprintf(uniqueIDStr, "%08X", chip);

  return String(uniqueIDStr);
#endif

}

Can you explain more on how the setUniqueID works? I had problems when I was using it because it breaks when I need to dynamically create these IDs.

The problem with that is, since all the parameters passed for names and uniqueid's in the homeassistant library are pointers, the strings you need to use have to be globally persistent and unchanging in their pointers. This means that the address of the string has to stay the same throughout the lifetime of the unit. This gets more complex if you are using a c++ object and initialising these entites in the object construction.

For Example:
https://github.com/matthewmcneill/HAIoT_TemperatureSensor/blob/main/home_assistant.h

As such constructing a unique ID like this does not work since the c_str() pointer is ephemeral, and changes to random stuff in nearby memory (like wifi passwords):

        temperature(String(getUniqueChipID() + "_temperature_" + String(clientID)).c_str(), HASensorNumber::PrecisionP1) 

As a workaround I had to write (some ugly) code to set up a static array of strings and store them with immutable pointers.

        temperature(storeStaticString(getUniqueChipID() + "_temperature_" + String(clientID)), HASensorNumber::PrecisionP1) 

Would it not be better for the homeassistant module to make local copies of all these parameters, and have them passed by value, so that we can use simpler client code?

M

[dawidchyrzynski]

@JacobChrist I'm not sure if I'm missing something, but everything you need is already implemented. https://dawidchyrzynski.github.io/arduino-home-assistant/documents/library/device-configuration.html

The method you're looking for is: bool setUniqueId(const byte* uniqueId, const uint16_t length)

@matthewmcneill To run the same code on multiple devices, you need to enable extended unique IDs in the library. This process is already explained in the documentation. https://dawidchyrzynski.github.io/arduino-home-assistant/documents/library/device-types.html

[JacobChrist]

@dawidchyrzynski This looks like you still need to hard code the mac address or use the mac address alone as the uniqueId? I'm reading my mac address out of my radio and it is unavailable during global construction and want to set it with the name of the device (so that its easy to read in mqtt explorer) as my uniqueId. My instantiation of the device looks like this:

WiFiClient client;
HADevice device; // Setting uniqueId to "FlowBot-" + MAC address in HAIntegration::configure()

And as stated above, I'm setting the mac address in HAIntegration::configure() like this:

void HAIntegration::configure() {

    (...snip...)

    ///////////////////////////////////////////
    // Set device ID as MAC address
    ///////////////////////////////////////////
    byte mac[WL_MAC_ADDR_LENGTH];
    WiFi.macAddress(mac);
    char mac_string[30];
    snprintf(mac_string,30,"FlowBot-%02x%02x%02x%02x%02x%02x", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5], mac[6]);
    device.setUniqueId(mac_string);    // [ALTERNATIVE] device.setUniqueId(mac, sizeof(mac));

    (...snip...)

However, it does seem like I'm behind the head of this project in my project because I don't seem to have the enableExtendedUniqueIds() method in my project. I'll will catch up and investigate what you are doing there because I might be not understanding something.

[matthewmcneill]

Hi Jacob,

I'm not having the problem you do. I'm initialising the class etc but setting the uniqueID for the device during a startup routine after the class is initialised and after the wifi is set up.

Have a look at this module here:

• the classes are initialised at the top
• the device's unique ID is set later on the mac address before the mqtt.begin() in the setupHA() at the bottom.

https://github.com/matthewmcneill/HAIoT_SmartMeter/blob/main/home_assistant.h#L247

Matt

[matthewmcneill]

@matthewmcneill To run the same code on multiple devices, you need to enable extended unique IDs in the library. This process is already explained in the documentation. https://dawidchyrzynski.github.io/arduino-home-assistant/documents/library/device-types.html

Ah awesome, I had missed this in the documentation. I have it in the versions of the libraries I am using.

That solves one of my problems.

The context for the second problem is as follows:

• I have an arduino interfacing to 2 meters that communicate over modbus. I may add more.
• Each meter serves 24 different pieces of data.
• I want to set up these meters programmatically and therefore dynamically create Names and UniqueIDs for the entities

e.g.

• each meter has an Active Power register
• I want to create two entities named "[UPS 1] Active Power" and "[UPS 2] Active Power" and so on for each of the 24 values with a uniqueID "activePower_1" and "activePower_2"

Because the library uses the pointer to the .c_str() of any given string, any String composed in the call is ephemeral, so you end up with dangling string pointers. Thus I have to create stable array holders for these strings.

HASensorNumber activePower(String("activePower" + String(modbusID)).c_str(), HASensorNumber::PrecisionP2)

This does not work, since the library does not copy the const* char locally, and the result is that the pointer dangles after the function call and then provides junk to the MQTT broker since it is using a dangling pointer.

Matt