code optimisation - stuck.

Using Arduino Programming Questions
1

This is an extract from an earlier part of the code.

The display has 4 lines but there are now more than 4 things to display.

My idea is to make it show 0 - 3 on one page, wait for the up/down key and display the next page, and if escape is pressed exit.

I could just write out the code for all the pages with the different start/stop values but I am sure there is a way to use the "middle" part of the code as a sub-function.

But I haven't quite got the hang of it yet.

void top_menu_function_4() //Display a list of the alarms and times.

/*
    What I have learnt:
    alarm_EEPROM_storage+n (where n is the alarm number) yields 0,1,2,3
    0 = off
    1 = Daily
    2 = M-F
    3 = Weekend
    4 = Once

bit of informatio about the RTC alarms.
clock1.alarms[ala].hr;
clock1.alarms[ala].mnt;
clock1.alarms[ala].dow;

*/

{
    byte boo;
    char msg1[9];
    char msg[14];
    int i;

//  From here down I would like to make it into a routine where I can call it with different values of 0 and 4
    for (int i=0;i<4;i++)
    {
        lcd.setCursor(0,i);
        sprintf(msg,"Alm %1d %2d:%02d ",i, clock1.alarms[i].hr, clock1.alarms[i].mnt);
        boo = clock1.alarms.[i].dow);
        lcd.print(msg);
        if (boo == 0)
        {
            lcd.print("OFF");
        }
        if (boo == 1)
        {
            lcd.print("Daily");
        }
        if (boo == 2)
        {
            lcd.print("Week Day");
        }
        if (boo == 3)
        {
            lcd.print("Week End");
        }
        if (boo == 4)
        {
            lcd.print("Once");
        }
    } 
    wait_on_escape(8000);


	//  Insert code here to loop on certain keys so the start/end values change or exit.

    return;
}
2

you could put all your text in a array and then just call

lcd.print(arrayname[boo]);

3

Osgeld,

Alas you lost me.

As much as I can sort of see what you mean, it is more the looping/calling which has me stumped.

4 
 boo = clock1.alarms.[i].dow);

I'm guessing the compiler isn't too keen on that line.

Your loop runs from zero to three, but you test "boo" from zero to four.

5

AWOL.

You are right.

Alas that was cut from some OLDER code.

"Regertably" the numbers are nearly like an Italian pinball machine with versions.

Some days it hardly moves, others it flys.

As I don't have the actual code, I had to do a bit of "on the fly" faking.

But I hope the real problem of what I am wanting to do is seen.

I can't seem to make a "sub-function" with that bit of alarm1.clock....... stuff inside that existing function.

There may be a way, but it illudes me just now.

I am reading a book on C++ as well, but putting it all together is real fun.

6

As I don't have the actual code, I had to do a bit of "on the fly" faking.

To save everyone's time, why don't you wait until you have the actual code to post?

Otherwise, it's just a really silly guessing game.

Alas that was cut from some OLDER code.

"Regertably" the numbers are nearly like an Italian pinball machine with versions.

Some days it hardly moves, others it flys

Looks like Google translate had some real difficulties there.

7

Ok here is the working version:

void top_menu_function_4() //Display a list of the alarms and times.
/*
    What I have learnt:
    alarm_EEPROM_storage+n (where n is the alarm number) yields 0,1,2,3
    0 = off
    1 = Daily
    2 = M-F
    3 = Weekend
    4 = Once
bit of information about the RTC alarms.
clock1.alarms[ala].hr;
clock1.alarms[ala].mnt;
clock1.alarms[ala].dow;

*/

{
      
    byte foo[4];
    byte boo;
    char msg1[9];
    char msg[14];
    int i;
    lcd.setCursor(0,0);

    for (int i=0;i<Max_alarms;i++)     //  the 0 and Max_alarms are now needing to be other variables.
    {
        //    from here......
        sprintf(msg,"Alm %1d %2d:%02d ",i, clock1.alarms[i].hr, clock1.alarms[i].mnt);
        boo = clock1.alarms[i].dow;
        lcd.print(msg);
        if (boo == 0)
        {
            lcd.print("OFF");
        }
        if (boo == 1)
        {
            lcd.print("Daily");
        }
        if (boo == 2)
        {
            lcd.print("Week Day");
        }
        if (boo == 3)
        {
            lcd.print("Week End");
        }
        if (boo == 4)
        {
            lcd.print("Once");
        }
     //  to here........
    } 
    wait_on_escape(8000);

//  Code in here to control start and stop numbers.

    return;
}

Ok: max_alarms is how many alarms in the code. It is now 5. That is one too many to display on the screen at once.

Looking for the "From here....." "to here......"

