Skip to content

fix(drivers): resolve serial_escserial.c TX-pin, HAL-timer, and RX ownership defects #1427

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

Merged
nerdCopter merged 1 commit into from
Sep 2, 2026
Merged

fix(drivers): resolve serial_escserial.c TX-pin, HAL-timer, and RX ownership defects #1427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
104 changes: 60 additions & 44 deletions src/main/drivers/serial_escserial.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,9 @@ enum {
#define STOP_BIT_MASK (1 << 0)
#define START_BIT_MASK (1 << (RX_TOTAL_BITS - 1))

// XXX No TIM_DeInit equivalent in HAL driver???
#ifdef USE_HAL_DRIVER
static void TIM_DeInit(TIM_TypeDef *tim) {
UNUSED(tim);
LL_TIM_DeInit(tim);
}
#endif

Expand Down Expand Up @@ -173,7 +172,7 @@ static void escSerialGPIOConfig(const timerHardware_t *timhw, ioConfig_t cfg) {
return;
}
IOInit(IOGetByTag(tag), OWNER_MOTOR, 0);
#ifdef STM32F7
#if defined(STM32F7) || defined(STM32H7)
IOConfigGPIOAF(IOGetByTag(tag), cfg, timhw->alternateFunction);
#else
IOConfigGPIO(IOGetByTag(tag), cfg);
Expand Down Expand Up @@ -322,6 +321,7 @@ static void serialTimerTxConfigBL(const timerHardware_t *timerHardwarePtr, uint8
}
}
} while (isTimerPeriodTooLarge(timerPeriod));
timerReconfigureTimeBase(timerHardwarePtr->tim, timerPeriod, clock);
timerConfigure(timerHardwarePtr, timerPeriod, clock);
timerChCCHandlerInit(&escSerialPorts[reference].timerCb, onSerialTimerBL);
timerChConfigCallbacks(timerHardwarePtr, &escSerialPorts[reference].timerCb, NULL);
Expand Down Expand Up @@ -370,6 +370,7 @@ static void onSerialRxPinChangeBL(timerCCHandlerRec_t *cbRec, captureCompare_t c
static void serialTimerRxConfigBL(const timerHardware_t *timerHardwarePtr, uint8_t reference, portOptions_e options) {
// start bit is usually a FALLING signal
TIM_DeInit(timerHardwarePtr->tim);
timerReconfigureTimeBase(timerHardwarePtr->tim, 0xFFFF, SystemCoreClock / 2);
timerConfigure(timerHardwarePtr, 0xFFFF, SystemCoreClock / 2);
timerChConfigIC(timerHardwarePtr, (options & SERIAL_INVERTED) ? ICPOLARITY_RISING : ICPOLARITY_FALLING, 0);
timerChCCHandlerInit(&escSerialPorts[reference].edgeCb, onSerialRxPinChangeBL);
Expand Down Expand Up @@ -467,6 +468,7 @@ static void onSerialTimerEsc(timerCCHandlerRec_t *cbRec, captureCompare_t captur
static void escSerialTimerTxConfig(const timerHardware_t *timerHardwarePtr, uint8_t reference) {
uint32_t timerPeriod = 34;
TIM_DeInit(timerHardwarePtr->tim);
timerReconfigureTimeBase(timerHardwarePtr->tim, timerPeriod, MHZ_TO_HZ(1));
timerConfigure(timerHardwarePtr, timerPeriod, MHZ_TO_HZ(1));
timerChCCHandlerInit(&escSerialPorts[reference].timerCb, onSerialTimerEsc);
timerChConfigCallbacks(timerHardwarePtr, &escSerialPorts[reference].timerCb, NULL);
Expand Down Expand Up @@ -535,6 +537,7 @@ static void onSerialRxPinChangeEsc(timerCCHandlerRec_t *cbRec, captureCompare_t
static void escSerialTimerRxConfig(const timerHardware_t *timerHardwarePtr, uint8_t reference) {
// start bit is usually a FALLING signal
TIM_DeInit(timerHardwarePtr->tim);
timerReconfigureTimeBase(timerHardwarePtr->tim, 0xFFFF, MHZ_TO_HZ(1));
timerConfigure(timerHardwarePtr, 0xFFFF, MHZ_TO_HZ(1));
timerChConfigIC(timerHardwarePtr, ICPOLARITY_FALLING, 0);
timerChCCHandlerInit(&escSerialPorts[reference].edgeCb, onSerialRxPinChangeEsc);
Expand All @@ -552,26 +555,45 @@ static void resetBuffers(escSerial_t *escSerial) {
escSerial->port.txBufferHead = 0;
}

static serialPort_t *openEscSerial(escSerialPortIndex_e portIndex, serialReceiveCallbackPtr callback, uint16_t output, uint32_t baud, portOptions_e options, uint8_t mode) {
static serialPort_t *openEscSerial(const motorDevConfig_t *motorConfig, escSerialPortIndex_e portIndex, serialReceiveCallbackPtr callback, uint16_t output, uint32_t baud, portOptions_e options, uint8_t mode) {
escSerial_t *escSerial = &(escSerialPorts[portIndex]);
if (mode != PROTOCOL_KISSALL) {
escSerial->rxTimerHardware = &(timerHardware[output]);
const ioTag_t tag = motorConfig->ioTags[output];
const timerHardware_t *timerHardware = timerAllocate(tag, OWNER_MOTOR, 0);
if (timerHardware == NULL) {
return NULL;
}
escSerial->rxTimerHardware = timerHardware;
// N-Channels can't be used as RX.
if (escSerial->rxTimerHardware->output & TIMER_OUTPUT_N_CHANNEL) {
return NULL;
}
#ifdef USE_HAL_DRIVER
escSerial->rxTimerHandle = timerFindTimerHandle(escSerial->rxTimerHardware->tim);
#endif
}
escSerial->mode = mode;
escSerial->txTimerHardware = timerAllocate(escSerialConfig()->ioTag, OWNER_MOTOR, 0);
if (escSerial->txTimerHardware == NULL) {
return NULL;
}
// Workaround to ensure that the timerHandle is configured before use, timer will be reconfigured to a different frequency below
// this prevents a null-pointer dereference in __HAL_TIM_CLEAR_FLAG called by timerChClearCCFlag and similar accesses of timerHandle without the Instance being configured first.
timerConfigure(escSerial->rxTimerHardware, 0xffff, 1);

// Allocate separate TX pin if configured, otherwise use RX pin for both TX and RX
if (escSerialConfig()->ioTag != IO_TAG_NONE) {
escSerial->txTimerHardware = timerAllocate(escSerialConfig()->ioTag, OWNER_MOTOR, 0);
if (escSerial->txTimerHardware == NULL) {
return NULL;
}
#ifdef USE_HAL_DRIVER
escSerial->txTimerHandle = timerFindTimerHandle(escSerial->txTimerHardware->tim);
escSerial->txTimerHandle = timerFindTimerHandle(escSerial->txTimerHardware->tim);
#endif
timerConfigure(escSerial->txTimerHardware, 0xffff, 1);
} else {
// No separate TX pin configured - use RX pin for both TX and RX (fallback mode)
escSerial->txTimerHardware = escSerial->rxTimerHardware;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
#ifdef USE_HAL_DRIVER
escSerial->txTimerHandle = escSerial->rxTimerHandle;
#endif
}
}
escSerial->mode = mode;
escSerial->port.vTable = escSerialVTable;
escSerial->port.baudRate = baud;
escSerial->port.mode = MODE_RXTX;
Expand Down Expand Up @@ -605,22 +627,28 @@ static serialPort_t *openEscSerial(escSerialPortIndex_e portIndex, serialReceive
memset(&escOutputs, 0, sizeof(escOutputs));
pwmOutputPort_t *pwmMotors = pwmGetMotors();
for (volatile uint8_t i = 0; i < MAX_SUPPORTED_MOTORS; i++) {
if (pwmMotors[i].enabled) {
if (pwmMotors[i].io != IO_NONE) {
for (volatile uint8_t j = 0; j < USABLE_TIMER_CHANNEL_COUNT; j++) {
if (pwmMotors[i].io == IOGetByTag(timerHardware[j].tag)) {
escSerialOutputPortConfig(&timerHardware[j]);
if (timerHardware[j].output & TIMER_OUTPUT_INVERTED) {
escOutputs[escSerial->outputCount].inverted = 1;
}
break;
if (pwmMotors[i].enabled && pwmMotors[i].io != IO_NONE) {
const ioTag_t tag = motorConfig->ioTags[i];
if (tag != IO_TAG_NONE) {
const timerHardware_t *timerHardware = timerAllocate(tag, OWNER_MOTOR, 0);
if (timerHardware) {
// Workaround to ensure that the timerHandle is configured before use, timer will be reconfigured to a different frequency below
// this prevents a null-pointer dereference in __HAL_TIM_CLEAR_FLAG called by timerChClearCCFlag and similar accesses of timerHandle without the Instance being configured first.
timerConfigure(timerHardware, 0xffff, 1);
escSerialOutputPortConfig(timerHardware);
escOutputs[escSerial->outputCount].io = pwmMotors[i].io;
if (timerHardware->output & TIMER_OUTPUT_INVERTED) {
escOutputs[escSerial->outputCount].inverted = 1;
}
escSerial->txTimerHardware = timerHardware;
escSerial->outputCount++;
}
escOutputs[escSerial->outputCount].io = pwmMotors[i].io;
escSerial->outputCount++;
}
}
}
if (escSerial->outputCount == 0) {
return NULL;
}
setTxSignalEsc(escSerial, ENABLE);
serialTimerTxConfigBL(escSerial->txTimerHardware, portIndex, baud);
} else if (mode == PROTOCOL_CASTLE) {
Expand Down Expand Up @@ -780,9 +808,9 @@ static bool processExitCommand(uint8_t c) {
}


void escEnablePassthrough(serialPort_t *escPassthroughPort, uint16_t output, uint8_t mode) {
bool escEnablePassthrough(serialPort_t *escPassthroughPort, const motorDevConfig_t *motorConfig, uint16_t output, uint8_t mode) {
bool exitEsc = false;
uint8_t motor_output = 0;
uint8_t motor_output = output;
LED0_OFF;
LED1_OFF;
//StopPwmAllMotors();
Expand All @@ -800,26 +828,14 @@ void escEnablePassthrough(serialPort_t *escPassthroughPort, uint16_t output, uin
escBaudrate = BAUDRATE_NORMAL;
break;
}
if ((mode == PROTOCOL_KISS) && (output == 255)) {
motor_output = 255;
if ((mode == PROTOCOL_KISS) && (motor_output == 255)) {
mode = PROTOCOL_KISSALL;
} else {
uint8_t first_output = 0;
for (unsigned i = 0; i < USABLE_TIMER_CHANNEL_COUNT; i++) {
if (timerHardware[i].usageFlags & TIM_USE_MOTOR) {
first_output = i;
break;
}
}
//doesn't work with messy timertable
motor_output = first_output + output;
if (motor_output >= USABLE_TIMER_CHANNEL_COUNT) {
return;
}
} else if (motor_output >= MAX_SUPPORTED_MOTORS) {
return false;
}
escPort = openEscSerial(ESCSERIAL1, NULL, motor_output, escBaudrate, 0, mode);
escPort = openEscSerial(motorConfig, ESCSERIAL1, NULL, motor_output, escBaudrate, 0, mode);
if (!escPort) {
return;
return false;
}
uint8_t ch;
while (1) {
Expand All @@ -846,9 +862,9 @@ void escEnablePassthrough(serialPort_t *escPassthroughPort, uint16_t output, uin
serialWrite(escPassthroughPort, 0xF4);
serialWrite(escPassthroughPort, 0xF4);
closeEscSerial(ESCSERIAL1, mode);
return;
return true;
}
if (mode == PROTOCOL_BLHELI) {
if (mode == PROTOCOL_BLHELI || mode == PROTOCOL_KISS || mode == PROTOCOL_KISSALL) {
serialWrite(escPassthroughPort, ch); // blheli loopback
}
serialWrite(escPort, ch);
Expand Down
3 changes: 2 additions & 1 deletion src/main/drivers/serial_escserial.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ typedef enum {
} escProtocol_e;

// serialPort API
void escEnablePassthrough(serialPort_t *escPassthroughPort, uint16_t output, uint8_t mode);
struct motorDevConfig_s;
bool escEnablePassthrough(serialPort_t *escPassthroughPort, const struct motorDevConfig_s *motorConfig, uint16_t output, uint8_t mode);

typedef struct escSerialConfig_s {
ioTag_t ioTag;
Expand Down
4 changes: 4 additions & 0 deletions src/main/drivers/timer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,10 @@ void configTimeBase(TIM_TypeDef *tim, uint16_t period, uint32_t hz) {
TIM_TimeBaseInit(tim, &TIM_TimeBaseStructure);
}

void timerReconfigureTimeBase(TIM_TypeDef *tim, uint16_t period, uint32_t hz) {
configTimeBase(tim, period, hz);
}

// old interface for PWM inputs. It should be replaced
void timerConfigure(const timerHardware_t *timerHardwarePtr, uint16_t period, uint32_t hz) {
configTimeBase(timerHardwarePtr->tim, period, hz);
Expand Down
1 change: 1 addition & 0 deletions src/main/drivers/timer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ typedef enum {
} channelType_t;

void timerConfigure(const timerHardware_t *timHw, uint16_t period, uint32_t hz); // This interface should be replaced.
void timerReconfigureTimeBase(TIM_TypeDef *tim, uint16_t period, uint32_t hz);

void timerChConfigIC(const timerHardware_t *timHw, bool polarityRising, unsigned inputFilterSamples);
void timerChConfigICDual(const timerHardware_t* timHw, bool polarityRising, unsigned inputFilterSamples);
Expand Down
10 changes: 10 additions & 0 deletions src/main/drivers/timer_hal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,16 @@ void configTimeBase(TIM_TypeDef *tim, uint16_t period, uint32_t hz) {
}
}

void timerReconfigureTimeBase(TIM_TypeDef *tim, uint16_t period, uint32_t hz) {
TIM_HandleTypeDef *handle = timerFindTimerHandle(tim);
if (handle == NULL) {
return;
}
handle->Init.Period = (period - 1) & 0xffff; // AKA TIMx_ARR
handle->Init.Prescaler = (timerClock(tim) / hz) - 1;
TIM_Base_SetConfig(handle->Instance, &handle->Init);
}

// old interface for PWM inputs. It should be replaced
void timerConfigure(const timerHardware_t *timerHardwarePtr, uint16_t period, uint32_t hz) {
uint8_t timerIndex = lookupTimerIndex(timerHardwarePtr->tim);
Expand Down
4 changes: 3 additions & 1 deletion src/main/interface/cli.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2946,7 +2946,9 @@ static void cliEscPassthrough(char *cmdline) {
pos++;
pch = strtok_r(NULL, " ", &saveptr);
}
escEnablePassthrough(cliPort, escIndex, mode);
if (!escEnablePassthrough(cliPort, &motorConfig()->dev, escIndex, mode)) {
cliPrintErrorLinef("Error starting ESC connection");
}
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/main/interface/msp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ uint8_t escPortIndex;

#ifdef USE_ESCSERIAL
static void mspEscPassthroughFn(serialPort_t *serialPort) {
escEnablePassthrough(serialPort, escPortIndex, escMode);
escEnablePassthrough(serialPort, &motorConfig()->dev, escPortIndex, escMode);
}
#endif

Expand Down
Loading