fix(integration): SystemController resolved=None 時 resume heartbeat#141
Open
csp0924 wants to merge 1 commit into
Open
fix(integration): SystemController resolved=None 時 resume heartbeat#141csp0924 wants to merge 1 commit into
csp0924 wants to merge 1 commit into
Conversation
… 避免卡 paused `_on_strategy_change` 原本在 `resolved is not None` 條件下才呼叫 pause/resume， 當前一個策略 suppress_heartbeat=True（如 BypassStrategy / CascadingStrategy）切換 到 resolved=None（remove_base_mode / deactivate_schedule_mode 後無 override）， heartbeat 會永久卡在 paused，設備端會因超時進入安全模式。 修復：resolved is None 時走 resume 分支（idempotent，無策略 assert suppress 時 安全預設為持續送心跳）。 加上兩個 regression test： - BypassStrategy → remove_base_mode 後 heartbeat.is_paused 必須為 False - 非 suppress 策略 → resolved=None 不應誤觸 pause
There was a problem hiding this comment.
Pull request overview
This PR fixes a control-lifecycle bug in SystemController where heartbeat could remain permanently paused when transitioning from a suppress_heartbeat=True strategy (e.g., BypassStrategy) to resolved=None (no effective strategy). This aligns heartbeat state with the actual effective strategy after mode removal, preventing field devices from timing out into safety mode.
Changes:
- Adjust
_on_strategy_changeso heartbeatresume()is executed whenresolved is None(no strategy asserting suppression). - Add integration regression tests covering the suppress → none transition and a non-suppress guard case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
csp_lib/integration/system_controller.py |
Moves the resolved is not None guard to only apply to the pause path, ensuring resolved=None triggers heartbeat resume. |
tests/integration/test_system_controller.py |
Adds regression tests validating heartbeat pause/resume behavior across strategy changes, including the resolved=None case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
問題
Closed-loop probe 發現：
SystemController._on_strategy_change（system_controller.py:1295）把 heartbeat pause + resume 兩個動作都包在if resolved is not None:下。從suppress_heartbeat=Truestrategy（BypassStrategy / CascadingStrategy）轉到resolved=None（如remove_base_mode把最後一個 base mode 移除、無 override）時，heartbeat 永遠卡 paused。field device 進 safety mode timeout，現場停機。Sandbox 證據（
sandbox/system_controller_lifecycle_timing_demo.pyH2）：BYPASS → set_strategy(None) →pause_calls=1, resume_calls=0, is_paused=True。改動
把
resolved is not None從外層 guard 移到內層 pause 判斷，resolved=None時 fall-through 到resume()（idempotent）。4 行改動。驗證
tests/integration/test_system_controller.py::TestHeartbeatPauseResumeOnStrategyChange2 案（核心案 + false-positive guard）。Pre-fix FAIL → post-fix PASS。state_after_None_paused: False✓