Simple timer with tinker interface

Dear All,

I have been using my spark core as a timer for my nephews PlayStation for a year now - it worked fine.
I am a beginner to programming but it only took a few simple lines.

All I did was to add the following lines in the void loop() part of tinker

if( digitalRead(D1) == 1){goto one;}
if( digitalRead(D2) == 1){goto two;}
if( digitalRead(D6) == 1){goto six;}

goto start;


six:
digitalWrite(D7,HIGH);
delay (6000);
digitalWrite(D7,LOW);
goto start;

two:  
digitalWrite(D7,HIGH);
delay (2000);
digitalWrite (D7,LOW);
goto start;

one:
digitalWrite(D7,HIGH);
delay (1000);
digitalWrite (D7,LOW);
goto start;

I dont know what happened but not it refuses to work now. Tinker does not seem to work with it. The core seems to be offline in tinker despite breathing cyan.

If I bring the physical inputs (D1, D2, D6) High it works- but tinker from my Android has been locked out. Plus to get the core back online I have to use putty to re-enter wifi credentials. Sometimes when I try to press the buttons on tinker it will make the core restart. It never used to do this - it was working fine.

        //PUT YOUR VARIABLES HERE
    
    int tinkerDigitalRead(String pin);
    int tinkerDigitalWrite(String command);
    int tinkerAnalogRead(String pin);
    int tinkerAnalogWrite(String command);
    
    void setup()
    {
    	Spark.function("digitalread", tinkerDigitalRead);
    	Spark.function("digitalwrite", tinkerDigitalWrite);
    	Spark.function("analogread", tinkerAnalogRead);
    	Spark.function("analogwrite", tinkerAnalogWrite);
    
    	//PUT YOUR SETUP CODE HERE
    	
    	  pinMode(D7,OUTPUT);
    	  pinMode(D1,INPUT_PULLDOWN);
          pinMode(D2,INPUT_PULLDOWN);
          pinMode(D6,INPUT_PULLDOWN);
    }
    
    void loop()
    {
    //PUT YOUR LOOP CODE HERE
    start:
    //int button2 = digitalRead(D2);
    //int button6 = digitalRead(D6);
    //int button1 = digitalRead(D1);
    
    //if(button6==1){goto six;}
    //if(button2==1){goto two;}
    //if(button1==1){goto one;}
    
    if( digitalRead(D1) == 1){goto one;}
    if( digitalRead(D2) == 1){goto two;}
    if( digitalRead(D6) == 1){goto six;}
    
    goto start;

    six:
    digitalWrite(D7,HIGH);
    delay (6000);
    digitalWrite(D7,LOW);
    goto start;
    
    two:  
    digitalWrite(D7,HIGH);
    delay (2000);
    digitalWrite (D7,LOW);
    goto start;
    
    one:
    digitalWrite(D7,HIGH);
    delay (1000);
    digitalWrite (D7,LOW);
    goto start;
    }
    
    int tinkerDigitalRead(String pin) {
    	int pinNumber = pin.charAt(1) - '0';
    	if (pinNumber< 0 || pinNumber >7) return -1;
    	if(pin.startsWith("D")) {
    		pinMode(pinNumber, INPUT_PULLDOWN);
    		return digitalRead(pinNumber);}
    	else if (pin.startsWith("A")){
    		pinMode(pinNumber+10, INPUT_PULLDOWN);
    		return digitalRead(pinNumber+10);}
    	return -2;}
    
    int tinkerDigitalWrite(String command){
    	bool value = 0;
    	int pinNumber = command.charAt(1) - '0';
    	if (pinNumber< 0 || pinNumber >7) return -1;
    	if(command.substring(3,7) == "HIGH") value = 1;
    	else if(command.substring(3,6) == "LOW") value = 0;
    	else return -2;
    	if(command.startsWith("D")){
    		pinMode(pinNumber, OUTPUT);
    		digitalWrite(pinNumber, value);
    		return 1;}
    	else if(command.startsWith("A")){
    		pinMode(pinNumber+10, OUTPUT);
    		digitalWrite(pinNumber+10, value);
    		return 1;}
    	else return -3;}
    
    int tinkerAnalogRead(String pin){
    	int pinNumber = pin.charAt(1) - '0';
    	if (pinNumber< 0 || pinNumber >7) return -1;
    	if(pin.startsWith("D")){
    		pinMode(pinNumber, INPUT);
    		return analogRead(pinNumber);}
    	else if (pin.startsWith("A")){
    		pinMode(pinNumber+10, INPUT);
    		return analogRead(pinNumber+10);}
    	return -2;}
    
    int tinkerAnalogWrite(String command){
    	int pinNumber = command.charAt(1) - '0';
    	if (pinNumber< 0 || pinNumber >7) return -1;
    	String value = command.substring(3);
    	if(command.startsWith("D")){
    		pinMode(pinNumber, OUTPUT);
    		analogWrite(pinNumber, value.toInt());
    		return 1;}
    	else if(command.startsWith("A")){
    		pinMode(pinNumber+10, OUTPUT);
    		analogWrite(pinNumber+10, value.toInt());
    		return 1;}
    	else return -2;}

