Help me to think like a [better] coder

Projects General Guidance
1

Hey All- So I started teaching myself the arduino. I'm new to both the physics side and the code side, but I was able to create my own problem! (And solve it! Yay.)

I wanted to create an arduino that can perform sound signals used while boating and sailing in the fog. Depending on if one is using a ship's motor, is under sail, moving or anchored there are different sound patterns (ie. 1 long blast, 2 short blasts, delay for two minutes, 1 long blast, 2 short blasts...) Link to COLREGS Sound Patterns
I wanted to be able to break out of a pattern to step into the next one without having to wait for the code to finish playing out one signal.

Like I say I did get a test to work. However two things:
•I'm sure the code is inefficient and would love to know what I'm doing poorly
•Figuring out where to begin was kind of daunting

It took me quite a bit of time to figure out how to start. After a few times trying to think it through with arrays, I decided to start sketching out the workflow Draw.io Wireframing which led me to a switch case solution and a slew of functions, one each for each signal pattern.

I still think there would be a way to use one function and send it parameters such as (dash, dash, 1 minute) or (dash,dot,dot,2 minutes)

Anyway here is my solution:https://circuits.io/circuits/2545224-conav-foghorn

Thanks for your help and insight!

const int ledPin = 13; // will be used for testing button
const int hornPin = 6; // horn out (buzzer)
int buttonPushCounter = 0;   // counter for the number of button presses
int buttonState = 0;         // current state of the button
const int buttonPin=2; //digital pin from button
const int powerPin=11;
const int movingPin=10;
const int anchorPin=9;

// the setup routine runs once when you press reset:
void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(hornPin, OUTPUT);
  pinMode(powerPin, OUTPUT);
  pinMode(movingPin, OUTPUT);
  pinMode(anchorPin, OUTPUT);
  pinMode(buttonPin, INPUT);
}

void loop() {

  // read the pushbutton input pin:
  readButton();
 
switch (buttonPushCounter)
    {
  case 0:
      
        digitalWrite(powerPin,LOW);
  	digitalWrite(movingPin,LOW);
  	digitalWrite(anchorPin,LOW);
        
  break;
  case 1: 
  
  	  digitalWrite(powerPin,HIGH);
  	  digitalWrite(movingPin,HIGH);
  	  digitalWrite(anchorPin,LOW);
          powerMake();
  
  break;
  case 2:
  
    digitalWrite(powerPin,HIGH);
    digitalWrite(movingPin,LOW);
    digitalWrite(anchorPin,LOW);
    powerStopped();
  
  break;
  case 3:
  
    digitalWrite(powerPin,LOW);
    digitalWrite(movingPin,HIGH);
    digitalWrite(anchorPin,LOW);
    sailingVessel();
  
  break;
  case 4:
  
    digitalWrite(powerPin,LOW);
    digitalWrite(movingPin,LOW);
    digitalWrite(anchorPin,HIGH);
    vesselAnchored();
  
  break;
  case 5:
  
    digitalWrite(powerPin,HIGH);
    digitalWrite(movingPin,HIGH);
    digitalWrite(anchorPin,LOW);
  
  break;
  default:
  
    digitalWrite(powerPin,HIGH);
    digitalWrite(movingPin,HIGH);
    digitalWrite(anchorPin,HIGH);
  }
} 

////////////////////////////////////functions
void readButton(){  
  buttonState = digitalRead(buttonPin);
  if (buttonState == HIGH) { 
    buttonPushCounter++;
    digitalWrite(ledPin,HIGH);
    delay(500);  //added to prevent double tapping
  } else {
    digitalWrite(ledPin,LOW);
    delay(1);
  }
    if (buttonPushCounter >=5) buttonPushCounter=0; //reset the counter
}

void powerMake()
{  // read the pushbutton input pin
readButton();
  while (buttonPushCounter == 1)
  {
    for (int i=0; i<=30 && buttonPushCounter == 1 ; i++) 
    { //pause for 3 secs before first note
      noTone(hornPin);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=111;
    }
    for (int i=0; i<=50 && buttonPushCounter == 1 ; i++) 
    { // play 6 seconds while checking button
      tone(hornPin,200);
      delay(100);
	  readButton();
      if (buttonState == HIGH) i=51;   
    }
    for (int i=0; i<=90 && buttonPushCounter == 1 ; i++) 
    { //pause for 2 minutes (or 9 secs) this is the length of time between signals
      noTone(hornPin);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=111;
    }
    noTone(hornPin);
  }
}

