Go Down

Topic: more with char arrays and class functions (Read 1 time) previous topic - next topic

GoForSmoke

I got to find out that this version IDE typecasts more retentively than my old one.

1) http://gammon.com.au/blink  <-- tasking Arduino 1-2-3
2) http://gammon.com.au/serial <-- techniques howto
3) http://gammon.com.au/interrupts
Your sketch can sense ongoing process events in time.
Your sketch can make events to control it over time.

petedd

Well, as usual, I am trying to take things further and hit a road block.  I clearly am missing a fundamental here...

We ended up with some great code for creating different-sized char arrays (buffers) and setting different streams for some class objects.   Now I want to do something else where all I need to do is create different-sized char arrays and will not need to set the streams.  Here is the code that creates the template class (focus on the first few lines)...

Code: [Select]
template<size_t N>
class Messages {
  public:
    Messages(Stream &str) : charStream(str), m_dataBufferSize(N) {
    }
    void setBuffer(const char *inString);
    void printBuffer();
    const char *getBufferPointer();
    size_t getStringLen();
    size_t getBufferSize();

  private:
    Stream &charStream;
    size_t m_dataBufferSize;
    char m_dataBuffer[N + 1] = {'\0'}; // Make sure there's always a null character at end of buffer
};


template<size_t N>
void Messages<N>::setBuffer(const char *inString) {
  strncpy(m_dataBuffer, inString, m_dataBufferSize);
}

template<size_t N>
void Messages<N>::printBuffer() {
  charStream.print(m_dataBuffer);
}

template<size_t N>
size_t Messages<N>::getBufferSize() {
  return (m_dataBufferSize);
}

template<size_t N>
size_t Messages<N>::getStringLen() {
  return strlen(m_dataBuffer);
}

template<size_t N>
const char * Messages<N>::getBufferPointer() {
  return m_dataBuffer;
}


Messages<80> GPSbufferSeed(Serial);
Messages<18> IMUbufferSeed(Serial);

void setup() {
  const char *ptr;

  Serial.begin(115200);
  delay(1000);

  Serial.print("GPSbufferSeed Buffer Size = ");
  Serial.println(GPSbufferSeed.getBufferSize());
  Serial.print("GPSbufferSeed String Size = ");
  Serial.println(GPSbufferSeed.getStringLen());
  GPSbufferSeed.setBuffer("Hello World");
  Serial.print("GPSbufferSeed Buffer String = ");
  GPSbufferSeed.printBuffer();
  Serial.println();
  ptr = GPSbufferSeed.getBufferPointer();
  Serial.print("GPSbufferSeed Buffer String = ");
  Serial.println(ptr);
  Serial.print("GPSbufferSeed String Size = ");
  Serial.println(GPSbufferSeed.getStringLen());

  Serial.println();
  Serial.println();

  Serial.print("IMUbufferSeed Buffer Size = ");
  Serial.println(IMUbufferSeed.getBufferSize());
  Serial.print("IMUbufferSeed String Size = ");
  Serial.println(IMUbufferSeed.getStringLen());
  IMUbufferSeed.setBuffer("Test 123");
  Serial.print("IMUbufferSeed Buffer String = ");
  IMUbufferSeed.printBuffer();
  Serial.println();
  ptr = IMUbufferSeed.getBufferPointer();
  Serial.print("IMUbufferSeed Buffer String = ");
  Serial.println(ptr);
  Serial.print("IMUbufferSeed String Size = ");
  Serial.println(IMUbufferSeed.getStringLen());
}

void loop() {
}


Now what if I want to take the stream setting out of this part:

Code: [Select]
template<size_t N>
class Messages {
  public:
    Messages(Stream &str) : charStream(str), m_dataBufferSize(N) {
    }


So I want to just set the objects up for varying m_dataBufferSize   So I try this:

Code: [Select]
template<size_t N>
class Messages {
  public:
    Messages m_dataBufferSize(N) {
    }

//..

};


and this as the constructor, for example:

Code: [Select]

Messages<80> GPSbufferSeed();


and I get the following compile-time error:  'N' is not a type
Pointing to this line:

