Automatische Code Formatierung

Edit von @biologist am 05.11.2023:

  • Wir haben uns final für clang-format entschieden.
  • Wie man das einrichtet ist weiter unten beschrieben.

Hallo,
Ich ziehe das Thema aus Zugangsdaten mehrerer Wlans speichern heraus. Hintergrund, ich habe einige Code Format Tools für VSCode (bzw für C und C++) angeschaut und wollte euch die mal vorstellen, bzw Inputs holen.
Bei der Auswahl habe ich mich in erster Linie auf Tools gestürzt, die ich selbst kenne.

Diese Punkte wären ein automatisches No-Go für ein Tool:

  1. Es muss von VSCode Native order per Plugin unterstützt sein, oder sich in Platformio / Github als Build integrieren lassen
  2. Es muss Binaries / Sources für die gängisten Betriebsysteme geben (Windows, Mac: Binary, Linux: min. Source)
  3. Die Konfiguration muss über ein Config-File im Projektordner erfolgen (können). Ohne dem müsste jeder alle Einstellungen manuell machen → No-Go
  4. Die Config-File muss Text-Basiert sein, damit Git es erkennen und aufnehmen kann.

Als Tool für die Erstellung der Beispiele weiter unten habe ich UniversalIndentGUI gefunden und damit herumgespielt gehabt.


C-Code

Original Code der für alle Formatter verwendet wurde
#include <Arduino.h>
#include "settings.h"
#include "Log.h"
#include "Button.h"
#include "Cmd.h"
#include "Port.h"
#include "System.h"

bool gButtonInitComplete = false;

// Only enable those buttons that are not disabled (99 or >115)
// 0 -> 39: GPIOs
// 100 -> 115: Port-expander
#if (NEXT_BUTTON >= 0 && NEXT_BUTTON <= MAX_GPIO)
	#define BUTTON_0_ENABLE
#elif (NEXT_BUTTON >= 100 && NEXT_BUTTON <= 115)
	#define EXPANDER_0_ENABLE
#endif

class z {
    public:
	z() { }
	void doStuff(int z) { return l; }
};

t_button gButtons[7];         // next + prev + pplay + rotEnc + button4 + button5 + dummy-button
uint8_t gShutdownButton = 99; // Helper used for Neopixel: stores button-number of shutdown-button
uint16_t gLongPressTime = 0;

#ifdef PORT_EXPANDER_ENABLE
	extern bool Port_AllowReadFromPortExpander;
#endif

static volatile SemaphoreHandle_t Button_TimerSemaphore;

void Button_Init() {
	#if (WAKEUP_BUTTON >= 0 && WAKEUP_BUTTON <= MAX_GPIO)
		if (ESP_ERR_INVALID_ARG == esp_sleep_enable_ext0_wakeup((gpio_num_t)WAKEUP_BUTTON, 0)) {
			snprintf(Log_Buffer, Log_BufferLength, "%s (GPIO: %u)", (char *) FPSTR(wrongWakeUpGpio), WAKEUP_BUTTON);
			Log_Println(Log_Buffer, LOGLEVEL_ERROR);
		}
	#endif
    
    	if (x) {
		y = z;
	}

	switch (a) {
		case b:
			break;
	}

	for (int i = 0; i < 5; i++) {
		i--;
	}

	#ifdef NEOPIXEL_ENABLE // Try to find button that is used for shutdown via longpress-action (only necessary for Neopixel)
		#if defined(BUTTON_0_ENABLE) || defined(EXPANDER_0_ENABLE)
			#if (BUTTON_0_LONG == CMD_SLEEPMODE)
				gShutdownButton = 0;
			#endif
		#endif
		#if defined(BUTTON_1_ENABLE) || defined(EXPANDER_1_ENABLE)
			#if (BUTTON_1_LONG == CMD_SLEEPMODE)
				gShutdownButton = 1;
			#endif
		#endif
		#if defined(BUTTON_2_ENABLE) || defined(EXPANDER_2_ENABLE)
			#if (BUTTON_2_LONG == CMD_SLEEPMODE)
				gShutdownButton = 2;
			#endif
		#endif
		#if defined(BUTTON_3_ENABLE) || defined(EXPANDER_3_ENABLE)
			#if (BUTTON_3_LONG == CMD_SLEEPMODE)
				gShutdownButton = 3;
			#endif
		#endif
		#if defined(BUTTON_4_ENABLE) || defined(EXPANDER_4_ENABLE)
			#if (BUTTON_4_LONG == CMD_SLEEPMODE)
				gShutdownButton = 4;
			#endif
		#endif
		#if defined(BUTTON_5_ENABLE) || defined(EXPANDER_5_ENABLE)
			#if (BUTTON_5_LONG == CMD_SLEEPMODE)
				gShutdownButton = 5;
			#endif
		#endif
	#endif
}

Clang-Format

Teil vom LLVM und wohl der Klassiker unter den Code Format Tools. Hat viele Einstellungsmöglichkeiten und wird von VSCode (bzw dem C++ Plugin) nativ unterstützt. Großes Manko: Hasst den Präprozessor

