From 1d766c596e8e457db3f5d7cdba1d3323d98439f9 Mon Sep 17 00:00:00 2001 From: Jonathan Arnett Date: Sun, 6 Nov 2016 13:48:20 -0500 Subject: [PATCH 1/6] Removed the control action when holding z or / --- keyboards/ergodox/keymaps/j3rn/keymap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/keyboards/ergodox/keymaps/j3rn/keymap.c b/keyboards/ergodox/keymaps/j3rn/keymap.c index d913ea4a5..2069f26ae 100644 --- a/keyboards/ergodox/keymaps/j3rn/keymap.c +++ b/keyboards/ergodox/keymaps/j3rn/keymap.c @@ -16,7 +16,7 @@ const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = { * |--------+------+------+------+------+------| | | |------+------+------+------+------+--------| * |Ctrl/Esc| A | S | D | F | G |------| |------| H | J | K | L | ; | ' | * |--------+------+------+------+------+------| Hyper| | Meh |------+------+------+------+------+--------| - * | LShift |Z/Ctrl| X | C | V | B | | | | N | M | , | . |//Ctrl| RShift | + * | LShift | Z | X | C | V | B | | | | N | M | , | . | / | RShift | * `--------+------+------+------+------+-------------' `-------------+------+------+------+------+--------' * | ~L1 | Alt |AltShf| Left | Right| | Up | Down | [ | ] | ~L1 | * `----------------------------------' `----------------------------------' @@ -35,7 +35,7 @@ const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = { KC_GRV, KC_1, KC_2, KC_3, KC_4, KC_5, KC_LBRC, KC_TAB, KC_Q, KC_W, KC_E, KC_R, KC_T, KC_MINS, CTL_T(KC_ESC), KC_A, KC_S, KC_D, KC_F, KC_G, - KC_LSFT, CTL_T(KC_Z), KC_X, KC_C, KC_V, KC_B, ALL_T(KC_NO), + KC_LSFT, KC_Z, KC_X, KC_C, KC_V, KC_B, ALL_T(KC_NO), MO(SYMB), KC_LALT, LALT(KC_LSFT), KC_LEFT,KC_RGHT, ALT_T(KC_APP), KC_HOME, @@ -45,7 +45,7 @@ const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = { KC_RBRC, KC_6, KC_7, KC_8, KC_9, KC_0, KC_BSPC, KC_EQL, KC_Y, KC_U, KC_I, KC_O, KC_P, KC_BSLS, KC_H, KC_J, KC_K, KC_L, KC_SCLN, KC_QUOT, - MEH_T(KC_NO),KC_N, KC_M, KC_COMM,KC_DOT, CTL_T(KC_SLSH), KC_RSFT, + MEH_T(KC_NO),KC_N, KC_M, KC_COMM,KC_DOT, KC_SLSH, KC_RSFT, KC_UP, KC_DOWN,KC_LBRC,KC_RBRC, MO(SYMB), KC_PGUP, CTL_T(KC_ESC), From f519b94be7086852f2afe4ec248786b47968f7ff Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Sun, 6 Nov 2016 21:57:26 +0200 Subject: [PATCH 2/6] Add variable trace For debugging changes to variables, either normally or as a result of a memory corruption. --- build_keyboard.mk | 2 + quantum/variable_trace.c | 108 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 quantum/variable_trace.c diff --git a/build_keyboard.mk b/build_keyboard.mk index 282adcb11..461b17cd7 100644 --- a/build_keyboard.mk +++ b/build_keyboard.mk @@ -180,6 +180,8 @@ ifeq ($(strip $(SERIAL_LINK_ENABLE)), yes) VAPTH += $(SERIAL_PATH) endif +SRC += $(QUANTUM_DIR)/variable_trace.c + # Optimize size but this may cause error "relocation truncated to fit" #EXTRALDFLAGS = -Wl,--relax diff --git a/quantum/variable_trace.c b/quantum/variable_trace.c new file mode 100644 index 000000000..dfa37bdef --- /dev/null +++ b/quantum/variable_trace.c @@ -0,0 +1,108 @@ +#include "variable_trace.h" +#include +#include + +#ifdef NO_PRINT +#error "You need undef NO_PRINT to use the variable trace feature" +#endif + +#ifndef CONSOLE_ENABLE +#error "The console needs to be enabled in the makefile to use the variable trace feature" +#endif + + +#define NUM_TRACED_VARIABLES 1 +#define MAX_TRACE_SIZE 4 + +typedef struct { + const char* name; + void* addr; + unsigned size; + const char* func; + int line; + uint8_t last_value[MAX_TRACE_SIZE]; + +} traced_variable_t; + +static traced_variable_t traced_variables[NUM_TRACED_VARIABLES]; + +void add_traced_variable(const char* name, void* addr, unsigned size, const char* func, int line) { + verify_traced_variables(func, line); + if (size > MAX_TRACE_SIZE) { +#if defined(__AVR__) + xprintf("Traced variable \"%S\" exceeds the maximum size %d\n", name, size); +#else + xprintf("Traced variable \"%s\" exceeds the maximum size %d\n", name, size); +#endif + size = MAX_TRACE_SIZE; + } + int index = -1; + for (int i = 0; i < NUM_TRACED_VARIABLES; i++) { + if (index == -1 && traced_variables[i].addr == NULL){ + index = i; + } + else if (strcmp_P(name, traced_variables[i].name)==0) { + index = i; + break; + } + } + + if (index == -1) { + xprintf("You can only trace %d variables at the same time\n", NUM_TRACED_VARIABLES); + return; + } + + traced_variable_t* t = &traced_variables[index]; + t->name = name; + t->addr = addr; + t->size = size; + t->func = func; + t->line = line; + memcpy(&t->last_value[0], addr, size); + +} + +void remove_traced_variable(const char* name, const char* func, int line) { + verify_traced_variables(func, line); + for (int i = 0; i < NUM_TRACED_VARIABLES; i++) { + if (strcmp_P(name, traced_variables[i].name)==0) { + traced_variables[i].name = 0; + traced_variables[i].addr = NULL; + break; + } + } +} + +void verify_traced_variables(const char* func, int line) { + for (int i = 0; i < NUM_TRACED_VARIABLES; i++) { + traced_variable_t* t = &traced_variables[i]; + if (t->addr != NULL && t->name != NULL) { + if (memcmp(t->last_value, t->addr, t->size)!=0){ +#if defined(__AVR__) + xprintf("Traced variable \"%S\" has been modified\n", t->name); + xprintf("Between %S:%d\n", t->func, t->line); + xprintf("And %S:%d\n", func, line); + +#else + xprintf("Traced variable \"%s\" has been modified\n", t->name); + xprintf("Between %s:%d\n", t->func, t->line); + xprintf("And %s:%d\n", func, line); +#endif + xprintf("Previous value "); + for (int j=0; jsize;j++) { + print_hex8(t->last_value[j]); + } + xprintf("\nNew value "); + uint8_t* addr = (uint8_t*)(t->addr); + for (int j=0; jsize;j++) { + print_hex8(addr[j]); + } + xprintf("\n"); + memcpy(t->last_value, addr, t->size); + } + } + + t->func = func; + t->line = line; + } +} From a377017c95b826d83ac7a46ef176d39a58294b44 Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Sun, 6 Nov 2016 22:11:24 +0200 Subject: [PATCH 3/6] Add possibility to control variable trace from make --- build_keyboard.mk | 8 +++++++- quantum/variable_trace.c | 10 ++++++---- quantum/variable_trace.h | 25 +++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 quantum/variable_trace.h diff --git a/build_keyboard.mk b/build_keyboard.mk index 461b17cd7..61aebf393 100644 --- a/build_keyboard.mk +++ b/build_keyboard.mk @@ -180,7 +180,13 @@ ifeq ($(strip $(SERIAL_LINK_ENABLE)), yes) VAPTH += $(SERIAL_PATH) endif -SRC += $(QUANTUM_DIR)/variable_trace.c +ifneq ($(strip $(VARIABLE_TRACE)),) + SRC += $(QUANTUM_DIR)/variable_trace.c + OPT_DEFS += -DNUM_TRACED_VARIABLES=$(strip $(VARIABLE_TRACE)) +ifneq ($(strip $(MAX_VARIABLE_TRACE_SIZE)),) + OPT_DEFS += -DMAX_VARIABLE_TRACE_SIZE=$(strip $(MAX_VARIABLE_TRACE_SIZE)) +endif +endif # Optimize size but this may cause error "relocation truncated to fit" #EXTRALDFLAGS = -Wl,--relax diff --git a/quantum/variable_trace.c b/quantum/variable_trace.c index dfa37bdef..de580244c 100644 --- a/quantum/variable_trace.c +++ b/quantum/variable_trace.c @@ -12,7 +12,9 @@ #define NUM_TRACED_VARIABLES 1 -#define MAX_TRACE_SIZE 4 +#ifndef MAX_VARIABLE_TRACE_SIZE + #define MAX_VARIABLE_TRACE_SIZE 4 +#endif typedef struct { const char* name; @@ -20,7 +22,7 @@ typedef struct { unsigned size; const char* func; int line; - uint8_t last_value[MAX_TRACE_SIZE]; + uint8_t last_value[MAX_VARIABLE_TRACE_SIZE]; } traced_variable_t; @@ -28,13 +30,13 @@ static traced_variable_t traced_variables[NUM_TRACED_VARIABLES]; void add_traced_variable(const char* name, void* addr, unsigned size, const char* func, int line) { verify_traced_variables(func, line); - if (size > MAX_TRACE_SIZE) { + if (size > MAX_VARIABLE_TRACE_SIZE) { #if defined(__AVR__) xprintf("Traced variable \"%S\" exceeds the maximum size %d\n", name, size); #else xprintf("Traced variable \"%s\" exceeds the maximum size %d\n", name, size); #endif - size = MAX_TRACE_SIZE; + size = MAX_VARIABLE_TRACE_SIZE; } int index = -1; for (int i = 0; i < NUM_TRACED_VARIABLES; i++) { diff --git a/quantum/variable_trace.h b/quantum/variable_trace.h new file mode 100644 index 000000000..9899816f6 --- /dev/null +++ b/quantum/variable_trace.h @@ -0,0 +1,25 @@ +#ifndef VARIABLE_TRACE_H +#define VARIABLE_TRACE_H + +#include "print.h" + +#ifdef NUM_TRACED_VARIABLES + +#define ADD_TRACED_VARIABLE(name, addr, size) \ + add_traced_variable(PSTR(name), (void*)addr, size, PSTR(__FILE__), __LINE__) +#define REMOVE_TRACED_VARIABLE(name) remove_traced_variable(PSTR(name), PSTR(__FILE__), __LINE__) +#define VERIFY_TRACED_VARIABLES() verify_traced_variables(PSTR(__FILE__), __LINE__) + +#else + +#define ADD_TRACED_VARIABLE(name, addr, size) +#define REMOVE_TRACED_VARIABLE(name) +#define VERIFY_TRACED_VARIABLES() + +#endif + +// Don't call directly, use the macros instead +void add_traced_variable(const char* name, void* addr, unsigned size, const char* func, int line); +void remove_traced_variable(const char* name, const char* func, int line); +void verify_traced_variables(const char* func, int line); +#endif From 0ba3e523a7c124e4ce54dfd043dc32e72ad3233b Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Sun, 6 Nov 2016 22:44:43 +0200 Subject: [PATCH 4/6] Add documentation for the variable tracing --- quantum/variable_trace.h | 9 +++++++++ readme.md | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/quantum/variable_trace.h b/quantum/variable_trace.h index 9899816f6..46bd82786 100644 --- a/quantum/variable_trace.h +++ b/quantum/variable_trace.h @@ -1,13 +1,22 @@ #ifndef VARIABLE_TRACE_H #define VARIABLE_TRACE_H +// For more information about the variable tracing see the readme. + #include "print.h" #ifdef NUM_TRACED_VARIABLES +// Start tracing a variable at the memory address addr +// The name can be anything and is used only for reporting +// The size should usually be the same size as the variable you are interested in #define ADD_TRACED_VARIABLE(name, addr, size) \ add_traced_variable(PSTR(name), (void*)addr, size, PSTR(__FILE__), __LINE__) + +// Stop tracing the variable with the given name #define REMOVE_TRACED_VARIABLE(name) remove_traced_variable(PSTR(name), PSTR(__FILE__), __LINE__) + +// Call to get messages when the variable has been changed #define VERIFY_TRACED_VARIABLES() verify_traced_variables(PSTR(__FILE__), __LINE__) #else diff --git a/readme.md b/readme.md index 60a94d7d6..d5a259ccb 100644 --- a/readme.md +++ b/readme.md @@ -343,6 +343,10 @@ This allows you to interface with a Bluefruit EZ-key to send keycodes wirelessly This allows you output audio on the C6 pin (needs abstracting). See the [audio section](#driving-a-speaker---audio-support) for more information. +`VARIABLE_TRACE` + +Use this to debug changes to variable values, see the [tracing variables](#tracing-variables) section for more information. + ### Customizing Makefile options on a per-keymap basis If your keymap directory has a file called `Makefile` (note the filename), any Makefile options you set in that file will take precedence over other Makefile options for your particular keyboard. @@ -1283,3 +1287,22 @@ If there are problems with the tests, you can find the executable in the `./buil It's not yet possible to do a full integration test, where you would compile the whole firmware and define a keymap that you are going to test. However there are plans for doing that, because writing tests that way would probably be easier, at least for people that are not used to unit testing. In that model you would emulate the input, and expect a certain output from the emulated keyboard. + +# Tracing variables + +Sometimes you might wonder why a variable gets changed and where, and this can be quite tricky to track down without having a debugger. It's of course possible to manually add print statements to track it, but you can also enable the variable trace feature. This works for both for variables that are changed by the code, and when the variable is changed by some memory corruption. + +To take the feature into use add `VARIABLE_TRACE=x` to the end of you make command. `x` represents the number of variables you want to trace, which is usually 1. + +Then at a suitable place in the code, call `ADD_TRACED_VARIABLE`, to begin the tracing. For example to trace all the layer changes, you can do this +```c +void matrix_init_user(void) { + ADD_TRACED_VARIABLE("layer", &layer_state, sizeof(layer_state)); +} +``` + +This will add a traced variable named "layer" (the name is just for your information), which tracks the memory location of `layer_state`. It tracks 4 bytes (the size of `layer_state`), so any modification to the variable will be reported. By default you can not specify a size bigger than 4, but you can change it by adding `MAX_VARIABLE_TRACE_SIZE=x` to the end of the make command line. + +In order to actually detect changes to the variables you should call `VERIFY_TRACED_VARIABLES` around the code that you think that modifies the variable. If a variable is modified it will tell you between which two `VERIFY_TRACED_VARIABLES` calls the modification happened. You can then add more calls to track it down further. I don't recommend spamming the codebase with calls. It's better to start with a few, and then keep adding them in a binary search fashion. You can also delete the ones you don't need, as each call need to store the file name and line number in the ROM, so you can run out of memory if you add too many calls. + +Also remember to delete all the tracing code ones you have found the bug, as you wouldn't want to create a pull request with tracing code. \ No newline at end of file From 7aa31ad338325477199f752ac3e344a6ab9b27d0 Mon Sep 17 00:00:00 2001 From: Potiguar Faga Date: Tue, 8 Nov 2016 13:32:04 -0200 Subject: [PATCH 5/6] Add brazilian ABNT2 keymap --- quantum/keymap_extras/keymap_br_abnt2.h | 58 +++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 quantum/keymap_extras/keymap_br_abnt2.h diff --git a/quantum/keymap_extras/keymap_br_abnt2.h b/quantum/keymap_extras/keymap_br_abnt2.h new file mode 100644 index 000000000..0df177721 --- /dev/null +++ b/quantum/keymap_extras/keymap_br_abnt2.h @@ -0,0 +1,58 @@ +#ifndef KEYMAP_BR_ABNT2_H +#define KEYMAP_BR_ABNT2_H + +#include "keymap_common.h" + +/* Scan codes for the Brazilian ABNT2 keyboard layout */ + +#define BR_CCDL KC_SCLN // Ç same scancode as ;: on US layout +#define BR_SCLN KC_SLSH // ;: same scancode as /? on US layout +#define BR_QUOT KC_GRV // '" same scancode as `~ on US layout +#define BR_TILD KC_QUOT // ~^ dead keys, same scancode as '" on US layout +#define BR_ACUT KC_LBRC // ´` dead keys, same scancode as [{ on US layout +#define BR_LBRC KC_RBRC // [{ same scancode as ]} on US layout +#define BR_RBRC KC_BSLS // ]} same scancode as \| on US layout +#define BR_BSLS KC_NUBS // \| uses the non-US hash scancode (#~, sometimes §±) +#define BR_SLSH KC_INT1 // /? uses the INTL1 scancode + +#define BR_COLN LSFT(BR_SCLN) // shifted : +#define BR_DQT LSFT(BR_QUOT) // shifted " +#define BR_CIRC LSFT(BR_TILD) // shifted ^ (dead key) +#define BR_GRAV LSFT(BR_ACUT) // shifted ` (dead key) +#define BR_LCBR LSFT(BR_LBRC) // shifted { +#define BR_RCBR LSFT(BR_RBRC) // shifted } +#define BR_PIPE LSFT(BR_BSLS) // shifted | +#define BR_QUES LSFT(BR_SLSH) // shifted ? +#define BR_TRMA LSFT(KC_6) // shifted ¨ (dead key - trema accent) + +// On the ABNT2 the keypad comma and the keypad dot scancodes are switched +// (presumably because in Brazil comma is used as the decimal separator) +#define BR_KPDT KC_KP_COMMA // keypad . +#define BR_KPCM KC_KP_DOT // keypad , + +#define BR_1UP LALT(KC_1) // 1 superscript ¹ alt+1 +#define BR_2UP LALT(KC_2) // 2 superscript ² alt+2 +#define BR_3UP LALT(KC_3) // 3 superscript ³ alt+3 +#define BR_PND LALT(KC_4) // Pound sign £ alt+4 +#define BR_CENT LALT(KC_5) // Cent sign ¢ alt+5 +#define BR_NOT LALT(KC_6) // Not sign ¬ alt+6 +#define BR_SECT LALT(KC_EQL) // Section sign § alt+= +#define BR_FORD LALT(BR_LBRC) // Feminine Ordinal Sign ª alt+[ +#define BR_MORD LALT(BR_RBRC) // Masculine Ordinal Sign º alt+] +#define BR_DGRE LALT(BR_SLSH) // Degree sign ° alt+/ + +#define BR_EURO LALT(KC_E) // Euro sign € alt+e +#define BR_NDTD LALT(BR_TILD) // Non-dead key tilde ~ alt+~ +#define BR_NDAC LALT(BR_ACUT) // Non-dead key acute accent ´ alt+´ +#define BR_NDGV LALT(BR_QUOT) // Non-dead key grave accent ` alt+' +#define BR_NDCR LALT(BR_CIRC) // Non-dead key circumflex accent ^ alt+^ (alt+shift+~) +#define BR_NDTR LALT(BR_TRMA) // Non-dead key trema accent ¨ alt+¨ (alt+shift+6) + +// For 101-key keyboard layouts, the ABNT2 layout allows +// the slash and question mark to be typed using alt+q and alt+w. +// The shortcuts are provided here for completeness' sake, +// but it's recommended to use BR_SLSH and BR_QUES instead +#define BR_ASLS LALT(KC_Q) +#define BR_AQST LALT(KC_W) + +#endif From 64218f0f7018cf0ebedb14197b07d05904b5bc09 Mon Sep 17 00:00:00 2001 From: Noah Andrews Date: Tue, 8 Nov 2016 22:05:24 -0500 Subject: [PATCH 6/6] Add proper shortcode for KC_DELETE to keycode.txt --- doc/keycode.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/keycode.txt b/doc/keycode.txt index 5a42c50bf..687406fda 100644 --- a/doc/keycode.txt +++ b/doc/keycode.txt @@ -84,7 +84,7 @@ KC_PAUSE KC_PAUS 48 Keyboard Pause1 KC_INSERT KC_INS 49 Keyboard Insert1 KC_HOME 4A Keyboard Home1 KC_PGUP 4B Keyboard PageUp1 -KC_DELETE KC_DELETE 4C Keyboard Delete Forward +KC_DELETE KC_DEL 4C Keyboard Delete Forward KC_END 4D Keyboard End1 KC_PGDOWN KC_PGDN 4E Keyboard PageDown1 KC_RIGHT KC_RGHT 4F Keyboard RightArrow1