Setting RTC via remote keypad

Hi. Can someone please help me in the syntax for the "for" statement I'm trying to write below. Part of the code is written below. The first error that comes up is:

time2.ino:966:7: error: 'tempDigitFlash' was not declared in this scope.

Firstly I'm wanting it to set tempDigitFlash1 to -1 and tempDigitFlash2 to -1, etc, etc. Not sure where my syntax is incorrect below. Hope it's enough code to understand the error.

byte tempDigit1 = 0;        // used in date_time setting left most digit
byte tempDigit2 = 0;        // used in date_time setting next right digit
byte tempDigit3 = 0;        // used in date_time setting next right digit
byte tempDigit4 = 0;        // used in date_time setting right most digit
byte tempDigitFlash1 = 0;   // used in flashing the first digit
byte tempDigitFlash2 = 0;   // used in flashing the first digit
byte tempDigitFlash3 = 0;   // used in flashing the first digit
byte tempDigitFlash4 = 0;   // used in flashing the first digit
byte timePreset = 0;        // used to flash the correct time digit whilst setting date_time



void flashTimeDigits (void) {
  // When setting the date_time flash the digits one at a time as follows

  // This is to blink the corresponding digit during setting the year
  for (byte t = 2; t < 5; t++) {
    if (timePreset == t && blinkState == blink_off) {  // flash 2nd digit off
      tempDigitFlash[t] =  -1;  // this dummy number that makes the digit "blank" while setting date_time
    } // end if
    if (timePreset == t && blinkState == blink_on) {  // flash 2nd digit on
      tempDigitFlash[t] = tempDigit[t];
    } // end if
  } // end for
} // end void flashTimeDigits

You don't have an array called "tempDigitFlash", you don't have an array called "tempDigit".
You have a whole bunch of variables with similar names, but similar means absolutely nothing to
the compiler.

At run time, variables do NOT have names. They have addresses. You can NOT do what you are trying to do. At least, not that way. You COULD use arrays and save yourself a LOT of grief.

But, don't expect the compiler to write your code for you.

MarkT:
You don't have an array called "tempDigitFlash", you don't have an array called "tempDigit".
You have a whole bunch of variables with similar names, but similar means absolutely nothing to
the compiler.

I tried to also add the following, but it doesn't work. I know I'm doing something wrong, but not sure what:

const byte tempDigit[4] = {1, 2, 3, 4};
const byte tempDigitFlash[4] = {1, 2, 3, 4};

I know I'm doing something wrong, but not sure what:

You are using that lame statement "it doesn't work".

You have some code you only posted a snippet of. It may, or may not, actually compile.

If it does, it does something. You expect it to do something. You need to post ALL of your code, and explain what it actually does, and how that differs from what you want.

If it does not compile, post ALL of the code AND the error messages.

PaulS:
You need to post ALL of your code, and explain what it actually does, and how that differs from what you want.