.clang-format
---
BasedOnStyle: WebKit
AlignArrayOfStructures: Right
AllowShortEnumsOnASingleLine: false
AllowShortFunctionsOnASingleLine: InlineOnly
BreakArrays: false
Cpp11BracedListStyle: true
FixNamespaceComments: true
IncludeBlocks: Regroup
IncludeCategories:
  - Regex: Arduino.h
    Priority: -1
    CaseSensitive: true
    SortPriority: -1
  - Regex: ^"
    Priority: 1
  - Regex: .*
    Priority: 4
IncludeIsMainRegex: (_test)?$
IndentCaseLabels: true
IndentPPDirectives: BeforeHash
IndentWidth: 8
InsertBraces: true
NamespaceIndentation: None
PointerAlignment: Right
SpaceAfterCStyleCast: true
UseTab: Always

Ergebnis
#include <Arduino.h>

#include "Button.h"
#include "Cmd.h"
#include "Log.h"
#include "Port.h"
#include "System.h"
#include "settings.h"

bool gButtonInitComplete = false;

// Only enable those buttons that are not disabled (99 or >115)
// 0 -> 39: GPIOs
// 100 -> 115: Port-expander
#if (NEXT_BUTTON >= 0 && NEXT_BUTTON <= MAX_GPIO)
	#define BUTTON_0_ENABLE
#elif (NEXT_BUTTON >= 100 && NEXT_BUTTON <= 115)
	#define EXPANDER_0_ENABLE
#endif

class z {
    public:
	z() { }
	void doStuff(int z) { return l; }
};

t_button gButtons[7]; // next + prev + pplay + rotEnc + button4 + button5 + dummy-button
uint8_t gShutdownButton = 99; // Helper used for Neopixel: stores button-number of shutdown-button
uint16_t gLongPressTime = 0;

#ifdef PORT_EXPANDER_ENABLE
extern bool Port_AllowReadFromPortExpander;
#endif

static volatile SemaphoreHandle_t Button_TimerSemaphore;

void Button_Init()
{
#if (WAKEUP_BUTTON >= 0 && WAKEUP_BUTTON <= MAX_GPIO)
	if (ESP_ERR_INVALID_ARG == esp_sleep_enable_ext0_wakeup((gpio_num_t) WAKEUP_BUTTON, 0)) {
		snprintf(Log_Buffer, Log_BufferLength, "%s (GPIO: %u)", (char *) FPSTR(wrongWakeUpGpio), WAKEUP_BUTTON);
		Log_Println(Log_Buffer, LOGLEVEL_ERROR);
	}
#endif

	if (x) {
		y = z;
	}

	switch (a) {
		case b:
			break;
	}

	for (int i = 0; i < 5; i++) {
		i--;
	}

#ifdef NEOPIXEL_ENABLE // Try to find button that is used for shutdown via longpress-action (only necessary for Neopixel)
	#if defined(BUTTON_0_ENABLE) || defined(EXPANDER_0_ENABLE)
		#if (BUTTON_0_LONG == CMD_SLEEPMODE)
	gShutdownButton = 0;
		#endif
	#endif
	#if defined(BUTTON_1_ENABLE) || defined(EXPANDER_1_ENABLE)
		#if (BUTTON_1_LONG == CMD_SLEEPMODE)
	gShutdownButton = 1;
		#endif
	#endif
	#if defined(BUTTON_2_ENABLE) || defined(EXPANDER_2_ENABLE)
		#if (BUTTON_2_LONG == CMD_SLEEPMODE)
	gShutdownButton = 2;
		#endif
	#endif
	#if defined(BUTTON_3_ENABLE) || defined(EXPANDER_3_ENABLE)
		#if (BUTTON_3_LONG == CMD_SLEEPMODE)
	gShutdownButton = 3;
		#endif
	#endif
	#if defined(BUTTON_4_ENABLE) || defined(EXPANDER_4_ENABLE)
		#if (BUTTON_4_LONG == CMD_SLEEPMODE)
	gShutdownButton = 4;
		#endif
	#endif
	#if defined(BUTTON_5_ENABLE) || defined(EXPANDER_5_ENABLE)
		#if (BUTTON_5_LONG == CMD_SLEEPMODE)
	gShutdownButton = 5;
		#endif
	#endif
#endif
}

Vorteile

Clang kann gut mit dem c und c++ Syntax umgehen. Es hat sehr viele Konfigurationsmöglichkeiten und (wahrscheinlich der größte Plus-Punkt), es ist von Haus aus in VSCode verfügbar. Auch ist es recht einfach es in Github Actions einzubinden (buut, I’ve never done that).

Nachteile

Clang-Format geht (wie auch die anderen Formatter) sehr stiefmütterlich mit Makros um. Es ist schon schwer ihn zu überreden diese einzurücken, aber folgenden Style geht einfach nicht:

void x() {
	#if x
		y = 0;
	#else
		y = 1;
	#endif
}

AStyle

