4

I have some code on Arduino (so, written in C++) that receives a String through the UART terminal, reads the String, then decides what String to print back and how many things to print depending on the data.

I've used an if-else stack to decide on what to print back. Something like this:

    if (cmdtype == "VALCN" && canSend == true) { //canSend is a flag from above. 
      cmdparam = "00000000";
      rtnProtocols(cmdtype, cmdparam);
    }
    else if (cmdtype == "FIRMV" && canSend == true) {
      cmdparam = "00000000";  //See desktop code for reason why cmdparam is what it is.
      rtnProtocols(cmdtype, cmdparam);
    }
    else if (cmdtype == "RQSPC" && canSend == true) { //RQSPC is a command
      //Need to get serial info, date, end of device info
      cmdtype = "GVSPC";    //Send device info signal
      cmdparam = "00101201";  //cmdparam is serial number
      rtnProtocols(cmdtype, cmdparam);
      
      cmdparam = "30082022";  //cmdparam is date
      rtnProtocols(cmdtype, cmdparam);

      cmdtype = "ENSPC";    //Send device info signal
      cmdparam = "00000000";  //cmdparam is date
      rtnProtocols(cmdtype, cmdparam);
    }

There are a lot of variations to the receivable String, so I'm foreseeing this if-else stack spiralling out of control. I am wondering whether there is a way of doing this without using an if-else stack. Or even if there is a way of doing this without conditionals, since all I'm doing with the branches is having it run the same function with different inputs, different times.

I've considered a switch-case, but wasn't able to do so since switch-case can't take Strings. Please let me know if this question is not suitable here, or if more information is needed.

2
  • You can pre-calculate a table with hash values, one for each command string. Make these values constants with the same name as the commands and use them as cases. Switch on the hash value of the incoming command. Chances you will hit a duplicate hash value are negligible but if it happens you would notice when making the table and you could pick a hash algorithm with better distribution. Aug 30, 2022 at 14:09
  • Does this answer your question? Best approach - convert multiple conditional if -else in a more handy design
    – gnat
    Aug 30, 2022 at 14:56

5 Answers 5

7

To prevent over-engineering, first boil down on what is done:

if (canSend) {
    if (cmdtype == "VALCN") {
      rtnProtocols(cmdtype, "00000000");
    }
    else if (cmdtype == "FIRMV") {
      rtnProtocols(cmdtype, "00000000");
    }
    else if (cmdtype == "RQSPC") {
      rtnProtocols("GVSPC", "00101201");
      rtnProtocols("GVSPC", "30082022");
      rtnProtocols("ENSPC", "00000000");
    }
}

This already looks very regular, but in reality irregularities happen.

The code above with named constants (and commented = explained again) would already be fine.

Finding common business rules, logic, would be nice:

if (canSend) {
    if (cmdtype == "RQSPC") { // First prepare
      rtnProtocols("GVSPC", "00101201");
      rtnProtocols("GVSPC", "30082022");
      cmdtype = "ENSPC";
    }
    rtnProtocols(cmdtype, "00000000"); // Do 000
}

Only if the code really diverges or is actually an other system component, use specialising class inheritance. Irregularities should not impact other cases, so then inheritance is best. The class hierarchy can also be just an Encoder object passed. That is the class itself need not be using inheritance. The code here is more or less doing some strategy. Details belong typical to some processing class one works with (Single Responsibility).

Above cmdtype could be an enum, enabling a switch. But it might even be superfluous when you have a child class for RSQPC, etcetera.

There is yet an other approach: declarative: the criteria as data. For instance in XML. You then have one piece of code interpreting the data.

<rtn cmdtype="VALCN">
    <protocols param="0000000"/>
</rtn>
...

This allows logging for instance, and finding uncovered cases. Also this XML is sufficiently readable for the business side. And for mass of report variants, you can store in the report case info, like a "form number" or other meta information.

2
  • 3
    For the declarative, beware of Inner-platform effect, where you're constructing a (usually poorly-design) language out of XML. IMO, this XML is exactly as clear as the code sample you suggest, so it's only applicable if you need to change it in the field where you can't recompile the source.
    – Jonathan
    Aug 31, 2022 at 6:40
  • 1
    @Jonathan thanks, that is worthwhile reading. Having many criteria in the business area, an XML that extracts this information, without doing the plumbing, is IMHO precious. A DSL as some are eager to create, indeed has that inner-platform effect. The example the OP chose, is constant rich, and more a kind of plugging cables. A declarative version is easier to trace, centralized, and less prone to similar code.
    – Joop Eggen
    Aug 31, 2022 at 7:42
