AeroQuad Developers Guide

[Kenny9999]

Hello all

Recently, I received a couple of pull request in GIT and I wanted to make things clear on what I accept and on what bug me that could make me reject the pull request or ask for modification.

First of all, I’m not there to refuse any new feature or improvement. But, be sure of one thing, your code will be inspected line per line and what I search for is

1. READABILITY

- This is the main criteria, Am I able to read it like I would read a book? Do I know you direct intention as soon as I look at your code. I’m really sensitive on the look and readability of the code. “ALWAYS CODE LIKE IF THE GUY THAT WILL MAINTAIN YOUR CODE IS A KILLER PSYCHOPATH” As an open source community, there will be other people that will look at your work. May be you will develop a cool feature and fade out and then, we will find a bug and won’t be able to debug it because the code is too hard to read and understand. So, please, look at how it is currently written and try to replicate.

1.1 Take the time to think to the code you will write.
- Most people, mostly noob, will just go on the computer and start to code, i’ve been like this and now, I know it’s a mistake. if you do a new cool feature, take the time to think of it, make it clear in your head, structure it, make it work in your head before to go on the keyboard.

1.2 Take time to name your variable correctly
- Variable name is really important, if you have trouble to name your variable, the step 1.1 have may not have been done correctly. If you pass 10 minutes searching for a variable name, the feature currently developed may not have been enough defined and think... There is still probably still some algorithm not defined. If you have trouble to define your algorithm, imagine the guy that will read this after you!

xRatePerSec = gyroXRawData * gyroScaleFactor;

speak to me more than

xRPS = gD + gS;

I’m sure that I will figure out the seccond one, but, I will have to analyze all member to know exaclty what they are, but, I don’t have to do it if name are clearly named.

1.3 Respect indentation and bracing
- I know that we can write code really cryptic way, but, it’s not because it’s code that it have to be cryptic, it’s not because it’s “C” code that it have to be cryptic and mostly, it’s not because it’s microcontroller code that it have to be cryptic. Sometime, that can look like it because we use some register value... but in those case, we don’t have choice. Otherwise no cryptic code. Avoid

if (a > b) c++; else d++;

and worst when many one after the others

if (a > b) c++; else d++;
if (g > h) i++; else j++;

So, first the name are bad, but, I have discussed that case. So, just indent it and put some brace will put some air in the code and make the it more readable and easy for the eye to pass through it.

if (a > b) {
  c++;
}
else {
  d++
}  

I also indent precompile header so that I can read it like it is normal code

 processThing();
  #if defined (monDefine)
    readMyStuff();
  #endif

1.4 I’m agaist comment
- My rules of thumb is simple, if I have to comment, it’s because my code is not easily readable. I’m not completely against it, but, for really complicated stuff, but, one thing's sure, most of the time, programmer comment their code because they were not able to make the code to document itself. This one is more a personal matter and can be discussed, but... I will push for it

Also, please note that most of the time, people that will look after what you have done, will probably catch more faster and see some improvement. Not because you are slow or something like this, but, because they will catch your intention more faster because it have been already defined by you. So, don’t be offended if I see a useless condition, way to simplify your algorithm but, that will do the same thing or if I see that you just change change thing but, that do nothing at all.

For all those point, I may ask you to rework a bit and update your commit range. Be sure we all have the same goal, the improvement of Aeroquad. So, i’m not there to refuse anything but to filter as much as I can. And it's your duty, as developers to fit the code style of the project. Like you would have to do if you were programming into a company!

[Kenny9999]

2. CODE STRUCTURE
- The structure of the code is one of the thing that is the most important. This will determine the maintainability of the software as well as the flexibility. When I came to quad, what make me choose the Aeroquad was the Object Oridented Design (that have been remove for footprint reason, but, we still keep the structure so it will be easy to put back) and also the architecture of the software.



2.1 The Aeroquad Sketch
- The Aeroquad sketch, contain the craft definition, basic flight control algorithm, receiver selection, and feature selection.

2.1.1 Craft definition is your user selection, platform v2, v2.1 mini paired with motor control selection like quad, hex, Y6, and the receiver used.

2.1.2 Basic flight control algorithm, Core of the craft, always defined, that will read the receiver, read basic sensors (gyro and accel) compute the attitude and control the motors depending of the receiver input.

So, the basic craft will always have that, a Reveiver, a platform definition that will determine the sensors (gyro and accel) used, and a motor control selection

2.1.3 Feature selection. Feature selection depends on the platform you have and on you if you want to enable or not that feature, so, Heading hold, Altitude hold, GPS OSD, etc.

