Skip to content

Refactor settings controller worker for FreeRTOS#144

Merged
baba-dev merged 1 commit into
mainfrom
codex/refactor-settings_controller-for-freertos
Sep 20, 2025
Merged

Refactor settings controller worker for FreeRTOS#144
baba-dev merged 1 commit into
mainfrom
codex/refactor-settings_controller-for-freertos

Conversation

@baba-dev

Copy link
Copy Markdown
Owner

Summary

  • replace the settings controller's std::thread backend with a FreeRTOS task/queue/semaphore on ESP builds while keeping the desktop path unchanged
  • move the worker loop into a static FreeRTOS entry point and dispatch queued lambdas through a FreeRTOS queue
  • update construction/destruction paths to create and clean up the new primitives safely

Testing

  • ⚠️ idf.py build (fails: command not found in container)

https://chatgpt.com/codex/tasks/task_e_68ce683f67688324abbffe5d219332c1

@baba-dev baba-dev merged commit 24d51b5 into main Sep 20, 2025
1 of 6 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex Review: Here are some suggestions.

Reply with @codex fix comments to fix any unresolved comments.

About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".

Comment on lines 748 to +768

void SettingsController::Impl::enqueue_task(std::function<void()> task)
{
#if defined(ESP_PLATFORM)
if (!running_.load() || task_queue_ == nullptr)
{
return;
}

auto* heap_task = new (std::nothrow) std::function<void()>(std::move(task));
if (heap_task == nullptr)
{
APP_LOG_WARN(kTag, "Failed to allocate task");
return;
}

if (xQueueSend(task_queue_, &heap_task, 0) != pdPASS)
{
APP_LOG_WARN(kTag, "Task queue full");
delete heap_task;
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Avoid dropping queued settings tasks when queue fills

The new FreeRTOS worker uses a queue with a fixed length of 8 and xQueueSend(..., 0) rejects enqueues when the queue is full. When a burst of UI actions enqueues more than 8 lambdas (for example moving the brightness slider or toggling multiple options quickly), later tasks are silently discarded after logging "Task queue full". Previously the std::queue path buffered every request, so the final user action was eventually executed. With this change, a user’s last action can be lost and the device ends up in a state that doesn’t match the UI. Consider either blocking until space is available or draining/overwriting older tasks so that the most recent command is processed.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant