int r = 12;
int g = 11;
int b = 10;
int sw = 4;
int x;
int c = 0;
void setup() {
pinMode(r, OUTPUT);
pinMode(g, OUTPUT);
pinMode(b, OUTPUT);
pinMode(sw, INPUT);
pinMode(13, OUTPUT);
}
void loop() {
x = digitalRead(sw);
if(x==HIGH) {
delay(250);
c++;
if(c==1) {
digitalWrite(r, HIGH);
digitalWrite(g, LOW);
digitalWrite(b, LOW);
{
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
}
}
else if(c==2) {
digitalWrite(r, LOW);
digitalWrite(g, HIGH);
digitalWrite(b, LOW);
{
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
}
}
else if(c==3) {
digitalWrite(r, LOW);
digitalWrite(g, LOW);
digitalWrite(b, HIGH);
{
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
}
}
else if(c==4) {
digitalWrite(r, HIGH);
digitalWrite(g, HIGH);
digitalWrite(b, LOW);
{
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
}
}
else if(c==5) {
digitalWrite(r, LOW);
digitalWrite(g, HIGH);
digitalWrite(b, HIGH);
{
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
}
}
else if(c==6) {
digitalWrite(r, HIGH);
digitalWrite(g, LOW);
digitalWrite(b, HIGH);
{
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
c=0;
}
}
}
}
4 Answers
int r = 12, g = 11,b = 10, sw = 4, x, c = 0;
void setup() {
pinMode(r, OUTPUT);
pinMode(g, OUTPUT);
pinMode(b, OUTPUT);
pinMode(sw, INPUT);
pinMode(13, OUTPUT);
}
void loop() {
x = digitalRead(sw);
if(x==HIGH) {
delay(250);
c++;
if(c==1) setRGBandBlink13(HIGH,LOW,LOW,1);
else if(c==2) setRGBandBlink13(LOW,HIGH,LOW,2);
else if(c==3) setRGBandBlink13(LOW,LOW,HIGH,3);
else if(c==4) setRGBandBlink13(HIGH,HIGH,LOW,4);
else if(c==5) setRGBandBlink13(LOW,HIGH,HIGH,5);
else if(c==6){
setRGBandBlink13(HIGH,LOW,HIGH,6);
c=0;
}
}
}
void setRGBandBlink13(bool newR,bool newG,bool newB,int times){
setRGB( newR, newG, newB);
blinkPin13(times);
}
void setRGB(bool newR,bool newG,bool newB){
digitalWrite(r, newR);
digitalWrite(g, newG);
digitalWrite(b, newB);
}
void blinkPin13(int times){
for(int i=0;i<times;i++){
digitalWrite(13,HIGH);
delay (250);
digitalWrite(13, LOW);
delay (250);
}
}
This Sketch is way shorter and way better to maintain.
Note that 1/3 (26 out of 78) of the lines are comments.
Note:
Your RGB is able to display 8 states, but only 7 can be reached.
binc - c - RGB
000 - 0 -
001 - 1 - R
010 - 2 - G
011 - 3 - RG
100 - 4 - B
101 - 5 - R B
110 - 6 - GB
111 - 7 - RGB - NEVER REACHED
int r = 12;
int g = 11;
int b = 10;
int sw = 4;
int c = 0;
int maxC = 6;
int minC = 0;
void setup() {
pinMode(r, OUTPUT);
pinMode(g, OUTPUT);
pinMode(b, OUTPUT);
pinMode(sw, INPUT);
pinMode(13, OUTPUT);
}
// for (pulses)-times
// set (targetPin) HIGH
// wait (msDelay)
// set (targetPin) LOW
// wait (msDelay)
void pulse( int targetPin, int msDelay, int pulses ){
// count to pulses
// for each count, set targetPin HIGH and LOW
// wait msDelay between each state change
for ( int i = 0; i <= pulses, i++ ){
digitalWrite( targetPin, HIGH );
delay( msDelay );
digitalWrite( targetPin, LOW );
delay( msDelay );
}
}
// int rgbBinary will set RGB according to its binary value.
// R ~= BIT_1
// G ~= BIT_2
// B ~= BIT_4
void setRGB( int rgbBinary ){
// set each LED OFF
// This is by far more performant than
// checking which should keep its state.
digitalWrite( r, LOW );
digitalWrite( g, LOW );
digitalWrite( b, LOW );
// if rgbBinary contains BIT value 1, set R to HIGH
if ( rgbBinary & 1 ) {
digitalWrite( r, HIGH) ;
}
// if rgbBinary contains BIT value 2, set R to HIGH
if ( rgbBinary & 2 ) {
digitalWrite( g, HIGH );
}
// if rgbBinary contains BIT value 4, set R to HIGH
if ( rgbBinary & 4 ) {
digitalWrite( b, HIGH );
}
}
void loop() {
// INPUT on SW ?
if ( digitalRead( sw ) == HIGH) {
// wait 250ms
delay( 250 );
// incement c
c++;
// increment RGB status
setRGB( c );
// pulse OUTPUT 13 (c)-times
// starting by HIGH
// delay 250ms between each HIGH/LOW
pulse( 13, 250, c );
// reset (c) on maxC
if ( c == maxC ){
c = minC;
}
}
}
I don't have an appropriate circuit handy for testing, but I've condensed the code a bit and renamed some things to be more readable. Hopefully the result is the same! Importantly, I've changed your c to step and it is now zero-based, so "step 1" is 0 and "step 6" is 5, etc.
const int rPin = 12;
const int gPin = 11;
const int bPin = 10;
const int swPin = 4;
const int pin13 = 13; // Needs a better name!
const int msDelay = 250;
int step = 0;
// By pure luck, your patterns overlap, so they can be condensed like so.
const byte patterns[13] = {
// 0 1 2 3 4 5
HIGH, LOW, LOW, HIGH, LOW, LOW, HIGH, HIGH, LOW, HIGH, HIGH, LOW, HIGH
};
void setup()
{
pinMode(rPin, OUTPUT);
pinMode(gPin, OUTPUT);
pinMode(bPin, OUTPUT);
pinMode(swPin, INPUT);
pinMode(pin13, OUTPUT);
}
void loop()
{
if (digitalRead(swPin) == HIGH)
{
delay(msDelay);
// Write red/green/blue pattern
int index = step * 2;
digitalWrite(rPin, patterns[index]);
digitalWrite(gPin, patterns[index + 1]);
digitalWrite(bPin, patterns[index + 2]);
// Blink once for each step (0 through 5)
for (int i = 0; i <= step; ++i)
{
digitalWrite(pin13, HIGH);
delay(msDelay);
digitalWrite(pin13, LOW);
delay(msDelay);
}
// Increment or reset step
step = (step < 5) ? step + 1 : 0;
}
}
Make all those ints in a single line.
int r = 12, g = 11,b = 10, sw = 4, x, c = 0;
-
While valid C, it is really bad coding style to do that. Declare each variable in its own line.JayEye– JayEye2016-05-02 06:15:48 +00:00Commented May 2, 2016 at 6:15
-
In this manner, compiler will trim code anyway. Declaring vars in a single row will decrease readability by a lot but won't trim the actual binary in the end. Nothing gained by that.jawo– jawo2016-05-02 06:18:54 +00:00Commented May 2, 2016 at 6:18
-
You could argue that putting everything in one line (
int r=12;int g=11; int b=10; int sw=4; int x; int c=0;) would also "shorten" the code. But it barely decreases the amount of functional code. I also highly doubt that @USF meant to shorten this part of the code.aaa– aaa2016-05-03 07:14:48 +00:00Commented May 3, 2016 at 7:14
if()block into a separate block? Certainly you could considerably shorten your code by putting those statements into a separate function that takes as its argument the number of times to toggle pin 13.cis? How about making that into a loop?