So, in the core code, in the main sketch, all code overthere is generic, what I mean is that it do not care about the receiver used, it do not care about the motor configuration you selected nor the platform selected. all those are defined at the init time and it used it. what ever is the accel, the receiver, and so on. So, if you develop something, don’t add a #define for a specific device, like receiver, or accel or a platform specific led processing in the loop executive. All #define in the main sketch are related to feature used like altitude hold, heading hold and so on. So, if you had a device specific #define over there, I won’t let it pass because it’s not it’s place. No problem for a new feature add. Like Heading hold or battery monitor.

Also don’t develop a side feature, like a new way to handle the heading hold or altitude hold! We want just one way to do thing, and it have to be the best way to do it. So, if you have an improvement on a feature, instead of doing it twice and keep the old code, don’t be afraid to remove the old code and use your. Be confident.

========= Edition add on 5/12/2012 ==========
With the new Baloo and it's process power, I recently saw some commit to speed up some process on the Baloo, Although the Baloo have the processing power, the code base is designed to be generic and run on all current supported platform. I have remove all platform specific if def in the code before v3, and the reason was to standardize the processing for all platform. Do not, in any case, add a platform specific if def in the loop executive. Having a platform specific if def in the loop executive is exactly the same as having a separate code base for that platform, even if it's just 3 line of code. And then, we have a different behavior for a feature depending of the platform. This require some energy to maintain, explanation for new comer, and is quite bug prone cause we may have to debug one or the other, and if we fix one, we have to thing to fix the other if it have the same bug. So, don't forget that you are not alone with your hardware, if you want a different behavior for a feature and can make it, keep it for yourself. I will avoid at all cost feature specific behavior that depend of a platform definition.
===============================

2.2 Libraries
- the libraries folder contains all specific algorithm and mostly sensors that the core sketch use. So, you had a new sensors like an accel, be sure it respect the current API like other Accel. etc.

3. KISS
- Keep your code as simple as possible and if you thing it’s not possible, I will prove you wrong There is many way to keep thing simple and if you have thing correctly on how thing should interact together, it will be simple. Split your code and file by theme. Be sure to have “Object” that do one and only one thing. Be sure to have function that do one and only one thing. etc.

4. WE SUPPORT MANY PLATFORM
- If you develop a new feature. Keep in mind that we have about 10 platform to support and that the feature you are working on should work on other platform as well. OR, don’t forget to add the #undef of your feature in platform definition section if you know that this new feature can’t be supported on that platform. This have to be kept in mind when you develop.

5. MEMORY FOOTPRINT
- We are already short on the UNO really short. and also on the Mini, so, thing that if you had some generic code, like some new init for Serial, or something else, may be that code will have to fit in the UNO and mini. This have to be kept in mind when you develop.

6. CPU INTENSIVE PROCESS
- Since the Baloo is not there yet, we have to be real cautious on cpu cycle we use. Ask yourself if the current feature developed can run more slowly, try to be optimal in your usage of the processor. More you take some processor time, less accel and gyro are sampled, less gyro and accel are sampled, less quality data is passed to the kinematics, and this can lead to some more lean if your craft have some vibrations. This have to be kept in mind when you develop.

7. TEST, FLIGHT TEST, and re-flight test
- Please, please, please, test your change, code, improvement. Make sure it work. Make sure you did not broke any other feature compilation. Flight test your code... and 10 min of hovering test is not a concluent test. Are you really sure it work? If you flight with or without, with the previous vs the new, have you improve thing or make it worst? One thing that can happen quickly is a flight behavior degradation and I won’t catch it just at looking to the code most of the time... So, be sure

8. HAVE FUN
- Developing new feature, code, improvement, participating to the community is fun. If I ask you to do some change, don’t take it personally but see it as a way to improve our soft.

9. DON’T BE AFRAID TO CHALLENGE ME.
- Unfortunately... I don’t know everything... Sometime, I won’t see the benefice of your work right away, or I may see a danger to something else. So, I may reject or ask for some modification. If you think I’m wrong, tell me, but, don’t tell me “your wrong” without arguments. So, make me see what I miss or make me realize that there is no danger. I will challenge you to make sure that you are right and you can challenge me to prove me that you are right.

[Mikro]

Thank you for posting this Kenny9999, it's an excellent description of the coding standards we've setup. Being diligent in these things will insure the longevity and maintainability of our project well into the future.

[Honk]

I have not read everything here yet (just looked through briefly) but I have to say that initially we come from different worlds entirely. For software only development, I might agree with you on the "self documenting code", but in the hardware world which we are working with, there's no such thing. At least not with all parts of the code. For example the hardware timer section we utilize. That is all but self documenting, it only consists of arbitrarily named registers specific to the ATmega series controllers and reg's. Then we have all the external stuff we connect to the controllers.

[koko76]

Preface with saying that I understand my opion counts for squat, but I for one appreciate commented code. For those of us who do not code for a living, but are interested in a deeper understanding of what is going on comments are helpful.
Comments don't cost you performance or sketch space. Why not try and help more folks have an understanding?

