improvement: newData Case statement - assign more meaningful words to integer values

[aviateur17]

In the UART and MQTT sketches there is a case statement which determines where the data is sent. Instead of using numbers 1 though 8 add #defines to name them in a more meaningful manner.

[Gulpman]

Hi @aviateur17 thanks for your input. This has already been implemented in the dev branch:

    switch (newData) {
      case 1:     //ESP-NOW #1
        ESPNOW1_ACT
        break;
      case 2:     //ESP-NOW #2
        ESPNOW2_ACT
        break;
      case 3:     //ESP-NOW General
        ESPNOWG_ACT
        break;
      case 4:     //Serial
        SERIAL_ACT
        break;
      case 5:     //MQTT
        MQTT_ACT
        break;
      case 6:     //LoRa General
        LORAG_ACT
        break;
      case 7:     //LoRa #1
        LORA1_ACT
        break;
      case 8:     //LoRa #2
        LORA2_ACT
        break;
    }

so consider it as done - not too far away in the future. ;)

[timmbogner]

Great minds think alike!

Yours, not mine. Thanks for fixing it!

[Gulpman]

Thanks for the cudos but... it wasn't me fixing it. I just remarked that it has been implemented in the dev code already. :) So fixing cudos must got to someone else. By checking the blame history I think it was you. :)

Great minds think alike!

Yours, not mine. Thanks for fixing it!

[aviateur17]

