Sorry, this might be asked many times, but I need to make this clear. I have seen different examples available and would like to know what is the proper way of waiting for commands in terms of performance?
I have currently this, but is it the most efficient approach?:
if the selection is by a simple type such as an int or char try using a switch statement
in terms of code generation there is probably not much difference but switch may be more readable
don't care to much if several ifs (or else Ifs) are more "efficent" than a switch case.
The compiler will do a pretty good job in optimizing your sketch anway.
Use what you can read best - even in some months.
What you can try is to put your if or switch within your if Serial available statement, so all the ifs / or switch case are only processed if you have readed something from serial:
struct Commands_devices_T
{
unsigned char command;
uint8_t pin;
bool value;
};
Commands_devices_T commands_devices[] =
{
{'a', device1, true},
{'b', device1, false},
{'c', device2, true},
//Etc etc.
};
void readInputCommands()
{
unsigned char choice;
if(Serial.available())
{
choice = Serial.read();
}
for (uint8_t c = 0; c < sizeof(commands_devices)/sizeof(commands_devices[0]), c ++)
{
if (commands_devices[c].command == choice)
{
digitalWrite(commands_devices[c].pin, commands_devices[c].value);
}
}
}
Something like this. Not faster than your solution or technically better, but somewhat easier to maintain IMO.
Disclaimer: I didn't compile this so there may be very silly bugs, and this is obviously not a complete sketch.
Well that code has a simple flaw - it repeatedly handles any command read forever, rather than just once when the command is actually read:
You need to only obey a command when its read. There is no reason for choice to be a global variable (this is part of the problem in fact).
And its normal to use "else if" when selecting alternative paths of execution.
Here's how I'd improve that:
void readInputCommands()
{
if(Serial.available() > 0)
{
char choice = Serial.read();
if(choice == 'a') // only interpret command if its newly arrived....
{
digitalWrite(device1, HIGH);
}
else if(choice == 'b')
{
digitalWrite(device1 LOW);
}
.... etc etc ...
else
{
// unknown command? Perhaps flag as an error somehow.
}
}
}
Note that in this example switch/case would be much neater than if/else if/else
It would also be cleaner to package the command execution in yet another function as its a different thing from reading the command. Lots of small well-named functions makes code much clearer and easier to evolve.
The reason I differentiated was two fold: making it a bit clearer to read (in my view) and also this is to make the switch effficient. Most of the time you’ll get -1 and a switch generates a costly branching mechanism. So if you need to save a few microseconds that helps.(but may be the optimizer handles this adjust as a whole, I have not checked the assembly code)
Logic: if I get command g = case 'g' , I need to turn Pin3 and Pin4 on. Then keep Pin3 for certain amount of time and switch it off. I also need be able to turn it on off, by certain commands e and f.
void readInputCommands1() {
int r = Serial.read();
if (r != -1)
{
switch (r)
{
case 'a': digitalWrite(Pin1, HIGH); break;
case 'b': digitalWrite(Pin1, LOW); break;
case 'c': digitalWrite(Pin2, HIGH); break;
case 'd': digitalWrite(Pin2, LOW); break;
case 'e': digitalWrite(Pin3, HIGH); break;
case 'f': digitalWrite(Pin3, LOW); break;
case 'g': switchState = HIGH;
digitalWrite(Pin4, HIGH);
digitalWrite(Pin3, HIGH);
currentMillis = millis();
break;
case 'h': digitalWrite(Pin4, LOW); break;
default: /* unknonw command */; break;
}
if (switchState == HIGH)
{
if (millis() - currentMillis >= period)
{
switchState = LOW;
}
digitalWrite(Pin3, switchState);
}
r = '\0';
}
}
as i suggested, you may want to have a generic commands to set, clr or toggle a specific pin, preceding the command with digits indicating the value of the pin
Robin2 handles that as shown here by using a flag called newData, initialised false. No actions are performed on data while that flag is false. It gets set true when data is* read, which enables the action, done under control of an if looking at that flag. Then as soon as the action is complete (in the example the action is simply to show the data, but it could be anything) the flag is set false again, so the action cannot happen again until the flag is set true on the arrival of more data.