Abürzung für Artistic Style. Hatte bis vor paar Wochen nichts von ihm gehört, aber es scheint relativ verbreitet zu sein und wird auch aktiv gepfelgt. Eine Unterstützung in VSCode muss sowohl Asytle (das Kommandozeilen Tool) als auch als Plugin Astyle installiert und konfiguriert werden.

.astylerc
-A2
--indent=tab
--indent-switches
--attach-closing-while
--indent-modifiers
--indent-preproc-cond
--indent-preproc-block
--indent-col1-comments
--min-conditional-indent=2
--pad-comma
--align-pointer=name
--align-reference=name
--break-one-line-headers
--add-braces
--attach-return-type-decl
--max-code-length=220

Ergebnis
#include <Arduino.h>
#include "settings.h"
#include "Log.h"
#include "Button.h"
#include "Cmd.h"
#include "Port.h"
#include "System.h"

bool gButtonInitComplete = false;

// Only enable those buttons that are not disabled (99 or >115)
// 0 -> 39: GPIOs
// 100 -> 115: Port-expander
#if (NEXT_BUTTON >= 0 && NEXT_BUTTON <= MAX_GPIO)
#define BUTTON_0_ENABLE
#elif (NEXT_BUTTON >= 100 && NEXT_BUTTON <= 115)
#define EXPANDER_0_ENABLE
#endif

class z {
public:
	z() { }
	void doStuff(int z) {
		return l;
	}
};

t_button gButtons[7];         // next + prev + pplay + rotEnc + button4 + button5 + dummy-button
uint8_t gShutdownButton = 99; // Helper used for Neopixel: stores button-number of shutdown-button
uint16_t gLongPressTime = 0;

#ifdef PORT_EXPANDER_ENABLE
extern bool Port_AllowReadFromPortExpander;
#endif

static volatile SemaphoreHandle_t Button_TimerSemaphore;

void Button_Init() {
#if (WAKEUP_BUTTON >= 0 && WAKEUP_BUTTON <= MAX_GPIO)
	if (ESP_ERR_INVALID_ARG == esp_sleep_enable_ext0_wakeup((gpio_num_t)WAKEUP_BUTTON, 0)) {
		snprintf(Log_Buffer, Log_BufferLength, "%s (GPIO: %u)", (char *) FPSTR(wrongWakeUpGpio), WAKEUP_BUTTON);
		Log_Println(Log_Buffer, LOGLEVEL_ERROR);
	}
#endif

	if (x) {
		y = z;
	}

	switch (a) {
		case b:
			break;
	}

	for (int i = 0; i < 5; i++) {
		i--;
	}

#ifdef NEOPIXEL_ENABLE // Try to find button that is used for shutdown via longpress-action (only necessary for Neopixel)
#if defined(BUTTON_0_ENABLE) || defined(EXPANDER_0_ENABLE)
#if (BUTTON_0_LONG == CMD_SLEEPMODE)
	gShutdownButton = 0;
#endif
#endif
#if defined(BUTTON_1_ENABLE) || defined(EXPANDER_1_ENABLE)
#if (BUTTON_1_LONG == CMD_SLEEPMODE)
	gShutdownButton = 1;
#endif
#endif
#if defined(BUTTON_2_ENABLE) || defined(EXPANDER_2_ENABLE)
#if (BUTTON_2_LONG == CMD_SLEEPMODE)
	gShutdownButton = 2;
#endif
#endif
#if defined(BUTTON_3_ENABLE) || defined(EXPANDER_3_ENABLE)
#if (BUTTON_3_LONG == CMD_SLEEPMODE)
	gShutdownButton = 3;
#endif
#endif
#if defined(BUTTON_4_ENABLE) || defined(EXPANDER_4_ENABLE)
#if (BUTTON_4_LONG == CMD_SLEEPMODE)
	gShutdownButton = 4;
#endif
#endif
#if defined(BUTTON_5_ENABLE) || defined(EXPANDER_5_ENABLE)
#if (BUTTON_5_LONG == CMD_SLEEPMODE)
	gShutdownButton = 5;
#endif
#endif
#endif
}

Vorteile

Der Formatter kann Preprozessor direktiven (halwegs) formatieren.

Nachteile

Das Tool muss manuell installiert und das VSCode Plugin eingestellt werden. Das war bei mir nicht am Einfachsten, aber wahrscheinlich mit einem kleinen How-To machbar.
Der Formatter hat gefühlt einige Bugs, wie zB wenn Abstände zwischen mathematischen Operatoren gewünscht sind, werden auch Pointer so formatiert (also zb aus Pointer *p wird Pointer * p).

Uncrustify

„Gefunden“ durch das UniversalIntendGUI. Sehr viele Einstellungsmöglichkeiten und vielleicht auch ein Volltreffer.