Sorry, I should have been more specific, I was thinking about something more like the below:`

#define INCMG_ESPNOW1 1
#define INCMG_ESPNOW2 2
#define INCMG_ESPNOWG 3
#define INCMG_SERIAL 4
#define INCMG_MQTT 5
#define INCMG_LORAG 6
#define INCMG_LORA1 7
#define INCMG_LORA2 8

switch (newData) {
  case INCMG_ESPNOW1:     //ESP-NOW #1
    ESPNOW1_ACT
    break;
  case INCMG_ESPNOW2:     //ESP-NOW #2
    ESPNOW2_ACT
    break;
  case INCMG_ESPNOWG:     //ESP-NOW General
    ESPNOWG_ACT
    break;
.....
.....

}

[Devilbinder]

Use a enumerator not defines.

enum{
    INCMG_ESPNOW1,
    INCMG_ESPNOW2,
    INCMG_ESPNOWG,
    INCMG_SERIAL,
    INCMG_MQTT,
   INCMG_LORAG,
   INCMG_LORA1,
   INCMG_LORA2,
   INCMG_MAX
}

[timmbogner]

Oh! Sorry, I actually understood what you meant, just had morning vision. Yes, good call. That should help improve readability and give me an excuse to learn about enums.

[timmbogner]

Okay so I think I have enumerators figured out now, but can someone confirm if there is a mistake on this website... shouldn't example 2 say year i; not int i; ?

[Gulpman]

Okay so I think I have enumerators figured out now, but can someone confirm if there is a mistake on this website... shouldn't example 2 say year i; not int i; ?

I tried both versions and cannot see a difference.

Here's the sketch I ran:

enum week {Monday=1, Tuesday=2, Wednesday=3, Thursday=4, Friday=5, Saturday=6, Sunday=7};

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
}

void loop() {
  // put your main code here, to run repeatedly:
    // creating eToday variable of enum week type
    enum week eToday;
    int iToday = Monday;
    eToday = Tuesday;
    Serial.print("Today as enum: ");
    Serial.println(eToday);
    Serial.print("Today as int: ");
    Serial.println(iToday);
    // lets change the values...
    iToday = Saturday;
    eToday = Sunday;
    Serial.println("Variables have changed!");
    Serial.print("Today as enum: ");
    Serial.println(eToday);
    Serial.print("Today as int: ");
    Serial.println(iToday);
    Serial.println("------------------------------");  

    delay(1000);
}

@Devilbinder may know better where or if there is a difference?

[timmbogner]

So then if all of that is correct, when the enum is declared, then Monday becomes equal to 0, Tuesday = 1, etc globally? I think I expected them to only hold those values when referring to an object of type week. You are also doing it differently than I see elsewhere: you write enum week eToday and I'd expect just week eToday from my understanding.

[Gulpman]

So then if all of that is correct, when the enum is declared, then Monday becomes equal to 0, Tuesday = 1, etc globally?

If you do not assign values: yes. In my example above: no. I assigned it to start with Monday=1 and concretly assigned values to each weekday.

I think I expected them to only hold those values when referring to an object of type week. You are also doing it differently than I see elsewhere: you write enum week eToday and I'd expect just week eToday from my understanding.

This was one of my sources (where I understood it best): https://www.programiz.com/c-programming/c-enumeration :) I liked it as there it was also explained how to use enums as flags.

But as I said: No professional programmer here as well. :)

I'm not at my PC so unfortunately I cannot try the example without the enum atm.

[Devilbinder]

2 steps back here. A enum starts at 0 by default. All names will follow sequentially by +1. The names must be unique. You'll commonly see it in tag_name format.

enum{
  la,
  di,
  da,
};

Is the same as

enum{
  la = 0,
  di = 1,
  da = 2,
};

You can change the number sequence at any point in the enum with a = operator

enum{
  la = 60,
  di = 50,
  da,
};

Result

enum{
  la = 60,
  di = 50,
  da = 51,
};

Naming a enum is optional. This us usually accompanied by a typedef .

enum foo{
  la,
  di,
  da,
};

typedef enum{
  la,
  di,
  da,
}foo;

[timmbogner]

Okedoke, I see I'm getting ahead of myself. The code will be...

enum{ event_clear, event_espnowg, event_espnow1, event_espnow2, event_serial.... };

...

void OnDataRecv(uint8_t* mac, uint8_t *incomingData, uint8_t len) {
#elif defined(ESP32)
void OnDataSent(const uint8_t *mac_addr, esp_now_send_status_t status) {
}
void OnDataRecv(const uint8_t * mac, const uint8_t *incomingData, int len) {
#endif
  memcpy(&theData, incomingData, sizeof(theData));
  memcpy(&incMAC, mac, sizeof(incMAC));
  DBG("Incoming ESP-NOW.");
  ln = len / sizeof(DataReading);
  if (memcmp(&incMAC, &ESPNOW1, 6) == 0) newData = event_espnowg;
  else if (memcmp(&incMAC, &ESPNOW2, 6) == 0) newData = event_espnow1;
  else newData = event_espnow2;

...

 if (newData) {
    switch (newData) {
      case event_espnowg:     //ESP-NOW #1
        ESPNOW1_ACT
        break;
      case event_espnow1:     //ESP-NOW #2
        ESPNOW2_ACT
        break;
      case event_espnow2:     //ESP-NOW General
        ESPNOWG_ACT
        break;
...
}

How'm I doin?

[Devilbinder]

The enum is fine and how it's used. enum are done line by line. Much easier to edit them if needed. Off topic. Rather do if else trees like this.

enum{ 
  event_clear, 
  event_espnowg, 
  event_espnow1,
   event_espnow2, 
   event_serial,
   .... 
};

...

void OnDataRecv(uint8_t* mac, uint8_t *incomingData, uint8_t len) {
#elif defined(ESP32)
void OnDataSent(const uint8_t *mac_addr, esp_now_send_status_t status) {
}
void OnDataRecv(const uint8_t * mac, const uint8_t *incomingData, int len) {
#endif
  memcpy(&theData, incomingData, sizeof(theData));
  memcpy(&incMAC, mac, sizeof(incMAC));
  DBG("Incoming ESP-NOW.");
  ln = len / sizeof(DataReading);
  if (memcmp(&incMAC, &ESPNOW1, 6) == 0){
    newData = event_espnowg;
    return;
  } 
  if (memcmp(&incMAC, &ESPNOW2, 6) == 0){
    newData = event_espnow1;
    return;
  } 
  newData = event_espnow2;

...

 if (newData) {
    switch (newData) {
      case event_espnowg:     //ESP-NOW #1
        ESPNOW1_ACT
        break;
      case event_espnow1:     //ESP-NOW #2
        ESPNOW2_ACT
        break;
      case event_espnow2:     //ESP-NOW General
        ESPNOWG_ACT
        break;
...
}

return is a function exit is it does not to have a value.

P.S. Code blocks can be tagged like this [```cpp] to add color

[timmbogner]

Added the enum with e10315b Fixed if/else statements 59196b7

[aviateur17]

Resolved in dev branch to be merged in main