LED dimming not works?

Hi everynoe!

I'm fairly new to the Arduino, but I like it very much. :grin:
But I have a problem.

I would like to make a program with c# and with an Arduino Uno R3 that sets the light intensity of an external led (not the onboard) through a scrollbar. The scroll bar has a 0 to 255 range.
I searched throughout the Net, and I got the analogWrite() solution. The only problem is, that it doesn't seem to work at all.
Here is my code for the Arduino:

// Led connected to the pin 12
int extled=12;
void setup()
{
pinMode(extled,OUTPUT);
Serial.begin(9600);
}
void loop()
{
if(Serial.available())
{
byte c =Serial.read();
analogWrite(extled,c);
}
}

And here is my c# code as well:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.IO.Ports;

namespace ledfade
{
public partial class Form1 : Form
{

public Form1()
{
InitializeComponent();
serialPort1.PortName = "COM4"; //using COM4
serialPort1.BaudRate = 9600; // Setting Baudrate
fadescroller.Maximum = 255; //Maximum value of the scrollbar
fadescroller.Minimum = 1; // Minimum value of the scrollbar
label1.Text = fadescroller.Value.ToString(); // Shows the value of the scrollbar currently on
timer1.Start(); //Starts a timer that I use to send the values continously to the Arduino

}

private void Form1_FormClosing(object sender, FormClosingEventArgs e)
{
if (serialPort1.IsOpen)
{
serialPort1.Close();
}
}

private void btnon_Click(object sender, EventArgs e) //on button
{
serialPort1.Open(); //opens the serial port
btnoff.Enabled = true; //enables the off button
btnon.Enabled = false; // disables the on button
textBox1.Text = "L.E.D is on!"; //led status
}

private void btnoff_Click(object sender, EventArgs e) //off button
{
btnoff.Enabled = false;
btnon.Enabled = true;
serialPort1.Close(); //closes the serial port
textBox1.Text = "L.E.D is off!";
}

private void timer1_Tick(object sender, EventArgs e) //every time the timer ticks it executes the tasks below. Timer interval 0,175 sec.
{
label1.Text = fadescroller.Value.ToString(); //shows the value of the scrollbar in a label. Labels can only show string type.
if (serialPort1.IsOpen)
{
serialPort1.Write(fadescroller.Value.ToString()); //it sends the scrollbars value through the serial port as string(Is this the source of the proplem?)
textBox2.Text = fadescroller.Value.ToString();
}
}
}
}

Oh, I'm using a 280 ohm resistor, but it didn't work without the resistor either. The led is operational, I tested it many times.
I hope you guys can provide me with some useful advices.

Thank you in advance!

Look at Serial.parseInt() - Arduino Reference - you are sending a string, but the arduino is reading a byte(binary number)

Pin 12 is not one of the PWM pins on a Uno so analogWrite is not going to give you a PWM signal on it.

The only problem is, that it doesn't seem to work at all.

That's not strictly true - as has been pointed-out, pin 12 is not a PWM pin, but analogWrite will still "work", in the sense that it will turn the LED on or off.

Look at Serial.parseInt() - Arduino Reference - you are sending a string, but the arduino is reading a byte(binary number)

Be prepared, with the C# code that you have, for a sizable delay while Serial decides that there will be no more characters in the value.

Or get smart about what you are sending, and how you read the code. Send something like "<185>" (instead of "185"), and use some code like this:

.

#define SOP '<'
#define EOP '>'

bool started = false;
bool ended = false;

char inData[80];
byte index;

void setup()
{
   Serial.begin(57600);
   // Other stuff...
}

void loop()
{
  // Read all serial data available, as fast as possible
  while(Serial.available() > 0)
  {
    char inChar = Serial.read();
    if(inChar == SOP)
    {
       index = 0;
       inData[index] = '\0';
       started = true;
       ended = false;
    }
    else if(inChar == EOP)
    {
       ended = true;
       break;
    }
    else
    {
      if(index < 79)
      {
        inData[index] = inChar;
        index++;
        inData[index] = '\0';
      }
    }
  }

  // We are here either because all pending serial
  // data has been read OR because an end of
  // packet marker arrived. Which is it?
  if(started && ended)
  {
    // The end of packet marker arrived. Process the packet

    // Reset for the next packet
    started = false;
    ended = false;
    index = 0;
    inData[index] = '\0';
  }
}

