Cramming code

I am always looking for ways to limit code length within reason of ease to use and pulling code snippets into a project. I would like comments regarding this code snippet to run motor speed and varying the duty cycle with a pot. Note it is single direction only with the PWM pin output to a BUZ11 MOSFET to limit also external component count (this is part of a larger project)

//Motor variables
int pot = A0;
int stirMotor = 3;
int val = 0;

void setup()
{
//the only needed statements in setup
pinMode(stirMotor, OUTPUT);
analogWrite(stirMotor, 0); //Initial state of PWM off
}

void getPot() { //call this function in the main program to set the duty cycle
val = analogRead(pot)/4;
//ADC value between 0 and 1047 - Duty cycle value between 0 and 255 thus ADC/4
val=constrain(val,50,255); //this restricts the analog value to between 50 and 255
analogWrite(stirMotor, val); //this will run the motor at the speed set by the pot value
}

void loop()
{
getPot(); //get the analog value. and write the duty cycle to the motor

}

1006 bytes storage space and 11 bytes dynamic memory
Any suggestions

const int pot = A0;
const int stirMotor = 3;

void setup() {
  pinMode(stirMotor, OUTPUT);
  analogWrite(stirMotor, LOW);    //Initial state of PWM off
}
void loop() {
  analogWrite(stirMotor, constrain(analogRead(pot) / 4, 50, 255));
}

If you want re-useable code, then you have to remove all of those hard-coded constants. Think of it as a class in a library. Your actual program may want to set up several motors, all on different pins, so you need to have the pin assignments part of the constructor or the begin() function.

If you want to use less memory on the Arduino then you need to think about how it will fit into the rest of a program. Remember that each function you call requires only one copy of the function in memory. For example, you use constrain() in this snippet. If the rest of the code never used constrain() then this snippet forces the compiler to include a copy of constrain(). If you were already using constrain() in the rest of the code then adding this snippet doesn't need to include a new copy, so it will add less bytes to the program than you would expect.

The real killer in that function re-use is the floating-point functions. Every time you multiply or divide a floating-point number, the compiler has to include a large amount of 'invisible' code. If you write your code in a way that means you don't need to use floating-point, then suddenly the entire program gets significantly smaller.

Next time, use [ code ] tags. The button on the full editor looks like </>

Oh, yeah, I just noticed the global variables.

If you plug this snippet into another program that already uses val for something else, then it is going to make the whole program incorrect. If you want good code re-use, you must use all local variables. Even the constants of pin numbers can be local to the getPot() function.

Unless you really need 32768 pins (plus negative ones), use const byte for pin numbers.

AWOL:
Unless you really need 32768 pins (plus negative ones), use const byte for pin numbers.

Not sure why, but changing int to byte doesn't reduce flash or RAM as reported by Arduino

It would if you'd had an array of pin numbers.

The compiler is damned clever when it comes to literals, specifically when they're of a type expected by pinMode, digitalRead/Write etc.

OK Thank you for the information,
it helps a lot.
I see your point regarding the call of a function but it do make writing code easier. I will replace then that call with just that line.
I see your point with the Byte() data type also, it will free up a lot of space.
Is there something similar for just a HIGH or LOW state like e.g. just a bit or is the Byte() the minimum length

vmasters:
Is there something similar for just a HIGH or LOW state like e.g. just a bit or is the Byte() the minimum length

HIGH and LOW are #define as 0x1 and 0x0 respectively. They are just English words that clarify your intent. They don't increase or decrease your compiled code in flash or RAM.

OK thank you.
Here is another piece of code a battled for a long time
It is based on the "switch" of "David A. Mellis" but I wanted to use an pull-up pin for the switch to toggle code. I am still thinking on using an interrupt to stop the functions abruptly but it is not a necessity.
What I am trying to achieve here is to execute a loop and then wait for the button toggle (tactile switch) to call other functions (again calling other functions) and when the button is pressed go back to standby mode by calling the standby function and stopping and resetting everything.
This is only a test snippet to run with a simulator to sort out what to do and then copying it in the main program

//button variables
const byte inPin = 2;         // the number of the input pin
const byte outPin = 13;       // the number of the output pin
const byte ledPin = 12;      
// second led pin to test if the call of other functions within the loop work
byte state = HIGH;      // the current state of the output pin
byte reading;           // the current reading from the input pin
byte previous = HIGH;    // the previous reading from the input pin

// the follow variables are long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long time = 0;         // the last time the output pin was toggled
long debounce = 200;   // the debounce time, increase if the output flickers

void setup()
{
	pinMode(inPin, INPUT_PULLUP);
	pinMode(outPin, OUTPUT);
  pinMode(ledPin, OUTPUT);
  digitalWrite(ledPin, LOW);
}
void loop()
{
	reading = digitalRead(inPin);
	// if the input just went from LOW and HIGH and debounce time
	if (reading == LOW && previous == HIGH && millis() - time > debounce) {
		if (state == LOW)
			state = HIGH;
		else
			state = LOW;
		time = millis();    
	}
	
	digitalWrite(outPin, !state); //State Inverted to sync the led with change function
	previous = reading;
 
	if(state == LOW){
  change();               //execute void (change)
    }
    else
  digitalWrite(ledPin, LOW);  //Need this routine to reset changes made in change() back to start conditions with button toggle
//or call a function to reset everything to the initial state
}
void change() //just a function to test the call in the main loop
{
	digitalWrite(ledPin, HIGH);
}

