How to shorten this code

Using Arduino Programming Questions
1

Not a programmer just been reading for a few weeks.
I wrote the following code which works but is clearly less than succinct.
I'm using it to to turn on my oled only when necessary .
I read up on (int i=0;i<=300;i++)
and tried that implementation but it didn't go very well.
Please let me know if this currently working code has any pitfalls
and how to shorten/rewrite it if necessary.
thanks.

int statusSensor1 = digitalRead (IRSensor1)
bool send;

if(statusSensor1 == 0)
  {
    display.ssd1306_command(SSD1306_DISPLAYON);
    send=HIGH;
  }

  if(send==HIGH)
  {
  countUp++;
  }
   if (countUp>=100) {
   
   display.ssd1306_command(SSD1306_DISPLAYOFF);
   send=LOW;
   countUp=0;
}
2

It's a bit hard to say without ALL the code. Can you post the complete sketch.

3

https://snippets-r-us.com/

4

You failed to set send to a value when statusSensor1 is not zero. That's a bug.

I suggest getting the code correct before you start messing around with "shorten this code".

5

good one, like what you did there. The rest of the code unfortunately isn't mine to post but isn't directly related to the code I posted. I wrote this as a standalone sketch which works without any additional code. I just added the "bool send" which may be completely unnecessary and is found nowhere else in the code. Why did I add it? who knows. I think i was in my hundreth iteration and that setup finally worked. The sensor read is equal to any input event like a pushbutton sink to ground.
So:


   if(digitalRead(pin7)==LOW)
  {
      display.ssd1306_command(SSD1306_DISPLAYON);
    send=HIGH; 
  }

  if(send==HIGH)
  {
  countUp++;
  }
   if (countUp>=100) {
   
   display.ssd1306_command(SSD1306_DISPLAYOFF);
   send=LOW;
   countUp=0;
}
6

tl;dr: make that a

  static bool send;

@pro_grammar You can try your snippet here in minimum context.

Press the button and it turns on, except for every N cycles you get one stab of off.

Try it here at this wokwi simulator link:

// https://forum.arduino.cc/t/how-to-shorten-this-code/1035307

void setup()
{
  Serial.begin(115200);
  Serial.println("\nhello world.\n");

  pinMode(8, INPUT_PULLUP);
}

int countUp;
# define pin7 8

void loop()
{
  static int lineCounter;

  delay(124); // slow it down... everything else whistles

int statusSensor1 = digitalRead(8);
bool send;

// adjusted code

  if(digitalRead(pin7) == LOW) {
//      display.ssd1306_command(SSD1306_DISPLAYON);
      Serial.print(lineCounter);
      lineCounter++;
      Serial.println("   display   on");
      send = HIGH; 
  }

  if(send == HIGH) {
    countUp++;
  }

  if (countUp >= 10) {  // again life too short
   
//    display.ssd1306_command(SSD1306_DISPLAYOFF);
    Serial.print(lineCounter);
    lineCounter++;
    Serial.println("   display   off");
    send = LOW;
    countUp = 0;
  }
}

You can change the wokwi, you automatically get a copy.

I have a working version, but try fixing it after you declare

  static bool send;

so send retains its value across loop() calls.

a7

7

He did say shorten or "rewrite". I take your advice

8

Appreciate it. Trying to see if I follow what you're saying, will post back.

9

consider

#define MAX_COUNT 100
int  countUp;
bool send;

void
func  (void)
{
    if (LOW == digitalRead (IRSensor1)) {
        if (false == send)  {
            display.ssd1306_command (SSD1306_DISPLAYON);
            send = true;
        }

        if (MAX_COUNT <= ++countUp)  {
            display.ssd1306_command (SSD1306_DISPLAYOFF);
            countUp = 0;
            send    = false;
        }
    }
}
10

This is @gcjr's #9 algorithm with some minor things changed until it worked the way I thought it should.

# define MAX_COUNT 50  // 2.5 seconds

unsigned long countUp;
bool send;

// call every 20 ms 
void
func  (void)
{
  if (LOW == digitalRead (pin7)) {
    if (false == send)  {
      //            display.ssd1306_command (SSD1306_DISPLAYON);
      Serial.print(lineCounter); lineCounter++;
      Serial.println("  turn  ON");
      
      send = true;
    }
    countUp = 0;
  }
  else {
    if (MAX_COUNT <= ++countUp)  {
      //            display.ssd1306_command (SSD1306_DISPLAYOFF);
      if (send == true) {
        Serial.print(lineCounter); lineCounter++;
      	Serial.println("  turn           OFF");
      }

      countUp = 0;
      send    = false;
    }
  }
}

And here is the entire scratch. I call func 20 times a second so timing is counting those. If you call it flat out, you have to count with a long variable, so countUp will always be unsigned long.

// https://forum.arduino.cc/t/how-to-shorten-this-code/1035307
// https://forum.arduino.cc/t/how-to-shorten-this-code/1035307/9 gcjr minimal

# define pin7 8

void setup()
{
  Serial.begin(115200);
  Serial.println("\nhello world.\n");

  pinMode(pin7, INPUT_PULLUP);
}

unsigned long lineCounter;

void loop()
{
  static unsigned long lastLook;
  unsigned long now = millis();
  if (now - lastLook < 50)
    return;

  lastLook = now;

  func();
}

# define MAX_COUNT 50  // 2.5 seconds

unsigned long countUp;
bool send;

