Multiple conditions in one if statement vs multiple if statements?

In this example (moving the procedure outside the void() loop) perhaps this would reflect how the code actually executes? bDataReady is set about every 250uS (4,000Hz sample rate).

//----------------------------------------------------------------------------
void GetData()
{
    bDataReady = false;
    ADXL355Data d;
    accel.readData(d);

    if (period_us < 1) {return;}
    if (abs(d.x) > 128000) {return;}
    if (abs(d.y) > 128000) {return;}
    if (rpm < (TargetRPM - TargetRPMTolerance)) {return;}
    if (rpm > (TargetRPM + TargetRPMTolerance)) {return;}
    int32_t delta = sample_time_us - last_tach_time_us;
    if (delta < 0)  {return;}
    if (delta >= period_us)  {return;}
    int index = (MAX_SAMPLES * delta) / period_us;
    // add values to one of MAX_SAMPLES slots or bins in rotation period
    sample_buffer[index].NumberOfSamples += 1;
    sample_buffer[index].x += d.x; 
    sample_buffer[index].y += d.y; 
    // location in buffer with highest value should be angle location
}

//----------------------------------------------------------------------------
void loop()
{
    if (bDataReady) GetData();
}

if the TargetRPMTolerance and targetRPM do not change (often)
and they are of the float data type,
you might pre-calculate them.
(float sub/add might be relative expensive)

    if (rpm < minimumRPM) {return;}
    if (rpm > maximumRPM) {return;}

They are integers, but good point and also much more readable.

Did an engine run-up with the =+ errors fixed and now getting sensible results.

Thanks again for spotting those errors!

My experience is the exact opposite: it makes it crystal clear what will happen without having to consider order of operations.

Claude.ai "improved" my code for me:

void loop() {
    if (!bDataReady) return; 
    bDataReady = false;
    ADXL355Data d;
    accel.readData(d);
    // --- Gate checks --------------------------------------------------------
    if (period == 0)                    return; // no tach yet
    if (sample_time < last_tach)        return; // sample before tach edge
    if (rpm < (TargetRPM - TargetRPMTolerance)) return;
    if (rpm > (TargetRPM + TargetRPMTolerance)) return;
...

I still don't understand why return; is allowed in loop().
Really illogic. What does it return to? loop() was not called and we certainly do not want to exit it. A google search says that loop() just starts a new iteration. Odd.

loop() loops. So you can explicitly return from it any time you are not interested in having the rest of the loop code execute.

A main() function is provided for you and it gets called when the processor resets. It does more than this but looks like

int main()
{
  setup();   // called once

  while (true)
    loop();   // called over and over forever
}

Real embedded system program main() functions can look just like that. The once part, and the forever over and over and over part.

Caukde outdented your logic.

a7

Is bDataReady being set by an interrupt service routine?

So the the main() is just not visible in the Arduino IDE?

What do you mean by "outdented"?

Getting rid of the indented condition statements?

Probably doesn't matter, but is there overhead involved with exiting loop() and Main() calling it again?

Yes

That how it works. What do you think happens if there is no explicit return statement and instead execution reaches the end of the loop() function?

ANSWER: Exactly the same thing, it returns to main() which again calls loop().

I used to work in an industry where something like below was a give-away to hackers.

bool somefunc()
{
  ...
  ...
  if (condition == false)
  {
    return false;
  }

  // do all kinds of stuff
  ...
  ...
  return true;
}

We made sure that the timing of the if took about the same time as the other part of the function.

So you can say that we optimised for slowness :slight_smile:

Duh! Obvious now that I think about it. I guess the word "loop" threw me for a loop :)

Outdenting is the opposite of indenting.

HTH

Found the one I just couldn't

a7

The only « issue » with return is that you don’t get a chance to do something else in the loop - so you kinda have a semi blocking conditional statement.

This is OK if there is really nothing else to do while you wait for the conditions to become OK but as a rule of thumb I tend to not return early and keep the loop going and reaching the natural end of the function.

For example say you get the idea to show a blinking led while waiting for the conditions or handle something else then you do want to give the loop a chance to do other stuff.

I would gather the conditions into a bool returning function to keep the loop code simple to read

inline bool systemIsReady() {
 if (sample_time < last_tach) return false; // sample before tach edge
 if (rpm < (TargetRPM - TargetRPMTolerance)) return false;
 if (rpm > (TargetRPM + TargetRPMTolerance)) return false;
  …
  return true;
}

void action() {
  …
}

void loop() {
  if (systemIsReady()) action();
  // here you can do other stuff
  …
}

Not perse slowness, more that no matter which path the code takes, it is equal in duration and if possible even in power usage and heat dissipation (use equally complex math in the "false path"). Clearly a case where high security has a priority over performance.

this is the Arduino main.cpp

/*
  main.cpp - Main loop for Arduino sketches
  Copyright (c) 2005-2013 Arduino Team.  All right reserved.

  This library is free software; you can redistribute it and/or
  modify it under the terms of the GNU Lesser General Public
  License as published by the Free Software Foundation; either
  version 2.1 of the License, or (at your option) any later version.

  This library is distributed in the hope that it will be useful,
  but WITHOUT ANY WARRANTY; without even the implied warranty of
  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  Lesser General Public License for more details.

  You should have received a copy of the GNU Lesser General Public
  License along with this library; if not, write to the Free Software
  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
*/

#include <Arduino.h>

// Declared weak in Arduino.h to allow user redefinitions.
int atexit(void (* /*func*/ )()) { return 0; }

// Weak empty variant initialization function.
// May be redefined by variant files.
void initVariant() __attribute__((weak));
void initVariant() { }

void setupUSB() __attribute__((weak));
void setupUSB() { }

int main(void)
{
	init();

	initVariant();

#if defined(USBCON)
	USBDevice.attach();
#endif
	
	setup();
    
	for (;;) {
		loop();
		if (serialEventRun) serialEventRun();
	}
        
	return 0;
}

In that case it needs to be protected with a critical section.

Maybe. In my experience LTO flattens main, setup, and loop into one function.