Code efficiency help for DIY RC transmitter and quadcopter

Before I start, I just want to say that as much as I want to test this, I cant. I am missing parts.

Anyways, the code shown is quite a large improvement to my first attempt at my quadcopter code.

I feel that, since I am a intermediate programmer, this code is rubbish. I think I need the help of someone with more programming experience

I also feel that someone here can help me get smaller code

The !radio.write() is for if it doesn't send, and the transmitter fails or quadcopter is out of range

Lastly, if you're wondering why I am using values 10000, it's like an encoding scheme I thought of. So if the Rx receives a value around 1000,it will be roll. In Rx, if such a value is received, it is.subtracted by 1000, leaving me with whatever servo signal I intend to send (so 1080-1000=80; motors react to that

Here it is. Please don't judge! They're only like less than 80% done.

Transmitter

#include <SPI.h>
#include <nRF24L01.h>
#include <RF24.h>

int
LeftStickX = 14,  //A0
LeftStickY = 15,  //A1
RightStickX = 16, //A2
RightStickY = 17; //A3

const byte address[5] = {'R','o','M','e','O'};

RF24 radio(9, 10);

long command;
bool sending;

void setup()
{
	
	pinMode(LeftStickX, INPUT);
	pinMode(LeftStickY, INPUT);
	pinMode(RightStickX, INPUT);
	pinMode(RightStickY, INPUT);
	
	radio.begin();
	radio.setDataRate(RF24_250KBPS);
	radio.setRetries(2, 15)
	radio.openWritingPipe(address);

}

void loop()
{
	char testingConnection= "CONNECT TEST";
	bool connected = false;
	bool test = radio.write(connectTest, &sizeof(connectTest));
	if(test && connected = false)
	{
		//BEEEEEEP
		bool connected = true
	}
	else
	{
		while(connected = false)
		{
			// BEEP BEEP
		}
		
	}
	
//Out of range test
	int currentTime = millis();
	int prevTime;
	if(currentTime - prevTime >= 1000)
	{
		send();
		prevTime = millis();
	}
}
}

void send()
{
	/*
	this is where potentiometer is
	read and sent
	*/
   
    int 
	THROTTLE,
	ROLL, 
	PITCH, 
	YAW;
	
	int 
	RAW_THROTTLE,
	RAW_ROLL,
	RAW_PITCH,
	RAW_YAW;
	
	RAW_YAW =
	analogRead(LeftStickX);
	YAW =
	map(RAW_YAW, 0,1023,-20200, 20200);	
	
	RAW_THROTTLE =
	analogRead(LeftStickY);
	THROTTLE = 
	map(RAW_THROTTLE,0,1023,0,200);
	
	RAW_ROLL =
	analogRead(RightStickX);
	ROLL =
	map(RAW_ROLL,0,1023,-1200,1200);
	
	RAW_PITCH =
	analogRead(RightStickY);
	PITCH =
	map(RAW_PITCH,0,1023,-10200,10200);
	
    radio.write(YAW, &sizeof(YAW));
    radio.write(THROTTLE, &sizeof(THROTLLE));
    radio.write(ROLL, &sizeof(ROLL));
    radio.write(PITCH, &sizeof(PITCH));

//radio has ACK pkgs for Tx, so I use that
//Credits to Robin2. Thanks.


    if(!radio.write(YAW, &sizeof(YAW)))
    {
    	//BUZZER BEEP
    }
    
    if(!radio.write(THROTTLE, &sizeof(THROTTLE)))
    {
    	//BUZZER BEEP
    }
    
    if(!radio.write(ROLL, &sizeof(ROLL)))
    {
    	//BUZZER BEEP
    }
    
    if(!radio.write(PITCH, &sizeof(PITCH)))
    {
    	//BUZZER BEEP
    }
}
    
}

/*
void verifyPotChange()
{
	supposed to record "now" pot values
	so that multiple values are not sent.
	with some thinking, i dont think this
	is necessary. just send values
	uncomment to test
	
	long 
	CURRENT_THROTTLE = RAW_THROTTLE,
	CURRENT_ROLL = RAW_ROLL,
	CURRENT_PITCH = RAW_PITCH,
	CURRENT_YAW = RAW_YAW;
	
	if(RAW_THROTTLE > || < CURRENT_THROTTLE)
	{
		radio.write()
	}
	
}
*/

