From e12e219b4e9fef7cfa2fd33250fd78c88839c0ea Mon Sep 17 00:00:00 2001 From: Kristian Sloth Lauszus Date: Sun, 11 Mar 2018 19:19:15 +0100 Subject: [PATCH] Various fixes and improvements (#96) * Global variables shared between an interrupt and the main code should be volatile See: https://www.embedded.com/electronics-blogs/beginner-s-corner/4023801/Introduction-to-the-Volatile-Keyword * There is no reason to change the active report if it has not changed * Declare local functions and variables static This both helps the compiler and the programmer --- right/src/eeprom.c | 4 +- right/src/eeprom.h | 3 +- right/src/init_peripherals.c | 18 +++---- right/src/init_peripherals.h | 4 +- right/src/main.c | 8 +-- right/src/peripherals/adc.c | 2 +- right/src/timer.c | 2 +- right/src/timer.h | 2 +- .../usb_interface_basic_keyboard.c | 4 +- .../usb_interface_basic_keyboard.h | 2 +- .../usb_interface_media_keyboard.c | 4 +- .../usb_interface_media_keyboard.h | 2 +- .../src/usb_interfaces/usb_interface_mouse.c | 4 +- .../src/usb_interfaces/usb_interface_mouse.h | 2 +- .../usb_interface_system_keyboard.c | 4 +- .../usb_interface_system_keyboard.h | 2 +- right/src/usb_report_updater.c | 54 ++++++++++++------- 17 files changed, 69 insertions(+), 52 deletions(-) diff --git a/right/src/eeprom.c b/right/src/eeprom.c index c51181e..5dd0893 100644 --- a/right/src/eeprom.c +++ b/right/src/eeprom.c @@ -6,10 +6,10 @@ #include "config_parser/config_globals.h" #include "buffer.h" -bool IsEepromBusy; +volatile bool IsEepromBusy; static eeprom_operation_t CurrentEepromOperation; static config_buffer_id_t CurrentConfigBufferId; -status_t LastEepromTransferStatus; +static status_t LastEepromTransferStatus; void (*SuccessCallback)(void); static i2c_master_handle_t i2cHandle; diff --git a/right/src/eeprom.h b/right/src/eeprom.h index 7de803a..4ca6ad7 100644 --- a/right/src/eeprom.h +++ b/right/src/eeprom.h @@ -24,8 +24,7 @@ // Variables: - extern bool IsEepromBusy; - extern status_t EepromTransferStatus; + extern volatile bool IsEepromBusy; // Functions: diff --git a/right/src/init_peripherals.c b/right/src/init_peripherals.c index a7fdc5e..44136a3 100644 --- a/right/src/init_peripherals.c +++ b/right/src/init_peripherals.c @@ -18,10 +18,10 @@ #include "bootloader/wormhole.h" bool IsBusPalOn; -uint32_t I2cMainBusRequestedBaudRateBps = I2C_MAIN_BUS_NORMAL_BAUD_RATE; -uint32_t I2cMainBusActualBaudRateBps; +volatile uint32_t I2cMainBusRequestedBaudRateBps = I2C_MAIN_BUS_NORMAL_BAUD_RATE; +volatile uint32_t I2cMainBusActualBaudRateBps; -void initBusPalState(void) { +static void initBusPalState(void) { IsBusPalOn = Wormhole.magicNumber == WORMHOLE_MAGIC_NUMBER && Wormhole.enumerationMode == EnumerationMode_BusPal; if (IsBusPalOn) { Wormhole.magicNumber = 0; @@ -29,7 +29,7 @@ void initBusPalState(void) { } } -void initInterruptPriorities(void) +static void initInterruptPriorities(void) { NVIC_SetPriority(PIT_I2C_WATCHDOG_IRQ_ID, 1); NVIC_SetPriority(PIT_TIMER_IRQ_ID, 2); @@ -40,12 +40,12 @@ void initInterruptPriorities(void) NVIC_SetPriority(USB_IRQ_ID, 3); } -void delay(void) +static void delay(void) { for (volatile uint32_t i=0; i<62; i++); } -void recoverI2c(void) +static void recoverI2c(void) { PORT_SetPinMux(I2C_MAIN_BUS_SDA_PORT, I2C_MAIN_BUS_SDA_PIN, kPORT_MuxAsGpio); PORT_SetPinMux(I2C_MAIN_BUS_SCL_PORT, I2C_MAIN_BUS_SCL_PIN, kPORT_MuxAsGpio); @@ -74,7 +74,7 @@ void recoverI2c(void) delay(); } -void initI2cMainBus(void) +static void initI2cMainBus(void) { CLOCK_EnableClock(I2C_MAIN_BUS_SDA_CLOCK); CLOCK_EnableClock(I2C_MAIN_BUS_SCL_CLOCK); @@ -105,7 +105,7 @@ void ReinitI2cMainBus(void) InitSlaveScheduler(); } -void initI2cEepromBus(void) +static void initI2cEepromBus(void) { port_pin_config_t pinConfig = { .pullSelect = kPORT_PullUp, @@ -126,7 +126,7 @@ void initI2cEepromBus(void) I2C_MasterInit(I2C_EEPROM_BUS_BASEADDR, &masterConfig, sourceClock); } -void initI2c(void) +static void initI2c(void) { initI2cMainBus(); initI2cEepromBus(); diff --git a/right/src/init_peripherals.h b/right/src/init_peripherals.h index 0b0ad61..0d66399 100644 --- a/right/src/init_peripherals.h +++ b/right/src/init_peripherals.h @@ -8,8 +8,8 @@ // Variables: extern bool IsBusPalOn; - extern uint32_t I2cMainBusRequestedBaudRateBps; - extern uint32_t I2cMainBusActualBaudRateBps; + extern volatile uint32_t I2cMainBusRequestedBaudRateBps; + extern volatile uint32_t I2cMainBusActualBaudRateBps; // Functions: diff --git a/right/src/main.c b/right/src/main.c index 60a8c61..2897680 100644 --- a/right/src/main.c +++ b/right/src/main.c @@ -11,15 +11,15 @@ #include "peripherals/reset_button.h" #include "usb_report_updater.h" -bool IsEepromInitialized = false; -bool IsConfigInitialized = false; +static bool IsEepromInitialized = false; +static bool IsConfigInitialized = false; -void userConfigurationReadFinished(void) +static void userConfigurationReadFinished(void) { IsEepromInitialized = true; } -void hardwareConfigurationReadFinished(void) +static void hardwareConfigurationReadFinished(void) { EEPROM_LaunchTransfer(EepromOperation_Read, ConfigBufferId_StagingUserConfig, userConfigurationReadFinished); } diff --git a/right/src/peripherals/adc.c b/right/src/peripherals/adc.c index 5803db0..0d79eeb 100644 --- a/right/src/peripherals/adc.c +++ b/right/src/peripherals/adc.c @@ -2,7 +2,7 @@ #include "fsl_port.h" #include "peripherals/adc.h" -adc16_channel_config_t adc16ChannelConfigStruct; +static adc16_channel_config_t adc16ChannelConfigStruct; void ADC_Init(void) { diff --git a/right/src/timer.c b/right/src/timer.c index c997310..ac96d4c 100644 --- a/right/src/timer.c +++ b/right/src/timer.c @@ -1,7 +1,7 @@ #include "fsl_pit.h" #include "timer.h" -uint32_t CurrentTime; +volatile uint32_t CurrentTime; void PIT_TIMER_HANDLER(void) { diff --git a/right/src/timer.h b/right/src/timer.h index 012bb49..69b54bc 100644 --- a/right/src/timer.h +++ b/right/src/timer.h @@ -11,7 +11,7 @@ // Variables: - extern uint32_t CurrentTime; + extern volatile uint32_t CurrentTime; // Functions: diff --git a/right/src/usb_interfaces/usb_interface_basic_keyboard.c b/right/src/usb_interfaces/usb_interface_basic_keyboard.c index 3062b28..f1bef4f 100644 --- a/right/src/usb_interfaces/usb_interface_basic_keyboard.c +++ b/right/src/usb_interfaces/usb_interface_basic_keyboard.c @@ -4,10 +4,10 @@ static usb_basic_keyboard_report_t usbBasicKeyboardReports[2]; uint32_t UsbBasicKeyboardActionCounter; usb_basic_keyboard_report_t* ActiveUsbBasicKeyboardReport = usbBasicKeyboardReports; -bool IsUsbBasicKeyboardReportSent = false; +volatile bool IsUsbBasicKeyboardReportSent = false; static uint8_t usbBasicKeyboardInBuffer[USB_BASIC_KEYBOARD_REPORT_LENGTH]; -usb_basic_keyboard_report_t* getInactiveUsbBasicKeyboardReport(void) +static usb_basic_keyboard_report_t* getInactiveUsbBasicKeyboardReport(void) { return ActiveUsbBasicKeyboardReport == usbBasicKeyboardReports ? usbBasicKeyboardReports+1 : usbBasicKeyboardReports; } diff --git a/right/src/usb_interfaces/usb_interface_basic_keyboard.h b/right/src/usb_interfaces/usb_interface_basic_keyboard.h index f3b1c02..2a7fea0 100644 --- a/right/src/usb_interfaces/usb_interface_basic_keyboard.h +++ b/right/src/usb_interfaces/usb_interface_basic_keyboard.h @@ -31,7 +31,7 @@ // Variables: - extern bool IsUsbBasicKeyboardReportSent; + extern volatile bool IsUsbBasicKeyboardReportSent; extern uint32_t UsbBasicKeyboardActionCounter; extern usb_basic_keyboard_report_t* ActiveUsbBasicKeyboardReport; diff --git a/right/src/usb_interfaces/usb_interface_media_keyboard.c b/right/src/usb_interfaces/usb_interface_media_keyboard.c index 4693dcc..8147734 100644 --- a/right/src/usb_interfaces/usb_interface_media_keyboard.c +++ b/right/src/usb_interfaces/usb_interface_media_keyboard.c @@ -3,9 +3,9 @@ uint32_t UsbMediaKeyboardActionCounter; static usb_media_keyboard_report_t usbMediaKeyboardReports[2]; usb_media_keyboard_report_t* ActiveUsbMediaKeyboardReport = usbMediaKeyboardReports; -bool IsUsbMediaKeyboardReportSent = false; +volatile bool IsUsbMediaKeyboardReportSent = false; -usb_media_keyboard_report_t* getInactiveUsbMediaKeyboardReport(void) +static usb_media_keyboard_report_t* getInactiveUsbMediaKeyboardReport(void) { return ActiveUsbMediaKeyboardReport == usbMediaKeyboardReports ? usbMediaKeyboardReports+1 : usbMediaKeyboardReports; } diff --git a/right/src/usb_interfaces/usb_interface_media_keyboard.h b/right/src/usb_interfaces/usb_interface_media_keyboard.h index 62859d6..8b39fb2 100644 --- a/right/src/usb_interfaces/usb_interface_media_keyboard.h +++ b/right/src/usb_interfaces/usb_interface_media_keyboard.h @@ -28,7 +28,7 @@ // Variables: - extern bool IsUsbMediaKeyboardReportSent; + extern volatile bool IsUsbMediaKeyboardReportSent; extern uint32_t UsbMediaKeyboardActionCounter; extern usb_media_keyboard_report_t* ActiveUsbMediaKeyboardReport; diff --git a/right/src/usb_interfaces/usb_interface_mouse.c b/right/src/usb_interfaces/usb_interface_mouse.c index 27acb64..711e3cd 100644 --- a/right/src/usb_interfaces/usb_interface_mouse.c +++ b/right/src/usb_interfaces/usb_interface_mouse.c @@ -3,9 +3,9 @@ uint32_t UsbMouseActionCounter; static usb_mouse_report_t usbMouseReports[2]; usb_mouse_report_t* ActiveUsbMouseReport = usbMouseReports; -bool IsUsbMouseReportSent = false; +volatile bool IsUsbMouseReportSent = false; -usb_mouse_report_t* getInactiveUsbMouseReport(void) +static usb_mouse_report_t* getInactiveUsbMouseReport(void) { return ActiveUsbMouseReport == usbMouseReports ? usbMouseReports+1 : usbMouseReports; } diff --git a/right/src/usb_interfaces/usb_interface_mouse.h b/right/src/usb_interfaces/usb_interface_mouse.h index e0e1e67..e1a2cc7 100644 --- a/right/src/usb_interfaces/usb_interface_mouse.h +++ b/right/src/usb_interfaces/usb_interface_mouse.h @@ -31,7 +31,7 @@ // Variables: - extern bool IsUsbMouseReportSent; + extern volatile bool IsUsbMouseReportSent; extern uint32_t UsbMouseActionCounter; extern usb_mouse_report_t* ActiveUsbMouseReport; diff --git a/right/src/usb_interfaces/usb_interface_system_keyboard.c b/right/src/usb_interfaces/usb_interface_system_keyboard.c index 7e6e61a..e039069 100644 --- a/right/src/usb_interfaces/usb_interface_system_keyboard.c +++ b/right/src/usb_interfaces/usb_interface_system_keyboard.c @@ -3,9 +3,9 @@ uint32_t UsbSystemKeyboardActionCounter; static usb_system_keyboard_report_t usbSystemKeyboardReports[2]; usb_system_keyboard_report_t* ActiveUsbSystemKeyboardReport = usbSystemKeyboardReports; -bool IsUsbSystemKeyboardReportSent = false; +volatile bool IsUsbSystemKeyboardReportSent = false; -usb_system_keyboard_report_t* getInactiveUsbSystemKeyboardReport() +static usb_system_keyboard_report_t* getInactiveUsbSystemKeyboardReport() { return ActiveUsbSystemKeyboardReport == usbSystemKeyboardReports ? usbSystemKeyboardReports+1 : usbSystemKeyboardReports; } diff --git a/right/src/usb_interfaces/usb_interface_system_keyboard.h b/right/src/usb_interfaces/usb_interface_system_keyboard.h index d19d362..0a84353 100644 --- a/right/src/usb_interfaces/usb_interface_system_keyboard.h +++ b/right/src/usb_interfaces/usb_interface_system_keyboard.h @@ -29,7 +29,7 @@ // Variables: - extern bool IsUsbSystemKeyboardReportSent; + extern volatile bool IsUsbSystemKeyboardReportSent; extern uint32_t UsbSystemKeyboardActionCounter; extern usb_system_keyboard_report_t* ActiveUsbSystemKeyboardReport; diff --git a/right/src/usb_report_updater.c b/right/src/usb_report_updater.c index f4b0277..7fb3eeb 100644 --- a/right/src/usb_report_updater.c +++ b/right/src/usb_report_updater.c @@ -17,11 +17,11 @@ #include "config_parser/parse_keymap.h" #include "usb_commands/usb_command_get_debug_buffer.h" -uint32_t UsbReportUpdateTime = 0; +static uint32_t UsbReportUpdateTime = 0; static uint32_t elapsedTime; uint16_t DoubleTapSwitchLayerTimeout = 150; -uint16_t DoubleTapSwitchLayerReleaseTimeout = 100; +static uint16_t DoubleTapSwitchLayerReleaseTimeout = 100; static bool activeMouseStates[ACTIVE_MOUSE_STATES_COUNT]; @@ -53,7 +53,7 @@ mouse_kinetic_state_t MouseScrollState = { .acceleratedSpeed = 50, }; -void processMouseKineticState(mouse_kinetic_state_t *kineticState) +static void processMouseKineticState(mouse_kinetic_state_t *kineticState) { float initialSpeed = kineticState->intMultiplier * kineticState->initialSpeed; float acceleration = kineticState->intMultiplier * kineticState->acceleration; @@ -155,7 +155,7 @@ void processMouseKineticState(mouse_kinetic_state_t *kineticState) kineticState->wasMoveAction = isMoveAction; } -void processMouseActions() +static void processMouseActions() { processMouseKineticState(&MouseMoveState); ActiveUsbMouseReport->x = MouseMoveState.xOut; @@ -193,7 +193,7 @@ static uint8_t basicScancodeIndex = 0; static uint8_t mediaScancodeIndex = 0; static uint8_t systemScancodeIndex = 0; -void applyKeyAction(key_state_t *keyState, key_action_t *action) +static void applyKeyAction(key_state_t *keyState, key_action_t *action) { static key_state_t *doubleTapSwitchLayerKey; static uint32_t doubleTapSwitchLayerStartTime; @@ -265,7 +265,7 @@ static uint8_t secondaryRoleSlotId; static uint8_t secondaryRoleKeyId; static secondary_role_t secondaryRole; -void updateActiveUsbReports(void) +static void updateActiveUsbReports(void) { memset(activeMouseStates, 0, ACTIVE_MOUSE_STATES_COUNT); @@ -369,10 +369,10 @@ void updateActiveUsbReports(void) previousLayer = activeLayer; } -bool UsbBasicKeyboardReportEverSent = false; -bool UsbMediaKeyboardReportEverSent = false; -bool UsbSystemKeyboardReportEverSent = false; -bool UsbMouseReportEverSentEverSent = false; +static bool UsbBasicKeyboardReportEverSent = false; +static bool UsbMediaKeyboardReportEverSent = false; +static bool UsbSystemKeyboardReportEverSent = false; +static bool UsbMouseReportEverSentEverSent = false; uint32_t UsbReportUpdateCounter; static uint32_t lastUsbUpdateTime; @@ -425,15 +425,33 @@ void UpdateUsbReports(void) updateActiveUsbReports(); - SwitchActiveUsbBasicKeyboardReport(); - SwitchActiveUsbMediaKeyboardReport(); - SwitchActiveUsbSystemKeyboardReport(); - SwitchActiveUsbMouseReport(); + static usb_basic_keyboard_report_t last_basic_report = { .scancodes[0] = 0xFF }; + if (memcmp(ActiveUsbBasicKeyboardReport, &last_basic_report, sizeof(usb_basic_keyboard_report_t)) != 0) { + last_basic_report = *ActiveUsbBasicKeyboardReport; + SwitchActiveUsbBasicKeyboardReport(); + IsUsbBasicKeyboardReportSent = false; + } - IsUsbBasicKeyboardReportSent = false; - IsUsbMediaKeyboardReportSent = false; - IsUsbSystemKeyboardReportSent = false; - IsUsbMouseReportSent = false; + static usb_media_keyboard_report_t last_media_report = { .scancodes[0] = 0xFF }; + if (memcmp(ActiveUsbMediaKeyboardReport, &last_media_report, sizeof(usb_media_keyboard_report_t)) != 0) { + last_media_report = *ActiveUsbMediaKeyboardReport; + SwitchActiveUsbMediaKeyboardReport(); + IsUsbMediaKeyboardReportSent = false; + } + + static usb_system_keyboard_report_t last_system_report = { .scancodes[0] = 0xFF }; + if (memcmp(ActiveUsbSystemKeyboardReport, &last_system_report, sizeof(usb_system_keyboard_report_t)) != 0) { + last_system_report = *ActiveUsbSystemKeyboardReport; + SwitchActiveUsbSystemKeyboardReport(); + IsUsbSystemKeyboardReportSent = false; + } + + static usb_mouse_report_t last_mouse_report = { .buttons = 0xFF }; + if (memcmp(ActiveUsbMouseReport, &last_mouse_report, sizeof(usb_mouse_report_t)) != 0) { + last_mouse_report = *ActiveUsbMouseReport; + SwitchActiveUsbMouseReport(); + IsUsbMouseReportSent = false; + } Timer_SetCurrentTime(&lastUsbUpdateTime); }