void
func  (void)
{
  if (LOW == digitalRead (pin7)) {
    if (false == send)  {
      //            display.ssd1306_command (SSD1306_DISPLAYON);
      Serial.print(lineCounter); lineCounter++;
      Serial.println("  turn  ON");
      
      send = true;
    }
    countUp = 0;
  }
  else {
    if (MAX_COUNT <= ++countUp)  {
      //            display.ssd1306_command (SSD1306_DISPLAYOFF);
      if (send == true) {
        Serial.print(lineCounter); lineCounter++;
      	Serial.println("  turn           OFF");
      }
      countUp = 0;
      send    = false;
    }
  }
}

Which is in a runnable context at this link:

Now I have to figure out how it works. :expressionless:

a7

11

This works, can't begin to understand it but i'll stare at it for interval >= plenty.
I apppreciate you, also much thanks to gcjr.

12

I realized at the same time and place where so many questions get answered that it is really just a slow motion debouncing applied only to the falling edge.

Here's a sketch that illustrates hysteresis-style debouncing repurposed as a slow on, slow off switch:

// https://forum.arduino.cc/t/how-to-shorten-this-code/1035307
// symmetric delayed on / off

# define pin7 8

void setup()
{
  Serial.begin(115200);
  Serial.println("\nhello world.\n");

  pinMode(pin7, INPUT_PULLUP);
}

unsigned long lineCounter;

void loop()
{
  static unsigned long lastLook;
  unsigned long now = millis();
  if (now - lastLook < 50)
    return;

  lastLook = now;

  func();
}

# define COMMON_DELAY 15    // 50 ms

unsigned long countUp;
unsigned char send;

void func()
{
  unsigned char button = (LOW == digitalRead (pin7));

  if (button == send) countUp = 0;
  else if (++countUp > COMMON_DELAY) {
    send = !send;
    Serial.print(lineCounter); lineCounter++;
    Serial.println(send ? " ON" : " OFF"); 
    countUp = 0;
  }
}

If the button is different, reset the counter. As long as button is the same, count until you pass the COMMON_DELAY count.

All the algorithms or snippets work (or are meant to work…) by counting how many times they have been called, therefor timing is dependent on… wait for it… how often you call them.

If you have a convenient periodic process like many loop()s might implement, hang it on that.

Otherwise the counting mechanism could be switched to the more often seen use of millis() to measure time that a button has been "stable", in the debouncing analogy.

If you call the function as fast as possible, you have to count quite high, and timing will always depend on what else you got going on, that is to say how fast is "as fast as possible"?

a7

13

shouldn't it work in both directions
with a 10 msec delay and 100 cnt thresholds, the input needs to be stable for 1sec before changing state

// simple button processing

const byte butPin = A1;
byte       ledPin = 13;

enum { Off = HIGH, On = LOW };

enum { OnThresh = 100, OffThresh = 0 };
int  onOffCnt;

// -----------------------------------------------------------------------------
void loop (void)
{
    onOffCnt = LOW == digitalRead (butPin) ? onOffCnt+1 : onOffCnt -1;

    if (OnThresh <= onOffCnt)  {
        onOffCnt = OnThresh;
        Serial.println ("On");
        digitalWrite (ledPin, On);
    }
    else if (OffThresh >= onOffCnt)  {
        onOffCnt = OffThresh;
        Serial.println ("Off");
        digitalWrite (ledPin, Off);
    }

    delay (10);
}

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

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

    pinMode     (butPin, INPUT_PULLUP);
}
14

Of course it can be made to work in both directions. My versions do, and so does your new idea, if it works.

The code in #9, besides not working, did not appear to address any concept of stability in the turning on of the display by the sensor.

Fixed, it was instant on, counted down to delayed really off.

The apparent desired behaviour of @pro_grammar's original seemed also to be instant on, delayed off.

Which is why I thought everyone was going for instant on / delayed off.

For all I know, the OP was aiming for both delayed on and delayed off. Who cares but s/he? Something to specify.

The edges can be handled by the same code, I mean literally the same lines of code. That gives rise to the symmetrical version above and in the simulation.

The delay period constant could, in that version, be selected by the state of send, to go on faster than it turns off. Or whatever.

a7

15

The sensor triggering oled "on" is a basic IR proximity sensor. I was going for
screen instantly on when triggered, remaining "on" for about 4 seconds then
immediately off. Though delayed off is also fine,

16

why is this needed?

17 

      if (button == send) countUp = 0;

Did you try running the code in #12 above with that commented out? Either the assignment statement or resturctruing?

You may have observed a serious flaw in the altered code.

Without the assignment, countUp will accumulate noise on the signal. Leading to a possible premature or unwarranted change of state.

It is easy to provoke spurious and brief changes to the output state.

If the button meant "explode the bomb", and as a precaution we made the device safe by requiring a 5 second button down event to trigger…

we would be dismayed to analyze the software later to discover that nervous tentative practicing on the button, once it adds up to 5 seconds boom.

a7

18

Remain on until 4 seconds after the last activity. Activity resets the timing period.

  • Instantly I want to start the camera rollin

  • but Imma leave it on for a while after I think the action has stopped, just in case it hasn't. I'd just as soon have kept rolling .

  • If nothing has happened for a while, OK then stop it.

Or a constant action scene would be needlessly slivered into 4 second "takes". You have to refresh the idea of tuning off any time soon. When there is action.

a7

19

At this point I'm pretty sure I've wandered into someone else's post...
ok... This pretty much covers it.

20

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.