void powerStopped()
{  // read the pushbutton input pin
readButton();
  while (buttonPushCounter == 2)
  {
    for (int i=0; i<=30 && buttonPushCounter == 2 ; i++) 
    { //pause for 3 secs before playing note
      noTone(hornPin);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=111;
    }
    for (int i=0; i<=50 && buttonPushCounter == 2 ; i++) 
    { // play 6 seconds while checking button
      tone(hornPin,200);
      delay(100);
	  readButton();
      if (buttonState == HIGH) i=51;   
    }
    for (int i=0; i<=8 && buttonPushCounter == 2  ; i++) 
    {  //pause < 1 sec unless button pushed
      noTone(hornPin);
      delay(100);
	  readButton();
      if (buttonState == HIGH) i=10;
    }
    for (int i=0; i<=50 && buttonPushCounter == 2 ; i++) 
    { //play one Long
      tone(hornPin,200);
      delay(100);
	  readButton();
      if (buttonState == HIGH) i=51;
    }
    for (int i=0; i<=90 && buttonPushCounter == 2 ; i++) 
    { //pause for 2 minutes (or testing 9 secs)
      noTone(hornPin);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=111;
    }
    noTone(hornPin); //in the result of button pushed, this cancels the horn.
  }
}


void sailingVessel()
{  // read the pushbutton input pin
readButton();
  while (buttonPushCounter == 3)
  {
    for (int i=0; i<=30 && buttonPushCounter == 3 ; i++) 
    { //pause for 3 secs
      noTone(hornPin);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=111;
    }
    for (int i=0; i<=50 && buttonPushCounter == 3 ; i++) 
    { // play 6 seconds while checking button
      tone(hornPin,200);
      delay(100);
	  readButton();
      if (buttonState == HIGH) i=51;   
    }
    for (int i=0; i<=8 && buttonPushCounter == 3  ; i++) 
    {  //pause < 1 sec unless button pushed
      noTone(hornPin);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=10;
    }
    for (int i=0; i<=8 && buttonPushCounter == 3 ; i++) 
    { //play one short
      tone(hornPin,200);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=11;
    }
    for (int i=0; i<=8 && buttonPushCounter == 3  ; i++) 
    {  //pause < 1 sec unless button pushed
      noTone(hornPin);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=10;
    }
    for (int i=0; i<=8 && buttonPushCounter == 3 ; i++) 
    { //play one short
      tone(hornPin,200);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=11;
    }

    for (int i=0; i<=90 && buttonPushCounter == 3 ; i++) 
    { //pause for 2 minutes (or 9 secs) 
      noTone(hornPin);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=111;
    }
    noTone(hornPin);
  }
}


void vesselAnchored()
{  // read the pushbutton input pin
readButton();
  while (buttonPushCounter == 4)
  {
    for (int i=0; i<=30 && buttonPushCounter == 4 ; i++) 
    { //pause for 1 minutes (or 3 secs)
      noTone(hornPin);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=51;
    }
    for (int i=0; i<=50 && buttonPushCounter == 4 ; i++) 
    { // ring BELL 5 seconds (while checking buttonread)
      // ideally instead of a piezo at high frequency this would rap a bell
      tone(hornPin,850);
      delay(70);
      noTone(hornPin);
      delay(24);
	  readButton();
      if (buttonState == HIGH) i=51;   
    }
    for (int i=0; i<=100 && buttonPushCounter == 4 ; i++) 
    { //pause no sound for 1 minutes (or 10 secs)
      noTone(hornPin);
      delay(100);
      readButton();
      if (buttonState == HIGH) i=51;
    }
    noTone(hornPin);
  }
}
2

Maybe have a look at Planning and Implementing a Program

I think, for your project, I would create a 2D array (boatNoises[][]) holding the sound sequences and then a single makeNoise() function could be used - something like this (very rough pseudo code)

void makeNoise(byte boatState) {
   byte numSounds = boatNoises[boatState][0];
   for (byte n = 1; n <= numSounds; n++;) {
      tone(boatNoises[boatState][n]);
   }
}

I know that is very crude because I have not studied the exact nature of the sound sequences but I hope it gives a glimpse of the idea

The idea is to avoid the need for a separate function for each state

...R

3

The solution depends on whether or not you want to specify the delay precisely or you can get away with just one minute/two minutes. If you need to specify it, say, down to the seconds, the best way would be to look into function pointers... (you can google that).

If you only care about distinguishing between 1 and 2 minute delays, that would be an overkill.

