Exiting a function on a key press.

Folks,

I am writing a bit of code for my DSLR camera to help when doing “Light painting”.

This is where I have the shutter open for 30 seconds and I am doing “second curtain flash”.

All the fuss is because I see the problem of pressing the shutter button, getting to the subject and doing the light stuff all the while counting down from 30.

This little box does some of the work for me.

At this stage I don’t have the RTC and may not really need it, but… Hey, It is an option.

Here is a walk through of what I want to happen:
The arduino boots, and it has 2 outputs to the camera. One to “focus”/“wake up” the camera and one to take the picture.
It has 3 inputs - for now. Two of the three start the sequence of taking the picture.
It has 2 x 7 Segment digits to count down and show time/s remaining.

On the arduino side of things:
3 pins for inputs.
4 pins for “7 segment display”
2 pins for digit selection (digit 1 and 2)
2 pins for the decimal points.
2 pins for the I2C bus if there is an RTC used.

I am using ClickButton to allow multiple functions from the 3 buttons/inputs.

I have really changed my way of writing code - included - from when I first started.
For the better? Dunno. But I am trying different ways and see which is better for me.

In the MENU / Set_Pre_time it reads the “key” value for what is pressed. One key increases the value, one decreases and the third “exits”.

I’m stuck how to do that. See code.

Sorry if it is a dumb question.
I have been out of the coding for a while and have forgotten bits of it and would appreciate a bit of help.

So to complete the walk through:
The arduino is waiting.

It seen either of the two inputs activate/trip/trigger/what ever.
It starts to count down and displays the time on the display.
While this is going on it looks and sees if a button is pressed.
If a button is pressed, it aborts taking the picture.
Otherwise it goes on, and takes the picture.
After that, it starts a count down from 30 (seconds and displays it on the screen/display.
Then it goes back to waiting.

I may include a bit of code so even after the photo has been started, if something goes wrong, I can reset the counter.
Academic for now, so I won’t worry about it.

Also, in the “Display_Time” it is a bit … long (?) in how I did it.
It has to translate the value into the binary and send it out the pins.

Is there a trick how to make it a bit smaller?
“arrays”? for instance?

When you look at the code, the thing to load is called “V1”. (That to me is “Version 1” of the code.)

Thanks in advance.

Camera timer V1.zip (2.68 KB)

All capital letters names are, by convention, reserved for constants. DISPLAY_TIME is not a constant. The convention for function names is camelCase, so the function should be displayTime().

You have two nearly identical blocks of code - one to display 0 to 9 on one side and one to display 0 to 9 on the other side. They differ only in the select pin. Create a function that takes a select pin, and get rid of half of the code.

  int key = KEY_SCAN();
  if (key = 1)
  {
    predelay = predelay + 1;
  }
  if (key = 2)
  {
    predelay = predelay - 1;
  }
  if (key = 3)
  {
    //  Exit from here.
    
  }

Why bother calling KEY_SCAN() if you are going to assign new values to key in the if statements? I suspect you wanted ==, not =.

Paul,

Thanks for the info. I am not a full time programmer and so don't know the rules.

What I did do was make a basic flow chart and work from there.

The "display time" part was duplicated a few times and so I made that into it's own little block of code. As with key scan.

I put them in UPPER CASE only because they were the names of the "functions" (or blocks of code) and for my benefit I put their names how I did.

Yeah, maybe not a good move for me if I want to become "one of the boys" at the IT department or write code for a job.

It isn't going to happen. I'm too old and many MANY other things.

I have read a few of the "standards" on how naming works, but it is ..... confusing to me. And honestly: Does it really matter? So long as the code works and is understandable - isn't that the important part?

Some people put the { at the end of the line and the } on a line by itself. I find it hard to read as I can't see the { and } easily - except for the indents - but I have to "read around" that problem. It is pointless me complaining.

I would like to know how you would make a "function" for the DISPLAY_TIME so the two blocks of code can be reduced. I am wort of working on it, but it is early days.

In another piece of code I worked out how to use .[ ] at the end to make arrays and saved 600 bytes of code. I was very pleased with my self for that. It took a lot of work and effort.

The KEY_SCAN part: Yeah, that is not nice. Originally it was going to return simple values - like: 1, 2 , 3, 11, 12, 13 but I started to notice that in a couple of calls to it, I was adding 1, 10, or subtracting 1 or 10.

The code to get the key value and then act on it was long.

if (key_value == 1)
{

}
if (key_value == 10)
{

}

and so on.

As that was pretty well already done when the keys were scanned I decided to return the +1, +10, -1, -10 values. I kept the global "key_value" just in case I did need it in future routines.

The code isn't finished.

I have since changed bits of the code and written more, but still not complete. What I am doing is keeping the parts of code in different tabs as much as possible to help break it down. Whether this is a good thing or not I am not sure, but for now it does help me with things, so I shall keep doing it.

Hope to hear back with more helpful suggestions. (Like the DISPLAY_TIME part.) Though I am looking at/working on it myself also.

This is a cut from the first part of what I have done to change it:

  if (gt == 0)
  {
    digitalWrite(D1_Select,HIGH);
  }
  if (gt == 1)
  {
    digitalWrite(D2_Select,HIGH);
  }
  if (digit == 0)
  {
    digitalWrite(Di1,LOW);
    digitalWrite(Di2,LOW);
    digitalWrite(Di3,LOW);
    digitalWrite(Di4,LOW);
  }

And so have basically halved the size of that part of the code.

Is that sort of what you were meaning?

I would like to know how you would make a "function" for the DISPLAY_TIME so the two blocks of code can be reduced. I am wort of working on it, but it is early days.

It's pretty easy, really. First, you identify the nearly identical code.

    digitalWrite(D1_Select,HIGH);
    if (digit == 0)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 1)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 2)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 3)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 4)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 5)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 6)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 7)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 8)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,HIGH);
    }
    if (digit == 9)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,HIGH);
    }
  }
  digitalWrite(D1_Select,LOW);

