Strange behavior of strcpy()

sekar1959 · February 17, 2022, 9:06am

hi i am working on a clock with timer2 as 32768Hz osc with internal 8 MHz with bare minimum ATMEGA328P. one of the function word_clock() i used from source GitHub - mrnick1234567/miniclock: Arduino based mini LED matrix clock .
on this threre are three strings used (str_a, str_b and str_c) to store and display the words of the clock.
Herewith i am copying only the portion of this function:

void word_clock()
{
  cls();
  if (powerFailed)
  {
    return;
  }
  const char numbers[19][9] PROGMEM = {
      "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten",
      "eleven", "twelve", "thirteen", "fourteen", "fifteen", "sixteen", "sevnteen", "eighteen", "nineteen"};
  const char numberstens[5][7] PROGMEM = {
      "ten", "twenty", "thirty", "forty", "fifty"};

  // potentially 3 lines to display
  char str_a[8];
  char str_b[8];
  char str_c[8];

  // byte hours_y, mins_y; //hours and mins and positions for hours and mins lines

  byte hours = t.hour;
  if (hours > 12)
  {
    hours = hours - twelveHr * 12;
    // operator * has higher precedence than + (or) -, so it first gets multiplied withtwelveHr*12 and then adds into hours.
  }
  if (hours < 1)
  {
    hours = hours + twelveHr * 12;
  }

  byte old_mins = 100; // store mins in old_mins. We compare mins and old mins & when they are different we redraw the display. Set this to 100 initially so display is drawn when mode starts.
  byte mins;

  // run clock main loop as long as run_mode returns true
  while (run_mode())
  {
    if (powerFailed)
    {
      return;
    }
    if (Serial.available() > 0)
    {
      setTimeFromSerial();
    }

    // check for button press
    if (buttonA.uniquePress())
    {
      switch_mode();
      return;
    }
    if (buttonB.uniquePress())
    {
      display_date();
    }

    mins = t.minute; // get mins

    // if mins is different from old_mins - redraw display
    if (mins != old_mins)
    {

      // update old_mins with current mins value
      old_mins = mins;

      // reset these for comparison next time
      mins = t.minute;
      hours = t.hour;

      // make hours into 12 hour format
      if (hours > 12)
      {
        hours = hours - 12;
      }
      if (hours == 0)
      {
        hours = 12;
      }

      // split mins value up into two separate digits
      int minsdigit = t.minute % 10;
      byte minsdigitten = (t.minute / 10) % 10;

      // if mins <= 10 , then top line has to read "minsdigti past" and bottom line reads hours
      if (mins < 10)
      {
        strcpy(str_a, numbers[minsdigit - 1]);
        strcpy(str_b, "PAST");
        strcpy(str_c, numbers[hours - 1]);
      }
      // if mins = 10, cant use minsdigit as above, so special case to print 10 past /n hour.
      else if (mins == 10)
      {
        strcpy(str_a, numbers[9]);
        strcpy(str_b, " PAST");
        strcpy(str_c, numbers[hours - 1]);
      }
      // if time is not on the hour - i.e. both mins digits are not zero,
      // then make first line read "hours" and 2 & 3rd lines read "minstens"  "mins" e.g. "three /n twenty /n one"
      else if (minsdigitten != 0 && minsdigit != 0)
      {

        // if mins is in the teens, use teens from the numbers array for the 2nd line, e.g. "fifteen"
        // if (mins >= 11 && mins <= 19) {
        if (mins <= 19)
        {
          strcpy(str_b, numbers[mins - 1]);
          strcpy(str_c, "");
        }
        else
        {
          strcpy(str_b, numberstens[minsdigitten - 1]);
          strcpy(str_c, numbers[minsdigit - 1]);
        }
        Serial.print("str_b @before str_a working: ");
        Serial.println(str_b);
        strcpy(str_a, numbers[hours - 1]);
        Serial.print("str_b @ after str_a working: ");
        Serial.println(str_b);
      }
      // if mins digit is zero, don't print it. read read "hours" "minstens" e.g. "three /n twenty"
      else if (minsdigitten != 0 && minsdigit == 0)
      {
        strcpy(str_a, numbers[hours - 1]);
        strcpy(str_b, numberstens[minsdigitten - 1]);
        strcpy(str_c, "");
      }
      // if both mins are zero, i.e. it is on the hour, the top line reads "hours" and bottom line reads "o'clock"
      else if (minsdigitten == 0 && minsdigit == 0)
      {
        strcpy(str_a, numbers[hours - 1]);
        strcpy(str_b, "O'CLOCK");
        strcpy(str_c, "");
      }

    } // end worknig out time
    Serial.println(String("str_a value: ") + str_a);

    Serial.println(String("str_b value: ") + str_b);

    Serial.println(String("str_c value: ") + str_c);
    Serial.println();
    // run in a loop
    // print line a "twelve"
    byte len = 0;
    while (str_a[len])
    {
      len++;
    };                                            // get length of message
    byte offset_top = (31 - ((len - 1) * 4)) / 2; //

    // plot hours line
    byte i = 0;
    while (str_a[i])
    {
      puttinychar((i * 4) + offset_top, 1, str_a[i]);
      i++;
    }

    // hold display but check for button presses
    int counter = 1000;
    while (counter > 0)
    {
      // check for button press
      if (buttonA.uniquePress())
      {
        switch_mode();
        return;
      }
      if (buttonB.uniquePress())
      {
        display_date();
      }
      delay(1);
      counter--;
    }
    fade_down();

    // print line b
    len = 0;
    while (str_b[len])
    {
      len++;
    }; // get length of message
    offset_top = (31 - ((len - 1) * 4)) / 2;

    i = 0;
    while (str_b[i])
    {
      puttinychar((i * 4) + offset_top, 1, str_b[i]);
      i++;
    }

    // hold display but check for button presses
    counter = 1000;
    while (counter > 0)
    {
      if (buttonA.uniquePress())
      {
        switch_mode();
        return;
      }
      if (buttonB.uniquePress())
      {
        display_date();
      }
      delay(1);
      counter--;
    }
    fade_down();

    // print line c if there.
    len = 0;
    while (str_c[len])
    {
      len++;
    }; // get length of message
    offset_top = (31 - ((len - 1) * 4)) / 2;

    i = 0;
    while (str_c[i])
    {
      puttinychar((i * 4) + offset_top, 1, str_c[i]);
      i++;
    }
    counter = 1000;
    while (counter > 0)
    {
      // check for button press
      if (buttonA.uniquePress())
      {
        switch_mode();
        return;
      }
      if (buttonB.uniquePress())
      {
        display_date();
      }
      delay(1);
      counter--;
    }
    fade_down();

    // hold display blank but check for button presses before starting again.
    counter = 1000;
    while (counter > 0)
    {
      // check for button press
      if (buttonA.uniquePress())
      {
        switch_mode();
        return;
      }
      if (buttonB.uniquePress())
      {
        display_date();
      }
      delay(1);
      counter--;
    }
    hourlyChime();
  }
  fade_down();
}

