Return sensor data as const references rather than by value

[rajat2004]

Fixes: #

About

This PR changes the sensor data fetch methods to return const Output& rather than Output. This avoids a copy when the methods are being used, even in the call is like - const auto& data = getImuData(), e.g. in ArduCopterApi.hpp, MavlinkMultiRotorApi.hpp. The copy of LidarData in particular will be more expensive

Note that a copy is still created if the method is called like - auto data = getImuData()

How Has This Been Tested?

This is supposed to be NFC, but will try to get some testing done later or tomorrow

I wrote a simple test script to check whether copies are occurring -

test.cpp

#include <iostream>

using namespace std;

class A {
public:
	int x;

	A() {
		cout << "Normal constructor" << endl;
	}
	
	A(const A& a) {
		cout << "Copy constructor" << endl;
	}
	
	~A() {
		cout << "Destructor" << endl;
	}
};

class B {
public:
	A a;
	
	A getA() {
//	const A& getA() {
		return a;
	}
};

A createA() {
	A a;
	return a;
}

int main() {
//	auto a1 = createA();
	B b;
	const auto& a2 = b.getA();
	return 0;
}

Try playing around with this and see what happens, the behaviour before and after the PR can be tested by modifying the return type of B::getA() and the line const auto& a2 = b.getA();

Before PR:
A getA() {, const auto& a2 = b.getA();
Output:

Normal constructor
Copy constructor
Destructor
Destructor

After PR:
const A& getA() {, const auto& a2 = b.getA();

Normal constructor
Destructor

The createA() method shows the effect of Return Value Optimization, where a copy isn't made even when return type and value are both A rather than refs. It only kicks in if the data to be returned is to be destroyed at the end.

Update:

Here's a more updated script, which more accurately reflects how things are working -

test2.cpp

#include <iostream>

using namespace std;

class A {
public:
	int x;

	A() {
		cout << "Normal constructor" << endl;
	}
	
	A(const A& a) {
		cout << "Copy constructor" << endl;
	}
	
	~A() {
		cout << "Destructor" << endl;
	}
};

class B {
public:
	A a;
	
//	A getA() {
	const A& getA() {
		return a;
	}
};

A createA() {
	A a;
	return a;
}

A getAfromB(B& b) {
	return b.getA();
}

int main() {
//	auto a1 = createA();
	B b;
//	const auto& a2 = b.getA();
	const auto& a3 = getAfromB(b);
	return 0;
}

a3 is a const ref, B::getA() also returns a const ref, but getAfromB() returns by value, output -

Normal constructor
Copy constructor
Destructor
Destructor

Change to const A& getAfromB(B& b) -

Normal constructor
Destructor

Screenshots (if appropriate):

[zimmy87]

Hey @rajat2004, can you take a look at resolving the current conflicts? We're ready to merge once you have those resolved.

[rajat2004]

@zimmy87 Done. I'll have to start going through my other PRs as well and fix the conflicts, will probably get on that tomorrow

[zimmy87]

Thanks for the contribution @rajat2004!