Skip to content

[M5] Release i2cMutex between Notehub connection polls - #45

Open
bsatrom wants to merge 1 commit into
mainfrom
fix/firmware/m5-waitconnection-mutex
Open

bsatrom wants to merge 1 commit into
mainfrom
fix/firmware/m5-waitconnection-mutex

Conversation

@bsatrom

@bsatrom bsatrom commented Jul 2, 2026

Copy link
Copy Markdown
Member

Finding M5

MainTask acquired g_i2cMutex and held it across the whole notecardWaitConnection(NOTEHUB_CONNECT_TIMEOUT_MS) call (SongbirdTasks.cpp:372-375). That call polls connection status every 1s for up to 30 seconds, so any other task needing the I2C bus (sensor reads, audio via Qwiic buzzer, command polls) was blocked for up to 30s on boot.

Fix

Inline the connection poll loop directly in MainTask:

  • Acquire the mutex, call notecardIsConnected() for a single probe, release the mutex.
  • Break on success; otherwise vTaskDelay(1000) with the bus released during the delay.
  • PVD shutdown request is still checked each iteration (preserving the pre-existing safety check).

notecardWaitConnection() remains part of the public Notecard API for other callers; MainTask no longer uses it so it can keep the bus free between probes.

Testing

Not host-testable — this is FreeRTOS mutex/timing behavior on the STM32 target with no native harness. Verification gate is a clean dual-env build.

Build

pio run — both envs pass (blues_cygnet + cygnet_debug SUCCESS).

Closes review finding M5

MainTask held g_i2cMutex across the entire notecardWaitConnection()
loop, which can block for up to NOTEHUB_CONNECT_TIMEOUT_MS (30s),
starving every other I2C bus user (sensor reads, audio, command polls)
for the full duration. Inline the poll loop in MainTask so the mutex is
acquired and released per 1s connection probe and is free during the
inter-probe delay. PVD shutdown is still checked each iteration.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant