Suggestions of my Code organisation

[e1m1]

Hey together,

first I have to thank you so much for all the input you gave me on my project.
I am currently at a point where could manage to get the following working:

• I have a "state machine" function that checks which Mode is currently selected.

• These modes are represented with enums to get the code clear and readable

• my main function currently checks if specific buttons have been pressed and then accordingly sets the corresponding enum.

• depending on this enum value i am going to create subfunctions that will do other stuff then.

• for "debugging" purposes i did a lot Serial.print commands which will be deleted in the final version once I have everything put into place.

I posted this video a few weeks ago, that is what should be controlled in the end: https://youtu.be/2DEnrEDu_PY

#######
my question is what do you think about the code.

• Is it organized or bs that needs serious rework
• What do you think about the function i created are they okay or should the be combined or broken down into smaller ones?

another question I want to ask you is on how to put separate code snippets like the print functions e.g.

print_buttonStates()

into separate code files.
I tried on a previous version using different tabs in the arduino IDE but i came to a point where the compiler got confused with linking the functions.

To condense everything: What do you think how I have written my code and what suggestions do you have to better organize it clean it up.

The code is currently working is it should to this point:

#include <Arduino.h>
#include <string.h>
#include <Button.h>

//Function Execution Timer
#include "NextExcecutionTime.h"

//DM13A classes
#include "DM13A_LedSegment.h"
#include "DM13A.h"

//FastLED
#include <FastLED.h>

/* std:array C++11 not allowed in arduino?
std::array<int, 5> scores ={0,1,2,3,4};
*/

// Define pin numbers as constants
const int PIN_WAND_TIP = 10;
const int PIN_START_UP1 = 8;
const int PIN_START_UP2 = 7;
const int PIN_INTENSIFY = 4;
const int PIN_BAR_GRAPH = 3;
const int PIN_VENT = 2;

// Button and Switches
Button SW_WandTip(PIN_WAND_TIP);
Button SW_StartUp1(PIN_START_UP1);
Button SW_StartUp2(PIN_START_UP2);
Button SW_Intensify(PIN_INTENSIFY);
Button SW_BarGraph(PIN_BAR_GRAPH);
Button SW_Vent(PIN_VENT);

int ButtonStates[6];  //-> löschen und code umbauen




//Cyclotron LEDs
constexpr int NUM_LEDS_CYCLOTRON = 28;
constexpr int CYCLOTRON_LED_PIN = A1;
CRGB cyclotron_leds[NUM_LEDS_CYCLOTRON];


//DM13A
constexpr byte LATCH_PIN = 12;  // ST_CP    // LAT   -> for LED-Shiftregisters
constexpr byte CLOCK_PIN = 11;  // SH_CP    // SCLK  -> for LED-Shiftregisters
constexpr byte DATA_PIN = 6;    // DS       // SIN   -> for LED-Shiftregisters
constexpr byte CHAINED_CHIPS = 2;
DM13A DM13ALeds(LATCH_PIN, CLOCK_PIN, DATA_PIN, (2 * CHAINED_CHIPS));

constexpr byte n_Led[][2] = { 2, 7 };
DM13A_LedSegment n_Segment(n_Led);

constexpr byte k_Led[][2] = { 3, 1 };
DM13A_LedSegment k_Segment(k_Led);

constexpr byte l_Led[][2] = { 3, 0 };
DM13A_LedSegment l_Segment(l_Led);


//for testing purpose only
void PROGRAM_LED_ON(DM13A_LedSegment &setSegment) {
  DM13ALeds.setOneBit(setSegment, 0);
  DM13ALeds.outputRefresh_direct();
}

void PROGRAM_LED_OFF(DM13A_LedSegment &clearSegment) {
  DM13ALeds.clearSegment(clearSegment);
  DM13ALeds.outputRefresh_direct();
}


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

  // Buttons
  SW_WandTip.begin();
  SW_StartUp1.begin();
  SW_StartUp2.begin();
  SW_Intensify.begin();
  SW_BarGraph.begin();
  SW_Vent.begin();

  DM13ALeds.begin();
}



void updateButtonStates() {  //-> nur Hilfe für die print function
  ButtonStates[0] = SW_WandTip.read();
  ButtonStates[1] = SW_StartUp1.read();
  ButtonStates[2] = SW_StartUp2.read();
  ButtonStates[3] = SW_Intensify.read();
  ButtonStates[4] = SW_BarGraph.read();
  ButtonStates[5] = SW_Vent.read();
}

void print_buttonStates() {
  Serial.print(ButtonStates[0]);
  Serial.print("|");
  Serial.print(ButtonStates[2]);
  Serial.print(ButtonStates[1]);
  Serial.print("|");
  Serial.print(ButtonStates[3]);
  Serial.print(ButtonStates[4]);
  Serial.println(ButtonStates[5]);
}



enum class PROTON_PACK_STANDARD_MODES{ //-> used for standard mode has to be implamented
  MODE_INITIATED,
  MODE_BOOTED,
  MODE_BAR_GRAPH_ON,
  MODE_CYCLOTRON_ON,
  FIRERING,
  OVERHEADED,
  };



enum class ProtonPackMainModes {  // or MainMode?
  MODE_PACK_OFF,
  MODE_CONFIG,
  MODE_STANDARD,
  MODE_QUICK_BOOT,
  MODE_PACK_SHUTTING_DOWN,
  MODE_PACK_SHUTTING_DOWN_FAST,
  MODE_MUSIC_PLAYER,
  MODE_ERROR
};


ProtonPackMainModes updateCurrentState();  //-> needs to stay here otherwise Arduino messes compilation up "explicit inclusion"
ProtonPackMainModes currentMainProgram = ProtonPackMainModes::MODE_PACK_OFF;
void printCurrentState(const ProtonPackMainModes &currentStandardMode);  // kein plan wieso aber sonst wird nicht compiliert


void printCurrentState(const ProtonPackMainModes &currentStandardMode) {  //genaueren Functions namen
  String modeString;

  switch (currentStandardMode) {
    case ProtonPackMainModes::MODE_PACK_OFF:
      modeString = "Pack off";
      break;
    case ProtonPackMainModes::MODE_CONFIG:
      modeString = "Config Menu";
      break;
    case ProtonPackMainModes::MODE_STANDARD:
      modeString = "Standard";
      break;
    case ProtonPackMainModes::MODE_QUICK_BOOT:
      modeString = "quick boot";
      break;
    case ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN:
      modeString = "shutting down";
      break;
    case ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN_FAST:
      modeString = "shutting down";
      break;
    case ProtonPackMainModes::MODE_MUSIC_PLAYER:
      modeString = "Music Player";
      break;
    case ProtonPackMainModes::MODE_ERROR:
      modeString = "ERROR";
      break;

    default:
      Serial.println("SWITCH ERROR");
      break;  // It's good practice to include a break in the default case as well
  }
  Serial.print("Current Mode: ");
  Serial.println(modeString);
}



