Architecturing a doorway LED project

[Highfire1]

Hi all,

I'm working on an arduino project that controls a LED light strip around the door of the makerspace at our college. When no one's around, it has a passive pattern, but when someone enters/exists through the doorway, it should create some nice lighting effects.

What I want to ask is how I should architecture the codebase. I initially looked into interrupts for the motion sensor as I thought they would be necessary to quickly transition from passive lighting to special effects, as currently all the passive effects have multiple hardcoded delay()s.

However, I'm wondering if there's anything wrong with this approach:

void sdelay(int d) {
  while(d > 0) {
    if( check_sensors() ) {
        play_special_effects(); // lasts for 5 seconds
    }
    delay(50);
    d -= 50;
  }
}

Currently the sensor is a PIR sensor but I'm also considering an ultrasonic sensor.

If anyone has recommendations for an alternative approach it would be appreciated.
(I'm also hoping to add a rotary encoder to select multiple lighting options, which would be pretty easy to tack on with this approach)

BOM:
Arduino Uno
Neopixel LED light strip
PIR MOTION SENSOR BREAKOUT

[kolaha]

any adressable RGB LED animation are static pattern that changes in short time, so it appears as movement or flow.
you can scan distance between each frame.

void loop() {
    if( isPIRactive() ) 
        play_special_effects(); 
    else  show_common_pattern();
  }

[DaveX]

@Highfire1

What I want to ask is how I should architecture the codebase. I initially looked into interrupts for the motion sensor as I thought they would be necessary to quickly transition from passive lighting to special effects, as currently all the passive effects have multiple hardcoded delay()s.

For architecture, look to:

@DaveX

• Planning and Implementing an Arduino Program by @Robin2-- A great way to plan your program to be flexible, easy to read, understand, maintain, and extend. ( Live Sim: Lesson A ... Lesson G )
• Demonstration code for several things at the same time by @Robin2-- An example running a couple blinking LEDS, a button and two servos, all independently. (Live Simulation)

It like doesn't need to transition instantly. If you must use an interrupt, only use it to set a flag that a transition is needed, and then adjust your longer animations with the many hard-coded delays to check the flag and abort with 'break;'. Inserting a check such as this won't slow your delays much and could improve responsiveness.

for(int ii = 0; ii< NUMLEDS;++ii){ 
  ...
  if(activity || checkSensors()){break;)
  delay(500);
  ...
}

[PaulRB]

@Highfire1

how I should architecture the codebase

That sounds very grand. Why not just say "how I should write the code"?

@Highfire1

interrupts for the motion sensor

Not necessary at all. The Arduino can check the sensor many times each second without missing a detection. But that can only happen if the rest of your code is written in the correct way, a "non-blocking" way. That means it must use millis() instead of delay() to time the pattern animations.

[rsmls]

@Highfire1

I'm wondering if there's anything wrong with this approach:

It depends mostly how the check_sensors() and play_special_effects() functions work. especially the former is critical. The approach you're suggesting is potentially blocking; i.e. it'll leave the system unresponsive to events in the outside world while it's playing the animation thing. Maybe that's OK. Maybe that's not desirable. Only you can decide.

@Highfire1

If anyone has recommendations for an alternative approach it would be appreciated.

Not necessarily different, but I'd challenge you to consider the different use cases and how to deal with them:

• A person enters the door.
• A person enters the door...very slowly.
• A person enters the door very rapidly.
• A person enters the door, then lingers on the doorstep...
• ...after lingering they step inside the room.
• ...after lingering they back out of the room.
• A person in a wheelchair goes through the door.
• A person with crutches goes through the door.
• A group of people goes through the door in rapid succession.
• A person preceded by an assistance dog goes through the door.
• The door is being shut.
• The door remains open on a crack.
• The door opens and closes (e.g. wind/draft)
• The sun shines momentarily on the door (possibly affecting the readings of optical sensors).
• Any of the above happens during school hours -vs- any of the above happens outside of school hours when nobody's there to see what happens.
  Etc. etc. etc.
  As you can see, something as simple as "when a person steps through the door" in reality is a pretty complex set of possible events that your system will need to deal with. Is it OK that it will 'stupidly' play the same animation regardless of whatever passes whatever kind of sensor gate you come up with? Is it OK that your system just keeps doing that even if someone starts to step in and then steps back? Is it OK that the animation is the same regardless of which direction they walk in?

So in the design of any system, you start by drafting a set of requirements that align with the desired behavior in the real world the system will need to function in. Only once you've done that, you start determining how to implement all this.
You can also do it the other way around, but then you'll have to guard against making decisions that involve a lot of rework at a later stage when you realize you should have done things differently. I.e. you may have to ditch the sensing system you installed (and drilled holes for etc.) when you realize that it's incapable of determining which direction someone walks in, and you have to come up with a new solution. (I suspect your relationship with the school janitor at this point may deteriorate.)

[Highfire1]

Hey everyone, thanks for all the lovely replies, I really appreciate all the feedback.

I ended up going with my initial approach of creating an improved delay(), but looking back on it, it a class based millis() system would have definitely been more systematic on the coding side.

My main concern is that creating new lighting effects would be much more difficult with a class based system than with delay() which is a lot more intuitive, so I'm happy with the system I have. Another concern is that I didn't want to leave behind a codebase that no one could understand and some of the stuff required to work with millis() efficiently is not really straightforward.

So far from testing at home it seems reasonably effective, but I'll have to test it at school next week to be sure that everything works together (fortunately we're mounting the setup on the wall, no hole drilling here :D)