and

  if (gt == 1)
  {
    digitalWrite(D2_Select,HIGH);
    if (digit == 0)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 1)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 2)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 3)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 4)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 5)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 6)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 7)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 8)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,HIGH);
    }
    if (digit == 9)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,HIGH);
    }
  }
  digitalWrite(D2_Select,LOW);

Look nearly identical.

The second step is to identify the minor differences and the common code

  if (gt == 0)
  {
    digitalWrite(D1_Select,HIGH);
    // Do a bunch of stuff
  }
  digitalWrite(D1_Select,LOW);

vs.

  if (gt == 1)
  {
    digitalWrite(D2_Select,HIGH);
    // Do a bunch of stuff
  }
  digitalWrite(D2_Select,LOW);

The differences are which display the value is going to and which pin to set high to make the digit go there. The common code actually makes the digit appear by turning on or off the correct pins.

So, create a function to contain the common code, that takes an argument - the digit to display.

void showDigit(byte digit)
{
    if (digit == 0)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 1)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 2)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 3)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,LOW);
    }
    if (digit == 4)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 5)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 6)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 7)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,HIGH);
      digitalWrite(Di3,HIGH);
      digitalWrite(Di4,LOW);
    }
    if (digit == 8)
    {
      digitalWrite(Di1,LOW);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,HIGH);
    }
    if (digit == 9)
    {
      digitalWrite(Di1,HIGH);
      digitalWrite(Di2,LOW);
      digitalWrite(Di3,LOW);
      digitalWrite(Di4,HIGH);
    }
}

Then, call that function:

int display_number(int digit,int gt)
{
  if (gt == 0)
  {
    digitalWrite(D1_Select,HIGH);
    showDigit(digit);
  }
  digitalWrite(D1_Select,LOW);
  if (gt == 1)
  {
    digitalWrite(D2_Select,HIGH);
    showDigit(digit);
  }
  digitalWrite(D2_Select,LOW);
}

As far as the keypad and getting multi-digit numbers, that requires a bit of thought and planning. How do you know that the user is done entering a value? That is, how do you distinguish between 1, 12, and 123? Typically, you'd have some kind of end-of-packet marker. Think about a calculator. It lets you add 123 + 45 = 168. How does it know that you've finished entering a value? Simple, really, you pressed a non-numeric key.

So, what are you going to use in indicate that the number being entered is complete? While you are thinking about that, how will you handle errors made during input? Suppose the value is supposed to be 120, and I fat-finger it and press 1, 2, 9, instead of 1, 2, 0?

Once you define the requirements, implementing them is easy.

If you want, when you have defined the requirements, I'll walk through how I would create a function to collect and process keypad input, returning a value when the input is complete.

i am also very much into Light Painting. Would love to see some of your photos? =)

What kind of camera are you using?
If you are using a Canon DSLR there is some “non-canon” softwares - i use Magic Lantern - which will really extend your camera’s functions. I use a LOT of these new functions in my light paintings, like the intervalometer or the “movement trigger”…
It worth to check out: http://www.magiclantern.fm/

maybe there is something similar for the other camera manufacturers.

How is your project going?

Good luck!
=)

PaulS,

Thanks very much.

I had nearly got what you said in your post. But with minor variations.

It is sort of re-assuring that I had come up with similar to what you suggested at the same time.

Mine is slightly different, but I think I will change it to how you said as it is better.

This is the block of code now:

void DISPLAY_TIME(int time)
{
  int d1;
  int d2;
  
  {
    d2 = time % 10;
    d1 = int(time/10);
  }

  digitalWrite(D1_Select,HIGH);
  display_number(d1);

  digitalWrite(D2_Select,HIGH);
  display_number(d2);

  if (time = 0)
  {
    //  Turn off the display.
    digitalWrite(D1_Select,LOW);
    digitalWrite(D2_Select,LOW);
  }
  return;  //  Needed?
}
//==================================================================
int display_number(int digit)
{
  if (digit == 0)
  {
    digitalWrite(Di1,LOW);
    digitalWrite(Di2,LOW);
    digitalWrite(Di3,LOW);
    digitalWrite(Di4,LOW);
  }
  if (digit == 1)
  {
    digitalWrite(Di1,HIGH);

Right?

Oh, is that return needed?

Boguz, Yes, I have a Canon 550d. I have just got and installed Magic Lantern. Kind of overwhelming. You may want to look at Trigger Trap too. I am waiting for their new version. (NOT the App.) This is a kind of "in between" thing for me which is camera independent.

I'll PM you soon with some of the piccies.

  int d1;
  int d2;
  
  {
    d2 = time % 10;
    d1 = int(time/10);
  }

The curly braces are useless. There is no good reason to separate the declaration from the initialization.

  if (time = 0)

Assigning a value to time in an if statement hardly seems like a good idea.

The return statement is not strictly needed. A return will happen when the end of the function is reached. But, it doesn't hurt anything.

Thanks Paul.

I am really having problems with the quirks of this language.

== vs =

{ } and their use sometimes.

and some other things which creep in now and then.

Yes, the if (time == 0) would be more correct. The two curly brackets are left overs from older code.

I am now "stuck" with keeping the code in the menu blocks waiting for the right code to exit from them. I think I have it right, but it probably not the best.