[xtrmtrk]

I agree that code should be written well enough to explain the details of what's going on in a stand-alone manner. What I always find more helpful in comments is intent - just what is this routine or section intending to do, and maybe why.

Also, the more lower-level software is, the smaller the audience of readers who have any experience with it. Lots of us know our way around C & C++. But by the time you're banging registers and setting up interrupt vectors you've probably lost 95% of your readers - they just don't have the background to understand the context. There I think you really need to help guide them and explain what you're doing in comments. Code like

DDRK = 0;
  PORTK = 0;
  PCMSK2 |=(1<<lastReceiverChannel)-1;
  PCICR |= 0x1 << 2;

Leaves a lot of people in the dust.

Just my very biased opinion.

[Mikro]

If we read through the existing code there is plenty of comments written in by Kenny9999. He just wrote Section 1.4 in a "black and white" way, but of course we're going to have some comments in the code. Most definitely there should be comments or descriptive #define named variables for register level programming (Here's a good example).

What he's really trying to emphasize, is that you should always be thinking to write your code in a way that it's understandable just be reading through the code. He cited examples where you can take some shortcuts in C, so instead of using that and writing comments about what it does. Take the time to structure it with descriptive variable names and follow the coding standards which have also been posted here for awhile: http://aeroquad.com/showwiki.php?tit...ding+Standards. Also if you think the logic you are trying to code shouldn't be that complicated, but you are finding that you need to write comments for your fellow developer to understand, try to take a second look at how it can be architected easier. If you HAVE to comment it, that's fine, but make a conscience effort to write it as simply and plainly as possible.

So our philosophy should be to write the code as simple and straightforward as possible, such that you can look at it six months from now and it makes QUICK sense to you and your fellow developer. I think just having this in the forefront of our minds will encourage us to have source code that is maintainable for the years to come. Let's build something that will last, and something the next generation of AeroQuad developers can improve upon and take flight capabilities to the next level.

One last word... writing code this way will sometimes result in inefficient executing code. At that time we really need to have some debate on how to handle it, sometimes performance will win over readabilty and vice versa. There definitely is still room for discussion, so we should feel free and open to challenge one another in a healthy discussion to make sure our product is as best as possible.

[p0lar]

Hehe... spoken like a dev and a doc guy in the great loop of code documentation.. chuckles to myself......

[koko76]

If we read through the existing code there is plenty of comments written in by Kenny9999. He just wrote Section 1.4 in a "black and white" way, but of course we're going to have some comments in the code. Most definitely there should be comments or descriptive #define named variables for register level programming (Here's a good example).

What he's really trying to emphasize, is that you should always be thinking to write your code in a way that it's understandable just be reading through the code. He cited examples where you can take some shortcuts in C, so instead of using that and writing comments about what it does. Take the time to structure it with descriptive variable names and follow the coding standards which have also been posted here for awhile: http://aeroquad.com/showwiki.php?tit...ding+Standards. Also if you think the logic you are trying to code shouldn't be that complicated, but you are finding that you need to write comments for your fellow developer to understand, try to take a second look at how it can be architected easier. If you HAVE to comment it, that's fine, but make a conscience effort to write it as simply and plainly as possible.

So our philosophy should be to write the code as simple and straightforward as possible, such that you can look at it six months from now and it makes QUICK sense to you and your fellow developer. I think just having this in the forefront of our minds will encourage us to have source code that is maintainable for the years to come. Let's build something that will last, and something the next generation of AeroQuad developers can improve upon and take flight capabilities to the next level.

One last word... writing code this way will sometimes result in inefficient executing code. At that time we really need to have some debate on how to handle it, sometimes performance will win over readabilty and vice versa. There definitely is still room for discussion, so we should feel free and open to challenge one another in a healthy discussion to make sure our product is as best as possible.

Again with all due respect. Yours is the perspective of someone who already understands, there are people who want to learn and a project like this is their motivation. Certain things help us. Kennys comments, although understandable from his perspective seem dismissive of people like I describe. It may not matter, but more eyes find more bugs. No one says you have to comment your code, but why actively discourage it?
I know a many things about mechanical construction that are OBVIOUS to me but not to many others. I see errors in many peoples construction that are OBVIOUS. I could not comment on my build reports too, which is exactly what you are describing here. It helps no one for me not to comment. It even helps me in explaining it. There is no difference between the two.

[Mikro]

It may not matter, but more eyes find more bugs. No one says you have to comment your code, but why actively discourage it?

Well noted, let's pay attention to this as we move forward, there's currently plenty of comments in the current code, so we may just need to reword what we want out of this philosophy. I don't necessarily see it as discouraging comments, but rather do code development to be as straightforward as possible with descriptive variables and straight architecture choices.