I ended up also creating a replacement for NeoPixel.show() so I ended up with lighting effects that look like this:

void FX_rainbow() {
  for (int j = 0; j < 256; j++) { // Cycle through the rainbow
      for (int i = 0; i < NUM_PIXELS; i++) {
        int pixelHue = (i * 256 / NUM_PIXELS + j) & 255;
        NeoPixel.setPixelColor(i, NeoPixel.ColorHSV(pixelHue * 65536 / 256));
      }
      neopixelPush();
      sdelay(10);
    }
}

void neopixelPush() {
  if (restart_loop) { return; }
  NeoPixel.show();
}

void sdelay(int d) {

  if (restart_loop) {
    return;
  }

  // scale time elapsed during sdelay according to our set speed
  // (delay*100) / percentage speedup
  // e.g. (100ms*100) / 250% speedup = 40ms final delay.
  int real_delay = (d*100) / SPEED_STEPS[speed_step_index];
  d = real_delay;
  
  // This checks the rotary encoder and PIR sensor.
  // if there is a hit on the PIR sensor, we set a global variable called restart_loop to true
  // from there, we try to skip out of the function we're currently in back to the main loop()
  // where we can run our special lighting fx
  check_sensors();

  while(d > 0 && !restart_loop) {
    check_sensors(); 
    delay(10); 
    d -= 10;
  }
}

full code if anyone is curious: https://gist.github.com/Highfire1/ac64be66cef190b22617c53515043855

[rsmls]

@Highfire1

sdelay(10);

@Highfire1

int real_delay = (d*100) / SPEED_STEPS[speed_step_index];
  d = real_delay;

@Highfire1

d -= 10;

I'd have a really good look at this and the way you handle the SPEED_STEPS array. I think you're setting yourself up for some unanticipated/undesired behavior due to the combination of integer math and overflows.

@Highfire1

Another concern is that I didn't want to leave behind a codebase that no one could understand and some of the stuff required to work with millis() efficiently is not really straightforward.

That's debatable.
Firstly, I'd decouple the pattern from the code that runs it. That way, people can add patterns without understanding the code that interprets the pattern. Secondly, I'm not really convinced that adhering to some practice that's suboptimal (or even 'bad') is justifiable from a viewpoint of simplicity (or, more simply put: two wrongs don't make a right). Thirdly, the 'smart delay'-solution you've presented evidently has its own issues and arguably combines the worst of both worlds, instead of the best (it blocks much of the time AND it's opaque/awkwardly structured to be honest).

Really, the elegant solution here (as virtually always) would be a state machine approach. As long as it's well-documented and clearly distinguishes data (e.g. patterns) from code, and offers a usable interface for the 'no-code people' who may need to work with the system, it'll be superior to what you have now.

Of course, it's ultimately your call and I readily accept that the real world doesn't always allow itself to be shaped to our ideals. Still, we sometimes do get to do a little carving here and there.

[DaveX]

I'd have sdelay return a boolean so it could potentially signal to short-circuit the processes. Maybe something like:

void setup() {
  Serial.begin(115200);
  pinMode(A0, INPUT_PULLUP);
}

void loop() {
  for (char ch = 'A'; ch < 'Z'; ch++ ) {
    for (int ii = 20; ii ; --ii) {
      if (! cooperativeDelay(100)) break;
      Serial.print(ii );
      Serial.print(' ');
    }
    if (!cooperativeDelay(0)){
       Serial.println("Interrupt!");
       break;
    }
    Serial.println(ch);
  }
  delay(1000);  // for visualization
}

boolean cooperativeDelay(long val) {
  boolean cooperating = true;
  do {
    if (digitalRead(A0) == LOW) {
      cooperating = false;
      break;
    }
    delay(val > 0);
  } while (cooperating && val-- );
  return cooperating;
}

[xfpd]

@Highfire1

Another concern is that I didn't want to leave behind a codebase that no one could understand and some of the stuff required to work with millis() efficiently is not really straightforward.

Thoroughly comment your code and everyone will understand it.

[rsmls]

@xfpd

Thoroughly comment your code and everyone will understand it.

And, I might add: structure it in a sensible fashion, which in practice meaning it's somehow modular or at least consists of logical chunks with the least degree of functional interactivity possible, and where this interactivity between building blocks is required, ensure to document the interfaces particularly well.