#define DIT 0
#define DAH 1
#define DELAY_1M 2
#define DELAY_2M 3
#define END 4


int pattern1[]={DIT, DIT, DAH, DELAY_1M, END};
int pattern1[]={DIT, DAH, DAH, DELAY_2M, DIT, END};

int* currentPattern=pattern1;


void pickPattern()
{
    //logic to check the buttons and see which pattern to pick, sets the global int* currentPattern.  Leave it alone (current pattern) if no button is pressed
    //if it's setting a new pattern (button is pressed), reset index to 0.
}


int index;

begin()
{
    index=0;
}

loop()
{
    
   pickPattern();
    if(currentPattern[index]!=END)
     {
           do_stuff(currentPattern[index]);
           index++;
     }
   else
    { 
        index=0;
    }

}

void doStuff(int stuff)//what a name!
{
switch(stuff)
{
case DIT:
dit();
break;
case DAH:
dah();
break;
case DELAY_1M:
del_1();
break;
//(...)
}
}

void del_1()
{
    for(int i=0;i<240;i++)//240*0.25 seconds=1 minute
    {
        delay(250);
        pickPattern();//see if we want to change the pattern 4 times a second. Or lower the delay and boost the for loop target number (240 now) accordingly. This could be done more enegantly using millis(), but this is okay too.
    }
}

Keep in mind this is pretty much a pseudocode...

4

If you ask for structure, a look at the task macros may be helpful. They allow to write down independent sequential actions (tasks) in an easy-to-read fashion, in contrast to state machines with large syntactic overhead. In fact the task macros construct a state machine, so the woodoo is only the representation in source code, not any unusual principle behind them.

First you write down the signal tasks. like this:

void signal1() {
beginTask();
 taskWaitFor(signalType==1); //wait for global trigger
 tone(...);
 taskDelay(duration1); //equivalent to delay(), but not blocking other tasks
 noTone(...);
 if (stopFlag) taskRestart(); //if you really want to abort the currently played signal
 taskDelay(duration2);
 ...
 if (stopFlag) {
   signalType=0; //no signal active
   stopFlag=false; //stop request handled
 }
endTask();
}

If you really want to terminate a signal prematurely, in favour of a different signal, you can break out of the sequence at any time, using taskRestart() to restart at beginTask(). But in the case of such signals I'd think that every signal should be finished regularly, not aborted. For that purpose I added the stopFlag handling to the above pattern.

Then create the signal dispatcher:

void dispatch(int type) {
 if (signalType != 0 && signalType != type)
   stopFlag=true; //abort the currently played signal
 else
   signalType = type; //start signal playback, 0 for none
}

This piece of code can become part of loop(), no need to wrap it into a function.

Of course it is possible to terminate a signal regularly, by simply setting signalType=0, but this would not give any feedback about the currently (still?) played signal. As is, the signalType is reset only when the current signal has been played completely. Perhaps nicer synchronization can be implemented?

Run the tasks:

#include <ALib0.h> //the task macros and more
int signalType;
bool stopFlag;

void loop() {
//give every task a chance to proceed
 signal1();
 signal2();
 ...
//add whatever remains to do, e.g. which signal to play
 int signalToPlay = 0;
 if (condition1) signalToPlay = 1;
 ...
 dispatch(signalToPlay); //or inline the above code
 ...
}
5

Awesome sauce.

Robin2 - I think that "Planning" post you created is exactly what I'm looking. In fact I'm already implementing the empty functions and a "signalData" 2D array in my next iteration. There's a lot of good info in your post, and that first bit about hacking from old tutorial files rings true. I'm taking my time going thru it.

I want to use the Millis interval check just to get the practice of using it, but for my purpose seems overly complex. As I understand it, the millis interval check is great for doing processes simultaneously, but for this project I execute one process at a time.

@DrDiettrich - Task Macros? First I'd ever heard of those. Are they similar to a library/collection that one imports into the sketch? I like the idea of Easy-to-read. If you could point me to a beginners guide to that I'd appreciate it. I did a quick search, but that quickly started to veer. I definitely need the sound signals to abort during play. My plan is to have one button to cycle thru the playout signals, and since each of those take at least 1 minute to play they need to cut out.
The idea is that our fearless solo sailor will flip this on, set the signal and it auto plays, then get back on the tiller and sail sheets, and keep an eye out for other boats.

