Is this the best way to achieve the result?

Hello,

I have written this code to control the brightness of an LED using a potentiometer and the PWM pins, is this the best way of doing this? And before you say it, the whole point is to learn, I don't want you telling me to use just the pot without the arduino! ;)

int Pot = A0;
int LED = 9;
int value = 0;
int analog = 0;

void setup()
{
  
}

void loop()
{
  value = analogRead(Pot);
  analogWrite(LED, value/4);
}

Thanks! :)

is this the best way of doing this?

It works, right? Isn't that the mark of a good program?

Yes, that's a good program and very nearly as simple as you can get. There are some minor changes you could make:

  • variable 'analog' is not used, so its declaration could be removed

  • variable 'value' could be declared locally to loop(), avoiding use of global memory, keeping the definition local to where it is used, and making the code a little smaller.

  • your pin numbers could be declared 'const', saving some more memory and ensuring they can't be changed in the program

Like this:

const int Pot = A0;
const int LED = 9;

void setup()
{ 
}

void loop()
{
  int value = analogRead(Pot);
  analogWrite(LED, value/4);
}

Looks nice and simple, so that's great!

I note you don't use the variable "analog".

To save RAM you might make the things that don't change const. And you could conceivably get rid of "value".

const int Pot = A0;
const int LED = 9;

void setup()
{
}

void loop()
{
  analogWrite(LED, analogRead(Pot)/4);
}

But that's nitpicking. :)

Have fun with your Arduino!

Wow thanks for all the help! The "analog" was left over from some other experimentation, I just forgot to delete it!

And you could conceivably get rid of "value".

I agree that I could do that, but if I expand the code I think it would be easier to use "value" rather than having to always type out "analogRead(Pot)" :P

It works, right? Isn't that the mark of a good program?

Well, I think working is the mark of an acceptable program, I think a good program would be one that works and goes about it in the most efficient way :)

Thanks again for all the help, the changes have been made :)

Nebula: I agree that I could do that, but if I expand the code I think it would be easier to use "value" rather than having to always type out "analogRead(Pot)" :P

The size of the generated code is identical whether you use:

  int value = analogRead(Pot);
  analogWrite(LED, value/4);

or:

  analogWrite(LED, analogRead(Pot)/4);

Declaring 'value' locally to the loop() function enables the compiler to keep ii in a register and effectively optimize it away. However, if you use the first form but declare 'value' as a global variable instead, the code gets bigger.

Sorry I didn't really notice your post in between the others! The way you've explained the corrections is brilliant, thank you!! :)

One-line version:

void setup () {} void loop () { analogWrite (9, analogRead (A0)/4); }

Just kidding. ;)

I agree that I could do that, but if I expand the code I think it would be easier to use “value” rather than having to always type out “analogRead(Pot)”

You can also reuse the value without the delay of reading the pin again.

int value = analogRead(Pot);
if(value < 100)
  analogWrite(9, 104);
else if(value < 300)
  analogWrite(9, 200);
else if(value < 800)
  digitalWrite(9, HIGH);

requires one read.

if(analogRead(Pot) < 100)
  analogWrite(9, 104);
else if(analogRead(Pot) < 300)
  analogWrite(9, 200);
else if(analogRead(Pot) < 800)
  digitalWrite(9, HIGH);

requires 3 reads, and each read could conceivably return a different value.