void checkIfModeHasChanged() {  // check and if it has changed print the new state
  static ProtonPackMainModes lastState = ProtonPackMainModes::MODE_PACK_OFF;
  if (lastState != currentMainProgram) {
    printCurrentState(currentMainProgram);
    lastState = currentMainProgram;
  }
}


bool enterMODE_PackOff() {
  return (!SW_Vent.read());
}

bool enterMODE_Config() {
  return (SW_WandTip.read() && SW_Intensify.read() && SW_StartUp1.read() && SW_Vent.read() && SW_Vent.toggled());  // -> StartUP1 und 2 prüfen und besseren namen für die variable finden
}

bool enterMODE_Standard() {
  return (SW_StartUp1.read() && SW_Vent.read() && SW_Vent.toggled());  // -> toggled notwendig?
}

bool enterMODE_QuickBoot() {
  return (SW_WandTip.read() && SW_StartUp1.read() && SW_Vent.read() && SW_Vent.toggled());
}

bool enterMODE_PackShuttingDown_normal() {
  return (!SW_Vent.read() && SW_Vent.toggled() && SW_StartUp1.read() && !SW_BarGraph.read());  //-> wenn alles aus ist und dann Vent ausgeschaltet wird
}

bool enterMODE_PackShuttingDown_emergency() {  //-> egal wie die Stellung ist nur wenn Vent ausgeschaltet wird
  return (!SW_Vent.read() && SW_Vent.toggled());
}

bool enterMODE_MusicPlayer() {
  return (SW_WandTip.read() && SW_Intensify.read() && SW_StartUp2.read() && SW_Vent.read() && SW_Vent.toggled());
}

//bool enterMODE_ERROR ????




ProtonPackMainModes updateCurrentState() {
  static ProtonPackMainModes currentState = ProtonPackMainModes::MODE_ERROR;  //
  if (currentMainProgram == ProtonPackMainModes::MODE_PACK_OFF) {
    if (enterMODE_PackOff()) {
      currentState = ProtonPackMainModes::MODE_PACK_OFF;
      Serial.println("entered MODE : PackOff");
      PROGRAM_LED_OFF(l_Segment);
      PROGRAM_LED_OFF(n_Segment);

    } else if (enterMODE_Config()) {
      currentState = ProtonPackMainModes::MODE_CONFIG;
      Serial.println("entered MODE : Config Mode");
      Mode_Config();
    } else if (enterMODE_Standard()) {
      currentState = ProtonPackMainModes::MODE_STANDARD;
      Serial.println("entered MODE : Standard");
      Mode_Stadard();
    }
    // -> add the missing Modes
    else {
      currentState = ProtonPackMainModes::MODE_ERROR;  //-> default
      Serial.println("entered MODE : ERROR");
    }
  }
  return currentState;
}

//-> only testing needs to be
void Mode_Stadard() {  //-> only testing needs to be
  PROGRAM_LED_ON(l_Segment);
}

void Mode_Config() {  //-> only testing needs to be
  PROGRAM_LED_ON(n_Segment);
}
//###############################

void loop() {
  updateButtonStates();
  currentMainProgram = updateCurrentState();
  checkIfModeHasChanged();

  if (!SW_Vent.read()) currentMainProgram = ProtonPackMainModes::MODE_PACK_OFF;  // -> vernünftig herausschreiben
}

[gcjr]

• why do you have ButtonStates[] but never use it? looks like you explicitly read buttons when needed

• instead of having a function that uses a switch statement to print state strings, you can use a string array and just use an index to print the string

const char * ModeStr [] = {
    "MODE_PACK_OFF",
    "MODE_CONFIG",
    "MODE_STANDARD",
    "MODE_QUICK_BOOT",
    "MODE_PACK_SHUTTING_DOWN",
    "MODE_PACK_SHUTTING_DOWN_FAST",
    "MODE_MUSIC_PLAYER",
    "MODE_ERROR"
};

• i see no need for the enums to be a class, avoiding the need to prepend the class name

enum  { 
    MODE_PACK_OFF,
    MODE_CONFIG,
    MODE_STANDARD,
    MODE_QUICK_BOOT,
    MODE_PACK_SHUTTING_DOWN,
    MODE_PACK_SHUTTING_DOWN_FAST,
    MODE_MUSIC_PLAYER,
    MODE_ERROR
};

• do these one line functions really need to be separate functions, for example

void Mode_Stadard () {  //-> only testing needs to be
    PROGRAM_LED_ON (l_Segment);
}

• a more conventional approach for a state machine, which is assume is your updatesCurrentState(), is a switch statement that has cases for each state and checks for stimuli (button presses in your case) that result in some action and possibly a state change. That code that changes state can also print the name of the state being changed to

• aren't these enterMODE function really just checking for a specific button press? why are they checking multiple buttons? for example

bool enterMODE_QuickBoot () {
    return (SW_WandTip.read() && SW_StartUp1.read() && SW_Vent.read() && SW_Vent.toggled());
}

why not just check for a specific button press?

• Looks like nothing depends on the value of `curretnState`; it doesn't look like a state machine
  

---

it's not uncommon for novice programmers to organize code in ways that make senses for larger programs that doesn't really help with smaller one.

These techniques help make larger programs easier to read. in such programs, the state machine might be in a separate file, it would have a state variables that tracks the state it's in and the state machine function might have an argument that is the stimulus to be processed. that stmulus might be set by some button routines; the state machine would have no idea what generates the simulus (e.g. timer, non-button input, button, ...)

[e1m1]

hey thank you for you reply.

1. the printing stuff is that "complicated" because I cannot directly print from a enum class

2. i used the enum class instead of o.g. enum because it is "better practice" and i want to avoid using multiply uses of e.g. mode_standard

3. the void Mode_Standard() is only a prototype for testing. this function will to much more in the future but for the first step i just wanted to get the "backbone" and if this works the way it should i'll put more stuff in it then only led a led be turned on

4. the "MODE" functions are for reading which buttons have been pressed and which mode has to be entered like pressing e.g. "ctrl+esc" for opening the windows menu or "ctrl+c" for coping stuff on a clipboard on windows

5. void printCurrentState() is only for "debugging". so when i press the buttons on my physical project I can check on the "serial" console in the arduino IDE if the code branches in the right path.
   Later I'll use this function and put the functions that has to be executed depending on the current state instead of only "printing" to the serial output.

[gfvalvo]

@e1m1

i used the enum class instead of o.g. enum because it is "better practice"

Exactly! Here: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-class

[gcjr]

@e1m1

the printing stuff is that "complicated" because I cannot directly print from a enum class

@e1m1

void printCurrentState() is only for "debugging". so when i press the buttons on my physical project I can check on the "serial" console in the arduino IDE if the code branches in the right path.

