From 97336455b5fd24bf09eeea3da7bf4432887c80b1 Mon Sep 17 00:00:00 2001 From: Eric Tang Date: Mon, 7 Aug 2017 11:19:41 -0700 Subject: [PATCH 01/10] Verify a new configuration before applying it --- right/src/config_parser/config_state.c | 7 +++++-- right/src/config_parser/config_state.h | 1 + right/src/config_parser/parse_keymap.c | 2 +- right/src/usb_protocol_handler.c | 19 ++++++++++++++----- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/right/src/config_parser/config_state.c b/right/src/config_parser/config_state.c index 79e6b12..564e76d 100644 --- a/right/src/config_parser/config_state.c +++ b/right/src/config_parser/config_state.c @@ -3,8 +3,11 @@ static uint8_t hardwareConfig[HARDWARE_CONFIG_SIZE]; config_buffer_t HardwareConfigBuffer = {hardwareConfig}; -static uint8_t userConfig[USER_CONFIG_SIZE]; -config_buffer_t UserConfigBuffer = {userConfig}; +static uint8_t userConfig1[USER_CONFIG_SIZE]; +static uint8_t userConfig2[USER_CONFIG_SIZE]; + +config_buffer_t UserConfigBuffer = { userConfig1 }; +config_buffer_t NewUserConfigBuffer = { userConfig2 }; uint8_t readUInt8(config_buffer_t *buffer) { return buffer->buffer[buffer->offset++]; diff --git a/right/src/config_parser/config_state.h b/right/src/config_parser/config_state.h index 1af70b8..f92630d 100644 --- a/right/src/config_parser/config_state.h +++ b/right/src/config_parser/config_state.h @@ -23,6 +23,7 @@ extern config_buffer_t HardwareConfigBuffer; extern config_buffer_t UserConfigBuffer; + extern config_buffer_t NewUserConfigBuffer; // Functions: diff --git a/right/src/config_parser/parse_keymap.c b/right/src/config_parser/parse_keymap.c index 15e51dc..1ae18ef 100644 --- a/right/src/config_parser/parse_keymap.c +++ b/right/src/config_parser/parse_keymap.c @@ -193,7 +193,7 @@ parser_error_t ParseKeymap(config_buffer_t *buffer) {; if (layerCount != LAYER_COUNT) { return ParserError_InvalidLayerCount; } - isDryRun = !isDefault; + isDryRun = buffer == &NewUserConfigBuffer || !isDefault; if (!isDryRun) { LedDisplay_SetText(abbreviationLen, abbreviation); } diff --git a/right/src/usb_protocol_handler.c b/right/src/usb_protocol_handler.c index 778ec7f..1d12f29 100644 --- a/right/src/usb_protocol_handler.c +++ b/right/src/usb_protocol_handler.c @@ -85,10 +85,19 @@ void readMergeSensor(void) void applyConfig(void) { - UserConfigBuffer.offset = 0; - GenericHidOutBuffer[0] = ParseConfig(&UserConfigBuffer); - GenericHidOutBuffer[1] = UserConfigBuffer.offset; - GenericHidOutBuffer[2] = UserConfigBuffer.offset >> 8; + uint8_t *temp; + + NewUserConfigBuffer.offset = 0; + GenericHidOutBuffer[0] = ParseConfig(&NewUserConfigBuffer); + GenericHidOutBuffer[1] = NewUserConfigBuffer.offset; + GenericHidOutBuffer[2] = NewUserConfigBuffer.offset >> 8; + if (GenericHidOutBuffer[0] == ParserError_Success) { + temp = UserConfigBuffer.buffer; + UserConfigBuffer.buffer = NewUserConfigBuffer.buffer; + NewUserConfigBuffer.buffer = temp; + UserConfigBuffer.offset = 0; + ParseConfig(&UserConfigBuffer); + } } void setLedPwm(void) @@ -144,7 +153,7 @@ void writeConfiguration(bool isHardware) return; } - uint8_t *buffer = isHardware ? HardwareConfigBuffer.buffer : UserConfigBuffer.buffer; + uint8_t *buffer = isHardware ? HardwareConfigBuffer.buffer : NewUserConfigBuffer.buffer; uint16_t bufferLength = isHardware ? HARDWARE_CONFIG_SIZE : USER_CONFIG_SIZE; if (offset + length > bufferLength) { From d035c8699b02ae28f94dcb3aea0dbb8b2e4ee405 Mon Sep 17 00:00:00 2001 From: Eric Tang Date: Mon, 7 Aug 2017 16:40:21 -0700 Subject: [PATCH 02/10] Try putting userConfig2 in the upper half of the RAM --- right/src/config_parser/config_state.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/right/src/config_parser/config_state.c b/right/src/config_parser/config_state.c index 564e76d..2912750 100644 --- a/right/src/config_parser/config_state.c +++ b/right/src/config_parser/config_state.c @@ -4,7 +4,7 @@ static uint8_t hardwareConfig[HARDWARE_CONFIG_SIZE]; config_buffer_t HardwareConfigBuffer = {hardwareConfig}; static uint8_t userConfig1[USER_CONFIG_SIZE]; -static uint8_t userConfig2[USER_CONFIG_SIZE]; +static uint8_t __attribute__((section (".m_data_2"))) userConfig2[USER_CONFIG_SIZE]; config_buffer_t UserConfigBuffer = { userConfig1 }; config_buffer_t NewUserConfigBuffer = { userConfig2 }; From 036b5c41725b440742c0a9d9e17bdb2975a6cce9 Mon Sep 17 00:00:00 2001 From: Eric Tang Date: Mon, 7 Aug 2017 17:39:14 -0700 Subject: [PATCH 03/10] Cut the lengths of userConfig1 and userConfig2 in half for now --- right/src/config_parser/config_state.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/right/src/config_parser/config_state.c b/right/src/config_parser/config_state.c index 2912750..5ab3ddb 100644 --- a/right/src/config_parser/config_state.c +++ b/right/src/config_parser/config_state.c @@ -3,8 +3,8 @@ static uint8_t hardwareConfig[HARDWARE_CONFIG_SIZE]; config_buffer_t HardwareConfigBuffer = {hardwareConfig}; -static uint8_t userConfig1[USER_CONFIG_SIZE]; -static uint8_t __attribute__((section (".m_data_2"))) userConfig2[USER_CONFIG_SIZE]; +static uint8_t userConfig1[USER_CONFIG_SIZE / 2]; +static uint8_t __attribute__((section (".m_data_2"))) userConfig2[USER_CONFIG_SIZE / 2]; config_buffer_t UserConfigBuffer = { userConfig1 }; config_buffer_t NewUserConfigBuffer = { userConfig2 }; From 29fabe5b2e4841dc82c24bcd7ccb594800009cf4 Mon Sep 17 00:00:00 2001 From: Eric Tang Date: Mon, 7 Aug 2017 18:18:42 -0700 Subject: [PATCH 04/10] Revert "Cut the lengths of userConfig1 and userConfig2 in half for now" This reverts commit 036b5c41725b440742c0a9d9e17bdb2975a6cce9. --- right/src/config_parser/config_state.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/right/src/config_parser/config_state.c b/right/src/config_parser/config_state.c index 5ab3ddb..2912750 100644 --- a/right/src/config_parser/config_state.c +++ b/right/src/config_parser/config_state.c @@ -3,8 +3,8 @@ static uint8_t hardwareConfig[HARDWARE_CONFIG_SIZE]; config_buffer_t HardwareConfigBuffer = {hardwareConfig}; -static uint8_t userConfig1[USER_CONFIG_SIZE / 2]; -static uint8_t __attribute__((section (".m_data_2"))) userConfig2[USER_CONFIG_SIZE / 2]; +static uint8_t userConfig1[USER_CONFIG_SIZE]; +static uint8_t __attribute__((section (".m_data_2"))) userConfig2[USER_CONFIG_SIZE]; config_buffer_t UserConfigBuffer = { userConfig1 }; config_buffer_t NewUserConfigBuffer = { userConfig2 }; From 0f2c7d53c9fe9fc7f1613c2dbe1046704ffd5843 Mon Sep 17 00:00:00 2001 From: Erich Styger Date: Tue, 8 Aug 2017 13:49:10 +0200 Subject: [PATCH 05/10] added section for custom user configuration used in right/src/config_parser/config_state.c --- right/build/kds/MK22FN512xxx12_flash.ld | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/right/build/kds/MK22FN512xxx12_flash.ld b/right/build/kds/MK22FN512xxx12_flash.ld index 7226037..4fad685 100644 --- a/right/build/kds/MK22FN512xxx12_flash.ld +++ b/right/build/kds/MK22FN512xxx12_flash.ld @@ -214,7 +214,14 @@ SECTIONS __bss_end__ = .; __END_BSS = .; } > m_data - + + .m_data_2 : + { + . = ALIGN(4); + *(.m_data_2) /* This is an User defined section */ + . = ALIGN(4); + } > m_data_2 + .heap : { . = ALIGN(8); From 079988146da40bf21af98cc12c68ad0134e973b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3=20Monda?= Date: Tue, 8 Aug 2017 19:20:01 +0200 Subject: [PATCH 06/10] Make the ParserRunDry global. Separate TestConfig() and ApplyConfig() --- right/src/config_parser/config_state.c | 10 ++++---- right/src/config_parser/config_state.h | 3 ++- right/src/config_parser/parse_keymap.c | 7 ++---- right/src/usb_protocol_handler.c | 32 +++++++++++++++----------- 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/right/src/config_parser/config_state.c b/right/src/config_parser/config_state.c index 2912750..0a299b9 100644 --- a/right/src/config_parser/config_state.c +++ b/right/src/config_parser/config_state.c @@ -3,11 +3,13 @@ static uint8_t hardwareConfig[HARDWARE_CONFIG_SIZE]; config_buffer_t HardwareConfigBuffer = {hardwareConfig}; -static uint8_t userConfig1[USER_CONFIG_SIZE]; -static uint8_t __attribute__((section (".m_data_2"))) userConfig2[USER_CONFIG_SIZE]; +static uint8_t userConfig[USER_CONFIG_SIZE]; +static uint8_t __attribute__((section (".m_data_2"))) stagingUserConfig[USER_CONFIG_SIZE]; -config_buffer_t UserConfigBuffer = { userConfig1 }; -config_buffer_t NewUserConfigBuffer = { userConfig2 }; +config_buffer_t UserConfigBuffer = { userConfig }; +config_buffer_t StagingUserConfigBuffer = { stagingUserConfig }; + +bool ParserRunDry; uint8_t readUInt8(config_buffer_t *buffer) { return buffer->buffer[buffer->offset++]; diff --git a/right/src/config_parser/config_state.h b/right/src/config_parser/config_state.h index f92630d..84dfd00 100644 --- a/right/src/config_parser/config_state.h +++ b/right/src/config_parser/config_state.h @@ -21,9 +21,10 @@ // Variables: + extern bool ParserRunDry; extern config_buffer_t HardwareConfigBuffer; extern config_buffer_t UserConfigBuffer; - extern config_buffer_t NewUserConfigBuffer; + extern config_buffer_t StagingUserConfigBuffer; // Functions: diff --git a/right/src/config_parser/parse_keymap.c b/right/src/config_parser/parse_keymap.c index 1ae18ef..455ba26 100644 --- a/right/src/config_parser/parse_keymap.c +++ b/right/src/config_parser/parse_keymap.c @@ -3,8 +3,6 @@ #include "current_keymap.h" #include "led_display.h" -static bool isDryRun; - static parser_error_t parseNoneAction(key_action_t *keyAction, config_buffer_t *buffer) { keyAction->type = KeyActionType_None; return ParserError_Success; @@ -146,7 +144,7 @@ static parser_error_t parseKeyActions(uint8_t targetLayer, config_buffer_t *buff return ParserError_InvalidActionCount; } for (uint16_t actionIdx = 0; actionIdx < actionCount; actionIdx++) { - errorCode = parseKeyAction(isDryRun ? &dummyKeyAction : &CurrentKeymap[targetLayer][moduleId][actionIdx], buffer); + errorCode = parseKeyAction(ParserRunDry ? &dummyKeyAction : &CurrentKeymap[targetLayer][moduleId][actionIdx], buffer); if (errorCode != ParserError_Success) { return errorCode; } @@ -193,8 +191,7 @@ parser_error_t ParseKeymap(config_buffer_t *buffer) {; if (layerCount != LAYER_COUNT) { return ParserError_InvalidLayerCount; } - isDryRun = buffer == &NewUserConfigBuffer || !isDefault; - if (!isDryRun) { + if (!ParserRunDry) { LedDisplay_SetText(abbreviationLen, abbreviation); } for (uint16_t layerIdx = 0; layerIdx < layerCount; layerIdx++) { diff --git a/right/src/usb_protocol_handler.c b/right/src/usb_protocol_handler.c index 1d12f29..b2b5917 100644 --- a/right/src/usb_protocol_handler.c +++ b/right/src/usb_protocol_handler.c @@ -83,21 +83,25 @@ void readMergeSensor(void) SetResponseByte(MERGE_SENSOR_IS_MERGED); } +// TODO: Expose this as a separate USB command and make Agent call it and check its output before calling applyConfig. +void TestConfig(void) +{ + ParserRunDry = true; + StagingUserConfigBuffer.offset = 0; + GenericHidOutBuffer[0] = ParseConfig(&StagingUserConfigBuffer); + GenericHidOutBuffer[1] = StagingUserConfigBuffer.offset; + GenericHidOutBuffer[2] = StagingUserConfigBuffer.offset >> 8; +} + void applyConfig(void) { - uint8_t *temp; - - NewUserConfigBuffer.offset = 0; - GenericHidOutBuffer[0] = ParseConfig(&NewUserConfigBuffer); - GenericHidOutBuffer[1] = NewUserConfigBuffer.offset; - GenericHidOutBuffer[2] = NewUserConfigBuffer.offset >> 8; - if (GenericHidOutBuffer[0] == ParserError_Success) { - temp = UserConfigBuffer.buffer; - UserConfigBuffer.buffer = NewUserConfigBuffer.buffer; - NewUserConfigBuffer.buffer = temp; - UserConfigBuffer.offset = 0; - ParseConfig(&UserConfigBuffer); - } + TestConfig(); // This line will be removed. TestConfig will be called by Agent separately. + memcpy(&UserConfigBuffer, &StagingUserConfigBuffer, USER_CONFIG_SIZE); + ParserRunDry = false; + UserConfigBuffer.offset = 0; + GenericHidOutBuffer[0] = ParseConfig(&UserConfigBuffer); + GenericHidOutBuffer[1] = UserConfigBuffer.offset; + GenericHidOutBuffer[2] = UserConfigBuffer.offset >> 8; } void setLedPwm(void) @@ -153,7 +157,7 @@ void writeConfiguration(bool isHardware) return; } - uint8_t *buffer = isHardware ? HardwareConfigBuffer.buffer : NewUserConfigBuffer.buffer; + uint8_t *buffer = isHardware ? HardwareConfigBuffer.buffer : StagingUserConfigBuffer.buffer; uint16_t bufferLength = isHardware ? HARDWARE_CONFIG_SIZE : USER_CONFIG_SIZE; if (offset + length > bufferLength) { From 406ea3782dd95b0fc204c1cfed197cfc77f2af9a Mon Sep 17 00:00:00 2001 From: Eric Tang Date: Tue, 8 Aug 2017 10:54:56 -0700 Subject: [PATCH 07/10] Move back to swapping pointers --- right/src/config_parser/config_state.c | 9 ++++----- right/src/usb_protocol_handler.c | 6 +++++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/right/src/config_parser/config_state.c b/right/src/config_parser/config_state.c index 0a299b9..3bb01c3 100644 --- a/right/src/config_parser/config_state.c +++ b/right/src/config_parser/config_state.c @@ -3,11 +3,10 @@ static uint8_t hardwareConfig[HARDWARE_CONFIG_SIZE]; config_buffer_t HardwareConfigBuffer = {hardwareConfig}; -static uint8_t userConfig[USER_CONFIG_SIZE]; -static uint8_t __attribute__((section (".m_data_2"))) stagingUserConfig[USER_CONFIG_SIZE]; - -config_buffer_t UserConfigBuffer = { userConfig }; -config_buffer_t StagingUserConfigBuffer = { stagingUserConfig }; +static uint8_t userConfig1[USER_CONFIG_SIZE]; +static uint8_t __attribute__((section (".m_data_2"))) userConfig2[USER_CONFIG_SIZE]; +config_buffer_t UserConfigBuffer = { userConfig1 }; +config_buffer_t StagingUserConfigBuffer = { userConfig2 }; bool ParserRunDry; diff --git a/right/src/usb_protocol_handler.c b/right/src/usb_protocol_handler.c index b2b5917..cbbce18 100644 --- a/right/src/usb_protocol_handler.c +++ b/right/src/usb_protocol_handler.c @@ -95,9 +95,13 @@ void TestConfig(void) void applyConfig(void) { + uint8_t *temp; + TestConfig(); // This line will be removed. TestConfig will be called by Agent separately. - memcpy(&UserConfigBuffer, &StagingUserConfigBuffer, USER_CONFIG_SIZE); ParserRunDry = false; + temp = UserConfigBuffer.buffer; + UserConfigBuffer.buffer = StagingUserConfigBuffer.buffer; + StagingUserConfigBuffer.buffer = temp; UserConfigBuffer.offset = 0; GenericHidOutBuffer[0] = ParseConfig(&UserConfigBuffer); GenericHidOutBuffer[1] = UserConfigBuffer.offset; From 2dd0f589db41fb53992e6c1ad4fb06e7799162c3 Mon Sep 17 00:00:00 2001 From: Eric Tang Date: Tue, 8 Aug 2017 11:27:15 -0700 Subject: [PATCH 08/10] Inline TestConfig --- right/src/usb_protocol_handler.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/right/src/usb_protocol_handler.c b/right/src/usb_protocol_handler.c index cbbce18..9645572 100644 --- a/right/src/usb_protocol_handler.c +++ b/right/src/usb_protocol_handler.c @@ -83,21 +83,15 @@ void readMergeSensor(void) SetResponseByte(MERGE_SENSOR_IS_MERGED); } -// TODO: Expose this as a separate USB command and make Agent call it and check its output before calling applyConfig. -void TestConfig(void) +void applyConfig(void) { + uint8_t *temp; + ParserRunDry = true; StagingUserConfigBuffer.offset = 0; GenericHidOutBuffer[0] = ParseConfig(&StagingUserConfigBuffer); GenericHidOutBuffer[1] = StagingUserConfigBuffer.offset; GenericHidOutBuffer[2] = StagingUserConfigBuffer.offset >> 8; -} - -void applyConfig(void) -{ - uint8_t *temp; - - TestConfig(); // This line will be removed. TestConfig will be called by Agent separately. ParserRunDry = false; temp = UserConfigBuffer.buffer; UserConfigBuffer.buffer = StagingUserConfigBuffer.buffer; From 521c84d326d06810bd706763d0d8f01b1f0bc121 Mon Sep 17 00:00:00 2001 From: Eric Tang Date: Tue, 8 Aug 2017 11:31:47 -0700 Subject: [PATCH 09/10] Set GenericHidOutBuffer[3] to 1 if an attempt was made to apply the configuration --- right/src/usb_protocol_handler.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/right/src/usb_protocol_handler.c b/right/src/usb_protocol_handler.c index 9645572..c1f7c08 100644 --- a/right/src/usb_protocol_handler.c +++ b/right/src/usb_protocol_handler.c @@ -92,6 +92,10 @@ void applyConfig(void) GenericHidOutBuffer[0] = ParseConfig(&StagingUserConfigBuffer); GenericHidOutBuffer[1] = StagingUserConfigBuffer.offset; GenericHidOutBuffer[2] = StagingUserConfigBuffer.offset >> 8; + GenericHidOutBuffer[3] = 0; + if (GenericHidOutBuffer[0]) { + return; + } ParserRunDry = false; temp = UserConfigBuffer.buffer; UserConfigBuffer.buffer = StagingUserConfigBuffer.buffer; @@ -100,6 +104,7 @@ void applyConfig(void) GenericHidOutBuffer[0] = ParseConfig(&UserConfigBuffer); GenericHidOutBuffer[1] = UserConfigBuffer.offset; GenericHidOutBuffer[2] = UserConfigBuffer.offset >> 8; + GenericHidOutBuffer[3] = 1; } void setLedPwm(void) From 711769cb50fed87c48211960ed7a141643232145 Mon Sep 17 00:00:00 2001 From: Eric Tang Date: Tue, 8 Aug 2017 11:42:35 -0700 Subject: [PATCH 10/10] Add some temporary code to ensure that only the default keymap is applied --- right/src/config_parser/parse_keymap.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/right/src/config_parser/parse_keymap.c b/right/src/config_parser/parse_keymap.c index 455ba26..27695cd 100644 --- a/right/src/config_parser/parse_keymap.c +++ b/right/src/config_parser/parse_keymap.c @@ -185,21 +185,28 @@ parser_error_t ParseKeymap(config_buffer_t *buffer) {; const char *name = readString(buffer, &nameLen); const char *description = readString(buffer, &descriptionLen); uint16_t layerCount = readCompactLength(buffer); + bool temp; (void)name; (void)description; if (layerCount != LAYER_COUNT) { return ParserError_InvalidLayerCount; } + temp = ParserRunDry; + if (!isDefault) { + ParserRunDry = true; + } if (!ParserRunDry) { LedDisplay_SetText(abbreviationLen, abbreviation); } for (uint16_t layerIdx = 0; layerIdx < layerCount; layerIdx++) { errorCode = parseLayer(buffer, layerIdx); if (errorCode != ParserError_Success) { + ParserRunDry = temp; return errorCode; } } + ParserRunDry = temp; return ParserError_Success; }