arduino won't go out of loop when new serial communication

Using Arduino Programming Questions
1

my project consists out of a visual studio application and an Arduino. The program has multiple options for a light color and an additional disco mode ( automatic switching between colors). The normal options work fine when you press them before the disco option, but whenever the disco button is clicked, it won't go out of the loop anymore when another button is pressed.

Arduino Code

int RedLight = 13;
int GreenLight = 12;
int BlueLight = 11;

unsigned long lasttime = 0;
unsigned long interval = 2000;
unsigned long currenttime = millis();
int colorMode = 0;

int message = 0;

void setup() {
  Serial.begin(9600) ;
  pinMode(RedLight, OUTPUT) ;
  pinMode(GreenLight, OUTPUT) ;
  pinMode(BlueLight, OUTPUT) ;
  colorSetting(LOW, LOW, LOW);
}

void loop() {

  if (Serial.available() > 0) { // Check to see if there is a new message
    message = Serial.read(); // Put the serial input into the message

    if (message == 'R') { // RED
      colorSetting(255, 0, 0);
      Serial.println("Red Light");
    } // Send back LED on

    if (message == 'Y') { // YELLOW
      colorSetting(255, 0, 255);
      Serial.println("Yellow Light");
    } // Send back LED on

    if (message == 'G') { //GREEN
      colorSetting(0, 0, 255 );
      Serial.println("Green Light"); // Send back LED on
    }

    if (message == 'C') { //CYAN
      colorSetting(0, 255, 255);
      Serial.println("Cyan Light"); // Send back LED on
    }

    if (message == 'B') { //BLUE
      colorSetting(0, 255, 0);
      Serial.println("Blue Light"); // Send back LED on
    }

    if (message == 'P') { //PINK
      colorSetting(255, 255, 0);
      Serial.println("Pink Light"); // Send back LED on
    }
    if (message == 'W') { //WHITE
      colorSetting(255, 255, 255);
      Serial.println("White Light"); // Send back LED on
    }

    do {
      discoMode();
      Serial.println("Disco Light"); // Send back LED on
    } while (message == 'D');
    }
}


void discoMode() {

  unsigned long currenttime = millis();

  if (currenttime - lasttime >= interval) {
    colorMode++;
    if (colorMode > 7) {
      colorMode = 1;
    }
    lasttime = currenttime;
  }

  if (colorMode == 1) {
    colorSetting(255, 0, 0);
  }

  if (colorMode == 2) {
    colorSetting(255, 0, 255);
  }

  if (colorMode == 3) {
    colorSetting(0, 0, 255 ) ;
  }

  if (colorMode == 4) {
    colorSetting(0, 255, 255);
  }

  if (colorMode == 5) {
    colorSetting(0, 255, 0);
  }

  if (colorMode == 6) {
    colorSetting(255, 255, 0);
  }

  if (colorMode == 7) {
    colorSetting(255, 255, 255);
  }
}


void colorSetting(int red, int green, int blue) //writes an analog value to a pin
{ // making use of common anode, so have to substract the color value from 255 in order to set the rgb color
  analogWrite( RedLight, 255 - red ) ;
  analogWrite( GreenLight, 255 - green ) ;
  analogWrite( BlueLight, 255 - blue ) ;
}

