From 916d2d0f5243c0e34dbc3bfeb98f4b6861a956e2 Mon Sep 17 00:00:00 2001 From: Joel Bodenmann Date: Mon, 6 Jan 2014 01:20:09 +0100 Subject: [PATCH 1/6] Removed codingstyle.txt as nearly every rule in there was ignored --- codingstyle.txt | 289 +----------------------------------------------- 1 file changed, 1 insertion(+), 288 deletions(-) diff --git a/codingstyle.txt b/codingstyle.txt index ceab047c..d2c1c3dc 100644 --- a/codingstyle.txt +++ b/codingstyle.txt @@ -1,289 +1,2 @@ - ChibiOS/GFX coding style +ToDo -To provide an easy-to-read code, we want to have a uniform -coding style within ChibiOS/GFX. -Because I personally like the widley used linux kernel coding style, -I decided to use it for ChibiOS/GFX as well. -Therefore, the coding style documentation is a 1:1 copy from the -codingstyle.txt of the linux kernel source code. - -Please make sure you match these coding styles before you contribute -any code. If you find any existing code which dosen't match these rules, -please feel free to submit a patch. - -There are only two rules which are not similar to the following -documentation: - - - Prefered tabsize is 4, not 8 - - We don't use 80 character columns - - -Please read through the following carefully: - - - - Linux kernel coding style - -This is a short document describing the preferred coding style for the -linux kernel. Coding style is very personal, and I won't _force_ my -views on anybody, but this is what goes for anything that I have to be -able to maintain, and I'd prefer it for most other things too. Please -at least consider the points made here. - -First off, I'd suggest printing out a copy of the GNU coding standards, -and NOT read it. Burn them, it's a great symbolic gesture. - -Anyway, here goes: - - - Chapter 1: Indentation - -Tabs are 8 characters, and thus indentations are also 8 characters. -There are heretic movements that try to make indentations 4 (or even 2!) -characters deep, and that is akin to trying to define the value of PI to -be 3. - -Rationale: The whole idea behind indentation is to clearly define where -a block of control starts and ends. Especially when you've been looking -at your screen for 20 straight hours, you'll find it a lot easier to see -how the indentation works if you have large indentations. - -Now, some people will claim that having 8-character indentations makes -the code move too far to the right, and makes it hard to read on a -80-character terminal screen. The answer to that is that if you need -more than 3 levels of indentation, you're screwed anyway, and should fix -your program. - -In short, 8-char indents make things easier to read, and have the added -benefit of warning you when you're nesting your functions too deep. -Heed that warning. - - - Chapter 2: Placing Braces - -The other issue that always comes up in C styling is the placement of -braces. Unlike the indent size, there are few technical reasons to -choose one placement strategy over the other, but the preferred way, as -shown to us by the prophets Kernighan and Ritchie, is to put the opening -brace last on the line, and put the closing brace first, thusly: - - if (x is true) { - we do y - } - -However, there is one special case, namely functions: they have the -opening brace at the beginning of the next line, thus: - - int function(int x) - { - body of function - } - -Heretic people all over the world have claimed that this inconsistency -is ... well ... inconsistent, but all right-thinking people know that -(a) K&R are _right_ and (b) K&R are right. Besides, functions are -special anyway (you can't nest them in C). - -Note that the closing brace is empty on a line of its own, _except_ in -the cases where it is followed by a continuation of the same statement, -ie a "while" in a do-statement or an "else" in an if-statement, like -this: - - do { - body of do-loop - } while (condition); - -and - - if (x == y) { - .. - } else if (x > y) { - ... - } else { - .... - } - -Rationale: K&R. - -Also, note that this brace-placement also minimizes the number of empty -(or almost empty) lines, without any loss of readability. Thus, as the -supply of new-lines on your screen is not a renewable resource (think -25-line terminal screens here), you have more empty lines to put -comments on. - - - Chapter 3: Naming - -C is a Spartan language, and so should your naming be. Unlike Modula-2 -and Pascal programmers, C programmers do not use cute names like -ThisVariableIsATemporaryCounter. A C programmer would call that -variable "tmp", which is much easier to write, and not the least more -difficult to understand. - -HOWEVER, while mixed-case names are frowned upon, descriptive names for -global variables are a must. To call a global function "foo" is a -shooting offense. - -GLOBAL variables (to be used only if you _really_ need them) need to -have descriptive names, as do global functions. If you have a function -that counts the number of active users, you should call that -"count_active_users()" or similar, you should _not_ call it "cntusr()". - -Encoding the type of a function into the name (so-called Hungarian -notation) is brain damaged - the compiler knows the types anyway and can -check those, and it only confuses the programmer. No wonder MicroSoft -makes buggy programs. - -LOCAL variable names should be short, and to the point. If you have -some random integer loop counter, it should probably be called "i". -Calling it "loop_counter" is non-productive, if there is no chance of it -being mis-understood. Similarly, "tmp" can be just about any type of -variable that is used to hold a temporary value. - -If you are afraid to mix up your local variable names, you have another -problem, which is called the function-growth-hormone-imbalance syndrome. -See next chapter. - - - Chapter 4: Functions - -Functions should be short and sweet, and do just one thing. They should -fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, -as we all know), and do one thing and do that well. - -The maximum length of a function is inversely proportional to the -complexity and indentation level of that function. So, if you have a -conceptually simple function that is just one long (but simple) -case-statement, where you have to do lots of small things for a lot of -different cases, it's OK to have a longer function. - -However, if you have a complex function, and you suspect that a -less-than-gifted first-year high-school student might not even -understand what the function is all about, you should adhere to the -maximum limits all the more closely. Use helper functions with -descriptive names (you can ask the compiler to in-line them if you think -it's performance-critical, and it will probably do a better job of it -that you would have done). - -Another measure of the function is the number of local variables. They -shouldn't exceed 5-10, or you're doing something wrong. Re-think the -function, and split it into smaller pieces. A human brain can -generally easily keep track of about 7 different things, anything more -and it gets confused. You know you're brilliant, but maybe you'd like -to understand what you did 2 weeks from now. - - - Chapter 5: Commenting - -Comments are good, but there is also a danger of over-commenting. NEVER -try to explain HOW your code works in a comment: it's much better to -write the code so that the _working_ is obvious, and it's a waste of -time to explain badly written code. - -Generally, you want your comments to tell WHAT your code does, not HOW. -Also, try to avoid putting comments inside a function body: if the -function is so complex that you need to separately comment parts of it, -you should probably go back to chapter 4 for a while. You can make -small comments to note or warn about something particularly clever (or -ugly), but try to avoid excess. Instead, put the comments at the head -of the function, telling people what it does, and possibly WHY it does -it. - - - Chapter 6: You've made a mess of it - -That's OK, we all do. You've probably been told by your long-time Unix -user helper that "GNU emacs" automatically formats the C sources for -you, and you've noticed that yes, it does do that, but the defaults it -uses are less than desirable (in fact, they are worse than random -typing - a infinite number of monkeys typing into GNU emacs would never -make a good program). - -So, you can either get rid of GNU emacs, or change it to use saner -values. To do the latter, you can stick the following in your .emacs file: - -(defun linux-c-mode () - "C mode with adjusted defaults for use with the Linux kernel." - (interactive) - (c-mode) - (c-set-style "K&R") - (setq c-basic-offset 8)) - -This will define the M-x linux-c-mode command. When hacking on a -module, if you put the string -*- linux-c -*- somewhere on the first -two lines, this mode will be automatically invoked. Also, you may want -to add - -(setq auto-mode-alist (cons '("/usr/src/linux.*/.*\\.[ch]$" . linux-c-mode) - auto-mode-alist)) - -to your .emacs file if you want to have linux-c-mode switched on -automagically when you edit source files under /usr/src/linux. - -But even if you fail in getting emacs to do sane formatting, not -everything is lost: use "indent". - -Now, again, GNU indent has the same brain dead settings that GNU emacs -has, which is why you need to give it a few command line options. -However, that's not too bad, because even the makers of GNU indent -recognize the authority of K&R (the GNU people aren't evil, they are -just severely misguided in this matter), so you just give indent the -options "-kr -i8" (stands for "K&R, 8 character indents"). - -"indent" has a lot of options, and especially when it comes to comment -re-formatting you may want to take a look at the manual page. But -remember: "indent" is not a fix for bad programming. - - - Chapter 7: Configuration-files - -For configuration options (arch/xxx/config.in, and all the Config.in files), -somewhat different indentation is used. - -An indention level of 3 is used in the code, while the text in the config- -options should have an indention-level of 2 to indicate dependencies. The -latter only applies to bool/tristate options. For other options, just use -common sense. An example: - -if [ "$CONFIG_EXPERIMENTAL" = "y" ]; then - tristate 'Apply nitroglycerine inside the keyboard (DANGEROUS)' CONFIG_BOOM - if [ "$CONFIG_BOOM" != "n" ]; then - bool ' Output nice messages when you explode' CONFIG_CHEER - fi -fi - -Generally, CONFIG_EXPERIMENTAL should surround all options not considered -stable. All options that are known to trash data (experimental write- -support for file-systems, for instance) should be denoted (DANGEROUS), other -Experimental options should be denoted (EXPERIMENTAL). - - - Chapter 8: Data structures - -Data structures that have visibility outside the single-threaded -environment they are created and destroyed in should always have -reference counts. In the kernel, garbage collection doesn't exist (and -outside the kernel garbage collection is slow and inefficient), which -means that you absolutely _have_ to reference count all your uses. - -Reference counting means that you can avoid locking, and allows multiple -users to have access to the data structure in parallel - and not having -to worry about the structure suddenly going away from under them just -because they slept or did something else for a while. - -Note that locking is _not_ a replacement for reference counting. -Locking is used to keep data structures coherent, while reference -counting is a memory management technique. Usually both are needed, and -they are not to be confused with each other. - -Many data structures can indeed have two levels of reference counting, -when there are users of different "classes". The subclass count counts -the number of subclass users, and decrements the global count just once -when the subclass count goes to zero. - -Examples of this kind of "multi-reference-counting" can be found in -memory management ("struct mm_struct": mm_users and mm_count), and in -filesystem code ("struct super_block": s_count and s_active). - -Remember: if another thread can find your data structure, and you don't -have a reference count on it, you almost certainly have a bug. From 192335ea3595a2c162857a6a1eebb672f82bf180 Mon Sep 17 00:00:00 2001 From: Joel Bodenmann Date: Mon, 6 Jan 2014 21:54:13 +0100 Subject: [PATCH 2/6] doc --- include/gdisp/gdisp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/gdisp/gdisp.h b/include/gdisp/gdisp.h index 8f379bfd..d1d78145 100644 --- a/include/gdisp/gdisp.h +++ b/include/gdisp/gdisp.h @@ -681,8 +681,8 @@ void gdispGDrawBox(GDisplay *g, coord_t x, coord_t y, coord_t cx, coord_t cy, co * * @param[in] g The display to use * @param[in] x,y The position for the text - * @param[in] font The font to use * @param[in] str The string to draw + * @param[in] font The font to use * @param[in] color The color to use * * @api From ac98de6b6934dffeee87d2cd6329d3504d2332d4 Mon Sep 17 00:00:00 2001 From: Joel Bodenmann Date: Mon, 6 Jan 2014 22:06:36 +0100 Subject: [PATCH 3/6] added gwinGetColor() and gwinGetBgColor() --- include/gwin/gwin.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/include/gwin/gwin.h b/include/gwin/gwin.h index 96055376..e964bc3a 100644 --- a/include/gwin/gwin.h +++ b/include/gwin/gwin.h @@ -275,6 +275,24 @@ extern "C" { */ #define gwinSetBgColor(gh, bgclr) (gh)->bgcolor = (bgclr) + /** + * @brief Get the foreground color of a window + * + * @param[in] gh The window + * + * @api + */ + #define gwinGetColor(gh) (gh)->color + + /** + * @brief Get the background color of a window + * + * @param[in] gh The window + * + * @api + */ + #define gwinGetBgColor(gh) (gh)->bgcolor + /** * @brief Sets whether a window is visible or not * From d30b8288ad38adb78726d5f8c064b3954619009c Mon Sep 17 00:00:00 2001 From: Joel Bodenmann Date: Tue, 7 Jan 2014 08:19:39 +0100 Subject: [PATCH 4/6] doc --- releases.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/releases.txt b/releases.txt index 3b0f617f..505bc763 100644 --- a/releases.txt +++ b/releases.txt @@ -9,6 +9,8 @@ FEATURE: Added progressbar widget FEATURE: Added gdispGDrawThickLine() by user jpa- DEPRECATE: TDISP module removed FIX: Console does not execute gwinPrintf() anymore if not visible +FEATURE: Added gwinGetColor() and gwinGetBgColor() +FEATURE: Console does now have an optional buffer (GWIN_CONSOLE_USE_HISTORY) *** changes after 1.9 *** From b0adc0bed3d03b36553ddf245a0f568cae61bc89 Mon Sep 17 00:00:00 2001 From: inmarket Date: Thu, 9 Jan 2014 08:36:25 +1000 Subject: [PATCH 5/6] New codingstyle.txt --- codingstyle.txt | 162 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 161 insertions(+), 1 deletion(-) diff --git a/codingstyle.txt b/codingstyle.txt index d2c1c3dc..5a397aeb 100644 --- a/codingstyle.txt +++ b/codingstyle.txt @@ -1,2 +1,162 @@ -ToDo + uGFX coding style + +This is a short document describing the preferred coding style for uGFX. + + Chapter 1: Indentation + +Tabs are 4 characters, and thus indentations are also 4 characters. + +Rationale: We like 4 character tabs much better than 8 character tabs. +It is more readable. + + Chapter 2: Placing Braces + +The preferred way, as shown to us by the prophets Kernighan and Ritchie, +is to put the opening brace last on the line, and put the closing brace first, +thusly: + + if (x is true) { + we do y + } + +However, there is one special case, namely functions: they have the +opening brace at the beginning of the next line, thus: + + int function(int x) + { + body of function + } + +We will however accept braces in the general block style for functions +but not the other way around. General blocks MUST have their opening brace +on the same line as the conditional statement. + +Note that the closing brace is empty on a line of its own, _except_ in +the cases where it is followed by a continuation of the same statement, +ie a "while" in a do-statement or an "else" in an if-statement, like +this: + + do { + body of do-loop + } while (condition); + +and + + if (x == y) { + .. + } else if (x > y) { + ... + } else { + .... + } + +Note that closing brace is indented to the level of the start of the block. +Structure definitions are an optional exception. Both of the below style are +acceptable: + + typedef struct { + int a; + ... + } mytype; + + struct mystruct { + int a; + ... + } + + Chapter 3: Naming + +C is a Spartan language, and so should your naming be. Unlike Modula-2 +and Pascal programmers, C programmers do not use cute names like +ThisVariableIsATemporaryCounter. A C programmer would call that +variable "tmp", which is much easier to write, and a lot less +difficult to understand. + +HOWEVER, while long mixed-case names are frowned upon, descriptive names for +global variables are a must. To call a global function "foo" is a +shooting offense. + +GLOBAL variables (to be used only if you _really_ need them) need to +have descriptive names, as do global functions. If you have a function +that counts the number of active users, you should call that +"countActiveUsers()" or similar, you should _not_ call it "cntusr()". + +WHERE long names are required as described above, we prefer the use of +capitalisation on subsequent words (but not the first) rather than underscores +to seperate the words. For example "countActiveUsers()" is preferred to +"count_active_users()" as it is at least as readable and is shorter. + +Encoding the type of a function into the name (so-called Hungarian +notation) is brain damaged - the compiler knows the types anyway and can +check those, and it only confuses the programmer. + +LOCAL variable names should be short, and to the point. If you have +some random integer loop counter, it should probably be called "i". +Calling it "loopCounter" is non-productive, if there is no chance of it +being mis-understood. Similarly, "tmp" can be just about any type of +variable that is used to hold a temporary value. + + Chapter 4: Functions + +Functions should be short and sweet, and do just one thing. + +The maximum length of a function is inversely proportional to the +complexity and indentation level of that function. So, if you have a +conceptually simple function that is just one long (but simple) +case-statement, where you have to do lots of small things for a lot of +different cases, it's OK to have a longer function. + +However, if you have a complex function, and you suspect that a +less-than-gifted first-year high-school student might not even +understand what the function is all about, you should adhere to the +maximum limits all the more closely. Use helper functions with +descriptive names (you can ask the compiler to in-line them if you think +it's performance-critical, and it will probably do a better job of it +that you would have done). + +Another measure of the function is the number of local variables. They +shouldn't exceed 5-10, or you're possibly doing something wrong. Re-think the +function, and split it into smaller pieces. A human brain can +generally easily keep track of about 7 different things, anything more +and it gets confused. You need to understand what you did 2 weeks from now. + +Because uGFX is intended for embedded platforms there are other considerations +that may cause exceptions or emphasise the above. For example, stack space is +a premium. This means that the number of local variables should be minimised as +should the number of parameters. Passing through multiple levels of functions +with lots of parameters is very bad indeed and this can override the desire to +keep functions short and sweet. Clarity however is still essential. + + + Chapter 5: Commenting + +Comments are good, but there is also a danger of over-commenting. NEVER +try to explain HOW your code works in a comment: it's much better to +write the code so that the _working_ is obvious, and it's a waste of +time to explain badly written code. Generally, you want your comments to tell +WHAT your code does, not HOW. + +We use doxygen to document the system. That means that most public functions +are documented in the header defintion file. We do not put doxygen comments in +the source file itself. + +Within the source file, comments should be used to seperate blocks of functions +or definitions within the file. This is to provide clarity to the structure of +the source file itself. An example could be: + /*************************** + * Drawing Functions + ***************************/ + +Single line comments using "//" to start the comment should be used for just that +purpose, to assist in the understanding of that single line. Mutliple single line +comments should never be used to create a block comment. For example, + // This is a very long + // comment spanning several + // lines +is a very bad use of comments. + +Comments within function bodies should be small comments to note or warn +about something particularly clever (or ugly), but try to avoid excess. +Instead, put the comments at the head of a block of code to explain the block +rather than a comment on each line. From 1791bec3483a27843dd279c51023526ced836aa3 Mon Sep 17 00:00:00 2001 From: inmarket Date: Thu, 9 Jan 2014 08:39:08 +1000 Subject: [PATCH 6/6] doc --- releases.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/releases.txt b/releases.txt index 505bc763..dc896f61 100644 --- a/releases.txt +++ b/releases.txt @@ -11,6 +11,7 @@ DEPRECATE: TDISP module removed FIX: Console does not execute gwinPrintf() anymore if not visible FEATURE: Added gwinGetColor() and gwinGetBgColor() FEATURE: Console does now have an optional buffer (GWIN_CONSOLE_USE_HISTORY) +FIX: Updated codingstyle.txt *** changes after 1.9 ***