Storing a struct containing day and hour plus value

Hi all,
I'm struggling a bit with this, not sure if it's the logic or code failing me!
What I want to do is set either a 1 or a 0 for each hour of each day of the week.
So ultimately I'll end up with:
day_0, hour_0 = 1
day_0, hour_1 = 1
day_0, hour_2 = 0
day_0, hour_3 = 0

And so on, so that I can then check the current hour and day, compare and decide if my lights (via relay) should be on or off.

I thought storing this in a struct should work well, so I've done this:

typedef struct _day {
  char *hours[2];
} mydaystruct;

typedef struct _week {
    mydaystruct days [7];
} myWeeksSchedule;

myWeeksSchedule days;

I can then reference this in code and read/write like this:
days.days[0].hours[0]
days.days[0].hours[1]
days.days[0].hours[2]
days.days[0].hours[3]

etc, but I think I've made an error as each time I write to them and read them back they always return 1.
I use myvar (set elsewhere) to then write the data into the array like this:

while(tmp_day < 7) {
    tmp_hour=0;
    while (tmp_hour< 24) {
        if(myvar == "1") {
          strncpy(days.days[tmp_day].hours[tmp_hour], "1",1);
        } else {
          strncpy(days.days[tmp_day].hours[tmp_hour], "0",1);
        };
      tmp_hour++;
    };
    tmp_day++;
  };

Anyone spot what stupid mistake I've made on the struct/array?

  char *hours[2];

How many hours are there in your day ?

First, can you post a complete example sketch that demonstrates your use of the structures?

Doesn't have to be the whole thing, in fact it would be better to get just this part working well in a stand alone.

Second, why store as characters, be they '0' chars or "0" strings?

All you need is the number 0 or 1, which can be used subsequentyl for false and true.

a7

IMHO It's not only the struct, but I'm sorry, it's quite a mess...

First, you set an unnecessary complex structure to store just boolean values for each hour of each one of the 7 days of a week. Make things easier makes an easier coding (and debugging).

Second, you need 24 hours for each day, so, as UKHeliBob suggested before, you need to change "something"...

Third, I can't imagine what you think to do with that while() cycle: it just sets either all "1" or all "0"s, is that what you want?... And why use while() instead of a more readable for()?

Fourth, you can't compare strings like if(myvar == "1") (I suppose "myvar" is a string -hope not a "String"...).

I'd use a lighter approach, a simple array with just 7 char arrays, one for each day, to store a 24 chars (i.e. 25) string each:

char sched[7][25];

void setup() {
  Serial.begin(9600);
  // Initial schedule
  for (int i=0; i<6; ++i)
    sched[i] = "000000111111111110000000";
}

...and then you can set a single hour by simply doing
sched[day][hour] = value;
and an entire day (like you see I did on setup), with a single 24 character string like
sched[day] = "00111100001011011101111";.

Or if you like a more structured programming, create a few functions to manipulate values (just to logically separate the usage from implementation), something like:

void setHourValue(byte day, byte hour, char value) {  
      sched[day][hour] = value;
}
void setDayValues(byte day, char value) {
    for (int hour=0; hour<24; ++hour)
      setHourValue(day, hour, value);
}
void setAllValues(char value) {
  for (int day=0; day<7; ++day)
    setDayValues(day, value);
}
//... etc...

Thank you guys, yes I think you're right, I've gone about this in a right mess, it's something I've plugged into an already over-complicated and old program of mine. I should really start again!

But in the meantime, yes all I'm trying to do is have a flag that I can later compare on if its true or false (the if (myvar) is just for example, it's actually stored to eeprom and the user can change using a web interface).

For completeness, here is a piece of testcode that isn't working!

#include <Arduino.h>
extern "C" {
#include <user_interface.h>
}

typedef struct _day {
  char *hours[2];
} mydaystruct;

typedef struct _week {
  mydaystruct days [7];
} myWeeksSchedule;

myWeeksSchedule days;
int tmp_day = 0;
int tmp_hour = 0;