Receiver

#include <SPI.h>
#include <nRF24L01.h>
#include <RF24.h>

#include <Servo.h>

const byte address[5] = {'R','o','M','e','O'};

RF24 radio (9, 10);

long command;

bool newData = false;

//init all vars using previous code
/*motor 1 is CW, 2 is front right, 3 is back left, 4 is back right */

void setup(){
	radio.begin();
	radio.setDataRate(RF24_250KBPS);
	radio.setRetries(2, 15);
	radio.openReadingPipe(1, address);
	radio.startListening();
}

void flightMode0()
{
	while(armed == true && mode0 = true)
	{
		if(radio.available())
		{
			radio.read(command,&sizeof(command));
			newData = true;
		}
		
		if(newData = true)
		
////////////////THROTTLE///////////////

			if(command >= 0 && command <= 200)
			{
				int throttle;
				throttle = command;
				1.write(throttle);
				2.write(throttle);
				3.write(throttle);
				4.write(throttle);
			}
		
/////////////////ROLL//////////////////

			if(command >= 1000 && command <= 1200)
			{
				int roll;
				roll = command - 1000;
				1.write(roll);
				2.write(roll -= 20);
				3.write(roll);
				4.write(roll -= 20);
			}

			if(command >= -1200 && command <= -1000)
			{
				int negRoll;
				negRoll = (command * -1) -1000;
				1.write(negRoll -= 10);
				2.write(negRoll);
				3.write(negRoll -= 10);
				4.write(negRoll);
			}

/////////////////PITCH/////////////////

			if(command >= 10000 && command <= 10200)
			{
				int pitch;
				pitch = command -= 10000;
				1.write(pitch -= 10);
				2.write(pitch -= 10);
				3.write(pitch);
				4.write(pitch);
			}

			if(command >= -10200 && command <= -10000)
			{
				int negPitch;
				negPitch = (command * -1) - 10000;
				1.write(negPitch);
				2.write(negPitch);
				3.write(negPitch -= 10);
				4.write(negPitch -= 10);
			}

/////////////////YAW///////////////////

			if(command >= 20000 && command <= 20200)
			{
				int yaw;
				yaw = command -= 20000;
				1.write(yaw);
				2.write(yaw -= 20);
				3.write(yaw -= 20);
				4.write(yaw);
			}

			if(command >= -20200 && command <= -20000)
			{
				int negYaw;
				negYaw = (command * -1) -20000;
				1.write(negYaw -= 20);
				2.write(negYaw);
				3.write(negYaw);
				4.write(negYaw -= 20);
			}
		
		newData = false;
		}
	}
}

void flightMode1(){
	//old flight mode settings here...
	while(armed == true && mode = 1){
		//main code belongs here...
		}
}

void changeMode(){
	if(command = 100000){
		mode0 = false;
		}

	if(command = 200000){
		mode0 = true;
	}
}

void preArm()
{
	if(armed == false)
	{
		1.write(5);
		2.write(5);
		3.write(5);
		4.write(5);
		delay(4000);
		armed == true;
	}
}

void loop(){
	bool armed = false;
	bool mode0 = true;

	preArm();

	flightMode0();

	flightMode1();

	changeMode();
}

You would make life very much easier for yourself if you send all the data in a single message. Just create an array of 4 ints and put the Yaw, Throttle, Roll, and Pitch numbers into it.

You will also make life much easier if your send() function does nothing except send data. Create another function to read the joystick values. That way you can test the different parts of the program separately.

Have a look at these links
Simple nRF24L01+ Tutorial
Planning and Implementing a Program

...R

Hmmm...

I'll do my research on arrays. Thanks you, sir

So there was a function I commented out of the transmitter. It was meant for checking which potentiometer had the latest output.

Can someone help me with that? I did some thinking and found the idea stupid because I can just send the value in a straightforward manner.

Does this send the whole array?

//Tx side
radio.write(command, &sizeof(command));

But how can i "retrieve the contents" of the array?

I assume that within the array, I use the same "encoding" scheme? And on Rx, how will it know if the command[0] was the one I want.

Or does the Rx act on the whole array itself?