uncrustify.cfg
tok_split_gte=false
utf8_byte=false
utf8_force=false
indent_cmt_with_tabs=true
indent_align_string=false
indent_braces=false
indent_braces_no_func=false
indent_braces_no_class=false
indent_braces_no_struct=false
indent_brace_parent=false
indent_namespace=false
indent_extern=false
indent_class=false
indent_class_colon=false
indent_else_if=false
indent_var_def_cont=false
indent_func_call_param=false
indent_func_def_param=false
indent_func_proto_param=false
indent_func_class_param=false
indent_func_ctor_var_param=false
indent_template_param=false
indent_func_param_double=false
indent_relative_single_line_comments=false
indent_col1_comment=false
indent_access_spec_body=false
indent_paren_nl=false
indent_comma_paren=false
indent_bool_paren=false
indent_first_bool_expr=false
indent_square_nl=false
indent_preserve_sql=false
indent_align_assign=true
sp_balance_nested_parens=false
align_keep_tabs=false
align_with_tabs=false
align_on_tabstop=false
align_number_left=false
align_func_params=false
align_same_func_call_params=false
align_var_def_colon=false
align_var_def_attribute=false
align_var_def_inline=false
align_right_cmt_mix=false
align_on_operator=false
align_mix_var_proto=false
align_single_line_func=false
align_single_line_brace=false
align_nl_cont=false
align_left_shift=true
align_oc_decl_colon=false
nl_collapse_empty_body=false
nl_assign_leave_one_liners=false
nl_class_leave_one_liners=false
nl_enum_leave_one_liners=false
nl_getset_leave_one_liners=false
nl_func_leave_one_liners=false
nl_if_leave_one_liners=false
nl_multi_line_cond=false
nl_multi_line_define=false
nl_before_case=false
nl_after_case=false
nl_after_return=false
nl_after_semicolon=false
nl_after_brace_open=false
nl_after_brace_open_cmt=false
nl_after_vbrace_open=false
nl_after_vbrace_open_empty=false
nl_after_brace_close=false
nl_after_vbrace_close=false
nl_define_macro=false
nl_squeeze_ifdef=false
nl_ds_struct_enum_cmt=false
nl_ds_struct_enum_close_brace=false
nl_create_if_one_liner=false
nl_create_for_one_liner=false
nl_create_while_one_liner=false
ls_for_split_full=false
ls_func_split_full=false
nl_after_multiline_comment=false
eat_blanks_after_open_brace=false
eat_blanks_before_close_brace=false
mod_full_brace_if_chain=false
mod_pawn_semicolon=false
mod_full_paren_if_bool=false
mod_remove_extra_semicolon=false
mod_sort_import=false
mod_sort_using=false
mod_sort_include=false
mod_move_case_break=false
mod_remove_empty_return=false
cmt_indent_multi=true
cmt_c_group=false
cmt_c_nl_start=false
cmt_c_nl_end=false
cmt_cpp_group=false
cmt_cpp_nl_start=false
cmt_cpp_nl_end=false
cmt_cpp_to_c=false
cmt_star_cont=false
cmt_multi_check_last=true
cmt_insert_before_preproc=false
pp_indent_at_level=false
pp_region_indent_code=false
pp_if_indent_code=true
pp_define_at_level=true
indent_switch_case=4
newlines=crlf
indent_with_tabs=2
sp_arith=add
sp_assign=add
sp_assign_default=add
mod_full_brace_do=force
mod_full_brace_for=force
mod_full_brace_function=force
mod_full_brace_if=force
mod_full_brace_while=force

Ergebnis
#include <Arduino.h>
#include "settings.h"
#include "Log.h"
#include "Button.h"
#include "Cmd.h"
#include "Port.h"
#include "System.h"

bool gButtonInitComplete = false;

// Only enable those buttons that are not disabled (99 or >115)
// 0 -> 39: GPIOs
// 100 -> 115: Port-expander
#if (NEXT_BUTTON >= 0 && NEXT_BUTTON <= MAX_GPIO)
	#define BUTTON_0_ENABLE
#elif (NEXT_BUTTON >= 100 && NEXT_BUTTON <= 115)
	#define EXPANDER_0_ENABLE
#endif

class z {
	public:
		z() { }
		void doStuff(int z) { return l; }
};

t_button gButtons[7];		// next + prev + pplay + rotEnc + button4 + button5 + dummy-button
uint8_t gShutdownButton = 99;	// Helper used for Neopixel: stores button-number of shutdown-button
uint16_t gLongPressTime = 0;

#ifdef PORT_EXPANDER_ENABLE
	extern bool Port_AllowReadFromPortExpander;
#endif

static volatile SemaphoreHandle_t Button_TimerSemaphore;

