An initial blink issue

I am trying to set a UK style level crossing light blinking system and have so far got the two red lights blinking correctly. A third and 4th yellow light need to blink together just once before the 4 red lights start. The red lights start when one sensor is triggered and stops when the second is activated I have been working on this for days perhaps someone can help. Thank you.

the code so far is

int sensor1 = 5; // IR sensor==Pin goes LOW when train detected
int sensor2 = 6; // IR sensor==Pin goes LOW when train detected
int led1 = 10; // Led 1 pin alternating flasher
int led2 = 11; // Led 2 pin alternating flasher
int led3 = 12; // Led 3 pin alternating flasher
int led4 = 13; // Led 4 pin alternating flasher
int ledy1 = 8;
int ledy2 = 9;
int entering_sensor = 5; // which sensor was triggered first
int leaving_sensor = 6; // which sensor shows train leaving
int sequence_started = 0; // this says if the crossing is active
int flash_state = 0; // variable for function flash_leds()
long flash_time = 0; // variable for function flash_leds()
long flash_interval = 900; // time in milliseconds between alternating flashes
int sensor_count = 0; // declare variable for while statement
void setup() {
pinMode(sensor1, INPUT); // sensor1 is an INPUT
pinMode(sensor2, INPUT); // sensor2 is an INPUT
pinMode(led1, OUTPUT); // led1 on pin 10 is an OUTPUT
pinMode(led2, OUTPUT); // led2 on pin 11 is an OUTPUT
pinMode(led3, OUTPUT); // led3 on pin 14 is an OUTPUT
pinMode(led4, OUTPUT); // led4 on pin 15 is an OUTPUT
pinMode(ledy1, OUTPUT);
pinMode (ledy2, OUTPUT);
digitalWrite(led1, LOW); // Start with flashers off
digitalWrite(led2, LOW); // Start with flashers off
digitalWrite(led3, LOW); // Start with flashers off
digitalWrite(led4, LOW); // Start with flashers off
digitalWrite(ledy1, LOW);
digitalWrite (ledy2,LOW);
flash_time = millis(); // for function flash_leds()
}
void loop() {
// The two if statements defines which sensor is which and
// changes sequesnc_started from FALSE to TRUE then starts
// the leds flashing

if ((digitalRead (sensor1) == LOW) && (sequence_started == 0)) {
sequence_started = 1;
leaving_sensor = sensor2;
flash_leds ();
}
if ((digitalRead (sensor2) == LOW) && (sequence_started == 0)) {
sequence_started = 1;
leaving_sensor = sensor1;
flash_leds();
}
// First if== sequence started and continue flashing leds
// Next if== train is leaving the crossing
if (sequence_started) flash_leds();
if ((digitalRead(leaving_sensor) == LOW) && (sequence_started == 1)) {
// as long as the leaving sensor is active the train is still in the crossing
// while checks if train is gone yet
while (sequence_started == 1) {
sensor_count = 0;
for (int i = 1; i < 40; i++) {
if (digitalRead(leaving_sensor) == LOW) sensor_count++;
delay (30);
flash_leds();
}
if (sensor_count == 0) sequence_started = 0;
flash_leds();
}
// At this point train is no longer over the leaving sensor
digitalWrite(led1, LOW); // Turn OFF led
digitalWrite(led2, LOW); // Turn OFF led
digitalWrite(led3, LOW); // Turn OFF led
digitalWrite(led4, LOW); // Turn OFF led
}
}
void flash_leds() {
if (flash_time > millis()) return;
flash_state = ~flash_state;
digitalWrite(led1, flash_state);
digitalWrite(led2, ~flash_state);
digitalWrite(led3, flash_state);
digitalWrite(led4, ~flash_state);
flash_time = millis() + flash_interval;
}

Please post code using code tags, after formatting the code in a standard way using ctrl-T or Auto Format in the IDE.

A third and 4th yellow light need to blink together just once before the 4 red lights start.

Sorry, this is not completely clear. First if they blink together, why are they not driven from the same I/O pin? Second, do the yellow lights blink in every blink cycle, or do they only flash once, before the red lights blink alone for the rest of the activation?

Any instance of future time stamp is a huge red flag that you will experience millis() overflow. In this instance, it will only mean an irregular flash every few days, but it's a bad habit to get into.

millis() timing should always be done by capturing its value in past time stamps, and subtracting it from those values to calculate elapsed time.

Sorry if it wasn't in the right format I am a newby here it is again after I have auto formatted it. Yes I hadn't thought I could use the same pin. The yellow blink should only be at the beggining of the cycle after that not required to blink again.

Thanks for looking at this for me