Visual studio code

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace WindowsFormsApp1
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
            serialPort1.PortName = "COM4";
            serialPort1.BaudRate = 9600;
            serialPort1.Open();
        }

        private void BtRed_Click(object sender, EventArgs e)
        {
            serialPort1.WriteLine("R");
            LbColor.Text = "The current color is Red";

        }

        private void BtBlue_Click(object sender, EventArgs e)
        {
            serialPort1.WriteLine("B");
            LbColor.Text = "The current color is Blue";
        }

        private void BtGreen_Click(object sender, EventArgs e)
        {
            serialPort1.WriteLine("G");
            LbColor.Text = "The current color is Green";
        }

        private void BtYellow_Click(object sender, EventArgs e)
        {
            serialPort1.WriteLine("Y");
            LbColor.Text = "The current color is Yellow";
        }

        private void BtCyan_Click(object sender, EventArgs e)
        {
            serialPort1.WriteLine("C");
            LbColor.Text = "The current color is Cyan";
        }

        private void BtPink_Click(object sender, EventArgs e)
        {
            serialPort1.WriteLine("P");
            LbColor.Text = "The current color is Pink";
        }

        private void BtWhite_Click(object sender, EventArgs e)
        {
            serialPort1.WriteLine("W");
            LbColor.Text = "The current color is White";
        }

        private void BtDisco_Click(object sender, EventArgs e)
        {
            serialPort1.WriteLine("D");
            LbColor.Text = "The current color is disco";
        }

    }
}
2

Here's why:

    do
    {
      discoMode();
      Serial.println("Disco Light"); // Send back LED on
    }
    while (message == 'D');

If message is 'D' that will loop forever, as you observe. There's nothing there to change message.

3

well no? bc when i press another button the message turns into a different letter?

also when i use an if statement, it will only go through the loop once.

4

caseystijlaart:
well no? bc when i press another button the message turns into a different letter?

Nope. During that while loop there is nothing that touches message. Your serial.read at the top of loop is never executed again.

5

So it would make more sense to do this instead?

while (message == 'D') {
      discoMode();
      Serial.println("Disco Light"); // Send back LED on
    }
6

caseystijlaart:
So it would make more sense to do this instead?

while (message == 'D') {

discoMode();
      Serial.println("Disco Light"); // Send back LED on
    }

No, that's just as bad.

You could do it the same as the other modes with an if. Move all of those ifs so that they run every time round loop. The other states will run repeatedly too, but that doesn't matter because they don't make a change. Then loop will be providing the repetition you want for disc mode.

7

sorry, i don't completely understand what you mean

8

Currently, your code checks for serial data and if there is any it reads it and uses the message to determine how to set the lights.

I'm suggesting that you read the data just the same, but you move all the light setting code out of the if controlled by Serial.available() so that they get checked on every iteration of loop whether there was new data or not.

9

wildbill:
Currently, your code checks for serial data and if there is any it reads it and uses the message to determine how to set the lights.

I'm suggesting that you read the data just the same, but you move all the light setting code out of the if controlled by Serial.available() so that they get checked on every iteration of loop whether there was new data or not.

so that's what i did now, but now my disco mode doesn't keep looping aka i need to press the button everytime to switch the color

void loop() {

  if (Serial.available() > 0) { // Check to see if there is a new message
    message = Serial.read(); // Put the serial input into the message
  }

  if (message == 'R') { // RED
    colorSetting(255, 0, 0);
    Serial.println("Red Light");
  } // Send back LED on

  if (message == 'Y') { // YELLOW
    colorSetting(255, 0, 255);
    Serial.println("Yellow Light");
  } // Send back LED on

  if (message == 'G') { //GREEN
    colorSetting(0, 0, 255 );
    Serial.println("Green Light"); // Send back LED on
  }

  if (message == 'C') { //CYAN
    colorSetting(0, 255, 255);
    Serial.println("Cyan Light"); // Send back LED on
  }

  if (message == 'B') { //BLUE
    colorSetting(0, 255, 0);
    Serial.println("Blue Light"); // Send back LED on
  }

  if (message == 'P') { //PINK
    colorSetting(255, 255, 0);
    Serial.println("Pink Light"); // Send back LED on
  }
  if (message == 'W') { //WHITE
    colorSetting(255, 255, 255);
    Serial.println("White Light"); // Send back LED on
  }

  if (message == 'D') {
    discoMode();
    Serial.println("Disco Light"); // Send back LED on
  }
}
10

It was probably a bad idea to use serialPort1.WriteLine. Try serialPort1.Write instead, at least for disco mode.

11

Oh wow, that did the thing! thank you!