Where it says "Process the packet", call atoi() with inData as the argument. Then, write the return value to the PWM pin, using analogWrite().

Thanks for the tips guys!
I rewired the and now connected to pin5.
The parseInt() method worked, but it was slow. It only set the light of the led, when I pressed the off button.
So, I used your code PaulS, but it doesn't seem to work properly.
As soon as I open the serial port with the program it lits the led and the intensity do not change as I scroll the scroll bar.
The program doesn't seem to get the end of package?
Here is your code in my version:

#define SOP '<'
#define EOP '>'
bool started = false;
bool ended = false;
char inchar;
const int extled=5;
char c[10]; //that's the indata.
byte index;

void setup()
{
  pinMode(extled,OUTPUT);
  Serial.begin(9600);
}

void loop()
{
  while(Serial.available()>0)
  {
    inchar=Serial.read();
    if( inchar==SOP)
    {
      index =0;
      c[index]='0';
      started = true;
    }
    else if(inchar ==EOP)
    {
      ended = true;
      break;
    }
    else
    {
      if (index<10)
      {
        c[index] = inchar;
        index++;
      }
    }
  }
  if(started==true&&ended==true) / 
  {
    atoi(c);
    analogWrite(extled,inchar); // I think the problem is realy here.
    started = false;
    ended = false;
    index=0;
    c[index] = '0';
  }
}

I also changed the c# code to send out the value as: serialPort1.Write("<"+fadescroller.Value.ToString()+">"); so it will look like this <180>.
Any ideas?

  c[index]='0';

That isn't what he wrote.

    atoi(c);
    analogWrite(extled,inchar); // I think the problem is realy here.

You writing the last single character received ( > ) to the LED instead of the result of the atoi() which you have thrown away.

Ok, I found the problem!

if(started==true&&ended==true)
  {
    atoi(c);
    analogWrite(extled,);
    started = false;
    ended = false;
    index=0;
    c[index] = '0';
 }

This is almost right but when I tried to send it to the Arduino it said the conversion is not good.
So I did this:

if(started==true&&ended==true)
  {
    analogWrite(extled,atoi(c));
    started = false;
    ended = false;
    index=0;
    c[index] = '0';
  }

It works!
Thanks everyone!

    c[index] = '0';

Still not what PaulS advised.

Tough it works it's not perfect.
When I set the increasing the intensity for the first time it's work like charm. But when I start to decrease it, the led sometimes flickers, and instead of decreasing the light intensity it grows. If I restart the arduino it works fine, but always just for the first try...

x[index]='/0';

Yes, it's not what he advised. I corrected it since your comment. Thank you.
Btw, what's the difference between: '\0' and '0' ?

x[index]='/0';

That's not what he said either.

c[index]='\0' then.
But you should tell me the difference if you're here between c[index]='\0' and c[index]='0';

But you should tell me the difference if you're here between c[index]='\0' and c[index]='0';

I should.
'0' is the ASCII character zero (0x30), and is used to display a zero on ASCII terminals.
'\0' is the binary value zero (0x00) and is used to terminate C strings.

Well... it seems irrelevant, because no matter which one I use, it's still blinking if I try to decrease the intensity.

Well... it seems irrelevant

No, it isn't irrelevant, it is fundamental to understanding C string representation and handling.

I see.
Then what could cause the problem?

Then what could cause the problem?

No idea.
Can't see your code. (That was a hint, by the way)

#define SOP '<'
#define EOP '>'
bool started = false;
bool ended = false;
char inchar;
const int extled=5;
char c[10];
byte index;

void setup()
{
  pinMode(extled,OUTPUT);
  Serial.begin(9600);
}

void loop()
{
  while(Serial.available()>0)
  {
    inchar=Serial.read();
    if( inchar==SOP)
    {
      index =0;
      c[index]='\0';
      started = true;
    }
    else if(inchar ==EOP)
    {
      ended = true;
      break;
    }
    else
    {
      if (index<10)
      {
        c[index] = inchar;
        index++;
      }
    }
  }
  if(started==true&&ended==true)
  {
    //atoi(c);
    analogWrite(extled,atoi(c));
    started = false;
    ended = false;
    index=0;
    c[index] = '\0';
  }
}

Here's the code for the arduino.

Now go back to reply #4 and see where you're going wrong, particularly around this if (index<10)

When I said '\0' wasn't irrelevant, I really meant it.