hi how can I simplify the following code?
with a for loop...
if (buttonState[1] == HIGH)
{
myArray[0] = 2;
radio.write(myArray, 1);
}
if (buttonState[1] != lastButtonState[1])
{
if (buttonState[1] == LOW)
{
myArray[0] = 0;
radio.write(myArray, 1);
}
// Delay a little bit to avoid bouncing
delay(5);
}
// save the current state as the last state, for next time through the loop
lastButtonState[1] = buttonState[1];
//************************************************************************************
if (buttonState[2] == HIGH)
{
myArray[0] = 3;
radio.write(myArray, 1);
}
if (buttonState[2] != lastButtonState[2])
{
if (buttonState[2] == LOW)
{
myArray[0] = 0;
radio.write(myArray, 1);
}
// Delay a little bit to avoid bouncing
delay(5);
}
// save the current state as the last state, for next time through the loop
lastButtonState[2] = buttonState[2];
bool buttonStates[] = {buttonState[1], buttonState[2], lastButtonState[1], lastButtonState[2]};
for ( byte x = 0 ; x < 2 ; x ++ ) {
if ( buttonStates[x] != buttonStates[x+2] ) {
if ( buttonStates[x] == LOW) {
myArray[0] = 0;
radio.write(myArray, 1);
}
// Delay a little bit to avoid bouncing
delay(5);
}
// save the current state as the last state, for next time through the loop
buttonStates[x+2] = buttonStates[x];
}
destiny2008:
yes i need help writing the for loop, i tried unsuccesful
It will be easier to provide useful help if you post the program (complete program, please) that represents your best attempt. That way we can see what you are having difficulty with.
Robin2:
It will be easier to provide useful help if you post the program (complete program, please) that represents your best attempt. That way we can see what you are having difficulty with.
…R
here is the working full code:.
#include <nRF24L01.h>
#include <printf.h>
#include <RF24.h>
#include <RF24_config.h>
RF24 radio(9, 10);
// Transmitter address
const uint64_t pipe = 0xE8E8F0F0E1LL;
const uint8_t buttonPin[] = {2, 3, 4};
uint8_t myArray[1];
bool buttonState[] = {0, 0, 0}; // current state of the button
bool lastButtonState[] = {0, 0, 0}; // previous state of the button
void setup(void)
{
Serial.begin(115200);
printf_begin();
for (uint8_t Pin = 2; Pin < 4 ; Pin++)
{
pinMode(Pin, INPUT);
}
radio.begin();
radio.setDataRate(RF24_250KBPS); // reducing bandwidth
radio.openWritingPipe(pipe); // set the transmitter
radio.printDetails();
}
void loop(void)
{
for (uint8_t j = 0; j <= 2; j++)
{
buttonState[j] = digitalRead(buttonPin[j]);
}
//**********************************************************************
if (buttonState[0] == HIGH)
{
myArray[0] = 1;
radio.write(myArray, 1);
}
if (buttonState[0] != lastButtonState[0])
{
if (buttonState[0] == LOW)
{
myArray[0] = 0;
radio.write(myArray, 1);
}
// Delay a little bit to avoid bouncing
delay(5);
}
// save the current state as the last state, for next time through the loop
lastButtonState[0] = buttonState[0];
//**************************************************************************************
if (buttonState[1] == HIGH)
{
myArray[0] = 2;
radio.write(myArray, 1);
}
if (buttonState[1] != lastButtonState[1])
{
if (buttonState[1] == LOW)
{
myArray[0] = 0;
radio.write(myArray, 1);
}
// Delay a little bit to avoid bouncing
delay(5);
}
// save the current state as the last state, for next time through the loop
lastButtonState[1] = buttonState[1];
//************************************************************************************
if (buttonState[2] == HIGH)
{
myArray[0] = 3;
radio.write(myArray, 1);
}
if (buttonState[2] != lastButtonState[2])
{
if (buttonState[2] == LOW)
{
myArray[0] = 0;
radio.write(myArray, 1);
}
// Delay a little bit to avoid bouncing
delay(5);
}
// save the current state as the last state, for next time through the loop
lastButtonState[2] = buttonState[2];
}
With a properly written for statement and body that is clearly too short.
Move the first block of code after the for statement body into the body of the for statement, and make the necessary edits (to use j instead of 0). Then, delete the repetitive code.
Unconditionally sending one piece of data when the pin is HIGH and conditionally (when the pin becomes LOW) sending another piece of data does not seem like the right thing to do.
PaulS:
With a properly written for statement and body that is clearly too short.
Move the first block of code after the for statement body into the body of the for statement, and make the necessary edits (to use j instead of 0). Then, delete the repetitive code.
Unconditionally sending one piece of data when the pin is HIGH and conditionally (when the pin becomes LOW) sending another piece of data does not seem like the right thing to do.
the following works but all 3 buttons repond to the same LED . is it because of myArray missing?
No, it is because you are not sending enough information. In addition to the state of the pin, you need to send the pin index, so you know to set the state of pin[index] to the appropriate value.
You'll need to restore myArray, but make it hold two elements - the index (j) and the state (buttonState[j]).
On the receiver, you need to size the array appropriately, too, and use both pieces of data.
PaulS:
No, it is because you are not sending enough information. In addition to the state of the pin, you need to send the pin index, so you know to set the state of pin[index] to the appropriate value.
You’ll need to restore myArray, but make it hold two elements - the index (j) and the state (buttonState[j]).
On the receiver, you need to size the array appropriately, too, and use both pieces of data.
this is what is working on tx for me :
can it be any simpler?
Store a value in the ith position, and then check the value in the jth position. Hmmm...
Simpler? It is far too early to be talking about simplify code that is WRONG.
You have gone back to sending a value when the pin IS high and another when the pin BECOMES low.
That is WRONG!
sorry posted before the fix :
buttonState[j] = digitalRead(buttonPin[j]);
if (buttonState[j] == HIGH)
but don’t see another way not to send another LOW and have it instruct the RX to turn led off.
unless you have an example tx and rx to turn it off without sending a high/low radio write, I don’t see it sorry.
not an expert at this.
You should ONLY send data when there has been a state change. The switch became pressed, so send one value. The switch became released, so send a different value. That's the way the code in reply #12 worked.
You can't just add the index value and the to-be state to form the value to send. index = 0 and state = 1 adds up to 1. So does index = 1 and state = 0. So, the receiver wouldn't know what to do to what pin.
You could, if you insist on sending just one byte, use value = index * 10 + state, which would work until you have more than 9 pins.
What's so hard about sending two bytes - one containing the index and one containing the state?
PaulS:
You should ONLY send data when there has been a state change. The switch became pressed, so send one value. The switch became released, so send a different value. That's the way the code in reply #12 worked.
You can't just add the index value and the to-be state to form the value to send. index = 0 and state = 1 adds up to 1. So does index = 1 and state = 0. So, the receiver wouldn't know what to do to what pin.
You could, if you insist on sending just one byte, use value = index * 10 + state, which would work until you have more than 9 pins.
What's so hard about sending two bytes - one containing the index and one containing the state?
so at the end of the day you will need only 1 radio write statement doing all this? or 2 ?