they don't need to be complicated, i showed you how you can create a string array providing strings indexed by the enum avoiding a funcion to do so

@e1m1

i used the enum class instead of o.g. enum because it is "better practice" and i want to avoid using multiply uses of e.g. mode_standard

naming things with similar prefices (e,g. MODE_) is bad practice along with using long symbol names in such a small program. why not SM_ and PM_? and if you insist on using a class, isn't the class name redundant, why not PacketMode::Config?

what's the purpose of current_state. why isn't the "state machine" condtional on state

[gcjr]

here's a more conventionally structures state machine with multiple stimuli in each state.

// -----------------------------------------------------------------------------
byte PinButs [] = { A1, A2, A3 };
#define N_BUT   sizeof(PinButs)
byte butState [N_BUT];

enum { B_0, B_1, B_2 };

// -------------------------------------
const int NoBut = -1;
int
chkButtons ()
{
    for (unsigned n = 0; n < sizeof(PinButs); n++)  {
        byte but = digitalRead (PinButs [n]);
        if (butState [n] != but)  {
            butState [n] = but;
            delay (10);     // debounce
            if (LOW == but)
                return n;
        }
    }
    return NoBut;
}

// -----------------------------------------------------------------------------
byte PinLeds [] = { 10, 11, 12, 13 };

enum { L_0, L_1, L_2, L_3 };
enum { Off = HIGH, On = LOW };

// -------------------------------------
void
ledSet (
    int idx )
{
    for (unsigned n = 0; n < sizeof(PinLeds); n++)
        digitalWrite (PinLeds [n], Off);

    digitalWrite (PinLeds [idx], On);
}

// -------------------------------------
void
ledBlink (
    int idx,
    unsigned long msecPeriod )
{
    static unsigned long msec0;
           unsigned long msec = millis ();

    if (msec - msec0 >= msecPeriod)  {
        msec0 += msecPeriod;
        digitalWrite (PinLeds [idx], !  digitalRead (PinLeds [idx]));
    }
}

// -----------------------------------------------------------------------------

enum { S_0, S_1, S_2, S_3, S_4, S_5 };

const char * StateStr [] =  { "S_0", "S_1", "S_2", "S_3", "S_4", "S_5" };

int state;

// -------------------------------------
void
stateMach (
    int stim )
{
    switch (state)  {
    case S_0:
        state = S_1;
        ledSet (L_1);
        break;

    case S_1:
        if (B_1 == stim)  {
            state = S_2;
            ledSet (L_2);
        }
        else if (B_2 == stim) {
            state = S_3;
            ledSet (L_3);
        }
        break;

    case S_2:
        if (B_0 == stim)  {
            state = S_1;
            ledSet (L_1);
        }

        else if (B_1 == stim) {
            state = S_2;
            ledSet (L_2);
        }

        else if (B_2 == stim)
            state = S_4;
        break;

    case S_3:
        if (B_0 == stim)  {
            state = S_1;
            ledSet (L_1);
        }
        else if (B_1 == stim) {
            state = S_2;
            ledSet (L_2);
        }

        else if (B_2 == stim)
            state = S_5;
        break;

    case S_4:
        ledBlink (L_0, 500);

        if (B_0 == stim)  {
            state = S_1;
            ledSet (L_1);
        }
        else if (B_1 == stim)  {
            state = S_2;
            ledSet (L_2);
        }
        else if (B_2 == stim)  {
            state = S_3;
            ledSet (L_3);
        }
        break;

    case S_5:
        ledBlink (L_0, 100);

        if (B_0 == stim)  {
            state = S_1;
            ledSet (L_1);
        }
        break;
    }

    if (NoBut != stim)
        Serial.println (StateStr [state]);
}

// -----------------------------------------------------------------------------
void
loop (void)
{
    stateMach (chkButtons ());
}

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

    for (unsigned n = 0; n < sizeof(PinButs); n++)  {
        pinMode (PinButs [n], INPUT_PULLUP);
        butState [n] = digitalRead (PinButs [n]);
    }

    for (unsigned n = 0; n < sizeof(PinLeds); n++)  {
        digitalWrite (PinLeds [n], Off);
        pinMode      (PinLeds [n], OUTPUT);
    }
}

[noiasca]

@e1m1

• the printing stuff is that "complicated" because I cannot directly print from a enum class
• i used the enum class instead of o.g. enum because it is "better practice" and i want to avoid using multiply uses of e.g. mode_standard

1. keep your general switch case for printing out the enumeration. I would not rely on the index of a char array. If for any reason you decide to rearrange the order of the enum, your switch case will still work. If you insert a new enumeration, the compiler will give you a warning about the missing case.

2. correct. enum classes are a good way to get scoped enumerations. Keep them.

back to your sketch I see:
sometimes you are using constexpr (+) , but you have still code parts where you use const only like:

const int PIN_WAND_TIP = 10;

Use it more consistent.
Pins don't need to be unsigned values, a single byte uint8_t is sufficent.

[6v6gt]

@e1m1

the printing stuff is that "complicated" because I cannot directly print from a enum class

It has already been pointed out but here is a concrete example.

Definitions ( example code extracted from the state machine at Bread_simulation_V0_01 - Wokwi ESP32, STM32, Arduino Simulator ) and modified to use enum class which then allows for duplicate names and does not clutter the global name space:

enum class state_t : uint8_t  { STOPPED = 0 , INITIAL_WARMING, MIXING, MOTOR_ON_RIGHT, MOTOR_OFF_RIGHT,
                 MOTOR_ON_LEFT, MOTOR_OFF_LEFT, OPERATOR_WAIT, SHUTDOWN
               } ;  

const char *stateText[] = { "STOPPED", "INITIAL_WARMING", "MIXING", "MOTOR_ON_RIGHT",
                      "MOTOR_OFF_RIGHT",  "MOTOR_ON_LEFT", "MOTOR_OFF_LEFT", "OPERATOR_WAIT", "SHUTDOWN"
                    } ;

state_t state = state_t::STOPPED ;

To print, say for debugging, you cast the variable state to an integer before use:

snprintf( buf, sizeof(buf) - 1, "New state: %s   \n" ,  stateText[ (int)state ]  ) ;

[gcjr]

@noiasca

correct. enum classes are a good way to get scoped enumerations. Keep them.

can you explain why, when it provides a benefit?
or is it just dogma?

[noiasca]

@gcjr

can you explain why, when it provides a benefit?

when I see two enumerations for different states in one sketch I see a benefit of having them scoped.

[gcjr]

@noiasca

I see two enumerations

but one is unused! wouldn't it be better practice to eliminate the unused symbol

it's not clear to me why there are two separate enumerations, i can understand that there are different categories of "modes", that the apply to different parts of the code, but what not a single enumeration of all "modes" (i.e. MODE_)?

[e1m1]

