Repository navigation
Conversation
notecardGetCommand() returns one note per call, so CommandTask previously cleared a command backlog at one command per poll interval (up to 60s each in storage mode). Loop note.get until the queue is empty or COMMAND_DRAIN_MAX_PER_POLL is reached, re-acquiring the I2C mutex per command so the bus is released between notes. Adds native unit tests for the drain-loop control flow. Closes review finding M7
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
CommandTaskprocessed only a single command per poll: it acquired the I2Cmutex, called
notecardGetCommand()once, then went back to sleep. BecausenotecardGetCommand()returns exactly one note per call(
SongbirdNotecard.cpp:585-682), a backlog of queued commands drained at onecommand per poll interval — up to 60s apart in storage mode. A user who queued
several commands could wait minutes for the last one to execute.
Fix
Wrap the get-execute step in a bounded drain loop: keep calling
notecardGetCommand()untilcommand.qiis empty orCOMMAND_DRAIN_MAX_PER_POLL(8) commands have been processed this poll. TheI2C mutex is re-acquired and released per command so the bus is not held for
the whole drain, and the cap prevents a flood of commands from monopolising the
bus or starving other
CommandTaskwork.src/core/SongbirdConfig.h— newCOMMAND_DRAIN_MAX_PER_POLLconstant with rationale.src/rtos/SongbirdTasks.cpp— drain loop inCommandTask, replacing the single-shot get.Tests
Adds
test/test_command_drain/native unit tests exercising the drain-loopcontrol flow against a mock command source (get-until-empty, budget cap,
empty-queue probe, exact-budget, positive-constant). Avoids the FreeRTOS/I2C
hardware dependencies while covering the same branch logic.
Verification
pio run— both envs pass:blues_cygnetSUCCESScygnet_debugSUCCESSpio test -e native— 96 test cases succeeded (incl. newtest_command_drain).Closes review finding M7