6

There are some things you can do, but ultimately you may need to live with some compromise due to Arduino resource limitations.

Cleanup approaches

One thing is to separate the input parsing from handling of the different cases. If there are a limited number of possible commands, you can represent them as enums (or simply #defined values) and convert from string to the appropriate enum using a table. That allows you to use a switch statement, which might be enough for your case.

You could also expand this table to include a function pointer and possibly arguments, so the reaction to each command would be a separate function that just handles that command. This is a pretty natural approach to interpreting commands in languages where you have function pointers.

Style suggestions

One thing that you should certainly do is to distinguish between inputs and outputs. You seem to be using cmdtype as an input to your sequential if statement but you modify it within the cases, which is bad style.

In your short excerpt, canSend is checked for each case - that's an opportunity to move that check to an outer if and avoid it in the inner cases.

2
  • What are the advantages of function pointer in this case? The way I understand it is that function pointers are computationally less expensive every time they are called (bc they only need to return the address of the function instead of the function), but I'd need to declare the pointer every time since I'm using the Arduino's loop function to house the if-else stack, while the function that runs in the stack is declared elsewhere. I hope I've voiced my question clearly enough within the limits of my understanding. Aug 30, 2022 at 13:43
  • 2
    I'm not really fluent in C or C++, but a function pointer can be statically assigned, i.e. the whole table could be constant, no need to declare it every time through the loop. Aug 30, 2022 at 14:07
5

The obvious answer here is a dispatch table, i.e. something like std::map<string, function_type> (assuming you have the STL available, if you don't you can probably fake this up usefully enough with an array).

My C++ is horribly, horribly rusty but this seems to compile and work...

#include <iostream>
#include <map>
#include <string>

typedef void (*action_fn)(void);

void valcn(void)
{   
    std::cout << "Processing VALCN\n";
}

void firmv(void)
{   
    std::cout << "Processing FIRMV\n";
}

int main(int argc, char **argv)
{   
    std::map<std::string, action_fn> dispatch;

    dispatch["VALCN"] = valcn;
    dispatch["FIRMV"] = firmv;
    // ...

    dispatch["FIRMV"]();

    return 0;
}
6
  • 2
    On an Arduino STL is likely to be unavailable (a lot of MCU programming does not even have heap available). But a linear search through an array would like work well enough.
    – jaskij
    Aug 30, 2022 at 19:31
  • @jaskij I found this, no idea if it works or not. But yeah, if it's only 5-10 entries a linear search is good enough. Aug 30, 2022 at 19:39
  • 2
    Sadly, you can forget about STL on Arduino. It would simply take too much memory, so almost nothing is available. Arduino C++ often looks like plain C. And Arduino Strings shouldn't be used either, they use too much heap and lead to random crashes. Aug 30, 2022 at 20:26
  • 1
    @EricDuminil That was my initial understanding, but the OP explicitly changed the question from C to C++, so... Aug 30, 2022 at 20:30
  • 2
    @PhilipKendall I don't have experience with Arduinos, but do have some experience with microcontroller programming. Suffice to say, you get very, very little of C++'s standard library. It's more like C with classes and templates.
    – jaskij
    Aug 30, 2022 at 20:48
2

Command pattern

The command pattern might help you.

You could define one function for each cmdtype, and call the corresponding function when you receive some input via terminal.

This approach was also suggested by @PhilipKendall. The problem is that Arduino-C++ isn't really a complete C++, and std::map<std::string, action_fn> isn't available.

Arduino Strings should be avoided, too, because they use too much heap.

Interactive shell

I wrote a small Arduino library for a similar use-case, and asked a related question on Codereview.

The corresponding functions have either no argument, one string argument or one integer argument (e.g. for booleans or serial numbers).

Here's a modified version, for your use-case:

#include "command_invoker.h" // https://github.com/EricDuminil/arduino_interactive_shell/tree/master/arduino_interactive_shell

/*
 * Define your logic in those functions:
 */

bool sendDisabled = true;

void allowSend(int32_t allowed) {
  Serial.print("Sending data is now ");
  Serial.println(allowed ? "allowed" : "disallowed");
  sendDisabled = !allowed;
}

bool cannotSend() {
  if (sendDisabled) {
    Serial.println("Not sending anything. Sorry. Call SEND 1 first.");
  }
  return sendDisabled;
}

void valcn() {
  if (cannotSend()) return;
  Serial.println("Let's call rtnProtocols with VALCN!");
}

void firmv() {
  if (cannotSend()) return;
  Serial.println("Let's call rtnProtocols with FIRMV!");
}

void rqspc(int32_t serial_number) {
  if (cannotSend()) return;
  Serial.print("Let's call RQSPC with ");
  Serial.print(serial_number);
  Serial.println("!");
}

/**
 * Setup
 */

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

  // Assign commands here:

  command_invoker::defineIntCommand("SEND", allowSend, F(" 0/1 (Allow SEND or not)"));
  command_invoker::defineIntCommand("RQSPC", rqspc, F(" 123456 (doc here)"));
  command_invoker::defineCommand("VALCN", valcn, F(" (doc here)"));
  command_invoker::defineCommand("FIRMV", firmv, F(" (doc here)"));

  Serial.println(F("Console is ready!"));
  Serial.print(F("> "));
}

/*
 * Saves bytes from Serial.read() until enter is pressed, and tries to run the corresponding command.
 *   http://www.gammon.com.au/serial
 */
void processSerialInput(const byte input_byte) {
  static char input_line[MAX_COMMAND_SIZE];
  static unsigned int input_pos = 0;
  switch (input_byte) {
  case '\n': // end of text
    Serial.println();
    input_line[input_pos] = 0;
    command_invoker::execute(input_line);
    input_pos = 0;
    Serial.print(F("> "));
    break;
  case '\r': // discard carriage return
    break;
  case '\b': // backspace
    if (input_pos > 0) {
      input_pos--;
      Serial.print(F("\b \b"));
    }
    break;
  default:
    // keep adding if not full ... allow for terminating null byte
    if (input_pos < (MAX_COMMAND_SIZE - 1)) {
      input_line[input_pos++] = input_byte;
      Serial.print((char) input_byte);
    }
    break;
  }
}

/**
 * Loop and wait for serial input.
 */

void loop() {
  while (Serial.available() > 0) {
    processSerialInput(Serial.read());
  }
  delay(50);
}

The code is a bit long, but most of it is generic (setup/loop/processSerialInput), and it wouldn't get more complex with more functions. You'd simply add functions and callbacks inside setup.

Example

The above code seems to work fine on my ESP8266. As a bonus, you can get a list of every defined command by typing an unknown one (e.g. 'help'), and you can use hex numbers as parameter (e.g. 'RQSPC 0xFF00FF'):

Console is ready!
> help
No argument
'help' not supported. Available commands :
  FIRMV (doc here).
  RQSPC 123456 (doc here).
  SEND 0/1 (Allow SEND or not).
  VALCN (doc here).
> FIRMV
No argument
Calling : FIRMV()
Not sending anything. Sorry. Call SEND 1 first.
> SEND 1
Calling : SEND(1)
Sending data is now allowed
> RQSPC 12345
Calling : RQSPC(12345)
Let's call RQSPC with 12345!
> FIRMV
No argument
Calling : FIRMV()
Let's call rtnProtocols with FIRMV!
> VALCN
No argument
Calling : VALCN()
Let's call rtnProtocols with VALCN!
> SEND 0
Calling : SEND(0)
Sending data is now disallowed
> VALCN
No argument
Calling : VALCN()
Not sending anything. Sorry. Call SEND 1 first.
1
  • Downvoter: constructive criticism is welcome. Aug 31, 2022 at 11:33
0

At least to me, this looks like a fairly simple case for a simple map to handle. I'd start by defining a type to hold the command type and command param for a call to rtnProtocols:

struct params {
    std::string cmdtype;
    std::string cmdparam;
};

I'd probably also write a wrapper for rtnProtocols to accept one of these directly:

void rtnProtocols(params const &p) {
    rtnProtocols(p.cmdtype, p.cmdparam);
}

Then I'd create a map (or unordered_map) with the input string as the key, and a vector of params as the associated value:

std::map<std::string, std::vector<params> responses {
    { "VALCN", {{ "VALCN", "00000000"}}},
    { "FIRMV", {{ "FIRMV", "00000000"}}},
    { "RQSPC", {{"GVSPC", "00101201"},
                {"GVSPC", "30082022" },
                {"ENSPC", "00000000" } 
               }
    }
};

Then when we get some input, we look it up in the map, find the vector of associated values, and process them accordingly:

if (canSend) { 
    auto *resp = responses.find(cmdtype);
    if (resp == responses.end()) {
       error("unrecognized command: ", cmdtype);
    }
    auto const &response_params = resp->second;
    for (auto const &p : response_params) {
        rtnProtocols(p);
    }
}

At least assuming the other results follow roughly the same pattern as the ones you've shown, dealing with more inputs should only involve adding entries for each additional input, and the parameters you want to pass to rtnProtocols when you receive each.

2

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.