That part needs to be a thing which I can do with 4 loops, and then call it with variable start and stop numbers for the loop controlled by code I put at the bottom.

But all the "variables" need to be passed/seen in the function thingy.

Is that better explained?

8

Folks,

Here is another section of code:

int light_toggle(int fctn)
{
    static int Light_status;
    if (fctn == 0)
    {
      return Light_status;
    }
    sleep_time(10,1);
    if (fctn == 1)
    {
       Light_status = Light_status +1;
       Light_status = Light_status%2;
       if (Light_status==0)    
       {
            CS.digitalWrite(1, LOW);
       }
       if (Light_status==1)    
       {
            CS.digitalWrite(1, HIGH);
       }
    }
    return;
}

Now what I am wondering is with the if { } if { } and all that.

But I could make it more like:

int light_toggle(int fctn)
{
    static int Light_status;
    if (fctn == 0)
    {
      return Light_status;
    }
    sleep_time(10,1);
    if (fctn == 1)
    {
       Light_status = Light_status +1;
       Light_status = Light_status%2;
       CS.digitalWrite(1, Light_status);
    }
   return
}

And save all those lines.

Are there any known pros and cons to either way?

9

just for giggles

byte Light_toggle(byte fctn)
{
  if(!fctn) return Light_status;
  else
  {
    Light_status = (++Light_status) % 2;    
    CS.digitalWrite(1, Light_status);
  }
  sleep_time(10,1);
}

Are there any known pros and cons to either way?

if they both work the same, the second one is slightly shorter to write, maybe if you sat down and clock it it might be a few cycles faster which can help in large/complex programs

10

Yes you can do that as long as LOW and HIGH are defined as 0 and 1. More generally though, they are not, so this form can be used:

       CS.digitalWrite(1, (Light_status == 0) ? LOW : HIGH);

Also you can simplify your arithmetic to

     Light_status = (++Light_status) % 2;

You last return statement also does not return anything.

11

Thanks both.

I "shot myself in the foot" because I was cut/pasting and forgot to change the == 0 to == 1 in the second instance.

I was pulling my hair out wondering why it wasn't working.

Then I got to thinking that as the "flag" is only 0 or 1, that it could be used as the state - as it pretty well is anyway - to write to the output.

Yeah, the end may not be the best. There was more code but I negated it as it wasn't important.

The routine only returns when I need to know the status.

Now you have me wondering:
As it returns "light_status" in some instances, and NOTHING in others, is there a problem waiting to be discovered?

Should I make it return 0 for the other time/s when nothing is happening? But then I would have to change the line calling it too - wouldn't I?

(Got to go home and read more in the C++ book)

12

im C-tarded but I am pretty sure if you dont return it just returns null or zero (neither is == to 1)

13

Osgeld:
im C-tarded but I am pretty sure if you dont return it just returns null or zero (neither is == to 1)

No, it returns whatever value just happens to be in the register that return values are returned in when the function exits. Different compilers and different levels of optimization wll change what it returns.

When talking about C, I tend to say that some languages go out of the way to protect you from hanging yourself, while C will happily drive to the hardware store, get some rope, make a hangman's noose, and give it to you, saying have fun.

14

Close equivalent -

const uint8_t   LIGHT_OFF       = LOW;
const uint8_t   LIGHT_ON        = HIGH;

enum { LIGHT_STATUS = 0, LIGHT_TOGGLE };

uint8_t light_toggle(int fctn)
{
    static uint8_t      s_Light_status;
    
    switch ( fctn )
    {
        case LIGHT_STATUS:
            break;

        case LIGHT_TOGGLE:  
            s_Light_status = ((s_Light_status == LIGHT_OFF) ? LIGHT_ON : LIGHT_OFF);
            CS.digitalWrite(1, (s_Light_status == LIGHT_OFF) ? LOW : HIGH);
            break;
    }
    
    // need to return something and it doesn't happen automatically
    // so might as well return the current state

    return s_Light_status;
}
15

Wow.

I never thought there were so many ways to do one thing. :wink:

Na, kidding.

Ok, I'll think about it and edit the code tonight when I get home.

Must do it as there is a mistake in there which I did when typing anyway.

Thanks for helping with this problem.

16

Osgeld:
im C-tarded but I am pretty sure if you dont return it just returns null or zero (neither is == to 1)

If the function is declared with a return type and the implementation does not explicitly return an expression that can be converted to that type, I'd expect that to cause a compilation error.

17

If the function is declared with a return type and the implementation does not explicitly return an expression that can be converted to that type, I'd expect that to cause a compilation error.

One would certainly hope so, but, sadly, that isn't necessarily the case. A function declared to return an int, for instance, that doesn't have a return statement still compiles, but, of course, doesn't actually return anything but garbage.