An Elegant way to make this code more organized

e1m1 · July 24, 2024, 9:23pm

Hey,

I am currently defining what code has to get executed under which conditions, here which switches are set. Is there a more elegant way to organize this code?

void loop() {
  //read the Poti Pin value and constrain it between 14 - 1005 instead 0 - 1023
  constrainedPoti = POTI_READ_AND_LIMIT(analogRead(Poti_Pin), 14, 1005);

  //playerSoftware0.setVolume(Pot_percentage_val); -> IMPLENET SOMEWHERE LATER
  //PROGRAM_POTI_VENT_BRIGHNTNESS(constrainedPoti); -> IMPLEMENT LATER


  // Serial.print(static_cast<int>(WandStartSequence));
  //Serial.print(" ");
  //Serial.println(hasBooted);

  if (!SW_Vent.read()) {
    COMPLETE_RESET();
    POROGRAM_CYCLOTRON_OFF();
  }

  if (SW_Vent.read() && !hasBooted) {
    hasBooted = PROGRAM_BOOTING();
    POROGRAM_CYCLOTRON_OFF();
  }

  if (SW_Vent.read() && hasBooted && !SW_BarGraph.read()) {
    BLINK_FUNCTION_LED_OF_SEGMENT(BarGraph_Segment, 0);
    PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
    POROGRAM_CYCLOTRON_OFF();
  }

  if (SW_Vent.read() && hasBooted && SW_BarGraph.read() && !SW_StartUp1.read() && !SW_Intensify.read()) {  // add Ventlight <= max specified in bootup
    PROGRAM_LED_IDEL_PUMP_UP(BarGraph_Segment, barGraphIdel, BarGraphIdleTimer);                           //put powercell idle timer in a new class
    PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
    PROGRAM_POTI_VENT_BRIGHNTNESS(constrainedPoti);
    POROGRAM_CYCLOTRON_OFF();
  }

  if (SW_Vent.read() && hasBooted && SW_BarGraph.read() && SW_StartUp1.read() && !SW_Intensify.read()) {
    PROGRAM_LED_IDEL_PUMP_UP(BarGraph_Segment, barGraphIdel, BarGraphIdleTimer);  // implement reset
    PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
    PROGRAM_CYCLOTRON_IDLE();
  }

  if (SW_Vent.read() && hasBooted && SW_BarGraph.read() && SW_StartUp1.read() && SW_Intensify.read()) {
    PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
    PROGRAM_CYCLOTRON_IDLE();
    PROGRAM_LED_IDEL_FIRING(BarGraph_Segment, barGraphFire);
  }


  if (SW_Vent.read() && hasBooted && SW_BarGraph.read() && SW_StartUp1.read() && !SW_Intensify.read() && SW_Intensify.toggled()) {
    //PROGRAM_LED_IDEL_PUMP_UP(BarGraph_Segment, barGraphIdel, BarGraphIdleTimer); // implement reset
    PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
    PROGRAM_CYCLOTRON_IDLE();
    BarGraphIdleTimer = 0;  // resets the bargrapg to zero and starts again from the bottom
    PROGRAM_LED_OFF(BarGraph_Segment);
  }

  // just for checking
  if (SW_WandTip.read()) {
    Serial.println("Wand-Mode");
    PROGRAM_LED_IDEL_FIRING(BarGraph_Segment, barGraphFire);
  }

kolaha · July 24, 2024, 9:34pm

byte States =
  (SW_Vent.read() << 4) |
  (hasBooted << 3) |
  (SW_BarGraph.read() << 2) |
  (SW_StartUp1.read() << 1) |
  SW_Intensify.read();
....
if (States == B11110) {
  PROGRAM_LED_IDEL_PUMP_UP(BarGraph_Segment, barGraphIdel, BarGraphIdleTimer);  // implement reset
  PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
  PROGRAM_CYCLOTRON_IDLE();
}
....

kolaha · July 24, 2024, 9:42pm

void loop() {
  //read the Poti Pin value and constrain it between 14 - 1005 instead 0 - 1023
  constrainedPoti = POTI_READ_AND_LIMIT(analogRead(Poti_Pin), 14, 1005);

  //playerSoftware0.setVolume(Pot_percentage_val); -> IMPLENET SOMEWHERE LATER
  //PROGRAM_POTI_VENT_BRIGHNTNESS(constrainedPoti); -> IMPLEMENT LATER


  // Serial.print(static_cast<int>(WandStartSequence));
  //Serial.print(" ");
  //Serial.println(hasBooted);

  if (!SW_Vent.read()) {
    COMPLETE_RESET();
    POROGRAM_CYCLOTRON_OFF();
  } else {

    if ( !hasBooted) {
      hasBooted = PROGRAM_BOOTING();
      POROGRAM_CYCLOTRON_OFF();
    } else {

      if ( !SW_BarGraph.read()) {
        BLINK_FUNCTION_LED_OF_SEGMENT(BarGraph_Segment, 0);
        PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
        POROGRAM_CYCLOTRON_OFF();
      } else {

        if ( !SW_StartUp1.read() && !SW_Intensify.read()) {  // add Ventlight <= max specified in bootup
          PROGRAM_LED_IDEL_PUMP_UP(BarGraph_Segment, barGraphIdel, BarGraphIdleTimer);                           //put powercell idle timer in a new class
          PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
          PROGRAM_POTI_VENT_BRIGHNTNESS(constrainedPoti);
          POROGRAM_CYCLOTRON_OFF();
        }

        else if ( SW_StartUp1.read() && !SW_Intensify.read()) {
          PROGRAM_LED_IDEL_PUMP_UP(BarGraph_Segment, barGraphIdel, BarGraphIdleTimer);  // implement reset
          PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
          PROGRAM_CYCLOTRON_IDLE();
        }

        else if ( SW_StartUp1.read() && SW_Intensify.read()) {
          PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
          PROGRAM_CYCLOTRON_IDLE();
          PROGRAM_LED_IDEL_FIRING(BarGraph_Segment, barGraphFire);
        }

        else if ( SW_StartUp1.read() && !SW_Intensify.read() && SW_Intensify.toggled()) {
          //PROGRAM_LED_IDEL_PUMP_UP(BarGraph_Segment, barGraphIdel, BarGraphIdleTimer); // implement reset
          PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
          PROGRAM_CYCLOTRON_IDLE();
          BarGraphIdleTimer = 0;  // resets the bargrapg to zero and starts again from the bottom
          PROGRAM_LED_OFF(BarGraph_Segment);
        }
      }
    }
  }

  // just for checking
  if (SW_WandTip.read()) {
    Serial.println("Wand-Mode");
    PROGRAM_LED_IDEL_FIRING(BarGraph_Segment, barGraphFire);
  }

gcjr · July 24, 2024, 9:44pm

can you provide a description of what the code is intended to do?

after a brief review

how many times do you need to execute SW_Vent.read()? why not read it once
looks like there are two distinct mode: has and has not booted.

if this essentially a state machine with a variety of stimuli that can transition between states?

and it's common practice to Capitalize Constants, not variables and function names.

J-M-L · July 25, 2024, 6:35am

What does this do ?

You have many of those in the various ifs and if the read removes something from a FIFO buffer you’ll be in trouble possibly.

May be you need to read once into a variable and then check that variable instead of reading again

Do you need else clauses in between those ifs?

kenb4 · July 25, 2024, 10:43am

Two seem to have duplicate actions (slightly reformatted for clarity)

e1m1:

  if (SW_Vent.read() && hasBooted && SW_BarGraph.read() && SW_StartUp1.read() &&
      !SW_Intensify.read()) {
    PROGRAM_LED_IDEL_PUMP_UP(BarGraph_Segment, barGraphIdel, BarGraphIdleTimer);  // implement reset
    PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
    PROGRAM_CYCLOTRON_IDLE();
  }

  if (SW_Vent.read() && hasBooted && SW_BarGraph.read() && SW_StartUp1.read() &&
      !SW_Intensify.read() && SW_Intensify.toggled()) {
    //PROGRAM_LED_IDEL_PUMP_UP(BarGraph_Segment, barGraphIdel, BarGraphIdleTimer); // implement reset
    PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCellIdle, PowerCellIdleTimer);
    PROGRAM_CYCLOTRON_IDLE();
    BarGraphIdleTimer = 0;  // resets the bargrapg to zero and starts again from the bottom
    PROGRAM_LED_OFF(BarGraph_Segment);
  }

If it's blah blah blah blah and !SW_Intensify.read(), then do this. But then if it's the same and also SW_Intensify.toggled(), do something similar a second time?

To avoid repeating each read and to ensure the actions are mutually exclusive, you might try

  if (SW_Vent.read()) {
    if (hasBooted) {
      if (SW_BarGraph.read()) {
        auto startup1 = SW_StartUp1.read();
        auto intensify = SW_Intensify.read();

        if (startup1 && intensify) {
        } else if (startup1 && !intensify) {
          if (SW_Intensify.toggled()) {           
          }
        } else if (!startup1 && !intensify) {
        } // unused: !startup1 && intensify
      } else { // !SW_BarGraph.read()
      }
    } else { // !hasBooted
    }
  } else { // !SW_Vent.read()
  }

Note this takes the nested

if not-condition
else

and avoids the not, turning the list "upside-down"; because each ! makes the conditions more complex to reason about, especially for someone not familiar with the system. If it really makes more sense to have the ! so that the options are "in order", you can keep it.

Toward the end, two of the conditions are stored in variables, and the tests are flattened slightly to avoid too much nesting. It also shows that instead of four combinations, only three are used. So maybe that part instead might be

        if (SW_StartUp1.read()) {
          if (SW_Intensify.read()) {
          } else if (SW_Intensify.toggled()) {           
          }
        } else { // !SW_StartUp1.read()
        }

if that is actually how it is supposed to work (three different possibilities).

With nested if-blocks, the cold folding feature of the editor can be handy.

e1m1 · July 25, 2024, 12:05pm

I uploaded a video on how it currently works.

In Short I want to use different LED blinking stuff in dependencies of whether or not a button/ switch has been changed or not.

For example here until the first switch has been switched and the boot up sequence has not been fnished you are not allowed to use the functions of any other switch

the button on the left can only be pressed and the alternating yellow leds will flash, after all other switches have been switched on in the right order.

I also have to add more dependencies to the function.
That's why I am asking for a good way to write down all dependencies and after that program the right conditions of if a switch has been switched or not.

Maybe there a some techniques that are usually be used for this kind of stuff like writing down a table with all the states or else.

e1m1 · July 25, 2024, 12:07pm

checks if the first switch the one in my video on the top is switched on.
after it has been switched on and the boot sequences has been finished the variable "hasBooted" gets true instead of false.

J-M-L · July 25, 2024, 12:11pm

if you have a bouncing switch, two consecutive read() (assuming they call digitalRead()) could return a different value.

TomGeorge · July 25, 2024, 12:29pm

Hi, @e1m1

Please post your complete code?

Can you please post a copy of your circuit, a picture of a hand drawn circuit in jpg, png?
Hand drawn and photographed is perfectly acceptable.
Please include ALL hardware, power supplies, component names and pin labels.

How have you got your switches wired?

Thanks... Tom..

alto777 · July 25, 2024, 12:52pm

Oooh, I want that! Reason enough to move to IDE 2.3.2.

a7

e1m1 · July 25, 2024, 1:15pm

I am still tweaking and trying to source fitting leds etc. once everything has the state i want it i can share if you want :).

e1m1 · July 25, 2024, 1:22pm

all right i guess i have to put it in a if else then.
but the button lib i included has a build in debounce that is really high.

J-M-L · July 25, 2024, 1:24pm

OK - some button library would return "pressed" only once when the button changes state. are you checking the state or the change of state with your read() function and are you 100% sure that read() I debounced? (which library do you use?)

e1m1 · July 25, 2024, 2:26pm

thanks.
the code isn‘t complete yet. first i want to get all buttons and functions working as they should and then i‘ll clean up the mess with your suggestions.

can you provide me a link to the cold folding feature of the arduino ide? i couldn‘t find reliable stuff via a quick search.

e1m1 · July 25, 2024, 2:29pm

i have to check the library again but i am certain that it gets debounced once read.

the library does a new debounced read when read is called and with toggled it checks if the states has chanced since last call. i am using this lib

alto777 · July 25, 2024, 2:40pm

It's code folding, sry!

This should fix you up, google

code folding arduino

My google autocorrected the error… so just in case

So how do you actually fold code well. It's pretty simple and an Arduino IDE 2 and Arduino ide1. You can simply go over to the margin of the code click that little arrow.

HTH

a7

kolaha · July 25, 2024, 3:52pm

i would make one switch - one function(device)

camsysca · July 25, 2024, 4:07pm

It's there in IDE 1.8.15; you enable it in Preferences.

alto777 · July 26, 2024, 1:08pm

Haha! I had, didn't even notice.

Or I used it and didn't find it helpful.

The cold folding in the wokwi simulator is not limited to { collapsing }, which is all the the IDE editor seems to offer unless I am missing something.

IDE 1.8.7, BTW, my forever portable IDE… and wouldn't you know, IDE 2.3.2 dropped support for the OS I am using.

Oh, wait. IDE 1.8.15 - maybe it is improved. Now I think I had trouble getting anything leter to run, oh well.

a7

Topic		Replies	Views
Seeking help arranging my Loop Programming Questions	71	614	March 1, 2023
Looking for guidance on structure and organization of code Project Guidance	25	490	September 30, 2023
Keeping code organized Programming Questions	6	1778	May 5, 2021
Basic programming question Programming Questions	9	1001	May 5, 2021
flow and pressure controller Programming Questions	3	687	May 5, 2021

An Elegant way to make this code more organized

Related Topics