My first Arduino project, and first attempt at prgramming in a very long time, is working well but I can't help thinking there may be a better way of doing this.
The project is a programmable delay timer to turn three separate stereo components on or off in a user defined sequence (output pin 4, 5, 6) . I've use a lot of IF statements. If the tuner is on (pin 2/interrupt 0 = high), jump to the powerUp sequence. If the more than output1Delay has elapsed turn on ouptut 1 (pin 4). If the more than output2Delay has elapsed turn on ouptut 2 (pin 5). If the more than output3Delay has elapsed turn on ouptut 3 (pin 6). This code is repeated for turning off
I've also done programming the delay in the same way. If the function button has been high for longer than 15000ms enter programming mode and flash a status LED. If the tuner is on start the programming timer. If button 1 is pressed save the time for button1OnDelay (pin 4). Repeat for button 2 & 3 (pin 5 & 6). If all the button delays have been programmed flash the statusLed and go through the same process again. After each stage check to see if we are still within the 2 minute time out period or exit.
The full skeetch is attached below. The long delays (5000ms to debounce, etc) are to slow the sketch for debugging. I will be saving the delay times to flash memory when I learn how to do it.
The approach of using IF statements certainly works. My question is, is there a better/more tidy way of achieving the same result?
Sometime "spaghetti" code cannot the avoided. Highliting code blocks in Arduino editor is a nice feature to help in such code structure.
Too bad it won't auto format, and it is little late( here and in code stage ) to try to find the offending LEFT brackets.
I try it like Chicago voters - auto format early and often!
As a personal preference I would replace the case variables with #define(s) just to make it more to stand out on longer switch code.
But that' s no biggie.
Nice commenting job, one seldom see decent comments in much of the code presented here. Kudos.
Good job.
Thanks, it's good to know I'm on the right track, as messy as it may be. I've fixed up the code so it will autoformat and compile - the function getDelays() [formerly program()] isn't finished so contains lots of bugs. I think I'll try splitting it into more functions once the I know it works.
I read somewhere that #define is being phased out in favour of cosnt int?
One lesson learned from my C programming course "Always comment. Your code will look like somone else's code tomorow."
chopsuwe:
Thanks, it's good to know I'm on the right track, as messy as it may be. I've fixed up the code so it will autoformat and compile - the function getDelays() [formerly program()] isn't finished so contains lots of bugs. I think I'll try splitting it into more functions once the I know it works.
I read somewhere that #define is being phased out in favour of cosnt int?
One lesson learned from my C programming course "Always comment. Your code will look like somone else's code tomorow."
There have been some heated discussion elsewhere on using #define or const.
I always wonder if specifying const is for benefits of people who write messy code and do not keep track what variable / piece of code does what or actually helps coding in general.
My argument against const instead of #define - how do you "define" stuff like #define RAIN "if it rains you do not need to go swimming if you want to get wet"
But it may be a crutch for people who are too busy to comment their code, as you pointed out.
I have learned commenting from my boss who ALWAYS handed me a buggy UNCOMMENTED spaghetti style code to finish. Yesterday, of course.
Have fun, it looks good.
Vaclav
Vaclav:
My argument against const instead of #define - how do you "define" stuff like #define RAIN "if it rains you do not need to go swimming if you want to get wet"
const char * RAIN = "if it rains you do not need to go swimming if you want to get wet" ;
// volatile - variables that can change without notice
volatile int sequence; // variable to contain the power sequence
...
// trigger button
unsigned long previousTriggerMillis = 0; // previous time when the trigger fired.
...
// function button
unsigned long previousFunctionMillis = 0; // previous time when the function fired.
...
/*** Trigger Interrupt ****************************************************************************/
// Interrupt to save the time when the power button's state changed and what it's state is now.
// Can't debounce inside the interupt as it is never called after the de-bounce delay
void interruptTrigger()
{
previousTriggerMillis = millis();
} /*** Trigger Interrupt end *******************************************************************/
/*** Function Interrupt ***************************************************************************/
// Interupt to save the time when the function button's state changed and it's state is now.
// Can't debounce inside the interupt as it is never called after the de-bounce delay
void interruptFunction()
{
previousFunctionMillis = millis();
digitalWrite(output3LEDPin, !digitalRead(output3LEDPin)); // debug - toggle LED3
} /*** Function Interrupt ***************************************************************************/
Why is sequence volatile? It isn't used in an interrupt. However previousTriggerMillis and previousFunctionMillis should both be volatile because they are used in an interrupt.
I was just being silly, now if you can show how to put stuff like that in program memory EASY way.
Actually since I got your attention - why does the above working macro stop printing about two thirds down?
(its 3 am here and I need to do some stuff so I don't have time to get you real printout, sorry . Maybe later today)
I put the TIME in it just for drill , but never gets there. And it confuses the auto formatter when continuation characters are used - too many brackets.
Vaclav
why does the above working macro stop printing about two thirds down?
You use a fair bit of functionality in it (Serial & LCD ), so it could be anything, but not Arduino code... Serial does not contain printf in standard releases.
I got the printf function(s) installed and it works fine by itself in Serial.
Here is the actual output on COM, it evaluates true for debugging purpose.
You see where Failed initialize steps over the macro output since the last \n was not executed.
Maybe printf does not like to print TIME as a string, I have not checked for that.
ASSERT error from here plain Serial.print
function Attach servo parameter 1 @line 3293 file Servo_7.ino Failed Initialize()
forward adjust servo angle in iStateMachine from 0 to 1800
Yes, I want convoluted code so I know what is being executed, especially when I run Processing PC time synchronization.
Thanks for your commentary.
Cheers Vaclav
There have been some heated discussion elsewhere on using #define or const.
Some say this distinction is a straw man and that there is no real difference. I don' t think that's true. A #define is a typeless construct so the macro can be used with any data type. For example, to get the number of elements in an array:
The macro ELEMENTCOUNT(x) returns the number of elements in the x array regardless of data type. You can't do that with const because it requires a type specifier. The cost of this flexibility is that you give up the safety that type checking brings to the party.
The sketch needs breaking up into many smaller functions - if a function is longer than
10 lines seriously consider splitting it up (unless it makes less sense to do so).
Give each function a descriptive but reasonable concise name.
For instance loop() should look like a set of tests, each leading to a function call
that does the approriate action and is named accordingly. That way the debounce
logic (for example) can be separated from the action that the button-press triggers,
its basic separation on concerns.
@MarkT: I agree. Too often students try to make a function do more than one task. Creating a Swiss Army Knife (SAK) function is rarely a good idea. In terms of OOA&D terms, a function should be cohesive. By that, I mean you should be able to describe exactly what the function does in two sentences or less. Any more than that and it is likely trying to do too much. SAK function tend to have long argument lists.
Almost all computer programs break down into five steps:
Initialization
Input
Processing
Output
Termination
Step 1 is usually found in setup(). It sets the environment before the program seems apparent to the user. Steps 2 through 4 are part of loop(). Microcontroller programs usally don't have an obvious Step 5, as they are designed to run until failure, power loss, or some explicit event. It's nice when loop() contains three function calls which can then call sub-functions in a "sideways" refinement of loosely-coupled functions. It makes debugging much easier.
A #define is a typeless construct so the macro can be used with any data type.
You keep mentioning this, however, it is not true. A macro is expanded before compilation, so type checking is still an active part of the code inside it. And no, your macro cannot be used by every data type. Its only applicable to array, pointer, and custom types which overload the subscript operator, try it...
The macro ELEMENTCOUNT(x) returns the number of elements in the x array regardless of data type. You can't do that with const because it requires a type specifier.
You can achieve this using templates, and it really has nothing to do with const.
You keep mentioning this, however, it is not true.
What I said is true:
The macro ELEMENTCOUNT(x) returns the number of elements in the x array regardless of data type.
Perhaps I should have clarified that the macro is useful for an array of basic data types, like char, int, long, etc. I was, however, quite clear that I was talking about an array. It is typeless in that you can use any basic data type with the macro whereas with const you must state the data type.
econjack:
Perhaps I should have clarified that the macro is useful for an array of basic data types, like char, int, long, etc. I was, however, quite clear that I was talking about an array.
Yes, I agree its fine with an array of any underlying type ( not just primitives. ), But I do not understand what you are referring to when you talk about const, its not a declaration, but an expression, which can be assigned to a const value. To write an expression without a define you will either have to write it inline, create a class, or a function.