void setup() {
  Serial.begin(115200);
  delay(3000);
  Serial.println("Starting up");
  Serial.println("Now writing");
  delay(1000);
  tmp_day = 0;
  tmp_hour = 0;
  while (tmp_day < 7) {
    tmp_hour = 0;
    while (tmp_hour < 24) {
      if (random(0, 2) == 1) {
        //strncpy(days.days[tmp_day].hours[tmp_hour], "1",1);
        days.days[tmp_day].hours[tmp_hour] = "1";
        Serial.println("Setting day: " + String(tmp_day) + " Hour: " + String(tmp_hour) + " to 1");
      } else {
        //strncpy(days.days[tmp_day].hours[tmp_hour], "0",1);
        days.days[tmp_day].hours[tmp_hour] = "0";
        Serial.println("Setting day: " + String(tmp_day) + " Hour: " + String(tmp_hour) + " to 0");
      };
      tmp_hour++;
    };
    tmp_day++;
  };

  Serial.println("Now read what we found:");
  tmp_day = 0;
  tmp_hour = 0;
  while (tmp_day < 7) {
    tmp_hour = 0;
    while (tmp_hour < 24) {
      Serial.println("day : " + String(tmp_day) + "   hour : " + String(tmp_hour) + " value : " + String(days.days[tmp_day].hours[tmp_hour]));
      tmp_hour++;
    };
    tmp_day++;
  };


}

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

}

Having said that, @docdoc example using the simple array:

char sched[7][25];
for (int i=0; i<6; ++i)
    sched[i] = "000000111111111110000000";

Would do this job nicely, I can then init in my loop (reading from eeprom or setting) either individually or as a bulk.
I'll swap the code around and give those suggestions a go, thank you.

One final thing, please excuse me, but, 24hours for each day, would that not be a 2 char array? (i.e. two characters, 00 to 23) ?

Thank you guys, I appreciate your time helping me

You could do that with an array of 24 bytes. One byte per hour and 1 bit of each hour per day and 1 wasted bit per hour

Just to conclude this, using a combination, here is my working sketch that I can base it on:


char *days[7][25];


void setup() {
  Serial.begin(115200);
  delay(3000);
  Serial.println("Starting up");
  Serial.println("Now writing");
  delay(1000);
  int tmp_day=0;
  int tmp_hour=0;
  while(tmp_day < 7) {
    tmp_hour=0;
    while (tmp_hour< 24) {
      if (random(0,2) == 1) {
        days[tmp_day][tmp_hour]="1";
      } else {
        days[tmp_day][tmp_hour]="0";
      };
      tmp_hour++;
    };
    tmp_day++;
  }; 

  Serial.println("Now display values:");
  tmp_day=0;
  tmp_hour=0;
  while(tmp_day < 7) {
    tmp_hour=0;
    while (tmp_hour< 24) {
      Serial.println("Day: " + String(tmp_day) + " Hour: " + String(tmp_hour) + "  value: " + String(days[tmp_day][tmp_hour]));
      tmp_hour++;
    };
    tmp_day++;
  }; 

}

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

}

Since these are char's I can use if condition to check elsewhere in my code like this:

if (days[0][0] == '1')

Yes?

Not quite. Sticking with strings means "0" takes two bytes, as you seem to have been hinting at or asking earlier.

But that array is fine for storing a number 1 or 0 as such.

Any other way of looking at it can be neatly handled by some functions between your use and the underlying data structure as demonstrated by @docdoc above.

It is quite nice to be able to get something low level to function, than step up and write some functions to do what you aren't doing directly in the data.

At the lowest level, a 24 * 8 array of bits could be used. If space was really at a premium, the smallest amount of storage used.

I think there are bit array libraries that let you act as if there was a real bit type variable. Again, a matter of abstraction or hiding or whatever.

Either way, at the higher level just write functions to make using the data more convenient or a better fit to the requirements I'm;lies by your top down look at how you want thing to work.

Edit: you could store '0', one character and be good. If you just want to use a character array like you have done.

TBC. '0' and '1' are, in the end, just numbers, the ASCII codes for the characters.

So I'd just use 1 and 0, the numbers, then those values are true and false ready to use without comparison to either a character code or a character string.

Hmm, I seem to have added a day to the standard week. 24 * 7 bits would do it...

a7

OK, never used the BitArray library, so I took your code and made it use a bit array.

My functions to set and get the data could be written to use any underlying method of storing and retireving the 168 actual bits of information.

It's your testing code, with the necessary changes which I "outdented" so you can spot them.

// https://forum.arduino.cc/t/storing-a-struct-containing-day-and-hour-plus-value/988274

# include "BitArray.h"

# define NBITS  168   // one entry per hour per day for one week
# define SIZE   1     // one bit per entry

BitArray hourlySetting;

void setDayHourFlag(unsigned char day, unsigned char hour, unsigned char onOff)
{
  hourlySetting.set(24 * day + hour, onOff);
}

unsigned char getDayHourFlag(unsigned char day, unsigned char hour)
{
  return hourlySetting.get(24 * day + hour);
}