Forgive my lengthy questions.

EDIT: How's this:

//sending an array and acting on it

//TX
int array[4];
void loop()
{
	radio.write(array, &sizeof(array));
}

//act on it
//Rx
void loop()
{
	if(radio.available())
	{
		char throttle, roll, pitch, yaw;
		throttle = array[0];
		roll = array[1];
		pitch = array[2];
		yaw = array[3];
		
		motors.write(throttle);
		motors.write(roll);
		motors.write(pitch);
		motors.write(yaw);
	}
}

If this is correct, I will be sooo happy because of the reduction of code.

Look at this example for RF24, Handling Data.

Note the structure created:

struct dataStruct{
  unsigned long _micros;
  float value;
}myData;

Get this example to work (don't add anything like reading inputs).

Once it does, you can change the structure to hold all the variables you need, e.g.

struct dataStruct {
  byte roll;
  byte pitch;
  byte yaw;
} myData;

If you have not used structure before, maybe have a ,quick google and then note that you can access the members of the structure simply like this:

myData.roll= 17;

Keep using some dummy data for now.

You can access the structure on the receiver to see what values you have received:

Serial.println(myData.roll);

You should develop the transmitter/receiver part of the program separate to the reading of inputs part. When they both work only then should you combine them (and any other separate parts).

JeromeAriola:
And on Rx, how will it know if the command[0] was the one I want.

Every time you send data, you will send the entire structure. The receiver has been given the same structure definition so it knows that the data it receives is in this structure, and therefore that the first byte is roll, second it pitch etc. So it will populate the roll, pitch and yaw components of the structure with whatever data you sent in the roll, pitch and yaw components.

EDIT: you edited your post while I was writing mine. Your example is incomplete, but I think you have the right idea. Again, I would recommend looking at the example I linked.

JeromeAriola:
But how can i "retrieve the contents" of the array?

You should have an array in the receiver with a definition that is identical to the array in the sender.

...R

With what bms001 I can use the same method I used for the array, except that I use structs

What I am trying to ask is if I want to do this

radio.write(someArray, &sizeof(somearray))

with structs I do

struct Struct{
    int data0;
    int data1;
}someStruct;

radio.write(someStruct.data0, &sizeof(someStruct.data0));

radio.write(someStruct.data1, &sizeof(someStruct.data1));

But isn't the array method more efficient because whatever data I need is contained in the array?

bms001:
Your example is incomplete, but I think you have the right idea. Again, I would recommend looking at the example I linked.

In this case, what am I missing?

JeromeAriola:

radio.write(someStruct.data0, &sizeof(someStruct.data0));

Wouldn't life be an awful lot easier with

radio.write(&someStruct, sizeof(someStruct));

And note that you have your & in the wrong place. The & character tells the compiler to use the address of a variable.

...R

JeromeAriola:
with structs I do

struct Struct{

int data0;
   int data1;
}someStruct;

radio.write(someStruct.data0, &sizeof(someStruct.data0));

radio.write(someStruct.data1, &sizeof(someStruct.data1));




But isn't the array method more efficient because whatever data I need is contained in the array?

No, with structs you would do this:

radio.write(someStruct, &sizeof(someStruct));

If your array someArray is a 2 integer array than then I assume that the practical result will be identical (you send 4 bytes of data).

On the receiver you would need one of these:

radio.read( &someArray , sizeof(someArray ) ); // if using array
radio.read( &someStruct, sizeof(someStruct) ); // if using struct

JeromeAriola:
In this case, what am I missing?

You are missing looking at the example I recommended.

Use either method, but the main point is that you send the entire array/structure to the sending nRF24, and you retrieve the entire array/structure from the receiving nRF24.
If you have different data types in your radio data package then a struct is probably easier.

Many thanks!

So to retrieve the contents of the message, I declare an identical array/struct on receiver. Then, on receiver I can go ahead and use something like someStruct.data0 and it would be the intended value.

This is a great help to my project and career as a programmer.

JeromeAriola:
So to retrieve the contents of the message, I declare an identical array/struct on receiver. Then, on receiver I can go ahead and use something like someStruct.data0 and it would be the intended value.

Correct (once you have done radio.read(...) and there was data for it to return).