From 020459c888f094a7dee2f117136c8b36ff718305 Mon Sep 17 00:00:00 2001 From: Patrick Baus Date: Fri, 4 Oct 2024 09:32:05 +0200 Subject: [PATCH] Improve speed (#13) * Only set/reset channels that need a state change. This reduces the number of requests over the slower peripheral bus. * Move reset code into the ISR to prevent race conditions --- Core/Src/decoder.c | 8 ++--- Core/Src/main.c | 79 +++++++++++++++++++++++++--------------------- Makefile | 2 +- Scan2000STM32.ioc | 8 +++-- 4 files changed, 53 insertions(+), 44 deletions(-) diff --git a/Core/Src/decoder.c b/Core/Src/decoder.c index baabc41..91b2151 100644 --- a/Core/Src/decoder.c +++ b/Core/Src/decoder.c @@ -37,9 +37,9 @@ decodeResult_t decode_10channels(uint32_t command, uint32_t *relaySetRegister, u // i + (i/5) * 5 term to skip CH6-CH10. // The MSB is the 4W relay, the LSB is CH1 - *relaySetRegister |= (uint32_t)!!(command & (1lu << scan2000ChannelOnSequence[i])) << (i + (i / 5) * 5); + *relaySetRegister |= (uint32_t)!!(command & (1UL << scan2000ChannelOnSequence[i])) << (i + (i / 5) * 5); // The list of channels, that are to tbe turned off - *relayUnsetRegister |= (uint32_t)!!(command & (1lu << scan2000ChannelOffSequence[i])) << (i + (i / 5) * 5); + *relayUnsetRegister |= (uint32_t)!!(command & (1UL << scan2000ChannelOffSequence[i])) << (i + (i / 5) * 5); } rv = decodeOK; } else { @@ -68,8 +68,8 @@ decodeResult_t decode_20channels(const uint64_t command, uint32_t *relaySetRegis if (command != 0x00000000) { // Process the channels (incl. CH21, 4W mode) for (size_t i = 0; i < sizeof(scan2000_20ChannelSequence)/sizeof(scan2000_20ChannelSequence[0]); i++) { - *relayUnsetRegister |= (uint32_t)!!(command & (1llu << (2 * (scan2000_20ChannelSequence[i] - 1)))) << i; // Even bits -> turn relays off - *relaySetRegister |= (uint32_t)!!(command & (1llu << (2 * (scan2000_20ChannelSequence[i] - 1) + 1))) << i; // Odd bits -> turn relays on + *relayUnsetRegister |= (uint32_t)!!(command & (1ULL << (2 * (scan2000_20ChannelSequence[i] - 1)))) << i; // Even bits -> turn relays off + *relaySetRegister |= (uint32_t)!!(command & (1ULL << (2 * (scan2000_20ChannelSequence[i] - 1) + 1))) << i; // Odd bits -> turn relays on } return decodeOK; } diff --git a/Core/Src/main.c b/Core/Src/main.c index ca61aca..675c776 100644 --- a/Core/Src/main.c +++ b/Core/Src/main.c @@ -19,6 +19,7 @@ /* USER CODE END Header */ /* Includes ------------------------------------------------------------------*/ #include "main.h" + /* Private includes ----------------------------------------------------------*/ /* USER CODE BEGIN Includes */ #include "decoder.h" @@ -178,17 +179,6 @@ int main(void) while (1) { MsgBuffer_print(); - const uint32_t now = HAL_GetTick(); - if (now - timeSinceLastClock > 1) { - // Wipe everything we received if the last clock pulse received was 1ms or more in the past. - // Normally clock the period is 10us, and strobe follows within 20us, so this will do. - // The handling of the clock and strobe, and the update of timeSinceLastClock - // is inside the interrupt handler (HAL_GPIO_EXTI_Rising_Callback), so the code here must be - // somewhat robust against concurrent access. - receivedCounter = 0; - receivedSequence = 0; - timeSinceLastClock = now; - } /* USER CODE END WHILE */ /* USER CODE BEGIN 3 */ @@ -287,7 +277,7 @@ static void MX_DMA_Init(void) __HAL_RCC_DMA1_CLK_ENABLE(); /* DMA interrupt init */ - /* DMA1_chnel1_IRQn interrupt configuration */ + /* DMA1_Channel1_IRQn interrupt configuration */ HAL_NVIC_SetPriority(DMA1_Channel1_IRQn, 0, 0); HAL_NVIC_EnableIRQ(DMA1_Channel1_IRQn); @@ -479,17 +469,19 @@ bool validateRelayState(const uint32_t channelState) { void setRelays(const uint32_t newChannelState) { // If we have a new state, update the relays if (newChannelState != channelState) { + const uint32_t resetChannels = ~newChannelState & channelState; // Channels that need to be turned off + const uint32_t setChannels = ~channelState & newChannelState; // Channels that need to be turned on channelState = newChannelState; // First disconnect all channels, that need to be disconnected for (size_t i=0; i 1) { + // Wipe everything we received if the last clock pulse received was more than 2ms the past. + // Normally clock the period is 10us, and strobe follows within 20us, so this will do. + // The handling of the clock and strobe, and the update of timeSinceLastClock + // is inside the interrupt handler (HAL_GPIO_EXTI_Rising_Callback), so the code here must be + // somewhat robust against concurrent access. + receivedCounter = 0; + } + if (GPIO_Pin == Clock_Pin) { // Clock in a new bit - receivedSequence = receivedSequence << 1; + if (!receivedCounter) { + receivedSequence = 0x00; + } else { + receivedSequence = receivedSequence << 1; + } receivedSequence |= HAL_GPIO_ReadPin(Data_GPIO_Port, Data_Pin); receivedCounter++; timeSinceLastClock = HAL_GetTick(); @@ -540,27 +545,29 @@ void HAL_GPIO_EXTI_Rising_Callback(uint16_t GPIO_Pin) { decodeResult = decodeLengthError; } - if ((decodeResult == decodeOK) || (decodeResult == decodeIgnored)) { - // Now apply the updates (can happen even when command is ignored, as we unset unwanted relays by default) - newChannelState |= relaySetRegister; // closed channels - newChannelState &= ~relayUnsetRegister; // opened channels - if (decodeResult == decodeOK) - msg.state = msgOK; - else + switch (decodeResult) { + case decodeOK: + // Now apply the updates + // We do not blindly apply the requested state, but rather test whether it is valid first to make sure + // we do damage neither the relays nor the DMM. + newChannelState |= relaySetRegister; // closed channels + newChannelState &= ~relayUnsetRegister; // opened channels + + // Test if the new state is valid and if so, apply it + if (validateRelayState(newChannelState)) { + setRelays(newChannelState); + msg.state = msgOK; + } else { + msg.state = msgRelayError; + } + break; + case decodeIgnored: msg.state = msgIgnored; - - // Test if the new state is valid and if so, apply it - if (validateRelayState(newChannelState)) { - setRelays(newChannelState); - } else { - msg.state = msgRelayError; - } - } else { - // very likely decodeResult == decodeXError - // Terminate here and signal an error - if (decodeResult == decodeDataError) + break; + case decodeDataError: msg.state = msgDataError; - else + break; + case decodeLengthError: msg.state = msgLengthError; } diff --git a/Makefile b/Makefile index fb2138e..3a51026 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ DEBUG = 0 # optimization OPT = -O3 -LOGLEVEL = 3 # Warning, 1 = debug, 2 = info +LOGLEVEL = 3 # 1 = debug, 2 = info, 3 = Warning # Create a version string from the Git commit # Taken from: https://eugene-babichenko.github.io/blog/2019/09/28/nightly-versions-makefiles/ diff --git a/Scan2000STM32.ioc b/Scan2000STM32.ioc index d646276..070d721 100644 --- a/Scan2000STM32.ioc +++ b/Scan2000STM32.ioc @@ -78,7 +78,7 @@ NVIC.HardFault_IRQn=true\:0\:0\:false\:false\:true\:false\:false\:false NVIC.NonMaskableInt_IRQn=true\:0\:0\:false\:false\:true\:false\:false\:false NVIC.PendSV_IRQn=true\:0\:0\:false\:false\:true\:false\:false\:false NVIC.SVC_IRQn=true\:0\:0\:false\:false\:true\:false\:false\:true -NVIC.SysTick_IRQn=true\:3\:0\:false\:false\:true\:false\:true\:false +NVIC.SysTick_IRQn=true\:3\:0\:true\:false\:true\:false\:true\:false NVIC.USART3_4_IRQn=true\:0\:0\:false\:false\:true\:true\:true\:true PA0.GPIOParameters=GPIO_PuPdOD,GPIO_Label,GPIO_ModeDefaultOD PA0.GPIO_Label=Tx uC @@ -188,12 +188,14 @@ PB9.GPIOParameters=GPIO_Label PB9.GPIO_Label=Data PB9.Locked=true PB9.Signal=GPIO_Input -PC14-OSC32_IN\ (PC14).GPIOParameters=GPIO_Label +PC14-OSC32_IN\ (PC14).GPIOParameters=GPIO_Label,GPIO_ModeDefaultEXTI PC14-OSC32_IN\ (PC14).GPIO_Label=Clock +PC14-OSC32_IN\ (PC14).GPIO_ModeDefaultEXTI=GPIO_MODE_IT_RISING PC14-OSC32_IN\ (PC14).Locked=true PC14-OSC32_IN\ (PC14).Signal=GPXTI14 -PC15-OSC32_OUT\ (PC15).GPIOParameters=GPIO_Label +PC15-OSC32_OUT\ (PC15).GPIOParameters=GPIO_Label,GPIO_ModeDefaultEXTI PC15-OSC32_OUT\ (PC15).GPIO_Label=Strobe +PC15-OSC32_OUT\ (PC15).GPIO_ModeDefaultEXTI=GPIO_MODE_IT_RISING PC15-OSC32_OUT\ (PC15).Locked=true PC15-OSC32_OUT\ (PC15).Signal=GPXTI15 PC6.GPIOParameters=GPIO_Label