What I actually want to do is omit the "Long" variables for Time and Debounce but I am not certain exactly how.

unsigned long time;         // the last time the output pin was toggled
const byte debounce = 200;

Pardon, I will return tomorrow as it is already 11:30 pm here and have to go to bed.
I would however appreciate some inputs and will read it tomorrow.
Regards :sleeping:

vmasters:
OK thank you.
Here is another piece of code a battled for a long time
It is based on the "switch" of "David A. Mellis" but I wanted to use an pull-up pin for the switch to toggle code. I am still thinking on using an interrupt to stop the functions abruptly but it is not a necessity.
What I am trying to achieve here is to execute a loop and then wait for the button toggle (tactile switch) to call other functions (again calling other functions) and when the button is pressed go back to standby mode by calling the standby function and stopping and resetting everything.
This is only a test snippet to run with a simulator to sort out what to do and then copying it in the main program

//button variables

const byte inPin = 2;         // the number of the input pin
const byte outPin = 13;       // the number of the output pin
const byte ledPin = 12;      
// second led pin to test if the call of other functions within the loop work
byte state = HIGH;      // the current state of the output pin
byte reading;           // the current reading from the input pin
byte previous = HIGH;    // the previous reading from the input pin

// the follow variables are long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long time = 0;         // the last time the output pin was toggled
long debounce = 200;   // the debounce time, increase if the output flickers

void setup()
{
pinMode(inPin, INPUT_PULLUP);
pinMode(outPin, OUTPUT);
 pinMode(ledPin, OUTPUT);
 digitalWrite(ledPin, LOW);
}
void loop()
{
reading = digitalRead(inPin);
// if the input just went from LOW and HIGH and debounce time
if (reading == LOW && previous == HIGH && millis() - time > debounce) {
if (state == LOW)
state = HIGH;
else
state = LOW;
time = millis();    
}

digitalWrite(outPin, !state); //State Inverted to sync the led with change function
previous = reading;

if(state == LOW){
 change();               //execute void (change)
   }
   else
 digitalWrite(ledPin, LOW);  //Need this routine to reset changes made in change() back to start conditions with button toggle
//or call a function to reset everything to the initial state
}
void change() //just a function to test the call in the main loop
{
digitalWrite(ledPin, HIGH);
}



What I actually want to do is omit the "Long" variables for Time and Debounce but I am not certain exactly how.

Study the use and actual functioning of an interrupt. It does NOT stop a function, merely "interrupts" the flow of instruction execution, executes the interrupt code, and returns to the next instruction in the function and continues merrily on it's way.

If you set a switch, flag, boolean,etc. in the interrupt code to signal that an interrupt has occurred, then your function can periodically test for the switch, flag, etc. and see if it is set, and if so, exit the function. For actual manual switches, you could just as efficiently test for the manual switch in your function code.

For manual switches, either way works, but wastes an interrupt that might be needed by some other device.

Paul

vmasters:
What I actually want to do is omit the "Long" variables for Time and Debounce but I am not certain exactly how.

unsigned int lessMilliseconds = millis() % 60000UL;

provided you won't need an interval of more than 60000 milliseconds, which is one minute.

const byte StartButtonPin = 2;
int lastStartButtonState = HIGH;
byte startButtonDebounceTimestamp = 0U;
byte debounceIntervalFlag = 0U;

void setup() {
  pinMode(StartButtonPin, INPUT_PULLUP);
}

void loop() {
  if (debounceIntervalFlag != 0U) {
    byte debounceResetTimestamp = millis() % 256UL;
    if (debounceResetTimestamp - startButtonDebounceTimestamp >= 25U) {
      debounceIntervalFlag = 0U;
    }
  }
  int startButtonState = digitalRead(StartButtonPin);
  if (startButtonState != lastStartButtonState && startButtonState == LOW) {
    byte startButtonTimestamp = millis() % 256UL;
    if (startButtonTimestamp - startButtonDebounceTimestamp >= 25U || debounceIntervalFlag == 0U) {
      startButtonDebounceTimestamp = startButtonTimestamp;
      lastStartButtonState = startButtonState;
      debounceIntervalFlag = 1U;
      doStartStuff();
    }
  }
  else lastStartButtonState = startButtonState;
}

void doStartStuff() {
  //do stuff
}

Perehama:

unsigned int lessMilliseconds = millis() % 60000UL;

provided you won't need an interval of more than 60000 milliseconds, which is one minute.

Terrible idea. The modulo operator uses a division internally. Remember long division at school? That is how the Arduino does it. But the Arduino can only hold 8 bits in its main accumulator so it has to do the division on 4 quarters of the long integer (really 4x4 combinations) then reassemble the result later.

Divide (and modulo) on longs are literally the slowest opeators on the Arduino. Even a floating-point divide is faster.

Perehama's use of 256 may enable the compiler to use a shortcut and not actually divide. But that is dependent upon the compiler and may change with any slight change in compiler version or optimiser settings. It is not worth it to save 3 bytes of memory.