that is work in progress.
the second enumerations will get used once i progressed more with the code.
if i get some time tomorrow i'll rework the code and send an update here.

[noiasca]

@e1m1

the printing stuff is that "complicated" because I cannot directly print from a enum class

This might get a longer post now. But it's raining in CEE, so I had some time...

Test 1
I've extracted your "printing stuff" in a short demo to have some comparison.
On an Arduino Uno this compiles with 3292 bytes of program memory and 294 bytes used SRAM for globals.

open the sketch

/*
    https://forum.arduino.cc/t/suggestions-of-my-code-organisation/1301250/
                      progmem/SRAM globals
    Test 1 as by the TO  3262/294
    
*/

enum class ProtonPackMainModes {  // or MainMode?
  MODE_PACK_OFF,
  MODE_CONFIG,
  MODE_STANDARD,
  MODE_QUICK_BOOT,
  MODE_PACK_SHUTTING_DOWN,
  MODE_PACK_SHUTTING_DOWN_FAST,
  MODE_MUSIC_PLAYER,
  MODE_ERROR
};


void printCurrentState(const ProtonPackMainModes &currentStandardMode) {  //genaueren Functions namen
  String modeString;

  switch (currentStandardMode) {
    case ProtonPackMainModes::MODE_PACK_OFF:
      modeString = "Pack off";
      break;
    case ProtonPackMainModes::MODE_CONFIG:
      modeString = "Config Menu";
      break;
    case ProtonPackMainModes::MODE_STANDARD:
      modeString = "Standard";
      break;
    case ProtonPackMainModes::MODE_QUICK_BOOT:
      modeString = "quick boot";
      break;
    case ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN:
      modeString = "shutting down";
      break;
    case ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN_FAST:
      modeString = "shutting down";
      break;
    case ProtonPackMainModes::MODE_MUSIC_PLAYER:
      modeString = "Music Player";
      break;
    case ProtonPackMainModes::MODE_ERROR:
      modeString = "ERROR";
      break;

    default:
      Serial.println("SWITCH ERROR");
      break;  // It's good practice to include a break in the default case as well
  }
  Serial.print("Current Mode: ");
  Serial.println(modeString);
}

void setup() {
  Serial.begin(115200);

  printCurrentState(ProtonPackMainModes::MODE_PACK_OFF);
  printCurrentState(ProtonPackMainModes::MODE_CONFIG);
  printCurrentState(ProtonPackMainModes::MODE_STANDARD);
  printCurrentState(ProtonPackMainModes::MODE_QUICK_BOOT);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN_FAST);
  printCurrentState(ProtonPackMainModes::MODE_MUSIC_PLAYER);
  printCurrentState(ProtonPackMainModes::MODE_ERROR);
}

void loop() {

}

Test 2
Using the F-Makro will bring down the used SRAM

open the sketch

/*
    https://forum.arduino.cc/t/suggestions-of-my-code-organisation/1301250/
                      progmem/SRAM globals
    Test 2 F-Makro:      3340/198
    Test 1 as by the TO  3262/294
    
*/

enum class ProtonPackMainModes {  // or MainMode?
  MODE_PACK_OFF,
  MODE_CONFIG,
  MODE_STANDARD,
  MODE_QUICK_BOOT,
  MODE_PACK_SHUTTING_DOWN,
  MODE_PACK_SHUTTING_DOWN_FAST,
  MODE_MUSIC_PLAYER,
  MODE_ERROR
};

void printCurrentState(const ProtonPackMainModes &currentStandardMode) {  //genaueren Functions namen
  String modeString;

  switch (currentStandardMode) {
    case ProtonPackMainModes::MODE_PACK_OFF:
      modeString = F("Pack off");
      break;
    case ProtonPackMainModes::MODE_CONFIG:
      modeString = F("Config Menu");
      break;
    case ProtonPackMainModes::MODE_STANDARD:
      modeString = F("Standard");
      break;
    case ProtonPackMainModes::MODE_QUICK_BOOT:
      modeString = F("quick boot");
      break;
    case ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN:
      modeString = F("shutting down");
      break;
    case ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN_FAST:
      modeString = F("shutting down");
      break;
    case ProtonPackMainModes::MODE_MUSIC_PLAYER:
      modeString = F("Music Player");
      break;
    case ProtonPackMainModes::MODE_ERROR:
      modeString = F("ERROR");
      break;

    default:
      Serial.println(F("SWITCH ERROR"));
      break;  // there is no need for this default that example - the enum class will cause a compiler Warning anyway if you missed a case
  }
  Serial.print(F("Current Mode: "));
  Serial.println(modeString);
}

void setup() {
  Serial.begin(115200);

  printCurrentState(ProtonPackMainModes::MODE_PACK_OFF);
  printCurrentState(ProtonPackMainModes::MODE_CONFIG);
  printCurrentState(ProtonPackMainModes::MODE_STANDARD);
  printCurrentState(ProtonPackMainModes::MODE_QUICK_BOOT);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN_FAST);
  printCurrentState(ProtonPackMainModes::MODE_MUSIC_PLAYER);
  printCurrentState(ProtonPackMainModes::MODE_ERROR);
}

void loop() {

}

Test 3
Don't mess around with String objects, just print it out.

Imho it is still a well readable sketch and the compiler will help that you don't miss any enumeration.

The result of 1880/188 bytes is already a huge improvement compared to the first version.

open the sketch

/*
    https://forum.arduino.cc/t/suggestions-of-my-code-organisation/1301250/
                           progmem/SRAM globals
    Test 3 remove String      1880/188
    Test 2 F-Makro:           3340/198
    Test 1 as by the TO       3262/294
    
*/

enum class ProtonPackMainModes {  // or MainMode?
  MODE_PACK_OFF,
  MODE_CONFIG,
  MODE_STANDARD,
  MODE_QUICK_BOOT,
  MODE_PACK_SHUTTING_DOWN,
  MODE_PACK_SHUTTING_DOWN_FAST,
  MODE_MUSIC_PLAYER,
  MODE_ERROR
};

void printCurrentState(const ProtonPackMainModes &currentStandardMode) {  //genaueren Functions namen
  Serial.print(F("Current Mode: "));
  switch (currentStandardMode) {
    case ProtonPackMainModes::MODE_PACK_OFF:
      Serial.println(F("Pack off"));
      break;
    case ProtonPackMainModes::MODE_CONFIG:
      Serial.println(F("Config Menu"));
      break;
    case ProtonPackMainModes::MODE_STANDARD:
      Serial.println(F("Standard"));
      break;
    case ProtonPackMainModes::MODE_QUICK_BOOT:
      Serial.println(F("quick boot"));
      break;
    case ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN:
      Serial.println(F("shutting down"));
      break;
    case ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN_FAST:
      Serial.println(F("shutting down"));
      break;
    case ProtonPackMainModes::MODE_MUSIC_PLAYER:
      Serial.println(F("Music Player"));
      break;
    case ProtonPackMainModes::MODE_ERROR:
      Serial.println(F("ERROR"));
      break;

    // will never be executed
    //default:
    //  Serial.println(F("SWITCH ERROR"));
    //  break;  // the compiler will warn you if you miss one mode as you are using enum class
  }
}

