Trouble with a simple linear program

Projects Programming
1

I've been trying for a couple of months now to learn enough about Arduino programming to write a simple program for an AdaFruit Trinket (ATTiny85) that uses only one input and two outputs. Any fool should be able to do that, right? Here's what the sketch does:

At the start of the loop, the program waits indefinitely for switch S1 to close. When that happens it blinks LED1, turns on LED2, and starts a 100 second timer. At the end of 100 seconds, or when S1 closes again, whichever occurs first, LED1 is blinked again, LED2 is turned off, and the program returns to the start.

I've purchased four books (the best one being Sams Teach Yourself Arduino Programming in 24 Hours) and read everything I could find, including Robin2's excellent "Demonstration Code for Several Things at the Same Time". I've tried splitting the program into several functions, following Robin2's "state" suggestion, using "delay" for blinking the LED and (millis() + 100000) for the running timer. But I can't find much info on how to "goto", "gosub", or "call" a function, and after compiling maybe a hundred times by trial and error, the pseudo "10" programming has brought me to my knees with its cryptic compile error messages and demands for placing brackets, braces, and semicolons just right. Even on those occasions when the compile was successful and the program was running (incorrectly) I usually couldn't tell how I had fixed an error. I keep thinking that if you put a hundred monkeys in a room full of Arduinos and wait a thousand years, one of them will eventually write some workable code.

I know the forum frowns on anyone trying to get someone else to write code for them, but is there a chance someone will give me a clue or two on this problem, or point me to something similar. If I can just get started I'll do all the debugging, the hard way. I'm nearly 87 years old and had hoped to finish this project before cashing in.

okasional

2

Mixing delay(...) and millis() can get you into trouble. Lose delay(...) and just use millis().

millis()+anything can get you into trouble. Only use subtraction with millis(). Nick Gammon has a web page that explains this and how to use millis().

If you want code written for you, head over to Gigs and Collaborations and bring your wallet.

If you want us to help, take your best code, autoformat it in the Arduino IDE using CTRL-T, use code tags and post it here, and explain what is wrong with it. This process takes a little time but is MUCH cheaper and makes you more prepared for your next project.

Have you debounced S1?

How do you have things wired?

3

OP, don't despair. Kudos for plugging away at this and continuing to learn. I'll make two suggestions:

  1. I've never used a Trinket, but I recall reading that it does not have a serial port available for debugging. Using a serial port is the only practical method for debugging in the Arduino ecosystem. Once you get your program compiled and uploaded, the serial port will be invaluable for printing variable contents and tracing program flow. So, I'd say start by picking up a simple Arduino Uno (or clone) to make things a lot easier. There's also a much larger Uno user base in these forums to help you.

  2. Once you have an Uno -- check out the example programs built into the Arduino IDE (File --> Examples). Compile them, upload them to your board, run them, STUDY them, modify them to do different things, learn from them. For your application, start with 01.Basics and 02.Digital.

4

As @vaj4088 says, post the code for your best attempt and we may be able to help.

...R

5

If you break it down into events and actions:
Events
loop starts
S1 closes
100 sec Timer elapses
loop finishes
Actions
start 100 sec Timer
Blink LED1
Turn LED2 on
Turn LED2 off
restart

Which actions are triggered by which events? When the loop starts we simply wait for S1 to close, in fact S1 restarts the loop each time it is pressed. Alternatively, the 100 second timer performs the same function. These events therefore have the same effects resulting in the same actions:
Blink LED1
Toggle LED2
Reset Timer

Okay so far?

We'll use some p-code so it doesn't look like I'm writing code for you.
LOOP START
WAIT for S1
Now we have S1
Blink LED1
Start Timer
Toggle LED2
WAIT for S1 OR Timer (to finish)
We have S1 or Timer finishing
Blink LED1
Toggle LED2
return to LOOP START
LOOP FINISH

A little deeper...

To wait for S1 we can use
while(!S1) delay(1); // !S1 assumes that S1 will become non-zero when closed
// also assumes S1 normally open momentary switch
This will sit in an indefinite loop until S1 is logic 1.
Once you break clear of that loop, call your functions to blink, toggle and start the timer.
To blink, turn on, wait, turn off.
To toggle, I use a ternary(trinary) function (test)?action_A_if_true:action_B_if_false; so you might say
LED2_ON?LED2_OFF:LED2_ON;
To start the timer, simply mark the time.

