Improve the functionality of qmk.questions (#8020)

* wip

* tested and working

* remove unused import

* Update lib/python/qmk/questions.py

Co-Authored-By: Erovia <[email protected]>

* fix flake8 errors

Co-authored-by: Erovia <[email protected]>

docs/breaking_changes.md

@@ -15,8 +15,8 @@ The next Breaking Change is scheduled for February 29, 2020.
 ### Important Dates
 
 * [x] 2019 Sep 21 - `future` is created. It will be rebased weekly.
-* [ ] 2020 Feb 1 - `future` closed to new PR's.
+* [x] 2020 Feb 1 - `future` closed to new PR's.
-* [ ] 2020 Feb 1 - Call for testers.
+* [x] 2020 Feb 1 - Call for testers.
 * [ ] 2020 Feb 26 - `master` is locked, no PR's merged.
 * [ ] 2020 Feb 28 - Merge `future` to `master`.
 * [ ] 2020 Feb 29 - `master` is unlocked. PR's can be merged again.

keyboards/fc660c/keymaps/mikefightsbears/keymap.c

@@ -0,0 +1,34 @@
+/*
+Copyright 2017 Balz Guenat
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+#include QMK_KEYBOARD_H
+
+const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {
+  [0] = LAYOUT(
+        KC_GESC,KC_1,   KC_2,   KC_3,   KC_4,   KC_5,   KC_6,   KC_7,   KC_8,   KC_9,   KC_0,   KC_MINS,KC_EQL, KC_BSPC,     KC_INS,
+        KC_TAB, KC_Q,   KC_W,   KC_E,   KC_R,   KC_T,   KC_Y,   KC_U,   KC_I,   KC_O,   KC_P,   KC_LBRC,KC_RBRC,KC_BSLS,     KC_DEL,
+        KC_LCTL,KC_A,   KC_S,   KC_D,   KC_F,   KC_G,   KC_H,   KC_J,   KC_K,   KC_L,   KC_SCLN,KC_QUOT,     KC_ENT,
+        KC_LSFT,KC_Z,   KC_X,   KC_C,   KC_V,   KC_B,   KC_N,   KC_M,   KC_COMM,KC_DOT, KC_SLSH,KC_RSFT,     KC_UP,
+        KC_CAPS,KC_LGUI,KC_LALT,          KC_SPC,                       KC_RALT,KC_RCTL,MO(1),       KC_LEFT,KC_DOWN,KC_RGHT
+    ),
+  [1] = LAYOUT(
+        KC_GRV, KC_F1,  KC_F2,  KC_F3,  KC_F4,  KC_F5,  KC_F6,  KC_F7,  KC_F8,  KC_F9,  KC_F10, KC_F11, KC_F12, _______,     _______,
+        _______,_______,_______,_______,_______,_______,_______,_______,_______,_______,KC_PSCR,KC_SLCK,KC_PAUS,_______,     _______,
+        _______,_______,_______,_______,_______,_______,_______,_______,_______,_______,_______,_______,     _______,
+        _______,_______,_______,_______,_______,_______,_______,KC_MUTE,KC_MPRV,KC_MNXT,KC_MPLY,_______,     KC_PGUP,
+        _______,_______,_______,                _______,                _______,_______,_______,     KC_HOME,KC_PGDN,KC_END
+    )
+};

keyboards/fc660c/keymaps/mikefightsbears/rules.mk

@@ -0,0 +1,6 @@
+# Build Options
+# #   change to "no" to disable the options, or define them in the Makefile in
+# #   the appropriate keymap folder that will get included automatically
+# #
+EXTRAKEY_ENABLE = yes       # Audio control and System control
+NKRO_ENABLE = yes           # Nkey Rollover - if this doesn't work, see here: https://github.com/tmk/tmk_keyboard/wiki/FAQ#nkro-doesnt-work

keyboards/kbdfans/kbd67/mkiirgb/keymaps/mikefightsbears/keymap.c

@@ -10,10 +10,10 @@ const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {
 	),
 	[1] = LAYOUT_65_ansi_blocker(
 		KC_GRV,  KC_F1,    KC_F2,   KC_F3,   KC_F4,   KC_F5,   KC_F6,   KC_F7,   KC_F8,   KC_F9,   KC_F10,  KC_F11,  KC_F12,  KC_DEL,  KC_DEL,  
-		KC_TRNS, KC_TRNS,  RGB_VAI, KC_TRNS, KC_TRNS, RGB_SAI, RGB_HUI, KC_TRNS, KC_TRNS, KC_TRNS, KC_PSCR, KC_SLCK, KC_PAUS, KC_TRNS, KC_END,  
+		_______, _______,  RGB_VAI, _______, _______, RGB_SAI, RGB_HUI, _______, _______, _______, KC_PSCR, KC_SLCK, KC_PAUS, _______, KC_END,  
-		KC_TRNS, RGB_SPD,  RGB_VAD, RGB_SPI, KC_TRNS, RGB_SAD, RGB_HUD, KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS, 	       KC_VOLU, 
+		_______, RGB_SPD,  RGB_VAD, RGB_SPI, _______, RGB_SAD, RGB_HUD, _______, _______, _______, _______, _______, _______, 	       KC_VOLU, 
-		KC_TRNS, RGB_RMOD, RGB_TOG, RGB_MOD, KC_TRNS, KC_TRNS, KC_TRNS, KC_MUTE, KC_MPRV, KC_MNXT, KC_MPLY, KC_TRNS, 	      KC_PGUP, KC_VOLD, 
+		_______, RGB_RMOD, RGB_TOG, RGB_MOD, _______, _______, _______, KC_MUTE, KC_MPRV, KC_MNXT, KC_MPLY, _______, 	      KC_PGUP, KC_VOLD, 
-		KC_TRNS, KC_TRNS,  KC_TRNS, KC_TRNS, 						  KC_TRNS, KC_TRNS,	     KC_HOME, KC_PGDN, KC_END   
+		_______, _______,  _______, _______, 						  _______, _______,	     KC_HOME, KC_PGDN, KC_END   
 	)
 };
 

lib/python/qmk/cli/cformat.py

@@ -29,6 +29,7 @@ def cformat(cli):
             for dirpath, dirnames, filenames in os.walk(dir):
                 if any(i in dirpath for i in ignores):
                     dirnames.clear()
+                    continue
 
                 for name in filenames:
                     if name.endswith(('.c', '.h', '.cpp')):

lib/python/qmk/questions.py

@@ -16,7 +16,7 @@ def yesno(prompt, *args, default=None, **kwargs):
 
     Arguments:
         prompt
-            The prompt to present to the user. Can include ANSI and format strings like milc's `cli.print()`.
+            The prompt to present to the user. Can include ANSI and format strings like milc's `cli.echo()`.
 
         default
             Whether to default to a Yes or No when the user presses enter.
@@ -43,9 +43,9 @@ def yesno(prompt, *args, default=None, **kwargs):
             prompt = prompt + ' [y/N] '
 
     while True:
-        print()
+        cli.echo('')
         answer = input(format_ansi(prompt % args))
-        print()
+        cli.echo('')
 
         if not answer and prompt is not None:
             return default
@@ -57,17 +57,26 @@ def yesno(prompt, *args, default=None, **kwargs):
             return False
 
 
-def question(prompt, *args, default=None, confirm=False, answer_type=str, **kwargs):
+def question(prompt, *args, default=None, confirm=False, answer_type=str, validate=None, **kwargs):
     """Prompt the user to answer a question with a free-form input.
 
+    Arguments:
         prompt
-            The prompt to present to the user. Can include ANSI and format strings like milc's `cli.print()`.
+            The prompt to present to the user. Can include ANSI and format strings like milc's `cli.echo()`.
 
         default
             The value to return when the user doesn't enter any value. Use None to prompt until they enter a value.
 
+        confirm
+            Present the user with a confirmation dialog before accepting their answer.
+
         answer_type
             Specify a type function for the answer. Will re-prompt the user if the function raises any errors. Common choices here include int, float, and decimal.Decimal.
+
+        validate
+            This is an optional function that can be used to validate the answer. It should return True or False and have the following signature:
+
+                def function_name(answer, *args, **kwargs):
     """
     if not args and kwargs:
         args = kwargs
@@ -76,17 +85,21 @@ def question(prompt, *args, default=None, confirm=False, answer_type=str, **kwar
         prompt = '%s [%s] ' % (prompt, default)
 
     while True:
-        print()
+        cli.echo('')
         answer = input(format_ansi(prompt % args))
-        print()
+        cli.echo('')
 
         if answer:
-            if confirm:
+            if validate is not None and not validate(answer, *args, **kwargs):
+                continue
+
+            elif confirm:
                 if yesno('Is the answer "%s" correct?', answer, default=True):
                     try:
                         return answer_type(answer)
                     except Exception as e:
                         cli.log.error('Could not convert answer (%s) to type %s: %s', answer, answer_type.__name__, str(e))
+
             else:
                 try:
                     return answer_type(answer)
@@ -95,3 +108,71 @@ def question(prompt, *args, default=None, confirm=False, answer_type=str, **kwar
 
         elif default is not None:
             return default
+
+
+def choice(heading, options, *args, default=None, confirm=False, prompt='Please enter your choice: ', **kwargs):
+    """Present the user with a list of options and let them pick one.
+
+    Users can enter either the number or the text of their choice.
+
+    This will return the value of the item they choose, not the numerical index.
+
+    Arguments:
+        heading
+            The text to place above the list of options.
+
+        options
+            A sequence of items to choose from.
+
+        default
+            The index of the item to return when the user doesn't enter any value. Use None to prompt until they enter a value.
+
+        confirm
+            Present the user with a confirmation dialog before accepting their answer.
+
+        prompt
+            The prompt to present to the user. Can include ANSI and format strings like milc's `cli.echo()`.
+    """
+    if not args and kwargs:
+        args = kwargs
+
+    if prompt and default:
+        prompt = prompt + ' [%s] ' % (default + 1,)
+
+    while True:
+        # Prompt for an answer.
+        cli.echo('')
+        cli.echo(heading % args)
+        cli.echo('')
+        for i, option in enumerate(options, 1):
+            cli.echo('\t{fg_cyan}%d.{fg_reset} %s', i, option)
+
+        cli.echo('')
+        answer = input(format_ansi(prompt))
+        cli.echo('')
+
+        # If the user types in one of the options exactly use that
+        if answer in options:
+            return answer
+
+        # Massage the answer into a valid integer
+        if answer == '' and default:
+            answer = default
+        else:
+            try:
+                answer = int(answer) - 1
+            except Exception:
+                # Normally we would log the exception here, but in the interest of clean UI we do not.
+                cli.log.error('Invalid choice: %s', answer + 1)
+                continue
+
+        # Validate the answer
+        if answer >= len(options) or answer < 0:
+            cli.log.error('Invalid choice: %s', answer + 1)
+            continue
+
+        if confirm and not yesno('Is the answer "%s" correct?', answer + 1, default=True):
+            continue
+
+        # Return the answer they chose.
+        return options[answer]

message.mk

@@ -54,11 +54,8 @@ MSG_COMPILING_CXX = Compiling:
 MSG_ASSEMBLING = Assembling:
 MSG_CLEANING = Cleaning project:
 MSG_CREATING_LIBRARY = Creating library:
-MSG_SUBMODULE_DIRTY = $(WARN_COLOR)WARNING:$(NO_COLOR)\n \
+MSG_SUBMODULE_DIRTY = $(WARN_COLOR)WARNING:$(NO_COLOR) Some git submodules are out of date or modified.\n\
-	Some git sub-modules are out of date or modified, please consider running:$(BOLD)\n\
+Please consider running $(BOLD)make git-submodule$(NO_COLOR).\n\n
-        make git-submodule\n\
-	You can ignore this warning if you are not compiling any ChibiOS keyboards,\n\
-	or if you have modified the ChibiOS libraries yourself. \n\n$(NO_COLOR)
 MSG_NO_CMP = $(ERROR_COLOR)Error:$(NO_COLOR)$(BOLD) cmp command not found, please install diffutils\n$(NO_COLOR)
 
 define GENERATE_MSG_MAKE_KB

tmk_core/common/report.h

@@ -150,6 +150,11 @@ typedef union {
 #endif
 } __attribute__((packed)) report_keyboard_t;
 
+typedef struct {
+    uint8_t  report_id;
+    uint16_t usage;
+} __attribute__((packed)) report_extra_t;
+
 typedef struct {
 #ifdef MOUSE_SHARED_EP
     uint8_t report_id;

tmk_core/protocol/arm_atsam/main_arm_atsam.c

@@ -110,40 +110,34 @@ void send_mouse(report_mouse_t *report) {
 #endif  // MOUSEKEY_ENABLE
 }
 
-void send_system(uint16_t data) {
 #ifdef EXTRAKEY_ENABLE
+void send_extra(uint8_t report_id, uint16_t data) {
     uint32_t irqflags;
 
     irqflags = __get_PRIMASK();
     __disable_irq();
     __DMB();
 
-    udi_hid_exk_report.desc.report_id = REPORT_ID_SYSTEM;
+    udi_hid_exk_report.desc.report_id   = report_id;
-    if (data != 0) data = data - SYSTEM_POWER_DOWN + 1;
     udi_hid_exk_report.desc.report_data = data;
     udi_hid_exk_b_report_valid          = 1;
     udi_hid_exk_send_report();
 
     __DMB();
     __set_PRIMASK(irqflags);
+}
+#endif  // EXTRAKEY_ENABLE
+
+void send_system(uint16_t data) {
+#ifdef EXTRAKEY_ENABLE
+    if (data != 0) data = data - SYSTEM_POWER_DOWN + 1;
+    send_extra(REPORT_ID_SYSTEM, data);
 #endif  // EXTRAKEY_ENABLE
 }
 
 void send_consumer(uint16_t data) {
 #ifdef EXTRAKEY_ENABLE
-    uint32_t irqflags;
+    send_extra(REPORT_ID_CONSUMER, data);
-
-    irqflags = __get_PRIMASK();
-    __disable_irq();
-    __DMB();
-
-    udi_hid_exk_report.desc.report_id   = REPORT_ID_CONSUMER;
-    udi_hid_exk_report.desc.report_data = data;
-    udi_hid_exk_b_report_valid          = 1;
-    udi_hid_exk_send_report();
-
-    __DMB();
-    __set_PRIMASK(irqflags);
 #endif  // EXTRAKEY_ENABLE
 }
 

tmk_core/protocol/chibios/usb_main.h

@@ -72,20 +72,6 @@ void mouse_in_cb(USBDriver *usbp, usbep_t ep);
 /* shared IN request callback handler */
 void shared_in_cb(USBDriver *usbp, usbep_t ep);
 
-/* ---------------
- * Extrakey header
- * ---------------
- */
-
-#ifdef EXTRAKEY_ENABLE
-
-/* extra report structure */
-typedef struct {
-    uint8_t  report_id;
-    uint16_t usage;
-} __attribute__((packed)) report_extra_t;
-#endif /* EXTRAKEY_ENABLE */
-
 /* --------------
  * Console header
  * --------------

tmk_core/protocol/lufa/lufa.c

@@ -662,17 +662,17 @@ static void send_mouse(report_mouse_t *report) {
 #endif
 }
 
-/** \brief Send System
+/** \brief Send Extra
  *
  * FIXME: Needs doc
  */
-static void send_system(uint16_t data) {
 #ifdef EXTRAKEY_ENABLE
+static void send_extra(uint8_t report_id, uint16_t data) {
     uint8_t timeout = 255;
 
     if (USB_DeviceState != DEVICE_STATE_Configured) return;
 
-    report_extra_t r = {.report_id = REPORT_ID_SYSTEM, .usage = data - SYSTEM_POWER_DOWN + 1};
+    report_extra_t r = {.report_id = report_id, .usage = data};
     Endpoint_SelectEndpoint(SHARED_IN_EPNUM);
 
     /* Check if write ready for a polling interval around 10ms */
@@ -681,6 +681,16 @@ static void send_system(uint16_t data) {
 
     Endpoint_Write_Stream_LE(&r, sizeof(report_extra_t), NULL);
     Endpoint_ClearIN();
+}
+#endif
+
+/** \brief Send System
+ *
+ * FIXME: Needs doc
+ */
+static void send_system(uint16_t data) {
+#ifdef EXTRAKEY_ENABLE
+    send_extra(REPORT_ID_SYSTEM, data - SYSTEM_POWER_DOWN + 1);
 #endif
 }
 
@@ -690,8 +700,7 @@ static void send_system(uint16_t data) {
  */
 static void send_consumer(uint16_t data) {
 #ifdef EXTRAKEY_ENABLE
-    uint8_t timeout = 255;
+    uint8_t where = where_to_send();
-    uint8_t where   = where_to_send();
 
 #    ifdef BLUETOOTH_ENABLE
     if (where == OUTPUT_BLUETOOTH || where == OUTPUT_USB_AND_BT) {
@@ -729,15 +738,7 @@ static void send_consumer(uint16_t data) {
         return;
     }
 
-    report_extra_t r = {.report_id = REPORT_ID_CONSUMER, .usage = data};
+    send_extra(REPORT_ID_CONSUMER, data);
-    Endpoint_SelectEndpoint(SHARED_IN_EPNUM);
-
-    /* Check if write ready for a polling interval around 10ms */
-    while (timeout-- && !Endpoint_IsReadWriteAllowed()) _delay_us(40);
-    if (!Endpoint_IsReadWriteAllowed()) return;
-
-    Endpoint_Write_Stream_LE(&r, sizeof(report_extra_t), NULL);
-    Endpoint_ClearIN();
 #endif
 }
 

tmk_core/protocol/lufa/lufa.h

@@ -58,12 +58,6 @@ extern host_driver_t lufa_driver;
 }
 #endif
 
-/* extra report structure */
-typedef struct {
-    uint8_t  report_id;
-    uint16_t usage;
-} __attribute__((packed)) report_extra_t;
-
 #ifdef API_ENABLE
 #    include "api.h"
 #endif

tmk_core/protocol/vusb/vusb.c

@@ -112,32 +112,22 @@ static void send_mouse(report_mouse_t *report) {
     }
 }
 
-typedef struct {
+static void send_extra(uint8_t report_id, uint16_t data) {
-    uint8_t  report_id;
+    static uint8_t  last_id   = 0;
-    uint16_t usage;
-} __attribute__((packed)) report_extra_t;
-
-static void send_system(uint16_t data) {
     static uint16_t last_data = 0;
-    if (data == last_data) return;
+    if ((report_id == last_id) && (data == last_data)) return;
+    last_id   = report_id;
     last_data = data;
 
-    report_extra_t report = {.report_id = REPORT_ID_SYSTEM, .usage = data};
+    report_extra_t report = {.report_id = report_id, .usage = data};
     if (usbInterruptIsReady3()) {
         usbSetInterrupt3((void *)&report, sizeof(report));
     }
 }
 
-static void send_consumer(uint16_t data) {
+static void send_system(uint16_t data) { send_extra(REPORT_ID_SYSTEM, data); }
-    static uint16_t last_data = 0;
-    if (data == last_data) return;
-    last_data = data;
 
-    report_extra_t report = {.report_id = REPORT_ID_CONSUMER, .usage = data};
+static void send_consumer(uint16_t data) { send_extra(REPORT_ID_CONSUMER, data); }
-    if (usbInterruptIsReady3()) {
-        usbSetInterrupt3((void *)&report, sizeof(report));
-    }
-}
 
 /*------------------------------------------------------------------*
  * Request from host                                                *