void setup() {
  Serial.begin(115200);

  printCurrentState(ProtonPackMainModes::MODE_PACK_OFF);
  printCurrentState(ProtonPackMainModes::MODE_CONFIG);
  printCurrentState(ProtonPackMainModes::MODE_STANDARD);
  printCurrentState(ProtonPackMainModes::MODE_QUICK_BOOT);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN_FAST);
  printCurrentState(ProtonPackMainModes::MODE_MUSIC_PLAYER);
  printCurrentState(ProtonPackMainModes::MODE_ERROR);
}

void loop() {

}

Test 4
Let's compare it to the usage of an array of char strings.
The program memory gets down, but the strings are still kept in SRAM.

open the sketch

/*
    https://forum.arduino.cc/t/suggestions-of-my-code-organisation/1301250/
                           progmem/SRAM globals
    Test 4 plain char array   1792/278 
    Test 3 remove String      1880/188          because no String class is used
    Test 2 F-Makro:           3340/198          Text don't need SRAM any more
    Test 1 as by the TO       3262/294
    
*/

enum class ProtonPackMainModes {  // or MainMode?
  MODE_PACK_OFF,
  MODE_CONFIG,
  MODE_STANDARD,
  MODE_QUICK_BOOT,
  MODE_PACK_SHUTTING_DOWN,
  MODE_PACK_SHUTTING_DOWN_FAST,
  MODE_MUSIC_PLAYER,
  MODE_ERROR
};

const char * ModeStr [] = {
    "Pack off",
    "Config Menu",
    "Standard",
    "quick boot",
    "shutting down",
    "shutting down",
    "Music Player",
    "ERROR"
};

void printCurrentState(const ProtonPackMainModes &currentStandardMode) {  //genaueren Functions namen
  Serial.print(F("Current Mode: "));
  Serial.println(ModeStr[(int)currentStandardMode]);
}

void setup() {
  Serial.begin(115200);

  printCurrentState(ProtonPackMainModes::MODE_PACK_OFF);
  printCurrentState(ProtonPackMainModes::MODE_CONFIG);
  printCurrentState(ProtonPackMainModes::MODE_STANDARD);
  printCurrentState(ProtonPackMainModes::MODE_QUICK_BOOT);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN_FAST);
  printCurrentState(ProtonPackMainModes::MODE_MUSIC_PLAYER);
  printCurrentState(ProtonPackMainModes::MODE_ERROR);
}

void loop() {

}

Test 5
I messed around with a local variable for the text constants, but that didn't help a lot, therefore no example.

Test 6
basically, what I have done with the simple F-Makro can also be done for the array.
It's not really intuitive, but Arduino provides an example. It might be good to read that:

PROGMEM - Arduino Reference

Memory wise it's a lot better than Test 4 but - I still don't like it.

open the sketch

/*
    https://forum.arduino.cc/t/suggestions-of-my-code-organisation/1301250/
                           progmem/SRAM globals
    Test 6 array PROGMEM      1872/188
    Test 5 local char array   1856/278
    Test 4 plain char array   1792/278
    Test 3 remove String      1880/188          because no String class is used
    Test 2 F-Makro:           3340/198          Text don't need SRAM any more
    Test 1 as by the TO       3262/294

*/

enum class ProtonPackMainModes {  // or MainMode?
  MODE_PACK_OFF,
  MODE_CONFIG,
  MODE_STANDARD,
  MODE_QUICK_BOOT,
  MODE_PACK_SHUTTING_DOWN,
  MODE_PACK_SHUTTING_DOWN_FAST,
  MODE_MUSIC_PLAYER,
  MODE_ERROR
};

// see https://www.arduino.cc/reference/en/language/variables/utilities/progmem/
// Setting up the strings is a two-step process. First, define the strings.
const char string_0[] PROGMEM = "Pack off";
const char string_1[] PROGMEM = "Config Menu";
const char string_2[] PROGMEM = "Standard";
const char string_3[] PROGMEM = "quick boot";
const char string_4[] PROGMEM = "shutting down";
const char string_5[] PROGMEM = "shutting down";
const char string_6[] PROGMEM = "Music Player";
const char string_7[] PROGMEM = "ERROR";
// Then set up a table to refer to your strings.
const char *const string_table[] PROGMEM = {string_0, string_1, string_2, string_3, string_4, string_5, string_6, string_7};

void printCurrentState(const ProtonPackMainModes &currentStandardMode) {  //genaueren Functions namen
  char buffer[30];  // make sure this is large enough for the largest string it must hold
  strcpy_P(buffer, (char *)pgm_read_ptr(&(string_table[(int)currentStandardMode])));
  Serial.print(F("Current Mode: "));
  Serial.println(buffer);
}

void setup() {
  Serial.begin(115200);

  printCurrentState(ProtonPackMainModes::MODE_PACK_OFF);
  printCurrentState(ProtonPackMainModes::MODE_CONFIG);
  printCurrentState(ProtonPackMainModes::MODE_STANDARD);
  printCurrentState(ProtonPackMainModes::MODE_QUICK_BOOT);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN_FAST);
  printCurrentState(ProtonPackMainModes::MODE_MUSIC_PLAYER);
  printCurrentState(ProtonPackMainModes::MODE_ERROR);
}

void loop() {

}

Test 7
Finally I played around to squeeze out some bytes of the "switch/case" option as I still think it is better readable, easier to maintain and safer to use.

I encourage you to check the "Streaming.h" library. It will make your printing much easier. Just see the two examples which are coming with the library.

open the sketch

/*
    https://forum.arduino.cc/t/suggestions-of-my-code-organisation/1301250/
                           progmem/SRAM globals
    Test 7 streaming 3        1850/188
    Test 6 array PROGMEM      1872/188
    Test 5 local char array   1856/278
    Test 4 plain char array   1792/278    
    Test 3 remove String      1880/198
    Test 2 F-Makro:           3340/198
    Test 1 as by the TO       3262/294

*/

#include <Streaming.h>

enum class ProtonPackMainModes {
  MODE_PACK_OFF,
  MODE_CONFIG,
  MODE_STANDARD,
  MODE_QUICK_BOOT,
  MODE_PACK_SHUTTING_DOWN,
  MODE_PACK_SHUTTING_DOWN_FAST,
  MODE_MUSIC_PLAYER,
  MODE_ERROR
};