void setup()
{
    Serial.begin(115200);
    Serial.println("hour by hour bit array test");

    hourlySetting.begin(SIZE, NBITS);
    hourlySetting.clear();      // or not

    testThisThing();
}

void testThisThing() {

  Serial.println("Starting up");
  Serial.println("Now writing");
  delay(1000);
  
  int tmp_day=0;
  int tmp_hour=0;
  while(tmp_day < 7) {
    tmp_hour=0;
    while (tmp_hour< 24) {


setDayHourFlag(tmp_day, tmp_hour, random(0, 2));
 
      
      tmp_hour++;
    };
    tmp_day++;
  }; 

  Serial.println("Now display values:");
  tmp_day=0;
  tmp_hour=0;
  while(tmp_day < 7) {
    tmp_hour=0;
    while (tmp_hour< 24) {
    	
      Serial.print("Day: " + String(tmp_day) + " Hour: " + String(tmp_hour) + "  value: ");

Serial.println(getDayHourFlag(tmp_day, tmp_hour) ? "ON" : "OFF");
 
      tmp_hour++;
    };
    tmp_day++;
  }; 

}


void loop()
{
}

Play with it here:

Not saying it's the only way to go, I was just truly interested in using your real life example as an excuse to see about that BitArray library.

Seems to work. :expressionless:

The only mildly tricky thing is calculating the bit number for a given day and hour. Just a bit of the old mathematics.

a7

1 Like

You could pack the data into 168 bits which would only require 21 bytes, but that is a step too far :grinning:

Actually, you could use "real" bits, so 24 bits can be stored in 3 bytes (3*8=24) and then use a byte mask to get the relevant bit (like a "boolean" value).
But it makes the coding a bit (hahaha!) more complicated, and I avoided to suggest that.

But just in case, the functions I suggested before come in help to decouple the upper logic from the lower one by just adding a "get" function, like this below, IMHO a more elegant implementation.
As an example, I used "long" variables (i.e. 4 bytes) instead of 3 separate bytes just for a bit (hahaha again, I'd be a comedian!) of clarity, and changed "values" to bool type as it makes more sense to me:

long sched[7];

void setup() {
  Serial.begin(9600);
  // Initial schedule: binary values are for the 24 hours backwards, so the 
  // first (leftmost) bit is 23, the second 22, and so on up to 00
  for (int i=0; i<7; ++i)
    setDayValues(i, 0b000000111111111110000000);

  Serial.print("Before: ");
  Serial.println(getHourValue(1,10)); // returns 1
  setHourValue(1,10,0);
  Serial.print("After : ");
  Serial.println(getHourValue(1,10)); // returns 0
}

// "hour" defines the byte where we have our bit inside
// sched, so we just need to set/reset/read the right 
// bit from our 24 bit variable
void setHourValue(byte day, byte hour, bool value) {  
  bitWrite(sched[day], hour, value);
}

bool getHourValue(byte day, byte hour) {
  return (bitRead(sched[day], hour));
}

void setDayValues(byte day, long value) {
  sched[day] = value;
}

byte getDayValues(byte day) {
  return (sched[day]);
}

void setAllValues(bool value) {
  for (int day=0; day<7; ++day)
    setDayValues(day, value);
}
void loop() {
   
}

Please note "setDayValues" and "setAllValues" haven't changed (except for the value parameter type), this is what I mean for "decoupling"!

That's OK for 1 day but what about the other 6 days of the week ?

It was using the actual bits of each byte that I was suggesting and to hold a week's data you need 168 bits

That's why I had this:

long sched[7];

Here you have an array of 7 long values, one for each day.

BTW, haven't you read (and understood) the entire message and code??? Please, do it before replying! Thanks...

PS: anyway, even if I quote you, my answers are hints for the OP, not you.. :wink:

I think anyone using a microprocessor should be, sooner later, able to write that kind of thing while sleeping.

And I normally don't use a library if I can write the code myself, that is where I get my own fun out of this.

But… I have to say that @alto777 has shown in #9 that using a library can come in handy.

One day perhaps that use of a library, if it rankles, can be revisited and replaced by code written to perform the same functions.

a7

The __uint24 data type is supported.

Right, but as far as I suppose the OP isn't skilled enough on C programming, I'd prefer keeping a "low to mid level" approach and let him gradually understand a deeper programming level.

Yep, but same answer i've just given to alto777... :wink:

1 Like

Yep, I completely agree. Anyway, there's no need of any library, Arduino alreadi has bitWrite() and bitRead() functions, we need no more than that... :wink:

I would expect @docdoc to say that even using those functions is crutch-like and should not be used. :wink:

a7

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