void Button_Init() {
	#if (WAKEUP_BUTTON >= 0 && WAKEUP_BUTTON <= MAX_GPIO)
		if (ESP_ERR_INVALID_ARG == esp_sleep_enable_ext0_wakeup((gpio_num_t)WAKEUP_BUTTON, 0)) {
			snprintf(Log_Buffer, Log_BufferLength, "%s (GPIO: %u)", (char *) FPSTR(wrongWakeUpGpio), WAKEUP_BUTTON);
			Log_Println(Log_Buffer, LOGLEVEL_ERROR);
		}
	#endif

	if (x) {
		y = z;
	}


	switch (a) {
	    case b:
		    break;
	}

	for (int i = 0; i < 5; i++) {
		i--;
	}

	#ifdef NEOPIXEL_ENABLE	// Try to find button that is used for shutdown via longpress-action (only necessary for Neopixel)
		#if defined(BUTTON_0_ENABLE) || defined(EXPANDER_0_ENABLE)
			#if (BUTTON_0_LONG == CMD_SLEEPMODE)
				gShutdownButton = 0;
			#endif
		#endif
		#if defined(BUTTON_1_ENABLE) || defined(EXPANDER_1_ENABLE)
			#if (BUTTON_1_LONG == CMD_SLEEPMODE)
				gShutdownButton = 1;
			#endif
		#endif
		#if defined(BUTTON_2_ENABLE) || defined(EXPANDER_2_ENABLE)
			#if (BUTTON_2_LONG == CMD_SLEEPMODE)
				gShutdownButton = 2;
			#endif
		#endif
		#if defined(BUTTON_3_ENABLE) || defined(EXPANDER_3_ENABLE)
			#if (BUTTON_3_LONG == CMD_SLEEPMODE)
				gShutdownButton = 3;
			#endif
		#endif
		#if defined(BUTTON_4_ENABLE) || defined(EXPANDER_4_ENABLE)
			#if (BUTTON_4_LONG == CMD_SLEEPMODE)
				gShutdownButton = 4;
			#endif
		#endif
		#if defined(BUTTON_5_ENABLE) || defined(EXPANDER_5_ENABLE)
			#if (BUTTON_5_LONG == CMD_SLEEPMODE)
				gShutdownButton = 5;
			#endif
		#endif
	#endif
}

Vorteile

Es gibt ein VSCode Plugin Uncrustify, den ich aber noch nicht ausgetestet habe. Das Tool hat sehr viele Einstellungsmöglichkeiten und wird aktiv entwickelt. Es gibt auch Github Actions um commits gegen eine Konfiguration zu testen.

Nachteile

Der Formatter muss extern instelliert werden, es gibt aber Builds für Linux und Windows (somit ist das kein echter Nachteil sondern eher ein „eh“). Die Masse an Einstellungen ist wortwörtlich erschlagend und ohne der GUI wäre es noch einmal schlimmer da die Parameter nicht selbsterklärend sind.

Builtin VSCode Formatter

Benutzt Clang, wenn die Datei .clang-format nicht gefunden wird, wird das default Style „Visual Studio“ verwendet. Das entspricht der folgenden Config:

UseTab: (VS Code current setting)
IndentWidth: (VS Code current setting)
BreakBeforeBraces: Allman
AllowShortIfStatementsOnASingleLine: false
IndentCaseLabels: false
ColumnLimit: 0

Damit habe ich das nicht weiter betrachtet.

HTML & Javascript

Prettier

Meiner Meinung nach eines der besten Formatter für Javascript und HTML. Kann mit fast allem was Web im Namen hat umgehen, HTML, Javascript, TypeScript und JSX werden bei uns wohl die wichtigsten sein (eine ausführliche Liste ist in der Doku zu finden). Es hat sehr viele Einstellungsmöglichkeiten und kann auch durch ein Config-File gesteuert werden.



TL;DR
Das sind meine gefunden Tools. Aktuell liebäuge ich mit Uncrustify, aber ich bin für Ideen offen, wenn ihr noch andere Tools für die Formatierung kennt. Was halt sehr angenehm wäre, wenn wir zu 100% Toolgestützt sind. Dann muss keine Zeit verbraten werden nur um die Formatierung richtig zu ziehen (ich erwische mich immer, dass ich reflexartig SHIFT+ALT+F drücke und die Formatierung zerstöre :laughing:)

4 „Gefällt mir“

Vielen Dank dir! Das sieht doch schon mal ziemlich vielversprechend aus.
Eine separate Installation finde ich nicht geeignet. Das machen dann doch nur die üblichen Verdächtigen :slight_smile: .
Finde den Clang gar nicht schlecht im Ergebnis und würde den dann eigentlich aufgrund der einfachen aktivierung bevorzugen, zumal wir ja zukünfig weniger Makros haben wollen :wink:

1 „Gefällt mir“

Also es muss auf jeden Fall einfach sein, es einzurichten. Sonst macht das keiner und dann gibt es wieder Wildwuchs :grin:

3 „Gefällt mir“

Hast du dir auch mal angeschaut was man hier von Seiten github machen könnte? Automatisierte git-hooks, ablehnen von unformatierten PRs, etc.

Also ich würde mich wegen der starken Verbreitung, der guten Integration und einfachen Verwendung auch für clang-format aussprechen. Das „Schwester-Tool“ clang-tidy zur statischen Code-Analyse könnte langfristig als Ergänzung auch sehr nützlich sein und ist sogar bereits in PlatformIO integriert.

Zu den Problemen mit dem Präprozessor: Ich denke das sollten wir uns mit dem Style abfinden, den die Tools, bzw. die Style Guides vorgeben auch wenn das nicht unbedingt dem entspricht wie es jetzt gerade aussieht. Außerdem können gefühlt 90% der aktuell im Code vorhandenen #ifdef ... entweder einfach vermieden oder gleichwertig ohne Präprozessor umgesetzt werden. Eine gewisse Vorarbeit dazu würde mein KConfig-Rework mit sich bringen.