const char * ModeStr [] = {
  "MODE_PACK_OFF",
  "MODE_CONFIG",
  "MODE_STANDARD",
  "MODE_QUICK_BOOT",
  "MODE_PACK_SHUTTING_DOWN",
  "MODE_PACK_SHUTTING_DOWN_FAST",
  "MODE_MUSIC_PLAYER",
  "MODE_ERROR"
};

void printCurrentState(const ProtonPackMainModes &currentStandardMode) {  //genaueren Functions namen
  Serial << F("Current Mode: ");
  switch (currentStandardMode) {
    case ProtonPackMainModes::MODE_PACK_OFF:
      Serial <<(F("Pack off"));
      break;
    case ProtonPackMainModes::MODE_CONFIG:
      Serial <<(F("Config Menu"));
      break;
    case ProtonPackMainModes::MODE_STANDARD:
      Serial << (F("Standard"));
      break;
    case ProtonPackMainModes::MODE_QUICK_BOOT:
      Serial <<(F("quick boot"));
      break;
    case ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN:
      Serial <<(F("shutting down"));
      break;
    case ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN_FAST:
      Serial <<(F("shutting down"));
      break;
    case ProtonPackMainModes::MODE_MUSIC_PLAYER:
      Serial <<(F("Music Player"));
      break;
    case ProtonPackMainModes::MODE_ERROR:
      Serial <<(F("ERROR"));
      break;

      // will never be executed
      //default:
      //  Serial.println(F("SWITCH ERROR"));
      //  break;  // the compiler will warn you if you miss one mode as you are using enum class
  }
  Serial << endl;
}

void setup() {
  Serial.begin(115200);

  printCurrentState(ProtonPackMainModes::MODE_PACK_OFF);
  printCurrentState(ProtonPackMainModes::MODE_CONFIG);
  printCurrentState(ProtonPackMainModes::MODE_STANDARD);
  printCurrentState(ProtonPackMainModes::MODE_QUICK_BOOT);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN);
  printCurrentState(ProtonPackMainModes::MODE_PACK_SHUTTING_DOWN_FAST);
  printCurrentState(ProtonPackMainModes::MODE_MUSIC_PLAYER);
  printCurrentState(ProtonPackMainModes::MODE_ERROR);
}

void loop() {

}

Summary
Do some tests of your own and decide which way to go.

[e1m1]

thanks for the long reply.

i used progmem in another project where i had to store a long array for music playback right out of the arduino. but i had a special library for this so i had not to use progmem and reload from mem for my self.

[e1m1]

here is the new version of my code.

which option would be better so implement?
after the current state, depending on the pressed buttons & switches, has been evaluated to start the corresponding program in the function

MainModes updateCurrentState()

or only use this function for setting the current state and executes the functions, in my code named:

PROGRAM_s

in the loop with if statements like:

  if (currentMainProgram == MainModes::MODE_STANDARD) PROGRAM_Standard();

???

• I corrected all consts to constexpr.
• I renamed some of the buttons depending on their physical appearance, some are buttons and some are switches, thus i used SW and BT
• I also added some of my DM13A codes for led effects

The code is still work in progress and far from finished, but if you want i would like to get some input from you to improve on coding and formatting as well as organizing the code.
How can i pass my function

MainModes updateCurrentState()

MainModes currentMainProgram

as input, because i think it is not good practice manipulate or read global variables with a function.

#include <Arduino.h>
#include <string.h>
#include <Button.h>

//Function Execution Timer
#include "NextExcecutionTime.h"

//DM13A classes
#include "DM13A_LedSegment.h"
#include "DM13A.h"

//FastLED
#include <FastLED.h>


// Define pin numbers as constants
constexpr int PIN_WAND_TIP = 10;
constexpr int PIN_START_UP1 = 8;
constexpr int PIN_START_UP2 = 7;
constexpr int PIN_INTENSIFY = 4;
constexpr int PIN_BAR_GRAPH = 3;
constexpr int PIN_VENT = 2;

// Button and Switches
Button BT_WandTip(PIN_WAND_TIP);
Button SW_Normal(PIN_START_UP1);
Button SW_Music(PIN_START_UP2);
Button BT_Intensify(PIN_INTENSIFY);
Button SW_BarGraph(PIN_BAR_GRAPH);
Button SW_Vent(PIN_VENT);

//Cyclotron LEDs
constexpr int NUM_LEDS_CYCLOTRON = 28;
constexpr int CYCLOTRON_LED_PIN = A1;
CRGB cyclotron_leds[NUM_LEDS_CYCLOTRON];

//DM13A
constexpr byte LATCH_PIN = 12;  // ST_CP    // LAT   -> for LED-Shiftregisters
constexpr byte CLOCK_PIN = 11;  // SH_CP    // SCLK  -> for LED-Shiftregisters
constexpr byte DATA_PIN = 6;    // DS       // SIN   -> for LED-Shiftregisters
constexpr byte CHAINED_CHIPS = 2;
DM13A DM13ALeds(LATCH_PIN, CLOCK_PIN, DATA_PIN, (2 * CHAINED_CHIPS));  //2* because DM13A accepts 8 Bit Chips, the used ones are 16 Bit chips

constexpr byte Power_Cell_Leds[][2] = { { 1, 6 }, { 1, 5 }, { 1, 4 }, { 1, 3 }, { 1, 2 }, { 1, 1 }, { 0, 7 }, { 0, 6 }, { 0, 5 }, { 0, 4 }, { 0, 3 }, { 0, 2 }, { 0, 1 }, { 0, 0 }, { 1, 7 } };
DM13A_LedSegment Power_Cell_Segment(Power_Cell_Leds);
NextExcecutionTime PowerCell_Idle(230);

constexpr byte BarG_LED[][2] = { { 2, 6 }, { 2, 5 }, { 2, 4 }, { 2, 3 }, { 2, 2 }, { 2, 1 }, { 3, 7 }, { 3, 6 }, { 3, 5 }, { 3, 4 }, { 3, 3 }, { 3, 2 } };
DM13A_LedSegment BarGraph_Segment(BarG_LED);

constexpr byte n_Led[][2] = { 2, 7 };
DM13A_LedSegment n_Segment(n_Led);

constexpr byte k_Led[][2] = { 3, 1 };
DM13A_LedSegment k_Segment(k_Led);

constexpr byte l_Led[][2] = { 3, 0 };
DM13A_LedSegment l_Segment(l_Led);

constexpr byte SlowBlow_Led[][2] = { 2, 0 };  //if used on board, instead use PMW stuff
DM13A_LedSegment SlowBlow_Segment(SlowBlow_Led);

constexpr int brightnessMax = 180;  // SlowBlow boot seems not to work    // % brightness = x%+254/100
NextExcecutionTime SlowBlowBoot(3500 / brightnessMax);

constexpr int L_SlowBlow = 5;