Now we want to wait again except there are two events that can interrupt this loop, either
S1 is closed or the timer expires, so..
while(!S1 && Timer) delay(1);
For this loop to remain active, both S1 has to be in the off position and Timer must be active. If
either of those conditions fail, then you break clear. Blink LED1, toggle LED2 and return to the start which will in fact simply be a matter of allowing the code to fall through to the end of the control loop.

I've tried to be as cryptic as possible without giving too much away. Let us know how it turns out.

I just turned the corner on 60 myself so I know how much fun you're having. Difference being, I've been doing this for 45 years already and still loving it.

dkw
semi-retired systems engineer

6

Here's a step-by-step on how, and why, to create functions. There are several tutorials in the series before this one if you want to work your way up.

IMHO, a switch/case state machine would work well for your project.

7

DKW, thank you, thank you, thank you. I've printed out your reply and am inspired to dig in again.

Your list of events and actions is correct, but not sure I understand your statement "...in fact S1 restarts the loop each time it is pressed." Your pseudo-code is better, but I would modify it slightly:

LOOP START
Wait for S1
Do a few things and start a Timer
Wait for Timer to finish or another S1
Do a few things
Return to LOOP START

I have been trying to set a variable Endtime = ((millis() + 10000)) at start of the 100 second Timer, then
recursively looking for (millis() > Endtime) || (switch closed) to trigger the second bunch of things. Is
that a reasonable approach? I didn't understand the recommendation to not use millis() and delay in the same program. They seem like apples and oranges to me.

Out of curiosity, would a "real" programmer possibly make a function out of (blink LED1 and reverse the state of LED2) and call that function at both occurances of S1 closing, to minimize code?

Thanks again for your constructive suggestions.

okasional

8

dougp,

That tutorial on creating and using functions will give me many hours of enlightenment. None of the books I've obtained go very deeply into the why and how of functions, and I'm very grateful.

okasional

9

Start writing your sketch.

What would you write for : "Wait for S1"
How would you "Start a timer"
How would you "Wait for timer"

Then post the code. You will have to get your hands dirty to get a feel for it.

10

The web page that I was thinking of is Gammon Forum : Electronics : Microprocessors : millis() overflow ... a bad thing?

There is nothing inherently wrong that I know of in mixing delay(...) and millis(), but many people when they try to mix these functions just make a complete mess of things. Besides, if you are using millis() for timing, you should have little need for delay(...). Unless you are expert enough to use interrupts, there is nothing that your Arduino can do while delay(...) is doing its thing. Nothing. delay(...) just wastes the considerable throughput of your Arduino, and keeps you from checking inputs or producing other outputs.

My philosophy: Avoid delay(...) except for toy examples and some debugging; use millis() and state machines for any timing that needs to be done.

11

If I've read your original post incorrectly, then the second pressing of S1, if the timer is still active will stop the process (blink LED1 and toggle LED2) and go into a wait state to be pressed again. Fair enough. Changes a couple of things but not by very much.

Other notes. As has been stated by others, there's nothing inherently wrong in using delay and/or millis, both or alone. Personally I would avoid using delay full stop as nothing else can be active during this wait state. You see my small delay in the indefinite loop, in reality this would be delayMicroseconds(1). The reason for this is the way the code gets compiled (depending on optimization). If you simply had a loop that was
while(something);
the compiled code would get into such a tight loop that the flags needed to test the condition would never be set in time for testing. Its like trying to read a character from the serial port before its had time to arrive. Sometimes we just have to wait for non-zero propagation delays.

As for what a 'real' programmer would do, beats me. I'm an engineer. I like to modularize as much as is reasonable. I'd make a function to blink LED1. Another function to toggle LED2. A third function that started the timer and another to check it.

Timing is a neat thing. Most of what I do deals with industrial automation, robotics, SCADA and such. Timing is present in everything. Firstly, never initiate a timing loop where you add the duration to the current time (i.e. Endtime = ((millis() + 10000)) is a big no-no). The reason for this is that if you should be unlucky enough to be testing the condition when the timer rolls over at the end of the big count, you'll miss it. Always mark your start time such as StartTime = millis() and then test the current time against it to see if your elapsed time has passed (i.e millis() - StartTime >= 10000). This way, even if you hit the rollover, 2's compliment subtraction will still give you the correct answer.