3 „Gefällt mir“

auch ein +1 für den clang, mit dabei sieht gut aus, passt für mich

(Das Einrücken von Makros wie als wenn sie Anweisungen wären habe ich eh nie verstanden, ist für mich nicht das gleiche bzw. gehört anders eingerückt)

1 „Gefällt mir“

Und ich hab’s nie verstanden, warum man das nicht einrückt.

1 „Gefällt mir“

Ich kenne es auch so, dass Makros nicht wie der Programmfluss formatiert werden (mit einigen Ausnahmen, wie zB FOREACH Makros).

Es gibt uralte Compiler (von vor C89), die benötigen, dass bei einer Präprozessor-Direktive das ‚#‘ an Erster Stelle in einer Zeile steht (moderne Compiler verlangen nur, dass nur Leerzeichen davor stehen).

Ein anderer Grund ist, dass der Präprozessor vor dem Compiler durchläuft und eigentlich nicht Teil vom Programmfluss ist. Ich kenne diese Arten für die Formatierung:

a)

#ifdef WIN32
    #include <a.h>
#else
    #include <b.h>
#endif

void x() {
    #if DEBUG
    do_something();
    #endif
}

b)

#ifdef WIN32
#include <a.h>
#else
#include <b.h>
#endif

void x() {
#if DEBUG
    do_something();
#endif
}

c) (den mag ich am wenigsten, da die Einfärbung ab und zu fehl schlägt)

#ifdef WIN32
#   include <a.h>
#else
#   include <b.h>
#endif

void x() {
#   if DEBUG
    do_something();
#   endif
}

Ich denke auch, dass wir die meisten großen #if Blöcke umändern können, dass sie „normalen“ Code erzeugen (Buttons.cpp ist solch ein Kandidat). Damit würde das Problem mit der Formatierung auf jeden Fall lösen.

3 „Gefällt mir“

So, ich habe mich mal mit Button.cpp beschäftigt (der nach meinem Geschmack das meiste Copy & Paste und Präprozessor Direktiven hat). Ist relativ einfach zu vereinfachen:

Die Branch für alle Mutigen. Es lässt sich compilieren und läuft :laughing:: GitHub - laszloh/ESPuino at devel/code_cleanup

Größenvergleich zwischen den Varianten:

Alte Buttons:

    Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
    RAM:   [==        ]  21.6% (used 70736 bytes from 327680 bytes)
    Flash: [======    ]  58.9% (used 2276609 bytes from 3866624 bytes)

Neue Buttons:

    Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
    RAM:   [==        ]  21.6% (used 70720 bytes from 327680 bytes)
    Flash: [======    ]  58.9% (used 2276269 bytes from 3866624 bytes)
4 „Gefällt mir“

Das sehe ich genau so.
Ich würde mich freuen, wenn es eine Entscheidung für ein Tool geben würde, damit zumindest optische Anforderungen, wie ein Leerzeichen oder kein Leerzeichen vor oder nach einer Klammer, Einrückungen etc. einheitlich sind.

Aber gerade die Formatierung der Prä-Prozessor Anweisungen wie #if und dergleichen wirft eigentlich ein ganz neues Thema auf. Hier sollte es auch Richtlinien geben zu welchem Zweck diese angewendet bzw. nicht angewendet werden sollten. Mir ist klar, dass das, so wie es ist, gewachsen ist und immer den neuen Wünschen und Anforderungen entsprechend erweitert wurde mit dem Ziel, eine passende Firmware zu erstellen. Aber auch offene Wünsche, wie z.B. fertige Builds, fordern diesbezügliche Überlegungen.

Für eine Entscheidungshilfe, würde ich folgenden Ansatz anbieten:
Meine Betrachtungsweise bezieht sich auf die Anwendung die erstellt wird.
Diese soll möglichst auf unterschiedlichsten Hosts (Mikroprozessor Plattformen, Developer Boards, etc.) lauffähig sein. Wenn das gegeben ist, dann sollte diese dann ohne Neucompilierung maximal bezüglich Hardwarkomponenteneinsatz und Featurewahl konfigurierbar sein. Damit lässt sich ein Build breitbandig einsetzen.

Daraus ergeben sich schon viele Entscheidungshilfen zu PräProzessor ja/nein

Klares Ja zu Präprozessor:

  • Bedingungen, die Sourceteile nicht compilieren, da diese nicht auf allen Hosts lauffähig sind
    z.B: Plattform: ESP32 oder ESP32_XY (da bei 2. z.B. Register angesprochen werden, die beim 1. nicht vorhanden sind) (verwandt mit nächstem Punkt)
  • Einbindung von libraries die unterschiedliche Funktionalitäten zu unterschiedlichen Performances bieten: Hier per #if definieren was beim aktuellen Build Priortität hat: Mehr Features oder HighPerformance

Klares Nein zu Präprozessor:

  • Konfiguration von gewünschten Features
  • Ersetzen von Laufzeitprogrammablauf (z.B: Ermittlung des Wertes einer Konfigurationskonstanten etc.)

