Simple serial communication problem

int byteBuffer[2]; // Store incoming bytes
int bytes_read = 0;
byte inByte; // Read incoming bytes and then store to byteBuffer

int dimmerPin = 11; // Dimmer pin

int dimmerValue = 0; // Dimmer value

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

void loop() {

while(bytes_read <= 1) {
  if(Serial.available() > 0) {
    
    inByte = Serial.read(); // Read first byte
    byteBuffer[bytes_read] = inByte; // Store it
    bytes_read++;
    
    if(bytes_read == 1) { // Start again after 2 bytes
      bytes_read = 0;
    }
  }
}


if(byteBuffer[0] == 3) {
  
  if(byteBuffer[1] == 1) {
    dimmerValue = 50;
  }
  
  if(byteBuffer[1] == 2) {
    dimmerValue = 150;
  }
  
  if(byteBuffer[1] == 3) {
    dimmerValue = 255;
  }
}  

analogWrite(dimmerPin, dimmerValue);

}

So I'm trying to read two bytes from serial which is ok. But when I'm trying to change value of dimmerValue based on the two bytes just readed and stored to array nothing happens. I just can find out what's the problem here :confused:

What exactly are you sending from the serial monitor? Is it by chance '1', '2' or '3'?

Two numbers like 31 or 32.
I tried if arduino can read bytes correctly and made it write to serial byteBuffer[0] and byteBuffer[1] and they were 3 and 1 if I entered 31 for example.

Two numbers like 31 or 32

Right. My guess was wrong. Because you happened to choose '3' for the first byte to receive, I thought maybe you were trying to decode ASCII. (1 is 0x31, 2 is 0x32, 3 is 0x33)

The first thing is to remove this from 'setup()':-

pinMode(dimmerPin, OUTPUT);

It makes the pin a digital output, instead of a PWM output.

Next, because chars are really being sent, each 'if' statement should be comparing a char, not a byte value.
Like this:-

if (byteBuffer[0] == '3')

instead of like this:-

if (byteBuffer[0] == 3)

And finally, because they are characters, you should really declare them as char:-

char byteBuffer[2]; // Store incoming bytes
char inByte; // Read incoming bytes and then store to byteBuffer

You could perhaps rename them too, to "charBuffer" and "inChar", just for clarity. (Not essential.)

    inByte = Serial.read(); // Read first byte
    byteBuffer[bytes_read] = inByte; // Store it
    bytes_read++;
    
    if(bytes_read == 1) { // Start again after 2 bytes
      bytes_read = 0;

Soooo..... Read a byte, increment bytes_read, then, since bytes_read is now 1, set it back to zero? How will bytes_read ever be anything but 0?

Then

if(byteBuffer[0] == 3) {
  
  if(byteBuffer[1] == 1) {
    dimmerValue = 50;
  }

Then check is byte_Buffer[0] is 3, which would be an ASCII ETX character. What you want is an ASCII character '3':

if(byteBuffer[0] == '3') {
  
  if(byteBuffer[1] == '1') {
    dimmerValue = 50;
  }

Regards,
Ray L.

RayLivingston:

    inByte = Serial.read(); // Read first byte

byteBuffer[bytes_read] = inByte; // Store it
   bytes_read++;
   
   if(bytes_read == 1) { // Start again after 2 bytes
     bytes_read = 0;



Soooo..... Read a byte, increment bytes_read, then, since bytes_read is now 1, set it back to zero? How will bytes_read ever be anything but 0?

I missed this somehow.

Then

if(byteBuffer[0] == 3) {

if(byteBuffer[1] == 1) {
   dimmerValue = 50;
 }




Then check is byte_Buffer[0] is 3, which would be an ASCII ETX character. What you want is an ASCII character '3':



if(byteBuffer[0] == '3') {
 
 if(byteBuffer[1] == '1') {
   dimmerValue = 50;
 }

But already mentioned this.

Thanks for answers. I got it working now with readBytesUntil() function.

Here is the working code:

byte byteBuffer[2]; // Store incoming bytes
int lenght = 2;

int dimmerPin = 11; // Dimmer pin

int dimmerValue = 0; // Dimmer value

void setup() {
  Serial.begin(9600);
  
}

void loop() {

  if(Serial.available() > 0) {
    
    Serial.readBytesUntil(0x30, byteBuffer, lenght);

    // Dimmer
    if(byteBuffer[0] == 0x33) {
  
      if(byteBuffer[1] == 0x31) {
        dimmerValue = 10;
        Serial.println("10");
      } 
      if(byteBuffer[1] == 0x32) {
        dimmerValue = 150;
        Serial.println("150");
      }
      if(byteBuffer[1] == 0x33) {
        dimmerValue = 255;
        Serial.println("255");
      }
    }
       
  }    

analogWrite(dimmerPin, dimmerValue);

}

Why would you write something like this:

 if(byteBuffer[0] == 0x33)

When you could more easily write:

 if(byteBuffer[0] == '3')

and then someone else reading the code could easily understand what you were doing?

And why write this:

    if(byteBuffer[0] == 0x33) {
  
      if(byteBuffer[1] == 0x31) {
        dimmerValue = 10;
        Serial.println("10");
      } 
      if(byteBuffer[1] == 0x32) {
        dimmerValue = 150;
        Serial.println("150");
      }
      if(byteBuffer[1] == 0x33) {
        dimmerValue = 255;
        Serial.println("255");
      }
    }

when you could just as easily write this:

    if(byteBuffer[0] == '3') {
  
      if(byteBuffer[1] == '1') {
        dimmerValue = 10;
      } 
      if(byteBuffer[1] == '2') {
        dimmerValue = 150;
      }
      if(byteBuffer[1] == '3') {
        dimmerValue = 255;
      }
      Serial.println(dimmerValue);
    }

And use less FLASH and less RAM?

Any why even send that leading '3' when all you do is throw it away, and only use the second character?

Regards,
Ray L.

That Serial.println was just to see if it works. It is not going to be in the complete code.
The first byte is to select what to control with second byte. There are five transistors that are controlled over serial. So that's why. I was just trying to get that work and then make other controlling options.

@Spooniest, you may find some useful stuff in Serial Input Basics

...R

OldSteve:
The first thing is to remove this from 'setup()':-

pinMode(dimmerPin, OUTPUT);

It makes the pin a digital output, instead of a PWM output.

You can remove it, but I think it's harmless. AFAIK analogWrite() just duplicates the action by setting the port to output again.

aarg:
You can remove it, but I think it's harmless. AFAIK analogWrite() just duplicates the action by setting the port to output again.

You're right. I realised that shortly after I wrote it, but left it anyway since that line wasn't actually needed.
(I was actually thinking more of 'analogRead()' at the time, and got my wires crossed. :slight_smile: )