Need help withstruct

[The_Cleaner]

Hi I have broken this code out of a larger code segment, basically I am using struct to define multiple sensors types and store the values. One particular value sTopic stores the value initially but then loses it, all the other values remain intact. To avoid confusion with any sensor information I just used millis() to give me a varying reading for the value.

const char* mqttServer = "192.168.2.1";
const char* mqttHost = "mySensor";
const char* mqttChannel = "Test";


struct Sensor {
    public:
      const char* sType;
      float sValue;
      const char* sMeasurement;
      const char* sTopic;
};

struct Sensor mySensor;

void setup() {
  Serial.begin(115200);
  mySensor.sType = "Temperature";
  mySensor.sValue = 0.00;
  mySensor.sMeasurement = "C";
  byte tTopicSize = strlen(mqttHost) + strlen(mySensor.sType) + 3;
  char tTopic[tTopicSize];
  strcpy(tTopic,mqttHost);
  strcat(tTopic,":");
  strcat(tTopic,mySensor.sType);
  mySensor.sTopic = tTopic;
  Serial.println(mySensor.sTopic);
  Serial.println("--------------");
  delay(10000);
  Serial.println(mySensor.sTopic);
  Serial.println("--------------");  
}

void loop() {
  Serial.println(mySensor.sType);
  Serial.println(mySensor.sValue);
  Serial.println(mySensor.sMeasurement);
  Serial.println(mySensor.sTopic);
  mySensor.sValue = millis();
  delay(10000);
}

The output I get is