Großer Vorteil: Änderung von Featurwünschen ohne Neucompilierung der Firmware und dem damit verbundenen zwingenden Update.

Wann gibt es ein JAIN?
Prinzipiell sind für die maximale Flexibilität zur Laufzeit PräProzessor Directiven möglichst zu vermeiden.
Wir bewegen uns aber doch noch im Bereich nicht unendlicher Ressourcen (vorallem Speicher).
Sobald nicht mehr alle Features gleichzeit aktivierbar sind bzw. überhaupt speichermäßig nicht mehr unterbringbar sind, kommt man um eine Vorauswahl nicht mehr herum.

Irgendwie komme ich zum Schluss, solange wir es uns ressourcenmäßig leisten können, #if entfernen und durch „normale“ if Abfragen ersetzen.
Das bedingt dann eine config Klasse, die ihre Werte persistent (NVS) pflegt.

Damit verliert das Thema der PräProzessor Formatierung stark an Bedeutung.
Die verschachtelten #if Anweisungen sollten sich weitgehend in ganz einfache Anweisungen am Begin der Zeile auflösen.

Was meint ihr?

2 „Gefällt mir“

Ja, leisten können wahrscheinlich schon was die Ressourcen der Hardware angeht (Speicher). Und deine Aufteilung wo der Präprozessor gebraucht wird und wo nicht passt m. E. auch soweit.

Aber: Das bedeutet einiges an tiefgreifenden Änderungen und einiges an Programmieraufwand, weil auch Webinterface, etc. betroffen sind. Als langfristiges Ziel ist das sicher gut, aber ich denke wir sollten schrittweise vorgehen.

Selbst ohne dass man Settings vom Präprozessor in NVS zur Laufzeit verlagert kann man die #ifdef ... recht einfach loswerden. Zum Teil so wie oben von @laszloh am Beispiel der Buttons gezeigt oder es gibt da auch noch einen netten Makro-Trick, der bei anderen Projekten wie z. B. Linux, U-Boot oder Zephyr eingesetzt wird um ein definiertes oder undefiniertes Präprozessor-Symbol in ein Boolean-Literal (0 oder 1) zu konvertieren.

Damit kann man dann z. B. einfach if IS_ENABLED(PORT_EXPANDER_ENABLE) statt #ifdef PORT_EXPANDER_ENABLE schreiben. Der Compiler sieht dann nur noch entweder if (1) oder if (0) und optimiert entsprechend.

Wen interessiert wie das im Hintergrund genau funktioniert kann sich z. B. zephyr/include/zephyr/sys/util_internal.h at 76078ea63ce77fa32260d82e913115569731f849 · zephyrproject-rtos/zephyr · GitHub anschauen.

3 „Gefällt mir“

Ich stimme fschrempf zu. Deine vorgeschalgenen Änderungen würden eine tiefgreifende Änderung in dem Programmcode bedeuten. Plus muss die Konfiguration wahrscheinlich zu einem großen Teil im RAM gehalten werden. Das bringt einge Herausforderungen mit sich, das optimal für den RAM-Verbrauch auszuführen (und dabei so flexibel wie möglich zu bleiben).

Ein Beispiel wie wir es nicht lösen sollten (was aber gleichzeitig natürlich die größte Flexibilität mit sich bringt) könnt ihr in squeezelite-esp32 anschauen. Da wird die Konfiguration als JSON Objekt in den externen RAM geladen. Das bringt aber mit sich, dass wir zwingend einen WROVER haben müssen. Sonst reicht der RAM & Flash nicht aus.

Aktuell geht die SW auch sehr gut mit WROOM (also ESP32’s ohen externen RAM) und das will ich ehrlich auch behalten.Da haben wir aktuell um die 60kB Heap für die MP3 Dekodierung. Bluetooth Source funktioniert zB nicht, da der Heap dann auf 5k - 25k zusammenbricht (siehe Neustartschleife nach gescheitertem Anschluss eines Bluetoothkopfhörers). Damit ist es dann nicht mehr verwendbar, da für die MP3 Dekodierung leider gilt: „Mehr ist immer Besser und nur noch mehr ist noch besser“


Ich habe die Makros gesucht (und bin halb verzweifelt dran). Jetzt weiß ich wo ich abkupfern muss :laughing: ! Danke!!