Any help much appreciated

One thing, please drop all thes goto statements and use switch() case or if() .. else if() ..
Next, replace all Spark.xxx() calls with Particle.xxx() - Spark is deprecated.
But the main point here seems to be that you never drop out of loop() which would be required in order to keep the cloud connected.
The only thing that saves you from time to time is the implicit call of Particle.process() when one of your branches hits and calls delay().

try this instead

void loop()
{
    if(digitalRead(D1))
    {
      digitalWrite(D7,HIGH);
      delay (1000);
      digitalWrite (D7,LOW);
    }

    if(digitalRead(D2))
    {
      digitalWrite(D7,HIGH);
      delay (2000);
      digitalWrite (D7,LOW);
    }

    if(digitalRead(D6))
    {
      digitalWrite(D7,HIGH);
      delay (6000);
      digitalWrite(D7,LOW);
    }
}

or a bit more “refined”

const int pins[] = { D1, D2, D6 };                 // a list of your pins to monitor
const int pinCount = sizeof(pins) / sizeof(pins[0]);
const int delays[pinCount] = { 1000, 2000, 6000 }; // a list of the respective delay times 

void loop()
{
  for (int i = 0; i < pinCount; i++)
  { // iterate over all elements in your pin list
    if (digitalRead(pins[i]))                     // read the current pin state
    {
      digitalWrite(D7,HIGH);
      delay(delays[i]);                          // use the respective delay time
      digitalWrite(D7,LOW);
    }
  }
}

Thank you very much for the reply ScrurrR - I will work through that and get back to you.

Please could you expand upon

"But the main point here seems to be that you never drop out of loop() which would be required in order to keep the cloud connected."

How is this normally achieved?

Thanks

I have now worked out why my core had been working fine for a year and only a few days ago it stopped working. I made a slight modification to my code. This is the original that worked.

void loop()
{
    
//PUT YOUR LOOP CODE HERE
 	
//int button2 = digitalRead(D2);
//int button6 = digitalRead(D6);
//int button1 = digitalRead(D1);





if( digitalRead(D1) == 1){goto one;}
if( digitalRead(D2) == 1){goto two;}
if( digitalRead(D6) == 1){goto six;}


goto end;

six:
digitalWrite(D7,HIGH);
delay (6000);
digitalWrite(D7,LOW);
goto end;

two:  
digitalWrite(D7,HIGH);
delay (2000);
digitalWrite (D7,LOW);
goto end;

one:
digitalWrite(D7,HIGH);
delay (1000);
digitalWrite (D7,LOW);
goto end;


end:  

digitalWrite(D7,LOW);

}

My program was not reaching the } of the loop.

Thanks for helping me to notice this - I will now try to work without the goto's.

Also im trying to move from delay() to millis()

This is exactly what I meant with

loop() is just a function (which doesn't really loop at all) of which your code flow will drop out once it reaches the final } and hence returns control to the background loop (which you don't see).
This background loop does all the cloud stuff and other things and then calls loop() again in a never ending cycle.


For this you'd need to rethink how to work with concurrency between the individual pins.
At the moment each pin state causes the D7 LED to light up for the respective time, but with the non-blocking millis() approach, you'd need to find a way to combine three seperate times into one output value/state.

Thanks again ScruffR - It was driving me crazy. I thought the core was broken.