   Messages m_dataBufferSize(N) {

I've tried a variety of variants and can't hit on the right code for this.  Clearly I am missing something very fundamental here and am eager to learn... after over two hours of working on this one little point...

Any pointers to what I should be studying to better understand this would be very appreciated.

petedd

Well, I got my own answer... I found this tutorial...I did not know I was looking for "non-type parameters"

https://www.learncpp.com/cpp-tutorial/134-template-non-type-parameters/

and then I wrote this demonstration code.

Code: [Select]


template <size_t arraysize> // size is the non-type parameter
class MakeArray
{
  public:
    MakeArray () {
      m_arraysize = arraysize;
    }
    void SetArray(const char *InputString);
    void PrintArray();

  private:
    size_t m_arraysize;
    char m_array[arraysize + 1] = {'\0'}; // Make sure there's always a null character at end of array
};



template <size_t arraysize>
void MakeArray<arraysize>::SetArray(const char *InputString) {
  strncpy(m_array, InputString, m_arraysize);
}

template <size_t arraysize>
void MakeArray<arraysize>::PrintArray() {
  Serial.println(m_array);
}

//create objects
MakeArray<5> SmallArray;
MakeArray<20> LargeArray;

void setup() {

  SmallArray.SetArray("12345");
  LargeArray.SetArray("12345678901234567890");

  Serial.begin(115200);
  delay(1000);
  Serial.println("Serial ready");

  SmallArray.PrintArray();
  LargeArray.PrintArray();
}

void loop() {
}

PieterP

A better answer, more in line with what you had before, would be
Code: [Select]
template <size_t ArraySize>
class MakeArray {
  public:
    MakeArray() : m_arraysize(ArraySize) {}
  private:
    size_t m_arraysize;
};

Or even better, use default member initialization, so you don't forget to initialize it when adding more constructors in the future:

Code: [Select]
template <size_t ArraySize>
class MakeArray {
  public:
    // no user-defined constructor here
  private:
    size_t m_arraysize = ArraySize;
};


That being said, I see no reason why you would store it as a member variable in the first place, you already have access to the template parameter. By saving it as a copy in a member variable, you're just wasting 4 bytes of RAM for each instance of your class.

petedd

Or even better, use default member initialization, so you don't forget to initialize it when adding more constructors in the future:


Hi Pieter,

Good point as usual regarding m_arraysize.   I agree that I can get away without the m_arraysize member variable and I fixed that.  Thank you.

With regards to your comment about initializing (above), I think I might be encountering that as I try to move from my example code to my real code.   I have tried to reduce the problem down to the following snippet.   What is happening is that I am getting an "expected initializer before '<' token" error when I try to define the UpdateDisplayField and SetNewValue methods outside of the class declaration.   Compiles fine if I delete these function definitions.

Code: [Select]




//FieldNumber

#define fnAV 1  // Alternator Volts
#define fnBV 2  // Battery Volts

//Justify
#define RJ 1 //Right
#define LJ 2 //Left
#define CJ 3 //Center

//Column (X) variable is number of pixels
#define screen2ColAlt     120
#define screen2ColBat     220

//Top row (Y) is 0
#define Y_Pixels_12pt  21  //without descenders, Theight is 18, with descenders (used in lower case letters), tHeight is 23
#define Y_Pixels_18pt  31  //without descenders, Theight is 26, with descenders (used in lower case letters), tHeight is 34

#define screen2RowTop       1 * Y_Pixels_18pt
#define screen2RowVolt      2 * Y_Pixels_18pt
#define screen2RowAmp       3 * Y_Pixels_18pt

// Class
template <size_t arraysize>
class LCDfield {
  public:
    //constructors
    //LCDfield(uint8_t object, int x,  int y, const GFXfont *Font, uint8_t just)
    LCDfield( int x,  int y, int Font, uint8_t just)
    {
      m_X = x;
      m_Y = y;
      m_Font = Font;
      m_Just = just;
      m_arraySize = arraysize;
    }

    void SetNewValue(const char* newValue);
    void UpdateDisplayField();

  private:
    size_t        m_arraySize;
    char          m_newValue[arraysize + 1] = {'\0'}; // Make sure there is always a null character at the end
    int           m_X, m_Y; // This is the cursor reference position (for example, the center of the string if using Center Justify)
    int           m_X1, m_Y1;  // Used for position of fillRect to write over last string printed
   // const         GFXfont *m_Font;
    int     m_Font;
    uint8_t       m_Just; // Specifies the text Justification (position relative to m_X, m_Y, as Right, Left, or Center)
    int           m_LLX, m_LLY; //corrected cursor position for the start of the string - Proportional Fonts use Lower Left for origin of string
    unsigned int  m_Twidth, m_Theight;
}; // don't forget the semicolon at the end of the class

template <size_t arraysize>
void LCDField<arraysize>::SetNewValue(const char* newValue) {
  strncpy(m_newValue, newValue, m_arraySize);
}


template <size_t arraysize>
void LCDField<arraysize>::UpdateDisplayField(void) {
  Serial.println(m_newValue);  //this is just dummy code
}//Update()


//LCDfield Screen 2 Objects                                             (Optional)
//construct instances  column,           row,               Font       Justification
LCDfield <5>fAV{       screen2ColAlt,    screen2RowVolt,   27,          LJ  };
LCDfield <5>fBV{       screen2ColBat,    screen2RowVolt,   28,         LJ  };


void setup() {
  // put your setup code here, to run once:

}

void loop() {
  // put your main code here, to run repeatedly:

}


Since this code closely follows the working demonstration code I posted earlier and subsequent versions of that code in which I passed variables to the methods which I defined outside of the class declaration, I am at a loss as to why I am throwing this error.  (I imagine the problem is obvious to you...)

PieterP

LCDfield != LCDField


Also, macros are evil. Never use them to define constants, only use them for conditional compilation. Use constants or enumerations instead:

Code: [Select]
const unsigned int screen2ColAlt = 120;
Or
Code: [Select]
constexpr unsigned int screen2ColAlt = 120;

Code: [Select]
enum Field {
  fnAV = 1,  // Alternator Volts
  fnBV = 2,  // Battery Volts
};

Preferably, use an enum class, so the enumerators are scoped and more strongly typed:

Code: [Select]
enum class Field {
  fnAV = 1,  // Alternator Volts
  fnBV = 2,  // Battery Volts
};

// In your code, use Field::fnAV,
// and pass them using the Field type instead of int

Do the same for the other macros.


petedd

Doh!   I can't believe I missed that.  LCDfield/LCDField...  I need to take more breaks...

I always appreciate your insights Pieter.  Could you tell me more on why macros "are evil"?  Do I burn any more or less memory space by using constants and enumerated instead?

(40+ years ago, at Georgia Tech, I did undergrad, grad, and post grad classes and projects and had access to some of the best development tools of that time... even one or two 8" floppy disks to store my work with Motorola 6800 series (not 68000) microprocessors.  The Data General machines I learned driver design on had a few K of magnetic core memory, if that, and everything was done in assembly code.  We had to switch flick the code we wrote into the front panel in order to enter the drivers we wrote for a paper tape reader so we could advance to using an ASR-33 to type our code to paper tape...  it all taught me to be very stingy with memory and I continue to try to learn the most efficient approaches.   Also, there are so many ways to set up class members in C++... I try to use the most blatant for easier readability (for me) but want to get better at using other approaches.)

PieterP

I always appreciate your insights Pieter.  Could you tell me more on why macros "are evil"?
Because macros are completely invisible to the compiler, they are just "stupid" text search and replace operations that happen before your code is passed to the compiler. This means that there are no checks, no type system, etc.

It also makes it hard for the programmer to see what's going on, having to expand possibly multiple layers of macros in your head. It's easy to forget to add parentheses around the operands or results, etc.

Finally, macros aren't scoped. If two libraries happen to use the same macro name, everything breaks, even if the macro is only used at a local scope or inside of a namespace block. This is a consequence of macros being invisible to the compiler.
If you use constants, they follow the scope and namespace rules of the language, and they won't clash if two constants in a different scope or namespace happen to have the same name.

Quote
Do I burn any more or less memory space by using constants and enumerated instead?
Macros will not save memory. Modern compilers are smart, if you're not using a constant, it'll optimize it out, it doesn't matter if it's a macro or a const/constexpr variable.

petedd

Thanks for that very good explanation.

petedd

#39
Oct 08, 2020, 05:57 am Last Edit: Oct 08, 2020, 06:16 am by petedd
So now things are looking up but I have another problem.  I construct my template class objects and then I construct a list of those objects (*Screen2Objects) so I can step though each of the objects in a for loop later.  I am not sure how to specify the non-type field (arraysize) in this case.  The compiler seems to want the arraysize for Screen2Objects to match each of the objects but since there are several sizes, that is not going to happen.   Is there a wildcard way to do this?  (only the relevant lines are shown here)

Code: [Select]

template <size_t arraysize>
class LCDfield {

  public:
    //constructors
    //constructor with justification field
    LCDfield( int x,  int y, const GFXfont *Font, uint8_t just=LJ)
    {
      m_X = x;
      m_Y = y;
      m_Font = Font;
      m_Just = just;
    }
 
    
    void SetNewValue(const char* newValue);
    void UpdateDisplayField();

  private:
    char          m_newValue[arraysize + 1] = {'\0'}; // Make sure there is always a null character at the end
    int           m_X, m_Y; // This is the cursor reference position (for example, the center of the string if using Center Justify)
    int           m_X1, m_Y1;  // Used for position of fillRect to write over last string printed
    const         GFXfont *m_Font;
    uint8_t       m_Just; // Specifies the text Justification (position relative to m_X, m_Y, as Right, Left, or Center)
    int           m_LLX, m_LLY; //corrected cursor position for the start of the string - Proportional Fonts use Lower Left for origin of string
    unsigned int  m_Twidth, m_Theight;
}; // don't forget the semicolon at the end of the class

//...

/create objects

LCDfield <5>fAV{       screen2ColAlt,    screen2RowVolt,   &FreeSans18pt7b,  LJ  };
LCDfield <5>fBV{       screen2ColBat,    screen2RowVolt,   &FreeSans18pt7b,  LJ  };
LCDfield <3>fAA{       screen2ColAlt,    screen2RowAmp,    &FreeSans18pt7b,  LJ  };
LCDfield <3>fBA{       screen2ColBat,    screen2RowAmp,    &FreeSans18pt7b,  LJ  };
LCDfield <4>fAT{       screen2ColAlt,    screen2RowTemp,   &FreeSans18pt7b,  LJ  };
LCDfield <4>fBT{       screen2ColBat,    screen2RowTemp,   &FreeSans18pt7b,  LJ  };
LCDfield <2>fCD{       screen2ColCount,  screen2RowCount,  &FreeSans18pt7b,  LJ  };
LCDfield <20>fCS{      screen2ColState,  screen2RowState,  &FreeSans18pt7b,  LJ  };
LCDfield <4>fPW{       screen2ColPWM,    screen2RowPWM,    &FreeSans18pt7b,  LJ  };
LCDfield <5>fSB{       screen2ColScuba,  screen2RowScuba,  &FreeSans18pt7b,  RJ  };
LCDfield <20>fFL{      screen2ColState,  screen2RowScuba,  &FreeSans18pt7b,  LJ  };

//List of Screen2 objects - does not include fault code field fFL
LCDfield *Screen2Objects[] = {&fAV, &fBV, &fAA, &fBA, &fAT, &fBT, &fCD, &fCS, &fPW, &fSB};

petedd

Well, researching further, with the valuable clue of using a interface class, I found the following link, adopted the third answer there, and came up with the following code. https://stackoverflow.com/questions/9961329/c-iterating-through-objects-to-call-the-same-method


This saved me 500 bytes of dynamic memory, almost half of what I was using before adopting the non-type parameter for arraysize.


Paying it forward, and hoping others might benefit, here is the code I ended up with (enough shown to demonstrate one use of the for loop (and there are plenty others in my code).


Just for the record, this was not at all obvious (to me, at least) and, if you compare to the code above, you can see that using the absrtact interface and the virtual functions really changed the syntax of the constructors and other aspects of the class statements.


Code: [Select]



// Class
class IScreens {
  public:
virtual void SetNewValue(const char* newValue) = 0;
virtual void UpdateDisplayField() = 0;
}; // don't forget the semicolon at the end of the class


template <size_t arraysize>
class LCDfield : public IScreens {
  public:
//constructor
LCDfield( uint16_t x,  uint16_t y, const GFXfont *Font, uint8_t just = LJ)
{
  m_X = x;
  m_Y = y;
  m_Font = Font;
  m_Just = just;
}


virtual void SetNewValue(const char* newValue);
virtual void UpdateDisplayField();


  private:
char   m_newValue[arraysize + 1] = {'\0'}; // Make sure there is always a null character at the end
int    m_X, m_Y; // This is the cursor reference position (for example, the center of the string if using Center Justify)
int    m_X1, m_Y1;  // Used for position of fillRect to write over last string printed
const GFXfont *m_Font;
uint8_t    m_Just; // Specifies the text Justification (position relative to m_X, m_Y, as Right, Left, or Center)
int    m_LLX, m_LLY; //corrected cursor position for the start of the string - Proportional Fonts use Lower Left for origin of string
unsigned int  m_Twidth, m_Theight;
}; // don't forget the semicolon at the end of the class


template <size_t arraysize>
void LCDfield<arraysize>::SetNewValue(const char* newValue) {
  strncpy(m_newValue, newValue, arraysize);
}


template <size_t arraysize>
void LCDfield<arraysize>::UpdateDisplayField(void) {
   Serial.println(m_newValue);  // Just a dummy function
}//DisplayField(void)




//LCDfield Screen 2 Objects (Optional)
//construct instances  column,    row,    Font    Justification
IScreens* fAV = new LCDfield <5> {    screen2ColAlt, screen2RowVolt,   &FreeSans18pt7b,  LJ  };
IScreens* fBV = new LCDfield <5> {    screen2ColBat, screen2RowVolt,   &FreeSans18pt7b,  LJ  };
IScreens* fAA = new LCDfield <3> {    screen2ColAlt, screen2RowAmp, &FreeSans18pt7b,  LJ  };
IScreens* fBA = new LCDfield <3> {    screen2ColBat, screen2RowAmp, &FreeSans18pt7b,  LJ  };
IScreens* fAT = new LCDfield <4> {    screen2ColAlt, screen2RowTemp,   &FreeSans18pt7b,  LJ  };
IScreens* fBT = new LCDfield <4> {    screen2ColBat, screen2RowTemp,   &FreeSans18pt7b,  LJ  };
IScreens* fCD = new LCDfield <2> {    screen2ColCount,  screen2RowCount,  &FreeSans18pt7b,  LJ  };
IScreens* fCS = new LCDfield <20> {   screen2ColState,  screen2RowState,  &FreeSans18pt7b,  LJ  };
IScreens* fPW = new LCDfield <4> {    screen2ColPWM, screen2RowPWM, &FreeSans18pt7b,  LJ  };
IScreens* fSB = new LCDfield <5> {    screen2ColScuba,  screen2RowScuba,  &FreeSans18pt7b,  RJ  };
IScreens* fFL = new LCDfield <20> {   screen2ColState,  screen2RowScuba,  &FreeSans18pt7b,  LJ  };


//List of Screen2 objects - does not include fault code field
IScreens* const Screen2Objects[] = {fAV, fBV, fAA, fBA, fAT, fBT, fCD, fCS, fPW, fSB};


//....


// Update screen2 fields
 for (size_t i = 0; i < sizeof Screen2Objects / sizeof Screen2Objects[0]; i++) {
Screen2Objects[i]->UpdateDisplayField();  //refresh using existing m_newValue data
 }



PieterP

This saved me 500 bytes of dynamic memory, almost half of what I was using before adopting the non-type parameter for arraysize.
You didn't save 500 bytes of dynamic memory, you hid it from the IDE, because it is dynamically allocated at runtime. Your code will use more memory than before, the IDE simply doesn't report it. (The IDE only reports the sizes of the .bss and .data sections. Local variables allocated on the stack and memory allocated on the heap using new are not reported.)

There's no need to allocate your LCDfields dynamically. 
Code: [Select]
LCDfield <5> fAV = {   screen2ColAlt, screen2RowVolt,   &FreeSans18pt7b,  LJ  };
LCDfield <5> fBV = {   screen2ColBat, screen2RowVolt,   &FreeSans18pt7b,  LJ  };
...
IScreens* const Screen2Objects[] = {&fAV, &fBV, ...};

You should avoid using new in C++ code, use smart pointers if you can.

petedd

Points made.  Thank you.  A little bit saved since the m_newValue char arrays are not all 20 char long anymore.  Also, dropping the dynamic allocation reduced the program memory by 1300 bytes just now.  Did not expect that big a change.

Go Up