fix(bt/osi): stop timer before re-arming in alarm_set (IDFGH-17864)#18762
Open
anuragak021 wants to merge 1 commit into
Open
fix(bt/osi): stop timer before re-arming in alarm_set (IDFGH-17864)#18762anuragak021 wants to merge 1 commit into
anuragak021 wants to merge 1 commit into
Conversation
alarm_set() called esp_timer_start_once/periodic without first stopping the timer handle. If the same alarm is set while it is already running (re-armed before it fires or is cancelled), esp_timer_start_* returns ESP_ERR_INVALID_STATE and the alarm is silently dropped, logged as: E BT_OSI: alarm_set failed to start timer, err 0x103 alarm_free() already calls esp_timer_stop before esp_timer_delete as the correct pattern. Apply the same stop-before-start here so that a double-set reschedules the alarm rather than failing silently.
Author
|
Reproduced consistently on ESP32-S3 custom board with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
alarm_set()callsesp_timer_start_once/esp_timer_start_periodicwithout first stopping the timer handle. If the Bluedroid stack re arms an alarm that is already running (e.g. a scheduling alarm fired before the previous instance was cancelled),esp_timer_start_*returnsESP_ERR_INVALID_STATEand the alarm is silently dropped, producing the log error:alarm_free()already uses the correct pattern it callsesp_timer_stopbeforeesp_timer_delete. This fix applies the same stop before start inalarm_setso that a double-set reschedules the alarm rather than failing.Change
Add
esp_timer_stop(alarm->alarm_hdl)before theesp_timer_start_once/esp_timer_start_periodiccall inside the staticalarm_set()helper incomponents/bt/common/osi/alarm.c.esp_timer_stopreturnsESP_ERR_INVALID_STATEwhen the timer is already stopped, which is expected and harmless; the subsequent start call then proceeds normally.Reproduction
Observed on ESP32 S3 with
CONFIG_BT_ALLOCATION_FROM_SPIRAM_FIRST=y. PSRAM allocation latency shifts task scheduling enough thatosi_alarm_setis called on a running timer handle during BLE GAP advertising setup, triggering the error on every boot.Test
Build with Bluedroid enabled (
CONFIG_BT_BLUEDROID_ENABLED=y,CONFIG_BT_BLE_ENABLED=y) and start BLE advertising thealarm_set failederror no longer appears.Note
Low Risk
Single-line behavioral fix in BT timer scheduling aligned with existing
alarm_free()usage; no auth, data, or API surface changes.Overview
Fixes Bluedroid alarms being silently dropped when
osi_alarm_set/osi_alarm_set_periodicruns on a handle that is still active.esp_timer_start_onceandesp_timer_start_periodicreturnESP_ERR_INVALID_STATEin that case, which surfaced asalarm_set failed to start timer, err 0x103during BLE GAP advertising on ESP32-S3 with SPIRAM-first BT allocation.alarm_set()inalarm.cnow callsesp_timer_stopon the handle before starting the timer, matching the stop-then-operate pattern already used inalarm_free(). Re-arming reschedules the alarm instead of failing; stopping an already-idle timer is harmless (ESP_ERR_INVALID_STATEis ignored).Reviewed by Cursor Bugbot for commit b7af0d4. Bugbot is set up for automated code reviews on this repo. Configure here.