i have inserted some Serial.println() at lines 116 (if (mins <= 19)) to 120 as str_a behaves strangely with str_b. same is also output on my display which is MAX7219 - 4 digit one.

the serial out put, when the minuets reached between 11 and 19, str_a value is appended to str_b eventhough on line 118 (strcpy(str_a, numbers[hours - 1]) i am doing only strcpy() on str_a.

below is serial output i am getting:
str_b @before str_a working: fifty
str_b @ after str_a working: fifty
str_a value: ten
str_b value: fifty
str_c value: five

str_a value: eight
str_b value: PAST
str_c value: eleven

str_b @before str_a working: thirteen
str_b @ after str_a working: thirteeneleven
str_a value: eleven
str_b value: thirteeneleven
str_c value:

str_b @before str_a working: eighteen
str_b @ after str_a working: eighteeneleven
str_a value: eleven
str_b value: eighteeneleven
str_c value:

str_b @before str_a working: twenty
str_b @ after str_a working: twenty
str_a value: eleven
str_b value: twenty
str_c value: three

at eleven thirteen suddenly str_a value is added to str_b (shows as thirteeneleven) and this is happening only between minute value 11 to 19. also happening during other hours but only during minutes 11 to 19

question is how strcpy() from previous operation passing the value to next operation without being doing anything on str_b?
help will be much obliged,

Koepel · February 17, 2022, 9:21am

The ATmega328P is a microcontroller that is optimized for many things, but not for memory usage. It can not run code from SRAM and it can not read data from Flash.

To read data from Flash, it has special instructions. The PROGMEM explanation shows how to use it: https://www.arduino.cc/reference/en/language/variables/utilities/progmem/

The pgm_read...() functions are low level functions that use the special instructions.
There are luckily also higher level functions, such as strcpy_P().
I think that using the strcpy_P() is enough for your sketch to make it work.

Whandall · February 17, 2022, 9:22am

As string of 8 chars needs a buffer of 9 bytes, to accommodate for the trailing zero-char.

sekar1959 · February 17, 2022, 12:27pm

You nailed it!! thanks for the suggestion, problem solved.
I changed

char str_a[8];
char str_b[8];
char str_c[8];

to

char str_a[9];
char str_b[9];
char str_c[9];

Koepel · February 17, 2022, 1:30pm

What did I miss ? Is local PROGMEM data not in Flash ?
Are you testing the code on a faster Arduino board ?

sekar1959 · March 4, 2022, 2:32am

Sorry for late reply, i am using barebone ATMEGA328P with 8 Mhz int osc and using Timer2 in asynchronous mode with 32768 crystal. so not an faster CPU.

The strings are in in PROGMEM and not using pgm_read_byte(); or pgm_read_byte_near() to access these strings. string are copies to RAM using standard
strcpy(str_c, numbers[value]);
hope it clears your doubts,

runaway_pancake · March 4, 2022, 3:13am

You credited yourself with the 'Solution' instead of Whandall.

david_2018 · March 4, 2022, 5:54am

Note that the arrays are being declared inside a function. Even through they are declared with PROGMEM, what the compiler actually does is create local variables in ram. To actually store the data in PROGMEM requires that the arrays also be declared 'static'. When that is done, strcpy will no longer function properly, instead use strcpy_P.

Koepel · March 4, 2022, 7:51am

Thank you !

@sekar1959, you can make those variables global and declare them above the setup() function like everyone else. When they are really PROGMEM data in Flash, then you have to use strcpy_P().

system · August 31, 2022, 7:51am

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.

Topic		Replies	Views
Word Clock Code - time is off? Programming	1	401	May 5, 2021
code problems.. Programming	11	3877	May 5, 2021
[SOLVED] String vs Char Array Programming	14	31540	May 5, 2021
Array size of a sprintf created string Programming	18	4773	May 5, 2021
Character Array Output Issues Programming	4	369	May 6, 2021

Strange behavior of strcpy()

Related topics