// DM13A FUNCTIONS
void PROGRAM_LED_IDEL_PUMP_UP(DM13A_LedSegment &LedSegment, NextExcecutionTime &functionCycleTimer, bool resetSegment = false) {
  //static int currentPosition = specificPosition;
  if (resetSegment) LedSegment.resetSegment();

  else {
    if (functionCycleTimer.isCycleElapsed()) {
      DM13ALeds.setOneBit(LedSegment, LedSegment.currentLedPosition);
      DM13ALeds.outputRefresh_direct();
      LedSegment.currentLedPosition++;

      if (LedSegment.currentLedPosition > LedSegment.ledCount) {
        DM13ALeds.clearSegment(LedSegment);
        DM13ALeds.outputRefresh_direct();
        LedSegment.currentLedPosition = 0;
      }
    }
  }
}

// FOR TESTING ONLY
void PROGRAM_LED_ON(DM13A_LedSegment &setSegment) {
  DM13ALeds.setOneBit(setSegment, 0);
  DM13ALeds.outputRefresh_direct();
}

void PROGRAM_LED_OFF(DM13A_LedSegment &clearSegment) {
  DM13ALeds.clearSegment(clearSegment);
  DM13ALeds.outputRefresh_direct();
}
// FOR TESTING ONLY



void setup() {
  Serial.begin(9600);  //-> needs to be turned of in final code, here only for "debugging"

  // Buttons
  BT_WandTip.begin();
  SW_Normal.begin();
  SW_Music.begin();
  BT_Intensify.begin();
  SW_BarGraph.begin();
  SW_Vent.begin();

  DM13ALeds.begin();
}


enum class MainModes {  // or MainMode?
  MODE_PACK_OFF,
  MODE_CONFIG,
  MODE_STANDARD,
  MODE_QUICK_BOOT,
  MODE_PACK_SHUTTING_DOWN,
  MODE_PACK_SHUTTING_DOWN_FAST,
  MODE_MUSIC_PLAYER,
  MODE_ERROR
};

MainModes updateCurrentState();  //-> needs to stay here otherwise Arduino messes compilation up "explicit inclusion"
MainModes currentMainProgram = MainModes::MODE_PACK_OFF;

// set conditions for entering the specified MODE
bool enterMODE_PackOff() {
  return (!SW_Vent.read());
}

bool enterMODE_Config() {
  return (BT_WandTip.read() && BT_Intensify.read() && SW_Normal.read() && SW_Vent.read() && SW_Vent.toggled());
}

bool enterMODE_Standard() {                                          // -> und alle andere off ?
  return (SW_Normal.read() && SW_Vent.read() && SW_Vent.toggled());  // -> toggled notwendig?
}

bool enterMODE_QuickBoot() {
  return (BT_WandTip.read() && SW_Normal.read() && SW_Vent.read() && SW_Vent.toggled());
}

bool enterMODE_PackShuttingDown_normal() {
  return (!SW_Vent.read() && SW_Vent.toggled() && SW_Normal.read() && !SW_BarGraph.read());  //-> wenn alles aus ist und dann Vent ausgeschaltet wird
}

bool enterMODE_PackShuttingDown_emergency() {  //-> egal wie die Stellung ist nur wenn Vent ausgeschaltet wird
  return (!SW_Vent.read() && SW_Vent.toggled());
}

bool enterMODE_MusicPlayer() {
  return (BT_WandTip.read() && BT_Intensify.read() && SW_Music.read() && SW_Vent.read() && SW_Vent.toggled());
}

//bool enterMODE_ERROR ????


MainModes updateCurrentState() {
  static MainModes currentState = MainModes::MODE_ERROR;  //
  static String modeString;

  if (currentMainProgram == MainModes::MODE_PACK_OFF) {
    modeString = "entered MODE: Pack off";
    if (enterMODE_Config()) {
      currentState = MainModes::MODE_CONFIG;
      modeString = "entered MODE  Config Mode";
      PROGRAM_Config();  //put it here or put it into the loop?

    } else if (enterMODE_QuickBoot()) {
      currentState = MainModes::MODE_QUICK_BOOT;
      modeString = "entered MODE: Quick Boot";

    } else if (enterMODE_Standard()) {
      currentState = MainModes::MODE_STANDARD;
      modeString = "entered MODE: Standard";

    } else if (enterMODE_PackShuttingDown_normal()) {
      currentState = MainModes::MODE_PACK_SHUTTING_DOWN;
      modeString = "entered MODE: Shut Down normal";

    } else if (enterMODE_PackShuttingDown_emergency()) {
      currentState = MainModes::MODE_PACK_SHUTTING_DOWN_FAST;
      modeString = "entered MODE: Shut down emergency";

    } else if (enterMODE_MusicPlayer()) {
      currentState = MainModes::MODE_MUSIC_PLAYER;
      modeString = "entered MODE: Music Player";

    } else if (enterMODE_PackOff()) {
      currentState = MainModes::MODE_PACK_OFF;
      modeString = "entered MODE: Pack OFF";
      PROGRAM_LED_OFF(l_Segment);
      PROGRAM_LED_OFF(n_Segment);

    } else {
      currentState = MainModes::MODE_ERROR;  //-> default // check if ERROR or Pack off is right here
      modeString = "entered MODE: ERROR";
    }
    Serial.println(modeString);
  }
  return currentState;
}


enum class StandardModes {  //-> if standard mode has entered this defines the sub states of standard mode
  MODE_INITIATED,
  MODE_BOOTED,
  MODE_BAR_GRAPH_ON,
  MODE_CYCLOTRON_ON,
  FIRING,
  OVERHEADED,
  ERROR
};


//StandardModes currentStandardVentMode = StandardModes::ERROR;  //-> Vent mode umbennnen, "currentStandardMode" ist schon belegt. ggf diesen auch ändern
void PROGRAM_Standard() {
  PROGRAM_LED_ON(l_Segment);
  /*
  static StandardModes currentSwitch = StandardModes::ERROR;

  switch (currentStandardVentMode) {
    case StandardModes::MODE_INITIATED:
      PROGRAM_LED_ON(l_Segment);
      currentStandardVentMode = StandardModes::MODE_BOOTED;
    case StandardModes::MODE_BOOTED:
      PROGRAM_LED_IDEL_PUMP_UP(Power_Cell_Segment, PowerCell_Idle);
      break;
  }
  */
}

void PROGRAM_Config() {  //-> only testing needs to be extended
  PROGRAM_LED_ON(n_Segment);
  //add config options
  // set audio style
  // set master volume
  // check battery status
}

void PROGRAM_quickBoot() {
  //set current subState to MODE_CYCLOTRON_ON and bypass the boot sequence of standard mode
}

void PROGRAM_shutDown_normal() {
  // cycle up power cell until max and the slowly cycle down
  // turn down bargraph
  // set cyclotron to max adn cycle down
  // play audio shutdown sequence long
  // turn all leds off
}