17:56:17.970 -> rst:0x1 (POWERON_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
17:56:17.970 -> configsip: 0, SPIWP:0xee
17:56:17.970 -> clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
17:56:17.970 -> mode:DIO, clock div:1
17:56:17.970 -> load:0x3fff0018,len:4
17:56:17.970 -> load:0x3fff001c,len:1100
17:56:17.970 -> load:0x40078000,len:9564
17:56:17.970 -> ho 0 tail 12 room 4
17:56:17.970 -> load:0x40080400,len:6320
17:56:17.970 -> entry 0x400806a8
17:56:18.073 -> mySensor:Temperature
17:56:18.073 -> --------------
17:56:28.099 -> mySensor:Temperature
17:56:28.099 -> --------------
17:56:28.099 -> Temperature
17:56:28.099 -> 0.00
17:56:28.099 -> C
17:56:28.099 -> 
17:56:38.059 -> Temperature
17:56:38.059 -> 10034.00
17:56:38.094 -> C
17:56:38.094 ->

[gfvalvo]

char tTopic[tTopicSize];

This variable is local to setup(). It ceases to exist once that function exits. Yet, you're trying to point mySensor.sTopic at it.

[The_Cleaner]

gfvalvo:

char tTopic[tTopicSize];

This variable is local to setup(). It ceases to exist once that function exits. Yet, you're trying to point mySensor.sTopic at it.

tTopic is only used to contruct the topic vairable before writing it to mySensor.sTopic, it is mySensor.sTopic that is losing its value.

[Blackfin]

The_Cleaner:
tTopic is only used to contruct the topic vairable before writing it to mySensor.sTopic, it is mySensor.sTopic that is losing its value.

You don't write "it" to mySensor.sTopic; that's a pointer so you're just assigning the address of tTopic.

As gfvalvo noted, once setup() is done, tTopic ceases to mean anything and the memory to which the pointer points can (and likely is) being used for other things.

[gfvalvo]

Nope, mySensor.sTopic is a pointer and it's keeping its value. However, the array it's pointing to is getting overwritten when setup() exits.

[The_Cleaner]

Blackfin:
You don't write "it" to mySensor.sTopic; that's a pointer so you're just assigning the address of tTopic.

As gfvalvo noted, once setup() is done, tTopic ceases to mean anything and the memory to which the pointer points can (and likely is) being used for other things.

Forgive my ignorance but does mySensor.sTopic = tTopic; not store the value of tTopic into mySensor.sTopic?

And why do I see the correct value returned when I use Serial.println(mySensor.sTopic); if the value is not stored?

[The_Cleaner]

gfvalvo:
Nope, mySensor.sTopic is a pointer and it's keeping its value. However, the array it's pointing to is getting overwritten when setup() exits.

What is the correct way to store the content of tTopic into mySensor.sTopic rtaher than the pointer address?

I guess that is what is confusing me.

[Blackfin]

The_Cleaner:
Forgive my ignorance but does mySensor.sTopic = tTopic; not store the value of tTopic into mySensor.sTopic?

No.

In your structure you have sTopic declared as:

const char* sTopic

which is basically saying that "sTopic" is a pointer (*) to a character (char). In addition, you've also declared it as "const" which is technically not changeable (do you get any warnings from the compiler if you turn on verbose messages...)

Anyway, the assignment mySensor.sTopic = tTopic just assigns the address of tTopic to the sTopic member.

And why do I see the correct value returned when I use Serial.println(mySensor.sTopic); if the value is not stored?

The Serial.printlns that occur within setup() -- while tTopic is still valid, will work fine.

When you leave setup(), the character array tTopic goes out of scope and its memory just becomes free for other use. It's actually kind of dangerous (from a coding and predictable operation perspective) to dereference what is now an invalid pointer.

[gfvalvo]

I'd go with:

struct Sensor {
    public:
      const char sType[typeSize];
      float sValue;
      const char sMeasurement[measurementSize];
      const char sTopic[topicSize];
};

[gfvalvo]

gfvalvo:
I'd go with:

struct Sensor {

public:
      const char sType[typeSize];
      float sValue;
      const char sMeasurement[measurementSize];
      const char sTopic[topicSize];
};

BTW, the "public:" specifier is superfluous. Elements of a struct are public by default.

[The_Cleaner]

@blackfin, thankyou for the detailed response, I have programmed for many years but never in C so this is a learning curve, and I appreciate the effort put into replying. I was aware of of using const char and it not bing changeable, and that was the desire.

@gfvalva, thankyou for your suggestion, I tried before to predefine the sizes in my struct and could not get it to work, most likely bad fromatting on my part, however, using your format, I now get the error "use of deleted function 'Sensor::Sensor()' for my line of code struct Sensor mySensor;
I was unsure about the public: definition and as it did not seem to break anything I had left it in place, I will remove it.

[gfvalvo]

You need to post a complete code. If your full code is too large and full of unrelated stuff, post an MCVE. This is the smallest possible complete code that reproduces the EXACT compiler error you're seeing.

[Blackfin]

Which Arduino are you using?

[The_Cleaner]

@blackfin, I am using ESP32 board, Arduino IDE 1.8.10

@gfvalvo, the code is the same as the inital post except for changing the struct definition per yor suggection, I have not changed the rest of the code but have recopied it all below

const char* mqttServer = "192.168.2.1";
const char* mqttHost = "mySensor";
const char* mqttChannel = "Test";


struct Sensor {
      const char sType[15];
      float sValue;
      const char sMeasurement[5];
      const char sTopic[30];
};

struct Sensor mySensor;

void setup() {
  Serial.begin(115200);
  mySensor.sType = "Temperature";
  mySensor.sValue = 0.00;
  mySensor.sMeasurement = "C";
  byte tTopicSize = strlen(mqttHost) + strlen(mySensor.sType) + 3;
  char tTopic[tTopicSize];
  strcpy(tTopic,mqttHost);
  strcat(tTopic,":");
  strcat(tTopic,mySensor.sType);
  mySensor.sTopic = tTopic;
  Serial.println(*tTopic);
  Serial.println(mySensor.sTopic);
  Serial.println("--------------");
  delay(10000);
  Serial.println(mySensor.sTopic);
  Serial.println("--------------");  
}

void loop() {
  Serial.println(mySensor.sType);
  Serial.println(mySensor.sValue);
  Serial.println(mySensor.sMeasurement);
  Serial.println(mySensor.sTopic);
  mySensor.sValue = millis();
  delay(10000);
}

[gfvalvo]

My bad, forgot to remove the const:

struct Sensor {
  char sType[15];
  float sValue;
  char sMeasurement[5];
  char sTopic[30];
};

Also, statements like this won't work:

mySensor.sTopic = tTopic;

Use strcpy:

strcpy(mySensor.sTopic, tTopic);

[The_Cleaner]

Thank-you, this has been really helpful and has solved the problem, at least in my example program, now I have to apply it to the bigger project.

It was leaving the const in the structure that I could not figure out myself previously when trying to predfine the array sizes.

[Blackfin]

If you don't like the idea of fixed array sizes in your structure you can use dynamic memory allocation. For example:

#include <stdlib.h>

const char* mqttServer = "192.168.2.1";
const char* mqttHost = "mySensor";
const char* mqttChannel = "Test";


struct Sensor 
{
    char *sType;
    float sValue;
    char *sMeasurement;
    char *sTopic;
};

struct Sensor mySensor;

void setup( void )
{
    Serial.begin(115200);
    
    mySensor.sType = (char *)malloc( strlen( "Temperature" ) + 1 );
    if( mySensor.sType != NULL )
        strcpy( mySensor.sType, "Temperature" );
        
    mySensor.sValue = 0.00;

    mySensor.sMeasurement = (char *)malloc( strlen( "C" ) + 1 );
    if( mySensor.sMeasurement != NULL )
        strcpy( mySensor.sMeasurement, "C" );
    
    mySensor.sTopic = (char *)malloc( strlen(mqttHost) + strlen(mySensor.sType) + 3 );
    if( mySensor.sTopic != NULL )
    {
        strcpy( mySensor.sTopic, mqttHost );
        strcat( mySensor.sTopic, ":" );
        strcat( mySensor.sTopic, mySensor.sType );

    }//if
    
    Serial.println( mySensor.sTopic );
    Serial.println( "--------------" );
    delay(10000);
    Serial.println( mySensor.sTopic );
    Serial.println( "--------------" ); 
    
}//setup

void loop( void )
{
    Serial.println( mySensor.sType );
    Serial.println( mySensor.sValue );
    Serial.println( mySensor.sMeasurement );
    Serial.println( mySensor.sTopic );
    
    mySensor.sValue = millis();
    
    delay(10000);
    
}//loop

You can technically use malloc() to assign heap (i.e. global) memory to the strings you want and leave only pointers in your structure. For example, in the above:

   mySensor.sType = (char *)malloc( strlen( "Temperature" ) + 1 );
    if( mySensor.sType != NULL )
        strcpy( mySensor.sType, "Temperature" );

malloc should return a pointer to a block of heap of type char reserved with a size of the length of "Temperature" plus one (for the NULL termination...) and assigns it to the sTopic member of the struct.

If the call fails, it will return NULL. You probably want to assert a condition in this even to halt program execution or otherwise "safe" the system.

If the return value is not NULL, it's a valid pointer to the char array in the heap. That will exist outside the scope of setup() and you can use it globally.

Note: Some folks will probably bellyache about the "dangers" of using malloc() in small-memory devices and to an extent, they'd have a point if you're doing lots of malloc/free calls that can quickly fragment and chew up heap space.

If you just need to do this once at setup(), you're probably safe.

[The_Cleaner]

Thank-you, I will look more into your suggestion if memory becomes an issue.

I have 12 sensors some of which will not alsways be used (depending on where the setup is deployed) but I wanted to be able to specify via variables whether to include the code, this part I have working.

And thanks to the help from yourself and @gfvalvo my struct is now correctly defined and working.