Tips or Alternatives

Hello all,

I am hoping that I can get some good replies as this code is very simple.

A little background, I am a university student using arduino UNO's for a project. This is the first code I have personally ever came up with and was pleased that it worked.

I was directed to control an LED (brightness) using a variable resistor (potentiometer) and after looking at some example code for LED's and POT's I came up with this:

/* LED Brightness control 

Aim: To control brightness of an LED using PWM output, and input from a Potentiometer


int ledPin = 9;          // set the led pin to pin 9
int sensorPin = A0;      // set sensor input to pin A0
int sensorValue = 0;     // variable to store the sensor value

void setup() {
  // set the led pin as an output
  pinMode(ledPin, OUTPUT);

void loop() {
  // read the sensor input and store it
  sensorValue = analogRead(sensorPin);
  // convert what is read to a new scale for PWM output
  float output = sensorValue * (255.0/1023.0);
  // write the output to the LED
  analogWrite(ledPin, output);
  // delay to see the effect, might remove this later

It works which is good, but I was hoping people could give me some points of improvement to make the code leaner and more efficient and hence if anyone has an alternative way of doing this I would love to hear your thoughts.

So fire away,

Regards Peter

Your sketch will compile to a smaller binary if you do the sensor scaling without floating point, using just integer arithmetic instead:

void loop() {
  analogWrite(ledPin, analogRead(sensorPin) >> 2);

Why a 20 mS delay? What useful purpose does it serve and have you read and unerstood the bilnk without delay sketch. A delay for viewing the data isn't necessary, you can change the displayed data when ever you choose... thus leaving time for other tasks?, Perhaps...


  float output = sensorValue * (255.0/1023.0);
  // write the output to the LED
  analogWrite(ledPin, output);

As analogWrite() does not take a float output is then cast to an int (or byte, the ref section does not say) so you could have saved SRAM space by using int or byte. Note that the Uno has no hardware support for * or / with bytes or ints, and no H/W support at all for floats. The above statement uses more than 10k clock cycles by my quick count.


First thanks for the replies already.

@billroy, what purpouse does the ">>2" serve? When I tried without the output was very twitchy and took much less adjustment to change.

Thanks for the tip, I obviously over complicated with the float feature.

@Docedison, the delay was because of what I seen in the "Fading" example, your right I didn't need it, hence the comment above it saying I may remove. I hadn't looked at that one as it was filed under the "digital" section and I was focused on the analogue section before getting hands on. Thanks for the input, I have had a quick look and will have to swat up on the "millis" function.

@holmes4, I took inspiration of an example that converted the 0 - 1023 range to a 0 - 5V range, thinking I could do the same with the 0 - 255. Thanks for the help.

The expression ">>2" is "shift right 2 bits", which is the same as dividing by four (but sometimes faster). You could say /4 and it would work the same way.


Thanks, a handy tip to know.

The >> op is shift right the two tells it the number of bits to shift. << is the left shift op.

float output = sensorValue * (255.0/1023.0) can be rewritten as

sensorValue * 255.0/1023.0 or (sensorValue * 255.0)/1023.0 or sensorValue * 255.0/1023.0 but you real should use 256 and not 255 as there are 256 values in the range 0-255. And 1024 as there are 1024 values in the the range 0-1023. 256/1024 = 1/4 is shift right 2 bits!.