Next, set your time check in its own function. This then can be summoned from anywhere else in your code. Use your time check to set a flag when the elapsed time has expired, in this way you can prioritize how you want to deal with events. So we can do something like this:
static inline bool timeCheck(long startTime, long Duration) {return (millis() - startTime >= Duration)?true:false;}
and then call timeCheck(StartTime,10000) anywhere, anytime and once the duration has elapsed, it will return true.

You can use similar functions to test all your conditions as well as for executing your actions so then your main() loop simply becomes a sequence of tests and function calls.

A project I'm just finishing now is a bit over 2,500 line of code. The main() loop, inside an indefinite while(1), is comprised of 9 (nine) if statements.

12

As for what a 'real' programmer would do, beats me. I'm an engineer.

Programmers are half engineers half artists. Algorithms to a solve software problem are numerous. Some will be fast, some will be elegant... Well, you know what I mean. :wink:

13

Nothing against programmers, we just think differently.

14

That's the half artist part.

Jacques

15

At the start of the loop, the program waits indefinitely for switch S1 to close. When that happens it blinks LED1, turns on LED2, and starts a 100 second timer. At the end of 100 seconds, or when S1 closes again, whichever occurs first, LED1 is blinked again, LED2 is turned off, and the program returns to the start.

So, at any given moment, your sketch can be in one of three states (you can use the word 'phase', but everyone else uses 'state'):

1 - waiting for S1
2 - in the 100 second timer
3 - performing that final blink before everything goes idle again

So, I usually start by creating a variable whose job it is to hold that state:

enum State {
  STANDBY,
  TIMING,
  FINAL_BLINK
} state = STANDBY;

My main loop has a section to read any inputs needing to be read, and then a section with a switch statement, which does what needs to be done at that moment. This will depend on the state the sketch is in;

void loop() {
  byte s1 = digitalRead(s1Pin);

  switch(state) {
    case STANDBY:
      break;

    case TIMING:
      break;

    case FINAL_BLINK:
      break;

  }

}

I then think about what needs to be done in each state. Most particularly, I address what events result in a state transition, and what needs to be done to execute that state transition.

The "standby" case:

case STANDBY:
  if(s1 is closed) {
    set the 100second timer to millis();
    do whatever needs to be done to start LED1 blinking
    turn LED2 on
    state = TIMING;
  }
  break;

Well, that's pretty straightforward. Except maybe the blinking bit. Maybe it word be woth breaking that out into a function named start_blinking(), which we will write later.

The "timing case"

case TIMING:
  if(s1has just closed OR 100 seconds have passed) {
    do the set up for the final blink
    state = FINAL_BLINK;
  }
  else {
    do some blinking.
  }

This requires another variable - we need to hold whether or not s1 was closed in the previous execution of the loop, so that we can compare it then to now. I usually use:

byte s1state;