As for exactness, @thegoodhen, not even a little bit. Normally this task is carried out by a crew member holding an airhorn, looking all around and hopefully remembering to blow the horn sometime before 2 minutes expires. Though that does give me an idea. Depending on how crowded the port is, one might want to increase the frequency of sounding signals. So I'll add a multiplier POT variable.

Good stuff I'm learning a lot. Mostly still the simple things like syntax at this point, but even learning concepts--if you nest a "for loop" in a "while loop", you need to break out of both to get out immediately. I highly recommend that website circuits.io to build little test circuit/sketches.

6

ericgfx:
<...>
I want to use the Millis interval check just to get the practice of using it, but for my purpose seems overly complex. As I understand it, the millis interval check is great for doing processes simultaneously, but for this project I execute one process at a time.
<...>

Welcome to the forum!

The function millis() is a counter that increments every millisecond, or 1000 a second. There is another timer, micros() that implements every microsecond. When you assign an unsigned long variable, UL, the value of the millis() function, that variable can be used as the beginning time for an event. So, if the variable was UL beginTime, then after you did some work you could print the value of millis() - beginTime and have the duration of the measured process.

Disregarding all of the background (DMA) and look-ahead (pipelining) features in modern microcontrollers, the cpu simply is a single threaded unit... there is no multiprocessing or parallel processing going on. It is all rather basic stuff.

Dots and Dashes sound a lots like Morse Code. So, sending out PARIS in Morse looks something like the below:

char PARIS(byte Letter)
{
  switch(Letter)
    {
    case 0:      // P
      tone(toneOutPin, toneHz); delay(DITmS);  // <dit>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      tone(toneOutPin,toneHz); delay(DAHmS);   // <dah>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      tone(toneOutPin,toneHz); delay(DAHmS);   // <dah>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      tone(toneOutPin, toneHz); delay(DITmS);  // <dit>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      return 'P';
    case 1:      // A
      tone(toneOutPin, toneHz); delay(DITmS);  // <dit>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      tone(toneOutPin,toneHz); delay(DAHmS);   // <dah>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      return 'A';
    case 2:      // R
      tone(toneOutPin, toneHz); delay(DITmS);  // <dit>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      tone(toneOutPin,toneHz); delay(DAHmS);   // <dah>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      tone(toneOutPin, toneHz); delay(DITmS);  // <dit>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      return 'R';
    case 3:      // I
      tone(toneOutPin, toneHz); delay(DITmS);  // <dit>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      tone(toneOutPin, toneHz); delay(DITmS);  // <dit>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      return 'I';
    case 4:      //S
      tone(toneOutPin, toneHz); delay(DITmS);  // <dit>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      tone(toneOutPin, toneHz); delay(DITmS);  // <dit>
      noTone(toneOutPin); delay(DITmS);        // <quiet>
      tone(toneOutPin, toneHz); delay(DITmS);  // <dit>
      noTone(toneOutPin);
      return 'S';
    }
}

When the timing looks like

void setspeed(byte value)  // see:http://kf7ekb.com/morse-code-cw/morse-code-spacing/  
{
  WPM        = value;
  DITmS      = 1200 / WPM;
  DAHmS      = 3 * 1200 / WPM;
  // character break is 3 counts of quiet where dah is 3 counts of tone
  // wordSpace  = 7 * 1200 / WPM;
  wordBreak  = 7 * DITmS;    // changed from wordSpace*2/3; Key UP time in mS for WORDBREAK (space)
  Elements   = MaxElement;   // International Morse is 5 characters but ProSigns are 6 characters
  halfDIT    = DITmS/2;      // Minimum mS that Key must be UP (quiet) before MM assignment to dot/dash
  quarterDIT = DITmS/4;      // Minimum accepted value in mS for a DIT element (sloppy)
  halfDAH    = DAHmS/2;      // Maximum accepted value in mS for a DIT element (sloppy)
  DITDAH     = DITmS + DAHmS;// Maximum accepted value in mS for a DAH element (sloppy)
  DiDiDi     = DITmS * 3;    // Minimum mS that Key must be up to decode a character via MM
}

Ray

7

mrburnette:
There is another timer, micros() that implements every microsecond.

Strictly speaking it increments by 4 every 4 µsecs.

ericgfx:
I want to use the Millis interval check just to get the practice of using it, but for my purpose seems overly complex. As I understand it, the millis interval check is great for doing processes simultaneously, but for this project I execute one process at a time.

It is certainly not obligatory to use millis(). But you might want to think about the fact that it is very time consuming to convert a program that has extensive use of the delay() function if, at a later stage in its development, you find that you really do need to use millis().

...R