Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in UC_RMOD, add shift and audio support for UC_MOD/UC_RMOD #8674

Merged
merged 6 commits into from
May 9, 2020
Merged

Fix bug in UC_RMOD, add shift and audio support for UC_MOD/UC_RMOD #8674

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3d4e696
Invert UC_MOD/UC_RMOD direction when Shift is held
vomindoraan Apr 4, 2020
17d2792
Allow audio to be played for UC_MOD, UC_RMOD keycodes as well
vomindoraan Apr 4, 2020
fba1e03
Fix signedness bug in reverse input mode cycling
vomindoraan Apr 4, 2020
b9f79c0
Misc formatting in process_unicode_common.c
vomindoraan Apr 4, 2020
e50da08
Address clang-format issues
vomindoraan Apr 25, 2020
65b3775
Make decode_utf8 helper function file-local (static)
vomindoraan May 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion quantum/process_keycode/process_rgb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ bool process_rgb(const uint16_t keycode, const keyrecord_t *record) {
// Split keyboards need to trigger on key-up for edge-case issue
if (!record->event.pressed) {
#endif
uint8_t shifted = get_mods() & (MOD_BIT(KC_LSHIFT) | MOD_BIT(KC_RSHIFT));
uint8_t shifted = get_mods() & MOD_MASK_SHIFT;
switch (keycode) {
case RGB_TOG:
rgblight_toggle();
Expand Down
116 changes: 69 additions & 47 deletions quantum/process_keycode/process_unicode_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ uint8_t unicode_saved_mods;

#if UNICODE_SELECTED_MODES != -1
static uint8_t selected[] = {UNICODE_SELECTED_MODES};
static uint8_t selected_count = sizeof selected / sizeof *selected;
static uint8_t selected_index;
static int8_t selected_count = sizeof selected / sizeof *selected;
static int8_t selected_index;
#endif

void unicode_input_mode_init(void) {
unicode_config.raw = eeprom_read_byte(EECONFIG_UNICODEMODE);
#if UNICODE_SELECTED_MODES != -1
# if UNICODE_CYCLE_PERSIST
// Find input_mode in selected modes
uint8_t i;
int8_t i;
for (i = 0; i < selected_count; i++) {
if (selected[i] == unicode_config.input_mode) {
selected_index = i;
Expand All @@ -60,9 +60,12 @@ void set_unicode_input_mode(uint8_t mode) {
dprintf("Unicode input mode set to: %u\n", unicode_config.input_mode);
}

void cycle_unicode_input_mode(uint8_t offset) {
void cycle_unicode_input_mode(int8_t offset) {
#if UNICODE_SELECTED_MODES != -1
selected_index = (selected_index + offset) % selected_count;
selected_index = (selected_index + offset) % selected_count;
if (selected_index < 0) {
selected_index += selected_count;
}
Comment on lines -63 to +68
Copy link
Contributor Author

@vomindoraan vomindoraan Apr 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explanation:
First off, since you're supposed to be able to cycle backward (using a negative offset) as well as forward, the parameter shouldn't be unsigned. The index calculation tries to take (selected_index + offset) mod selected_count, but only succeeds in doing so partially because the C % operator is a remainder, not a modulo operation. In other words, its result can be negative, and if it is, it needs to be shifted back into the [0, selected_count) range.

The current behavior of this section (prior to the changes) is equivalent to the following pseudocode:

if offset < 0 then
    selected_index ← (selected_index + offset + 1) mod selected_count 
else
    selected_index ← (selected_index + offset) mod selected_count
fi

This is a quirk of the operands being unsigned. Needless to say, it's not the desired behavior, and any arithmetic that expects logically negative operands should be changed to signed arithmetic.

unicode_config.input_mode = selected[selected_index];
# if UNICODE_CYCLE_PERSIST
persist_unicode_input_mode();
Expand Down Expand Up @@ -168,19 +171,20 @@ void register_hex32(uint32_t hex) {
}
}

// clang-format off

void send_unicode_hex_string(const char *str) {
if (!str) {
return;
}

while (*str) {
// Find the next code point (token) in the string
for (; *str == ' '; str++)
;
for (; *str == ' '; str++); // Skip leading spaces
size_t n = strcspn(str, " "); // Length of the current token
char code_point[n + 1];
strncpy(code_point, str, n);
code_point[n] = '\0'; // Make sure it's null-terminated
char code_point[n + 1];
strncpy(code_point, str, n); // Copy token into buffer
code_point[n] = '\0'; // Make sure it's null-terminated

// Normalize the code point: make all hex digits lowercase
for (char *p = code_point; *p; p++) {
Expand All @@ -196,6 +200,8 @@ void send_unicode_hex_string(const char *str) {
}
}

// clang-format on

// Borrowed from https://nullprogram.com/blog/2017/10/06/
const char *decode_utf8(const char *str, int32_t *code_point) {
const char *next;
Expand Down Expand Up @@ -231,7 +237,6 @@ void send_unicode_string(const char *str) {
}

int32_t code_point = 0;

while (*str) {
str = decode_utf8(str, &code_point);

Expand All @@ -243,53 +248,70 @@ void send_unicode_string(const char *str) {
}
}

// clang-format off

static void audio_helper(void) {
#ifdef AUDIO_ENABLE
switch (get_unicode_input_mode()) {
# ifdef UNICODE_SONG_MAC
static float song_mac[][2] = UNICODE_SONG_MAC;
case UC_MAC:
PLAY_SONG(song_mac);
break;
# endif
# ifdef UNICODE_SONG_LNX
static float song_lnx[][2] = UNICODE_SONG_LNX;
case UC_LNX:
PLAY_SONG(song_lnx);
break;
# endif
# ifdef UNICODE_SONG_WIN
static float song_win[][2] = UNICODE_SONG_WIN;
case UC_WIN:
PLAY_SONG(song_win);
break;
# endif
# ifdef UNICODE_SONG_BSD
static float song_bsd[][2] = UNICODE_SONG_BSD;
case UC_BSD:
PLAY_SONG(song_bsd);
break;
# endif
# ifdef UNICODE_SONG_WINC
static float song_winc[][2] = UNICODE_SONG_WINC;
case UC_WINC:
PLAY_SONG(song_winc);
break;
# endif
}
#endif
}

// clang-format on

bool process_unicode_common(uint16_t keycode, keyrecord_t *record) {
if (record->event.pressed) {
bool shifted = get_mods() & MOD_MASK_SHIFT;
switch (keycode) {
case UNICODE_MODE_FORWARD:
cycle_unicode_input_mode(+1);
cycle_unicode_input_mode(shifted ? -1 : +1);
audio_helper();
break;
case UNICODE_MODE_REVERSE:
cycle_unicode_input_mode(-1);
cycle_unicode_input_mode(shifted ? +1 : -1);
audio_helper();
break;

case UNICODE_MODE_MAC:
set_unicode_input_mode(UC_MAC);
#if defined(AUDIO_ENABLE) && defined(UNICODE_SONG_MAC)
static float song_mac[][2] = UNICODE_SONG_MAC;
PLAY_SONG(song_mac);
#endif
break;
case UNICODE_MODE_LNX:
set_unicode_input_mode(UC_LNX);
#if defined(AUDIO_ENABLE) && defined(UNICODE_SONG_LNX)
static float song_lnx[][2] = UNICODE_SONG_LNX;
PLAY_SONG(song_lnx);
#endif
break;
case UNICODE_MODE_WIN:
set_unicode_input_mode(UC_WIN);
#if defined(AUDIO_ENABLE) && defined(UNICODE_SONG_WIN)
static float song_win[][2] = UNICODE_SONG_WIN;
PLAY_SONG(song_win);
#endif
break;
case UNICODE_MODE_BSD:
set_unicode_input_mode(UC_BSD);
#if defined(AUDIO_ENABLE) && defined(UNICODE_SONG_BSD)
static float song_bsd[][2] = UNICODE_SONG_BSD;
PLAY_SONG(song_bsd);
#endif
break;
case UNICODE_MODE_WINC:
set_unicode_input_mode(UC_WINC);
#if defined(AUDIO_ENABLE) && defined(UNICODE_SONG_WINC)
static float song_winc[][2] = UNICODE_SONG_WINC;
PLAY_SONG(song_winc);
#endif
case UNICODE_MODE_MAC ... UNICODE_MODE_WINC: {
// Keycodes and input modes follow the same ordering
uint8_t delta = keycode - UNICODE_MODE_MAC;
set_unicode_input_mode(UC_MAC + delta);
audio_helper();
break;
}
}
}

#if defined(UNICODE_ENABLE)
return process_unicode(keycode, record);
#elif defined(UNICODEMAP_ENABLE)
Expand Down
2 changes: 1 addition & 1 deletion quantum/process_keycode/process_unicode_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ extern uint8_t unicode_saved_mods;
void unicode_input_mode_init(void);
uint8_t get_unicode_input_mode(void);
void set_unicode_input_mode(uint8_t mode);
void cycle_unicode_input_mode(uint8_t offset);
void cycle_unicode_input_mode(int8_t offset);
void persist_unicode_input_mode(void);

void unicode_input_start(void);
Expand Down
3 changes: 2 additions & 1 deletion quantum/process_keycode/process_unicodemap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ __attribute__((weak)) uint16_t unicodemap_index(uint16_t keycode) {
// Keycode is a pair: extract index based on Shift / Caps Lock state
uint16_t index = keycode - QK_UNICODEMAP_PAIR;

bool shift = unicode_saved_mods & MOD_MASK_SHIFT, caps = IS_HOST_LED_ON(USB_LED_CAPS_LOCK);
bool shift = unicode_saved_mods & MOD_MASK_SHIFT;
bool caps = IS_HOST_LED_ON(USB_LED_CAPS_LOCK);
if (shift ^ caps) {
index >>= 7;
}
Expand Down