Des weiteren kann mit c++17 constexpr if verwendet werden indem wir schreiben if constexpr(IS_ENABLED(XYZ)) (order wie in der LED PR if constexpr(NUM_LEDS ==1).

Das sind Ausdrücke die zur Compile-Zeit ausgewertet werden und entsprechend dem Ergenbis Pfade genommen werden. Der andere Pfad wird aus dem resultierenden Binary gelöscht.
Bei unserer Verwendung ist das equivalent zu dem #if, aber mit dem riesen Vorteil, dass man zu dem Zeitpunkt einen c++17 Compiler am „Steuerrad“ hat. D.h. er findet auch Tipp- und Syntaxfehler in dem „nicht genommenem“ Zweig und es könne auch komplexe Ausdrücke und std-Funktionen wie zb if constexpr (std::is_pointer_v<T>) verwendet werden.

EIn zusätzlicher Vorteil von constexpr if’s ist, wenn der Ausdruck nicht zur Compile-Zeit evaluiert werden kann (weil man zB den Inhalt einer nicht const variable abfragt), kriegt man das als Fehler vom Compiler um die Ohren geschmissen. Das heißt, man kann darauf vertrauen, dass dann auch nur 1 Pfad ohne eine if-Abfrage schlussendlich drin sein wird.

Hier ist eine sehr gute Diskussion drüber: https://www.reddit.com/r/cpp_questions/comments/x6didv/is_using_if_constexpr_equal_to_using_preprocessor/

3 „Gefällt mir“

Es ging mir darum, das Thema anzusprechen und insbesondere ggf. bei neuem Code in die gewünschte Richtung zu gehen.
Wie sieht es nun mit den vorgestellten Tools zur Codeformatierung aus?
Soll das kommen und wenn ja welches.
Clang und uncrustify würden mir gefallen.

Niko, nicht falsch verstehen, deine Idee ist gut. Bei einem Projekt mit mehr RAM & Flash würde ich auch definitiv in die Richtung gehen, dass so viel wie möglich Konfiguriert werden kann (am Besten in einfach zu ändernden JSON Files).
Das Problem ist halt hier wirklich das Audio Decoding (plus my :heart: dass das mit WROOM geht). Und ja, einiges mehr per Config modifizierbar zu machen ist sicherlich eine gute Idee und etwas was wir als Ziel haben sollten (ich schaue dich an cmdShort & cmdLong in Buttons.cpp :slight_smile:). Nur halt nicht alles, vor allem da aktuell teilweise einzelne Funktionen sich in die Quere kommen, bzw kein Sinn machen dynamisch geändert werden zu können ← zB Anzahl an LEDs in dem Ring oder die Anzahl an Tasten.

Aber ja, back to topic :smiley:


Ich plädiere für Clang. Einfach wegen seiner Verfügbarkeit (basically jeder der VSCode & Platformio verwendet hat es schon installiert). Damit halt wirklich keiner eine Ausrede, wieso er eine unformatierte PR eröffnen sollte und es gibt auch Github Actions, die eine PR automatisch gegen eine Config checken.

Wir müssen halt für die nächste Zeit noch mit dem „schlecht“ formatierten Präprozessor Direktiven leben, bis wir die Kompliziertesten aus dem Code bekommen.

4 „Gefällt mir“

Danke für die Erklärung. Außer dass if constexpr eben eine Prüfung mit sich bringt ob der Ausdruck wirklich konstant ist hat es aber keinen Vorteil gegenüber einem normalen if, oder?

Wenn ich if constexpr(NUM_LEDS == 1) verwende resultiert das doch binär in den selben Instruktionen, wie wenn ich if (NUM_LEDS == 1) schreibe, oder? Beides mal optimiert der Compiler so, dass nur ein Pfad und keine Verzweigung übrig bleibt.

Oder geht es im Wesentlichen darum den Compiler zu der Optimierung zu zwingen, während ohne constexpr nicht garantiert ist, dass die Verzweigung wegoptimiert wird (z. B. mit deaktivierter Optimierung -O0)?

Genau, if constexpr zwingt den Compiler zur Optimierung der Verzweigung (und den Programmierer dazu den Ausdruck auch konstant zu machen). Ansonsten hat der Compiler, wie du sagst, je nach -Ox die Freiheit Pfade drin zu lassen oder wegzuoptimieren.

Bei den NUM_LEDS == 1 wird das bei uns jedes Mal den gleichen Code erzeugen (da das am Ende nur ein Vergleich 12 == 1 ist).

1 „Gefällt mir“

Ich finde auch, clang ist eine gute Auswahl, da weit verbreitet.

Noch OT zur Runtime-config: ich meine es gab da mal den Traum ein Universal-image bereitzustellen. Dort müsste die hardware-abstraction - also so etwas wie Anzahl LEDs, pin-Belegung, etc. - komplett konfigurierbar sein. Vielleicht muss man sich dafür vom WROOM verabschieden, und Nutzer von WROOM müssen halt selbst compilen. Die Schwierigkeit dabei wäre wiederum ordentlich zwischen compile-time und runtime config zu abstrahieren.

Warum muss man sich dafür vom Wroom verabschieden? Man kann doch mit Pointern arbeiten und die Instanzierung macht man nur wenn notwendig.

Also es wäre definitiv unschön, wroom auszuklammern. Das wird auf jeden Fall von mehreren Dutzend Personen verwendet.

Mein Kommentar bezog sich darauf, dass der RAM für die MP3-Dekodierung nicht mehr reicht, wenn zusätzlich die Konfigurationsvariablen im Ram gehalten werden.
Ob das notwendigerweise so ist, sei dahingestellt.

Und ich bezog mich auch nur darauf den WROOM von der dynamischen Kondiguration auszuschließen. WROOMs müssten dann wie bisher konfiguriert werden. Aber das schweift schon wieder sehr ab.

Hab ich da etwas übersehen? Von vieviel Bytes an RAM Bedarf gehst du aus?