Sorry Paul. The code I have now is over 1500 lines long, so a bit long to post and hard to follow (as I'm not good at code - self taught). I've been adding to it over time.

What I'm trying to do is:

  • The code is for an scoreboard I've been working on - currently in operation
  • The circuitboard includes a DS3231 real time clock
  • To alter the date-time, I need to connect a laptop to the board and upload via serial monitor
  • I'm trying to start to add code to change the date-time via my remote keypad
  • The intent is that on 4 of the digits of the board, once I hold down the # key for 2 seconds, it goes into date-time program mode and this is visibly seen on 4 LED digits on the board
  • The first 4 presses would be the year - each saved in a variable
  • Then the month and date, and finally the hour and minutes
  • Uploading to the RTC would be the next hurdle
  • I started to write the code in a really long format, but was now trying to tighten it up

Happy to attach the code in a file, but not sure if anyone is interested in reading through it.

With a change by adding the "array", I now get another error further down. The first error says:

date-time_set_2.ino:1171:18: error: 'd' cannot appear in a constant-expression

date-time_set_2.ino (63.1 KB)

        switch (CmdChar)
        {
            // look for number press 0,1,2,...9 and set to that digit
            for (byte d = 65; d < 75; d++)    // looking at ascii press for 0,1,2...9
            {
            case d:

This, and the many places in the code where it occurs, does not make sense.

You can't use a value that can change as a case value in switch/case.

        switch (CmdChar) {
            // look for number press 0,1,2,...9 and set to that digit
            for (byte d = 65; d < 75; d++) {  // looking at ascii press for 0,1,2...9
            case d:
              cancelTimeSetting = millis();  // set in case time_date single stage not set within 10 seconds
              tempDigit2 = d - 65;           // this will set it to the pressed keypad button number
              yearHundSet = tempDigit2;

You really should have functions for each case. If you had a function called setYearDigit() that took an argument that defined which digit to set, you'd call that when the CmdChar was d (whatever d is).

Then, if you wanted to change the way that setYearDigit() worked, you wouldn't have the switch statement and the case label distracting you.

You would NOT try to add a for loop between the switch statement and the case label.

As to how to fix this code, I really have no idea. It looks like a ransom letter with phrases snipped from magazines.

I'd need to see the code from before you started mangling it.

PaulS:
I'd need to see the code from before you started mangling it.

The original code is currently working well - but didn't include setting the date-time this way.

Over the past few days whilst travelling to work on the train, I've tried to write this new addition (see below for the addition on it's own).

However my guess from all the replies I've received in this thread is that my understanding of code isn't what it needs to be. Maybe I should start from scratch. It might be worth trying to write this in long form as I was originally planning, then asking for assistance in simplifying it???

part_code.ino (14 KB)

However my guess from all the replies I've received in this thread is that my understanding of code isn't what it needs to be.

Oh, good. I was afraid that we were too subtle. 8)

Maybe I should start from scratch.

What you should do is create a new sketch where you try out ideas. When the idea proves a dismal failure, consign the sketch to the bit bucket.

If the idea works, you'll understand why, and can put the new idea into a function that you then copy into the working sketch.

It might be worth trying to write this in long form as I was originally planning, then asking for assistance in simplifying it???

If "this" is your new idea, yes, that would be a good idea. The code you posted in your last reply has the same problem as the code in the earlier reply.

I'm suspecting that you are attempting to modify code that was written by someone else without really knowing much about programming. I gather this from some of the mistakes you appear to be making and from the fact that this slab of code is 1500 lines long and already does a lot. You don't understand how case statements work. You don't understand variable naming. If this sketch works at all, then someone else wrote it.

I suppose the other alternative is that you sat down and tried to write the whole thing in one big effort, rather than writing (and testing) small bits and adding the functionality that you need. This never works. I have been programming for 30 years, and I don't attempt to do it that way. If this is what you are trying to do, then there is no doubt that not only does the sketch not work, it never worked - not even a little bit.

If you are determined to do this yourself rather than getting someone else to do it, there's not much alternative other than to actually learn to code. Do a basic C++ tutorial. Work through the examples in the IDE. Write the "Blinking LED" sketch and understand it.

Attempting to make the sketch you already have do what you want by banging away on it until it looks right just won't work. You wouldn't attempt to fix or modify anything else complicated with that method, right? You wouldn't open up the back of your fridge or the engine of your car and expect to fix it without knowing much about fridges or cars, would you? Well - it won't work on a 1500-line sketch, either.

Another thing that won't work is coming back to these boards again and again with code fragments, trying to fix each bit in isolation. We get a few people trying to do that: "this line won't compile, can someone explain to me what's wrong with this line?". Because the problems generally aren't in this or that line; and even if they are, if you don't know what you are looking at then you won't know how to frame your questions so as to actually get help. This post is a fine example: you ask for help with for statement syntax, but there is nothing wrong with the syntax of your for statement! At this point, you don't know enough to know what to ask.

PaulMurrayCbr:
I'm suspecting that you are attempting to modify code that was written by someone else without really knowing much about programming

Thanks PaulM and PaulS for your replies.

I have infact written the odd 1500 lines of of the original code myself over about a year. It's been a big project for me between building the 2 large scoreboards for my soccer club, using shift registers in lieu of discrete components and writing the lengthy code. It's been a steep learning curve for me. But I enjoy the challenge.

Where I did get help however was in the "case" syntax. I don't really understand the rules about it.

I can easily write code now to act when a key from the remote is pressed and received by the control board - in this case the #symbol. I can then call up a string of switch statement looking for the next press (0 or 1 or 2 ... or 9). This however is 10 case statements. Then as the date I need is YYYY,MM,DD,HH,MM,SS, excluding seconds this is 10 X 12 case statements. This is a bit long so I am trying to write it a bit shorter.

I'll google search more info to learn about case statements. I'll get it eventually. Cheers.

Where I did get help however was in the "case" syntax. I don't really understand the rules about it.

From looking at how you used it in your program either you were given the wrong advice or did not understand it.

As to how switch/case works it simply acts like a series of if/else if statements

if (the switch variable equals this value)
  {
    //execute the code here
  }
else if (the switch variable equals this other value)
  {
    //execute the code here
  }
else if (the switch variable equals some other value)
  {
    //execute the code here
  }

The values that are tested are fixed into the program at compile time so cannot be changed when the code is run as you try to in your program with the numerous for loops with a variable d.

UKHeliBob:
From looking at how you used it in your program either you were given the wrong advice or did not understand it.

Bob, yes I am struggling a bit here. Could I ask some questions to get my head around my errors.
Firstly is this syntax now correct (in using an array):

int tempDigit[] = {1, 2, 3, 4};
int tempDigitFlash[] = {1, 2, 3, 4};

void flashTimeDigits (void) {
  for (byte t = 0; t < 4; t++) {
    if (timePreset == t) {
      tempDigitFlash[t] =  -1;
    } // end if
  } // end for

Also, this is then what I'm typically using for a switch case for my button presses:

void keyRead () {
  if ( myRadio.available()) {
    while (myRadio.available()) {
      myRadio.read( &dataReceived, sizeof(dataReceived) );
      if ((asterix == 0) && (dataReceived == 33)) {
        switch (CmdChar) {

          case 65:
            // 0 pressed to turn off the siren if active
            if (sirenState == active) {    // check if the siren is active
              sirenState = inactive;       // if it is, make it inactive and the strobe as well
              strobeState = not_flashing;
              break;
            }
          case 66:
            if (onesHome < 9) {
 
          etc, etc

What I'm trying to now do using the switch case method above, is to add another piece of code for changing the date. But for yyyymmddhhmm ends up 10 x 12 (120) switch cases. Can you point out where the syntax below is incorrect, or better method to start off. Here's a snippit:

      if ((hash == 1) && (dataReceived == 33) && (timePreset == 2)) {
        switch (CmdChar) {
            // look for number press 0,1,2,...9 and set to that digit
            for (byte d = 65; d < 75; d++) {  // looking at ascii press for 0,1,2...9
            case d:
              cancelTimeSetting = millis();
              tempDigit2 = d - 65;
              yearHundSet = tempDigit2;
              timePreset = 3;
            } // end for
         } // end switch
      } // end if

Is there a way to loop the case number (which would be 65 for a "0" keypress, 66 for a "1" keypress, etc.?

Aren't the ascii values for numerals 48, 49, 50, ... 57? I thought 65, 66, 67 ... was A, B, C ...

Is there a way to loop the case number

Look at the definition of a switch statement, and how the case values are used. Your question does not make sense.

Maybe I should start from scratch.

We'all do that!
Often it ends up being the best way to clean up code - and to implement what you have learned since version n-1

But for yyyymmddhhmm ends up 10 x 12 (120) switch cases.

120 switch/cases would be absurd. At the very least you should use a single input function and call it as many times as needed.

What form do you want the yyyymmddhhmm to be in ? Should it be in a zero terminated string, for instance, or should each element of it be a discrete number ?

UKHeliBob:
What form do you want the yyyymmddhhmm to be in ? Should it be in a zero terminated string, for instance, or should each element of it be a discrete number ?

Currently I'm passing the information through via the serial monitor in the form YYYY,MM,DD,HH,MM,SS. I haven't worked out yet how to pass this info through yet with code instead. That was going to be my next challenge once I worked out how to collect the info.

Currently I'm passing the information through via the serial monitor in the form YYYY,MM,DD,HH,MM,SS.

SO at the moment the user types the date/time in the format the form YYYY,MM,DD,HH,MM,SS in the Serial monitor input prompt then presses return, but you want them to enter it on some kind of remote device. Is that correct ?

From reply #9 it seems that the system should react when a '#' is received, then receive the next 14 characters and use them as the date/time. Do you want each part of the date/time in separate variables, perhaps in an array and should the data be saved as strings or as numerical values ?

If not then please explain what you want to do do in more detail.