Flash led on and off for a defined time when action occurs using millis

Hi all,
Hoping someone can point me in the right direction.
I am not much of a programmer, but pretty good at finding what I need to make things work (generally). I have looked on the forum already and found a great post on pretty much what I wanted - push button led timer example
So rather than a button like in the example, I am using a lora module (dorji sx1276) and this has rx and tx on it's board which is hooked up to my wemos d1 mini r2.
I am sending a "ping" from a raspberry pi over lora every 5 seconds. I first tested this just using serial print on the arduino. and sure enough, every 5 seconds it would print "still connected" if it received a y or Y.
However! With the led, it comes on.. and it never goes off. I have played around trying to mod it a bit, but no matter what I do, it stays on all the time after it receives the first ping!
here is an excerpt from the code - it is actually within another tutorial I modded on a keypad lock:

void loop() {


if (Serial.available()){
    char ch = Serial.read();
    if (ch == 'y' ||  ch == 'Y'){
      
   digitalWrite(13, HIGH);  // LED comes On  
          onTime = millis();  
          }
      if (onTime > 0 && millis() - onTime > 2000)  
            {      
   digitalWrite(13, LOW);  // LED goes off
             onTime = 0;
            }
 } 

I also have this above void setup

unsigned long elapsedTime;
unsigned long onTime;

The only other thing to note is if I pull the power from the dorji module, it does turn the led off, and the led comes back on as soon as I plug it back in again - I would say suspicious, but I can see the rx pulsing every 5 sec when it receives the "Y", and as I mentioned when I used print, it did it every 5 seconds as it arrived, not constantly.
oh the arduino code is in python that is sending and that is:

#!/usr/bin/env python3
import serial
import time

ser=serial.Serial('/dev/ttyS0', 9600, timeout=1)
counter=0

while 1:
ser.write("Y".encode())
time.sleep(5)
counter +=1

I am not using the counter, again this was an example I found I modified. I will need to tidy it at some point.

Hope someone can help and point out what I am doing wrong. It is driving me crazy! :rofl:

For the LED, you need two millis() timers.
One to to time the blink rate, and one to timeout the duration.

consider


enum { Off = HIGH, On = LOW };

const byte ledPin = 13;

#define OnTime      5000
#define FlashTime   100

unsigned long msecLst;
unsigned long msecOn;
unsigned long msec;
bool          on;

void loop() {
    msec = millis ();

    if (on && (msec - msecOn) < OnTime)  {
        if ( (msec - msecLst) > FlashTime)  {
            msecLst = msec;
            digitalWrite (ledPin, ! digitalRead (ledPin));
        }
    }

    if (Serial.available())  {
        char ch = Serial.read();
        if (ch == 'y' ||  ch == 'Y')  {
            digitalWrite(13, On);
            msecLst = msecOn = msec;
            on      = true;
        }
    }
}

void
setup (void) 
{
    Serial.begin (9600);

    pinMode (ledPin, OUTPUT);
    digitalWrite(13, Off);
}


thanks for the replies
@gcjr I tried what you wrote, however as soon as the first "ping" comes from the raspberry pi, the led just flashes constantly. I do notice a blip in the led when a new ping is coming in. I reduced the on time and flash time, but it was just the same with a different flash rate.
What I am looking to do is to flash the led once every time a ping comes in.
I also checked the arduino code and added a "print" function to check it definitely is only sending every 5 seconds, and this seems to be the case.
Thanks,
David

Welcome to the forum.

I hope you don't mind, but there is something that I have to get out of the way first:

You can at least try to act like one :wink:

In the Arduino IDE, you can press Ctrl+T or in the menu: Tools / Auto Format
Make every space, every comma, every indent look good. This is very important. It is not possible to make reliable code when the text does not look good.

You can have your own style. This is the code from your first post in my style:

void loop() 
{
  if( Serial.available())
  {
    char ch = Serial.read();
    if( ch == 'y' ||  ch == 'Y')
    {
      digitalWrite( 13, HIGH);  // LED comes On  
      onTime = millis();  
    }
    
    if( onTime > 0 && millis() - onTime > 2000)  
    {      
      digitalWrite(13, LOW);  // LED goes off
      onTime = 0;
    }
  }

In that code, the 'onTime' is used to hold the millis-value and also as a flag. That is not good programming. Use an extra 'bool' variable for that as gcjr did.
The normal way to turn the led off is also make that 'bool' variable false.

It can be called a pulse-stretcher or a single shot delay after a trigger.
My millis_single_delay.ino is the most basic code that I could think off.

Thanks Koepel,

Yes you are right, I should do a better job on my formatting. I definitely need to clean up the rest of the code too! Can you imagine what my desk at work is like?! :wink:

I did turn on the autoformatting, but it kept putting in curly bracket closers every time I pressed return, so I turned it off just now. (probably makes sense when writing programmes, but maybe not editing? )

I copied your code and applied it to my ping coming in on the serial pin.
I had a panic as the lcd display on the arduino stopped working, as did the keypad and sending data, but it just seemed to be a glitch. After I rebooted the wemos D1 it was fine. :slightly_smiling_face:

The wemos is a bit fickle, I have discovered that when the dorji module is connected to the tx rx pins the arduino IDE can't upload to it as it can't see it on the comms port. As soon as I disconnect power to the dorji module, it works fine. Must share comms or something. Maybe this is a common known thing, but I am fairly new to this.

Thanks for everyone's help on this, it is working great!

Now just to build a headless ad hoc webserver on my pi and write the data sent from the arduino over lora to this :joy: Good luck me!

works for me. stops flashing after 5 sec

Ah cool, I know what it must be. It must be because the ping from the pi is every 5 seconds so it looks continuous. Thanks :+1: