Open
Conversation
📝 WalkthroughWalkthroughThe pull request transforms MimiClaw from a comprehensive embedded AI assistant with multiple chat channels, memory persistence, cron scheduling, skills, and tool ecosystems into a minimal "MVP" that performs a single non-streaming LLM query to DeepSeek via WiFi. This involves removing entire subsystems (message bus, agent loop, Telegram/Feishu bots, serial CLI, WebSocket server, cron/heartbeat, memory/session management, tool registry and all tools), eliminating SPIFFS filesystem support from the build, simplifying the LLM interface from multi-turn tool-calling to a single "chat_once" function, and reducing WiFi configuration to compile-time constants. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The review requires understanding the architectural shift from multi-subsystem design to MVP single-call flow, verifying all removed module dependencies are eliminated from CMake/includes, confirming WiFi/LLM/device-output paths function correctly with new config, and validating that no dangling references to deleted public APIs remain in the build. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 73-76: The release workflow currently allows tagged public
releases that may embed plaintext secrets from the firmware config (referenced
by main/mvp_config.h); add a CI guard to block/tagged releases when hardcoded
secrets are present and surface a clear warning in the release job.
Specifically, update the release workflow job (the release workflow step in
release.yml) to run a lightweight preflight that scans main/mvp_config.h for
common secret patterns (SSID, PSK, API_KEY, secret-like strings) and fail the
job with an actionable error message if any are found; additionally, modify the
workflow to require an explicit override (e.g., a dedicated
release_secret_approval boolean input or required env var) before allowing a
tagged release to proceed so maintainers must consciously opt in to releasing
binaries that may include credentials.
In `@AGENTS.md`:
- Around line 1-2: The referenced standards path in AGENTS.md is incorrect;
replace the string ”.docs/嵌入式C Doxygen 注释规范.md“ with the actual filename added
in this PR (match the repo file name exactly, e.g., .docs/嵌入式C_Doxygen_注释规范.md
or the precise added path) so readers can resolve the rule set; update the
AGENTS.md entry accordingly and verify the link opens the file in the repo
browser.
In `@docs/嵌入式C` Doxygen 注释规范.md:
- Around line 66-68: Remove the orphaned lone "c" markers that appear before
fenced code blocks so examples render correctly; locate the malformed fenced
examples that currently have a separate line with just c and delete those
standalone "c" lines, ensuring each sample only uses the triple-backtick fence
with the language specifier (```c) at the start and the matching ``` at the end
(i.e., remove the extra isolated "c" lines that precede the ```c fences).
In `@main/llm/llm_proxy.c`:
- Around line 366-372: The code currently treats a NULL http_status as an error
(the condition "if (!http_status || *http_status != 200)") causing an
unnecessary failure; instead read the status into a local int (e.g., int status
= http_status ? *http_status : -1) and then check "if (status != 200)"
unconditionally; when failing, use that local status in the snprintf call, free
the response via response_buffer_free(&response), and return ESP_FAIL so callers
passing NULL will be handled correctly.
In `@main/mimi.c`:
- Around line 104-109: The early-return branches after llm_proxy_init() (and the
similar branches at the other error checks) bypass the existing deallocation of
answer and error, leaking memory; instead of calling delay_and_idle() and
returning directly, route those error exits through the same cleanup path that
frees answer and error (e.g., jump/return via the existing cleanup label or call
the existing cleanup function) so that answer and error are freed before
delay_and_idle() is invoked; apply this change for the llm_proxy_init() branch
and the other mentioned branches so all failure paths run the common cleanup
code prior to idle/return.
In `@main/mvp_config.h`:
- Around line 9-10: The header currently hardcodes sensitive values (e.g.
WIFI_SSID, WIFI_PASSWORD, API_KEY) into source control; remove these literals
from main/mvp_config.h, replace them with references to a non-tracked config or
build-time passthrough (e.g. read from a gitignored local_config.h,
environment/build variables, or a provisioning API), update the code that uses
WIFI_SSID/WIFI_PASSWORD/API_KEY to load the values from that new source, add the
new local config to .gitignore, and ensure the exposed credentials are
rotated/invalidated before merging.
In `@main/wifi/wifi_manager.c`:
- Around line 71-79: On disconnect, in addition to setting s_connected = false,
clear the WIFI_CONNECTED_BIT from s_wifi_event_group and reset s_ip_str (e.g.,
empty string or zero it) so leftover state doesn't make
wifi_manager_wait_connected() report success; update the disconnect branch that
uses s_connected, s_retry_count, s_wifi_event_group, WIFI_CONNECTED_BIT,
WIFI_FAIL_BIT and s_ip_str accordingly.
- Around line 82-91: The event handler is blocking the default event loop by
calling vTaskDelay inside the disconnect path; change the reconnect logic in the
event handler so it does not sleep: instead enqueue/schedule the reconnect using
a FreeRTOS timer or spawn a dedicated reconnection task that reads s_retry_count
and calls esp_wifi_connect(), and remove vTaskDelay from the esp event callback
(look for uses of s_retry_count and the sequence that does ESP_LOGW + vTaskDelay
+ s_retry_count++ + esp_wifi_connect()). Also, in the disconnect handling code
clear the WIFI_CONNECTED_BIT and reset s_ip_str to an empty state so
wifi_manager_wait_connected() and wifi_manager_get_ip() cannot return stale
status after a disconnect.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5ec597b-6dbe-4e99-87a4-471762017bc6
⛔ Files ignored due to path filters (5)
assets/banner.pngis excluded by!**/*.pngassets/esp32s3-usb-port.jpgis excluded by!**/*.jpgassets/mimiclaw.pngis excluded by!**/*.pngdocs/tool-setup/images/tavily-og.pngis excluded by!**/*.pngpartitions.csvis excluded by!**/*.csv
📒 Files selected for processing (85)
.github/workflows/release.ymlAGENTS.mdCMakeLists.txtCONTRIBUTING.mdREADME.mdREADME_CN.mdREADME_JA.mddocs/ARCHITECTURE.mddocs/TODO.mddocs/WIFI_ONBOARDING_AP.mddocs/im-integration/FEISHU_SETUP.mddocs/im-integration/README.mddocs/tool-setup/README.mddocs/tool-setup/TAVILY_SETUP.mddocs/嵌入式C Doxygen 注释规范.mdmain/CMakeLists.txtmain/agent/agent_loop.cmain/agent/agent_loop.hmain/agent/context_builder.cmain/agent/context_builder.hmain/bus/message_bus.cmain/bus/message_bus.hmain/channels/feishu/README.mdmain/channels/feishu/feishu_bot.cmain/channels/feishu/feishu_bot.hmain/channels/telegram/telegram_bot.cmain/channels/telegram/telegram_bot.hmain/cli/serial_cli.cmain/cli/serial_cli.hmain/cron/cron_service.cmain/cron/cron_service.hmain/gateway/ws_server.cmain/gateway/ws_server.hmain/heartbeat/heartbeat.cmain/heartbeat/heartbeat.hmain/idf_component.ymlmain/llm/llm_proxy.cmain/llm/llm_proxy.hmain/memory/memory_store.cmain/memory/memory_store.hmain/memory/session_mgr.cmain/memory/session_mgr.hmain/mimi.cmain/mimi_config.hmain/mimi_secrets.h.examplemain/mvp_config.hmain/onboard/onboard_html.hmain/onboard/wifi_onboard.cmain/onboard/wifi_onboard.hmain/ota/ota_manager.cmain/ota/ota_manager.hmain/proxy/http_proxy.cmain/proxy/http_proxy.hmain/skills/skill_loader.cmain/skills/skill_loader.hmain/tools/gpio_policy.cmain/tools/gpio_policy.hmain/tools/tool_cron.cmain/tools/tool_cron.hmain/tools/tool_files.cmain/tools/tool_files.hmain/tools/tool_get_time.cmain/tools/tool_get_time.hmain/tools/tool_gpio.cmain/tools/tool_gpio.hmain/tools/tool_registry.cmain/tools/tool_registry.hmain/tools/tool_web_search.cmain/tools/tool_web_search.hmain/wifi/wifi_manager.cmain/wifi/wifi_manager.hscripts/build_macos.shscripts/build_ubuntu.shscripts/setup_idf_macos.shscripts/setup_idf_ubuntu.shskills/deploy/SKILL.mdskills/deploy/scripts/deploy.shskills/deploy/scripts/validate.shspiffs_data/config/SOUL.mdspiffs_data/config/USER.mdspiffs_data/memory/MEMORY.mdspiffs_data/skills/daily-briefing.mdspiffs_data/skills/gpio-control.mdspiffs_data/skills/skill-creator.mdspiffs_data/skills/weather.md
💤 Files with no reviewable changes (75)
- spiffs_data/config/USER.md
- main/idf_component.yml
- CONTRIBUTING.md
- spiffs_data/skills/skill-creator.md
- main/agent/agent_loop.h
- spiffs_data/memory/MEMORY.md
- main/cli/serial_cli.h
- docs/im-integration/README.md
- docs/TODO.md
- README.md
- main/agent/context_builder.h
- main/ota/ota_manager.h
- spiffs_data/skills/daily-briefing.md
- main/tools/tool_get_time.h
- docs/WIFI_ONBOARDING_AP.md
- CMakeLists.txt
- spiffs_data/skills/gpio-control.md
- main/channels/telegram/telegram_bot.h
- docs/tool-setup/TAVILY_SETUP.md
- spiffs_data/skills/weather.md
- spiffs_data/config/SOUL.md
- scripts/build_ubuntu.sh
- main/ota/ota_manager.c
- main/skills/skill_loader.h
- main/memory/session_mgr.c
- README_CN.md
- README_JA.md
- main/heartbeat/heartbeat.h
- main/agent/context_builder.c
- skills/deploy/SKILL.md
- main/onboard/onboard_html.h
- main/gateway/ws_server.h
- scripts/setup_idf_macos.sh
- docs/ARCHITECTURE.md
- main/channels/feishu/feishu_bot.h
- main/mimi_config.h
- main/onboard/wifi_onboard.h
- main/cli/serial_cli.c
- main/heartbeat/heartbeat.c
- main/channels/feishu/README.md
- scripts/setup_idf_ubuntu.sh
- main/tools/gpio_policy.c
- main/tools/tool_get_time.c
- main/skills/skill_loader.c
- main/tools/tool_files.h
- main/tools/tool_gpio.h
- main/onboard/wifi_onboard.c
- skills/deploy/scripts/deploy.sh
- main/tools/tool_gpio.c
- main/tools/tool_registry.c
- main/memory/memory_store.c
- main/tools/tool_cron.h
- main/memory/memory_store.h
- main/cron/cron_service.h
- main/tools/tool_registry.h
- main/tools/tool_files.c
- main/gateway/ws_server.c
- main/proxy/http_proxy.h
- main/tools/gpio_policy.h
- docs/tool-setup/README.md
- main/tools/tool_web_search.c
- main/proxy/http_proxy.c
- scripts/build_macos.sh
- main/bus/message_bus.h
- main/cron/cron_service.c
- main/channels/telegram/telegram_bot.c
- docs/im-integration/FEISHU_SETUP.md
- main/tools/tool_web_search.h
- main/memory/session_mgr.h
- main/channels/feishu/feishu_bot.c
- skills/deploy/scripts/validate.sh
- main/bus/message_bus.c
- main/tools/tool_cron.c
- main/mimi_secrets.h.example
- main/agent/agent_loop.c
Comment on lines
+73
to
+76
| ### 3. 固件行为 | ||
|
|
||
| Upload `mimiclaw-${{ github.ref_name }}.bin` via the OTA endpoint. | ||
|
|
||
| ### 4. First-time setup | ||
|
|
||
| After flashing, connect via serial (115200 baud) and configure: | ||
|
|
||
| ``` | ||
| wifi <ssid> <password> | ||
| tg_token <your-telegram-bot-token> | ||
| api_key <your-anthropic-api-key> | ||
| reboot | ||
| ``` | ||
| 这是当前的 LLM MVP 固件。上电后会直接读取代码里的 WiFi 和 DeepSeek 配置， | ||
| 自动联网、发起一次固定问题请求，并把结果打印到串口。 |
There was a problem hiding this comment.
Blocker: tagged public releases can ship embedded WiFi/API secrets without an explicit safeguard.
The firmware behavior text now confirms config is read from code, but release builds still publish binaries that may contain plaintext credentials (main/mvp_config.h:1-36). This is an immediate secret-leak risk for any public tag release.
🔒 Proposed fix (release warning + CI guard)
- name: Build firmware
shell: bash
run: |
. $IDF_PATH/export.sh
+ # Block public release if obvious real secrets are still hardcoded in MVP config.
+ grep -Eq 'MVP_LLM_API_KEY\s+"sk-[^"]+"' main/mvp_config.h && {
+ echo "::error::Hardcoded DeepSeek API key detected in main/mvp_config.h. Replace with placeholder before tagging a release."
+ exit 1
+ }
idf.py set-target esp32s3
idf.py build
@@
- ### 3. 固件行为
+ ### 3. 固件行为
这是当前的 LLM MVP 固件。上电后会直接读取代码里的 WiFi 和 DeepSeek 配置，
自动联网、发起一次固定问题请求，并把结果打印到串口。
+
+ > ⚠️ 安全提示：该 MVP 从编译期宏读取配置。请勿发布包含真实 WiFi 密码或 API Key 的固件。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 73 - 76, The release workflow
currently allows tagged public releases that may embed plaintext secrets from
the firmware config (referenced by main/mvp_config.h); add a CI guard to
block/tagged releases when hardcoded secrets are present and surface a clear
warning in the release job. Specifically, update the release workflow job (the
release workflow step in release.yml) to run a lightweight preflight that scans
main/mvp_config.h for common secret patterns (SSID, PSK, API_KEY, secret-like
strings) and fail the job with an actionable error message if any are found;
additionally, modify the workflow to require an explicit override (e.g., a
dedicated release_secret_approval boolean input or required env var) before
allowing a tagged release to proceed so maintainers must consciously opt in to
releasing binaries that may include credentials.
Comment on lines
+1
to
+2
| ## Coding Standards | ||
| - 遵守注释规范：”.docs/嵌入式C Doxygen 注释规范.md“ No newline at end of file |
There was a problem hiding this comment.
Fix the referenced standards path.
”.docs/嵌入式C Doxygen 注释规范.md“ does not resolve to the file added in this PR, so agents/readers will miss the rule set.
📌 Suggested fix
## Coding Standards
-- 遵守注释规范：”.docs/嵌入式C Doxygen 注释规范.md“
+- 遵守注释规范：`docs/嵌入式C Doxygen 注释规范.md`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 1 - 2, The referenced standards path in AGENTS.md is
incorrect; replace the string ”.docs/嵌入式C Doxygen 注释规范.md“ with the actual
filename added in this PR (match the repo file name exactly, e.g.,
.docs/嵌入式C_Doxygen_注释规范.md or the precise added path) so readers can resolve the
rule set; update the AGENTS.md entry accordingly and verify the link opens the
file in the repo browser.
Comment on lines
+66
to
+68
| c | ||
|
|
||
| ```c |
There was a problem hiding this comment.
Remove the orphaned c markers before these fenced examples.
The standalone c lines at Lines 66, 87, 114, and 135 render as plain text and make the samples look malformed. Keep only the surrounding ```c fences here.
Also applies to: 87-89, 114-116, 135-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/嵌入式C` Doxygen 注释规范.md around lines 66 - 68, Remove the orphaned lone "c"
markers that appear before fenced code blocks so examples render correctly;
locate the malformed fenced examples that currently have a separate line with
just c and delete those standalone "c" lines, ensuring each sample only uses the
triple-backtick fence with the language specifier (```c) at the start and the
matching ``` at the end (i.e., remove the extra isolated "c" lines that precede
the ```c fences).
Comment on lines
+366
to
+372
| if (!http_status || *http_status != 200) { | ||
| snprintf(error, error_size, "HTTP %d: %s", | ||
| http_status ? *http_status : -1, | ||
| response_preview); | ||
| response_buffer_free(&response); | ||
| return ESP_FAIL; | ||
| } |
There was a problem hiding this comment.
Bug: Function always fails when http_status is NULL.
If the caller passes NULL for http_status, the condition !http_status is true, causing the function to return ESP_FAIL even when the HTTP request succeeded with status 200.
Store the status locally and check it unconditionally:
🐛 Proposed fix
+ int local_status = esp_http_client_get_status_code(client);
if (http_status) {
- *http_status = esp_http_client_get_status_code(client);
+ *http_status = local_status;
}
esp_http_client_cleanup(client);
free(body);
@@ ... @@
- if (!http_status || *http_status != 200) {
+ if (local_status != 200) {
snprintf(error, error_size, "HTTP %d: %s",
- http_status ? *http_status : -1,
+ local_status,
response_preview);
response_buffer_free(&response);
return ESP_FAIL;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main/llm/llm_proxy.c` around lines 366 - 372, The code currently treats a
NULL http_status as an error (the condition "if (!http_status || *http_status !=
200)") causing an unnecessary failure; instead read the status into a local int
(e.g., int status = http_status ? *http_status : -1) and then check "if (status
!= 200)" unconditionally; when failing, use that local status in the snprintf
call, free the response via response_buffer_free(&response), and return ESP_FAIL
so callers passing NULL will be handled correctly.
Comment on lines
+104
to
+109
| err = llm_proxy_init(); | ||
| if (err != ESP_OK) { | ||
| ESP_LOGE(TAG, "LLM init failed: %s", esp_err_to_name(err)); | ||
| print_block("MVP ERROR", "LLM 初始化失败，请检查 mvp_config.h 里的 DeepSeek API Key 和 endpoint。"); | ||
| delay_and_idle(); | ||
| return; |
There was a problem hiding this comment.
Route these error exits through cleanup.
Once answer and error have been allocated, these branches bypass the free at Lines 163-164 and jump straight into delay_and_idle(). On an ESP32-S3 that leaves roughly 24 KB pinned for the rest of the process lifetime on every init/connect failure.
Also applies to: 115-127, 163-165
🧰 Tools
🪛 Cppcheck (2.20.0)
[error] 109-109: Memory leak
(memleak)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main/mimi.c` around lines 104 - 109, The early-return branches after
llm_proxy_init() (and the similar branches at the other error checks) bypass the
existing deallocation of answer and error, leaking memory; instead of calling
delay_and_idle() and returning directly, route those error exits through the
same cleanup path that frees answer and error (e.g., jump/return via the
existing cleanup label or call the existing cleanup function) so that answer and
error are freed before delay_and_idle() is invoked; apply this change for the
llm_proxy_init() branch and the other mentioned branches so all failure paths
run the common cleanup code prior to idle/return.
Comment on lines
+9
to
+10
| * @warning 该文件包含明文 WiFi 和 API Key，只适合本地开发演示 | ||
| */ |
There was a problem hiding this comment.
Remove the committed credentials before merge.
This header puts WiFi credentials and an API key directly into tracked source. Even for an MVP, once these values are pushed they should be treated as leaked and become hard to rotate per device/build. Please move them to a gitignored local config, provisioning flow, or build-time secret injection, and rotate the exposed values.
Also applies to: 13-24
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main/mvp_config.h` around lines 9 - 10, The header currently hardcodes
sensitive values (e.g. WIFI_SSID, WIFI_PASSWORD, API_KEY) into source control;
remove these literals from main/mvp_config.h, replace them with references to a
non-tracked config or build-time passthrough (e.g. read from a gitignored
local_config.h, environment/build variables, or a provisioning API), update the
code that uses WIFI_SSID/WIFI_PASSWORD/API_KEY to load the values from that new
source, add the new local config to .gitignore, and ensure the exposed
credentials are rotated/invalidated before merging.
Comment on lines
+71
to
+79
| s_connected = false; | ||
| if (disc) { | ||
| ESP_LOGW(TAG, "Disconnected (reason=%d:%s)", disc->reason, wifi_reason_to_str(disc->reason)); | ||
| } | ||
| if (s_reconnect_enabled && s_retry_count < MIMI_WIFI_MAX_RETRY) { | ||
| /* Exponential backoff: 1s, 2s, 4s, 8s, ... capped at 30s */ | ||
| uint32_t delay_ms = MIMI_WIFI_RETRY_BASE_MS << s_retry_count; | ||
| if (delay_ms > MIMI_WIFI_RETRY_MAX_MS) { | ||
| delay_ms = MIMI_WIFI_RETRY_MAX_MS; | ||
| } | ||
| ESP_LOGW(TAG, "Disconnected, retry %d/%d in %" PRIu32 "ms", | ||
| s_retry_count + 1, MIMI_WIFI_MAX_RETRY, delay_ms); | ||
| vTaskDelay(pdMS_TO_TICKS(delay_ms)); | ||
| esp_wifi_connect(); | ||
| s_retry_count++; | ||
| } else { | ||
| ESP_LOGE(TAG, "Failed to connect after %d retries", MIMI_WIFI_MAX_RETRY); | ||
|
|
||
| if (s_retry_count >= MVP_WIFI_MAX_RETRY) { | ||
| ESP_LOGE(TAG, "Failed to connect after %d retries", MVP_WIFI_MAX_RETRY); | ||
| xEventGroupSetBits(s_wifi_event_group, WIFI_FAIL_BIT); | ||
| return; |
There was a problem hiding this comment.
Clear the success state on disconnect.
This branch flips s_connected to false, but it leaves WIFI_CONNECTED_BIT and s_ip_str from the last successful join. A later wifi_manager_wait_connected() can therefore report success immediately while the station is still down or retrying.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main/wifi/wifi_manager.c` around lines 71 - 79, On disconnect, in addition to
setting s_connected = false, clear the WIFI_CONNECTED_BIT from
s_wifi_event_group and reset s_ip_str (e.g., empty string or zero it) so
leftover state doesn't make wifi_manager_wait_connected() report success; update
the disconnect branch that uses s_connected, s_retry_count, s_wifi_event_group,
WIFI_CONNECTED_BIT, WIFI_FAIL_BIT and s_ip_str accordingly.
Comment on lines
+82
to
+91
| delay_ms = MVP_WIFI_RETRY_BASE_MS << s_retry_count; | ||
| if (delay_ms > MVP_WIFI_RETRY_MAX_MS) { | ||
| delay_ms = MVP_WIFI_RETRY_MAX_MS; | ||
| } | ||
|
|
||
| /* → RETRY：按指数退避等待后再次连接，避免失败时立刻高频重试 */ | ||
| ESP_LOGW(TAG, "Retry %d/%d in %" PRIu32 " ms", s_retry_count + 1, MVP_WIFI_MAX_RETRY, delay_ms); | ||
| vTaskDelay(pdMS_TO_TICKS(delay_ms)); | ||
| s_retry_count++; | ||
| esp_wifi_connect(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n main/wifi/wifi_manager.c | head -120Repository: memovai/mimiclaw
Length of output: 4463
🏁 Script executed:
# Search for the event_handler function and its full implementation
rg -n "event_handler\|WIFI_CONNECTED_BIT\|s_ip_str\|s_connected" main/wifi/wifi_manager.c -A 2 -B 1 | head -100Repository: memovai/mimiclaw
Length of output: 42
🏁 Script executed:
# Check if there's documentation or comments about the event loop in the file
rg -n "event_loop\|event\|callback" main/wifi/wifi_manager.c | head -30Repository: memovai/mimiclaw
Length of output: 42
🏁 Script executed:
# Find where event_handler is registered
rg -n "event_handler\|esp_event_handler_register" main/wifi/wifi_manager.c -B 2 -A 2Repository: memovai/mimiclaw
Length of output: 42
🏁 Script executed:
# Find WIFI_CONNECTED_BIT definition and wifi_manager_wait_connected/get_ip functions
rg -n "WIFI_CONNECTED_BIT\|WIFI_FAIL_BIT\|wifi_manager_wait_connected\|wifi_manager_get_ip" main/wifi/ -A 3Repository: memovai/mimiclaw
Length of output: 42
🏁 Script executed:
# Find the header file to understand the full API
cat -n main/wifi/wifi_manager.hRepository: memovai/mimiclaw
Length of output: 1852
🏁 Script executed:
# Search more carefully for the implementation of wait_connected, is_connected, and get_ip
rg -n "^esp_err_t wifi_manager_wait_connected|^bool wifi_manager_is_connected|^const char \*wifi_manager_get_ip" main/wifi/wifi_manager.c -A 20Repository: memovai/mimiclaw
Length of output: 1008
🏁 Script executed:
# Find where event_handler is registered
rg -n "esp_event_handler_register\|event_handler" main/wifi/wifi_manager.c -B 1 -A 1Repository: memovai/mimiclaw
Length of output: 42
🏁 Script executed:
# Search more broadly for event handler registration in the entire file
rg -n "register\|ESP_EVENT" main/wifi/wifi_manager.cRepository: memovai/mimiclaw
Length of output: 42
🏁 Script executed:
# Look at the full wifi_manager_init function to see how the event handler is registered
sed -n '113,160p' main/wifi/wifi_manager.cRepository: memovai/mimiclaw
Length of output: 1584
🏁 Script executed:
# Verify the event loop behavior by checking ESP-IDF documentation comments in the codebase
rg -n "event.*loop\|default.*loop" main/wifi/wifi_manager.c -B 2 -A 2Repository: memovai/mimiclaw
Length of output: 42
🏁 Script executed:
# Check if there are any other event handlers or tasks that might be affected by the delay
rg -n "vTaskDelay\|event_handler" main/ -rRepository: memovai/mimiclaw
Length of output: 473
🏁 Script executed:
# Verify the state management issue more carefully - when is WIFI_CONNECTED_BIT cleared?
rg -n "WIFI_CONNECTED_BIT\|xEventGroupClearBits" main/wifi/wifi_manager.c -B 2 -A 2Repository: memovai/mimiclaw
Length of output: 42
🏁 Script executed:
# Check where WIFI_CONNECTED_BIT is cleared
rg -n "WIFI_CONNECTED_BIT\|xEventGroupClearBits" main/wifi/wifi_manager.c -B 2 -A 2Repository: memovai/mimiclaw
Length of output: 42
🏁 Script executed:
# Search for any documentation or issues related to blocking in ESP-IDF event callbacks
rg -n "note\|NOTE\|default event\|event loop" main/wifi/wifi_manager.cRepository: memovai/mimiclaw
Length of output: 42
🏁 Script executed:
# Let me read the complete wifi_manager.c file to see all the details
wc -l main/wifi/wifi_manager.cRepository: memovai/mimiclaw
Length of output: 88
🏁 Script executed:
# Read the parts we haven't seen yet
sed -n '140,165p' main/wifi/wifi_manager.cRepository: memovai/mimiclaw
Length of output: 859
Schedule WiFi reconnection asynchronously instead of blocking in the event callback.
Line 89 sleeps directly in the ESP event handler via vTaskDelay(). Event callbacks in ESP-IDF run in the context of the default event loop task, so this blocks that task for the full retry delay. During that block, other WiFi/IP events cannot be processed, stalling event subscribers and state updates.
Additionally, the WIFI_CONNECTED_BIT is never cleared when the WiFi disconnects after an initial successful connection (it's only cleared in wifi_manager_start()). This means wifi_manager_wait_connected() and wifi_manager_get_ip() may return stale state after a disconnect. Reset both the event bit and s_ip_str in the disconnect handler to keep state consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main/wifi/wifi_manager.c` around lines 82 - 91, The event handler is blocking
the default event loop by calling vTaskDelay inside the disconnect path; change
the reconnect logic in the event handler so it does not sleep: instead
enqueue/schedule the reconnect using a FreeRTOS timer or spawn a dedicated
reconnection task that reads s_retry_count and calls esp_wifi_connect(), and
remove vTaskDelay from the esp event callback (look for uses of s_retry_count
and the sequence that does ESP_LOGW + vTaskDelay + s_retry_count++ +
esp_wifi_connect()). Also, in the disconnect handling code clear the
WIFI_CONNECTED_BIT and reset s_ip_str to an empty state so
wifi_manager_wait_connected() and wifi_manager_get_ip() cannot return stale
status after a disconnect.
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
1 participant
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.
test
Summary by CodeRabbit
Release Notes