void loop() {
  byte s1PrevState = s1State;
  s1State = digitalRead(s1Pin);
  boolean s1_closing = (s1PrevState==HIGH) && (s1state == LOW);

The variable s1_closing now contains true if the s1 switch closed on this iteration of loop.

The final state is FINAL_BLINK. To detect the end of the final blink, you need to catch when LED1 changes from unlit to lit followed by a change from lit to unlit.

case FINAL_BLINK:
  if(there has been a complete final blink) {
    turn off blinking.,
    turn off led 2
    state = STANDBY;
  }
  else {
    do some blinking.
  }

To detect that final blink, you count each change of the LED either way. The trick is: at the time when you change state (ie: in the if() in TIMING state), if the LED is lit , set the initial value of that count to 0; and if the LED is already dark, then set the count to 1. When the count reaches three, the LED will have changed from light to dark after a final blink.

(alternatively, just count the 'led just gone dark events' and wait for it to reach 2 - initialising it to 1 if the LED is already dark)

The business of counting the changes can be done in the blink code. There's no need to not count when the sketch is not in FINAL_BLINK state, it can just sit there ticking over the 'led just gone dark' counter all the time.

16

Wow! You guys have given me so many suggestions and explained so many things so that I now believe I can write this sketch, and that's scarry.

First, to PaulMurrayCbr: Your response is a poor man's tutorial on state machines, and I'm looking forward to studying it. I apologize to you for my poor choice of words in the description. It's really simpler than you thought. Everywhere I said "blink LED1" I really meant just one short blink. This is actually an output to another circuit which I've simulated with an LED for testing. (As has been mentioned, the little ATTiny85 chip doesn't support serial comm for troubleshooting.) I should have said "pulse" LED1 instead of "blink" it. And as long as I'm coming clean, the switch S1 is actually to simulate a signal input from another circuit, so there is no bounce to worry about.

And to DKWatson, I still don't understand the "adding to millis()" problem. Suppose I start the 100 second timer with a variable Endtime = millis() + 100000, and every iteration of the loop includes a test to see if millis() > Endtime. There is no rollover to worry about. Sooner or later millis() will be greater than Endtime and I will see that the next time through the loop. I don't have to spot the exact moment Endtime is exceeded.

I'm starting to build the sketch little by little, testing as I go. My first test makes sure all my declarations and setup function are correct. I have a problem with the initial wait for S1 in the loop function:

const byte S1=0; // control switch connected from pin #0 to ground
const byte LED1=1; // blue LED connected via resistor to pin #1
const byte LED2=2; // red LED connected via resistor to pin #2

void setup()
{
pinMode(S1,INPUT_PULLUP);
pinMode(LED1,OUTPUT);
pinMode(LED2,OUTPUT);
}

void loop()
{
if (digitalRead(S1) == LOW) // If I use this instruction the sketch compiles and runs just fine.

while(!S1) delay(100) // If I use this instruction the sketch compiles without error but nothing
happens when S1 is closed.

In either case above the remaining code in loop() does a bunch of blinking of both LEDs so I can tell if the switch is detected.

What dumb mistake have I already made?

okasional

17

okasional:
And to DKWatson, I still don't understand the "adding to millis()" problem. Suppose I start the 100 second timer with a variable Endtime = millis() + 100000, and every iteration of the loop includes a test to see if millis() > Endtime. There is no rollover to worry about. Sooner or later millis() will be greater than Endtime and I will see that the next time through the loop. I don't have to spot the exact moment Endtime is exceeded.

If the current value of the millis() counter happens to be 0xFFFFFFF5 (which it would be approximately 1193 hours after power-up), then adding 100 (0x64 hex) to it would overflow the maximum value possible in an ‘unsigned long’. The result would be Endtime = 0x00000059.

Now, the next time through ‘loop()’, the millis() counter will still be 0xFFFFFFF5 (or 0xFFFFFFF6 at most). Thus your millis() > Endtime test condition will be true almost immediately, not 100 milliseconds later as you wanted.

If you always use unsigned long subtraction for the comparison when working with millis() timers, the rollover problem is totally avoided (unless you try to time an interval longer than 1193 hours). See: Arduino Playground - HomePage

18

Forget everything I said about millis. I understand the problem completely now. I should have realized that millis can't just go on increasing forever, without limit. I'll use the subtraction method.

okasional

19

Here's my first shot at the complete sketch. I hope I inserted the code correctly. It compiles with no errors, but nothing happens when when I close switch S1. I tried replacing the "while" statement with "if digitalRead(S1,LOW);" but it still didn't work.

okasional

[CODE
// Simple Linear Sketch version 1.0 18 Sept 2017 okasional

const byte S1=0; // control switch connected from pin #0 to ground
const byte LED1=1; // blue LED connected via resistor to pin #1
const byte LED2=2; // red LED connected via resistor to pin #2

void setup()
{
pinMode(S1,INPUT_PULLUP);
pinMode(LED1,OUTPUT);
pinMode(LED2,OUTPUT);
}
void loop()
{
unsigned long startTime = millis();
unsigned long interval = 5000; // timer period = 5 seconds
while(!S1); // wait for S1 to close
delay(1); //
digitalWrite(LED2,HIGH); // blink LED2 1/2 sec to simulate output
delay(500); //
digitalWrite(LED2,LOW); //
digitalWrite(LED1,HIGH); // turn on blue LED while timer active

while (millis() - startTime) >= (interval)&&(digitalRead(S1) = HIGH); // wait for S1 closure
delay(1); // but not longer than 5 seconds

digitalWrite(LED2,HIGH); // blink LED2 1/2 sec to simulate output
delay(500); //
digitalWrite(LED2,LOW); //
digitalWrite(LED1,LOW); // turn off LED1 while timer idle
}

//compile error "expected primary-expression before '>=' token" ?????????
CODE]

20

vaj4088:
If you want us to help, take your best code, autoformat it in the Arduino IDE using CTRL-T, use code tags and post it here, and explain what is wrong with it. This process takes a little time but is MUCH cheaper and makes you more prepared for your next project.