void PROGRAM_shutDown_emergency() {
  // play 3 beep sound
  // led cyclotron blink 3 times
  // led Powercell & BarGraph blink alternating 3 times
}

void PROGRAM_MusicPlayer() {
  // set volume (poti)
  // cycle playable tracks wandTip -> show current sond in bargraph -> only select !!!
  // play and stop with intensify
  // for v2 -> take speaker output as input for fastled effects
}

//###############################

void loop() {
  currentMainProgram = updateCurrentState();
  if (currentMainProgram == MainModes::MODE_STANDARD) PROGRAM_Standard();

  if (!SW_Vent.read()) currentMainProgram = MainModes::MODE_PACK_OFF;  // -> vernünftig herausschreiben
}

[gcjr]

it's not clear (to me) what you're trying to do because so much of your code is unused. currently looks the code only executes standard() when the state is STANDARD.

it is more conventional to name updateCurrentState(), 'stateMach()' and handle all state machine processing, including processing all stimuli that change state and the case for setting the state to PACK_OFF instead of having an extra case in loop()

as i demonstrated in post #6, the state machine execute every cycle, within it there can be a switch statement that determines which sub-block or sub-function is executed, within which is a check for a one or more stimuli that would change state

why not have a function that evaluate all button press combinations and returns a stimuli: Config, QuickBoot, Standard, Shutdown, Play, Off, as well as None

another very common convention is to only Capitalize Constants. functions shouldn't be Capitalize, while Macros would be

(what value is there to prepend "PROGRAM_" to each function and "MODE_" to each mode)? shorter, less repetitive symbol names are easier to read

[e1m1]

In short,
I am working on a 3d printed Ghostbusters Protonpack. And I want to light it up and play sounds according to the movie prop.

And because I don't want to buy preassembled stuff I want to do all the electronics on my own.
I want to recreate as much as possible of the audio and lightning stuff as shown in this video here:

GPStar Proton Pack Kit FEATURE REEL!

the blue leds on the top right of the pack are my "PowerCell Leds", the yellow ones are the "bargraph" ones which i realise with the shiftregister class DM13A.

the red ones on the bottom are the "cyclotron" leds i am going to control with the fastLed lib.

and the "modes" and different led effects are depending on which buttons and switches have been pressed or set, here in the video all switches and buttons are on the "wand" the thing the guy holds in his hands.

[gcjr]

so this is most likely just a single file program, possibly < 1000 lines, nothing too complicated where you should use some advanced techniques.

a state machine approach seems fine but may no be explicitly necessary.

simply monitoring for button presses may be sufficient to either just turn something On or enable some processing that uses a timer to a turn things on/off periodically, sequentially or randomly.

there can be multiple actions based on independent timers

it's not clear if some actions require turning something else Off or if that somethign else can just be left On, in other words this "mode" or some other "mode"

and of course there can be some button press sequence that just turns everything off.

the first thing in design is to figure out WHAT needs to be done. It's only afterwards that you figure out HOW to do it

[gcjr]

here's another approach, while state-driven, it's really not a state machine. buttons just enable various actions that are then invoked from loop() when enabled

const byte PinButs [] = { A1, A2, A3 };
const int  Nbut       = sizeof (PinButs);
      byte butState [Nbut];

const byte PinLeds [] = { 10, 11, 12, 13 };
const int  Nled       = sizeof (PinLeds);

enum { Off = HIGH, On = LOW };

// -----------------------------------------------------------------------------
enum { Blue, Yellow, Red, Nact };
bool enabled [Nact];
int  state   [Nact];

char s [90];

// -------------------------------------
// alternately toggle 2 LEDs
void actBlue ()
{
    if (! enabled [Blue])  {
        digitalWrite (PinLeds [0], Off);
        digitalWrite (PinLeds [1], Off);
        return;
    }

    if (state [Blue])  {
        digitalWrite (PinLeds [0], Off);
        digitalWrite (PinLeds [1], On);
    }
    else  {
        digitalWrite (PinLeds [0], On);
        digitalWrite (PinLeds [1], Off);
    }

    state [Blue] = ! state [Blue];
}

// -------------------------------------
// bargraph
void actYellow ()
{
    const  int       BarSz = 10;
    static char      bar [BarSz];
    static unsigned  pos;

    if (! enabled [Yellow])  {
        memset (bar, '\0', BarSz);
        return;
    }

    if (pos < BarSz-1)
        bar [pos++] = '*';
    Serial.println (bar);
}

// -------------------------------------
// rotating sequence
void actRed ()
{
    const  int       SeqSz = 20;
    static char      seq [SeqSz];
    static unsigned  pos;

    if (! enabled [Red])  {
        memset (seq, '\0', SeqSz);
        return;
    }
    
    for (unsigned i = 0; i < SeqSz-1; i++) {
        seq [i] = ' ';
        if (pos == (i % 3))
            seq [i] = '*';
    }

    if (3 <= ++pos)
        pos = 0;
    seq [SeqSz-1] = '\0';

    Serial.print   ("                   ");
    Serial.println (seq);
}

// -------------------------------------
// all actions occur periodically

unsigned long MsecPeriod = 100;
unsigned long msec0;

void actions ()
{
    unsigned long msec = millis ();
    if (msec - msec0 < MsecPeriod)
        return;
    msec0 += MsecPeriod;

    for (unsigned n = 0; n < Nbut; n++)  {
        switch (n)  {
        case Blue:
            actBlue ();
            break;
        case Yellow:
            actYellow ();
            break;
        case Red:
            actRed ();
            break;
        }
    }
}

// -----------------------------------------------------------------------------
void chkButs ()
{
    for (unsigned n = 0; n < Nbut; n++)  {
        byte but = digitalRead (PinButs [n]);
        if (butState [n] != but)  {
            butState [n]  = but;
            delay (20);             // debounce

            if (LOW == but)
                enabled [n] = ! enabled [n];
        }
    }
}

// -----------------------------------------------------------------------------
void loop ()
{
    chkButs ();
    actions ();
}

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

    for (unsigned n = 0; n < Nbut; n++)  {
        pinMode (PinButs [n], INPUT_PULLUP);
        butState [n] = digitalRead (PinButs [n]);

        pinMode (PinLeds [n], OUTPUT);
        digitalWrite (PinLeds [n], Off);
    }
}

[noiasca]

@e1m1

I am working on a 3d printed Ghostbusters Protonpack. And I want to light it up and play sounds according to the movie prop.

I understand that you have different modes.
One button should activate a specific pattern.
You could define a pattern where you specify how long which LED is on.
I just found something I had on wokwi - may be it is of any help:

Pattern with time - Wokwi ESP32, STM32, Arduino Simulator

Currently it is for 16 outputs, but if you just use a larger variable, you can easily use 32 or 64 outputs. It doesn't matter so much if these are discrete pins, neopixels or sounds.

To switch between several modes you just define another pattern to be executed.