Dynamically created property name, format, unit

[sovcik]

My app creates nodes dynamically based on provided configuration file and I was struggling for some time with strange node/property names. Then I realized that while node/property id is copied using strdup, node/property name, unit, datatype, format are only referenced

Example of problematic code:

for(int j=0;j<NUMBER_OF_VALVES;j++){
    String vid = valves[j]->getIdStr() + String("rt");
    String vin = valves[i]->getIdStr() + String(" Runtime");
    prg_node[i]->advertise(vid.c_str()).setName(vin.c_str()).setDatatype("integer").setFormat("0:120").settable();
}

In above code:

• setting property id works without problem
• setting property name is an issue as string buffer is released after finishing the cycle

I assume the reason is saving memory by avoiding string duplication.

Question
Would it be OK to copy all strings to achieve more consistent behavior? And for memory limiting situations implement e.g. setName with progmem signature, so developer can store strings to progmem and setName will copy them from there?

I can create PR for that.

[RunningPenguin]

You used:
vid = valves[j]
and
vin = valves[i]
are you sure the index "i" and "j" are correctly used and you didn't mix them up?

[sovcik]

Sorry, typo in provided example. Otherwise it wouldn't compile, right? :-)
The issue is clearly visible in reference source code.

[RunningPenguin]

perhaps you could use a temporary buffer and strcpy/strcat functions to build your "vin" string. I have done this here for the id but I think it should work for the name too.
Of course you have to write some more lines of code but perhaps it's easier than changing the behavior of the advertise function.

char tempIdBuffer[254] = {0};
char tempAdvertiseIndexStr[10] = {0};
uint8_t readableInvId = 0;

for (uint8_t j = 0; j < number_of_inverters_to_advertise; j++)
   {
       readableInvId = j + 1;                                               //because humans count most of the time from 1 onwards not from 0
       memset(&tempAdvertiseIndexStr[0], 0, sizeof(tempAdvertiseIndexStr)); //zero out all buffers we could work with "messageToDecode"
       delayMicroseconds(250);                                              //give memset a little bit of time to empty all the buffers
       itoa(readableInvId, tempAdvertiseIndexStr, 10);
       delayMicroseconds(250); //give memset a little bit of time to empty all the buffers

       //Frequency
       memset(&tempIdBuffer[0], 0, sizeof(tempIdBuffer)); //zero out all buffers we could work with "messageToDecode"
       delayMicroseconds(250);                            //give memset a little bit of time to empty all the buffers
       strncpy(tempIdBuffer, cPropFrequency, strlen(cPropFrequency));
       delayMicroseconds(250); //give memset a little bit of time to empty all the buffers
       strncat(tempIdBuffer, tempAdvertiseIndexStr, sizeof(tempAdvertiseIndexStr));
       delayMicroseconds(250); //give memset a little bit of time to empty all the buffers
       Homie.getLogger() << "_advertise tempIdBuffer 1: " << tempIdBuffer << endl;
       advertise(tempIdBuffer).setName("Frequency").setDatatype("float").setFormat("%.2f").setUnit("Hz").settable();
   }

[sovcik]

Well, actually not possible that way. You created tempbuffer for property id, which is being "copied" and so temporary buffering is possible. The problem with e.g. property name is that you have to pass buffer/variable which will be available during the property life-time.