int sensor1 = 5; // IR sensor==Pin goes LOW when train detected
int sensor2 = 6; // IR sensor==Pin goes LOW when train detected
int led1 = 10; // Led 1 pin alternating flasher
int led2 = 11; // Led 2 pin alternating flasher
int led3 = 12; // Led 3 pin alternating flasher
int led4 = 13; // Led 4 pin alternating flasher
int ledy1 = 8;
int ledy2 = 9;
int entering_sensor = 5; // which sensor was triggered first
int leaving_sensor = 6; // which sensor shows train leaving
int sequence_started = 0; // this says if the crossing is active
int flash_state = 0; // variable for function flash_leds()
long flash_time = 0; // variable for function flash_leds()
long flash_interval = 900; // time in milliseconds between alternating flashes
int sensor_count = 0; // declare variable for while statement
void setup() {
pinMode(sensor1, INPUT); // sensor1 is an INPUT
pinMode(sensor2, INPUT); // sensor2 is an INPUT
pinMode(led1, OUTPUT); // led1 on pin 10 is an OUTPUT
pinMode(led2, OUTPUT); // led2 on pin 11 is an OUTPUT
pinMode(led3, OUTPUT); // led3 on pin 14 is an OUTPUT
pinMode(led4, OUTPUT); // led4 on pin 15 is an OUTPUT
pinMode(ledy1, OUTPUT);
pinMode (ledy2, OUTPUT);
digitalWrite(led1, LOW); // Start with flashers off
digitalWrite(led2, LOW); // Start with flashers off
digitalWrite(led3, LOW); // Start with flashers off
digitalWrite(led4, LOW); // Start with flashers off
digitalWrite(ledy1, LOW);
digitalWrite (ledy2, LOW);
flash_time = millis(); // for function flash_leds()
}
void loop() {
// The two if statements defines which sensor is which and
// changes sequesnc_started from FALSE to TRUE then starts
// the leds flashing

if ((digitalRead (sensor1) == LOW) && (sequence_started == 0)) {
sequence_started = 1;
leaving_sensor = sensor2;
flash_leds ();
}
if ((digitalRead (sensor2) == LOW) && (sequence_started == 0)) {
sequence_started = 1;
leaving_sensor = sensor1;
flash_leds();
}
// First if== sequence started and continue flashing leds
// Next if== train is leaving the crossing
if (sequence_started) flash_leds();
if ((digitalRead(leaving_sensor) == LOW) && (sequence_started == 1)) {
// as long as the leaving sensor is active the train is still in the crossing
// while checks if train is gone yet
while (sequence_started == 1) {
sensor_count = 0;
for (int i = 1; i < 40; i++) {
if (digitalRead(leaving_sensor) == LOW) sensor_count++;
delay (30);
flash_leds();
}
if (sensor_count == 0) sequence_started = 0;
flash_leds();
}
// At this point train is no longer over the leaving sensor
digitalWrite(led1, LOW); // Turn OFF led
digitalWrite(led2, LOW); // Turn OFF led
digitalWrite(led3, LOW); // Turn OFF led
digitalWrite(led4, LOW); // Turn OFF led
}
}
void flash_leds() {
if (flash_time > millis()) return;
flash_state = ~flash_state;
digitalWrite(led1, flash_state);
digitalWrite(led2, ~flash_state);
digitalWrite(led3, flash_state);
digitalWrite(led4, ~flash_state);
flash_time = millis() + flash_interval;
}

Please, it's isn't formatted and there are no code tags.

sorry don't understand what are code tags and I autoformatted please forgive my ignorance

You should read the forum guidelines.
before_adding_code_tags
then paste

After you do that, please try to answer the questions I asked...

Here it is as a public service :wink:

int sensor1 = 5; // IR sensor==Pin goes LOW when train detected
int sensor2 = 6; // IR sensor==Pin goes LOW when train detected
int led1 = 10; // Led 1 pin alternating flasher
int led2 = 11; // Led 2 pin alternating flasher
int led3 = 12; // Led 3 pin alternating flasher
int led4 = 13; // Led 4 pin alternating flasher
int ledy1 = 8;
int ledy2 = 9;
int entering_sensor = 5; // which sensor was triggered first
int leaving_sensor = 6; // which sensor shows train leaving
int sequence_started = 0; // this says if the crossing is active
int flash_state = 0; // variable for function flash_leds()
long flash_time = 0; // variable for function flash_leds()
long flash_interval = 900; // time in milliseconds between alternating flashes
int sensor_count = 0; // declare variable for while statement
void setup() {
  pinMode(sensor1, INPUT); // sensor1 is an INPUT
  pinMode(sensor2, INPUT); // sensor2 is an INPUT
  pinMode(led1, OUTPUT); // led1 on pin 10 is an OUTPUT
  pinMode(led2, OUTPUT); // led2 on pin 11 is an OUTPUT
  pinMode(led3, OUTPUT); // led3 on pin 14 is an OUTPUT
  pinMode(led4, OUTPUT); // led4 on pin 15 is an OUTPUT
  pinMode(ledy1, OUTPUT);
  pinMode (ledy2, OUTPUT);
  digitalWrite(led1, LOW); // Start with flashers off
  digitalWrite(led2, LOW); // Start with flashers off
  digitalWrite(led3, LOW); // Start with flashers off
  digitalWrite(led4, LOW); // Start with flashers off
  digitalWrite(ledy1, LOW);
  digitalWrite (ledy2, LOW);
  flash_time = millis(); // for function flash_leds()
}
void loop() {
  // The two if statements defines which sensor is which and
  // changes sequesnc_started from FALSE to TRUE then starts
  // the leds flashing

  if ((digitalRead (sensor1) == LOW) && (sequence_started == 0)) {
    sequence_started = 1;
    leaving_sensor = sensor2;
    flash_leds ();
  }
  if ((digitalRead (sensor2) == LOW) && (sequence_started == 0)) {
    sequence_started = 1;
    leaving_sensor = sensor1;
    flash_leds();
  }
  // First if== sequence started and continue flashing leds
  // Next if== train is leaving the crossing
  if (sequence_started) flash_leds();
  if ((digitalRead(leaving_sensor) == LOW) && (sequence_started == 1)) {
    // as long as the leaving sensor is active the train is still in the crossing
    // while checks if train is gone yet
    while (sequence_started == 1) {
      sensor_count = 0;
      for (int i = 1; i < 40; i++) {
        if (digitalRead(leaving_sensor) == LOW) sensor_count++;
        delay (30);
        flash_leds();
      }
      if (sensor_count == 0) sequence_started = 0;
      flash_leds();
    }
    // At this point train is no longer over the leaving sensor
    digitalWrite(led1, LOW); // Turn OFF led
    digitalWrite(led2, LOW); // Turn OFF led
    digitalWrite(led3, LOW); // Turn OFF led
    digitalWrite(led4, LOW); // Turn OFF led
  }
}
void flash_leds() {
  if (flash_time > millis()) return;
  flash_state = ~flash_state;
  digitalWrite(led1, flash_state);
  digitalWrite(led2, ~flash_state);
  digitalWrite(led3, flash_state);
  digitalWrite(led4, ~flash_state);
  flash_time = millis() + flash_interval;
}

It's not helpful that you have given the LEDs anonymous names "led1, led2..." . I can't tell which are red and which are yellow, and if they belong in pairs. Edit, I see the variables now, "ledy" but I don't see where they are used...

There is no code size penalty for variable names in C, that could be "ledYellow1" for example and be a lot more readable.

I think the real problem will be unravelling and re-writing your timing. You have combined incorrect millis() use with delay() calls, and you might have some blocking going on, it's not immediately obvious, which is itself another issue.

Ok thanks I will try to replay all delays with Millis deal with this, will also rename leds as suggested. I will deal with one issue at a time to try to sort this out.

Thanks for this guidance and will try to format my code better next time

I didn't want to "muddy the water" but you should (or might end up having to) use a state machine for this.

In the ide just go:

  1. control-T to format the indents nicely, then
  2. control-shift-C to copy for forum

And then in the post:

  1. control-V to paste

Thanks really helpful I have copied your advice to my notes

Yeah I was going to say that in OP's initial approach on the subject here.

I was going to code it for that thread just for fun, but can't find any decent sensors and mimicing it with buttons is a pitb. So might go to brick and mortar local shop and get 2 sensors and do this.

edit @GreyOwl here is a generic state diagram which might get you thinking. The states are, well, states :wink: which in your case might be "crossing open" and "crossing closed" and the events are things that cause states to change, like "train arrived" or "pedestrian button pressed" and so on.

image

I find it very difficult to write code without first having created such a diagram, however rough it may be to start with.

more edit...

Well they have stock and are picking for me, so will swing by and get some sensors. Then tomorrow's play exercise will be to code this up.

Please edit your posts #1 & #4 and put the code tags in those.

But other than formatting using "Auto Format" - Ctrl-T - do not change the actual code.

How to post code

The formatted and tagged code in my #8 is the code from OP's #4 which in turn is an unsuccessful attempt to better display the code in #1.

To some extent, the OP is better served by correcting his own posts at least in part because coming anew to the thread, seeing that glob of poorly readable stuff is extremely off-putting and may discourage helpers.

Perhaps it seems so to me as I expect it to be formatted according to convention in order to read it. But then most people who are in a position to help, have the same expectation, if you are less critical of details, neither do you have the experience to do so! :thinking:

Now just as with text, the lack of paragraphs is a problem and that is a serious matter. Multiple blank lines would also severely impair readability and this sometimes happens. There should be a single blank line between each function definition and maybe between distinct functional sections within a function.

Finally, there is inappropriate commenting such as:

should be simply

pinMode(led1, OUTPUT); // led1 on pin 10

should be simply

#define led1 = 10; // alternating flasher

Same comment on every line just muddies the description. All this is distracting.

Does it matter? :face_with_raised_eyebrow: Yes.

1 Like

(apologies for continuing the "off topic" discussion on coding style and comments...)

Well written code with good structure and carefully chosen variable and function names should mean that the coder rarely needs to add comments to describe what is going on. Ideally, the what should be clear from simply reading the code. The coder can add value to the code when adding comments by using the comments to describe the why rather than the what. This gives some context to the reader so they understand why the what needs to be done :grinning:

1 Like

Or to summarise briefly:

A good sketch is a good sketch if it can be read without additional comments like a good cooking recipe.